diff --git a/.github/workflows/e2e.yaml b/.github/workflows/e2e.yaml index eb12098d5..d44ac70fb 100644 --- a/.github/workflows/e2e.yaml +++ b/.github/workflows/e2e.yaml @@ -138,6 +138,11 @@ jobs: kubectl -n install-create-target-ns get deployment install-create-target-ns-install-create-target-ns-podinfo kubectl -n helm-system delete -f config/testdata/install-create-target-ns + - name: Run install from ocirepo test + run: | + kubectl -n helm-system apply -f config/testdata/install-from-ocirepo-source + kubectl -n helm-system wait helmreleases/podinfo-from-ocirepo --for=condition=ready --timeout=4m + kubectl -n helm-system delete -f config/testdata/install-from-ocirepo-source - name: Run install fail test run: | test_name=install-fail @@ -458,6 +463,45 @@ jobs: fi kubectl delete -n helm-system -f config/testdata/$test_name/install.yaml + - name: Run upgrade from ocirepo source + run: | + test_name=upgrade-from-ocirepo-source + kubectl -n helm-system apply -f config/testdata/$test_name/install.yaml + echo -n ">>> Waiting for expected conditions" + count=0 + until [ 'true' == "$( kubectl -n helm-system get helmrelease/$test_name -o json | jq '.status.conditions | map( { (.type): .status } ) | add | .Released=="True" and .Ready=="True"' )" ]; do + echo -n '.' + sleep 5 + count=$((count + 1)) + if [[ ${count} -eq 24 ]]; then + echo ' No more retries left!' + exit 1 + fi + done + echo ' done' + + # Validate release was installed. + REVISION_COUNT=$(helm -n helm-system history -o json $test_name | jq 'length') + if [ "$REVISION_COUNT" != 1 ]; then + echo -e "Unexpected revision count: $REVISION_COUNT" + exit 1 + fi + + kubectl -n helm-system apply -f config/testdata/$test_name/upgrade.yaml + echo -n ">>> Waiting for expected conditions" + count=0 + until [ 'true' == "$( kubectl -n helm-system get helmrelease/$test_name -o json | jq '.status.conditions | map( { (.type): .status } ) | add | .Released=="True" and .Ready=="True"' )" ]; do + echo -n '.' + sleep 5 + count=$((count + 1)) + if [[ ${count} -eq 24 ]]; then + echo ' No more retries left!' + exit 1 + fi + done + echo ' done' + + kubectl delete -n helm-system -f config/testdata/$test_name/install.yaml - name: Run upgrade fail with uninstall remediation strategy test run: | test_name=upgrade-fail-remediate-uninstall diff --git a/api/v2beta2/helmrelease_types.go b/api/v2beta2/helmrelease_types.go index 2c280849e..9db38102f 100644 --- a/api/v2beta2/helmrelease_types.go +++ b/api/v2beta2/helmrelease_types.go @@ -74,11 +74,17 @@ type PostRenderer struct { } // HelmReleaseSpec defines the desired state of a Helm release. +// +kubebuilder:validation:XValidation:rule="(has(self.chart) && !has(self.chartRef)) || (!has(self.chart) && has(self.chartRef))", message="either chart or chartRef must be set" type HelmReleaseSpec struct { // Chart defines the template of the v1beta2.HelmChart that should be created // for this HelmRelease. - // +required - Chart HelmChartTemplate `json:"chart"` + // +optional + Chart HelmChartTemplate `json:"chart,omitempty"` + + // ChartRef holds a reference to a source controller resource containing the + // Helm chart artifact. + // +optional + ChartRef *CrossNamespaceSourceReference `json:"chartRef,omitempty"` // Interval at which to reconcile the Helm release. // +kubebuilder:validation:Type=string @@ -996,10 +1002,16 @@ type HelmReleaseStatus struct { LastAppliedRevision string `json:"lastAppliedRevision,omitempty"` // LastAttemptedRevision is the Source revision of the last reconciliation - // attempt. + // attempt. For OCIRepository sources, the 12 first characters of the digest are + // appended to the chart version e.g. "1.2.3+1234567890ab". // +optional LastAttemptedRevision string `json:"lastAttemptedRevision,omitempty"` + // LastAttemptedRevisionDigest is the digest of the last reconciliation attempt. + // This is only set for OCIRepository sources. + // +optional + LastAttemptedRevisionDigest string `json:"lastAttemptedRevisionDigest,omitempty"` + // LastAttemptedValuesChecksum is the SHA1 checksum for the values of the last // reconciliation attempt. // Deprecated: Use LastAttemptedConfigDigest instead. @@ -1052,6 +1064,10 @@ func (in HelmReleaseStatus) GetHelmChart() (string, string) { return "", "" } +func (in *HelmReleaseStatus) GetLastAttemptedRevision() string { + return in.LastAttemptedRevision +} + const ( // SourceIndexKey is the key used for indexing HelmReleases based on // their sources. @@ -1241,6 +1257,16 @@ func (in *HelmRelease) GetStatusConditions() *[]metav1.Condition { return &in.Status.Conditions } +// IsChartRefPresent returns true if the HelmRelease has a ChartRef. +func (in *HelmRelease) HasChartRef() bool { + return in.Spec.ChartRef != nil +} + +// IsChartTemplatePresent returns true if the HelmRelease has a ChartTemplate. +func (in *HelmRelease) HasChartTemplate() bool { + return in.Spec.Chart.Spec.Chart != "" +} + // +kubebuilder:object:root=true // HelmReleaseList contains a list of HelmRelease objects. diff --git a/api/v2beta2/reference_types.go b/api/v2beta2/reference_types.go index 4c899fe5d..344e5038f 100644 --- a/api/v2beta2/reference_types.go +++ b/api/v2beta2/reference_types.go @@ -42,6 +42,33 @@ type CrossNamespaceObjectReference struct { Namespace string `json:"namespace,omitempty"` } +// CrossNamespaceSourceReference contains enough information to let you locate +// the typed referenced object at cluster level. +type CrossNamespaceSourceReference struct { + // APIVersion of the referent. + // +optional + APIVersion string `json:"apiVersion,omitempty"` + + // Kind of the referent. + // +kubebuilder:validation:Enum=OCIRepository + // +required + Kind string `json:"kind"` + + // Name of the referent. + // +kubebuilder:validation:MinLength=1 + // +kubebuilder:validation:MaxLength=253 + // +required + Name string `json:"name"` + + // Namespace of the referent, defaults to the namespace of the Kubernetes + // resource object that contains the reference. + // +kubebuilder:validation:MinLength=1 + // +kubebuilder:validation:MaxLength=63 + // +kubebuilder:validation:Optional + // +optional + Namespace string `json:"namespace,omitempty"` +} + // ValuesReference contains a reference to a resource containing Helm values, // and optionally the key they can be found at. type ValuesReference struct { diff --git a/api/v2beta2/snapshot_types.go b/api/v2beta2/snapshot_types.go index 587667665..ca7589a32 100644 --- a/api/v2beta2/snapshot_types.go +++ b/api/v2beta2/snapshot_types.go @@ -156,6 +156,9 @@ type Snapshot struct { // run by the controller. // +optional TestHooks *map[string]*TestHookStatus `json:"testHooks,omitempty"` + // OCIDigest is the digest of the OCI artifact associated with the release. + // +optional + OCIDigest string `json:"ociDigest,omitempty"` } // FullReleaseName returns the full name of the release in the format diff --git a/api/v2beta2/zz_generated.deepcopy.go b/api/v2beta2/zz_generated.deepcopy.go index c9db4f094..7d7f3200a 100644 --- a/api/v2beta2/zz_generated.deepcopy.go +++ b/api/v2beta2/zz_generated.deepcopy.go @@ -43,6 +43,21 @@ func (in *CrossNamespaceObjectReference) DeepCopy() *CrossNamespaceObjectReferen return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *CrossNamespaceSourceReference) DeepCopyInto(out *CrossNamespaceSourceReference) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new CrossNamespaceSourceReference. +func (in *CrossNamespaceSourceReference) DeepCopy() *CrossNamespaceSourceReference { + if in == nil { + return nil + } + out := new(CrossNamespaceSourceReference) + 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 @@ -244,6 +259,11 @@ func (in *HelmReleaseList) DeepCopyObject() runtime.Object { func (in *HelmReleaseSpec) DeepCopyInto(out *HelmReleaseSpec) { *out = *in in.Chart.DeepCopyInto(&out.Chart) + if in.ChartRef != nil { + in, out := &in.ChartRef, &out.ChartRef + *out = new(CrossNamespaceSourceReference) + **out = **in + } out.Interval = in.Interval if in.KubeConfig != nil { in, out := &in.KubeConfig, &out.KubeConfig diff --git a/config/crd/bases/helm.toolkit.fluxcd.io_helmreleases.yaml b/config/crd/bases/helm.toolkit.fluxcd.io_helmreleases.yaml index 55ab127b9..e38f8922e 100644 --- a/config/crd/bases/helm.toolkit.fluxcd.io_helmreleases.yaml +++ b/config/crd/bases/helm.toolkit.fluxcd.io_helmreleases.yaml @@ -1100,6 +1100,10 @@ spec: description: Namespace is the namespace the release is deployed to. type: string + ociDigest: + description: OCIDigest is the digest of the OCI artifact associated + with the release. + type: string status: description: Status is the current state of the release. type: string @@ -1420,6 +1424,35 @@ spec: required: - spec type: object + chartRef: + description: |- + ChartRef holds a reference to a source controller resource containing the + Helm chart artifact. + properties: + apiVersion: + description: APIVersion of the referent. + type: string + kind: + description: Kind of the referent. + enum: + - OCIRepository + type: string + name: + description: Name of the referent. + maxLength: 253 + minLength: 1 + type: string + namespace: + description: |- + Namespace of the referent, defaults to the namespace of the Kubernetes + resource object that contains the reference. + maxLength: 63 + minLength: 1 + type: string + required: + - kind + - name + type: object dependsOn: description: |- DependsOn may contain a meta.NamespacedObjectReference slice with @@ -2199,9 +2232,12 @@ spec: type: object type: array required: - - chart - interval type: object + x-kubernetes-validations: + - message: either chart or chartRef must be set + rule: (has(self.chart) && !has(self.chartRef)) || (!has(self.chart) + && has(self.chartRef)) status: default: observedGeneration: -1 @@ -2342,6 +2378,10 @@ spec: description: Namespace is the namespace the release is deployed to. type: string + ociDigest: + description: OCIDigest is the digest of the OCI artifact associated + with the release. + type: string status: description: Status is the current state of the release. type: string @@ -2420,7 +2460,13 @@ spec: lastAttemptedRevision: description: |- LastAttemptedRevision is the Source revision of the last reconciliation - attempt. + attempt. For OCIRepository sources, the 12 first characters of the digest are + appended to the chart version e.g. "1.2.3+1234567890ab". + type: string + lastAttemptedRevisionDigest: + description: |- + LastAttemptedRevisionDigest is the digest of the last reconciliation attempt. + This is only set for OCIRepository sources. type: string lastAttemptedValuesChecksum: description: |- diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index 954df8e75..52b74ee1f 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -55,3 +55,17 @@ rules: - helmcharts/status verbs: - get +- apiGroups: + - source.toolkit.fluxcd.io + resources: + - ocirepositories + verbs: + - get + - list + - watch +- apiGroups: + - source.toolkit.fluxcd.io + resources: + - ocirepositories/status + verbs: + - get diff --git a/config/testdata/install-from-ocirepo-source/test.yaml b/config/testdata/install-from-ocirepo-source/test.yaml new file mode 100644 index 000000000..f7f75dd80 --- /dev/null +++ b/config/testdata/install-from-ocirepo-source/test.yaml @@ -0,0 +1,25 @@ +--- +apiVersion: source.toolkit.fluxcd.io/v1beta2 +kind: OCIRepository +metadata: + name: podinfo-ocirepo +spec: + interval: 30s + url: oci://ghcr.io/stefanprodan/charts/podinfo + ref: + tag: 6.6.0 +--- +apiVersion: helm.toolkit.fluxcd.io/v2beta2 +kind: HelmRelease +metadata: + name: podinfo-from-ocirepo +spec: + chartRef: + kind: OCIRepository + name: podinfo-ocirepo + interval: 30s + values: + resources: + requests: + cpu: 100m + memory: 64Mi diff --git a/config/testdata/upgrade-from-ocirepo-source/install.yaml b/config/testdata/upgrade-from-ocirepo-source/install.yaml new file mode 100644 index 000000000..18430c96f --- /dev/null +++ b/config/testdata/upgrade-from-ocirepo-source/install.yaml @@ -0,0 +1,25 @@ +--- +apiVersion: source.toolkit.fluxcd.io/v1beta2 +kind: OCIRepository +metadata: + name: upgrade-from-ocirepo-source +spec: + interval: 30s + url: oci://ghcr.io/stefanprodan/charts/podinfo + ref: + digest: "sha256:cdd538a0167e4b51152b71a477e51eb6737553510ce8797dbcc537e1342311bb" +--- +apiVersion: helm.toolkit.fluxcd.io/v2beta2 +kind: HelmRelease +metadata: + name: upgrade-from-ocirepo-source +spec: + chartRef: + kind: OCIRepository + name: upgrade-from-ocirepo-source + interval: 30s + values: + resources: + requests: + cpu: 100m + memory: 64Mi diff --git a/config/testdata/upgrade-from-ocirepo-source/upgrade.yaml b/config/testdata/upgrade-from-ocirepo-source/upgrade.yaml new file mode 100644 index 000000000..8478426a0 --- /dev/null +++ b/config/testdata/upgrade-from-ocirepo-source/upgrade.yaml @@ -0,0 +1,10 @@ +--- +apiVersion: source.toolkit.fluxcd.io/v1beta2 +kind: OCIRepository +metadata: + name: upgrade-from-ocirepo-source +spec: + interval: 30s + url: oci://ghcr.io/stefanprodan/charts/podinfo + ref: + digest: "sha256:0cc9a8446c95009ef382f5eade883a67c257f77d50f84e78ecef2aac9428d1e5" diff --git a/docs/api/v2beta2/helm.md b/docs/api/v2beta2/helm.md index 840a1ca2d..020813053 100644 --- a/docs/api/v2beta2/helm.md +++ b/docs/api/v2beta2/helm.md @@ -78,12 +78,28 @@ HelmChartTemplate +(Optional)

Chart defines the template of the v1beta2.HelmChart that should be created for this HelmRelease.

+chartRef
+ + +CrossNamespaceSourceReference + + + + +(Optional) +

ChartRef holds a reference to a source controller resource containing the +Helm chart artifact.

+ + + + interval
@@ -469,6 +485,75 @@ string +

CrossNamespaceSourceReference +

+

+(Appears on: +HelmReleaseSpec) +

+

CrossNamespaceSourceReference contains enough information to let you locate +the typed referenced object at cluster level.

+
+
+ + + + + + + + + + + + + + + + + + + + + + + + + +
FieldDescription
+apiVersion
+ +string + +
+(Optional) +

APIVersion of the referent.

+
+kind
+ +string + +
+

Kind of the referent.

+
+name
+ +string + +
+

Name of the referent.

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

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

+
+
+

DriftDetection

@@ -1009,12 +1094,28 @@ HelmChartTemplate +(Optional)

Chart defines the template of the v1beta2.HelmChart that should be created for this HelmRelease.

+chartRef
+ + +CrossNamespaceSourceReference + + + + +(Optional) +

ChartRef holds a reference to a source controller resource containing the +Helm chart artifact.

+ + + + interval
@@ -1483,7 +1584,21 @@ string (Optional)

LastAttemptedRevision is the Source revision of the last reconciliation -attempt.

