diff --git a/cmd/helm/flags.go b/cmd/helm/flags.go index 0cc0564e2..9cf7d04e6 100644 --- a/cmd/helm/flags.go +++ b/cmd/helm/flags.go @@ -63,6 +63,11 @@ func addChartPathOptionsFlags(f *pflag.FlagSet, c *action.ChartPathOptions) { f.BoolVar(&c.PassCredentialsAll, "pass-credentials", false, "pass credentials to all domains") } +// addExternalPathsFlags adds flags to the given command +func addExternalPathsFlags(f *pflag.FlagSet, v *[]string) { + f.StringArrayVar(v, "include-path", []string{}, "paths to local directories to add during chart installation") +} + // bindOutputFlag will add the output flag to the given command and bind the // value to the given format pointer func bindOutputFlag(cmd *cobra.Command, varRef *output.Format) { diff --git a/cmd/helm/helm.go b/cmd/helm/helm.go index 15b0d5c76..a98af62e4 100644 --- a/cmd/helm/helm.go +++ b/cmd/helm/helm.go @@ -80,6 +80,11 @@ func main() { } }) + if err := actionConfig.Init(settings.RESTClientGetter(), settings.Namespace(), os.Getenv("HELM_DRIVER"), debug); err != nil { + debug("%+v", err) + os.Exit(1) + } + if err := cmd.Execute(); err != nil { debug("%+v", err) switch e := err.(type) { diff --git a/cmd/helm/install.go b/cmd/helm/install.go index 0e63ab3a5..fe7e3ba6d 100644 --- a/cmd/helm/install.go +++ b/cmd/helm/install.go @@ -157,6 +157,7 @@ func addInstallFlags(cmd *cobra.Command, f *pflag.FlagSet, client *action.Instal f.BoolVar(&client.SubNotes, "render-subchart-notes", false, "if set, render subchart notes along with the parent") addValueOptionsFlags(f, valueOpts) addChartPathOptionsFlags(f, &client.ChartPathOptions) + addExternalPathsFlags(f, &client.ExternalPaths) err := cmd.RegisterFlagCompletionFunc("version", func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { requiredArgs := 2 diff --git a/cmd/helm/install_test.go b/cmd/helm/install_test.go index 46e8eedce..b71d62cd5 100644 --- a/cmd/helm/install_test.go +++ b/cmd/helm/install_test.go @@ -77,6 +77,18 @@ func TestInstall(t *testing.T) { cmd: "install virgil testdata/testcharts/alpine -f testdata/testcharts/alpine/extra_values.yaml", golden: "output/install-with-values-file.txt", }, + // Install, external dir + { + name: "install with external dir", + cmd: "install virgil testdata/testcharts/external --set glob.enabled=true --include-path testdata/files/", + golden: "output/install-with-external-files.txt", + }, + // Install, external dir with alias + { + name: "chart with template with external dir and alias", + cmd: "install virgil testdata/testcharts/externalAlias --set glob.enabled=true --include-path vol1@testdata/files/", + golden: "output/install-with-external-files.txt", + }, // Install, no hooks { name: "install without hooks", diff --git a/cmd/helm/template_test.go b/cmd/helm/template_test.go index d1f17fe98..4861b1775 100644 --- a/cmd/helm/template_test.go +++ b/cmd/helm/template_test.go @@ -131,6 +131,16 @@ func TestTemplateCmd(t *testing.T) { cmd: fmt.Sprintf(`template '%s' --skip-tests`, chartPath), golden: "output/template-skip-tests.txt", }, + { + name: "chart with template with external dir", + cmd: fmt.Sprintf("template '%s' --set glob.enabled=true --include-path testdata/files/", "testdata/testcharts/external"), + golden: "output/template-with-external-dir.txt", + }, + { + name: "chart with template with external dir and alias", + cmd: fmt.Sprintf("template '%s' --set glob.enabled=true --include-path vol1@testdata/files/", "testdata/testcharts/externalAlias"), + golden: "output/template-with-external-dir-and-alias.txt", + }, } runTestCmd(t, tests) } diff --git a/cmd/helm/testdata/files/external.1.conf b/cmd/helm/testdata/files/external.1.conf new file mode 100644 index 000000000..0f8c1e827 --- /dev/null +++ b/cmd/helm/testdata/files/external.1.conf @@ -0,0 +1 @@ +external-1 diff --git a/cmd/helm/testdata/files/external.2.conf b/cmd/helm/testdata/files/external.2.conf new file mode 100644 index 000000000..9f0613e8a --- /dev/null +++ b/cmd/helm/testdata/files/external.2.conf @@ -0,0 +1 @@ +external-2 diff --git a/cmd/helm/testdata/files/external.txt b/cmd/helm/testdata/files/external.txt new file mode 100644 index 000000000..068c4db58 --- /dev/null +++ b/cmd/helm/testdata/files/external.txt @@ -0,0 +1 @@ +out-of-chart-dir diff --git a/cmd/helm/testdata/output/install-with-external-files.txt b/cmd/helm/testdata/output/install-with-external-files.txt new file mode 100644 index 000000000..406e522a9 --- /dev/null +++ b/cmd/helm/testdata/output/install-with-external-files.txt @@ -0,0 +1,6 @@ +NAME: virgil +LAST DEPLOYED: Fri Sep 2 22:04:05 1977 +NAMESPACE: default +STATUS: deployed +REVISION: 1 +TEST SUITE: None diff --git a/cmd/helm/testdata/output/template-with-external-dir-and-alias.txt b/cmd/helm/testdata/output/template-with-external-dir-and-alias.txt new file mode 100644 index 000000000..981f6119a --- /dev/null +++ b/cmd/helm/testdata/output/template-with-external-dir-and-alias.txt @@ -0,0 +1,25 @@ +--- +# Source: configmap/templates/config-map.yaml +apiVersion: v1 +kind: ConfigMap +metadata: + name: "release-name-" + labels: + # The "app.kubernetes.io/managed-by" label is used to track which tool + # deployed a given chart. It is useful for admins who want to see what + # releases a particular tool is responsible for. + app.kubernetes.io/managed-by: "Helm" + # The "app.kubernetes.io/instance" convention makes it easy to tie a release + # to all of the Kubernetes resources that were created as part of that + # release. + app.kubernetes.io/instance: "release-name" + app.kubernetes.io/version: 1.0 + # This makes it easy to audit chart usage. + helm.sh/chart: "configmap-0.1.0" +data: + external.1.conf: | + external-1 + external.2.conf: | + external-2 + external.txt: | + out-of-chart-dir diff --git a/cmd/helm/testdata/output/template-with-external-dir.txt b/cmd/helm/testdata/output/template-with-external-dir.txt new file mode 100644 index 000000000..981f6119a --- /dev/null +++ b/cmd/helm/testdata/output/template-with-external-dir.txt @@ -0,0 +1,25 @@ +--- +# Source: configmap/templates/config-map.yaml +apiVersion: v1 +kind: ConfigMap +metadata: + name: "release-name-" + labels: + # The "app.kubernetes.io/managed-by" label is used to track which tool + # deployed a given chart. It is useful for admins who want to see what + # releases a particular tool is responsible for. + app.kubernetes.io/managed-by: "Helm" + # The "app.kubernetes.io/instance" convention makes it easy to tie a release + # to all of the Kubernetes resources that were created as part of that + # release. + app.kubernetes.io/instance: "release-name" + app.kubernetes.io/version: 1.0 + # This makes it easy to audit chart usage. + helm.sh/chart: "configmap-0.1.0" +data: + external.1.conf: | + external-1 + external.2.conf: | + external-2 + external.txt: | + out-of-chart-dir diff --git a/cmd/helm/testdata/output/template-with-external-file.txt b/cmd/helm/testdata/output/template-with-external-file.txt new file mode 100644 index 000000000..fcbdad5fe --- /dev/null +++ b/cmd/helm/testdata/output/template-with-external-file.txt @@ -0,0 +1,21 @@ +--- +# Source: configmap/templates/config-map.yaml +apiVersion: v1 +kind: ConfigMap +metadata: + name: "RELEASE-NAME-" + labels: + # The "app.kubernetes.io/managed-by" label is used to track which tool + # deployed a given chart. It is useful for admins who want to see what + # releases a particular tool is responsible for. + app.kubernetes.io/managed-by: "Helm" + # The "app.kubernetes.io/instance" convention makes it easy to tie a release + # to all of the Kubernetes resources that were created as part of that + # release. + app.kubernetes.io/instance: "RELEASE-NAME" + app.kubernetes.io/version: 1.0 + # This makes it easy to audit chart usage. + helm.sh/chart: "configmap-0.1.0" +data: + external.txt: | + out-of-chart-dir diff --git a/cmd/helm/testdata/testcharts/external/Chart.yaml b/cmd/helm/testdata/testcharts/external/Chart.yaml new file mode 100644 index 000000000..0b070f43e --- /dev/null +++ b/cmd/helm/testdata/testcharts/external/Chart.yaml @@ -0,0 +1,8 @@ +apiVersion: v1 +appVersion: "1.0" +description: Deploy a basic Config Map from an external file +home: https://helm.sh/helm +name: configmap +sources: +- https://github.com/helm/helm +version: 0.1.0 diff --git a/cmd/helm/testdata/testcharts/external/external.txt b/cmd/helm/testdata/testcharts/external/external.txt new file mode 100644 index 000000000..d8e8e4de4 --- /dev/null +++ b/cmd/helm/testdata/testcharts/external/external.txt @@ -0,0 +1 @@ +in-chart diff --git a/cmd/helm/testdata/testcharts/external/templates/config-map.yaml b/cmd/helm/testdata/testcharts/external/templates/config-map.yaml new file mode 100644 index 000000000..f644c307e --- /dev/null +++ b/cmd/helm/testdata/testcharts/external/templates/config-map.yaml @@ -0,0 +1,23 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: "{{.Release.Name}}-{{.Values.Name}}" + labels: + # The "app.kubernetes.io/managed-by" label is used to track which tool + # deployed a given chart. It is useful for admins who want to see what + # releases a particular tool is responsible for. + app.kubernetes.io/managed-by: {{.Release.Service | quote }} + # The "app.kubernetes.io/instance" convention makes it easy to tie a release + # to all of the Kubernetes resources that were created as part of that + # release. + app.kubernetes.io/instance: {{.Release.Name | quote }} + app.kubernetes.io/version: {{ .Chart.AppVersion }} + # This makes it easy to audit chart usage. + helm.sh/chart: "{{.Chart.Name}}-{{.Chart.Version}}" +data: +{{- if .Values.external }} +{{ (.Files.Glob .Values.external).AsConfig | indent 2 }} +{{- end }} +{{- if .Values.glob.enabled }} +{{ (.Files.Glob .Values.glob.path).AsConfig | indent 2 }} +{{- end }} diff --git a/cmd/helm/testdata/testcharts/external/values.yaml b/cmd/helm/testdata/testcharts/external/values.yaml new file mode 100644 index 000000000..a8f39b2ad --- /dev/null +++ b/cmd/helm/testdata/testcharts/external/values.yaml @@ -0,0 +1,4 @@ +external: false +glob: + enabled: false + path: "*" diff --git a/cmd/helm/testdata/testcharts/externalAlias/Chart.yaml b/cmd/helm/testdata/testcharts/externalAlias/Chart.yaml new file mode 100644 index 000000000..0b070f43e --- /dev/null +++ b/cmd/helm/testdata/testcharts/externalAlias/Chart.yaml @@ -0,0 +1,8 @@ +apiVersion: v1 +appVersion: "1.0" +description: Deploy a basic Config Map from an external file +home: https://helm.sh/helm +name: configmap +sources: +- https://github.com/helm/helm +version: 0.1.0 diff --git a/cmd/helm/testdata/testcharts/externalAlias/external.txt b/cmd/helm/testdata/testcharts/externalAlias/external.txt new file mode 100644 index 000000000..d8e8e4de4 --- /dev/null +++ b/cmd/helm/testdata/testcharts/externalAlias/external.txt @@ -0,0 +1 @@ +in-chart diff --git a/cmd/helm/testdata/testcharts/externalAlias/templates/config-map.yaml b/cmd/helm/testdata/testcharts/externalAlias/templates/config-map.yaml new file mode 100644 index 000000000..f644c307e --- /dev/null +++ b/cmd/helm/testdata/testcharts/externalAlias/templates/config-map.yaml @@ -0,0 +1,23 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: "{{.Release.Name}}-{{.Values.Name}}" + labels: + # The "app.kubernetes.io/managed-by" label is used to track which tool + # deployed a given chart. It is useful for admins who want to see what + # releases a particular tool is responsible for. + app.kubernetes.io/managed-by: {{.Release.Service | quote }} + # The "app.kubernetes.io/instance" convention makes it easy to tie a release + # to all of the Kubernetes resources that were created as part of that + # release. + app.kubernetes.io/instance: {{.Release.Name | quote }} + app.kubernetes.io/version: {{ .Chart.AppVersion }} + # This makes it easy to audit chart usage. + helm.sh/chart: "{{.Chart.Name}}-{{.Chart.Version}}" +data: +{{- if .Values.external }} +{{ (.Files.Glob .Values.external).AsConfig | indent 2 }} +{{- end }} +{{- if .Values.glob.enabled }} +{{ (.Files.Glob .Values.glob.path).AsConfig | indent 2 }} +{{- end }} diff --git a/cmd/helm/testdata/testcharts/externalAlias/values.yaml b/cmd/helm/testdata/testcharts/externalAlias/values.yaml new file mode 100644 index 000000000..a0a34c7e6 --- /dev/null +++ b/cmd/helm/testdata/testcharts/externalAlias/values.yaml @@ -0,0 +1,4 @@ +external: false +glob: + enabled: false + path: "vol1/*" diff --git a/cmd/helm/upgrade.go b/cmd/helm/upgrade.go index 7ada8e3b1..fdaf2f985 100644 --- a/cmd/helm/upgrade.go +++ b/cmd/helm/upgrade.go @@ -115,6 +115,7 @@ func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { instClient.PostRenderer = client.PostRenderer instClient.DisableOpenAPIValidation = client.DisableOpenAPIValidation instClient.SubNotes = client.SubNotes + instClient.ExternalPaths = client.ExternalPaths instClient.Description = client.Description rel, err := runInstall(args, instClient, valueOpts, out) @@ -233,6 +234,7 @@ func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { addValueOptionsFlags(f, valueOpts) bindOutputFlag(cmd, &outfmt) bindPostRenderFlag(cmd, &client.PostRenderer) + addExternalPathsFlags(f, &client.ExternalPaths) err := cmd.RegisterFlagCompletionFunc("version", func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { if len(args) != 2 { diff --git a/cmd/helm/upgrade_test.go b/cmd/helm/upgrade_test.go index 8afcb139b..29a397e2f 100644 --- a/cmd/helm/upgrade_test.go +++ b/cmd/helm/upgrade_test.go @@ -357,6 +357,38 @@ func TestUpgradeInstallWithValuesFromStdin(t *testing.T) { } +func TestUpgradeWithExternalFile(t *testing.T) { + + releaseName := "funny-bunny-v7" + + exFiles := []*chart.File{ + {Name: "testdata/files/external.txt", Data: []byte("from-external-file")}, + } + + relMock, ch, chartPath := prepareMockReleaseWithExternal(releaseName, exFiles, t) + + defer resetEnv()() + + store := storageFixture() + + store.Create(relMock(releaseName, 3, ch)) + + cmd := fmt.Sprintf("upgrade %s --set glob.enabled=false --set external=testdata/files/external.txt '%s'", releaseName, chartPath) + _, _, err := executeActionCommandC(store, cmd) + if err != nil { + t.Errorf("unexpected error, got '%v'", err) + } + + updatedRel, err := store.Get(releaseName, 4) + if err != nil { + t.Errorf("unexpected error, got '%v'", err) + } + + if !strings.Contains(updatedRel.Manifest, "from-external-file") { + t.Errorf("The value is not set correctly. manifest: %s", updatedRel.Manifest) + } +} + func prepareMockRelease(releaseName string, t *testing.T) (func(n string, v int, ch *chart.Chart) *release.Release, *chart.Chart, string) { tmpChart := ensure.TempDir(t) configmapData, err := ioutil.ReadFile("testdata/testcharts/upgradetest/templates/configmap.yaml") @@ -392,6 +424,43 @@ func prepareMockRelease(releaseName string, t *testing.T) (func(n string, v int, return relMock, ch, chartPath } +func prepareMockReleaseWithExternal(releaseName string, exFiles []*chart.File, t *testing.T) (func(n string, v int, ch *chart.Chart) *release.Release, *chart.Chart, string) { + tmpChart := ensure.TempDir(t) + configmapData, err := ioutil.ReadFile("testdata/testcharts/external/templates/config-map.yaml") + + if err != nil { + t.Fatalf("Error loading template yaml %v", err) + } + cfile := &chart.Chart{ + Metadata: &chart.Metadata{ + APIVersion: chart.APIVersionV1, + Name: "testUpgradeChart", + Description: "A Helm chart for Kubernetes", + Version: "0.1.0", + }, + Templates: []*chart.File{{Name: "templates/configmap.yaml", Data: configmapData}}, + Files: exFiles, + } + chartPath := filepath.Join(tmpChart, cfile.Metadata.Name) + if err := chartutil.SaveDir(cfile, tmpChart); err != nil { + t.Fatalf("Error creating chart for upgrade: %v", err) + } + ch, err := loader.Load(chartPath) + if err != nil { + t.Fatalf("Error loading chart: %v", err) + } + _ = release.Mock(&release.MockReleaseOptions{ + Name: releaseName, + Chart: ch, + }) + + relMock := func(n string, v int, ch *chart.Chart) *release.Release { + return release.Mock(&release.MockReleaseOptions{Name: n, Version: v, Chart: ch}) + } + + return relMock, ch, chartPath +} + func TestUpgradeOutputCompletion(t *testing.T) { outputFlagCompletionTest(t, "upgrade") } diff --git a/internal/test/test.go b/internal/test/test.go index 646037606..e4f7f639a 100644 --- a/internal/test/test.go +++ b/internal/test/test.go @@ -88,7 +88,7 @@ func compare(actual []byte, filename string) error { } expected = normalize(expected) if !bytes.Equal(expected, actual) { - return errors.Errorf("does not match golden file %s\n\nWANT:\n'%s'\n\nGOT:\n'%s'\n", filename, expected, actual) + return errors.Errorf("does not match golden file %s\n\nWANT:\n'%s'\n\nGOT:\n'%s'", filename, expected, actual) } return nil } diff --git a/pkg/action/action_test.go b/pkg/action/action_test.go index 190d00cae..94ce3343d 100644 --- a/pkg/action/action_test.go +++ b/pkg/action/action_test.go @@ -17,6 +17,7 @@ package action import ( "flag" + "fmt" "io/ioutil" "os" "testing" @@ -217,6 +218,15 @@ func withSampleIncludingIncorrectTemplates() chartOption { } } +func withExternalFileTemplate(externalPath string) chartOption { + return func(opts *chartOptions) { + externalFilesTemplates := []*chart.File{ + {Name: "templates/with-external-paths", Data: []byte(fmt.Sprintf(`data: {{ .Files.Get "%s" }}`, externalPath))}, + } + opts.Templates = append(opts.Templates, externalFilesTemplates...) + } +} + func withMultipleManifestTemplate() chartOption { return func(opts *chartOptions) { sampleTemplates := []*chart.File{ diff --git a/pkg/action/install.go b/pkg/action/install.go index 3872ed5c9..96c44e4e1 100644 --- a/pkg/action/install.go +++ b/pkg/action/install.go @@ -25,11 +25,14 @@ import ( "os" "path" "path/filepath" + "strconv" "strings" "sync" "text/template" "time" + "helm.sh/helm/v3/pkg/chart/loader" + "github.com/Masterminds/sprig/v3" "github.com/pkg/errors" v1 "k8s.io/api/core/v1" @@ -102,6 +105,8 @@ type Install struct { PostRenderer postrender.PostRenderer // Lock to control raceconditions when the process receives a SIGTERM Lock sync.Mutex + // ExternalPaths is list of included files in configuration + ExternalPaths []string } // ChartPathOptions captures common options used for controlling chart paths @@ -176,10 +181,76 @@ func (i *Install) installCRDs(crds []chart.CRD) error { return nil } +// loadExternalPaths takes external paths from a given configuration and puts the files in a chart. +// The alias can be defined, but if not, the alias will be the ordinal number of the path in the list starting from 0. +// Example: install test ./bin/test --include-path vol_name1@/path/to/dir1 --include-path /path/to/dir2 +// The first included file has the alias vol_name1, and it can be used from the config map: {{ (.Files.Glob "vol1/**").AsConfig | nindent 2 }} +// The Second included file hasn't an alias but is generated with the number 1, and it can be used from the config map {{ (.Files.Glob "1/**").AsConfig | nindent 2 }} +func loadExternalPaths(ch *chart.Chart, externalPaths []string) error { + var errs []string + includeDefaultAlias := false + if len(externalPaths) > 1 { + includeDefaultAlias = true + } + for i, p := range externalPaths { + var alias string + if strings.Contains(p, "@") { + aliasPath := strings.Split(p, "@") + alias = aliasPath[0] + p = aliasPath[1] + } + allPaths, err := loader.ExpandFilePath(p) + if err != nil { + errs = append(errs, fmt.Sprintf("%s (path not accessible)", p)) + } + + for _, currentPath := range allPaths { + fileContentBytes, err := ioutil.ReadFile(currentPath) + if err != nil { + errs = append(errs, fmt.Sprintf("%s (not readable)", currentPath)) + continue + } + + p = strings.ReplaceAll(p, "\\", "/") + if !strings.HasSuffix(p, "/") { + p = p + "/" + } + if currentPath == p { + errs = append(errs, fmt.Sprintf("%s (accepts only directory, not file)", currentPath)) + } + if includeDefaultAlias { + if alias != "" { + currentPath = strings.Replace(currentPath, p, alias+"/", 1) + } else { + currentPath = strings.Replace(currentPath, p, strconv.Itoa(i)+"/", 1) + } + } else { + if alias != "" { + currentPath = strings.Replace(currentPath, p, alias+"/", 1) + } else { + currentPath = strings.Replace(currentPath, p, "", 1) + } + } + + newFile := chart.File{Name: currentPath, Data: fileContentBytes} + for _, file := range ch.Files { + if file.Name == newFile.Name && bytes.Equal(file.Data, newFile.Data) { + continue + } + } + ch.Files = append(ch.Files, &newFile) + } + } + + if len(errs) > 0 { + return errors.New(fmt.Sprint("Failed to load external paths: ", strings.Join(errs, "; "))) + } + return nil +} + // Run executes the installation // // If DryRun is set to true, this will prepare the release, but not install it - func (i *Install) Run(chrt *chart.Chart, vals map[string]interface{}) (*release.Release, error) { ctx := context.Background() return i.RunWithContext(ctx, chrt, vals) @@ -187,6 +258,9 @@ func (i *Install) Run(chrt *chart.Chart, vals map[string]interface{}) (*release. // Run executes the installation with Context func (i *Install) RunWithContext(ctx context.Context, chrt *chart.Chart, vals map[string]interface{}) (*release.Release, error) { + if err := loadExternalPaths(chrt, i.ExternalPaths); err != nil { + return nil, err + } // Check reachability of cluster unless in client-only mode (e.g. `helm template` without `--validate`) if !i.ClientOnly { if err := i.cfg.KubeClient.IsReachable(); err != nil { diff --git a/pkg/action/install_test.go b/pkg/action/install_test.go index 39520d620..7fe1adfeb 100644 --- a/pkg/action/install_test.go +++ b/pkg/action/install_test.go @@ -40,12 +40,21 @@ import ( helmtime "helm.sh/helm/v3/pkg/time" ) +const ExternalFileRelPath = "testdata/files" +const ExternalFileName = "external.txt" + type nameTemplateTestCase struct { tpl string expected string expectedErrorStr string } +type includeExternalPathTestCase struct { + Name string + IncludedFilePath string + ExternalPath string +} + func installAction(t *testing.T) *Install { config := actionConfigFixture(t) instAction := NewInstall(config) @@ -726,3 +735,51 @@ func TestNameAndChartGenerateName(t *testing.T) { }) } } + +func TestInstallFailsWhenWrongPathsIncluded(t *testing.T) { + is := assert.New(t) + vals := map[string]interface{}{} + + tests := []includeExternalPathTestCase{ + { + Name: "included paths not passed", + IncludedFilePath: "", + ExternalPath: ExternalFileRelPath, + }, + } + + for _, tc := range tests { + t.Run(tc.Name, func(t *testing.T) { + instAction := installAction(t) + instAction.ExternalPaths = append(instAction.ExternalPaths, tc.IncludedFilePath) + + _, err := instAction.Run(buildChart(withExternalFileTemplate(tc.ExternalPath)), vals) + expectedErr := fmt.Sprintf("<.Files.Get>: error calling Get: file %s not included", tc.ExternalPath) + is.Error(err, expectedErr) + }) + } +} + +func TestInstallWhenIncludePathsPassed(t *testing.T) { + is := assert.New(t) + vals := map[string]interface{}{} + + tests := []includeExternalPathTestCase{ + { + Name: "relative path is included and external file is relative", + IncludedFilePath: ExternalFileRelPath, + ExternalPath: ExternalFileName, + }, + } + + for _, tc := range tests { + t.Run(tc.Name, func(t *testing.T) { + instAction := installAction(t) + instAction.ExternalPaths = append(instAction.ExternalPaths, tc.IncludedFilePath) + + installRelease, err := instAction.Run(buildChart(withExternalFileTemplate(tc.ExternalPath)), vals) + is.Contains(installRelease.Manifest, "out-of-chart-dir") + is.NoError(err) + }) + } +} diff --git a/pkg/action/rollback_test.go b/pkg/action/rollback_test.go new file mode 100644 index 000000000..ee22c5153 --- /dev/null +++ b/pkg/action/rollback_test.go @@ -0,0 +1,63 @@ +/* +Copyright The Helm 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 action + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func rollbackAction(t *testing.T) *Rollback { + config := actionConfigFixture(t) + rollbackAction := NewRollback(config) + + return rollbackAction +} + +func TestRollbackToReleaseWithExternalFile(t *testing.T) { + is := assert.New(t) + vals := map[string]interface{}{} + + chartVersion1 := buildChart(withExternalFileTemplate(ExternalFileName)) + chartVersion2 := buildChart() + + instAction := installAction(t) + instAction.ExternalPaths = append(instAction.ExternalPaths, ExternalFileRelPath) + relVersion1, err := instAction.Run(chartVersion1, vals) + is.Contains(relVersion1.Manifest, "out-of-chart-dir") + is.NoError(err) + + upAction := upgradeAction(t) + err = upAction.cfg.Releases.Create(relVersion1) + is.NoError(err) + relVersion2, err := upAction.Run(relVersion1.Name, chartVersion2, vals) + is.NotContains(relVersion2.Manifest, "out-out-chart-dir") + is.NoError(err) + + rollAction := rollbackAction(t) + err = rollAction.cfg.Releases.Create(relVersion1) + is.NoError(err) + err = rollAction.cfg.Releases.Create(relVersion2) + is.NoError(err) + currentRelease, targetRelease, err := rollAction.prepareRollback(relVersion2.Name) + is.NoError(err) + relVersion3, err := rollAction.performRollback(currentRelease, targetRelease) + is.NoError(err) + + is.Contains(relVersion3.Manifest, "out-of-chart-dir") +} diff --git a/pkg/action/testdata/files/external.txt b/pkg/action/testdata/files/external.txt new file mode 100644 index 000000000..068c4db58 --- /dev/null +++ b/pkg/action/testdata/files/external.txt @@ -0,0 +1 @@ +out-of-chart-dir diff --git a/pkg/action/upgrade.go b/pkg/action/upgrade.go index c98b524bc..7b8c4d473 100644 --- a/pkg/action/upgrade.go +++ b/pkg/action/upgrade.go @@ -101,8 +101,10 @@ type Upgrade struct { DisableOpenAPIValidation bool // Get missing dependencies DependencyUpdate bool - // Lock to control raceconditions when the process receives a SIGTERM + // Lock to control race conditions when the process receives a SIGTERM Lock sync.Mutex + // ExternalPaths is list of included files in configuration + ExternalPaths []string } type resultMessage struct { @@ -179,6 +181,10 @@ func (u *Upgrade) prepareUpgrade(name string, chart *chart.Chart, vals map[strin return nil, nil, err } + if err := loadExternalPaths(chart, u.ExternalPaths); err != nil { + return nil, nil, err + } + // Concurrent `helm upgrade`s will either fail here with `errPending` or when creating the release with "already exists". This should act as a pessimistic lock. if lastRelease.Info.Status.IsPending() { return nil, nil, errPending diff --git a/pkg/action/upgrade_test.go b/pkg/action/upgrade_test.go index 62922b373..684932c9c 100644 --- a/pkg/action/upgrade_test.go +++ b/pkg/action/upgrade_test.go @@ -326,6 +326,87 @@ func TestUpgradeRelease_Pending(t *testing.T) { req.Contains(err.Error(), "progress", err) } +func TestUpgradeFailsWhenEmptyPathIncluded(t *testing.T) { + is := assert.New(t) + vals := map[string]interface{}{} + + t.Run("included paths not passed", func(t *testing.T) { + upAction := upgradeAction(t) + upAction.ExternalPaths = append(upAction.ExternalPaths, "") + + rel := releaseStub() + rel.Name = "test" + rel.Info.Status = release.StatusDeployed + upAction.cfg.Releases.Create(rel) + + _, err := upAction.Run(rel.Name, buildChart(withExternalFileTemplate("external.txt")), vals) + expectedErr := "Failed to load external paths: (path not accessible)" + is.Error(err, expectedErr) + }) +} + +func TestUpgradeFailsWhenWrongPathsIncluded(t *testing.T) { + is := assert.New(t) + vals := map[string]interface{}{} + + tests := []includeExternalPathTestCase{ + { + Name: "included paths not passed", + IncludedFilePath: "testdata/files2", + ExternalPath: "external.txt", + }, + { + Name: "included paths not passed", + IncludedFilePath: "testdata2/files", + ExternalPath: "external.txt", + }, + } + + for _, tc := range tests { + t.Run(tc.Name, func(t *testing.T) { + upAction := upgradeAction(t) + upAction.ExternalPaths = append(upAction.ExternalPaths, tc.IncludedFilePath) + + rel := releaseStub() + rel.Name = "test" + rel.Info.Status = release.StatusDeployed + upAction.cfg.Releases.Create(rel) + + _, err := upAction.Run(rel.Name, buildChart(withExternalFileTemplate(tc.ExternalPath)), vals) + expectedErr := fmt.Sprintf("Failed to load external paths:%s(path not accessible)", tc.IncludedFilePath) + is.Error(err, expectedErr) + }) + } +} + +func TestUpgradeWhenIncludePathsPassed(t *testing.T) { + is := assert.New(t) + vals := map[string]interface{}{} + + tests := []includeExternalPathTestCase{ + { + Name: "relative path of directory is included and external file is relative", + IncludedFilePath: "testdata/files", + ExternalPath: "external.txt", + }, + } + + for _, tc := range tests { + t.Run(tc.Name, func(t *testing.T) { + upAction := upgradeAction(t) + upAction.ExternalPaths = append(upAction.ExternalPaths, tc.IncludedFilePath) + + rel := releaseStub() + rel.Name = "test" + rel.Info.Status = release.StatusDeployed + upAction.cfg.Releases.Create(rel) + + upgradeRelease, err := upAction.Run(rel.Name, buildChart(withExternalFileTemplate(tc.ExternalPath)), vals) + is.Contains(upgradeRelease.Manifest, "out-of-chart-dir") + is.NoError(err) + }) + } +} func TestUpgradeRelease_Interrupted_Wait(t *testing.T) { is := assert.New(t) diff --git a/pkg/chart/loader/local.go b/pkg/chart/loader/local.go new file mode 100644 index 000000000..b053a15c0 --- /dev/null +++ b/pkg/chart/loader/local.go @@ -0,0 +1,38 @@ +/* +Copyright The Helm 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 loader + +import ( + "io/fs" + "os" + "path/filepath" +) + +// ExpandFilePath expands a local file, dir or glob path to a list of files +func ExpandFilePath(path string) ([]string, error) { + _, err := os.Stat(path) + if err != nil { + return nil, err + } + + var files []string + filepath.WalkDir(path, func(path string, d fs.DirEntry, err error) error { + if !d.IsDir() { + files = append(files, filepath.ToSlash(path)) + } + return nil + }) + + return files, nil +} diff --git a/pkg/chart/loader/local_test.go b/pkg/chart/loader/local_test.go new file mode 100644 index 000000000..b2f3480ec --- /dev/null +++ b/pkg/chart/loader/local_test.go @@ -0,0 +1,33 @@ +/* +Copyright The Helm 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 loader + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestExpandFilePath(t *testing.T) { + req := require.New(t) + + dir, err := ExpandFilePath("testdata/albatross/") + req.NoError(err) + req.Contains(dir, "testdata/albatross/Chart.yaml") + req.Contains(dir, "testdata/albatross/values.yaml") + + file, err := ExpandFilePath("testdata/albatross/Chart.yaml") + req.NoError(err) + req.Contains(file, "testdata/albatross/Chart.yaml") +} diff --git a/pkg/engine/files.go b/pkg/engine/files.go index d7e62da5a..a13fb8c37 100644 --- a/pkg/engine/files.go +++ b/pkg/engine/files.go @@ -18,6 +18,7 @@ package engine import ( "encoding/base64" + "fmt" "path" "strings" @@ -50,6 +51,7 @@ func (f files) GetBytes(name string) []byte { if v, ok := f[name]; ok { return v } + fmt.Printf("file %s not included", name) return []byte{} } @@ -60,7 +62,8 @@ func (f files) GetBytes(name string) []byte { // // {{.Files.Get "foo"}} func (f files) Get(name string) string { - return string(f.GetBytes(name)) + content := f.GetBytes(name) + return string(content) } // Glob takes a glob pattern and returns another files object only containing @@ -102,7 +105,8 @@ func (f files) Glob(pattern string) files { // data: // {{ .Files.Glob("config/**").AsConfig() | indent 4 }} func (f files) AsConfig() string { - if f == nil { + if f == nil || len(f) == 0 { + fmt.Println("must pass path") return "" } @@ -131,7 +135,8 @@ func (f files) AsConfig() string { // data: // {{ .Files.Glob("secrets/*").AsSecrets() }} func (f files) AsSecrets() string { - if f == nil { + if f == nil || len(f) == 0 { + fmt.Println("must pass files") return "" } @@ -153,7 +158,8 @@ func (f files) AsSecrets() string { // {{ . }}{{ end }} func (f files) Lines(path string) []string { if f == nil || f[path] == nil { - return []string{} + fmt.Println("must pass files") + return nil } return strings.Split(string(f[path]), "\n") diff --git a/pkg/engine/files_test.go b/pkg/engine/files_test.go index 4b37724f9..9f2b73392 100644 --- a/pkg/engine/files_test.go +++ b/pkg/engine/files_test.go @@ -21,6 +21,8 @@ import ( "github.com/stretchr/testify/assert" ) +const NonExistingFileName = "no_such_file.txt" + var cases = []struct { path, data string }{ @@ -49,12 +51,22 @@ func TestNewFiles(t *testing.T) { if got := string(files.GetBytes(f.path)); got != f.data { t.Errorf("%d: expected %q, got %q", i, f.data, got) } + if got := files.Get(f.path); got != f.data { t.Errorf("%d: expected %q, got %q", i, f.data, got) } } } +func TestGetNonExistingFile(t *testing.T) { + as := assert.New(t) + + f := getTestFiles() + + content := f.Get(NonExistingFileName) + as.Empty(content) +} + func TestFileGlob(t *testing.T) { as := assert.New(t) @@ -63,6 +75,7 @@ func TestFileGlob(t *testing.T) { matched := f.Glob("story/**") as.Len(matched, 2, "Should be two files in glob story/**") + as.Equal("Joseph Conrad", matched.Get("story/author.txt")) } @@ -75,6 +88,9 @@ func TestToConfig(t *testing.T) { out = f.Glob("ship/**").AsConfig() as.Equal("captain.txt: The Captain\nstowaway.txt: Legatt", out) + + out = f.Glob(NonExistingFileName).AsConfig() + as.Empty(out) } func TestToSecret(t *testing.T) { @@ -84,6 +100,9 @@ func TestToSecret(t *testing.T) { out := f.Glob("ship/**").AsSecrets() as.Equal("captain.txt: VGhlIENhcHRhaW4=\nstowaway.txt: TGVnYXR0", out) + + out = f.Glob(NonExistingFileName).AsSecrets() + as.Empty(out) } func TestLines(t *testing.T) { @@ -93,6 +112,8 @@ func TestLines(t *testing.T) { out := f.Lines("multiline/test.txt") as.Len(out, 2) - as.Equal("bar", out[0]) + + out = f.Lines(NonExistingFileName) + as.Nil(out) }