diff --git a/internal/helm/chart.go b/internal/helm/chart.go index accbc69a9..dcc868c1d 100644 --- a/internal/helm/chart.go +++ b/internal/helm/chart.go @@ -70,17 +70,17 @@ func OverwriteChartDefaultValues(chart *helmchart.Chart, data []byte) (bool, err // LoadChartMetadata attempts to load the chart.Metadata from the "Chart.yaml" file in the directory or archive at the // given chartPath. It takes "requirements.yaml" files into account, and is therefore compatible with the // chart.APIVersionV1 format. -func LoadChartMetadata(chartPath string) (*helmchart.Metadata, error) { +func LoadChartMetadata(chartPath string) (meta *helmchart.Metadata, err error) { i, err := os.Stat(chartPath) if err != nil { return nil, err } - switch { - case i.IsDir(): - return LoadChartMetadataFromDir(chartPath) - default: - return LoadChartMetadataFromArchive(chartPath) + if i.IsDir() { + meta, err = LoadChartMetadataFromDir(chartPath) + return } + meta, err = LoadChartMetadataFromArchive(chartPath) + return } // LoadChartMetadataFromDir loads the chart.Metadata from the "Chart.yaml" file in the directory at the given path. diff --git a/internal/helm/chart_builder.go b/internal/helm/chart_builder.go new file mode 100644 index 000000000..7b90cba81 --- /dev/null +++ b/internal/helm/chart_builder.go @@ -0,0 +1,384 @@ +/* +Copyright 2021 The Flux authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package helm + +import ( + "context" + "fmt" + "os" + "path/filepath" + "strings" + "sync" + + securejoin "github.com/cyphar/filepath-securejoin" + "github.com/fluxcd/source-controller/internal/fs" + helmchart "helm.sh/helm/v3/pkg/chart" + "helm.sh/helm/v3/pkg/chart/loader" + "helm.sh/helm/v3/pkg/chartutil" + "sigs.k8s.io/yaml" + + "github.com/fluxcd/pkg/runtime/transform" +) + +// ChartBuilder aims to efficiently build a Helm chart from a directory or packaged chart. +// It avoids or delays loading the chart into memory in full, working with chart.Metadata +// as much as it can, and returns early (by copying over the already packaged source chart) +// if no modifications were made during the build process. +type ChartBuilder struct { + // baseDir is the chroot for the chart builder when path isDir. + // It must be (a higher) relative to path. File references (during e.g. + // value file merge operations) are not allowed to traverse out of it. + baseDir string + + // path is the file or directory path to a chart source. + path string + + // chart holds a (partly) loaded chart.Chart, it contains at least the + // chart.Metadata, which may expand to the full chart.Chart if required + // for Build operations. + chart *helmchart.Chart + + // valueFiles holds a list of path references of valueFiles that should be + // merged and packaged as a single "values.yaml" during Build. + valueFiles []string + + // repositories holds an index of repository URLs and their ChartRepository. + // They are used to configure a DependencyManager for missing chart dependencies + // if isDir is true. + repositories map[string]*ChartRepository + + // getChartRepositoryCallback is used to configure a DependencyManager for + // missing chart dependencies if isDir is true. + getChartRepositoryCallback GetChartRepositoryCallback + + mu sync.Mutex +} + +// NewChartBuilder constructs a new ChartBuilder for the given chart path. +// It returns an error if no chart.Metadata can be loaded from the path. +func NewChartBuilder(path string) (*ChartBuilder, error) { + metadata, err := LoadChartMetadata(path) + if err != nil { + return nil, fmt.Errorf("could not create new chart builder: %w", err) + } + return &ChartBuilder{ + path: path, + chart: &helmchart.Chart{ + Metadata: metadata, + }, + }, nil +} + +// WithBaseDir configures the base dir on the ChartBuilder. +func (b *ChartBuilder) WithBaseDir(p string) *ChartBuilder { + b.mu.Lock() + b.baseDir = p + b.mu.Unlock() + return b +} + +// WithValueFiles appends the given paths to the ChartBuilder's valueFiles. +func (b *ChartBuilder) WithValueFiles(path ...string) *ChartBuilder { + b.mu.Lock() + b.valueFiles = append(b.valueFiles, path...) + b.mu.Unlock() + return b +} + +// WithChartRepository indexes the given ChartRepository by the NormalizeChartRepositoryURL, +// used to configure the DependencyManager if the chart is not packaged. +func (b *ChartBuilder) WithChartRepository(url string, index *ChartRepository) *ChartBuilder { + b.mu.Lock() + b.repositories[NormalizeChartRepositoryURL(url)] = index + b.mu.Unlock() + return b +} + +// WithChartRepositoryCallback configures the GetChartRepositoryCallback used by the +// DependencyManager if the chart is not packaged. +func (b *ChartBuilder) WithChartRepositoryCallback(c GetChartRepositoryCallback) *ChartBuilder { + b.mu.Lock() + b.getChartRepositoryCallback = c + b.mu.Unlock() + return b +} + +// ChartBuildResult contains the ChartBuilder result, including build specific +// information about the chart. +type ChartBuildResult struct { + // SourceIsDir indicates if the chart was build from a directory. + SourceIsDir bool + // Path contains the absolute path to the packaged chart. + Path string + // ValuesOverwrite holds a structured map with the merged values used + // to overwrite chart default "values.yaml". + ValuesOverwrite map[string]interface{} + // CollectedDependencies contains the number of missing local and remote + // dependencies that were collected by the DependencyManager before building + // the chart. + CollectedDependencies int + // Packaged indicates if the ChartBuilder has packaged the chart. + // This can for example be false if SourceIsDir is false and ValuesOverwrite + // is nil, which makes the ChartBuilder copy the chart source to Path without + // making any modifications. + Packaged bool +} + +// String returns the Path of the ChartBuildResult. +func (b *ChartBuildResult) String() string { + if b != nil { + return b.Path + } + return "" +} + +// Build attempts to build a new chart using ChartBuilder configuration, +// writing it to the provided path. +// It returns a ChartBuildResult containing all information about the resulting chart, +// or an error. +func (b *ChartBuilder) Build(ctx context.Context, p string) (_ *ChartBuildResult, err error) { + b.mu.Lock() + defer b.mu.Unlock() + + if b.chart == nil { + err = fmt.Errorf("chart build failed: no initial chart (metadata) loaded") + return + } + if b.path == "" { + err = fmt.Errorf("chart build failed: no path set") + return + } + + result := &ChartBuildResult{} + result.SourceIsDir = pathIsDir(b.path) + result.Path = p + + // Merge chart values + if err = b.mergeValues(result); err != nil { + err = fmt.Errorf("chart build failed: %w", err) + return + } + + // Ensure chart has all dependencies + if err = b.buildDependencies(ctx, result); err != nil { + err = fmt.Errorf("chart build failed: %w", err) + return + } + + // Package (or copy) chart + if err = b.packageChart(result); err != nil { + err = fmt.Errorf("chart package failed: %w", err) + return + } + return result, nil +} + +// load lazy-loads chart.Chart into chart from the set path, it replaces any previously set +// chart.Metadata shim. +func (b *ChartBuilder) load() (err error) { + if b.chart == nil || len(b.chart.Files) <= 0 { + if b.path == "" { + return fmt.Errorf("failed to load chart: path not set") + } + chart, err := loader.Load(b.path) + if err != nil { + return fmt.Errorf("failed to load chart: %w", err) + } + b.chart = chart + } + return +} + +// buildDependencies builds the missing dependencies for a chart from a directory. +// Using the chart using a NewDependencyManager and the configured repositories +// and getChartRepositoryCallback +// It returns the number of dependencies it collected, or an error. +func (b *ChartBuilder) buildDependencies(ctx context.Context, result *ChartBuildResult) (err error) { + if !result.SourceIsDir { + return + } + + if err = b.load(); err != nil { + err = fmt.Errorf("failed to ensure chart has no missing dependencies: %w", err) + return + } + + dm := NewDependencyManager(b.chart, b.baseDir, strings.TrimLeft(b.path, b.baseDir)). + WithRepositories(b.repositories). + WithChartRepositoryCallback(b.getChartRepositoryCallback) + + result.CollectedDependencies, err = dm.Build(ctx) + return +} + +// mergeValues strategically merges the valueFiles, it merges using mergeFileValues +// or mergeChartValues depending on if the chart is sourced from a package or directory. +// Ir only calls load to propagate the chart if required by the strategy. +// It returns the merged values, or an error. +func (b *ChartBuilder) mergeValues(result *ChartBuildResult) (err error) { + if len(b.valueFiles) == 0 { + return + } + + if result.SourceIsDir { + result.ValuesOverwrite, err = mergeFileValues(b.baseDir, b.valueFiles) + if err != nil { + err = fmt.Errorf("failed to merge value files: %w", err) + } + return + } + + // Values equal to default + if len(b.valueFiles) == 1 && b.valueFiles[0] == chartutil.ValuesfileName { + return + } + + if err = b.load(); err != nil { + err = fmt.Errorf("failed to merge chart values: %w", err) + return + } + + if result.ValuesOverwrite, err = mergeChartValues(b.chart, b.valueFiles); err != nil { + err = fmt.Errorf("failed to merge chart values: %w", err) + return + } + return nil +} + +// packageChart determines if it should copyFileToPath or packageToPath +// based on the provided result. It sets Packaged on ChartBuildResult to +// true if packageToPath is successful. +func (b *ChartBuilder) packageChart(result *ChartBuildResult) error { + // If we are not building from a directory, and we do not have any + // replacement values, we can copy over the already packaged source + // chart without making any modifications + if !result.SourceIsDir && len(result.ValuesOverwrite) == 0 { + if err := copyFileToPath(b.path, result.Path); err != nil { + return fmt.Errorf("chart build failed: %w", err) + } + return nil + } + + // Package chart to a new temporary directory + if err := packageToPath(b.chart, result.Path); err != nil { + return fmt.Errorf("chart build failed: %w", err) + } + result.Packaged = true + return nil +} + +// mergeChartValues merges the given chart.Chart Files paths into a single "values.yaml" map. +// It returns the merge result, or an error. +func mergeChartValues(chart *helmchart.Chart, paths []string) (map[string]interface{}, error) { + mergedValues := make(map[string]interface{}) + for _, p := range paths { + cfn := filepath.Clean(p) + if cfn == chartutil.ValuesfileName { + mergedValues = transform.MergeMaps(mergedValues, chart.Values) + continue + } + var b []byte + for _, f := range chart.Files { + if f.Name == cfn { + b = f.Data + break + } + } + if b == nil { + return nil, fmt.Errorf("no values file found at path '%s'", p) + } + values := make(map[string]interface{}) + if err := yaml.Unmarshal(b, &values); err != nil { + return nil, fmt.Errorf("unmarshaling values from '%s' failed: %w", p, err) + } + mergedValues = transform.MergeMaps(mergedValues, values) + } + return mergedValues, nil +} + +// 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 +// result, or an error. +func mergeFileValues(baseDir string, paths []string) (map[string]interface{}, error) { + mergedValues := make(map[string]interface{}) + for _, p := range paths { + secureP, err := securejoin.SecureJoin(baseDir, p) + if err != nil { + return nil, err + } + if f, err := os.Stat(secureP); os.IsNotExist(err) || !f.Mode().IsRegular() { + return nil, fmt.Errorf("no values file found at path '%s' (reference '%s')", + strings.TrimPrefix(secureP, baseDir), p) + } + b, err := os.ReadFile(secureP) + if err != nil { + return nil, fmt.Errorf("could not read values from file '%s': %w", p, err) + } + values := make(map[string]interface{}) + err = yaml.Unmarshal(b, &values) + if err != nil { + return nil, fmt.Errorf("unmarshaling values from '%s' failed: %w", p, err) + } + mergedValues = transform.MergeMaps(mergedValues, values) + } + return mergedValues, nil +} + +// copyFileToPath attempts to copy in to out. It returns an error if out already exists. +func copyFileToPath(in, out string) error { + o, err := os.Create(out) + if err != nil { + return fmt.Errorf("failed to create copy target: %w", err) + } + defer o.Close() + i, err := os.Open(in) + if err != nil { + return fmt.Errorf("failed to open file to copy from: %w", err) + } + defer i.Close() + if _, err := o.ReadFrom(i); err != nil { + return fmt.Errorf("failed to read from source during copy: %w", err) + } + return nil +} + +// packageToPath attempts to package the given chart.Chart to the out filepath. +func packageToPath(chart *helmchart.Chart, out string) error { + o, err := os.MkdirTemp("", "chart-build-*") + if err != nil { + return fmt.Errorf("failed to create temporary directory for chart: %w", err) + } + defer os.RemoveAll(o) + + p, err := chartutil.Save(chart, o) + if err != nil { + return fmt.Errorf("failed to package chart: %w", err) + } + return fs.RenameWithFallback(p, out) +} + +// pathIsDir returns a boolean indicating if the given path points to a directory. +// In case os.Stat on the given path returns an error it returns false as well. +func pathIsDir(p string) bool { + if p == "" { + return false + } + if i, err := os.Stat(p); err != nil || !i.IsDir() { + return false + } + return true +} diff --git a/internal/helm/chart_builder_test.go b/internal/helm/chart_builder_test.go new file mode 100644 index 000000000..afc0107ce --- /dev/null +++ b/internal/helm/chart_builder_test.go @@ -0,0 +1,598 @@ +/* +Copyright 2021 The Flux authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package helm + +import ( + "context" + "encoding/hex" + "fmt" + "math/rand" + "os" + "path/filepath" + "sync" + "testing" + + . "github.com/onsi/gomega" + helmchart "helm.sh/helm/v3/pkg/chart" + "helm.sh/helm/v3/pkg/chart/loader" + "helm.sh/helm/v3/pkg/chartutil" + "helm.sh/helm/v3/pkg/repo" +) + +func TestChartBuildResult_String(t *testing.T) { + g := NewWithT(t) + + var result *ChartBuildResult + g.Expect(result.String()).To(Equal("")) + result = &ChartBuildResult{} + g.Expect(result.String()).To(Equal("")) + result = &ChartBuildResult{Path: "/foo/"} + g.Expect(result.String()).To(Equal("/foo/")) +} + +func TestChartBuilder_Build(t *testing.T) { + tests := []struct { + name string + baseDir string + path string + valueFiles []string + repositories map[string]*ChartRepository + getChartRepositoryCallback GetChartRepositoryCallback + wantErr string + }{ + { + name: "builds chart from directory", + path: "testdata/charts/helmchart", + }, + { + name: "builds chart from package", + path: "testdata/charts/helmchart-0.1.0.tgz", + }, + { + // TODO(hidde): add more diverse tests + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + b, err := NewChartBuilder(tt.path) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(b).ToNot(BeNil()) + + b.WithBaseDir(tt.baseDir) + b.WithValueFiles(tt.valueFiles...) + b.WithChartRepositoryCallback(b.getChartRepositoryCallback) + for k, v := range tt.repositories { + b.WithChartRepository(k, v) + } + + out := tmpFile("build-0.1.0", ".tgz") + defer os.RemoveAll(out) + got, err := b.Build(context.TODO(), out) + if tt.wantErr != "" { + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(ContainSubstring(tt.wantErr)) + g.Expect(got).To(BeNil()) + return + } + + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(got).ToNot(BeNil()) + + g.Expect(got.Path).ToNot(BeEmpty()) + g.Expect(got.Path).To(Equal(out)) + g.Expect(got.Path).To(BeARegularFile()) + _, err = loader.Load(got.Path) + g.Expect(err).ToNot(HaveOccurred()) + }) + } +} + +func TestChartBuilder_load(t *testing.T) { + tests := []struct { + name string + path string + chart *helmchart.Chart + wantFunc func(g *WithT, c *helmchart.Chart) + wantErr string + }{ + { + name: "loads chart", + chart: nil, + path: "testdata/charts/helmchart-0.1.0.tgz", + wantFunc: func(g *WithT, c *helmchart.Chart) { + g.Expect(c.Metadata.Name).To(Equal("helmchart")) + g.Expect(c.Files).ToNot(BeZero()) + }, + }, + { + name: "overwrites chart without any files (metadata shim)", + chart: &helmchart.Chart{ + Metadata: &helmchart.Metadata{Name: "dummy"}, + }, + path: "testdata/charts/helmchart-0.1.0.tgz", + wantFunc: func(g *WithT, c *helmchart.Chart) { + g.Expect(c.Metadata.Name).To(Equal("helmchart")) + g.Expect(c.Files).ToNot(BeZero()) + }, + }, + { + name: "does not overwrite loaded chart", + chart: &helmchart.Chart{ + Metadata: &helmchart.Metadata{Name: "dummy"}, + Files: []*helmchart.File{ + {Name: "mock.yaml", Data: []byte("loaded chart")}, + }, + }, + path: "testdata/charts/helmchart-0.1.0.tgz", + wantFunc: func(g *WithT, c *helmchart.Chart) { + g.Expect(c.Metadata.Name).To(Equal("dummy")) + g.Expect(c.Files).To(HaveLen(1)) + }, + }, + { + name: "no path", + wantErr: "failed to load chart: path not set", + }, + { + name: "invalid chart", + path: "testdata/charts/empty.tgz", + wantErr: "failed to load chart: no files in chart archive", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + b := &ChartBuilder{ + path: tt.path, + chart: tt.chart, + } + err := b.load() + if tt.wantErr != "" { + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(ContainSubstring(tt.wantErr)) + return + } + + g.Expect(err).ToNot(HaveOccurred()) + if tt.wantFunc != nil { + tt.wantFunc(g, b.chart) + } + }) + } +} + +func TestChartBuilder_buildDependencies(t *testing.T) { + g := NewWithT(t) + + chartB, err := os.ReadFile("testdata/charts/helmchart-0.1.0.tgz") + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(chartB).ToNot(BeEmpty()) + + mockRepo := func() *ChartRepository { + return &ChartRepository{ + Client: &mockGetter{ + response: chartB, + }, + Index: &repo.IndexFile{ + Entries: map[string]repo.ChartVersions{ + "grafana": { + &repo.ChartVersion{ + Metadata: &helmchart.Metadata{ + Name: "grafana", + Version: "6.17.4", + }, + URLs: []string{"https://example.com/chart.tgz"}, + }, + }, + }, + }, + RWMutex: &sync.RWMutex{}, + } + } + + var mockCallback GetChartRepositoryCallback = func(url string) (*ChartRepository, error) { + if url == "https://grafana.github.io/helm-charts/" { + return mockRepo(), nil + } + return nil, fmt.Errorf("no repository for URL") + } + + tests := []struct { + name string + baseDir string + path string + chart *helmchart.Chart + fromDir bool + repositories map[string]*ChartRepository + getChartRepositoryCallback GetChartRepositoryCallback + wantCollectedDependencies int + wantErr string + }{ + { + name: "builds dependencies using callback", + fromDir: true, + baseDir: "testdata/charts", + path: "testdata/charts/helmchartwithdeps", + getChartRepositoryCallback: mockCallback, + wantCollectedDependencies: 2, + }, + { + name: "builds dependencies using repositories", + fromDir: true, + baseDir: "testdata/charts", + path: "testdata/charts/helmchartwithdeps", + repositories: map[string]*ChartRepository{ + "https://grafana.github.io/helm-charts/": mockRepo(), + }, + wantCollectedDependencies: 2, + }, + { + name: "skips dependency build for packaged chart", + path: "testdata/charts/helmchart-0.1.0.tgz", + }, + { + name: "attempts to load chart", + fromDir: true, + path: "testdata", + wantErr: "failed to ensure chart has no missing dependencies", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + b := &ChartBuilder{ + baseDir: tt.baseDir, + path: tt.path, + chart: tt.chart, + repositories: tt.repositories, + getChartRepositoryCallback: tt.getChartRepositoryCallback, + } + + result := &ChartBuildResult{SourceIsDir: tt.fromDir} + err := b.buildDependencies(context.TODO(), result) + if tt.wantErr != "" { + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(ContainSubstring(tt.wantErr)) + g.Expect(result.CollectedDependencies).To(BeZero()) + g.Expect(b.chart).To(Equal(tt.chart)) + return + } + + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(result).ToNot(BeNil()) + g.Expect(result.CollectedDependencies).To(Equal(tt.wantCollectedDependencies)) + if tt.wantCollectedDependencies > 0 { + g.Expect(b.chart).ToNot(Equal(tt.chart)) + } + }) + } +} + +func TestChartBuilder_mergeValues(t *testing.T) { + tests := []struct { + name string + baseDir string + path string + isDir bool + chart *helmchart.Chart + valueFiles []string + want map[string]interface{} + wantErr string + }{ + { + name: "merges chart values", + chart: &helmchart.Chart{ + Files: []*helmchart.File{ + {Name: "a.yaml", Data: []byte("a: b")}, + {Name: "b.yaml", Data: []byte("a: c")}, + }, + }, + valueFiles: []string{"a.yaml", "b.yaml"}, + want: map[string]interface{}{ + "a": "c", + }, + }, + { + name: "chart values merge error", + chart: &helmchart.Chart{ + Files: []*helmchart.File{ + {Name: "b.yaml", Data: []byte("a: c")}, + }, + }, + valueFiles: []string{"a.yaml"}, + wantErr: "failed to merge chart values", + }, + { + name: "merges file values", + isDir: true, + baseDir: "testdata/charts", + path: "helmchart", + valueFiles: []string{"helmchart/values-prod.yaml"}, + want: map[string]interface{}{ + "replicaCount": float64(2), + }, + }, + { + name: "file values merge error", + isDir: true, + baseDir: "testdata/charts", + path: "helmchart", + valueFiles: []string{"invalid.yaml"}, + wantErr: "failed to merge value files", + }, + { + name: "error on chart load failure", + baseDir: "testdata/charts", + path: "invalid", + wantErr: "failed to load chart", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + b := &ChartBuilder{ + baseDir: tt.baseDir, + path: tt.path, + chart: tt.chart, + valueFiles: tt.valueFiles, + } + + result := &ChartBuildResult{SourceIsDir: tt.isDir} + err := b.mergeValues(result) + if tt.wantErr != "" { + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(ContainSubstring(tt.wantErr)) + g.Expect(result.ValuesOverwrite).To(BeNil()) + return + } + + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(result.ValuesOverwrite).To(Equal(tt.want)) + }) + } +} + +func Test_mergeChartValues(t *testing.T) { + tests := []struct { + name string + chart *helmchart.Chart + paths []string + want map[string]interface{} + wantErr string + }{ + { + name: "merges values", + chart: &helmchart.Chart{ + Files: []*helmchart.File{ + {Name: "a.yaml", Data: []byte("a: b")}, + {Name: "b.yaml", Data: []byte("b: c")}, + {Name: "c.yaml", Data: []byte("b: d")}, + }, + }, + paths: []string{"a.yaml", "b.yaml", "c.yaml"}, + want: map[string]interface{}{ + "a": "b", + "b": "d", + }, + }, + { + name: "uses chart values", + chart: &helmchart.Chart{ + Files: []*helmchart.File{ + {Name: "c.yaml", Data: []byte("b: d")}, + }, + Values: map[string]interface{}{ + "a": "b", + }, + }, + paths: []string{chartutil.ValuesfileName, "c.yaml"}, + want: map[string]interface{}{ + "a": "b", + "b": "d", + }, + }, + { + name: "unmarshal error", + chart: &helmchart.Chart{ + Files: []*helmchart.File{ + {Name: "invalid", Data: []byte("abcd")}, + }, + }, + paths: []string{"invalid"}, + wantErr: "unmarshaling values from 'invalid' failed", + }, + { + name: "error on invalid path", + chart: &helmchart.Chart{}, + paths: []string{"a.yaml"}, + wantErr: "no values file found at path 'a.yaml'", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + got, err := mergeChartValues(tt.chart, tt.paths) + if tt.wantErr != "" { + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(ContainSubstring(tt.wantErr)) + g.Expect(got).To(BeNil()) + return + } + + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(got).To(Equal(tt.want)) + }) + } +} + +func Test_mergeFileValues(t *testing.T) { + tests := []struct { + name string + files []*helmchart.File + paths []string + want map[string]interface{} + wantErr string + }{ + { + name: "merges values from files", + files: []*helmchart.File{ + {Name: "a.yaml", Data: []byte("a: b")}, + {Name: "b.yaml", Data: []byte("b: c")}, + {Name: "c.yaml", Data: []byte("b: d")}, + }, + paths: []string{"a.yaml", "b.yaml", "c.yaml"}, + want: map[string]interface{}{ + "a": "b", + "b": "d", + }, + }, + { + name: "illegal traverse", + paths: []string{"../../../traversing/illegally/a/p/a/b"}, + wantErr: "no values file found at path '/traversing/illegally/a/p/a/b'", + }, + { + name: "unmarshal error", + files: []*helmchart.File{ + {Name: "invalid", Data: []byte("abcd")}, + }, + paths: []string{"invalid"}, + wantErr: "unmarshaling values from 'invalid' failed", + }, + { + name: "error on invalid path", + paths: []string{"a.yaml"}, + wantErr: "no values file found at path '/a.yaml'", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + baseDir, err := os.MkdirTemp("", "merge-file-values-*") + g.Expect(err).ToNot(HaveOccurred()) + defer os.RemoveAll(baseDir) + + for _, f := range tt.files { + g.Expect(os.WriteFile(filepath.Join(baseDir, f.Name), f.Data, 0644)).To(Succeed()) + } + + got, err := mergeFileValues(baseDir, tt.paths) + if tt.wantErr != "" { + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(ContainSubstring(tt.wantErr)) + g.Expect(got).To(BeNil()) + return + } + + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(got).To(Equal(tt.want)) + }) + } +} + +func Test_copyFileToPath(t *testing.T) { + tests := []struct { + name string + in string + wantErr string + }{ + { + name: "copies input file", + in: "testdata/local-index.yaml", + }, + { + name: "invalid input file", + in: "testdata/invalid.tgz", + wantErr: "failed to open file to copy from", + }, + { + name: "invalid input directory", + in: "testdata/charts", + wantErr: "failed to read from source during copy", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + out := tmpFile("copy-0.1.0", ".tgz") + defer os.RemoveAll(out) + err := copyFileToPath(tt.in, out) + if tt.wantErr != "" { + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(ContainSubstring(tt.wantErr)) + return + } + + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(out).To(BeARegularFile()) + f1, err := os.ReadFile(tt.in) + g.Expect(err).ToNot(HaveOccurred()) + f2, err := os.ReadFile(out) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(f2).To(Equal(f1)) + }) + } +} + +func Test_packageToPath(t *testing.T) { + g := NewWithT(t) + + chart, err := loader.Load("testdata/charts/helmchart-0.1.0.tgz") + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(chart).ToNot(BeNil()) + + out := tmpFile("chart-0.1.0", ".tgz") + defer os.RemoveAll(out) + err = packageToPath(chart, out) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(out).To(BeARegularFile()) + _, err = loader.Load(out) + g.Expect(err).ToNot(HaveOccurred()) +} + +func Test_pathIsDir(t *testing.T) { + tests := []struct { + name string + p string + want bool + }{ + {name: "directory", p: "testdata/", want: true}, + {name: "file", p: "testdata/local-index.yaml", want: false}, + {name: "not found error", p: "testdata/does-not-exist.yaml", want: false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + g.Expect(pathIsDir(tt.p)).To(Equal(tt.want)) + }) + } +} + +func tmpFile(prefix, suffix string) string { + randBytes := make([]byte, 16) + rand.Read(randBytes) + return filepath.Join(os.TempDir(), prefix+hex.EncodeToString(randBytes)+suffix) +} diff --git a/internal/helm/chart_test.go b/internal/helm/chart_test.go index 7afa2a3f6..23d50b96b 100644 --- a/internal/helm/chart_test.go +++ b/internal/helm/chart_test.go @@ -17,7 +17,6 @@ limitations under the License. package helm import ( - "reflect" "testing" . "github.com/onsi/gomega" @@ -87,33 +86,35 @@ func TestOverwriteChartDefaultValues(t *testing.T) { } for _, tt := range testCases { t.Run(tt.desc, func(t *testing.T) { + g := NewWithT(t) + fixture := tt.chart ok, err := OverwriteChartDefaultValues(&fixture, tt.data) - if ok != tt.ok { - t.Fatalf("should return %v, returned %v", tt.ok, ok) - } - if err != nil && !tt.expectErr { - t.Fatalf("returned unexpected error: %v", err) - } - if err == nil && tt.expectErr { - t.Fatal("expected error") + g.Expect(ok).To(Equal(tt.ok)) + + if tt.expectErr { + g.Expect(err).To(HaveOccurred()) + g.Expect(ok).To(Equal(tt.ok)) + return } - for _, f := range fixture.Raw { - if f.Name == chartutil.ValuesfileName && reflect.DeepEqual(f.Data, originalValuesFixture) && tt.ok { - t.Error("should override values.yaml in Raw field") + if tt.ok { + for _, f := range fixture.Raw { + if f.Name == chartutil.ValuesfileName { + g.Expect(f.Data).To(Equal(tt.data)) + } } - } - for _, f := range fixture.Files { - if f.Name == chartutil.ValuesfileName && reflect.DeepEqual(f.Data, originalValuesFixture) && tt.ok { - t.Error("should override values.yaml in Files field") + for _, f := range fixture.Files { + if f.Name == chartutil.ValuesfileName { + g.Expect(f.Data).To(Equal(tt.data)) + } } } }) } } -func Test_LoadChartMetadataFromDir(t *testing.T) { +func TestLoadChartMetadataFromDir(t *testing.T) { tests := []struct { name string dir string diff --git a/internal/helm/dependency_manager.go b/internal/helm/dependency_manager.go index 19d56c884..043b0e7e3 100644 --- a/internal/helm/dependency_manager.go +++ b/internal/helm/dependency_manager.go @@ -33,165 +33,282 @@ import ( "helm.sh/helm/v3/pkg/chart/loader" ) -// DependencyWithRepository is a container for a Helm chart dependency -// and its respective repository. -type DependencyWithRepository struct { - // Dependency holds the reference to a chart.Chart dependency. - Dependency *helmchart.Dependency - // Repository is the ChartRepository the dependency should be - // available at and can be downloaded from. If there is none, - // a local ('file://') dependency is assumed. - Repository *ChartRepository -} +// GetChartRepositoryCallback must return a ChartRepository for the URL, +// or an error describing why it could not be returned. +type GetChartRepositoryCallback func(url string) (*ChartRepository, error) -// DependencyManager manages dependencies for a Helm chart. +// DependencyManager manages dependencies for a Helm chart, downloading +// only those that are missing from the chart it holds. type DependencyManager struct { - // WorkingDir is the chroot path for dependency manager operations, + // chart contains the chart.Chart from the path. + chart *helmchart.Chart + + // baseDir is the chroot path for dependency manager operations, // Dependencies that hold a local (relative) path reference are not // allowed to traverse outside this directory. - WorkingDir string - // ChartPath is the path of the Chart relative to the WorkingDir, - // the combination of the WorkingDir and ChartPath is used to + baseDir string + + // path is the path of the chart relative to the baseDir, + // the combination of the baseDir and path is used to // determine the absolute path of a local dependency. - ChartPath string - // Chart holds the loaded chart.Chart from the ChartPath. - Chart *helmchart.Chart - // Dependencies contains a list of dependencies, and the respective - // repository the dependency can be found at. - Dependencies []*DependencyWithRepository - // Workers is the number of concurrent chart-add operations during + path string + + // repositories contains a map of ChartRepository indexed by their + // normalized URL. It is used as a lookup table for missing + // dependencies. + repositories map[string]*ChartRepository + + // getChartRepositoryCallback can be set to an on-demand get + // callback which returned result is cached to repositories. + getChartRepositoryCallback GetChartRepositoryCallback + + // workers is the number of concurrent chart-add operations during // Build. Defaults to 1 (non-concurrent). - Workers int64 + workers int64 + // mu contains the lock for chart writes. mu sync.Mutex } -// Build compiles and builds the dependencies of the Chart with the -// configured number of Workers. -func (dm *DependencyManager) Build(ctx context.Context) error { - if len(dm.Dependencies) == 0 { - return nil +func NewDependencyManager(chart *helmchart.Chart, baseDir, path string) *DependencyManager { + return &DependencyManager{ + chart: chart, + baseDir: baseDir, + path: path, + } +} + +func (dm *DependencyManager) WithRepositories(r map[string]*ChartRepository) *DependencyManager { + dm.repositories = r + return dm +} + +func (dm *DependencyManager) WithChartRepositoryCallback(c GetChartRepositoryCallback) *DependencyManager { + dm.getChartRepositoryCallback = c + return dm +} + +func (dm *DependencyManager) WithWorkers(w int64) *DependencyManager { + dm.workers = w + return dm +} + +// Build compiles and builds the dependencies of the chart with the +// configured number of workers. +func (dm *DependencyManager) Build(ctx context.Context) (int, error) { + // Collect dependency metadata + var ( + deps = dm.chart.Dependencies() + reqs = dm.chart.Metadata.Dependencies + ) + // Lock file takes precedence + if lock := dm.chart.Lock; lock != nil { + reqs = lock.Dependencies + } + + // Collect missing dependencies + missing := collectMissing(deps, reqs) + if len(missing) == 0 { + return 0, nil + } + + // Run the build for the missing dependencies + if err := dm.build(ctx, missing); err != nil { + return 0, err } + return len(missing), nil +} - workers := dm.Workers +// build (concurrently) adds the given list of deps to the chart with the configured +// number of workers. It returns the first error, cancelling all other workers. +func (dm *DependencyManager) build(ctx context.Context, deps map[string]*helmchart.Dependency) error { + workers := dm.workers if workers <= 0 { workers = 1 } + // Garbage collect temporary cached ChartRepository indexes defer func() { - for _, dep := range dm.Dependencies { - dep.Repository.UnloadIndex() + for _, v := range dm.repositories { + v.Unload() + _ = v.RemoveCache() } }() group, groupCtx := errgroup.WithContext(ctx) group.Go(func() error { sem := semaphore.NewWeighted(workers) - for _, dep := range dm.Dependencies { - dep := dep + for name, dep := range deps { + name, dep := name, dep if err := sem.Acquire(groupCtx, 1); err != nil { return err } - group.Go(func() error { + group.Go(func() (err error) { defer sem.Release(1) - if dep.Repository == nil { - return dm.addLocalDependency(dep) + if isLocalDep(dep) { + if err = dm.addLocalDependency(dep); err != nil { + err = fmt.Errorf("failed to add local dependency '%s': %w", name, err) + } + return } - return dm.addRemoteDependency(dep) + if err = dm.addRemoteDependency(dep); err != nil { + err = fmt.Errorf("failed to add remote dependency '%s': %w", name, err) + } + return }) } return nil }) - return group.Wait() } -func (dm *DependencyManager) addLocalDependency(dpr *DependencyWithRepository) error { - sLocalChartPath, err := dm.secureLocalChartPath(dpr) +// addLocalDependency attempts to resolve and add the given local chart.Dependency to the chart. +func (dm *DependencyManager) addLocalDependency(dep *helmchart.Dependency) error { + sLocalChartPath, err := dm.secureLocalChartPath(dep) if err != nil { return err } if _, err := os.Stat(sLocalChartPath); err != nil { if os.IsNotExist(err) { - return fmt.Errorf("no chart found at '%s' (reference '%s') for dependency '%s'", - strings.TrimPrefix(sLocalChartPath, dm.WorkingDir), dpr.Dependency.Repository, dpr.Dependency.Name) + return fmt.Errorf("no chart found at '%s' (reference '%s')", + strings.TrimPrefix(sLocalChartPath, dm.baseDir), dep.Repository) } return err } - ch, err := loader.Load(sLocalChartPath) + constraint, err := semver.NewConstraint(dep.Version) if err != nil { + err = fmt.Errorf("invalid version/constraint format '%s': %w", dep.Version, err) return err } - constraint, err := semver.NewConstraint(dpr.Dependency.Version) + ch, err := loader.Load(sLocalChartPath) if err != nil { - err := fmt.Errorf("dependency '%s' has an invalid version/constraint format: %w", dpr.Dependency.Name, err) - return err + return fmt.Errorf("failed to load chart from '%s' (reference '%s'): %w", + strings.TrimPrefix(sLocalChartPath, dm.baseDir), dep.Repository, err) } - v, err := semver.NewVersion(ch.Metadata.Version) + ver, err := semver.NewVersion(ch.Metadata.Version) if err != nil { return err } - if !constraint.Check(v) { - err = fmt.Errorf("can't get a valid version for dependency '%s'", dpr.Dependency.Name) + if !constraint.Check(ver) { + err = fmt.Errorf("can't get a valid version for constraint '%s'", dep.Version) return err } dm.mu.Lock() - dm.Chart.AddDependency(ch) + dm.chart.AddDependency(ch) dm.mu.Unlock() - return nil } -func (dm *DependencyManager) addRemoteDependency(dpr *DependencyWithRepository) error { - if dpr.Repository == nil { - return fmt.Errorf("no HelmRepository for '%s' dependency", dpr.Dependency.Name) +// addRemoteDependency attempts to resolve and add the given remote chart.Dependency to the chart. +func (dm *DependencyManager) addRemoteDependency(dep *helmchart.Dependency) error { + repo, err := dm.resolveRepository(dep.Repository) + if err != nil { + return err } - if !dpr.Repository.HasIndex() { - if !dpr.Repository.HasCacheFile() { - if _, err := dpr.Repository.CacheIndex(); err != nil { - return err - } - } - if err := dpr.Repository.LoadFromCache(); err != nil { - return err - } + if err = repo.StrategicallyLoadIndex(); err != nil { + return fmt.Errorf("failed to load index for '%s': %w", dep.Name, err) } - chartVer, err := dpr.Repository.Get(dpr.Dependency.Name, dpr.Dependency.Version) + + ver, err := repo.Get(dep.Name, dep.Version) if err != nil { return err } - - res, err := dpr.Repository.DownloadChart(chartVer) + res, err := repo.DownloadChart(ver) if err != nil { - return err + return fmt.Errorf("chart download of version '%s' failed: %w", ver.Version, err) } - ch, err := loader.LoadArchive(res) if err != nil { - return err + return fmt.Errorf("failed to load downloaded archive of version '%s': %w", ver.Version, err) } dm.mu.Lock() - dm.Chart.AddDependency(ch) + dm.chart.AddDependency(ch) dm.mu.Unlock() return nil } -func (dm *DependencyManager) secureLocalChartPath(dep *DependencyWithRepository) (string, error) { - localUrl, err := url.Parse(dep.Dependency.Repository) +// resolveRepository first attempts to resolve the url from the repositories, falling back +// to getChartRepositoryCallback if set. It returns the resolved ChartRepository, or an error. +func (dm *DependencyManager) resolveRepository(url string) (_ *ChartRepository, err error) { + dm.mu.Lock() + defer dm.mu.Unlock() + + nUrl := NormalizeChartRepositoryURL(url) + if _, ok := dm.repositories[nUrl]; !ok { + if dm.getChartRepositoryCallback == nil { + err = fmt.Errorf("no chart repository for URL '%s'", nUrl) + return + } + if dm.repositories == nil { + dm.repositories = map[string]*ChartRepository{} + } + if dm.repositories[nUrl], err = dm.getChartRepositoryCallback(nUrl); err != nil { + err = fmt.Errorf("failed to get chart repository for URL '%s': %w", nUrl, err) + return + } + } + return dm.repositories[nUrl], nil +} + +// secureLocalChartPath returns the secure absolute path of a local dependency. +// It does not allow the dependency's path to be outside the scope of baseDir. +func (dm *DependencyManager) secureLocalChartPath(dep *helmchart.Dependency) (string, error) { + localUrl, err := url.Parse(dep.Repository) if err != nil { return "", fmt.Errorf("failed to parse alleged local chart reference: %w", err) } if localUrl.Scheme != "" && localUrl.Scheme != "file" { - return "", fmt.Errorf("'%s' is not a local chart reference", dep.Dependency.Repository) + return "", fmt.Errorf("'%s' is not a local chart reference", dep.Repository) + } + return securejoin.SecureJoin(dm.baseDir, filepath.Join(dm.path, localUrl.Host, localUrl.Path)) +} + +// 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. +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 + if len(current) == len(reqs) { + return nil + } + + // Build up a map of reqs that are not in current, indexed by their + // alias or name + var missing map[string]*helmchart.Dependency + for _, dep := range reqs { + name := dep.Name + if dep.Alias != "" { + name = dep.Alias + } + // Exclude existing dependencies + found := false + for _, existing := range current { + if existing.Name() == name { + found = true + } + } + if found { + continue + } + if missing == nil { + missing = map[string]*helmchart.Dependency{} + } + missing[name] = dep } - return securejoin.SecureJoin(dm.WorkingDir, filepath.Join(dm.ChartPath, localUrl.Host, localUrl.Path)) + return missing +} + +// isLocalDep returns true if the given chart.Dependency contains a local (file) path reference. +func isLocalDep(dep *helmchart.Dependency) bool { + return dep.Repository == "" || strings.HasPrefix(dep.Repository, "file://") } diff --git a/internal/helm/dependency_manager_test.go b/internal/helm/dependency_manager_test.go index a8e6a0480..e51e6b768 100644 --- a/internal/helm/dependency_manager_test.go +++ b/internal/helm/dependency_manager_test.go @@ -18,12 +18,16 @@ package helm import ( "context" + "errors" "fmt" "os" - "strings" + "path/filepath" + "sync" "testing" + . "github.com/onsi/gomega" helmchart "helm.sh/helm/v3/pkg/chart" + "helm.sh/helm/v3/pkg/chart/loader" "helm.sh/helm/v3/pkg/repo" ) @@ -47,177 +51,585 @@ var ( chartVersionV1 = "0.3.0" ) -func TestBuild_WithEmptyDependencies(t *testing.T) { - dm := DependencyManager{ - Dependencies: nil, +func TestDependencyManager_Build(t *testing.T) { + tests := []struct { + name string + baseDir string + path string + repositories map[string]*ChartRepository + getChartRepositoryCallback GetChartRepositoryCallback + want int + wantChartFunc func(g *WithT, c *helmchart.Chart) + wantErr string + }{ + //{ + // // TODO(hidde): add various happy paths + //}, + //{ + // // TODO(hidde): test Chart.lock + //}, + { + name: "build failure returns error", + baseDir: "testdata/charts", + path: "helmchartwithdeps", + wantErr: "failed to add remote dependency 'grafana': no chart repository for URL", + }, + { + name: "no dependencies returns zero", + baseDir: "testdata/charts", + path: "helmchart", + want: 0, + }, } - if err := dm.Build(context.TODO()); err != nil { - t.Errorf("Build() should return nil") + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + chart, err := loader.Load(filepath.Join(tt.baseDir, tt.path)) + g.Expect(err).ToNot(HaveOccurred()) + + got, err := NewDependencyManager(chart, tt.baseDir, tt.path). + WithRepositories(tt.repositories). + WithChartRepositoryCallback(tt.getChartRepositoryCallback). + Build(context.TODO()) + + if tt.wantErr != "" { + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(ContainSubstring(tt.wantErr)) + g.Expect(got).To(BeZero()) + return + } + + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(got).To(Equal(tt.want)) + if tt.wantChartFunc != nil { + tt.wantChartFunc(g, chart) + } + }) } } -func TestBuild_WithLocalChart(t *testing.T) { +func TestDependencyManager_build(t *testing.T) { tests := []struct { name string - dep helmchart.Dependency - wantErr bool - errMsg string + deps map[string]*helmchart.Dependency + wantErr string + }{ + { + name: "error remote dependency", + deps: map[string]*helmchart.Dependency{ + "example": {Repository: "https://example.com"}, + }, + wantErr: "failed to add remote dependency", + }, + { + name: "error local dependency", + deps: map[string]*helmchart.Dependency{ + "example": {Repository: "file:///invalid"}, + }, + wantErr: "failed to add remote dependency", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + dm := &DependencyManager{ + baseDir: "testdata/charts", + } + err := dm.build(context.TODO(), tt.deps) + if tt.wantErr != "" { + g.Expect(err).To(HaveOccurred()) + return + } + + g.Expect(err).ToNot(HaveOccurred()) + }) + } +} + +func TestDependencyManager_addLocalDependency(t *testing.T) { + tests := []struct { + name string + dep *helmchart.Dependency + wantErr string + wantFunc func(g *WithT, c *helmchart.Chart) }{ { - name: "valid path", - dep: helmchart.Dependency{ + name: "local dependency", + dep: &helmchart.Dependency{ Name: chartName, Version: chartVersion, - Repository: chartLocalRepository, + Repository: "file://../helmchart", + }, + wantFunc: func(g *WithT, c *helmchart.Chart) { + g.Expect(c.Dependencies()).To(HaveLen(1)) }, }, { - name: "valid path", - dep: helmchart.Dependency{ + name: "version not matching constraint", + dep: &helmchart.Dependency{ Name: chartName, - Alias: "aliased", - Version: chartVersion, - Repository: chartLocalRepository, + Version: "0.2.0", + Repository: "file://../helmchart", }, + wantErr: "can't get a valid version for constraint '0.2.0'", }, { - name: "allowed traversing path", - dep: helmchart.Dependency{ + name: "invalid local reference", + dep: &helmchart.Dependency{ Name: chartName, - Alias: "aliased", Version: chartVersion, - Repository: "file://../../../testdata/charts/helmchartwithdeps/../helmchart", + Repository: "file://../../../absolutely/invalid", }, + wantErr: "no chart found at 'absolutely/invalid'", }, { - name: "invalid path", - dep: helmchart.Dependency{ + name: "invalid chart archive", + dep: &helmchart.Dependency{ Name: chartName, Version: chartVersion, - Repository: "file://../invalid", + Repository: "file://../empty.tgz", }, - wantErr: true, - errMsg: "no chart found at", + wantErr: "failed to load chart from 'empty.tgz'", }, { - name: "illegal traversing path", - dep: helmchart.Dependency{ + name: "invalid constraint", + dep: &helmchart.Dependency{ Name: chartName, - Version: chartVersion, - Repository: "file://../../../../../controllers/testdata/charts/helmchart", + Version: "invalid", + Repository: "file://../helmchart", + }, + wantErr: "invalid version/constraint format 'invalid'", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + dm := &DependencyManager{ + chart: &helmchart.Chart{}, + baseDir: "testdata/charts/", + path: "helmchartwithdeps", + } + + err := dm.addLocalDependency(tt.dep) + if tt.wantErr != "" { + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(ContainSubstring(tt.wantErr)) + return + } + g.Expect(err).ToNot(HaveOccurred()) + }) + } +} + +func TestDependencyManager_addRemoteDependency(t *testing.T) { + g := NewWithT(t) + + chartB, err := os.ReadFile("testdata/charts/helmchart-0.1.0.tgz") + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(chartB).ToNot(BeEmpty()) + + tests := []struct { + name string + repositories map[string]*ChartRepository + dep *helmchart.Dependency + wantFunc func(g *WithT, c *helmchart.Chart) + wantErr string + }{ + { + name: "adds remote dependency", + repositories: map[string]*ChartRepository{ + "https://example.com/": { + Client: &mockGetter{ + response: chartB, + }, + Index: &repo.IndexFile{ + Entries: map[string]repo.ChartVersions{ + chartName: { + &repo.ChartVersion{ + Metadata: &helmchart.Metadata{ + Name: chartName, + Version: chartVersion, + }, + URLs: []string{"https://example.com/foo.tgz"}, + }, + }, + }, + }, + RWMutex: &sync.RWMutex{}, + }, + }, + dep: &helmchart.Dependency{ + Name: chartName, + Repository: "https://example.com", + }, + wantFunc: func(g *WithT, c *helmchart.Chart) { + g.Expect(c.Dependencies()).To(HaveLen(1)) + }, + }, + { + name: "resolve repository error", + repositories: map[string]*ChartRepository{}, + dep: &helmchart.Dependency{ + Repository: "https://example.com", + }, + wantErr: "no chart repository for URL", + }, + { + name: "strategic load error", + repositories: map[string]*ChartRepository{ + "https://example.com/": { + CachePath: "/invalid/cache/path/foo", + RWMutex: &sync.RWMutex{}, + }, + }, + dep: &helmchart.Dependency{ + Repository: "https://example.com", + }, + wantErr: "failed to strategically load index", + }, + { + name: "repository get error", + repositories: map[string]*ChartRepository{ + "https://example.com/": { + Index: &repo.IndexFile{}, + RWMutex: &sync.RWMutex{}, + }, + }, + dep: &helmchart.Dependency{ + Repository: "https://example.com", }, - wantErr: true, - errMsg: "no chart found at", + wantErr: "no chart name found", }, { - name: "invalid version constraint format", - dep: helmchart.Dependency{ + name: "repository version constraint error", + repositories: map[string]*ChartRepository{ + "https://example.com/": { + Index: &repo.IndexFile{ + Entries: map[string]repo.ChartVersions{ + chartName: { + &repo.ChartVersion{ + Metadata: &helmchart.Metadata{ + Name: chartName, + Version: "0.1.0", + }, + }, + }, + }, + }, + RWMutex: &sync.RWMutex{}, + }, + }, + dep: &helmchart.Dependency{ Name: chartName, - Version: "!2.0", - Repository: chartLocalRepository, + Version: "0.2.0", + Repository: "https://example.com", }, - wantErr: true, - errMsg: "has an invalid version/constraint format", + wantErr: fmt.Sprintf("no '%s' chart with version matching '0.2.0' found", chartName), }, { - name: "invalid version", - dep: helmchart.Dependency{ + name: "repository chart download error", + repositories: map[string]*ChartRepository{ + "https://example.com/": { + Index: &repo.IndexFile{ + Entries: map[string]repo.ChartVersions{ + chartName: { + &repo.ChartVersion{ + Metadata: &helmchart.Metadata{ + Name: chartName, + Version: chartVersion, + }, + }, + }, + }, + }, + RWMutex: &sync.RWMutex{}, + }, + }, + dep: &helmchart.Dependency{ Name: chartName, Version: chartVersion, - Repository: chartLocalRepository, + Repository: "https://example.com", }, - wantErr: true, - errMsg: "can't get a valid version for dependency", + wantErr: "chart download of version '0.1.0' failed", + }, + { + name: "chart load error", + repositories: map[string]*ChartRepository{ + "https://example.com/": { + Client: &mockGetter{}, + Index: &repo.IndexFile{ + Entries: map[string]repo.ChartVersions{ + chartName: { + &repo.ChartVersion{ + Metadata: &helmchart.Metadata{ + Name: chartName, + Version: chartVersion, + }, + URLs: []string{"https://example.com/foo.tgz"}, + }, + }, + }, + }, + RWMutex: &sync.RWMutex{}, + }, + }, + dep: &helmchart.Dependency{ + Name: chartName, + Version: chartVersion, + Repository: "https://example.com", + }, + wantErr: "failed to load downloaded archive of version '0.1.0'", }, } - for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - c := chartFixture - dm := DependencyManager{ - WorkingDir: "./", - ChartPath: "testdata/charts/helmchart", - Chart: &c, - Dependencies: []*DependencyWithRepository{ - { - Dependency: &tt.dep, - Repository: nil, - }, - }, - } + g := NewWithT(t) - err := dm.Build(context.TODO()) - deps := dm.Chart.Dependencies() - - if (err != nil) && tt.wantErr { - if !strings.Contains(err.Error(), tt.errMsg) { - t.Errorf("Build() expected to return error: %s, got: %s", tt.errMsg, err) - } - if len(deps) > 0 { - t.Fatalf("chart expected to have no dependencies registered") - } - return - } else if err != nil { - t.Errorf("Build() not expected to return an error: %s", err) + dm := &DependencyManager{ + chart: &helmchart.Chart{}, + repositories: tt.repositories, + } + err := dm.addRemoteDependency(tt.dep) + if tt.wantErr != "" { + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(ContainSubstring(tt.wantErr)) return } + g.Expect(err).ToNot(HaveOccurred()) + if tt.wantFunc != nil { + tt.wantFunc(g, dm.chart) + } + }) + } +} + +func TestDependencyManager_resolveRepository(t *testing.T) { + tests := []struct { + name string + repositories map[string]*ChartRepository + getChartRepositoryCallback GetChartRepositoryCallback + url string + want *ChartRepository + wantRepositories map[string]*ChartRepository + wantErr string + }{ + { + name: "resolves from repositories index", + url: "https://example.com", + repositories: map[string]*ChartRepository{ + "https://example.com/": {URL: "https://example.com"}, + }, + want: &ChartRepository{URL: "https://example.com"}, + }, + { + name: "resolves from callback", + url: "https://example.com", + getChartRepositoryCallback: func(url string) (*ChartRepository, error) { + return &ChartRepository{URL: "https://example.com"}, nil + }, + want: &ChartRepository{URL: "https://example.com"}, + wantRepositories: map[string]*ChartRepository{ + "https://example.com/": {URL: "https://example.com"}, + }, + }, + { + name: "error from callback", + url: "https://example.com", + getChartRepositoryCallback: func(url string) (*ChartRepository, error) { + return nil, errors.New("a very unique error") + }, + wantErr: "a very unique error", + wantRepositories: map[string]*ChartRepository{}, + }, + { + name: "error on not found", + url: "https://example.com", + wantErr: "no chart repository for URL", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) - if len(deps) == 0 { - t.Fatalf("chart expected to have at least one dependency registered") + dm := &DependencyManager{ + repositories: tt.repositories, + getChartRepositoryCallback: tt.getChartRepositoryCallback, } - if deps[0].Metadata.Name != chartName { - t.Errorf("chart dependency has incorrect name, expected: %s, got: %s", chartName, deps[0].Metadata.Name) + + got, err := dm.resolveRepository(tt.url) + if tt.wantErr != "" { + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(ContainSubstring(tt.wantErr)) + g.Expect(got).To(BeNil()) + return } - if deps[0].Metadata.Version != chartVersion { - t.Errorf("chart dependency has incorrect version, expected: %s, got: %s", chartVersion, deps[0].Metadata.Version) + + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(got).To(Equal(tt.want)) + if tt.wantRepositories != nil { + g.Expect(dm.repositories).To(Equal(tt.wantRepositories)) } }) } } -func TestBuild_WithRemoteChart(t *testing.T) { - chart := chartFixture - b, err := os.ReadFile(helmPackageFile) - if err != nil { - t.Fatal(err) - } - i := repo.NewIndexFile() - i.MustAdd(&helmchart.Metadata{Name: chartName, Version: chartVersion}, fmt.Sprintf("%s-%s.tgz", chartName, chartVersion), "http://example.com/charts", "sha256:1234567890") - mg := mockGetter{response: b} - cr := newChartRepository() - cr.URL = remoteDepFixture.Repository - cr.Index = i - cr.Client = &mg - dm := DependencyManager{ - Chart: &chart, - Dependencies: []*DependencyWithRepository{ - { - Dependency: &remoteDepFixture, - Repository: cr, +func TestDependencyManager_secureLocalChartPath(t *testing.T) { + tests := []struct { + name string + baseDir string + path string + dep *helmchart.Dependency + want string + wantErr string + }{ + { + name: "secure local file path", + baseDir: "/tmp/workdir", + path: "/chart", + dep: &helmchart.Dependency{ + Repository: "../dep", + }, + want: "/tmp/workdir/dep", + }, + { + name: "insecure local file path", + baseDir: "/tmp/workdir", + path: "/", + dep: &helmchart.Dependency{ + Repository: "/../../dep", }, + want: "/tmp/workdir/dep", + }, + { + name: "URL parse error", + dep: &helmchart.Dependency{ + Repository: ": //example.com", + }, + wantErr: "missing protocol scheme", + }, + { + name: "error on URL scheme other than file", + dep: &helmchart.Dependency{ + Repository: "https://example.com", + }, + wantErr: "not a local chart reference", }, } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) - if err := dm.Build(context.TODO()); err != nil { - t.Errorf("Build() expected to not return error: %s", err) + dm := &DependencyManager{ + baseDir: tt.baseDir, + path: tt.path, + } + got, err := dm.secureLocalChartPath(tt.dep) + if tt.wantErr != "" { + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(ContainSubstring(tt.wantErr)) + return + } + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(got).ToNot(BeEmpty()) + g.Expect(got).To(Equal(tt.want)) + }) } +} - deps := dm.Chart.Dependencies() - if len(deps) != 1 { - t.Fatalf("chart expected to have one dependency registered") - } - if deps[0].Metadata.Name != chartName { - t.Errorf("chart dependency has incorrect name, expected: %s, got: %s", chartName, deps[0].Metadata.Name) +func Test_collectMissing(t *testing.T) { + tests := []struct { + name string + current []*helmchart.Chart + reqs []*helmchart.Dependency + want map[string]*helmchart.Dependency + }{ + { + name: "one missing", + current: []*helmchart.Chart{}, + reqs: []*helmchart.Dependency{ + {Name: chartName}, + }, + want: map[string]*helmchart.Dependency{ + chartName: {Name: chartName}, + }, + }, + { + name: "alias missing", + current: []*helmchart.Chart{ + { + Metadata: &helmchart.Metadata{ + Name: chartName, + }, + }, + }, + reqs: []*helmchart.Dependency{ + {Name: chartName}, + {Name: chartName, Alias: chartName + "-alias"}, + }, + want: map[string]*helmchart.Dependency{ + chartName + "-alias": {Name: chartName, Alias: chartName + "-alias"}, + }, + }, + { + name: "all current", + current: []*helmchart.Chart{ + { + Metadata: &helmchart.Metadata{ + Name: chartName, + }, + }, + }, + reqs: []*helmchart.Dependency{ + {Name: chartName}, + }, + want: nil, + }, + { + name: "nil", + current: nil, + reqs: nil, + want: nil, + }, } - if deps[0].Metadata.Version != chartVersion { - t.Errorf("chart dependency has incorrect version, expected: %s, got: %s", chartVersion, deps[0].Metadata.Version) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + g.Expect(collectMissing(tt.current, tt.reqs)).To(Equal(tt.want)) + }) + }) } +} - // When repo is not set - dm.Dependencies[0].Repository = nil - if err := dm.Build(context.TODO()); err == nil { - t.Errorf("Build() expected to return error") - } else if !strings.Contains(err.Error(), "is not a local chart reference") { - t.Errorf("Build() expected to return different error, got: %s", err) +func Test_isLocalDep(t *testing.T) { + tests := []struct { + name string + dep *helmchart.Dependency + want bool + }{ + { + name: "file protocol", + dep: &helmchart.Dependency{Repository: "file:///some/path"}, + want: true, + }, + { + name: "empty", + dep: &helmchart.Dependency{Repository: ""}, + want: true, + }, + { + name: "https url", + dep: &helmchart.Dependency{Repository: "https://example.com"}, + want: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + g.Expect(isLocalDep(tt.dep)).To(Equal(tt.want)) + }) } } diff --git a/internal/helm/repository.go b/internal/helm/repository.go index c57df111f..e2446f944 100644 --- a/internal/helm/repository.go +++ b/internal/helm/repository.go @@ -54,6 +54,9 @@ type ChartRepository struct { Options []getter.Option // CachePath is the path of a cached index.yaml for read-only operations. CachePath string + // Cached indicates if the ChartRepository index.yaml has been cached + // to CachePath. + Cached bool // Index contains a loaded chart repository index if not nil. Index *repo.IndexFile // Checksum contains the SHA256 checksum of the loaded chart repository @@ -68,7 +71,6 @@ type ChartRepository struct { // repository URL scheme. It returns an error on URL parsing failures, // or if there is no getter available for the scheme. func NewChartRepository(repositoryURL, cachePath string, providers getter.Providers, opts []getter.Option) (*ChartRepository, error) { - r := newChartRepository() u, err := url.Parse(repositoryURL) if err != nil { return nil, err @@ -77,6 +79,8 @@ func NewChartRepository(repositoryURL, cachePath string, providers getter.Provid if err != nil { return nil, err } + + r := newChartRepository() r.URL = repositoryURL r.CachePath = cachePath r.Client = c @@ -238,7 +242,7 @@ func (r *ChartRepository) LoadFromFile(path string) error { } // CacheIndex attempts to write the index from the remote into a new temporary file -// using DownloadIndex, and sets CachePath. +// using DownloadIndex, and sets CachePath and Cached. // It returns the SHA256 checksum of the downloaded index bytes, or an error. // The caller is expected to handle the garbage collection of CachePath, and to // load the Index separately using LoadFromCache if required. @@ -262,19 +266,40 @@ func (r *ChartRepository) CacheIndex() (string, error) { r.Lock() r.CachePath = f.Name() + r.Cached = true r.Unlock() return hex.EncodeToString(h.Sum(nil)), nil } +// StrategicallyLoadIndex lazy-loads the Index from CachePath using +// LoadFromCache if it does not HasIndex. +// If it not HasCacheFile, a cache attempt is made using CacheIndex +// before continuing to load. +// It returns a boolean indicating if it cached the index before +// loading, or an error. +func (r *ChartRepository) StrategicallyLoadIndex() (err error) { + if r.HasIndex() { + return + } + if !r.HasCacheFile() { + if _, err = r.CacheIndex(); err != nil { + err = fmt.Errorf("failed to strategically load index: %w", err) + return + } + } + if err = r.LoadFromCache(); err != nil { + err = fmt.Errorf("failed to strategically load index: %w", err) + return + } + return +} + // LoadFromCache attempts to load the Index from the configured CachePath. // It returns an error if no CachePath is set, or if the load failed. func (r *ChartRepository) LoadFromCache() error { - r.RLock() if cachePath := r.CachePath; cachePath != "" { - r.RUnlock() return r.LoadFromFile(cachePath) } - r.RUnlock() return fmt.Errorf("no cache path set") } @@ -314,11 +339,34 @@ func (r *ChartRepository) HasCacheFile() bool { return r.CachePath != "" } -// UnloadIndex sets the Index to nil. -func (r *ChartRepository) UnloadIndex() { - if r != nil { - r.Lock() - r.Index = nil - r.Unlock() +// Unload can be used to signal the Go garbage collector the Index can +// be freed from memory if the ChartRepository object is expected to +// continue to exist in the stack for some time. +func (r *ChartRepository) Unload() { + if r == nil { + return } + + r.Lock() + defer r.Unlock() + r.Index = nil +} + +// RemoveCache removes the CachePath if Cached. +func (r *ChartRepository) RemoveCache() error { + if r == nil { + return nil + } + + r.Lock() + defer r.Unlock() + + if r.Cached { + if err := os.Remove(r.CachePath); err != nil && !os.IsNotExist(err) { + return err + } + r.CachePath = "" + r.Cached = false + } + return nil } diff --git a/internal/helm/repository_test.go b/internal/helm/repository_test.go index 95ccc7b80..0d2077dfd 100644 --- a/internal/helm/repository_test.go +++ b/internal/helm/repository_test.go @@ -47,7 +47,8 @@ type mockGetter struct { func (g *mockGetter) Get(url string, _ ...getter.Option) (*bytes.Buffer, error) { g.requestedURL = url - return bytes.NewBuffer(g.response), nil + r := g.response + return bytes.NewBuffer(r), nil } func TestNewChartRepository(t *testing.T) { @@ -402,7 +403,7 @@ func TestChartRepository_CacheIndex(t *testing.T) { g.Expect(sum).To(BeEquivalentTo(expectSum)) } -func TestChartRepository_LoadIndexFromCache(t *testing.T) { +func TestChartRepository_LoadFromCache(t *testing.T) { tests := []struct { name string cachePath string @@ -458,7 +459,7 @@ func TestChartRepository_UnloadIndex(t *testing.T) { r := newChartRepository() g.Expect(r.HasIndex()).To(BeFalse()) r.Index = repo.NewIndexFile() - r.UnloadIndex() + r.Unload() g.Expect(r.Index).To(BeNil()) } diff --git a/internal/helm/testdata/charts/helmchart/values-prod.yaml b/internal/helm/testdata/charts/helmchart/values-prod.yaml new file mode 100644 index 000000000..5ef7832ca --- /dev/null +++ b/internal/helm/testdata/charts/helmchart/values-prod.yaml @@ -0,0 +1 @@ +replicaCount: 2 diff --git a/internal/helm/testdata/charts/helmchartwithdeps/Chart.lock b/internal/helm/testdata/charts/helmchartwithdeps/Chart.lock new file mode 100644 index 000000000..83401ac65 --- /dev/null +++ b/internal/helm/testdata/charts/helmchartwithdeps/Chart.lock @@ -0,0 +1,12 @@ +dependencies: +- name: helmchart + repository: file://../helmchart + version: 0.1.0 +- name: helmchart + repository: file://../helmchart + version: 0.1.0 +- name: grafana + repository: https://grafana.github.io/helm-charts + version: 6.17.4 +digest: sha256:1e41c97e27347f433ff0212bf52c344bc82dd435f70129d15e96cd2c8fcc32bb +generated: "2021-11-02T01:25:59.624290788+01:00"