diff --git a/controllers/gitopsservice_controller.go b/controllers/gitopsservice_controller.go index 4782e40e8f0..3838801beff 100644 --- a/controllers/gitopsservice_controller.go +++ b/controllers/gitopsservice_controller.go @@ -73,6 +73,9 @@ var ( const ( gitopsServicePrefix = "gitops-service-" + // PodSecurityLabelSyncLabel enables OpenShift to manage pod-security.kubernetes.io/* on the namespace. + PodSecurityLabelSyncLabel = "security.openshift.io/scc.podSecurityLabelSync" + PodSecurityLabelSyncLabelValue = "true" ) // SetupWithManager sets up the controller with the Manager. @@ -873,14 +876,7 @@ func newRestrictedNamespace(ns string) *corev1.Namespace { } if strings.HasPrefix(ns, "openshift-") { - // Set pod security policy, which is required for namespaces pre-fixed with openshift - // as the pod security label syncer doesn't set them on OCP namespaces. - objectMeta.Labels["pod-security.kubernetes.io/enforce"] = "restricted" - objectMeta.Labels["pod-security.kubernetes.io/enforce-version"] = "v1.29" - objectMeta.Labels["pod-security.kubernetes.io/audit"] = "restricted" - objectMeta.Labels["pod-security.kubernetes.io/audit-version"] = "latest" - objectMeta.Labels["pod-security.kubernetes.io/warn"] = "restricted" - objectMeta.Labels["pod-security.kubernetes.io/warn-version"] = "latest" + objectMeta.Labels[PodSecurityLabelSyncLabel] = PodSecurityLabelSyncLabelValue } return &corev1.Namespace{ @@ -967,23 +963,12 @@ func policyRuleForBackendServiceClusterRole() []rbacv1.PolicyRule { } func ensurePodSecurityLabels(namespace *corev1.Namespace) (bool, *corev1.Namespace) { - - pssLabels := map[string]string{ - "pod-security.kubernetes.io/enforce": "restricted", - "pod-security.kubernetes.io/enforce-version": "v1.29", - "pod-security.kubernetes.io/audit": "restricted", - "pod-security.kubernetes.io/audit-version": "latest", - "pod-security.kubernetes.io/warn": "restricted", - "pod-security.kubernetes.io/warn-version": "latest", + if namespace.Labels == nil { + namespace.Labels = make(map[string]string) } - - changed := false - for pssKey, pssVal := range pssLabels { - if nsVal, exists := namespace.Labels[pssKey]; !exists || nsVal != pssVal { - namespace.Labels[pssKey] = pssVal - changed = true - } - + if namespace.Labels[PodSecurityLabelSyncLabel] != PodSecurityLabelSyncLabelValue { + namespace.Labels[PodSecurityLabelSyncLabel] = PodSecurityLabelSyncLabelValue + return true, namespace } - return changed, namespace + return false, namespace } diff --git a/controllers/gitopsservice_controller_test.go b/controllers/gitopsservice_controller_test.go index ddfad2bf3d0..2950bc2a76a 100644 --- a/controllers/gitopsservice_controller_test.go +++ b/controllers/gitopsservice_controller_test.go @@ -641,42 +641,38 @@ func TestReconcile_PSSLabels(t *testing.T) { s := scheme.Scheme addKnownTypesToScheme(s) + // Unit tests only assert the operator-managed sync label; pod-security.kubernetes.io/* is owned by OpenShift (e2e). testCases := []struct { - name string - namespace string - labels map[string]string + name string + namespace string + initial_labels map[string]string + wantPodSecurityLabelSync bool }{ { - name: "modified valid PSS labels for openshift-gitops ns", + name: "openshift-gitops: podSecurityLabelSync absent", namespace: "openshift-gitops", - labels: map[string]string{ - "pod-security.kubernetes.io/enforce": "privileged", - "pod-security.kubernetes.io/enforce-version": "v1.30", - "pod-security.kubernetes.io/audit": "privileged", - "pod-security.kubernetes.io/audit-version": "v1.29", - "pod-security.kubernetes.io/warn": "privileged", - "pod-security.kubernetes.io/warn-version": "v1.29", + initial_labels: map[string]string{ + "openshift.io/cluster-monitoring": "true", }, + wantPodSecurityLabelSync: true, }, { - name: "modified invalid and empty PSS labels for openshift-gitops ns", + name: "openshift-gitops: podSecurityLabelSync wrong value", namespace: "openshift-gitops", - labels: map[string]string{ - "pod-security.kubernetes.io/enforce": "invalid", - "pod-security.kubernetes.io/enforce-version": "invalid", - "pod-security.kubernetes.io/warn": "invalid", - "pod-security.kubernetes.io/warn-version": "invalid", + initial_labels: map[string]string{ + "openshift.io/cluster-monitoring": "true", + PodSecurityLabelSyncLabel: "false", }, + wantPodSecurityLabelSync: true, + }, + { + name: "test: operator does not set podSecurityLabelSync on non-openshift-* namespaces", + namespace: "test", + initial_labels: map[string]string{ + "openshift.io/cluster-monitoring": "true", + }, + wantPodSecurityLabelSync: false, }, - } - - expected_labels := map[string]string{ - "pod-security.kubernetes.io/enforce": "restricted", - "pod-security.kubernetes.io/enforce-version": "v1.29", - "pod-security.kubernetes.io/audit": "restricted", - "pod-security.kubernetes.io/audit-version": "latest", - "pod-security.kubernetes.io/warn": "restricted", - "pod-security.kubernetes.io/warn-version": "latest", } fakeClient := fake.NewFakeClient(util.NewClusterVersion("4.7.1"), newGitopsService()) @@ -704,40 +700,24 @@ func TestReconcile_PSSLabels(t *testing.T) { _, err = reconciler.Reconcile(context.TODO(), newRequest("test", "test")) assertNoError(t, err) - // Check if PSS labels are addded to the user defined ns - reconciled_ns := &corev1.Namespace{} - err = fakeClient.Get(context.TODO(), types.NamespacedName{Name: "test"}, - reconciled_ns) - assertNoError(t, err) - - for label := range reconciled_ns.Labels { - _, found := expected_labels[label] - // Fail if label is found - assert.Check(t, found != true) - } - for _, tc := range testCases { - existing_ns := &corev1.Namespace{} - assert.NilError(t, fakeClient.Get(context.TODO(), types.NamespacedName{Name: tc.namespace}, existing_ns), err) - - // Assign new values, confirm the assignment and update the PSS labels - existing_ns.Labels = tc.labels - err := fakeClient.Update(context.TODO(), existing_ns) - assert.NilError(t, err) - assert.NilError(t, fakeClient.Get(context.TODO(), types.NamespacedName{Name: tc.namespace}, existing_ns), err) - assert.DeepEqual(t, existing_ns.Labels, tc.labels) - - _, err = reconciler.Reconcile(context.TODO(), newRequest("test", "test")) - assertNoError(t, err) - - assert.NilError(t, fakeClient.Get(context.TODO(), types.NamespacedName{Name: tc.namespace}, reconciled_ns), err) - - for key, value := range expected_labels { - label, found := reconciled_ns.Labels[key] - // Fail if label is not found, comapre the values with the expected values if found - assert.Check(t, found) - assert.Equal(t, label, value) - } + t.Run(tc.name, func(t *testing.T) { + ns := &corev1.Namespace{} + assert.NilError(t, fakeClient.Get(context.TODO(), types.NamespacedName{Name: tc.namespace}, ns)) + ns.Labels = tc.initial_labels + assert.NilError(t, fakeClient.Update(context.TODO(), ns)) + + _, err := reconciler.Reconcile(context.TODO(), newRequest("test", "test")) + assertNoError(t, err) + + reconciled_ns := &corev1.Namespace{} + assert.NilError(t, fakeClient.Get(context.TODO(), types.NamespacedName{Name: tc.namespace}, reconciled_ns)) + if tc.wantPodSecurityLabelSync { + assert.Equal(t, PodSecurityLabelSyncLabelValue, reconciled_ns.Labels[PodSecurityLabelSyncLabel]) + } else { + assert.Check(t, reconciled_ns.Labels[PodSecurityLabelSyncLabel] != PodSecurityLabelSyncLabelValue) + } + }) } } diff --git a/test/openshift/e2e/ginkgo/sequential/1-110_validate_podsecurity_alerts_test.go b/test/openshift/e2e/ginkgo/sequential/1-110_validate_podsecurity_alerts_test.go index fe4cde6cd77..e8d31909822 100644 --- a/test/openshift/e2e/ginkgo/sequential/1-110_validate_podsecurity_alerts_test.go +++ b/test/openshift/e2e/ginkgo/sequential/1-110_validate_podsecurity_alerts_test.go @@ -17,8 +17,7 @@ var _ = Describe("GitOps Operator Sequential E2E Tests", func() { fixture.EnsureSequentialCleanSlate() }) - It("verifies openshift-gitops Namespace has expected pod-security labels", func() { - + It("verifies openshift-gitops: operator sets podSecurityLabelSync and OpenShift sets audit to restricted", func() { gitopsNS := &corev1.Namespace{ ObjectMeta: metav1.ObjectMeta{ Name: "openshift-gitops", @@ -26,12 +25,13 @@ var _ = Describe("GitOps Operator Sequential E2E Tests", func() { } Eventually(gitopsNS).Should(k8sFixture.ExistByName()) - Eventually(gitopsNS).Should(k8sFixture.HaveLabelWithValue("pod-security.kubernetes.io/audit", "restricted")) - Eventually(gitopsNS).Should(k8sFixture.HaveLabelWithValue("pod-security.kubernetes.io/audit-version", "latest")) - Eventually(gitopsNS).Should(k8sFixture.HaveLabelWithValue("pod-security.kubernetes.io/enforce", "restricted")) - Eventually(gitopsNS).Should(k8sFixture.HaveLabelWithValue("pod-security.kubernetes.io/enforce-version", "v1.29")) - Eventually(gitopsNS).Should(k8sFixture.HaveLabelWithValue("pod-security.kubernetes.io/warn", "restricted")) - Eventually(gitopsNS).Should(k8sFixture.HaveLabelWithValue("pod-security.kubernetes.io/warn-version", "latest")) + By("GitOps operator ensures security.openshift.io/scc.podSecurityLabelSync=true") + Eventually(gitopsNS, "5m", "5s").Should( + k8sFixture.HaveLabelWithValue("security.openshift.io/scc.podSecurityLabelSync", "true")) + + By("OpenShift sets pod-security.kubernetes.io/audit=restricted (pod-security *-version labels vary by cluster and are not asserted)") + Eventually(gitopsNS, "5m", "5s").Should( + k8sFixture.HaveLabelWithValue("pod-security.kubernetes.io/audit", "restricted")) }) })