Skip to content

Commit f652316

Browse files
varshab1210varshab1210
authored
Fix: PSS label reset issue (#821)
* fix PSS label reset issue Signed-off-by: Varsha B <[email protected]> * addressed review comments Signed-off-by: Varsha B <[email protected]> * addressed review comment Signed-off-by: Varsha B <[email protected]> --------- Signed-off-by: Varsha B <[email protected]>
1 parent 4e9539c commit f652316

File tree

2 files changed

+120
-7
lines changed

2 files changed

+120
-7
lines changed

Diff for: ‎controllers/gitopsservice_controller.go

+14-6
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ import (
5050
"sigs.k8s.io/controller-runtime/pkg/client"
5151
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
5252
"sigs.k8s.io/controller-runtime/pkg/event"
53+
"sigs.k8s.io/controller-runtime/pkg/handler"
5354
logf "sigs.k8s.io/controller-runtime/pkg/log"
5455
"sigs.k8s.io/controller-runtime/pkg/predicate"
5556
"sigs.k8s.io/controller-runtime/pkg/reconcile"
@@ -106,6 +107,13 @@ func (r *ReconcileGitopsService) SetupWithManager(mgr ctrl.Manager) error {
106107
Owns(&appsv1.Deployment{}, builder.WithPredicates(pred)).
107108
Owns(&corev1.Service{}, builder.WithPredicates(pred)).
108109
Owns(&routev1.Route{}, builder.WithPredicates(pred)).
110+
Watches(
111+
&corev1.Namespace{},
112+
&handler.EnqueueRequestForObject{},
113+
builder.WithPredicates(predicate.NewPredicateFuncs(func(obj client.Object) bool {
114+
return obj.GetName() == "openshift-gitops"
115+
})),
116+
).
109117
Complete(r)
110118
}
111119

@@ -219,7 +227,7 @@ func (r *ReconcileGitopsService) Reconcile(ctx context.Context, request reconcil
219227

220228
// Create namespace if it doesn't already exist
221229
namespaceRef := newRestrictedNamespace(namespace)
222-
err = r.Client.Get(ctx, types.NamespacedName{Name: namespace}, &corev1.Namespace{})
230+
err = r.Client.Get(ctx, types.NamespacedName{Name: namespace}, namespaceRef)
223231
if err != nil {
224232
if errors.IsNotFound(err) {
225233
reqLogger.Info("Creating a new Namespace", "Name", namespace)
@@ -231,8 +239,8 @@ func (r *ReconcileGitopsService) Reconcile(ctx context.Context, request reconcil
231239
return reconcile.Result{}, err
232240
}
233241
} else {
234-
needUpdate, updateNameSpace := ensurePodSecurityLabels(namespaceRef)
235-
if needUpdate {
242+
needsUpdate, updateNameSpace := ensurePodSecurityLabels(namespaceRef)
243+
if needsUpdate {
236244
err = r.Client.Update(context.TODO(), updateNameSpace)
237245
if err != nil {
238246
return reconcile.Result{}, err
@@ -345,7 +353,7 @@ func (r *ReconcileGitopsService) reconcileDefaultArgoCDInstance(instance *pipeli
345353
// 4.6 Cluster: Backend in openshift-pipelines-app-delivery namespace and argocd in openshift-gitops namespace
346354
// 4.7 Cluster: Both backend and argocd instance in openshift-gitops namespace
347355
argocdNS := newRestrictedNamespace(defaultArgoCDInstance.Namespace)
348-
err = r.Client.Get(context.TODO(), types.NamespacedName{Name: argocdNS.Name}, &corev1.Namespace{})
356+
err = r.Client.Get(context.TODO(), types.NamespacedName{Name: argocdNS.Name}, argocdNS)
349357
if err != nil {
350358
if errors.IsNotFound(err) {
351359
reqLogger.Info("Creating a new Namespace", "Name", argocdNS.Name)
@@ -378,8 +386,8 @@ func (r *ReconcileGitopsService) reconcileDefaultArgoCDInstance(instance *pipeli
378386
}
379387
}
380388

381-
needUpdate, updateNameSpace := ensurePodSecurityLabels(argocdNS)
382-
if needUpdate {
389+
needsUpdate, updateNameSpace := ensurePodSecurityLabels(argocdNS)
390+
if needsUpdate {
383391
err = r.Client.Update(context.TODO(), updateNameSpace)
384392
if err != nil {
385393
return reconcile.Result{}, err

Diff for: ‎controllers/gitopsservice_controller_test.go

+106-1
Original file line numberDiff line numberDiff line change
@@ -542,7 +542,8 @@ func TestReconcile_VerifyResourceQuotaDeletionForUpgrade(t *testing.T) {
542542
// Create namespace object for default ArgoCD instance and set resource quota to it.
543543
defaultArgoNS := &corev1.Namespace{
544544
ObjectMeta: v1.ObjectMeta{
545-
Name: serviceNamespace,
545+
Name: serviceNamespace,
546+
Namespace: serviceNamespace,
546547
},
547548
}
548549
fakeClient.Create(context.TODO(), defaultArgoNS)
@@ -632,6 +633,110 @@ func TestReconcile_InfrastructureNode(t *testing.T) {
632633

633634
}
634635

636+
func TestReconcile_PSSLabels(t *testing.T) {
637+
logf.SetLogger(argocd.ZapLogger(true))
638+
s := scheme.Scheme
639+
addKnownTypesToScheme(s)
640+
641+
testCases := []struct {
642+
name string
643+
namespace string
644+
labels map[string]string
645+
}{
646+
{
647+
name: "modified valid PSS labels for openshift-gitops ns",
648+
namespace: "openshift-gitops",
649+
labels: map[string]string{
650+
"pod-security.kubernetes.io/enforce": "privileged",
651+
"pod-security.kubernetes.io/enforce-version": "v1.30",
652+
"pod-security.kubernetes.io/audit": "privileged",
653+
"pod-security.kubernetes.io/audit-version": "v1.29",
654+
"pod-security.kubernetes.io/warn": "privileged",
655+
"pod-security.kubernetes.io/warn-version": "v1.29",
656+
},
657+
},
658+
{
659+
name: "modified invalid and empty PSS labels for openshift-gitops ns",
660+
namespace: "openshift-gitops",
661+
labels: map[string]string{
662+
"pod-security.kubernetes.io/enforce": "invalid",
663+
"pod-security.kubernetes.io/enforce-version": "invalid",
664+
"pod-security.kubernetes.io/warn": "invalid",
665+
"pod-security.kubernetes.io/warn-version": "invalid",
666+
},
667+
},
668+
}
669+
670+
expected_labels := map[string]string{
671+
"pod-security.kubernetes.io/enforce": "restricted",
672+
"pod-security.kubernetes.io/enforce-version": "v1.29",
673+
"pod-security.kubernetes.io/audit": "restricted",
674+
"pod-security.kubernetes.io/audit-version": "latest",
675+
"pod-security.kubernetes.io/warn": "restricted",
676+
"pod-security.kubernetes.io/warn-version": "latest",
677+
}
678+
679+
fakeClient := fake.NewFakeClient(util.NewClusterVersion("4.7.1"), newGitopsService())
680+
reconciler := newReconcileGitOpsService(fakeClient, s)
681+
682+
_, err := reconciler.Reconcile(context.TODO(), newRequest("test", "test"))
683+
assertNoError(t, err)
684+
685+
// Create a user defined namespace
686+
testNS := newRestrictedNamespace("test")
687+
err = fakeClient.Create(context.TODO(), testNS)
688+
assertNoError(t, err)
689+
690+
// Create an ArgoCD instance in the user defined namespace
691+
testArgoCD := &argoapp.ArgoCD{
692+
ObjectMeta: v1.ObjectMeta{
693+
Name: "test",
694+
Namespace: "test",
695+
},
696+
Spec: argoapp.ArgoCDSpec{},
697+
}
698+
err = fakeClient.Create(context.TODO(), testArgoCD)
699+
assertNoError(t, err)
700+
701+
_, err = reconciler.Reconcile(context.TODO(), newRequest("test", "test"))
702+
assertNoError(t, err)
703+
704+
// Check if PSS labels are addded to the user defined ns
705+
reconciled_ns := &corev1.Namespace{}
706+
err = fakeClient.Get(context.TODO(), types.NamespacedName{Name: "test"},
707+
reconciled_ns)
708+
assertNoError(t, err)
709+
710+
for label, _ := range reconciled_ns.ObjectMeta.Labels {
711+
_, found := expected_labels[label]
712+
// Fail if label is found
713+
assert.Check(t, found != true)
714+
}
715+
716+
for _, tc := range testCases {
717+
existing_ns := &corev1.Namespace{}
718+
assert.NilError(t, fakeClient.Get(context.TODO(), types.NamespacedName{Name: tc.namespace}, existing_ns), err)
719+
720+
// Assign new values, confirm the assignment and update the PSS labels
721+
existing_ns.ObjectMeta.Labels = tc.labels
722+
fakeClient.Update(context.TODO(), existing_ns)
723+
assert.NilError(t, fakeClient.Get(context.TODO(), types.NamespacedName{Name: tc.namespace}, existing_ns), err)
724+
assert.DeepEqual(t, existing_ns.ObjectMeta.Labels, tc.labels)
725+
726+
_, err := reconciler.Reconcile(context.TODO(), newRequest("test", "test"))
727+
assertNoError(t, err)
728+
729+
assert.NilError(t, fakeClient.Get(context.TODO(), types.NamespacedName{Name: tc.namespace}, reconciled_ns), err)
730+
731+
for key, value := range expected_labels {
732+
label, found := reconciled_ns.ObjectMeta.Labels[key]
733+
// Fail if label is not found, comapre the values with the expected values if found
734+
assert.Check(t, found)
735+
assert.Equal(t, label, value)
736+
}
737+
}
738+
}
739+
635740
func addKnownTypesToScheme(scheme *runtime.Scheme) {
636741
scheme.AddKnownTypes(configv1.GroupVersion, &configv1.ClusterVersion{})
637742
scheme.AddKnownTypes(pipelinesv1alpha1.GroupVersion, &pipelinesv1alpha1.GitopsService{})

0 commit comments

Comments
 (0)