implementing new interfaces, overall cleanup and refactoring

Author: PatrickLaabs

Makefile

@@ -231,7 +231,7 @@ CAPI_KIND_CLUSTER_NAME ?= capi-test
 # It is set by Prow GIT_TAG, a git-based tag of the form vYYYYMMDD-hash, e.g., v20210120-v0.3.10-308-gc61521971
 
 # Next release is: v1.0.0-preview
-TAG ?= v1.0.0-preview.5
+TAG ?= v1.0.0-preview.16
 ARCH ?= $(shell go env GOARCH)
 ALL_ARCH = amd64 arm arm64
 

README.md

@@ -48,7 +48,7 @@ spec:
   gitRepository:
     url: "https://github.com/PatrickLaabs/cluster-api-addon-provider-cdk8s"
     reference: "main"
-    referencePollInterval: '5'
+    referencePollInterval: "5m"
     path: "examples/cdk8s-sample-deployment"
   clusterSelector: {}
           # matchLabels:

api/v1alpha1/cdk8sappproxy_types.go

@@ -62,14 +62,6 @@ type Cdk8sAppProxyStatus struct {
 	// ObservedGeneration is the last generation observed by the controller.
 	// +optional
 	ObservedGeneration int64 `json:"observedGeneration,omitempty"`
-
-	// LastProcessedGitHash stores the commit hash of the last successfully reconciled Git state.
-	// +optional
-	LastProcessedGitHash string `json:"lastProcessedGitHash,omitempty"`
-
-	// LastRemoteGitHash is the last commit hash fetched from the remote git repository.
-	// +optional
-	LastRemoteGitHash string `json:"lastRemoteGitHash,omitempty"`
 }
 
 // +kubebuilder:object:root=true

config/crd/bases/addons.cluster.x-k8s.io_cdk8sappproxies.yaml

@@ -175,14 +175,6 @@ spec:
                   - type
                   type: object
                 type: array
-              lastProcessedGitHash:
-                description: LastProcessedGitHash stores the commit hash of the last
-                  successfully reconciled Git state.
-                type: string
-              lastRemoteGitHash:
-                description: LastRemoteGitHash is the last commit hash fetched from
-                  the remote git repository.
-                type: string
               observedGeneration:
                 description: ObservedGeneration is the last generation observed by
                   the controller.

config/default/manager_image_patch.yaml

@@ -7,5 +7,5 @@ spec:
   template:
     spec:
       containers:
-      - image: ghcr.io/patricklaabs/cluster-api-addon-provider-cdk8s/cluster-api-cdk8s-controller:v1.0.0-preview.5
+      - image: ghcr.io/patricklaabs/cluster-api-addon-provider-cdk8s/cluster-api-cdk8s-controller:v1.0.0-preview.16
         name: manager

config/default/manager_image_patch.yaml-e

@@ -7,5 +7,5 @@ spec:
   template:
     spec:
       containers:
-      - image: ghcr.io/patricklaabs/cluster-api-addon-provider-cdk8s/cluster-api-cdk8s-controller:v1.0.0-preview.5
+      - image: ghcr.io/patricklaabs/cluster-api-addon-provider-cdk8s/cluster-api-cdk8s-controller:v1.0.0-preview.16
         name: manager

controllers/cdk8sappproxy/cdk8sappproxy_controller.go

@@ -17,26 +17,17 @@ limitations under the License.
 package cdk8sappproxy
 
 import (
-	"bytes"
 	"context"
-	"io/fs"
-	"os"
-	"os/exec"
-	"path/filepath"
 	"strings"
 
 	addonsv1alpha1 "github.com/PatrickLaabs/cluster-api-addon-provider-cdk8s/api/v1alpha1"
-	"github.com/go-logr/logr"
 	"github.com/pkg/errors"
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/labels"
-	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/types"
-	k8syaml "k8s.io/apimachinery/pkg/util/yaml"
 	"k8s.io/client-go/dynamic"
 	"k8s.io/client-go/tools/clientcmd"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
@@ -56,7 +47,7 @@ func (r *Reconciler) checkIfResourceExists(ctx context.Context, dynClient dynami
 			if apierrors.IsNotFound(err) {
 				return false, nil
 			}
-			// Some other error occurred
+
 			return false, errors.Wrapf(err, "failed to get namespaced resource %s/%s with GVR %s", namespace, name, gvr.String())
 		}
 	} else {
@@ -65,154 +56,20 @@ func (r *Reconciler) checkIfResourceExists(ctx context.Context, dynClient dynami
 			if apierrors.IsNotFound(err) {
 				return false, nil
 			}
-			// Some other error occurred
+
 			return false, errors.Wrapf(err, "failed to get cluster-scoped resource %s with GVR %s", name, gvr.String())
 		}
 	}
 
 	return true, nil
 }
 
