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..da80e4a95af 100644 --- a/controllers/gitopsservice_controller_test.go +++ b/controllers/gitopsservice_controller_test.go @@ -642,14 +642,15 @@ func TestReconcile_PSSLabels(t *testing.T) { addKnownTypesToScheme(s) testCases := []struct { - name string - namespace string - labels map[string]string + name string + namespace string + initial_labels map[string]string + expected_labels map[string]string }{ { - name: "modified valid PSS labels for openshift-gitops ns", + name: "openshift-gitops: podSecurityLabelSync absent, valid PSS labels only", namespace: "openshift-gitops", - labels: map[string]string{ + initial_labels: map[string]string{ "pod-security.kubernetes.io/enforce": "privileged", "pod-security.kubernetes.io/enforce-version": "v1.30", "pod-security.kubernetes.io/audit": "privileged", @@ -657,26 +658,67 @@ func TestReconcile_PSSLabels(t *testing.T) { "pod-security.kubernetes.io/warn": "privileged", "pod-security.kubernetes.io/warn-version": "v1.29", }, + expected_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", + PodSecurityLabelSyncLabel: PodSecurityLabelSyncLabelValue, + }, }, { - name: "modified invalid and empty PSS labels for openshift-gitops ns", + name: "openshift-gitops: podSecurityLabelSync absent, invalid PSS labels only", namespace: "openshift-gitops", - labels: map[string]string{ + initial_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", }, + expected_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", + PodSecurityLabelSyncLabel: PodSecurityLabelSyncLabelValue, + }, + }, + { + name: "openshift-gitops: podSecurityLabelSync wrong value", + namespace: "openshift-gitops", + initial_labels: map[string]string{ + "openshift.io/cluster-monitoring": "true", + PodSecurityLabelSyncLabel: "false", + "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", + }, + expected_labels: map[string]string{ + "openshift.io/cluster-monitoring": "true", + PodSecurityLabelSyncLabel: PodSecurityLabelSyncLabelValue, + "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", + }, + }, + { + name: "test: user namespace labels unchanged by reconcile (no PSS / no sync)", + namespace: "test", + initial_labels: map[string]string{ + "openshift.io/cluster-monitoring": "true", + }, + expected_labels: map[string]string{ + "openshift.io/cluster-monitoring": "true", + }, }, - } - - 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 +746,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)) + for key, value := range tc.expected_labels { + label, found := reconciled_ns.Labels[key] + assert.Check(t, found) + assert.Equal(t, label, value) + } + }) } } 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..49b9f21e92b 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,12 +1,16 @@ package sequential import ( + "context" + . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "github.com/redhat-developer/gitops-operator/test/openshift/e2e/ginkgo/fixture" k8sFixture "github.com/redhat-developer/gitops-operator/test/openshift/e2e/ginkgo/fixture/k8s" + fixtureUtils "github.com/redhat-developer/gitops-operator/test/openshift/e2e/ginkgo/fixture/utils" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" ) var _ = Describe("GitOps Operator Sequential E2E Tests", func() { @@ -17,8 +21,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 populates pod-security labels", func() { gitopsNS := &corev1.Namespace{ ObjectMeta: metav1.ObjectMeta{ Name: "openshift-gitops", @@ -26,12 +29,42 @@ 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 populates at least one pod-security.kubernetes.io/* label") + pssLabelKeys := []string{ + "pod-security.kubernetes.io/audit", + "pod-security.kubernetes.io/audit-version", + "pod-security.kubernetes.io/enforce", + "pod-security.kubernetes.io/enforce-version", + "pod-security.kubernetes.io/warn", + "pod-security.kubernetes.io/warn-version", + } + Eventually(func() bool { + k8sClient, _ := fixtureUtils.GetE2ETestKubeClient() + ns := &corev1.Namespace{} + if err := k8sClient.Get(context.Background(), client.ObjectKey{Name: "openshift-gitops"}, ns); err != nil { + return false + } + if ns.Labels == nil { + return false + } + for _, key := range pssLabelKeys { + if ns.Labels[key] != "" { + return true + } + } + return false + }, "5m", "5s").Should(BeTrue(), "expected at least one pod-security.kubernetes.io/* label to be set by OpenShift") + + k8sClient, _ := fixtureUtils.GetE2ETestKubeClient() + ns := &corev1.Namespace{} + Expect(k8sClient.Get(context.Background(), client.ObjectKey{Name: "openshift-gitops"}, ns)).To(Succeed()) + for _, key := range pssLabelKeys { + GinkgoWriter.Printf("observed namespace label %s=%q\n", key, ns.Labels[key]) + } }) })