diff --git a/docs/spec/v1beta2/helmcharts.md b/docs/spec/v1beta2/helmcharts.md index 107ef640..3932a969 100644 --- a/docs/spec/v1beta2/helmcharts.md +++ b/docs/spec/v1beta2/helmcharts.md @@ -208,9 +208,9 @@ Values files also affect the generated artifact revision, see values files should be ignored rather than be considered errors. It defaults to `false`. -When `.spec.valuesFiles` is specified, the `.status.observedValuesFiles` field -is populated with the list of values files that were found and actually -contributed to the packaged chart. +When `.spec.valuesFiles` and `.spec.ignoreMissingValuesFiles` are specified, +the `.status.observedValuesFiles` field is populated with the list of values +files that were found and actually contributed to the packaged chart. ### Reconcile strategy diff --git a/internal/controller/helmchart_controller.go b/internal/controller/helmchart_controller.go index 547ccac4..608b8382 100644 --- a/internal/controller/helmchart_controller.go +++ b/internal/controller/helmchart_controller.go @@ -666,7 +666,6 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj * cb := chart.NewRemoteBuilder(chartRepo) opts := chart.BuildOptions{ ValuesFiles: obj.GetValuesFiles(), - 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 @@ -676,6 +675,7 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj * } if artifact := obj.GetArtifact(); artifact != nil { opts.CachedChart = r.Storage.LocalPath(*artifact) + opts.CachedChartValuesFiles = obj.Status.ObservedValuesFiles } // Set the VersionMetadata to the object's Generation if ValuesFiles is defined @@ -888,7 +888,11 @@ func (r *HelmChartReconciler) reconcileArtifact(ctx context.Context, _ *patch.Se // Record it on the object obj.Status.Artifact = artifact.DeepCopy() obj.Status.ObservedChartName = b.Name - obj.Status.ObservedValuesFiles = b.ValuesFiles + if obj.Spec.IgnoreMissingValuesFiles { + obj.Status.ObservedValuesFiles = b.ValuesFiles + } else { + obj.Status.ObservedValuesFiles = nil + } // Update symlink on a "best effort" basis symURL, err := r.Storage.Symlink(artifact, "latest.tar.gz") diff --git a/internal/controller/helmchart_controller_test.go b/internal/controller/helmchart_controller_test.go index 2c06955c..b15fcf6d 100644 --- a/internal/controller/helmchart_controller_test.go +++ b/internal/controller/helmchart_controller_test.go @@ -1661,7 +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()) + t.Expect(obj.Status.ObservedValuesFiles).To(BeNil()) }, want: sreconcile.ResultSuccess, assertConditions: []metav1.Condition{ @@ -1684,7 +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.ObservedValuesFiles).To(BeNil()) t.Expect(obj.Status.URL).To(BeEmpty()) }, }, @@ -1724,7 +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()) + t.Expect(obj.Status.ObservedValuesFiles).To(BeNil()) }, want: sreconcile.ResultSuccess, assertConditions: []metav1.Condition{ @@ -1754,6 +1754,27 @@ func TestHelmChartReconciler_reconcileArtifact(t *testing.T) { 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(BeNil()) + }, + want: sreconcile.ResultSuccess, + assertConditions: []metav1.Condition{ + *conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, helmv1.ChartPullSucceededReason, "pulled 'helmchart' chart with version '0.1.0'"), + }, + }, + { + name: "Updates ObservedValuesFiles with IgnoreMissingValuesFiles 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", "") + obj.Spec.ValuesFiles = []string{"values.yaml", "missing.yaml", "override.yaml"} + obj.Spec.IgnoreMissingValuesFiles = true + }, afterFunc: func(t *WithT, obj *helmv1.HelmChart) { t.Expect(obj.GetArtifact()).ToNot(BeNil()) t.Expect(obj.GetArtifact().Digest).To(Equal("sha256:bbdf96023c912c393b49d5238e227576ed0d20d1bb145d7476d817b80e20c11a")) diff --git a/internal/helm/chart/builder_local.go b/internal/helm/chart/builder_local.go index 23e433cc..44399a80 100644 --- a/internal/helm/chart/builder_local.go +++ b/internal/helm/chart/builder_local.go @@ -120,7 +120,12 @@ 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.CachedChartValuesFiles + result.ValuesFiles = opts.GetValuesFiles() + if opts.CachedChartValuesFiles != nil { + // If the cached chart values files are set, we should use them + // instead of reporting the values files. + result.ValuesFiles = opts.CachedChartValuesFiles + } result.Packaged = requiresPackaging return result, nil diff --git a/internal/helm/chart/builder_remote.go b/internal/helm/chart/builder_remote.go index e076680b..1010d8cc 100644 --- a/internal/helm/chart/builder_remote.go +++ b/internal/helm/chart/builder_remote.go @@ -202,7 +202,12 @@ 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.CachedChartValuesFiles + result.ValuesFiles = opts.GetValuesFiles() + if opts.CachedChartValuesFiles != nil { + // If the cached chart values files are set, we should use them + // instead of reporting the values files. + result.ValuesFiles = opts.CachedChartValuesFiles + } result.Packaged = requiresPackaging return result, true, nil }