refactor: Added pattern checker. Handled deletion errors. Added test cases.

.gitignore

@@ -27,15 +27,21 @@ go.work
 *.swo
 *~
 
-appconfig/local.yaml
-
 # Any certificate files
 *.key
 *.csr
 *.p12
 *.crt
 *.pem
+
+# Environment variables and secrets
 .env
+.env.local
+.env.*.local
+
+# Local development config with secrets
+appconfig/local*.yaml
+appconfig/.env*
 
 # Secrets
 secrets/

Makefile

@@ -120,9 +120,9 @@ mockgen: ## Generate mocks.
 		go install github.com/golang/mock/mockgen@latest; \
 	fi
 	@echo "Generating mocks"
-	@mockgen -source=pkg/clients/ldap/client.go -destination=pkg/clients/ldap/mocks/ldap_mock.go -package=mocks LDAPConnClient
-	@mockgen -source=pkg/clients/ldap/client.go -destination=internal/controller/mocks/ldap_mock.go -package=mocks LDAPClient
-	@mockgen -source=pkg/clients/client.go -destination=internal/controller/periodicjobs/mocks/client_mock.go -package=mocks Client
+	@$(GOBIN)/mockgen -source=pkg/clients/ldap/client.go -destination=pkg/clients/ldap/mocks/ldap_mock.go -package=mocks LDAPConnClient
+	@$(GOBIN)/mockgen -source=pkg/clients/ldap/client.go -destination=internal/controller/mocks/ldap_mock.go -package=mocks LDAPClient
+	@$(GOBIN)/mockgen -source=pkg/clients/client.go -destination=internal/controller/periodicjobs/mocks/client_mock.go -package=mocks Client
 
 .PHONY: test
 test: mockgen manifests generate fmt vet envtest ## Run tests.

appconfig/default.yaml

@@ -119,4 +119,3 @@ offboardUserExclusionListConfigPath: "default_offboard_user_exclusion_list"
 # Controller configuration
 controllerConfig:
   maxConcurrentReconciles: 1
-

config/samples/v1alpha1_group.yaml

@@ -11,7 +11,7 @@ spec:
   members:
    users:
     - subhatta
-    - vinod
+    - rmandal
   group_params:
     - backend: gitlab
       name: gitlab
@@ -23,4 +23,5 @@ spec:
   backends:
   - name: fivetran
     type: fivetran
-
+  - name: rhplatformtest
+    type: "snowflake"

config/samples/v2_cleanup_group.yaml

@@ -0,0 +1,19 @@
+apiVersion: operator.dataverse.redhat.com/v1alpha1
+kind: Group
+metadata:
+  labels:
+    app.kubernetes.io/name: usernaut
+    app.kubernetes.io/managed-by: kustomize
+  name: dataverse-source-cleanup-test
+  namespace: usernaut
+spec:
+  group_name: dataverse-source-cleanup-test
+  members:
+   users:
+    - ruagrawa
+    - rmandal
+    - subhatta
+  backends:
+  - name: fivetran
+    type: fivetran
+

config/samples/v3_consumer_group.yaml

@@ -0,0 +1,19 @@
+apiVersion: operator.dataverse.redhat.com/v1alpha1
+kind: Group
+metadata:
+  labels:
+    app.kubernetes.io/name: usernaut
+    app.kubernetes.io/managed-by: kustomize
+  name: dataverse-consumer-bookingmaster-test
+  namespace: usernaut
+spec:
+  group_name: dataverse-consumer-bookingmaster-test
+  members:
+   users:
+    - ruagrawa
+    - rmandal
+    - atikande
+  backends:
+  - name: fivetran
+    type: fivetran
+

config/samples/v3_invalid_group.yaml

@@ -0,0 +1,19 @@
+apiVersion: operator.dataverse.redhat.com/v1alpha1
+kind: Group
+metadata:
+  labels:
+    app.kubernetes.io/name: usernaut
+    app.kubernetes.io/managed-by: kustomize
+  name: invalid-group-test-name
+  namespace: usernaut
+spec:
+  group_name: invalid-group-test-name
+  members:
+   users:
+    - ruagrawa
+    - rmandal
+    - atikande
+  backends:
+  - name: fivetran
+    type: fivetran
+

