chore: update tests

Signed-off-by: Robin Breathe <robin@isometry.net>

api/v1beta2/helmchart_types.go

@@ -147,7 +147,9 @@ type HelmChartStatus struct {
 	// +optional
 	ObservedChartName string `json:"observedChartName,omitempty"`
 
-	// ObservedValuesFiles are the last observed value files.
+	// ObservedValuesFiles are the observed value files of the last successful
+	// reconciliation.
+	// It matches the chart in the last successfully reconciled artifact.
 	// +optional
 	ObservedValuesFiles []string `json:"observedValuesFiles,omitempty"`
 

config/crd/bases/source.toolkit.fluxcd.io_helmcharts.yaml

@@ -644,7 +644,10 @@ spec:
                   of the HelmChartSpec.SourceRef.
                 type: string
               observedValuesFiles:
-                description: ObservedValuesFiles are the last observed value files.
+                description: |-
+                  ObservedValuesFiles are the observed value files of the last successful
+                  reconciliation.
+                  It matches the chart in the last successfully reconciled artifact.
                 items:
                   type: string
                 type: array

docs/api/v1beta2/source.md

@@ -2469,7 +2469,9 @@ resolved chart reference.</p>
 </td>
 <td>
 <em>(Optional)</em>
-<p>ObservedValuesFiles are the last observed value files.</p>
+<p>ObservedValuesFiles are the observed value files of the last successful
+reconciliation.
+It matches the chart in the last successfully reconciled artifact.</p>
 </td>
 </tr>
 <tr>

internal/controller/helmchart_controller.go

@@ -666,7 +666,7 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
 	cb := chart.NewRemoteBuilder(chartRepo)
 	opts := chart.BuildOptions{
 		ValuesFiles:              obj.GetValuesFiles(),
-		ObservedValuesFiles:      obj.Status.ObservedValuesFiles,
+		CachedChartValuesFiles:   obj.Status.ObservedValuesFiles,
 		IgnoreMissingValuesFiles: obj.Spec.IgnoreMissingValuesFiles,
 		Force:                    obj.Generation != obj.Status.ObservedGeneration,
 		// The remote builder will not attempt to download the chart if
@@ -763,12 +763,12 @@ func (r *HelmChartReconciler) buildFromTarballArtifact(ctx context.Context, obj
 	// Configure builder options, including any previously cached chart
 	opts := chart.BuildOptions{
 		ValuesFiles:              obj.GetValuesFiles(),
-		ObservedValuesFiles:      obj.Status.ObservedValuesFiles,
 		IgnoreMissingValuesFiles: obj.Spec.IgnoreMissingValuesFiles,
 		Force:                    obj.Generation != obj.Status.ObservedGeneration,
 	}
-	if artifact := obj.Status.Artifact; artifact != nil {
+	if artifact := obj.GetArtifact(); artifact != nil {
 		opts.CachedChart = r.Storage.LocalPath(*artifact)
+		opts.CachedChartValuesFiles = obj.Status.ObservedValuesFiles
 	}
 
 	// Configure revision metadata for chart build if we should react to revision changes

internal/controller/helmchart_controller_test.go

@@ -927,6 +927,23 @@ func TestHelmChartReconciler_buildFromHelmRepository(t *testing.T) {
 				g.Expect(build.Path).To(BeARegularFile())
 			},
 		},
+		{
+			name: "Uses artifact as build cache with observedValuesFiles",
+			beforeFunc: func(obj *helmv1.HelmChart, repository *helmv1.HelmRepository) {
+				obj.Spec.Chart = chartName
+				obj.Spec.Version = chartVersion
+				obj.Status.Artifact = &sourcev1.Artifact{Path: chartName + "-" + chartVersion + ".tgz"}
+				obj.Status.ObservedValuesFiles = []string{"values.yaml", "override.yaml"}
+			},
+			want: sreconcile.ResultSuccess,
+			assertFunc: func(g *WithT, obj *helmv1.HelmChart, build chart.Build) {
+				g.Expect(build.Name).To(Equal(chartName))
+				g.Expect(build.Version).To(Equal(chartVersion))
+				g.Expect(build.Path).To(Equal(filepath.Join(serverFactory.Root(), obj.Status.Artifact.Path)))
+				g.Expect(build.Path).To(BeARegularFile())
+				g.Expect(build.ValuesFiles).To(Equal([]string{"values.yaml", "override.yaml"}))
+			},
+		},
 		{
 			name: "Sets Generation as VersionMetadata with values files",
 			beforeFunc: func(obj *helmv1.HelmChart, repository *helmv1.HelmRepository) {
@@ -940,6 +957,51 @@ func TestHelmChartReconciler_buildFromHelmRepository(t *testing.T) {
 				g.Expect(build.Version).To(Equal(higherChartVersion + "+3"))
 				g.Expect(build.Path).ToNot(BeEmpty())
 				g.Expect(build.Path).To(BeARegularFile())
+				g.Expect(build.ValuesFiles).To(Equal([]string{"values.yaml", "override.yaml"}))
+			},
+			cleanFunc: func(g *WithT, build *chart.Build) {
+				g.Expect(os.Remove(build.Path)).To(Succeed())
+			},
+		},
+		{
+			name: "Missing values files are an error",
+			beforeFunc: func(obj *helmv1.HelmChart, repository *helmv1.HelmRepository) {
+				obj.Spec.Chart = chartName
+				obj.Spec.ValuesFiles = []string{"missing.yaml"}
+			},
+			wantErr: &chart.BuildError{Err: errors.New("values files merge error: failed to merge chart values: no values file found at path 'missing.yaml'")},
+		},
+		{
+			name: "All missing values files ignored",
+			beforeFunc: func(obj *helmv1.HelmChart, repository *helmv1.HelmRepository) {
+				obj.Spec.Chart = chartName
+				obj.Spec.Version = chartVersion
+				obj.Spec.ValuesFiles = []string{"missing.yaml"}
+				obj.Spec.IgnoreMissingValuesFiles = true
+			},
+			want: sreconcile.ResultSuccess,
+			assertFunc: func(g *WithT, obj *helmv1.HelmChart, build chart.Build) {
+				g.Expect(build.Name).To(Equal(chartName))
+				g.Expect(build.Version).To(Equal(chartVersion + "+0"))
+				g.Expect(build.ValuesFiles).To(BeEmpty())
+			},
+			cleanFunc: func(g *WithT, build *chart.Build) {
+				g.Expect(os.Remove(build.Path)).To(Succeed())
+			},
+		},
+		{
+			name: "Partial missing values files ignored",
+			beforeFunc: func(obj *helmv1.HelmChart, repository *helmv1.HelmRepository) {
+				obj.Spec.Chart = chartName
+				obj.Spec.Version = chartVersion
+				obj.Spec.ValuesFiles = []string{"values.yaml", "override.yaml", "invalid.yaml"}
+				obj.Spec.IgnoreMissingValuesFiles = true
+			},
+			want: sreconcile.ResultSuccess,
+			assertFunc: func(g *WithT, obj *helmv1.HelmChart, build chart.Build) {
+				g.Expect(build.Name).To(Equal(chartName))
+				g.Expect(build.Version).To(Equal(chartVersion + "+0"))
+				g.Expect(build.ValuesFiles).To(Equal([]string{"values.yaml", "override.yaml"}))
 			},
 			cleanFunc: func(g *WithT, build *chart.Build) {
 				g.Expect(os.Remove(build.Path)).To(Succeed())
@@ -1211,6 +1273,7 @@ func TestHelmChartReconciler_buildFromOCIHelmRepository(t *testing.T) {
 				g.Expect(build.Version).To(Equal(metadata.Version))
 				g.Expect(build.Path).To(Equal(storage.LocalPath(*cachedArtifact.DeepCopy())))
 				g.Expect(build.Path).To(BeARegularFile())
+				g.Expect(build.ValuesFiles).To(BeEmpty())
 			},
 		},
 		{
@@ -1433,6 +1496,10 @@ func TestHelmChartReconciler_buildFromTarballArtifact(t *testing.T) {
 				g.Expect(build.Version).To(Equal("0.1.0+3"))
 				g.Expect(build.ResolvedDependencies).To(Equal(0))
 				g.Expect(build.Path).To(BeARegularFile())
+				g.Expect(build.ValuesFiles).To(Equal([]string{
+					"testdata/charts/helmchart/values.yaml",
+					"testdata/charts/helmchart/override.yaml",
+				}))
 			},
 			cleanFunc: func(g *WithT, build *chart.Build) {
 				g.Expect(os.Remove(build.Path)).To(Succeed())
@@ -1451,6 +1518,24 @@ func TestHelmChartReconciler_buildFromTarballArtifact(t *testing.T) {
 				g.Expect(build.Version).To(Equal("0.1.0"))
 				g.Expect(build.Path).To(Equal(storage.LocalPath(*cachedArtifact.DeepCopy())))
 				g.Expect(build.Path).To(BeARegularFile())
+				g.Expect(build.ValuesFiles).To(BeEmpty())
+			},
+		},
+		{
+			name:   "Chart from storage cache with ObservedValuesFiles",
+			source: *chartsArtifact.DeepCopy(),
+			beforeFunc: func(obj *helmv1.HelmChart) {
+				obj.Spec.Chart = "testdata/charts/helmchart-0.1.0.tgz"
+				obj.Status.Artifact = cachedArtifact.DeepCopy()
+				obj.Status.ObservedValuesFiles = []string{"values.yaml", "override.yaml"}
+			},
+			want: sreconcile.ResultSuccess,
+			assertFunc: func(g *WithT, build chart.Build) {
+				g.Expect(build.Name).To(Equal("helmchart"))
+				g.Expect(build.Version).To(Equal("0.1.0"))
+				g.Expect(build.Path).To(Equal(storage.LocalPath(*cachedArtifact.DeepCopy())))
+				g.Expect(build.Path).To(BeARegularFile())
+				g.Expect(build.ValuesFiles).To(Equal([]string{"values.yaml", "override.yaml"}))
 			},
 		},
 		{
@@ -1468,6 +1553,7 @@ func TestHelmChartReconciler_buildFromTarballArtifact(t *testing.T) {
 				g.Expect(build.Version).To(Equal("0.1.0"))
 				g.Expect(build.Path).ToNot(Equal(storage.LocalPath(*cachedArtifact.DeepCopy())))
 				g.Expect(build.Path).To(BeARegularFile())
+				g.Expect(build.ValuesFiles).To(BeEmpty())
 			},
 			cleanFunc: func(g *WithT, build *chart.Build) {
 				g.Expect(os.Remove(build.Path)).To(Succeed())
@@ -1565,7 +1651,7 @@ func TestHelmChartReconciler_reconcileArtifact(t *testing.T) {
 		},
 		{
 			name:  "Copying artifact to storage from build makes ArtifactInStorage=True",
-			build: mockChartBuild("helmchart", "0.1.0", "testdata/charts/helmchart-0.1.0.tgz"),
+			build: mockChartBuild("helmchart", "0.1.0", "testdata/charts/helmchart-0.1.0.tgz", nil),
 			beforeFunc: func(obj *helmv1.HelmChart) {
 				conditions.MarkTrue(obj, sourcev1.ArtifactOutdatedCondition, "Foo", "")
 			},
@@ -1575,6 +1661,7 @@ func TestHelmChartReconciler_reconcileArtifact(t *testing.T) {
 				t.Expect(obj.GetArtifact().Revision).To(Equal("0.1.0"))
 				t.Expect(obj.Status.URL).ToNot(BeEmpty())
 				t.Expect(obj.Status.ObservedChartName).To(Equal("helmchart"))
+				t.Expect(obj.Status.ObservedValuesFiles).To(BeEmpty())
 			},
 			want: sreconcile.ResultSuccess,
 			assertConditions: []metav1.Condition{
@@ -1597,6 +1684,7 @@ func TestHelmChartReconciler_reconcileArtifact(t *testing.T) {
 			afterFunc: func(t *WithT, obj *helmv1.HelmChart) {
 				t.Expect(obj.Status.Artifact.Path).To(Equal("testdata/charts/helmchart-0.1.0.tgz"))
 				t.Expect(obj.Status.ObservedChartName).To(BeEmpty())
+				t.Expect(obj.Status.ObservedValuesFiles).To(BeEmpty())
 				t.Expect(obj.Status.URL).To(BeEmpty())
 			},
 		},
@@ -1626,7 +1714,7 @@ func TestHelmChartReconciler_reconcileArtifact(t *testing.T) {
 		},
 		{
 			name:  "Removes ArtifactOutdatedCondition after creating new artifact",
-			build: mockChartBuild("helmchart", "0.1.0", "testdata/charts/helmchart-0.1.0.tgz"),
+			build: mockChartBuild("helmchart", "0.1.0", "testdata/charts/helmchart-0.1.0.tgz", nil),
 			beforeFunc: func(obj *helmv1.HelmChart) {
 				conditions.MarkTrue(obj, sourcev1.ArtifactOutdatedCondition, "Foo", "")
 			},
@@ -1636,6 +1724,7 @@ func TestHelmChartReconciler_reconcileArtifact(t *testing.T) {
 				t.Expect(obj.GetArtifact().Revision).To(Equal("0.1.0"))
 				t.Expect(obj.Status.URL).ToNot(BeEmpty())
 				t.Expect(obj.Status.ObservedChartName).To(Equal("helmchart"))
+				t.Expect(obj.Status.ObservedValuesFiles).To(BeEmpty())
 			},
 			want: sreconcile.ResultSuccess,
 			assertConditions: []metav1.Condition{
@@ -1644,7 +1733,7 @@ func TestHelmChartReconciler_reconcileArtifact(t *testing.T) {
 		},
 		{
 			name:  "Creates latest symlink to the created artifact",
-			build: mockChartBuild("helmchart", "0.1.0", "testdata/charts/helmchart-0.1.0.tgz"),
+			build: mockChartBuild("helmchart", "0.1.0", "testdata/charts/helmchart-0.1.0.tgz", nil),
 			afterFunc: func(t *WithT, obj *helmv1.HelmChart) {
 				t.Expect(obj.GetArtifact()).ToNot(BeNil())
 
@@ -1659,6 +1748,25 @@ func TestHelmChartReconciler_reconcileArtifact(t *testing.T) {
 				*conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, helmv1.ChartPullSucceededReason, "pulled 'helmchart' chart with version '0.1.0'"),
 			},
 		},
+		{
+			name:  "Updates ObservedValuesFiles after creating new artifact",
+			build: mockChartBuild("helmchart", "0.1.0", "testdata/charts/helmchart-0.1.0.tgz", []string{"values.yaml", "override.yaml"}),
+			beforeFunc: func(obj *helmv1.HelmChart) {
+				conditions.MarkTrue(obj, sourcev1.ArtifactOutdatedCondition, "Foo", "")
+			},
+			afterFunc: func(t *WithT, obj *helmv1.HelmChart) {
+				t.Expect(obj.GetArtifact()).ToNot(BeNil())
+				t.Expect(obj.GetArtifact().Digest).To(Equal("sha256:bbdf96023c912c393b49d5238e227576ed0d20d1bb145d7476d817b80e20c11a"))
+				t.Expect(obj.GetArtifact().Revision).To(Equal("0.1.0"))
+				t.Expect(obj.Status.URL).ToNot(BeEmpty())
+				t.Expect(obj.Status.ObservedChartName).To(Equal("helmchart"))
+				t.Expect(obj.Status.ObservedValuesFiles).To(Equal([]string{"values.yaml", "override.yaml"}))
+			},
+			want: sreconcile.ResultSuccess,
+			assertConditions: []metav1.Condition{
+				*conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, helmv1.ChartPullSucceededReason, "pulled 'helmchart' chart with version '0.1.0'"),
+			},
+		},
 	}
 
 	for _, tt := range tests {
@@ -2016,7 +2124,7 @@ func TestHelmChartReconciler_reconcileSubRecs(t *testing.T) {
 	}
 }
 
-func mockChartBuild(name, version, path string) *chart.Build {
+func mockChartBuild(name, version, path string, valuesFiles []string) *chart.Build {
 	var copyP string
 	if path != "" {
 		f, err := os.Open(path)
@@ -2032,9 +2140,10 @@ func mockChartBuild(name, version, path string) *chart.Build {
 		}
 	}
 	return &chart.Build{
-		Name:    name,
+		Name:        name,
-		Version: version,
+		Version:     version,
-		Path:    copyP,
+		Path:        copyP,
+		ValuesFiles: valuesFiles,
 	}
 }
 

internal/helm/chart/builder.go

@@ -107,10 +107,11 @@ type BuildOptions struct {
 	// ValuesFiles can be set to a list of relative paths, used to compose
 	// and overwrite an alternative default "values.yaml" for the chart.
 	ValuesFiles []string
-	// ObservedValuesFiles is calculated when the chart is built. If BuildOptions.IgnoreMissingValuesFiles is set,
+	// CachedChartValuesFiles is a list of relative paths that were used to
-	// this list will contain the values files that were actually found on disk.
+	// build the cached chart.
-	ObservedValuesFiles []string
+	CachedChartValuesFiles []string
-	// IgnoreMissingValuesFiles controls whether to silently ignore missing values files rather than failing.
+	// IgnoreMissingValuesFiles controls whether to silently ignore missing
+	// values files rather than failing.
 	IgnoreMissingValuesFiles bool
 	// CachedChart can be set to the absolute path of a chart stored on
 	// the local filesystem, and is used for simple validation by metadata

internal/helm/chart/builder_local.go

@@ -120,7 +120,7 @@ func (b *localChartBuilder) Build(ctx context.Context, ref Reference, p string,
 			if err = curMeta.Validate(); err == nil {
 				if result.Name == curMeta.Name && result.Version == curMeta.Version {
 					result.Path = opts.CachedChart
-					result.ValuesFiles = opts.ObservedValuesFiles
+					result.ValuesFiles = opts.CachedChartValuesFiles
 					result.Packaged = requiresPackaging
 
 					return result, nil
@@ -191,7 +191,7 @@ func (b *localChartBuilder) Build(ctx context.Context, ref Reference, p string,
 
 // mergeFileValues merges the given value file paths into a single "values.yaml" map.
 // The provided (relative) paths may not traverse outside baseDir. By default, a missing
-// file is considered an error. If ignoreMissing is set true, missing files are ignored.
+// file is considered an error. If ignoreMissing is true, missing files are ignored.
 // It returns the merge result and the list of files that contributed to that result,
 // or an error.
 func mergeFileValues(baseDir string, paths []string, ignoreMissing bool) (map[string]interface{}, []string, error) {

internal/helm/chart/builder_remote.go

@@ -202,7 +202,7 @@ func generateBuildResult(cv *repo.ChartVersion, opts BuildOptions) (*Build, bool
 			if err = curMeta.Validate(); err == nil {
 				if result.Name == curMeta.Name && result.Version == curMeta.Version {
 					result.Path = opts.CachedChart
-					result.ValuesFiles = opts.ObservedValuesFiles
+					result.ValuesFiles = opts.CachedChartValuesFiles
 					result.Packaged = requiresPackaging
 					return result, true, nil
 				}