diff --git a/controllers/gitopsservice_controller.go b/controllers/gitopsservice_controller.go index b3cf06f5900..8e9f4e15471 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. @@ -856,14 +859,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{ @@ -950,23 +946,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 e1bbc300588..f542f8abb20 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..4db69db8a75 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 @@ -1,6 +1,8 @@ package sequential import ( + "time" + . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "github.com/redhat-developer/gitops-operator/test/openshift/e2e/ginkgo/fixture" @@ -9,6 +11,15 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) +const ( + psaAudit = "pod-security.kubernetes.io/audit" + psaAuditVersion = "pod-security.kubernetes.io/audit-version" + psaEnforce = "pod-security.kubernetes.io/enforce" + psaEnforceVersion = "pod-security.kubernetes.io/enforce-version" + psaWarn = "pod-security.kubernetes.io/warn" + psaWarnVersion = "pod-security.kubernetes.io/warn-version" +) + var _ = Describe("GitOps Operator Sequential E2E Tests", func() { Context("1-110_validate_podsecurity_alerts", func() { @@ -17,8 +28,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 +36,33 @@ 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 PSA label sync: audit and warn must be restricted; *-version present for each set mode. Enforce must be restricted when set (may be omitted by OpenShift)") + Eventually(func() bool { + if gitopsNS.Labels == nil { + GinkgoWriter.Println("[1-110] openshift-gitops metadata.labels: ") + return false + } + l := gitopsNS.Labels + + ok := l[psaAudit] == "restricted" && l[psaWarn] == "restricted" && l[psaAuditVersion] != "" && l[psaWarnVersion] != "" + if enforceValue := l[psaEnforce]; enforceValue != "" { // enforce may be omitted by OpenShift. If the label is set, it must be restricted and pod-security.kubernetes.io/enforce-version must be non-empty. + ok = ok && enforceValue == "restricted" && l[psaEnforceVersion] != "" + } + keys := make([]string, 0, len(gitopsNS.Labels)) + for k := range gitopsNS.Labels { + keys = append(keys, k) + } + GinkgoWriter.Printf("[1-110] openshift-gitops metadata.labels (%d):\n", len(gitopsNS.Labels)) + for _, k := range keys { + GinkgoWriter.Printf(" %s=%q\n", k, gitopsNS.Labels[k]) + } + return ok + }).WithTimeout(5*time.Minute).WithPolling(5*time.Second).Should(BeTrue(), + "expected pod-security audit+warn=restricted with non-empty audit-version and warn-version; enforce=restricted+enforce-version when enforce label exists") }) })