+attempt. For OCIRepository sources, the 12 first characters of the digest are +appended to the chart version e.g. “1.2.3+1234567890ab”.

+ + + + +lastAttemptedRevisionDigest
+ +string + + + +(Optional) +

LastAttemptedRevisionDigest is the digest of the last reconciliation attempt. +This is only set for OCIRepository sources.

@@ -2278,6 +2393,18 @@ TestHookStatus run by the controller.

+ + +ociDigest
+ +string + + + +(Optional) +

OCIDigest is the digest of the OCI artifact associated with the release.

+ + diff --git a/docs/spec/v2beta2/helmreleases.md b/docs/spec/v2beta2/helmreleases.md index b37bbb25a..02f475145 100644 --- a/docs/spec/v2beta2/helmreleases.md +++ b/docs/spec/v2beta2/helmreleases.md @@ -176,7 +176,7 @@ A HelmRelease also needs a ### Chart template -`.spec.chart` is a required field used by the helm-controller as a template to +`.spec.chart` is an optional field used by the helm-controller as a template to create a new [HelmChart resource](https://fluxcd.io/flux/components/source/helmcharts/). The spec for the HelmChart is provided via `.spec.chart.spec`, refer to @@ -203,6 +203,55 @@ references with the `--no-cross-namespace-refs=true` flag. When this flag is set, the HelmRelease can only refer to Sources in the same namespace as the HelmRelease object. +### Chart reference + +`.spec.chartRef` is an optional field used to refer to an [OCIRepository resource](https://fluxcd.io/flux/components/source/ocirepositories/) +from which to fetch the Helm chart. The chart is fetched by the controller with the +information provided by `.status.artifact` of the OCIRepository. + +The chart version of the last release attempt is reported in `.status.lastAttemptedRevision`. +The version is in the format `+`. The digest of the OCI artifact +is appended to the version to ensure that a change in the artifact content triggers +a new release. The controller will automatically perform a Helm upgrade when the +OCIRepository detects a new digest in the OCI artifact stored in registry, even if +the version inside `Chart.yaml` is unchanged. + +**Warning:** One of `.spec.chart` or `.spec.chartRef` must be set, but not both. +When switching from `.spec.chart` to `.spec.chartRef`, the controller will perform +an Helm upgrade and will garbage collect the old HelmChart object. + +**Note:** On multi-tenant clusters, platform admins can disable cross-namespace +references with the `--no-cross-namespace-refs=true` controller flag. When this flag is +set, the HelmRelease can only refer to OCIRepositories in the same namespace as the +HelmRelease object. + +```yaml +apiVersion: source.toolkit.fluxcd.io/v1beta2 +kind: OCIRepository +metadata: + name: podinfo + namespace: default +spec: + interval: 30s + url: oci://ghcr.io/stefanprodan/charts/podinfo + ref: + tag: 6.6.0 +--- +apiVersion: helm.toolkit.fluxcd.io/v2beta2 +kind: HelmRelease +metadata: + name: podinfo + namespace: default +spec: + interval: 10m + chartRef: + kind: OCIRepository + name: podinfo + namespace: default + values: + replicaCount: 2 +``` + ### Release name `.spec.releaseName` is an optional field used to specify the name of the Helm diff --git a/go.mod b/go.mod index 29df92c59..2860dfc60 100644 --- a/go.mod +++ b/go.mod @@ -15,6 +15,7 @@ replace ( ) require ( + github.com/Masterminds/semver v1.5.0 github.com/fluxcd/cli-utils v0.36.0-flux.5 github.com/fluxcd/helm-controller/api v0.37.4 github.com/fluxcd/pkg/apis/acl v0.2.0 diff --git a/go.sum b/go.sum index 7eee71cea..77a580de8 100644 --- a/go.sum +++ b/go.sum @@ -10,6 +10,8 @@ github.com/MakeNowJust/heredoc v1.0.0 h1:cXCdzVdstXyiTqTvfqk9SDHpKNjxuom+DOlyEeQ github.com/MakeNowJust/heredoc v1.0.0/go.mod h1:mG5amYoWBHf8vpLOuehzbGGw0EHxpZZ6lCpQ4fNJ8LE= github.com/Masterminds/goutils v1.1.1 h1:5nUrii3FMTL5diU80unEVvNevw1nH4+ZV4DSLVJLSYI= github.com/Masterminds/goutils v1.1.1/go.mod h1:8cTjp+g8YejhMuvIA5y2vz3BpJxksy863GQaJW2MFNU= +github.com/Masterminds/semver v1.5.0 h1:H65muMkzWKEuNDnfl9d70GUjFniHKHRbFPGBuZ3QEww= +github.com/Masterminds/semver v1.5.0/go.mod h1:MB6lktGJrhw8PrUyiEoblNEGEQ+RzHPF078ddwwvV3Y= github.com/Masterminds/semver/v3 v3.2.0/go.mod h1:qvl/7zhW3nngYb5+80sSMF+FG2BjYrf8m9wsX0PNOMQ= github.com/Masterminds/semver/v3 v3.2.1 h1:RN9w6+7QoMeJVGyfmbcgs28Br8cvmnucEXnY0rYXWg0= github.com/Masterminds/semver/v3 v3.2.1/go.mod h1:qvl/7zhW3nngYb5+80sSMF+FG2BjYrf8m9wsX0PNOMQ= diff --git a/internal/action/reset.go b/internal/action/reset.go index a293b7386..0b579ca6d 100644 --- a/internal/action/reset.go +++ b/internal/action/reset.go @@ -48,7 +48,7 @@ func MustResetFailures(obj *v2.HelmRelease, chart *chart.Metadata, values chartu switch { case obj.Status.LastAttemptedGeneration != obj.Generation: return differentGenerationReason, true - case obj.Status.LastAttemptedRevision != chart.Version: + case obj.Status.GetLastAttemptedRevision() != chart.Version: return differentRevisionReason, true case obj.Status.LastAttemptedConfigDigest != "" || obj.Status.LastAttemptedValuesChecksum != "": d := obj.Status.LastAttemptedConfigDigest diff --git a/internal/action/verify.go b/internal/action/verify.go index 3c6852260..aaa143e1b 100644 --- a/internal/action/verify.go +++ b/internal/action/verify.go @@ -126,6 +126,11 @@ func VerifyReleaseObject(snapshot *v2.Snapshot, rls *helmrelease.Release) error verifier := relDig.Verifier() obs := release.ObserveRelease(rls) + + // unfortunately we have to pass in the OciDigest as is, because helmrelease.Release + // does not have a field for it. + obs.OCIDigest = snapshot.OCIDigest + if err = obs.Encode(verifier); err != nil { // We are expected to be able to encode valid JSON, error out without a // typed error assuming malfunction to signal to e.g. retry. diff --git a/internal/controller/helmrelease_controller.go b/internal/controller/helmrelease_controller.go index 8e5d84e09..ba7c0fe85 100644 --- a/internal/controller/helmrelease_controller.go +++ b/internal/controller/helmrelease_controller.go @@ -23,6 +23,7 @@ import ( "strings" "time" + "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" @@ -42,6 +43,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/ratelimiter" "sigs.k8s.io/controller-runtime/pkg/reconcile" + "github.com/Masterminds/semver" aclv1 "github.com/fluxcd/pkg/apis/acl" "github.com/fluxcd/pkg/apis/meta" "github.com/fluxcd/pkg/runtime/acl" @@ -50,8 +52,10 @@ import ( helper "github.com/fluxcd/pkg/runtime/controller" "github.com/fluxcd/pkg/runtime/jitter" "github.com/fluxcd/pkg/runtime/logger" + "github.com/fluxcd/pkg/runtime/object" "github.com/fluxcd/pkg/runtime/patch" "github.com/fluxcd/pkg/runtime/predicates" + source "github.com/fluxcd/source-controller/api/v1" sourcev1 "github.com/fluxcd/source-controller/api/v1beta2" v2 "github.com/fluxcd/helm-controller/api/v2beta2" @@ -73,6 +77,8 @@ import ( // +kubebuilder:rbac:groups=helm.toolkit.fluxcd.io,resources=helmreleases/finalizers,verbs=get;create;update;patch;delete // +kubebuilder:rbac:groups=source.toolkit.fluxcd.io,resources=helmcharts,verbs=get;list;watch // +kubebuilder:rbac:groups=source.toolkit.fluxcd.io,resources=helmcharts/status,verbs=get +// +kubebuilder:rbac:groups=source.toolkit.fluxcd.io,resources=ocirepositories,verbs=get;list;watch +// +kubebuilder:rbac:groups=source.toolkit.fluxcd.io,resources=ocirepositories/status,verbs=get // +kubebuilder:rbac:groups="",resources=events,verbs=create;patch // HelmReleaseReconciler reconciles a HelmRelease object. @@ -104,15 +110,16 @@ var ( ) func (r *HelmReleaseReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, opts HelmReleaseReconcilerOptions) error { - // Index the HelmRelease by the HelmChart references they point at + // 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) + namespacedName, err := getNamespacedName(obj) + if err != nil { + return nil + } return []string{ - types.NamespacedName{ - Namespace: obj.Spec.Chart.GetNamespace(obj.GetNamespace()), - Name: obj.GetHelmChartName(), - }.String(), + namespacedName.String(), } }, ); err != nil { @@ -131,6 +138,11 @@ func (r *HelmReleaseReconciler) SetupWithManager(ctx context.Context, mgr ctrl.M handler.EnqueueRequestsFromMapFunc(r.requestsForHelmChartChange), builder.WithPredicates(intpredicates.SourceRevisionChangePredicate{}), ). + Watches( + &sourcev1.OCIRepository{}, + handler.EnqueueRequestsFromMapFunc(r.requestsForOCIRrepositoryChange), + builder.WithPredicates(intpredicates.SourceRevisionChangePredicate{}), + ). WithOptions(controller.Options{ RateLimiter: opts.RateLimiter, }). @@ -147,6 +159,10 @@ func (r *HelmReleaseReconciler) Reconcile(ctx context.Context, req ctrl.Request) return ctrl.Result{}, client.IgnoreNotFound(err) } + if !isValidChartRef(obj) { + return ctrl.Result{}, reconcile.TerminalError(fmt.Errorf("invalid Chart reference")) + } + // Initialize the patch helper with the current version of the object. patchHelper := patch.NewSerialPatcher(obj, r.Client) @@ -251,8 +267,8 @@ func (r *HelmReleaseReconciler) reconcileRelease(ctx context.Context, patchHelpe conditions.MarkUnknown(obj, meta.ReadyCondition, meta.ProgressingReason, "reconciliation in progress") } - // Get the HelmChart object for the release. - hc, err := r.getHelmChart(ctx, obj) + // Get the source object containing the HelmChart. + source, err := r.getSource(ctx, obj) if err != nil { if acl.IsAccessDenied(err) { conditions.MarkStalled(obj, aclv1.AccessDeniedReason, err.Error()) @@ -266,7 +282,7 @@ func (r *HelmReleaseReconciler) reconcileRelease(ctx context.Context, patchHelpe return ctrl.Result{}, reconcile.TerminalError(err) } - msg := fmt.Sprintf("could not get HelmChart object: %s", err.Error()) + msg := fmt.Sprintf("could not get Source object: %s", err.Error()) conditions.MarkFalse(obj, meta.ReadyCondition, v2.ArtifactFailedReason, msg) return ctrl.Result{}, err } @@ -275,17 +291,16 @@ func (r *HelmReleaseReconciler) reconcileRelease(ctx context.Context, patchHelpe conditions.MarkUnknown(obj, meta.ReadyCondition, meta.ProgressingReason, "reconciliation in progress") } - // Check if the HelmChart is ready. - if ready, reason := isHelmChartReady(hc); !ready { - msg := fmt.Sprintf("HelmChart '%s/%s' is not ready: %s", hc.GetNamespace(), hc.GetName(), reason) + // Check if the source is ready. + if ready, msg := isSourceReady(source); !ready { log.Info(msg) - conditions.MarkFalse(obj, meta.ReadyCondition, "HelmChartNotReady", msg) + conditions.MarkFalse(obj, meta.ReadyCondition, "SourceNotReady", msg) // Do not requeue immediately, when the artifact is created // the watcher should trigger a reconciliation. return jitter.JitteredRequeueInterval(ctrl.Result{RequeueAfter: obj.GetRequeueAfter()}), errWaitForChart } // Remove any stale corresponding Ready=False condition with Unknown. - if conditions.HasAnyReason(obj, meta.ReadyCondition, "HelmChartNotReady") { + if conditions.HasAnyReason(obj, meta.ReadyCondition, "SourceNotReady") { conditions.MarkUnknown(obj, meta.ReadyCondition, meta.ProgressingReason, "reconciliation in progress") } @@ -302,10 +317,10 @@ func (r *HelmReleaseReconciler) reconcileRelease(ctx context.Context, patchHelpe } // Load chart from artifact. - loadedChart, err := loader.SecureLoadChartFromURL(loader.NewRetryableHTTPClient(ctx, r.artifactFetchRetries), hc.GetArtifact().URL, hc.GetArtifact().Digest) + loadedChart, err := loader.SecureLoadChartFromURL(loader.NewRetryableHTTPClient(ctx, r.artifactFetchRetries), source.GetArtifact().URL, source.GetArtifact().Digest) if err != nil { if errors.Is(err, loader.ErrFileNotFound) { - msg := fmt.Sprintf("Chart not ready: artifact not found. Retrying in %s", r.requeueDependency.String()) + msg := fmt.Sprintf("Source not ready: artifact not found. Retrying in %s", r.requeueDependency.String()) conditions.MarkFalse(obj, meta.ReadyCondition, v2.ArtifactFailedReason, msg) log.Info(msg) return ctrl.Result{RequeueAfter: r.requeueDependency}, errWaitForDependency @@ -320,6 +335,12 @@ func (r *HelmReleaseReconciler) reconcileRelease(ctx context.Context, patchHelpe conditions.MarkUnknown(obj, meta.ReadyCondition, meta.ProgressingReason, "reconciliation in progress") } + ociDigest, err := mutateChartWithSourceRevision(loadedChart, source) + if err != nil { + conditions.MarkFalse(obj, meta.ReadyCondition, "ChartMutateError", err.Error()) + return ctrl.Result{}, err + } + // Build the REST client getter. getter, err := r.buildRESTClientGetter(ctx, obj) if err != nil { @@ -367,6 +388,7 @@ func (r *HelmReleaseReconciler) reconcileRelease(ctx context.Context, patchHelpe // Set last attempt values. obj.Status.LastAttemptedGeneration = obj.Generation obj.Status.LastAttemptedRevision = loadedChart.Metadata.Version + obj.Status.LastAttemptedRevisionDigest = ociDigest obj.Status.LastAttemptedConfigDigest = chartutil.DigestValues(digest.Canonical, values).String() obj.Status.LastAttemptedValuesChecksum = "" obj.Status.LastReleaseRevision = 0 @@ -657,11 +679,24 @@ func (r *HelmReleaseReconciler) buildRESTClientGetter(ctx context.Context, obj * return kube.NewMemoryRESTClientGetter(cfg, opts...), nil } -// getHelmChart retrieves the v1beta2.HelmChart for the given v2beta2.HelmRelease -// using the name that is advertised in the status object. -// It returns the v1beta2.HelmChart, or an error. -func (r *HelmReleaseReconciler) getHelmChart(ctx context.Context, obj *v2.HelmRelease) (*sourcev1.HelmChart, error) { - namespace, name := obj.Status.GetHelmChart() +// getSource returns the source object containing the HelmChart, either by +// using the chartRef in the spec, or by looking up the HelmChart +// referenced in the status object. +// It returns the source object or an error. +func (r *HelmReleaseReconciler) getSource(ctx context.Context, obj *v2.HelmRelease) (source.Source, error) { + var name, namespace string + if obj.HasChartRef() { + if obj.Spec.ChartRef.Kind == sourcev1.OCIRepositoryKind { + return r.getSourceFromOCIRef(ctx, obj) + } + name, namespace = obj.Spec.ChartRef.Name, obj.Spec.ChartRef.Namespace + if namespace == "" { + namespace = obj.GetNamespace() + } + } else { + namespace, name = obj.Status.GetHelmChart() + } + chartRef := types.NamespacedName{Namespace: namespace, Name: name} if err := intacl.AllowsAccessTo(obj, sourcev1.HelmChartKind, chartRef); err != nil { @@ -675,6 +710,24 @@ func (r *HelmReleaseReconciler) getHelmChart(ctx context.Context, obj *v2.HelmRe return &hc, nil } +func (r *HelmReleaseReconciler) getSourceFromOCIRef(ctx context.Context, obj *v2.HelmRelease) (source.Source, error) { + name, namespace := obj.Spec.ChartRef.Name, obj.Spec.ChartRef.Namespace + if namespace == "" { + namespace = obj.GetNamespace() + } + ociRepoRef := types.NamespacedName{Namespace: namespace, Name: name} + + if err := intacl.AllowsAccessTo(obj, sourcev1.OCIRepositoryKind, ociRepoRef); err != nil { + return nil, err + } + + or := sourcev1.OCIRepository{} + if err := r.Client.Get(ctx, ociRepoRef, &or); err != nil { + return nil, err + } + return &or, nil +} + // waitForHistoryCacheSync returns a function that can be used to wait for the // cache backing the Kubernetes client to be in sync with the current state of // the v2beta2.HelmRelease. @@ -717,7 +770,7 @@ func (r *HelmReleaseReconciler) requestsForHelmChartChange(ctx context.Context, 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.LastAttemptedRevision) { + if conditions.IsReady(&list.Items[i]) && hc.GetArtifact().HasRevision(hr.Status.GetLastAttemptedRevision()) { continue } reqs = append(reqs, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(&list.Items[i])}) @@ -725,28 +778,181 @@ func (r *HelmReleaseReconciler) requestsForHelmChartChange(ctx context.Context, return reqs } -// isHelmChartReady returns true if the given HelmChart is ready, and a reason -// why it is not ready otherwise. -func isHelmChartReady(obj *sourcev1.HelmChart) (bool, string) { +func (r *HelmReleaseReconciler) requestsForOCIRrepositoryChange(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: 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 +} + +func isSourceReady(obj source.Source) (bool, string) { + if o, ok := obj.(conditions.Getter); ok { + return isReady(o, obj.GetArtifact()) + } + return false, fmt.Sprintf("unknown source type: %T", obj) +} + +func isReady(obj conditions.Getter, artifact *source.Artifact) (bool, string) { + observedGen, err := object.GetStatusObservedGeneration(obj) + if err != nil { + return false, err.Error() + } + + kind := obj.GetObjectKind().GroupVersionKind().Kind + switch { - case obj.Generation != obj.Status.ObservedGeneration: + case obj.GetGeneration() != observedGen: msg := "latest generation of object has not been reconciled" - // If the chart is not ready, we can likely provide a more - // concise reason why. - // We do not do this while the Generation matches the Observed - // Generation, as we could then potentially stall on e.g. - // temporary errors which do not have an impact as long as - // there is an Artifact for the current Generation. if conditions.IsFalse(obj, meta.ReadyCondition) { msg = conditions.GetMessage(obj, meta.ReadyCondition) } - return false, msg + return false, fmt.Sprintf("%s '%s/%s' is not ready: %s", + kind, obj.GetNamespace(), obj.GetName(), msg) case conditions.IsStalled(obj): - return false, conditions.GetMessage(obj, meta.StalledCondition) - case obj.Status.Artifact == nil: - return false, "does not have an artifact" + return false, fmt.Sprintf("%s '%s/%s' is not ready: %s", + kind, obj.GetNamespace(), obj.GetName(), conditions.GetMessage(obj, meta.StalledCondition)) + case artifact == nil: + return false, fmt.Sprintf("%s '%s/%s' is not ready: %s", + kind, obj.GetNamespace(), obj.GetName(), "does not have an artifact") default: return true, "" } } + +func isValidChartRef(obj *v2.HelmRelease) bool { + return (obj.HasChartRef() && !obj.HasChartTemplate()) || + (!obj.HasChartRef() && obj.HasChartTemplate()) +} + +func getNamespacedName(obj *v2.HelmRelease) (types.NamespacedName, error) { + namespacedName := types.NamespacedName{} + switch { + case obj.HasChartRef() && !obj.HasChartTemplate(): + namespacedName.Namespace = obj.Spec.ChartRef.Namespace + if namespacedName.Namespace == "" { + namespacedName.Namespace = obj.GetNamespace() + } + namespacedName.Name = obj.Spec.ChartRef.Name + case !obj.HasChartRef() && obj.HasChartTemplate(): + namespacedName.Namespace = obj.Spec.Chart.GetNamespace(obj.GetNamespace()) + namespacedName.Name = obj.GetHelmChartName() + default: + return namespacedName, fmt.Errorf("one of chartRef or chart must be present") + } + + return namespacedName, nil +} + +func mutateChartWithSourceRevision(chart *chart.Chart, source source.Source) (string, error) { + // If the source is an OCIRepository, we can try to mutate the chart version + // with the artifact revision. The revision is either a @ or + // just a digest. + obj, ok := source.(*sourcev1.OCIRepository) + if !ok { + // if not make sure to return an empty string to delete the digest of the + // last attempted revision + return "", nil + } + ver, err := semver.NewVersion(chart.Metadata.Version) + if err != nil { + return "", err + } + + var ociDigest string + revision := obj.GetArtifact().Revision + switch { + case strings.Contains(revision, "@"): + tagD := strings.Split(revision, "@") + if len(tagD) != 2 || tagD[0] != chart.Metadata.Version { + return "", fmt.Errorf("artifact revision %s does not match chart version %s", tagD[0], chart.Metadata.Version) + } + // algotithm are sha256, sha384, sha512 with the canonical being sha256 + // So every digest starts with a sha algorithm and a colon + sha, err := extractDigestSubString(tagD[1]) + if err != nil { + return "", err + } + // add the digest to the chart version to make sure mutable tags are detected + *ver, err = ver.SetMetadata(sha) + if err != nil { + return "", err + } + ociDigest = tagD[1] + default: + // default to the digest + sha, err := extractDigestSubString(revision) + if err != nil { + return "", err + } + *ver, err = ver.SetMetadata(sha) + if err != nil { + return "", err + } + ociDigest = revision + } + + chart.Metadata.Version = ver.String() + return ociDigest, nil +} + +func extractDigestSubString(revision string) (string, error) { + var sha string + // expects a revision in the : format + if pair := strings.Split(revision, ":"); len(pair) != 2 { + return "", fmt.Errorf("invalid artifact revision %s", revision) + } else { + sha = pair[1] + } + if len(sha) < 12 { + return "", fmt.Errorf("invalid artifact revision %s", revision) + } + return sha[0:12], nil +} + +func extractDigest(revision string) string { + if strings.Contains(revision, "@") { + // expects a revision in the @: format + tagD := strings.Split(revision, "@") + if len(tagD) != 2 { + return "" + } + return tagD[1] + } else { + // revision in the : format + return revision + } +} diff --git a/internal/controller/helmrelease_controller_test.go b/internal/controller/helmrelease_controller_test.go index 54166b445..6cd56bc72 100644 --- a/internal/controller/helmrelease_controller_test.go +++ b/internal/controller/helmrelease_controller_test.go @@ -19,12 +19,22 @@ package controller import ( "context" "errors" + "fmt" "strings" "testing" "time" + "github.com/fluxcd/pkg/apis/acl" + aclv1 "github.com/fluxcd/pkg/apis/acl" + "github.com/fluxcd/pkg/apis/meta" + "github.com/fluxcd/pkg/runtime/conditions" + feathelper "github.com/fluxcd/pkg/runtime/features" + "github.com/fluxcd/pkg/runtime/patch" + sourcev1 "github.com/fluxcd/source-controller/api/v1" + sourcev1b2 "github.com/fluxcd/source-controller/api/v1beta2" . "github.com/onsi/gomega" "github.com/opencontainers/go-digest" + "helm.sh/helm/v3/pkg/chart" helmrelease "helm.sh/helm/v3/pkg/release" helmstorage "helm.sh/helm/v3/pkg/storage" helmdriver "helm.sh/helm/v3/pkg/storage/driver" @@ -32,6 +42,7 @@ import ( apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" "k8s.io/cli-runtime/pkg/genericclioptions" "k8s.io/client-go/rest" "k8s.io/client-go/tools/clientcmd" @@ -42,15 +53,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/reconcile" "sigs.k8s.io/yaml" - "github.com/fluxcd/pkg/apis/acl" - aclv1 "github.com/fluxcd/pkg/apis/acl" - "github.com/fluxcd/pkg/apis/meta" - "github.com/fluxcd/pkg/runtime/conditions" - feathelper "github.com/fluxcd/pkg/runtime/features" - "github.com/fluxcd/pkg/runtime/patch" - sourcev1 "github.com/fluxcd/source-controller/api/v1" - sourcev1b2 "github.com/fluxcd/source-controller/api/v1beta2" - v2 "github.com/fluxcd/helm-controller/api/v2beta2" intacl "github.com/fluxcd/helm-controller/internal/acl" "github.com/fluxcd/helm-controller/internal/action" @@ -143,7 +145,7 @@ func TestHelmReleaseReconciler_reconcileRelease(t *testing.T) { g.Expect(obj.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{ *conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "Fulfilling prerequisites"), - *conditions.FalseCondition(meta.ReadyCondition, v2.ArtifactFailedReason, "could not get HelmChart object"), + *conditions.FalseCondition(meta.ReadyCondition, v2.ArtifactFailedReason, "could not get Source object"), })) }) @@ -184,6 +186,10 @@ func TestHelmReleaseReconciler_reconcileRelease(t *testing.T) { g := NewWithT(t) chart := &sourcev1b2.HelmChart{ + TypeMeta: metav1.TypeMeta{ + APIVersion: sourcev1b2.GroupVersion.String(), + Kind: sourcev1b2.HelmChartKind, + }, ObjectMeta: metav1.ObjectMeta{ Name: "chart", Namespace: "mock", @@ -228,7 +234,7 @@ func TestHelmReleaseReconciler_reconcileRelease(t *testing.T) { g.Expect(obj.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{ *conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, ""), - *conditions.FalseCondition(meta.ReadyCondition, "HelmChartNotReady", "HelmChart 'mock/chart' is not ready"), + *conditions.FalseCondition(meta.ReadyCondition, "SourceNotReady", "HelmChart 'mock/chart' is not ready"), })) }) @@ -236,6 +242,10 @@ func TestHelmReleaseReconciler_reconcileRelease(t *testing.T) { g := NewWithT(t) chart := &sourcev1b2.HelmChart{ + TypeMeta: metav1.TypeMeta{ + APIVersion: sourcev1b2.GroupVersion.String(), + Kind: sourcev1b2.HelmChartKind, + }, ObjectMeta: metav1.ObjectMeta{ Name: "chart", Namespace: "mock", @@ -280,59 +290,7 @@ func TestHelmReleaseReconciler_reconcileRelease(t *testing.T) { g.Expect(obj.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{ *conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, ""), - *conditions.FalseCondition(meta.ReadyCondition, "HelmChartNotReady", "HelmChart 'mock/chart' is not ready"), - })) - }) - - t.Run("confirms HelmChart has an Artifact", func(t *testing.T) { - g := NewWithT(t) - - chart := &sourcev1b2.HelmChart{ - ObjectMeta: metav1.ObjectMeta{ - Name: "chart", - Namespace: "mock", - Generation: 2, - }, - Status: sourcev1b2.HelmChartStatus{ - ObservedGeneration: 2, - Artifact: nil, - Conditions: []metav1.Condition{ - { - Type: meta.ReadyCondition, - Status: metav1.ConditionTrue, - }, - }, - }, - } - - obj := &v2.HelmRelease{ - ObjectMeta: metav1.ObjectMeta{ - Name: "release", - Namespace: "mock", - }, - Spec: v2.HelmReleaseSpec{ - Interval: metav1.Duration{Duration: 1 * time.Second}, - }, - Status: v2.HelmReleaseStatus{ - HelmChart: "mock/chart", - }, - } - - r := &HelmReleaseReconciler{ - Client: fake.NewClientBuilder(). - WithScheme(NewTestScheme()). - WithStatusSubresource(&v2.HelmRelease{}). - WithObjects(chart, obj). - Build(), - } - - res, err := r.reconcileRelease(context.TODO(), patch.NewSerialPatcher(obj, r.Client), obj) - g.Expect(err).To(Equal(errWaitForChart)) - g.Expect(res.RequeueAfter).To(Equal(obj.Spec.Interval.Duration)) - - g.Expect(obj.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{ - *conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, ""), - *conditions.FalseCondition(meta.ReadyCondition, "HelmChartNotReady", "HelmChart 'mock/chart' is not ready"), + *conditions.FalseCondition(meta.ReadyCondition, "SourceNotReady", "HelmChart 'mock/chart' is not ready"), })) }) @@ -444,7 +402,7 @@ func TestHelmReleaseReconciler_reconcileRelease(t *testing.T) { g.Expect(obj.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{ *conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, ""), - *conditions.FalseCondition(meta.ReadyCondition, v2.ArtifactFailedReason, "Chart not ready"), + *conditions.FalseCondition(meta.ReadyCondition, v2.ArtifactFailedReason, "Source not ready"), })) }) @@ -679,35 +637,611 @@ func TestHelmReleaseReconciler_reconcileRelease(t *testing.T) { Build() r := &HelmReleaseReconciler{ - Client: c, - GetClusterConfig: GetTestClusterConfig, - EventRecorder: record.NewFakeRecorder(32), + Client: c, + GetClusterConfig: GetTestClusterConfig, + EventRecorder: record.NewFakeRecorder(32), + } + + _, err = r.reconcileRelease(context.TODO(), patch.NewSerialPatcher(obj, c), obj) + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(ContainSubstring("namespaces \"not-exist\" not found")) + + // Verify failure counts are reset. + g.Expect(obj.Status.InstallFailures).To(Equal(int64(0))) + g.Expect(obj.Status.UpgradeFailures).To(Equal(int64(0))) + g.Expect(obj.Status.Failures).To(Equal(int64(1))) + }) + + t.Run("sets last attempted values", func(t *testing.T) { + g := NewWithT(t) + + chartMock := testutil.BuildChart() + chartArtifact, err := testutil.SaveChartAsArtifact(chartMock, digest.SHA256, testServer.URL(), testServer.Root()) + g.Expect(err).ToNot(HaveOccurred()) + + chart := &sourcev1b2.HelmChart{ + ObjectMeta: metav1.ObjectMeta{ + Name: "chart", + Namespace: "mock", + Generation: 1, + }, + Status: sourcev1b2.HelmChartStatus{ + ObservedGeneration: 1, + Artifact: chartArtifact, + Conditions: []metav1.Condition{ + { + Type: meta.ReadyCondition, + Status: metav1.ConditionTrue, + }, + }, + }, + } + + obj := &v2.HelmRelease{ + ObjectMeta: metav1.ObjectMeta{ + Name: "release", + Namespace: "mock", + Generation: 2, + }, + Spec: v2.HelmReleaseSpec{ + // Trigger a failure by setting an invalid storage namespace, + // preventing the release from actually being installed. + // This allows us to just test the values being set, without + // having to facilitate a full install. + StorageNamespace: "not-exist", + Values: &apiextensionsv1.JSON{ + Raw: []byte(`{"foo":"bar"}`), + }, + }, + Status: v2.HelmReleaseStatus{ + HelmChart: "mock/chart", + ObservedGeneration: 2, + // Confirm deprecated value is cleared. + LastAttemptedValuesChecksum: "b5cbcf5c23cfd945d2cdf0ffaab387a46f2d054f", + }, + } + + c := fake.NewClientBuilder(). + WithScheme(NewTestScheme()). + WithStatusSubresource(&v2.HelmRelease{}). + WithObjects(chart, obj). + Build() + + r := &HelmReleaseReconciler{ + Client: c, + GetClusterConfig: GetTestClusterConfig, + EventRecorder: record.NewFakeRecorder(32), + } + + _, err = r.reconcileRelease(context.TODO(), patch.NewSerialPatcher(obj, c), obj) + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(ContainSubstring("namespaces \"not-exist\" not found")) + + // Verify attempted values are set. + g.Expect(obj.Status.LastAttemptedGeneration).To(Equal(obj.Generation)) + g.Expect(obj.Status.LastAttemptedRevision).To(Equal(chartMock.Metadata.Version)) + g.Expect(obj.Status.LastAttemptedConfigDigest).To(Equal("sha256:1dabc4e3cbbd6a0818bd460f3a6c9855bfe95d506c74726bc0f2edb0aecb1f4e")) + g.Expect(obj.Status.LastAttemptedValuesChecksum).To(BeEmpty()) + }) + + t.Run("error recovery updates ready condition", func(t *testing.T) { + g := NewWithT(t) + + // Create a test namespace for storing the Helm release mock. + ns, err := testEnv.CreateNamespace(context.TODO(), "error-recovery") + g.Expect(err).ToNot(HaveOccurred()) + t.Cleanup(func() { + _ = testEnv.Delete(context.TODO(), ns) + }) + + // Create HelmChart mock. + chartMock := testutil.BuildChart() + chartArtifact, err := testutil.SaveChartAsArtifact(chartMock, digest.SHA256, testServer.URL(), testServer.Root()) + g.Expect(err).ToNot(HaveOccurred()) + + chart := &sourcev1b2.HelmChart{ + ObjectMeta: metav1.ObjectMeta{ + Name: "error-recovery", + Namespace: ns.Name, + Generation: 1, + }, + Spec: sourcev1b2.HelmChartSpec{ + Chart: "testdata/test-helmrepo", + Version: "0.1.0", + SourceRef: sourcev1b2.LocalHelmChartSourceReference{ + Kind: sourcev1b2.HelmRepositoryKind, + Name: "error-recovery", + }, + }, + Status: sourcev1b2.HelmChartStatus{ + ObservedGeneration: 1, + Artifact: chartArtifact, + Conditions: []metav1.Condition{ + { + Type: meta.ReadyCondition, + Status: metav1.ConditionTrue, + }, + }, + }, + } + + // Create a test Helm release storage mock. + rls := testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + Name: "error-recovery", + Namespace: ns.Name, + Version: 1, + Chart: chartMock, + Status: helmrelease.StatusDeployed, + }, testutil.ReleaseWithConfig(nil)) + + obj := &v2.HelmRelease{ + ObjectMeta: metav1.ObjectMeta{ + Name: "error-recovery", + Namespace: ns.Name, + }, + Spec: v2.HelmReleaseSpec{ + StorageNamespace: ns.Name, + }, + Status: v2.HelmReleaseStatus{ + HelmChart: chart.Namespace + "/" + chart.Name, + History: v2.Snapshots{ + release.ObservedToSnapshot(release.ObserveRelease(rls)), + }, + }, + } + + c := fake.NewClientBuilder(). + WithScheme(NewTestScheme()). + WithStatusSubresource(&v2.HelmRelease{}). + WithObjects(chart, obj). + Build() + + r := &HelmReleaseReconciler{ + Client: c, + GetClusterConfig: GetTestClusterConfig, + EventRecorder: record.NewFakeRecorder(32), + FieldManager: "test", + } + + // Store the Helm release mock in the test namespace. + getter, err := r.buildRESTClientGetter(context.TODO(), obj) + g.Expect(err).ToNot(HaveOccurred()) + + cfg, err := action.NewConfigFactory(getter, action.WithStorage(helmdriver.SecretsDriverName, obj.GetStorageNamespace())) + g.Expect(err).ToNot(HaveOccurred()) + + store := helmstorage.Init(cfg.Driver) + g.Expect(store.Create(rls)).To(Succeed()) + + sp := patch.NewSerialPatcher(obj, r.Client) + + // List of failure reasons to test. + prereqFailures := []string{ + v2.DependencyNotReadyReason, + aclv1.AccessDeniedReason, + v2.ArtifactFailedReason, + "SourceNotReady", + "ValuesError", + "RESTClientError", + "FactoryError", + } + + // Update ready condition for each failure, reconcile and check if the + // stale failure condition gets updated. + for _, failReason := range prereqFailures { + conditions.MarkFalse(obj, meta.ReadyCondition, failReason, "foo") + err := sp.Patch(context.TODO(), obj, + patch.WithOwnedConditions{Conditions: intreconcile.OwnedConditions}, + patch.WithFieldOwner(r.FieldManager), + ) + g.Expect(err).ToNot(HaveOccurred()) + + _, err = r.reconcileRelease(context.TODO(), sp, obj) + g.Expect(err).ToNot(HaveOccurred()) + + ready := conditions.Get(obj, meta.ReadyCondition) + g.Expect(ready.Status).To(Equal(metav1.ConditionUnknown)) + g.Expect(ready.Reason).To(Equal(meta.ProgressingReason)) + } + }) +} + +func TestHelmReleaseReconciler_reconcileReleaseFromOCIRepositorySource(t *testing.T) { + + t.Run("handles chartRef and Chart definition failure", func(t *testing.T) { + g := NewWithT(t) + + obj := &v2.HelmRelease{ + ObjectMeta: metav1.ObjectMeta{ + Name: "release", + Namespace: "mock", + }, + Spec: v2.HelmReleaseSpec{ + ChartRef: &v2.CrossNamespaceSourceReference{ + Kind: "OCIRepository", + Name: "ocirepo", + Namespace: "mock", + }, + Chart: v2.HelmChartTemplate{ + Spec: v2.HelmChartTemplateSpec{ + Chart: "mychart", + SourceRef: v2.CrossNamespaceObjectReference{ + Name: "something", + }, + }, + }, + }, + } + + r := &HelmReleaseReconciler{ + Client: fake.NewClientBuilder(). + WithScheme(NewTestScheme()). + WithStatusSubresource(&v2.HelmRelease{}). + WithObjects(obj). + Build(), + } + + res, err := r.Reconcile(context.TODO(), reconcile.Request{ + NamespacedName: types.NamespacedName{ + Namespace: obj.GetNamespace(), + Name: obj.GetName(), + }, + }) + + // only chartRef or Chart must be set + g.Expect(errors.Is(err, reconcile.TerminalError(fmt.Errorf("invalid Chart reference")))).To(BeTrue()) + g.Expect(res.IsZero()).To(BeTrue()) + }) + t.Run("handles ChartRef get failure", func(t *testing.T) { + g := NewWithT(t) + + obj := &v2.HelmRelease{ + ObjectMeta: metav1.ObjectMeta{ + Name: "release", + Namespace: "mock", + }, + Spec: v2.HelmReleaseSpec{ + ChartRef: &v2.CrossNamespaceSourceReference{ + Kind: "OCIRepository", + Name: "ocirepo", + Namespace: "mock", + }, + }, + } + + r := &HelmReleaseReconciler{ + Client: fake.NewClientBuilder(). + WithScheme(NewTestScheme()). + WithStatusSubresource(&v2.HelmRelease{}). + WithObjects(obj). + Build(), + EventRecorder: record.NewFakeRecorder(32), + } + + _, err := r.reconcileRelease(context.TODO(), patch.NewSerialPatcher(obj, r.Client), obj) + g.Expect(err).To(HaveOccurred()) + + g.Expect(obj.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{ + *conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "Fulfilling prerequisites"), + *conditions.FalseCondition(meta.ReadyCondition, v2.ArtifactFailedReason, "could not get Source object"), + })) + }) + + t.Run("handles ACL error for ChartRef", func(t *testing.T) { + g := NewWithT(t) + + obj := &v2.HelmRelease{ + ObjectMeta: metav1.ObjectMeta{ + Name: "release", + Namespace: "mock", + }, + Spec: v2.HelmReleaseSpec{ + ChartRef: &v2.CrossNamespaceSourceReference{ + Kind: "OCIRepository", + Name: "ocirepo", + Namespace: "mock-other", + }, + }, + } + + r := &HelmReleaseReconciler{ + Client: fake.NewClientBuilder(). + WithScheme(NewTestScheme()). + WithStatusSubresource(&v2.HelmRelease{}). + WithObjects(obj). + Build(), + EventRecorder: record.NewFakeRecorder(32), + } + + res, err := r.reconcileRelease(context.TODO(), patch.NewSerialPatcher(obj, r.Client), obj) + g.Expect(err).To(HaveOccurred()) + g.Expect(errors.Is(err, reconcile.TerminalError(nil))).To(BeTrue()) + g.Expect(res.IsZero()).To(BeTrue()) + + g.Expect(obj.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{ + *conditions.TrueCondition(meta.StalledCondition, acl.AccessDeniedReason, "cross-namespace references are not allowed"), + *conditions.FalseCondition(meta.ReadyCondition, acl.AccessDeniedReason, "cross-namespace references are not allowed"), + })) + }) + + t.Run("waits for ChartRef to have an Artifact", func(t *testing.T) { + g := NewWithT(t) + + ocirepo := &sourcev1b2.OCIRepository{ + TypeMeta: metav1.TypeMeta{ + APIVersion: sourcev1b2.GroupVersion.String(), + Kind: sourcev1b2.OCIRepositoryKind, + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "ocirepo", + Namespace: "mock", + Generation: 2, + }, + Status: sourcev1b2.OCIRepositoryStatus{ + ObservedGeneration: 2, + Conditions: []metav1.Condition{ + { + Type: meta.ReadyCondition, + Status: metav1.ConditionTrue, + }, + }, + }, + } + + obj := &v2.HelmRelease{ + ObjectMeta: metav1.ObjectMeta{ + Name: "release", + Namespace: "mock", + }, + Spec: v2.HelmReleaseSpec{ + ChartRef: &v2.CrossNamespaceSourceReference{ + Kind: "OCIRepository", + Name: "ocirepo", + Namespace: "mock", + }, + Interval: metav1.Duration{Duration: 1 * time.Second}, + }, + } + + r := &HelmReleaseReconciler{ + Client: fake.NewClientBuilder(). + WithScheme(NewTestScheme()). + WithStatusSubresource(&v2.HelmRelease{}). + WithObjects(ocirepo, obj). + Build(), + } + + res, err := r.reconcileRelease(context.TODO(), patch.NewSerialPatcher(obj, r.Client), obj) + g.Expect(err).To(Equal(errWaitForChart)) + g.Expect(res.RequeueAfter).To(Equal(obj.Spec.Interval.Duration)) + + g.Expect(obj.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{ + *conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, ""), + *conditions.FalseCondition(meta.ReadyCondition, "SourceNotReady", "OCIRepository 'mock/ocirepo' is not ready"), + })) + }) + + t.Run("reports values composition failure", func(t *testing.T) { + g := NewWithT(t) + + ocirepo := &sourcev1b2.OCIRepository{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ocirepo", + Namespace: "mock", + Generation: 2, + }, + Spec: sourcev1b2.OCIRepositorySpec{ + Interval: metav1.Duration{Duration: 1 * time.Second}, + }, + Status: sourcev1b2.OCIRepositoryStatus{ + ObservedGeneration: 2, + Artifact: &sourcev1.Artifact{}, + Conditions: []metav1.Condition{ + { + Type: meta.ReadyCondition, + Status: metav1.ConditionTrue, + }, + }, + }, + } + + obj := &v2.HelmRelease{ + ObjectMeta: metav1.ObjectMeta{ + Name: "release", + Namespace: "mock", + }, + Spec: v2.HelmReleaseSpec{ + ChartRef: &v2.CrossNamespaceSourceReference{ + Kind: "OCIRepository", + Name: "ocirepo", + Namespace: "mock", + }, + ValuesFrom: []v2.ValuesReference{ + { + Kind: "Secret", + Name: "missing", + }, + }, + }, + } + + r := &HelmReleaseReconciler{ + Client: fake.NewClientBuilder(). + WithScheme(NewTestScheme()). + WithStatusSubresource(&v2.HelmRelease{}). + WithObjects(ocirepo, obj). + Build(), + EventRecorder: record.NewFakeRecorder(32), + } + + _, err := r.reconcileRelease(context.TODO(), patch.NewSerialPatcher(obj, r.Client), obj) + g.Expect(err).To(HaveOccurred()) + + g.Expect(obj.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{ + *conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "Fulfilling prerequisites"), + *conditions.FalseCondition(meta.ReadyCondition, "ValuesError", "could not resolve Secret chart values reference 'mock/missing' with key 'values.yaml'"), + })) + }) + + t.Run("reports Helm chart load failure", func(t *testing.T) { + g := NewWithT(t) + + ocirepo := &sourcev1b2.OCIRepository{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ocirepo", + Namespace: "mock", + Generation: 2, + }, + Spec: sourcev1b2.OCIRepositorySpec{ + Interval: metav1.Duration{Duration: 1 * time.Second}, + }, + Status: sourcev1b2.OCIRepositoryStatus{ + ObservedGeneration: 2, + Artifact: &sourcev1.Artifact{ + URL: testServer.URL() + "/does-not-exist", + }, + Conditions: []metav1.Condition{ + { + Type: meta.ReadyCondition, + Status: metav1.ConditionTrue, + }, + }, + }, + } + + obj := &v2.HelmRelease{ + ObjectMeta: metav1.ObjectMeta{ + Name: "release", + Namespace: "mock", + }, + Spec: v2.HelmReleaseSpec{ + ChartRef: &v2.CrossNamespaceSourceReference{ + Kind: "OCIRepository", + Name: "ocirepo", + Namespace: "mock", + }, + }, + } + + r := &HelmReleaseReconciler{ + Client: fake.NewClientBuilder(). + WithScheme(NewTestScheme()). + WithStatusSubresource(&v2.HelmRelease{}). + WithObjects(ocirepo, obj). + Build(), + requeueDependency: 10 * time.Second, + EventRecorder: record.NewFakeRecorder(32), + } + + res, err := r.reconcileRelease(context.TODO(), patch.NewSerialPatcher(obj, r.Client), obj) + g.Expect(err).To(Equal(errWaitForDependency)) + g.Expect(res.RequeueAfter).To(Equal(r.requeueDependency)) + + g.Expect(obj.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{ + *conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, ""), + *conditions.FalseCondition(meta.ReadyCondition, v2.ArtifactFailedReason, "Source not ready"), + })) + }) + t.Run("report helmChart load failure when switching from existing HelmChat to chartRef", func(t *testing.T) { + g := NewWithT(t) + + chart := &sourcev1b2.HelmChart{ + ObjectMeta: metav1.ObjectMeta{ + Name: "chart", + Namespace: "mock", + Generation: 1, + }, + Status: sourcev1b2.HelmChartStatus{ + ObservedGeneration: 1, + Artifact: &sourcev1.Artifact{}, + Conditions: []metav1.Condition{ + { + Type: meta.ReadyCondition, + Status: metav1.ConditionTrue, + }, + }, + }, + } + + ocirepo := &sourcev1b2.OCIRepository{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ocirepo", + Namespace: "mock", + Generation: 2, + }, + Spec: sourcev1b2.OCIRepositorySpec{ + Interval: metav1.Duration{Duration: 1 * time.Second}, + }, + Status: sourcev1b2.OCIRepositoryStatus{ + ObservedGeneration: 2, + Artifact: &sourcev1.Artifact{ + URL: testServer.URL() + "/does-not-exist", + }, + Conditions: []metav1.Condition{ + { + Type: meta.ReadyCondition, + Status: metav1.ConditionTrue, + }, + }, + }, + } + + obj := &v2.HelmRelease{ + ObjectMeta: metav1.ObjectMeta{ + Name: "release", + Namespace: "mock", + }, + Spec: v2.HelmReleaseSpec{ + ChartRef: &v2.CrossNamespaceSourceReference{ + Kind: "OCIRepository", + Name: "ocirepo", + Namespace: "mock", + }, + }, + Status: v2.HelmReleaseStatus{ + HelmChart: "mock/chart", + }, + } + + r := &HelmReleaseReconciler{ + Client: fake.NewClientBuilder(). + WithScheme(NewTestScheme()). + WithStatusSubresource(&v2.HelmRelease{}). + WithObjects(chart, ocirepo, obj). + Build(), + requeueDependency: 10 * time.Second, + EventRecorder: record.NewFakeRecorder(32), } - _, err = r.reconcileRelease(context.TODO(), patch.NewSerialPatcher(obj, c), obj) - g.Expect(err).To(HaveOccurred()) - g.Expect(err.Error()).To(ContainSubstring("namespaces \"not-exist\" not found")) + res, err := r.reconcileRelease(context.TODO(), patch.NewSerialPatcher(obj, r.Client), obj) + g.Expect(err).To(Equal(errWaitForDependency)) + g.Expect(res.RequeueAfter).To(Equal(r.requeueDependency)) - // Verify failure counts are reset. - g.Expect(obj.Status.InstallFailures).To(Equal(int64(0))) - g.Expect(obj.Status.UpgradeFailures).To(Equal(int64(0))) - g.Expect(obj.Status.Failures).To(Equal(int64(1))) + g.Expect(obj.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{ + *conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, ""), + *conditions.FalseCondition(meta.ReadyCondition, v2.ArtifactFailedReason, "Source not ready"), + })) }) - t.Run("sets last attempted values", func(t *testing.T) { + t.Run("handle chartRef mutable tag", func(t *testing.T) { g := NewWithT(t) + // Create HelmChart mock. chartMock := testutil.BuildChart() chartArtifact, err := testutil.SaveChartAsArtifact(chartMock, digest.SHA256, testServer.URL(), testServer.Root()) g.Expect(err).ToNot(HaveOccurred()) + chartArtifact.Revision += "@" + chartArtifact.Digest - chart := &sourcev1b2.HelmChart{ + ocirepo := &sourcev1b2.OCIRepository{ ObjectMeta: metav1.ObjectMeta{ - Name: "chart", + Name: "ocirepo", Namespace: "mock", Generation: 1, }, - Status: sourcev1b2.HelmChartStatus{ + Spec: sourcev1b2.OCIRepositorySpec{ + Interval: metav1.Duration{Duration: 1 * time.Second}, + }, + Status: sourcev1b2.OCIRepositoryStatus{ ObservedGeneration: 1, Artifact: chartArtifact, Conditions: []metav1.Condition{ @@ -721,69 +1255,77 @@ func TestHelmReleaseReconciler_reconcileRelease(t *testing.T) { obj := &v2.HelmRelease{ ObjectMeta: metav1.ObjectMeta{ - Name: "release", - Namespace: "mock", - Generation: 2, + Name: "release", + Namespace: "mock", }, Spec: v2.HelmReleaseSpec{ - // Trigger a failure by setting an invalid storage namespace, - // preventing the release from actually being installed. - // This allows us to just test the values being set, without - // having to facilitate a full install. - StorageNamespace: "not-exist", - Values: &apiextensionsv1.JSON{ - Raw: []byte(`{"foo":"bar"}`), + ChartRef: &v2.CrossNamespaceSourceReference{ + Kind: "OCIRepository", + Name: "ocirepo", + Namespace: "mock", }, }, - Status: v2.HelmReleaseStatus{ - HelmChart: "mock/chart", - ObservedGeneration: 2, - // Confirm deprecated value is cleared. - LastAttemptedValuesChecksum: "b5cbcf5c23cfd945d2cdf0ffaab387a46f2d054f", - }, } - c := fake.NewClientBuilder(). - WithScheme(NewTestScheme()). - WithStatusSubresource(&v2.HelmRelease{}). - WithObjects(chart, obj). - Build() - r := &HelmReleaseReconciler{ - Client: c, + Client: fake.NewClientBuilder(). + WithScheme(NewTestScheme()). + WithStatusSubresource(&v2.HelmRelease{}). + WithObjects(ocirepo, obj). + Build(), GetClusterConfig: GetTestClusterConfig, EventRecorder: record.NewFakeRecorder(32), } - _, err = r.reconcileRelease(context.TODO(), patch.NewSerialPatcher(obj, c), obj) + _, err = r.reconcileRelease(context.TODO(), patch.NewSerialPatcher(obj, r.Client), obj) g.Expect(err).To(HaveOccurred()) - g.Expect(err.Error()).To(ContainSubstring("namespaces \"not-exist\" not found")) + g.Expect(err.Error()).To(ContainSubstring("namespaces \"mock\" not found")) // Verify attempted values are set. g.Expect(obj.Status.LastAttemptedGeneration).To(Equal(obj.Generation)) - g.Expect(obj.Status.LastAttemptedRevision).To(Equal(chartMock.Metadata.Version)) - g.Expect(obj.Status.LastAttemptedConfigDigest).To(Equal("sha256:1dabc4e3cbbd6a0818bd460f3a6c9855bfe95d506c74726bc0f2edb0aecb1f4e")) + dig := strings.Split(chartArtifact.Revision, ":")[1][0:12] + g.Expect(obj.Status.LastAttemptedRevision).To(Equal(chartMock.Metadata.Version + "+" + dig)) + g.Expect(obj.Status.LastAttemptedConfigDigest).To(Equal("sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855")) + g.Expect(obj.Status.LastAttemptedValuesChecksum).To(BeEmpty()) + + // change the chart revision to simulate a new digest + chartArtifact.Revision = chartMock.Metadata.Version + "@" + "sha256:adebc5e3cbcd6a0918bd470f3a6c9855bfe95d506c74726bc0f2edb0aecb1f4e" + ocirepo.Status.Artifact = chartArtifact + r.Client.Update(context.Background(), ocirepo) + + _, err = r.reconcileRelease(context.TODO(), patch.NewSerialPatcher(obj, r.Client), obj) + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(ContainSubstring("namespaces \"mock\" not found")) + + // Verify attempted values are set. + g.Expect(obj.Status.LastAttemptedGeneration).To(Equal(obj.Generation)) + dig = strings.Split(chartArtifact.Revision, ":")[1][0:12] + g.Expect(obj.Status.LastAttemptedRevision).To(Equal(chartMock.Metadata.Version + "+" + dig)) + g.Expect(obj.Status.LastAttemptedConfigDigest).To(Equal("sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855")) g.Expect(obj.Status.LastAttemptedValuesChecksum).To(BeEmpty()) }) - t.Run("error recovery updates ready condition", func(t *testing.T) { + t.Run("upgrade by switching from existing HelmChat to chartRef", func(t *testing.T) { g := NewWithT(t) - // Create a test namespace for storing the Helm release mock. - ns, err := testEnv.CreateNamespace(context.TODO(), "error-recovery") - g.Expect(err).ToNot(HaveOccurred()) - t.Cleanup(func() { - _ = testEnv.Delete(context.TODO(), ns) - }) - // Create HelmChart mock. chartMock := testutil.BuildChart() chartArtifact, err := testutil.SaveChartAsArtifact(chartMock, digest.SHA256, testServer.URL(), testServer.Root()) g.Expect(err).ToNot(HaveOccurred()) + // copy the artifact to mutate the revision + ociArtifact := chartArtifact.DeepCopy() + ociArtifact.Revision += "@" + chartArtifact.Digest - chart := &sourcev1b2.HelmChart{ + ns, err := testEnv.CreateNamespace(context.TODO(), "mock") + g.Expect(err).ToNot(HaveOccurred()) + t.Cleanup(func() { + _ = testEnv.Delete(context.TODO(), ns) + }) + + // hc is the HelmChart object created by the HelmRelease object. + hc := &sourcev1b2.HelmChart{ ObjectMeta: metav1.ObjectMeta{ - Name: "error-recovery", + Name: "chart", Namespace: ns.Name, Generation: 1, }, @@ -792,7 +1334,7 @@ func TestHelmReleaseReconciler_reconcileRelease(t *testing.T) { Version: "0.1.0", SourceRef: sourcev1b2.LocalHelmChartSourceReference{ Kind: sourcev1b2.HelmRepositoryKind, - Name: "error-recovery", + Name: "test-helmrepo", }, }, Status: sourcev1b2.HelmChartStatus{ @@ -807,84 +1349,99 @@ func TestHelmReleaseReconciler_reconcileRelease(t *testing.T) { }, } + // ocirepo is the chartRef object to switch to. + ocirepo := &sourcev1b2.OCIRepository{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ocirepo", + Namespace: ns.Name, + Generation: 1, + }, + Spec: sourcev1b2.OCIRepositorySpec{ + URL: "oci://test-example.com", + Interval: metav1.Duration{Duration: 1 * time.Second}, + }, + Status: sourcev1b2.OCIRepositoryStatus{ + ObservedGeneration: 1, + Artifact: ociArtifact, + Conditions: []metav1.Condition{ + { + Type: meta.ReadyCondition, + Status: metav1.ConditionTrue, + }, + }, + }, + } + // Create a test Helm release storage mock. rls := testutil.BuildRelease(&helmrelease.MockReleaseOptions{ - Name: "error-recovery", + Name: "release", Namespace: ns.Name, Version: 1, Chart: chartMock, Status: helmrelease.StatusDeployed, - }, testutil.ReleaseWithConfig(nil)) + }) obj := &v2.HelmRelease{ ObjectMeta: metav1.ObjectMeta{ - Name: "error-recovery", + Name: "release", Namespace: ns.Name, }, Spec: v2.HelmReleaseSpec{ - StorageNamespace: ns.Name, + ChartRef: &v2.CrossNamespaceSourceReference{ + Kind: "OCIRepository", + Name: "ocirepo", + }, }, Status: v2.HelmReleaseStatus{ - HelmChart: chart.Namespace + "/" + chart.Name, + StorageNamespace: ns.Name, History: v2.Snapshots{ release.ObservedToSnapshot(release.ObserveRelease(rls)), }, + HelmChart: hc.Namespace + "/" + hc.Name, }, } c := fake.NewClientBuilder(). WithScheme(NewTestScheme()). WithStatusSubresource(&v2.HelmRelease{}). - WithObjects(chart, obj). + WithObjects(hc, ocirepo, obj). Build() r := &HelmReleaseReconciler{ Client: c, GetClusterConfig: GetTestClusterConfig, EventRecorder: record.NewFakeRecorder(32), - FieldManager: "test", } // Store the Helm release mock in the test namespace. getter, err := r.buildRESTClientGetter(context.TODO(), obj) g.Expect(err).ToNot(HaveOccurred()) - cfg, err := action.NewConfigFactory(getter, action.WithStorage(helmdriver.SecretsDriverName, obj.GetStorageNamespace())) + cfg, err := action.NewConfigFactory(getter, action.WithStorage(helmdriver.SecretsDriverName, obj.Status.StorageNamespace)) g.Expect(err).ToNot(HaveOccurred()) store := helmstorage.Init(cfg.Driver) g.Expect(store.Create(rls)).To(Succeed()) - sp := patch.NewSerialPatcher(obj, r.Client) - - // List of failure reasons to test. - prereqFailures := []string{ - v2.DependencyNotReadyReason, - aclv1.AccessDeniedReason, - v2.ArtifactFailedReason, - "HelmChartNotReady", - "ValuesError", - "RESTClientError", - "FactoryError", - } - - // Update ready condition for each failure, reconcile and check if the - // stale failure condition gets updated. - for _, failReason := range prereqFailures { - conditions.MarkFalse(obj, meta.ReadyCondition, failReason, "foo") - err := sp.Patch(context.TODO(), obj, - patch.WithOwnedConditions{Conditions: intreconcile.OwnedConditions}, - patch.WithFieldOwner(r.FieldManager), - ) - g.Expect(err).ToNot(HaveOccurred()) + _, err = r.reconcileRelease(context.TODO(), patch.NewSerialPatcher(obj, r.Client), obj) + g.Expect(err).ToNot(HaveOccurred()) - _, err = r.reconcileRelease(context.TODO(), sp, obj) - g.Expect(err).ToNot(HaveOccurred()) + // Verify attempted values are set. + g.Expect(obj.Status.LastAttemptedGeneration).To(Equal(obj.Generation)) + dig := strings.Split(ociArtifact.Revision, ":")[1][0:12] + g.Expect(obj.Status.LastAttemptedRevision).To(Equal(chartMock.Metadata.Version + "+" + dig)) + g.Expect(obj.Status.LastAttemptedConfigDigest).To(Equal("sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855")) + g.Expect(obj.Status.LastAttemptedValuesChecksum).To(BeEmpty()) - ready := conditions.Get(obj, meta.ReadyCondition) - g.Expect(ready.Status).To(Equal(metav1.ConditionUnknown)) - g.Expect(ready.Reason).To(Equal(meta.ProgressingReason)) - } + // verify upgrade succeeded + g.Expect(obj.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{ + *conditions.TrueCondition(meta.ReadyCondition, v2.UpgradeSucceededReason, "Helm upgrade succeeded for release %s with chart %s", + fmt.Sprintf("%s/%s.v%d", rls.Namespace, rls.Name, rls.Version+1), fmt.Sprintf("%s@%s", chartMock.Name(), + fmt.Sprintf("%s+%s", chartMock.Metadata.Version, dig))), + *conditions.TrueCondition(v2.ReleasedCondition, v2.UpgradeSucceededReason, "Helm upgrade succeeded for release %s with chart %s", + fmt.Sprintf("%s/%s.v%d", rls.Namespace, rls.Name, rls.Version+1), fmt.Sprintf("%s@%s", chartMock.Name(), + fmt.Sprintf("%s+%s", chartMock.Metadata.Version, dig))), + })) }) } @@ -1020,7 +1577,7 @@ func TestHelmReleaseReconciler_reconcileDelete(t *testing.T) { }) } -func TestHelmReleaseReconciler_reconileReleaseDeletion(t *testing.T) { +func TestHelmReleaseReconciler_reconcileReleaseDeletion(t *testing.T) { t.Run("uninstalls Helm release", func(t *testing.T) { g := NewWithT(t) @@ -2034,14 +2591,16 @@ func TestHelmReleaseReconciler_getHelmChart(t *testing.T) { intacl.AllowCrossNamespaceRef = !tt.disallowCrossNS t.Cleanup(func() { intacl.AllowCrossNamespaceRef = !curAllow }) - got, err := r.getHelmChart(context.TODO(), tt.rel) + got, err := r.getSource(context.TODO(), tt.rel) if tt.wantErr { g.Expect(err).To(HaveOccurred()) g.Expect(got).To(BeNil()) return } g.Expect(err).ToNot(HaveOccurred()) - expect := g.Expect(got.ObjectMeta) + hc, ok := got.(*sourcev1b2.HelmChart) + g.Expect(ok).To(BeTrue()) + expect := g.Expect(hc.ObjectMeta) if tt.expectChart { expect.To(BeEquivalentTo(tt.chart.ObjectMeta)) } else { @@ -2329,7 +2888,13 @@ func TestValuesReferenceValidation(t *testing.T) { func Test_isHelmChartReady(t *testing.T) { mock := &sourcev1b2.HelmChart{ + TypeMeta: metav1.TypeMeta{ + Kind: "HelmChart", + APIVersion: sourcev1b2.GroupVersion.String(), + }, ObjectMeta: metav1.ObjectMeta{ + Name: "mock", + Namespace: "default", Generation: 2, }, Status: sourcev1b2.HelmChartStatus{ @@ -2363,7 +2928,7 @@ func Test_isHelmChartReady(t *testing.T) { return m }(), want: false, - wantReason: "latest generation of object has not been reconciled", + wantReason: "HelmChart 'default/mock' is not ready: latest generation of object has not been reconciled", }, { name: "chart generation differs from observed generation while Ready=False", @@ -2374,7 +2939,7 @@ func Test_isHelmChartReady(t *testing.T) { return m }(), want: false, - wantReason: "some reason", + wantReason: "HelmChart 'default/mock' is not ready: some reason", }, { name: "chart has Stalled=True", @@ -2385,7 +2950,7 @@ func Test_isHelmChartReady(t *testing.T) { return m }(), want: false, - wantReason: "some stalled reason", + wantReason: "HelmChart 'default/mock' is not ready: some stalled reason", }, { name: "chart does not have an Artifact", @@ -2395,12 +2960,12 @@ func Test_isHelmChartReady(t *testing.T) { return m }(), want: false, - wantReason: "does not have an artifact", + wantReason: "HelmChart 'default/mock' is not ready: does not have an artifact", }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, gotReason := isHelmChartReady(tt.obj) + got, gotReason := isReady(tt.obj, tt.obj.GetArtifact()) if got != tt.want { t.Errorf("isHelmChartReady() got = %v, want %v", got, tt.want) } @@ -2410,3 +2975,161 @@ func Test_isHelmChartReady(t *testing.T) { }) } } + +func Test_isOCIRepositoryReady(t *testing.T) { + mock := &sourcev1b2.OCIRepository{ + TypeMeta: metav1.TypeMeta{ + Kind: "OCIRepository", + APIVersion: sourcev1b2.GroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "mock", + Namespace: "default", + Generation: 2, + }, + Status: sourcev1b2.OCIRepositoryStatus{ + ObservedGeneration: 2, + Conditions: []metav1.Condition{ + { + Type: meta.ReadyCondition, + Status: metav1.ConditionTrue, + }, + }, + Artifact: &sourcev1.Artifact{}, + }, + } + + tests := []struct { + name string + obj *sourcev1b2.OCIRepository + want bool + wantReason string + }{ + { + name: "OCIRepository is ready", + obj: mock.DeepCopy(), + want: true, + }, + { + name: "OCIRepository generation differs from observed generation while Ready=True", + obj: func() *sourcev1b2.OCIRepository { + m := mock.DeepCopy() + m.Generation = 3 + return m + }(), + want: false, + wantReason: "OCIRepository 'default/mock' is not ready: latest generation of object has not been reconciled", + }, + { + name: "OCIRepository generation differs from observed generation while Ready=False", + obj: func() *sourcev1b2.OCIRepository { + m := mock.DeepCopy() + m.Generation = 3 + conditions.MarkFalse(m, meta.ReadyCondition, "Reason", "some reason") + return m + }(), + want: false, + wantReason: "OCIRepository 'default/mock' is not ready: some reason", + }, + { + name: "OCIRepository has Stalled=True", + obj: func() *sourcev1b2.OCIRepository { + m := mock.DeepCopy() + conditions.MarkFalse(m, meta.ReadyCondition, "Reason", "some reason") + conditions.MarkStalled(m, "Reason", "some stalled reason") + return m + }(), + want: false, + wantReason: "OCIRepository 'default/mock' is not ready: some stalled reason", + }, + { + name: "OCIRepository does not have an Artifact", + obj: func() *sourcev1b2.OCIRepository { + m := mock.DeepCopy() + m.Status.Artifact = nil + return m + }(), + want: false, + wantReason: "OCIRepository 'default/mock' is not ready: does not have an artifact", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, gotReason := isReady(tt.obj, tt.obj.GetArtifact()) + if got != tt.want { + t.Errorf("isOCIRepositoryReady() got = %v, want %v", got, tt.want) + } + if gotReason != tt.wantReason { + t.Errorf("isOCIRepositoryReady() reason = %v, want %v", gotReason, tt.wantReason) + } + }) + } +} + +func Test_TryMutateChartWithSourceRevision(t *testing.T) { + tests := []struct { + name string + version string + revision string + wantVersion string + wantErr bool + }{ + { + name: "valid version and revision", + version: "1.2.3", + revision: "1.2.3@sha256:9933f58f8bf459eb199d59ebc8a05683f3944e1242d9f5467d99aa2cf08a5370", + wantVersion: "1.2.3+9933f58f8bf4", + wantErr: false, + }, + { + name: "valid version and invalid revision", + version: "1.2.3", + revision: "1.2.4@sha256:9933f58f8bf459eb199d59ebc8a05683f3944e1242d9f5467d99aa2cf08a5370", + wantVersion: "", + wantErr: true, + }, + { + name: "valid version and revision without version", + version: "1.2.3", + revision: "sha256:9933f58f8bf459eb199d59ebc8a05683f3944e1242d9f5467d99aa2cf08a5370", + wantVersion: "1.2.3+9933f58f8bf4", + wantErr: false, + }, + { + name: "invalid version", + version: "sha:123456", + revision: "1.2.3@sha:123456", + wantVersion: "", + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + c := &chart.Chart{ + Metadata: &chart.Metadata{ + Version: tt.version, + }, + } + + s := &sourcev1b2.OCIRepository{ + Status: sourcev1b2.OCIRepositoryStatus{ + Artifact: &sourcev1.Artifact{ + Revision: tt.revision, + }, + }, + } + + _, err := mutateChartWithSourceRevision(c, s) + if tt.wantErr { + g.Expect(err).To(HaveOccurred()) + } else { + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(c.Metadata.Version).To(Equal(tt.wantVersion)) + } + }) + } + +} diff --git a/internal/reconcile/correct_cluster_drift.go b/internal/reconcile/correct_cluster_drift.go index bbd6f7a31..a6b4c8891 100644 --- a/internal/reconcile/correct_cluster_drift.go +++ b/internal/reconcile/correct_cluster_drift.go @@ -99,10 +99,10 @@ func (r *CorrectClusterDrift) report(obj *v2.HelmRelease, changeSet *ssa.ChangeS sb.WriteString(changeSet.String()) } - r.eventRecorder.AnnotatedEventf(obj, eventMeta(cur.ChartVersion, cur.ConfigDigest), corev1.EventTypeWarning, + r.eventRecorder.AnnotatedEventf(obj, eventMeta(cur.ChartVersion, cur.ConfigDigest, addOCIDigest(cur.OCIDigest)), corev1.EventTypeWarning, "DriftCorrectionFailed", sb.String()) case changeSet != nil && len(changeSet.Entries) > 0: - r.eventRecorder.AnnotatedEventf(obj, eventMeta(cur.ChartVersion, cur.ConfigDigest), corev1.EventTypeNormal, + r.eventRecorder.AnnotatedEventf(obj, eventMeta(cur.ChartVersion, cur.ConfigDigest, addOCIDigest(cur.OCIDigest)), corev1.EventTypeNormal, "DriftCorrected", "Cluster state of release %s has been corrected:\n%s", obj.Status.History.Latest().FullReleaseName(), changeSet.String()) } diff --git a/internal/reconcile/helmchart_template.go b/internal/reconcile/helmchart_template.go index 5bf5a9630..4769c21dd 100644 --- a/internal/reconcile/helmchart_template.go +++ b/internal/reconcile/helmchart_template.go @@ -95,6 +95,20 @@ func (r *HelmChartTemplate) Reconcile(ctx context.Context, req *Request) error { } } + if mustCleanDeployedChart(obj) { + // If the HelmRelease has a ChartRef and no Chart template, and the + // HelmChart is present, we need to clean it up. + if err := r.reconcileDelete(ctx, req.Object); err != nil { + return err + } + return nil + } + + if obj.HasChartRef() { + // if a chartRef is present, we do not need to reconcile the HelmChart from the template. + return nil + } + // Confirm we are allowed to fetch the HelmChart. if err := acl.AllowsAccessTo(req.Object, sourcev1.HelmChartKind, chartRef); err != nil { return err @@ -230,3 +244,13 @@ func buildHelmChartFromTemplate(obj *v2.HelmRelease) *sourcev1.HelmChart { } return result } + +func mustCleanDeployedChart(obj *v2.HelmRelease) bool { + if obj.HasChartRef() && !obj.HasChartTemplate() { + if obj.Status.HelmChart != "" { + return true + } + } + + return false +} diff --git a/internal/reconcile/helmchart_template_test.go b/internal/reconcile/helmchart_template_test.go index 0385997ff..5265d5778 100644 --- a/internal/reconcile/helmchart_template_test.go +++ b/internal/reconcile/helmchart_template_test.go @@ -442,6 +442,60 @@ func TestHelmChartTemplate_Reconcile(t *testing.T) { g.Expect(err).To(HaveOccurred()) g.Expect(apierrors.IsNotFound(err)).To(BeTrue()) }) + + t.Run("Spec ChartRef and existing chart trigger delete", func(t *testing.T) { + g := NewWithT(t) + + releaseName := "garbage-collection" + existingChart := sourcev1.HelmChart{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: namespace.GetName(), + Name: fmt.Sprintf("%s-%s", namespace.GetName(), releaseName), + Labels: map[string]string{ + v2.GroupVersion.Group + "/name": releaseName, + v2.GroupVersion.Group + "/namespace": namespace.GetName(), + }, + }, + Spec: sourcev1.HelmChartSpec{ + Chart: "./bar", + SourceRef: sourcev1.LocalHelmChartSourceReference{ + Kind: sourcev1.HelmRepositoryKind, + Name: "bar-repository", + }, + }, + } + g.Expect(testEnv.CreateAndWait(context.TODO(), &existingChart)).To(Succeed()) + t.Cleanup(func() { + g.Expect(testEnv.Cleanup(context.Background(), &existingChart)).To(Succeed()) + }) + + recorder := record.NewFakeRecorder(32) + r := &HelmChartTemplate{ + client: testEnv, + eventRecorder: recorder, + fieldManager: testFieldManager, + } + + obj := &v2.HelmRelease{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: namespace.GetName(), + Name: releaseName, + }, + Spec: v2.HelmReleaseSpec{ + Interval: metav1.Duration{Duration: 1 * time.Hour}, + ChartRef: &v2.CrossNamespaceSourceReference{ + Kind: sourcev1.OCIRepositoryKind, + Name: "oci-repository", + }, + }, + Status: v2.HelmReleaseStatus{ + HelmChart: fmt.Sprintf("%s/%s", existingChart.GetNamespace(), existingChart.GetName()), + }, + } + err := r.Reconcile(context.TODO(), &Request{Object: obj}) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(obj.Status.HelmChart).To(BeEmpty()) + }) } func TestHelmChartTemplate_reconcileDelete(t *testing.T) { diff --git a/internal/reconcile/install.go b/internal/reconcile/install.go index 0567b549d..0d51a5556 100644 --- a/internal/reconcile/install.go +++ b/internal/reconcile/install.go @@ -92,7 +92,7 @@ func (r *Install) Reconcile(ctx context.Context, req *Request) error { _, err := action.Install(ctx, cfg, req.Object, req.Chart, req.Values) // Record the history of releases observed during the install. - obsReleases.recordOnObject(req.Object) + obsReleases.recordOnObject(req.Object, mutateOCIDigest) if err != nil { r.failure(req, logBuf, err) @@ -154,7 +154,8 @@ func (r *Install) failure(req *Request, buffer *action.LogBuffer, err error) { // Condition summary. r.eventRecorder.AnnotatedEventf( req.Object, - eventMeta(req.Chart.Metadata.Version, chartutil.DigestValues(digest.Canonical, req.Values).String()), + eventMeta(req.Chart.Metadata.Version, chartutil.DigestValues(digest.Canonical, req.Values).String(), + addOCIDigest(req.Object.Status.LastAttemptedRevisionDigest)), corev1.EventTypeWarning, v2.InstallFailedReason, eventMessageWithLog(msg, buffer), @@ -181,7 +182,7 @@ func (r *Install) success(req *Request) { // Record event. r.eventRecorder.AnnotatedEventf( req.Object, - eventMeta(cur.ChartVersion, cur.ConfigDigest), + eventMeta(cur.ChartVersion, cur.ConfigDigest, addOCIDigest(cur.OCIDigest)), corev1.EventTypeNormal, v2.InstallSucceededReason, msg, diff --git a/internal/reconcile/install_test.go b/internal/reconcile/install_test.go index d156e45ca..4b4fe3efe 100644 --- a/internal/reconcile/install_test.go +++ b/internal/reconcile/install_test.go @@ -307,6 +307,9 @@ func TestInstall_failure(t *testing.T) { ReleaseName: mockReleaseName, TargetNamespace: mockReleaseNamespace, }, + Status: v2.HelmReleaseStatus{ + LastAttemptedRevisionDigest: "sha256:1234567890", + }, } chrt = testutil.BuildChart() err = errors.New("installation error") @@ -337,6 +340,7 @@ func TestInstall_failure(t *testing.T) { Message: expectMsg, ObjectMeta: metav1.ObjectMeta{ Annotations: map[string]string{ + eventMetaGroupKey(metaOCIDigestKey): obj.Status.LastAttemptedRevisionDigest, eventMetaGroupKey(eventv1.MetaRevisionKey): chrt.Metadata.Version, eventMetaGroupKey(eventv1.MetaTokenKey): chartutil.DigestValues(digest.Canonical, req.Values).String(), }, diff --git a/internal/reconcile/release.go b/internal/reconcile/release.go index ce0a74d50..825d81291 100644 --- a/internal/reconcile/release.go +++ b/internal/reconcile/release.go @@ -43,6 +43,10 @@ var ( ErrReleaseMismatch = errors.New("release mismatch") ) +// mutateObservedRelease is a function that mutates the Observation with the +// given HelmRelease object. +type mutateObservedRelease func(*v2.HelmRelease, release.Observation) release.Observation + // observedReleases is a map of Helm releases as observed to be written to the // Helm storage. The key is the version of the release. type observedReleases map[int]release.Observation @@ -58,7 +62,7 @@ func (r observedReleases) sortedVersions() (versions []int) { } // recordOnObject records the observed releases on the HelmRelease object. -func (r observedReleases) recordOnObject(obj *v2.HelmRelease) { +func (r observedReleases) recordOnObject(obj *v2.HelmRelease, mutators ...mutateObservedRelease) { switch len(r) { case 0: return @@ -67,17 +71,25 @@ func (r observedReleases) recordOnObject(obj *v2.HelmRelease) { for _, o := range r { obs = o } + for _, mut := range mutators { + obs = mut(obj, obs) + } obj.Status.History = append(v2.Snapshots{release.ObservedToSnapshot(obs)}, obj.Status.History...) default: versions := r.sortedVersions() - - obj.Status.History = append(v2.Snapshots{release.ObservedToSnapshot(r[versions[0]])}, obj.Status.History...) + obs := r[versions[0]] + for _, mut := range mutators { + obs = mut(obj, obs) + } + obj.Status.History = append(v2.Snapshots{release.ObservedToSnapshot(obs)}, obj.Status.History...) for _, ver := range versions[1:] { for i := range obj.Status.History { snap := obj.Status.History[i] if snap.Targets(r[ver].Name, r[ver].Namespace, r[ver].Version) { - newSnap := release.ObservedToSnapshot(r[ver]) + obs := r[ver] + obs.OCIDigest = snap.OCIDigest + newSnap := release.ObservedToSnapshot(obs) newSnap.SetTestHooks(snap.GetTestHooks()) obj.Status.History[i] = newSnap return @@ -87,6 +99,17 @@ func (r observedReleases) recordOnObject(obj *v2.HelmRelease) { } } +func mutateOCIDigest(obj *v2.HelmRelease, obs release.Observation) release.Observation { + obs.OCIDigest = obj.Status.LastAttemptedRevisionDigest + return obs +} + +func releaseToObservation(rls *helmrelease.Release, snapshot *v2.Snapshot) release.Observation { + obs := release.ObserveRelease(rls) + obs.OCIDigest = snapshot.OCIDigest + return obs +} + // observeRelease returns a storage.ObserveFunc that stores the observed // releases in the given observedReleases map. // It can be used for Helm actions that modify multiple releases in the @@ -174,9 +197,15 @@ func eventMessageWithLog(msg string, log *action.LogBuffer) string { return msg } +// addMeta is a function that adds metadata to an event map. +type addMeta func(map[string]string) + +// metaOCIDigestKey is the key for the OCI digest metadata. +const metaOCIDigestKey = "oci-digest" + // eventMeta returns the event (annotation) metadata based on the given // parameters. -func eventMeta(revision, token string) map[string]string { +func eventMeta(revision, token string, metas ...addMeta) map[string]string { var metadata map[string]string if revision != "" || token != "" { metadata = make(map[string]string) @@ -187,9 +216,25 @@ func eventMeta(revision, token string) map[string]string { metadata[eventMetaGroupKey(eventv1.MetaTokenKey)] = token } } + + for _, add := range metas { + add(metadata) + } + return metadata } +func addOCIDigest(digest string) addMeta { + return func(m map[string]string) { + if digest != "" { + if m == nil { + m = make(map[string]string) + } + m[eventMetaGroupKey(metaOCIDigestKey)] = digest + } + } +} + // eventMetaGroupKey returns the event (annotation) metadata key prefixed with // the group. func eventMetaGroupKey(key string) string { diff --git a/internal/reconcile/release_test.go b/internal/reconcile/release_test.go index 167b8df72..66ca6c9b2 100644 --- a/internal/reconcile/release_test.go +++ b/internal/reconcile/release_test.go @@ -17,10 +17,12 @@ limitations under the License. package reconcile import ( + "fmt" "testing" "github.com/go-logr/logr" . "github.com/onsi/gomega" + "helm.sh/helm/v3/pkg/chart" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/fluxcd/pkg/apis/meta" @@ -454,3 +456,179 @@ func mockLogBuffer(size int, lines int) *action.LogBuffer { } return log } + +func Test_RecordOnObject(t *testing.T) { + tests := []struct { + name string + obj *v2.HelmRelease + r observedReleases + mutate bool + testFunc func(*v2.HelmRelease) error + }{ + { + name: "record observed releases", + obj: &v2.HelmRelease{ + ObjectMeta: metav1.ObjectMeta{ + Name: mockReleaseName, + Namespace: mockReleaseNamespace, + }, + }, + r: observedReleases{ + 1: { + Name: mockReleaseName, + Version: 1, + ChartMetadata: chart.Metadata{ + Name: mockReleaseName, + Version: "1.0.0", + }, + }, + }, + testFunc: func(obj *v2.HelmRelease) error { + if len(obj.Status.History) != 1 { + return fmt.Errorf("history length is not 1") + } + if obj.Status.History[0].Name != mockReleaseName { + return fmt.Errorf("release name is not %s", mockReleaseName) + } + return nil + }, + }, + { + name: "record observed releases with multiple versions", + obj: &v2.HelmRelease{ + ObjectMeta: metav1.ObjectMeta{ + Name: mockReleaseName, + Namespace: mockReleaseNamespace, + }, + }, + r: observedReleases{ + 1: { + Name: mockReleaseName, + Version: 1, + ChartMetadata: chart.Metadata{ + Name: mockReleaseName, + Version: "1.0.0", + }, + }, + 2: { + Name: mockReleaseName, + Version: 2, + ChartMetadata: chart.Metadata{ + Name: mockReleaseName, + Version: "2.0.0", + }, + }, + }, + testFunc: func(obj *v2.HelmRelease) error { + if len(obj.Status.History) != 1 { + return fmt.Errorf("want history length 1, got %d", len(obj.Status.History)) + } + if obj.Status.History[0].Name != mockReleaseName { + return fmt.Errorf("release name is not %s", mockReleaseName) + } + if obj.Status.History[0].ChartVersion != "2.0.0" { + return fmt.Errorf("want chart version %s, got %s", "2.0.0", obj.Status.History[0].ChartVersion) + } + return nil + }, + }, + { + name: "record observed releases with status digest", + obj: &v2.HelmRelease{ + ObjectMeta: metav1.ObjectMeta{ + Name: mockReleaseName, + Namespace: mockReleaseNamespace, + }, + Status: v2.HelmReleaseStatus{ + LastAttemptedRevisionDigest: "sha256:123456", + }, + }, + r: observedReleases{ + 1: { + Name: mockReleaseName, + Version: 1, + ChartMetadata: chart.Metadata{ + Name: mockReleaseName, + Version: "1.0.0", + }, + }, + }, + mutate: true, + testFunc: func(obj *v2.HelmRelease) error { + h := obj.Status.History.Latest() + if h.Name != mockReleaseName { + return fmt.Errorf("release name is not %s", mockReleaseName) + } + if h.ChartVersion != "1.0.0" { + return fmt.Errorf("want chart version %s, got %s", "1.0.0", h.ChartVersion) + } + if h.OCIDigest != obj.Status.LastAttemptedRevisionDigest { + return fmt.Errorf("want digest %s, got %s", obj.Status.LastAttemptedRevisionDigest, h.OCIDigest) + } + return nil + }, + }, + { + name: "record observed releases with multiple versions and status digest", + obj: &v2.HelmRelease{ + ObjectMeta: metav1.ObjectMeta{ + Name: mockReleaseName, + Namespace: mockReleaseNamespace, + }, + Status: v2.HelmReleaseStatus{ + LastAttemptedRevisionDigest: "sha256:123456", + }, + }, + r: observedReleases{ + 1: { + Name: mockReleaseName, + Version: 1, + ChartMetadata: chart.Metadata{ + Name: mockReleaseName, + Version: "1.0.0", + }, + }, + 2: { + Name: mockReleaseName, + Version: 2, + ChartMetadata: chart.Metadata{ + Name: mockReleaseName, + Version: "2.0.0", + }, + }, + }, + mutate: true, + testFunc: func(obj *v2.HelmRelease) error { + if len(obj.Status.History) != 1 { + return fmt.Errorf("want history length 1, got %d", len(obj.Status.History)) + } + h := obj.Status.History.Latest() + if h.Name != mockReleaseName { + return fmt.Errorf("release name is not %s", mockReleaseName) + } + if h.ChartVersion != "2.0.0" { + return fmt.Errorf("want chart version %s, got %s", "2.0.0", h.ChartVersion) + } + if h.OCIDigest != obj.Status.LastAttemptedRevisionDigest { + return fmt.Errorf("want digest %s, got %s", obj.Status.LastAttemptedRevisionDigest, h.OCIDigest) + } + return nil + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + if tt.mutate { + tt.r.recordOnObject(tt.obj, mutateOCIDigest) + } else { + tt.r.recordOnObject(tt.obj) + } + err := tt.testFunc(tt.obj) + g.Expect(err).ToNot(HaveOccurred()) + }) + } + +} diff --git a/internal/reconcile/rollback_remediation.go b/internal/reconcile/rollback_remediation.go index e614f5a30..3fbad7099 100644 --- a/internal/reconcile/rollback_remediation.go +++ b/internal/reconcile/rollback_remediation.go @@ -143,7 +143,7 @@ func (r *RollbackRemediation) failure(req *Request, prev *v2.Snapshot, buffer *a // Condition summary. r.eventRecorder.AnnotatedEventf( req.Object, - eventMeta(prev.ChartVersion, chartutil.DigestValues(digest.Canonical, req.Values).String()), + eventMeta(prev.ChartVersion, chartutil.DigestValues(digest.Canonical, req.Values).String(), addOCIDigest(prev.OCIDigest)), corev1.EventTypeWarning, v2.RollbackFailedReason, eventMessageWithLog(msg, buffer), @@ -162,7 +162,7 @@ func (r *RollbackRemediation) success(req *Request, prev *v2.Snapshot) { // Record event. r.eventRecorder.AnnotatedEventf( req.Object, - eventMeta(prev.ChartVersion, chartutil.DigestValues(digest.Canonical, req.Values).String()), + eventMeta(prev.ChartVersion, chartutil.DigestValues(digest.Canonical, req.Values).String(), addOCIDigest(prev.OCIDigest)), corev1.EventTypeNormal, v2.RollbackSucceededReason, msg, @@ -182,7 +182,7 @@ func observeRollback(obj *v2.HelmRelease) storage.ObserveFunc { for i := range obj.Status.History { snap := obj.Status.History[i] if snap.Targets(rls.Name, rls.Namespace, rls.Version) { - newSnap := release.ObservedToSnapshot(release.ObserveRelease(rls)) + newSnap := release.ObservedToSnapshot(releaseToObservation(rls, snap)) newSnap.SetTestHooks(snap.GetTestHooks()) obj.Status.History[i] = newSnap return diff --git a/internal/reconcile/rollback_remediation_test.go b/internal/reconcile/rollback_remediation_test.go index 2300aefc3..71d0bb134 100644 --- a/internal/reconcile/rollback_remediation_test.go +++ b/internal/reconcile/rollback_remediation_test.go @@ -613,4 +613,47 @@ func Test_observeRollback(t *testing.T) { expect, })) }) + + t.Run("rollback with update to previous deployed with OCI Digest", func(t *testing.T) { + g := NewWithT(t) + + previous := &v2.Snapshot{ + Name: mockReleaseName, + Namespace: mockReleaseNamespace, + Version: 2, + Status: helmrelease.StatusFailed.String(), + OCIDigest: "sha256:fcdc2b0de1581a3633ada4afee3f918f6eaa5b5ab38c3fef03d5b48d3f85d9f6", + } + latest := &v2.Snapshot{ + Name: mockReleaseName, + Namespace: mockReleaseNamespace, + Version: 3, + Status: helmrelease.StatusDeployed.String(), + OCIDigest: "sha256:aedc2b0de1576a3633ada4afee3f918f6eaa5b5ab38c3fef03d5b48d3f85d9f6", + } + + obj := &v2.HelmRelease{ + Status: v2.HelmReleaseStatus{ + History: v2.Snapshots{ + latest, + previous, + }, + }, + } + rls := helmrelease.Mock(&helmrelease.MockReleaseOptions{ + Name: previous.Name, + Namespace: previous.Namespace, + Version: previous.Version, + Status: helmrelease.StatusSuperseded, + }) + obs := release.ObserveRelease(rls) + obs.OCIDigest = "sha256:fcdc2b0de1581a3633ada4afee3f918f6eaa5b5ab38c3fef03d5b48d3f85d9f6" + expect := release.ObservedToSnapshot(obs) + + observeRollback(obj)(rls) + g.Expect(obj.Status.History).To(testutil.Equal(v2.Snapshots{ + latest, + expect, + })) + }) } diff --git a/internal/reconcile/test.go b/internal/reconcile/test.go index f40a3d4cf..56e9882ef 100644 --- a/internal/reconcile/test.go +++ b/internal/reconcile/test.go @@ -145,7 +145,7 @@ func (r *Test) failure(req *Request, err error) { // Condition summary. r.eventRecorder.AnnotatedEventf( req.Object, - eventMeta(cur.ChartVersion, cur.ConfigDigest), + eventMeta(cur.ChartVersion, cur.ConfigDigest, addOCIDigest(cur.OCIDigest)), corev1.EventTypeWarning, v2.TestFailedReason, msg, @@ -181,7 +181,7 @@ func (r *Test) success(req *Request) { // Record event. r.eventRecorder.AnnotatedEventf( req.Object, - eventMeta(cur.ChartVersion, cur.ConfigDigest), + eventMeta(cur.ChartVersion, cur.ConfigDigest, addOCIDigest(cur.OCIDigest)), corev1.EventTypeNormal, v2.TestSucceededReason, msg, @@ -200,7 +200,8 @@ func observeTest(obj *v2.HelmRelease) storage.ObserveFunc { } // Update the latest snapshot with the test result. - tested := release.ObservedToSnapshot(release.ObserveRelease(rls)) + latest := obj.Status.History.Latest() + tested := release.ObservedToSnapshot(releaseToObservation(rls, latest)) tested.SetTestHooks(release.TestHooksFromRelease(rls)) obj.Status.History[0] = tested } diff --git a/internal/reconcile/test_test.go b/internal/reconcile/test_test.go index d97dbe0c9..8ed0bdd8c 100644 --- a/internal/reconcile/test_test.go +++ b/internal/reconcile/test_test.go @@ -376,6 +376,38 @@ func Test_observeTest(t *testing.T) { })) }) + t.Run("test with current OCI Digest", func(t *testing.T) { + g := NewWithT(t) + + obj := &v2.HelmRelease{ + Status: v2.HelmReleaseStatus{ + History: v2.Snapshots{ + &v2.Snapshot{ + Name: mockReleaseName, + Namespace: mockReleaseNamespace, + Version: 1, + OCIDigest: "sha256:fcdc2b0de1581a3633ada4afee3f918f6eaa5b5ab38c3fef03d5b48d3f85d9f6", + }, + }, + }, + } + rls := testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + Name: mockReleaseName, + Namespace: mockReleaseNamespace, + Version: 1, + }, testutil.ReleaseWithHooks(testHookFixtures)) + + obs := release.ObserveRelease(rls) + obs.OCIDigest = "sha256:fcdc2b0de1581a3633ada4afee3f918f6eaa5b5ab38c3fef03d5b48d3f85d9f6" + expect := release.ObservedToSnapshot(obs) + expect.SetTestHooks(release.TestHooksFromRelease(rls)) + + observeTest(obj)(rls) + g.Expect(obj.Status.History).To(testutil.Equal(v2.Snapshots{ + expect, + })) + }) + t.Run("test targeting different version than latest", func(t *testing.T) { g := NewWithT(t) diff --git a/internal/reconcile/uninstall.go b/internal/reconcile/uninstall.go index 8c8158745..f92843499 100644 --- a/internal/reconcile/uninstall.go +++ b/internal/reconcile/uninstall.go @@ -180,7 +180,7 @@ func (r *Uninstall) failure(req *Request, buffer *action.LogBuffer, err error) { // Condition summary. r.eventRecorder.AnnotatedEventf( req.Object, - eventMeta(cur.ChartVersion, cur.ConfigDigest), + eventMeta(cur.ChartVersion, cur.ConfigDigest, addOCIDigest(cur.OCIDigest)), corev1.EventTypeWarning, v2.UninstallFailedReason, eventMessageWithLog(msg, buffer), ) @@ -201,7 +201,7 @@ func (r *Uninstall) success(req *Request) { // Condition summary. r.eventRecorder.AnnotatedEventf( req.Object, - eventMeta(cur.ChartVersion, cur.ConfigDigest), + eventMeta(cur.ChartVersion, cur.ConfigDigest, addOCIDigest(cur.OCIDigest)), corev1.EventTypeNormal, v2.UninstallSucceededReason, msg, @@ -224,7 +224,7 @@ func observeUninstall(obj *v2.HelmRelease) storage.ObserveFunc { for i := range obj.Status.History { snap := obj.Status.History[i] if snap.Targets(rls.Name, rls.Namespace, rls.Version) { - newSnap := release.ObservedToSnapshot(release.ObserveRelease(rls)) + newSnap := release.ObservedToSnapshot(releaseToObservation(rls, snap)) newSnap.SetTestHooks(snap.GetTestHooks()) obj.Status.History[i] = newSnap return diff --git a/internal/reconcile/uninstall_remediation.go b/internal/reconcile/uninstall_remediation.go index 4e244cdc0..eeea2d0fc 100644 --- a/internal/reconcile/uninstall_remediation.go +++ b/internal/reconcile/uninstall_remediation.go @@ -154,7 +154,7 @@ func (r *UninstallRemediation) failure(req *Request, buffer *action.LogBuffer, e // Condition summary. r.eventRecorder.AnnotatedEventf( req.Object, - eventMeta(cur.ChartVersion, cur.ConfigDigest), + eventMeta(cur.ChartVersion, cur.ConfigDigest, addOCIDigest(cur.OCIDigest)), corev1.EventTypeWarning, v2.UninstallFailedReason, eventMessageWithLog(msg, buffer), @@ -175,7 +175,7 @@ func (r *UninstallRemediation) success(req *Request) { // Record event. r.eventRecorder.AnnotatedEventf( req.Object, - eventMeta(cur.ChartVersion, cur.ConfigDigest), + eventMeta(cur.ChartVersion, cur.ConfigDigest, addOCIDigest(cur.OCIDigest)), corev1.EventTypeNormal, v2.UninstallSucceededReason, msg, diff --git a/internal/reconcile/uninstall_test.go b/internal/reconcile/uninstall_test.go index ec0a9e23a..65b118ccc 100644 --- a/internal/reconcile/uninstall_test.go +++ b/internal/reconcile/uninstall_test.go @@ -702,4 +702,36 @@ func Test_observeUninstall(t *testing.T) { current, })) }) + t.Run("uninstall of current with OCI Digest", func(t *testing.T) { + g := NewWithT(t) + + current := &v2.Snapshot{ + Name: mockReleaseName, + Namespace: mockReleaseNamespace, + Version: 1, + Status: helmrelease.StatusDeployed.String(), + OCIDigest: "sha256:fcdc2b0de1581a3633ada4afee3f918f6eaa5b5ab38c3fef03d5b48d3f85d9f6", + } + obj := &v2.HelmRelease{ + Status: v2.HelmReleaseStatus{ + History: v2.Snapshots{ + current, + }, + }, + } + rls := testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + Name: current.Name, + Namespace: current.Namespace, + Version: current.Version, + Status: helmrelease.StatusUninstalled, + }) + obs := release.ObserveRelease(rls) + obs.OCIDigest = "sha256:fcdc2b0de1581a3633ada4afee3f918f6eaa5b5ab38c3fef03d5b48d3f85d9f6" + expect := release.ObservedToSnapshot(obs) + + observeUninstall(obj)(rls) + g.Expect(obj.Status.History).To(testutil.Equal(v2.Snapshots{ + expect, + })) + }) } diff --git a/internal/reconcile/unlock.go b/internal/reconcile/unlock.go index 7d045856c..e6e4da5fa 100644 --- a/internal/reconcile/unlock.go +++ b/internal/reconcile/unlock.go @@ -77,7 +77,7 @@ func (r *Unlock) Reconcile(_ context.Context, req *Request) error { } // Ensure the release is in a pending state. - cur := release.ObservedToSnapshot(release.ObserveRelease(rls)) + cur := processCurrentSnaphot(req.Object, rls) if status := rls.Info.Status; status.IsPending() { // Update pending status to failed and persist. rls.SetStatus(helmrelease.StatusFailed, fmt.Sprintf("Release unlocked from stale '%s' state", status.String())) @@ -119,7 +119,7 @@ func (r *Unlock) failure(req *Request, cur *v2.Snapshot, status helmrelease.Stat // Record warning event. r.eventRecorder.AnnotatedEventf( req.Object, - eventMeta(cur.ChartVersion, cur.ConfigDigest), + eventMeta(cur.ChartVersion, cur.ConfigDigest, addOCIDigest(cur.OCIDigest)), corev1.EventTypeWarning, "PendingRelease", msg, @@ -138,7 +138,7 @@ func (r *Unlock) success(req *Request, cur *v2.Snapshot, status helmrelease.Stat // Record event. r.eventRecorder.AnnotatedEventf( req.Object, - eventMeta(cur.ChartVersion, cur.ConfigDigest), + eventMeta(cur.ChartVersion, cur.ConfigDigest, addOCIDigest(cur.OCIDigest)), corev1.EventTypeNormal, "PendingRelease", msg, @@ -154,9 +154,23 @@ func observeUnlock(obj *v2.HelmRelease) storage.ObserveFunc { for i := range obj.Status.History { snap := obj.Status.History[i] if snap.Targets(rls.Name, rls.Namespace, rls.Version) { - obj.Status.History[i] = release.ObservedToSnapshot(release.ObserveRelease(rls)) + obj.Status.History[i] = release.ObservedToSnapshot(releaseToObservation(rls, snap)) return } } } } + +// processCurrentSnaphot processes the current snapshot based on a Helm release. +// It also looks for the OCIDigest in the corresponding v2.HelmRelease history and +// updates the current snapshot with the OCIDigest if found. +func processCurrentSnaphot(obj *v2.HelmRelease, rls *helmrelease.Release) *v2.Snapshot { + cur := release.ObservedToSnapshot(release.ObserveRelease(rls)) + for i := range obj.Status.History { + snap := obj.Status.History[i] + if snap.Targets(rls.Name, rls.Namespace, rls.Version) { + cur.OCIDigest = snap.OCIDigest + } + } + return cur +} diff --git a/internal/reconcile/unlock_test.go b/internal/reconcile/unlock_test.go index 6799fe198..7b13c8300 100644 --- a/internal/reconcile/unlock_test.go +++ b/internal/reconcile/unlock_test.go @@ -457,6 +457,95 @@ func TestUnlock_success(t *testing.T) { })) } +func TestUnlock_withOCIDigest(t *testing.T) { + g := NewWithT(t) + + namedNS, err := testEnv.CreateNamespace(context.TODO(), mockReleaseNamespace) + g.Expect(err).NotTo(HaveOccurred()) + t.Cleanup(func() { + _ = testEnv.Delete(context.TODO(), namedNS) + }) + releaseNamespace := namedNS.Name + + rls := testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + Name: mockReleaseName, + Namespace: releaseNamespace, + Chart: testutil.BuildChart(), + Version: 4, + Status: helmrelease.StatusPendingInstall, + }) + + obs := release.ObserveRelease(rls) + obs.OCIDigest = "sha256:fcdc2b0de1581a3633ada4afee3f918f6eaa5b5ab38c3fef03d5b48d3f85d9f6" + snap := release.ObservedToSnapshot(obs) + + obj := &v2.HelmRelease{ + Spec: v2.HelmReleaseSpec{ + ReleaseName: mockReleaseName, + TargetNamespace: releaseNamespace, + StorageNamespace: releaseNamespace, + Timeout: &metav1.Duration{Duration: 100 * time.Millisecond}, + }, + Status: v2.HelmReleaseStatus{ + History: v2.Snapshots{ + snap, + }, + }, + } + + getter, err := RESTClientGetterFromManager(testEnv.Manager, obj.GetReleaseNamespace()) + g.Expect(err).ToNot(HaveOccurred()) + + cfg, err := action.NewConfigFactory(getter, + action.WithStorage(action.DefaultStorageDriver, obj.GetStorageNamespace()), + ) + g.Expect(err).ToNot(HaveOccurred()) + + store := helmstorage.Init(cfg.Driver) + g.Expect(store.Create(rls)).To(Succeed()) + + recorder := testutil.NewFakeRecorder(10, false) + got := NewUnlock(cfg, recorder).Reconcile(context.TODO(), &Request{ + Object: obj, + }) + + g.Expect(got).ToNot(HaveOccurred()) + + g.Expect(obj.Status.Conditions).To(conditions.MatchConditions( + []metav1.Condition{ + *conditions.FalseCondition(meta.ReadyCondition, "PendingRelease", "Unlocked Helm release"), + *conditions.FalseCondition(v2.ReleasedCondition, "PendingRelease", "Unlocked Helm release"), + })) + + releases, _ := store.History(mockReleaseName) + helmreleaseutil.SortByRevision(releases) + expected := release.ObserveRelease(releases[0]) + expected.OCIDigest = "sha256:fcdc2b0de1581a3633ada4afee3f918f6eaa5b5ab38c3fef03d5b48d3f85d9f6" + g.Expect(obj.Status.History).To(testutil.Equal(v2.Snapshots{ + release.ObservedToSnapshot(expected), + })) + + expectMsg := fmt.Sprintf(fmtUnlockSuccess, + fmt.Sprintf("%s/%s.v%d", rls.Namespace, snap.Name, snap.Version), + fmt.Sprintf("%s@%s", rls.Chart.Name(), rls.Chart.Metadata.Version), + rls.Info.Status.String()) + + g.Expect(recorder.GetEvents()).To(ConsistOf([]corev1.Event{ + { + Type: corev1.EventTypeNormal, + Reason: "PendingRelease", + Message: expectMsg, + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + eventMetaGroupKey(metaOCIDigestKey): expected.OCIDigest, + eventMetaGroupKey(eventv1.MetaRevisionKey): rls.Chart.Metadata.Version, + eventMetaGroupKey(eventv1.MetaTokenKey): chartutil.DigestValues(digest.Canonical, rls.Config).String(), + }, + }, + }, + })) +} + func Test_observeUnlock(t *testing.T) { t.Run("unlock", func(t *testing.T) { g := NewWithT(t) @@ -487,6 +576,38 @@ func Test_observeUnlock(t *testing.T) { })) }) + t.Run("unlock with OCI Digest", func(t *testing.T) { + g := NewWithT(t) + + obj := &v2.HelmRelease{ + Status: v2.HelmReleaseStatus{ + History: v2.Snapshots{ + { + Name: mockReleaseName, + Namespace: mockReleaseNamespace, + Version: 1, + Status: helmrelease.StatusPendingRollback.String(), + OCIDigest: "sha256:fcdc2b0de1581a3633ada4afee3f918f6eaa5b5ab38c3fef03d5b48d3f85d9f6", + }, + }, + }, + } + rls := helmrelease.Mock(&helmrelease.MockReleaseOptions{ + Name: mockReleaseName, + Namespace: mockReleaseNamespace, + Version: 1, + Status: helmrelease.StatusFailed, + }) + obs := release.ObserveRelease(rls) + obs.OCIDigest = "sha256:fcdc2b0de1581a3633ada4afee3f918f6eaa5b5ab38c3fef03d5b48d3f85d9f6" + expect := release.ObservedToSnapshot(obs) + observeUnlock(obj)(rls) + + g.Expect(obj.Status.History).To(testutil.Equal(v2.Snapshots{ + expect, + })) + }) + t.Run("unlock without current", func(t *testing.T) { g := NewWithT(t) diff --git a/internal/reconcile/upgrade.go b/internal/reconcile/upgrade.go index 8cdbb0828..6a43b79d1 100644 --- a/internal/reconcile/upgrade.go +++ b/internal/reconcile/upgrade.go @@ -83,7 +83,7 @@ func (r *Upgrade) Reconcile(ctx context.Context, req *Request) error { _, err := action.Upgrade(ctx, cfg, req.Object, req.Chart, req.Values) // Record the history of releases observed during the upgrade. - obsReleases.recordOnObject(req.Object) + obsReleases.recordOnObject(req.Object, mutateOCIDigest) if err != nil { r.failure(req, logBuf, err) @@ -144,7 +144,8 @@ func (r *Upgrade) failure(req *Request, buffer *action.LogBuffer, err error) { // Condition summary. r.eventRecorder.AnnotatedEventf( req.Object, - eventMeta(req.Chart.Metadata.Version, chartutil.DigestValues(digest.Canonical, req.Values).String()), + eventMeta(req.Chart.Metadata.Version, chartutil.DigestValues(digest.Canonical, req.Values).String(), + addOCIDigest(req.Object.Status.LastAttemptedRevisionDigest)), corev1.EventTypeWarning, v2.UpgradeFailedReason, eventMessageWithLog(msg, buffer), @@ -171,7 +172,7 @@ func (r *Upgrade) success(req *Request) { // Record event. r.eventRecorder.AnnotatedEventf( req.Object, - eventMeta(cur.ChartVersion, cur.ConfigDigest), + eventMeta(cur.ChartVersion, cur.ConfigDigest, addOCIDigest(cur.OCIDigest)), corev1.EventTypeNormal, v2.UpgradeSucceededReason, msg, diff --git a/internal/reconcile/upgrade_test.go b/internal/reconcile/upgrade_test.go index dbfb85b7c..08bc98632 100644 --- a/internal/reconcile/upgrade_test.go +++ b/internal/reconcile/upgrade_test.go @@ -438,6 +438,9 @@ func TestUpgrade_failure(t *testing.T) { ReleaseName: mockReleaseName, TargetNamespace: mockReleaseNamespace, }, + Status: v2.HelmReleaseStatus{ + LastAttemptedRevisionDigest: "sha256:1234567890", + }, } chrt = testutil.BuildChart() err = errors.New("upgrade error") @@ -468,6 +471,7 @@ func TestUpgrade_failure(t *testing.T) { Message: expectMsg, ObjectMeta: metav1.ObjectMeta{ Annotations: map[string]string{ + eventMetaGroupKey(metaOCIDigestKey): obj.Status.LastAttemptedRevisionDigest, eventMetaGroupKey(eventv1.MetaRevisionKey): chrt.Metadata.Version, eventMetaGroupKey(eventv1.MetaTokenKey): chartutil.DigestValues(digest.Canonical, req.Values).String(), }, diff --git a/internal/release/digest_test.go b/internal/release/digest_test.go index c340ca965..3aca5b8eb 100644 --- a/internal/release/digest_test.go +++ b/internal/release/digest_test.go @@ -37,7 +37,7 @@ func TestDigest(t *testing.T) { rel: Observation{ Name: "foo", }, - exp: "sha256:91b6773f7696d3eb405708a07e2daedc6e69664dabac8e10af7d570d09f947d5", + exp: "sha256:ca8901e499a79368647134cc4f1c2118f8e7ec64c8a4703b281d17fb01acfbed", }, } for _, tt := range tests { diff --git a/internal/release/observation.go b/internal/release/observation.go index 35c0a4340..0afcc0be9 100644 --- a/internal/release/observation.go +++ b/internal/release/observation.go @@ -79,6 +79,8 @@ type Observation struct { Hooks []helmrelease.Hook `json:"hooks"` // Namespace is the Kubernetes namespace of the release. Namespace string `json:"namespace"` + // OCIDigest is the digest of the OCI artifact that was used to + OCIDigest string `json:"ociDigest"` } // Targets returns if the release matches the given name, namespace and @@ -166,6 +168,7 @@ func ObservedToSnapshot(rls Observation) *v2.Snapshot { LastDeployed: metav1.NewTime(rls.Info.LastDeployed.Time), Deleted: metav1.NewTime(rls.Info.Deleted.Time), Status: rls.Info.Status.String(), + OCIDigest: rls.OCIDigest, } }