Merge pull request #1447 from isometry/feature/ignore-missing-values-files

Add `.spec.ignoreMissingValuesFiles` to HelmChart API
This commit is contained in:
Stefan Prodan 2024-05-02 15:12:32 +03:00 committed by GitHub
commit 5fcae5c475
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
13 changed files with 374 additions and 51 deletions

View File

@ -188,7 +188,7 @@ TMP_DIR=$$(mktemp -d) ;\
cd $$TMP_DIR ;\ cd $$TMP_DIR ;\
go mod init tmp ;\ go mod init tmp ;\
echo "Downloading $(2)" ;\ echo "Downloading $(2)" ;\
env -i bash -c "GOBIN=$(GOBIN) PATH=$(PATH) GOPATH=$(shell go env GOPATH) GOCACHE=$(shell go env GOCACHE) go install $(2)" ;\ env -i bash -c "GOBIN=$(GOBIN) PATH=\"$(PATH)\" GOPATH=$(shell go env GOPATH) GOCACHE=$(shell go env GOCACHE) go install $(2)" ;\
rm -rf $$TMP_DIR ;\ rm -rf $$TMP_DIR ;\
} }
endef endef

View File

@ -79,6 +79,11 @@ type HelmChartSpec struct {
// +deprecated // +deprecated
ValuesFile string `json:"valuesFile,omitempty"` ValuesFile string `json:"valuesFile,omitempty"`
// IgnoreMissingValuesFiles controls whether to silently ignore missing values
// files rather than failing.
// +optional
IgnoreMissingValuesFiles bool `json:"ignoreMissingValuesFiles,omitempty"`
// Suspend tells the controller to suspend the reconciliation of this // Suspend tells the controller to suspend the reconciliation of this
// source. // source.
// +optional // +optional
@ -142,6 +147,12 @@ type HelmChartStatus struct {
// +optional // +optional
ObservedChartName string `json:"observedChartName,omitempty"` ObservedChartName string `json:"observedChartName,omitempty"`
// 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"`
// Conditions holds the conditions for the HelmChart. // Conditions holds the conditions for the HelmChart.
// +optional // +optional
Conditions []metav1.Condition `json:"conditions,omitempty"` Conditions []metav1.Condition `json:"conditions,omitempty"`

View File

@ -484,6 +484,11 @@ func (in *HelmChartSpec) DeepCopy() *HelmChartSpec {
// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
func (in *HelmChartStatus) DeepCopyInto(out *HelmChartStatus) { func (in *HelmChartStatus) DeepCopyInto(out *HelmChartStatus) {
*out = *in *out = *in
if in.ObservedValuesFiles != nil {
in, out := &in.ObservedValuesFiles, &out.ObservedValuesFiles
*out = make([]string, len(*in))
copy(*out, *in)
}
if in.Conditions != nil { if in.Conditions != nil {
in, out := &in.Conditions, &out.Conditions in, out := &in.Conditions, &out.Conditions
*out = make([]v1.Condition, len(*in)) *out = make([]v1.Condition, len(*in))

View File

@ -363,6 +363,11 @@ spec:
Chart is the name or path the Helm chart is available at in the Chart is the name or path the Helm chart is available at in the
SourceRef. SourceRef.
type: string type: string
ignoreMissingValuesFiles:
description: |-
IgnoreMissingValuesFiles controls whether to silently ignore missing values
files rather than failing.
type: boolean
interval: interval:
description: |- description: |-
Interval at which the HelmChart SourceRef is checked for updates. Interval at which the HelmChart SourceRef is checked for updates.
@ -638,6 +643,14 @@ spec:
ObservedSourceArtifactRevision is the last observed Artifact.Revision ObservedSourceArtifactRevision is the last observed Artifact.Revision
of the HelmChartSpec.SourceRef. of the HelmChartSpec.SourceRef.
type: string type: string
observedValuesFiles:
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
url: url:
description: |- description: |-
URL is the dynamic fetch link for the latest Artifact. URL is the dynamic fetch link for the latest Artifact.

View File

@ -660,6 +660,19 @@ is merged before the ValuesFiles items. Ignored when omitted.</p>
</tr> </tr>
<tr> <tr>
<td> <td>
<code>ignoreMissingValuesFiles</code><br>
<em>
bool
</em>
</td>
<td>
<em>(Optional)</em>
<p>IgnoreMissingValuesFiles controls whether to silently ignore missing values
files rather than failing.</p>
</td>
</tr>
<tr>
<td>
<code>suspend</code><br> <code>suspend</code><br>
<em> <em>
bool bool
@ -2329,6 +2342,19 @@ is merged before the ValuesFiles items. Ignored when omitted.</p>
</tr> </tr>
<tr> <tr>
<td> <td>
<code>ignoreMissingValuesFiles</code><br>
<em>
bool
</em>
</td>
<td>
<em>(Optional)</em>
<p>IgnoreMissingValuesFiles controls whether to silently ignore missing values
files rather than failing.</p>
</td>
</tr>
<tr>
<td>
<code>suspend</code><br> <code>suspend</code><br>
<em> <em>
bool bool
@ -2436,6 +2462,20 @@ resolved chart reference.</p>
</tr> </tr>
<tr> <tr>
<td> <td>
<code>observedValuesFiles</code><br>
<em>
[]string
</em>
</td>
<td>
<em>(Optional)</em>
<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>
<td>
<code>conditions</code><br> <code>conditions</code><br>
<em> <em>
<a href="https://pkg.go.dev/k8s.io/apimachinery/pkg/apis/meta/v1#Condition"> <a href="https://pkg.go.dev/k8s.io/apimachinery/pkg/apis/meta/v1#Condition">

View File

@ -202,6 +202,16 @@ spec:
Values files also affect the generated artifact revision, see Values files also affect the generated artifact revision, see
[artifact](#artifact). [artifact](#artifact).
### Ignore missing values files
`.spec.ignoreMissingValuesFiles` is an optional field to specify whether missing
values files should be ignored rather than be considered errors. It defaults to
`false`.
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 ### Reconcile strategy
`.spec.reconcileStrategy` is an optional field to specify what enables the `.spec.reconcileStrategy` is an optional field to specify what enables the

View File

@ -665,8 +665,9 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
// Construct the chart builder with scoped configuration // Construct the chart builder with scoped configuration
cb := chart.NewRemoteBuilder(chartRepo) cb := chart.NewRemoteBuilder(chartRepo)
opts := chart.BuildOptions{ opts := chart.BuildOptions{
ValuesFiles: obj.GetValuesFiles(), ValuesFiles: obj.GetValuesFiles(),
Force: obj.Generation != obj.Status.ObservedGeneration, IgnoreMissingValuesFiles: obj.Spec.IgnoreMissingValuesFiles,
Force: obj.Generation != obj.Status.ObservedGeneration,
// The remote builder will not attempt to download the chart if // The remote builder will not attempt to download the chart if
// an artifact exists with the same name and version and `Force` is false. // an artifact exists with the same name and version and `Force` is false.
// It will however try to verify the chart if `obj.Spec.Verify` is set, at every reconciliation. // It will however try to verify the chart if `obj.Spec.Verify` is set, at every reconciliation.
@ -674,6 +675,7 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
} }
if artifact := obj.GetArtifact(); artifact != nil { if artifact := obj.GetArtifact(); artifact != nil {
opts.CachedChart = r.Storage.LocalPath(*artifact) opts.CachedChart = r.Storage.LocalPath(*artifact)
opts.CachedChartValuesFiles = obj.Status.ObservedValuesFiles
} }
// Set the VersionMetadata to the object's Generation if ValuesFiles is defined // Set the VersionMetadata to the object's Generation if ValuesFiles is defined
@ -760,11 +762,13 @@ func (r *HelmChartReconciler) buildFromTarballArtifact(ctx context.Context, obj
// Configure builder options, including any previously cached chart // Configure builder options, including any previously cached chart
opts := chart.BuildOptions{ opts := chart.BuildOptions{
ValuesFiles: obj.GetValuesFiles(), ValuesFiles: obj.GetValuesFiles(),
Force: obj.Generation != obj.Status.ObservedGeneration, 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.CachedChart = r.Storage.LocalPath(*artifact)
opts.CachedChartValuesFiles = obj.Status.ObservedValuesFiles
} }
// Configure revision metadata for chart build if we should react to revision changes // Configure revision metadata for chart build if we should react to revision changes
@ -884,6 +888,11 @@ func (r *HelmChartReconciler) reconcileArtifact(ctx context.Context, _ *patch.Se
// Record it on the object // Record it on the object
obj.Status.Artifact = artifact.DeepCopy() obj.Status.Artifact = artifact.DeepCopy()
obj.Status.ObservedChartName = b.Name obj.Status.ObservedChartName = b.Name
if obj.Spec.IgnoreMissingValuesFiles {
obj.Status.ObservedValuesFiles = b.ValuesFiles
} else {
obj.Status.ObservedValuesFiles = nil
}
// Update symlink on a "best effort" basis // Update symlink on a "best effort" basis
symURL, err := r.Storage.Symlink(artifact, "latest.tar.gz") symURL, err := r.Storage.Symlink(artifact, "latest.tar.gz")

View File

@ -927,6 +927,23 @@ func TestHelmChartReconciler_buildFromHelmRepository(t *testing.T) {
g.Expect(build.Path).To(BeARegularFile()) 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", name: "Sets Generation as VersionMetadata with values files",
beforeFunc: func(obj *helmv1.HelmChart, repository *helmv1.HelmRepository) { 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.Version).To(Equal(higherChartVersion + "+3"))
g.Expect(build.Path).ToNot(BeEmpty()) g.Expect(build.Path).ToNot(BeEmpty())
g.Expect(build.Path).To(BeARegularFile()) 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) { cleanFunc: func(g *WithT, build *chart.Build) {
g.Expect(os.Remove(build.Path)).To(Succeed()) 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.Version).To(Equal(metadata.Version))
g.Expect(build.Path).To(Equal(storage.LocalPath(*cachedArtifact.DeepCopy()))) g.Expect(build.Path).To(Equal(storage.LocalPath(*cachedArtifact.DeepCopy())))
g.Expect(build.Path).To(BeARegularFile()) 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.Version).To(Equal("0.1.0+3"))
g.Expect(build.ResolvedDependencies).To(Equal(0)) g.Expect(build.ResolvedDependencies).To(Equal(0))
g.Expect(build.Path).To(BeARegularFile()) 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) { cleanFunc: func(g *WithT, build *chart.Build) {
g.Expect(os.Remove(build.Path)).To(Succeed()) 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.Version).To(Equal("0.1.0"))
g.Expect(build.Path).To(Equal(storage.LocalPath(*cachedArtifact.DeepCopy()))) g.Expect(build.Path).To(Equal(storage.LocalPath(*cachedArtifact.DeepCopy())))
g.Expect(build.Path).To(BeARegularFile()) 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.Version).To(Equal("0.1.0"))
g.Expect(build.Path).ToNot(Equal(storage.LocalPath(*cachedArtifact.DeepCopy()))) g.Expect(build.Path).ToNot(Equal(storage.LocalPath(*cachedArtifact.DeepCopy())))
g.Expect(build.Path).To(BeARegularFile()) g.Expect(build.Path).To(BeARegularFile())
g.Expect(build.ValuesFiles).To(BeEmpty())
}, },
cleanFunc: func(g *WithT, build *chart.Build) { cleanFunc: func(g *WithT, build *chart.Build) {
g.Expect(os.Remove(build.Path)).To(Succeed()) 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", 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) { beforeFunc: func(obj *helmv1.HelmChart) {
conditions.MarkTrue(obj, sourcev1.ArtifactOutdatedCondition, "Foo", "") 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.GetArtifact().Revision).To(Equal("0.1.0"))
t.Expect(obj.Status.URL).ToNot(BeEmpty()) t.Expect(obj.Status.URL).ToNot(BeEmpty())
t.Expect(obj.Status.ObservedChartName).To(Equal("helmchart")) t.Expect(obj.Status.ObservedChartName).To(Equal("helmchart"))
t.Expect(obj.Status.ObservedValuesFiles).To(BeNil())
}, },
want: sreconcile.ResultSuccess, want: sreconcile.ResultSuccess,
assertConditions: []metav1.Condition{ assertConditions: []metav1.Condition{
@ -1597,6 +1684,7 @@ func TestHelmChartReconciler_reconcileArtifact(t *testing.T) {
afterFunc: func(t *WithT, obj *helmv1.HelmChart) { 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.Artifact.Path).To(Equal("testdata/charts/helmchart-0.1.0.tgz"))
t.Expect(obj.Status.ObservedChartName).To(BeEmpty()) t.Expect(obj.Status.ObservedChartName).To(BeEmpty())
t.Expect(obj.Status.ObservedValuesFiles).To(BeNil())
t.Expect(obj.Status.URL).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", 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) { beforeFunc: func(obj *helmv1.HelmChart) {
conditions.MarkTrue(obj, sourcev1.ArtifactOutdatedCondition, "Foo", "") 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.GetArtifact().Revision).To(Equal("0.1.0"))
t.Expect(obj.Status.URL).ToNot(BeEmpty()) t.Expect(obj.Status.URL).ToNot(BeEmpty())
t.Expect(obj.Status.ObservedChartName).To(Equal("helmchart")) t.Expect(obj.Status.ObservedChartName).To(Equal("helmchart"))
t.Expect(obj.Status.ObservedValuesFiles).To(BeNil())
}, },
want: sreconcile.ResultSuccess, want: sreconcile.ResultSuccess,
assertConditions: []metav1.Condition{ assertConditions: []metav1.Condition{
@ -1644,7 +1733,7 @@ func TestHelmChartReconciler_reconcileArtifact(t *testing.T) {
}, },
{ {
name: "Creates latest symlink to the created artifact", 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) { afterFunc: func(t *WithT, obj *helmv1.HelmChart) {
t.Expect(obj.GetArtifact()).ToNot(BeNil()) t.Expect(obj.GetArtifact()).ToNot(BeNil())
@ -1659,6 +1748,46 @@ func TestHelmChartReconciler_reconcileArtifact(t *testing.T) {
*conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, helmv1.ChartPullSucceededReason, "pulled 'helmchart' chart with version '0.1.0'"), *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(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"))
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 { for _, tt := range tests {
@ -2016,7 +2145,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 var copyP string
if path != "" { if path != "" {
f, err := os.Open(path) f, err := os.Open(path)
@ -2032,9 +2161,10 @@ func mockChartBuild(name, version, path string) *chart.Build {
} }
} }
return &chart.Build{ return &chart.Build{
Name: name, Name: name,
Version: version, Version: version,
Path: copyP, Path: copyP,
ValuesFiles: valuesFiles,
} }
} }

View File

@ -107,6 +107,12 @@ type BuildOptions struct {
// ValuesFiles can be set to a list of relative paths, used to compose // ValuesFiles can be set to a list of relative paths, used to compose
// and overwrite an alternative default "values.yaml" for the chart. // and overwrite an alternative default "values.yaml" for the chart.
ValuesFiles []string ValuesFiles []string
// CachedChartValuesFiles is a list of relative paths that were used to
// build the cached chart.
CachedChartValuesFiles []string
// 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 // CachedChart can be set to the absolute path of a chart stored on
// the local filesystem, and is used for simple validation by metadata // the local filesystem, and is used for simple validation by metadata
// comparisons. // comparisons.

View File

@ -121,6 +121,11 @@ func (b *localChartBuilder) Build(ctx context.Context, ref Reference, p string,
if result.Name == curMeta.Name && result.Version == curMeta.Version { if result.Name == curMeta.Name && result.Version == curMeta.Version {
result.Path = opts.CachedChart result.Path = opts.CachedChart
result.ValuesFiles = opts.GetValuesFiles() 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 result.Packaged = requiresPackaging
return result, nil return result, nil
@ -140,9 +145,12 @@ func (b *localChartBuilder) Build(ctx context.Context, ref Reference, p string,
} }
// Merge chart values, if instructed // Merge chart values, if instructed
var mergedValues map[string]interface{} var (
mergedValues map[string]interface{}
valuesFiles []string
)
if len(opts.GetValuesFiles()) > 0 { if len(opts.GetValuesFiles()) > 0 {
if mergedValues, err = mergeFileValues(localRef.WorkDir, opts.ValuesFiles); err != nil { if mergedValues, valuesFiles, err = mergeFileValues(localRef.WorkDir, opts.ValuesFiles, opts.IgnoreMissingValuesFiles); err != nil {
return result, &BuildError{Reason: ErrValuesFilesMerge, Err: err} return result, &BuildError{Reason: ErrValuesFilesMerge, Err: err}
} }
} }
@ -163,7 +171,7 @@ func (b *localChartBuilder) Build(ctx context.Context, ref Reference, p string,
if err != nil { if err != nil {
return result, &BuildError{Reason: ErrValuesFilesMerge, Err: err} return result, &BuildError{Reason: ErrValuesFilesMerge, Err: err}
} }
result.ValuesFiles = opts.GetValuesFiles() result.ValuesFiles = valuesFiles
} }
// Ensure dependencies are fetched if building from a directory // Ensure dependencies are fetched if building from a directory
@ -187,31 +195,42 @@ func (b *localChartBuilder) Build(ctx context.Context, ref Reference, p string,
} }
// mergeFileValues merges the given value file paths into a single "values.yaml" map. // mergeFileValues merges the given value file paths into a single "values.yaml" map.
// The provided (relative) paths may not traverse outside baseDir. It returns the merge // The provided (relative) paths may not traverse outside baseDir. By default, a missing
// result, or an error. // file is considered an error. If ignoreMissing is true, missing files are ignored.
func mergeFileValues(baseDir string, paths []string) (map[string]interface{}, error) { // 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) {
mergedValues := make(map[string]interface{}) mergedValues := make(map[string]interface{})
valuesFiles := make([]string, 0, len(paths))
for _, p := range paths { for _, p := range paths {
secureP, err := securejoin.SecureJoin(baseDir, p) secureP, err := securejoin.SecureJoin(baseDir, p)
if err != nil { if err != nil {
return nil, err return nil, nil, err
} }
if f, err := os.Stat(secureP); err != nil || !f.Mode().IsRegular() { f, err := os.Stat(secureP)
return nil, fmt.Errorf("no values file found at path '%s' (reference '%s')", switch {
case err != nil:
if ignoreMissing && os.IsNotExist(err) {
continue
}
fallthrough
case !f.Mode().IsRegular():
return nil, nil, fmt.Errorf("no values file found at path '%s' (reference '%s')",
strings.TrimPrefix(secureP, baseDir), p) strings.TrimPrefix(secureP, baseDir), p)
} }
b, err := os.ReadFile(secureP) b, err := os.ReadFile(secureP)
if err != nil { if err != nil {
return nil, fmt.Errorf("could not read values from file '%s': %w", p, err) return nil, nil, fmt.Errorf("could not read values from file '%s': %w", p, err)
} }
values := make(map[string]interface{}) values := make(map[string]interface{})
err = yaml.Unmarshal(b, &values) err = yaml.Unmarshal(b, &values)
if err != nil { if err != nil {
return nil, fmt.Errorf("unmarshaling values from '%s' failed: %w", p, err) return nil, nil, fmt.Errorf("unmarshaling values from '%s' failed: %w", p, err)
} }
mergedValues = transform.MergeMaps(mergedValues, values) mergedValues = transform.MergeMaps(mergedValues, values)
valuesFiles = append(valuesFiles, p)
} }
return mergedValues, nil return mergedValues, valuesFiles, nil
} }
// copyFileToPath attempts to copy in to out. It returns an error if out already exists. // copyFileToPath attempts to copy in to out. It returns an error if out already exists.

View File

@ -281,11 +281,13 @@ func TestLocalBuilder_Build_CachedChart(t *testing.T) {
func Test_mergeFileValues(t *testing.T) { func Test_mergeFileValues(t *testing.T) {
tests := []struct { tests := []struct {
name string name string
files []*helmchart.File files []*helmchart.File
paths []string paths []string
want map[string]interface{} ignoreMissing bool
wantErr string wantValues map[string]interface{}
wantFiles []string
wantErr string
}{ }{
{ {
name: "merges values from files", name: "merges values from files",
@ -295,10 +297,11 @@ func Test_mergeFileValues(t *testing.T) {
{Name: "c.yaml", Data: []byte("b: d")}, {Name: "c.yaml", Data: []byte("b: d")},
}, },
paths: []string{"a.yaml", "b.yaml", "c.yaml"}, paths: []string{"a.yaml", "b.yaml", "c.yaml"},
want: map[string]interface{}{ wantValues: map[string]interface{}{
"a": "b", "a": "b",
"b": "d", "b": "d",
}, },
wantFiles: []string{"a.yaml", "b.yaml", "c.yaml"},
}, },
{ {
name: "illegal traverse", name: "illegal traverse",
@ -318,6 +321,25 @@ func Test_mergeFileValues(t *testing.T) {
paths: []string{"a.yaml"}, paths: []string{"a.yaml"},
wantErr: "no values file found at path '/a.yaml'", wantErr: "no values file found at path '/a.yaml'",
}, },
{
name: "ignore missing files",
files: []*helmchart.File{
{Name: "a.yaml", Data: []byte("a: b")},
},
paths: []string{"a.yaml", "b.yaml"},
ignoreMissing: true,
wantValues: map[string]interface{}{
"a": "b",
},
wantFiles: []string{"a.yaml"},
},
{
name: "all files missing",
paths: []string{"a.yaml"},
ignoreMissing: true,
wantValues: map[string]interface{}{},
wantFiles: []string{},
},
} }
for _, tt := range tests { for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) { t.Run(tt.name, func(t *testing.T) {
@ -329,16 +351,18 @@ func Test_mergeFileValues(t *testing.T) {
g.Expect(os.WriteFile(filepath.Join(baseDir, f.Name), f.Data, 0o640)).To(Succeed()) g.Expect(os.WriteFile(filepath.Join(baseDir, f.Name), f.Data, 0o640)).To(Succeed())
} }
got, err := mergeFileValues(baseDir, tt.paths) gotValues, gotFiles, err := mergeFileValues(baseDir, tt.paths, tt.ignoreMissing)
if tt.wantErr != "" { if tt.wantErr != "" {
g.Expect(err).To(HaveOccurred()) g.Expect(err).To(HaveOccurred())
g.Expect(err.Error()).To(ContainSubstring(tt.wantErr)) g.Expect(err.Error()).To(ContainSubstring(tt.wantErr))
g.Expect(got).To(BeNil()) g.Expect(gotValues).To(BeNil())
g.Expect(gotFiles).To(BeNil())
return return
} }
g.Expect(err).ToNot(HaveOccurred()) g.Expect(err).ToNot(HaveOccurred())
g.Expect(got).To(Equal(tt.want)) g.Expect(gotValues).To(Equal(tt.wantValues))
g.Expect(gotFiles).To(Equal(tt.wantFiles))
}) })
} }
} }

View File

@ -103,7 +103,7 @@ func (b *remoteChartBuilder) Build(ctx context.Context, ref Reference, p string,
} }
chart.Metadata.Version = result.Version chart.Metadata.Version = result.Version
mergedValues, err := mergeChartValues(chart, opts.ValuesFiles) mergedValues, valuesFiles, err := mergeChartValues(chart, opts.ValuesFiles, opts.IgnoreMissingValuesFiles)
if err != nil { if err != nil {
err = fmt.Errorf("failed to merge chart values: %w", err) err = fmt.Errorf("failed to merge chart values: %w", err)
return result, &BuildError{Reason: ErrValuesFilesMerge, Err: err} return result, &BuildError{Reason: ErrValuesFilesMerge, Err: err}
@ -113,7 +113,7 @@ func (b *remoteChartBuilder) Build(ctx context.Context, ref Reference, p string,
if err != nil { if err != nil {
return nil, &BuildError{Reason: ErrValuesFilesMerge, Err: err} return nil, &BuildError{Reason: ErrValuesFilesMerge, Err: err}
} }
result.ValuesFiles = opts.GetValuesFiles() result.ValuesFiles = valuesFiles
} }
// Package the chart with the custom values // Package the chart with the custom values
@ -203,6 +203,11 @@ func generateBuildResult(cv *repo.ChartVersion, opts BuildOptions) (*Build, bool
if result.Name == curMeta.Name && result.Version == curMeta.Version { if result.Name == curMeta.Name && result.Version == curMeta.Version {
result.Path = opts.CachedChart result.Path = opts.CachedChart
result.ValuesFiles = opts.GetValuesFiles() 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 result.Packaged = requiresPackaging
return result, true, nil return result, true, nil
} }
@ -226,13 +231,18 @@ func setBuildMetaData(version, versionMetadata string) (*semver.Version, error)
} }
// mergeChartValues merges the given chart.Chart Files paths into a single "values.yaml" map. // mergeChartValues merges the given chart.Chart Files paths into a single "values.yaml" map.
// It returns the merge result, or an error. // By default, a missing file is considered an error. If ignoreMissing is set true,
func mergeChartValues(chart *helmchart.Chart, paths []string) (map[string]interface{}, error) { // missing files are ignored.
// It returns the merge result and the list of files that contributed to that result,
// or an error.
func mergeChartValues(chart *helmchart.Chart, paths []string, ignoreMissing bool) (map[string]interface{}, []string, error) {
mergedValues := make(map[string]interface{}) mergedValues := make(map[string]interface{})
valuesFiles := make([]string, 0, len(paths))
for _, p := range paths { for _, p := range paths {
cfn := filepath.Clean(p) cfn := filepath.Clean(p)
if cfn == chartutil.ValuesfileName { if cfn == chartutil.ValuesfileName {
mergedValues = transform.MergeMaps(mergedValues, chart.Values) mergedValues = transform.MergeMaps(mergedValues, chart.Values)
valuesFiles = append(valuesFiles, p)
continue continue
} }
var b []byte var b []byte
@ -243,15 +253,19 @@ func mergeChartValues(chart *helmchart.Chart, paths []string) (map[string]interf
} }
} }
if b == nil { if b == nil {
return nil, fmt.Errorf("no values file found at path '%s'", p) if ignoreMissing {
continue
}
return nil, nil, fmt.Errorf("no values file found at path '%s'", p)
} }
values := make(map[string]interface{}) values := make(map[string]interface{})
if err := yaml.Unmarshal(b, &values); err != nil { if err := yaml.Unmarshal(b, &values); err != nil {
return nil, fmt.Errorf("unmarshaling values from '%s' failed: %w", p, err) return nil, nil, fmt.Errorf("unmarshaling values from '%s' failed: %w", p, err)
} }
mergedValues = transform.MergeMaps(mergedValues, values) mergedValues = transform.MergeMaps(mergedValues, values)
valuesFiles = append(valuesFiles, p)
} }
return mergedValues, nil return mergedValues, valuesFiles, nil
} }
// validatePackageAndWriteToPath atomically writes the packaged chart from reader // validatePackageAndWriteToPath atomically writes the packaged chart from reader

View File

@ -443,11 +443,13 @@ entries:
func Test_mergeChartValues(t *testing.T) { func Test_mergeChartValues(t *testing.T) {
tests := []struct { tests := []struct {
name string name string
chart *helmchart.Chart chart *helmchart.Chart
paths []string paths []string
want map[string]interface{} ignoreMissing bool
wantErr string wantValues map[string]interface{}
wantFiles []string
wantErr string
}{ }{
{ {
name: "merges values", name: "merges values",
@ -459,10 +461,11 @@ func Test_mergeChartValues(t *testing.T) {
}, },
}, },
paths: []string{"a.yaml", "b.yaml", "c.yaml"}, paths: []string{"a.yaml", "b.yaml", "c.yaml"},
want: map[string]interface{}{ wantValues: map[string]interface{}{
"a": "b", "a": "b",
"b": "d", "b": "d",
}, },
wantFiles: []string{"a.yaml", "b.yaml", "c.yaml"},
}, },
{ {
name: "uses chart values", name: "uses chart values",
@ -475,10 +478,11 @@ func Test_mergeChartValues(t *testing.T) {
}, },
}, },
paths: []string{chartutil.ValuesfileName, "c.yaml"}, paths: []string{chartutil.ValuesfileName, "c.yaml"},
want: map[string]interface{}{ wantValues: map[string]interface{}{
"a": "b", "a": "b",
"b": "d", "b": "d",
}, },
wantFiles: []string{chartutil.ValuesfileName, "c.yaml"},
}, },
{ {
name: "unmarshal error", name: "unmarshal error",
@ -496,21 +500,59 @@ func Test_mergeChartValues(t *testing.T) {
paths: []string{"a.yaml"}, paths: []string{"a.yaml"},
wantErr: "no values file found at path 'a.yaml'", wantErr: "no values file found at path 'a.yaml'",
}, },
{
name: "merges values ignoring file missing",
chart: &helmchart.Chart{
Files: []*helmchart.File{
{Name: "a.yaml", Data: []byte("a: b")},
},
},
paths: []string{"a.yaml", "b.yaml"},
ignoreMissing: true,
wantValues: map[string]interface{}{
"a": "b",
},
wantFiles: []string{"a.yaml"},
},
{
name: "merges values ignoring all missing",
chart: &helmchart.Chart{},
paths: []string{"a.yaml"},
ignoreMissing: true,
wantValues: map[string]interface{}{},
wantFiles: []string{},
},
{
name: "uses chart values ignoring missing file",
chart: &helmchart.Chart{
Values: map[string]interface{}{
"a": "b",
},
},
paths: []string{chartutil.ValuesfileName, "c.yaml"},
ignoreMissing: true,
wantValues: map[string]interface{}{
"a": "b",
},
wantFiles: []string{chartutil.ValuesfileName},
},
} }
for _, tt := range tests { for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) { t.Run(tt.name, func(t *testing.T) {
g := NewWithT(t) g := NewWithT(t)
got, err := mergeChartValues(tt.chart, tt.paths) gotValues, gotFiles, err := mergeChartValues(tt.chart, tt.paths, tt.ignoreMissing)
if tt.wantErr != "" { if tt.wantErr != "" {
g.Expect(err).To(HaveOccurred()) g.Expect(err).To(HaveOccurred())
g.Expect(err.Error()).To(ContainSubstring(tt.wantErr)) g.Expect(err.Error()).To(ContainSubstring(tt.wantErr))
g.Expect(got).To(BeNil()) g.Expect(gotValues).To(BeNil())
g.Expect(gotFiles).To(BeNil())
return return
} }
g.Expect(err).ToNot(HaveOccurred()) g.Expect(err).ToNot(HaveOccurred())
g.Expect(got).To(Equal(tt.want)) g.Expect(gotValues).To(Equal(tt.wantValues))
g.Expect(gotFiles).To(Equal(tt.wantFiles))
}) })
} }
} }