From 9235368497e4261391b078441337e772677e5ff6 Mon Sep 17 00:00:00 2001 From: Uburro Date: Thu, 3 Jul 2025 13:36:01 +0200 Subject: [PATCH] add watch external cm and secret from values and addd list of hr in same cm --- cmd/main.go | 21 + go.mod | 2 + internal/controller/basecontroller.go | 323 +++++++- internal/controller/basecontroller_test.go | 753 +++++++++--------- .../controller/hrautodiscovercontroller.go | 127 +-- 5 files changed, 720 insertions(+), 506 deletions(-) diff --git a/cmd/main.go b/cmd/main.go index 1525a8d..5f57e4f 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -17,6 +17,7 @@ limitations under the License. package main import ( + "context" "crypto/tls" "flag" "os" @@ -32,6 +33,7 @@ import ( clientgoscheme "k8s.io/client-go/kubernetes/scheme" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/certwatcher" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/healthz" "sigs.k8s.io/controller-runtime/pkg/log/zap" "sigs.k8s.io/controller-runtime/pkg/metrics/filters" @@ -310,6 +312,25 @@ func main() { } if config.enableHRAutodiscovery { + ctx := context.Background() + + // Index HelmRelease by ConfigMaps or Secret used in ValuesFrom + if err := mgr.GetFieldIndexer().IndexField( + ctx, &helmv2.HelmRelease{}, "spec.valuesFrom", func(rawObj client.Object) []string { + hr := rawObj.(*helmv2.HelmRelease) + var helmReleaseNames []string + + for _, valuesRef := range hr.Spec.ValuesFrom { + if valuesRef.Kind == "ConfigMap" || valuesRef.Kind == "Secret" { + helmReleaseNames = append(helmReleaseNames, valuesRef.Name) + } + } + return helmReleaseNames + }); err != nil { + setupLog.Error(err, "unable to setup ConfigMap index field for HelmRelease") + os.Exit(1) + } + if err = controller.NewHRAutodicoverReconciler( mgr.GetClient(), mgr.GetScheme(), diff --git a/go.mod b/go.mod index deafd1b..edead2e 100644 --- a/go.mod +++ b/go.mod @@ -8,6 +8,7 @@ require ( github.com/fluxcd/helm-controller/api v1.3.0 github.com/onsi/ginkgo/v2 v2.22.0 github.com/onsi/gomega v1.36.1 + github.com/stretchr/testify v1.10.0 go.uber.org/zap v1.27.0 k8s.io/api v0.33.0 k8s.io/apimachinery v0.33.0 @@ -54,6 +55,7 @@ require ( github.com/modern-go/reflect2 v1.0.2 // indirect github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect github.com/pkg/errors v0.9.1 // indirect + github.com/pmezard/go-difflib v1.0.0 // indirect github.com/prometheus/client_golang v1.22.0 // indirect github.com/prometheus/client_model v0.6.1 // indirect github.com/prometheus/common v0.62.0 // indirect diff --git a/internal/controller/basecontroller.go b/internal/controller/basecontroller.go index 445c328..6e1372d 100644 --- a/internal/controller/basecontroller.go +++ b/internal/controller/basecontroller.go @@ -4,12 +4,16 @@ import ( "context" "fmt" "hash/adler32" + "strings" "sync" "time" helmv2 "github.com/fluxcd/helm-controller/api/v2" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/util/workqueue" "k8s.io/kubernetes/pkg/util/hash" ctrl "sigs.k8s.io/controller-runtime" @@ -61,11 +65,6 @@ type BaseReconciler struct { Scheme *runtime.Scheme } -type ResourceMetadata interface { - GetAnnotations() map[string]string - GetLabels() map[string]string -} - func (r *BaseReconciler) ReconcileResource( ctx context.Context, req ctrl.Request, @@ -80,63 +79,107 @@ func (r *BaseReconciler) ReconcileResource( return ctrl.Result{}, err } - hrName, hrNamespace, oldDigest := r.ExtractFromAnnotations(resource) + listHRName, hrNamespace, oldDigest := r.ExtractFromAnnotations(resource) + + for _, hrName := range listHRName { + hr, err := r.GetHelmRelease(ctx, hrName, hrNamespace) + if err != nil { + logger.V(1).Error(err, "Getting HelmRelease produced an error", + "helmRelease", hrName, "namespace", hrNamespace) + if apierrors.IsNotFound(err) { + logger.V(1).Info("HelmRelease not found, removing from annotations", + "helmRelease", hrName, "namespace", hrNamespace) + + if err := r.RemoveHRNameFromAnnotations(ctx, resource, hrName); err != nil { + logger.Error(err, "Failed to remove HelmRelease name from annotations", + "helmRelease", hrName, "namespace", hrNamespace) + return ctrl.Result{}, err + } + // Re-fetch the resource to ensure we have the latest state after patching + if err := r.Get(ctx, req.NamespacedName, resource); err != nil { + logger.V(1).Error(err, fmt.Sprintf("Failed to get %s after removing HelmRelease name from annotations", resourceType), + "resource", req.Name) + return ctrl.Result{}, err + } + continue + } + return ctrl.Result{}, err + } - if hrName == "" { - logger.V(1).Info(fmt.Sprintf("No HelmRelease name found in %s annotations", resourceType), "resource", req.Name) - return ctrl.Result{}, nil - } + if !r.IsDeployed(hr) { + logger.V(1).Info("HelmRelease is not deployed, skipping patch", "helmRelease", + hrName, "namespace", hrNamespace) + continue + } - hr, err := r.GetHelmRelease(ctx, hrName, hrNamespace) - if err != nil { - logger.V(1).Error(err, "Getting HelmRelease produced an error", - "helmRelease", hrName, "namespace", hrNamespace) - return ctrl.Result{}, err - } + newDigest := r.GetNewDigest(resource) - if !r.IsDeployed(hr) { - logger.V(1).Info("HelmRelease is not deployed, skipping patch", "helmRelease", - hrName, "namespace", hrNamespace) - return ctrl.Result{}, nil - } + logger.V(1).Info("old digest", "digest", oldDigest) + logger.V(1).Info("new digest", "digest", newDigest) - newDigest := r.GetNewDigest(resource) + if oldDigest == newDigest { + logger.V(1).Info(fmt.Sprintf("No changes detected in %s, skipping HelmRelease patch", resourceType), "resource", req.Name) + return ctrl.Result{}, nil + } + + if err := r.PatchHR(ctx, hr); err != nil { + logger.Error(err, "failed to patch HelmRelease", "name", hr.Name) + return ctrl.Result{}, err + } - logger.V(1).Info("old digest", "digest", oldDigest) - logger.V(1).Info("new digest", "digest", newDigest) + if err := r.UpdateDigest(ctx, resource, newDigest); err != nil { + logger.Error(err, fmt.Sprintf("failed to patch %s", resourceType), "name", resource.GetName()) + return ctrl.Result{}, err + } - if oldDigest == newDigest { - logger.V(1).Info(fmt.Sprintf("No changes detected in %s, skipping HelmRelease patch", resourceType), "resource", req.Name) - return ctrl.Result{}, nil + logger.Info("patched HelmRelease with new digest", + "name", hr.Name, + "digest", newDigest, + "version", hr.Status.History[0].Version) } - if err := r.PatchHR(ctx, hr); err != nil { - logger.Error(err, "failed to patch HelmRelease", "name", hr.Name) - return ctrl.Result{}, err + return ctrl.Result{}, nil +} + +func (r *BaseReconciler) RemoveHRNameFromAnnotations( + ctx context.Context, resource client.Object, hrName string, +) error { + patchTarget := resource.DeepCopyObject().(client.Object) + annotations := patchTarget.GetAnnotations() + if annotations == nil { + return nil } - if err := r.UpdateDigest(ctx, resource, newDigest); err != nil { - logger.Error(err, fmt.Sprintf("failed to patch %s", resourceType), "name", resource.GetName()) - return ctrl.Result{}, err + listHRName := strings.Split(annotations[HRNameAnnotation], ",") + newList := []string{} + for _, name := range listHRName { + if name != hrName { + newList = append(newList, name) + } } - logger.Info("patched HelmRelease with new digest", - "name", hr.Name, - "digest", newDigest, - "version", hr.Status.History[0].Version) + if len(newList) == 0 { + delete(annotations, HRNameAnnotation) + } else { + annotations[HRNameAnnotation] = strings.Join(newList, ",") + } - return ctrl.Result{}, nil + patchTarget.SetAnnotations(annotations) + + patch := client.MergeFrom(resource.DeepCopyObject().(client.Object)) + return r.Patch(ctx, patchTarget, patch) } -func (r *BaseReconciler) ExtractFromAnnotations(resource ResourceMetadata) (string, string, string) { +func (r *BaseReconciler) ExtractFromAnnotations(resource client.Object) ([]string, string, string) { annotations := resource.GetAnnotations() - hrName := annotations[HRNameAnnotation] hrNamespace := annotations[HRNSAnnotation] if hrNamespace == "" { hrNamespace = DefaultFluxcdNamespace } + hrName := annotations[HRNameAnnotation] + listHRName := strings.Split(hrName, ",") oldDigest := annotations[HashAnnotation] - return hrName, hrNamespace, oldDigest + return listHRName, hrNamespace, oldDigest } func (r *BaseReconciler) GetHelmRelease(ctx context.Context, name, namespace string) (*helmv2.HelmRelease, error) { @@ -171,7 +214,7 @@ func (r *BaseReconciler) GetCurrentDigest(hr *helmv2.HelmRelease) string { } func (r *BaseReconciler) PatchHR(ctx context.Context, hr *helmv2.HelmRelease) error { - patchTarget := hr.DeepCopy() + patchTarget := hr.DeepCopyObject().(*helmv2.HelmRelease) ts := time.Now().Format(time.RFC3339Nano) if patchTarget.Annotations == nil { @@ -180,7 +223,7 @@ func (r *BaseReconciler) PatchHR(ctx context.Context, hr *helmv2.HelmRelease) er patchTarget.Annotations["reconcile.fluxcd.io/forceAt"] = ts patchTarget.Annotations["reconcile.fluxcd.io/requestedAt"] = ts - patch := client.MergeFrom(hr.DeepCopy()) + patch := client.MergeFrom(hr.DeepCopyObject().(*helmv2.HelmRelease)) return r.Patch(ctx, patchTarget, patch) } @@ -197,3 +240,197 @@ func (r *BaseReconciler) UpdateDigest(ctx context.Context, obj client.Object, ne patch := client.MergeFrom(obj.DeepCopyObject().(client.Object)) return r.Patch(ctx, patchTarget, patch) } + +func (r *BaseReconciler) ReconcileHRSecretOrConfigMap(ctx context.Context, hr *helmv2.HelmRelease, valueFrom helmv2.ValuesReference) error { + log := log.FromContext(ctx) + log.Info("Reconciling resource for autodiscovery", + "kind", valueFrom.Kind, + "name", valueFrom.Name, + "namespace", hr.Namespace) + + var resource client.Object + switch valueFrom.Kind { + case "ConfigMap": + resource = &corev1.ConfigMap{} + case "Secret": + resource = &corev1.Secret{} + default: + log.Error(fmt.Errorf("unsupported kind: %s", valueFrom.Kind), + "Failed to reconcile resource for autodiscovery") + return nil + } + + objRef := &corev1.ObjectReference{ + Name: valueFrom.Name, + Namespace: hr.Namespace, + } + + if err := r.Get(ctx, types.NamespacedName{ + Name: objRef.Name, + Namespace: objRef.Namespace, + }, resource); err != nil { + return err + } + + annotations := r.GetOrGenerateAnnotations(resource, HRNameAnnotation, hr.Namespace, hr.Name) + + labels := map[string]string{ + LabelReconcilerNameSourceKey: "true", + } + + return r.AddAnnotationsAndLabel(ctx, resource, annotations, labels) +} + +func (r *BaseReconciler) GetOrGenerateAnnotations(resource client.Object, hrNameAnnotation, ns, hrName string) map[string]string { + annotations := map[string]string{ + HashAnnotation: r.GetNewDigest(resource), + } + + existingAnnotations := resource.GetAnnotations() + if existingAnnotations == nil { + existingAnnotations = make(map[string]string) + } + + existingHRNames := existingAnnotations[hrNameAnnotation] + if existingHRNames == "" { + annotations[hrNameAnnotation] = hrName + } else { + hrNames := strings.Split(existingHRNames, ",") + found := false + for _, name := range hrNames { + if strings.TrimSpace(name) == hrName { + found = true + break + } + } + if !found { + annotations[hrNameAnnotation] = existingHRNames + "," + hrName + } else { + annotations[hrNameAnnotation] = existingHRNames + } + } + + annotations[HRNSAnnotation] = ns + + return annotations +} + +func (r *BaseReconciler) AddAnnotationsAndLabel( + ctx context.Context, resource client.Object, annotations, labels map[string]string) error { + log := log.FromContext(ctx).V(1) + + patchTarget := resource.DeepCopyObject().(client.Object) + + if patchTarget.GetAnnotations() == nil { + patchTarget.SetAnnotations(make(map[string]string)) + } + if patchTarget.GetLabels() == nil { + patchTarget.SetLabels(make(map[string]string)) + } + + updatedAnnotations := false + for key, value := range annotations { + if patchTarget.GetAnnotations()[key] != value { + patchTarget.GetAnnotations()[key] = value + updatedAnnotations = true + } + } + + updatedLabels := false + for key, value := range labels { + if patchTarget.GetLabels()[key] != value { + patchTarget.GetLabels()[key] = value + updatedLabels = true + } + } + + if updatedAnnotations || updatedLabels { + patch := client.MergeFrom(resource.DeepCopyObject().(client.Object)) + if err := r.Patch(ctx, patchTarget, patch); err != nil { + return err + } + log.Info("Updated resource with annotations and labels for autodiscovery", + "kind", resource.GetObjectKind().GroupVersionKind().Kind, + "name", resource.GetName(), + "namespace", resource.GetNamespace()) + } + + return nil +} + +func (r *BaseReconciler) findHelmReleasesForConfigMap(ctx context.Context, obj client.Object) []reconcile.Request { + configMap, ok := obj.(*corev1.ConfigMap) + if !ok { + return nil + } + + var helmReleases helmv2.HelmReleaseList + listOps := &client.ListOptions{ + Namespace: obj.GetNamespace(), + } + if err := r.List(ctx, &helmReleases, listOps); err != nil { + return nil + } + + var requests []reconcile.Request + for _, hr := range helmReleases.Items { + if r.helmReleaseUsesConfigMap(&hr, configMap) { + requests = append(requests, reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: hr.Name, + Namespace: hr.Namespace, + }, + }) + } + } + return requests +} + +func (r *BaseReconciler) helmReleaseUsesConfigMap(hr *helmv2.HelmRelease, cm *corev1.ConfigMap) bool { + for _, valuesRef := range hr.Spec.ValuesFrom { + if valuesRef.Kind == "ConfigMap" && + valuesRef.Name == cm.Name { + return true + } + } + return false +} + +func (r *BaseReconciler) findHelmReleasesForSecret(ctx context.Context, obj client.Object) []reconcile.Request { + secret, ok := obj.(*corev1.Secret) + if !ok { + return nil + } + + var helmReleases helmv2.HelmReleaseList + listOps := &client.ListOptions{ + Namespace: obj.GetNamespace(), + } + if err := r.List(ctx, &helmReleases, listOps); err != nil { + return nil + } + + var requests []reconcile.Request + for _, hr := range helmReleases.Items { + if r.helmReleaseUsesSecret(&hr, secret) { + requests = append(requests, reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: hr.Name, + Namespace: hr.Namespace, + }, + }) + } + } + + return requests +} + +func (r *BaseReconciler) helmReleaseUsesSecret(hr *helmv2.HelmRelease, secret *corev1.Secret) bool { + for _, valuesRef := range hr.Spec.ValuesFrom { + if valuesRef.Kind == "Secret" && + valuesRef.Name == secret.Name { + return true + } + } + return false +} diff --git a/internal/controller/basecontroller_test.go b/internal/controller/basecontroller_test.go index 3d15cfb..234a43d 100644 --- a/internal/controller/basecontroller_test.go +++ b/internal/controller/basecontroller_test.go @@ -2,396 +2,399 @@ package controller_test import ( "context" - "time" + "testing" - helmv2 "github.com/fluxcd/helm-controller/api/v2" - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + "github.com/stretchr/testify/assert" + . "github.com/uburro/helmrelease-trigger-operator/internal/controller" 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/runtime" "k8s.io/apimachinery/pkg/types" - ctrl "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/client/fake" - - "github.com/uburro/helmrelease-trigger-operator/internal/controller" ) -var _ = Describe("Controller", func() { - var ( - reconciler *controller.BaseReconciler - fakeClient client.Client - ctx context.Context - scheme *runtime.Scheme - testNamespace = "test-namespace" - fluxNamespace = "flux-system" - testConfigMap *corev1.ConfigMap - testSecret *corev1.Secret - testHelmRelease *helmv2.HelmRelease - ) - - BeforeEach(func() { - ctx = context.Background() - scheme = runtime.NewScheme() - Expect(corev1.AddToScheme(scheme)).To(Succeed()) - Expect(helmv2.AddToScheme(scheme)).To(Succeed()) - - fakeClient = fake.NewClientBuilder().WithScheme(scheme).Build() - reconciler = &controller.BaseReconciler{ - Client: fakeClient, - Scheme: scheme, - } - - testHelmRelease = &helmv2.HelmRelease{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-helm-release", - Namespace: fluxNamespace, +func TestExtractFromAnnotations(t *testing.T) { + + tests := []struct { + name string + annotations map[string]string + expectedHRName []string + expectedNS string + expectedDigest string + }{ + { + name: "All annotations present", + annotations: map[string]string{ + HRNSAnnotation: "custom-namespace", + HRNameAnnotation: "hr1,hr2,hr3", + HashAnnotation: "digest123", }, - Status: helmv2.HelmReleaseStatus{ - History: helmv2.Snapshots{ - { - Version: 1, - Status: "deployed", - Digest: "old-digest", - }, - }, + expectedHRName: []string{"hr1", "hr2", "hr3"}, + expectedNS: "custom-namespace", + expectedDigest: "digest123", + }, + { + name: "Missing HRNSAnnotation", + annotations: map[string]string{ + HRNameAnnotation: "hr1,hr2", + HashAnnotation: "digest456", }, - } - - testConfigMap = &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-configmap", - Namespace: testNamespace, - Annotations: map[string]string{ - controller.HRNameAnnotation: "test-helm-release", - controller.HRNSAnnotation: fluxNamespace, - controller.HashAnnotation: "old-hash", - }, + expectedHRName: []string{"hr1", "hr2"}, + expectedNS: DefaultFluxcdNamespace, + expectedDigest: "digest456", + }, + { + name: "Missing HRNameAnnotation", + annotations: map[string]string{ + HRNSAnnotation: "custom-namespace", + HashAnnotation: "digest789", }, - Data: map[string]string{ - "key1": "value1", - "key2": "value2", + expectedHRName: []string{""}, + expectedNS: "custom-namespace", + expectedDigest: "digest789", + }, + { + name: "No annotations", + annotations: map[string]string{}, + expectedHRName: []string{""}, + expectedNS: DefaultFluxcdNamespace, + expectedDigest: "", + }, + { + name: "HRNameAnnotation with single value", + annotations: map[string]string{ + HRNSAnnotation: "custom-namespace", + HRNameAnnotation: "hr1", + HashAnnotation: "digest000", }, - } - - testSecret = &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-secret", - Namespace: testNamespace, - Annotations: map[string]string{ - controller.HRNameAnnotation: "test-helm-release", - controller.HRNSAnnotation: fluxNamespace, - controller.HashAnnotation: "old-hash", - }, + expectedHRName: []string{"hr1"}, + expectedNS: "custom-namespace", + expectedDigest: "digest000", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Arrange + resource := &unstructured.Unstructured{} + resource.SetAnnotations(tt.annotations) + + r := &BaseReconciler{} + + // Act + listHRName, hrNamespace, oldDigest := r.ExtractFromAnnotations(resource) + + // Assert + assert.Equal(t, tt.expectedHRName, listHRName) + assert.Equal(t, tt.expectedNS, hrNamespace) + assert.Equal(t, tt.expectedDigest, oldDigest) + }) + } +} + +func TestGetOrGenerateAnnotations(t *testing.T) { + r := &BaseReconciler{} + + tests := []struct { + name string + existingAnnotations map[string]string + hrNameAnnotation string + ns string + hrName string + expectedAnnotations map[string]string + }{ + { + name: "No existing annotations", + existingAnnotations: nil, + hrNameAnnotation: HRNameAnnotation, + ns: "namespace1", + hrName: "hr1", + expectedAnnotations: map[string]string{ + HRNameAnnotation: "hr1", + HRNSAnnotation: "namespace1", + HashAnnotation: "dccb0d0c", }, - Data: map[string][]byte{ - "username": []byte("admin"), - "password": []byte("secret"), + }, + { + name: "Existing annotations without HRNameAnnotation", + existingAnnotations: map[string]string{ + HRNSAnnotation: "namespace1", }, - } - }) - - Describe("ControllerOptions", func() { - It("should return controller options with correct configuration", func() { - maxConcurrency := 5 - cacheSyncTimeout := 30 * time.Second - - options := controller.ControllerOptions(maxConcurrency, cacheSyncTimeout) - - Expect(options.MaxConcurrentReconciles).To(Equal(maxConcurrency)) - Expect(options.CacheSyncTimeout).To(Equal(cacheSyncTimeout)) - Expect(options.RateLimiter).NotTo(BeNil()) - }) - - It("should return the same options on subsequent calls (singleton pattern)", func() { - options1 := controller.ControllerOptions(5, 30*time.Second) - options2 := controller.ControllerOptions(10, 60*time.Second) - - Expect(options1.MaxConcurrentReconciles).To(Equal(options2.MaxConcurrentReconciles)) - Expect(options1.CacheSyncTimeout).To(Equal(options2.CacheSyncTimeout)) - }) - }) - - Describe("ExtractFromAnnotations", func() { - It("should extract HelmRelease name, namespace and digest from annotations", func() { - hrName, hrNamespace, digest := reconciler.ExtractFromAnnotations(testConfigMap) - - Expect(hrName).To(Equal("test-helm-release")) - Expect(hrNamespace).To(Equal(fluxNamespace)) - Expect(digest).To(Equal("old-hash")) - }) - - It("should use default namespace when namespace annotation is missing", func() { - testConfigMap.Annotations[controller.HRNSAnnotation] = "" - - hrName, hrNamespace, digest := reconciler.ExtractFromAnnotations(testConfigMap) - - Expect(hrName).To(Equal("test-helm-release")) - Expect(hrNamespace).To(Equal(controller.DefaultFluxcdNamespace)) - Expect(digest).To(Equal("old-hash")) - }) - - It("should handle missing annotations gracefully", func() { - testConfigMap.Annotations = nil - - hrName, hrNamespace, digest := reconciler.ExtractFromAnnotations(testConfigMap) - - Expect(hrName).To(BeEmpty()) - Expect(hrNamespace).To(Equal(controller.DefaultFluxcdNamespace)) - Expect(digest).To(BeEmpty()) - }) - }) - - Describe("GetHelmRelease", func() { - BeforeEach(func() { - Expect(fakeClient.Create(ctx, testHelmRelease)).To(Succeed()) - }) - - It("should retrieve HelmRelease successfully", func() { - hr, err := reconciler.GetHelmRelease(ctx, "test-helm-release", fluxNamespace) - - Expect(err).NotTo(HaveOccurred()) - Expect(hr.Name).To(Equal("test-helm-release")) - Expect(hr.Namespace).To(Equal(fluxNamespace)) - }) - - It("should return error when HelmRelease does not exist", func() { - _, err := reconciler.GetHelmRelease(ctx, "non-existent", fluxNamespace) - - Expect(err).To(HaveOccurred()) - }) - }) - - Describe("IsDeployed", func() { - It("should return true when HelmRelease is deployed", func() { - isDeployed := reconciler.IsDeployed(testHelmRelease) - Expect(isDeployed).To(BeTrue()) - }) - - It("should return false when HelmRelease has no history", func() { - testHelmRelease.Status.History = helmv2.Snapshots([]*helmv2.Snapshot{}) - - isDeployed := reconciler.IsDeployed(testHelmRelease) - Expect(isDeployed).To(BeFalse()) - }) - - It("should return false when HelmRelease status is not deployed", func() { - testHelmRelease.Status.History[0].Status = "failed" - - isDeployed := reconciler.IsDeployed(testHelmRelease) - Expect(isDeployed).To(BeFalse()) - }) - }) - - Describe("PatchHR", func() { - BeforeEach(func() { - Expect(fakeClient.Create(ctx, testHelmRelease)).To(Succeed()) - }) - - It("should patch HelmRelease with force reconciliation annotations", func() { - err := reconciler.PatchHR(ctx, testHelmRelease) - Expect(err).NotTo(HaveOccurred()) - - updatedHR := &helmv2.HelmRelease{} - err = fakeClient.Get(ctx, types.NamespacedName{ - Name: testHelmRelease.Name, - Namespace: testHelmRelease.Namespace, - }, updatedHR) - Expect(err).NotTo(HaveOccurred()) - - Expect(updatedHR.Annotations).To(HaveKey("reconcile.fluxcd.io/forceAt")) - Expect(updatedHR.Annotations).To(HaveKey("reconcile.fluxcd.io/requestedAt")) - }) - }) - - Describe("UpdateDigest", func() { - BeforeEach(func() { - Expect(fakeClient.Create(ctx, testConfigMap)).To(Succeed()) - }) - - It("should update digest annotation on resource", func() { - newDigest := "new-digest-value" - - err := reconciler.UpdateDigest(ctx, testConfigMap, newDigest) - Expect(err).NotTo(HaveOccurred()) - - updatedCM := &corev1.ConfigMap{} - err = fakeClient.Get(ctx, types.NamespacedName{ - Name: testConfigMap.Name, - Namespace: testConfigMap.Namespace, - }, updatedCM) - Expect(err).NotTo(HaveOccurred()) - - Expect(updatedCM.Annotations[controller.HashAnnotation]).To(Equal(newDigest)) - }) - - It("should create annotations map if it doesn't exist", func() { - testConfigMap.Annotations = nil - Expect(fakeClient.Update(ctx, testConfigMap)).To(Succeed()) - - newDigest := "new-digest-value" - err := reconciler.UpdateDigest(ctx, testConfigMap, newDigest) - Expect(err).NotTo(HaveOccurred()) - - updatedCM := &corev1.ConfigMap{} - err = fakeClient.Get(ctx, types.NamespacedName{ - Name: testConfigMap.Name, - Namespace: testConfigMap.Namespace, - }, updatedCM) - Expect(err).NotTo(HaveOccurred()) - - Expect(updatedCM.Annotations).NotTo(BeNil()) - Expect(updatedCM.Annotations[controller.HashAnnotation]).To(Equal(newDigest)) - }) - }) - - Describe("ReconcileResource", func() { - Context("when reconciling ConfigMap", func() { - BeforeEach(func() { - Expect(fakeClient.Create(ctx, testConfigMap)).To(Succeed()) - Expect(fakeClient.Create(ctx, testHelmRelease)).To(Succeed()) - }) - - It("should successfully reconcile when data changes", func() { - testConfigMap.Data["key3"] = "value3" - Expect(fakeClient.Update(ctx, testConfigMap)).To(Succeed()) - - req := ctrl.Request{ - NamespacedName: types.NamespacedName{ - Name: testConfigMap.Name, - Namespace: testConfigMap.Namespace, - }, - } - - result, err := reconciler.ReconcileResource(ctx, req, &corev1.ConfigMap{}) - - Expect(err).NotTo(HaveOccurred()) - Expect(result).To(Equal(ctrl.Result{})) - }) - - It("should skip reconciliation when HelmRelease annotation is missing", func() { - delete(testConfigMap.Annotations, controller.HRNameAnnotation) - Expect(fakeClient.Update(ctx, testConfigMap)).To(Succeed()) - - req := ctrl.Request{ - NamespacedName: types.NamespacedName{ - Name: testConfigMap.Name, - Namespace: testConfigMap.Namespace, - }, - } - - result, err := reconciler.ReconcileResource(ctx, req, &corev1.ConfigMap{}) - - Expect(err).NotTo(HaveOccurred()) - Expect(result).To(Equal(ctrl.Result{})) - }) - - It("should skip reconciliation when HelmRelease is not deployed", func() { - testHelmRelease.Status.History[0].Status = "failed" - Expect(fakeClient.Update(ctx, testHelmRelease)).To(Succeed()) - - req := ctrl.Request{ - NamespacedName: types.NamespacedName{ - Name: testConfigMap.Name, - Namespace: testConfigMap.Namespace, - }, - } - - result, err := reconciler.ReconcileResource(ctx, req, &corev1.ConfigMap{}) - - Expect(err).NotTo(HaveOccurred()) - Expect(result).To(Equal(ctrl.Result{})) - }) - - It("should skip reconciliation when digest hasn't changed", func() { - req := ctrl.Request{ - NamespacedName: types.NamespacedName{ - Name: testConfigMap.Name, - Namespace: testConfigMap.Namespace, - }, - } - - _, err := reconciler.ReconcileResource(ctx, req, &corev1.ConfigMap{}) - Expect(err).NotTo(HaveOccurred()) - - result, err := reconciler.ReconcileResource(ctx, req, &corev1.ConfigMap{}) - - Expect(err).NotTo(HaveOccurred()) - Expect(result).To(Equal(ctrl.Result{})) - }) - }) - - Context("when reconciling Secret", func() { - BeforeEach(func() { - Expect(fakeClient.Create(ctx, testSecret)).To(Succeed()) - Expect(fakeClient.Create(ctx, testHelmRelease)).To(Succeed()) - }) - - It("should successfully reconcile Secret when data changes", func() { - testSecret.Data["newkey"] = []byte("newvalue") - Expect(fakeClient.Update(ctx, testSecret)).To(Succeed()) - - req := ctrl.Request{ - NamespacedName: types.NamespacedName{ - Name: testSecret.Name, - Namespace: testSecret.Namespace, - }, - } - - result, err := reconciler.ReconcileResource(ctx, req, &corev1.Secret{}) - - Expect(err).NotTo(HaveOccurred()) - Expect(result).To(Equal(ctrl.Result{})) - }) - }) - - Context("when resource doesn't exist", func() { - It("should return error when trying to reconcile non-existent resource", func() { - req := ctrl.Request{ - NamespacedName: types.NamespacedName{ - Name: "non-existent", - Namespace: testNamespace, - }, - } - - _, err := reconciler.ReconcileResource(ctx, req, &corev1.ConfigMap{}) - - Expect(err).To(HaveOccurred()) - }) - }) - - Context("when HelmRelease doesn't exist", func() { - BeforeEach(func() { - Expect(fakeClient.Create(ctx, testConfigMap)).To(Succeed()) - }) - - It("should return error when HelmRelease is not found", func() { - req := ctrl.Request{ - NamespacedName: types.NamespacedName{ - Name: testConfigMap.Name, - Namespace: testConfigMap.Namespace, - }, - } + hrNameAnnotation: HRNameAnnotation, + ns: "namespace2", + hrName: "hr2", + expectedAnnotations: map[string]string{ + HRNameAnnotation: "hr2", + HRNSAnnotation: "namespace2", + HashAnnotation: "dccb0d0c", + }, + }, + { + name: "Existing HRNameAnnotation without duplicate", + existingAnnotations: map[string]string{ + HRNameAnnotation: "hr1", + HRNSAnnotation: "namespace1", + }, + hrNameAnnotation: HRNameAnnotation, + ns: "namespace1", + hrName: "hr2", + expectedAnnotations: map[string]string{ + HRNameAnnotation: "hr1,hr2", + HRNSAnnotation: "namespace1", + HashAnnotation: "dccb0d0c", + }, + }, + { + name: "Existing HRNameAnnotation with duplicate", + existingAnnotations: map[string]string{ + HRNameAnnotation: "hr1", + HRNSAnnotation: "namespace1", + }, + hrNameAnnotation: HRNameAnnotation, + ns: "namespace1", + hrName: "hr1", + expectedAnnotations: map[string]string{ + HRNameAnnotation: "hr1", + HRNSAnnotation: "namespace1", + HashAnnotation: "dccb0d0c", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + resource := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-resource", + }, + Data: map[string]string{ + "key": "value", + }, + } + resource.SetAnnotations(tt.existingAnnotations) - _, err := reconciler.ReconcileResource(ctx, req, &corev1.ConfigMap{}) + updatedAnnotations := r.GetOrGenerateAnnotations(resource, tt.hrNameAnnotation, tt.ns, tt.hrName) - Expect(err).To(HaveOccurred()) - }) + assert.Equal(t, tt.expectedAnnotations, updatedAnnotations) }) - }) - - Describe("GetCurrentDigest", func() { - It("should return digest from HelmRelease history", func() { - digest := reconciler.GetCurrentDigest(testHelmRelease) - Expect(digest).To(Equal("old-digest")) + } +} + +func TestAddAnnotationsAndLabel(t *testing.T) { + scheme := runtime.NewScheme() + _ = corev1.AddToScheme(scheme) + fakeClient := fake.NewClientBuilder().WithScheme(scheme).Build() + r := &BaseReconciler{ + Client: fakeClient, + } + + tests := []struct { + name string + existingAnnotations map[string]string + existingLabels map[string]string + newAnnotations map[string]string + newLabels map[string]string + expectedAnnotations map[string]string + expectedLabels map[string]string + }{ + { + name: "Add new annotations and labels", + existingAnnotations: nil, + existingLabels: nil, + newAnnotations: map[string]string{"annotation1": "value1"}, + newLabels: map[string]string{"label1": "value1"}, + expectedAnnotations: map[string]string{"annotation1": "value1"}, + expectedLabels: map[string]string{"label1": "value1"}, + }, + { + name: "Update existing annotations and labels", + existingAnnotations: map[string]string{"annotation1": "oldValue"}, + existingLabels: map[string]string{"label1": "oldValue"}, + newAnnotations: map[string]string{"annotation1": "newValue"}, + newLabels: map[string]string{"label1": "newValue"}, + expectedAnnotations: map[string]string{"annotation1": "newValue"}, + expectedLabels: map[string]string{"label1": "newValue"}, + }, + { + name: "No changes to annotations and labels", + existingAnnotations: map[string]string{"annotation1": "value1"}, + existingLabels: map[string]string{"label1": "value1"}, + newAnnotations: map[string]string{"annotation1": "value1"}, + newLabels: map[string]string{"label1": "value1"}, + expectedAnnotations: map[string]string{"annotation1": "value1"}, + expectedLabels: map[string]string{"label1": "value1"}, + }, + { + name: "Add annotations without labels", + existingAnnotations: nil, + existingLabels: map[string]string{"label1": "value1"}, + newAnnotations: map[string]string{"annotation1": "value1"}, + newLabels: nil, + expectedAnnotations: map[string]string{"annotation1": "value1"}, + expectedLabels: map[string]string{"label1": "value1"}, + }, + { + name: "Add labels without annotations", + existingAnnotations: map[string]string{"annotation1": "value1"}, + existingLabels: nil, + newAnnotations: nil, + newLabels: map[string]string{"label1": "value1"}, + expectedAnnotations: map[string]string{"annotation1": "value1"}, + expectedLabels: map[string]string{"label1": "value1"}, + }, + { + name: "Add multiple annotations", + existingAnnotations: nil, + existingLabels: nil, + newAnnotations: map[string]string{ + "annotation1": "value1", "annotation2": "value2"}, + newLabels: map[string]string{"label1": "value1"}, + expectedAnnotations: map[string]string{ + "annotation1": "value1", "annotation2": "value2"}, + expectedLabels: map[string]string{"label1": "value1"}, + }, + { + name: "Update multiple existing annotations", + existingAnnotations: map[string]string{ + "annotation1": "oldValue1", "annotation2": "oldValue2"}, + existingLabels: map[string]string{"label1": "oldValue"}, + newAnnotations: map[string]string{ + "annotation3": "newValue3"}, + newLabels: map[string]string{"label1": "newValue"}, + expectedAnnotations: map[string]string{ + "annotation1": "oldValue1", "annotation2": "oldValue2", "annotation3": "newValue3"}, + expectedLabels: map[string]string{"label1": "newValue"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cmName := "test-configmap" + cm := &corev1.ConfigMap{} + err := fakeClient.Get(context.TODO(), types.NamespacedName{ + Name: cmName, + Namespace: "default", + }, cm) + if err == nil { + err = fakeClient.Delete(context.TODO(), cm) + } + if err != nil && !apierrors.IsNotFound(err) { + assert.NoError(t, err) + } + + resource := &corev1.ConfigMap{} + resource.SetName(cmName) + resource.SetNamespace("default") + resource.SetAnnotations(tt.existingAnnotations) + resource.SetLabels(tt.existingLabels) + + err = fakeClient.Create(context.TODO(), resource) + assert.NoError(t, err) + + err = r.AddAnnotationsAndLabel(context.TODO(), resource, tt.newAnnotations, tt.newLabels) + + assert.NoError(t, err) + + updatedResource := &corev1.ConfigMap{} + err = fakeClient.Get(context.TODO(), types.NamespacedName{ + Name: cmName, + Namespace: "default", + }, updatedResource) + assert.NoError(t, err) + + assert.Equal(t, tt.expectedAnnotations, updatedResource.GetAnnotations()) + + assert.Equal(t, tt.expectedLabels, updatedResource.GetLabels()) }) - - It("should return empty string when no history exists", func() { - testHelmRelease.Status.History = helmv2.Snapshots([]*helmv2.Snapshot{}) - - digest := reconciler.GetCurrentDigest(testHelmRelease) - Expect(digest).To(BeEmpty()) + } +} + +func TestRemoveHRNameFromAnnotations(t *testing.T) { + scheme := runtime.NewScheme() + _ = corev1.AddToScheme(scheme) + fakeClient := fake.NewClientBuilder().WithScheme(scheme).Build() + + r := &BaseReconciler{ + Client: fakeClient, + } + + tests := []struct { + name string + existingAnnotations map[string]string + hrNameToRemove string + expectedAnnotations map[string]string + }{ + { + name: "Remove single HR name from annotations", + existingAnnotations: map[string]string{HRNameAnnotation: "hr1"}, + hrNameToRemove: "hr1", + expectedAnnotations: map[string]string(nil), + }, + { + name: "Remove one HR name from a list", + existingAnnotations: map[string]string{HRNameAnnotation: "hr1,hr2,hr3"}, + hrNameToRemove: "hr2", + expectedAnnotations: map[string]string{HRNameAnnotation: "hr1,hr3"}, + }, + { + name: "HR name not found in annotations", + existingAnnotations: map[string]string{HRNameAnnotation: "hr1,hr2"}, + hrNameToRemove: "hr3", + expectedAnnotations: map[string]string{HRNameAnnotation: "hr1,hr2"}, + }, + { + name: "No annotations present", + existingAnnotations: nil, + hrNameToRemove: "hr1", + expectedAnnotations: nil, + }, + { + name: "Remove last HR name from a list", + existingAnnotations: map[string]string{HRNameAnnotation: "hr1"}, + hrNameToRemove: "hr1", + expectedAnnotations: map[string]string(nil), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cmName := "test-configmap" + cm := &corev1.ConfigMap{} + err := fakeClient.Get(context.TODO(), types.NamespacedName{ + Name: cmName, + Namespace: "default", + }, cm) + if err == nil { + err = fakeClient.Delete(context.TODO(), cm) + } + if err != nil && !apierrors.IsNotFound(err) { + assert.NoError(t, err) + } + + resource := &corev1.ConfigMap{} + resource.SetName(cmName) + resource.SetNamespace("default") + resource.SetAnnotations(tt.existingAnnotations) + + err = fakeClient.Create(context.TODO(), resource) + assert.NoError(t, err) + + err = r.RemoveHRNameFromAnnotations(context.TODO(), resource, tt.hrNameToRemove) + + assert.NoError(t, err) + + updatedResource := &corev1.ConfigMap{} + err = fakeClient.Get(context.TODO(), client.ObjectKey{ + Name: cmName, + Namespace: "default", + }, updatedResource) + assert.NoError(t, err) + + assert.Equal(t, tt.expectedAnnotations, updatedResource.GetAnnotations()) }) - }) -}) + } +} diff --git a/internal/controller/hrautodiscovercontroller.go b/internal/controller/hrautodiscovercontroller.go index 885cc7c..e54e9aa 100644 --- a/internal/controller/hrautodiscovercontroller.go +++ b/internal/controller/hrautodiscovercontroller.go @@ -2,18 +2,17 @@ package controller import ( "context" - "fmt" "reflect" "time" helmv2 "github.com/fluxcd/helm-controller/api/v2" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/predicate" ) @@ -41,7 +40,7 @@ func NewHRAutodicoverReconciler(client client.Client, scheme *runtime.Scheme) *H // +kubebuilder:rbac:groups=core,resources=secrets,verbs=get;list;watch;patch func (r *HRAutodicoverReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { - log := log.FromContext(ctx) + log := log.FromContext(ctx).WithName(HRAutodicoverReconcilerName) log.Info("Reconciling HelmRelease for autodiscovery", "name", req.NamespacedName) var hr helmv2.HelmRelease if err := r.Get(ctx, req.NamespacedName, &hr); err != nil { @@ -50,7 +49,7 @@ func (r *HRAutodicoverReconciler) Reconcile(ctx context.Context, req ctrl.Reques if hr.Spec.ValuesFrom != nil { for _, valueFrom := range hr.Spec.ValuesFrom { - if err := r.ReconcileResource(ctx, &hr, valueFrom); err != nil { + if err := r.ReconcileHRSecretOrConfigMap(ctx, &hr, valueFrom); err != nil { if client.IgnoreNotFound(err) == nil { log.Info("Resource not found, skipping", "kind", valueFrom.Kind, "name", valueFrom.Name, "namespace", hr.Namespace) continue @@ -68,90 +67,6 @@ func (r *HRAutodicoverReconciler) Reconcile(ctx context.Context, req ctrl.Reques return ctrl.Result{}, nil } -func (r *HRAutodicoverReconciler) ReconcileResource(ctx context.Context, hr *helmv2.HelmRelease, valueFrom helmv2.ValuesReference) error { - log := log.FromContext(ctx) - log.Info("Reconciling resource for autodiscovery", - "kind", valueFrom.Kind, - "name", valueFrom.Name, - "namespace", hr.Namespace) - - var resource client.Object - switch valueFrom.Kind { - case "ConfigMap": - resource = &corev1.ConfigMap{} - case "Secret": - resource = &corev1.Secret{} - default: - log.Error(fmt.Errorf("unsupported kind: %s", valueFrom.Kind), - "Failed to reconcile resource for autodiscovery") - return nil - } - - objRef := &corev1.ObjectReference{ - Name: valueFrom.Name, - Namespace: hr.Namespace, - } - - if err := r.Get(ctx, types.NamespacedName{ - Name: objRef.Name, - Namespace: objRef.Namespace, - }, resource); err != nil { - return err - } - - annotations := map[string]string{ - HRNameAnnotation: hr.Name, - HRNSAnnotation: hr.Namespace, - HashAnnotation: r.GetNewDigest(resource), - } - - labels := map[string]string{ - LabelReconcilerNameSourceKey: "true", - } - - return r.addAnnotationsAndLabel(ctx, resource, annotations, labels) -} - -func (r *HRAutodicoverReconciler) addAnnotationsAndLabel( - ctx context.Context, resource client.Object, annotations, labels map[string]string) error { - log := log.FromContext(ctx).V(1) - - if resource.GetAnnotations() == nil { - resource.SetAnnotations(make(map[string]string)) - } - if resource.GetLabels() == nil { - resource.SetLabels(make(map[string]string)) - } - - updatedAnnotations := false - for key, value := range annotations { - if resource.GetAnnotations()[key] != value { - resource.GetAnnotations()[key] = value - updatedAnnotations = true - } - } - - updatedLabels := false - for key, value := range labels { - if resource.GetLabels()[key] != value { - resource.GetLabels()[key] = value - updatedLabels = true - } - } - - if updatedAnnotations || updatedLabels { - if err := r.Patch(ctx, resource, client.MergeFrom(resource.DeepCopyObject().(client.Object))); err != nil { - return err - } - log.Info("Updated resource with annotations and labels for autodiscovery", - "kind", resource.GetObjectKind().GroupVersionKind().Kind, - "name", resource.GetName(), - "namespace", resource.GetNamespace()) - } - - return nil -} - func (r *HRAutodicoverReconciler) SetupWithManager(mgr ctrl.Manager, maxConcurrency int, cacheSyncTimeout time.Duration) error { return ctrl.NewControllerManagedBy(mgr).Named(HRAutodicoverReconcilerName). @@ -175,6 +90,42 @@ func (r *HRAutodicoverReconciler) SetupWithManager(mgr ctrl.Manager, return false }, })). + Watches( + &corev1.Secret{}, + handler.EnqueueRequestsFromMapFunc(r.findHelmReleasesForSecret), + builder.WithPredicates(predicate.Funcs{ + CreateFunc: func(e event.CreateEvent) bool { + return true + }, + DeleteFunc: func(e event.DeleteEvent) bool { + return false + }, + UpdateFunc: func(e event.UpdateEvent) bool { + return false + }, + GenericFunc: func(e event.GenericEvent) bool { + return false + }, + }), + ). + Watches( + &corev1.ConfigMap{}, + handler.EnqueueRequestsFromMapFunc(r.findHelmReleasesForConfigMap), + builder.WithPredicates(predicate.Funcs{ + CreateFunc: func(e event.CreateEvent) bool { + return true + }, + DeleteFunc: func(e event.DeleteEvent) bool { + return false + }, + UpdateFunc: func(e event.UpdateEvent) bool { + return false + }, + GenericFunc: func(e event.GenericEvent) bool { + return false + }, + }), + ). WithOptions(ControllerOptions(maxConcurrency, cacheSyncTimeout)). Complete(r) }