From a7d2f6abcfd129e13c76089d7bc956937e390d6d Mon Sep 17 00:00:00 2001 From: mprahl Date: Mon, 18 Sep 2023 16:12:54 -0400 Subject: [PATCH] Use random UID and GID when running on OpenShift When running on OpenShift, allow OpenShift to assign a random UID and GID for the Gatekeeper containers. When it's not OpenShift, fallback to running as a non-privileged user and group. Additionally, for backwards compatibility with OpenShift 4.10, seccomp profile is left unset. See the following for this recommendation: https://connect.redhat.com/en/blog/important-openshift-changes-pod-security-standards Signed-off-by: mprahl --- config/gatekeeper/kustomization.yaml | 21 ------- .../apps_v1_deployment_gatekeeper-audit.yaml | 10 ---- ...loyment_gatekeeper-controller-manager.yaml | 10 ---- controllers/gatekeeper_controller.go | 40 +++++++++++++ controllers/gatekeeper_controller_test.go | 58 +++++++++++++++++++ pkg/bindata/bindata.go | 12 ---- 6 files changed, 98 insertions(+), 53 deletions(-) delete mode 100644 config/gatekeeper/patches/apps_v1_deployment_gatekeeper-audit.yaml delete mode 100644 config/gatekeeper/patches/apps_v1_deployment_gatekeeper-controller-manager.yaml diff --git a/config/gatekeeper/kustomization.yaml b/config/gatekeeper/kustomization.yaml index c46e41487..37c393ad8 100644 --- a/config/gatekeeper/kustomization.yaml +++ b/config/gatekeeper/kustomization.yaml @@ -25,30 +25,9 @@ resources: - v1_secret_gatekeeper-webhook-server-cert.yaml - v1_serviceaccount_gatekeeper-admin.yaml - v1_service_gatekeeper-webhook-service.yaml -# Add annotations for backwards compatibility -# Add OpenShift specific RBAC # Remove --disable-cert-rotation # Set a CPU limit patches: -- path: patches/apps_v1_deployment_gatekeeper-audit.yaml -- path: patches/apps_v1_deployment_gatekeeper-controller-manager.yaml -- patch: |- - - op: add - path: /rules/- - value: - apiGroups: - - security.openshift.io - resourceNames: - - anyuid - resources: - - securitycontextconstraints - verbs: - - use - target: - group: rbac.authorization.k8s.io - kind: Role - name: gatekeeper-manager-role - version: v1 - patch: |- - op: remove path: /spec/template/spec/containers/0/args/5 diff --git a/config/gatekeeper/patches/apps_v1_deployment_gatekeeper-audit.yaml b/config/gatekeeper/patches/apps_v1_deployment_gatekeeper-audit.yaml deleted file mode 100644 index a2f779368..000000000 --- a/config/gatekeeper/patches/apps_v1_deployment_gatekeeper-audit.yaml +++ /dev/null @@ -1,10 +0,0 @@ -apiVersion: apps/v1 -kind: Deployment -metadata: - name: gatekeeper-audit - namespace: gatekeeper-system -spec: - template: - metadata: - annotations: - container.seccomp.security.alpha.kubernetes.io/manager: runtime/default diff --git a/config/gatekeeper/patches/apps_v1_deployment_gatekeeper-controller-manager.yaml b/config/gatekeeper/patches/apps_v1_deployment_gatekeeper-controller-manager.yaml deleted file mode 100644 index 5393e71d1..000000000 --- a/config/gatekeeper/patches/apps_v1_deployment_gatekeeper-controller-manager.yaml +++ /dev/null @@ -1,10 +0,0 @@ -apiVersion: apps/v1 -kind: Deployment -metadata: - name: gatekeeper-controller-manager - namespace: gatekeeper-system -spec: - template: - metadata: - annotations: - container.seccomp.security.alpha.kubernetes.io/manager: runtime/default diff --git a/controllers/gatekeeper_controller.go b/controllers/gatekeeper_controller.go index f1d54c79c..76633e2d9 100644 --- a/controllers/gatekeeper_controller.go +++ b/controllers/gatekeeper_controller.go @@ -499,6 +499,10 @@ func crOverrides(gatekeeper *operatorv1alpha1.Gatekeeper, asset string, obj *uns if err := removeAnnotations(obj); err != nil { return err } + + if err := openShiftDeploymentOverrides(obj); err != nil { + return err + } } // webhook overrides case WebhookFile: @@ -512,6 +516,10 @@ func crOverrides(gatekeeper *operatorv1alpha1.Gatekeeper, asset string, obj *uns if err := removeAnnotations(obj); err != nil { return err } + + if err := openShiftDeploymentOverrides(obj); err != nil { + return err + } } // ValidatingWebhookConfiguration overrides case ValidatingWebhookConfiguration: @@ -742,6 +750,38 @@ func removeAnnotations(obj *unstructured.Unstructured) error { return nil } +// openShiftDeploymentOverrides will remove runAsUser, runAsGroup, and seccompProfile on every container in the +// Deployment manifest. The seccompProfile is removed for backwards compatibility with OpenShift <= v4.10. Setting +// seccompProfile=runtime/default in such versions explicitly disqualified the workload from the restricted SCC. +// In OpenShift v4.11+, any workload running in a namespace prefixed with "openshift-*" must use the "restricted" +// profile unless there is a ClusterServiceVersion present, which is not the case for the Gatekeeper operand namespace. +func openShiftDeploymentOverrides(obj *unstructured.Unstructured) error { + containers, _, err := unstructured.NestedSlice(obj.Object, "spec", "template", "spec", "containers") + if err != nil { + return errors.Wrapf(err, "Failed to parse the deployment's containers") + } + + for i := range containers { + container, ok := containers[i].(map[string]interface{}) + if !ok { + continue + } + + unstructured.RemoveNestedField(container, "securityContext", "runAsUser") + unstructured.RemoveNestedField(container, "securityContext", "runAsGroup") + unstructured.RemoveNestedField(container, "securityContext", "seccompProfile") + + containers[i] = container + } + + err = unstructured.SetNestedField(obj.Object, containers, "spec", "template", "spec", "containers") + if err != nil { + return errors.Wrapf(err, "Failed to set the OpenShift overrides") + } + + return nil +} + func setLogLevel(obj *unstructured.Unstructured, logLevel *operatorv1alpha1.LogLevelMode) error { if logLevel != nil { return setContainerArg(obj, managerContainer, LogLevelArg, string(*logLevel), false) diff --git a/controllers/gatekeeper_controller_test.go b/controllers/gatekeeper_controller_test.go index 4eb216a78..5a6028958 100644 --- a/controllers/gatekeeper_controller_test.go +++ b/controllers/gatekeeper_controller_test.go @@ -303,6 +303,64 @@ func assertAffinity(g *WithT, expected *corev1.Affinity, current interface{}) { g.Expect(util.ToMap(expected)).To(BeEquivalentTo(util.ToMap(current))) } +func TestOpenShiftOverrides(t *testing.T) { + g := NewWithT(t) + gatekeeper := &operatorv1alpha1.Gatekeeper{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + }, + } + + auditObj, err := util.GetManifestObject(AuditFile) + g.Expect(err).ToNot(HaveOccurred()) + + webhookObj, err := util.GetManifestObject(WebhookFile) + g.Expect(err).ToNot(HaveOccurred()) + + // Test that no OpenShift overrides take place when it's not OpenShift + err = crOverrides(gatekeeper, AuditFile, auditObj, namespace, false, false) + g.Expect(err).ToNot(HaveOccurred()) + assertOverrides(g, auditObj, true) + + err = crOverrides(gatekeeper, WebhookFile, webhookObj, namespace, false, false) + g.Expect(err).ToNot(HaveOccurred()) + assertOverrides(g, webhookObj, true) + + // Test that OpenShift overrides take place + err = crOverrides(gatekeeper, AuditFile, auditObj, namespace, true, false) + g.Expect(err).ToNot(HaveOccurred()) + assertOverrides(g, auditObj, false) + + err = crOverrides(gatekeeper, WebhookFile, webhookObj, namespace, true, false) + g.Expect(err).ToNot(HaveOccurred()) + assertOverrides(g, webhookObj, false) +} + +func assertOverrides(g *WithT, current *unstructured.Unstructured, isSet bool) { + containers, _, err := unstructured.NestedSlice(current.Object, "spec", "template", "spec", "containers") + g.ExpectWithOffset(1, err).ToNot(HaveOccurred()) + g.ExpectWithOffset(1, containers).ToNot(BeEmpty()) + + for i := range containers { + container, ok := containers[i].(map[string]interface{}) + if !ok { + continue + } + + _, runAsUserFound, err := unstructured.NestedInt64(container, "securityContext", "runAsUser") + g.ExpectWithOffset(1, err).ToNot(HaveOccurred()) + g.ExpectWithOffset(1, runAsUserFound).To(Equal(isSet)) + + _, runAsGroupFound, err := unstructured.NestedInt64(container, "securityContext", "runAsGroup") + g.ExpectWithOffset(1, err).ToNot(HaveOccurred()) + g.ExpectWithOffset(1, runAsGroupFound).To(Equal(isSet)) + + _, seccompProfileFound, err := unstructured.NestedMap(container, "securityContext", "seccompProfile") + g.ExpectWithOffset(1, err).ToNot(HaveOccurred()) + g.ExpectWithOffset(1, seccompProfileFound).To(Equal(isSet)) + } +} + func TestNodeSelector(t *testing.T) { g := NewWithT(t) nodeSelector := map[string]string{ diff --git a/pkg/bindata/bindata.go b/pkg/bindata/bindata.go index 1de9b841c..4a26d3013 100644 --- a/pkg/bindata/bindata.go +++ b/pkg/bindata/bindata.go @@ -4219,8 +4219,6 @@ spec: gatekeeper.sh/system: "yes" template: metadata: - annotations: - container.seccomp.security.alpha.kubernetes.io/manager: runtime/default labels: control-plane: audit-controller gatekeeper.sh/operation: audit @@ -4342,8 +4340,6 @@ spec: gatekeeper.sh/system: "yes" template: metadata: - annotations: - container.seccomp.security.alpha.kubernetes.io/manager: runtime/default labels: control-plane: controller-manager gatekeeper.sh/operation: webhook @@ -4725,14 +4721,6 @@ rules: - patch - update - watch -- apiGroups: - - security.openshift.io - resourceNames: - - anyuid - resources: - - securitycontextconstraints - verbs: - - use `) func configGatekeeperRenderedRbacAuthorizationK8sIo_v1_role_gatekeeperManagerRoleYamlBytes() ([]byte, error) {