From 4ffe621c6bde7d4284b37811710ec13b4b8e7e45 Mon Sep 17 00:00:00 2001 From: Stefan Prodan Date: Tue, 22 Jul 2025 21:09:30 +0300 Subject: [PATCH 1/5] api: Add the `readyExpr` field to `dependsOn` Extend the readiness evaluation of dependencies with CEL expressions Signed-off-by: Stefan Prodan --- api/v2/helmrelease_types.go | 18 ++++- api/v2/reference_types.go | 20 +++++ api/v2/zz_generated.deepcopy.go | 17 ++++- .../helm.toolkit.fluxcd.io_helmreleases.yaml | 20 +++-- docs/api/v2/helm.md | 73 +++++++++++++++++-- .../controller/helmrelease_controller_test.go | 12 +-- 6 files changed, 137 insertions(+), 23 deletions(-) diff --git a/api/v2/helmrelease_types.go b/api/v2/helmrelease_types.go index d53042bcb..92457f325 100644 --- a/api/v2/helmrelease_types.go +++ b/api/v2/helmrelease_types.go @@ -100,11 +100,11 @@ type HelmReleaseSpec struct { // +optional StorageNamespace string `json:"storageNamespace,omitempty"` - // DependsOn may contain a meta.NamespacedObjectReference slice with + // DependsOn may contain a DependencyReference slice with // references to HelmRelease resources that must be ready before this HelmRelease // can be reconciled. // +optional - DependsOn []meta.NamespacedObjectReference `json:"dependsOn,omitempty"` + DependsOn []DependencyReference `json:"dependsOn,omitempty"` // Timeout is the time to wait for any individual Kubernetes operation (like Jobs // for hooks) during the performance of a Helm action. Defaults to '5m0s'. @@ -1265,9 +1265,19 @@ func (in HelmRelease) UsePersistentClient() bool { return *in.Spec.PersistentClient } -// GetDependsOn returns the list of dependencies across-namespaces. +// GetDependsOn returns the dependencies as a list of meta.NamespacedObjectReference. +// +// This function makes the HelmRelease type conformant with the meta.ObjectWithDependencies interface +// and allows the controller-runtime to index HelmReleases by their dependencies. func (in HelmRelease) GetDependsOn() []meta.NamespacedObjectReference { - return in.Spec.DependsOn + deps := make([]meta.NamespacedObjectReference, len(in.Spec.DependsOn)) + for i := range in.Spec.DependsOn { + deps[i] = meta.NamespacedObjectReference{ + Name: in.Spec.DependsOn[i].Name, + Namespace: in.Spec.DependsOn[i].Namespace, + } + } + return deps } // GetConditions returns the status conditions of the object. diff --git a/api/v2/reference_types.go b/api/v2/reference_types.go index 419913b12..7e19b2269 100644 --- a/api/v2/reference_types.go +++ b/api/v2/reference_types.go @@ -68,3 +68,23 @@ type CrossNamespaceSourceReference struct { // +optional Namespace string `json:"namespace,omitempty"` } + +// DependencyReference defines a HelmRelease dependency on another HelmRelease resource. +type DependencyReference struct { + // Name of the referent. + // +required + Name string `json:"name"` + + // Namespace of the referent, defaults to the namespace of the HelmRelease + // resource object that contains the reference. + // +optional + Namespace string `json:"namespace,omitempty"` + + // ReadyExpr is a CEL expression that can be used to assess the readiness + // of a dependency. When specified, the built-in readiness check + // is replaced by the logic defined in the CEL expression. + // To make the CEL expression additive to the built-in readiness check, + // the feature gate `AdditiveCELDependencyCheck` must be set to `true`. + // +optional + ReadyExpr string `json:"readyExpr,omitempty"` +} diff --git a/api/v2/zz_generated.deepcopy.go b/api/v2/zz_generated.deepcopy.go index 903efbaad..cfcc81c4f 100644 --- a/api/v2/zz_generated.deepcopy.go +++ b/api/v2/zz_generated.deepcopy.go @@ -87,6 +87,21 @@ func (in *CrossNamespaceSourceReference) DeepCopy() *CrossNamespaceSourceReferen return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *DependencyReference) DeepCopyInto(out *DependencyReference) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new DependencyReference. +func (in *DependencyReference) DeepCopy() *DependencyReference { + if in == nil { + return nil + } + out := new(DependencyReference) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *DriftDetection) DeepCopyInto(out *DriftDetection) { *out = *in @@ -305,7 +320,7 @@ func (in *HelmReleaseSpec) DeepCopyInto(out *HelmReleaseSpec) { } if in.DependsOn != nil { in, out := &in.DependsOn, &out.DependsOn - *out = make([]meta.NamespacedObjectReference, len(*in)) + *out = make([]DependencyReference, len(*in)) copy(*out, *in) } if in.Timeout != nil { diff --git a/config/crd/bases/helm.toolkit.fluxcd.io_helmreleases.yaml b/config/crd/bases/helm.toolkit.fluxcd.io_helmreleases.yaml index a1156cf3c..8ad103da8 100644 --- a/config/crd/bases/helm.toolkit.fluxcd.io_helmreleases.yaml +++ b/config/crd/bases/helm.toolkit.fluxcd.io_helmreleases.yaml @@ -239,20 +239,28 @@ spec: type: object dependsOn: description: |- - DependsOn may contain a meta.NamespacedObjectReference slice with + DependsOn may contain a DependencyReference slice with references to HelmRelease resources that must be ready before this HelmRelease can be reconciled. items: - description: |- - NamespacedObjectReference contains enough information to locate the referenced Kubernetes resource object in any - namespace. + description: DependencyReference defines a HelmRelease dependency + on another HelmRelease resource. properties: name: description: Name of the referent. type: string namespace: - description: Namespace of the referent, when not specified it - acts as LocalObjectReference. + description: |- + Namespace of the referent, defaults to the namespace of the HelmRelease + resource object that contains the reference. + type: string + readyExpr: + description: |- + ReadyExpr is a CEL expression that can be used to assess the readiness + of a dependency. When specified, the built-in readiness check + is replaced by the logic defined in the CEL expression. + To make the CEL expression additive to the built-in readiness check, + the feature gate `AdditiveCELDependencyCheck` must be set to `true`. type: string required: - name diff --git a/docs/api/v2/helm.md b/docs/api/v2/helm.md index 1e4f5bef8..b4af0cfe2 100644 --- a/docs/api/v2/helm.md +++ b/docs/api/v2/helm.md @@ -187,14 +187,14 @@ Defaults to the namespace of the HelmRelease.

dependsOn
- -[]github.com/fluxcd/pkg/apis/meta.NamespacedObjectReference + +[]DependencyReference (Optional) -

DependsOn may contain a meta.NamespacedObjectReference slice with +

DependsOn may contain a DependencyReference slice with references to HelmRelease resources that must be ready before this HelmRelease can be reconciled.

@@ -615,6 +615,67 @@ resource object that contains the reference.

+

DependencyReference +

+

+(Appears on: +HelmReleaseSpec) +

+

DependencyReference defines a HelmRelease dependency on another HelmRelease resource.

+
+
+ + + + + + + + + + + + + + + + + + + + + +
FieldDescription
+name
+ +string + +		 +

Name of the referent.

+
+namespace
+ +string + +		 +(Optional) +

Namespace of the referent, defaults to the namespace of the HelmRelease +resource object that contains the reference.

+
+readyExpr
+ +string + +		 +(Optional) +

ReadyExpr is a CEL expression that can be used to assess the readiness +of a dependency. When specified, the built-in readiness check +is replaced by the logic defined in the CEL expression. +To make the CEL expression additive to the built-in readiness check, +the feature gate AdditiveCELDependencyCheck must be set to true.

+
+
+

DriftDetection

@@ -1258,14 +1319,14 @@ Defaults to the namespace of the HelmRelease.

dependsOn
- -[]github.com/fluxcd/pkg/apis/meta.NamespacedObjectReference + +[]DependencyReference (Optional) -

DependsOn may contain a meta.NamespacedObjectReference slice with +

DependsOn may contain a DependencyReference slice with references to HelmRelease resources that must be ready before this HelmRelease can be reconciled.

diff --git a/internal/controller/helmrelease_controller_test.go b/internal/controller/helmrelease_controller_test.go index be52e1ddd..d559a909c 100644 --- a/internal/controller/helmrelease_controller_test.go +++ b/internal/controller/helmrelease_controller_test.go @@ -93,7 +93,7 @@ func TestHelmReleaseReconciler_reconcileRelease(t *testing.T) { Namespace: "mock", }, Spec: v2.HelmReleaseSpec{ - DependsOn: []meta.NamespacedObjectReference{ + DependsOn: []v2.DependencyReference{ { Name: "dependency", }, @@ -2818,7 +2818,7 @@ func TestHelmReleaseReconciler_checkDependencies(t *testing.T) { Namespace: "some-namespace", }, Spec: v2.HelmReleaseSpec{ - DependsOn: []meta.NamespacedObjectReference{ + DependsOn: []v2.DependencyReference{ { Name: "dependency-1", }, @@ -2869,7 +2869,7 @@ func TestHelmReleaseReconciler_checkDependencies(t *testing.T) { Namespace: "some-namespace", }, Spec: v2.HelmReleaseSpec{ - DependsOn: []meta.NamespacedObjectReference{ + DependsOn: []v2.DependencyReference{ { Name: "dependency-1", }, @@ -2904,7 +2904,7 @@ func TestHelmReleaseReconciler_checkDependencies(t *testing.T) { Namespace: "some-namespace", }, Spec: v2.HelmReleaseSpec{ - DependsOn: []meta.NamespacedObjectReference{ + DependsOn: []v2.DependencyReference{ { Name: "dependency-1", }, @@ -2939,7 +2939,7 @@ func TestHelmReleaseReconciler_checkDependencies(t *testing.T) { Namespace: "some-namespace", }, Spec: v2.HelmReleaseSpec{ - DependsOn: []meta.NamespacedObjectReference{ + DependsOn: []v2.DependencyReference{ { Name: "dependency-1", }, @@ -2971,7 +2971,7 @@ func TestHelmReleaseReconciler_checkDependencies(t *testing.T) { Namespace: "some-namespace", }, Spec: v2.HelmReleaseSpec{ - DependsOn: []meta.NamespacedObjectReference{ + DependsOn: []v2.DependencyReference{ { Name: "dependency-1", }, From 6d5856c00eb83b8d17e61d079f815ac6273fc003 Mon Sep 17 00:00:00 2001 From: Stefan Prodan Date: Tue, 22 Jul 2025 21:18:55 +0300 Subject: [PATCH 2/5] controller: Move manager and indexers to dedicated files Signed-off-by: Stefan Prodan --- internal/controller/helmrelease_controller.go | 221 ------------------ internal/controller/helmrelease_indexers.go | 136 +++++++++++ internal/controller/helmrelease_manager.go | 150 ++++++++++++ 3 files changed, 286 insertions(+), 221 deletions(-) create mode 100644 internal/controller/helmrelease_indexers.go create mode 100644 internal/controller/helmrelease_manager.go diff --git a/internal/controller/helmrelease_controller.go b/internal/controller/helmrelease_controller.go index e9a8efba0..5371c07f5 100644 --- a/internal/controller/helmrelease_controller.go +++ b/internal/controller/helmrelease_controller.go @@ -33,14 +33,9 @@ import ( "k8s.io/cli-runtime/pkg/genericclioptions" "k8s.io/client-go/rest" kuberecorder "k8s.io/client-go/tools/record" - "k8s.io/client-go/util/workqueue" 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/controller" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" - "sigs.k8s.io/controller-runtime/pkg/handler" - "sigs.k8s.io/controller-runtime/pkg/predicate" "sigs.k8s.io/controller-runtime/pkg/reconcile" "github.com/Masterminds/semver/v3" @@ -57,7 +52,6 @@ import ( "github.com/fluxcd/pkg/runtime/logger" "github.com/fluxcd/pkg/runtime/object" "github.com/fluxcd/pkg/runtime/patch" - "github.com/fluxcd/pkg/runtime/predicates" sourcev1 "github.com/fluxcd/source-controller/api/v1" "github.com/fluxcd/pkg/chartutil" @@ -71,7 +65,6 @@ import ( "github.com/fluxcd/helm-controller/internal/kube" "github.com/fluxcd/helm-controller/internal/loader" "github.com/fluxcd/helm-controller/internal/postrender" - intpredicates "github.com/fluxcd/helm-controller/internal/predicates" intreconcile "github.com/fluxcd/helm-controller/internal/reconcile" "github.com/fluxcd/helm-controller/internal/release" ) @@ -105,123 +98,11 @@ type HelmReleaseReconciler struct { artifactFetchRetries int } -type HelmReleaseReconcilerOptions struct { - HTTPRetry int - DependencyRequeueInterval time.Duration - RateLimiter workqueue.TypedRateLimiter[reconcile.Request] - WatchConfigsPredicate predicate.Predicate -} - var ( errWaitForDependency = errors.New("must wait for dependency") errWaitForChart = errors.New("must wait for chart") ) -func (r *HelmReleaseReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, opts HelmReleaseReconcilerOptions) error { - const ( - indexConfigMap = ".metadata.configMap" - indexSecret = ".metadata.secret" - ) - - // Index the HelmRelease by the Source reference they point to. - if err := mgr.GetFieldIndexer().IndexField(ctx, &v2.HelmRelease{}, v2.SourceIndexKey, - func(o client.Object) []string { - obj := o.(*v2.HelmRelease) - var kind, name, namespace string - switch { - case obj.HasChartRef() && !obj.HasChartTemplate(): - kind = obj.Spec.ChartRef.Kind - name = obj.Spec.ChartRef.Name - namespace = obj.Spec.ChartRef.Namespace - if namespace == "" { - namespace = obj.GetNamespace() - } - case !obj.HasChartRef() && obj.HasChartTemplate(): - kind = sourcev1.HelmChartKind - name = obj.GetHelmChartName() - namespace = obj.Spec.Chart.GetNamespace(obj.GetNamespace()) - default: - return nil - } - return []string{fmt.Sprintf("%s/%s/%s", kind, namespace, name)} - }, - ); err != nil { - return err - } - - // Index the HelmRelease by the ConfigMap references they point to. - if err := mgr.GetFieldIndexer().IndexField(ctx, &v2.HelmRelease{}, indexConfigMap, - func(o client.Object) []string { - obj := o.(*v2.HelmRelease) - namespace := obj.GetNamespace() - var keys []string - if kc := obj.Spec.KubeConfig; kc != nil && kc.ConfigMapRef != nil { - keys = append(keys, fmt.Sprintf("%s/%s", namespace, kc.ConfigMapRef.Name)) - } - for _, ref := range obj.Spec.ValuesFrom { - if ref.Kind == "ConfigMap" { - keys = append(keys, fmt.Sprintf("%s/%s", namespace, ref.Name)) - } - } - return keys - }, - ); err != nil { - return err - } - - // Index the HelmRelease by the Secret references they point to. - if err := mgr.GetFieldIndexer().IndexField(ctx, &v2.HelmRelease{}, indexSecret, - func(o client.Object) []string { - obj := o.(*v2.HelmRelease) - namespace := obj.GetNamespace() - var keys []string - if kc := obj.Spec.KubeConfig; kc != nil && kc.SecretRef != nil { - keys = append(keys, fmt.Sprintf("%s/%s", namespace, kc.SecretRef.Name)) - } - for _, ref := range obj.Spec.ValuesFrom { - if ref.Kind == "Secret" { - keys = append(keys, fmt.Sprintf("%s/%s", namespace, ref.Name)) - } - } - return keys - }, - ); err != nil { - return err - } - - r.requeueDependency = opts.DependencyRequeueInterval - r.artifactFetchRetries = opts.HTTPRetry - - return ctrl.NewControllerManagedBy(mgr). - For(&v2.HelmRelease{}, builder.WithPredicates( - predicate.Or(predicate.GenerationChangedPredicate{}, predicates.ReconcileRequestedPredicate{}), - )). - Watches( - &sourcev1.HelmChart{}, - handler.EnqueueRequestsFromMapFunc(r.requestsForHelmChartChange), - builder.WithPredicates(intpredicates.SourceRevisionChangePredicate{}), - ). - Watches( - &sourcev1.OCIRepository{}, - handler.EnqueueRequestsFromMapFunc(r.requestsForOCIRepositoryChange), - builder.WithPredicates(intpredicates.SourceRevisionChangePredicate{}), - ). - WatchesMetadata( - &corev1.ConfigMap{}, - handler.EnqueueRequestsFromMapFunc(r.requestsForConfigDependency(indexConfigMap)), - builder.WithPredicates(predicate.ResourceVersionChangedPredicate{}, opts.WatchConfigsPredicate), - ). - WatchesMetadata( - &corev1.Secret{}, - handler.EnqueueRequestsFromMapFunc(r.requestsForConfigDependency(indexSecret)), - builder.WithPredicates(predicate.ResourceVersionChangedPredicate{}, opts.WatchConfigsPredicate), - ). - WithOptions(controller.Options{ - RateLimiter: opts.RateLimiter, - }). - Complete(r) -} - func (r *HelmReleaseReconciler) Reconcile(ctx context.Context, req ctrl.Request) (result ctrl.Result, retErr error) { start := time.Now() log := ctrl.LoggerFrom(ctx) @@ -871,108 +752,6 @@ func (r *HelmReleaseReconciler) waitForHistoryCacheSync(obj *v2.HelmRelease) wai } } -func (r *HelmReleaseReconciler) requestsForHelmChartChange(ctx context.Context, o client.Object) []reconcile.Request { - hc, ok := o.(*sourcev1.HelmChart) - if !ok { - err := fmt.Errorf("expected a HelmChart, got %T", o) - ctrl.LoggerFrom(ctx).Error(err, "failed to get requests for HelmChart change") - return nil - } - // If we do not have an artifact, we have no requests to make - if hc.GetArtifact() == nil { - return nil - } - - var list v2.HelmReleaseList - if err := r.List(ctx, &list, client.MatchingFields{ - v2.SourceIndexKey: sourcev1.HelmChartKind + "/" + client.ObjectKeyFromObject(hc).String(), - }); err != nil { - ctrl.LoggerFrom(ctx).Error(err, "failed to list HelmReleases for HelmChart change") - return nil - } - - var reqs []reconcile.Request - for i, hr := range list.Items { - // If the HelmRelease is ready and the revision of the artifact equals to the - // last attempted revision, we should not make a request for this HelmRelease - if conditions.IsReady(&list.Items[i]) && hc.GetArtifact().HasRevision(hr.Status.GetLastAttemptedRevision()) { - continue - } - reqs = append(reqs, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(&list.Items[i])}) - } - return reqs -} - -func (r *HelmReleaseReconciler) requestsForOCIRepositoryChange(ctx context.Context, o client.Object) []reconcile.Request { - or, ok := o.(*sourcev1.OCIRepository) - if !ok { - err := fmt.Errorf("expected an OCIRepository, got %T", o) - ctrl.LoggerFrom(ctx).Error(err, "failed to get requests for OCIRepository change") - return nil - } - // If we do not have an artifact, we have no requests to make - if or.GetArtifact() == nil { - return nil - } - - var list v2.HelmReleaseList - if err := r.List(ctx, &list, client.MatchingFields{ - v2.SourceIndexKey: sourcev1.OCIRepositoryKind + "/" + client.ObjectKeyFromObject(or).String(), - }); err != nil { - ctrl.LoggerFrom(ctx).Error(err, "failed to list HelmReleases for OCIRepository change") - return nil - } - - var reqs []reconcile.Request - for i, hr := range list.Items { - // If the HelmRelease is ready and the digest of the artifact equals to the - // last attempted revision digest, we should not make a request for this HelmRelease, - // likewise if we cannot retrieve the artifact digest. - digest := extractDigest(or.GetArtifact().Revision) - if digest == "" { - ctrl.LoggerFrom(ctx).Error(fmt.Errorf("wrong digest for %T", or), "failed to get requests for OCIRepository change") - continue - } - - if digest == hr.Status.LastAttemptedRevisionDigest { - continue - } - - reqs = append(reqs, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(&list.Items[i])}) - } - return reqs -} - -// requestsForConfigDependency enqueues requests for watched ConfigMaps or Secrets -// according to the specified index. -func (r *HelmReleaseReconciler) requestsForConfigDependency( - index string) func(ctx context.Context, o client.Object) []reconcile.Request { - - return func(ctx context.Context, o client.Object) []reconcile.Request { - // List HelmReleases that have a dependency on the ConfigMap or Secret. - var list v2.HelmReleaseList - if err := r.List(ctx, &list, client.MatchingFields{ - index: client.ObjectKeyFromObject(o).String(), - }); err != nil { - ctrl.LoggerFrom(ctx).Error(err, "failed to list HelmReleases for config dependency change", - "index", index, "objectRef", map[string]string{ - "name": o.GetName(), - "namespace": o.GetNamespace(), - }) - return nil - } - - // Enqueue requests for each HelmRelease in the list. - reqs := make([]reconcile.Request, 0, len(list.Items)) - for i := range list.Items { - reqs = append(reqs, reconcile.Request{ - NamespacedName: client.ObjectKeyFromObject(&list.Items[i]), - }) - } - return reqs - } -} - func isSourceReady(obj sourcev1.Source) (bool, string) { if o, ok := obj.(conditions.Getter); ok { return isReady(o, obj.GetArtifact()) diff --git a/internal/controller/helmrelease_indexers.go b/internal/controller/helmrelease_indexers.go new file mode 100644 index 000000000..fab44af50 --- /dev/null +++ b/internal/controller/helmrelease_indexers.go @@ -0,0 +1,136 @@ +/* +Copyright 2025 The Flux authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controller + +import ( + "context" + "fmt" + + "github.com/fluxcd/pkg/runtime/conditions" + sourcev1 "github.com/fluxcd/source-controller/api/v1" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + v2 "github.com/fluxcd/helm-controller/api/v2" +) + +// requestsForHelmChartChange enqueues requests for watched HelmCharts +// according to the specified index. +func (r *HelmReleaseReconciler) requestsForHelmChartChange(ctx context.Context, o client.Object) []reconcile.Request { + hc, ok := o.(*sourcev1.HelmChart) + if !ok { + err := fmt.Errorf("expected a HelmChart, got %T", o) + ctrl.LoggerFrom(ctx).Error(err, "failed to get requests for HelmChart change") + return nil + } + // If we do not have an artifact, we have no requests to make + if hc.GetArtifact() == nil { + return nil + } + + var list v2.HelmReleaseList + if err := r.List(ctx, &list, client.MatchingFields{ + v2.SourceIndexKey: sourcev1.HelmChartKind + "/" + client.ObjectKeyFromObject(hc).String(), + }); err != nil { + ctrl.LoggerFrom(ctx).Error(err, "failed to list HelmReleases for HelmChart change") + return nil + } + + var reqs []reconcile.Request + for i, hr := range list.Items { + // If the HelmRelease is ready and the revision of the artifact equals to the + // last attempted revision, we should not make a request for this HelmRelease + if conditions.IsReady(&list.Items[i]) && hc.GetArtifact().HasRevision(hr.Status.GetLastAttemptedRevision()) { + continue + } + reqs = append(reqs, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(&list.Items[i])}) + } + return reqs +} + +// requestsForOCIRepositoryChange enqueues requests for watched OCIRepositories +// according to the specified index. +func (r *HelmReleaseReconciler) requestsForOCIRepositoryChange(ctx context.Context, o client.Object) []reconcile.Request { + or, ok := o.(*sourcev1.OCIRepository) + if !ok { + err := fmt.Errorf("expected an OCIRepository, got %T", o) + ctrl.LoggerFrom(ctx).Error(err, "failed to get requests for OCIRepository change") + return nil + } + // If we do not have an artifact, we have no requests to make + if or.GetArtifact() == nil { + return nil + } + + var list v2.HelmReleaseList + if err := r.List(ctx, &list, client.MatchingFields{ + v2.SourceIndexKey: sourcev1.OCIRepositoryKind + "/" + client.ObjectKeyFromObject(or).String(), + }); err != nil { + ctrl.LoggerFrom(ctx).Error(err, "failed to list HelmReleases for OCIRepository change") + return nil + } + + var reqs []reconcile.Request + for i, hr := range list.Items { + // If the HelmRelease is ready and the digest of the artifact equals to the + // last attempted revision digest, we should not make a request for this HelmRelease, + // likewise if we cannot retrieve the artifact digest. + digest := extractDigest(or.GetArtifact().Revision) + if digest == "" { + ctrl.LoggerFrom(ctx).Error(fmt.Errorf("wrong digest for %T", or), "failed to get requests for OCIRepository change") + continue + } + + if digest == hr.Status.LastAttemptedRevisionDigest { + continue + } + + reqs = append(reqs, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(&list.Items[i])}) + } + return reqs +} + +// requestsForConfigDependency enqueues requests for watched ConfigMaps or Secrets +// according to the specified index. +func (r *HelmReleaseReconciler) requestsForConfigDependency( + index string) func(ctx context.Context, o client.Object) []reconcile.Request { + + return func(ctx context.Context, o client.Object) []reconcile.Request { + // List HelmReleases that have a dependency on the ConfigMap or Secret. + var list v2.HelmReleaseList + if err := r.List(ctx, &list, client.MatchingFields{ + index: client.ObjectKeyFromObject(o).String(), + }); err != nil { + ctrl.LoggerFrom(ctx).Error(err, "failed to list HelmReleases for config dependency change", + "index", index, "objectRef", map[string]string{ + "name": o.GetName(), + "namespace": o.GetNamespace(), + }) + return nil + } + + // Enqueue requests for each HelmRelease in the list. + reqs := make([]reconcile.Request, 0, len(list.Items)) + for i := range list.Items { + reqs = append(reqs, reconcile.Request{ + NamespacedName: client.ObjectKeyFromObject(&list.Items[i]), + }) + } + return reqs + } +} diff --git a/internal/controller/helmrelease_manager.go b/internal/controller/helmrelease_manager.go new file mode 100644 index 000000000..2f50c6d35 --- /dev/null +++ b/internal/controller/helmrelease_manager.go @@ -0,0 +1,150 @@ +/* +Copyright 2025 The Flux authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controller + +import ( + "context" + "fmt" + "time" + + "github.com/fluxcd/pkg/runtime/predicates" + sourcev1 "github.com/fluxcd/source-controller/api/v1" + corev1 "k8s.io/api/core/v1" + "k8s.io/client-go/util/workqueue" + 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/controller" + "sigs.k8s.io/controller-runtime/pkg/handler" + "sigs.k8s.io/controller-runtime/pkg/predicate" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + v2 "github.com/fluxcd/helm-controller/api/v2" + intpredicates "github.com/fluxcd/helm-controller/internal/predicates" +) + +type HelmReleaseReconcilerOptions struct { + HTTPRetry int + DependencyRequeueInterval time.Duration + RateLimiter workqueue.TypedRateLimiter[reconcile.Request] + WatchConfigsPredicate predicate.Predicate +} + +func (r *HelmReleaseReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, opts HelmReleaseReconcilerOptions) error { + const ( + indexConfigMap = ".metadata.configMap" + indexSecret = ".metadata.secret" + ) + + // Index the HelmRelease by the Source reference they point to. + if err := mgr.GetFieldIndexer().IndexField(ctx, &v2.HelmRelease{}, v2.SourceIndexKey, + func(o client.Object) []string { + obj := o.(*v2.HelmRelease) + var kind, name, namespace string + switch { + case obj.HasChartRef() && !obj.HasChartTemplate(): + kind = obj.Spec.ChartRef.Kind + name = obj.Spec.ChartRef.Name + namespace = obj.Spec.ChartRef.Namespace + if namespace == "" { + namespace = obj.GetNamespace() + } + case !obj.HasChartRef() && obj.HasChartTemplate(): + kind = sourcev1.HelmChartKind + name = obj.GetHelmChartName() + namespace = obj.Spec.Chart.GetNamespace(obj.GetNamespace()) + default: + return nil + } + return []string{fmt.Sprintf("%s/%s/%s", kind, namespace, name)} + }, + ); err != nil { + return err + } + + // Index the HelmRelease by the ConfigMap references they point to. + if err := mgr.GetFieldIndexer().IndexField(ctx, &v2.HelmRelease{}, indexConfigMap, + func(o client.Object) []string { + obj := o.(*v2.HelmRelease) + namespace := obj.GetNamespace() + var keys []string + if kc := obj.Spec.KubeConfig; kc != nil && kc.ConfigMapRef != nil { + keys = append(keys, fmt.Sprintf("%s/%s", namespace, kc.ConfigMapRef.Name)) + } + for _, ref := range obj.Spec.ValuesFrom { + if ref.Kind == "ConfigMap" { + keys = append(keys, fmt.Sprintf("%s/%s", namespace, ref.Name)) + } + } + return keys + }, + ); err != nil { + return err + } + + // Index the HelmRelease by the Secret references they point to. + if err := mgr.GetFieldIndexer().IndexField(ctx, &v2.HelmRelease{}, indexSecret, + func(o client.Object) []string { + obj := o.(*v2.HelmRelease) + namespace := obj.GetNamespace() + var keys []string + if kc := obj.Spec.KubeConfig; kc != nil && kc.SecretRef != nil { + keys = append(keys, fmt.Sprintf("%s/%s", namespace, kc.SecretRef.Name)) + } + for _, ref := range obj.Spec.ValuesFrom { + if ref.Kind == "Secret" { + keys = append(keys, fmt.Sprintf("%s/%s", namespace, ref.Name)) + } + } + return keys + }, + ); err != nil { + return err + } + + r.requeueDependency = opts.DependencyRequeueInterval + r.artifactFetchRetries = opts.HTTPRetry + + return ctrl.NewControllerManagedBy(mgr). + For(&v2.HelmRelease{}, builder.WithPredicates( + predicate.Or(predicate.GenerationChangedPredicate{}, predicates.ReconcileRequestedPredicate{}), + )). + Watches( + &sourcev1.HelmChart{}, + handler.EnqueueRequestsFromMapFunc(r.requestsForHelmChartChange), + builder.WithPredicates(intpredicates.SourceRevisionChangePredicate{}), + ). + Watches( + &sourcev1.OCIRepository{}, + handler.EnqueueRequestsFromMapFunc(r.requestsForOCIRepositoryChange), + builder.WithPredicates(intpredicates.SourceRevisionChangePredicate{}), + ). + WatchesMetadata( + &corev1.ConfigMap{}, + handler.EnqueueRequestsFromMapFunc(r.requestsForConfigDependency(indexConfigMap)), + builder.WithPredicates(predicate.ResourceVersionChangedPredicate{}, opts.WatchConfigsPredicate), + ). + WatchesMetadata( + &corev1.Secret{}, + handler.EnqueueRequestsFromMapFunc(r.requestsForConfigDependency(indexSecret)), + builder.WithPredicates(predicate.ResourceVersionChangedPredicate{}, opts.WatchConfigsPredicate), + ). + WithOptions(controller.Options{ + RateLimiter: opts.RateLimiter, + }). + Complete(r) +} From 9b6b090ca408d058fd0be169c44f594892ff32ab Mon Sep 17 00:00:00 2001 From: Stefan Prodan Date: Tue, 22 Jul 2025 22:06:12 +0300 Subject: [PATCH 3/5] controller: Add `AdditiveCELDependencyCheck` feature gate Signed-off-by: Stefan Prodan --- internal/features/features.go | 8 ++++++++ main.go | 9 ++++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/internal/features/features.go b/internal/features/features.go index 37eb9a863..de5bdb205 100644 --- a/internal/features/features.go +++ b/internal/features/features.go @@ -68,6 +68,11 @@ const ( // digest changes. When enabled, the controller will not // append the digest to the chart version in Chart.yaml. DisableChartDigestTracking = "DisableChartDigestTracking" + + // AdditiveCELDependencyCheck controls whether the CEL dependency check + // should be additive, meaning that the built-in readiness check will + // be added to the user-defined CEL expressions. + AdditiveCELDependencyCheck = "AdditiveCELDependencyCheck" ) var features = map[string]bool{ @@ -92,6 +97,9 @@ var features = map[string]bool{ // DisableChartDigestTracking // opt-in from v1.3.0 DisableChartDigestTracking: false, + // AdditiveCELDependencyCheck + // opt-in from v1.4.0 + AdditiveCELDependencyCheck: false, } func init() { diff --git a/main.go b/main.go index 90759048f..bb759c127 100644 --- a/main.go +++ b/main.go @@ -204,7 +204,13 @@ func main() { disableChartDigestTracking, err := features.Enabled(features.DisableChartDigestTracking) if err != nil { - setupLog.Error(err, "unable to check feature gate DisableChartDigestTracking") + setupLog.Error(err, "unable to check feature gate "+features.DisableChartDigestTracking) + os.Exit(1) + } + + additiveCELDependencyCheck, err := features.Enabled(features.AdditiveCELDependencyCheck) + if err != nil { + setupLog.Error(err, "unable to check feature gate "+features.AdditiveCELDependencyCheck) os.Exit(1) } @@ -318,6 +324,7 @@ func main() { KubeConfigOpts: kubeConfigOpts, FieldManager: controllerName, DisableChartDigestTracking: disableChartDigestTracking, + AdditiveCELDependencyCheck: additiveCELDependencyCheck, TokenCache: tokenCache, }).SetupWithManager(ctx, mgr, controller.HelmReleaseReconcilerOptions{ DependencyRequeueInterval: requeueDependency, From 32f20f73440759397466faa0444944dccdb15a5d Mon Sep 17 00:00:00 2001 From: Stefan Prodan Date: Tue, 22 Jul 2025 22:06:33 +0300 Subject: [PATCH 4/5] controller: Implement CEL evaluation for dependency checks Signed-off-by: Stefan Prodan --- go.mod | 6 + go.sum | 15 ++ internal/controller/helmrelease_controller.go | 107 ++++++++++++-- .../controller/helmrelease_controller_test.go | 130 ++++++++++++++++++ 4 files changed, 243 insertions(+), 15 deletions(-) diff --git a/go.mod b/go.mod index 673cf84d2..69ed9d4b8 100644 --- a/go.mod +++ b/go.mod @@ -30,6 +30,7 @@ require ( github.com/fluxcd/pkg/testserver v0.11.0 github.com/fluxcd/source-controller/api v1.6.0 github.com/go-logr/logr v1.4.3 + github.com/google/cel-go v0.23.2 github.com/google/go-cmp v0.7.0 github.com/hashicorp/go-retryablehttp v0.7.8 github.com/mitchellh/copystructure v1.2.0 @@ -54,6 +55,7 @@ require ( ) require ( + cel.dev/expr v0.23.0 // indirect cloud.google.com/go/auth v0.16.2 // indirect cloud.google.com/go/auth/oauth2adapt v0.2.8 // indirect cloud.google.com/go/compute/metadata v0.7.0 // indirect @@ -70,6 +72,7 @@ require ( github.com/Masterminds/goutils v1.1.1 // indirect github.com/Masterminds/sprig/v3 v3.3.0 // indirect github.com/Masterminds/squirrel v1.5.4 // indirect + github.com/antlr4-go/antlr/v4 v4.13.0 // indirect github.com/asaskevich/govalidator v0.0.0-20230301143203-a9d515a09cc2 // indirect github.com/aws/aws-sdk-go-v2 v1.36.5 // indirect github.com/aws/aws-sdk-go-v2/config v1.29.17 // indirect @@ -172,6 +175,7 @@ require ( github.com/sirupsen/logrus v1.9.3 // indirect github.com/spf13/cast v1.7.0 // indirect github.com/spf13/cobra v1.9.1 // indirect + github.com/stoewer/go-strcase v1.3.0 // indirect github.com/stretchr/testify v1.10.0 // indirect github.com/tidwall/gjson v1.18.0 // indirect github.com/tidwall/match v1.1.1 // indirect @@ -193,6 +197,7 @@ require ( go.yaml.in/yaml/v2 v2.4.2 // indirect go.yaml.in/yaml/v3 v3.0.4 // indirect golang.org/x/crypto v0.39.0 // indirect + golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56 // indirect golang.org/x/net v0.41.0 // indirect golang.org/x/oauth2 v0.30.0 // indirect golang.org/x/sync v0.16.0 // indirect @@ -201,6 +206,7 @@ require ( golang.org/x/time v0.12.0 // indirect gomodules.xyz/jsonpatch/v2 v2.5.0 // indirect google.golang.org/api v0.241.0 // indirect + google.golang.org/genproto/googleapis/api v0.0.0-20250505200425-f936aa4a68b2 // indirect google.golang.org/genproto/googleapis/rpc v0.0.0-20250603155806-513f23925822 // indirect google.golang.org/grpc v1.73.0 // indirect google.golang.org/protobuf v1.36.6 // indirect diff --git a/go.sum b/go.sum index 613328340..f7395e6cf 100644 --- a/go.sum +++ b/go.sum @@ -1,3 +1,5 @@ +cel.dev/expr v0.23.0 h1:wUb94w6OYQS4uXraxo9U+wUAs9jT47Xvl4iPgAwM2ss= +cel.dev/expr v0.23.0/go.mod h1:hLPLo1W4QUmuYdA72RBX06QTs6MXw941piREPl3Yfiw= cloud.google.com/go/auth v0.16.2 h1:QvBAGFPLrDeoiNjyfVunhQ10HKNYuOwZ5noee0M5df4= cloud.google.com/go/auth v0.16.2/go.mod h1:sRBas2Y1fB1vZTdurouM0AzuYQBMZinrUYL8EufhtEA= cloud.google.com/go/auth/oauth2adapt v0.2.8 h1:keo8NaayQZ6wimpNSmW5OPc283g65QNIiLpZnkHRbnc= @@ -42,6 +44,8 @@ github.com/Masterminds/sprig/v3 v3.3.0 h1:mQh0Yrg1XPo6vjYXgtf5OtijNAKJRNcTdOOGZe github.com/Masterminds/sprig/v3 v3.3.0/go.mod h1:Zy1iXRYNqNLUolqCpL4uhk6SHUMAOSCzdgBfDb35Lz0= github.com/Masterminds/squirrel v1.5.4 h1:uUcX/aBc8O7Fg9kaISIUsHXdKuqehiXAMQTYX8afzqM= github.com/Masterminds/squirrel v1.5.4/go.mod h1:NNaOrjSoIDfDA40n7sr2tPNZRfjzjA400rg+riTZj10= +github.com/antlr4-go/antlr/v4 v4.13.0 h1:lxCg3LAv+EUK6t1i0y1V6/SLeUi0eKEKdhQAlS8TVTI= +github.com/antlr4-go/antlr/v4 v4.13.0/go.mod h1:pfChB/xh/Unjila75QW7+VU4TSnWnnk9UTnmpPaOR2g= github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5 h1:0CwZNZbxp69SHPdPJAN/hZIm0C4OItdklCFmMRWYpio= github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5/go.mod h1:wHh0iHkYZB8zMSxRWpUBQtwG5a7fFgvEO+odwuTv2gs= github.com/asaskevich/govalidator v0.0.0-20230301143203-a9d515a09cc2 h1:DklsrG3dyBCFEj5IhUbnKptjxatkF07cF2ak3yi77so= @@ -205,6 +209,8 @@ github.com/golang/protobuf v1.5.4 h1:i7eJL8qZTpSEXOPTxNKhASYpMn+8e5Q6AdndVa1dWek github.com/golang/protobuf v1.5.4/go.mod h1:lnTiLA8Wa4RWRcIUkrtSVa5nRhsEGBg48fD6rSs7xps= github.com/google/btree v1.1.3 h1:CVpQJjYgC4VbzxeGVHfvZrv1ctoYCAI8vbl07Fcxlyg= github.com/google/btree v1.1.3/go.mod h1:qOPhT0dTNdNzV6Z/lhRX0YXUafgPLFUh+gZMl761Gm4= +github.com/google/cel-go v0.23.2 h1:UdEe3CvQh3Nv+E/j9r1Y//WO0K0cSyD7/y0bzyLIMI4= +github.com/google/cel-go v0.23.2/go.mod h1:52Pb6QsDbC5kvgxvZhiL9QX1oZEkcUF/ZqaPx1J5Wwo= github.com/google/gnostic-models v0.7.0 h1:qwTtogB15McXDaNqTZdzPJRHvaVJlAl+HVQnLmJEJxo= github.com/google/gnostic-models v0.7.0/go.mod h1:whL5G0m6dmc5cPxKc5bdKdEN3UjI7OUGxBlw57miDrQ= github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= @@ -378,13 +384,20 @@ github.com/spf13/cobra v1.9.1 h1:CXSaggrXdbHK9CF+8ywj8Amf7PBRmPCOJugH954Nnlo= github.com/spf13/cobra v1.9.1/go.mod h1:nDyEzZ8ogv936Cinf6g1RU9MRY64Ir93oCnqb9wxYW0= github.com/spf13/pflag v1.0.6 h1:jFzHGLGAlb3ruxLB8MhbI6A8+AQX/2eW4qeyNZXNp2o= github.com/spf13/pflag v1.0.6/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= +github.com/stoewer/go-strcase v1.3.0 h1:g0eASXYtp+yvN9fK8sH94oCIk0fau9uV1/ZdJ0AVEzs= +github.com/stoewer/go-strcase v1.3.0/go.mod h1:fAH5hQ5pehh+j3nZfvwdk2RgEgQjAoM8wodgtPmh1xo= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= +github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= +github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo= github.com/stretchr/objx v0.5.2 h1:xuMeJ0Sdp5ZMRXx/aWO6RZxdr3beISkG5/G/aIRr3pY= github.com/stretchr/objx v0.5.2/go.mod h1:FRsXN1f5AsAjCGJKqEizvkpNtU+EGNCLh3NxZ/8L+MA= github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= +github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= +github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= +github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= github.com/stretchr/testify v1.10.0 h1:Xv5erBjTwe/5IxqUQTdXv5kgmIvbHo3QQyRwhJsOfJA= github.com/stretchr/testify v1.10.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= github.com/tidwall/gjson v1.14.2/go.mod h1:/wbyibRr2FHMks5tjHJ5F8dMZh3AcwJEMf5vlfC0lxk= @@ -481,6 +494,8 @@ golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8U golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= golang.org/x/crypto v0.39.0 h1:SHs+kF4LP+f+p14esP5jAoDpHU8Gu/v9lFRK6IT5imM= golang.org/x/crypto v0.39.0/go.mod h1:L+Xg3Wf6HoL4Bn4238Z6ft6KfEpN0tJGo53AAPC632U= +golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56 h1:2dVuKD2vS7b0QIHQbpyTISPd0LeHDbnYEryqj5Q1ug8= +golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56/go.mod h1:M4RDyNAINzryxdtnbRXRL/OHtkFuWGRjvuhBJpk2IlY= golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.25.0 h1:n7a+ZbQKQA/Ysbyb0/6IbB1H/X41mKgbhfv7AfG/44w= diff --git a/internal/controller/helmrelease_controller.go b/internal/controller/helmrelease_controller.go index 5371c07f5..123be24c6 100644 --- a/internal/controller/helmrelease_controller.go +++ b/internal/controller/helmrelease_controller.go @@ -23,10 +23,13 @@ import ( "strings" "time" + "github.com/fluxcd/pkg/runtime/cel" + celtypes "github.com/google/cel-go/common/types" "helm.sh/helm/v3/pkg/chart" corev1 "k8s.io/api/core/v1" apiequality "k8s.io/apimachinery/pkg/api/equality" apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" apierrutil "k8s.io/apimachinery/pkg/util/errors" "k8s.io/apimachinery/pkg/util/wait" @@ -93,6 +96,7 @@ type HelmReleaseReconciler struct { FieldManager string DefaultServiceAccount string DisableChartDigestTracking bool + AdditiveCELDependencyCheck bool requeueDependency time.Duration artifactFetchRetries int @@ -205,6 +209,17 @@ func (r *HelmReleaseReconciler) reconcileRelease(ctx context.Context, patchHelpe log.Info(fmt.Sprintf("checking %d dependencies", c)) if err := r.checkDependencies(ctx, obj); err != nil { + // Check if this is a terminal error that should not trigger retries + if errors.Is(err, reconcile.TerminalError(nil)) { + const terminalErrorMessage = "Reconciliation failed terminally due to configuration error" + errMsg := fmt.Sprintf("%s: %v", terminalErrorMessage, err) + conditions.MarkFalse(obj, meta.ReadyCondition, meta.InvalidCELExpressionReason, "%s", errMsg) + conditions.MarkStalled(obj, meta.InvalidCELExpressionReason, "%s", errMsg) + r.Eventf(obj, corev1.EventTypeWarning, meta.InvalidCELExpressionReason, err.Error()) + return ctrl.Result{}, err + } + + // Retry on transient errors. msg := fmt.Sprintf("dependencies do not meet ready condition (%s): retrying in %s", err.Error(), r.requeueDependency.String()) conditions.MarkFalse(obj, meta.ReadyCondition, v2.DependencyNotReadyReason, "%s", err) @@ -534,32 +549,94 @@ func (r *HelmReleaseReconciler) reconcileUninstall(ctx context.Context, getter g return intreconcile.NewUninstall(cfg, r.EventRecorder).Reconcile(ctx, &intreconcile.Request{Object: obj}) } -// checkDependencies checks if the dependencies of the given v2.HelmRelease -// are Ready. -// It returns an error if a dependency can not be retrieved or is not Ready, -// otherwise nil. +// checkDependencies checks if the dependencies of the current HelmRelease are ready. +// To be considered ready, a dependencies must meet the following criteria: +// - The dependency exists in the API server. +// - The CEL expression (if provided) must evaluate to true. +// - The dependency observed generation must match the current generation. +// - The dependency Ready condition must be true. func (r *HelmReleaseReconciler) checkDependencies(ctx context.Context, obj *v2.HelmRelease) error { - for _, d := range obj.Spec.DependsOn { - ref := types.NamespacedName{ - Namespace: d.Namespace, - Name: d.Name, + // Convert the HelmRelease object to Unstructured for CEL evaluation. + objMap, err := runtime.DefaultUnstructuredConverter.ToUnstructured(obj) + if err != nil { + return fmt.Errorf("failed to convert HelmRelease to unstructured: %w", err) + } + + for _, depRef := range obj.Spec.DependsOn { + depName := types.NamespacedName{ + Namespace: depRef.Namespace, + Name: depRef.Name, + } + if depName.Namespace == "" { + depName.Namespace = obj.GetNamespace() } - if ref.Namespace == "" { - ref.Namespace = obj.GetNamespace() + + // Check if the dependency exists by querying + // the API server bypassing the cache. + dep := &v2.HelmRelease{} + if err := r.APIReader.Get(ctx, depName, dep); err != nil { + return fmt.Errorf("unable to get '%s' dependency: %w", depName, err) + } + + // Evaluate the CEL expression (if specified) to determine if the dependency is ready. + if depRef.ReadyExpr != "" { + ready, err := r.evalReadyExpr(ctx, depRef.ReadyExpr, objMap, dep) + if err != nil { + return err + } + if !ready { + return fmt.Errorf("dependency '%s' is not ready according to readyExpr eval", depName) + } } - dHr := &v2.HelmRelease{} - if err := r.APIReader.Get(ctx, ref, dHr); err != nil { - return fmt.Errorf("unable to get '%s' dependency: %w", ref, err) + // Skip the built-in readiness check if the CEL expression is provided + // and the AdditiveCELDependencyCheck feature gate is not enabled. + if depRef.ReadyExpr != "" && !r.AdditiveCELDependencyCheck { + continue } - if dHr.Generation != dHr.Status.ObservedGeneration || !conditions.IsTrue(dHr, meta.ReadyCondition) { - return fmt.Errorf("dependency '%s' is not ready", ref) + // Check if the dependency observed generation is up to date + // and if the dependency is in a ready state. + if dep.Generation != dep.Status.ObservedGeneration || !conditions.IsTrue(dep, meta.ReadyCondition) { + return fmt.Errorf("dependency '%s' is not ready", depName) } } return nil } +// evalReadyExpr evaluates the CEL expression for the dependency readiness check. +func (r *HelmReleaseReconciler) evalReadyExpr( + ctx context.Context, + expr string, + selfMap map[string]any, + dep *v2.HelmRelease, +) (bool, error) { + const ( + selfName = "self" + depName = "dep" + ) + + celExpr, err := cel.NewExpression(expr, + cel.WithCompile(), + cel.WithOutputType(celtypes.BoolType), + cel.WithStructVariables(selfName, depName)) + if err != nil { + return false, reconcile.TerminalError(fmt.Errorf("failed to evaluate dependency %s: %w", dep.Name, err)) + } + + depMap, err := runtime.DefaultUnstructuredConverter.ToUnstructured(dep) + if err != nil { + return false, fmt.Errorf("failed to convert %s object to map: %w", depName, err) + } + + vars := map[string]any{ + selfName: selfMap, + depName: depMap, + } + + return celExpr.EvaluateBoolean(ctx, vars) +} + // adoptLegacyRelease attempts to adopt a v2beta1 release into a v2 // release. // This is done by retrieving the last successful release from the Helm storage diff --git a/internal/controller/helmrelease_controller_test.go b/internal/controller/helmrelease_controller_test.go index d559a909c..40e333dd8 100644 --- a/internal/controller/helmrelease_controller_test.go +++ b/internal/controller/helmrelease_controller_test.go @@ -2861,6 +2861,69 @@ func TestHelmReleaseReconciler_checkDependencies(t *testing.T) { g.Expect(err).ToNot(HaveOccurred()) }, }, + { + name: "all dependencies ready with readyExpr", + obj: &v2.HelmRelease{ + ObjectMeta: metav1.ObjectMeta{ + Name: "dependant", + Namespace: "some-namespace", + Labels: map[string]string{ + "app/version": "v1.2.3", + }, + }, + Spec: v2.HelmReleaseSpec{ + Values: &apiextensionsv1.JSON{ + Raw: []byte(`{"version":"v1.2.3"}`), + }, + DependsOn: []v2.DependencyReference{ + { + Name: "dependency-1", + ReadyExpr: "self.spec.values.version == dep.spec.values.version", + }, + { + Name: "dependency-2", + ReadyExpr: ` +dep.metadata.labels['app/version'] == self.metadata.labels['app/version'] && +dep.status.conditions.filter(e, e.type == 'Ready').all(e, e.status == 'True') && +dep.metadata.generation == dep.status.observedGeneration +`, + }, + }, + }, + }, + objects: []client.Object{ + &v2.HelmRelease{ + ObjectMeta: metav1.ObjectMeta{ + Name: "dependency-1", + Namespace: "some-namespace", + }, + Spec: v2.HelmReleaseSpec{ + Values: &apiextensionsv1.JSON{ + Raw: []byte(`{"version":"v1.2.3"}`), + }, + }, + }, + &v2.HelmRelease{ + ObjectMeta: metav1.ObjectMeta{ + Generation: 2, + Name: "dependency-2", + Namespace: "some-namespace", + Labels: map[string]string{ + "app/version": "v1.2.3", + }, + }, + Status: v2.HelmReleaseStatus{ + ObservedGeneration: 2, + Conditions: []metav1.Condition{ + {Type: meta.ReadyCondition, Status: metav1.ConditionTrue}, + }, + }, + }, + }, + expect: func(g *WithT, err error) { + g.Expect(err).ToNot(HaveOccurred()) + }, + }, { name: "error on dependency with ObservedGeneration < Generation", obj: &v2.HelmRelease{ @@ -2983,6 +3046,73 @@ func TestHelmReleaseReconciler_checkDependencies(t *testing.T) { g.Expect(apierrors.IsNotFound(err)).To(BeTrue()) }, }, + { + name: "error on dependency with readyExpr", + obj: &v2.HelmRelease{ + ObjectMeta: metav1.ObjectMeta{ + Name: "dependant", + Namespace: "some-namespace", + }, + Spec: v2.HelmReleaseSpec{ + DependsOn: []v2.DependencyReference{ + { + Name: "dependency-1", + ReadyExpr: "self.metadata.name == dep.metadata.name", + }, + }, + }, + }, + objects: []client.Object{ + &v2.HelmRelease{ + ObjectMeta: metav1.ObjectMeta{ + Generation: 1, + Name: "dependency-1", + Namespace: "some-namespace", + }, + Status: v2.HelmReleaseStatus{ + ObservedGeneration: 1, + }, + }, + }, + expect: func(g *WithT, err error) { + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(ContainSubstring("is not ready according to readyExpr eval")) + }, + }, + { + name: "terminal error on dependency with invalid readyExpr", + obj: &v2.HelmRelease{ + ObjectMeta: metav1.ObjectMeta{ + Name: "dependant", + Namespace: "some-namespace", + }, + Spec: v2.HelmReleaseSpec{ + DependsOn: []v2.DependencyReference{ + { + Name: "dependency-1", + ReadyExpr: "self.generation == deps.generation", + }, + }, + }, + }, + objects: []client.Object{ + &v2.HelmRelease{ + ObjectMeta: metav1.ObjectMeta{ + Generation: 1, + Name: "dependency-1", + Namespace: "some-namespace", + }, + Status: v2.HelmReleaseStatus{ + ObservedGeneration: 1, + }, + }, + }, + expect: func(g *WithT, err error) { + g.Expect(err).To(HaveOccurred()) + g.Expect(errors.Is(err, reconcile.TerminalError(nil))).To(BeTrue()) + g.Expect(err.Error()).To(ContainSubstring("failed to parse")) + }, + }, } for _, tt := range tests { From 40d128a05d67407b8f43eaa5bf04910ec02184e6 Mon Sep 17 00:00:00 2001 From: Stefan Prodan Date: Tue, 22 Jul 2025 22:17:24 +0300 Subject: [PATCH 5/5] docs: Add dependency ready expression to API docs Signed-off-by: Stefan Prodan --- docs/spec/v2/helmreleases.md | 55 +++++++++++++++++++++++++++++++++--- 1 file changed, 51 insertions(+), 4 deletions(-) diff --git a/docs/spec/v2/helmreleases.md b/docs/spec/v2/helmreleases.md index 86365069a..630aca61d 100644 --- a/docs/spec/v2/helmreleases.md +++ b/docs/spec/v2/helmreleases.md @@ -396,12 +396,59 @@ spec: - name: backend ``` -**Note:** This does not account for upgrade ordering. Kubernetes only allows -applying one resource (HelmRelease in this case) at a time, so there is no -way for the controller to know when a dependency HelmRelease may be updated. -Also, circular dependencies between HelmRelease resources must be avoided, +**Note:** Circular dependencies between HelmRelease resources must be avoided, otherwise the interdependent HelmRelease resources will never be reconciled. +#### Dependency Ready Expression + +`.spec.dependsOn[].readyExpr` is an optional field that can be used to define a CEL expression +to determine the readiness of a HelmRelease dependency. + +This is helpful for when custom logic is needed to determine if a dependency is ready. +For example, when performing a lockstep upgrade, the `readyExpr` can be used to +verify that a dependency has a matching version in values before proceeding with the +reconciliation of the dependent HelmRelease. + +```yaml +apiVersion: helm.toolkit.fluxcd.io/v2 +kind: HelmRelease +metadata: + name: backend + namespace: default +spec: + # ...omitted for brevity + values: + app: + version: v1.2.3 +--- +apiVersion: helm.toolkit.fluxcd.io/v2 +kind: HelmRelease +metadata: + name: frontend + namespace: default +spec: + # ...omitted for brevity + values: + app: + version: v1.2.3 + dependsOn: + - name: backend + readyExpr: > + dep.spec.values.app.version == self.spec.values.app.version && + dep.status.conditions.filter(e, e.type == 'Ready').all(e, e.status == 'True') && + dep.metadata.generation == dep.status.observedGeneration +``` + +The CEL expression contains the following variables: + +- `dep`: The dependency HelmRelease object being evaluated. +- `self`: The HelmRelease object being reconciled. + +**Note:** When `readyExpr` is specified, the built-in readiness check is replaced by the logic +defined in the CEL expression. You can configure the controller to run both the CEL expression +evaluation and the built-in readiness check, with the `AdditiveCELDependencyCheck` +[feature gate](https://fluxcd.io/flux/components/helm/options/#feature-gates). + ### Values The values for the Helm release can be specified in two ways: