Skip to content

OCPBUGS-5011: Backport SecurityContext reconciliation behavior #940

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
openshift-merge-robot merged 2 commits into from
Jul 21, 2023
Merged

OCPBUGS-5011: Backport SecurityContext reconciliation behavior #940

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
617e84d
lib/resourcemerge: change SecurityContext reconcile
jottofar Jul 25, 2022
c1c2109
lib/resourcemerge/core: Reconcile seccompProfile in ensurePodSecurity…
wking Aug 31, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 54 additions & 19 deletions lib/resourcemerge/core.go
View file
Open in desktop
Original file line number Diff line number Diff line change
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
Open in desktop
Original file line number Diff line number Diff line change
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
Open in desktop
Original file line number Diff line number Diff line change
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