config/samples/v3_valid_consumer_group.yaml

@@ -0,0 +1,18 @@
+apiVersion: operator.dataverse.redhat.com/v1alpha1
+kind: Group
+metadata:
+  labels:
+    app.kubernetes.io/name: usernaut
+    app.kubernetes.io/managed-by: kustomize
+  name: dataverse-consumer-bookingmaster-test
+  namespace: usernaut
+spec:
+  group_name: dataverse-consumer-bookingmaster-test
+  members:
+   users:
+    - ruagrawa
+    - atikande
+  backends:
+  - name: fivetran
+    type: fivetran
+

config/samples/v4_invalid_consumer_group.yaml

@@ -0,0 +1,19 @@
+apiVersion: operator.dataverse.redhat.com/v1alpha1
+kind: Group
+metadata:
+  labels:
+    app.kubernetes.io/name: usernaut
+    app.kubernetes.io/managed-by: kustomize
+  name: invalid-consumer-usernaut-test
+  namespace: usernaut
+spec:
+  group_name: invalid-consumer-usernaut-test
+  members:
+   users:
+    - ruagrawa
+    - atikande
+    - subhatta
+  backends:
+  - name: fivetran
+    type: fivetran
+

config/samples/v5_valid_consumer_group.yaml

@@ -0,0 +1,17 @@
+apiVersion: operator.dataverse.redhat.com/v1alpha1
+kind: Group
+metadata:
+  labels:
+    app.kubernetes.io/name: usernaut
+    app.kubernetes.io/managed-by: kustomize
+  name: dataverse-aggregate-usernaut
+  namespace: usernaut
+spec:
+  group_name: dataverse-aggregate-usernaut
+  members:
+   users:
+    - ruagrawa
+    - rmandal
+  backends:
+  - name: fivetran
+    type: fivetran

internal/controller/group_controller.go

@@ -130,6 +130,28 @@ func (r *GroupReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl
 		"groups":         groupCR.Spec.Members.Groups,
 	})
 
