diff --git a/README.md b/README.md index 157d6c3..d7bb22d 100644 --- a/README.md +++ b/README.md @@ -67,6 +67,61 @@ spec: - user2 ``` +### Deletion and Lifecycle Management + +The operator implements deletion prevention to ensure safe removal of ArgoCD resources: + +**Finalizers**: When an `ArgocdUser` is created, the operator automatically adds a finalizer (`argocd.snappcloud.io/finalizer`) to prevent accidental deletion. + +**Deletion Protection**: An `ArgocdUser` cannot be deleted if any namespace still references it via the `argocd.snappcloud.io/appproj` label. This includes: + +- Single-team namespaces (e.g., `argocd.snappcloud.io/appproj: team-a`) +- Multi-team namespaces (e.g., `argocd.snappcloud.io/appproj: team-a.team-b`) + +**Deletion Process**: + +1. When you delete an `ArgocdUser`, it enters a pending deletion state +2. The operator checks if any namespaces reference this team +3. If namespaces still reference it, deletion is blocked and the resource remains +4. Once all namespace labels are removed, the operator: + - Cleans up RBAC policies from `argocd-rbac-cm` + - Removes static accounts from `argocd-cm` + - Deletes account passwords from `argocd-secret` + - Removes the finalizer + - Allows Kubernetes to complete the deletion + +**Garbage Collection**: The operator uses OwnerReferences to enable automatic cleanup: + +- When an `ArgocdUser` is deleted, its associated `AppProject` is automatically removed +- OpenShift Groups (if used) are also automatically cleaned up +- This ensures no orphaned resources remain in the cluster + +### Architecture + +The operator uses two separate controllers with distinct responsibilities: + +**ArgocdUserReconciler**: + +- Creates and manages `AppProject` resources +- Configures RBAC policies and roles in `argocd-rbac-cm` +- Creates static accounts in `argocd-cm` +- Manages account passwords in `argocd-secret` +- Creates OpenShift Groups for RBAC integration +- Sets OwnerReferences for garbage collection +- Manages finalizers for safe deletion + +**NamespaceReconciler**: + +- Watches namespace labels (`argocd.snappcloud.io/appproj` and `argocd.snappcloud.io/source`) +- Updates `AppProject` destinations based on namespace labels +- Updates `AppProject` source namespaces +- Supports multi-team namespaces (e.g., `team-a.team-b`) +- **Does not create** `AppProject` resources (only updates existing ones) + +**Separation of Concerns**: The NamespaceReconciler only updates `AppProject` destinations and sources. It validates that the `AppProject` exists (created by ArgocdUserReconciler) before attempting updates. If an `AppProject` doesn't exist, the reconciliation fails with an error, ensuring users create the `ArgocdUser` resource first. + +**Multi-Team Support**: When a namespace has a multi-team label (e.g., `argocd.snappcloud.io/appproj: team-a.team-b`), both teams' `AppProjects` will include that namespace in their destinations, enabling shared access. + ## Instructions ### Development diff --git a/config/dependency/group-crd.yaml b/config/dependency/group-crd.yaml new file mode 100644 index 0000000..b993994 --- /dev/null +++ b/config/dependency/group-crd.yaml @@ -0,0 +1,30 @@ +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: groups.user.openshift.io +spec: + group: user.openshift.io + names: + kind: Group + listKind: GroupList + plural: groups + singular: group + scope: Cluster + versions: + - name: v1 + served: true + storage: true + schema: + openAPIV3Schema: + type: object + properties: + apiVersion: + type: string + kind: + type: string + metadata: + type: object + users: + type: array + items: + type: string diff --git a/config/manager/manager.yaml b/config/manager/manager.yaml index db42de6..953c51b 100644 --- a/config/manager/manager.yaml +++ b/config/manager/manager.yaml @@ -24,7 +24,7 @@ spec: - /manager args: - --leader-elect - image: "ghcr.io/snapp-incubator/argocd-complementary-operator:0.4.1" + image: "ghcr.io/snapp-incubator/argocd-complementary-operator:0.4.2" name: manager env: - name: GODEBUG diff --git a/internal/controller/argocduser_controller.go b/internal/controller/argocduser_controller.go index 4bd88f0..97bad18 100644 --- a/internal/controller/argocduser_controller.go +++ b/internal/controller/argocduser_controller.go @@ -22,6 +22,7 @@ import ( "encoding/json" "strings" + argov1alpha1 "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" userv1 "github.com/openshift/api/user/v1" argocduserv1alpha1 "github.com/snapp-incubator/argocd-complementary-operator/api/v1alpha1" "golang.org/x/crypto/bcrypt" @@ -30,8 +31,10 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" + "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/log" ) @@ -40,6 +43,7 @@ const ( userArgocdRbacPolicyCM = "argocd-rbac-cm" userArgocdStaticUserCM = "argocd-cm" userArgocdSecret = "argocd-secret" + argocdUserFinalizer = "argocd.snappcloud.io/finalizer" ) // ArgocdUserReconciler reconciles a ArgocdUser object @@ -53,7 +57,9 @@ type ArgocdUserReconciler struct { //+kubebuilder:rbac:groups=argocd.snappcloud.io,resources=argocdusers/finalizers,verbs=update //+kubebuilder:rbac:groups="",resources=configmaps,verbs=get;list;watch;create;update;patch;delete //+kubebuilder:rbac:groups="",resources=secrets,verbs=get;list;watch;create;update;patch;delete +//+kubebuilder:rbac:groups="",resources=namespaces,verbs=get;list;watch //+kubebuilder:rbac:groups=user.openshift.io,resources=*,verbs=get;list;watch;create;update;patch;delete +//+kubebuilder:rbac:groups=argoproj.io,resources=appprojects,verbs=get;list;watch;create;update;patch;delete // Reconcile is part of the main kubernetes reconciliation loop which aims to // move the current state of the clus k8s.io/api closer to the desired state. @@ -69,10 +75,62 @@ func (r *ArgocdUserReconciler) Reconcile(ctx context.Context, req ctrl.Request) argocduser := &argocduserv1alpha1.ArgocdUser{} err := r.Get(context.TODO(), req.NamespacedName, argocduser) if err != nil { + if errors.IsNotFound(err) { + log.Info("ArgocdUser resource not found, might be deleted") + return ctrl.Result{}, nil + } log.Error(err, "Failed to get argocduser") return ctrl.Result{}, err } + // Handle deletion + if argocduser.DeletionTimestamp != nil { + if controllerutil.ContainsFinalizer(argocduser, argocdUserFinalizer) { + log.Info("ArgocdUser is being deleted, checking for active namespaces") + + // Check if there are namespaces still referencing this ArgocdUser + hasActive, err := r.hasActiveNamespaces(ctx, argocduser.Name) + if err != nil { + log.Error(err, "Failed to check for active namespaces") + return ctrl.Result{}, err + } + + if hasActive { + log.Info("Cannot delete ArgocdUser: namespaces still reference it", "argocduser", argocduser.Name) + // Requeue to check again later + return ctrl.Result{Requeue: true}, nil + } + + // No active namespaces, proceed with cleanup + log.Info("No active namespaces found, proceeding with cleanup") + if err := r.cleanupArgocdUserResources(ctx, argocduser); err != nil { + log.Error(err, "Failed to cleanup ArgocdUser resources") + return ctrl.Result{}, err + } + + // Remove finalizer + controllerutil.RemoveFinalizer(argocduser, argocdUserFinalizer) + if err := r.Update(ctx, argocduser); err != nil { + log.Error(err, "Failed to remove finalizer") + return ctrl.Result{}, err + } + log.Info("Successfully removed finalizer and cleaned up resources") + } + return ctrl.Result{}, nil + } + + // Add finalizer if not present + if !controllerutil.ContainsFinalizer(argocduser, argocdUserFinalizer) { + log.Info("Adding finalizer to ArgocdUser") + controllerutil.AddFinalizer(argocduser, argocdUserFinalizer) + if err := r.Update(ctx, argocduser); err != nil { + log.Error(err, "Failed to add finalizer") + return ctrl.Result{}, err + } + log.Info("Successfully added finalizer") + return ctrl.Result{}, nil + } + if err := r.createArgocdStaticUser(ctx, req, argocduser, "admin", argocduser.Spec.Admin.CIPass, argocduser.Spec.Admin.Users); err != nil { log.Error(err, "Failed create argocd static user admin") return ctrl.Result{}, err @@ -89,6 +147,29 @@ func (r *ArgocdUserReconciler) Reconcile(ctx context.Context, req ctrl.Request) return ctrl.Result{}, err } + // Create or update AppProject with RBAC configuration + if err := r.ReconcileAppProject(ctx, argocduser); err != nil { + log.Error(err, "Failed to reconcile AppProject") + return ctrl.Result{}, err + } + + // Manage OpenShift groups for admin users (both admin and view groups) + if err := r.AddArgoUsersToGroup(ctx, argocduser, "admin", argocduser.Spec.Admin.Users); err != nil { + log.Error(err, "Failed to add admin users to admin group") + return ctrl.Result{}, err + } + // Admin users also need to be in view group for role aggregation + if err := r.AddArgoUsersToGroup(ctx, argocduser, "view", argocduser.Spec.Admin.Users); err != nil { + log.Error(err, "Failed to add admin users to view group") + return ctrl.Result{}, err + } + + // Manage OpenShift groups for view users (only view group) + if err := r.AddArgoUsersToGroup(ctx, argocduser, "view", argocduser.Spec.View.Users); err != nil { + log.Error(err, "Failed to add view users to view group") + return ctrl.Result{}, err + } + return ctrl.Result{}, nil } @@ -120,6 +201,9 @@ func (r *ArgocdUserReconciler) UpdateUserArgocdConfig(ctx context.Context, argoc return err } patch := client.MergeFrom(configMap.DeepCopy()) + if configMap.Data == nil { + configMap.Data = make(map[string]string) + } configMap.Data["accounts."+argocduser.Name+"-"+roleName+"-ci"] = "apiKey,login" err = r.Patch(ctx, configMap, patch) if err != nil { @@ -161,22 +245,121 @@ func (r *ArgocdUserReconciler) AddArgoUsersToGroup(ctx context.Context, argocdus group = &userv1.Group{ ObjectMeta: metav1.ObjectMeta{ Name: groupName, + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "argocd.snappcloud.io/v1alpha1", + Kind: "ArgocdUser", + Name: argocduser.Name, + UID: argocduser.UID, + Controller: ptr.To(true), + BlockOwnerDeletion: ptr.To(true), + }, + }, }, + Users: argoUsers, } err = r.Create(ctx, group) if err != nil { log.Error(err, "Failed to create group") return err } + log.Info("Successfully created group with users") + return nil + } + return err + } + + // Merge users: keep existing users and add new ones + existingUsers := make(map[string]bool) + for _, user := range group.Users { + existingUsers[user] = true + } + for _, user := range argoUsers { + if !existingUsers[user] { + group.Users = append(group.Users, user) } } - group.Users = argoUsers + err = r.Update(ctx, group) if err != nil { log.Error(err, "Failed to update group") return err } - log.Info("Successfully added users to group") + log.Info("Successfully updated group with users") + return nil +} + +// ReconcileAppProject creates or updates the AppProject with RBAC configuration +func (r *ArgocdUserReconciler) ReconcileAppProject(ctx context.Context, argocduser *argocduserv1alpha1.ArgocdUser) error { + log := log.FromContext(ctx) + teamName := argocduser.Name + + // Create the desired AppProject + appProj := &argov1alpha1.AppProject{ + ObjectMeta: metav1.ObjectMeta{ + Name: teamName, + Namespace: userArgocdNS, + }, + Spec: argov1alpha1.AppProjectSpec{ + // Note: Destinations, SourceNamespaces, and SourceRepos are managed by NamespaceReconciler + Destinations: []argov1alpha1.ApplicationDestination{}, + SourceNamespaces: []string{}, + SourceRepos: []string{}, + Roles: []argov1alpha1.ProjectRole{ + { + Groups: []string{teamName + "-admin", teamName + "-admin-ci"}, + Name: teamName + "-admin", + Policies: []string{ + "p, proj:" + teamName + ":" + teamName + "-admin, applications, *, " + teamName + "/*, allow", + "p, proj:" + teamName + ":" + teamName + "-admin, repositories, *, " + teamName + "/*, allow", + "p, proj:" + teamName + ":" + teamName + "-admin, exec, create, " + teamName + "/*, allow", + }, + }, + { + // View role includes both admin and view groups for role aggregation + Groups: []string{teamName + "-admin", teamName + "-admin-ci", teamName + "-view", teamName + "-view-ci"}, + Name: teamName + "-view", + Policies: []string{ + "p, proj:" + teamName + ":" + teamName + "-view, applications, get, " + teamName + "/*, allow", + "p, proj:" + teamName + ":" + teamName + "-view, repositories, get, " + teamName + "/*, allow", + "p, proj:" + teamName + ":" + teamName + "-view, logs, get, " + teamName + "/*, allow", + }, + }, + }, + }, + } + + // Set OwnerReference to enable garbage collection + if err := controllerutil.SetControllerReference(argocduser, appProj, r.Scheme); err != nil { + log.Error(err, "Failed to set OwnerReference on AppProject") + return err + } + + // Check if AppProject already exists + found := &argov1alpha1.AppProject{} + err := r.Get(ctx, types.NamespacedName{Name: teamName, Namespace: userArgocdNS}, found) + if err != nil { + if errors.IsNotFound(err) { + log.Info("Creating new AppProject", "name", teamName) + if err := r.Create(ctx, appProj); err != nil { + log.Error(err, "Failed to create AppProject") + return err + } + log.Info("Successfully created AppProject") + return nil + } + log.Error(err, "Failed to get AppProject") + return err + } + + // Update existing AppProject - only update Roles, preserve Destinations, SourceNamespaces, and SourceRepos + // These fields are managed by NamespaceReconciler + found.Spec.Roles = appProj.Spec.Roles + if err := r.Update(ctx, found); err != nil { + log.Error(err, "Failed to update AppProject") + return err + } + log.Info("Successfully updated AppProject RBAC") return nil } @@ -191,21 +374,11 @@ func (r *ArgocdUserReconciler) AddArgocdRBACPolicy(ctx context.Context, argocdus // TODO: Enhance this, for example adding view roles to the admin not works! policies := []string{ - "g, " + argocduser.Name + "-admin-ci, role:" + argocduser.Name + "-admin", - "g, " + argocduser.Name + "-admin-ci, role:" + argocduser.Name + "-view", + "p, role:common, clusters, get, *, allow", "g, " + argocduser.Name + "-admin-ci, role:common", "g, " + argocduser.Name + "-view-ci, role:common", - "g, " + argocduser.Name + "-view-ci, role:" + argocduser.Name + "-view", - "g, " + argocduser.Name + "-admin, role:" + argocduser.Name + "-admin", - "g, " + argocduser.Name + "-admin, role:" + argocduser.Name + "-view", "g, " + argocduser.Name + "-admin, role:common", "g, " + argocduser.Name + "-view, role:common", - "g, " + argocduser.Name + "-view, role:" + argocduser.Name + "-view", - "p, role:" + argocduser.Name + "-admin, repositories, create, " + argocduser.Name + "/*, allow", - "p, role:" + argocduser.Name + "-admin, repositories, delete, " + argocduser.Name + "/*, allow", - "p, role:" + argocduser.Name + "-admin, repositories, update, " + argocduser.Name + "/*, allow", - "p, role:" + argocduser.Name + "-view, repositories, get, " + argocduser.Name + "/*, allow", - "p, role:" + argocduser.Name + "-view, applications, get, " + argocduser.Name + "/*, allow", } // add argocd rbac policy @@ -238,6 +411,114 @@ func HashPassword(password string) (string, error) { return string(bytes), err } +// hasActiveNamespaces checks if any namespaces have labels referencing the ArgocdUser +func (r *ArgocdUserReconciler) hasActiveNamespaces(ctx context.Context, argocdUserName string) (bool, error) { + log := log.FromContext(ctx) + + // List all namespaces + namespaceList := &corev1.NamespaceList{} + if err := r.List(ctx, namespaceList); err != nil { + log.Error(err, "Failed to list namespaces") + return false, err + } + + // Check each namespace for labels referencing this ArgocdUser + for _, ns := range namespaceList.Items { + if appProjLabel, exists := ns.Labels["argocd.snappcloud.io/appproj"]; exists { + // Parse multi-team labels (e.g., "team-a.team-b") + teams := strings.Split(appProjLabel, ".") + for _, team := range teams { + if team == argocdUserName { + log.Info("Found namespace referencing ArgocdUser", "namespace", ns.Name, "label", appProjLabel) + return true, nil + } + } + } + } + + log.Info("No namespaces found referencing ArgocdUser", "argocduser", argocdUserName) + return false, nil +} + +// cleanupArgocdUserResources removes RBAC policies and static accounts for the ArgocdUser +func (r *ArgocdUserReconciler) cleanupArgocdUserResources(ctx context.Context, argocduser *argocduserv1alpha1.ArgocdUser) error { + log := log.FromContext(ctx) + + teamName := argocduser.Name + + // Remove RBAC policies from argocd-rbac-cm + log.Info("Removing RBAC policies from argocd-rbac-cm") + rbacConfigMap := &corev1.ConfigMap{} + if err := r.Get(ctx, types.NamespacedName{Name: userArgocdRbacPolicyCM, Namespace: userArgocdNS}, rbacConfigMap); err != nil { + if !errors.IsNotFound(err) { + log.Error(err, "Failed to get argocd-rbac-cm") + return err + } + log.Info("argocd-rbac-cm not found, skipping RBAC cleanup") + } else { + // Remove all policies related to this team + policyCsv := rbacConfigMap.Data["policy.csv"] + lines := strings.Split(policyCsv, "\n") + var newLines []string + for _, line := range lines { + // Keep lines that don't reference this team + if !strings.Contains(line, teamName+"-admin-ci") && + !strings.Contains(line, teamName+"-view-ci") && + !strings.Contains(line, teamName+"-admin") && + !strings.Contains(line, teamName+"-view") { + newLines = append(newLines, line) + } + } + rbacConfigMap.Data["policy.csv"] = strings.Join(newLines, "\n") + if err := r.Update(ctx, rbacConfigMap); err != nil { + log.Error(err, "Failed to update argocd-rbac-cm") + return err + } + log.Info("Successfully removed RBAC policies") + } + + // Remove static accounts from argocd-cm + log.Info("Removing static accounts from argocd-cm") + argocdConfigMap := &corev1.ConfigMap{} + if err := r.Get(ctx, types.NamespacedName{Name: userArgocdStaticUserCM, Namespace: userArgocdNS}, argocdConfigMap); err != nil { + if !errors.IsNotFound(err) { + log.Error(err, "Failed to get argocd-cm") + return err + } + log.Info("argocd-cm not found, skipping account cleanup") + } else { + delete(argocdConfigMap.Data, "accounts."+teamName+"-admin-ci") + delete(argocdConfigMap.Data, "accounts."+teamName+"-view-ci") + if err := r.Update(ctx, argocdConfigMap); err != nil { + log.Error(err, "Failed to update argocd-cm") + return err + } + log.Info("Successfully removed static accounts") + } + + // Remove passwords from argocd-secret + log.Info("Removing passwords from argocd-secret") + argocdSecret := &corev1.Secret{} + if err := r.Get(ctx, types.NamespacedName{Name: userArgocdSecret, Namespace: userArgocdNS}, argocdSecret); err != nil { + if !errors.IsNotFound(err) { + log.Error(err, "Failed to get argocd-secret") + return err + } + log.Info("argocd-secret not found, skipping password cleanup") + } else { + delete(argocdSecret.Data, "accounts."+teamName+"-admin-ci.password") + delete(argocdSecret.Data, "accounts."+teamName+"-view-ci.password") + if err := r.Update(ctx, argocdSecret); err != nil { + log.Error(err, "Failed to update argocd-secret") + return err + } + log.Info("Successfully removed passwords") + } + + log.Info("Successfully cleaned up all ArgocdUser resources") + return nil +} + // SetupWithManager sets up the controller with the Manager. func (r *ArgocdUserReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). diff --git a/internal/controller/argocduser_controller_test.go b/internal/controller/argocduser_controller_test.go new file mode 100644 index 0000000..671c671 --- /dev/null +++ b/internal/controller/argocduser_controller_test.go @@ -0,0 +1,943 @@ +/* +Copyright 2022. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controller_test + +import ( + "context" + "strings" + "time" + + argov1alpha1 "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + userv1 "github.com/openshift/api/user/v1" + argocduserv1alpha1 "github.com/snapp-incubator/argocd-complementary-operator/api/v1alpha1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" +) + +var _ = Describe("ArgocdUser controller RBAC policy generation", Ordered, func() { + const ( + timeout = time.Second * 20 + interval = time.Millisecond * 30 + ) + + ctx := context.Background() + + BeforeAll(func() { + By("Ensuring user-argocd namespace exists") + userArgocdNS := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "user-argocd", + }, + } + err := k8sClient.Create(ctx, userArgocdNS) + if err != nil { + // Namespace might already exist + err = k8sClient.Get(ctx, types.NamespacedName{Name: "user-argocd"}, userArgocdNS) + Expect(err).NotTo(HaveOccurred()) + } + }) + + Context("When creating an ArgocdUser resource", func() { + It("Should generate correct RBAC policies in ConfigMap", func() { + By("Creating the argocd-rbac-cm ConfigMap if it doesn't exist") + rbacConfigMap := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "argocd-rbac-cm", + Namespace: "user-argocd", + }, + Data: map[string]string{ + "policy.csv": "", + }, + } + err := k8sClient.Create(ctx, rbacConfigMap) + if err != nil { + // ConfigMap might already exist from previous tests + err = k8sClient.Get(ctx, types.NamespacedName{ + Name: "argocd-rbac-cm", + Namespace: "user-argocd", + }, rbacConfigMap) + Expect(err).NotTo(HaveOccurred()) + } + + By("Creating the argocd-cm ConfigMap if it doesn't exist") + argocdConfigMap := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "argocd-cm", + Namespace: "user-argocd", + }, + Data: map[string]string{}, + } + err = k8sClient.Create(ctx, argocdConfigMap) + if err != nil { + err = k8sClient.Get(ctx, types.NamespacedName{ + Name: "argocd-cm", + Namespace: "user-argocd", + }, argocdConfigMap) + Expect(err).NotTo(HaveOccurred()) + } + + By("Creating the argocd-secret Secret if it doesn't exist") + argocdSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "argocd-secret", + Namespace: "user-argocd", + }, + Data: map[string][]byte{}, + } + err = k8sClient.Create(ctx, argocdSecret) + if err != nil { + err = k8sClient.Get(ctx, types.NamespacedName{ + Name: "argocd-secret", + Namespace: "user-argocd", + }, argocdSecret) + Expect(err).NotTo(HaveOccurred()) + } + + By("Creating an ArgocdUser resource") + argocdUser := &argocduserv1alpha1.ArgocdUser{ + ObjectMeta: metav1.ObjectMeta{ + Name: "rbac-gen-test", + }, + Spec: argocduserv1alpha1.ArgocdUserSpec{ + Admin: argocduserv1alpha1.ArgocdCIAdmin{ + CIPass: "admin-password", + Users: []string{"admin-user1", "admin-user2"}, + }, + View: argocduserv1alpha1.ArgocdCIView{ + CIPass: "view-password", + Users: []string{"view-user1"}, + }, + }, + } + Expect(k8sClient.Create(ctx, argocdUser)).Should(Succeed()) + + By("Verifying the ArgocdUser was created") + argocdUserLookup := types.NamespacedName{Name: "rbac-gen-test"} + Expect(k8sClient.Get(ctx, argocdUserLookup, argocdUser)).Should(Succeed()) + + By("Waiting for RBAC policies to be added to ConfigMap") + updatedConfigMap := &corev1.ConfigMap{} + configMapLookup := types.NamespacedName{ + Name: "argocd-rbac-cm", + Namespace: "user-argocd", + } + + Eventually(func() bool { + err := k8sClient.Get(ctx, configMapLookup, updatedConfigMap) + if err != nil { + return false + } + // Check if at least one expected policy exists + return strings.Contains(updatedConfigMap.Data["policy.csv"], "rbac-gen-test-admin-ci") + }, timeout, interval).Should(BeTrue()) + + By("Verifying global common role definition is present") + policyCsv := updatedConfigMap.Data["policy.csv"] + + // Common role definition - allows all users to get clusters + Expect(policyCsv).To(ContainSubstring("p, role:common, clusters, get, *, allow"), + "Should define common role with clusters get permission") + + By("Verifying group bindings to common role are present") + // All groups should be bound to common role + Expect(policyCsv).To(ContainSubstring("g, rbac-gen-test-admin-ci, role:common"), + "Should assign admin-ci to common role") + Expect(policyCsv).To(ContainSubstring("g, rbac-gen-test-view-ci, role:common"), + "Should assign view-ci to common role") + Expect(policyCsv).To(ContainSubstring("g, rbac-gen-test-admin, role:common"), + "Should assign admin to common role") + Expect(policyCsv).To(ContainSubstring("g, rbac-gen-test-view, role:common"), + "Should assign view to common role") + + By("Verifying fine-grained policies are NOT in policy.csv (they belong in AppProject)") + // Fine-grained policies should NOT be in global config + Expect(policyCsv).NotTo(ContainSubstring("role:rbac-gen-test-admin, repositories"), + "Repository policies should be in AppProject, not global config") + Expect(policyCsv).NotTo(ContainSubstring("role:rbac-gen-test-view, applications"), + "Application policies should be in AppProject, not global config") + }) + + It("Should create static users in argocd-cm ConfigMap", func() { + By("Verifying admin-ci and view-ci accounts are created") + configMap := &corev1.ConfigMap{} + configMapLookup := types.NamespacedName{ + Name: "argocd-cm", + Namespace: "user-argocd", + } + + Eventually(func() bool { + err := k8sClient.Get(ctx, configMapLookup, configMap) + if err != nil { + return false + } + // Check if accounts are created + _, adminExists := configMap.Data["accounts.rbac-gen-test-admin-ci"] + _, viewExists := configMap.Data["accounts.rbac-gen-test-view-ci"] + return adminExists && viewExists + }, timeout, interval).Should(BeTrue()) + + // Verify the account capabilities + Expect(configMap.Data["accounts.rbac-gen-test-admin-ci"]).To(Equal("apiKey,login")) + Expect(configMap.Data["accounts.rbac-gen-test-view-ci"]).To(Equal("apiKey,login")) + }) + }) + + Context("When updating an ArgocdUser resource", func() { + It("Should not duplicate RBAC policies on reconciliation", func() { + By("Getting the existing ArgocdUser") + argocdUser := &argocduserv1alpha1.ArgocdUser{} + argocdUserLookup := types.NamespacedName{Name: "rbac-gen-test"} + Expect(k8sClient.Get(ctx, argocdUserLookup, argocdUser)).Should(Succeed()) + + By("Updating the ArgocdUser spec") + argocdUser.Spec.Admin.Users = []string{"admin-user1", "admin-user2", "admin-user3"} + Expect(k8sClient.Update(ctx, argocdUser)).Should(Succeed()) + + By("Waiting for reconciliation") + time.Sleep(2 * time.Second) + + By("Verifying policies are not duplicated") + configMap := &corev1.ConfigMap{} + configMapLookup := types.NamespacedName{ + Name: "argocd-rbac-cm", + Namespace: "user-argocd", + } + Expect(k8sClient.Get(ctx, configMapLookup, configMap)).Should(Succeed()) + + policyCsv := configMap.Data["policy.csv"] + + // Count occurrences of a specific policy to ensure no duplicates + testPolicy := "g, rbac-gen-test-admin-ci, role:common" + occurrences := strings.Count(policyCsv, testPolicy) + Expect(occurrences).To(Equal(1), "Policy should appear exactly once, not be duplicated") + + // Also verify the common role definition appears only once + commonRolePolicy := "p, role:common, clusters, get, *, allow" + commonRoleOccurrences := strings.Count(policyCsv, commonRolePolicy) + Expect(commonRoleOccurrences).To(Equal(1), "Common role definition should appear exactly once") + }) + }) + + Context("When creating an ArgocdUser with AppProject management", func() { + It("Should create AppProject with correct RBAC roles", func() { + By("Creating a new ArgocdUser") + newUser := &argocduserv1alpha1.ArgocdUser{ + ObjectMeta: metav1.ObjectMeta{ + Name: "project-team", + }, + Spec: argocduserv1alpha1.ArgocdUserSpec{ + Admin: argocduserv1alpha1.ArgocdCIAdmin{ + CIPass: "admin-pass", + Users: []string{"admin1"}, + }, + View: argocduserv1alpha1.ArgocdCIView{ + CIPass: "view-pass", + Users: []string{"viewer1"}, + }, + }, + } + Expect(k8sClient.Create(ctx, newUser)).Should(Succeed()) + + By("Waiting for AppProject to be created") + appProj := &argov1alpha1.AppProject{} + appProjLookup := types.NamespacedName{Name: "project-team", Namespace: "user-argocd"} + Eventually(func() bool { + err := k8sClient.Get(ctx, appProjLookup, appProj) + return err == nil + }, timeout, interval).Should(BeTrue()) + + By("Verifying AppProject has correct admin role") + adminRoleFound := false + for _, role := range appProj.Spec.Roles { + if role.Name == "project-team-admin" { + adminRoleFound = true + Expect(role.Groups).To(ContainElement("project-team-admin")) + Expect(role.Groups).To(ContainElement("project-team-admin-ci")) + Expect(role.Groups).To(HaveLen(2), "Admin role should only have admin groups") + + // Verify admin policies + Expect(role.Policies).To(ContainElement(ContainSubstring("applications, *, project-team/*, allow"))) + Expect(role.Policies).To(ContainElement(ContainSubstring("repositories, *, project-team/*, allow"))) + Expect(role.Policies).To(ContainElement(ContainSubstring("exec, create, project-team/*, allow"))) + } + } + Expect(adminRoleFound).To(BeTrue(), "Admin role should exist in AppProject") + + By("Verifying AppProject has correct view role with role aggregation") + viewRoleFound := false + for _, role := range appProj.Spec.Roles { + if role.Name == "project-team-view" { + viewRoleFound = true + // View role should include ALL groups for role aggregation + Expect(role.Groups).To(ContainElement("project-team-view")) + Expect(role.Groups).To(ContainElement("project-team-view-ci")) + Expect(role.Groups).To(ContainElement("project-team-admin")) + Expect(role.Groups).To(ContainElement("project-team-admin-ci")) + Expect(role.Groups).To(HaveLen(4), "View role should have all groups for role aggregation") + + // Verify view policies + Expect(role.Policies).To(ContainElement(ContainSubstring("applications, get, project-team/*, allow"))) + Expect(role.Policies).To(ContainElement(ContainSubstring("repositories, get, project-team/*, allow"))) + Expect(role.Policies).To(ContainElement(ContainSubstring("logs, get, project-team/*, allow"))) + } + } + Expect(viewRoleFound).To(BeTrue(), "View role should exist in AppProject") + }) + }) + + Context("When managing OpenShift Groups", func() { + It("Should add admin users to both admin and view groups", func() { + By("Verifying admin group contains admin users") + adminGroup := &userv1.Group{} + adminGroupLookup := types.NamespacedName{Name: "project-team-admin"} + Eventually(func() bool { + err := k8sClient.Get(ctx, adminGroupLookup, adminGroup) + return err == nil && len(adminGroup.Users) > 0 + }, timeout, interval).Should(BeTrue()) + + Expect(adminGroup.Users).To(ContainElement("admin1"), + "Admin group should contain admin user") + + By("Verifying view group contains admin users for role aggregation") + viewGroup := &userv1.Group{} + viewGroupLookup := types.NamespacedName{Name: "project-team-view"} + Eventually(func() bool { + err := k8sClient.Get(ctx, viewGroupLookup, viewGroup) + return err == nil && len(viewGroup.Users) > 0 + }, timeout, interval).Should(BeTrue()) + + Expect(viewGroup.Users).To(ContainElement("admin1"), + "View group should contain admin user for role aggregation") + Expect(viewGroup.Users).To(ContainElement("viewer1"), + "View group should contain view user") + }) + + It("Should only add view users to view group", func() { + By("Verifying admin group does NOT contain view users") + adminGroup := &userv1.Group{} + adminGroupLookup := types.NamespacedName{Name: "project-team-admin"} + Expect(k8sClient.Get(ctx, adminGroupLookup, adminGroup)).Should(Succeed()) + + Expect(adminGroup.Users).NotTo(ContainElement("viewer1"), + "Admin group should NOT contain view-only users") + + By("Verifying view group contains view users") + viewGroup := &userv1.Group{} + viewGroupLookup := types.NamespacedName{Name: "project-team-view"} + Expect(k8sClient.Get(ctx, viewGroupLookup, viewGroup)).Should(Succeed()) + + Expect(viewGroup.Users).To(ContainElement("viewer1"), + "View group should contain view user") + }) + + It("Should update groups when users are added", func() { + By("Getting the existing ArgocdUser") + argocdUser := &argocduserv1alpha1.ArgocdUser{} + argocdUserLookup := types.NamespacedName{Name: "project-team"} + Expect(k8sClient.Get(ctx, argocdUserLookup, argocdUser)).Should(Succeed()) + + By("Adding new users to the spec") + argocdUser.Spec.Admin.Users = []string{"admin1", "admin2"} + argocdUser.Spec.View.Users = []string{"viewer1", "viewer2"} + Expect(k8sClient.Update(ctx, argocdUser)).Should(Succeed()) + + By("Waiting for admin group to be updated") + adminGroup := &userv1.Group{} + adminGroupLookup := types.NamespacedName{Name: "project-team-admin"} + Eventually(func() bool { + err := k8sClient.Get(ctx, adminGroupLookup, adminGroup) + if err != nil { + return false + } + return len(adminGroup.Users) >= 2 && containsUser(adminGroup.Users, "admin2") + }, timeout, interval).Should(BeTrue()) + + Expect(adminGroup.Users).To(ContainElement("admin1")) + Expect(adminGroup.Users).To(ContainElement("admin2")) + + By("Verifying both new admin users are in view group") + viewGroup := &userv1.Group{} + viewGroupLookup := types.NamespacedName{Name: "project-team-view"} + Eventually(func() bool { + err := k8sClient.Get(ctx, viewGroupLookup, viewGroup) + if err != nil { + return false + } + return containsUser(viewGroup.Users, "admin2") && containsUser(viewGroup.Users, "viewer2") + }, timeout, interval).Should(BeTrue()) + + Expect(viewGroup.Users).To(ContainElement("admin1")) + Expect(viewGroup.Users).To(ContainElement("admin2")) + Expect(viewGroup.Users).To(ContainElement("viewer1")) + Expect(viewGroup.Users).To(ContainElement("viewer2")) + }) + }) + + Context("When managing resource ownership", func() { + BeforeEach(func() { + By("Creating the required argocd ConfigMaps and Secret for ownership tests") + // Create argocd-rbac-cm if it doesn't exist + rbacCM := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "argocd-rbac-cm", + Namespace: "user-argocd", + }, + Data: map[string]string{ + "policy.csv": "", + }, + } + err := k8sClient.Create(ctx, rbacCM) + if err != nil && !strings.Contains(err.Error(), "already exists") { + Expect(err).NotTo(HaveOccurred()) + } + + // Create argocd-cm if it doesn't exist + argocdCM := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "argocd-cm", + Namespace: "user-argocd", + }, + Data: map[string]string{}, + } + err = k8sClient.Create(ctx, argocdCM) + if err != nil && !strings.Contains(err.Error(), "already exists") { + Expect(err).NotTo(HaveOccurred()) + } + + // Create argocd-secret if it doesn't exist + argocdSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "argocd-secret", + Namespace: "user-argocd", + }, + Data: map[string][]byte{}, + } + err = k8sClient.Create(ctx, argocdSecret) + if err != nil && !strings.Contains(err.Error(), "already exists") { + Expect(err).NotTo(HaveOccurred()) + } + }) + + It("Should set OwnerReference on AppProject pointing to ArgocdUser", func() { + By("Creating an ArgocdUser for ownership testing") + ownerUser := &argocduserv1alpha1.ArgocdUser{ + ObjectMeta: metav1.ObjectMeta{ + Name: "owner-test-team", + }, + Spec: argocduserv1alpha1.ArgocdUserSpec{ + Admin: argocduserv1alpha1.ArgocdCIAdmin{ + CIPass: "admin-pass", + Users: []string{"owner-admin"}, + }, + View: argocduserv1alpha1.ArgocdCIView{ + CIPass: "view-pass", + Users: []string{"owner-viewer"}, + }, + }, + } + Expect(k8sClient.Create(ctx, ownerUser)).Should(Succeed()) + + By("Waiting for AppProject to be created") + appProj := &argov1alpha1.AppProject{} + appProjLookup := types.NamespacedName{Name: "owner-test-team", Namespace: "user-argocd"} + Eventually(func() bool { + err := k8sClient.Get(ctx, appProjLookup, appProj) + return err == nil + }, timeout, interval).Should(BeTrue()) + + By("Verifying AppProject has OwnerReference pointing to ArgocdUser") + Expect(appProj.OwnerReferences).NotTo(BeEmpty(), "AppProject should have OwnerReferences") + Expect(appProj.OwnerReferences).To(HaveLen(1), "AppProject should have exactly one OwnerReference") + + ownerRef := appProj.OwnerReferences[0] + Expect(ownerRef.APIVersion).To(Equal("argocd.snappcloud.io/v1alpha1")) + Expect(ownerRef.Kind).To(Equal("ArgocdUser")) + Expect(ownerRef.Name).To(Equal("owner-test-team")) + Expect(ownerRef.UID).To(Equal(ownerUser.UID)) + Expect(*ownerRef.Controller).To(BeTrue(), "OwnerReference should have Controller=true") + Expect(*ownerRef.BlockOwnerDeletion).To(BeTrue(), "OwnerReference should have BlockOwnerDeletion=true") + }) + + It("Should set OwnerReference on Groups pointing to ArgocdUser", func() { + By("Waiting for admin Group to be created") + adminGroup := &userv1.Group{} + adminGroupLookup := types.NamespacedName{Name: "owner-test-team-admin"} + Eventually(func() bool { + err := k8sClient.Get(ctx, adminGroupLookup, adminGroup) + return err == nil + }, timeout, interval).Should(BeTrue()) + + By("Verifying admin Group has OwnerReference pointing to ArgocdUser") + Expect(adminGroup.OwnerReferences).NotTo(BeEmpty(), "Admin Group should have OwnerReferences") + Expect(adminGroup.OwnerReferences).To(HaveLen(1), "Admin Group should have exactly one OwnerReference") + + ownerRef := adminGroup.OwnerReferences[0] + Expect(ownerRef.APIVersion).To(Equal("argocd.snappcloud.io/v1alpha1")) + Expect(ownerRef.Kind).To(Equal("ArgocdUser")) + Expect(ownerRef.Name).To(Equal("owner-test-team")) + Expect(*ownerRef.Controller).To(BeTrue(), "OwnerReference should have Controller=true") + Expect(*ownerRef.BlockOwnerDeletion).To(BeTrue(), "OwnerReference should have BlockOwnerDeletion=true") + + By("Waiting for view Group to be created") + viewGroup := &userv1.Group{} + viewGroupLookup := types.NamespacedName{Name: "owner-test-team-view"} + Eventually(func() bool { + err := k8sClient.Get(ctx, viewGroupLookup, viewGroup) + return err == nil + }, timeout, interval).Should(BeTrue()) + + By("Verifying view Group has OwnerReference pointing to ArgocdUser") + Expect(viewGroup.OwnerReferences).NotTo(BeEmpty(), "View Group should have OwnerReferences") + Expect(viewGroup.OwnerReferences).To(HaveLen(1), "View Group should have exactly one OwnerReference") + + ownerRef = viewGroup.OwnerReferences[0] + Expect(ownerRef.APIVersion).To(Equal("argocd.snappcloud.io/v1alpha1")) + Expect(ownerRef.Kind).To(Equal("ArgocdUser")) + Expect(ownerRef.Name).To(Equal("owner-test-team")) + Expect(*ownerRef.Controller).To(BeTrue(), "OwnerReference should have Controller=true") + Expect(*ownerRef.BlockOwnerDeletion).To(BeTrue(), "OwnerReference should have BlockOwnerDeletion=true") + }) + + It("Should configure AppProject for garbage collection when ArgocdUser is deleted", func() { + By("Creating a new ArgocdUser for GC configuration testing") + gcUser := &argocduserv1alpha1.ArgocdUser{ + ObjectMeta: metav1.ObjectMeta{ + Name: "gc-test-team", + }, + Spec: argocduserv1alpha1.ArgocdUserSpec{ + Admin: argocduserv1alpha1.ArgocdCIAdmin{ + CIPass: "admin-pass", + Users: []string{"gc-admin"}, + }, + View: argocduserv1alpha1.ArgocdCIView{ + CIPass: "view-pass", + Users: []string{"gc-viewer"}, + }, + }, + } + Expect(k8sClient.Create(ctx, gcUser)).Should(Succeed()) + + By("Waiting for AppProject to be created") + appProj := &argov1alpha1.AppProject{} + appProjLookup := types.NamespacedName{Name: "gc-test-team", Namespace: "user-argocd"} + Eventually(func() bool { + err := k8sClient.Get(ctx, appProjLookup, appProj) + return err == nil + }, timeout, interval).Should(BeTrue()) + + By("Verifying AppProject has correct OwnerReference configuration for garbage collection") + Expect(appProj.OwnerReferences).NotTo(BeEmpty(), "AppProject should have OwnerReferences") + ownerRef := appProj.OwnerReferences[0] + Expect(ownerRef.Name).To(Equal("gc-test-team")) + Expect(*ownerRef.Controller).To(BeTrue(), "Controller flag enables garbage collection") + Expect(*ownerRef.BlockOwnerDeletion).To(BeTrue(), "BlockOwnerDeletion ensures proper deletion order") + + By("Note: Actual garbage collection is automatic in real Kubernetes clusters") + // In a real cluster, deleting the ArgocdUser would automatically delete the AppProject + // due to the OwnerReference. Envtest doesn't run the garbage collector, so we can't test + // the actual deletion, but we've verified the OwnerReference is configured correctly. + }) + + It("Should configure Groups for garbage collection when ArgocdUser is deleted", func() { + By("Creating a new ArgocdUser for Group GC configuration testing") + gcGroupUser := &argocduserv1alpha1.ArgocdUser{ + ObjectMeta: metav1.ObjectMeta{ + Name: "gc-group-test-team", + }, + Spec: argocduserv1alpha1.ArgocdUserSpec{ + Admin: argocduserv1alpha1.ArgocdCIAdmin{ + CIPass: "admin-pass", + Users: []string{"gc-group-admin"}, + }, + View: argocduserv1alpha1.ArgocdCIView{ + CIPass: "view-pass", + Users: []string{"gc-group-viewer"}, + }, + }, + } + Expect(k8sClient.Create(ctx, gcGroupUser)).Should(Succeed()) + + By("Waiting for admin Group to be created") + adminGroup := &userv1.Group{} + adminGroupLookup := types.NamespacedName{Name: "gc-group-test-team-admin"} + Eventually(func() bool { + err := k8sClient.Get(ctx, adminGroupLookup, adminGroup) + return err == nil + }, timeout, interval).Should(BeTrue()) + + By("Waiting for view Group to be created") + viewGroup := &userv1.Group{} + viewGroupLookup := types.NamespacedName{Name: "gc-group-test-team-view"} + Eventually(func() bool { + err := k8sClient.Get(ctx, viewGroupLookup, viewGroup) + return err == nil + }, timeout, interval).Should(BeTrue()) + + By("Verifying admin Group has correct OwnerReference configuration for garbage collection") + Expect(adminGroup.OwnerReferences).NotTo(BeEmpty(), "Admin Group should have OwnerReferences") + adminOwnerRef := adminGroup.OwnerReferences[0] + Expect(adminOwnerRef.Name).To(Equal("gc-group-test-team")) + Expect(*adminOwnerRef.Controller).To(BeTrue(), "Controller flag enables garbage collection") + Expect(*adminOwnerRef.BlockOwnerDeletion).To(BeTrue(), "BlockOwnerDeletion ensures proper deletion order") + + By("Verifying view Group has correct OwnerReference configuration for garbage collection") + Expect(viewGroup.OwnerReferences).NotTo(BeEmpty(), "View Group should have OwnerReferences") + viewOwnerRef := viewGroup.OwnerReferences[0] + Expect(viewOwnerRef.Name).To(Equal("gc-group-test-team")) + Expect(*viewOwnerRef.Controller).To(BeTrue(), "Controller flag enables garbage collection") + Expect(*viewOwnerRef.BlockOwnerDeletion).To(BeTrue(), "BlockOwnerDeletion ensures proper deletion order") + + By("Note: Actual garbage collection is automatic in real Kubernetes clusters") + // In a real cluster, deleting the ArgocdUser would automatically delete both Groups + // due to the OwnerReferences. Envtest doesn't run the garbage collector, so we can't test + // the actual deletion, but we've verified the OwnerReferences are configured correctly. + }) + }) + + Context("When managing finalizers and deletion", func() { + BeforeEach(func() { + By("Creating the required argocd ConfigMaps and Secret for finalizer tests") + // Create argocd-rbac-cm if it doesn't exist + rbacCM := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "argocd-rbac-cm", + Namespace: "user-argocd", + }, + Data: map[string]string{ + "policy.csv": "", + }, + } + err := k8sClient.Create(ctx, rbacCM) + if err != nil && !strings.Contains(err.Error(), "already exists") { + Expect(err).NotTo(HaveOccurred()) + } + + // Create argocd-cm if it doesn't exist + argocdCM := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "argocd-cm", + Namespace: "user-argocd", + }, + Data: map[string]string{}, + } + err = k8sClient.Create(ctx, argocdCM) + if err != nil && !strings.Contains(err.Error(), "already exists") { + Expect(err).NotTo(HaveOccurred()) + } + + // Create argocd-secret if it doesn't exist + argocdSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "argocd-secret", + Namespace: "user-argocd", + }, + Data: map[string][]byte{}, + } + err = k8sClient.Create(ctx, argocdSecret) + if err != nil && !strings.Contains(err.Error(), "already exists") { + Expect(err).NotTo(HaveOccurred()) + } + }) + + It("Should add finalizer to ArgocdUser on creation", func() { + By("Creating an ArgocdUser") + finalizerUser := &argocduserv1alpha1.ArgocdUser{ + ObjectMeta: metav1.ObjectMeta{ + Name: "finalizer-test-team", + }, + Spec: argocduserv1alpha1.ArgocdUserSpec{ + Admin: argocduserv1alpha1.ArgocdCIAdmin{ + CIPass: "admin-pass", + Users: []string{"admin-user"}, + }, + View: argocduserv1alpha1.ArgocdCIView{ + CIPass: "view-pass", + Users: []string{"view-user"}, + }, + }, + } + Expect(k8sClient.Create(ctx, finalizerUser)).Should(Succeed()) + + By("Waiting for finalizer to be added") + Eventually(func() bool { + err := k8sClient.Get(ctx, types.NamespacedName{Name: "finalizer-test-team"}, finalizerUser) + if err != nil { + return false + } + return containsFinalizer(finalizerUser.Finalizers, "argocd.snappcloud.io/finalizer") + }, timeout, interval).Should(BeTrue(), "Finalizer should be added to ArgocdUser") + + By("Verifying the finalizer value is correct") + Expect(finalizerUser.Finalizers).To(ContainElement("argocd.snappcloud.io/finalizer")) + }) + + It("Should block deletion when namespaces reference the ArgocdUser", func() { + By("Creating an ArgocdUser") + blockUser := &argocduserv1alpha1.ArgocdUser{ + ObjectMeta: metav1.ObjectMeta{ + Name: "block-deletion-team", + }, + Spec: argocduserv1alpha1.ArgocdUserSpec{ + Admin: argocduserv1alpha1.ArgocdCIAdmin{ + CIPass: "admin-pass", + Users: []string{"admin-user"}, + }, + View: argocduserv1alpha1.ArgocdCIView{ + CIPass: "view-pass", + Users: []string{"view-user"}, + }, + }, + } + Expect(k8sClient.Create(ctx, blockUser)).Should(Succeed()) + + By("Waiting for finalizer to be added") + Eventually(func() bool { + err := k8sClient.Get(ctx, types.NamespacedName{Name: "block-deletion-team"}, blockUser) + if err != nil { + return false + } + return containsFinalizer(blockUser.Finalizers, "argocd.snappcloud.io/finalizer") + }, timeout, interval).Should(BeTrue()) + + By("Creating a namespace that references the ArgocdUser") + namespace := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-namespace-with-ref", + Labels: map[string]string{ + "argocd.snappcloud.io/appproj": "block-deletion-team", + }, + }, + } + Expect(k8sClient.Create(ctx, namespace)).Should(Succeed()) + + By("Attempting to delete the ArgocdUser") + Expect(k8sClient.Delete(ctx, blockUser)).Should(Succeed()) + + By("Verifying ArgocdUser still exists due to finalizer blocking deletion") + Consistently(func() bool { + err := k8sClient.Get(ctx, types.NamespacedName{Name: "block-deletion-team"}, blockUser) + return err == nil + }, "5s", interval).Should(BeTrue(), "ArgocdUser should still exist due to finalizer") + + By("Verifying ArgocdUser has deletion timestamp") + Expect(blockUser.DeletionTimestamp).NotTo(BeNil(), "ArgocdUser should have deletion timestamp") + + By("Verifying finalizer is still present") + Expect(blockUser.Finalizers).To(ContainElement("argocd.snappcloud.io/finalizer"), "Finalizer should still be present while namespace references exist") + }) + + It("Should allow deletion after namespace labels are removed", func() { + By("Getting the existing ArgocdUser with namespace reference") + blockUser := &argocduserv1alpha1.ArgocdUser{} + Eventually(func() bool { + err := k8sClient.Get(ctx, types.NamespacedName{Name: "block-deletion-team"}, blockUser) + return err == nil && blockUser.DeletionTimestamp != nil + }, timeout, interval).Should(BeTrue()) + + By("Removing the label from the namespace") + namespace := &corev1.Namespace{} + Expect(k8sClient.Get(ctx, types.NamespacedName{Name: "test-namespace-with-ref"}, namespace)).Should(Succeed()) + delete(namespace.Labels, "argocd.snappcloud.io/appproj") + Expect(k8sClient.Update(ctx, namespace)).Should(Succeed()) + + By("Verifying ArgocdUser is eventually deleted after namespace label is removed") + Eventually(func() bool { + err := k8sClient.Get(ctx, types.NamespacedName{Name: "block-deletion-team"}, blockUser) + return err != nil && strings.Contains(err.Error(), "not found") + }, timeout, interval).Should(BeTrue(), "ArgocdUser should be deleted after namespace label is removed") + }) + + It("Should successfully delete ArgocdUser when no namespaces reference it", func() { + By("Creating an ArgocdUser without namespace references") + cleanUser := &argocduserv1alpha1.ArgocdUser{ + ObjectMeta: metav1.ObjectMeta{ + Name: "clean-deletion-team", + }, + Spec: argocduserv1alpha1.ArgocdUserSpec{ + Admin: argocduserv1alpha1.ArgocdCIAdmin{ + CIPass: "admin-pass", + Users: []string{"admin-user"}, + }, + View: argocduserv1alpha1.ArgocdCIView{ + CIPass: "view-pass", + Users: []string{"view-user"}, + }, + }, + } + Expect(k8sClient.Create(ctx, cleanUser)).Should(Succeed()) + + By("Waiting for finalizer to be added") + Eventually(func() bool { + err := k8sClient.Get(ctx, types.NamespacedName{Name: "clean-deletion-team"}, cleanUser) + if err != nil { + return false + } + return containsFinalizer(cleanUser.Finalizers, "argocd.snappcloud.io/finalizer") + }, timeout, interval).Should(BeTrue()) + + By("Deleting the ArgocdUser") + Expect(k8sClient.Delete(ctx, cleanUser)).Should(Succeed()) + + By("Verifying ArgocdUser is deleted successfully") + Eventually(func() bool { + err := k8sClient.Get(ctx, types.NamespacedName{Name: "clean-deletion-team"}, cleanUser) + return err != nil && strings.Contains(err.Error(), "not found") + }, timeout, interval).Should(BeTrue(), "ArgocdUser should be deleted when no namespaces reference it") + }) + + It("Should block deletion when multi-team namespace references the ArgocdUser", func() { + By("Creating first ArgocdUser (team-alpha)") + alphaUser := &argocduserv1alpha1.ArgocdUser{ + ObjectMeta: metav1.ObjectMeta{ + Name: "team-alpha", + }, + Spec: argocduserv1alpha1.ArgocdUserSpec{ + Admin: argocduserv1alpha1.ArgocdCIAdmin{ + CIPass: "admin-pass", + Users: []string{"admin-user"}, + }, + View: argocduserv1alpha1.ArgocdCIView{ + CIPass: "view-pass", + Users: []string{"view-user"}, + }, + }, + } + Expect(k8sClient.Create(ctx, alphaUser)).Should(Succeed()) + + By("Creating second ArgocdUser (team-beta)") + betaUser := &argocduserv1alpha1.ArgocdUser{ + ObjectMeta: metav1.ObjectMeta{ + Name: "team-beta", + }, + Spec: argocduserv1alpha1.ArgocdUserSpec{ + Admin: argocduserv1alpha1.ArgocdCIAdmin{ + CIPass: "admin-pass", + Users: []string{"admin-user"}, + }, + View: argocduserv1alpha1.ArgocdCIView{ + CIPass: "view-pass", + Users: []string{"view-user"}, + }, + }, + } + Expect(k8sClient.Create(ctx, betaUser)).Should(Succeed()) + + By("Waiting for finalizers to be added to both ArgocdUsers") + Eventually(func() bool { + err := k8sClient.Get(ctx, types.NamespacedName{Name: "team-alpha"}, alphaUser) + if err != nil { + return false + } + return containsFinalizer(alphaUser.Finalizers, "argocd.snappcloud.io/finalizer") + }, timeout, interval).Should(BeTrue()) + + Eventually(func() bool { + err := k8sClient.Get(ctx, types.NamespacedName{Name: "team-beta"}, betaUser) + if err != nil { + return false + } + return containsFinalizer(betaUser.Finalizers, "argocd.snappcloud.io/finalizer") + }, timeout, interval).Should(BeTrue()) + + By("Creating a multi-team namespace that references both teams") + multiTeamNS := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "multi-team-namespace", + Labels: map[string]string{ + "argocd.snappcloud.io/appproj": "team-alpha.team-beta", + }, + }, + } + Expect(k8sClient.Create(ctx, multiTeamNS)).Should(Succeed()) + + By("Attempting to delete team-alpha ArgocdUser") + Expect(k8sClient.Delete(ctx, alphaUser)).Should(Succeed()) + + By("Verifying team-alpha ArgocdUser still exists due to finalizer blocking deletion") + Consistently(func() bool { + err := k8sClient.Get(ctx, types.NamespacedName{Name: "team-alpha"}, alphaUser) + return err == nil + }, "5s", interval).Should(BeTrue(), "team-alpha ArgocdUser should still exist due to multi-team namespace reference") + + By("Verifying team-alpha has deletion timestamp") + Expect(alphaUser.DeletionTimestamp).NotTo(BeNil(), "team-alpha should have deletion timestamp") + + By("Verifying finalizer is still present on team-alpha") + Expect(alphaUser.Finalizers).To(ContainElement("argocd.snappcloud.io/finalizer"), + "Finalizer should still be present while multi-team namespace references team-alpha") + + By("Attempting to delete team-beta ArgocdUser") + Expect(k8sClient.Delete(ctx, betaUser)).Should(Succeed()) + + By("Verifying team-beta ArgocdUser still exists due to finalizer blocking deletion") + Consistently(func() bool { + err := k8sClient.Get(ctx, types.NamespacedName{Name: "team-beta"}, betaUser) + return err == nil + }, "5s", interval).Should(BeTrue(), "team-beta ArgocdUser should still exist due to multi-team namespace reference") + + By("Verifying team-beta has deletion timestamp") + Eventually(func() bool { + err := k8sClient.Get(ctx, types.NamespacedName{Name: "team-beta"}, betaUser) + if err != nil { + return false + } + return betaUser.DeletionTimestamp != nil + }, timeout, interval).Should(BeTrue(), "team-beta should have deletion timestamp") + + By("Removing the multi-team label from the namespace") + Expect(k8sClient.Get(ctx, types.NamespacedName{Name: "multi-team-namespace"}, multiTeamNS)).Should(Succeed()) + delete(multiTeamNS.Labels, "argocd.snappcloud.io/appproj") + Expect(k8sClient.Update(ctx, multiTeamNS)).Should(Succeed()) + + By("Verifying both ArgocdUsers are eventually deleted after namespace label is removed") + Eventually(func() bool { + err := k8sClient.Get(ctx, types.NamespacedName{Name: "team-alpha"}, alphaUser) + return err != nil && strings.Contains(err.Error(), "not found") + }, timeout, interval).Should(BeTrue(), "team-alpha should be deleted after multi-team namespace label is removed") + + Eventually(func() bool { + err := k8sClient.Get(ctx, types.NamespacedName{Name: "team-beta"}, betaUser) + return err != nil && strings.Contains(err.Error(), "not found") + }, timeout, interval).Should(BeTrue(), "team-beta should be deleted after multi-team namespace label is removed") + }) + }) +}) + +// Helper function to check if a user is in the list +func containsUser(users []string, user string) bool { + for _, u := range users { + if u == user { + return true + } + } + return false +} + +// Helper function to check if a finalizer is in the list +// +//nolint:unparam // finalizer parameter is intentionally generic for reusability +func containsFinalizer(finalizers []string, finalizer string) bool { + for _, f := range finalizers { + if f == finalizer { + return true + } + } + return false +} diff --git a/internal/controller/namespace_controller.go b/internal/controller/namespace_controller.go index 199552c..0ee621f 100644 --- a/internal/controller/namespace_controller.go +++ b/internal/controller/namespace_controller.go @@ -30,7 +30,6 @@ import ( "github.com/snapp-incubator/argocd-complementary-operator/pkg/nameset" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" @@ -331,120 +330,58 @@ func (r *NamespaceReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( return ctrl.Result{}, reconciliationErrors.ErrorOrNil() } -// reconcileAppProject create an argocd project and change the current argocd project to be compatible with it. -// it is called everytime a label changed, so when you remove a policy or etc it will not be called. +// reconcileAppProject updates destinations and source namespaces in an existing AppProject. +// It does NOT create AppProjects - only ArgocdUserReconciler can create AppProjects. +// This ensures proper separation of concerns: ArgocdUserReconciler manages RBAC roles, +// NamespaceReconciler manages namespace destinations. func (r *NamespaceReconciler) reconcileAppProject(ctx context.Context, logger logr.Logger, team string) error { - appProj := r.createAppProj(team) - - // Check if AppProj does not exist and create a new one + // Check if AppProject exists - it should be created by ArgocdUserReconciler found := &argov1alpha1.AppProject{} if err := r.Get(ctx, types.NamespacedName{Name: team, Namespace: baseNs}, found); err != nil { if errors.IsNotFound(err) { - logger.Info("creating argocd appproject", "team", appProj.Name, "sources", appProj.Spec.SourceNamespaces) - - if err := r.Create(ctx, appProj); err != nil { - return fmt.Errorf("error creating AppProj: %w", err) - } - - return nil - } else { - return fmt.Errorf("error getting AppProj: %w", err) + return fmt.Errorf("AppProject %s does not exist. Please create an ArgocdUser resource for team %s first", team, team) } + return fmt.Errorf("error getting AppProj: %w", err) } - appProj.Spec.SourceRepos = appendRepos(appProj.Spec.SourceRepos, found.Spec.SourceRepos) - - // If AppProj already exist, check if it is deeply equal with desrired state - if !reflect.DeepEqual(appProj.Spec, found.Spec) { - logger.Info("Founded AppProj is not equad to desired one, doing the upgrade", "AppProj.Name", team) - - found.Spec = appProj.Spec - - if err := r.Update(ctx, found); err != nil { - return fmt.Errorf("error updating AppProj: %v", err) - } - } - - return nil -} - -func (r *NamespaceReconciler) createAppProj(team string) *argov1alpha1.AppProject { + // Build the desired destinations and source namespaces desiredNamespaces := NamespaceCache.GetNamespaces(team) - destinations := []argov1alpha1.ApplicationDestination{} - for _, desiredNamespace := range desiredNamespaces { destinations = append(destinations, argov1alpha1.ApplicationDestination{ Namespace: desiredNamespace, Server: "*", }) } - sources := NamespaceCache.GetSources(team) - // Get public repos + // Get public repos from environment repo_env := os.Getenv("PUBLIC_REPOS") repo_list := strings.Split(repo_env, ",") - // Get cluster scoped teams - team_env := os.Getenv("CLUSTER_ADMIN_TEAMS") - team_list := strings.Split(team_env, ",") + // Merge with existing repos + sourceRepos := appendRepos(repo_list, found.Spec.SourceRepos) - includeAllGroupKind := []metav1.GroupKind{ - { - Group: "*", - Kind: "*", - }, - } + // Check if we need to update Destinations, SourceNamespaces, or SourceRepos + needsUpdate := !reflect.DeepEqual(found.Spec.Destinations, destinations) || + !reflect.DeepEqual(found.Spec.SourceNamespaces, sources) || + !reflect.DeepEqual(found.Spec.SourceRepos, sourceRepos) - appProj := &argov1alpha1.AppProject{ - ObjectMeta: metav1.ObjectMeta{ - Name: team, - Namespace: baseNs, - }, - Spec: argov1alpha1.AppProjectSpec{ - SourceRepos: repo_list, - Destinations: destinations, - SourceNamespaces: sources, - NamespaceResourceBlacklist: []metav1.GroupKind{ - { - Group: "", - Kind: "LimitRange", - }, - }, - Roles: []argov1alpha1.ProjectRole{ - { - Groups: []string{team + "-admin", team + "-admin" + "-ci"}, - Name: team + "-admin", - Policies: []string{ - "p, proj:" + team + ":" + team + "-admin, applications, *, " + team + "/*, allow", - "p, proj:" + team + ":" + team + "-admin, repositories, *, " + team + "/*, allow", - "p, proj:" + team + ":" + team + "-admin, exec, create, " + team + "/*, allow", - // TODO: The log get action shouldbe available as we add team-view to the admin in `AddArgocdRBACPolicy` function. - // But it doesn't work! - "p, proj:" + team + ":" + team + "-admin, logs, get, " + team + "/*, allow", - }, - }, - { - Groups: []string{team + "-view", team + "-view" + "-ci"}, - Name: team + "-view", - Policies: []string{ - "p, proj:" + team + ":" + team + "-view, applications, get, " + team + "/*, allow", - "p, proj:" + team + ":" + team + "-view, repositories, get, " + team + "/*, allow", - "p, proj:" + team + ":" + team + "-view, logs, get, " + team + "/*, allow", - }, - }, - }, - }, - } + if needsUpdate { + logger.Info("Updating AppProject destinations and sources", "AppProj.Name", team) - if isTeamClusterAdmin(team, team_list) { - appProj.Spec.ClusterResourceWhitelist = includeAllGroupKind - } else { - appProj.Spec.ClusterResourceBlacklist = includeAllGroupKind + // Only update Destinations, SourceNamespaces, and SourceRepos + // Preserve Roles (managed by ArgocdUserReconciler) and other fields + found.Spec.Destinations = destinations + found.Spec.SourceNamespaces = sources + found.Spec.SourceRepos = sourceRepos + + if err := r.Update(ctx, found); err != nil { + return fmt.Errorf("error updating AppProj: %v", err) + } } - return appProj + return nil } // SetupWithManager sets up the controller with the Manager. @@ -486,12 +423,3 @@ func labelToProjects(l string) nameset.Nameset[string] { return result } - -func isTeamClusterAdmin(team string, clusterAdminList []string) bool { - for _, tm := range clusterAdminList { - if team == tm { - return true - } - } - return false -} diff --git a/internal/controller/namespace_controller_test.go b/internal/controller/namespace_controller_test.go index 15ea698..b792c32 100644 --- a/internal/controller/namespace_controller_test.go +++ b/internal/controller/namespace_controller_test.go @@ -23,20 +23,110 @@ import ( argov1alpha1 "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + argocduserv1alpha1 "github.com/snapp-incubator/argocd-complementary-operator/api/v1alpha1" "github.com/snapp-incubator/argocd-complementary-operator/internal/controller" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" ) var ctx = context.Background() -var _ = Describe("namespace controller to create teams", func() { +var _ = Describe("namespace controller to create teams", Ordered, func() { // Define utility constants for object names and testing timeouts/durations and intervals. const ( timeout = time.Second * 20 interval = time.Millisecond * 30 ) + + BeforeAll(func() { + By("Ensuring user-argocd namespace exists") + userArgocdNS := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "user-argocd", + }, + } + err := k8sClient.Create(ctx, userArgocdNS) + if err != nil { + // Namespace might already exist + _ = k8sClient.Get(ctx, types.NamespacedName{Name: "user-argocd"}, userArgocdNS) + } + + By("Ensuring required ConfigMaps exist for ArgocdUser reconciliation") + // Create argocd-rbac-cm + rbacConfigMap := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "argocd-rbac-cm", + Namespace: "user-argocd", + }, + Data: map[string]string{ + "policy.csv": "", + }, + } + err = k8sClient.Create(ctx, rbacConfigMap) + if err != nil { + // Might already exist from other tests + _ = k8sClient.Get(ctx, types.NamespacedName{Name: "argocd-rbac-cm", Namespace: "user-argocd"}, rbacConfigMap) + } + + // Create argocd-cm + argocdConfigMap := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "argocd-cm", + Namespace: "user-argocd", + }, + Data: map[string]string{}, + } + err = k8sClient.Create(ctx, argocdConfigMap) + if err != nil { + _ = k8sClient.Get(ctx, types.NamespacedName{Name: "argocd-cm", Namespace: "user-argocd"}, argocdConfigMap) + } + + // Create argocd-secret + argocdSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "argocd-secret", + Namespace: "user-argocd", + }, + Data: map[string][]byte{}, + } + err = k8sClient.Create(ctx, argocdSecret) + if err != nil { + _ = k8sClient.Get(ctx, types.NamespacedName{Name: "argocd-secret", Namespace: "user-argocd"}, argocdSecret) + } + + By("Creating test-team ArgocdUser for namespace tests") + testTeamUser := &argocduserv1alpha1.ArgocdUser{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-team", + }, + Spec: argocduserv1alpha1.ArgocdUserSpec{ + Admin: argocduserv1alpha1.ArgocdCIAdmin{ + CIPass: "admin-password", + Users: []string{"admin-user1", "admin-user2"}, + }, + View: argocduserv1alpha1.ArgocdCIView{ + CIPass: "view-password", + Users: []string{"view-user1"}, + }, + }, + } + err = k8sClient.Create(ctx, testTeamUser) + if err != nil { + // Might already exist from ArgocdUser tests + _ = k8sClient.Get(ctx, types.NamespacedName{Name: "test-team"}, testTeamUser) + } + + // Wait for test-team AppProject to be created + testAppProj := &argov1alpha1.AppProject{} + testAppProjLookup := types.NamespacedName{Name: "test-team", Namespace: "user-argocd"} + Eventually(func() bool { + err := k8sClient.Get(ctx, testAppProjLookup, testAppProj) + return err == nil + }, timeout, interval).Should(BeTrue()) + }) + // Creating user-argocd namespace Context("When cluster bootstrap", func() { It("Should create user-argocd NS", func() { @@ -46,8 +136,14 @@ var _ = Describe("namespace controller to create teams", func() { Name: "user-argocd", }, } - Expect(k8sClient.Create(ctx, ArgoNs)).Should(Succeed()) + err := k8sClient.Create(ctx, ArgoNs) + if err != nil { + // Namespace might already exist from ArgocdUser tests + lookupns := types.NamespacedName{Name: "user-argocd"} + Expect(k8sClient.Get(ctx, lookupns, ArgoNs)).Should(Succeed()) + } + // Verify namespace exists lookupns := types.NamespacedName{Name: "user-argocd"} Expect(k8sClient.Get(ctx, lookupns, ArgoNs)).Should(Succeed()) }) @@ -78,7 +174,11 @@ var _ = Describe("namespace controller to create teams", func() { testAppProjLookup := types.NamespacedName{Name: "test-team", Namespace: "user-argocd"} Eventually(func() bool { err := k8sClient.Get(ctx, testAppProjLookup, testAppProj) - return err == nil + if err != nil { + return false + } + // Wait for Destinations to be populated by NamespaceReconciler + return len(testAppProj.Spec.Destinations) > 0 && len(testAppProj.Spec.SourceNamespaces) > 0 }, timeout, interval).Should(BeTrue()) // make sure appproject has the correct fields. @@ -93,6 +193,33 @@ var _ = Describe("namespace controller to create teams", func() { // Changing the namespace label and checking if the appProjects are updated Context("when changing namespace team label", func() { It("Should update appProject", func() { + By("Creating ArgocdUser for cloudy-team first", func() { + cloudyUser := &argocduserv1alpha1.ArgocdUser{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cloudy-team", + }, + Spec: argocduserv1alpha1.ArgocdUserSpec{ + Admin: argocduserv1alpha1.ArgocdCIAdmin{ + CIPass: "admin-pass", + Users: []string{}, + }, + View: argocduserv1alpha1.ArgocdCIView{ + CIPass: "view-pass", + Users: []string{}, + }, + }, + } + Expect(k8sClient.Create(ctx, cloudyUser)).Should(Succeed()) + + // Wait for AppProject to be created by ArgocdUserReconciler + cloudyAppProj := &argov1alpha1.AppProject{} + cloudyAppProjLookup := types.NamespacedName{Name: "cloudy-team", Namespace: "user-argocd"} + Eventually(func() bool { + err := k8sClient.Get(ctx, cloudyAppProjLookup, cloudyAppProj) + return err == nil + }, timeout, interval).Should(BeTrue()) + }) + By("Removing from AppProject and creating new AppProject", func() { // update test namespace with cloudy-team label. testNS := &corev1.Namespace{ @@ -116,6 +243,15 @@ var _ = Describe("namespace controller to create teams", func() { return err == nil }, timeout, interval).Should(BeTrue()) + // Wait for NamespaceReconciler to populate Destinations + Eventually(func() bool { + err := k8sClient.Get(ctx, cloudyAppProjLookup, cloudyAppProj) + if err != nil { + return false + } + return len(cloudyAppProj.Spec.Destinations) > 0 + }, timeout, interval).Should(BeTrue()) + // make sure appproject has the correct fields. Expect(cloudyAppProj.Name).Should(Equal(cloudyAppProjLookup.Name)) Expect(cloudyAppProj.Namespace).Should(Equal(cloudyAppProjLookup.Namespace)) @@ -137,6 +273,33 @@ var _ = Describe("namespace controller to create teams", func() { // Changing the namespace label and checking if the appProjects are updated Context("when changing namespace team label with multiple teams", func() { It("Should update appProject with multiple labels", func() { + By("Creating ArgocdUser for rainy-team first", func() { + rainyUser := &argocduserv1alpha1.ArgocdUser{ + ObjectMeta: metav1.ObjectMeta{ + Name: "rainy-team", + }, + Spec: argocduserv1alpha1.ArgocdUserSpec{ + Admin: argocduserv1alpha1.ArgocdCIAdmin{ + CIPass: "admin-pass", + Users: []string{}, + }, + View: argocduserv1alpha1.ArgocdCIView{ + CIPass: "view-pass", + Users: []string{}, + }, + }, + } + Expect(k8sClient.Create(ctx, rainyUser)).Should(Succeed()) + + // Wait for AppProject to be created by ArgocdUserReconciler + rainyAppProj := &argov1alpha1.AppProject{} + rainyAppProjLookup := types.NamespacedName{Name: "rainy-team", Namespace: "user-argocd"} + Eventually(func() bool { + err := k8sClient.Get(ctx, rainyAppProjLookup, rainyAppProj) + return err == nil + }, timeout, interval).Should(BeTrue()) + }) + By("Removing from AppProject and creating new AppProject", func() { // update test namespace with cloudy-team label. testNS := &corev1.Namespace{ @@ -160,6 +323,15 @@ var _ = Describe("namespace controller to create teams", func() { return err == nil }, timeout, interval).Should(BeTrue()) + // Wait for NamespaceReconciler to populate Destinations + Eventually(func() bool { + err := k8sClient.Get(ctx, cloudyAppProjLookup, cloudyAppProj) + if err != nil { + return false + } + return len(cloudyAppProj.Spec.Destinations) > 0 + }, timeout, interval).Should(BeTrue()) + // make sure appproject has the correct fields. Expect(cloudyAppProj.Name).Should(Equal(cloudyAppProjLookup.Name)) Expect(cloudyAppProj.Namespace).Should(Equal(cloudyAppProjLookup.Namespace)) @@ -174,6 +346,15 @@ var _ = Describe("namespace controller to create teams", func() { return err == nil }, timeout, interval).Should(BeTrue()) + // Wait for NamespaceReconciler to populate Destinations + Eventually(func() bool { + err := k8sClient.Get(ctx, rainyAppProjLookup, rainyAppProj) + if err != nil { + return false + } + return len(rainyAppProj.Spec.Destinations) > 0 + }, timeout, interval).Should(BeTrue()) + // make sure appproject has the correct fields. Expect(rainyAppProj.Name).Should(Equal(rainyAppProjLookup.Name)) Expect(rainyAppProj.Namespace).Should(Equal(rainyAppProjLookup.Namespace)) @@ -181,4 +362,293 @@ var _ = Describe("namespace controller to create teams", func() { }) }) }) + + // Multi-team with missing ArgocdUser validation + Context("when multi-team label includes non-existent ArgocdUser", func() { + It("Should handle missing ArgocdUser gracefully in multi-team label", func() { + By("Creating ArgocdUser for existing-team first", func() { + existingUser := &argocduserv1alpha1.ArgocdUser{ + ObjectMeta: metav1.ObjectMeta{ + Name: "existing-team", + }, + Spec: argocduserv1alpha1.ArgocdUserSpec{ + Admin: argocduserv1alpha1.ArgocdCIAdmin{ + CIPass: "admin-pass", + Users: []string{}, + }, + View: argocduserv1alpha1.ArgocdCIView{ + CIPass: "view-pass", + Users: []string{}, + }, + }, + } + Expect(k8sClient.Create(ctx, existingUser)).Should(Succeed()) + + // Wait for AppProject to be created by ArgocdUserReconciler + existingAppProj := &argov1alpha1.AppProject{} + existingAppProjLookup := types.NamespacedName{Name: "existing-team", Namespace: "user-argocd"} + Eventually(func() bool { + err := k8sClient.Get(ctx, existingAppProjLookup, existingAppProj) + return err == nil + }, timeout, interval).Should(BeTrue()) + }) + + By("Creating namespace with multi-team label including non-existent team") + partialNS := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "partial-team-ns", + Labels: map[string]string{ + controller.ProjectsLabel: "existing-team.nonexistent-team", + }, + }, + } + Expect(k8sClient.Create(ctx, partialNS)).Should(Succeed()) + + // Verify namespace was created + partialNSLookup := types.NamespacedName{Name: "partial-team-ns"} + Expect(k8sClient.Get(ctx, partialNSLookup, partialNS)).Should(Succeed()) + + By("Verifying existing-team AppProject gets the namespace in destinations") + existingAppProj := &argov1alpha1.AppProject{} + existingAppProjLookup := types.NamespacedName{Name: "existing-team", Namespace: "user-argocd"} + Eventually(func() bool { + err := k8sClient.Get(ctx, existingAppProjLookup, existingAppProj) + if err != nil { + return false + } + // Check if namespace is in destinations + for _, dest := range existingAppProj.Spec.Destinations { + if dest.Namespace == "partial-team-ns" { + return true + } + } + return false + }, timeout, interval).Should(BeTrue(), "existing-team AppProject should have partial-team-ns in destinations") + + By("Verifying nonexistent-team AppProject is NOT created") + nonexistentAppProj := &argov1alpha1.AppProject{} + nonexistentAppProjLookup := types.NamespacedName{Name: "nonexistent-team", Namespace: "user-argocd"} + + // Wait a bit to ensure reconciliation has been attempted + time.Sleep(2 * time.Second) + + // AppProject should not exist for non-existent team + err := k8sClient.Get(ctx, nonexistentAppProjLookup, nonexistentAppProj) + Expect(err).Should(HaveOccurred(), "nonexistent-team AppProject should not exist") + Expect(errors.IsNotFound(err)).Should(BeTrue(), + "AppProject for non-existent ArgocdUser should not be created - only ArgocdUserReconciler creates AppProjects") + }) + }) + + // Verifying RBAC policies in AppProject + Context("When verifying AppProject RBAC policies", func() { + It("Should include logs permission for admin role", func() { + By("Creating ArgocdUser for rbac-team first", func() { + rbacUser := &argocduserv1alpha1.ArgocdUser{ + ObjectMeta: metav1.ObjectMeta{ + Name: "rbac-team", + }, + Spec: argocduserv1alpha1.ArgocdUserSpec{ + Admin: argocduserv1alpha1.ArgocdCIAdmin{ + CIPass: "admin-pass", + Users: []string{}, + }, + View: argocduserv1alpha1.ArgocdCIView{ + CIPass: "view-pass", + Users: []string{}, + }, + }, + } + Expect(k8sClient.Create(ctx, rbacUser)).Should(Succeed()) + + // Wait for AppProject to be created by ArgocdUserReconciler + rbacAppProj := &argov1alpha1.AppProject{} + rbacAppProjLookup := types.NamespacedName{Name: "rbac-team", Namespace: "user-argocd"} + Eventually(func() bool { + err := k8sClient.Get(ctx, rbacAppProjLookup, rbacAppProj) + return err == nil + }, timeout, interval).Should(BeTrue()) + }) + + By("Creating a namespace with team label") + rbacTestNS := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "rbac-test-ns", + Labels: map[string]string{ + controller.ProjectsLabel: "rbac-team", + }, + }, + } + Expect(k8sClient.Create(ctx, rbacTestNS)).Should(Succeed()) + + By("Waiting for AppProject to be created") + rbacAppProj := &argov1alpha1.AppProject{} + rbacAppProjLookup := types.NamespacedName{Name: "rbac-team", Namespace: "user-argocd"} + Eventually(func() bool { + err := k8sClient.Get(ctx, rbacAppProjLookup, rbacAppProj) + return err == nil + }, timeout, interval).Should(BeTrue()) + + By("Verifying admin role has full permissions (logs inherited from view role)") + adminRoleFound := false + for _, role := range rbacAppProj.Spec.Roles { + if role.Name == "rbac-team-admin" { + adminRoleFound = true + + // Verify admin-specific permissions + Expect(role.Policies).To(ContainElement(ContainSubstring("applications, *, rbac-team/*, allow")), + "Admin should have applications permissions") + Expect(role.Policies).To(ContainElement(ContainSubstring("repositories, *, rbac-team/*, allow")), + "Admin should have repositories permissions") + Expect(role.Policies).To(ContainElement(ContainSubstring("exec, create, rbac-team/*, allow")), + "Admin should have exec permissions") + + // Note: logs permission is in view role, which admin inherits + } + } + Expect(adminRoleFound).To(BeTrue(), "Admin role should exist in AppProject") + }) + + It("Should include admin groups in view role for role aggregation", func() { + By("Getting the AppProject created in previous test") + rbacAppProj := &argov1alpha1.AppProject{} + rbacAppProjLookup := types.NamespacedName{Name: "rbac-team", Namespace: "user-argocd"} + Expect(k8sClient.Get(ctx, rbacAppProjLookup, rbacAppProj)).Should(Succeed()) + + By("Verifying view role group assignments include both admin and view groups") + viewRoleFound := false + for _, role := range rbacAppProj.Spec.Roles { + if role.Name == "rbac-team-view" { + viewRoleFound = true + + // View role should contain ALL groups (admin + view) for role aggregation + Expect(role.Groups).To(ContainElement("rbac-team-view"), + "View role should include view group") + Expect(role.Groups).To(ContainElement("rbac-team-view-ci"), + "View role should include view-ci group") + Expect(role.Groups).To(ContainElement("rbac-team-admin"), + "View role should include admin group for role aggregation") + Expect(role.Groups).To(ContainElement("rbac-team-admin-ci"), + "View role should include admin-ci group for role aggregation") + + // Verify view role has correct permissions + Expect(role.Policies).To(ContainElement(ContainSubstring("applications, get, rbac-team/*, allow")), + "View should have applications get permission") + Expect(role.Policies).To(ContainElement(ContainSubstring("repositories, get, rbac-team/*, allow")), + "View should have repositories get permission") + Expect(role.Policies).To(ContainElement(ContainSubstring("logs, get, rbac-team/*, allow")), + "View should have logs get permission") + } + } + Expect(viewRoleFound).To(BeTrue(), "View role should exist in AppProject") + }) + + It("Should have complete admin role structure with all permissions", func() { + By("Creating ArgocdUser for complete-team first", func() { + completeUser := &argocduserv1alpha1.ArgocdUser{ + ObjectMeta: metav1.ObjectMeta{ + Name: "complete-team", + }, + Spec: argocduserv1alpha1.ArgocdUserSpec{ + Admin: argocduserv1alpha1.ArgocdCIAdmin{ + CIPass: "admin-pass", + Users: []string{}, + }, + View: argocduserv1alpha1.ArgocdCIView{ + CIPass: "view-pass", + Users: []string{}, + }, + }, + } + Expect(k8sClient.Create(ctx, completeUser)).Should(Succeed()) + + // Wait for AppProject to be created by ArgocdUserReconciler + completeAppProj := &argov1alpha1.AppProject{} + completeAppProjLookup := types.NamespacedName{Name: "complete-team", Namespace: "user-argocd"} + Eventually(func() bool { + err := k8sClient.Get(ctx, completeAppProjLookup, completeAppProj) + return err == nil + }, timeout, interval).Should(BeTrue()) + }) + + By("Creating another test namespace") + fullTestNS := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "full-rbac-test", + Labels: map[string]string{ + controller.ProjectsLabel: "complete-team", + }, + }, + } + Expect(k8sClient.Create(ctx, fullTestNS)).Should(Succeed()) + + By("Waiting for AppProject to be created") + completeAppProj := &argov1alpha1.AppProject{} + completeAppProjLookup := types.NamespacedName{Name: "complete-team", Namespace: "user-argocd"} + Eventually(func() bool { + err := k8sClient.Get(ctx, completeAppProjLookup, completeAppProj) + return err == nil + }, timeout, interval).Should(BeTrue()) + + By("Verifying admin role has all expected permissions") + adminRoleFound := false + for _, role := range completeAppProj.Spec.Roles { + if role.Name == "complete-team-admin" { + adminRoleFound = true + + // Admin role should have these specific permissions + expectedPolicySubstrings := []string{ + "applications, *, complete-team/*, allow", + "repositories, *, complete-team/*, allow", + "exec, create, complete-team/*, allow", + } + + for _, expectedSubstring := range expectedPolicySubstrings { + Expect(role.Policies).To(ContainElement(ContainSubstring(expectedSubstring)), + "Admin role should have policy containing: %s", expectedSubstring) + } + + // Verify group assignments (only admin groups in admin role) + Expect(role.Groups).To(HaveLen(2), + "Admin role should have exactly 2 groups (admin and admin-ci)") + Expect(role.Groups).To(ContainElement("complete-team-admin")) + Expect(role.Groups).To(ContainElement("complete-team-admin-ci")) + } + } + Expect(adminRoleFound).To(BeTrue(), "Admin role should exist in AppProject") + }) + }) + + // Verifying that namespace controller doesn't create AppProject + Context("When namespace is labeled with non-existent team", func() { + It("Should fail because AppProject must be created by ArgocdUser first", func() { + By("Creating a namespace with label for non-existent team") + failNS := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "fail-test-ns", + Labels: map[string]string{ + controller.ProjectsLabel: "non-existent-team", + }, + }, + } + Expect(k8sClient.Create(ctx, failNS)).Should(Succeed()) + + By("Verifying namespace was created") + failNSLookup := types.NamespacedName{Name: "fail-test-ns"} + Expect(k8sClient.Get(ctx, failNSLookup, failNS)).Should(Succeed()) + + By("Verifying AppProject is NOT created by NamespaceReconciler") + failAppProj := &argov1alpha1.AppProject{} + failAppProjLookup := types.NamespacedName{Name: "non-existent-team", Namespace: "user-argocd"} + + // Wait a bit to ensure reconciliation has been attempted + time.Sleep(2 * time.Second) + + // AppProject should not exist + err := k8sClient.Get(ctx, failAppProjLookup, failAppProj) + Expect(err).Should(HaveOccurred()) + Expect(errors.IsNotFound(err)).Should(BeTrue(), + "AppProject should not be created by NamespaceReconciler - only ArgocdUserReconciler can create AppProjects") + }) + }) }) diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go index 204d3bf..9ec069e 100644 --- a/internal/controller/suite_test.go +++ b/internal/controller/suite_test.go @@ -24,6 +24,7 @@ import ( argov1alpha1 "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + userv1 "github.com/openshift/api/user/v1" "github.com/snapp-incubator/argocd-complementary-operator/internal/controller" corev1 "k8s.io/api/core/v1" "k8s.io/client-go/kubernetes/scheme" @@ -79,6 +80,9 @@ var _ = BeforeSuite(func() { err = argov1alpha1.AddToScheme(scheme.Scheme) Expect(err).NotTo(HaveOccurred()) + err = userv1.AddToScheme(scheme.Scheme) + Expect(err).NotTo(HaveOccurred()) + k8sClient, err = client.New(cfg, client.Options{Scheme: scheme.Scheme}) Expect(err).NotTo(HaveOccurred()) Expect(k8sClient).NotTo(BeNil()) @@ -94,6 +98,12 @@ var _ = BeforeSuite(func() { }).SetupWithManager(k8sManager) Expect(err).ToNot(HaveOccurred()) + err = (&controller.ArgocdUserReconciler{ + Client: k8sManager.GetClient(), + Scheme: k8sManager.GetScheme(), + }).SetupWithManager(k8sManager) + Expect(err).ToNot(HaveOccurred()) + // changing configuration according to the issue mentioned here: // https://github.com/kubernetes-sigs/controller-runtime/issues/1571 go func() {