diff --git a/lib/resourcemerge/core.go b/lib/resourcemerge/core.go index 8793f6c89..da4c50f7c 100644 --- a/lib/resourcemerge/core.go +++ b/lib/resourcemerge/core.go @@ -18,7 +18,7 @@ func EnsureConfigMap(modified *bool, existing *corev1.ConfigMap, required corev1 mergeMap(modified, &existing.Data, required.Data) } -// EnsureServiceAccount ensures that the existing mathces the required. +// EnsureServiceAccount ensures that the existing matches the required. // modified is set to true when existing had to be updated with required. func EnsureServiceAccount(modified *bool, existing *corev1.ServiceAccount, required corev1.ServiceAccount) { EnsureObjectMeta(modified, &existing.ObjectMeta, required.ObjectMeta) @@ -466,12 +466,13 @@ func ensureVolumeSourceDefaults(required *corev1.VolumeSource) { } func ensureSecurityContextPtr(modified *bool, existing **corev1.SecurityContext, required *corev1.SecurityContext) { - // if we have no required, then we don't care what someone else has set - if required == nil { + if *existing == nil && required == nil { return } - if *existing == nil { + // Check if we can simply set to required. This can be done if existing is not set or it is set + // but required is not set. + if *existing == nil || (required == nil && *existing != nil) { *modified = true *existing = required return @@ -490,12 +491,13 @@ func ensureSecurityContext(modified *bool, existing *corev1.SecurityContext, req } func ensureCapabilitiesPtr(modified *bool, existing **corev1.Capabilities, required *corev1.Capabilities) { - // if we have no required, then we don't care what someone else has set - if required == nil { + if *existing == nil && required == nil { return } - if *existing == nil { + // Check if we can simply set to required. This can be done if existing is not set or it is set + // but required is not set. + if *existing == nil || (required == nil && *existing != nil) { *modified = true *existing = required return @@ -619,12 +621,13 @@ func ensureAffinity(modified *bool, existing *corev1.Affinity, required corev1.A } func ensurePodSecurityContextPtr(modified *bool, existing **corev1.PodSecurityContext, required *corev1.PodSecurityContext) { - // if we have no required, then we don't care what someone else has set - if required == nil { + if *existing == nil && required == nil { return } - if *existing == nil { + // Check if we can simply set to required. This can be done if existing is not set or it is set + // but required is not set. + if *existing == nil || (required == nil && *existing != nil) { *modified = true *existing = required return @@ -676,12 +679,13 @@ func ensurePodSecurityContext(modified *bool, existing *corev1.PodSecurityContex } func ensureSELinuxOptionsPtr(modified *bool, existing **corev1.SELinuxOptions, required *corev1.SELinuxOptions) { - // if we have no required, then we don't care what someone else has set - if required == nil { + if *existing == nil && required == nil { return } - if *existing == nil { + // Check if we can simply set to required. This can be done if existing is not set or it is set + // but required is not set. + if *existing == nil || (required == nil && *existing != nil) { *modified = true *existing = required return @@ -734,12 +738,13 @@ func setBool(modified *bool, existing *bool, required bool) { } func setBoolPtr(modified *bool, existing **bool, required *bool) { - // if we have no required, then we don't care what someone else has set - if required == nil { + if *existing == nil && required == nil { return } - if *existing == nil { + // Check if we can simply set to required. This can be done if existing is not set or it is set + // but required is not set. + if *existing == nil || (required == nil && *existing != nil) { *modified = true *existing = required return @@ -758,6 +763,9 @@ func setInt32Ptr(modified *bool, existing **int32, required *int32) { if *existing == nil && required == nil { return } + + // Check if we can simply set to required. This can be done if existing is not set or it is set + // but required is not set. if *existing == nil || (required == nil && *existing != nil) { *modified = true *existing = required @@ -774,12 +782,13 @@ func setInt64(modified *bool, existing *int64, required int64) { } func setInt64Ptr(modified *bool, existing **int64, required *int64) { - // if we have no required, then we don't care what someone else has set - if required == nil { + if *existing == nil && required == nil { return } - if *existing == nil { + // Check if we can simply set to required. This can be done if existing is not set or it is set + // but required is not set. + if *existing == nil || (required == nil && *existing != nil) { *modified = true *existing = required return diff --git a/lib/resourcemerge/core_test.go b/lib/resourcemerge/core_test.go index b7686c3ad..82382c706 100644 --- a/lib/resourcemerge/core_test.go +++ b/lib/resourcemerge/core_test.go @@ -30,11 +30,12 @@ func TestEnsurePodSpec(t *testing.T) { expected: corev1.PodSpec{}, }, { - name: "remove regular containers from existing", + name: "remove regular containers from existing, PodSecurityContext none", existing: corev1.PodSpec{ Containers: []corev1.Container{ {Name: "test"}, - {Name: "to-be-removed"}}}, + {Name: "to-be-removed"}}, + SecurityContext: &corev1.PodSecurityContext{RunAsNonRoot: boolPtr(false)}}, input: corev1.PodSpec{ Containers: []corev1.Container{ {Name: "test"}}}, @@ -44,6 +45,71 @@ func TestEnsurePodSpec(t *testing.T) { Containers: []corev1.Container{ {Name: "test"}}}, }, + { + name: "PodSecurityContext empty", + existing: corev1.PodSpec{ + SecurityContext: &corev1.PodSecurityContext{RunAsNonRoot: boolPtr(false), + RunAsUser: int64Ptr(int64(1234)), + RunAsGroup: int64Ptr(int64(1234)), + FSGroup: int64Ptr(int64(1234)), + SELinuxOptions: &corev1.SELinuxOptions{User: "foo"}, + }}, + input: corev1.PodSpec{ + SecurityContext: &corev1.PodSecurityContext{}}, + + expectedModified: true, + expected: corev1.PodSpec{ + SecurityContext: &corev1.PodSecurityContext{}}, + }, + { + name: "PodSecurityContext changes", + existing: corev1.PodSpec{ + SecurityContext: &corev1.PodSecurityContext{RunAsNonRoot: boolPtr(true), + RunAsUser: int64Ptr(int64(1234)), + RunAsGroup: int64Ptr(int64(1234))}}, + input: corev1.PodSpec{ + SecurityContext: &corev1.PodSecurityContext{RunAsNonRoot: boolPtr(false), + RunAsGroup: int64Ptr(int64(5))}}, + + expectedModified: true, + expected: corev1.PodSpec{ + SecurityContext: &corev1.PodSecurityContext{RunAsNonRoot: boolPtr(false), + RunAsGroup: int64Ptr(int64(5))}}, + }, + { + name: "container SecurityContext none", + existing: corev1.PodSpec{ + Containers: []corev1.Container{ + {SecurityContext: &corev1.SecurityContext{RunAsNonRoot: boolPtr(false), + RunAsUser: int64Ptr(int64(1234)), + Capabilities: &corev1.Capabilities{ + Add: []corev1.Capability{"bar"}}, + SELinuxOptions: &corev1.SELinuxOptions{User: "foo"}, + }}}}, + input: corev1.PodSpec{}, + + expectedModified: true, + expected: corev1.PodSpec{}, + }, + { + name: "container SecurityContext empty", + existing: corev1.PodSpec{ + Containers: []corev1.Container{ + {SecurityContext: &corev1.SecurityContext{RunAsNonRoot: boolPtr(false), + RunAsUser: int64Ptr(int64(1234)), + Capabilities: &corev1.Capabilities{ + Add: []corev1.Capability{"bar"}}, + SELinuxOptions: &corev1.SELinuxOptions{User: "foo"}, + }}}}, + input: corev1.PodSpec{ + Containers: []corev1.Container{ + {SecurityContext: &corev1.SecurityContext{}}}}, + + expectedModified: true, + expected: corev1.PodSpec{ + Containers: []corev1.Container{ + {SecurityContext: &corev1.SecurityContext{}}}}, + }, { name: "remove regular and init containers from existing", existing: corev1.PodSpec{ @@ -1559,3 +1625,11 @@ func defaultPodSpec(in *corev1.PodSpec, from corev1.PodSpec) { modified := pointer.BoolPtr(false) ensurePodSpec(modified, in, from) } + +func boolPtr(b bool) *bool { + return &b +} + +func int64Ptr(i int64) *int64 { + return &i +} diff --git a/lib/resourcemerge/meta_test.go b/lib/resourcemerge/meta_test.go index 59cef15b2..ee88c8e2e 100644 --- a/lib/resourcemerge/meta_test.go +++ b/lib/resourcemerge/meta_test.go @@ -14,6 +14,8 @@ import ( func init() { klog.InitFlags(flag.CommandLine) + _ = flag.CommandLine.Lookup("v").Value.Set("2") + _ = flag.CommandLine.Lookup("alsologtostderr").Value.Set("true") } func TestMergeOwnerRefs(t *testing.T) {