Skip to content

Commit

Permalink
Fix: PSS label reset issue (#821)
Browse files Browse the repository at this point in the history 
* fix PSS label reset issue

Signed-off-by: Varsha B <vab@redhat.com>

* addressed review comments

Signed-off-by: Varsha B <vab@redhat.com>

* addressed review comment

Signed-off-by: Varsha B <vab@redhat.com>

---------

Signed-off-by: Varsha B <vab@redhat.com>
  • Loading branch information
@varshab1210
varshab1210 authored Jan 16, 2025
1 parent 4e9539c commit f652316
Show file tree
Hide file tree
Showing 2 changed files with 120 additions and 7 deletions.
20 changes: 14 additions & 6 deletions controllers/gitopsservice_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
"sigs.k8s.io/controller-runtime/pkg/event"
"sigs.k8s.io/controller-runtime/pkg/handler"
logf "sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/predicate"
"sigs.k8s.io/controller-runtime/pkg/reconcile"
Expand Down Expand Up @@ -106,6 +107,13 @@ func (r *ReconcileGitopsService) SetupWithManager(mgr ctrl.Manager) error {
Owns(&appsv1.Deployment{}, builder.WithPredicates(pred)).
Owns(&corev1.Service{}, builder.WithPredicates(pred)).
Owns(&routev1.Route{}, builder.WithPredicates(pred)).
Watches(
&corev1.Namespace{},
&handler.EnqueueRequestForObject{},
builder.WithPredicates(predicate.NewPredicateFuncs(func(obj client.Object) bool {
return obj.GetName() == "openshift-gitops"
})),
).
Complete(r)
}

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

// Create namespace if it doesn't already exist
namespaceRef := newRestrictedNamespace(namespace)
err = r.Client.Get(ctx, types.NamespacedName{Name: namespace}, &corev1.Namespace{})
err = r.Client.Get(ctx, types.NamespacedName{Name: namespace}, namespaceRef)
if err != nil {
if errors.IsNotFound(err) {
reqLogger.Info("Creating a new Namespace", "Name", namespace)
Expand All @@ -231,8 +239,8 @@ func (r *ReconcileGitopsService) Reconcile(ctx context.Context, request reconcil
return reconcile.Result{}, err
}
} else {
needUpdate, updateNameSpace := ensurePodSecurityLabels(namespaceRef)
if needUpdate {
needsUpdate, updateNameSpace := ensurePodSecurityLabels(namespaceRef)
if needsUpdate {
err = r.Client.Update(context.TODO(), updateNameSpace)
if err != nil {
return reconcile.Result{}, err
Expand Down Expand Up @@ -345,7 +353,7 @@ func (r *ReconcileGitopsService) reconcileDefaultArgoCDInstance(instance *pipeli
// 4.6 Cluster: Backend in openshift-pipelines-app-delivery namespace and argocd in openshift-gitops namespace
// 4.7 Cluster: Both backend and argocd instance in openshift-gitops namespace
argocdNS := newRestrictedNamespace(defaultArgoCDInstance.Namespace)
err = r.Client.Get(context.TODO(), types.NamespacedName{Name: argocdNS.Name}, &corev1.Namespace{})
err = r.Client.Get(context.TODO(), types.NamespacedName{Name: argocdNS.Name}, argocdNS)
if err != nil {
if errors.IsNotFound(err) {
reqLogger.Info("Creating a new Namespace", "Name", argocdNS.Name)
Expand Down Expand Up @@ -378,8 +386,8 @@ func (r *ReconcileGitopsService) reconcileDefaultArgoCDInstance(instance *pipeli
}
}

needUpdate, updateNameSpace := ensurePodSecurityLabels(argocdNS)
if needUpdate {
needsUpdate, updateNameSpace := ensurePodSecurityLabels(argocdNS)
if needsUpdate {
err = r.Client.Update(context.TODO(), updateNameSpace)
if err != nil {
return reconcile.Result{}, err
Expand Down
107 changes: 106 additions & 1 deletion controllers/gitopsservice_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -542,7 +542,8 @@ func TestReconcile_VerifyResourceQuotaDeletionForUpgrade(t *testing.T) {
// Create namespace object for default ArgoCD instance and set resource quota to it.
defaultArgoNS := &corev1.Namespace{
ObjectMeta: v1.ObjectMeta{
Name: serviceNamespace,
Name: serviceNamespace,
Namespace: serviceNamespace,
},
}
fakeClient.Create(context.TODO(), defaultArgoNS)
Expand Down Expand Up @@ -632,6 +633,110 @@ func TestReconcile_InfrastructureNode(t *testing.T) {

}

func TestReconcile_PSSLabels(t *testing.T) {
logf.SetLogger(argocd.ZapLogger(true))
s := scheme.Scheme
addKnownTypesToScheme(s)

testCases := []struct {
name string
namespace string
labels map[string]string
}{
{
name: "modified valid PSS labels for openshift-gitops ns",
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",
},
},
{
name: "modified invalid and empty PSS labels for openshift-gitops ns",
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",
},
},
}

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())
reconciler := newReconcileGitOpsService(fakeClient, s)

_, err := reconciler.Reconcile(context.TODO(), newRequest("test", "test"))
assertNoError(t, err)

// Create a user defined namespace
testNS := newRestrictedNamespace("test")
err = fakeClient.Create(context.TODO(), testNS)
assertNoError(t, err)

// Create an ArgoCD instance in the user defined namespace
testArgoCD := &argoapp.ArgoCD{
ObjectMeta: v1.ObjectMeta{
Name: "test",
Namespace: "test",
},
Spec: argoapp.ArgoCDSpec{},
}
err = fakeClient.Create(context.TODO(), testArgoCD)
assertNoError(t, err)

_, 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.ObjectMeta.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.ObjectMeta.Labels = tc.labels
fakeClient.Update(context.TODO(), existing_ns)
assert.NilError(t, fakeClient.Get(context.TODO(), types.NamespacedName{Name: tc.namespace}, existing_ns), err)
assert.DeepEqual(t, existing_ns.ObjectMeta.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.ObjectMeta.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)
}
}
}

func addKnownTypesToScheme(scheme *runtime.Scheme) {
scheme.AddKnownTypes(configv1.GroupVersion, &configv1.ClusterVersion{})
scheme.AddKnownTypes(pipelinesv1alpha1.GroupVersion, &pipelinesv1alpha1.GitopsService{})
Expand Down

0 comments on commit f652316

Please sign in to comment.