Skip to content

Commit 6e93255

Browse files
nmiraschnmirasch
committed
fix: add podSecurityLabelSync=true to openshift-gitops
Signed-off-by: nmirasch <neus.miras@gmail.com>
1 parent c4bdd09 commit 6e93255

3 files changed

Lines changed: 117 additions & 82 deletions

File tree

‎controllers/gitopsservice_controller.go‎

Lines changed: 10 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,9 @@ var (
7373

7474
const (
7575
gitopsServicePrefix = "gitops-service-"
76+
// PodSecurityLabelSyncLabel enables OpenShift to manage pod-security.kubernetes.io/* on the namespace.
77+
PodSecurityLabelSyncLabel = "security.openshift.io/scc.podSecurityLabelSync"
78+
PodSecurityLabelSyncLabelValue = "true"
7679
)
7780

7881
// SetupWithManager sets up the controller with the Manager.
@@ -873,14 +876,7 @@ func newRestrictedNamespace(ns string) *corev1.Namespace {
873876
}
874877

875878
if strings.HasPrefix(ns, "openshift-") {
876-
// Set pod security policy, which is required for namespaces pre-fixed with openshift
877-
// as the pod security label syncer doesn't set them on OCP namespaces.
878-
objectMeta.Labels["pod-security.kubernetes.io/enforce"] = "restricted"
879-
objectMeta.Labels["pod-security.kubernetes.io/enforce-version"] = "v1.29"
880-
objectMeta.Labels["pod-security.kubernetes.io/audit"] = "restricted"
881-
objectMeta.Labels["pod-security.kubernetes.io/audit-version"] = "latest"
882-
objectMeta.Labels["pod-security.kubernetes.io/warn"] = "restricted"
883-
objectMeta.Labels["pod-security.kubernetes.io/warn-version"] = "latest"
879+
objectMeta.Labels[PodSecurityLabelSyncLabel] = PodSecurityLabelSyncLabelValue
884880
}
885881

886882
return &corev1.Namespace{
@@ -967,23 +963,12 @@ func policyRuleForBackendServiceClusterRole() []rbacv1.PolicyRule {
967963
}
968964

969965
func ensurePodSecurityLabels(namespace *corev1.Namespace) (bool, *corev1.Namespace) {
970-
971-
pssLabels := map[string]string{
972-
"pod-security.kubernetes.io/enforce": "restricted",
973-
"pod-security.kubernetes.io/enforce-version": "v1.29",
974-
"pod-security.kubernetes.io/audit": "restricted",
975-
"pod-security.kubernetes.io/audit-version": "latest",
976-
"pod-security.kubernetes.io/warn": "restricted",
977-
"pod-security.kubernetes.io/warn-version": "latest",
966+
if namespace.Labels == nil {
967+
namespace.Labels = make(map[string]string)
978968
}
979-
980-
changed := false
981-
for pssKey, pssVal := range pssLabels {
982-
if nsVal, exists := namespace.Labels[pssKey]; !exists || nsVal != pssVal {
983-
namespace.Labels[pssKey] = pssVal
984-
changed = true
985-
}
986-
969+
if namespace.Labels[PodSecurityLabelSyncLabel] != PodSecurityLabelSyncLabelValue {
970+
namespace.Labels[PodSecurityLabelSyncLabel] = PodSecurityLabelSyncLabelValue
971+
return true, namespace
987972
}
988-
return changed, namespace
973+
return false, namespace
989974
}

‎controllers/gitopsservice_controller_test.go‎

Lines changed: 75 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -642,41 +642,83 @@ func TestReconcile_PSSLabels(t *testing.T) {
642642
addKnownTypesToScheme(s)
643643

644644
testCases := []struct {
645-
name string
646-
namespace string
647-
labels map[string]string
645+
name string
646+
namespace string
647+
initial_labels map[string]string
648+
expected_labels map[string]string
648649
}{
649650
{
650-
name: "modified valid PSS labels for openshift-gitops ns",
651+
name: "openshift-gitops: podSecurityLabelSync absent, valid PSS labels only",
651652
namespace: "openshift-gitops",
652-
labels: map[string]string{
653+
initial_labels: map[string]string{
653654
"pod-security.kubernetes.io/enforce": "privileged",
654655
"pod-security.kubernetes.io/enforce-version": "v1.30",
655656
"pod-security.kubernetes.io/audit": "privileged",
656657
"pod-security.kubernetes.io/audit-version": "v1.29",
657658
"pod-security.kubernetes.io/warn": "privileged",
658659
"pod-security.kubernetes.io/warn-version": "v1.29",
659660
},
661+
expected_labels: map[string]string{
662+
"pod-security.kubernetes.io/enforce": "privileged",
663+
"pod-security.kubernetes.io/enforce-version": "v1.30",
664+
"pod-security.kubernetes.io/audit": "privileged",
665+
"pod-security.kubernetes.io/audit-version": "v1.29",
666+
"pod-security.kubernetes.io/warn": "privileged",
667+
"pod-security.kubernetes.io/warn-version": "v1.29",
668+
PodSecurityLabelSyncLabel: PodSecurityLabelSyncLabelValue,
669+
},
660670
},
661671
{
662-
name: "modified invalid and empty PSS labels for openshift-gitops ns",
672+
name: "openshift-gitops: podSecurityLabelSync absent, invalid PSS labels only",
663673
namespace: "openshift-gitops",
664-
labels: map[string]string{
674+
initial_labels: map[string]string{
665675
"pod-security.kubernetes.io/enforce": "invalid",
666676
"pod-security.kubernetes.io/enforce-version": "invalid",
667677
"pod-security.kubernetes.io/warn": "invalid",
668678
"pod-security.kubernetes.io/warn-version": "invalid",
669679
},
680+
expected_labels: map[string]string{
681+
"pod-security.kubernetes.io/enforce": "invalid",
682+
"pod-security.kubernetes.io/enforce-version": "invalid",
683+
"pod-security.kubernetes.io/warn": "invalid",
684+
"pod-security.kubernetes.io/warn-version": "invalid",
685+
PodSecurityLabelSyncLabel: PodSecurityLabelSyncLabelValue,
686+
},
687+
},
688+
{
689+
name: "openshift-gitops: podSecurityLabelSync wrong value",
690+
namespace: "openshift-gitops",
691+
initial_labels: map[string]string{
692+
"openshift.io/cluster-monitoring": "true",
693+
PodSecurityLabelSyncLabel: "false",
694+
"pod-security.kubernetes.io/enforce": "restricted",
695+
"pod-security.kubernetes.io/enforce-version": "v1.29",
696+
"pod-security.kubernetes.io/audit": "restricted",
697+
"pod-security.kubernetes.io/audit-version": "latest",
698+
"pod-security.kubernetes.io/warn": "restricted",
699+
"pod-security.kubernetes.io/warn-version": "latest",
700+
},
701+
expected_labels: map[string]string{
702+
"openshift.io/cluster-monitoring": "true",
703+
PodSecurityLabelSyncLabel: PodSecurityLabelSyncLabelValue,
704+
"pod-security.kubernetes.io/enforce": "restricted",
705+
"pod-security.kubernetes.io/enforce-version": "v1.29",
706+
"pod-security.kubernetes.io/audit": "restricted",
707+
"pod-security.kubernetes.io/audit-version": "latest",
708+
"pod-security.kubernetes.io/warn": "restricted",
709+
"pod-security.kubernetes.io/warn-version": "latest",
710+
},
711+
},
712+
{
713+
name: "test: user namespace labels unchanged by reconcile (no PSS / no sync)",
714+
namespace: "test",
715+
initial_labels: map[string]string{
716+
"openshift.io/cluster-monitoring": "true",
717+
},
718+
expected_labels: map[string]string{
719+
"openshift.io/cluster-monitoring": "true",
720+
},
670721
},
671-
}
672-
673-
expected_labels := map[string]string{
674-
"pod-security.kubernetes.io/enforce": "restricted",
675-
"pod-security.kubernetes.io/enforce-version": "v1.29",
676-
"pod-security.kubernetes.io/audit": "restricted",
677-
"pod-security.kubernetes.io/audit-version": "latest",
678-
"pod-security.kubernetes.io/warn": "restricted",
679-
"pod-security.kubernetes.io/warn-version": "latest",
680722
}
681723

682724
fakeClient := fake.NewFakeClient(util.NewClusterVersion("4.7.1"), newGitopsService())
@@ -704,40 +746,24 @@ func TestReconcile_PSSLabels(t *testing.T) {
704746
_, err = reconciler.Reconcile(context.TODO(), newRequest("test", "test"))
705747
assertNoError(t, err)
706748

707-
// Check if PSS labels are addded to the user defined ns
708-
reconciled_ns := &corev1.Namespace{}
709-
err = fakeClient.Get(context.TODO(), types.NamespacedName{Name: "test"},
710-
reconciled_ns)
711-
assertNoError(t, err)
712-
713-
for label := range reconciled_ns.Labels {
714-
_, found := expected_labels[label]
715-
// Fail if label is found
716-
assert.Check(t, found != true)
717-
}
718-
719749
for _, tc := range testCases {
720-
existing_ns := &corev1.Namespace{}
721-
assert.NilError(t, fakeClient.Get(context.TODO(), types.NamespacedName{Name: tc.namespace}, existing_ns), err)
722-
723-
// Assign new values, confirm the assignment and update the PSS labels
724-
existing_ns.Labels = tc.labels
725-
err := fakeClient.Update(context.TODO(), existing_ns)
726-
assert.NilError(t, err)
727-
assert.NilError(t, fakeClient.Get(context.TODO(), types.NamespacedName{Name: tc.namespace}, existing_ns), err)
728-
assert.DeepEqual(t, existing_ns.Labels, tc.labels)
729-
730-
_, err = reconciler.Reconcile(context.TODO(), newRequest("test", "test"))
731-
assertNoError(t, err)
732-
733-
assert.NilError(t, fakeClient.Get(context.TODO(), types.NamespacedName{Name: tc.namespace}, reconciled_ns), err)
734-
735-
for key, value := range expected_labels {
736-
label, found := reconciled_ns.Labels[key]
737-
// Fail if label is not found, comapre the values with the expected values if found
738-
assert.Check(t, found)
739-
assert.Equal(t, label, value)
740-
}
750+
t.Run(tc.name, func(t *testing.T) {
751+
ns := &corev1.Namespace{}
752+
assert.NilError(t, fakeClient.Get(context.TODO(), types.NamespacedName{Name: tc.namespace}, ns))
753+
ns.Labels = tc.initial_labels
754+
assert.NilError(t, fakeClient.Update(context.TODO(), ns))
755+
756+
_, err := reconciler.Reconcile(context.TODO(), newRequest("test", "test"))
757+
assertNoError(t, err)
758+
759+
reconciled_ns := &corev1.Namespace{}
760+
assert.NilError(t, fakeClient.Get(context.TODO(), types.NamespacedName{Name: tc.namespace}, reconciled_ns))
761+
for key, value := range tc.expected_labels {
762+
label, found := reconciled_ns.Labels[key]
763+
assert.Check(t, found)
764+
assert.Equal(t, label, value)
765+
}
766+
})
741767
}
742768
}
743769

‎test/openshift/e2e/ginkgo/sequential/1-110_validate_podsecurity_alerts_test.go‎

Lines changed: 32 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,17 @@
11
package sequential
22

33
import (
4+
"context"
5+
"time"
6+
47
. "github.com/onsi/ginkgo/v2"
58
. "github.com/onsi/gomega"
69
"github.com/redhat-developer/gitops-operator/test/openshift/e2e/ginkgo/fixture"
710
k8sFixture "github.com/redhat-developer/gitops-operator/test/openshift/e2e/ginkgo/fixture/k8s"
11+
fixtureUtils "github.com/redhat-developer/gitops-operator/test/openshift/e2e/ginkgo/fixture/utils"
812
corev1 "k8s.io/api/core/v1"
913
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
14+
"sigs.k8s.io/controller-runtime/pkg/client"
1015
)
1116

1217
var _ = Describe("GitOps Operator Sequential E2E Tests", func() {
@@ -17,21 +22,40 @@ var _ = Describe("GitOps Operator Sequential E2E Tests", func() {
1722
fixture.EnsureSequentialCleanSlate()
1823
})
1924

20-
It("verifies openshift-gitops Namespace has expected pod-security labels", func() {
21-
25+
It("verifies openshift-gitops: operator sets podSecurityLabelSync and OpenShift populates pod-security label keys", func() {
2226
gitopsNS := &corev1.Namespace{
2327
ObjectMeta: metav1.ObjectMeta{
2428
Name: "openshift-gitops",
2529
},
2630
}
2731
Eventually(gitopsNS).Should(k8sFixture.ExistByName())
2832

29-
Eventually(gitopsNS).Should(k8sFixture.HaveLabelWithValue("pod-security.kubernetes.io/audit", "restricted"))
30-
Eventually(gitopsNS).Should(k8sFixture.HaveLabelWithValue("pod-security.kubernetes.io/audit-version", "latest"))
31-
Eventually(gitopsNS).Should(k8sFixture.HaveLabelWithValue("pod-security.kubernetes.io/enforce", "restricted"))
32-
Eventually(gitopsNS).Should(k8sFixture.HaveLabelWithValue("pod-security.kubernetes.io/enforce-version", "v1.29"))
33-
Eventually(gitopsNS).Should(k8sFixture.HaveLabelWithValue("pod-security.kubernetes.io/warn", "restricted"))
34-
Eventually(gitopsNS).Should(k8sFixture.HaveLabelWithValue("pod-security.kubernetes.io/warn-version", "latest"))
33+
By("GitOps operator ensures security.openshift.io/scc.podSecurityLabelSync=true")
34+
Eventually(gitopsNS, "5m", "5s").Should(
35+
k8sFixture.HaveLabelWithValue("security.openshift.io/scc.podSecurityLabelSync", "true"))
36+
37+
By("OpenShift pod security label syncer sets pod-security.kubernetes.io/* (values depend on OCP version; only non-empty keys are asserted)")
38+
for _, key := range []string{
39+
"pod-security.kubernetes.io/audit",
40+
"pod-security.kubernetes.io/audit-version",
41+
"pod-security.kubernetes.io/enforce",
42+
"pod-security.kubernetes.io/enforce-version",
43+
"pod-security.kubernetes.io/warn",
44+
"pod-security.kubernetes.io/warn-version",
45+
} {
46+
labelKey := key
47+
Eventually(func() bool {
48+
k8sClient, _ := fixtureUtils.GetE2ETestKubeClient()
49+
ns := &corev1.Namespace{}
50+
if err := k8sClient.Get(context.Background(), client.ObjectKey{Name: "openshift-gitops"}, ns); err != nil {
51+
return false
52+
}
53+
if ns.Labels == nil {
54+
return false
55+
}
56+
return ns.Labels[labelKey] != ""
57+
}).WithTimeout(5*time.Minute).WithPolling(5*time.Second).Should(BeTrue(), "expected label %s to be set by OpenShift", labelKey)
58+
}
3559
})
3660

3761
})

0 commit comments

Comments
 (0)