-func (r *Reconciler) synthesizeAndParseResources(appSourcePath string, logger logr.Logger) ([]*unstructured.Unstructured, error) {
-	reason := addonsv1alpha1.Cdk8sSynthFailedReason
-	// Synthesize cdk8s application
-	if err := r.synthesizeCdk8sApp(appSourcePath, logger, reason); err != nil {
-		return nil, err
-	}
-
-	// Find manifest files
-	manifestFiles, err := r.findManifestFiles(appSourcePath, logger, reason)
-	if err != nil {
-		return nil, err
-	}
-
-	// Parse resources from manifest files using the consolidated function
-	return r.parseManifestFiles(manifestFiles, logger, reason)
-}
-
-func (r *Reconciler) synthesizeCdk8sApp(appSourcePath string, logger logr.Logger, operation string) error {
-	logger.Info("Synthesizing cdk8s application", "effectiveSourcePath", appSourcePath, "operation", operation)
-
-	// npmInstall := cmdRunnerFactory("npm", "install")
-	// npmInstall.SetDir(appSourcePath)
-	// output, err := npmInstall.CombinedOutput()
-	// if err != nil {
-	//	logger.Error(err, "npm installation failed", "output", string(output), "operation:", OperationNpmInstall)
-	// }
-
-	synth := exec.Command("cdk8s", "synth")
-	synth.Dir = appSourcePath
-	if err := synth.Run(); err != nil {
-		logger.Error(err, "Failed to synth cdk8s application", "effectiveSourcePath", appSourcePath)
-
-		return err
-	}
-
-	logger.Info("Synthesized cdk8s application", "effectiveSourcePath", appSourcePath, "operation", operation)
-
-	return nil
-}
-
-func (r *Reconciler) findManifestFiles(appSourcePath string, logger logr.Logger, operation string) ([]string, error) {
-	distPath := filepath.Join(appSourcePath, "dist")
-
-	var manifestFiles []string
-	walkErr := filepath.WalkDir(distPath, func(path string, d fs.DirEntry, err error) error {
-		if err != nil {
-			return err
-		}
-		if !d.IsDir() && (strings.HasSuffix(d.Name(), ".yaml") || strings.HasSuffix(d.Name(), ".yml")) {
-			manifestFiles = append(manifestFiles, path)
-		}
-
-		return nil
-	})
-
-	if walkErr != nil {
-		logger.Error(walkErr, "Failed to walk dist directory", "operation", operation)
-
-		return nil, walkErr
-	}
-
-	logger.Info("Found manifest files", "count", len(manifestFiles), "operation", operation)
-
-	return manifestFiles, nil
-}
-
-func (r *Reconciler) parseManifestFiles(manifestFiles []string, logger logr.Logger, operation string) ([]*unstructured.Unstructured, error) {
-	var parsedResources []*unstructured.Unstructured
-
-	for _, manifestFile := range manifestFiles {
-		logger.Info("Processing manifest file", "file", manifestFile, "operation", operation)
-
-		fileContent, readErr := os.ReadFile(manifestFile)
-		if readErr != nil {
-			logger.Error(readErr, "Failed to read manifest file", "file", manifestFile, "operation", operation)
-
-			return nil, readErr
-		}
-
-		yamlDecoder := k8syaml.NewYAMLOrJSONDecoder(bytes.NewReader(fileContent), 100)
-
-		for {
-			var rawObj runtime.RawExtension
-			if err := yamlDecoder.Decode(&rawObj); err != nil {
-				if err.Error() == "EOF" {
-					break
-				}
-				logger.Error(err, "Failed to decode YAML from manifest file", "file", manifestFile, "operation", operation)
-
-				return nil, err
-			}
-
-			if rawObj.Raw == nil {
-				continue
-			}
-
-			u := &unstructured.Unstructured{}
-			if _, _, err := unstructured.UnstructuredJSONScheme.Decode(rawObj.Raw, nil, u); err != nil {
-				logger.Error(err, "Failed to decode RawExtension to Unstructured", "file", manifestFile, "operation", operation)
-
-				return nil, err
-			}
-
-			parsedResources = append(parsedResources, u)
-			logger.Info("Parsed resource", "GVK", u.GroupVersionKind().String(), "Name", u.GetName(), "Namespace", u.GetNamespace(), "operation", operation)
-		}
-	}
-
-	logger.Info("Total resources parsed", "count", len(parsedResources), "operation", operation)
-
-	return parsedResources, nil
-}
-
-func (r *Reconciler) finalizeDeletion(ctx context.Context, cdk8sAppProxy *addonsv1alpha1.Cdk8sAppProxy, proxyNamespacedName types.NamespacedName, logger logr.Logger) error {
-	logger.Info("Starting finalization process")
-
-	// Cancel any active watches for this Cdk8sAppProxy using the new WatchManager
-	logger.Info("Cleaning up active watches for Cdk8sAppProxy before deletion")
-	r.WatchManager.CleanupWatches(proxyNamespacedName)
-	logger.Info("Completed cleanup of active watches")
-
-	// Remove finalizer
-	logger.Info("Finished deletion logic, removing finalizer")
-	controllerutil.RemoveFinalizer(cdk8sAppProxy, Finalizer)
-	if err := r.Update(ctx, cdk8sAppProxy); err != nil {
-		logger.Error(err, "Failed to remove finalizer")
-
-		return err
-	}
-	logger.Info("Finalizer removed successfully")
-
-	return nil
-}
-
-func (r *Reconciler) getDynamicClientForCluster(ctx context.Context, secretNamespace, clusterName string) (dynamic.Interface, error) {
+func (r *Reconciler) getDynamicClientForCluster(ctx context.Context, secretNamespace, clusterName string) (dynamicClient dynamic.Interface, err error) {
 	logger := log.FromContext(ctx).WithValues("secretNamespace", secretNamespace, "clusterName", clusterName)
 	kubeconfigSecretName := clusterName + "-kubeconfig"
 
 	kubeconfigSecret := &corev1.Secret{}
-	if err := r.Get(ctx, client.ObjectKey{Namespace: secretNamespace, Name: kubeconfigSecretName}, kubeconfigSecret); err != nil {
+	if err = r.Get(ctx, client.ObjectKey{Namespace: secretNamespace, Name: kubeconfigSecretName}, kubeconfigSecret); err != nil {
 		logger.Error(err, "Failed to get Kubeconfig secret")
 
 		return nil, errors.Wrapf(err, "failed to get kubeconfig secret %s/%s", secretNamespace, kubeconfigSecretName)
@@ -229,17 +86,17 @@ func (r *Reconciler) getDynamicClientForCluster(ctx context.Context, secretNames
 	if err != nil {
 		logger.Error(err, "Failed to create REST config from Kubeconfig")
 
-		return nil, errors.Wrapf(err, "failed to create REST config from kubeconfig for cluster %s", clusterName)
+		return dynamicClient, errors.Wrapf(err, "failed to create REST config from kubeconfig for cluster %s", clusterName)
 	}
-	dynamicClient, err := dynamic.NewForConfig(restConfig)
+	dynamicClient, err = dynamic.NewForConfig(restConfig)
 	if err != nil {
 		logger.Error(err, "Failed to create dynamic client")
 
-		return nil, errors.Wrapf(err, "failed to create dynamic client for cluster %s", clusterName)
+		return dynamicClient, errors.Wrapf(err, "failed to create dynamic client for cluster %s", clusterName)
 	}
 	logger.Info("Successfully created dynamic client")
 
-	return dynamicClient, nil
+	return dynamicClient, err
 }
 
 // Consolidated error handling.
@@ -298,13 +155,13 @@ func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error {
 
 // ClusterToCdk8sAppProxyMapper is a handler.ToRequestsFunc to be used to enqeue requests for Cdk8sAppProxyReconciler.
 // It maps CAPI Cluster events to Cdk8sAppProxy events.
-func (r *Reconciler) ClusterToCdk8sAppProxyMapper(ctx context.Context, o client.Object) []ctrl.Request {
+func (r *Reconciler) ClusterToCdk8sAppProxyMapper(ctx context.Context, o client.Object) (requests []ctrl.Request) {
 	logger := log.FromContext(ctx)
 	cluster, ok := o.(*clusterv1.Cluster)
 	if !ok {
 		logger.Error(errors.Errorf("unexpected type %T, expected Cluster", o), "failed to cast object to Cluster", "object", o)
 
-		return nil
+		return requests
 	}
 
 	logger = logger.WithValues("clusterName", cluster.Name, "clusterNamespace", cluster.Namespace)
@@ -325,7 +182,6 @@ func (r *Reconciler) ClusterToCdk8sAppProxyMapper(ctx context.Context, o client.
 	}
 	logger.Info("Checking Cdk8sAppProxies for matches", "count", len(proxies.Items))
 
-	var requests []ctrl.Request
 	for _, proxy := range proxies.Items {
 		proxyLogger := logger.WithValues("cdk8sAppProxyName", proxy.Name, "cdk8sAppProxyNamespace", proxy.Namespace)
 		proxyLogger.Info("Evaluating Cdk8sAppProxy")