diff --git a/internal/controller/overrides.go b/internal/controller/overrides.go index cb3d325c..6244fdd0 100644 --- a/internal/controller/overrides.go +++ b/internal/controller/overrides.go @@ -138,8 +138,25 @@ func mergeAffinity(base, patch *corev1.Affinity) *corev1.Affinity { // t is treated as read-only; it is only marshaled, never written to. // VolumeMounts are handled separately to allow multiple mounts at the same path; handled by ProjectVolumes. func ApplyContainerTemplateOverrides(container *corev1.Container, t *v1.ContainerTemplateSpec) (corev1.Container, error) { + base := container + + // Strategic merge patch deep-merges corev1.Capabilities field-by-field; the schema + // has no patchStrategy=replace directive on Capabilities. A user setting only + // Drop:[ALL] would otherwise leave the operator-defaulted Add list in place, + // producing a spec rejected by `restricted` PodSecurity. Treat user-provided + // Capabilities as authoritative for the whole sub-struct by clearing it on the + // base before SMP. Other SecurityContext fields still deep-merge as expected. + if t.SecurityContext != nil && t.SecurityContext.Capabilities != nil && + base.SecurityContext != nil && base.SecurityContext.Capabilities != nil { + baseCopy := *base + scCopy := *base.SecurityContext + scCopy.Capabilities = nil + baseCopy.SecurityContext = &scCopy + base = &baseCopy + } + patchContainer := corev1.Container{ - Name: container.Name, + Name: base.Name, Image: t.Image.String(), ImagePullPolicy: t.ImagePullPolicy, Resources: t.Resources, @@ -150,7 +167,7 @@ func ApplyContainerTemplateOverrides(container *corev1.Container, t *v1.Containe // ReadinessProbe is handled manually } - mergedContainer, err := patchResource(container, patchContainer, containerSchema) + mergedContainer, err := patchResource(base, patchContainer, containerSchema) if err != nil { return corev1.Container{}, fmt.Errorf("patch container: %w", err) } diff --git a/internal/controller/overrides_test.go b/internal/controller/overrides_test.go index a9e19fcb..23625e5c 100644 --- a/internal/controller/overrides_test.go +++ b/internal/controller/overrides_test.go @@ -101,6 +101,116 @@ var _ = Describe("ApplyContainerTemplateOverrides", func() { Expect(container.SecurityContext.RunAsUser).NotTo(BeNil()) Expect(*container.SecurityContext.RunAsUser).To(Equal(int64(1000)), "user RunAsUser should be applied") }) + + Describe("Capabilities", func() { + // Strategic merge patch deep-merges corev1.Capabilities field-by-field, + // so a user setting only Drop:[ALL] would otherwise leave the operator's + // defaulted Add list in place — producing a spec rejected by `restricted` + // PodSecurity. The merge must treat user-provided Capabilities as + // authoritative for the whole sub-struct. + // See https://github.com/ClickHouse/clickhouse-operator/issues/174. + It("should let user Drop:[ALL] override operator-defaulted Add", func() { + container, err := ApplyContainerTemplateOverrides( + &corev1.Container{ + Name: "server", + SecurityContext: &corev1.SecurityContext{ + Capabilities: &corev1.Capabilities{ + Add: []corev1.Capability{"IPC_LOCK", "PERFMON", "SYS_PTRACE"}, + }, + }, + }, + &v1.ContainerTemplateSpec{ + SecurityContext: &corev1.SecurityContext{ + Capabilities: &corev1.Capabilities{ + Drop: []corev1.Capability{"ALL"}, + }, + }, + }, + ) + + Expect(err).ToNot(HaveOccurred()) + Expect(container.SecurityContext).NotTo(BeNil()) + Expect(container.SecurityContext.Capabilities).NotTo(BeNil()) + Expect(container.SecurityContext.Capabilities.Drop).To(Equal([]corev1.Capability{"ALL"})) + Expect(container.SecurityContext.Capabilities.Add).To(BeEmpty(), + "operator-defaulted Add must not survive when the user sets Drop:[ALL]") + }) + + It("should let user-set Add replace operator-defaulted Add", func() { + container, err := ApplyContainerTemplateOverrides( + &corev1.Container{ + Name: "server", + SecurityContext: &corev1.SecurityContext{ + Capabilities: &corev1.Capabilities{ + Add: []corev1.Capability{"IPC_LOCK", "PERFMON", "SYS_PTRACE"}, + }, + }, + }, + &v1.ContainerTemplateSpec{ + SecurityContext: &corev1.SecurityContext{ + Capabilities: &corev1.Capabilities{ + Add: []corev1.Capability{"NET_BIND_SERVICE"}, + }, + }, + }, + ) + + Expect(err).ToNot(HaveOccurred()) + Expect(container.SecurityContext.Capabilities).NotTo(BeNil()) + Expect(container.SecurityContext.Capabilities.Add).To(Equal([]corev1.Capability{"NET_BIND_SERVICE"}), + "user Add list should replace, not merge with, operator defaults") + }) + + It("should preserve operator capabilities when user SecurityContext omits Capabilities", func() { + container, err := ApplyContainerTemplateOverrides( + &corev1.Container{ + Name: "server", + SecurityContext: &corev1.SecurityContext{ + Capabilities: &corev1.Capabilities{ + Add: []corev1.Capability{"IPC_LOCK", "PERFMON", "SYS_PTRACE"}, + }, + }, + }, + &v1.ContainerTemplateSpec{ + SecurityContext: &corev1.SecurityContext{ + RunAsNonRoot: new(true), + }, + }, + ) + + Expect(err).ToNot(HaveOccurred()) + Expect(container.SecurityContext.Capabilities).NotTo(BeNil()) + Expect(container.SecurityContext.Capabilities.Add).To(Equal( + []corev1.Capability{"IPC_LOCK", "PERFMON", "SYS_PTRACE"}), + "operator capabilities must survive when the user does not set capabilities") + Expect(container.SecurityContext.RunAsNonRoot).NotTo(BeNil()) + Expect(*container.SecurityContext.RunAsNonRoot).To(BeTrue()) + }) + + It("should not mutate the caller's container or its SecurityContext", func() { + baseCaps := &corev1.Capabilities{ + Add: []corev1.Capability{"IPC_LOCK", "PERFMON", "SYS_PTRACE"}, + } + baseSC := &corev1.SecurityContext{Capabilities: baseCaps} + base := &corev1.Container{Name: "server", SecurityContext: baseSC} + + _, err := ApplyContainerTemplateOverrides( + base, + &v1.ContainerTemplateSpec{ + SecurityContext: &corev1.SecurityContext{ + Capabilities: &corev1.Capabilities{ + Drop: []corev1.Capability{"ALL"}, + }, + }, + }, + ) + + Expect(err).ToNot(HaveOccurred()) + Expect(base.SecurityContext).To(BeIdenticalTo(baseSC), "caller's SecurityContext pointer should be untouched") + Expect(baseSC.Capabilities).To(BeIdenticalTo(baseCaps), "caller's Capabilities pointer should be untouched") + Expect(baseCaps.Add).To(Equal([]corev1.Capability{"IPC_LOCK", "PERFMON", "SYS_PTRACE"})) + }) + }) }) Describe("Probes", func() {