Skip to content
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.

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

LOG-3302: Update scc to desired #1806

Merged
merged 1 commit into from Jan 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
34 changes: 32 additions & 2 deletions internal/reconcile/scc.go
Expand Up @@ -14,7 +14,7 @@ import (
func SecurityContextConstraints(k8Client client.Client, desired *security.SecurityContextConstraints) error {
retryErr := retry.RetryOnConflict(retry.DefaultRetry, func() error {
current := &security.SecurityContextConstraints{}
key := client.ObjectKeyFromObject(desired)
key := client.ObjectKey{Name: desired.Name}
if err := k8Client.Get(context.TODO(), key, current); err != nil {
if errors.IsNotFound(err) {
return k8Client.Create(context.TODO(), desired)
Expand All @@ -27,7 +27,37 @@ func SecurityContextConstraints(k8Client client.Client, desired *security.Securi
log.V(3).Info("SecurityContextConstraints are the same skipping update")
return nil
}
return k8Client.Update(context.TODO(), current)
return k8Client.Update(context.TODO(), update(desired, current))
})
return retryErr
}

func update(from, to *security.SecurityContextConstraints) *security.SecurityContextConstraints {
to.Labels = from.Labels
to.Priority = from.Priority
to.AllowPrivilegedContainer = from.AllowPrivilegedContainer
to.DefaultAddCapabilities = from.DefaultAddCapabilities
to.RequiredDropCapabilities = from.RequiredDropCapabilities
to.AllowedCapabilities = from.AllowedCapabilities
to.AllowHostDirVolumePlugin = from.AllowHostDirVolumePlugin
to.Volumes = from.Volumes
to.AllowedFlexVolumes = from.AllowedFlexVolumes
to.AllowHostNetwork = from.AllowHostNetwork
to.AllowHostPorts = from.AllowHostPorts
to.AllowHostPID = from.AllowHostPID
to.AllowHostIPC = from.AllowHostIPC
to.DefaultAllowPrivilegeEscalation = from.DefaultAllowPrivilegeEscalation
to.AllowPrivilegeEscalation = from.AllowPrivilegeEscalation
to.SELinuxContext = from.SELinuxContext
to.RunAsUser = from.RunAsUser
to.SupplementalGroups = from.SupplementalGroups
to.FSGroup = from.FSGroup
to.ReadOnlyRootFilesystem = from.ReadOnlyRootFilesystem
to.Users = from.Users
to.Groups = from.Groups
to.SeccompProfiles = from.SeccompProfiles
to.AllowedUnsafeSysctls = from.AllowedUnsafeSysctls
to.ForbiddenSysctls = from.ForbiddenSysctls

Comment on lines +36 to +61
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we replace this block with *to=*from if all fields are to be copied?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SCCs don't have a "spec" field, they live off the root of the type. The only change we possibly could make is to use reflection over the fields

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agree. a *to=*from would copy more fields than we need.

return to
}
61 changes: 61 additions & 0 deletions internal/reconcile/scc_test.go
@@ -0,0 +1,61 @@
package reconcile_test

import (
"context"
security "github.com/openshift/api/security/v1"
"github.com/openshift/cluster-logging-operator/internal/reconcile"
"github.com/openshift/cluster-logging-operator/internal/runtime"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/kubernetes/scheme"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/fake"

"github.com/google/go-cmp/cmp"

. "github.com/onsi/ginkgo"
. "github.com/onsi/ginkgo/extensions/table"
. "github.com/onsi/gomega"
)

var _ = Describe("reconciling ", func() {

var (
anSCC = &security.SecurityContextConstraints{
TypeMeta: metav1.TypeMeta{
Kind: "SecurityContextConstraints",
APIVersion: security.SchemeGroupVersion.String(),
},
ObjectMeta: metav1.ObjectMeta{
Name: "bar",
},
ReadOnlyRootFilesystem: true,
}
)

var _ = DescribeTable("SCC", func(initial *security.SecurityContextConstraints, desired security.SecurityContextConstraints) {

globalScheme := scheme.Scheme
Expect(security.Install(globalScheme)).To(Succeed())
builder := fake.NewClientBuilder().WithScheme(globalScheme)
if initial != nil {
builder.WithRuntimeObjects(initial)
}
k8sClient := builder.Build()

Expect(reconcile.SecurityContextConstraints(k8sClient, &desired)).To(Succeed(), "Expect no error reconciling secrets")

key := client.ObjectKey{Name: desired.Name}
act := &security.SecurityContextConstraints{}
Expect(k8sClient.Get(context.TODO(), key, act)).To(Succeed(), "Exp. no error after reconciliation to try and verify")

act.ResourceVersion = "" //dont care here
desired.ResourceVersion = ""

Expect(cmp.Diff(act, &desired)).To(BeEmpty(), "Exp. the spec to be the same")
Expect(cmp.Diff(act, initial)).To(Not(BeEmpty()), "Exp. the spec to have been updated")
},
Entry("when it does not exist should create it", nil, *anSCC),
Entry("when spec is modified it should revert it", runtime.NewSCC("bar"), *anSCC),
Entry("when spec is not modified it should do nothing", anSCC, *anSCC),
)
})