Skip to content

[v1.19] fix: add podSecurityLabelSync=true to openshift-gitops (#1105) #1121

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
nmirasch wants to merge 1 commit into
base: v1.19
Choose a base branch
from
+88 −92
Open

[v1.19] fix: add podSecurityLabelSync=true to openshift-gitops (#1105) #1121

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 10 additions & 25 deletions controllers/gitopsservice_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -872,14 +875,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{
Expand Down Expand Up @@ -966,23 +962,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
}
98 changes: 39 additions & 59 deletions controllers/gitopsservice_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down Expand Up @@ -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)
}
})
}
}

Expand Down
47 changes: 39 additions & 8 deletions test/openshift/e2e/ginkgo/sequential/1-110_validate_podsecurity_alerts_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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"
Expand All @@ -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() {
Expand All @@ -17,21 +28,41 @@ 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",
},
}
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: <nil>")
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")
})

})
Expand Down
Loading