From a1e9302b7dce0ce73496de4379394e63f1002fe6 Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Thu, 18 Nov 2021 09:24:34 +0100 Subject: [PATCH] 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 --- controllers/helmchart_controller.go | 8 ++--- internal/helm/chart/builder.go | 32 +++++++++---------- internal/helm/chart/builder_local.go | 16 +++++----- internal/helm/chart/builder_local_test.go | 10 +++--- internal/helm/chart/builder_remote.go | 14 ++++----- internal/helm/chart/builder_remote_test.go | 2 +- internal/helm/chart/builder_test.go | 36 +++++++++++----------- internal/helm/chart/dependency_manager.go | 2 +- internal/helm/chart/errors.go | 2 +- internal/helm/chart/errors_test.go | 10 +++--- 10 files changed, 66 insertions(+), 66 deletions(-) diff --git a/controllers/helmchart_controller.go b/controllers/helmchart_controller.go index 0e0b2cd2..685a43a5 100644 --- a/controllers/helmchart_controller.go +++ b/controllers/helmchart_controller.go @@ -335,7 +335,7 @@ func (r *HelmChartReconciler) fromHelmRepository(ctx context.Context, repo sourc cBuilder := chart.NewRemoteBuilder(chartRepo) ref := chart.RemoteReference{Name: c.Spec.Chart, Version: c.Spec.Version} opts := chart.BuildOptions{ - ValueFiles: c.GetValuesFiles(), + ValuesFiles: c.GetValuesFiles(), CachedChart: cachedChart, Force: force, } @@ -431,8 +431,8 @@ func (r *HelmChartReconciler) fromTarballArtifact(ctx context.Context, source so // Configure builder options, including any previously cached chart buildsOpts := chart.BuildOptions{ - ValueFiles: c.GetValuesFiles(), - Force: force, + ValuesFiles: c.GetValuesFiles(), + Force: force, } if artifact := c.Status.Artifact; artifact != nil { buildsOpts.CachedChart = artifact.Path @@ -815,7 +815,7 @@ func reasonForBuildError(err error) string { return sourcev1.ChartPullFailedReason } switch buildErr.Reason { - case chart.ErrChartMetadataPatch, chart.ErrValueFilesMerge, chart.ErrDependencyBuild, chart.ErrChartPackage: + case chart.ErrChartMetadataPatch, chart.ErrValuesFilesMerge, chart.ErrDependencyBuild, chart.ErrChartPackage: return sourcev1.ChartPackageFailedReason default: return sourcev1.ChartPullFailedReason diff --git a/internal/helm/chart/builder.go b/internal/helm/chart/builder.go index d1b0f747..9aa2a17e 100644 --- a/internal/helm/chart/builder.go +++ b/internal/helm/chart/builder.go @@ -81,10 +81,10 @@ func (r RemoteReference) Validate() error { // Builder is capable of building a (specific) chart Reference. type Builder interface { - // Build builds and packages a Helm chart with the given Reference - // and BuildOptions and writes it to p. It returns the Build result, - // or an error. It may return an error for unsupported Reference - // implementations. + // Build pulls and (optionally) packages a Helm chart with the given + // Reference and BuildOptions, and writes it to p. + // It returns the Build result, or an error. + // It may return an error for unsupported Reference implementations. 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. // Ref: https://semver.org/#spec-item-10 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. - ValueFiles []string + ValuesFiles []string // CachedChart can be set to the absolute path of a chart stored on // the local filesystem, and is used for simple validation by metadata // comparisons. CachedChart string // 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 } -// GetValueFiles returns BuildOptions.ValueFiles, except if it equals +// GetValuesFiles returns BuildOptions.ValuesFiles, except if it equals // "values.yaml", which returns nil. -func (o BuildOptions) GetValueFiles() []string { - if len(o.ValueFiles) == 1 && filepath.Clean(o.ValueFiles[0]) == filepath.Clean(chartutil.ValuesfileName) { +func (o BuildOptions) GetValuesFiles() []string { + if len(o.ValuesFiles) == 1 && filepath.Clean(o.ValuesFiles[0]) == filepath.Clean(chartutil.ValuesfileName) { return nil } - return o.ValueFiles + return o.ValuesFiles } // Build contains the Builder.Build result, including specific @@ -124,14 +124,14 @@ type Build struct { Name string // Version of the packaged chart. 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". - ValueFiles []string + ValuesFiles []string // ResolvedDependencies is the number of local and remote dependencies // collected by the DependencyManager before building the chart. ResolvedDependencies int // 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. 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)) - if b.Packaged && len(b.ValueFiles) > 0 { - s.WriteString(fmt.Sprintf(", with merged value files %v", b.ValueFiles)) + if b.Packaged && len(b.ValuesFiles) > 0 { + s.WriteString(fmt.Sprintf(", with merged values files %v", b.ValuesFiles)) } if b.Packaged && b.ResolvedDependencies > 0 { diff --git a/internal/helm/chart/builder_local.go b/internal/helm/chart/builder_local.go index 2f27b8b2..5d79da3d 100644 --- a/internal/helm/chart/builder_local.go +++ b/internal/helm/chart/builder_local.go @@ -85,15 +85,15 @@ func (b *localChartBuilder) Build(ctx context.Context, ref Reference, p string, if opts.CachedChart != "" && !opts.Force { if curMeta, err = LoadChartMetadataFromArchive(opts.CachedChart); err == nil && result.Version == curMeta.Version { result.Path = opts.CachedChart - result.ValueFiles = opts.ValueFiles + result.ValuesFiles = opts.ValuesFiles 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 isChartDir := pathIsDir(localRef.Path) - if !isChartDir && len(opts.GetValueFiles()) == 0 { + if !isChartDir && len(opts.GetValuesFiles()) == 0 { if err = copyFileToPath(localRef.Path, p); err != nil { 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 var mergedValues map[string]interface{} - if len(opts.GetValueFiles()) > 0 { - if mergedValues, err = mergeFileValues(localRef.WorkDir, opts.ValueFiles); err != nil { - return nil, &BuildError{Reason: ErrValueFilesMerge, Err: err} + if len(opts.GetValuesFiles()) > 0 { + if mergedValues, err = mergeFileValues(localRef.WorkDir, opts.ValuesFiles); err != nil { + 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 if ok, err = OverwriteChartDefaultValues(chart, mergedValues); ok || 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 diff --git a/internal/helm/chart/builder_local_test.go b/internal/helm/chart/builder_local_test.go index 5691371f..cff5f180 100644 --- a/internal/helm/chart/builder_local_test.go +++ b/internal/helm/chart/builder_local_test.go @@ -66,7 +66,7 @@ func TestLocalBuilder_Build(t *testing.T) { name string reference Reference buildOpts BuildOptions - valueFiles []helmchart.File + valuesFiles []helmchart.File repositories map[string]*repository.ChartRepository dependentChartPaths []string wantValues chartutil.Values @@ -118,12 +118,12 @@ func TestLocalBuilder_Build(t *testing.T) { wantPackaged: true, }, { - name: "with value files", + name: "with values files", reference: LocalReference{Path: "./../testdata/charts/helmchart"}, 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", Data: []byte(`replicaCount: 11 @@ -189,7 +189,7 @@ fullnameOverride: "full-foo-name-override"`), } // Write value file in the base dir. - for _, f := range tt.valueFiles { + for _, f := range tt.valuesFiles { vPath := filepath.Join(workDir, f.Name) g.Expect(os.WriteFile(vPath, f.Data, 0644)).ToNot(HaveOccurred()) } diff --git a/internal/helm/chart/builder_remote.go b/internal/helm/chart/builder_remote.go index f03c1a8d..e9cfb9a9 100644 --- a/internal/helm/chart/builder_remote.go +++ b/internal/helm/chart/builder_remote.go @@ -93,7 +93,7 @@ func (b *remoteChartBuilder) Build(_ context.Context, ref Reference, p string, o if opts.CachedChart != "" && !opts.Force { if curMeta, err := LoadChartMetadataFromArchive(opts.CachedChart); err == nil && result.Version == curMeta.Version { result.Path = opts.CachedChart - result.ValueFiles = opts.GetValueFiles() + result.ValuesFiles = opts.GetValuesFiles() 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} } - // 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. - if len(opts.GetValueFiles()) == 0 && opts.VersionMetadata == "" { + if len(opts.GetValuesFiles()) == 0 && opts.VersionMetadata == "" { if err = validatePackageAndWriteToPath(res, p); err != nil { 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 - mergedValues, err := mergeChartValues(chart, opts.ValueFiles) + mergedValues, err := mergeChartValues(chart, opts.ValuesFiles) if err != nil { 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 if ok, err = OverwriteChartDefaultValues(chart, mergedValues); ok || 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 diff --git a/internal/helm/chart/builder_remote_test.go b/internal/helm/chart/builder_remote_test.go index 80534c60..a2c33a6f 100644 --- a/internal/helm/chart/builder_remote_test.go +++ b/internal/helm/chart/builder_remote_test.go @@ -144,7 +144,7 @@ entries: name: "merge values", reference: RemoteReference{Name: "grafana"}, buildOpts: BuildOptions{ - ValueFiles: []string{"a.yaml", "b.yaml", "c.yaml"}, + ValuesFiles: []string{"a.yaml", "b.yaml", "c.yaml"}, }, repository: mockRepo(), wantVersion: "6.17.4", diff --git a/internal/helm/chart/builder_test.go b/internal/helm/chart/builder_test.go index cd64ac41..d797a209 100644 --- a/internal/helm/chart/builder_test.go +++ b/internal/helm/chart/builder_test.go @@ -104,29 +104,29 @@ func TestRemoteReference_Validate(t *testing.T) { } } -func TestBuildOptions_GetValueFiles(t *testing.T) { +func TestBuildOptions_GetValuesFiles(t *testing.T) { tests := []struct { - name string - valueFiles []string - want []string + name string + valuesFiles []string + want []string }{ { - name: "Default values.yaml", - valueFiles: []string{chartutil.ValuesfileName}, - want: nil, + name: "Default values.yaml", + valuesFiles: []string{chartutil.ValuesfileName}, + want: nil, }, { - name: "Value files", - valueFiles: []string{chartutil.ValuesfileName, "foo.yaml"}, - want: []string{chartutil.ValuesfileName, "foo.yaml"}, + name: "Values files", + valuesFiles: []string{chartutil.ValuesfileName, "foo.yaml"}, + want: []string{chartutil.ValuesfileName, "foo.yaml"}, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { g := NewWithT(t) - o := BuildOptions{ValueFiles: tt.valueFiles} - g.Expect(o.GetValueFiles()).To(Equal(tt.want)) + o := BuildOptions{ValuesFiles: tt.valuesFiles} + 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'.", }, { - name: "With value files", + name: "With values files", build: &Build{ - Name: "chart", - Version: "arbitrary-version", - Packaged: true, - ValueFiles: []string{"a.yaml", "b.yaml"}, + Name: "chart", + Version: "arbitrary-version", + Packaged: true, + 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", diff --git a/internal/helm/chart/dependency_manager.go b/internal/helm/chart/dependency_manager.go index 798f6df9..e4102065 100644 --- a/internal/helm/chart/dependency_manager.go +++ b/internal/helm/chart/dependency_manager.go @@ -296,7 +296,7 @@ func (dm *DependencyManager) secureLocalChartPath(ref LocalReference, dep *helmc // collectMissing returns a map with reqs that are missing from current, // 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 { // If the number of dependencies equals the number of requested // dependencies, there are no missing dependencies diff --git a/internal/helm/chart/errors.go b/internal/helm/chart/errors.go index 746017f2..696cecc5 100644 --- a/internal/helm/chart/errors.go +++ b/internal/helm/chart/errors.go @@ -59,7 +59,7 @@ func (e *BuildError) Unwrap() error { var ( ErrChartPull = BuildErrorReason("chart pull error") ErrChartMetadataPatch = BuildErrorReason("chart metadata patch error") - ErrValueFilesMerge = BuildErrorReason("value files merge error") + ErrValuesFilesMerge = BuildErrorReason("values files merge error") ErrDependencyBuild = BuildErrorReason("dependency build error") ErrChartPackage = BuildErrorReason("chart package error") ) diff --git a/internal/helm/chart/errors_test.go b/internal/helm/chart/errors_test.go index 7a33c543..f006f336 100644 --- a/internal/helm/chart/errors_test.go +++ b/internal/helm/chart/errors_test.go @@ -32,15 +32,15 @@ func TestBuildErrorReason_Error(t *testing.T) { func TestBuildError_Error(t *testing.T) { tests := []struct { - name string - err *BuildError - want string + name string + err *BuildError + want string }{ { name: "with reason", err: &BuildError{ Reason: BuildErrorReason("reason"), - Err: errors.New("error"), + Err: errors.New("error"), }, want: "reason: error", }, @@ -67,7 +67,7 @@ func TestBuildError_Is(t *testing.T) { wrappedErr := errors.New("wrapped") err := &BuildError{ Reason: ErrChartPackage, - Err: wrappedErr, + Err: wrappedErr, } g.Expect(err.Is(ErrChartPackage)).To(BeTrue())