diff --git a/cmd/helm/flags.go b/cmd/helm/flags.go index 76d6e0476..378730f32 100644 --- a/cmd/helm/flags.go +++ b/cmd/helm/flags.go @@ -64,6 +64,10 @@ func addChartPathOptionsFlags(f *pflag.FlagSet, c *action.ChartPathOptions) { f.BoolVar(&c.PassCredentialsAll, "pass-credentials", false, "pass credentials to all domains") } +func addExternalPathsFlags(f *pflag.FlagSet, v *[]string) { + f.StringArrayVar(v, "include-path", []string{}, "paths or globs to local files and 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/install.go b/cmd/helm/install.go index 13c674066..5f4c81cfe 100644 --- a/cmd/helm/install.go +++ b/cmd/helm/install.go @@ -179,6 +179,7 @@ func addInstallFlags(cmd *cobra.Command, f *pflag.FlagSet, client *action.Instal f.BoolVar(&client.EnableDNS, "enable-dns", false, "enable DNS lookups when rendering templates") 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 b34d1455c..54079d17d 100644 --- a/cmd/helm/install_test.go +++ b/cmd/helm/install_test.go @@ -77,6 +77,24 @@ 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 files + { + name: "install with external files", + cmd: "install virgil testdata/testcharts/external --include-path testdata/files/external.txt --set external=testdata/files/external.txt", + golden: "output/install-with-external-files.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 glob files + { + name: "install with external globbed files", + cmd: "install virgil testdata/testcharts/external --set glob.enabled=true --include-path testdata/files/external.*.conf", + 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..25bf05c4c 100644 --- a/cmd/helm/template_test.go +++ b/cmd/helm/template_test.go @@ -131,6 +131,21 @@ 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 file", + cmd: fmt.Sprintf("template '%s' --set external=testdata/files/external.txt --include-path testdata/files/external.txt", "testdata/testcharts/external"), + golden: "output/template-with-external-file.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 globbed files", + cmd: fmt.Sprintf("template '%s' --set glob.enabled=true --include-path testdata/files/external.*.conf", "testdata/testcharts/external"), + golden: "output/template-with-external-glob.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..065cf1bd9 --- /dev/null +++ b/cmd/helm/testdata/files/external.1.conf @@ -0,0 +1 @@ +glob-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..92c587e84 --- /dev/null +++ b/cmd/helm/testdata/files/external.2.conf @@ -0,0 +1 @@ +glob-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.txt b/cmd/helm/testdata/output/template-with-external-dir.txt new file mode 100644 index 000000000..469ff133f --- /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: | + glob-external-1 + external.2.conf: | + glob-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..047b30e2d --- /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/output/template-with-external-glob.txt b/cmd/helm/testdata/output/template-with-external-glob.txt new file mode 100644 index 000000000..026cd0f7e --- /dev/null +++ b/cmd/helm/testdata/output/template-with-external-glob.txt @@ -0,0 +1,23 @@ +--- +# 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: | + glob-external-1 + external.2.conf: | + glob-external-2 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..422adf0ef --- /dev/null +++ b/cmd/helm/testdata/testcharts/external/values.yaml @@ -0,0 +1,4 @@ +external: false +glob: + enabled: false + path: "testdata/files/*" diff --git a/cmd/helm/upgrade.go b/cmd/helm/upgrade.go index 145d342b7..c1bc48b92 100644 --- a/cmd/helm/upgrade.go +++ b/cmd/helm/upgrade.go @@ -123,6 +123,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 instClient.DependencyUpdate = client.DependencyUpdate instClient.EnableDNS = client.EnableDNS @@ -244,6 +245,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/pkg/action/action_test.go b/pkg/action/action_test.go index c816c84af..f0c8a0c51 100644 --- a/pkg/action/action_test.go +++ b/pkg/action/action_test.go @@ -17,6 +17,7 @@ package action import ( "flag" + "fmt" "io/ioutil" "testing" @@ -32,6 +33,8 @@ import ( "helm.sh/helm/v3/pkg/time" ) +const withExternalPathsTemplatePath = "templates/with-external-paths" + var verbose = flag.Bool("test.log", false, "enable test logging") func actionConfigFixture(t *testing.T) *Configuration { @@ -209,6 +212,15 @@ func withSampleIncludingIncorrectTemplates() chartOption { } } +func withExternalFileTemplate(externalPath string) chartOption { + return func(opts *chartOptions) { + externalFilesTemplates := []*chart.File{ + {Name: withExternalPathsTemplatePath, 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 881b5a68b..9f3d434b7 100644 --- a/pkg/action/install.go +++ b/pkg/action/install.go @@ -104,6 +104,7 @@ type Install struct { // OutputDir/ UseReleaseName bool PostRenderer postrender.PostRenderer + ExternalPaths []string // Lock to control raceconditions when the process receives a SIGTERM Lock sync.Mutex } @@ -201,6 +202,38 @@ func (i *Install) installCRDs(crds []chart.CRD) error { return nil } +func loadExternalPaths(ch *chart.Chart, externalPaths []string) error { + var errs []string + + for _, p := range externalPaths { + allPaths, err := expandFilePath(p) + if err != nil { + errs = append(errs, fmt.Sprintf("%s (path not accessible)", p)) + } + + for _, currentPath := range allPaths { + fileContentBytes, err := os.ReadFile(currentPath) + if err != nil { + errs = append(errs, fmt.Sprintf("%s (not readable)", currentPath)) + continue + } + + 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 @@ -212,6 +245,10 @@ 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 45e5a2670..f5e842076 100644 --- a/pkg/action/install_test.go +++ b/pkg/action/install_test.go @@ -17,9 +17,11 @@ limitations under the License. package action import ( + "bytes" "context" "fmt" "io/ioutil" + "log" "os" "path/filepath" "regexp" @@ -39,12 +41,20 @@ import ( helmtime "helm.sh/helm/v3/pkg/time" ) +const ExternalFileRelPath = "testdata/files/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) @@ -54,6 +64,11 @@ func installAction(t *testing.T) *Install { return instAction } +func getAbsPath(path string) string { + absPath, _ := filepath.Abs(path) + return absPath +} + func TestInstallRelease(t *testing.T) { is := assert.New(t) req := require.New(t) @@ -717,3 +732,99 @@ func TestNameAndChartGenerateName(t *testing.T) { }) } } + +func TestInstallUsesEmptyContentWrongPathsIncluded(t *testing.T) { + is := assert.New(t) + vals := map[string]interface{}{} + + tests := []includeExternalPathTestCase{ + { + Name: "included paths not passed", + IncludedFilePath: "", + ExternalPath: ExternalFileRelPath, + }, + { + Name: "absolute path of file is included and external file is relative", + IncludedFilePath: getAbsPath(ExternalFileRelPath), + ExternalPath: ExternalFileRelPath, + }, + { + Name: "relative path of file is included and external file is absolute", + IncludedFilePath: ExternalFileRelPath, + ExternalPath: getAbsPath(ExternalFileRelPath), + }, + { + Name: "absolute path of directory is included and external file is relative", + IncludedFilePath: getAbsPath("testdata/files"), + ExternalPath: ExternalFileRelPath, + }, + { + Name: "relative path of directory is included and external file is absolute", + IncludedFilePath: "testdata/files", + ExternalPath: getAbsPath(ExternalFileRelPath), + }, + } + + var buf bytes.Buffer + log.SetOutput(&buf) + defer func() { + log.SetOutput(os.Stderr) + }() + + for _, tc := range tests { + t.Run(tc.Name, func(t *testing.T) { + instAction := installAction(t) + if tc.IncludedFilePath != "" { + instAction.ExternalPaths = append(instAction.ExternalPaths, tc.IncludedFilePath) + } + + rel, err := instAction.Run(buildChart(withExternalFileTemplate(tc.ExternalPath)), vals) + is.Contains(buf.String(), "not included") + is.NoError(err) + is.Equal( + rel.Manifest, + "---\n# Source: hello/templates/hello\nhello: world\n---\n# Source: hello/templates/with-external-paths\ndata:\n", + ) + buf.Reset() + }) + } +} + +func TestInstallWhenIncludePathsPassed(t *testing.T) { + is := assert.New(t) + vals := map[string]interface{}{} + + tests := []includeExternalPathTestCase{ + { + Name: "relative path of file is included and external file is relative", + IncludedFilePath: ExternalFileRelPath, + ExternalPath: ExternalFileRelPath, + }, + { + Name: "absolute path of file is included and external file is absolute", + IncludedFilePath: getAbsPath(ExternalFileRelPath), + ExternalPath: getAbsPath(ExternalFileRelPath), + }, + { + Name: "relative path of directory is included and external file is relative", + IncludedFilePath: "testdata/files", + ExternalPath: ExternalFileRelPath, + }, + { + Name: "absolute path of directory is included and external file is absolute", + IncludedFilePath: getAbsPath("testdata/files"), + ExternalPath: getAbsPath(ExternalFileRelPath), + }, + } + + 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/local.go b/pkg/action/local.go new file mode 100644 index 000000000..21d6c6ca4 --- /dev/null +++ b/pkg/action/local.go @@ -0,0 +1,75 @@ +/* +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 ( + "errors" + "os" + "path/filepath" + "strings" +) + +// expandFilePath expands a local file, dir or glob path to a list of files +func expandFilePath(path string) ([]string, error) { + if strings.Contains(path, "*") { + // if this is a glob, we expand it and return a list of files + return expandGlob(path) + } + + fileInfo, err := os.Stat(path) + if err != nil { + return nil, err + } + + if fileInfo.IsDir() { + // if this is a valid dir, we return all files within + return expandDir(path) + } + + // finally, this is a file, so we return it + return []string{path}, nil +} + +func expandGlob(path string) ([]string, error) { + paths, err := filepath.Glob(path) + if err != nil { + return nil, err + } + if len(paths) == 0 { + return nil, errors.New("empty glob") + } + + return paths, err +} + +func expandDir(path string) ([]string, error) { + f, err := os.Open(path) + + if err != nil { + return nil, err + } + defer f.Close() + + filesInfos, err := f.Readdir(-1) + if err != nil { + return nil, err + } + + var filesPaths []string + localDirName := strings.TrimRight(path, "/") + "/" + for _, file := range filesInfos { + filesPaths = append(filesPaths, localDirName+file.Name()) + } + return filesPaths, nil +} diff --git a/pkg/action/local_test.go b/pkg/action/local_test.go new file mode 100644 index 000000000..29ac7c107 --- /dev/null +++ b/pkg/action/local_test.go @@ -0,0 +1,36 @@ +/* +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/require" +) + +func TestExpandLocalPath(t *testing.T) { + req := require.New(t) + + glob, err := expandFilePath("testdata/output/*.txt") + req.NoError(err) + req.Contains(glob, "testdata/output/list-compressed-deps.txt") + req.Contains(glob, "testdata/output/list-missing-deps.txt") + + dir, err := expandFilePath("testdata/files/") + req.NoError(err) + req.Contains(dir, "testdata/files/external.txt") + + _, err = expandFilePath("testdata/non_existing") + req.Error(err) +} diff --git a/pkg/action/rollback_test.go b/pkg/action/rollback_test.go new file mode 100644 index 000000000..1fa705718 --- /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(ExternalFileRelPath)) + 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 829be51df..2b0277740 100644 --- a/pkg/action/upgrade.go +++ b/pkg/action/upgrade.go @@ -102,6 +102,7 @@ type Upgrade struct { DisableOpenAPIValidation bool // Get missing dependencies DependencyUpdate bool + ExternalPaths []string // Lock to control raceconditions when the process receives a SIGTERM Lock sync.Mutex // Enable DNS lookups when rendering templates @@ -187,6 +188,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..6ef058555 100644 --- a/pkg/action/upgrade_test.go +++ b/pkg/action/upgrade_test.go @@ -17,8 +17,11 @@ limitations under the License. package action import ( + "bytes" "context" "fmt" + "log" + "os" "testing" "time" @@ -388,3 +391,109 @@ func TestUpgradeRelease_Interrupted_Atomic(t *testing.T) { is.Equal(updatedRes.Info.Status, release.StatusDeployed) } + +func TestUpgradeFailsWhenWrongPathsIncluded(t *testing.T) { + is := assert.New(t) + vals := map[string]interface{}{} + + tests := []includeExternalPathTestCase{ + { + Name: "included paths not passed", + IncludedFilePath: "", + ExternalPath: "testdata/files/external.txt", + }, + { + Name: "absolute path of file is included and external file is relative", + IncludedFilePath: getAbsPath("testdata/files/external.txt"), + ExternalPath: "testdata/files/external.txt", + }, + { + Name: "relative path of file is included and external file is absolute", + IncludedFilePath: "testdata/files/external.txt", + ExternalPath: getAbsPath("testdata/files/external.txt"), + }, + { + Name: "absolute path of directory is included and external file is relative", + IncludedFilePath: getAbsPath("testdata/files"), + ExternalPath: "testdata/files/external.txt", + }, + { + Name: "relative path of directory is included and external file is absolute", + IncludedFilePath: "testdata/files", + ExternalPath: getAbsPath("testdata/files/external.txt"), + }, + } + + var buf bytes.Buffer + log.SetOutput(&buf) + defer func() { + log.SetOutput(os.Stderr) + }() + + for _, tc := range tests { + t.Run(tc.Name, func(t *testing.T) { + upAction := upgradeAction(t) + if tc.IncludedFilePath != "" { + upAction.ExternalPaths = append(upAction.ExternalPaths, tc.IncludedFilePath) + } + + rel := releaseStub() + rel.Name = "test" + rel.Info.Status = release.StatusDeployed + upAction.cfg.Releases.Create(rel) + + rel, err := upAction.Run(rel.Name, buildChart(withExternalFileTemplate(tc.ExternalPath)), vals) + is.Contains(buf.String(), "not included") + is.NoError(err) + is.Equal( + rel.Manifest, + "---\n# Source: hello/templates/hello\nhello: world\n---\n# Source: hello/templates/with-external-paths\ndata:\n", + ) + buf.Reset() + }) + } +} + +func TestUpgradeWhenIncludePathsPassed(t *testing.T) { + is := assert.New(t) + vals := map[string]interface{}{} + + tests := []includeExternalPathTestCase{ + { + Name: "relative path of file is included and external file is relative", + IncludedFilePath: "testdata/files/external.txt", + ExternalPath: "testdata/files/external.txt", + }, + { + Name: "relative path of file is included and external file is absolute", + IncludedFilePath: getAbsPath("testdata/files/external.txt"), + ExternalPath: getAbsPath("testdata/files/external.txt"), + }, + { + Name: "relative path of directory is included and external file is relative", + IncludedFilePath: "testdata/files", + ExternalPath: "testdata/files/external.txt", + }, + { + Name: "absolute path of directory is included and external file is absolute", + IncludedFilePath: getAbsPath("testdata/files"), + ExternalPath: getAbsPath("testdata/files/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) + }) + } +} diff --git a/pkg/engine/files.go b/pkg/engine/files.go index d7e62da5a..427664850 100644 --- a/pkg/engine/files.go +++ b/pkg/engine/files.go @@ -18,6 +18,7 @@ package engine import ( "encoding/base64" + "log" "path" "strings" @@ -50,7 +51,8 @@ func (f files) GetBytes(name string) []byte { if v, ok := f[name]; ok { return v } - return []byte{} + log.Printf("WARNING: %s not included", name) + return nil } // Get returns a string representation of the given file. @@ -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 len(f) == 0 { + log.Printf("must pass files") return "" } @@ -131,7 +135,8 @@ func (f files) AsConfig() string { // data: // {{ .Files.Glob("secrets/*").AsSecrets() }} func (f files) AsSecrets() string { - if f == nil { + if len(f) == 0 { + log.Printf("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{} + log.Printf("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..80a71478f 100644 --- a/pkg/engine/files_test.go +++ b/pkg/engine/files_test.go @@ -16,11 +16,16 @@ limitations under the License. package engine import ( + "bytes" + "log" + "os" "testing" "github.com/stretchr/testify/assert" ) +const NonExistingFileName = "no_such_file.txt" + var cases = []struct { path, data string }{ @@ -46,15 +51,36 @@ func TestNewFiles(t *testing.T) { } for i, f := range cases { - if got := string(files.GetBytes(f.path)); got != f.data { + gotBytes := files.GetBytes(f.path) + got := string(gotBytes) + if got != f.data { t.Errorf("%d: expected %q, got %q", i, f.data, got) } - if got := files.Get(f.path); got != f.data { + + gotBytes = files.GetBytes(f.path) + got = string(gotBytes) + if 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() + + var buf bytes.Buffer + log.SetOutput(&buf) + defer func() { + log.SetOutput(os.Stderr) + }() + + content := f.Get(NonExistingFileName) + as.Empty(content) + as.Contains(buf.String(), "not included") +} + func TestFileGlob(t *testing.T) { as := assert.New(t) @@ -63,18 +89,31 @@ 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")) + + content := matched.Get("story/author.txt") + as.Equal("Joseph Conrad", content) } func TestToConfig(t *testing.T) { as := assert.New(t) f := getTestFiles() + + var buf bytes.Buffer + log.SetOutput(&buf) + defer func() { + log.SetOutput(os.Stderr) + }() + out := f.Glob("**/captain.txt").AsConfig() as.Equal("captain.txt: The Captain", out) out = f.Glob("ship/**").AsConfig() as.Equal("captain.txt: The Captain\nstowaway.txt: Legatt", out) + + out = f.Glob(NonExistingFileName).AsConfig() + as.Empty(out) + as.Contains(buf.String(), "must pass files") } func TestToSecret(t *testing.T) { @@ -82,8 +121,18 @@ func TestToSecret(t *testing.T) { f := getTestFiles() + var buf bytes.Buffer + log.SetOutput(&buf) + defer func() { + log.SetOutput(os.Stderr) + }() + out := f.Glob("ship/**").AsSecrets() as.Equal("captain.txt: VGhlIENhcHRhaW4=\nstowaway.txt: TGVnYXR0", out) + + out = f.Glob(NonExistingFileName).AsSecrets() + as.Empty(out) + as.Contains(buf.String(), "must pass files") } func TestLines(t *testing.T) { @@ -91,8 +140,17 @@ func TestLines(t *testing.T) { f := getTestFiles() + var buf bytes.Buffer + log.SetOutput(&buf) + defer func() { + log.SetOutput(os.Stderr) + }() + out := f.Lines("multiline/test.txt") as.Len(out, 2) - as.Equal("bar", out[0]) + + out = f.Lines(NonExistingFileName) + as.Nil(out) + as.Contains(buf.String(), "must pass files") }