refactoring

Author: PatrickLaabs

controllers/cdk8sappproxy/cdk8sappproxy_controller.go

@@ -186,71 +186,6 @@ func (r *Reconciler) parseManifestFiles(manifestFiles []string, logger logr.Logg
 	return parsedResources, nil
 }
 
-func (r *Reconciler) deleteResourcesFromClusters(ctx context.Context, cdk8sAppProxy *addonsv1alpha1.Cdk8sAppProxy, parsedResources []*unstructured.Unstructured, logger logr.Logger) error {
-	// Get target clusters
-	clusterList, err := r.getTargetClustersForDeletion(ctx, cdk8sAppProxy, logger)
-	if err != nil {
-		return err
-	}
-
-	// Delete resources from each cluster
-	for _, cluster := range clusterList.Items {
-		clusterLogger := logger.WithValues("targetCluster", cluster.Name)
-
-		dynamicClient, err := r.getDynamicClientForCluster(ctx, cdk8sAppProxy.Namespace, cluster.Name)
-		if err != nil {
-			clusterLogger.Error(err, "Failed to get dynamic client for cluster during deletion, skipping this cluster")
-
-			continue
-		}
-
-		clusterLogger.Info("Successfully created dynamic client for cluster deletion")
-
-		for _, resource := range parsedResources {
-			gvr := resource.GroupVersionKind().GroupVersion().WithResource(getPluralFromKind(resource.GetKind()))
-			clusterLogger.Info("Deleting resource from cluster", "GVK", resource.GroupVersionKind().String(), "Name", resource.GetName(), "Namespace", resource.GetNamespace())
-
-			err := dynamicClient.Resource(gvr).Namespace(resource.GetNamespace()).Delete(ctx, resource.GetName(), metav1.DeleteOptions{})
-
-			switch {
-			case err != nil && !apierrors.IsNotFound(err):
-				clusterLogger.Error(err, "Failed to delete resource from cluster", "resourceName", resource.GetName())
-			case apierrors.IsNotFound(err):
-				clusterLogger.Info("Resource already deleted from cluster", "resourceName", resource.GetName())
-			case err == nil:
-				clusterLogger.Info("Successfully deleted resource from cluster", "resourceName", resource.GetName())
-			}
-		}
-	}
-
-	return nil
-}
-
-func (r *Reconciler) getTargetClustersForDeletion(ctx context.Context, cdk8sAppProxy *addonsv1alpha1.Cdk8sAppProxy, logger logr.Logger) (*clusterv1.ClusterList, error) {
-	clusterList := &clusterv1.ClusterList{}
-	selector, err := metav1.LabelSelectorAsSelector(&cdk8sAppProxy.Spec.ClusterSelector)
-	if err != nil {
-		logger.Error(err, "failed to parse ClusterSelector during deletion")
-
-		return nil, err
-	}
-
-	logger.Info("Listing clusters for deletion", "selector", selector.String(), "namespace", cdk8sAppProxy.Namespace)
-	if err := r.List(ctx, clusterList, client.MatchingLabelsSelector{Selector: selector}, client.InNamespace(cdk8sAppProxy.Namespace)); err != nil {
-		logger.Error(err, "Failed to list clusters during deletion, requeuing")
-
-		return nil, err
-	}
-
-	clusterNames := make([]string, 0, len(clusterList.Items))
-	for _, c := range clusterList.Items {
-		clusterNames = append(clusterNames, c.Name)
-	}
-	logger.Info("Found clusters for deletion", "count", len(clusterList.Items), "names", clusterNames)
-
-	return clusterList, nil
-}
-
 func (r *Reconciler) finalizeDeletion(ctx context.Context, cdk8sAppProxy *addonsv1alpha1.Cdk8sAppProxy, proxyNamespacedName types.NamespacedName, logger logr.Logger) error {
 	logger.Info("Starting finalization process")
 

controllers/cdk8sappproxy/cdk8sappproxy_git_operator.go

@@ -8,13 +8,12 @@ import (
 	"path/filepath"
 )
 
-func (r *Reconciler) prepareSource(cdk8sAppProxy *addonsv1alpha1.Cdk8sAppProxy, logger logr.Logger) (appSourcePath string, currentCommitHash string, err error) {
+func (r *Reconciler) prepareSource(cdk8sAppProxy *addonsv1alpha1.Cdk8sAppProxy, logger logr.Logger) (appSourcePath string, err error) {
 	gitImpl := &gitoperator.GitImplementer{}
 	var buf bytes.Buffer
 	gitSpec := cdk8sAppProxy.Spec.GitRepository
 
 	if cdk8sAppProxy.Spec.GitRepository != nil && cdk8sAppProxy.Spec.GitRepository.URL != "" {
-
 		directory, err := gitImpl.Clone(gitSpec.URL, &buf)
 		if err != nil {
 			logger.Error(err, addonsv1alpha1.GitCloneFailedCondition, "Failed to clone git repository")
@@ -25,7 +24,7 @@ func (r *Reconciler) prepareSource(cdk8sAppProxy *addonsv1alpha1.Cdk8sAppProxy,
 			logger.Error(err, addonsv1alpha1.GitHashFailureReason, "Failed to get local git hash")
 		}
 
-		currentCommitHash = retrieveCommitHash
+		currentCommitHash := retrieveCommitHash
 		if currentCommitHash != "" && cdk8sAppProxy != nil {
 			cdk8sAppProxy.Status.LastRemoteGitHash = currentCommitHash
 			logger.Info("Updated cdk8sAppProxy.Status.LastRemoteGitHash with the latest commit hash from remote", "lastRemoteGitHash", currentCommitHash)
@@ -36,8 +35,8 @@ func (r *Reconciler) prepareSource(cdk8sAppProxy *addonsv1alpha1.Cdk8sAppProxy,
 			logger.Info("Adjusted appSourcePath for repository subpath", "subPath", gitSpec.Path, "finalPath", appSourcePath)
 		}
 
-		return appSourcePath, currentCommitHash, err
+		return appSourcePath, err
 	}
 
-	return appSourcePath, currentCommitHash, nil
+	return appSourcePath, err
 }

controllers/cdk8sappproxy/cdk8sappproxy_reconciler.go

@@ -89,7 +89,7 @@ func (r *Reconciler) reconcileNormal(ctx context.Context, cdk8sAppProxy *addonsv
 	}
 
 	// Prepare a source path and get current commit hash
-	appSourcePath, _, err := r.prepareSource(cdk8sAppProxy, logger)
+	appSourcePath, err := r.prepareSource(cdk8sAppProxy, logger)
 	if err != nil {
 		return ctrl.Result{}, err
 	}
@@ -108,10 +108,9 @@ func (r *Reconciler) reconcileNormal(ctx context.Context, cdk8sAppProxy *addonsv
 	}
 
 	// Determine if apply is needed
-	_, clusterList, err := r.applyNeeded(ctx, cdk8sAppProxy, parsedResources, logger)
+	clusterList, err := r.applyNeeded(ctx, cdk8sAppProxy, parsedResources, logger)
 	if err != nil {
 		logger.Error(err, "Failed to determine if apply is needed")
-		//return ctrl.Result{}, err
 	}
 
 	_, err = r.applyResourcesToClusters(ctx, cdk8sAppProxy, parsedResources, clusterList, proxyNamespacedName, logger)
@@ -146,7 +145,7 @@ func (r *Reconciler) reconcileNormal(ctx context.Context, cdk8sAppProxy *addonsv
 				}
 				if polling {
 					logger.Info("Detected changes in git repository, proceeding with reconciliation.")
-					appSourcePath, _, err = r.prepareSource(cdk8sAppProxy, logger)
+					appSourcePath, err = r.prepareSource(cdk8sAppProxy, logger)
 					if err != nil {
 						logger.Error(err, "Prepare source for reconciliation")
 					}
@@ -169,17 +168,17 @@ func (r *Reconciler) reconcileNormal(ctx context.Context, cdk8sAppProxy *addonsv
 					if len(parsedResources) == 0 {
 						if err := r.handleNoResources(ctx, cdk8sAppProxy, logger); err != nil {
 							logger.Error(err, "Failed to handle no resources case")
+
 							return
 						}
 
 						logger.Info("No valid Kubernetes resources parsed from manifest files, skipping apply.")
 					}
 
 					// Determine if apply is needed
-					_, clusterList, err := r.applyNeeded(ctx, cdk8sAppProxy, parsedResources, logger)
+					clusterList, err := r.applyNeeded(ctx, cdk8sAppProxy, parsedResources, logger)
 					if err != nil {
 						logger.Error(err, "Failed to determine if apply is needed")
-						//return ctrl.Result{}, err
 					}
 
 					_, err = r.applyResourcesToClusters(ctx, cdk8sAppProxy, parsedResources, clusterList, proxyNamespacedName, logger)
@@ -191,6 +190,7 @@ func (r *Reconciler) reconcileNormal(ctx context.Context, cdk8sAppProxy *addonsv
 				}
 			case <-ctx.Done():
 				logger.Info("Stopping git repository polling loop due to context cancellation.")
+
 				return
 			}
 		}
@@ -228,16 +228,16 @@ func (r *Reconciler) handleNoResources(ctx context.Context, cdk8sAppProxy *addon
 	return nil
 }
 
-func (r *Reconciler) applyNeeded(ctx context.Context, cdk8sAppProxy *addonsv1alpha1.Cdk8sAppProxy, parsedResources []*unstructured.Unstructured, logger logr.Logger) (bool, clusterv1.ClusterList, error) {
+func (r *Reconciler) applyNeeded(ctx context.Context, cdk8sAppProxy *addonsv1alpha1.Cdk8sAppProxy, parsedResources []*unstructured.Unstructured, logger logr.Logger) (clusterv1.ClusterList, error) {
 	var clusterList clusterv1.ClusterList
 
 	_, list, err := r.verifyResourcesOnClusters(ctx, cdk8sAppProxy, parsedResources, logger)
 	if err != nil {
-		return false, clusterList, err
+		return clusterList, err
 	}
 	clusterList = list
 
-	return true, clusterList, nil
+	return clusterList, nil
 }
 
 func (r *Reconciler) verifyResourcesOnClusters(ctx context.Context, cdk8sAppProxy *addonsv1alpha1.Cdk8sAppProxy, parsedResources []*unstructured.Unstructured, logger logr.Logger) (bool, clusterv1.ClusterList, error) {
@@ -305,107 +305,6 @@ func (r *Reconciler) verifyResourcesOnClusters(ctx context.Context, cdk8sAppProx
 	return foundMissingResourcesOnAnyCluster, clusterList, nil
 }
 
-func (r *Reconciler) checkGitOrAnnotationTriggers(cdk8sAppProxy *addonsv1alpha1.Cdk8sAppProxy, currentCommitHash string, forceSynthAndApplyDueToDeletion bool, logger logr.Logger) bool {
-	// Check for periodic git poller trigger
-	if cdk8sAppProxy.Status.LastRemoteGitHash != "" &&
-		cdk8sAppProxy.Status.LastRemoteGitHash != cdk8sAppProxy.Status.LastProcessedGitHash &&
-		cdk8sAppProxy.Status.LastRemoteGitHash != currentCommitHash {
-		logger.Info("Reconciliation proceeding due to change detected by git poller.",
-			"lastRemoteGitHash", cdk8sAppProxy.Status.LastRemoteGitHash,
-			"lastProcessedGitHash", cdk8sAppProxy.Status.LastProcessedGitHash,
-			"currentCommitHash", currentCommitHash)
-
-		return true
-	}
-
-	// Check for git repository changes
-	if cdk8sAppProxy.Spec.GitRepository != nil && cdk8sAppProxy.Spec.GitRepository.URL != "" {
-		if currentCommitHash == "" {
-			logger.Info("currentCommitHash is unexpectedly empty for Git source; proceeding with update as a precaution.")
-
-			return true
-		}
-
-		lastProcessedGitHash := cdk8sAppProxy.Status.LastProcessedGitHash
-		gitSpecRef := cdk8sAppProxy.Spec.GitRepository.Reference
-		repositoryHasChanged := currentCommitHash != lastProcessedGitHash
-		isInitialDeployment := lastProcessedGitHash == ""
-
-		if isInitialDeployment {
-			logger.Info("Initial deployment or no last processed hash found. Proceeding with cdk8s synth and apply.", "currentCommitHash", currentCommitHash, "reference", gitSpecRef)
-
-			return true
-		}
-		if repositoryHasChanged {
-			logger.Info("Git repository has changed (current clone vs last processed), proceeding with update.", "currentCommitHash", currentCommitHash, "lastProcessedGitHash", lastProcessedGitHash, "reference", gitSpecRef)
-
-			return true
-		}
-		logger.Info("No new Git changes detected (current clone matches last processed, and no pending poller detection).", "commitHash", currentCommitHash, "reference", gitSpecRef)
-	} else if cdk8sAppProxy.Status.ObservedGeneration == 0 {
-		logger.Info("Initial processing for source type without explicit change detection. Proceeding with cdk8s synth and apply.")
-
-		return true
-	}
-
-	// Check for deletion trigger
-	if forceSynthAndApplyDueToDeletion {
-		logger.Info("Forcing synth and apply because reconciliation was triggered by a resource deletion")
-
-		return true
-	}
-
-	return false
-}
-
-func (r *Reconciler) reestablishWatchesForExistingResources(ctx context.Context, cdk8sAppProxy *addonsv1alpha1.Cdk8sAppProxy, logger logr.Logger) error {
-	// Get the source and parse resources to know what should be watched
-	appSourcePath, _, err := r.prepareSource(cdk8sAppProxy, logger)
-	if err != nil {
-		return err
-	}
-
-	parsedResources, err := r.synthesizeAndParseResources(appSourcePath, logger)
-	if err != nil {
-		return err
-	}
-
-	// Get target clusters
-	selector, err := metav1.LabelSelectorAsSelector(&cdk8sAppProxy.Spec.ClusterSelector)
-	if err != nil {
-		return err
-	}
-
-	var clusterList clusterv1.ClusterList
-	if err := r.List(ctx, &clusterList, client.MatchingLabelsSelector{Selector: selector}); err != nil {
-		return err
-	}
-
-	proxyNamespacedName := types.NamespacedName{Name: cdk8sAppProxy.Name, Namespace: cdk8sAppProxy.Namespace}
-
-	// Re-establish watches for each resource on each cluster
-	for _, cluster := range clusterList.Items {
-		dynamicClient, err := r.getDynamicClientForCluster(ctx, cluster.Namespace, cluster.Name)
-		if err != nil {
-			logger.Error(err, "Failed to get dynamic client for watch re-establishment", "cluster", cluster.Name)
-
-			continue
-		}
-
-		for _, resource := range parsedResources {
-			gvk := resource.GroupVersionKind()
-
-			if err := r.WatchManager.StartWatch(ctx, dynamicClient, gvk, resource.GetNamespace(), resource.GetName(), proxyNamespacedName); err != nil {
-				logger.Error(err, "Failed to re-establish watch", "cluster", cluster.Name, "resource", resource.GetName())
-			} else {
-				logger.Info("Re-established watch for existing resource", "cluster", cluster.Name, "resource", resource.GetName())
-			}
-		}
-	}
-
-	return nil
-}
-
 //nolint:unparam // ctrl.Result is required for controller-runtime reconciler pattern
 func (r *Reconciler) applyResourcesToClusters(ctx context.Context, cdk8sAppProxy *addonsv1alpha1.Cdk8sAppProxy, parsedResources []*unstructured.Unstructured, clusterList clusterv1.ClusterList, proxyNamespacedName types.NamespacedName, logger logr.Logger) (ctrl.Result, error) {
 	logger.Info("Proceeding with application of resources to target clusters.")
@@ -440,7 +339,6 @@ func (r *Reconciler) applyResourcesToClusters(ctx context.Context, cdk8sAppProxy
 		cdk8sAppProxy.Status.ObservedGeneration = cdk8sAppProxy.Generation
 		conditions.MarkTrue(cdk8sAppProxy, addonsv1alpha1.DeploymentProgressingCondition)
 		if cdk8sAppProxy.Spec.GitRepository != nil && cdk8sAppProxy.Spec.GitRepository.URL != "" {
-			//cdk8sAppProxy.Status.LastProcessedGitHash = currentCommitHash
 		}
 		if err := r.Status().Update(ctx, cdk8sAppProxy); err != nil {
 			logger.Error(err, "Failed to update status when no resources to apply")
@@ -504,8 +402,6 @@ func (r *Reconciler) applyResourcesToClusters(ctx context.Context, cdk8sAppProxy
 
 	// If we reach here, the overallSuccess is true.
 	if cdk8sAppProxy.Spec.GitRepository != nil && cdk8sAppProxy.Spec.GitRepository.URL != "" {
-		//cdk8sAppProxy.Status.LastProcessedGitHash = currentCommitHash
-		//logger.Info("Successfully updated LastProcessedGitHash in status after application", "hash", currentCommitHash)
 	}
 
 	cdk8sAppProxy.Status.ObservedGeneration = cdk8sAppProxy.Generation

controllers/cdk8sappproxy/git/git.go

@@ -31,7 +31,7 @@ func (g *GitImplementer) Clone(repoUrl string, writer *bytes.Buffer) (directory
 
 	directory, err = os.MkdirTemp("", tempDirPattern)
 	if err != nil {
-		return directory, fmt.Errorf("failed to create temporary directory: %v", err)
+		return directory, fmt.Errorf("failed to create temporary directory: %w", err)
 	}
 
 	// Check if repo and directory are empty.
@@ -102,7 +102,7 @@ func (g *GitImplementer) Hash(repo string, branch string) (hash string, err erro
 		refs, err := remoterepo.List(&git.ListOptions{})
 		if err != nil {
 			//return hash, fmt.Errorf("%s", addonsv1alpha1.GitHashFailureReason)
-			return hash, fmt.Errorf("failed to list remote refs: %v", err)
+			return hash, fmt.Errorf("failed to list remote refs: %w", err)
 		}
 
 		refName := plumbing.NewBranchReferenceName(branch)
@@ -118,7 +118,7 @@ func (g *GitImplementer) Hash(repo string, branch string) (hash string, err erro
 		localRepo, err := git.PlainOpen(repo)
 		if err != nil {
 			//return hash, fmt.Errorf("%s", addonsv1alpha1.GitHashFailureReason)
-			return hash, fmt.Errorf("failed to open local git repository: %v", err)
+			return hash, fmt.Errorf("failed to open local git repository: %w", err)
 		}
 
 		headRef, err := localRepo.Head()
@@ -135,6 +135,7 @@ func (g *GitImplementer) Hash(repo string, branch string) (hash string, err erro
 
 		return hash, err
 	}
+
 	return hash, err
 }
 

controllers/cdk8sappproxy/git/git_test.go

@@ -897,7 +897,7 @@ func TestEmptyChecker(t *testing.T) {
 	}
 }
 
-// Helper functions
+// Helper functions.
 func setupTestRepo(t *testing.T) string {
 	t.Helper()
 	tempDir := t.TempDir()