fix: add guard for deletion depending on if CR owns APIService for promoter

argocd-operator/controllers/argocd/util.go

@@ -734,7 +734,8 @@ func (r *ReconcileArgoCD) deleteClusterResources(cr *argoproj.ArgoCD) error {
 		return fmt.Errorf("failed to filter APIServices for %s: %w", cr.Name, err)
 	}
 
-	if err := gitopspromoter.DeleteAPIServices(r.Client, apiSvcList); err != nil {
+	apiServerCompName := string(argoproj.PromoterComponentTypeAPIServer)
+	if err := gitopspromoter.DeleteAPIServices(r.Client, apiSvcList, apiServerCompName, cr); err != nil {
 		return err
 	}
 
@@ -826,7 +827,6 @@ func removeString(slice []string, s string) []string {
 
 // setResourceWatches will register Watches for each of the supported Resources.
 func (r *ReconcileArgoCD) setResourceWatches(bldr *builder.Builder, clusterResourceMapper, tlsSecretMapper, namespaceResourceMapper, clusterSecretResourceMapper, applicationSetGitlabSCMTLSConfigMapMapper, nmMapper, systemCATrustMapper, imagePullSecretMapper handler.MapFunc) *builder.Builder {
-
 	// Add new predicate to delete Notifications Resources. The predicate watches the Argo CD CR for changes to the `.spec.Notifications.Enabled`
 	// field. When a change is detected that results in notifications being disabled, we trigger deletion of notifications resources
 	deleteNotificationsPred := predicate.Funcs{

argocd-operator/controllers/gitopspromoter/apiservice.go

@@ -56,10 +56,17 @@ func ReconcilePromoterAPIServerAPIService(client client.Client, compName string,
 	}
 
 	if exists {
+		// Check to see if referenced service was created by the CR if it is not do not do any operations on it
+		// This safe guards from another ArgoCD CR instance from deleting the APIService even if the promoter is disabled
+		// If the service field is for some reason nil be safe and skip deleting anyway
+		if !ownsAPISvc(compName, cr, apiSvc) {
+			return apiSvc, nil
+		}
+
 		if !cr.Spec.Promoter.IsEnabled() || !enabled || !allowed {
 			argoutil.LogResourceDeletion(log, apiSvc, fmt.Sprintf("promoter apiservice for component %s is being deleted due to being disabled", compName))
 			if err := client.Delete(context.Background(), apiSvc); err != nil {
-				return nil, fmt.Errorf("failed to delete promoter service %s: %v", apiSvc.Name, err)
+				return nil, fmt.Errorf("failed to delete promoter api service %s: %v", apiSvc.Name, err)
 			}
 			return apiSvc, nil
 		}
@@ -149,12 +156,20 @@ func buildAPIServiceSpec(client client.Client, compName string, cr *argoproj.Arg
 }
 
 // DeleteAPIServices deletes a list of API Services
-func DeleteAPIServices(c client.Client, apiSvcList *apiregistrationv1.APIServiceList) error {
+func DeleteAPIServices(c client.Client, apiSvcList *apiregistrationv1.APIServiceList, compName string, cr *argoproj.ArgoCD) error {
 	for _, apiSvc := range apiSvcList.Items {
+		if !ownsAPISvc(compName, cr, &apiSvc) {
+			continue
+		}
+
 		argoutil.LogResourceDeletion(log, &apiSvc, "cleaning up cluster resources")
 		if err := c.Delete(context.TODO(), &apiSvc); err != nil {
 			return fmt.Errorf("failed to delete APIService %s during cleanup: %w", apiSvc.Name, err)
 		}
 	}
 	return nil
 }
+
+func ownsAPISvc(compName string, cr *argoproj.ArgoCD, apiSvc *apiregistrationv1.APIService) bool {
+	return apiSvc.Spec.Service != nil && cr.Namespace == apiSvc.Spec.Service.Namespace && generatePromoterResourceName(compName, cr) == apiSvc.Spec.Service.Name
+}

argocd-operator/controllers/gitopspromoter/apiservice_test.go

@@ -370,8 +370,6 @@ func TestReconcilePromoterAPIServerAPIService_Exists_Update(t *testing.T) {
 	cr := makeTestArgoCD(withPromoterEnabled(true), withPromoterAPIServerEnabled(true))
 
 	existingAPIService := makeExistingAPIService(cr)
-	existingAPIService.Spec.Service.Name = "not-a-real-service"
-	existingAPIService.Spec.Service.Namespace = "not-a-real-namespace"
 	existingAPIService.Spec.Service.Port = ptr.To(int32(25565))
 
 	resObjs := []client.Object{cr, existingAPIService}
@@ -392,3 +390,38 @@ func TestReconcilePromoterAPIServerAPIService_Exists_Update(t *testing.T) {
 	assert.Equal(t, cr.Namespace, retrievedAPIService.Spec.Service.Namespace)
 	assert.Equal(t, ptr.To(int32(APIServerPort)), retrievedAPIService.Spec.Service.Port)
 }
+
+func TestReconcilePromoterAPIServerAPIService_MultipleCRsPresent(t *testing.T) {
+	// Test case: APIService gets reconciled then another CR with the promoter disabled gets reconciled
+	// Expected behavior: the APIService should not get deleted
+
+	crEnabled := makeTestArgoCD(withPromoterEnabled(true), withPromoterAPIServerEnabled(true))
+	crDisabled := makeTestArgoCD(withPromoterEnabled(false), withPromoterAPIServerEnabled(false))
+	crDisabled.Name = "disabled-cr"
+	crDisabled.Namespace = "different-namespace"
+
+	resObjs := []client.Object{crEnabled, crDisabled}
+	sch := makeTestReconcilerScheme()
+	client := makeTestReconcilerClient(sch, resObjs)
+
+	apiService, err := ReconcilePromoterAPIServerAPIService(client, testCompName, crEnabled)
+	assert.NoError(t, err)
+	assert.NotNil(t, apiService)
+
+	// Make sure APIService as created
+	retrievedAPIService := &apiregistrationv1.APIService{}
+	err = client.Get(context.Background(), types.NamespacedName{
+		Name: "v1alpha1.view.promoter.argoproj.io",
+	}, retrievedAPIService)
+	assert.NoError(t, err)
+
+	apiService, err = ReconcilePromoterAPIServerAPIService(client, testCompName, crDisabled)
+	assert.NoError(t, err)
+	assert.NotNil(t, apiService)
+
+	// Get APIService again to make sure it is not deleted
+	err = client.Get(context.Background(), types.NamespacedName{
+		Name: "v1alpha1.view.promoter.argoproj.io",
+	}, retrievedAPIService)
+	assert.NoError(t, err)
+}