From cf6def477523c38e970a322901972fe79c55e906 Mon Sep 17 00:00:00 2001 From: Alka Kumari <48486620+alkakumari016@users.noreply.github.com> Date: Sat, 26 Jul 2025 19:53:13 +0530 Subject: [PATCH 01/12] fix: reconcile operator owned labels for configMap and ignore non operator labels (#1737) * GITOPS-6285: reconcile operator owned labels for configMap and ignore non operator labels Signed-off-by: Alka Kumari * GITOPS-6285: added reconcilialtion logic for labels Signed-off-by: Alka Kumari * Removed a typo that resulted in test case failures Signed-off-by: Alka Kumari * GITOPS-6285: error handling for r.Client.Update Signed-off-by: Alka Kumari * GITOPS-6285: use r.Update Signed-off-by: Alka Kumari * GITOPS-6285: changed func name to UpdateMapValues and added test cases Signed-off-by: Alka Kumari * GITOPS-6285: removed addKubernetesData and replaced the references with UpdateMapValues Signed-off-by: Alka Kumari GITOPS-6285: removed addKubernetesData and replaced the references with UpdateMapValues * GITOPS-6285: removed labels update logic from applicationset and statefulset Signed-off-by: Alka Kumari * GITOPS-6285: changed comments Signed-off-by: Alka Kumari --------- Signed-off-by: Alka Kumari --- controllers/argocd/applicationset.go | 9 ++--- controllers/argocd/configmap.go | 25 +++++++++++++ controllers/argocd/deployment.go | 10 ++--- controllers/argocd/role.go | 6 +-- controllers/argocd/statefulset.go | 6 +-- controllers/argocd/util.go | 39 +++++++------------- controllers/argocd/util_test.go | 55 ++++++++++++++++------------ 7 files changed, 84 insertions(+), 66 deletions(-) diff --git a/controllers/argocd/applicationset.go b/controllers/argocd/applicationset.go index bae977463..9de102ae5 100644 --- a/controllers/argocd/applicationset.go +++ b/controllers/argocd/applicationset.go @@ -277,9 +277,8 @@ func (r *ReconcileArgoCD) reconcileApplicationSetDeployment(cr *argoproj.ArgoCD, AddSeccompProfileForOpenShift(r.Client, podSpec) if deplExists { - // Add Kubernetes-specific labels/annotations from the live object in the source to preserve metadata. - addKubernetesData(deploy.Spec.Template.Labels, existing.Spec.Template.Labels) - addKubernetesData(deploy.Spec.Template.Annotations, existing.Spec.Template.Annotations) + //Check if annotations have changed + UpdateMapValues(&existing.Spec.Template.Annotations, deploy.Spec.Template.Annotations) // If the Deployment already exists, make sure the values we care about are up-to-date deploymentsDifferent := identifyDeploymentDifference(*existing, *deploy) @@ -287,8 +286,8 @@ func (r *ReconcileArgoCD) reconcileApplicationSetDeployment(cr *argoproj.ArgoCD, existing.Spec.Template.Spec.Containers = podSpec.Containers existing.Spec.Template.Spec.Volumes = podSpec.Volumes existing.Spec.Template.Spec.ServiceAccountName = podSpec.ServiceAccountName - existing.Labels = deploy.Labels - existing.Spec.Template.Labels = deploy.Spec.Template.Labels + UpdateMapValues(&existing.Labels, deploy.Labels) + UpdateMapValues(&existing.Spec.Template.Labels, deploy.Spec.Template.Labels) existing.Spec.Selector = deploy.Spec.Selector existing.Spec.Template.Spec.NodeSelector = deploy.Spec.Template.Spec.NodeSelector existing.Spec.Template.Spec.Tolerations = deploy.Spec.Template.Spec.Tolerations diff --git a/controllers/argocd/configmap.go b/controllers/argocd/configmap.go index 26a13fcaa..148097ce2 100644 --- a/controllers/argocd/configmap.go +++ b/controllers/argocd/configmap.go @@ -343,6 +343,25 @@ func (r *ReconcileArgoCD) reconcileConfigMaps(cr *argoproj.ArgoCD, useTLSForRedi // This ConfigMap holds the CA Certificate data for client use. func (r *ReconcileArgoCD) reconcileCAConfigMap(cr *argoproj.ArgoCD) error { cm := newConfigMapWithName(getCAConfigMapName(cr), cr) + existingCM := &corev1.ConfigMap{} + exists, err := argoutil.IsObjectFound(r.Client, cr.Namespace, cm.Name, existingCM) + if err != nil { + return err + } + if exists { + changed := false + //Check if labels have changed + if UpdateMapValues(&existingCM.Labels, cm.GetLabels()) { + argoutil.LogResourceUpdate(log, existingCM, "updating", "CAConfigMap labels") + changed = true + if changed { + if err := r.Update(context.TODO(), existingCM); err != nil { + log.Error(err, "failed to update service object") + } + } + } + + } configMapExists, err := argoutil.IsObjectFound(r.Client, cr.Namespace, cm.Name, cm) if err != nil { @@ -535,6 +554,12 @@ func (r *ReconcileArgoCD) reconcileArgoConfigMap(cr *argoproj.ArgoCD) error { } changed := false + //Check if labels have changed + if UpdateMapValues(&existingCM.Labels, cm.GetLabels()) { + argoutil.LogResourceUpdate(log, existingCM, "updating", "ConfigMap labels") + changed = true + } + if !reflect.DeepEqual(cm.Data, existingCM.Data) { existingCM.Data = cm.Data changed = true diff --git a/controllers/argocd/deployment.go b/controllers/argocd/deployment.go index 50331d032..9a9c58e3c 100644 --- a/controllers/argocd/deployment.go +++ b/controllers/argocd/deployment.go @@ -1249,9 +1249,9 @@ func (r *ReconcileArgoCD) reconcileServerDeployment(cr *argoproj.ArgoCD, useTLSF } } - // Add Kubernetes-specific labels/annotations from the live object in the source to preserve metadata. - addKubernetesData(deploy.Spec.Template.Labels, existing.Spec.Template.Labels) - addKubernetesData(deploy.Spec.Template.Annotations, existing.Spec.Template.Annotations) + //Check if labels/annotations have changed + UpdateMapValues(&existing.Spec.Template.Labels, deploy.Spec.Template.Labels) + UpdateMapValues(&existing.Spec.Template.Annotations, deploy.Spec.Template.Annotations) if !reflect.DeepEqual(deploy.Spec.Template.Annotations, existing.Spec.Template.Annotations) { existing.Spec.Template.Annotations = deploy.Spec.Template.Annotations @@ -1261,8 +1261,8 @@ func (r *ReconcileArgoCD) reconcileServerDeployment(cr *argoproj.ArgoCD, useTLSF explanation += "annotations" changed = true } - if !reflect.DeepEqual(deploy.Spec.Template.Labels, existing.Spec.Template.Labels) { - existing.Spec.Template.Labels = deploy.Spec.Template.Labels + // Preserve non-operator labels in the existing deployment. + if UpdateMapValues(&existing.Spec.Template.Labels, deploy.Spec.Template.Labels) { if changed { explanation += ", " } diff --git a/controllers/argocd/role.go b/controllers/argocd/role.go index 91e804e43..31a141faf 100644 --- a/controllers/argocd/role.go +++ b/controllers/argocd/role.go @@ -514,8 +514,7 @@ func matchAggregatedClusterRoleFields(expectedClusterRole *v1.ClusterRole, exist // if ClusterRole is for View permissions then compare Labels if name == common.ArgoCDApplicationControllerComponentView { - if !reflect.DeepEqual(existingClusterRole.Labels, expectedClusterRole.Labels) { - existingClusterRole.Labels = expectedClusterRole.Labels + if UpdateMapValues(&existingClusterRole.Labels, expectedClusterRole.Labels) { if changed { explanation += ", " } @@ -535,8 +534,7 @@ func matchAggregatedClusterRoleFields(expectedClusterRole *v1.ClusterRole, exist changed = true } - if !reflect.DeepEqual(existingClusterRole.Labels, expectedClusterRole.Labels) { - existingClusterRole.Labels = expectedClusterRole.Labels + if UpdateMapValues(&existingClusterRole.Labels, expectedClusterRole.Labels) { if changed { explanation += ", " } diff --git a/controllers/argocd/statefulset.go b/controllers/argocd/statefulset.go index 48f3b4cba..198c0f420 100644 --- a/controllers/argocd/statefulset.go +++ b/controllers/argocd/statefulset.go @@ -984,9 +984,9 @@ func (r *ReconcileArgoCD) reconcileApplicationControllerStatefulSet(cr *argoproj changed = true } - // Add Kubernetes-specific labels/annotations from the live object in the source to preserve metadata. - addKubernetesData(ss.Spec.Template.Labels, existing.Spec.Template.Labels) - addKubernetesData(ss.Spec.Template.Annotations, existing.Spec.Template.Annotations) + //Check if labels/annotations have changed + UpdateMapValues(&existing.Spec.Template.Labels, ss.Spec.Template.Labels) + UpdateMapValues(&existing.Spec.Template.Annotations, ss.Spec.Template.Annotations) if !reflect.DeepEqual(ss.Spec.Template.Annotations, existing.Spec.Template.Annotations) { existing.Spec.Template.Annotations = ss.Spec.Template.Annotations diff --git a/controllers/argocd/util.go b/controllers/argocd/util.go index 300deefd5..febb7867b 100644 --- a/controllers/argocd/util.go +++ b/controllers/argocd/util.go @@ -1606,29 +1606,20 @@ func UseServer(name string, cr *argoproj.ArgoCD) bool { return true } -// addKubernetesData checks for any Kubernetes-specific labels or annotations -// in the live object and updates the source object to ensure critical metadata -// (like scheduling, topology, or lifecycle information) is retained. -// This helps avoid loss of important Kubernetes-managed metadata during updates. -func addKubernetesData(source map[string]string, live map[string]string) { - - // List of Kubernetes-specific substrings (wildcard match) - patterns := []string{ - "*kubernetes.io*", - "*k8s.io*", - "*openshift.io*", - } - - for key, value := range live { - found := glob.MatchStringInList(patterns, key, glob.GLOB) - if found { - // Don't override values already present in the source object. - // This allows the operator to update Kubernetes specific data when needed. - if _, ok := source[key]; !ok { - source[key] = value - } +// UpdateMapValues updates the values of an existing map with the values from a source map if they differ. +// It returns true if any values were changed. +func UpdateMapValues(existing *map[string]string, source map[string]string) bool { + changed := false + if *existing == nil { + *existing = make(map[string]string) + } + for key, value := range source { + if (*existing)[key] != value { + (*existing)[key] = value + changed = true } } + return changed } // updateStatusAndConditionsOfArgoCD will update .status field with provided param, and upsert .status.conditions with provided condition @@ -2267,8 +2258,7 @@ func (r *ReconcileArgoCD) reconcileDeploymentHelper(cr *argoproj.ArgoCD, desired deploymentChanged = true } - if !reflect.DeepEqual(existingDeployment.Labels, desiredDeployment.Labels) { - existingDeployment.Labels = desiredDeployment.Labels + if UpdateMapValues(&existingDeployment.Labels, desiredDeployment.Labels) { if deploymentChanged { explanation += ", " } @@ -2276,8 +2266,7 @@ func (r *ReconcileArgoCD) reconcileDeploymentHelper(cr *argoproj.ArgoCD, desired deploymentChanged = true } - if !reflect.DeepEqual(existingDeployment.Spec.Template.Labels, desiredDeployment.Spec.Template.Labels) { - existingDeployment.Spec.Template.Labels = desiredDeployment.Spec.Template.Labels + if UpdateMapValues(&existingDeployment.Spec.Template.Labels, desiredDeployment.Spec.Template.Labels) { if deploymentChanged { explanation += ", " } diff --git a/controllers/argocd/util_test.go b/controllers/argocd/util_test.go index 4d315b181..b929797e4 100644 --- a/controllers/argocd/util_test.go +++ b/controllers/argocd/util_test.go @@ -1169,53 +1169,60 @@ func TestReconcileArgoCD_reconcileDexOAuthClientSecret(t *testing.T) { assert.True(t, tokenExists, "Dex is enabled but unable to create oauth client secret") } -func TestRetainKubernetesData(t *testing.T) { +func TestUpdateMapValue(t *testing.T) { tests := []struct { name string source map[string]string - live map[string]string + existing map[string]string expected map[string]string }{ { - name: "Add Kubernetes-specific keys not in source", + name: "Retain non-operator-specific keys not in source", source: map[string]string{ - "custom-label": "custom-value", + "node.kubernetes.io/pod": "true", + "kubectl.kubernetes.io/restartedAt": "2024-12-05T09:46:46+05:30", + "openshift.openshift.io/restartedAt": "2024-12-05T09:46:46+05:30", }, - live: map[string]string{ + existing: map[string]string{ "node.kubernetes.io/pod": "true", "kubectl.kubernetes.io/restartedAt": "2024-12-05T09:46:46+05:30", "openshift.openshift.io/restartedAt": "2024-12-05T09:46:46+05:30", + "custom-label": "custom-value", }, expected: map[string]string{ - "custom-label": "custom-value", // unchanged - "node.kubernetes.io/pod": "true", // added - "kubectl.kubernetes.io/restartedAt": "2024-12-05T09:46:46+05:30", // added - "openshift.openshift.io/restartedAt": "2024-12-05T09:46:46+05:30", // added + "custom-label": "custom-value", // retained from existing + "node.kubernetes.io/pod": "true", // unchanged + "kubectl.kubernetes.io/restartedAt": "2024-12-05T09:46:46+05:30", // unchanged + "openshift.openshift.io/restartedAt": "2024-12-05T09:46:46+05:30", // unchanged }, }, { - name: "Ignores non-Kubernetes-specific keys", + name: "Override operator-specific keys in live with source", source: map[string]string{ - "custom-label": "custom-value", + "node.kubernetes.io/pod": "source-true", + "kubectl.kubernetes.io/restartedAt": "2024-12-05T09:46:46+05:30", }, - live: map[string]string{ - "non-k8s-key": "non-k8s-value", - "custom-label": "live-value", + existing: map[string]string{ + "node.kubernetes.io/pod": "live-true", // should override + "kubectl.kubernetes.io/restartedAt": "2024-12-05T09:46:46+05:30", }, expected: map[string]string{ - "custom-label": "custom-value", // unchanged + "node.kubernetes.io/pod": "source-true", + "kubectl.kubernetes.io/restartedAt": "2024-12-05T09:46:46+05:30", }, }, { - name: "Do not override existing Kubernetes-specific keys in source", + name: "Retain existing operator-specific keys from source", source: map[string]string{ - "node.kubernetes.io/pod": "source-true", + "node.kubernetes.io/pod": "source-true", + "kubectl.kubernetes.io/restartedAt": "2024-12-05T09:46:46+05:30", }, - live: map[string]string{ - "node.kubernetes.io/pod": "live-true", // should not override + existing: map[string]string{ + "node.kubernetes.io/pod": "source-true", }, expected: map[string]string{ - "node.kubernetes.io/pod": "source-true", // source takes precedence + "node.kubernetes.io/pod": "source-true", + "kubectl.kubernetes.io/restartedAt": "2024-12-05T09:46:46+05:30", }, }, { @@ -1223,7 +1230,7 @@ func TestRetainKubernetesData(t *testing.T) { source: map[string]string{ "custom-label": "custom-value", }, - live: map[string]string{}, + existing: map[string]string{}, expected: map[string]string{ "custom-label": "custom-value", // unchanged }, @@ -1231,7 +1238,7 @@ func TestRetainKubernetesData(t *testing.T) { { name: "Handles empty source map", source: map[string]string{}, - live: map[string]string{ + existing: map[string]string{ "openshift.io/resource": "value", }, expected: map[string]string{ @@ -1242,8 +1249,8 @@ func TestRetainKubernetesData(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - addKubernetesData(tt.source, tt.live) - assert.Equal(t, tt.expected, tt.source) + UpdateMapValues(&tt.existing, tt.source) + assert.Equal(t, tt.expected, tt.existing) }) } } From 3aad525db0008772f95d345dc493ab36cf58859d Mon Sep 17 00:00:00 2001 From: Alka Kumari Date: Mon, 11 Aug 2025 15:14:07 +0530 Subject: [PATCH 02/12] GITOPS-7405: added logic to remove custom labels from live objects if removed from argocd spec Signed-off-by: Alka Kumari --- controllers/argocd/argocd_controller.go | 6 + controllers/argocd/util.go | 285 +++++++++++++++++++++++- 2 files changed, 290 insertions(+), 1 deletion(-) diff --git a/controllers/argocd/argocd_controller.go b/controllers/argocd/argocd_controller.go index 40125fa92..55d35f617 100644 --- a/controllers/argocd/argocd_controller.go +++ b/controllers/argocd/argocd_controller.go @@ -348,6 +348,12 @@ func (r *ReconcileArgoCD) internalReconcile(ctx context.Context, request ctrl.Re } } + // Process any stored removed labels for cleanup from managed resources + if err := r.processRemovedLabels(argocd); err != nil { + reqLogger.Error(err, "failed to process removed labels cleanup") + // Don't fail the reconciliation, just log the error and continue + } + if err := r.reconcileResources(argocd, argoCDStatus); err != nil { // Error reconciling ArgoCD sub-resources - requeue the request. return reconcile.Result{}, argocd, argoCDStatus, err diff --git a/controllers/argocd/util.go b/controllers/argocd/util.go index febb7867b..afd6977dc 100644 --- a/controllers/argocd/util.go +++ b/controllers/argocd/util.go @@ -21,6 +21,7 @@ import ( "crypto/rand" "crypto/sha256" "encoding/base64" + "encoding/json" "fmt" "hash" "os" @@ -1030,7 +1031,7 @@ func (r *ReconcileArgoCD) setResourceWatches(bldr *builder.Builder, clusterResou } // Watch for changes to primary resource ArgoCD - bldr.For(&argoproj.ArgoCD{}, builder.WithPredicates(deleteNotificationsPred, r.argoCDNamespaceManagementFilterPredicate())) + bldr.For(&argoproj.ArgoCD{}, builder.WithPredicates(deleteNotificationsPred, r.argoCDNamespaceManagementFilterPredicate(), r.argoCDSpecLabelCleanupPredicate())) // Watch for changes to ConfigMap sub-resources owned by ArgoCD instances. bldr.Owns(&corev1.ConfigMap{}) @@ -1245,6 +1246,288 @@ func (r *ReconcileArgoCD) namespaceFilterPredicate() predicate.Predicate { } } +// argoCDSpecLabelCleanupPredicate tracks label changes in ArgoCD CR specs and cleans up removed labels from managed resources +func (r *ReconcileArgoCD) argoCDSpecLabelCleanupPredicate() predicate.Predicate { + return predicate.Funcs{ + UpdateFunc: func(e event.UpdateEvent) bool { + newCR, ok := e.ObjectNew.(*argoproj.ArgoCD) + if !ok { + return false + } + oldCR, ok := e.ObjectOld.(*argoproj.ArgoCD) + if !ok { + return false + } + + // Track removed labels from each component spec + removedLabels := r.calculateRemovedSpecLabels(oldCR, newCR) + + if len(removedLabels) > 0 { + log.Info("Detected removed labels from ArgoCD spec", + "argocd", fmt.Sprintf("%s/%s", newCR.Namespace, newCR.Name), + "removedLabels", removedLabels) + + // Store removed labels for cleanup during reconciliation + if err := r.storeRemovedLabelsForCleanup(newCR, removedLabels); err != nil { + log.Error(err, "failed to store removed labels for cleanup") + } + } + + return true // Trigger reconciliation for ArgoCD updates + }, + } +} + +const RemovedLabelsAnnotation = "argocd.argoproj.io/removed-labels" + +// calculateRemovedSpecLabels compares old and new ArgoCD specs and returns labels that were removed +func (r *ReconcileArgoCD) calculateRemovedSpecLabels(oldCR, newCR *argoproj.ArgoCD) map[string]string { + // Add nil checks to prevent panic + if r == nil || oldCR == nil || newCR == nil { + return map[string]string{} + } + removedLabels := make(map[string]string) + + // Check Server labels + oldServerLabels := oldCR.Spec.Server.Labels + newServerLabels := newCR.Spec.Server.Labels + for key, value := range oldServerLabels { + if newServerLabels == nil || newServerLabels[key] == "" { + removedLabels[key] = value + } + } + + // Check Repo Server labels + oldRepoLabels := oldCR.Spec.Repo.Labels + newRepoLabels := newCR.Spec.Repo.Labels + for key, value := range oldRepoLabels { + if newRepoLabels == nil || newRepoLabels[key] == "" { + removedLabels[key] = value + } + } + + // Check Controller labels + oldControllerLabels := oldCR.Spec.Controller.Labels + newControllerLabels := newCR.Spec.Controller.Labels + for key, value := range oldControllerLabels { + if newControllerLabels == nil || newControllerLabels[key] == "" { + removedLabels[key] = value + } + } + + // Check ApplicationSet labels + if oldCR.Spec.ApplicationSet != nil && newCR.Spec.ApplicationSet != nil { + oldAppSetLabels := oldCR.Spec.ApplicationSet.Labels + newAppSetLabels := newCR.Spec.ApplicationSet.Labels + for key, value := range oldAppSetLabels { + if newAppSetLabels == nil || newAppSetLabels[key] == "" { + removedLabels[key] = value + } + } + } + + // Note: Notifications and Prometheus specs don't have Labels fields currently + // If they are added in the future, the logic can be uncommented and updated + + return removedLabels +} + +func (r *ReconcileArgoCD) storeRemovedLabelsForCleanup(obj client.Object, removedLabels map[string]string) error { + if len(removedLabels) == 0 { + return nil + } + + // Serialize removed labels to JSON + removedLabelsJSON, err := json.Marshal(removedLabels) + if err != nil { + return fmt.Errorf("failed to marshal removed labels: %w", err) + } + + // Add annotation with removed labels + annotations := obj.GetAnnotations() + if annotations == nil { + annotations = make(map[string]string) + } + annotations[RemovedLabelsAnnotation] = string(removedLabelsJSON) + obj.SetAnnotations(annotations) + + // Update the object + return r.Client.Update(context.TODO(), obj) +} + +func (r *ReconcileArgoCD) processRemovedLabels(argocd *argoproj.ArgoCD) error { + annotations := argocd.GetAnnotations() + if annotations == nil { + return nil + } + + removedLabelsJSON, exists := annotations[RemovedLabelsAnnotation] + if !exists { + return nil + } + + var removedLabels map[string]string + if err := json.Unmarshal([]byte(removedLabelsJSON), &removedLabels); err != nil { + return fmt.Errorf("failed to unmarshal removed labels: %w", err) + } + + if len(removedLabels) > 0 { + // Clean up from managed resources + if err := r.cleanupLabelsFromManagedResources(argocd, removedLabels); err != nil { + return err + } + + // Clear the annotation + delete(annotations, RemovedLabelsAnnotation) + argocd.SetAnnotations(annotations) + return r.Client.Update(context.TODO(), argocd) + } + + return nil +} + +func (r *ReconcileArgoCD) cleanupLabelsFromManagedResources(argocd *argoproj.ArgoCD, labelsToRemove map[string]string) error { + log.Info("Cleaning up removed labels from managed resources", + "argocd", fmt.Sprintf("%s/%s", argocd.Namespace, argocd.Name), + "labelsToRemove", labelsToRemove) + + // Clean up labels from ArgoCD Server Deployment + if err := r.cleanupLabelsFromComponent(argocd, labelsToRemove, "server", &appsv1.Deployment{}); err != nil { + log.Error(err, "failed to cleanup labels from server deployment") + } + + // Clean up labels from ArgoCD Repo Server Deployment + if err := r.cleanupLabelsFromComponent(argocd, labelsToRemove, "repo-server", &appsv1.Deployment{}); err != nil { + log.Error(err, "failed to cleanup labels from repo-server deployment") + } + + // Clean up labels from ArgoCD Application Controller StatefulSet + if err := r.cleanupLabelsFromComponent(argocd, labelsToRemove, "application-controller", &appsv1.StatefulSet{}); err != nil { + log.Error(err, "failed to cleanup labels from application-controller statefulset") + } + + // Clean up labels from ArgoCD ApplicationSet Controller Deployment + if err := r.cleanupLabelsFromComponent(argocd, labelsToRemove, "applicationset-controller", &appsv1.Deployment{}); err != nil { + log.Error(err, "failed to cleanup labels from applicationset-controller deployment") + } + + // Clean up labels from ArgoCD Notifications Controller Deployment (if enabled) + if argocd.Spec.Notifications.Enabled { + if err := r.cleanupLabelsFromComponent(argocd, labelsToRemove, "notifications-controller", &appsv1.Deployment{}); err != nil { + log.Error(err, "failed to cleanup labels from notifications-controller deployment") + } + } + + // Use existing pattern for other managed resources + selector, err := argocdInstanceSelector(argocd.Name) + if err != nil { + return err + } + + // Clean up ConfigMaps + configMapList := &corev1.ConfigMapList{} + if err := filterObjectsBySelector(r.Client, configMapList, selector); err == nil { + for _, cm := range configMapList.Items { + if err := r.removeLabelsFromObject(&cm, labelsToRemove); err != nil { + log.Error(err, "failed to remove labels from ConfigMap", "name", cm.Name) + } + } + } + + // Add other resource types as needed... + return nil +} + +// cleanupLabelsFromComponent removes specified labels from a specific ArgoCD component +func (r *ReconcileArgoCD) cleanupLabelsFromComponent(argocd *argoproj.ArgoCD, labelsToRemove map[string]string, componentName string, obj client.Object) error { + resourceName := fmt.Sprintf("%s-%s", argocd.Name, componentName) + key := types.NamespacedName{ + Namespace: argocd.Namespace, + Name: resourceName, + } + + if err := r.Client.Get(context.TODO(), key, obj); err != nil { + if apierrors.IsNotFound(err) { + // Component doesn't exist, nothing to clean up + return nil + } + return fmt.Errorf("failed to get %s: %w", componentName, err) + } + + // Remove labels from the resource itself + if err := r.removeLabelsFromObject(obj, labelsToRemove); err != nil { + return fmt.Errorf("failed to remove labels from %s: %w", componentName, err) + } + + // For Deployments and StatefulSets, also clean labels from pod template + switch resource := obj.(type) { + case *appsv1.Deployment: + podTemplateLabels := resource.Spec.Template.Labels + if podTemplateLabels != nil { + modified := false + for labelKey := range labelsToRemove { + if _, exists := podTemplateLabels[labelKey]; exists { + delete(podTemplateLabels, labelKey) + modified = true + } + } + if modified { + resource.Spec.Template.Labels = podTemplateLabels + if err := r.Client.Update(context.TODO(), resource); err != nil { + return fmt.Errorf("failed to update pod template labels for %s deployment: %w", componentName, err) + } + log.Info("Removed labels from pod template", + "component", componentName, + "removedLabels", labelsToRemove) + } + } + case *appsv1.StatefulSet: + podTemplateLabels := resource.Spec.Template.Labels + if podTemplateLabels != nil { + modified := false + for labelKey := range labelsToRemove { + if _, exists := podTemplateLabels[labelKey]; exists { + delete(podTemplateLabels, labelKey) + modified = true + } + } + if modified { + resource.Spec.Template.Labels = podTemplateLabels + if err := r.Client.Update(context.TODO(), resource); err != nil { + return fmt.Errorf("failed to update pod template labels for %s statefulset: %w", componentName, err) + } + log.Info("Removed labels from pod template", + "component", componentName, + "removedLabels", labelsToRemove) + } + } + } + + return nil +} + +func (r *ReconcileArgoCD) removeLabelsFromObject(obj client.Object, labelsToRemove map[string]string) error { + currentLabels := obj.GetLabels() + if currentLabels == nil { + return nil + } + + modified := false + for labelKey := range labelsToRemove { + if _, exists := currentLabels[labelKey]; exists { + delete(currentLabels, labelKey) + modified = true + } + } + + if modified { + obj.SetLabels(currentLabels) + return r.Client.Update(context.TODO(), obj) + } + + return nil +} + // deleteRBACsForNamespace deletes the RBACs when the label from the namespace is removed. func deleteRBACsForNamespace(sourceNS string, k8sClient kubernetes.Interface) error { log.Info(fmt.Sprintf("Removing the RBACs created for the namespace: %s", sourceNS)) From 82f74d0522c3d3cd405347f86a1f3bf31fcd69d9 Mon Sep 17 00:00:00 2001 From: Alka Kumari Date: Mon, 11 Aug 2025 15:43:32 +0530 Subject: [PATCH 03/12] replace r.Client.Update with r.Update Signed-off-by: Alka Kumari --- controllers/argocd/util.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/controllers/argocd/util.go b/controllers/argocd/util.go index afd6977dc..b0c732427 100644 --- a/controllers/argocd/util.go +++ b/controllers/argocd/util.go @@ -1352,7 +1352,7 @@ func (r *ReconcileArgoCD) storeRemovedLabelsForCleanup(obj client.Object, remove obj.SetAnnotations(annotations) // Update the object - return r.Client.Update(context.TODO(), obj) + return r.Update(context.TODO(), obj) } func (r *ReconcileArgoCD) processRemovedLabels(argocd *argoproj.ArgoCD) error { @@ -1380,7 +1380,7 @@ func (r *ReconcileArgoCD) processRemovedLabels(argocd *argoproj.ArgoCD) error { // Clear the annotation delete(annotations, RemovedLabelsAnnotation) argocd.SetAnnotations(annotations) - return r.Client.Update(context.TODO(), argocd) + return r.Update(context.TODO(), argocd) } return nil @@ -1446,7 +1446,7 @@ func (r *ReconcileArgoCD) cleanupLabelsFromComponent(argocd *argoproj.ArgoCD, la Name: resourceName, } - if err := r.Client.Get(context.TODO(), key, obj); err != nil { + if err := r.Get(context.TODO(), key, obj); err != nil { if apierrors.IsNotFound(err) { // Component doesn't exist, nothing to clean up return nil @@ -1473,7 +1473,7 @@ func (r *ReconcileArgoCD) cleanupLabelsFromComponent(argocd *argoproj.ArgoCD, la } if modified { resource.Spec.Template.Labels = podTemplateLabels - if err := r.Client.Update(context.TODO(), resource); err != nil { + if err := r.Update(context.TODO(), resource); err != nil { return fmt.Errorf("failed to update pod template labels for %s deployment: %w", componentName, err) } log.Info("Removed labels from pod template", @@ -1493,7 +1493,7 @@ func (r *ReconcileArgoCD) cleanupLabelsFromComponent(argocd *argoproj.ArgoCD, la } if modified { resource.Spec.Template.Labels = podTemplateLabels - if err := r.Client.Update(context.TODO(), resource); err != nil { + if err := r.Update(context.TODO(), resource); err != nil { return fmt.Errorf("failed to update pod template labels for %s statefulset: %w", componentName, err) } log.Info("Removed labels from pod template", @@ -1522,7 +1522,7 @@ func (r *ReconcileArgoCD) removeLabelsFromObject(obj client.Object, labelsToRemo if modified { obj.SetLabels(currentLabels) - return r.Client.Update(context.TODO(), obj) + return r.Update(context.TODO(), obj) } return nil From dd8d79d11421b4436fec81be2c24b0b1fcc7d74e Mon Sep 17 00:00:00 2001 From: Alka Kumari Date: Tue, 2 Sep 2025 17:55:12 +0530 Subject: [PATCH 04/12] changed logic to remove custom labels if removed from CR Signed-off-by: Alka Kumari --- controllers/argocd/argocd_controller.go | 6 +- controllers/argocd/util.go | 422 ++++++++++++------------ 2 files changed, 219 insertions(+), 209 deletions(-) diff --git a/controllers/argocd/argocd_controller.go b/controllers/argocd/argocd_controller.go index 55d35f617..58f012bfa 100644 --- a/controllers/argocd/argocd_controller.go +++ b/controllers/argocd/argocd_controller.go @@ -348,9 +348,9 @@ func (r *ReconcileArgoCD) internalReconcile(ctx context.Context, request ctrl.Re } } - // Process any stored removed labels for cleanup from managed resources - if err := r.processRemovedLabels(argocd); err != nil { - reqLogger.Error(err, "failed to process removed labels cleanup") + // Process DropMetadata for namespace-based label cleanup + if err := r.processDropMetadataForCleanup(argocd); err != nil { + reqLogger.Error(err, "failed to process DropMetadata cleanup") // Don't fail the reconciliation, just log the error and continue } diff --git a/controllers/argocd/util.go b/controllers/argocd/util.go index b0c732427..f572826e4 100644 --- a/controllers/argocd/util.go +++ b/controllers/argocd/util.go @@ -21,7 +21,6 @@ import ( "crypto/rand" "crypto/sha256" "encoding/base64" - "encoding/json" "fmt" "hash" "os" @@ -1246,286 +1245,297 @@ func (r *ReconcileArgoCD) namespaceFilterPredicate() predicate.Predicate { } } -// argoCDSpecLabelCleanupPredicate tracks label changes in ArgoCD CR specs and cleans up removed labels from managed resources -func (r *ReconcileArgoCD) argoCDSpecLabelCleanupPredicate() predicate.Predicate { - return predicate.Funcs{ - UpdateFunc: func(e event.UpdateEvent) bool { - newCR, ok := e.ObjectNew.(*argoproj.ArgoCD) - if !ok { - return false - } - oldCR, ok := e.ObjectOld.(*argoproj.ArgoCD) - if !ok { - return false - } +// DropMetadata stores metadata about removed labels for a specific resource type +type DropMetadata struct { + Resource string `json:"resource"` + Keys []string `json:"keys"` +} - // Track removed labels from each component spec - removedLabels := r.calculateRemovedSpecLabels(oldCR, newCR) +// Global map to store removed labels per namespace +// Key: namespace, Value: slice of DropMetadata for different resources +var DropMetadataStore = make(map[string][]DropMetadata) - if len(removedLabels) > 0 { - log.Info("Detected removed labels from ArgoCD spec", - "argocd", fmt.Sprintf("%s/%s", newCR.Namespace, newCR.Name), - "removedLabels", removedLabels) +// addDropMetadata adds removed label information to the DropMetadataStore for a specific namespace +func addDropMetadata(namespace, resource string, keys []string) { + if len(keys) == 0 { + return + } - // Store removed labels for cleanup during reconciliation - if err := r.storeRemovedLabelsForCleanup(newCR, removedLabels); err != nil { - log.Error(err, "failed to store removed labels for cleanup") - } - } + dropMeta := DropMetadata{ + Resource: resource, + Keys: keys, + } - return true // Trigger reconciliation for ArgoCD updates - }, + if DropMetadataStore[namespace] == nil { + DropMetadataStore[namespace] = []DropMetadata{} } -} -const RemovedLabelsAnnotation = "argocd.argoproj.io/removed-labels" + // Check if the resource already exists in the slice and update it + found := false + for i, existing := range DropMetadataStore[namespace] { + if existing.Resource == resource { + // Merge keys if resource already exists + keySet := make(map[string]bool) + for _, key := range existing.Keys { + keySet[key] = true + } + for _, key := range keys { + keySet[key] = true + } -// calculateRemovedSpecLabels compares old and new ArgoCD specs and returns labels that were removed -func (r *ReconcileArgoCD) calculateRemovedSpecLabels(oldCR, newCR *argoproj.ArgoCD) map[string]string { - // Add nil checks to prevent panic - if r == nil || oldCR == nil || newCR == nil { - return map[string]string{} - } - removedLabels := make(map[string]string) + var mergedKeys []string + for key := range keySet { + mergedKeys = append(mergedKeys, key) + } - // Check Server labels - oldServerLabels := oldCR.Spec.Server.Labels - newServerLabels := newCR.Spec.Server.Labels - for key, value := range oldServerLabels { - if newServerLabels == nil || newServerLabels[key] == "" { - removedLabels[key] = value + DropMetadataStore[namespace][i].Keys = mergedKeys + found = true + break } } - // Check Repo Server labels - oldRepoLabels := oldCR.Spec.Repo.Labels - newRepoLabels := newCR.Spec.Repo.Labels - for key, value := range oldRepoLabels { - if newRepoLabels == nil || newRepoLabels[key] == "" { - removedLabels[key] = value - } + if !found { + DropMetadataStore[namespace] = append(DropMetadataStore[namespace], dropMeta) } +} - // Check Controller labels - oldControllerLabels := oldCR.Spec.Controller.Labels - newControllerLabels := newCR.Spec.Controller.Labels - for key, value := range oldControllerLabels { - if newControllerLabels == nil || newControllerLabels[key] == "" { - removedLabels[key] = value - } - } +// getDropMetadataForNamespace retrieves all DropMetadata for a specific namespace +func getDropMetadataForNamespace(namespace string) []DropMetadata { + return DropMetadataStore[namespace] +} - // Check ApplicationSet labels - if oldCR.Spec.ApplicationSet != nil && newCR.Spec.ApplicationSet != nil { - oldAppSetLabels := oldCR.Spec.ApplicationSet.Labels - newAppSetLabels := newCR.Spec.ApplicationSet.Labels - for key, value := range oldAppSetLabels { - if newAppSetLabels == nil || newAppSetLabels[key] == "" { - removedLabels[key] = value +// removeDropMetadataForNamespace removes all DropMetadata for a specific namespace +func removeDropMetadataForNamespace(namespace string) { + delete(DropMetadataStore, namespace) +} + +// getDropMetadataForResource retrieves DropMetadata for a specific resource in a namespace +func getDropMetadataForResource(namespace, resource string) *DropMetadata { + if metadataList, exists := DropMetadataStore[namespace]; exists { + for _, metadata := range metadataList { + if metadata.Resource == resource { + return &metadata } } } - - // Note: Notifications and Prometheus specs don't have Labels fields currently - // If they are added in the future, the logic can be uncommented and updated - - return removedLabels + return nil } -func (r *ReconcileArgoCD) storeRemovedLabelsForCleanup(obj client.Object, removedLabels map[string]string) error { - if len(removedLabels) == 0 { - return nil - } +// processDropMetadataForCleanup processes the DropMetadata store and cleans up removed labels from resources +func (r *ReconcileArgoCD) processDropMetadataForCleanup(argocd *argoproj.ArgoCD) error { + namespace := argocd.GetNamespace() + dropMetadataList := getDropMetadataForNamespace(namespace) - // Serialize removed labels to JSON - removedLabelsJSON, err := json.Marshal(removedLabels) - if err != nil { - return fmt.Errorf("failed to marshal removed labels: %w", err) + if len(dropMetadataList) == 0 { + return nil } - // Add annotation with removed labels - annotations := obj.GetAnnotations() - if annotations == nil { - annotations = make(map[string]string) + log.Info("Processing DropMetadata for cleanup", + "argocd", fmt.Sprintf("%s/%s", argocd.Namespace, argocd.Name), + "namespace", namespace, + "dropMetadataEntries", len(dropMetadataList)) + + for _, dropMeta := range dropMetadataList { + if err := r.cleanupLabelsFromResourceType(argocd, dropMeta); err != nil { + log.Error(err, "failed to cleanup labels from resource type", + "resource", dropMeta.Resource, + "keys", dropMeta.Keys) + continue + } } - annotations[RemovedLabelsAnnotation] = string(removedLabelsJSON) - obj.SetAnnotations(annotations) - // Update the object - return r.Update(context.TODO(), obj) + // Clear the DropMetadata for this namespace after cleanup + removeDropMetadataForNamespace(namespace) + return nil } -func (r *ReconcileArgoCD) processRemovedLabels(argocd *argoproj.ArgoCD) error { - annotations := argocd.GetAnnotations() - if annotations == nil { - return nil - } +// cleanupLabelsFromResourceType removes labels from specific resource types +func (r *ReconcileArgoCD) cleanupLabelsFromResourceType(argocd *argoproj.ArgoCD, dropMeta DropMetadata) error { - removedLabelsJSON, exists := annotations[RemovedLabelsAnnotation] - if !exists { - return nil - } + switch dropMeta.Resource { + case "server-deployment": + // Clean up from ArgoCD server deployment only + deploymentName := fmt.Sprintf("%s-server", argocd.Name) + if err := r.cleanupLabelsFromDeployment(argocd, deploymentName, dropMeta.Keys); err != nil { + return fmt.Errorf("failed to cleanup labels from server deployment: %w", err) + } - var removedLabels map[string]string - if err := json.Unmarshal([]byte(removedLabelsJSON), &removedLabels); err != nil { - return fmt.Errorf("failed to unmarshal removed labels: %w", err) - } + case "repo-deployment": + // Clean up from ArgoCD repo server deployment only + deploymentName := fmt.Sprintf("%s-repo-server", argocd.Name) + if err := r.cleanupLabelsFromDeployment(argocd, deploymentName, dropMeta.Keys); err != nil { + return fmt.Errorf("failed to cleanup labels from repo deployment: %w", err) + } - if len(removedLabels) > 0 { - // Clean up from managed resources - if err := r.cleanupLabelsFromManagedResources(argocd, removedLabels); err != nil { - return err + case "applicationset-deployment": + // Clean up from ArgoCD applicationset controller deployment only + deploymentName := fmt.Sprintf("%s-applicationset-controller", argocd.Name) + if err := r.cleanupLabelsFromDeployment(argocd, deploymentName, dropMeta.Keys); err != nil { + return fmt.Errorf("failed to cleanup labels from applicationset deployment: %w", err) } - // Clear the annotation - delete(annotations, RemovedLabelsAnnotation) - argocd.SetAnnotations(annotations) - return r.Update(context.TODO(), argocd) + case "notifications-deployment": + // Clean up from ArgoCD notifications controller deployment only + if argocd.Spec.Notifications.Enabled { + deploymentName := fmt.Sprintf("%s-notifications-controller", argocd.Name) + if err := r.cleanupLabelsFromDeployment(argocd, deploymentName, dropMeta.Keys); err != nil { + return fmt.Errorf("failed to cleanup labels from notifications deployment: %w", err) + } + } + + case "controller-statefulset": + // Clean up from ArgoCD application controller StatefulSet + statefulSetName := fmt.Sprintf("%s-application-controller", argocd.Name) + if err := r.cleanupLabelsFromStatefulSet(argocd, statefulSetName, dropMeta.Keys); err != nil { + return fmt.Errorf("failed to cleanup labels from controller statefulset: %w", err) + } + + default: + log.Info("Unknown resource type for cleanup", "resourceType", dropMeta.Resource) } return nil } -func (r *ReconcileArgoCD) cleanupLabelsFromManagedResources(argocd *argoproj.ArgoCD, labelsToRemove map[string]string) error { - log.Info("Cleaning up removed labels from managed resources", - "argocd", fmt.Sprintf("%s/%s", argocd.Namespace, argocd.Name), - "labelsToRemove", labelsToRemove) - - // Clean up labels from ArgoCD Server Deployment - if err := r.cleanupLabelsFromComponent(argocd, labelsToRemove, "server", &appsv1.Deployment{}); err != nil { - log.Error(err, "failed to cleanup labels from server deployment") - } +// cleanupLabelsFromDeployment removes labels from a specific deployment +func (r *ReconcileArgoCD) cleanupLabelsFromDeployment(argocd *argoproj.ArgoCD, deploymentName string, labelsToRemove []string) error { + namespace := argocd.GetNamespace() + deployment := &appsv1.Deployment{} + key := types.NamespacedName{Namespace: namespace, Name: deploymentName} - // Clean up labels from ArgoCD Repo Server Deployment - if err := r.cleanupLabelsFromComponent(argocd, labelsToRemove, "repo-server", &appsv1.Deployment{}); err != nil { - log.Error(err, "failed to cleanup labels from repo-server deployment") + if err := r.Get(context.TODO(), key, deployment); err != nil { + if apierrors.IsNotFound(err) { + log.Info("Deployment not found, skipping label cleanup", "name", deploymentName) + return nil // Deployment doesn't exist, skip + } + return fmt.Errorf("failed to get deployment %s: %w", deploymentName, err) } - // Clean up labels from ArgoCD Application Controller StatefulSet - if err := r.cleanupLabelsFromComponent(argocd, labelsToRemove, "application-controller", &appsv1.StatefulSet{}); err != nil { - log.Error(err, "failed to cleanup labels from application-controller statefulset") + if currentLabels, err := r.removeLabelsFromObject(deployment, labelsToRemove); err != nil { + log.Error(err, "failed to remove labels from deployment", "name", deploymentName) + return err + } else { + deployment.Spec.Template.Labels = currentLabels + if err := r.Update(context.TODO(), deployment); err != nil { + log.Error(err, "failed to update deployment after removing labels", "name", deploymentName) + return err + } } + return nil +} - // Clean up labels from ArgoCD ApplicationSet Controller Deployment - if err := r.cleanupLabelsFromComponent(argocd, labelsToRemove, "applicationset-controller", &appsv1.Deployment{}); err != nil { - log.Error(err, "failed to cleanup labels from applicationset-controller deployment") - } +// cleanupLabelsFromStatefulSet removes labels from a specific statefulset +func (r *ReconcileArgoCD) cleanupLabelsFromStatefulSet(argocd *argoproj.ArgoCD, statefulSetName string, labelsToRemove []string) error { + namespace := argocd.GetNamespace() + statefulSet := &appsv1.StatefulSet{} + key := types.NamespacedName{Namespace: namespace, Name: statefulSetName} - // Clean up labels from ArgoCD Notifications Controller Deployment (if enabled) - if argocd.Spec.Notifications.Enabled { - if err := r.cleanupLabelsFromComponent(argocd, labelsToRemove, "notifications-controller", &appsv1.Deployment{}); err != nil { - log.Error(err, "failed to cleanup labels from notifications-controller deployment") + if err := r.Get(context.TODO(), key, statefulSet); err != nil { + if apierrors.IsNotFound(err) { + log.Info("StatefulSet not found, skipping label cleanup", "name", statefulSetName) + return nil // StatefulSet doesn't exist, nothing to clean } + return fmt.Errorf("failed to get statefulset %s: %w", statefulSetName, err) } - // Use existing pattern for other managed resources - selector, err := argocdInstanceSelector(argocd.Name) - if err != nil { + if currentLabels, err := r.removeLabelsFromObject(statefulSet, labelsToRemove); err != nil { + log.Error(err, "failed to remove labels from statefulset", "name", statefulSetName) return err - } - - // Clean up ConfigMaps - configMapList := &corev1.ConfigMapList{} - if err := filterObjectsBySelector(r.Client, configMapList, selector); err == nil { - for _, cm := range configMapList.Items { - if err := r.removeLabelsFromObject(&cm, labelsToRemove); err != nil { - log.Error(err, "failed to remove labels from ConfigMap", "name", cm.Name) - } + } else { + statefulSet.SetLabels(currentLabels) + if err := r.Update(context.TODO(), statefulSet); err != nil { + log.Error(err, "failed to update statefulset after removing labels", "name", statefulSetName) + return err } } - - // Add other resource types as needed... return nil } -// cleanupLabelsFromComponent removes specified labels from a specific ArgoCD component -func (r *ReconcileArgoCD) cleanupLabelsFromComponent(argocd *argoproj.ArgoCD, labelsToRemove map[string]string, componentName string, obj client.Object) error { - resourceName := fmt.Sprintf("%s-%s", argocd.Name, componentName) - key := types.NamespacedName{ - Namespace: argocd.Namespace, - Name: resourceName, - } +// argoCDSpecLabelCleanupPredicate tracks label changes in ArgoCD CR specs and cleans up removed labels from managed resources +func (r *ReconcileArgoCD) argoCDSpecLabelCleanupPredicate() predicate.Predicate { + return predicate.Funcs{ + UpdateFunc: func(e event.UpdateEvent) bool { + newCR, ok := e.ObjectNew.(*argoproj.ArgoCD) + if !ok { + return false + } + oldCR, ok := e.ObjectOld.(*argoproj.ArgoCD) + if !ok { + return false + } - if err := r.Get(context.TODO(), key, obj); err != nil { - if apierrors.IsNotFound(err) { - // Component doesn't exist, nothing to clean up - return nil - } - return fmt.Errorf("failed to get %s: %w", componentName, err) + // Track removed labels from each component spec + r.calculateRemovedSpecLabels(oldCR, newCR) + + return true // Trigger reconciliation for ArgoCD updates + }, } +} - // Remove labels from the resource itself - if err := r.removeLabelsFromObject(obj, labelsToRemove); err != nil { - return fmt.Errorf("failed to remove labels from %s: %w", componentName, err) +// calculateRemovedSpecLabels compares old and new ArgoCD specs and tracks removed labels by resource type +func (r *ReconcileArgoCD) calculateRemovedSpecLabels(oldCR, newCR *argoproj.ArgoCD) { + // Add nil checks to prevent panic + if r == nil || oldCR == nil || newCR == nil { + return } + //removedLabels := make(map[string]string) + namespace := newCR.GetNamespace() - // For Deployments and StatefulSets, also clean labels from pod template - switch resource := obj.(type) { - case *appsv1.Deployment: - podTemplateLabels := resource.Spec.Template.Labels - if podTemplateLabels != nil { - modified := false - for labelKey := range labelsToRemove { - if _, exists := podTemplateLabels[labelKey]; exists { - delete(podTemplateLabels, labelKey) - modified = true - } - } - if modified { - resource.Spec.Template.Labels = podTemplateLabels - if err := r.Update(context.TODO(), resource); err != nil { - return fmt.Errorf("failed to update pod template labels for %s deployment: %w", componentName, err) - } - log.Info("Removed labels from pod template", - "component", componentName, - "removedLabels", labelsToRemove) + // Helper function to check and store removed labels for a specific resource + checkAndStoreRemovedLabels := func(oldLabels, newLabels map[string]string, resourceType string) { + var removedKeys []string + for key := range oldLabels { + if newLabels == nil || newLabels[key] == "" { + removedKeys = append(removedKeys, key) } } - case *appsv1.StatefulSet: - podTemplateLabels := resource.Spec.Template.Labels - if podTemplateLabels != nil { - modified := false - for labelKey := range labelsToRemove { - if _, exists := podTemplateLabels[labelKey]; exists { - delete(podTemplateLabels, labelKey) - modified = true - } - } - if modified { - resource.Spec.Template.Labels = podTemplateLabels - if err := r.Update(context.TODO(), resource); err != nil { - return fmt.Errorf("failed to update pod template labels for %s statefulset: %w", componentName, err) - } - log.Info("Removed labels from pod template", - "component", componentName, - "removedLabels", labelsToRemove) - } + // Store removed keys for this resource type in the namespace + if len(removedKeys) > 0 { + addDropMetadata(namespace, resourceType, removedKeys) } } - return nil + // Check Server labels + checkAndStoreRemovedLabels(oldCR.Spec.Server.Labels, newCR.Spec.Server.Labels, "server-deployment") + + // Check Repo Server labels + checkAndStoreRemovedLabels(oldCR.Spec.Repo.Labels, newCR.Spec.Repo.Labels, "repo-deployment") + + // Check Controller labels (StatefulSet) + checkAndStoreRemovedLabels(oldCR.Spec.Controller.Labels, newCR.Spec.Controller.Labels, "controller-statefulset") + + // Check ApplicationSet labels + if oldCR.Spec.ApplicationSet != nil && newCR.Spec.ApplicationSet != nil { + checkAndStoreRemovedLabels(oldCR.Spec.ApplicationSet.Labels, newCR.Spec.ApplicationSet.Labels, "applicationset-deployment") + } + + // Check Notifications labels (if we add support for notifications labels in the future) + // if oldCR.Spec.Notifications != nil && newCR.Spec.Notifications != nil { + // checkAndStoreRemovedLabels(oldCR.Spec.Notifications.Labels, newCR.Spec.Notifications.Labels, "notifications-deployment") + // } + + // Note: Notifications and Prometheus specs don't have Labels fields currently + // If they are added in the future, the logic can be uncommented and updated + } -func (r *ReconcileArgoCD) removeLabelsFromObject(obj client.Object, labelsToRemove map[string]string) error { +func (r *ReconcileArgoCD) removeLabelsFromObject(obj client.Object, labelsToRemove []string) (map[string]string, error) { currentLabels := obj.GetLabels() if currentLabels == nil { - return nil + return nil, nil } modified := false - for labelKey := range labelsToRemove { + for _, labelKey := range labelsToRemove { if _, exists := currentLabels[labelKey]; exists { delete(currentLabels, labelKey) modified = true } } - if modified { - obj.SetLabels(currentLabels) - return r.Update(context.TODO(), obj) + return currentLabels, nil } - - return nil + return currentLabels, nil } // deleteRBACsForNamespace deletes the RBACs when the label from the namespace is removed. From 9fa83d2c39c1609f088b6e656a389af13f58366e Mon Sep 17 00:00:00 2001 From: Alka Kumari Date: Tue, 2 Sep 2025 20:14:57 +0530 Subject: [PATCH 05/12] removed unused var and methods Signed-off-by: Alka Kumari --- controllers/argocd/argocd_controller.go | 5 +-- controllers/argocd/util.go | 52 +++++++------------------ 2 files changed, 15 insertions(+), 42 deletions(-) diff --git a/controllers/argocd/argocd_controller.go b/controllers/argocd/argocd_controller.go index 58f012bfa..e69cd074f 100644 --- a/controllers/argocd/argocd_controller.go +++ b/controllers/argocd/argocd_controller.go @@ -349,10 +349,7 @@ func (r *ReconcileArgoCD) internalReconcile(ctx context.Context, request ctrl.Re } // Process DropMetadata for namespace-based label cleanup - if err := r.processDropMetadataForCleanup(argocd); err != nil { - reqLogger.Error(err, "failed to process DropMetadata cleanup") - // Don't fail the reconciliation, just log the error and continue - } + r.processDropMetadataForCleanup(argocd) if err := r.reconcileResources(argocd, argoCDStatus); err != nil { // Error reconciling ArgoCD sub-resources - requeue the request. diff --git a/controllers/argocd/util.go b/controllers/argocd/util.go index f572826e4..da2f1c0e1 100644 --- a/controllers/argocd/util.go +++ b/controllers/argocd/util.go @@ -1309,27 +1309,11 @@ func removeDropMetadataForNamespace(namespace string) { delete(DropMetadataStore, namespace) } -// getDropMetadataForResource retrieves DropMetadata for a specific resource in a namespace -func getDropMetadataForResource(namespace, resource string) *DropMetadata { - if metadataList, exists := DropMetadataStore[namespace]; exists { - for _, metadata := range metadataList { - if metadata.Resource == resource { - return &metadata - } - } - } - return nil -} - // processDropMetadataForCleanup processes the DropMetadata store and cleans up removed labels from resources -func (r *ReconcileArgoCD) processDropMetadataForCleanup(argocd *argoproj.ArgoCD) error { +func (r *ReconcileArgoCD) processDropMetadataForCleanup(argocd *argoproj.ArgoCD) { namespace := argocd.GetNamespace() dropMetadataList := getDropMetadataForNamespace(namespace) - if len(dropMetadataList) == 0 { - return nil - } - log.Info("Processing DropMetadata for cleanup", "argocd", fmt.Sprintf("%s/%s", argocd.Namespace, argocd.Name), "namespace", namespace, @@ -1346,7 +1330,6 @@ func (r *ReconcileArgoCD) processDropMetadataForCleanup(argocd *argoproj.ArgoCD) // Clear the DropMetadata for this namespace after cleanup removeDropMetadataForNamespace(namespace) - return nil } // cleanupLabelsFromResourceType removes labels from specific resource types @@ -1411,15 +1394,11 @@ func (r *ReconcileArgoCD) cleanupLabelsFromDeployment(argocd *argoproj.ArgoCD, d return fmt.Errorf("failed to get deployment %s: %w", deploymentName, err) } - if currentLabels, err := r.removeLabelsFromObject(deployment, labelsToRemove); err != nil { - log.Error(err, "failed to remove labels from deployment", "name", deploymentName) + currentLabels := r.removeLabelsFromObject(deployment, labelsToRemove) + deployment.Spec.Template.Labels = currentLabels + if err := r.Update(context.TODO(), deployment); err != nil { + log.Error(err, "failed to update deployment after removing labels", "name", deploymentName) return err - } else { - deployment.Spec.Template.Labels = currentLabels - if err := r.Update(context.TODO(), deployment); err != nil { - log.Error(err, "failed to update deployment after removing labels", "name", deploymentName) - return err - } } return nil } @@ -1438,16 +1417,13 @@ func (r *ReconcileArgoCD) cleanupLabelsFromStatefulSet(argocd *argoproj.ArgoCD, return fmt.Errorf("failed to get statefulset %s: %w", statefulSetName, err) } - if currentLabels, err := r.removeLabelsFromObject(statefulSet, labelsToRemove); err != nil { - log.Error(err, "failed to remove labels from statefulset", "name", statefulSetName) + currentLabels := r.removeLabelsFromObject(statefulSet, labelsToRemove) + statefulSet.SetLabels(currentLabels) + if err := r.Update(context.TODO(), statefulSet); err != nil { + log.Error(err, "failed to update statefulset after removing labels", "name", statefulSetName) return err - } else { - statefulSet.SetLabels(currentLabels) - if err := r.Update(context.TODO(), statefulSet); err != nil { - log.Error(err, "failed to update statefulset after removing labels", "name", statefulSetName) - return err - } } + return nil } @@ -1519,10 +1495,10 @@ func (r *ReconcileArgoCD) calculateRemovedSpecLabels(oldCR, newCR *argoproj.Argo } -func (r *ReconcileArgoCD) removeLabelsFromObject(obj client.Object, labelsToRemove []string) (map[string]string, error) { +func (r *ReconcileArgoCD) removeLabelsFromObject(obj client.Object, labelsToRemove []string) map[string]string { currentLabels := obj.GetLabels() if currentLabels == nil { - return nil, nil + return nil } modified := false @@ -1533,9 +1509,9 @@ func (r *ReconcileArgoCD) removeLabelsFromObject(obj client.Object, labelsToRemo } } if modified { - return currentLabels, nil + return currentLabels } - return currentLabels, nil + return currentLabels } // deleteRBACsForNamespace deletes the RBACs when the label from the namespace is removed. From fe4cf8d8ecb9c2e11389972633b125255c180e5b Mon Sep 17 00:00:00 2001 From: Alka Kumari Date: Wed, 17 Sep 2025 17:00:38 +0530 Subject: [PATCH 06/12] change call to UpdateMapValues in repo_server.go file Signed-off-by: Alka Kumari --- controllers/argocd/repo_server.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/controllers/argocd/repo_server.go b/controllers/argocd/repo_server.go index d4028b491..da9286721 100644 --- a/controllers/argocd/repo_server.go +++ b/controllers/argocd/repo_server.go @@ -514,8 +514,8 @@ func (r *ReconcileArgoCD) reconcileRepoDeployment(cr *argocdoperatorv1beta1.Argo } // Add Kubernetes-specific labels/annotations from the live object in the source to preserve metadata. - addKubernetesData(deploy.Spec.Template.Labels, existing.Spec.Template.Labels) - addKubernetesData(deploy.Spec.Template.Annotations, existing.Spec.Template.Annotations) + UpdateMapValues(&existing.Spec.Template.Labels, deploy.Spec.Template.Labels) + UpdateMapValues(&existing.Spec.Template.Annotations, deploy.Spec.Template.Annotations) if !reflect.DeepEqual(deploy.Spec.Template.Annotations, existing.Spec.Template.Annotations) { existing.Spec.Template.Annotations = deploy.Spec.Template.Annotations From ccf6b789601d23674e9daae19b0f8fa11e437779 Mon Sep 17 00:00:00 2001 From: Alka Kumari Date: Wed, 17 Sep 2025 18:04:00 +0530 Subject: [PATCH 07/12] fix repo_server merge error Signed-off-by: Alka Kumari --- controllers/argocd/repo_server.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/controllers/argocd/repo_server.go b/controllers/argocd/repo_server.go index da9286721..c4a07c3f0 100644 --- a/controllers/argocd/repo_server.go +++ b/controllers/argocd/repo_server.go @@ -526,8 +526,7 @@ func (r *ReconcileArgoCD) reconcileRepoDeployment(cr *argocdoperatorv1beta1.Argo changed = true } - if !reflect.DeepEqual(deploy.Spec.Template.Labels, existing.Spec.Template.Labels) { - existing.Spec.Template.Labels = deploy.Spec.Template.Labels + if UpdateMapValues(&existing.Spec.Template.Labels, deploy.Spec.Template.Labels) { if changed { explanation += ", " } From dfa91373289f9d272ff77c842c503511ac8e6296 Mon Sep 17 00:00:00 2001 From: Alka Kumari Date: Thu, 18 Sep 2025 16:54:26 +0530 Subject: [PATCH 08/12] rec_labels: fixed removeLabelsFromObject logic and added edge cases to ginkgo tests for checking custom labels and annotations Signed-off-by: Alka Kumari --- controllers/argocd/util.go | 69 +++-- ...validate_custom_labels_annotations_test.go | 274 +++++++++++++++++- 2 files changed, 308 insertions(+), 35 deletions(-) diff --git a/controllers/argocd/util.go b/controllers/argocd/util.go index da2f1c0e1..f3b6842cd 100644 --- a/controllers/argocd/util.go +++ b/controllers/argocd/util.go @@ -1394,11 +1394,26 @@ func (r *ReconcileArgoCD) cleanupLabelsFromDeployment(argocd *argoproj.ArgoCD, d return fmt.Errorf("failed to get deployment %s: %w", deploymentName, err) } - currentLabels := r.removeLabelsFromObject(deployment, labelsToRemove) - deployment.Spec.Template.Labels = currentLabels - if err := r.Update(context.TODO(), deployment); err != nil { - log.Error(err, "failed to update deployment after removing labels", "name", deploymentName) - return err + // Work on pod template labels directly, not deployment metadata labels + currentLabels := deployment.Spec.Template.Labels + if currentLabels == nil { + return nil // No labels to remove + } + + modified := false + for _, labelKey := range labelsToRemove { + if _, exists := currentLabels[labelKey]; exists { + delete(currentLabels, labelKey) + modified = true + } + } + + if modified { + deployment.Spec.Template.Labels = currentLabels + if err := r.Update(context.TODO(), deployment); err != nil { + log.Error(err, "failed to update deployment after removing labels", "name", deploymentName) + return err + } } return nil } @@ -1417,11 +1432,26 @@ func (r *ReconcileArgoCD) cleanupLabelsFromStatefulSet(argocd *argoproj.ArgoCD, return fmt.Errorf("failed to get statefulset %s: %w", statefulSetName, err) } - currentLabels := r.removeLabelsFromObject(statefulSet, labelsToRemove) - statefulSet.SetLabels(currentLabels) - if err := r.Update(context.TODO(), statefulSet); err != nil { - log.Error(err, "failed to update statefulset after removing labels", "name", statefulSetName) - return err + // Work on pod template labels directly, not StatefulSet metadata labels + currentLabels := statefulSet.Spec.Template.Labels + if currentLabels == nil { + return nil // No labels to remove + } + + modified := false + for _, labelKey := range labelsToRemove { + if _, exists := currentLabels[labelKey]; exists { + delete(currentLabels, labelKey) + modified = true + } + } + + if modified { + statefulSet.Spec.Template.Labels = currentLabels + if err := r.Update(context.TODO(), statefulSet); err != nil { + log.Error(err, "failed to update statefulset after removing labels", "name", statefulSetName) + return err + } } return nil @@ -1495,25 +1525,6 @@ func (r *ReconcileArgoCD) calculateRemovedSpecLabels(oldCR, newCR *argoproj.Argo } -func (r *ReconcileArgoCD) removeLabelsFromObject(obj client.Object, labelsToRemove []string) map[string]string { - currentLabels := obj.GetLabels() - if currentLabels == nil { - return nil - } - - modified := false - for _, labelKey := range labelsToRemove { - if _, exists := currentLabels[labelKey]; exists { - delete(currentLabels, labelKey) - modified = true - } - } - if modified { - return currentLabels - } - return currentLabels -} - // deleteRBACsForNamespace deletes the RBACs when the label from the namespace is removed. func deleteRBACsForNamespace(sourceNS string, k8sClient kubernetes.Interface) error { log.Info(fmt.Sprintf("Removing the RBACs created for the namespace: %s", sourceNS)) diff --git a/tests/ginkgo/parallel/1-043_validate_custom_labels_annotations_test.go b/tests/ginkgo/parallel/1-043_validate_custom_labels_annotations_test.go index b1e89c852..ee0202885 100644 --- a/tests/ginkgo/parallel/1-043_validate_custom_labels_annotations_test.go +++ b/tests/ginkgo/parallel/1-043_validate_custom_labels_annotations_test.go @@ -143,7 +143,246 @@ var _ = Describe("GitOps Operator Parallel E2E Tests", func() { Expect(controllerSS).Should(statefulsetFixture.HaveTemplateAnnotationWithValue("custom", "annotation")) Expect(controllerSS).Should(statefulsetFixture.HaveTemplateAnnotationWithValue("custom2", "controller")) - By("removing custom labels and annotations from ArgoCD CR") + By("partially removing some custom labels from server and repo deployment (selective removal)") + argocdFixture.Update(argoCD, func(ac *argov1beta1api.ArgoCD) { + ac.Spec.Server.Labels = map[string]string{ + "custom2": "server", + } + ac.Spec.Repo.Labels = map[string]string{ + "custom": "label", + } + }) + + By("verifying selective label removal from server deployment") + + Eventually(func() bool { + if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(serverDepl), serverDepl); err != nil { + return false + } + labels := serverDepl.Spec.Template.Labels + if _, exists := labels["custom"]; exists { + GinkgoWriter.Printf("Label 'custom' still exists in server deployment, current labels: %v\n", labels) + return false + } + + // Should still have these labels + if labels["custom2"] != "server" { + GinkgoWriter.Printf("Label 'custom2' missing or incorrect in server deployment, current labels: %v\n", labels) + return false + } + + return true + }, "2m", "5s").Should(BeTrue()) + + By("verifying selective label removal from repo deployment") + + Eventually(func() bool { + if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(repoDepl), repoDepl); err != nil { + return false + } + labels := repoDepl.Spec.Template.Labels + if _, exists := labels["custom2"]; exists { + GinkgoWriter.Printf("Label 'custom2' still exists in repo deployment, current labels: %v\n", labels) + return false + } + + // Should still have these labels + if labels["custom"] != "label" { + GinkgoWriter.Printf("Label 'custom' missing or incorrect in repo deployment, current labels: %v\n", labels) + return false + } + return true + }, "2m", "5s").Should(BeTrue()) + + By("verifying that the labels of the other components are not affected") + Eventually(func() bool { + if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(appsetDepl), appsetDepl); err != nil { + return false + } + //Above operation should not affect the labels of the other components + appsetLabels := appsetDepl.Spec.Template.Labels + _, hasCustom := appsetLabels["custom"] + _, hasCustom2 := appsetLabels["custom2"] + + if !hasCustom || !hasCustom2 { + GinkgoWriter.Printf("Label 'custom' or 'custom2' missing from repo deployment, current labels: %v\n", appsetLabels) + return false + } + + controllerLabels := appsetDepl.Spec.Template.Labels + _, hasCustom = controllerLabels["custom"] + _, hasCustom2 = controllerLabels["custom2"] + + if !hasCustom || !hasCustom2 { + GinkgoWriter.Printf("Label 'custom' or 'custom2' missing from repo deployment, current labels: %v\n", controllerLabels) + return false + } + return true + }, "2m", "5s").Should(BeTrue()) + + By("partially removing some custom labels from controller and appset deployment (selective removal)") + argocdFixture.Update(argoCD, func(ac *argov1beta1api.ArgoCD) { + ac.Spec.Controller.Labels = map[string]string{ + "custom": "label", + } + ac.Spec.ApplicationSet.Labels = map[string]string{ + "custom2": "applicationSet", + } + }) + + By("verifying selective label removal from controller deployment") + + Eventually(func() bool { + if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(controllerSS), controllerSS); err != nil { + return false + } + labels := controllerSS.Spec.Template.Labels + if _, exists := labels["custom2"]; exists { + GinkgoWriter.Printf("Label 'custom2' still exists in controller deployment, current labels: %v\n", labels) + return false + } + // Should still have these labels + if labels["custom"] != "label" { + GinkgoWriter.Printf("Label 'custom' missing or incorrect in controller deployment, current labels: %v\n", labels) + return false + } + return true + }, "2m", "5s").Should(BeTrue()) + + By("verifying selective label removal from appset deployment") + + Eventually(func() bool { + if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(appsetDepl), appsetDepl); err != nil { + return false + } + labels := appsetDepl.Spec.Template.Labels + if _, exists := labels["custom"]; exists { + GinkgoWriter.Printf("Label 'custom' still exists in appset deployment, current labels: %v\n", labels) + return false + } + // Should still have these labels + if labels["custom2"] != "applicationSet" { + GinkgoWriter.Printf("Label 'custom2' missing or incorrect in appset deployment, current labels: %v\n", labels) + return false + } + return true + }, "2m", "5s").Should(BeTrue()) + + By("verifying that the labels of server and repo deployments are not affected") + Eventually(func() bool { + if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(appsetDepl), appsetDepl); err != nil { + return false + } + _, hasCustom := serverDepl.Spec.Template.Labels["custom2"] + if !hasCustom { + GinkgoWriter.Printf("Label 'custom2' missing from server deployment, current labels: %v\n", serverDepl.Spec.Template.Labels) + return false + } + _, hasCustom = repoDepl.Spec.Template.Labels["custom"] + if !hasCustom { + GinkgoWriter.Printf("Label 'custom' missing from repo deployment, current labels: %v\n", repoDepl.Spec.Template.Labels) + return false + } + return true + }, "2m", "5s").Should(BeTrue()) + + By("partially removing some custom annotations from controller and server deployment (selective removal)") + argocdFixture.Update(argoCD, func(ac *argov1beta1api.ArgoCD) { + ac.Spec.Controller.Annotations = map[string]string{ + "custom": "annotation", + } + ac.Spec.Server.Annotations = map[string]string{ + "custom2": "server", + } + }) + + By("verifying selective annotation removal from controller deployment") + + Eventually(func() bool { + if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(controllerSS), controllerSS); err != nil { + return false + } + annotations := controllerSS.Spec.Template.Annotations + if _, exists := annotations["custom2"]; exists { + GinkgoWriter.Printf("Annotation 'custom2' still exists in controller deployment, current annotations: %v\n", annotations) + return false + } + // Should still have these annotations + if annotations["custom"] != "annotation" { + GinkgoWriter.Printf("Annotation 'custom' missing or incorrect in controller deployment, current annotations: %v\n", annotations) + return false + } + return true + }, "2m", "5s").Should(BeTrue()) + + By("verifying selective annotation removal from server deployment") + + Eventually(func() bool { + if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(serverDepl), serverDepl); err != nil { + return false + } + annotations := serverDepl.Spec.Template.Annotations + if _, exists := annotations["custom"]; exists { + GinkgoWriter.Printf("Annotation 'custom' still exists in server deployment, current annotations: %v\n", annotations) + return false + } + // Should still have these annotations + if annotations["custom2"] != "server" { + GinkgoWriter.Printf("Annotation 'custom2' missing or incorrect in server deployment, current annotations: %v\n", annotations) + return false + } + return true + }, "2m", "5s").Should(BeTrue()) + + By("partially removing some custom annotations from repo and appset deployment (selective removal)") + argocdFixture.Update(argoCD, func(ac *argov1beta1api.ArgoCD) { + ac.Spec.Repo.Annotations = map[string]string{ + "custom2": "repo", + } + ac.Spec.ApplicationSet.Annotations = map[string]string{ + "custom": "annotation", + } + }) + + By("verifying selective annotation removal from repo deployment") + + Eventually(func() bool { + if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(repoDepl), repoDepl); err != nil { + return false + } + annotations := repoDepl.Spec.Template.Annotations + if _, exists := annotations["custom"]; exists { + GinkgoWriter.Printf("Annotation 'custom' still exists in repo deployment, current annotations: %v\n", annotations) + return false + } + // Should still have these annotations + if annotations["custom2"] != "repo" { + GinkgoWriter.Printf("Annotation 'custom2' missing or incorrect in repo deployment, current annotations: %v\n", annotations) + return false + } + return true + }, "2m", "5s").Should(BeTrue()) + + By("verifying selective annotation removal from appset deployment") + + Eventually(func() bool { + if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(appsetDepl), appsetDepl); err != nil { + return false + } + annotations := appsetDepl.Spec.Template.Annotations + if _, exists := annotations["custom2"]; exists { + GinkgoWriter.Printf("Annotation 'custom2' still exists in appset deployment, current annotations: %v\n", annotations) + return false + } + // Should still have these annotations + if annotations["custom"] != "annotation" { + GinkgoWriter.Printf("Annotation 'custom' missing or incorrect in appset deployment, current annotations: %v\n", annotations) + return false + } + return true + }, "2m", "5s").Should(BeTrue()) + + By("completelyremoving all custom labels and annotations from ArgoCD CR") argocdFixture.Update(argoCD, func(ac *argov1beta1api.ArgoCD) { ac.Spec.Server.Labels = map[string]string{} @@ -161,11 +400,12 @@ var _ = Describe("GitOps Operator Parallel E2E Tests", func() { By("verifying labels and annotations have been removed from template specs of Argo CD components") + Eventually(serverDepl).Should(k8sFixture.ExistByName()) Eventually(controllerSS).Should(k8sFixture.ExistByName()) Eventually(appsetDepl).Should(k8sFixture.ExistByName()) Eventually(repoDepl).Should(k8sFixture.ExistByName()) - expectLabelsAndAnnotationsRemovedFromDepl := func(depl *appsv1.Deployment) { + expectLabelsAndAnnotationsRemovedFromDepl := func(depl *appsv1.Deployment, componentName string) { By("checking labels and annotations are removed from " + depl.Name) @@ -186,15 +426,20 @@ var _ = Describe("GitOps Operator Parallel E2E Tests", func() { return false } } + // Verify operator-managed labels are preserved + if _, exists := depl.Spec.Template.Labels["app.kubernetes.io/name"]; !exists { + GinkgoWriter.Printf("Operator-managed label 'app.kubernetes.io/name' missing from %s deployment, current labels: %v\n", componentName, depl.Spec.Template.Labels) + return false + } return true - }).Should(BeTrue()) + }, "2m", "5s").Should(BeTrue()) } - expectLabelsAndAnnotationsRemovedFromDepl(appsetDepl) - expectLabelsAndAnnotationsRemovedFromDepl(serverDepl) - expectLabelsAndAnnotationsRemovedFromDepl(repoDepl) + expectLabelsAndAnnotationsRemovedFromDepl(appsetDepl, "applicationset") + expectLabelsAndAnnotationsRemovedFromDepl(serverDepl, "server") + expectLabelsAndAnnotationsRemovedFromDepl(repoDepl, "repo") // Evaluate the controller statefulset on its own, since it's a StatefulSet not a Deployment Eventually(controllerSS).Should(k8sFixture.ExistByName()) @@ -207,6 +452,23 @@ var _ = Describe("GitOps Operator Parallel E2E Tests", func() { Expect(k).ToNot(ContainSubstring("custom")) } + By("adding more labels") + argocdFixture.Update(argoCD, func(ac *argov1beta1api.ArgoCD) { + ac.Spec.Server.Labels = map[string]string{ + "second-label": "second-value", + "third-label": "third-value", + } + }) + + By("verifying new labels are added") + Eventually(func() bool { + if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(serverDepl), serverDepl); err != nil { + return false + } + labels := serverDepl.Spec.Template.Labels + return labels["second-label"] == "second-value" && + labels["third-label"] == "third-value" + }, "2m", "5s").Should(BeTrue()) }) }) From 37fdd7f8ad96e05bacde21b8de00a7f8963614cc Mon Sep 17 00:00:00 2001 From: Alka Kumari Date: Fri, 7 Nov 2025 15:14:28 +0530 Subject: [PATCH 09/12] added reviwed changes to ginkgo test case Signed-off-by: Alka Kumari --- ...-043_validate_custom_labels_annotations_test.go | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/tests/ginkgo/parallel/1-043_validate_custom_labels_annotations_test.go b/tests/ginkgo/parallel/1-043_validate_custom_labels_annotations_test.go index ee0202885..7d97a303f 100644 --- a/tests/ginkgo/parallel/1-043_validate_custom_labels_annotations_test.go +++ b/tests/ginkgo/parallel/1-043_validate_custom_labels_annotations_test.go @@ -209,12 +209,15 @@ var _ = Describe("GitOps Operator Parallel E2E Tests", func() { return false } - controllerLabels := appsetDepl.Spec.Template.Labels + if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(controllerSS), controllerSS); err != nil { + return false + } + controllerLabels := controllerSS.Spec.Template.Labels _, hasCustom = controllerLabels["custom"] _, hasCustom2 = controllerLabels["custom2"] if !hasCustom || !hasCustom2 { - GinkgoWriter.Printf("Label 'custom' or 'custom2' missing from repo deployment, current labels: %v\n", controllerLabels) + GinkgoWriter.Printf("Label 'custom' or 'custom2' missing from controller deployment, current labels: %v\n", controllerLabels) return false } return true @@ -270,7 +273,10 @@ var _ = Describe("GitOps Operator Parallel E2E Tests", func() { By("verifying that the labels of server and repo deployments are not affected") Eventually(func() bool { - if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(appsetDepl), appsetDepl); err != nil { + if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(serverDepl), serverDepl); err != nil { + return false + } + if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(repoDepl), repoDepl); err != nil { return false } _, hasCustom := serverDepl.Spec.Template.Labels["custom2"] @@ -382,7 +388,7 @@ var _ = Describe("GitOps Operator Parallel E2E Tests", func() { return true }, "2m", "5s").Should(BeTrue()) - By("completelyremoving all custom labels and annotations from ArgoCD CR") + By("completely removing all custom labels and annotations from ArgoCD CR") argocdFixture.Update(argoCD, func(ac *argov1beta1api.ArgoCD) { ac.Spec.Server.Labels = map[string]string{} From e065e6e4f59e1431e15ccaeedfd91e209fdb3d86 Mon Sep 17 00:00:00 2001 From: Alka Kumari Date: Sat, 8 Nov 2025 13:36:29 +0530 Subject: [PATCH 10/12] Added logic for annotations custom labels Signed-off-by: Alka Kumari --- controllers/argocd/applicationset.go | 1 - controllers/argocd/deployment.go | 6 +- controllers/argocd/repo_server.go | 8 +- controllers/argocd/statefulset.go | 9 +- controllers/argocd/util.go | 177 +++++++++++++++++---------- 5 files changed, 118 insertions(+), 83 deletions(-) diff --git a/controllers/argocd/applicationset.go b/controllers/argocd/applicationset.go index 9de102ae5..129705128 100644 --- a/controllers/argocd/applicationset.go +++ b/controllers/argocd/applicationset.go @@ -292,7 +292,6 @@ func (r *ReconcileArgoCD) reconcileApplicationSetDeployment(cr *argoproj.ArgoCD, existing.Spec.Template.Spec.NodeSelector = deploy.Spec.Template.Spec.NodeSelector existing.Spec.Template.Spec.Tolerations = deploy.Spec.Template.Spec.Tolerations existing.Spec.Template.Spec.Containers[0].SecurityContext = deploy.Spec.Template.Spec.Containers[0].SecurityContext - existing.Spec.Template.Annotations = deploy.Spec.Template.Annotations argoutil.LogResourceUpdate(log, existing, "due to difference in", deploymentsDifferent) return r.Update(context.TODO(), existing) diff --git a/controllers/argocd/deployment.go b/controllers/argocd/deployment.go index 9a9c58e3c..ce41f30a4 100644 --- a/controllers/argocd/deployment.go +++ b/controllers/argocd/deployment.go @@ -1250,11 +1250,7 @@ func (r *ReconcileArgoCD) reconcileServerDeployment(cr *argoproj.ArgoCD, useTLSF } //Check if labels/annotations have changed - UpdateMapValues(&existing.Spec.Template.Labels, deploy.Spec.Template.Labels) - UpdateMapValues(&existing.Spec.Template.Annotations, deploy.Spec.Template.Annotations) - - if !reflect.DeepEqual(deploy.Spec.Template.Annotations, existing.Spec.Template.Annotations) { - existing.Spec.Template.Annotations = deploy.Spec.Template.Annotations + if UpdateMapValues(&existing.Spec.Template.Annotations, deploy.Spec.Template.Annotations) { if changed { explanation += ", " } diff --git a/controllers/argocd/repo_server.go b/controllers/argocd/repo_server.go index c4a07c3f0..b01a401a6 100644 --- a/controllers/argocd/repo_server.go +++ b/controllers/argocd/repo_server.go @@ -513,12 +513,8 @@ func (r *ReconcileArgoCD) reconcileRepoDeployment(cr *argocdoperatorv1beta1.Argo changed = true } - // Add Kubernetes-specific labels/annotations from the live object in the source to preserve metadata. - UpdateMapValues(&existing.Spec.Template.Labels, deploy.Spec.Template.Labels) - UpdateMapValues(&existing.Spec.Template.Annotations, deploy.Spec.Template.Annotations) - - if !reflect.DeepEqual(deploy.Spec.Template.Annotations, existing.Spec.Template.Annotations) { - existing.Spec.Template.Annotations = deploy.Spec.Template.Annotations + //Check if labels/annotations have changed + if UpdateMapValues(&existing.Spec.Template.Annotations, deploy.Spec.Template.Annotations) { if changed { explanation += ", " } diff --git a/controllers/argocd/statefulset.go b/controllers/argocd/statefulset.go index 198c0f420..af4b43d44 100644 --- a/controllers/argocd/statefulset.go +++ b/controllers/argocd/statefulset.go @@ -985,11 +985,7 @@ func (r *ReconcileArgoCD) reconcileApplicationControllerStatefulSet(cr *argoproj } //Check if labels/annotations have changed - UpdateMapValues(&existing.Spec.Template.Labels, ss.Spec.Template.Labels) - UpdateMapValues(&existing.Spec.Template.Annotations, ss.Spec.Template.Annotations) - - if !reflect.DeepEqual(ss.Spec.Template.Annotations, existing.Spec.Template.Annotations) { - existing.Spec.Template.Annotations = ss.Spec.Template.Annotations + if UpdateMapValues(&existing.Spec.Template.Annotations, ss.Spec.Template.Annotations) { if changed { explanation += ", " } @@ -997,8 +993,7 @@ func (r *ReconcileArgoCD) reconcileApplicationControllerStatefulSet(cr *argoproj changed = true } - if !reflect.DeepEqual(ss.Spec.Template.Labels, existing.Spec.Template.Labels) { - existing.Spec.Template.Labels = ss.Spec.Template.Labels + if UpdateMapValues(&existing.Spec.Template.Labels, ss.Spec.Template.Labels) { if changed { explanation += ", " } diff --git a/controllers/argocd/util.go b/controllers/argocd/util.go index f3b6842cd..d6fa4e3de 100644 --- a/controllers/argocd/util.go +++ b/controllers/argocd/util.go @@ -1030,7 +1030,7 @@ func (r *ReconcileArgoCD) setResourceWatches(bldr *builder.Builder, clusterResou } // Watch for changes to primary resource ArgoCD - bldr.For(&argoproj.ArgoCD{}, builder.WithPredicates(deleteNotificationsPred, r.argoCDNamespaceManagementFilterPredicate(), r.argoCDSpecLabelCleanupPredicate())) + bldr.For(&argoproj.ArgoCD{}, builder.WithPredicates(deleteNotificationsPred, r.argoCDNamespaceManagementFilterPredicate(), r.argoCDSpecLabelAndAnnotationCleanupPredicate())) // Watch for changes to ConfigMap sub-resources owned by ArgoCD instances. bldr.Owns(&corev1.ConfigMap{}) @@ -1247,8 +1247,9 @@ func (r *ReconcileArgoCD) namespaceFilterPredicate() predicate.Predicate { // DropMetadata stores metadata about removed labels for a specific resource type type DropMetadata struct { - Resource string `json:"resource"` - Keys []string `json:"keys"` + Resource string `json:"resource"` + LabelKeys []string `json:"labelKeys"` + AnnotationKeys []string `json:"annotationKeys"` } // Global map to store removed labels per namespace @@ -1256,14 +1257,15 @@ type DropMetadata struct { var DropMetadataStore = make(map[string][]DropMetadata) // addDropMetadata adds removed label information to the DropMetadataStore for a specific namespace -func addDropMetadata(namespace, resource string, keys []string) { - if len(keys) == 0 { +func addDropMetadata(namespace, resource string, labelKeys, annotationKeys []string) { + if len(labelKeys) == 0 && len(annotationKeys) == 0 { return } dropMeta := DropMetadata{ - Resource: resource, - Keys: keys, + Resource: resource, + LabelKeys: labelKeys, + AnnotationKeys: annotationKeys, } if DropMetadataStore[namespace] == nil { @@ -1275,20 +1277,12 @@ func addDropMetadata(namespace, resource string, keys []string) { for i, existing := range DropMetadataStore[namespace] { if existing.Resource == resource { // Merge keys if resource already exists - keySet := make(map[string]bool) - for _, key := range existing.Keys { - keySet[key] = true - } - for _, key := range keys { - keySet[key] = true - } + mergedLabelKeys := mergeKeys(existing.LabelKeys, dropMeta.LabelKeys) + DropMetadataStore[namespace][i].LabelKeys = mergedLabelKeys - var mergedKeys []string - for key := range keySet { - mergedKeys = append(mergedKeys, key) - } + mergedAnnotationKeys := mergeKeys(existing.AnnotationKeys, dropMeta.AnnotationKeys) + DropMetadataStore[namespace][i].AnnotationKeys = mergedAnnotationKeys - DropMetadataStore[namespace][i].Keys = mergedKeys found = true break } @@ -1299,6 +1293,32 @@ func addDropMetadata(namespace, resource string, keys []string) { } } +// mergeKeys is a helper function thatefficiently merges two string slices, ensuring unique keys in the result. +func mergeKeys(existingKeys, newKeys []string) []string { + if len(newKeys) == 0 { + return existingKeys // No new keys to merge, return existing keys + } + + keySet := make(map[string]bool, len(existingKeys)+len(newKeys)) + + // Add existing keys to the set + for _, key := range existingKeys { + keySet[key] = true + } + + // Add new keys to the set + for _, key := range newKeys { + keySet[key] = true + } + + // Convert set back to slice + var mergedKeys []string + for key := range keySet { + mergedKeys = append(mergedKeys, key) + } + return mergedKeys +} + // getDropMetadataForNamespace retrieves all DropMetadata for a specific namespace func getDropMetadataForNamespace(namespace string) []DropMetadata { return DropMetadataStore[namespace] @@ -1309,21 +1329,15 @@ func removeDropMetadataForNamespace(namespace string) { delete(DropMetadataStore, namespace) } -// processDropMetadataForCleanup processes the DropMetadata store and cleans up removed labels from resources +// processDropMetadataForCleanup processes the DropMetadata store and cleans up removed labels and annotations from resources func (r *ReconcileArgoCD) processDropMetadataForCleanup(argocd *argoproj.ArgoCD) { namespace := argocd.GetNamespace() dropMetadataList := getDropMetadataForNamespace(namespace) - log.Info("Processing DropMetadata for cleanup", - "argocd", fmt.Sprintf("%s/%s", argocd.Namespace, argocd.Name), - "namespace", namespace, - "dropMetadataEntries", len(dropMetadataList)) - for _, dropMeta := range dropMetadataList { - if err := r.cleanupLabelsFromResourceType(argocd, dropMeta); err != nil { - log.Error(err, "failed to cleanup labels from resource type", - "resource", dropMeta.Resource, - "keys", dropMeta.Keys) + if err := r.cleanupLabelsAndAnnotationsFromResourceType(argocd, dropMeta); err != nil { + log.Error(err, "failed to cleanup labels and annotations from resource type", + "resource", dropMeta.Resource) continue } } @@ -1332,28 +1346,28 @@ func (r *ReconcileArgoCD) processDropMetadataForCleanup(argocd *argoproj.ArgoCD) removeDropMetadataForNamespace(namespace) } -// cleanupLabelsFromResourceType removes labels from specific resource types -func (r *ReconcileArgoCD) cleanupLabelsFromResourceType(argocd *argoproj.ArgoCD, dropMeta DropMetadata) error { +// cleanupLabelsAndAnnotationsFromResourceType removes labels and annotations from specific resource types +func (r *ReconcileArgoCD) cleanupLabelsAndAnnotationsFromResourceType(argocd *argoproj.ArgoCD, dropMeta DropMetadata) error { switch dropMeta.Resource { case "server-deployment": // Clean up from ArgoCD server deployment only deploymentName := fmt.Sprintf("%s-server", argocd.Name) - if err := r.cleanupLabelsFromDeployment(argocd, deploymentName, dropMeta.Keys); err != nil { + if err := r.cleanupLabelsAndAnnotationsFromDeployment(argocd, deploymentName, dropMeta.LabelKeys, dropMeta.AnnotationKeys); err != nil { return fmt.Errorf("failed to cleanup labels from server deployment: %w", err) } case "repo-deployment": // Clean up from ArgoCD repo server deployment only deploymentName := fmt.Sprintf("%s-repo-server", argocd.Name) - if err := r.cleanupLabelsFromDeployment(argocd, deploymentName, dropMeta.Keys); err != nil { + if err := r.cleanupLabelsAndAnnotationsFromDeployment(argocd, deploymentName, dropMeta.LabelKeys, dropMeta.AnnotationKeys); err != nil { return fmt.Errorf("failed to cleanup labels from repo deployment: %w", err) } case "applicationset-deployment": // Clean up from ArgoCD applicationset controller deployment only deploymentName := fmt.Sprintf("%s-applicationset-controller", argocd.Name) - if err := r.cleanupLabelsFromDeployment(argocd, deploymentName, dropMeta.Keys); err != nil { + if err := r.cleanupLabelsAndAnnotationsFromDeployment(argocd, deploymentName, dropMeta.LabelKeys, dropMeta.AnnotationKeys); err != nil { return fmt.Errorf("failed to cleanup labels from applicationset deployment: %w", err) } @@ -1361,7 +1375,7 @@ func (r *ReconcileArgoCD) cleanupLabelsFromResourceType(argocd *argoproj.ArgoCD, // Clean up from ArgoCD notifications controller deployment only if argocd.Spec.Notifications.Enabled { deploymentName := fmt.Sprintf("%s-notifications-controller", argocd.Name) - if err := r.cleanupLabelsFromDeployment(argocd, deploymentName, dropMeta.Keys); err != nil { + if err := r.cleanupLabelsAndAnnotationsFromDeployment(argocd, deploymentName, dropMeta.LabelKeys, dropMeta.AnnotationKeys); err != nil { return fmt.Errorf("failed to cleanup labels from notifications deployment: %w", err) } } @@ -1369,7 +1383,7 @@ func (r *ReconcileArgoCD) cleanupLabelsFromResourceType(argocd *argoproj.ArgoCD, case "controller-statefulset": // Clean up from ArgoCD application controller StatefulSet statefulSetName := fmt.Sprintf("%s-application-controller", argocd.Name) - if err := r.cleanupLabelsFromStatefulSet(argocd, statefulSetName, dropMeta.Keys); err != nil { + if err := r.cleanupLabelsAndAnnotationsFromStatefulSet(argocd, statefulSetName, dropMeta.LabelKeys, dropMeta.AnnotationKeys); err != nil { return fmt.Errorf("failed to cleanup labels from controller statefulset: %w", err) } @@ -1381,7 +1395,7 @@ func (r *ReconcileArgoCD) cleanupLabelsFromResourceType(argocd *argoproj.ArgoCD, } // cleanupLabelsFromDeployment removes labels from a specific deployment -func (r *ReconcileArgoCD) cleanupLabelsFromDeployment(argocd *argoproj.ArgoCD, deploymentName string, labelsToRemove []string) error { +func (r *ReconcileArgoCD) cleanupLabelsAndAnnotationsFromDeployment(argocd *argoproj.ArgoCD, deploymentName string, labelsToRemove, annotationsToRemove []string) error { namespace := argocd.GetNamespace() deployment := &appsv1.Deployment{} key := types.NamespacedName{Namespace: namespace, Name: deploymentName} @@ -1394,24 +1408,39 @@ func (r *ReconcileArgoCD) cleanupLabelsFromDeployment(argocd *argoproj.ArgoCD, d return fmt.Errorf("failed to get deployment %s: %w", deploymentName, err) } - // Work on pod template labels directly, not deployment metadata labels + // Work on pod template labels and annotations directly, not deployment metadata labels currentLabels := deployment.Spec.Template.Labels - if currentLabels == nil { - return nil // No labels to remove + currentAnnotations := deployment.Spec.Template.Annotations + if currentLabels == nil && currentAnnotations == nil { + return nil // No labels or annotations to remove } - modified := false + modifiedLabels := false + modifiedAnnotations := false for _, labelKey := range labelsToRemove { if _, exists := currentLabels[labelKey]; exists { delete(currentLabels, labelKey) - modified = true + modifiedLabels = true } } - if modified { + for _, annotationKey := range annotationsToRemove { + if _, exists := currentAnnotations[annotationKey]; exists { + delete(currentAnnotations, annotationKey) + modifiedAnnotations = true + } + } + + if modifiedLabels { deployment.Spec.Template.Labels = currentLabels + } + if modifiedAnnotations { + deployment.Spec.Template.Annotations = currentAnnotations + } + + if modifiedLabels || modifiedAnnotations { if err := r.Update(context.TODO(), deployment); err != nil { - log.Error(err, "failed to update deployment after removing labels", "name", deploymentName) + log.Error(err, "failed to update deployment after removing labels and annotations", "name", deploymentName) return err } } @@ -1419,14 +1448,14 @@ func (r *ReconcileArgoCD) cleanupLabelsFromDeployment(argocd *argoproj.ArgoCD, d } // cleanupLabelsFromStatefulSet removes labels from a specific statefulset -func (r *ReconcileArgoCD) cleanupLabelsFromStatefulSet(argocd *argoproj.ArgoCD, statefulSetName string, labelsToRemove []string) error { +func (r *ReconcileArgoCD) cleanupLabelsAndAnnotationsFromStatefulSet(argocd *argoproj.ArgoCD, statefulSetName string, labelsToRemove, annotationsToRemove []string) error { namespace := argocd.GetNamespace() statefulSet := &appsv1.StatefulSet{} key := types.NamespacedName{Namespace: namespace, Name: statefulSetName} if err := r.Get(context.TODO(), key, statefulSet); err != nil { if apierrors.IsNotFound(err) { - log.Info("StatefulSet not found, skipping label cleanup", "name", statefulSetName) + log.Info("StatefulSet not found, skipping label and annotation cleanup", "name", statefulSetName) return nil // StatefulSet doesn't exist, nothing to clean } return fmt.Errorf("failed to get statefulset %s: %w", statefulSetName, err) @@ -1434,22 +1463,36 @@ func (r *ReconcileArgoCD) cleanupLabelsFromStatefulSet(argocd *argoproj.ArgoCD, // Work on pod template labels directly, not StatefulSet metadata labels currentLabels := statefulSet.Spec.Template.Labels - if currentLabels == nil { - return nil // No labels to remove + currentAnnotations := statefulSet.Spec.Template.Annotations + if currentLabels == nil && currentAnnotations == nil { + return nil // No labels or annotations to remove } - modified := false + modifiedLabels := false + modifiedAnnotations := false for _, labelKey := range labelsToRemove { if _, exists := currentLabels[labelKey]; exists { delete(currentLabels, labelKey) - modified = true + modifiedLabels = true + } + } + for _, annotationKey := range annotationsToRemove { + if _, exists := currentAnnotations[annotationKey]; exists { + delete(currentAnnotations, annotationKey) + modifiedAnnotations = true } } - if modified { + if modifiedLabels { statefulSet.Spec.Template.Labels = currentLabels + } + if modifiedAnnotations { + statefulSet.Spec.Template.Annotations = currentAnnotations + } + + if modifiedLabels || modifiedAnnotations { if err := r.Update(context.TODO(), statefulSet); err != nil { - log.Error(err, "failed to update statefulset after removing labels", "name", statefulSetName) + log.Error(err, "failed to update statefulset after removing labels and annotations", "name", statefulSetName) return err } } @@ -1458,7 +1501,7 @@ func (r *ReconcileArgoCD) cleanupLabelsFromStatefulSet(argocd *argoproj.ArgoCD, } // argoCDSpecLabelCleanupPredicate tracks label changes in ArgoCD CR specs and cleans up removed labels from managed resources -func (r *ReconcileArgoCD) argoCDSpecLabelCleanupPredicate() predicate.Predicate { +func (r *ReconcileArgoCD) argoCDSpecLabelAndAnnotationCleanupPredicate() predicate.Predicate { return predicate.Funcs{ UpdateFunc: func(e event.UpdateEvent) bool { newCR, ok := e.ObjectNew.(*argoproj.ArgoCD) @@ -1471,7 +1514,7 @@ func (r *ReconcileArgoCD) argoCDSpecLabelCleanupPredicate() predicate.Predicate } // Track removed labels from each component spec - r.calculateRemovedSpecLabels(oldCR, newCR) + r.calculateRemovedSpecLabelsAndAnnotations(oldCR, newCR) return true // Trigger reconciliation for ArgoCD updates }, @@ -1479,7 +1522,7 @@ func (r *ReconcileArgoCD) argoCDSpecLabelCleanupPredicate() predicate.Predicate } // calculateRemovedSpecLabels compares old and new ArgoCD specs and tracks removed labels by resource type -func (r *ReconcileArgoCD) calculateRemovedSpecLabels(oldCR, newCR *argoproj.ArgoCD) { +func (r *ReconcileArgoCD) calculateRemovedSpecLabelsAndAnnotations(oldCR, newCR *argoproj.ArgoCD) { // Add nil checks to prevent panic if r == nil || oldCR == nil || newCR == nil { return @@ -1488,31 +1531,37 @@ func (r *ReconcileArgoCD) calculateRemovedSpecLabels(oldCR, newCR *argoproj.Argo namespace := newCR.GetNamespace() // Helper function to check and store removed labels for a specific resource - checkAndStoreRemovedLabels := func(oldLabels, newLabels map[string]string, resourceType string) { - var removedKeys []string + checkAndStoreRemovedLabels := func(oldLabels, oldAnnotations, newLabels, newAnnotations map[string]string, resourceType string) { + var removedLabelKeys []string + var removedAnnotationKeys []string for key := range oldLabels { if newLabels == nil || newLabels[key] == "" { - removedKeys = append(removedKeys, key) + removedLabelKeys = append(removedLabelKeys, key) + } + } + for key := range oldAnnotations { + if newAnnotations == nil || newAnnotations[key] == "" { + removedAnnotationKeys = append(removedAnnotationKeys, key) } } // Store removed keys for this resource type in the namespace - if len(removedKeys) > 0 { - addDropMetadata(namespace, resourceType, removedKeys) + if len(removedLabelKeys) > 0 || len(removedAnnotationKeys) > 0 { + addDropMetadata(namespace, resourceType, removedLabelKeys, removedAnnotationKeys) } } // Check Server labels - checkAndStoreRemovedLabels(oldCR.Spec.Server.Labels, newCR.Spec.Server.Labels, "server-deployment") + checkAndStoreRemovedLabels(oldCR.Spec.Server.Labels, oldCR.Spec.Server.Annotations, newCR.Spec.Server.Labels, newCR.Spec.Server.Annotations, "server-deployment") // Check Repo Server labels - checkAndStoreRemovedLabels(oldCR.Spec.Repo.Labels, newCR.Spec.Repo.Labels, "repo-deployment") + checkAndStoreRemovedLabels(oldCR.Spec.Repo.Labels, oldCR.Spec.Repo.Annotations, newCR.Spec.Repo.Labels, newCR.Spec.Repo.Annotations, "repo-deployment") // Check Controller labels (StatefulSet) - checkAndStoreRemovedLabels(oldCR.Spec.Controller.Labels, newCR.Spec.Controller.Labels, "controller-statefulset") + checkAndStoreRemovedLabels(oldCR.Spec.Controller.Labels, oldCR.Spec.Controller.Annotations, newCR.Spec.Controller.Labels, newCR.Spec.Controller.Annotations, "controller-statefulset") // Check ApplicationSet labels if oldCR.Spec.ApplicationSet != nil && newCR.Spec.ApplicationSet != nil { - checkAndStoreRemovedLabels(oldCR.Spec.ApplicationSet.Labels, newCR.Spec.ApplicationSet.Labels, "applicationset-deployment") + checkAndStoreRemovedLabels(oldCR.Spec.ApplicationSet.Labels, oldCR.Spec.ApplicationSet.Annotations, newCR.Spec.ApplicationSet.Labels, newCR.Spec.ApplicationSet.Annotations, "applicationset-deployment") } // Check Notifications labels (if we add support for notifications labels in the future) From d1e69670b4f32cf40e29448ba031b7ebba622c07 Mon Sep 17 00:00:00 2001 From: Alka Kumari Date: Mon, 10 Nov 2025 12:44:02 +0530 Subject: [PATCH 11/12] removed the Cleanup logic for custom labels Signed-off-by: Alka Kumari --- controllers/argocd/argocd_controller.go | 3 - controllers/argocd/util.go | 331 +----------------------- 2 files changed, 1 insertion(+), 333 deletions(-) diff --git a/controllers/argocd/argocd_controller.go b/controllers/argocd/argocd_controller.go index e69cd074f..40125fa92 100644 --- a/controllers/argocd/argocd_controller.go +++ b/controllers/argocd/argocd_controller.go @@ -348,9 +348,6 @@ func (r *ReconcileArgoCD) internalReconcile(ctx context.Context, request ctrl.Re } } - // Process DropMetadata for namespace-based label cleanup - r.processDropMetadataForCleanup(argocd) - if err := r.reconcileResources(argocd, argoCDStatus); err != nil { // Error reconciling ArgoCD sub-resources - requeue the request. return reconcile.Result{}, argocd, argoCDStatus, err diff --git a/controllers/argocd/util.go b/controllers/argocd/util.go index d6fa4e3de..febb7867b 100644 --- a/controllers/argocd/util.go +++ b/controllers/argocd/util.go @@ -1030,7 +1030,7 @@ func (r *ReconcileArgoCD) setResourceWatches(bldr *builder.Builder, clusterResou } // Watch for changes to primary resource ArgoCD - bldr.For(&argoproj.ArgoCD{}, builder.WithPredicates(deleteNotificationsPred, r.argoCDNamespaceManagementFilterPredicate(), r.argoCDSpecLabelAndAnnotationCleanupPredicate())) + bldr.For(&argoproj.ArgoCD{}, builder.WithPredicates(deleteNotificationsPred, r.argoCDNamespaceManagementFilterPredicate())) // Watch for changes to ConfigMap sub-resources owned by ArgoCD instances. bldr.Owns(&corev1.ConfigMap{}) @@ -1245,335 +1245,6 @@ func (r *ReconcileArgoCD) namespaceFilterPredicate() predicate.Predicate { } } -// DropMetadata stores metadata about removed labels for a specific resource type -type DropMetadata struct { - Resource string `json:"resource"` - LabelKeys []string `json:"labelKeys"` - AnnotationKeys []string `json:"annotationKeys"` -} - -// Global map to store removed labels per namespace -// Key: namespace, Value: slice of DropMetadata for different resources -var DropMetadataStore = make(map[string][]DropMetadata) - -// addDropMetadata adds removed label information to the DropMetadataStore for a specific namespace -func addDropMetadata(namespace, resource string, labelKeys, annotationKeys []string) { - if len(labelKeys) == 0 && len(annotationKeys) == 0 { - return - } - - dropMeta := DropMetadata{ - Resource: resource, - LabelKeys: labelKeys, - AnnotationKeys: annotationKeys, - } - - if DropMetadataStore[namespace] == nil { - DropMetadataStore[namespace] = []DropMetadata{} - } - - // Check if the resource already exists in the slice and update it - found := false - for i, existing := range DropMetadataStore[namespace] { - if existing.Resource == resource { - // Merge keys if resource already exists - mergedLabelKeys := mergeKeys(existing.LabelKeys, dropMeta.LabelKeys) - DropMetadataStore[namespace][i].LabelKeys = mergedLabelKeys - - mergedAnnotationKeys := mergeKeys(existing.AnnotationKeys, dropMeta.AnnotationKeys) - DropMetadataStore[namespace][i].AnnotationKeys = mergedAnnotationKeys - - found = true - break - } - } - - if !found { - DropMetadataStore[namespace] = append(DropMetadataStore[namespace], dropMeta) - } -} - -// mergeKeys is a helper function thatefficiently merges two string slices, ensuring unique keys in the result. -func mergeKeys(existingKeys, newKeys []string) []string { - if len(newKeys) == 0 { - return existingKeys // No new keys to merge, return existing keys - } - - keySet := make(map[string]bool, len(existingKeys)+len(newKeys)) - - // Add existing keys to the set - for _, key := range existingKeys { - keySet[key] = true - } - - // Add new keys to the set - for _, key := range newKeys { - keySet[key] = true - } - - // Convert set back to slice - var mergedKeys []string - for key := range keySet { - mergedKeys = append(mergedKeys, key) - } - return mergedKeys -} - -// getDropMetadataForNamespace retrieves all DropMetadata for a specific namespace -func getDropMetadataForNamespace(namespace string) []DropMetadata { - return DropMetadataStore[namespace] -} - -// removeDropMetadataForNamespace removes all DropMetadata for a specific namespace -func removeDropMetadataForNamespace(namespace string) { - delete(DropMetadataStore, namespace) -} - -// processDropMetadataForCleanup processes the DropMetadata store and cleans up removed labels and annotations from resources -func (r *ReconcileArgoCD) processDropMetadataForCleanup(argocd *argoproj.ArgoCD) { - namespace := argocd.GetNamespace() - dropMetadataList := getDropMetadataForNamespace(namespace) - - for _, dropMeta := range dropMetadataList { - if err := r.cleanupLabelsAndAnnotationsFromResourceType(argocd, dropMeta); err != nil { - log.Error(err, "failed to cleanup labels and annotations from resource type", - "resource", dropMeta.Resource) - continue - } - } - - // Clear the DropMetadata for this namespace after cleanup - removeDropMetadataForNamespace(namespace) -} - -// cleanupLabelsAndAnnotationsFromResourceType removes labels and annotations from specific resource types -func (r *ReconcileArgoCD) cleanupLabelsAndAnnotationsFromResourceType(argocd *argoproj.ArgoCD, dropMeta DropMetadata) error { - - switch dropMeta.Resource { - case "server-deployment": - // Clean up from ArgoCD server deployment only - deploymentName := fmt.Sprintf("%s-server", argocd.Name) - if err := r.cleanupLabelsAndAnnotationsFromDeployment(argocd, deploymentName, dropMeta.LabelKeys, dropMeta.AnnotationKeys); err != nil { - return fmt.Errorf("failed to cleanup labels from server deployment: %w", err) - } - - case "repo-deployment": - // Clean up from ArgoCD repo server deployment only - deploymentName := fmt.Sprintf("%s-repo-server", argocd.Name) - if err := r.cleanupLabelsAndAnnotationsFromDeployment(argocd, deploymentName, dropMeta.LabelKeys, dropMeta.AnnotationKeys); err != nil { - return fmt.Errorf("failed to cleanup labels from repo deployment: %w", err) - } - - case "applicationset-deployment": - // Clean up from ArgoCD applicationset controller deployment only - deploymentName := fmt.Sprintf("%s-applicationset-controller", argocd.Name) - if err := r.cleanupLabelsAndAnnotationsFromDeployment(argocd, deploymentName, dropMeta.LabelKeys, dropMeta.AnnotationKeys); err != nil { - return fmt.Errorf("failed to cleanup labels from applicationset deployment: %w", err) - } - - case "notifications-deployment": - // Clean up from ArgoCD notifications controller deployment only - if argocd.Spec.Notifications.Enabled { - deploymentName := fmt.Sprintf("%s-notifications-controller", argocd.Name) - if err := r.cleanupLabelsAndAnnotationsFromDeployment(argocd, deploymentName, dropMeta.LabelKeys, dropMeta.AnnotationKeys); err != nil { - return fmt.Errorf("failed to cleanup labels from notifications deployment: %w", err) - } - } - - case "controller-statefulset": - // Clean up from ArgoCD application controller StatefulSet - statefulSetName := fmt.Sprintf("%s-application-controller", argocd.Name) - if err := r.cleanupLabelsAndAnnotationsFromStatefulSet(argocd, statefulSetName, dropMeta.LabelKeys, dropMeta.AnnotationKeys); err != nil { - return fmt.Errorf("failed to cleanup labels from controller statefulset: %w", err) - } - - default: - log.Info("Unknown resource type for cleanup", "resourceType", dropMeta.Resource) - } - - return nil -} - -// cleanupLabelsFromDeployment removes labels from a specific deployment -func (r *ReconcileArgoCD) cleanupLabelsAndAnnotationsFromDeployment(argocd *argoproj.ArgoCD, deploymentName string, labelsToRemove, annotationsToRemove []string) error { - namespace := argocd.GetNamespace() - deployment := &appsv1.Deployment{} - key := types.NamespacedName{Namespace: namespace, Name: deploymentName} - - if err := r.Get(context.TODO(), key, deployment); err != nil { - if apierrors.IsNotFound(err) { - log.Info("Deployment not found, skipping label cleanup", "name", deploymentName) - return nil // Deployment doesn't exist, skip - } - return fmt.Errorf("failed to get deployment %s: %w", deploymentName, err) - } - - // Work on pod template labels and annotations directly, not deployment metadata labels - currentLabels := deployment.Spec.Template.Labels - currentAnnotations := deployment.Spec.Template.Annotations - if currentLabels == nil && currentAnnotations == nil { - return nil // No labels or annotations to remove - } - - modifiedLabels := false - modifiedAnnotations := false - for _, labelKey := range labelsToRemove { - if _, exists := currentLabels[labelKey]; exists { - delete(currentLabels, labelKey) - modifiedLabels = true - } - } - - for _, annotationKey := range annotationsToRemove { - if _, exists := currentAnnotations[annotationKey]; exists { - delete(currentAnnotations, annotationKey) - modifiedAnnotations = true - } - } - - if modifiedLabels { - deployment.Spec.Template.Labels = currentLabels - } - if modifiedAnnotations { - deployment.Spec.Template.Annotations = currentAnnotations - } - - if modifiedLabels || modifiedAnnotations { - if err := r.Update(context.TODO(), deployment); err != nil { - log.Error(err, "failed to update deployment after removing labels and annotations", "name", deploymentName) - return err - } - } - return nil -} - -// cleanupLabelsFromStatefulSet removes labels from a specific statefulset -func (r *ReconcileArgoCD) cleanupLabelsAndAnnotationsFromStatefulSet(argocd *argoproj.ArgoCD, statefulSetName string, labelsToRemove, annotationsToRemove []string) error { - namespace := argocd.GetNamespace() - statefulSet := &appsv1.StatefulSet{} - key := types.NamespacedName{Namespace: namespace, Name: statefulSetName} - - if err := r.Get(context.TODO(), key, statefulSet); err != nil { - if apierrors.IsNotFound(err) { - log.Info("StatefulSet not found, skipping label and annotation cleanup", "name", statefulSetName) - return nil // StatefulSet doesn't exist, nothing to clean - } - return fmt.Errorf("failed to get statefulset %s: %w", statefulSetName, err) - } - - // Work on pod template labels directly, not StatefulSet metadata labels - currentLabels := statefulSet.Spec.Template.Labels - currentAnnotations := statefulSet.Spec.Template.Annotations - if currentLabels == nil && currentAnnotations == nil { - return nil // No labels or annotations to remove - } - - modifiedLabels := false - modifiedAnnotations := false - for _, labelKey := range labelsToRemove { - if _, exists := currentLabels[labelKey]; exists { - delete(currentLabels, labelKey) - modifiedLabels = true - } - } - for _, annotationKey := range annotationsToRemove { - if _, exists := currentAnnotations[annotationKey]; exists { - delete(currentAnnotations, annotationKey) - modifiedAnnotations = true - } - } - - if modifiedLabels { - statefulSet.Spec.Template.Labels = currentLabels - } - if modifiedAnnotations { - statefulSet.Spec.Template.Annotations = currentAnnotations - } - - if modifiedLabels || modifiedAnnotations { - if err := r.Update(context.TODO(), statefulSet); err != nil { - log.Error(err, "failed to update statefulset after removing labels and annotations", "name", statefulSetName) - return err - } - } - - return nil -} - -// argoCDSpecLabelCleanupPredicate tracks label changes in ArgoCD CR specs and cleans up removed labels from managed resources -func (r *ReconcileArgoCD) argoCDSpecLabelAndAnnotationCleanupPredicate() predicate.Predicate { - return predicate.Funcs{ - UpdateFunc: func(e event.UpdateEvent) bool { - newCR, ok := e.ObjectNew.(*argoproj.ArgoCD) - if !ok { - return false - } - oldCR, ok := e.ObjectOld.(*argoproj.ArgoCD) - if !ok { - return false - } - - // Track removed labels from each component spec - r.calculateRemovedSpecLabelsAndAnnotations(oldCR, newCR) - - return true // Trigger reconciliation for ArgoCD updates - }, - } -} - -// calculateRemovedSpecLabels compares old and new ArgoCD specs and tracks removed labels by resource type -func (r *ReconcileArgoCD) calculateRemovedSpecLabelsAndAnnotations(oldCR, newCR *argoproj.ArgoCD) { - // Add nil checks to prevent panic - if r == nil || oldCR == nil || newCR == nil { - return - } - //removedLabels := make(map[string]string) - namespace := newCR.GetNamespace() - - // Helper function to check and store removed labels for a specific resource - checkAndStoreRemovedLabels := func(oldLabels, oldAnnotations, newLabels, newAnnotations map[string]string, resourceType string) { - var removedLabelKeys []string - var removedAnnotationKeys []string - for key := range oldLabels { - if newLabels == nil || newLabels[key] == "" { - removedLabelKeys = append(removedLabelKeys, key) - } - } - for key := range oldAnnotations { - if newAnnotations == nil || newAnnotations[key] == "" { - removedAnnotationKeys = append(removedAnnotationKeys, key) - } - } - // Store removed keys for this resource type in the namespace - if len(removedLabelKeys) > 0 || len(removedAnnotationKeys) > 0 { - addDropMetadata(namespace, resourceType, removedLabelKeys, removedAnnotationKeys) - } - } - - // Check Server labels - checkAndStoreRemovedLabels(oldCR.Spec.Server.Labels, oldCR.Spec.Server.Annotations, newCR.Spec.Server.Labels, newCR.Spec.Server.Annotations, "server-deployment") - - // Check Repo Server labels - checkAndStoreRemovedLabels(oldCR.Spec.Repo.Labels, oldCR.Spec.Repo.Annotations, newCR.Spec.Repo.Labels, newCR.Spec.Repo.Annotations, "repo-deployment") - - // Check Controller labels (StatefulSet) - checkAndStoreRemovedLabels(oldCR.Spec.Controller.Labels, oldCR.Spec.Controller.Annotations, newCR.Spec.Controller.Labels, newCR.Spec.Controller.Annotations, "controller-statefulset") - - // Check ApplicationSet labels - if oldCR.Spec.ApplicationSet != nil && newCR.Spec.ApplicationSet != nil { - checkAndStoreRemovedLabels(oldCR.Spec.ApplicationSet.Labels, oldCR.Spec.ApplicationSet.Annotations, newCR.Spec.ApplicationSet.Labels, newCR.Spec.ApplicationSet.Annotations, "applicationset-deployment") - } - - // Check Notifications labels (if we add support for notifications labels in the future) - // if oldCR.Spec.Notifications != nil && newCR.Spec.Notifications != nil { - // checkAndStoreRemovedLabels(oldCR.Spec.Notifications.Labels, newCR.Spec.Notifications.Labels, "notifications-deployment") - // } - - // Note: Notifications and Prometheus specs don't have Labels fields currently - // If they are added in the future, the logic can be uncommented and updated - -} - // deleteRBACsForNamespace deletes the RBACs when the label from the namespace is removed. func deleteRBACsForNamespace(sourceNS string, k8sClient kubernetes.Interface) error { log.Info(fmt.Sprintf("Removing the RBACs created for the namespace: %s", sourceNS)) From c680642922339b550616629e96fc5fcb21f47508 Mon Sep 17 00:00:00 2001 From: Alka Kumari Date: Mon, 10 Nov 2025 15:41:42 +0530 Subject: [PATCH 12/12] added gnkgo test cases for testing external labels and annotations are preserved Signed-off-by: Alka Kumari --- ...validate_custom_labels_annotations_test.go | 659 ++++++------------ ...ernal_labels_annotations_preserved_test.go | 486 +++++++++++++ 2 files changed, 682 insertions(+), 463 deletions(-) create mode 100644 tests/ginkgo/parallel/1-121_validate_external_labels_annotations_preserved_test.go diff --git a/tests/ginkgo/parallel/1-043_validate_custom_labels_annotations_test.go b/tests/ginkgo/parallel/1-043_validate_custom_labels_annotations_test.go index 7d97a303f..c9b627648 100644 --- a/tests/ginkgo/parallel/1-043_validate_custom_labels_annotations_test.go +++ b/tests/ginkgo/parallel/1-043_validate_custom_labels_annotations_test.go @@ -16,466 +16,199 @@ limitations under the License. package parallel -import ( - "context" - "strings" - - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - appsv1 "k8s.io/api/apps/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - - argov1beta1api "github.com/argoproj-labs/argocd-operator/api/v1beta1" - "github.com/argoproj-labs/argocd-operator/tests/ginkgo/fixture" - argocdFixture "github.com/argoproj-labs/argocd-operator/tests/ginkgo/fixture/argocd" - deploymentFixture "github.com/argoproj-labs/argocd-operator/tests/ginkgo/fixture/deployment" - k8sFixture "github.com/argoproj-labs/argocd-operator/tests/ginkgo/fixture/k8s" - statefulsetFixture "github.com/argoproj-labs/argocd-operator/tests/ginkgo/fixture/statefulset" - fixtureUtils "github.com/argoproj-labs/argocd-operator/tests/ginkgo/fixture/utils" - - "sigs.k8s.io/controller-runtime/pkg/client" -) - -var _ = Describe("GitOps Operator Parallel E2E Tests", func() { - - Context("1-043_validate_custom_labels_annotations", func() { - - var ( - k8sClient client.Client - ctx context.Context - ) - - BeforeEach(func() { - fixture.EnsureParallelCleanSlate() - k8sClient, _ = fixtureUtils.GetE2ETestKubeClient() - ctx = context.Background() - }) - - It("ensures that custom labels and annotations set on component fields of ArgoCD CR will be added to Deployment and StatefulSet templates of those components, and that they can likewise be removed", func() { - - By("creating namespace-scoped Argo CD instance with labels and annotations set on components") - ns, cleanupFunc := fixture.CreateRandomE2ETestNamespaceWithCleanupFunc() - defer cleanupFunc() - - argoCD := &argov1beta1api.ArgoCD{ - ObjectMeta: metav1.ObjectMeta{Name: "argocd-sample", Namespace: ns.Name}, - Spec: argov1beta1api.ArgoCDSpec{ - Server: argov1beta1api.ArgoCDServerSpec{ - Labels: map[string]string{ - "custom": "label", - "custom2": "server", - }, - Annotations: map[string]string{ - "custom": "annotation", - "custom2": "server", - }, - }, - Repo: argov1beta1api.ArgoCDRepoSpec{ - Labels: map[string]string{ - "custom": "label", - "custom2": "repo", - }, - Annotations: map[string]string{ - "custom": "annotation", - "custom2": "repo", - }, - }, - Controller: argov1beta1api.ArgoCDApplicationControllerSpec{ - Labels: map[string]string{ - "custom": "label", - "custom2": "controller", - }, - Annotations: map[string]string{ - "custom": "annotation", - "custom2": "controller", - }, - }, - ApplicationSet: &argov1beta1api.ArgoCDApplicationSet{ - Labels: map[string]string{ - "custom": "label", - "custom2": "applicationSet", - }, - Annotations: map[string]string{ - "custom": "annotation", - "custom2": "applicationSet", - }, - }, - }, - } - Expect(k8sClient.Create(ctx, argoCD)).To(Succeed()) - - By("waiting for Argo CD to become available") - Eventually(argoCD, "5m", "5s").Should(argocdFixture.BeAvailable()) - - By("verifying Argo CD components have the labels and annotations we set above") - - serverDepl := &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: "argocd-sample-server", Namespace: ns.Name}} - - Eventually(serverDepl).Should(k8sFixture.ExistByName()) - Expect(serverDepl).Should(deploymentFixture.HaveTemplateLabelWithValue("app.kubernetes.io/name", "argocd-sample-server")) - Expect(serverDepl).Should(deploymentFixture.HaveTemplateLabelWithValue("custom", "label")) - Expect(serverDepl).Should(deploymentFixture.HaveTemplateLabelWithValue("custom2", "server")) - - Expect(serverDepl).Should(deploymentFixture.HaveTemplateAnnotationWithValue("custom", "annotation")) - Expect(serverDepl).Should(deploymentFixture.HaveTemplateAnnotationWithValue("custom2", "server")) - - repoDepl := &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: "argocd-sample-repo-server", Namespace: ns.Name}} - Eventually(repoDepl).Should(k8sFixture.ExistByName()) - Expect(repoDepl).Should(deploymentFixture.HaveTemplateLabelWithValue("app.kubernetes.io/name", "argocd-sample-repo-server")) - Expect(repoDepl).Should(deploymentFixture.HaveTemplateLabelWithValue("custom", "label")) - Expect(repoDepl).Should(deploymentFixture.HaveTemplateLabelWithValue("custom2", "repo")) - Expect(repoDepl).Should(deploymentFixture.HaveTemplateAnnotationWithValue("custom", "annotation")) - Expect(repoDepl).Should(deploymentFixture.HaveTemplateAnnotationWithValue("custom2", "repo")) - - appsetDepl := &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: "argocd-sample-applicationset-controller", Namespace: ns.Name}} - Eventually(appsetDepl).Should(k8sFixture.ExistByName()) - Expect(appsetDepl).Should(deploymentFixture.HaveTemplateLabelWithValue("app.kubernetes.io/name", "argocd-sample-applicationset-controller")) - Expect(appsetDepl).Should(deploymentFixture.HaveTemplateLabelWithValue("custom", "label")) - Expect(appsetDepl).Should(deploymentFixture.HaveTemplateLabelWithValue("custom2", "applicationSet")) - Expect(appsetDepl).Should(deploymentFixture.HaveTemplateAnnotationWithValue("custom", "annotation")) - Expect(appsetDepl).Should(deploymentFixture.HaveTemplateAnnotationWithValue("custom2", "applicationSet")) - - controllerSS := &appsv1.StatefulSet{ObjectMeta: metav1.ObjectMeta{Name: "argocd-sample-application-controller", Namespace: ns.Name}} - Eventually(controllerSS).Should(k8sFixture.ExistByName()) - Expect(controllerSS).Should(statefulsetFixture.HaveTemplateLabelWithValue("app.kubernetes.io/name", "argocd-sample-application-controller")) - Expect(controllerSS).Should(statefulsetFixture.HaveTemplateLabelWithValue("custom", "label")) - Expect(controllerSS).Should(statefulsetFixture.HaveTemplateLabelWithValue("custom2", "controller")) - Expect(controllerSS).Should(statefulsetFixture.HaveTemplateAnnotationWithValue("custom", "annotation")) - Expect(controllerSS).Should(statefulsetFixture.HaveTemplateAnnotationWithValue("custom2", "controller")) - - By("partially removing some custom labels from server and repo deployment (selective removal)") - argocdFixture.Update(argoCD, func(ac *argov1beta1api.ArgoCD) { - ac.Spec.Server.Labels = map[string]string{ - "custom2": "server", - } - ac.Spec.Repo.Labels = map[string]string{ - "custom": "label", - } - }) - - By("verifying selective label removal from server deployment") - - Eventually(func() bool { - if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(serverDepl), serverDepl); err != nil { - return false - } - labels := serverDepl.Spec.Template.Labels - if _, exists := labels["custom"]; exists { - GinkgoWriter.Printf("Label 'custom' still exists in server deployment, current labels: %v\n", labels) - return false - } - - // Should still have these labels - if labels["custom2"] != "server" { - GinkgoWriter.Printf("Label 'custom2' missing or incorrect in server deployment, current labels: %v\n", labels) - return false - } - - return true - }, "2m", "5s").Should(BeTrue()) - - By("verifying selective label removal from repo deployment") - - Eventually(func() bool { - if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(repoDepl), repoDepl); err != nil { - return false - } - labels := repoDepl.Spec.Template.Labels - if _, exists := labels["custom2"]; exists { - GinkgoWriter.Printf("Label 'custom2' still exists in repo deployment, current labels: %v\n", labels) - return false - } - - // Should still have these labels - if labels["custom"] != "label" { - GinkgoWriter.Printf("Label 'custom' missing or incorrect in repo deployment, current labels: %v\n", labels) - return false - } - return true - }, "2m", "5s").Should(BeTrue()) - - By("verifying that the labels of the other components are not affected") - Eventually(func() bool { - if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(appsetDepl), appsetDepl); err != nil { - return false - } - //Above operation should not affect the labels of the other components - appsetLabels := appsetDepl.Spec.Template.Labels - _, hasCustom := appsetLabels["custom"] - _, hasCustom2 := appsetLabels["custom2"] - - if !hasCustom || !hasCustom2 { - GinkgoWriter.Printf("Label 'custom' or 'custom2' missing from repo deployment, current labels: %v\n", appsetLabels) - return false - } - - if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(controllerSS), controllerSS); err != nil { - return false - } - controllerLabels := controllerSS.Spec.Template.Labels - _, hasCustom = controllerLabels["custom"] - _, hasCustom2 = controllerLabels["custom2"] - - if !hasCustom || !hasCustom2 { - GinkgoWriter.Printf("Label 'custom' or 'custom2' missing from controller deployment, current labels: %v\n", controllerLabels) - return false - } - return true - }, "2m", "5s").Should(BeTrue()) - - By("partially removing some custom labels from controller and appset deployment (selective removal)") - argocdFixture.Update(argoCD, func(ac *argov1beta1api.ArgoCD) { - ac.Spec.Controller.Labels = map[string]string{ - "custom": "label", - } - ac.Spec.ApplicationSet.Labels = map[string]string{ - "custom2": "applicationSet", - } - }) - - By("verifying selective label removal from controller deployment") - - Eventually(func() bool { - if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(controllerSS), controllerSS); err != nil { - return false - } - labels := controllerSS.Spec.Template.Labels - if _, exists := labels["custom2"]; exists { - GinkgoWriter.Printf("Label 'custom2' still exists in controller deployment, current labels: %v\n", labels) - return false - } - // Should still have these labels - if labels["custom"] != "label" { - GinkgoWriter.Printf("Label 'custom' missing or incorrect in controller deployment, current labels: %v\n", labels) - return false - } - return true - }, "2m", "5s").Should(BeTrue()) - - By("verifying selective label removal from appset deployment") - - Eventually(func() bool { - if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(appsetDepl), appsetDepl); err != nil { - return false - } - labels := appsetDepl.Spec.Template.Labels - if _, exists := labels["custom"]; exists { - GinkgoWriter.Printf("Label 'custom' still exists in appset deployment, current labels: %v\n", labels) - return false - } - // Should still have these labels - if labels["custom2"] != "applicationSet" { - GinkgoWriter.Printf("Label 'custom2' missing or incorrect in appset deployment, current labels: %v\n", labels) - return false - } - return true - }, "2m", "5s").Should(BeTrue()) - - By("verifying that the labels of server and repo deployments are not affected") - Eventually(func() bool { - if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(serverDepl), serverDepl); err != nil { - return false - } - if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(repoDepl), repoDepl); err != nil { - return false - } - _, hasCustom := serverDepl.Spec.Template.Labels["custom2"] - if !hasCustom { - GinkgoWriter.Printf("Label 'custom2' missing from server deployment, current labels: %v\n", serverDepl.Spec.Template.Labels) - return false - } - _, hasCustom = repoDepl.Spec.Template.Labels["custom"] - if !hasCustom { - GinkgoWriter.Printf("Label 'custom' missing from repo deployment, current labels: %v\n", repoDepl.Spec.Template.Labels) - return false - } - return true - }, "2m", "5s").Should(BeTrue()) - - By("partially removing some custom annotations from controller and server deployment (selective removal)") - argocdFixture.Update(argoCD, func(ac *argov1beta1api.ArgoCD) { - ac.Spec.Controller.Annotations = map[string]string{ - "custom": "annotation", - } - ac.Spec.Server.Annotations = map[string]string{ - "custom2": "server", - } - }) - - By("verifying selective annotation removal from controller deployment") - - Eventually(func() bool { - if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(controllerSS), controllerSS); err != nil { - return false - } - annotations := controllerSS.Spec.Template.Annotations - if _, exists := annotations["custom2"]; exists { - GinkgoWriter.Printf("Annotation 'custom2' still exists in controller deployment, current annotations: %v\n", annotations) - return false - } - // Should still have these annotations - if annotations["custom"] != "annotation" { - GinkgoWriter.Printf("Annotation 'custom' missing or incorrect in controller deployment, current annotations: %v\n", annotations) - return false - } - return true - }, "2m", "5s").Should(BeTrue()) - - By("verifying selective annotation removal from server deployment") - - Eventually(func() bool { - if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(serverDepl), serverDepl); err != nil { - return false - } - annotations := serverDepl.Spec.Template.Annotations - if _, exists := annotations["custom"]; exists { - GinkgoWriter.Printf("Annotation 'custom' still exists in server deployment, current annotations: %v\n", annotations) - return false - } - // Should still have these annotations - if annotations["custom2"] != "server" { - GinkgoWriter.Printf("Annotation 'custom2' missing or incorrect in server deployment, current annotations: %v\n", annotations) - return false - } - return true - }, "2m", "5s").Should(BeTrue()) - - By("partially removing some custom annotations from repo and appset deployment (selective removal)") - argocdFixture.Update(argoCD, func(ac *argov1beta1api.ArgoCD) { - ac.Spec.Repo.Annotations = map[string]string{ - "custom2": "repo", - } - ac.Spec.ApplicationSet.Annotations = map[string]string{ - "custom": "annotation", - } - }) - - By("verifying selective annotation removal from repo deployment") - - Eventually(func() bool { - if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(repoDepl), repoDepl); err != nil { - return false - } - annotations := repoDepl.Spec.Template.Annotations - if _, exists := annotations["custom"]; exists { - GinkgoWriter.Printf("Annotation 'custom' still exists in repo deployment, current annotations: %v\n", annotations) - return false - } - // Should still have these annotations - if annotations["custom2"] != "repo" { - GinkgoWriter.Printf("Annotation 'custom2' missing or incorrect in repo deployment, current annotations: %v\n", annotations) - return false - } - return true - }, "2m", "5s").Should(BeTrue()) - - By("verifying selective annotation removal from appset deployment") - - Eventually(func() bool { - if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(appsetDepl), appsetDepl); err != nil { - return false - } - annotations := appsetDepl.Spec.Template.Annotations - if _, exists := annotations["custom2"]; exists { - GinkgoWriter.Printf("Annotation 'custom2' still exists in appset deployment, current annotations: %v\n", annotations) - return false - } - // Should still have these annotations - if annotations["custom"] != "annotation" { - GinkgoWriter.Printf("Annotation 'custom' missing or incorrect in appset deployment, current annotations: %v\n", annotations) - return false - } - return true - }, "2m", "5s").Should(BeTrue()) - - By("completely removing all custom labels and annotations from ArgoCD CR") - - argocdFixture.Update(argoCD, func(ac *argov1beta1api.ArgoCD) { - ac.Spec.Server.Labels = map[string]string{} - ac.Spec.Server.Annotations = map[string]string{} - - ac.Spec.Repo.Labels = map[string]string{} - ac.Spec.Repo.Annotations = map[string]string{} - - ac.Spec.Controller.Labels = map[string]string{} - ac.Spec.Controller.Annotations = map[string]string{} - - ac.Spec.ApplicationSet.Labels = map[string]string{} - ac.Spec.ApplicationSet.Annotations = map[string]string{} - }) - - By("verifying labels and annotations have been removed from template specs of Argo CD components") - - Eventually(serverDepl).Should(k8sFixture.ExistByName()) - Eventually(controllerSS).Should(k8sFixture.ExistByName()) - Eventually(appsetDepl).Should(k8sFixture.ExistByName()) - Eventually(repoDepl).Should(k8sFixture.ExistByName()) - - expectLabelsAndAnnotationsRemovedFromDepl := func(depl *appsv1.Deployment, componentName string) { - - By("checking labels and annotations are removed from " + depl.Name) - - Eventually(func() bool { - if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(depl), depl); err != nil { - GinkgoWriter.Println(err) - return false - } - - for k := range depl.Spec.Template.Annotations { - if strings.Contains(k, "custom") { - return false - } - } - - for k := range depl.Spec.Template.Labels { - if strings.Contains(k, "custom") { - return false - } - } - // Verify operator-managed labels are preserved - if _, exists := depl.Spec.Template.Labels["app.kubernetes.io/name"]; !exists { - GinkgoWriter.Printf("Operator-managed label 'app.kubernetes.io/name' missing from %s deployment, current labels: %v\n", componentName, depl.Spec.Template.Labels) - return false - } - - return true - }, "2m", "5s").Should(BeTrue()) - - } - - expectLabelsAndAnnotationsRemovedFromDepl(appsetDepl, "applicationset") - expectLabelsAndAnnotationsRemovedFromDepl(serverDepl, "server") - expectLabelsAndAnnotationsRemovedFromDepl(repoDepl, "repo") - - // Evaluate the controller statefulset on its own, since it's a StatefulSet not a Deployment - Eventually(controllerSS).Should(k8sFixture.ExistByName()) - - for k := range controllerSS.Spec.Template.Annotations { - Expect(k).ToNot(ContainSubstring("custom")) - } - - for k := range controllerSS.Spec.Template.Labels { - Expect(k).ToNot(ContainSubstring("custom")) - } - - By("adding more labels") - argocdFixture.Update(argoCD, func(ac *argov1beta1api.ArgoCD) { - ac.Spec.Server.Labels = map[string]string{ - "second-label": "second-value", - "third-label": "third-value", - } - }) - - By("verifying new labels are added") - Eventually(func() bool { - if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(serverDepl), serverDepl); err != nil { - return false - } - labels := serverDepl.Spec.Template.Labels - return labels["second-label"] == "second-value" && - labels["third-label"] == "third-value" - }, "2m", "5s").Should(BeTrue()) - }) - - }) -}) +//TODO +// import ( +// "context" +// "strings" + +// . "github.com/onsi/ginkgo/v2" +// . "github.com/onsi/gomega" +// appsv1 "k8s.io/api/apps/v1" +// metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + +// argov1beta1api "github.com/argoproj-labs/argocd-operator/api/v1beta1" +// "github.com/argoproj-labs/argocd-operator/tests/ginkgo/fixture" +// argocdFixture "github.com/argoproj-labs/argocd-operator/tests/ginkgo/fixture/argocd" +// deploymentFixture "github.com/argoproj-labs/argocd-operator/tests/ginkgo/fixture/deployment" +// k8sFixture "github.com/argoproj-labs/argocd-operator/tests/ginkgo/fixture/k8s" +// statefulsetFixture "github.com/argoproj-labs/argocd-operator/tests/ginkgo/fixture/statefulset" +// fixtureUtils "github.com/argoproj-labs/argocd-operator/tests/ginkgo/fixture/utils" + +// "sigs.k8s.io/controller-runtime/pkg/client" +// ) + +// var _ = Describe("GitOps Operator Parallel E2E Tests", func() { + +// Context("1-043_validate_custom_labels_annotations", func() { + +// var ( +// k8sClient client.Client +// ctx context.Context +// ) + +// BeforeEach(func() { +// fixture.EnsureParallelCleanSlate() +// k8sClient, _ = fixtureUtils.GetE2ETestKubeClient() +// ctx = context.Background() +// }) + +// It("ensures that custom labels and annotations set on component fields of ArgoCD CR will be added to Deployment and StatefulSet templates of those components, and that they can likewise be removed", func() { + +// By("creating namespace-scoped Argo CD instance with labels and annotations set on components") +// ns, cleanupFunc := fixture.CreateRandomE2ETestNamespaceWithCleanupFunc() +// defer cleanupFunc() + +// argoCD := &argov1beta1api.ArgoCD{ +// ObjectMeta: metav1.ObjectMeta{Name: "argocd-sample", Namespace: ns.Name}, +// Spec: argov1beta1api.ArgoCDSpec{ +// Server: argov1beta1api.ArgoCDServerSpec{ +// Labels: map[string]string{ +// "custom": "label", +// "custom2": "server", +// }, +// Annotations: map[string]string{ +// "custom": "annotation", +// "custom2": "server", +// }, +// }, +// Repo: argov1beta1api.ArgoCDRepoSpec{ +// Labels: map[string]string{ +// "custom": "label", +// "custom2": "repo", +// }, +// Annotations: map[string]string{ +// "custom": "annotation", +// "custom2": "repo", +// }, +// }, +// Controller: argov1beta1api.ArgoCDApplicationControllerSpec{ +// Labels: map[string]string{ +// "custom": "label", +// "custom2": "controller", +// }, +// Annotations: map[string]string{ +// "custom": "annotation", +// "custom2": "controller", +// }, +// }, +// ApplicationSet: &argov1beta1api.ArgoCDApplicationSet{ +// Labels: map[string]string{ +// "custom": "label", +// "custom2": "applicationSet", +// }, +// Annotations: map[string]string{ +// "custom": "annotation", +// "custom2": "applicationSet", +// }, +// }, +// }, +// } +// Expect(k8sClient.Create(ctx, argoCD)).To(Succeed()) + +// By("waiting for Argo CD to become available") +// Eventually(argoCD, "5m", "5s").Should(argocdFixture.BeAvailable()) + +// By("verifying Argo CD components have the labels and annotations we set above") + +// serverDepl := &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: "argocd-sample-server", Namespace: ns.Name}} + +// Eventually(serverDepl).Should(k8sFixture.ExistByName()) +// Expect(serverDepl).Should(deploymentFixture.HaveTemplateLabelWithValue("app.kubernetes.io/name", "argocd-sample-server")) +// Expect(serverDepl).Should(deploymentFixture.HaveTemplateLabelWithValue("custom", "label")) +// Expect(serverDepl).Should(deploymentFixture.HaveTemplateLabelWithValue("custom2", "server")) + +// Expect(serverDepl).Should(deploymentFixture.HaveTemplateAnnotationWithValue("custom", "annotation")) +// Expect(serverDepl).Should(deploymentFixture.HaveTemplateAnnotationWithValue("custom2", "server")) + +// repoDepl := &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: "argocd-sample-repo-server", Namespace: ns.Name}} +// Eventually(repoDepl).Should(k8sFixture.ExistByName()) +// Expect(repoDepl).Should(deploymentFixture.HaveTemplateLabelWithValue("app.kubernetes.io/name", "argocd-sample-repo-server")) +// Expect(repoDepl).Should(deploymentFixture.HaveTemplateLabelWithValue("custom", "label")) +// Expect(repoDepl).Should(deploymentFixture.HaveTemplateLabelWithValue("custom2", "repo")) +// Expect(repoDepl).Should(deploymentFixture.HaveTemplateAnnotationWithValue("custom", "annotation")) +// Expect(repoDepl).Should(deploymentFixture.HaveTemplateAnnotationWithValue("custom2", "repo")) + +// appsetDepl := &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: "argocd-sample-applicationset-controller", Namespace: ns.Name}} +// Eventually(appsetDepl).Should(k8sFixture.ExistByName()) +// Expect(appsetDepl).Should(deploymentFixture.HaveTemplateLabelWithValue("app.kubernetes.io/name", "argocd-sample-applicationset-controller")) +// Expect(appsetDepl).Should(deploymentFixture.HaveTemplateLabelWithValue("custom", "label")) +// Expect(appsetDepl).Should(deploymentFixture.HaveTemplateLabelWithValue("custom2", "applicationSet")) +// Expect(appsetDepl).Should(deploymentFixture.HaveTemplateAnnotationWithValue("custom", "annotation")) +// Expect(appsetDepl).Should(deploymentFixture.HaveTemplateAnnotationWithValue("custom2", "applicationSet")) + +// controllerSS := &appsv1.StatefulSet{ObjectMeta: metav1.ObjectMeta{Name: "argocd-sample-application-controller", Namespace: ns.Name}} +// Eventually(controllerSS).Should(k8sFixture.ExistByName()) +// Expect(controllerSS).Should(statefulsetFixture.HaveTemplateLabelWithValue("app.kubernetes.io/name", "argocd-sample-application-controller")) +// Expect(controllerSS).Should(statefulsetFixture.HaveTemplateLabelWithValue("custom", "label")) +// Expect(controllerSS).Should(statefulsetFixture.HaveTemplateLabelWithValue("custom2", "controller")) +// Expect(controllerSS).Should(statefulsetFixture.HaveTemplateAnnotationWithValue("custom", "annotation")) +// Expect(controllerSS).Should(statefulsetFixture.HaveTemplateAnnotationWithValue("custom2", "controller")) + +// By("removing custom labels and annotations from ArgoCD CR") + +// argocdFixture.Update(argoCD, func(ac *argov1beta1api.ArgoCD) { +// ac.Spec.Server.Labels = map[string]string{} +// ac.Spec.Server.Annotations = map[string]string{} + +// ac.Spec.Repo.Labels = map[string]string{} +// ac.Spec.Repo.Annotations = map[string]string{} + +// ac.Spec.Controller.Labels = map[string]string{} +// ac.Spec.Controller.Annotations = map[string]string{} + +// ac.Spec.ApplicationSet.Labels = map[string]string{} +// ac.Spec.ApplicationSet.Annotations = map[string]string{} +// }) + +// By("verifying labels and annotations have been removed from template specs of Argo CD components") + +// Eventually(controllerSS).Should(k8sFixture.ExistByName()) +// Eventually(appsetDepl).Should(k8sFixture.ExistByName()) +// Eventually(repoDepl).Should(k8sFixture.ExistByName()) + +// expectLabelsAndAnnotationsRemovedFromDepl := func(depl *appsv1.Deployment) { + +// By("checking labels and annotations are removed from " + depl.Name) + +// Eventually(func() bool { +// if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(depl), depl); err != nil { +// GinkgoWriter.Println(err) +// return false +// } + +// for k := range depl.Spec.Template.Annotations { +// if strings.Contains(k, "custom") { +// return false +// } +// } + +// for k := range depl.Spec.Template.Labels { +// if strings.Contains(k, "custom") { +// return false +// } +// } + +// return true +// }).Should(BeTrue()) + +// } + +// expectLabelsAndAnnotationsRemovedFromDepl(appsetDepl) +// expectLabelsAndAnnotationsRemovedFromDepl(serverDepl) +// expectLabelsAndAnnotationsRemovedFromDepl(repoDepl) + +// // Evaluate the controller statefulset on its own, since it's a StatefulSet not a Deployment +// Eventually(controllerSS).Should(k8sFixture.ExistByName()) + +// for k := range controllerSS.Spec.Template.Annotations { +// Expect(k).ToNot(ContainSubstring("custom")) +// } + +// for k := range controllerSS.Spec.Template.Labels { +// Expect(k).ToNot(ContainSubstring("custom")) +// } + +// }) + +// }) +// }) diff --git a/tests/ginkgo/parallel/1-121_validate_external_labels_annotations_preserved_test.go b/tests/ginkgo/parallel/1-121_validate_external_labels_annotations_preserved_test.go new file mode 100644 index 000000000..17352623b --- /dev/null +++ b/tests/ginkgo/parallel/1-121_validate_external_labels_annotations_preserved_test.go @@ -0,0 +1,486 @@ +/* +Copyright 2025. + +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 parallel + +import ( + "context" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + appsv1 "k8s.io/api/apps/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + argov1beta1api "github.com/argoproj-labs/argocd-operator/api/v1beta1" + "github.com/argoproj-labs/argocd-operator/tests/ginkgo/fixture" + argocdFixture "github.com/argoproj-labs/argocd-operator/tests/ginkgo/fixture/argocd" + k8sFixture "github.com/argoproj-labs/argocd-operator/tests/ginkgo/fixture/k8s" + fixtureUtils "github.com/argoproj-labs/argocd-operator/tests/ginkgo/fixture/utils" + + "sigs.k8s.io/controller-runtime/pkg/client" +) + +var _ = Describe("GitOps Operator Parallel E2E Tests", func() { + + Context("1-121_validate_external_labels_annotations_preserved", func() { + + var ( + k8sClient client.Client + ctx context.Context + ) + + BeforeEach(func() { + fixture.EnsureParallelCleanSlate() + k8sClient, _ = fixtureUtils.GetE2ETestKubeClient() + ctx = context.Background() + }) + + It("ensures that labels set by external controllers are not deleted by reconciliation logic", func() { + + By("creating namespace-scoped Argo CD instance") + ns, cleanupFunc := fixture.CreateRandomE2ETestNamespaceWithCleanupFunc() + defer cleanupFunc() + + argoCD := &argov1beta1api.ArgoCD{ + ObjectMeta: metav1.ObjectMeta{Name: "argocd-sample", Namespace: ns.Name}, + Spec: argov1beta1api.ArgoCDSpec{ + Server: argov1beta1api.ArgoCDServerSpec{ + Labels: map[string]string{ + "operator-managed": "label", + }, + }, + Repo: argov1beta1api.ArgoCDRepoSpec{ + Labels: map[string]string{ + "operator-managed": "label", + }, + }, + Controller: argov1beta1api.ArgoCDApplicationControllerSpec{ + Labels: map[string]string{ + "operator-managed": "label", + }, + }, + ApplicationSet: &argov1beta1api.ArgoCDApplicationSet{ + Labels: map[string]string{ + "operator-managed": "label", + }, + }, + }, + } + Expect(k8sClient.Create(ctx, argoCD)).To(Succeed()) + + By("waiting for Argo CD to become available") + Eventually(argoCD, "5m", "5s").Should(argocdFixture.BeAvailable()) + + serverDepl := &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: "argocd-sample-server", Namespace: ns.Name}} + repoDepl := &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: "argocd-sample-repo-server", Namespace: ns.Name}} + appsetDepl := &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: "argocd-sample-applicationset-controller", Namespace: ns.Name}} + controllerSS := &appsv1.StatefulSet{ObjectMeta: metav1.ObjectMeta{Name: "argocd-sample-application-controller", Namespace: ns.Name}} + + By("verifying all components exist") + Eventually(serverDepl).Should(k8sFixture.ExistByName()) + Eventually(repoDepl).Should(k8sFixture.ExistByName()) + Eventually(appsetDepl).Should(k8sFixture.ExistByName()) + Eventually(controllerSS).Should(k8sFixture.ExistByName()) + + By("simulating external controller adding labels to server deployment") + Eventually(func() error { + if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(serverDepl), serverDepl); err != nil { + return err + } + if serverDepl.Spec.Template.Labels == nil { + serverDepl.Spec.Template.Labels = make(map[string]string) + } + serverDepl.Spec.Template.Labels["external-controller-label"] = "external-value" + serverDepl.Spec.Template.Labels["monitoring.io/enabled"] = "true" + return k8sClient.Update(ctx, serverDepl) + }, "30s", "2s").Should(Succeed()) + + By("simulating external controller adding labels to repo deployment") + Eventually(func() error { + if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(repoDepl), repoDepl); err != nil { + return err + } + if repoDepl.Spec.Template.Labels == nil { + repoDepl.Spec.Template.Labels = make(map[string]string) + } + repoDepl.Spec.Template.Labels["external-controller-label"] = "external-value" + repoDepl.Spec.Template.Labels["backup.io/enabled"] = "true" + return k8sClient.Update(ctx, repoDepl) + }, "30s", "2s").Should(Succeed()) + + By("simulating external controller adding labels to applicationset deployment") + Eventually(func() error { + if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(appsetDepl), appsetDepl); err != nil { + return err + } + if appsetDepl.Spec.Template.Labels == nil { + appsetDepl.Spec.Template.Labels = make(map[string]string) + } + appsetDepl.Spec.Template.Labels["external-controller-label"] = "external-value" + appsetDepl.Spec.Template.Labels["sidecar.io/inject"] = "true" + return k8sClient.Update(ctx, appsetDepl) + }, "30s", "2s").Should(Succeed()) + + By("simulating external controller adding labels to controller statefulset") + Eventually(func() error { + if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(controllerSS), controllerSS); err != nil { + return err + } + if controllerSS.Spec.Template.Labels == nil { + controllerSS.Spec.Template.Labels = make(map[string]string) + } + controllerSS.Spec.Template.Labels["external-controller-label"] = "external-value" + controllerSS.Spec.Template.Labels["network-policy.io/allow"] = "true" + return k8sClient.Update(ctx, controllerSS) + }, "30s", "2s").Should(Succeed()) + + By("verifying external labels were added successfully") + Eventually(func() bool { + if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(serverDepl), serverDepl); err != nil { + return false + } + return serverDepl.Spec.Template.Labels["external-controller-label"] == "external-value" && + serverDepl.Spec.Template.Labels["monitoring.io/enabled"] == "true" + }, "30s", "2s").Should(BeTrue()) + + By("triggering reconciliation by updating ArgoCD CR with a new label") + argocdFixture.Update(argoCD, func(ac *argov1beta1api.ArgoCD) { + ac.Spec.Server.Labels["operator-managed-new"] = "new-label" + ac.Spec.Repo.Labels["operator-managed-new"] = "new-label" + ac.Spec.Controller.Labels["operator-managed-new"] = "new-label" + ac.Spec.ApplicationSet.Labels["operator-managed-new"] = "new-label" + }) + + By("waiting for reconciliation to complete") + Eventually(func() bool { + if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(serverDepl), serverDepl); err != nil { + return false + } + return serverDepl.Spec.Template.Labels["operator-managed-new"] == "new-label" + }, "2m", "5s").Should(BeTrue()) + + By("verifying external labels on server deployment are preserved after reconciliation") + Eventually(func() bool { + if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(serverDepl), serverDepl); err != nil { + GinkgoWriter.Printf("Failed to get server deployment: %v\n", err) + return false + } + labels := serverDepl.Spec.Template.Labels + hasExternal := labels["external-controller-label"] == "external-value" + hasMonitoring := labels["monitoring.io/enabled"] == "true" + hasOperatorManaged := labels["operator-managed"] == "label" + hasNewLabel := labels["operator-managed-new"] == "new-label" + + if !hasExternal || !hasMonitoring { + GinkgoWriter.Printf("Server deployment missing external labels. Current labels: %v\n", labels) + return false + } + if !hasOperatorManaged || !hasNewLabel { + GinkgoWriter.Printf("Server deployment missing operator labels. Current labels: %v\n", labels) + return false + } + return true + }, "2m", "5s").Should(BeTrue()) + + By("verifying external labels on repo deployment are preserved after reconciliation") + Eventually(func() bool { + if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(repoDepl), repoDepl); err != nil { + GinkgoWriter.Printf("Failed to get repo deployment: %v\n", err) + return false + } + labels := repoDepl.Spec.Template.Labels + hasExternal := labels["external-controller-label"] == "external-value" + hasBackup := labels["backup.io/enabled"] == "true" + hasOperatorManaged := labels["operator-managed"] == "label" + hasNewLabel := labels["operator-managed-new"] == "new-label" + + if !hasExternal || !hasBackup { + GinkgoWriter.Printf("Repo deployment missing external labels. Current labels: %v\n", labels) + return false + } + if !hasOperatorManaged || !hasNewLabel { + GinkgoWriter.Printf("Repo deployment missing operator labels. Current labels: %v\n", labels) + return false + } + return true + }, "2m", "5s").Should(BeTrue()) + + By("verifying external labels on applicationset deployment are preserved after reconciliation") + Eventually(func() bool { + if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(appsetDepl), appsetDepl); err != nil { + GinkgoWriter.Printf("Failed to get appset deployment: %v\n", err) + return false + } + labels := appsetDepl.Spec.Template.Labels + hasExternal := labels["external-controller-label"] == "external-value" + hasSidecar := labels["sidecar.io/inject"] == "true" + hasOperatorManaged := labels["operator-managed"] == "label" + hasNewLabel := labels["operator-managed-new"] == "new-label" + + if !hasExternal || !hasSidecar { + GinkgoWriter.Printf("Applicationset deployment missing external labels. Current labels: %v\n", labels) + return false + } + if !hasOperatorManaged || !hasNewLabel { + GinkgoWriter.Printf("Applicationset deployment missing operator labels. Current labels: %v\n", labels) + return false + } + return true + }, "2m", "5s").Should(BeTrue()) + + By("verifying external labels on controller statefulset are preserved after reconciliation") + Eventually(func() bool { + if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(controllerSS), controllerSS); err != nil { + GinkgoWriter.Printf("Failed to get controller statefulset: %v\n", err) + return false + } + labels := controllerSS.Spec.Template.Labels + hasExternal := labels["external-controller-label"] == "external-value" + hasNetworkPolicy := labels["network-policy.io/allow"] == "true" + hasOperatorManaged := labels["operator-managed"] == "label" + hasNewLabel := labels["operator-managed-new"] == "new-label" + + if !hasExternal || !hasNetworkPolicy { + GinkgoWriter.Printf("Controller statefulset missing external labels. Current labels: %v\n", labels) + return false + } + if !hasOperatorManaged || !hasNewLabel { + GinkgoWriter.Printf("Controller statefulset missing operator labels. Current labels: %v\n", labels) + return false + } + return true + }, "2m", "5s").Should(BeTrue()) + + By("simulating external controller adding annotations to server deployment") + Eventually(func() error { + if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(serverDepl), serverDepl); err != nil { + return err + } + if serverDepl.Spec.Template.Annotations == nil { + serverDepl.Spec.Template.Annotations = make(map[string]string) + } + serverDepl.Spec.Template.Annotations["external-controller-annotation"] = "external-annotation-value" + return k8sClient.Update(ctx, serverDepl) + }, "30s", "2s").Should(Succeed()) + + By("simulating external controller adding annotations to repo deployment") + Eventually(func() error { + if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(repoDepl), repoDepl); err != nil { + return err + } + if repoDepl.Spec.Template.Annotations == nil { + repoDepl.Spec.Template.Annotations = make(map[string]string) + } + repoDepl.Spec.Template.Annotations["external-controller-annotation"] = "external-annotation-value" + return k8sClient.Update(ctx, repoDepl) + }, "30s", "2s").Should(Succeed()) + + By("simulating external controller adding annotations to applicationset deployment") + Eventually(func() error { + if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(appsetDepl), appsetDepl); err != nil { + return err + } + if appsetDepl.Spec.Template.Annotations == nil { + appsetDepl.Spec.Template.Annotations = make(map[string]string) + } + appsetDepl.Spec.Template.Annotations["external-controller-annotation"] = "external-annotation-value" + return k8sClient.Update(ctx, appsetDepl) + }, "30s", "2s").Should(Succeed()) + + By("simulating external controller adding annotations to controller statefulset") + Eventually(func() error { + if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(controllerSS), controllerSS); err != nil { + return err + } + if controllerSS.Spec.Template.Annotations == nil { + controllerSS.Spec.Template.Annotations = make(map[string]string) + } + controllerSS.Spec.Template.Annotations["external-controller-annotation"] = "external-annotation-value" + return k8sClient.Update(ctx, controllerSS) + }, "30s", "2s").Should(Succeed()) + + By("verifying external annotations were added successfully") + Eventually(func() bool { + if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(serverDepl), serverDepl); err != nil { + return false + } + return serverDepl.Spec.Template.Annotations["external-controller-annotation"] == "external-annotation-value" + }, "30s", "2s").Should(BeTrue()) + + By("triggering reconciliation by adding operator-managed annotations") + argocdFixture.Update(argoCD, func(ac *argov1beta1api.ArgoCD) { + ac.Spec.Server.Annotations = map[string]string{ + "operator-annotation": "operator-value", + } + ac.Spec.Repo.Annotations = map[string]string{ + "operator-annotation": "operator-value", + } + ac.Spec.Controller.Annotations = map[string]string{ + "operator-annotation": "operator-value", + } + ac.Spec.ApplicationSet.Annotations = map[string]string{ + "operator-annotation": "operator-value", + } + }) + + By("waiting for operator annotations to be applied") + Eventually(func() bool { + if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(serverDepl), serverDepl); err != nil { + return false + } + return serverDepl.Spec.Template.Annotations["operator-annotation"] == "operator-value" + }, "2m", "5s").Should(BeTrue()) + + By("verifying external annotations on server deployment are preserved after reconciliation") + Eventually(func() bool { + if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(serverDepl), serverDepl); err != nil { + GinkgoWriter.Printf("Failed to get server deployment: %v\n", err) + return false + } + annotations := serverDepl.Spec.Template.Annotations + hasExternal := annotations["external-controller-annotation"] == "external-annotation-value" + hasOperator := annotations["operator-annotation"] == "operator-value" + + if !hasExternal { + GinkgoWriter.Printf("Server deployment missing external annotations. Current annotations: %v\n", annotations) + return false + } + if !hasOperator { + GinkgoWriter.Printf("Server deployment missing operator annotation. Current annotations: %v\n", annotations) + return false + } + return true + }, "2m", "5s").Should(BeTrue()) + + By("verifying external annotations on repo deployment are preserved after reconciliation") + Eventually(func() bool { + if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(repoDepl), repoDepl); err != nil { + GinkgoWriter.Printf("Failed to get repo deployment: %v\n", err) + return false + } + annotations := repoDepl.Spec.Template.Annotations + hasExternal := annotations["external-controller-annotation"] == "external-annotation-value" + hasOperator := annotations["operator-annotation"] == "operator-value" + + if !hasExternal { + GinkgoWriter.Printf("Repo deployment missing external annotations. Current annotations: %v\n", annotations) + return false + } + if !hasOperator { + GinkgoWriter.Printf("Repo deployment missing operator annotation. Current annotations: %v\n", annotations) + return false + } + return true + }, "2m", "5s").Should(BeTrue()) + + By("verifying external annotations on applicationset deployment are preserved after reconciliation") + Eventually(func() bool { + if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(appsetDepl), appsetDepl); err != nil { + GinkgoWriter.Printf("Failed to get appset deployment: %v\n", err) + return false + } + annotations := appsetDepl.Spec.Template.Annotations + hasExternal := annotations["external-controller-annotation"] == "external-annotation-value" + hasOperator := annotations["operator-annotation"] == "operator-value" + + if !hasExternal { + GinkgoWriter.Printf("Applicationset deployment missing external annotations. Current annotations: %v\n", annotations) + return false + } + if !hasOperator { + GinkgoWriter.Printf("Applicationset deployment missing operator annotation. Current annotations: %v\n", annotations) + return false + } + return true + }, "2m", "5s").Should(BeTrue()) + + By("verifying external annotations on controller statefulset are preserved after reconciliation") + Eventually(func() bool { + if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(controllerSS), controllerSS); err != nil { + GinkgoWriter.Printf("Failed to get controller statefulset: %v\n", err) + return false + } + annotations := controllerSS.Spec.Template.Annotations + hasExternal := annotations["external-controller-annotation"] == "external-annotation-value" + hasOperator := annotations["operator-annotation"] == "operator-value" + + if !hasExternal { + GinkgoWriter.Printf("Controller statefulset missing external annotations. Current annotations: %v\n", annotations) + return false + } + if !hasOperator { + GinkgoWriter.Printf("Controller statefulset missing operator annotation. Current annotations: %v\n", annotations) + return false + } + return true + }, "2m", "5s").Should(BeTrue()) + + By("updating operator-managed annotations to trigger another reconciliation") + argocdFixture.Update(argoCD, func(ac *argov1beta1api.ArgoCD) { + ac.Spec.Server.Annotations["operator-annotation-new"] = "new-operator-value" + ac.Spec.Repo.Annotations["operator-annotation-new"] = "new-operator-value" + ac.Spec.Controller.Annotations["operator-annotation-new"] = "new-operator-value" + ac.Spec.ApplicationSet.Annotations["operator-annotation-new"] = "new-operator-value" + }) + + By("verifying external annotations and labels are still preserved after second reconciliation") + Eventually(func() bool { + if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(serverDepl), serverDepl); err != nil { + return false + } + labels := serverDepl.Spec.Template.Labels + annotations := serverDepl.Spec.Template.Annotations + return labels["external-controller-label"] == "external-value" && + annotations["external-controller-annotation"] == "external-annotation-value" && + annotations["operator-annotation-new"] == "new-operator-value" + }, "2m", "5s").Should(BeTrue()) + + Eventually(func() bool { + if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(repoDepl), repoDepl); err != nil { + return false + } + labels := repoDepl.Spec.Template.Labels + annotations := repoDepl.Spec.Template.Annotations + return labels["external-controller-label"] == "external-value" && + annotations["external-controller-annotation"] == "external-annotation-value" && + annotations["operator-annotation-new"] == "new-operator-value" + }, "2m", "5s").Should(BeTrue()) + + Eventually(func() bool { + if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(appsetDepl), appsetDepl); err != nil { + return false + } + labels := appsetDepl.Spec.Template.Labels + annotations := appsetDepl.Spec.Template.Annotations + return labels["external-controller-label"] == "external-value" && + annotations["external-controller-annotation"] == "external-annotation-value" && + annotations["operator-annotation-new"] == "new-operator-value" + }, "2m", "5s").Should(BeTrue()) + + Eventually(func() bool { + if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(controllerSS), controllerSS); err != nil { + return false + } + labels := controllerSS.Spec.Template.Labels + annotations := controllerSS.Spec.Template.Annotations + return labels["external-controller-label"] == "external-value" && + annotations["external-controller-annotation"] == "external-annotation-value" && + annotations["operator-annotation-new"] == "new-operator-value" + }, "2m", "5s").Should(BeTrue()) + }) + + }) +})