+	// Check if the group is configurable (has matching patterns for its backends)
+	isConfigurable := r.isGroupConfigurable(groupCR)
+	if !isConfigurable {
+		r.log.Warn("group is not configurable - no matching patterns found for backends")
+		// Mark as non-configurable in status
+		groupCR.Status.ReconciledUsers = []string{}
+		condition := metav1.Condition{
+			Type:               usernautdevv1alpha1.GroupReadyCondition,
+			LastTransitionTime: metav1.Now(),
+			Status:             metav1.ConditionFalse,
+			Message:            "Group is not configurable - no matching patterns found in backend configuration",
+			Reason:             "NonConfigurable",
+			ObservedGeneration: groupCR.Generation,
+		}
+		r.setCondition(&groupCR.Status.Conditions, condition)
+		if err := r.Status().Update(ctx, groupCR); err != nil {
+			r.log.WithError(err).Error("error updating group status for non-configurable group")
+			return ctrl.Result{}, err
+		}
+		return ctrl.Result{}, nil
+	}
+
 	var err error
 	queryMembers := []string{}
 	if groupCR.Spec.Members.LDAPQuery != nil {
@@ -697,9 +719,7 @@ func (r *GroupReconciler) handleDeletion(ctx context.Context, groupCR *usernautd
 		// Clean up user:groups reverse index for all members of this group
 		r.cleanupUserGroupsIndex(ctx, groupCR.Spec.GroupName)
 
-		if err := r.deleteBackendsTeam(ctx, groupCR); err != nil {
-			return err
-		}
+		r.deleteBackendsTeam(ctx, groupCR)
 
 		controllerutil.RemoveFinalizer(groupCR, groupFinalizer)
 		if err := r.Update(ctx, groupCR); err != nil {
@@ -736,63 +756,97 @@ func (r *GroupReconciler) cleanupUserGroupsIndex(ctx context.Context, groupName
 	r.log.WithField("group", groupName).Info("cleaned up user groups index successfully")
 }
 
-func (r *GroupReconciler) deleteBackendsTeam(ctx context.Context, groupCR *usernautdevv1alpha1.Group) error {
+// deleteBackendsTeam performs best-effort backend and cache cleanup during deletion.
+// It does not return an error: failures are logged so the finalizer can still be removed.
+func (r *GroupReconciler) deleteBackendsTeam(ctx context.Context, groupCR *usernautdevv1alpha1.Group) {
 	r.log.Info("Finalizer: starting Backends team deletion cleanup")
 	groupName := groupCR.Spec.GroupName
+	hasErrors := false
 
 	for _, backend := range groupCR.Spec.Backends {
-		transformedGroupName, err := utils.GetTransformedGroupName(r.AppConfig, backend.Type, groupName)
+		// Use graceful fallback for deletion - we want to clean up even if pattern doesn't match
+		transformedGroupName := utils.GetTransformedGroupNameOrFallback(r.AppConfig, backend.Type, groupName)
 		backendLoggerInfo := r.log.WithFields(logrus.Fields{
 			"group_name":            groupName,
 			"transformed_team_name": transformedGroupName,
 			"backend":               backend.Name,
 			"backend_type":          backend.Type,
 		})
 		backendLoggerInfo.Info("Finalizer: Deleting team from backend")
-		if err != nil {
-			backendLoggerInfo.WithError(err).Error("Finalizer: Error in transforming group name")
-			return err
-		}
 
 		backendClient, err := clients.New(backend.Name, backend.Type, r.AppConfig.BackendMap)
 		if err != nil {
-			backendLoggerInfo.WithError(err).Errorf("Finalizer: error creating client for backend %s", backend.Name)
-			return err
+			backendLoggerInfo.WithError(err).Warnf("Finalizer: error creating client for backend %s, skipping this backend", backend.Name)
+			hasErrors = true
+			continue // Skip this backend but continue with others
 		}
 
 		// Get team ID from consolidated group store (using original group name)
 		// NOTE: CacheMutex is already held by caller (handleDeletion)
 		teamID, err := r.Store.Group.GetBackendID(ctx, groupName, backend.Name, backend.Type)
 		if err != nil {
-			backendLoggerInfo.WithError(err).Error("Finalizer: error fetching team details from cache")
-			return err
+			backendLoggerInfo.WithError(err).Warn("Finalizer: error fetching team details from cache, team may not have been created")
+			hasErrors = true
+		}
+
+		// Same resolution order as fetchOrCreateTeam: GroupStore first, then TeamStore (preload) by transformed name
+		if teamID == "" && transformedGroupName != "" {
+			backendKey := backend.Name + "_" + backend.Type
+			teamBackends, tsErr := r.Store.Team.GetBackends(ctx, transformedGroupName)
+			if tsErr != nil {
+				backendLoggerInfo.WithError(tsErr).Warn("Finalizer: error fetching team from TeamStore during deletion")
+				hasErrors = true
+			} else if id, ok := teamBackends[backendKey]; ok && id != "" {
+				backendLoggerInfo.WithField("team_id", id).Info("Finalizer: resolved team ID from TeamStore for backend deletion")
+				teamID = id
+			}
 		}
 
 		if teamID != "" {
 			backendLoggerInfo.Infof("Finalizer: Deleting team with (ID: %s) from Backend %s", teamID, backend.Type)
 
 			if err := backendClient.DeleteTeamByID(ctx, teamID); err != nil {
-				backendLoggerInfo.WithError(err).Error("Finalizer: failed to delete team from the backend")
-				return err
+				backendLoggerInfo.WithError(err).Warn("Finalizer: failed to delete team from the backend, team may already be deleted")
+				hasErrors = true
+				// Continue processing - best effort deletion
+			} else {
+				backendLoggerInfo.Infof("Finalizer: Successfully deleted team with id '%s' from Backend %s", teamID, backend.Type)
+			}
+		} else if strings.EqualFold(backend.Type, "snowflake") && transformedGroupName != "" {
+			// Snowflake uses the role name as the REST identifier (see snowflake.CreateTeam / DeleteTeamByID).
+			roleName := strings.ToLower(transformedGroupName)
+			backendLoggerInfo.WithField("role_name", roleName).Info("Finalizer: no cached team ID; attempting Snowflake role delete by name")
+			if err := backendClient.DeleteTeamByID(ctx, roleName); err != nil {
+				backendLoggerInfo.WithError(err).Warn("Finalizer: Snowflake delete by role name failed; role may not exist or actual name may differ (e.g. pattern changed since create)")
+				hasErrors = true
+			} else {
+				backendLoggerInfo.Infof("Finalizer: Successfully deleted Snowflake role '%s'", roleName)
 			}
-			backendLoggerInfo.Infof("Finalizer: Successfully deleted team with id '%s' from Backend %s", teamID, backend.Type)
+		} else {
+			backendLoggerInfo.Info("Finalizer: No team ID found in cache, skipping backend deletion")
 		}
 
 		// Delete team entry from TeamStore (used for preload lookups)
-		if err := r.Store.Team.Delete(ctx, transformedGroupName); err != nil {
-			backendLoggerInfo.WithError(err).Warn("Finalizer: failed to delete team from TeamStore cache")
-			// Continue processing - TeamStore is secondary cache
+		if transformedGroupName != "" {
+			if err := r.Store.Team.Delete(ctx, transformedGroupName); err != nil {
+				backendLoggerInfo.WithError(err).Warn("Finalizer: failed to delete team from TeamStore cache")
+				// Continue processing - TeamStore is secondary cache
+			}
 		}
 	}
 
 	// Delete the entire group entry from cache (includes all backends and members)
 	if err := r.Store.Group.Delete(ctx, groupName); err != nil {
-		r.log.WithError(err).Error("Finalizer: failed to delete group from cache")
-		return err
+		r.log.WithError(err).Warn("Finalizer: failed to delete group from cache, may already be deleted")
+		hasErrors = true
+		// Don't return error - allow finalizer to complete
+	} else {
+		r.log.WithField("group", groupName).Info("Finalizer: Successfully deleted group from cache")
 	}
-	r.log.WithField("group", groupName).Info("Finalizer: Successfully deleted group from cache")
 
-	return nil
+	if hasErrors {
+		r.log.Warn("Finalizer: completed with some errors, but allowing deletion to proceed as it is a best-effort cleanup")
+	}
 }
 
 func (r *GroupReconciler) processUsers(ctx context.Context,
@@ -980,6 +1034,42 @@ func (r *GroupReconciler) fetchOrCreateTeam(ctx context.Context,
 	return newTeam.ID, nil
 }
 
+// isGroupConfigurable checks if a group has matching patterns for all its backends
+// A group is considered configurable if at least one backend has a pattern that matches the group name
+func (r *GroupReconciler) isGroupConfigurable(groupCR *usernautdevv1alpha1.Group) bool {
+	if len(groupCR.Spec.Backends) == 0 {
+		// No backends specified, consider it non-configurable
+		return false
+	}
+
+	for _, backend := range groupCR.Spec.Backends {
+		_, err := utils.GetTransformedGroupName(r.AppConfig, backend.Type, groupCR.Spec.GroupName)
+		if err == nil {
+			// At least one backend has a matching pattern
+			return true
+		}
+	}
+
+	// No backends have matching patterns
+	return false
+}
+
+// setCondition updates or adds a condition to the condition slice
+func (r *GroupReconciler) setCondition(conditions *[]metav1.Condition, newCondition metav1.Condition) {
+	if conditions == nil {
+		*conditions = []metav1.Condition{}
+	}
+
+	for i, cond := range *conditions {
+		if cond.Type == newCondition.Type {
+			(*conditions)[i] = newCondition
+			return
+		}
+	}
+
+	*conditions = append(*conditions, newCondition)
+}
+
 // SetupWithManager sets up the controller with the Manager.
 func (r *GroupReconciler) SetupWithManager(mgr ctrl.Manager) error {
 	// Add an index field for referenced groups