feat(HelmChartSpec): optionally ignore missing valuesFiles

Signed-off-by: Robin Breathe <robin@isometry.net>
This commit is contained in:
Robin Breathe 2024-04-17 14:27:11 +02:00
parent 0fe64864d4
commit b84ab9e698
No known key found for this signature in database
9 changed files with 176 additions and 42 deletions

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

View File

@ -93,6 +93,11 @@ spec:
description: The name or path the Helm chart is available at in the description: 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: The interval at which to check the Source for updates. description: The interval at which to check the Source for updates.
type: string type: string
@ -363,6 +368,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.

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>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>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

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.
@ -760,8 +761,9 @@ 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.Status.Artifact; artifact != nil {
opts.CachedChart = r.Storage.LocalPath(*artifact) opts.CachedChart = r.Storage.LocalPath(*artifact)

View File

@ -107,6 +107,8 @@ 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
// 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

@ -140,9 +140,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 +166,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 +190,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 set 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
@ -226,13 +226,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 +248,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))
}) })
} }
} }