internal/helm: "value files" -> "values files"

Previous usage while consistent, was incorrect, and inconsitent with
the field in the API spec.

Signed-off-by: Hidde Beydals <hello@hidde.co>
This commit is contained in:
Hidde Beydals 2021-11-18 09:24:34 +01:00
parent 37ac5a9679
commit a1e9302b7d
10 changed files with 66 additions and 66 deletions

View File

@ -335,7 +335,7 @@ func (r *HelmChartReconciler) fromHelmRepository(ctx context.Context, repo sourc
cBuilder := chart.NewRemoteBuilder(chartRepo) cBuilder := chart.NewRemoteBuilder(chartRepo)
ref := chart.RemoteReference{Name: c.Spec.Chart, Version: c.Spec.Version} ref := chart.RemoteReference{Name: c.Spec.Chart, Version: c.Spec.Version}
opts := chart.BuildOptions{ opts := chart.BuildOptions{
ValueFiles: c.GetValuesFiles(), ValuesFiles: c.GetValuesFiles(),
CachedChart: cachedChart, CachedChart: cachedChart,
Force: force, Force: force,
} }
@ -431,7 +431,7 @@ func (r *HelmChartReconciler) fromTarballArtifact(ctx context.Context, source so
// Configure builder options, including any previously cached chart // Configure builder options, including any previously cached chart
buildsOpts := chart.BuildOptions{ buildsOpts := chart.BuildOptions{
ValueFiles: c.GetValuesFiles(), ValuesFiles: c.GetValuesFiles(),
Force: force, Force: force,
} }
if artifact := c.Status.Artifact; artifact != nil { if artifact := c.Status.Artifact; artifact != nil {
@ -815,7 +815,7 @@ func reasonForBuildError(err error) string {
return sourcev1.ChartPullFailedReason return sourcev1.ChartPullFailedReason
} }
switch buildErr.Reason { switch buildErr.Reason {
case chart.ErrChartMetadataPatch, chart.ErrValueFilesMerge, chart.ErrDependencyBuild, chart.ErrChartPackage: case chart.ErrChartMetadataPatch, chart.ErrValuesFilesMerge, chart.ErrDependencyBuild, chart.ErrChartPackage:
return sourcev1.ChartPackageFailedReason return sourcev1.ChartPackageFailedReason
default: default:
return sourcev1.ChartPullFailedReason return sourcev1.ChartPullFailedReason

View File

@ -81,10 +81,10 @@ func (r RemoteReference) Validate() error {
// Builder is capable of building a (specific) chart Reference. // Builder is capable of building a (specific) chart Reference.
type Builder interface { type Builder interface {
// Build builds and packages a Helm chart with the given Reference // Build pulls and (optionally) packages a Helm chart with the given
// and BuildOptions and writes it to p. It returns the Build result, // Reference and BuildOptions, and writes it to p.
// or an error. It may return an error for unsupported Reference // It returns the Build result, or an error.
// implementations. // It may return an error for unsupported Reference implementations.
Build(ctx context.Context, ref Reference, p string, opts BuildOptions) (*Build, error) Build(ctx context.Context, ref Reference, p string, opts BuildOptions) (*Build, error)
} }
@ -94,25 +94,25 @@ type BuildOptions struct {
// the spec, and is included during packaging. // the spec, and is included during packaging.
// Ref: https://semver.org/#spec-item-10 // Ref: https://semver.org/#spec-item-10
VersionMetadata string VersionMetadata string
// ValueFiles 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.
ValueFiles []string ValuesFiles []string
// 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.
CachedChart string CachedChart string
// Force can be set to force the build of the chart, for example // Force can be set to force the build of the chart, for example
// because the list of ValueFiles has changed. // because the list of ValuesFiles has changed.
Force bool Force bool
} }
// GetValueFiles returns BuildOptions.ValueFiles, except if it equals // GetValuesFiles returns BuildOptions.ValuesFiles, except if it equals
// "values.yaml", which returns nil. // "values.yaml", which returns nil.
func (o BuildOptions) GetValueFiles() []string { func (o BuildOptions) GetValuesFiles() []string {
if len(o.ValueFiles) == 1 && filepath.Clean(o.ValueFiles[0]) == filepath.Clean(chartutil.ValuesfileName) { if len(o.ValuesFiles) == 1 && filepath.Clean(o.ValuesFiles[0]) == filepath.Clean(chartutil.ValuesfileName) {
return nil return nil
} }
return o.ValueFiles return o.ValuesFiles
} }
// Build contains the Builder.Build result, including specific // Build contains the Builder.Build result, including specific
@ -124,14 +124,14 @@ type Build struct {
Name string Name string
// Version of the packaged chart. // Version of the packaged chart.
Version string Version string
// ValueFiles is the list of files used to compose the chart's // ValuesFiles is the list of files used to compose the chart's
// default "values.yaml". // default "values.yaml".
ValueFiles []string ValuesFiles []string
// ResolvedDependencies is the number of local and remote dependencies // ResolvedDependencies is the number of local and remote dependencies
// collected by the DependencyManager before building the chart. // collected by the DependencyManager before building the chart.
ResolvedDependencies int ResolvedDependencies int
// Packaged indicates if the Builder has packaged the chart. // Packaged indicates if the Builder has packaged the chart.
// This can for example be false if ValueFiles is empty and the chart // This can for example be false if ValuesFiles is empty and the chart
// source was already packaged. // source was already packaged.
Packaged bool Packaged bool
} }
@ -150,8 +150,8 @@ func (b *Build) Summary() string {
} }
s.WriteString(fmt.Sprintf("%s '%s' chart with version '%s'", action, b.Name, b.Version)) s.WriteString(fmt.Sprintf("%s '%s' chart with version '%s'", action, b.Name, b.Version))
if b.Packaged && len(b.ValueFiles) > 0 { if b.Packaged && len(b.ValuesFiles) > 0 {
s.WriteString(fmt.Sprintf(", with merged value files %v", b.ValueFiles)) s.WriteString(fmt.Sprintf(", with merged values files %v", b.ValuesFiles))
} }
if b.Packaged && b.ResolvedDependencies > 0 { if b.Packaged && b.ResolvedDependencies > 0 {

View File

@ -85,15 +85,15 @@ func (b *localChartBuilder) Build(ctx context.Context, ref Reference, p string,
if opts.CachedChart != "" && !opts.Force { if opts.CachedChart != "" && !opts.Force {
if curMeta, err = LoadChartMetadataFromArchive(opts.CachedChart); err == nil && result.Version == curMeta.Version { if curMeta, err = LoadChartMetadataFromArchive(opts.CachedChart); err == nil && result.Version == curMeta.Version {
result.Path = opts.CachedChart result.Path = opts.CachedChart
result.ValueFiles = opts.ValueFiles result.ValuesFiles = opts.ValuesFiles
return result, nil return result, nil
} }
} }
// If the chart at the path is already packaged and no custom value files // If the chart at the path is already packaged and no custom values files
// options are set, we can copy the chart without making modifications // options are set, we can copy the chart without making modifications
isChartDir := pathIsDir(localRef.Path) isChartDir := pathIsDir(localRef.Path)
if !isChartDir && len(opts.GetValueFiles()) == 0 { if !isChartDir && len(opts.GetValuesFiles()) == 0 {
if err = copyFileToPath(localRef.Path, p); err != nil { if err = copyFileToPath(localRef.Path, p); err != nil {
return nil, &BuildError{Reason: ErrChartPull, Err: err} return nil, &BuildError{Reason: ErrChartPull, Err: err}
} }
@ -103,9 +103,9 @@ 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{}
if len(opts.GetValueFiles()) > 0 { if len(opts.GetValuesFiles()) > 0 {
if mergedValues, err = mergeFileValues(localRef.WorkDir, opts.ValueFiles); err != nil { if mergedValues, err = mergeFileValues(localRef.WorkDir, opts.ValuesFiles); err != nil {
return nil, &BuildError{Reason: ErrValueFilesMerge, Err: err} return nil, &BuildError{Reason: ErrValuesFilesMerge, Err: err}
} }
} }
@ -122,9 +122,9 @@ func (b *localChartBuilder) Build(ctx context.Context, ref Reference, p string,
// Overwrite default values with merged values, if any // Overwrite default values with merged values, if any
if ok, err = OverwriteChartDefaultValues(chart, mergedValues); ok || err != nil { if ok, err = OverwriteChartDefaultValues(chart, mergedValues); ok || err != nil {
if err != nil { if err != nil {
return nil, &BuildError{Reason: ErrValueFilesMerge, Err: err} return nil, &BuildError{Reason: ErrValuesFilesMerge, Err: err}
} }
result.ValueFiles = opts.GetValueFiles() result.ValuesFiles = opts.GetValuesFiles()
} }
// Ensure dependencies are fetched if building from a directory // Ensure dependencies are fetched if building from a directory

View File

@ -66,7 +66,7 @@ func TestLocalBuilder_Build(t *testing.T) {
name string name string
reference Reference reference Reference
buildOpts BuildOptions buildOpts BuildOptions
valueFiles []helmchart.File valuesFiles []helmchart.File
repositories map[string]*repository.ChartRepository repositories map[string]*repository.ChartRepository
dependentChartPaths []string dependentChartPaths []string
wantValues chartutil.Values wantValues chartutil.Values
@ -118,12 +118,12 @@ func TestLocalBuilder_Build(t *testing.T) {
wantPackaged: true, wantPackaged: true,
}, },
{ {
name: "with value files", name: "with values files",
reference: LocalReference{Path: "./../testdata/charts/helmchart"}, reference: LocalReference{Path: "./../testdata/charts/helmchart"},
buildOpts: BuildOptions{ buildOpts: BuildOptions{
ValueFiles: []string{"custom-values1.yaml", "custom-values2.yaml"}, ValuesFiles: []string{"custom-values1.yaml", "custom-values2.yaml"},
}, },
valueFiles: []helmchart.File{ valuesFiles: []helmchart.File{
{ {
Name: "custom-values1.yaml", Name: "custom-values1.yaml",
Data: []byte(`replicaCount: 11 Data: []byte(`replicaCount: 11
@ -189,7 +189,7 @@ fullnameOverride: "full-foo-name-override"`),
} }
// Write value file in the base dir. // Write value file in the base dir.
for _, f := range tt.valueFiles { for _, f := range tt.valuesFiles {
vPath := filepath.Join(workDir, f.Name) vPath := filepath.Join(workDir, f.Name)
g.Expect(os.WriteFile(vPath, f.Data, 0644)).ToNot(HaveOccurred()) g.Expect(os.WriteFile(vPath, f.Data, 0644)).ToNot(HaveOccurred())
} }

View File

@ -93,7 +93,7 @@ func (b *remoteChartBuilder) Build(_ context.Context, ref Reference, p string, o
if opts.CachedChart != "" && !opts.Force { if opts.CachedChart != "" && !opts.Force {
if curMeta, err := LoadChartMetadataFromArchive(opts.CachedChart); err == nil && result.Version == curMeta.Version { if curMeta, err := LoadChartMetadataFromArchive(opts.CachedChart); err == nil && result.Version == curMeta.Version {
result.Path = opts.CachedChart result.Path = opts.CachedChart
result.ValueFiles = opts.GetValueFiles() result.ValuesFiles = opts.GetValuesFiles()
return result, nil return result, nil
} }
} }
@ -105,9 +105,9 @@ func (b *remoteChartBuilder) Build(_ context.Context, ref Reference, p string, o
return nil, &BuildError{Reason: ErrChartPull, Err: err} return nil, &BuildError{Reason: ErrChartPull, Err: err}
} }
// Use literal chart copy from remote if no custom value files options are // Use literal chart copy from remote if no custom values files options are
// set or build option version metadata isn't set. // set or build option version metadata isn't set.
if len(opts.GetValueFiles()) == 0 && opts.VersionMetadata == "" { if len(opts.GetValuesFiles()) == 0 && opts.VersionMetadata == "" {
if err = validatePackageAndWriteToPath(res, p); err != nil { if err = validatePackageAndWriteToPath(res, p); err != nil {
return nil, &BuildError{Reason: ErrChartPull, Err: err} return nil, &BuildError{Reason: ErrChartPull, Err: err}
} }
@ -123,17 +123,17 @@ func (b *remoteChartBuilder) Build(_ context.Context, ref Reference, p string, o
} }
chart.Metadata.Version = result.Version chart.Metadata.Version = result.Version
mergedValues, err := mergeChartValues(chart, opts.ValueFiles) mergedValues, err := mergeChartValues(chart, opts.ValuesFiles)
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 nil, &BuildError{Reason: ErrValueFilesMerge, Err: err} return nil, &BuildError{Reason: ErrValuesFilesMerge, Err: err}
} }
// Overwrite default values with merged values, if any // Overwrite default values with merged values, if any
if ok, err = OverwriteChartDefaultValues(chart, mergedValues); ok || err != nil { if ok, err = OverwriteChartDefaultValues(chart, mergedValues); ok || err != nil {
if err != nil { if err != nil {
return nil, &BuildError{Reason: ErrValueFilesMerge, Err: err} return nil, &BuildError{Reason: ErrValuesFilesMerge, Err: err}
} }
result.ValueFiles = opts.GetValueFiles() result.ValuesFiles = opts.GetValuesFiles()
} }
// Package the chart with the custom values // Package the chart with the custom values

View File

@ -144,7 +144,7 @@ entries:
name: "merge values", name: "merge values",
reference: RemoteReference{Name: "grafana"}, reference: RemoteReference{Name: "grafana"},
buildOpts: BuildOptions{ buildOpts: BuildOptions{
ValueFiles: []string{"a.yaml", "b.yaml", "c.yaml"}, ValuesFiles: []string{"a.yaml", "b.yaml", "c.yaml"},
}, },
repository: mockRepo(), repository: mockRepo(),
wantVersion: "6.17.4", wantVersion: "6.17.4",

View File

@ -104,20 +104,20 @@ func TestRemoteReference_Validate(t *testing.T) {
} }
} }
func TestBuildOptions_GetValueFiles(t *testing.T) { func TestBuildOptions_GetValuesFiles(t *testing.T) {
tests := []struct { tests := []struct {
name string name string
valueFiles []string valuesFiles []string
want []string want []string
}{ }{
{ {
name: "Default values.yaml", name: "Default values.yaml",
valueFiles: []string{chartutil.ValuesfileName}, valuesFiles: []string{chartutil.ValuesfileName},
want: nil, want: nil,
}, },
{ {
name: "Value files", name: "Values files",
valueFiles: []string{chartutil.ValuesfileName, "foo.yaml"}, valuesFiles: []string{chartutil.ValuesfileName, "foo.yaml"},
want: []string{chartutil.ValuesfileName, "foo.yaml"}, want: []string{chartutil.ValuesfileName, "foo.yaml"},
}, },
} }
@ -125,8 +125,8 @@ func TestBuildOptions_GetValueFiles(t *testing.T) {
t.Run(tt.name, func(t *testing.T) { t.Run(tt.name, func(t *testing.T) {
g := NewWithT(t) g := NewWithT(t)
o := BuildOptions{ValueFiles: tt.valueFiles} o := BuildOptions{ValuesFiles: tt.valuesFiles}
g.Expect(o.GetValueFiles()).To(Equal(tt.want)) g.Expect(o.GetValuesFiles()).To(Equal(tt.want))
}) })
} }
} }
@ -146,14 +146,14 @@ func TestChartBuildResult_Summary(t *testing.T) {
want: "Pulled 'chart' chart with version '1.2.3-rc.1+bd6bf40'.", want: "Pulled 'chart' chart with version '1.2.3-rc.1+bd6bf40'.",
}, },
{ {
name: "With value files", name: "With values files",
build: &Build{ build: &Build{
Name: "chart", Name: "chart",
Version: "arbitrary-version", Version: "arbitrary-version",
Packaged: true, Packaged: true,
ValueFiles: []string{"a.yaml", "b.yaml"}, ValuesFiles: []string{"a.yaml", "b.yaml"},
}, },
want: "Packaged 'chart' chart with version 'arbitrary-version', with merged value files [a.yaml b.yaml].", want: "Packaged 'chart' chart with version 'arbitrary-version', with merged values files [a.yaml b.yaml].",
}, },
{ {
name: "With dependencies", name: "With dependencies",

View File

@ -296,7 +296,7 @@ func (dm *DependencyManager) secureLocalChartPath(ref LocalReference, dep *helmc
// collectMissing returns a map with reqs that are missing from current, // collectMissing returns a map with reqs that are missing from current,
// indexed by their alias or name. All dependencies of a chart are present // indexed by their alias or name. All dependencies of a chart are present
// if len of returned value == 0. // if len of returned map == 0.
func collectMissing(current []*helmchart.Chart, reqs []*helmchart.Dependency) map[string]*helmchart.Dependency { func collectMissing(current []*helmchart.Chart, reqs []*helmchart.Dependency) map[string]*helmchart.Dependency {
// If the number of dependencies equals the number of requested // If the number of dependencies equals the number of requested
// dependencies, there are no missing dependencies // dependencies, there are no missing dependencies

View File

@ -59,7 +59,7 @@ func (e *BuildError) Unwrap() error {
var ( var (
ErrChartPull = BuildErrorReason("chart pull error") ErrChartPull = BuildErrorReason("chart pull error")
ErrChartMetadataPatch = BuildErrorReason("chart metadata patch error") ErrChartMetadataPatch = BuildErrorReason("chart metadata patch error")
ErrValueFilesMerge = BuildErrorReason("value files merge error") ErrValuesFilesMerge = BuildErrorReason("values files merge error")
ErrDependencyBuild = BuildErrorReason("dependency build error") ErrDependencyBuild = BuildErrorReason("dependency build error")
ErrChartPackage = BuildErrorReason("chart package error") ErrChartPackage = BuildErrorReason("chart package error")
) )