From 0cfb1431ac510943e23de871e74ec89105f368f1 Mon Sep 17 00:00:00 2001 From: Soule BA Date: Fri, 12 Apr 2024 14:01:15 +0200 Subject: [PATCH] Add a test when switching from chart template to chartRef The test case successfully upgrade with the same chart because version is not computed the same way (12 digits of digest appended for OCIRepository source). Signed-off-by: Soule BA --- docs/spec/v2beta2/helmreleases.md | 20 +-- internal/controller/helmrelease_controller.go | 27 +++- .../controller/helmrelease_controller_test.go | 142 +++++++++++++++++- 3 files changed, 174 insertions(+), 15 deletions(-) diff --git a/docs/spec/v2beta2/helmreleases.md b/docs/spec/v2beta2/helmreleases.md index a8efba435..540c500e3 100644 --- a/docs/spec/v2beta2/helmreleases.md +++ b/docs/spec/v2beta2/helmreleases.md @@ -210,25 +210,27 @@ from which to fetch the Helm chart. The chart is fetched by the controller with 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 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 release when the OCIRepository produces a new artifact. +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. -If switching from `.spec.chart` to `.spec.chartRef`, the HelmRelease will uninstall -the previous release before installing the new one. +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` flag. When this flag is -set, the HelmRelease can only refer to Sources in the same namespace as the +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-ocirepo + name: podinfo + namespace: default spec: interval: 30s url: oci://ghcr.io/stefanprodan/charts/podinfo diff --git a/internal/controller/helmrelease_controller.go b/internal/controller/helmrelease_controller.go index a997a1cab..634557725 100644 --- a/internal/controller/helmrelease_controller.go +++ b/internal/controller/helmrelease_controller.go @@ -891,17 +891,23 @@ func mutateChartWithSourceRevision(chart *chart.Chart, source source.Source) (st } // algotithm are sha256, sha384, sha512 with the canonical being sha256 // So every digest starts with a sha algorithm and a colon - sha := strings.Split(tagD[1], ":")[1] + sha, err := extractDigetString(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[0:12]) + *ver, err = ver.SetMetadata(sha) if err != nil { return "", err } ociDigest = tagD[1] default: // default to the digest - sha := strings.Split(revision, ":")[1] - *ver, err = ver.SetMetadata(sha[0:12]) + sha, err := extractDigetString(revision) + if err != nil { + return "", err + } + *ver, err = ver.SetMetadata(sha) if err != nil { return "", err } @@ -911,3 +917,16 @@ func mutateChartWithSourceRevision(chart *chart.Chart, source source.Source) (st chart.Metadata.Version = ver.String() return ociDigest, nil } + +func extractDigetString(revision string) (string, error) { + var sha string + 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 +} diff --git a/internal/controller/helmrelease_controller_test.go b/internal/controller/helmrelease_controller_test.go index e54b495fc..6cd56bc72 100644 --- a/internal/controller/helmrelease_controller_test.go +++ b/internal/controller/helmrelease_controller_test.go @@ -1299,11 +1299,149 @@ func TestHelmReleaseReconciler_reconcileReleaseFromOCIRepositorySource(t *testin // Verify attempted values are set. g.Expect(obj.Status.LastAttemptedGeneration).To(Equal(obj.Generation)) - dig2 := strings.Split(chartArtifact.Revision, ":")[1][0:12] - g.Expect(obj.Status.LastAttemptedRevision).To(Equal(chartMock.Metadata.Version + "+" + dig2)) + 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("upgrade by switching from existing HelmChat to chartRef", 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()) + // copy the artifact to mutate the revision + ociArtifact := chartArtifact.DeepCopy() + ociArtifact.Revision += "@" + chartArtifact.Digest + + 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: "chart", + Namespace: ns.Name, + Generation: 1, + }, + Spec: sourcev1b2.HelmChartSpec{ + Chart: "testdata/test-helmrepo", + Version: "0.1.0", + SourceRef: sourcev1b2.LocalHelmChartSourceReference{ + Kind: sourcev1b2.HelmRepositoryKind, + Name: "test-helmrepo", + }, + }, + Status: sourcev1b2.HelmChartStatus{ + ObservedGeneration: 1, + Artifact: chartArtifact, + Conditions: []metav1.Condition{ + { + Type: meta.ReadyCondition, + Status: metav1.ConditionTrue, + }, + }, + }, + } + + // 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: "release", + Namespace: ns.Name, + Version: 1, + Chart: chartMock, + Status: helmrelease.StatusDeployed, + }) + + obj := &v2.HelmRelease{ + ObjectMeta: metav1.ObjectMeta{ + Name: "release", + Namespace: ns.Name, + }, + Spec: v2.HelmReleaseSpec{ + ChartRef: &v2.CrossNamespaceSourceReference{ + Kind: "OCIRepository", + Name: "ocirepo", + }, + }, + Status: v2.HelmReleaseStatus{ + 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(hc, ocirepo, obj). + Build() + + r := &HelmReleaseReconciler{ + Client: c, + GetClusterConfig: GetTestClusterConfig, + EventRecorder: record.NewFakeRecorder(32), + } + + // 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.Status.StorageNamespace)) + g.Expect(err).ToNot(HaveOccurred()) + + store := helmstorage.Init(cfg.Driver) + g.Expect(store.Create(rls)).To(Succeed()) + + _, err = r.reconcileRelease(context.TODO(), patch.NewSerialPatcher(obj, r.Client), 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()) + + // 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))), + })) }) }