Skip to content

Commit

Permalink
Merge pull request #940 from petr-muller/ocpbugs-5011-backport-ocpbug…
Browse files Browse the repository at this point in the history 
…s-575

OCPBUGS-5011: Backport `SecurityContext` reconciliation behavior
  • Loading branch information
@openshift-merge-robot
openshift-merge-robot committed Jul 21, 2023
2 parents 1cb2fa2 + c1c2109 commit 8966b29
Show file tree
Hide file tree
Showing 3 changed files with 134 additions and 21 deletions.
73 changes: 54 additions & 19 deletions lib/resourcemerge/core.go
View file
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -487,15 +488,17 @@ func ensureSecurityContext(modified *bool, existing *corev1.SecurityContext, req
setBoolPtr(modified, &existing.RunAsNonRoot, required.RunAsNonRoot)
setBoolPtr(modified, &existing.ReadOnlyRootFilesystem, required.ReadOnlyRootFilesystem)
setBoolPtr(modified, &existing.AllowPrivilegeEscalation, required.AllowPrivilegeEscalation)
ensureSeccompProfilePtr(modified, &existing.SeccompProfile, required.SeccompProfile)
}

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
Expand Down Expand Up @@ -535,6 +538,30 @@ func ensureCapabilities(modified *bool, existing *corev1.Capabilities, required
}
}

func ensureSeccompProfilePtr(modified *bool, existing **corev1.SeccompProfile, required *corev1.SeccompProfile) {
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
return
}
ensureSeccompProfile(modified, *existing, *required)
}

func ensureSeccompProfile(modified *bool, existing *corev1.SeccompProfile, required corev1.SeccompProfile) {
if equality.Semantic.DeepEqual(existing, required) {
return
}

*modified = true
*existing = required
}

func setStringSlice(modified *bool, existing *[]string, required []string) {
if !reflect.DeepEqual(required, *existing) {
*existing = required
Expand Down Expand Up @@ -619,12 +646,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
Expand All @@ -637,6 +665,7 @@ func ensurePodSecurityContext(modified *bool, existing *corev1.PodSecurityContex
setInt64Ptr(modified, &existing.RunAsUser, required.RunAsUser)
setInt64Ptr(modified, &existing.RunAsGroup, required.RunAsGroup)
setBoolPtr(modified, &existing.RunAsNonRoot, required.RunAsNonRoot)
ensureSeccompProfilePtr(modified, &existing.SeccompProfile, required.SeccompProfile)

// any SupplementalGroups we specify, we require.
for _, required := range required.SupplementalGroups {
Expand Down Expand Up @@ -676,12 +705,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
Expand Down Expand Up @@ -734,12 +764,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
Expand All @@ -758,6 +789,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
Expand All @@ -774,12 +808,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
Expand Down
80 changes: 78 additions & 2 deletions lib/resourcemerge/core_test.go
View file
Expand Up @@ -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"}}},
Expand All @@ -44,6 +45,73 @@ 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)),
SeccompProfile: &corev1.SeccompProfile{Type: corev1.SeccompProfileTypeRuntimeDefault}}},

expectedModified: true,
expected: corev1.PodSpec{
SecurityContext: &corev1.PodSecurityContext{RunAsNonRoot: boolPtr(false),
RunAsGroup: int64Ptr(int64(5)),
SeccompProfile: &corev1.SeccompProfile{Type: corev1.SeccompProfileTypeRuntimeDefault}}},
},
{
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{
Expand Down Expand Up @@ -1559,3 +1627,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
}
2 changes: 2 additions & 0 deletions lib/resourcemerge/meta_test.go
View file
Expand Up @@ -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) {
Expand Down

0 comments on commit 8966b29

Please sign in to comment.