Skip to content

Commit facc084

Browse files
nmiraschnmirasch
committed
fix: add podSecurityLabelSync=true to openshift-gitops (#1105)
* fix: add podSecurityLabelSync=true to openshift-gitops Signed-off-by: nmirasch <neus.miras@gmail.com> * e2e check at least one PSS label is set by Openshift Signed-off-by: nmirasch <neus.miras@gmail.com> * assert podSecurityLabelSync and audit=restricted for openshift-gitops ns Signed-off-by: nmirasch <neus.miras@gmail.com> * added podSecurityLabels validation for enforce, warn and *-version presence Signed-off-by: nmirasch <neus.miras@gmail.com> --------- Signed-off-by: nmirasch <neus.miras@gmail.com> (cherry picked from commit 88b2301)
1 parent c46cd66 commit facc084

3 files changed

Lines changed: 88 additions & 92 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: 39 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -641,42 +641,38 @@ func TestReconcile_PSSLabels(t *testing.T) {
641641
s := scheme.Scheme
642642
addKnownTypesToScheme(s)
643643

644+
// Unit tests only assert the operator-managed sync label; pod-security.kubernetes.io/* is owned by OpenShift (e2e).
644645
testCases := []struct {
645-
name string
646-
namespace string
647-
labels map[string]string
646+
name string
647+
namespace string
648+
initial_labels map[string]string
649+
wantPodSecurityLabelSync bool
648650
}{
649651
{
650-
name: "modified valid PSS labels for openshift-gitops ns",
652+
name: "openshift-gitops: podSecurityLabelSync absent",
651653
namespace: "openshift-gitops",
652-
labels: map[string]string{
653-
"pod-security.kubernetes.io/enforce": "privileged",
654-
"pod-security.kubernetes.io/enforce-version": "v1.30",
655-
"pod-security.kubernetes.io/audit": "privileged",
656-
"pod-security.kubernetes.io/audit-version": "v1.29",
657-
"pod-security.kubernetes.io/warn": "privileged",
658-
"pod-security.kubernetes.io/warn-version": "v1.29",
654+
initial_labels: map[string]string{
655+
"openshift.io/cluster-monitoring": "true",
659656
},
657+
wantPodSecurityLabelSync: true,
660658
},
661659
{
662-
name: "modified invalid and empty PSS labels for openshift-gitops ns",
660+
name: "openshift-gitops: podSecurityLabelSync wrong value",
663661
namespace: "openshift-gitops",
664-
labels: map[string]string{
665-
"pod-security.kubernetes.io/enforce": "invalid",
666-
"pod-security.kubernetes.io/enforce-version": "invalid",
667-
"pod-security.kubernetes.io/warn": "invalid",
668-
"pod-security.kubernetes.io/warn-version": "invalid",
662+
initial_labels: map[string]string{
663+
"openshift.io/cluster-monitoring": "true",
664+
PodSecurityLabelSyncLabel: "false",
669665
},
666+
wantPodSecurityLabelSync: true,
667+
},
668+
{
669+
name: "test: operator does not set podSecurityLabelSync on non-openshift-* namespaces",
670+
namespace: "test",
671+
initial_labels: map[string]string{
672+
"openshift.io/cluster-monitoring": "true",
673+
},
674+
wantPodSecurityLabelSync: false,
670675
},
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",
680676
}
681677

682678
fakeClient := fake.NewFakeClient(util.NewClusterVersion("4.7.1"), newGitopsService())
@@ -704,40 +700,24 @@ func TestReconcile_PSSLabels(t *testing.T) {
704700
_, err = reconciler.Reconcile(context.TODO(), newRequest("test", "test"))
705701
assertNoError(t, err)
706702

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-
719703
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-
}
704+
t.Run(tc.name, func(t *testing.T) {
705+
ns := &corev1.Namespace{}
706+
assert.NilError(t, fakeClient.Get(context.TODO(), types.NamespacedName{Name: tc.namespace}, ns))
707+
ns.Labels = tc.initial_labels
708+
assert.NilError(t, fakeClient.Update(context.TODO(), ns))
709+
710+
_, err := reconciler.Reconcile(context.TODO(), newRequest("test", "test"))
711+
assertNoError(t, err)
712+
713+
reconciled_ns := &corev1.Namespace{}
714+
assert.NilError(t, fakeClient.Get(context.TODO(), types.NamespacedName{Name: tc.namespace}, reconciled_ns))
715+
if tc.wantPodSecurityLabelSync {
716+
assert.Equal(t, PodSecurityLabelSyncLabelValue, reconciled_ns.Labels[PodSecurityLabelSyncLabel])
717+
} else {
718+
assert.Check(t, reconciled_ns.Labels[PodSecurityLabelSyncLabel] != PodSecurityLabelSyncLabelValue)
719+
}
720+
})
741721
}
742722
}
743723

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

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

33
import (
4+
"time"
5+
46
. "github.com/onsi/ginkgo/v2"
57
. "github.com/onsi/gomega"
68
"github.com/redhat-developer/gitops-operator/test/openshift/e2e/ginkgo/fixture"
@@ -9,6 +11,15 @@ import (
911
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1012
)
1113

14+
const (
15+
psaAudit = "pod-security.kubernetes.io/audit"
16+
psaAuditVersion = "pod-security.kubernetes.io/audit-version"
17+
psaEnforce = "pod-security.kubernetes.io/enforce"
18+
psaEnforceVersion = "pod-security.kubernetes.io/enforce-version"
19+
psaWarn = "pod-security.kubernetes.io/warn"
20+
psaWarnVersion = "pod-security.kubernetes.io/warn-version"
21+
)
22+
1223
var _ = Describe("GitOps Operator Sequential E2E Tests", func() {
1324

1425
Context("1-110_validate_podsecurity_alerts", func() {
@@ -17,21 +28,41 @@ var _ = Describe("GitOps Operator Sequential E2E Tests", func() {
1728
fixture.EnsureSequentialCleanSlate()
1829
})
1930

20-
It("verifies openshift-gitops Namespace has expected pod-security labels", func() {
21-
31+
It("verifies openshift-gitops: operator sets podSecurityLabelSync and OpenShift sets audit to restricted", func() {
2232
gitopsNS := &corev1.Namespace{
2333
ObjectMeta: metav1.ObjectMeta{
2434
Name: "openshift-gitops",
2535
},
2636
}
2737
Eventually(gitopsNS).Should(k8sFixture.ExistByName())
2838

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"))
39+
By("GitOps operator ensures security.openshift.io/scc.podSecurityLabelSync=true")
40+
Eventually(gitopsNS, "5m", "5s").Should(
41+
k8sFixture.HaveLabelWithValue("security.openshift.io/scc.podSecurityLabelSync", "true"))
42+
43+
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)")
44+
Eventually(func() bool {
45+
if gitopsNS.Labels == nil {
46+
GinkgoWriter.Println("[1-110] openshift-gitops metadata.labels: <nil>")
47+
return false
48+
}
49+
l := gitopsNS.Labels
50+
51+
ok := l[psaAudit] == "restricted" && l[psaWarn] == "restricted" && l[psaAuditVersion] != "" && l[psaWarnVersion] != ""
52+
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.
53+
ok = ok && enforceValue == "restricted" && l[psaEnforceVersion] != ""
54+
}
55+
keys := make([]string, 0, len(gitopsNS.Labels))
56+
for k := range gitopsNS.Labels {
57+
keys = append(keys, k)
58+
}
59+
GinkgoWriter.Printf("[1-110] openshift-gitops metadata.labels (%d):\n", len(gitopsNS.Labels))
60+
for _, k := range keys {
61+
GinkgoWriter.Printf(" %s=%q\n", k, gitopsNS.Labels[k])
62+
}
63+
return ok
64+
}).WithTimeout(5*time.Minute).WithPolling(5*time.Second).Should(BeTrue(),
65+
"expected pod-security audit+warn=restricted with non-empty audit-version and warn-version; enforce=restricted+enforce-version when enforce label exists")
3566
})
3667

3768
})

0 commit comments

Comments
 (0)