diff --git a/.circleci/config.yml b/.circleci/config.yml index ef19b8ee7..e6ce2e242 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -5,7 +5,7 @@ jobs: build: working_directory: ~/helm.sh/helm docker: - - image: circleci/golang:1.13 + - image: circleci/golang:1.14 environment: GOCACHE: "/tmp/go/cache" diff --git a/cmd/helm/env.go b/cmd/helm/env.go index efb6dfea9..0fbfb9da4 100644 --- a/cmd/helm/env.go +++ b/cmd/helm/env.go @@ -21,53 +21,35 @@ import ( "io" "sort" - "helm.sh/helm/v3/pkg/cli" - "github.com/spf13/cobra" "helm.sh/helm/v3/cmd/helm/require" ) -var ( - envHelp = ` +var envHelp = ` Env prints out all the environment information in use by Helm. ` -) func newEnvCmd(out io.Writer) *cobra.Command { - o := &envOptions{} - o.settings = cli.New() - cmd := &cobra.Command{ Use: "env", Short: "helm client environment information", Long: envHelp, Args: require.NoArgs, - RunE: func(cmd *cobra.Command, args []string) error { - return o.run(out) + Run: func(cmd *cobra.Command, args []string) { + envVars := settings.EnvVars() + + // Sort the variables by alphabetical order. + // This allows for a constant output across calls to 'helm env'. + var keys []string + for k := range envVars { + keys = append(keys, k) + } + sort.Strings(keys) + for _, k := range keys { + fmt.Fprintf(out, "%s=\"%s\"\n", k, envVars[k]) + } }, } - return cmd } - -type envOptions struct { - settings *cli.EnvSettings -} - -func (o *envOptions) run(out io.Writer) error { - envVars := o.settings.EnvVars() - - // Sort the variables by alphabetical order. - // This allows for a constant output across calls to 'helm env'. - var keys []string - for k := range envVars { - keys = append(keys, k) - } - sort.Strings(keys) - - for _, k := range keys { - fmt.Printf("%s=\"%s\"\n", k, envVars[k]) - } - return nil -} diff --git a/cmd/helm/helm.go b/cmd/helm/helm.go index 7e1fcb6e1..fcc7315f5 100644 --- a/cmd/helm/helm.go +++ b/cmd/helm/helm.go @@ -43,9 +43,7 @@ import ( // FeatureGateOCI is the feature gate for checking if `helm chart` and `helm registry` commands should work const FeatureGateOCI = gates.Gate("HELM_EXPERIMENTAL_OCI") -var ( - settings = cli.New() -) +var settings = cli.New() func init() { log.SetFlags(log.Lshortfile) @@ -72,13 +70,16 @@ func main() { actionConfig := new(action.Configuration) cmd := newRootCmd(actionConfig, os.Stdout, os.Args[1:]) - helmDriver := os.Getenv("HELM_DRIVER") - if err := actionConfig.Init(settings.RESTClientGetter(), settings.Namespace(), helmDriver, debug); err != nil { - log.Fatal(err) - } - if helmDriver == "memory" { - loadReleasesInMemory(actionConfig) - } + // run when each command's execute method is called + cobra.OnInitialize(func() { + helmDriver := os.Getenv("HELM_DRIVER") + if err := actionConfig.Init(settings.RESTClientGetter(), settings.Namespace(), helmDriver, debug); err != nil { + log.Fatal(err) + } + if helmDriver == "memory" { + loadReleasesInMemory(actionConfig) + } + }) if err := cmd.Execute(); err != nil { debug("%+v", err) diff --git a/cmd/helm/load_plugins.go b/cmd/helm/load_plugins.go index e56feab40..a23a067fb 100644 --- a/cmd/helm/load_plugins.go +++ b/cmd/helm/load_plugins.go @@ -58,7 +58,7 @@ func loadPlugins(baseCmd *cobra.Command, out io.Writer) { return } - found, err := findPlugins(settings.PluginsDirectory) + found, err := plugin.FindPlugins(settings.PluginsDirectory) if err != nil { fmt.Fprintf(os.Stderr, "failed to load plugins: %s", err) return @@ -238,20 +238,6 @@ func manuallyProcessArgs(args []string) ([]string, []string) { return known, unknown } -// findPlugins returns a list of YAML files that describe plugins. -func findPlugins(plugdirs string) ([]*plugin.Plugin, error) { - found := []*plugin.Plugin{} - // Let's get all UNIXy and allow path separators - for _, p := range filepath.SplitList(plugdirs) { - matches, err := plugin.LoadAll(p) - if err != nil { - return matches, err - } - found = append(found, matches...) - } - return found, nil -} - // pluginCommand represents the optional completion.yaml file of a plugin type pluginCommand struct { Name string `json:"name"` diff --git a/cmd/helm/plugin_list.go b/cmd/helm/plugin_list.go index 0440b0b5e..49a454963 100644 --- a/cmd/helm/plugin_list.go +++ b/cmd/helm/plugin_list.go @@ -22,6 +22,8 @@ import ( "github.com/gosuri/uitable" "github.com/spf13/cobra" + + "helm.sh/helm/v3/pkg/plugin" ) func newPluginListCmd(out io.Writer) *cobra.Command { @@ -31,7 +33,7 @@ func newPluginListCmd(out io.Writer) *cobra.Command { Short: "list installed Helm plugins", RunE: func(cmd *cobra.Command, args []string) error { debug("pluginDirs: %s", settings.PluginsDirectory) - plugins, err := findPlugins(settings.PluginsDirectory) + plugins, err := plugin.FindPlugins(settings.PluginsDirectory) if err != nil { return err } @@ -51,7 +53,7 @@ func newPluginListCmd(out io.Writer) *cobra.Command { // Provide dynamic auto-completion for plugin names func compListPlugins(toComplete string) []string { var pNames []string - plugins, err := findPlugins(settings.PluginsDirectory) + plugins, err := plugin.FindPlugins(settings.PluginsDirectory) if err == nil { for _, p := range plugins { if strings.HasPrefix(p.Metadata.Name, toComplete) { diff --git a/cmd/helm/plugin_uninstall.go b/cmd/helm/plugin_uninstall.go index f703ddcfb..66cdaccdc 100644 --- a/cmd/helm/plugin_uninstall.go +++ b/cmd/helm/plugin_uninstall.go @@ -68,7 +68,7 @@ func (o *pluginUninstallOptions) complete(args []string) error { func (o *pluginUninstallOptions) run(out io.Writer) error { debug("loading installed plugins from %s", settings.PluginsDirectory) - plugins, err := findPlugins(settings.PluginsDirectory) + plugins, err := plugin.FindPlugins(settings.PluginsDirectory) if err != nil { return err } diff --git a/cmd/helm/plugin_update.go b/cmd/helm/plugin_update.go index a24e80518..23f840b4a 100644 --- a/cmd/helm/plugin_update.go +++ b/cmd/helm/plugin_update.go @@ -70,7 +70,7 @@ func (o *pluginUpdateOptions) complete(args []string) error { func (o *pluginUpdateOptions) run(out io.Writer) error { installer.Debug = settings.Debug debug("loading installed plugins from %s", settings.PluginsDirectory) - plugins, err := findPlugins(settings.PluginsDirectory) + plugins, err := plugin.FindPlugins(settings.PluginsDirectory) if err != nil { return err } diff --git a/internal/fileutil/fileutil.go b/internal/fileutil/fileutil.go new file mode 100644 index 000000000..739093f3b --- /dev/null +++ b/internal/fileutil/fileutil.go @@ -0,0 +1,51 @@ +/* +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 fileutil + +import ( + "io" + "io/ioutil" + "os" + "path/filepath" + + "helm.sh/helm/v3/internal/third_party/dep/fs" +) + +// AtomicWriteFile atomically (as atomic as os.Rename allows) writes a file to a +// disk. +func AtomicWriteFile(filename string, reader io.Reader, mode os.FileMode) error { + tempFile, err := ioutil.TempFile(filepath.Split(filename)) + if err != nil { + return err + } + tempName := tempFile.Name() + + if _, err := io.Copy(tempFile, reader); err != nil { + tempFile.Close() // return value is ignored as we are already on error path + return err + } + + if err := tempFile.Close(); err != nil { + return err + } + + if err := os.Chmod(tempName, mode); err != nil { + return err + } + + return fs.RenameWithFallback(tempName, filename) +} diff --git a/internal/fileutil/fileutil_test.go b/internal/fileutil/fileutil_test.go new file mode 100644 index 000000000..9a4bc32c9 --- /dev/null +++ b/internal/fileutil/fileutil_test.go @@ -0,0 +1,62 @@ +/* +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 fileutil + +import ( + "bytes" + "io/ioutil" + "os" + "path/filepath" + "testing" +) + +func TestAtomicWriteFile(t *testing.T) { + dir, err := ioutil.TempDir("", "helm-tmp") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(dir) + + testpath := filepath.Join(dir, "test") + stringContent := "Test content" + reader := bytes.NewReader([]byte(stringContent)) + mode := os.FileMode(0644) + + err = AtomicWriteFile(testpath, reader, mode) + if err != nil { + t.Errorf("AtomicWriteFile error: %s", err) + } + + got, err := ioutil.ReadFile(testpath) + if err != nil { + t.Fatal(err) + } + + if stringContent != string(got) { + t.Fatalf("expected: %s, got: %s", stringContent, string(got)) + } + + gotinfo, err := os.Stat(testpath) + if err != nil { + t.Fatal(err) + } + + if mode != gotinfo.Mode() { + t.Fatalf("expected %s: to be the same mode as %s", + mode, gotinfo.Mode()) + } +} diff --git a/pkg/action/action.go b/pkg/action/action.go index 5da901635..a8437d729 100644 --- a/pkg/action/action.go +++ b/pkg/action/action.go @@ -17,10 +17,13 @@ limitations under the License. package action import ( + "bytes" "fmt" "os" "path" + "path/filepath" "regexp" + "strings" "github.com/pkg/errors" "k8s.io/apimachinery/pkg/api/meta" @@ -30,9 +33,13 @@ import ( "k8s.io/client-go/rest" "helm.sh/helm/v3/internal/experimental/registry" + "helm.sh/helm/v3/pkg/chart" "helm.sh/helm/v3/pkg/chartutil" + "helm.sh/helm/v3/pkg/engine" "helm.sh/helm/v3/pkg/kube" + "helm.sh/helm/v3/pkg/postrender" "helm.sh/helm/v3/pkg/release" + "helm.sh/helm/v3/pkg/releaseutil" "helm.sh/helm/v3/pkg/storage" "helm.sh/helm/v3/pkg/storage/driver" "helm.sh/helm/v3/pkg/time" @@ -86,6 +93,132 @@ type Configuration struct { Log func(string, ...interface{}) } +// renderResources renders the templates in a chart +// +// TODO: This function is badly in need of a refactor. +func (c *Configuration) renderResources(ch *chart.Chart, values chartutil.Values, releaseName, outputDir string, subNotes, useReleaseName, includeCrds bool, pr postrender.PostRenderer, dryRun bool) ([]*release.Hook, *bytes.Buffer, string, error) { + hs := []*release.Hook{} + b := bytes.NewBuffer(nil) + + caps, err := c.getCapabilities() + if err != nil { + return hs, b, "", err + } + + if ch.Metadata.KubeVersion != "" { + if !chartutil.IsCompatibleRange(ch.Metadata.KubeVersion, caps.KubeVersion.String()) { + return hs, b, "", errors.Errorf("chart requires kubeVersion: %s which is incompatible with Kubernetes %s", ch.Metadata.KubeVersion, caps.KubeVersion.String()) + } + } + + var files map[string]string + var err2 error + + // A `helm template` or `helm install --dry-run` should not talk to the remote cluster. + // It will break in interesting and exotic ways because other data (e.g. discovery) + // is mocked. It is not up to the template author to decide when the user wants to + // connect to the cluster. So when the user says to dry run, respect the user's + // wishes and do not connect to the cluster. + if !dryRun && c.RESTClientGetter != nil { + rest, err := c.RESTClientGetter.ToRESTConfig() + if err != nil { + return hs, b, "", err + } + files, err2 = engine.RenderWithClient(ch, values, rest) + } else { + files, err2 = engine.Render(ch, values) + } + + if err2 != nil { + return hs, b, "", err2 + } + + // NOTES.txt gets rendered like all the other files, but because it's not a hook nor a resource, + // pull it out of here into a separate file so that we can actually use the output of the rendered + // text file. We have to spin through this map because the file contains path information, so we + // look for terminating NOTES.txt. We also remove it from the files so that we don't have to skip + // it in the sortHooks. + var notesBuffer bytes.Buffer + for k, v := range files { + if strings.HasSuffix(k, notesFileSuffix) { + if subNotes || (k == path.Join(ch.Name(), "templates", notesFileSuffix)) { + // If buffer contains data, add newline before adding more + if notesBuffer.Len() > 0 { + notesBuffer.WriteString("\n") + } + notesBuffer.WriteString(v) + } + delete(files, k) + } + } + notes := notesBuffer.String() + + // Sort hooks, manifests, and partials. Only hooks and manifests are returned, + // as partials are not used after renderer.Render. Empty manifests are also + // removed here. + hs, manifests, err := releaseutil.SortManifests(files, caps.APIVersions, releaseutil.InstallOrder) + if err != nil { + // By catching parse errors here, we can prevent bogus releases from going + // to Kubernetes. + // + // We return the files as a big blob of data to help the user debug parser + // errors. + for name, content := range files { + if strings.TrimSpace(content) == "" { + continue + } + fmt.Fprintf(b, "---\n# Source: %s\n%s\n", name, content) + } + return hs, b, "", err + } + + // Aggregate all valid manifests into one big doc. + fileWritten := make(map[string]bool) + + if includeCrds { + for _, crd := range ch.CRDObjects() { + if outputDir == "" { + fmt.Fprintf(b, "---\n# Source: %s\n%s\n", crd.Name, string(crd.File.Data[:])) + } else { + err = writeToFile(outputDir, crd.Filename, string(crd.File.Data[:]), fileWritten[crd.Name]) + if err != nil { + return hs, b, "", err + } + fileWritten[crd.Name] = true + } + } + } + + for _, m := range manifests { + if outputDir == "" { + fmt.Fprintf(b, "---\n# Source: %s\n%s\n", m.Name, m.Content) + } else { + newDir := outputDir + if useReleaseName { + newDir = filepath.Join(outputDir, releaseName) + } + // NOTE: We do not have to worry about the post-renderer because + // output dir is only used by `helm template`. In the next major + // release, we should move this logic to template only as it is not + // used by install or upgrade + err = writeToFile(newDir, m.Name, m.Content, fileWritten[m.Name]) + if err != nil { + return hs, b, "", err + } + fileWritten[m.Name] = true + } + } + + if pr != nil { + b, err = pr.Run(b) + if err != nil { + return hs, b, notes, errors.Wrap(err, "error while running post render on files") + } + } + + return hs, b, notes, nil +} + // RESTClientGetter gets the rest client type RESTClientGetter interface { ToRESTConfig() (*rest.Config, error) diff --git a/pkg/action/install.go b/pkg/action/install.go index 81106d77f..4aec8fc2c 100644 --- a/pkg/action/install.go +++ b/pkg/action/install.go @@ -39,7 +39,6 @@ import ( "helm.sh/helm/v3/pkg/chartutil" "helm.sh/helm/v3/pkg/cli" "helm.sh/helm/v3/pkg/downloader" - "helm.sh/helm/v3/pkg/engine" "helm.sh/helm/v3/pkg/getter" "helm.sh/helm/v3/pkg/kube" kubefake "helm.sh/helm/v3/pkg/kube/fake" @@ -232,7 +231,7 @@ func (i *Install) Run(chrt *chart.Chart, vals map[string]interface{}) (*release. rel := i.createRelease(chrt, vals) var manifestDoc *bytes.Buffer - rel.Hooks, manifestDoc, rel.Info.Notes, err = i.cfg.renderResources(chrt, valuesToRender, i.ReleaseName, i.OutputDir, i.SubNotes, i.UseReleaseName, i.IncludeCRDs, i.PostRenderer) + rel.Hooks, manifestDoc, rel.Info.Notes, err = i.cfg.renderResources(chrt, valuesToRender, i.ReleaseName, i.OutputDir, i.SubNotes, i.UseReleaseName, i.IncludeCRDs, i.PostRenderer, i.DryRun) // Even for errors, attach this if available if manifestDoc != nil { rel.Manifest = manifestDoc.String() @@ -475,125 +474,6 @@ func (i *Install) replaceRelease(rel *release.Release) error { return i.recordRelease(last) } -// renderResources renders the templates in a chart -func (c *Configuration) renderResources(ch *chart.Chart, values chartutil.Values, releaseName, outputDir string, subNotes, useReleaseName, includeCrds bool, pr postrender.PostRenderer) ([]*release.Hook, *bytes.Buffer, string, error) { - hs := []*release.Hook{} - b := bytes.NewBuffer(nil) - - caps, err := c.getCapabilities() - if err != nil { - return hs, b, "", err - } - - if ch.Metadata.KubeVersion != "" { - if !chartutil.IsCompatibleRange(ch.Metadata.KubeVersion, caps.KubeVersion.String()) { - return hs, b, "", errors.Errorf("chart requires kubeVersion: %s which is incompatible with Kubernetes %s", ch.Metadata.KubeVersion, caps.KubeVersion.String()) - } - } - - var files map[string]string - var err2 error - - if c.RESTClientGetter != nil { - rest, err := c.RESTClientGetter.ToRESTConfig() - if err != nil { - return hs, b, "", err - } - files, err2 = engine.RenderWithClient(ch, values, rest) - } else { - files, err2 = engine.Render(ch, values) - } - - if err2 != nil { - return hs, b, "", err2 - } - - // NOTES.txt gets rendered like all the other files, but because it's not a hook nor a resource, - // pull it out of here into a separate file so that we can actually use the output of the rendered - // text file. We have to spin through this map because the file contains path information, so we - // look for terminating NOTES.txt. We also remove it from the files so that we don't have to skip - // it in the sortHooks. - var notesBuffer bytes.Buffer - for k, v := range files { - if strings.HasSuffix(k, notesFileSuffix) { - if subNotes || (k == path.Join(ch.Name(), "templates", notesFileSuffix)) { - // If buffer contains data, add newline before adding more - if notesBuffer.Len() > 0 { - notesBuffer.WriteString("\n") - } - notesBuffer.WriteString(v) - } - delete(files, k) - } - } - notes := notesBuffer.String() - - // Sort hooks, manifests, and partials. Only hooks and manifests are returned, - // as partials are not used after renderer.Render. Empty manifests are also - // removed here. - hs, manifests, err := releaseutil.SortManifests(files, caps.APIVersions, releaseutil.InstallOrder) - if err != nil { - // By catching parse errors here, we can prevent bogus releases from going - // to Kubernetes. - // - // We return the files as a big blob of data to help the user debug parser - // errors. - for name, content := range files { - if strings.TrimSpace(content) == "" { - continue - } - fmt.Fprintf(b, "---\n# Source: %s\n%s\n", name, content) - } - return hs, b, "", err - } - - // Aggregate all valid manifests into one big doc. - fileWritten := make(map[string]bool) - - if includeCrds { - for _, crd := range ch.CRDObjects() { - if outputDir == "" { - fmt.Fprintf(b, "---\n# Source: %s\n%s\n", crd.Name, string(crd.File.Data[:])) - } else { - err = writeToFile(outputDir, crd.Filename, string(crd.File.Data[:]), fileWritten[crd.Name]) - if err != nil { - return hs, b, "", err - } - fileWritten[crd.Name] = true - } - } - } - - for _, m := range manifests { - if outputDir == "" { - fmt.Fprintf(b, "---\n# Source: %s\n%s\n", m.Name, m.Content) - } else { - newDir := outputDir - if useReleaseName { - newDir = filepath.Join(outputDir, releaseName) - } - // NOTE: We do not have to worry about the post-renderer because - // output dir is only used by `helm template`. In the next major - // release, we should move this logic to template only as it is not - // used by install or upgrade - err = writeToFile(newDir, m.Name, m.Content, fileWritten[m.Name]) - if err != nil { - return hs, b, "", err - } - fileWritten[m.Name] = true - } - } - - if pr != nil { - b, err = pr.Run(b) - if err != nil { - return hs, b, notes, errors.Wrap(err, "error while running post render on files") - } - } - - return hs, b, notes, nil -} - // write the to /. controls if the file is created or content will be appended func writeToFile(outputDir string, name string, data string, append bool) error { outfileName := strings.Join([]string{outputDir, name}, string(filepath.Separator)) diff --git a/pkg/action/install_test.go b/pkg/action/install_test.go index 7cde9363a..46351cb2d 100644 --- a/pkg/action/install_test.go +++ b/pkg/action/install_test.go @@ -29,6 +29,7 @@ import ( "github.com/stretchr/testify/assert" "helm.sh/helm/v3/internal/test" + "helm.sh/helm/v3/pkg/chart" "helm.sh/helm/v3/pkg/chartutil" "helm.sh/helm/v3/pkg/cli" kubefake "helm.sh/helm/v3/pkg/kube/fake" @@ -242,6 +243,27 @@ func TestInstallRelease_DryRun(t *testing.T) { is.Equal(res.Info.Description, "Dry run complete") } +// Regression test for #7955: Lookup must not connect to Kubernetes on a dry-run. +func TestInstallRelease_DryRun_Lookup(t *testing.T) { + is := assert.New(t) + instAction := installAction(t) + instAction.DryRun = true + vals := map[string]interface{}{} + + mockChart := buildChart(withSampleTemplates()) + mockChart.Templates = append(mockChart.Templates, &chart.File{ + Name: "templates/lookup", + Data: []byte(`goodbye: {{ lookup "v1" "Namespace" "" "___" }}`), + }) + + res, err := instAction.Run(mockChart, vals) + if err != nil { + t.Fatalf("Failed install: %s", err) + } + + is.Contains(res.Manifest, "goodbye: map[]") +} + func TestInstallReleaseIncorrectTemplate_DryRun(t *testing.T) { is := assert.New(t) instAction := installAction(t) diff --git a/pkg/action/upgrade.go b/pkg/action/upgrade.go index 67872aa2f..fc289dbab 100644 --- a/pkg/action/upgrade.go +++ b/pkg/action/upgrade.go @@ -217,7 +217,7 @@ func (u *Upgrade) prepareUpgrade(name string, chart *chart.Chart, vals map[strin return nil, nil, err } - hooks, manifestDoc, notesTxt, err := u.cfg.renderResources(chart, valuesToRender, "", "", u.SubNotes, false, false, u.PostRenderer) + hooks, manifestDoc, notesTxt, err := u.cfg.renderResources(chart, valuesToRender, "", "", u.SubNotes, false, false, u.PostRenderer, u.DryRun) if err != nil { return nil, nil, err } diff --git a/pkg/chartutil/coalesce.go b/pkg/chartutil/coalesce.go index 94b7f35fa..1d3d45e99 100644 --- a/pkg/chartutil/coalesce.go +++ b/pkg/chartutil/coalesce.go @@ -175,7 +175,11 @@ func coalesceValues(c *chart.Chart, v map[string]interface{}) { // // dest is considered authoritative. func CoalesceTables(dst, src map[string]interface{}) map[string]interface{} { - if dst == nil || src == nil { + // When --reuse-values is set but there are no modifications yet, return new values + if src == nil { + return dst + } + if dst == nil { return src } // Because dest has higher precedence than src, dest values override src diff --git a/pkg/chartutil/coalesce_test.go b/pkg/chartutil/coalesce_test.go index dc1017385..2a3d848fa 100644 --- a/pkg/chartutil/coalesce_test.go +++ b/pkg/chartutil/coalesce_test.go @@ -211,4 +211,57 @@ func TestCoalesceTables(t *testing.T) { if _, ok = dst["hole"]; ok { t.Error("The hole still exists.") } + + dst2 := map[string]interface{}{ + "name": "Ishmael", + "address": map[string]interface{}{ + "street": "123 Spouter Inn Ct.", + "city": "Nantucket", + "country": "US", + }, + "details": map[string]interface{}{ + "friends": []string{"Tashtego"}, + }, + "boat": "pequod", + "hole": "black", + } + + // What we expect is that anything in dst should have all values set, + // this happens when the --reuse-values flag is set but the chart has no modifications yet + CoalesceTables(dst2, nil) + + if dst2["name"] != "Ishmael" { + t.Errorf("Unexpected name: %s", dst2["name"]) + } + + addr2, ok := dst2["address"].(map[string]interface{}) + if !ok { + t.Fatal("Address went away.") + } + + if addr2["street"].(string) != "123 Spouter Inn Ct." { + t.Errorf("Unexpected address: %v", addr2["street"]) + } + + if addr2["city"].(string) != "Nantucket" { + t.Errorf("Unexpected city: %v", addr2["city"]) + } + + if addr2["country"].(string) != "US" { + t.Errorf("Unexpected Country: %v", addr2["country"]) + } + + if det2, ok := dst2["details"].(map[string]interface{}); !ok { + t.Fatalf("Details is the wrong type: %v", dst2["details"]) + } else if _, ok := det2["friends"]; !ok { + t.Error("Could not find your friends. Maybe you don't have any. :-(") + } + + if dst2["boat"].(string) != "pequod" { + t.Errorf("Expected boat string, got %v", dst2["boat"]) + } + + if dst2["hole"].(string) != "black" { + t.Errorf("Expected hole string, got %v", dst2["boat"]) + } } diff --git a/pkg/chartutil/create.go b/pkg/chartutil/create.go index 28fb28e00..0e87c7b47 100644 --- a/pkg/chartutil/create.go +++ b/pkg/chartutil/create.go @@ -99,7 +99,7 @@ replicaCount: 1 image: repository: nginx pullPolicy: IfNotPresent - # Overrides the image tag whose default is the chart version. + # Overrides the image tag whose default is the chart appVersion. tag: "" imagePullSecrets: [] diff --git a/pkg/cli/environment.go b/pkg/cli/environment.go index e279331b0..2e64e0810 100644 --- a/pkg/cli/environment.go +++ b/pkg/cli/environment.go @@ -26,21 +26,17 @@ import ( "fmt" "os" "strconv" - "sync" "github.com/spf13/pflag" - "k8s.io/cli-runtime/pkg/genericclioptions" "helm.sh/helm/v3/pkg/helmpath" - "helm.sh/helm/v3/pkg/kube" ) // EnvSettings describes all of the environment settings. type EnvSettings struct { - namespace string - config genericclioptions.RESTClientGetter - configOnce sync.Once + namespace string + config *genericclioptions.ConfigFlags // KubeConfig is the path to the kubeconfig file KubeConfig string @@ -63,8 +59,7 @@ type EnvSettings struct { } func New() *EnvSettings { - - env := EnvSettings{ + env := &EnvSettings{ namespace: os.Getenv("HELM_NAMESPACE"), KubeContext: os.Getenv("HELM_KUBECONTEXT"), KubeToken: os.Getenv("HELM_KUBETOKEN"), @@ -75,7 +70,16 @@ func New() *EnvSettings { RepositoryCache: envOr("HELM_REPOSITORY_CACHE", helmpath.CachePath("repository")), } env.Debug, _ = strconv.ParseBool(os.Getenv("HELM_DEBUG")) - return &env + + // bind to kubernetes config flags + env.config = &genericclioptions.ConfigFlags{ + Namespace: &env.namespace, + Context: &env.KubeContext, + BearerToken: &env.KubeToken, + APIServer: &env.KubeAPIServer, + KubeConfig: &env.KubeConfig, + } + return env } // AddFlags binds flags to the given flagset. @@ -107,42 +111,27 @@ func (s *EnvSettings) EnvVars() map[string]string { "HELM_REPOSITORY_CACHE": s.RepositoryCache, "HELM_REPOSITORY_CONFIG": s.RepositoryConfig, "HELM_NAMESPACE": s.Namespace(), - "HELM_KUBECONTEXT": s.KubeContext, - "HELM_KUBETOKEN": s.KubeToken, - "HELM_KUBEAPISERVER": s.KubeAPIServer, - } + // broken, these are populated from helm flags and not kubeconfig. + "HELM_KUBECONTEXT": s.KubeContext, + "HELM_KUBETOKEN": s.KubeToken, + "HELM_KUBEAPISERVER": s.KubeAPIServer, + } if s.KubeConfig != "" { envvars["KUBECONFIG"] = s.KubeConfig } - return envvars } -//Namespace gets the namespace from the configuration +// Namespace gets the namespace from the configuration func (s *EnvSettings) Namespace() string { - if s.namespace != "" { - return s.namespace - } - - if ns, _, err := s.RESTClientGetter().ToRawKubeConfigLoader().Namespace(); err == nil { + if ns, _, err := s.config.ToRawKubeConfigLoader().Namespace(); err == nil { return ns } return "default" } -//RESTClientGetter gets the kubeconfig from EnvSettings +// RESTClientGetter gets the kubeconfig from EnvSettings func (s *EnvSettings) RESTClientGetter() genericclioptions.RESTClientGetter { - s.configOnce.Do(func() { - clientConfig := kube.GetConfig(s.KubeConfig, s.KubeContext, s.namespace) - if s.KubeToken != "" { - clientConfig.BearerToken = &s.KubeToken - } - if s.KubeAPIServer != "" { - clientConfig.APIServer = &s.KubeAPIServer - } - - s.config = clientConfig - }) return s.config } diff --git a/pkg/downloader/chart_downloader.go b/pkg/downloader/chart_downloader.go index 0013dbdf0..ef26f3348 100644 --- a/pkg/downloader/chart_downloader.go +++ b/pkg/downloader/chart_downloader.go @@ -18,7 +18,6 @@ package downloader import ( "fmt" "io" - "io/ioutil" "net/url" "os" "path/filepath" @@ -26,6 +25,7 @@ import ( "github.com/pkg/errors" + "helm.sh/helm/v3/internal/fileutil" "helm.sh/helm/v3/internal/urlutil" "helm.sh/helm/v3/pkg/getter" "helm.sh/helm/v3/pkg/helmpath" @@ -72,31 +72,6 @@ type ChartDownloader struct { RepositoryCache string } -// atomicWriteFile atomically (as atomic as os.Rename allows) writes a file to a -// disk. -func atomicWriteFile(filename string, body io.Reader, mode os.FileMode) error { - tempFile, err := ioutil.TempFile(filepath.Split(filename)) - if err != nil { - return err - } - tempName := tempFile.Name() - - if _, err := io.Copy(tempFile, body); err != nil { - tempFile.Close() // return value is ignored as we are already on error path - return err - } - - if err := tempFile.Close(); err != nil { - return err - } - - if err := os.Chmod(tempName, mode); err != nil { - return err - } - - return os.Rename(tempName, filename) -} - // DownloadTo retrieves a chart. Depending on the settings, it may also download a provenance file. // // If Verify is set to VerifyNever, the verification will be nil. @@ -126,7 +101,7 @@ func (c *ChartDownloader) DownloadTo(ref, version, dest string) (string, *proven name := filepath.Base(u.Path) destfile := filepath.Join(dest, name) - if err := atomicWriteFile(destfile, data, 0644); err != nil { + if err := fileutil.AtomicWriteFile(destfile, data, 0644); err != nil { return destfile, nil, err } @@ -142,7 +117,7 @@ func (c *ChartDownloader) DownloadTo(ref, version, dest string) (string, *proven return destfile, ver, nil } provfile := destfile + ".prov" - if err := atomicWriteFile(provfile, body, 0644); err != nil { + if err := fileutil.AtomicWriteFile(provfile, body, 0644); err != nil { return destfile, nil, err } diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go index 94ab1da95..c5d064ad5 100644 --- a/pkg/engine/engine.go +++ b/pkg/engine/engine.go @@ -172,7 +172,10 @@ func (e Engine) initFunMap(t *template.Template, referenceTpls map[string]render } return val, nil } - if e.config != nil { + + // If we are not linting and have a cluster connection, provide a Kubernetes-backed + // implementation. + if !e.LintMode && e.config != nil { funcMap["lookup"] = NewLookupFunction(e.config) } diff --git a/pkg/engine/engine_test.go b/pkg/engine/engine_test.go index c1cdf625e..87e84c48b 100644 --- a/pkg/engine/engine_test.go +++ b/pkg/engine/engine_test.go @@ -70,7 +70,7 @@ func TestFuncMap(t *testing.T) { } // Test for Engine-specific template functions. - expect := []string{"include", "required", "tpl", "toYaml", "fromYaml", "toToml", "toJson", "fromJson"} + expect := []string{"include", "required", "tpl", "toYaml", "fromYaml", "toToml", "toJson", "fromJson", "lookup"} for _, f := range expect { if _, ok := fns[f]; !ok { t.Errorf("Expected add-on function %q", f) diff --git a/pkg/engine/funcs.go b/pkg/engine/funcs.go index e5769cbe0..92b4c3383 100644 --- a/pkg/engine/funcs.go +++ b/pkg/engine/funcs.go @@ -62,6 +62,11 @@ func funcMap() template.FuncMap { "include": func(string, interface{}) string { return "not implemented" }, "tpl": func(string, interface{}) interface{} { return "not implemented" }, "required": func(string, interface{}) (interface{}, error) { return "not implemented", nil }, + // Provide a placeholder for the "lookup" function, which requires a kubernetes + // connection. + "lookup": func(string, string, string, string) (map[string]interface{}, error) { + return map[string]interface{}{}, nil + }, } for k, v := range extra { diff --git a/pkg/engine/funcs_test.go b/pkg/engine/funcs_test.go index ddcf6624c..62c63ec2b 100644 --- a/pkg/engine/funcs_test.go +++ b/pkg/engine/funcs_test.go @@ -94,6 +94,11 @@ func TestFuncs(t *testing.T) { tpl: `{{ fromYamlArray . }}`, expect: `[error unmarshaling JSON: while decoding JSON: json: cannot unmarshal object into Go value of type []interface {}]`, vars: `hello: world`, + }, { + // This should never result in a network lookup. Regression for #7955 + tpl: `{{ lookup "v1" "Namespace" "" "unlikelynamespace99999999" }}`, + expect: `map[]`, + vars: `["one", "two"]`, }} for _, tt := range tests { diff --git a/pkg/engine/lookup_func.go b/pkg/engine/lookup_func.go index 2527954fa..20be9189e 100644 --- a/pkg/engine/lookup_func.go +++ b/pkg/engine/lookup_func.go @@ -32,8 +32,12 @@ import ( type lookupFunc = func(apiversion string, resource string, namespace string, name string) (map[string]interface{}, error) -// NewLookupFunction returns a function for looking up objects in the cluster. If the resource does not exist, no error -// is raised. +// NewLookupFunction returns a function for looking up objects in the cluster. +// +// If the resource does not exist, no error is raised. +// +// This function is considered deprecated, and will be renamed in Helm 4. It will no +// longer be a public function. func NewLookupFunction(config *rest.Config) lookupFunc { return func(apiversion string, resource string, namespace string, name string) (map[string]interface{}, error) { var client dynamic.ResourceInterface diff --git a/pkg/getter/getter_test.go b/pkg/getter/getter_test.go index 60eb4738e..79a3338e9 100644 --- a/pkg/getter/getter_test.go +++ b/pkg/getter/getter_test.go @@ -53,9 +53,10 @@ func TestProviders(t *testing.T) { } func TestAll(t *testing.T) { - all := All(&cli.EnvSettings{ - PluginsDirectory: pluginDir, - }) + env := cli.New() + env.PluginsDirectory = pluginDir + + all := All(env) if len(all) != 3 { t.Errorf("expected 3 providers (default plus two plugins), got %d", len(all)) } @@ -66,9 +67,10 @@ func TestAll(t *testing.T) { } func TestByScheme(t *testing.T) { - g := All(&cli.EnvSettings{ - PluginsDirectory: pluginDir, - }) + env := cli.New() + env.PluginsDirectory = pluginDir + + g := All(env) if _, err := g.ByScheme("test"); err != nil { t.Error(err) } diff --git a/pkg/getter/httpgetter_test.go b/pkg/getter/httpgetter_test.go index a1288bf47..4d7ada852 100644 --- a/pkg/getter/httpgetter_test.go +++ b/pkg/getter/httpgetter_test.go @@ -122,7 +122,7 @@ func TestDownload(t *testing.T) { })) defer srv.Close() - g, err := All(new(cli.EnvSettings)).ByScheme("http") + g, err := All(cli.New()).ByScheme("http") if err != nil { t.Fatal(err) } diff --git a/pkg/getter/plugingetter_test.go b/pkg/getter/plugingetter_test.go index 71563e169..a18fa302b 100644 --- a/pkg/getter/plugingetter_test.go +++ b/pkg/getter/plugingetter_test.go @@ -24,9 +24,9 @@ import ( ) func TestCollectPlugins(t *testing.T) { - env := &cli.EnvSettings{ - PluginsDirectory: pluginDir, - } + env := cli.New() + env.PluginsDirectory = pluginDir + p, err := collectPlugins(env) if err != nil { t.Fatal(err) @@ -54,9 +54,8 @@ func TestPluginGetter(t *testing.T) { t.Skip("TODO: refactor this test to work on windows") } - env := &cli.EnvSettings{ - PluginsDirectory: pluginDir, - } + env := cli.New() + env.PluginsDirectory = pluginDir pg := NewPluginGetter("echo", env, "test", ".") g, err := pg() if err != nil { @@ -80,9 +79,9 @@ func TestPluginSubCommands(t *testing.T) { t.Skip("TODO: refactor this test to work on windows") } - env := &cli.EnvSettings{ - PluginsDirectory: pluginDir, - } + env := cli.New() + env.PluginsDirectory = pluginDir + pg := NewPluginGetter("echo -n", env, "test", ".") g, err := pg() if err != nil { diff --git a/pkg/kube/client.go b/pkg/kube/client.go index 05b26b12a..8a4831ffb 100644 --- a/pkg/kube/client.go +++ b/pkg/kube/client.go @@ -223,6 +223,7 @@ func (c *Client) Update(original, target ResourceList, force bool) (*Result, err if err := info.Get(); err != nil { c.Log("Unable to get obj %q, err: %s", info.Name, err) + continue } annotations, err := metadataAccessor.Annotations(info.Object) if err != nil { @@ -232,16 +233,11 @@ func (c *Client) Update(original, target ResourceList, force bool) (*Result, err c.Log("Skipping delete of %q due to annotation [%s=%s]", info.Name, ResourcePolicyAnno, KeepPolicy) continue } - - res.Deleted = append(res.Deleted, info) if err := deleteResource(info); err != nil { - if apierrors.IsNotFound(err) { - c.Log("Attempted to delete %q, but the resource was missing", info.Name) - } else { - c.Log("Failed to delete %q, err: %s", info.Name, err) - return res, errors.Wrapf(err, "Failed to delete %q", info.Name) - } + c.Log("Failed to delete %q, err: %s", info.ObjectName(), err) + continue } + res.Deleted = append(res.Deleted, info) } return res, nil } diff --git a/pkg/kube/client_test.go b/pkg/kube/client_test.go index aa081423c..568afa094 100644 --- a/pkg/kube/client_test.go +++ b/pkg/kube/client_test.go @@ -164,9 +164,21 @@ func TestUpdate(t *testing.T) { t.Fatal(err) } - if _, err := c.Update(first, second, false); err != nil { + result, err := c.Update(first, second, false) + if err != nil { t.Fatal(err) } + + if len(result.Created) != 1 { + t.Errorf("expected 1 resource created, got %d", len(result.Created)) + } + if len(result.Updated) != 2 { + t.Errorf("expected 2 resource updated, got %d", len(result.Updated)) + } + if len(result.Deleted) != 1 { + t.Errorf("expected 1 resource deleted, got %d", len(result.Deleted)) + } + // TODO: Find a way to test methods that use Client Set // Test with a wait // if err := c.Update("test", objBody(codec, &listB), objBody(codec, &listC), false, 300, true); err != nil { @@ -190,8 +202,7 @@ func TestUpdate(t *testing.T) { "/namespaces/default/pods/squid:DELETE", } if len(expectedActions) != len(actions) { - t.Errorf("unexpected number of requests, expected %d, got %d", len(expectedActions), len(actions)) - return + t.Fatalf("unexpected number of requests, expected %d, got %d", len(expectedActions), len(actions)) } for k, v := range expectedActions { if actions[k] != v { diff --git a/pkg/kube/config.go b/pkg/kube/config.go index 624c4a1f7..e00c9acb1 100644 --- a/pkg/kube/config.go +++ b/pkg/kube/config.go @@ -19,6 +19,8 @@ package kube // import "helm.sh/helm/v3/pkg/kube" import "k8s.io/cli-runtime/pkg/genericclioptions" // GetConfig returns a Kubernetes client config. +// +// Deprecated func GetConfig(kubeconfig, context, namespace string) *genericclioptions.ConfigFlags { cf := genericclioptions.NewConfigFlags(true) cf.Namespace = &namespace diff --git a/pkg/plugin/installer/base.go b/pkg/plugin/installer/base.go index a8ec97416..dcc3ad644 100644 --- a/pkg/plugin/installer/base.go +++ b/pkg/plugin/installer/base.go @@ -16,7 +16,6 @@ limitations under the License. package installer // import "helm.sh/helm/v3/pkg/plugin/installer" import ( - "os" "path/filepath" "helm.sh/helm/v3/pkg/helmpath" @@ -31,13 +30,7 @@ func newBase(source string) base { return base{source} } -// link creates a symlink from the plugin source to the base path. -func (b *base) link(from string) error { - debug("symlinking %s to %s", from, b.Path()) - return os.Symlink(from, b.Path()) -} - -// Path is where the plugin will be symlinked to. +// Path is where the plugin will be installed. func (b *base) Path() string { if b.Source == "" { return "" diff --git a/pkg/plugin/installer/http_installer.go b/pkg/plugin/installer/http_installer.go index ea4ac7bcd..629bbec39 100644 --- a/pkg/plugin/installer/http_installer.go +++ b/pkg/plugin/installer/http_installer.go @@ -27,6 +27,7 @@ import ( "github.com/pkg/errors" + "helm.sh/helm/v3/internal/third_party/dep/fs" "helm.sh/helm/v3/pkg/cli" "helm.sh/helm/v3/pkg/getter" "helm.sh/helm/v3/pkg/helmpath" @@ -68,7 +69,6 @@ func NewExtractor(source string) (Extractor, error) { // NewHTTPInstaller creates a new HttpInstaller. func NewHTTPInstaller(source string) (*HTTPInstaller, error) { - key, err := cache.Key(source) if err != nil { return nil, err @@ -108,18 +108,16 @@ func stripPluginName(name string) string { } // Install downloads and extracts the tarball into the cache directory -// and creates a symlink to the plugin directory. +// and installs into the plugin directory. // // Implements Installer. func (i *HTTPInstaller) Install() error { - pluginData, err := i.getter.Get(i.Source) if err != nil { return err } - err = i.extractor.Extract(pluginData, i.CacheDir) - if err != nil { + if err := i.extractor.Extract(pluginData, i.CacheDir); err != nil { return err } @@ -132,7 +130,8 @@ func (i *HTTPInstaller) Install() error { return err } - return i.link(src) + debug("copying %s to %s", src, i.Path()) + return fs.CopyDir(src, i.Path()) } // Update updates a local repository @@ -141,12 +140,6 @@ func (i *HTTPInstaller) Update() error { return errors.Errorf("method Update() not implemented for HttpInstaller") } -// Override link because we want to use HttpInstaller.Path() not base.Path() -func (i *HTTPInstaller) link(from string) error { - debug("symlinking %s to %s", from, i.Path()) - return os.Symlink(from, i.Path()) -} - // Path is overridden because we want to join on the plugin name not the file name func (i HTTPInstaller) Path() string { if i.base.Source == "" { @@ -164,17 +157,16 @@ func (g *TarGzExtractor) Extract(buffer *bytes.Buffer, targetDir string) error { return err } - tarReader := tar.NewReader(uncompressedStream) - - os.MkdirAll(targetDir, 0755) + if err := os.MkdirAll(targetDir, 0755); err != nil { + return err + } + tarReader := tar.NewReader(uncompressedStream) for { header, err := tarReader.Next() - if err == io.EOF { break } - if err != nil { return err } @@ -200,7 +192,5 @@ func (g *TarGzExtractor) Extract(buffer *bytes.Buffer, targetDir string) error { return errors.Errorf("unknown type: %b in %s", header.Typeflag, header.Name) } } - return nil - } diff --git a/pkg/plugin/installer/http_installer_test.go b/pkg/plugin/installer/http_installer_test.go index cfa2e4cbe..b496a1b01 100644 --- a/pkg/plugin/installer/http_installer_test.go +++ b/pkg/plugin/installer/http_installer_test.go @@ -73,13 +73,13 @@ func TestHTTPInstaller(t *testing.T) { i, err := NewForSource(source, "0.0.1") if err != nil { - t.Errorf("unexpected error: %s", err) + t.Fatalf("unexpected error: %s", err) } // ensure a HTTPInstaller was returned httpInstaller, ok := i.(*HTTPInstaller) if !ok { - t.Error("expected a HTTPInstaller") + t.Fatal("expected a HTTPInstaller") } // inject fake http client responding with minimal plugin tarball @@ -94,17 +94,17 @@ func TestHTTPInstaller(t *testing.T) { // install the plugin if err := Install(i); err != nil { - t.Error(err) + t.Fatal(err) } if i.Path() != helmpath.DataPath("plugins", "fake-plugin") { - t.Errorf("expected path '$XDG_CONFIG_HOME/helm/plugins/fake-plugin', got %q", i.Path()) + t.Fatalf("expected path '$XDG_CONFIG_HOME/helm/plugins/fake-plugin', got %q", i.Path()) } // Install again to test plugin exists error if err := Install(i); err == nil { - t.Error("expected error for plugin exists, got none") + t.Fatal("expected error for plugin exists, got none") } else if err.Error() != "plugin already exists" { - t.Errorf("expected error for plugin exists, got (%v)", err) + t.Fatalf("expected error for plugin exists, got (%v)", err) } } @@ -119,13 +119,13 @@ func TestHTTPInstallerNonExistentVersion(t *testing.T) { i, err := NewForSource(source, "0.0.2") if err != nil { - t.Errorf("unexpected error: %s", err) + t.Fatalf("unexpected error: %s", err) } // ensure a HTTPInstaller was returned httpInstaller, ok := i.(*HTTPInstaller) if !ok { - t.Error("expected a HTTPInstaller") + t.Fatal("expected a HTTPInstaller") } // inject fake http client responding with error @@ -135,7 +135,7 @@ func TestHTTPInstallerNonExistentVersion(t *testing.T) { // attempt to install the plugin if err := Install(i); err == nil { - t.Error("expected error from http client") + t.Fatal("expected error from http client") } } @@ -150,13 +150,13 @@ func TestHTTPInstallerUpdate(t *testing.T) { i, err := NewForSource(source, "0.0.1") if err != nil { - t.Errorf("unexpected error: %s", err) + t.Fatalf("unexpected error: %s", err) } // ensure a HTTPInstaller was returned httpInstaller, ok := i.(*HTTPInstaller) if !ok { - t.Error("expected a HTTPInstaller") + t.Fatal("expected a HTTPInstaller") } // inject fake http client responding with minimal plugin tarball @@ -171,15 +171,15 @@ func TestHTTPInstallerUpdate(t *testing.T) { // install the plugin before updating if err := Install(i); err != nil { - t.Error(err) + t.Fatal(err) } if i.Path() != helmpath.DataPath("plugins", "fake-plugin") { - t.Errorf("expected path '$XDG_CONFIG_HOME/helm/plugins/fake-plugin', got %q", i.Path()) + t.Fatalf("expected path '$XDG_CONFIG_HOME/helm/plugins/fake-plugin', got %q", i.Path()) } // Update plugin, should fail because it is not implemented if err := Update(i); err == nil { - t.Error("update method not implemented for http installer") + t.Fatal("update method not implemented for http installer") } } @@ -240,29 +240,27 @@ func TestExtract(t *testing.T) { } if err = extractor.Extract(&buf, tempDir); err != nil { - t.Errorf("Did not expect error but got error: %v", err) + t.Fatalf("Did not expect error but got error: %v", err) } pluginYAMLFullPath := filepath.Join(tempDir, "plugin.yaml") if info, err := os.Stat(pluginYAMLFullPath); err != nil { if os.IsNotExist(err) { - t.Errorf("Expected %s to exist but doesn't", pluginYAMLFullPath) - } else { - t.Error(err) + t.Fatalf("Expected %s to exist but doesn't", pluginYAMLFullPath) } + t.Fatal(err) } else if info.Mode().Perm() != 0600 { - t.Errorf("Expected %s to have 0600 mode it but has %o", pluginYAMLFullPath, info.Mode().Perm()) + t.Fatalf("Expected %s to have 0600 mode it but has %o", pluginYAMLFullPath, info.Mode().Perm()) } readmeFullPath := filepath.Join(tempDir, "README.md") if info, err := os.Stat(readmeFullPath); err != nil { if os.IsNotExist(err) { - t.Errorf("Expected %s to exist but doesn't", readmeFullPath) - } else { - t.Error(err) + t.Fatalf("Expected %s to exist but doesn't", readmeFullPath) } + t.Fatal(err) } else if info.Mode().Perm() != 0777 { - t.Errorf("Expected %s to have 0777 mode it but has %o", readmeFullPath, info.Mode().Perm()) + t.Fatalf("Expected %s to have 0777 mode it but has %o", readmeFullPath, info.Mode().Perm()) } } diff --git a/pkg/plugin/installer/installer.go b/pkg/plugin/installer/installer.go index 14a02a87e..65c61cd7d 100644 --- a/pkg/plugin/installer/installer.go +++ b/pkg/plugin/installer/installer.go @@ -17,6 +17,7 @@ package installer import ( "fmt" + "log" "os" "path/filepath" "strings" @@ -103,9 +104,10 @@ func isPlugin(dirname string) bool { return err == nil } +var logger = log.New(os.Stderr, "[debug] ", log.Lshortfile) + func debug(format string, args ...interface{}) { if Debug { - format = fmt.Sprintf("[debug] %s\n", format) - fmt.Printf(format, args...) + logger.Output(2, fmt.Sprintf(format, args...)) } } diff --git a/pkg/plugin/installer/local_installer.go b/pkg/plugin/installer/local_installer.go index 662ef5b29..c92bc3fb0 100644 --- a/pkg/plugin/installer/local_installer.go +++ b/pkg/plugin/installer/local_installer.go @@ -16,6 +16,7 @@ limitations under the License. package installer // import "helm.sh/helm/v3/pkg/plugin/installer" import ( + "os" "path/filepath" "github.com/pkg/errors" @@ -45,7 +46,8 @@ func (i *LocalInstaller) Install() error { if !isPlugin(i.Source) { return ErrMissingMetadata } - return i.link(i.Source) + debug("symlinking %s to %s", i.Source, i.Path()) + return os.Symlink(i.Source, i.Path()) } // Update updates a local repository diff --git a/pkg/plugin/installer/local_installer_test.go b/pkg/plugin/installer/local_installer_test.go index d11e44860..3d9607331 100644 --- a/pkg/plugin/installer/local_installer_test.go +++ b/pkg/plugin/installer/local_installer_test.go @@ -40,7 +40,7 @@ func TestLocalInstaller(t *testing.T) { source := "../testdata/plugdir/echo" i, err := NewForSource(source, "") if err != nil { - t.Errorf("unexpected error: %s", err) + t.Fatalf("unexpected error: %s", err) } if err := Install(i); err != nil { @@ -48,6 +48,6 @@ func TestLocalInstaller(t *testing.T) { } if i.Path() != helmpath.DataPath("plugins", "echo") { - t.Errorf("expected path '$XDG_CONFIG_HOME/helm/plugins/helm-env', got %q", i.Path()) + t.Fatalf("expected path '$XDG_CONFIG_HOME/helm/plugins/helm-env', got %q", i.Path()) } } diff --git a/pkg/plugin/installer/vcs_installer.go b/pkg/plugin/installer/vcs_installer.go index 1a5d74cca..f7df5b322 100644 --- a/pkg/plugin/installer/vcs_installer.go +++ b/pkg/plugin/installer/vcs_installer.go @@ -23,6 +23,7 @@ import ( "github.com/Masterminds/vcs" "github.com/pkg/errors" + "helm.sh/helm/v3/internal/third_party/dep/fs" "helm.sh/helm/v3/pkg/helmpath" "helm.sh/helm/v3/pkg/plugin/cache" ) @@ -43,7 +44,7 @@ func existingVCSRepo(location string) (Installer, error) { Repo: repo, base: newBase(repo.Remote()), } - return i, err + return i, nil } // NewVCSInstaller creates a new VCSInstaller. @@ -65,7 +66,7 @@ func NewVCSInstaller(source, version string) (*VCSInstaller, error) { return i, err } -// Install clones a remote repository and creates a symlink to the plugin directory. +// Install clones a remote repository and installs into the plugin directory. // // Implements Installer. func (i *VCSInstaller) Install() error { @@ -87,7 +88,8 @@ func (i *VCSInstaller) Install() error { return ErrMissingMetadata } - return i.link(i.Repo.LocalPath()) + debug("copying %s to %s", i.Repo.LocalPath(), i.Path()) + return fs.CopyDir(i.Repo.LocalPath(), i.Path()) } // Update updates a remote repository diff --git a/pkg/plugin/installer/vcs_installer_test.go b/pkg/plugin/installer/vcs_installer_test.go index ce1ee609e..b8dc6b1e2 100644 --- a/pkg/plugin/installer/vcs_installer_test.go +++ b/pkg/plugin/installer/vcs_installer_test.go @@ -80,24 +80,24 @@ func TestVCSInstaller(t *testing.T) { t.Fatal(err) } if repo.current != "0.1.1" { - t.Errorf("expected version '0.1.1', got %q", repo.current) + t.Fatalf("expected version '0.1.1', got %q", repo.current) } if i.Path() != helmpath.DataPath("plugins", "helm-env") { - t.Errorf("expected path '$XDG_CONFIG_HOME/helm/plugins/helm-env', got %q", i.Path()) + t.Fatalf("expected path '$XDG_CONFIG_HOME/helm/plugins/helm-env', got %q", i.Path()) } // Install again to test plugin exists error if err := Install(i); err == nil { - t.Error("expected error for plugin exists, got none") + t.Fatalf("expected error for plugin exists, got none") } else if err.Error() != "plugin already exists" { - t.Errorf("expected error for plugin exists, got (%v)", err) + t.Fatalf("expected error for plugin exists, got (%v)", err) } // Testing FindSource method, expect error because plugin code is not a cloned repository if _, err := FindSource(i.Path()); err == nil { - t.Error("expected error for inability to find plugin source, got none") + t.Fatalf("expected error for inability to find plugin source, got none") } else if err.Error() != "cannot get information about plugin source" { - t.Errorf("expected error for inability to find plugin source, got (%v)", err) + t.Fatalf("expected error for inability to find plugin source, got (%v)", err) } } @@ -113,15 +113,14 @@ func TestVCSInstallerNonExistentVersion(t *testing.T) { } // ensure a VCSInstaller was returned - _, ok := i.(*VCSInstaller) - if !ok { + if _, ok := i.(*VCSInstaller); !ok { t.Fatal("expected a VCSInstaller") } if err := Install(i); err == nil { - t.Error("expected error for version does not exists, got none") + t.Fatalf("expected error for version does not exists, got none") } else if err.Error() != fmt.Sprintf("requested version %q does not exist for plugin %q", version, source) { - t.Errorf("expected error for version does not exists, got (%v)", err) + t.Fatalf("expected error for version does not exists, got (%v)", err) } } func TestVCSInstallerUpdate(t *testing.T) { @@ -135,8 +134,7 @@ func TestVCSInstallerUpdate(t *testing.T) { } // ensure a VCSInstaller was returned - _, ok := i.(*VCSInstaller) - if !ok { + if _, ok := i.(*VCSInstaller); !ok { t.Fatal("expected a VCSInstaller") } @@ -157,7 +155,9 @@ func TestVCSInstallerUpdate(t *testing.T) { t.Fatal(err) } - repoRemote := pluginInfo.(*VCSInstaller).Repo.Remote() + vcsInstaller := pluginInfo.(*VCSInstaller) + + repoRemote := vcsInstaller.Repo.Remote() if repoRemote != source { t.Fatalf("invalid source found, expected %q got %q", source, repoRemote) } @@ -168,12 +168,14 @@ func TestVCSInstallerUpdate(t *testing.T) { } // Test update failure - os.Remove(filepath.Join(i.Path(), "plugin.yaml")) + if err := os.Remove(filepath.Join(vcsInstaller.Repo.LocalPath(), "plugin.yaml")); err != nil { + t.Fatal(err) + } // Testing update for error - if err := Update(i); err == nil { - t.Error("expected error for plugin modified, got none") + if err := Update(vcsInstaller); err == nil { + t.Fatalf("expected error for plugin modified, got none") } else if err.Error() != "plugin repo was modified" { - t.Errorf("expected error for plugin modified, got (%v)", err) + t.Fatalf("expected error for plugin modified, got (%v)", err) } } diff --git a/pkg/plugin/plugin_test.go b/pkg/plugin/plugin_test.go index 7bbc3b442..af0b61846 100644 --- a/pkg/plugin/plugin_test.go +++ b/pkg/plugin/plugin_test.go @@ -28,14 +28,14 @@ import ( func checkCommand(p *Plugin, extraArgs []string, osStrCmp string, t *testing.T) { cmd, args, err := p.PrepareCommand(extraArgs) if err != nil { - t.Errorf(err.Error()) + t.Fatal(err) } if cmd != "echo" { - t.Errorf("Expected echo, got %q", cmd) + t.Fatalf("Expected echo, got %q", cmd) } if l := len(args); l != 5 { - t.Errorf("expected 5 args, got %d", l) + t.Fatalf("expected 5 args, got %d", l) } expect := []string{"-n", osStrCmp, "--debug", "--foo", "bar"} @@ -49,13 +49,13 @@ func checkCommand(p *Plugin, extraArgs []string, osStrCmp string, t *testing.T) p.Metadata.IgnoreFlags = true cmd, args, err = p.PrepareCommand(extraArgs) if err != nil { - t.Errorf(err.Error()) + t.Fatal(err) } if cmd != "echo" { - t.Errorf("Expected echo, got %q", cmd) + t.Fatalf("Expected echo, got %q", cmd) } if l := len(args); l != 2 { - t.Errorf("expected 2 args, got %d", l) + t.Fatalf("expected 2 args, got %d", l) } expect = []string{"-n", osStrCmp} for i := 0; i < len(args); i++ { @@ -155,7 +155,7 @@ func TestNoPrepareCommand(t *testing.T) { _, _, err := p.PrepareCommand(argv) if err == nil { - t.Errorf("Expected error to be returned") + t.Fatalf("Expected error to be returned") } } @@ -172,7 +172,7 @@ func TestNoMatchPrepareCommand(t *testing.T) { argv := []string{"--debug", "--foo", "bar"} if _, _, err := p.PrepareCommand(argv); err == nil { - t.Errorf("Expected error to be returned") + t.Fatalf("Expected error to be returned") } } @@ -184,7 +184,7 @@ func TestLoadDir(t *testing.T) { } if plug.Dir != dirname { - t.Errorf("Expected dir %q, got %q", dirname, plug.Dir) + t.Fatalf("Expected dir %q, got %q", dirname, plug.Dir) } expect := &Metadata{ @@ -200,7 +200,7 @@ func TestLoadDir(t *testing.T) { } if !reflect.DeepEqual(expect, plug.Metadata) { - t.Errorf("Expected plugin metadata %v, got %v", expect, plug.Metadata) + t.Fatalf("Expected plugin metadata %v, got %v", expect, plug.Metadata) } } @@ -212,7 +212,7 @@ func TestDownloader(t *testing.T) { } if plug.Dir != dirname { - t.Errorf("Expected dir %q, got %q", dirname, plug.Dir) + t.Fatalf("Expected dir %q, got %q", dirname, plug.Dir) } expect := &Metadata{ @@ -230,7 +230,7 @@ func TestDownloader(t *testing.T) { } if !reflect.DeepEqual(expect, plug.Metadata) { - t.Errorf("Expected metadata %v, got %v", expect, plug.Metadata) + t.Fatalf("Expected metadata %v, got %v", expect, plug.Metadata) } } @@ -305,9 +305,8 @@ func TestSetupEnv(t *testing.T) { name := "pequod" base := filepath.Join("testdata/helmhome/helm/plugins", name) - s := &cli.EnvSettings{ - PluginsDirectory: "testdata/helmhome/helm/plugins", - } + s := cli.New() + s.PluginsDirectory = "testdata/helmhome/helm/plugins" SetupPluginEnv(s, name, base) for _, tt := range []struct { diff --git a/pkg/repo/index.go b/pkg/repo/index.go index 36386665e..6ef2cf8b5 100644 --- a/pkg/repo/index.go +++ b/pkg/repo/index.go @@ -17,6 +17,7 @@ limitations under the License. package repo import ( + "bytes" "io/ioutil" "os" "path" @@ -29,6 +30,7 @@ import ( "github.com/pkg/errors" "sigs.k8s.io/yaml" + "helm.sh/helm/v3/internal/fileutil" "helm.sh/helm/v3/internal/urlutil" "helm.sh/helm/v3/pkg/chart" "helm.sh/helm/v3/pkg/chart/loader" @@ -197,7 +199,7 @@ func (i IndexFile) WriteFile(dest string, mode os.FileMode) error { if err != nil { return err } - return ioutil.WriteFile(dest, b, mode) + return fileutil.AtomicWriteFile(dest, bytes.NewReader(b), mode) } // Merge merges the given index file into this index. diff --git a/pkg/repo/index_test.go b/pkg/repo/index_test.go index 5dbd5e551..466a2c306 100644 --- a/pkg/repo/index_test.go +++ b/pkg/repo/index_test.go @@ -428,3 +428,23 @@ func TestIndexAdd(t *testing.T) { t.Errorf("Expected http://example.com/charts/deis-0.1.0.tgz, got %s", i.Entries["deis"][0].URLs[0]) } } + +func TestIndexWrite(t *testing.T) { + i := NewIndexFile() + i.Add(&chart.Metadata{Name: "clipper", Version: "0.1.0"}, "clipper-0.1.0.tgz", "http://example.com/charts", "sha256:1234567890") + dir, err := ioutil.TempDir("", "helm-tmp") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(dir) + testpath := filepath.Join(dir, "test") + i.WriteFile(testpath, 0600) + + got, err := ioutil.ReadFile(testpath) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(got), "clipper-0.1.0.tgz") { + t.Fatal("Index files doesn't contain expected content") + } +}