diff --git a/bundle/manifests/gatekeeper-operator.clusterserviceversion.yaml b/bundle/manifests/gatekeeper-operator.clusterserviceversion.yaml index b0e9e0df..32d0c469 100644 --- a/bundle/manifests/gatekeeper-operator.clusterserviceversion.yaml +++ b/bundle/manifests/gatekeeper-operator.clusterserviceversion.yaml @@ -493,14 +493,6 @@ spec: - patch - update - watch - - apiGroups: - - security.openshift.io - resourceNames: - - anyuid - resources: - - securitycontextconstraints - verbs: - - use serviceAccountName: gatekeeper-operator-controller-manager strategy: deployment installModes: diff --git a/config/gatekeeper/kustomization.yaml b/config/gatekeeper/kustomization.yaml index c46e4148..37c393ad 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 a2f77936..00000000 --- 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 5393e71d..00000000 --- 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/config/rbac/role.yaml b/config/rbac/role.yaml index 85cbba4f..7888bab8 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -268,11 +268,3 @@ rules: - patch - update - watch -- apiGroups: - - security.openshift.io - resourceNames: - - anyuid - resources: - - securitycontextconstraints - verbs: - - use diff --git a/controllers/gatekeeper_controller.go b/controllers/gatekeeper_controller.go index f1d54c79..7752f85e 100644 --- a/controllers/gatekeeper_controller.go +++ b/controllers/gatekeeper_controller.go @@ -168,7 +168,6 @@ const ( // +kubebuilder:rbac:groups=rbac.authorization.k8s.io,namespace="system",resources=roles;rolebindings,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=apps,namespace="system",resources=deployments,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=policy,namespace="system",resources=poddisruptionbudgets,verbs=create;delete;update;use -// +kubebuilder:rbac:groups=security.openshift.io,namespace="system",resources=securitycontextconstraints,resourceNames=anyuid,verbs=use // Reconcile is part of the main kubernetes reconciliation loop which aims to // move the current state of the cluster closer to the desired state. @@ -499,6 +498,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 +515,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 +749,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 4eb216a7..a9663cf6 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{ @@ -395,10 +453,12 @@ func assertPodAnnotations(g *WithT, obj *unstructured.Unstructured, expected map g.Expect(obj).NotTo(BeNil()) current, found, err := unstructured.NestedStringMap(obj.Object, "spec", "template", "metadata", "annotations") g.Expect(err).ToNot(HaveOccurred()) - g.Expect(found).To(BeTrue()) + if expected == nil { + g.Expect(found).To(BeFalse()) g.Expect(test.DefaultDeployment.PodAnnotations).To(BeEquivalentTo(current)) } else { + g.Expect(found).To(BeTrue()) g.Expect(expected).To(BeEquivalentTo(current)) } } diff --git a/pkg/bindata/bindata.go b/pkg/bindata/bindata.go index 1de9b841..4a26d301 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) { diff --git a/test/e2e/util/util.go b/test/e2e/util/util.go index 2c7d367e..de25a3f2 100644 --- a/test/e2e/util/util.go +++ b/test/e2e/util/util.go @@ -64,9 +64,6 @@ var DefaultDeployment = defaultConfig{ NodeSelector: map[string]string{ "kubernetes.io/os": "linux", }, - PodAnnotations: map[string]string{ - "container.seccomp.security.alpha.kubernetes.io/manager": "runtime/default", - }, Resources: &corev1.ResourceRequirements{ Limits: corev1.ResourceList{ corev1.ResourceCPU: resource.MustParse("1000m"),