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/.github/pull_request_template.md b/.github/pull_request_template.md new file mode 100644 index 000000000..595b50218 --- /dev/null +++ b/.github/pull_request_template.md @@ -0,0 +1,12 @@ + + +**What this PR does / why we need it**: + +**Special notes for your reviewer**: + +**If applicable**: +- [ ] this PR contains documentation +- [ ] this PR contains unit tests +- [ ] this PR has been tested for backwards compatibility diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 63780365e..a637f9255 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -184,9 +184,9 @@ contributing to Helm. All issue types follow the same general lifecycle. Differe ## How to Contribute a Patch -1. If you haven't already done so, sign a Contributor License Agreement (see details above). -2. Fork the desired repo, develop and test your code changes. -3. Submit a pull request. +1. Identify or create the related issue. +2. Fork the desired repo; develop and test your code changes. +3. Submit a pull request, making sure to sign your work and link the related issue. Coding conventions and standards are explained in the [official developer docs](https://helm.sh/docs/developers/). 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/repo_list.go b/cmd/helm/repo_list.go index 51b4f0d58..ed1c9573c 100644 --- a/cmd/helm/repo_list.go +++ b/cmd/helm/repo_list.go @@ -38,7 +38,7 @@ func newRepoListCmd(out io.Writer) *cobra.Command { Args: require.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { f, err := repo.LoadFile(settings.RepositoryConfig) - if isNotExist(err) || len(f.Repositories) == 0 { + if isNotExist(err) || (len(f.Repositories) == 0 && !(outfmt == output.JSON || outfmt == output.YAML)) { return errors.New("no repositories to show") } diff --git a/cmd/helm/root.go b/cmd/helm/root.go index e9bc26fe4..143745f29 100644 --- a/cmd/helm/root.go +++ b/cmd/helm/root.go @@ -43,17 +43,15 @@ Common actions for Helm: Environment variables: -+------------------+--------------------------------------------------------------------------------------------------------+ -| Name | Description | -+------------------+--------------------------------------------------------------------------------------------------------+ -| $HELM_CACHE_HOME | set an alternative location for storing cached files. | -| $HELM_CONFIG_HOME | set an alternative location for storing Helm configuration. | -| $HELM_DATA_HOME | set an alternative location for storing Helm data. | -| $HELM_DRIVER | set the backend storage driver. Values are: configmap, secret, memory, postgres | -| $HELM_DRIVER_SQL_CONNECTION_STRING | set the connection string the SQL storage driver should use. | -| $HELM_NO_PLUGINS | disable plugins. Set HELM_NO_PLUGINS=1 to disable plugins. | -| $KUBECONFIG | set an alternative Kubernetes configuration file (default "~/.kube/config") | -+------------------+--------------------------------------------------------------------------------------------------------+ +| Name | Description | +|------------------------------------|-----------------------------------------------------------------------------------| +| $HELM_CACHE_HOME | set an alternative location for storing cached files. | +| $HELM_CONFIG_HOME | set an alternative location for storing Helm configuration. | +| $HELM_DATA_HOME | set an alternative location for storing Helm data. | +| $HELM_DRIVER | set the backend storage driver. Values are: configmap, secret, memory, postgres | +| $HELM_DRIVER_SQL_CONNECTION_STRING | set the connection string the SQL storage driver should use. | +| $HELM_NO_PLUGINS | disable plugins. Set HELM_NO_PLUGINS=1 to disable plugins. | +| $KUBECONFIG | set an alternative Kubernetes configuration file (default "~/.kube/config") | Helm stores cache, configuration, and data based on the following configuration order: @@ -63,13 +61,11 @@ Helm stores cache, configuration, and data based on the following configuration By default, the default directories depend on the Operating System. The defaults are listed below: -+------------------+---------------------------+--------------------------------+-------------------------+ | Operating System | Cache Path | Configuration Path | Data Path | -+------------------+---------------------------+--------------------------------+-------------------------+ +|------------------|---------------------------|--------------------------------|-------------------------| | Linux | $HOME/.cache/helm | $HOME/.config/helm | $HOME/.local/share/helm | | macOS | $HOME/Library/Caches/helm | $HOME/Library/Preferences/helm | $HOME/Library/helm | | Windows | %TEMP%\helm | %APPDATA%\helm | %APPDATA%\helm | -+------------------+---------------------------+--------------------------------+-------------------------+ ` func newRootCmd(actionConfig *action.Configuration, out io.Writer, args []string) *cobra.Command { diff --git a/cmd/helm/testdata/output/version-client-shorthand.txt b/cmd/helm/testdata/output/version-client-shorthand.txt index 8f9ed6136..d613309fe 100644 --- a/cmd/helm/testdata/output/version-client-shorthand.txt +++ b/cmd/helm/testdata/output/version-client-shorthand.txt @@ -1 +1 @@ -version.BuildInfo{Version:"v3.1", GitCommit:"", GitTreeState:"", GoVersion:""} +version.BuildInfo{Version:"v3.2", GitCommit:"", GitTreeState:"", GoVersion:""} diff --git a/cmd/helm/testdata/output/version-client.txt b/cmd/helm/testdata/output/version-client.txt index 8f9ed6136..d613309fe 100644 --- a/cmd/helm/testdata/output/version-client.txt +++ b/cmd/helm/testdata/output/version-client.txt @@ -1 +1 @@ -version.BuildInfo{Version:"v3.1", GitCommit:"", GitTreeState:"", GoVersion:""} +version.BuildInfo{Version:"v3.2", GitCommit:"", GitTreeState:"", GoVersion:""} diff --git a/cmd/helm/testdata/output/version-short.txt b/cmd/helm/testdata/output/version-short.txt index 861668947..4d5034cea 100644 --- a/cmd/helm/testdata/output/version-short.txt +++ b/cmd/helm/testdata/output/version-short.txt @@ -1 +1 @@ -v3.1 +v3.2 diff --git a/cmd/helm/testdata/output/version-template.txt b/cmd/helm/testdata/output/version-template.txt index e5a779bbf..7c09e8d57 100644 --- a/cmd/helm/testdata/output/version-template.txt +++ b/cmd/helm/testdata/output/version-template.txt @@ -1 +1 @@ -Version: v3.1 \ No newline at end of file +Version: v3.2 \ No newline at end of file diff --git a/cmd/helm/testdata/output/version.txt b/cmd/helm/testdata/output/version.txt index 8f9ed6136..d613309fe 100644 --- a/cmd/helm/testdata/output/version.txt +++ b/cmd/helm/testdata/output/version.txt @@ -1 +1 @@ -version.BuildInfo{Version:"v3.1", GitCommit:"", GitTreeState:"", GoVersion:""} +version.BuildInfo{Version:"v3.2", GitCommit:"", GitTreeState:"", GoVersion:""} 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/internal/ignore/rules.go b/internal/ignore/rules.go index 9049aff0d..a80923baf 100644 --- a/internal/ignore/rules.go +++ b/internal/ignore/rules.go @@ -18,6 +18,7 @@ package ignore import ( "bufio" + "bytes" "io" "log" "os" @@ -65,8 +66,18 @@ func Parse(file io.Reader) (*Rules, error) { r := &Rules{patterns: []*pattern{}} s := bufio.NewScanner(file) + currentLine := 0 + utf8bom := []byte{0xEF, 0xBB, 0xBF} for s.Scan() { - if err := r.parseRule(s.Text()); err != nil { + scannedBytes := s.Bytes() + // We trim UTF8 BOM + if currentLine == 0 { + scannedBytes = bytes.TrimPrefix(scannedBytes, utf8bom) + } + line := string(scannedBytes) + currentLine++ + + if err := r.parseRule(line); err != nil { return r, err } } diff --git a/internal/version/version.go b/internal/version/version.go index fd0616920..baa65a028 100644 --- a/internal/version/version.go +++ b/internal/version/version.go @@ -30,7 +30,7 @@ var ( // Increment major number for new feature additions and behavioral changes. // Increment minor number for bug fixes and performance enhancements. // Increment patch number for critical fixes to existing releases. - version = "v3.1" + version = "v3.2" // metadata is extra build time data metadata = "" diff --git a/pkg/action/action.go b/pkg/action/action.go index a8437d729..bb9ef5f71 100644 --- a/pkg/action/action.go +++ b/pkg/action/action.go @@ -62,16 +62,17 @@ var ( errInvalidName = errors.New("invalid release name, must match regex ^(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])+$ and the length must not longer than 53") ) -// ValidName is a regular expression for names. +// ValidName is a regular expression for resource names. // // According to the Kubernetes help text, the regular expression it uses is: // -// (([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])? +// [a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)* // -// We modified that. First, we added start and end delimiters. Second, we changed -// the final ? to + to require that the pattern match at least once. This modification -// prevents an empty string from matching. -var ValidName = regexp.MustCompile("^(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])+$") +// This follows the above regular expression (but requires a full string match, not partial). +// +// The Kubernetes documentation is here, though it is not entirely correct: +// https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names +var ValidName = regexp.MustCompile(`^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$`) // Configuration injects the dependencies that all actions share. type Configuration struct { diff --git a/pkg/action/action_test.go b/pkg/action/action_test.go index 36ef261a3..0cbdb162b 100644 --- a/pkg/action/action_test.go +++ b/pkg/action/action_test.go @@ -316,3 +316,40 @@ func TestGetVersionSet(t *testing.T) { t.Error("Non-existent version is reported found.") } } + +// TestValidName is a regression test for ValidName +// +// Kubernetes has strict naming conventions for resource names. This test represents +// those conventions. +// +// See https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names +// +// NOTE: At the time of this writing, the docs above say that names cannot begin with +// digits. However, `kubectl`'s regular expression explicit allows this, and +// Kubernetes (at least as of 1.18) also accepts resources whose names begin with digits. +func TestValidName(t *testing.T) { + names := map[string]bool{ + "": false, + "foo": true, + "foo.bar1234baz.seventyone": true, + "FOO": false, + "123baz": true, + "foo.BAR.baz": false, + "one-two": true, + "-two": false, + "one_two": false, + "a..b": false, + "%^&#$%*@^*@&#^": false, + "example:com": false, + "example%%com": false, + } + for input, expectPass := range names { + if ValidName.MatchString(input) != expectPass { + st := "fail" + if expectPass { + st = "succeed" + } + t.Errorf("Expected %q to %s", input, st) + } + } +} diff --git a/pkg/chart/chart_test.go b/pkg/chart/chart_test.go index 1b8669ac8..ef8cec3ad 100644 --- a/pkg/chart/chart_test.go +++ b/pkg/chart/chart_test.go @@ -159,3 +159,53 @@ func TestChartFullPath(t *testing.T) { is.Equal("foo/charts/", chrt1.ChartFullPath()) is.Equal("foo", chrt2.ChartFullPath()) } + +func TestCRDObjects(t *testing.T) { + chrt := Chart{ + Files: []*File{ + { + Name: "crds/foo.yaml", + Data: []byte("hello"), + }, + { + Name: "bar.yaml", + Data: []byte("hello"), + }, + { + Name: "crds/foo/bar/baz.yaml", + Data: []byte("hello"), + }, + { + Name: "crdsfoo/bar/baz.yaml", + Data: []byte("hello"), + }, + { + Name: "crds/README.md", + Data: []byte("# hello"), + }, + }, + } + + expected := []CRD{ + { + Name: "crds/foo.yaml", + Filename: "crds/foo.yaml", + File: &File{ + Name: "crds/foo.yaml", + Data: []byte("hello"), + }, + }, + { + Name: "crds/foo/bar/baz.yaml", + Filename: "crds/foo/bar/baz.yaml", + File: &File{ + Name: "crds/foo/bar/baz.yaml", + Data: []byte("hello"), + }, + }, + } + + is := assert.New(t) + crds := chrt.CRDObjects() + is.Equal(expected, crds) +} diff --git a/pkg/chart/loader/archive.go b/pkg/chart/loader/archive.go index 7e187a170..8b38cb89f 100644 --- a/pkg/chart/loader/archive.go +++ b/pkg/chart/loader/archive.go @@ -173,7 +173,9 @@ func LoadArchiveFiles(in io.Reader) ([]*BufferedFile, error) { return nil, err } - files = append(files, &BufferedFile{Name: n, Data: b.Bytes()}) + data := bytes.TrimPrefix(b.Bytes(), utf8bom) + + files = append(files, &BufferedFile{Name: n, Data: data}) b.Reset() } diff --git a/pkg/chart/loader/directory.go b/pkg/chart/loader/directory.go index a12c5158e..bbe543870 100644 --- a/pkg/chart/loader/directory.go +++ b/pkg/chart/loader/directory.go @@ -17,6 +17,7 @@ limitations under the License. package loader import ( + "bytes" "fmt" "io/ioutil" "os" @@ -30,6 +31,8 @@ import ( "helm.sh/helm/v3/pkg/chart" ) +var utf8bom = []byte{0xEF, 0xBB, 0xBF} + // DirLoader loads a chart from a directory type DirLoader string @@ -104,6 +107,8 @@ func LoadDir(dir string) (*chart.Chart, error) { return errors.Wrapf(err, "error reading %s", n) } + data = bytes.TrimPrefix(data, utf8bom) + files = append(files, &BufferedFile{Name: n, Data: data}) return nil } diff --git a/pkg/chart/loader/load_test.go b/pkg/chart/loader/load_test.go index 26513d359..40b86dec2 100644 --- a/pkg/chart/loader/load_test.go +++ b/pkg/chart/loader/load_test.go @@ -20,6 +20,7 @@ import ( "archive/tar" "bytes" "compress/gzip" + "io" "io/ioutil" "os" "path/filepath" @@ -85,6 +86,86 @@ func TestLoadDirWithSymlink(t *testing.T) { verifyDependenciesLock(t, c) } +func TestBomTestData(t *testing.T) { + testFiles := []string{"frobnitz_with_bom/.helmignore", "frobnitz_with_bom/templates/template.tpl", "frobnitz_with_bom/Chart.yaml"} + for _, file := range testFiles { + data, err := ioutil.ReadFile("testdata/" + file) + if err != nil || !bytes.HasPrefix(data, utf8bom) { + t.Errorf("Test file has no BOM or is invalid: testdata/%s", file) + } + } + + archive, err := ioutil.ReadFile("testdata/frobnitz_with_bom.tgz") + if err != nil { + t.Fatalf("Error reading archive frobnitz_with_bom.tgz: %s", err) + } + unzipped, err := gzip.NewReader(bytes.NewReader(archive)) + if err != nil { + t.Fatalf("Error reading archive frobnitz_with_bom.tgz: %s", err) + } + defer unzipped.Close() + for _, testFile := range testFiles { + data := make([]byte, 3) + err := unzipped.Reset(bytes.NewReader(archive)) + if err != nil { + t.Fatalf("Error reading archive frobnitz_with_bom.tgz: %s", err) + } + tr := tar.NewReader(unzipped) + for { + file, err := tr.Next() + if err == io.EOF { + break + } + if err != nil { + t.Fatalf("Error reading archive frobnitz_with_bom.tgz: %s", err) + } + if file != nil && strings.EqualFold(file.Name, testFile) { + _, err := tr.Read(data) + if err != nil { + t.Fatalf("Error reading archive frobnitz_with_bom.tgz: %s", err) + } else { + break + } + } + } + if !bytes.Equal(data, utf8bom) { + t.Fatalf("Test file has no BOM or is invalid: frobnitz_with_bom.tgz/%s", testFile) + } + } +} + +func TestLoadDirWithUTFBOM(t *testing.T) { + l, err := Loader("testdata/frobnitz_with_bom") + if err != nil { + t.Fatalf("Failed to load testdata: %s", err) + } + c, err := l.Load() + if err != nil { + t.Fatalf("Failed to load testdata: %s", err) + } + verifyFrobnitz(t, c) + verifyChart(t, c) + verifyDependencies(t, c) + verifyDependenciesLock(t, c) + verifyBomStripped(t, c.Files) +} + +func TestLoadArchiveWithUTFBOM(t *testing.T) { + l, err := Loader("testdata/frobnitz_with_bom.tgz") + if err != nil { + t.Fatalf("Failed to load testdata: %s", err) + } + c, err := l.Load() + if err != nil { + t.Fatalf("Failed to load testdata: %s", err) + } + verifyFrobnitz(t, c) + verifyChart(t, c) + verifyDependencies(t, c) + verifyDependenciesLock(t, c) + verifyBomStripped(t, c.Files) +} + func TestLoadV1(t *testing.T) { l, err := Loader("testdata/frobnitz.v1") if err != nil { @@ -465,3 +546,11 @@ func verifyChartFileAndTemplate(t *testing.T, c *chart.Chart, name string) { } } } + +func verifyBomStripped(t *testing.T, files []*chart.File) { + for _, file := range files { + if bytes.HasPrefix(file.Data, utf8bom) { + t.Errorf("Byte Order Mark still present in processed file %s", file.Name) + } + } +} diff --git a/pkg/chart/loader/testdata/frobnitz_with_bom.tgz b/pkg/chart/loader/testdata/frobnitz_with_bom.tgz new file mode 100644 index 000000000..be0cd027d Binary files /dev/null and b/pkg/chart/loader/testdata/frobnitz_with_bom.tgz differ diff --git a/pkg/chart/loader/testdata/frobnitz_with_bom/.helmignore b/pkg/chart/loader/testdata/frobnitz_with_bom/.helmignore new file mode 100644 index 000000000..7a4b92da2 --- /dev/null +++ b/pkg/chart/loader/testdata/frobnitz_with_bom/.helmignore @@ -0,0 +1 @@ +ignore/ diff --git a/pkg/chart/loader/testdata/frobnitz_with_bom/Chart.lock b/pkg/chart/loader/testdata/frobnitz_with_bom/Chart.lock new file mode 100644 index 000000000..ed43b227f --- /dev/null +++ b/pkg/chart/loader/testdata/frobnitz_with_bom/Chart.lock @@ -0,0 +1,8 @@ +dependencies: + - name: alpine + version: "0.1.0" + repository: https://example.com/charts + - name: mariner + version: "4.3.2" + repository: https://example.com/charts +digest: invalid diff --git a/pkg/chart/loader/testdata/frobnitz_with_bom/Chart.yaml b/pkg/chart/loader/testdata/frobnitz_with_bom/Chart.yaml new file mode 100644 index 000000000..21b21f0b5 --- /dev/null +++ b/pkg/chart/loader/testdata/frobnitz_with_bom/Chart.yaml @@ -0,0 +1,27 @@ +apiVersion: v1 +name: frobnitz +description: This is a frobnitz. +version: "1.2.3" +keywords: + - frobnitz + - sprocket + - dodad +maintainers: + - name: The Helm Team + email: helm@example.com + - name: Someone Else + email: nobody@example.com +sources: + - https://example.com/foo/bar +home: http://example.com +icon: https://example.com/64x64.png +annotations: + extrakey: extravalue + anotherkey: anothervalue +dependencies: + - name: alpine + version: "0.1.0" + repository: https://example.com/charts + - name: mariner + version: "4.3.2" + repository: https://example.com/charts diff --git a/pkg/chart/loader/testdata/frobnitz_with_bom/INSTALL.txt b/pkg/chart/loader/testdata/frobnitz_with_bom/INSTALL.txt new file mode 100644 index 000000000..77c4e724a --- /dev/null +++ b/pkg/chart/loader/testdata/frobnitz_with_bom/INSTALL.txt @@ -0,0 +1 @@ +This is an install document. The client may display this. diff --git a/pkg/chart/loader/testdata/frobnitz_with_bom/LICENSE b/pkg/chart/loader/testdata/frobnitz_with_bom/LICENSE new file mode 100644 index 000000000..c27b00bf2 --- /dev/null +++ b/pkg/chart/loader/testdata/frobnitz_with_bom/LICENSE @@ -0,0 +1 @@ +LICENSE placeholder. diff --git a/pkg/chart/loader/testdata/frobnitz_with_bom/README.md b/pkg/chart/loader/testdata/frobnitz_with_bom/README.md new file mode 100644 index 000000000..e9c40031b --- /dev/null +++ b/pkg/chart/loader/testdata/frobnitz_with_bom/README.md @@ -0,0 +1,11 @@ +# Frobnitz + +This is an example chart. + +## Usage + +This is an example. It has no usage. + +## Development + +For developer info, see the top-level repository. diff --git a/pkg/chart/loader/testdata/frobnitz_with_bom/charts/_ignore_me b/pkg/chart/loader/testdata/frobnitz_with_bom/charts/_ignore_me new file mode 100644 index 000000000..a7e3a38b7 --- /dev/null +++ b/pkg/chart/loader/testdata/frobnitz_with_bom/charts/_ignore_me @@ -0,0 +1 @@ +This should be ignored by the loader, but may be included in a chart. diff --git a/pkg/chart/loader/testdata/frobnitz_with_bom/charts/alpine/Chart.yaml b/pkg/chart/loader/testdata/frobnitz_with_bom/charts/alpine/Chart.yaml new file mode 100644 index 000000000..adb9853c6 --- /dev/null +++ b/pkg/chart/loader/testdata/frobnitz_with_bom/charts/alpine/Chart.yaml @@ -0,0 +1,5 @@ +apiVersion: v1 +name: alpine +description: Deploy a basic Alpine Linux pod +version: 0.1.0 +home: https://helm.sh/helm diff --git a/pkg/chart/loader/testdata/frobnitz_with_bom/charts/alpine/README.md b/pkg/chart/loader/testdata/frobnitz_with_bom/charts/alpine/README.md new file mode 100644 index 000000000..ea7526bee --- /dev/null +++ b/pkg/chart/loader/testdata/frobnitz_with_bom/charts/alpine/README.md @@ -0,0 +1,9 @@ +This example was generated using the command `helm create alpine`. + +The `templates/` directory contains a very simple pod resource with a +couple of parameters. + +The `values.toml` file contains the default values for the +`alpine-pod.yaml` template. + +You can install this example using `helm install ./alpine`. diff --git a/pkg/chart/loader/testdata/frobnitz_with_bom/charts/alpine/charts/mast1/Chart.yaml b/pkg/chart/loader/testdata/frobnitz_with_bom/charts/alpine/charts/mast1/Chart.yaml new file mode 100644 index 000000000..1ad84b346 --- /dev/null +++ b/pkg/chart/loader/testdata/frobnitz_with_bom/charts/alpine/charts/mast1/Chart.yaml @@ -0,0 +1,5 @@ +apiVersion: v1 +name: mast1 +description: A Helm chart for Kubernetes +version: 0.1.0 +home: "" diff --git a/pkg/chart/loader/testdata/frobnitz_with_bom/charts/alpine/charts/mast1/values.yaml b/pkg/chart/loader/testdata/frobnitz_with_bom/charts/alpine/charts/mast1/values.yaml new file mode 100644 index 000000000..f690d53c4 --- /dev/null +++ b/pkg/chart/loader/testdata/frobnitz_with_bom/charts/alpine/charts/mast1/values.yaml @@ -0,0 +1,4 @@ +# Default values for mast1. +# This is a YAML-formatted file. +# Declare name/value pairs to be passed into your templates. +# name = "value" diff --git a/pkg/chart/loader/testdata/frobnitz_with_bom/charts/alpine/charts/mast2-0.1.0.tgz b/pkg/chart/loader/testdata/frobnitz_with_bom/charts/alpine/charts/mast2-0.1.0.tgz new file mode 100644 index 000000000..61cb62051 Binary files /dev/null and b/pkg/chart/loader/testdata/frobnitz_with_bom/charts/alpine/charts/mast2-0.1.0.tgz differ diff --git a/pkg/chart/loader/testdata/frobnitz_with_bom/charts/alpine/templates/alpine-pod.yaml b/pkg/chart/loader/testdata/frobnitz_with_bom/charts/alpine/templates/alpine-pod.yaml new file mode 100644 index 000000000..f3e662a28 --- /dev/null +++ b/pkg/chart/loader/testdata/frobnitz_with_bom/charts/alpine/templates/alpine-pod.yaml @@ -0,0 +1,14 @@ +apiVersion: v1 +kind: Pod +metadata: + name: {{.Release.Name}}-{{.Chart.Name}} + labels: + app.kubernetes.io/managed-by: {{.Release.Service}} + app.kubernetes.io/name: {{.Chart.Name}} + helm.sh/chart: "{{.Chart.Name}}-{{.Chart.Version}}" +spec: + restartPolicy: {{default "Never" .restart_policy}} + containers: + - name: waiter + image: "alpine:3.9" + command: ["/bin/sleep","9000"] diff --git a/pkg/chart/loader/testdata/frobnitz_with_bom/charts/alpine/values.yaml b/pkg/chart/loader/testdata/frobnitz_with_bom/charts/alpine/values.yaml new file mode 100644 index 000000000..6b7cb2596 --- /dev/null +++ b/pkg/chart/loader/testdata/frobnitz_with_bom/charts/alpine/values.yaml @@ -0,0 +1,2 @@ +# The pod name +name: "my-alpine" diff --git a/pkg/chart/loader/testdata/frobnitz_with_bom/charts/mariner-4.3.2.tgz b/pkg/chart/loader/testdata/frobnitz_with_bom/charts/mariner-4.3.2.tgz new file mode 100644 index 000000000..3190136b0 Binary files /dev/null and b/pkg/chart/loader/testdata/frobnitz_with_bom/charts/mariner-4.3.2.tgz differ diff --git a/pkg/chart/loader/testdata/frobnitz_with_bom/docs/README.md b/pkg/chart/loader/testdata/frobnitz_with_bom/docs/README.md new file mode 100644 index 000000000..816c3e431 --- /dev/null +++ b/pkg/chart/loader/testdata/frobnitz_with_bom/docs/README.md @@ -0,0 +1 @@ +This is a placeholder for documentation. diff --git a/pkg/chart/loader/testdata/frobnitz_with_bom/icon.svg b/pkg/chart/loader/testdata/frobnitz_with_bom/icon.svg new file mode 100644 index 000000000..892130606 --- /dev/null +++ b/pkg/chart/loader/testdata/frobnitz_with_bom/icon.svg @@ -0,0 +1,8 @@ + + + Example icon + + + diff --git a/pkg/chart/loader/testdata/frobnitz_with_bom/ignore/me.txt b/pkg/chart/loader/testdata/frobnitz_with_bom/ignore/me.txt new file mode 100644 index 000000000..e69de29bb diff --git a/pkg/chart/loader/testdata/frobnitz_with_bom/templates/template.tpl b/pkg/chart/loader/testdata/frobnitz_with_bom/templates/template.tpl new file mode 100644 index 000000000..bb29c5491 --- /dev/null +++ b/pkg/chart/loader/testdata/frobnitz_with_bom/templates/template.tpl @@ -0,0 +1 @@ +Hello {{.Name | default "world"}} diff --git a/pkg/chart/loader/testdata/frobnitz_with_bom/values.yaml b/pkg/chart/loader/testdata/frobnitz_with_bom/values.yaml new file mode 100644 index 000000000..c24ceadf9 --- /dev/null +++ b/pkg/chart/loader/testdata/frobnitz_with_bom/values.yaml @@ -0,0 +1,6 @@ +# A values file contains configuration. + +name: "Some Name" + +section: + name: "Name in a section" 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/chartutil/save.go b/pkg/chartutil/save.go index be5d151d7..1011436b5 100644 --- a/pkg/chartutil/save.go +++ b/pkg/chartutil/save.go @@ -34,6 +34,9 @@ import ( var headerBytes = []byte("+aHR0cHM6Ly95b3V0dS5iZS96OVV6MWljandyTQo=") // SaveDir saves a chart as files in a directory. +// +// This takes the chart name, and creates a new subdirectory inside of the given dest +// directory, writing the chart's contents to that subdirectory. func SaveDir(c *chart.Chart, dest string) error { // Create the chart directory outdir := filepath.Join(dest, c.Name()) diff --git a/pkg/cli/environment.go b/pkg/cli/environment.go index 8e8f4ce8d..d62f57a55 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. @@ -110,42 +114,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/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..c1de2b299 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 } @@ -443,7 +439,7 @@ func updateResource(c *Client, target *resource.Info, currentObj runtime.Object, if err != nil { return errors.Wrap(err, "failed to replace object") } - c.Log("Replaced %q with kind %s for kind %s\n", target.Name, currentObj.GetObjectKind().GroupVersionKind().Kind, kind) + c.Log("Replaced %q with kind %s for kind %s", target.Name, currentObj.GetObjectKind().GroupVersionKind().Kind, kind) } else { // send patch to server obj, err = helper.Patch(target.Namespace, target.Name, patchType, patch, 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/lint/lint_test.go b/pkg/lint/lint_test.go index 2c110009d..e7ff4cd7a 100644 --- a/pkg/lint/lint_test.go +++ b/pkg/lint/lint_test.go @@ -104,7 +104,10 @@ func TestBadValues(t *testing.T) { func TestGoodChart(t *testing.T) { m := All(goodChartDir, values, namespace, strict).Messages if len(m) != 0 { - t.Errorf("All failed but shouldn't have: %#v", m) + t.Error("All returned linter messages when it shouldn't have") + for i, msg := range m { + t.Logf("Message %d: %s", i, msg) + } } } @@ -130,6 +133,9 @@ func TestHelmCreateChart(t *testing.T) { m := All(createdChart, values, namespace, true).Messages if ll := len(m); ll != 1 { t.Errorf("All should have had exactly 1 error. Got %d", ll) + for i, msg := range m { + t.Logf("Message %d: %s", i, msg.Error()) + } } else if msg := m[0].Err.Error(); !strings.Contains(msg, "icon is recommended") { t.Errorf("Unexpected lint error: %s", msg) } diff --git a/pkg/lint/rules/deprecations.go b/pkg/lint/rules/deprecations.go new file mode 100644 index 000000000..c14fedec6 --- /dev/null +++ b/pkg/lint/rules/deprecations.go @@ -0,0 +1,64 @@ +/* +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 rules // import "helm.sh/helm/v3/pkg/lint/rules" + +import "fmt" + +// deprecatedAPIs lists APIs that are deprecated (left) with suggested alternatives (right). +// +// An empty rvalue indicates that the API is completely deprecated. +var deprecatedAPIs = map[string]string{ + "extensions/v1 Deployment": "apps/v1 Deployment", + "extensions/v1 DaemonSet": "apps/v1 DaemonSet", + "extensions/v1 ReplicaSet": "apps/v1 ReplicaSet", + "extensions/v1beta1 PodSecurityPolicy": "policy/v1beta1 PodSecurityPolicy", + "extensions/v1beta1 NetworkPolicy": "networking.k8s.io/v1beta1 NetworkPolicy", + "extensions/v1beta1 Ingress": "networking.k8s.io/v1beta1 Ingress", + "apps/v1beta1 Deployment": "apps/v1 Deployment", + "apps/v1beta1 StatefulSet": "apps/v1 StatefulSet", + "apps/v1beta1 DaemonSet": "apps/v1 DaemonSet", + "apps/v1beta1 ReplicaSet": "apps/v1 ReplicaSet", + "apps/v1beta2 Deployment": "apps/v1 Deployment", + "apps/v1beta2 StatefulSet": "apps/v1 StatefulSet", + "apps/v1beta2 DaemonSet": "apps/v1 DaemonSet", + "apps/v1beta2 ReplicaSet": "apps/v1 ReplicaSet", +} + +// deprecatedAPIError indicates than an API is deprecated in Kubernetes +type deprecatedAPIError struct { + Deprecated string + Alternative string +} + +func (e deprecatedAPIError) Error() string { + msg := fmt.Sprintf("the kind %q is deprecated", e.Deprecated) + if e.Alternative != "" { + msg += fmt.Sprintf(" in favor of %q", e.Alternative) + } + return msg +} + +func validateNoDeprecations(resource *K8sYamlStruct) error { + gvk := fmt.Sprintf("%s %s", resource.APIVersion, resource.Kind) + if alt, ok := deprecatedAPIs[gvk]; ok { + return deprecatedAPIError{ + Deprecated: gvk, + Alternative: alt, + } + } + return nil +} diff --git a/pkg/lint/rules/deprecations_test.go b/pkg/lint/rules/deprecations_test.go new file mode 100644 index 000000000..f85d58a0c --- /dev/null +++ b/pkg/lint/rules/deprecations_test.go @@ -0,0 +1,42 @@ +/* +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 rules // import "helm.sh/helm/v3/pkg/lint/rules" + +import "testing" + +func TestValidateNoDeprecations(t *testing.T) { + deprecated := &K8sYamlStruct{ + APIVersion: "extensions/v1", + Kind: "Deployment", + } + err := validateNoDeprecations(deprecated) + if err == nil { + t.Fatal("Expected deprecated extension to be flagged") + } + + depErr := err.(deprecatedAPIError) + if depErr.Alternative != "apps/v1 Deployment" { + t.Errorf("Expected %q to be replaced by %q", depErr.Deprecated, depErr.Alternative) + } + + if err := validateNoDeprecations(&K8sYamlStruct{ + APIVersion: "v1", + Kind: "Pod", + }); err != nil { + t.Errorf("Expected a v1 Pod to not be deprecated") + } +} diff --git a/pkg/lint/rules/template.go b/pkg/lint/rules/template.go index 3d388f81b..787c5b26a 100644 --- a/pkg/lint/rules/template.go +++ b/pkg/lint/rules/template.go @@ -17,9 +17,11 @@ limitations under the License. package rules import ( + "fmt" "os" "path/filepath" "regexp" + "strings" "github.com/pkg/errors" "sigs.k8s.io/yaml" @@ -35,6 +37,14 @@ var ( releaseTimeSearch = regexp.MustCompile(`\.Release\.Time`) ) +// validName is a regular expression for names. +// +// This is different than action.ValidName. It conforms to the regular expression +// `kubectl` says it uses, plus it disallows empty names. +// +// For details, see https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names +var validName = regexp.MustCompile(`^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$`) + // Templates lints the templates in the Linter. func Templates(linter *support.Linter, values map[string]interface{}, namespace string, strict bool) { path := "templates/" @@ -57,7 +67,7 @@ func Templates(linter *support.Linter, values map[string]interface{}, namespace } options := chartutil.ReleaseOptions{ - Name: "testRelease", + Name: "test-release", Namespace: namespace, } @@ -71,7 +81,6 @@ func Templates(linter *support.Linter, values map[string]interface{}, namespace return } var e engine.Engine - e.Strict = strict e.LintMode = true renderedContentMap, err := e.Render(chart, valuesToRender) @@ -111,14 +120,18 @@ func Templates(linter *support.Linter, values map[string]interface{}, namespace // linter.RunLinterRule(support.WarningSev, path, validateQuotes(string(preExecutedTemplate))) renderedContent := renderedContentMap[filepath.Join(chart.Name(), fileName)] - var yamlStruct K8sYamlStruct - // Even though K8sYamlStruct only defines Metadata namespace, an error in any other - // key will be raised as well - err := yaml.Unmarshal([]byte(renderedContent), &yamlStruct) - - // If YAML linting fails, we sill progress. So we don't capture the returned state - // on this linter run. - linter.RunLinterRule(support.ErrorSev, path, validateYamlContent(err)) + if strings.TrimSpace(renderedContent) != "" { + var yamlStruct K8sYamlStruct + // Even though K8sYamlStruct only defines a few fields, an error in any other + // key will be raised as well + err := yaml.Unmarshal([]byte(renderedContent), &yamlStruct) + + // If YAML linting fails, we sill progress. So we don't capture the returned state + // on this linter run. + linter.RunLinterRule(support.ErrorSev, path, validateYamlContent(err)) + linter.RunLinterRule(support.ErrorSev, path, validateMetadataName(&yamlStruct)) + linter.RunLinterRule(support.ErrorSev, path, validateNoDeprecations(&yamlStruct)) + } } } @@ -149,6 +162,15 @@ func validateYamlContent(err error) error { return errors.Wrap(err, "unable to parse YAML") } +func validateMetadataName(obj *K8sYamlStruct) error { + // This will return an error if the characters do not abide by the standard OR if the + // name is left empty. + if validName.MatchString(obj.Metadata.Name) { + return nil + } + return fmt.Errorf("object name does not conform to Kubernetes naming requirements: %q", obj.Metadata.Name) +} + func validateNoCRDHooks(manifest []byte) error { if crdHookSearch.Match(manifest) { return errors.New("manifest is a crd-install hook. This hook is no longer supported in v3 and all CRDs should also exist the crds/ directory at the top level of the chart") @@ -164,9 +186,16 @@ func validateNoReleaseTime(manifest []byte) error { } // K8sYamlStruct stubs a Kubernetes YAML file. -// Need to access for now to Namespace only +// +// DEPRECATED: In Helm 4, this will be made a private type, as it is for use only within +// the rules package. type K8sYamlStruct struct { - Metadata struct { - Namespace string - } + APIVersion string `json:"apiVersion"` + Kind string + Metadata k8sYamlMetadata +} + +type k8sYamlMetadata struct { + Namespace string + Name string } diff --git a/pkg/lint/rules/template_test.go b/pkg/lint/rules/template_test.go index ddb46aba0..991c6c2f6 100644 --- a/pkg/lint/rules/template_test.go +++ b/pkg/lint/rules/template_test.go @@ -22,6 +22,9 @@ import ( "strings" "testing" + "helm.sh/helm/v3/internal/test/ensure" + "helm.sh/helm/v3/pkg/chart" + "helm.sh/helm/v3/pkg/chartutil" "helm.sh/helm/v3/pkg/lint/support" ) @@ -101,3 +104,125 @@ func TestV3Fail(t *testing.T) { t.Errorf("Unexpected error: %s", res[2].Err) } } + +func TestValidateMetadataName(t *testing.T) { + names := map[string]bool{ + "": false, + "foo": true, + "foo.bar1234baz.seventyone": true, + "FOO": false, + "123baz": true, + "foo.BAR.baz": false, + "one-two": true, + "-two": false, + "one_two": false, + "a..b": false, + "%^&#$%*@^*@&#^": false, + } + for input, expectPass := range names { + obj := K8sYamlStruct{Metadata: k8sYamlMetadata{Name: input}} + if err := validateMetadataName(&obj); (err == nil) != expectPass { + st := "fail" + if expectPass { + st = "succeed" + } + t.Errorf("Expected %q to %s", input, st) + if err != nil { + t.Log(err) + } + } + } +} + +func TestDeprecatedAPIFails(t *testing.T) { + mychart := chart.Chart{ + Metadata: &chart.Metadata{ + APIVersion: "v2", + Name: "failapi", + Version: "0.1.0", + Icon: "satisfy-the-linting-gods.gif", + }, + Templates: []*chart.File{ + { + Name: "templates/baddeployment.yaml", + Data: []byte("apiVersion: apps/v1beta1\nkind: Deployment\nmetadata:\n name: baddep"), + }, + { + Name: "templates/goodsecret.yaml", + Data: []byte("apiVersion: v1\nkind: Secret\nmetadata:\n name: goodsecret"), + }, + }, + } + tmpdir := ensure.TempDir(t) + defer os.RemoveAll(tmpdir) + + if err := chartutil.SaveDir(&mychart, tmpdir); err != nil { + t.Fatal(err) + } + + linter := support.Linter{ChartDir: filepath.Join(tmpdir, mychart.Name())} + Templates(&linter, values, namespace, strict) + if l := len(linter.Messages); l != 1 { + for i, msg := range linter.Messages { + t.Logf("Message %d: %s", i, msg) + } + t.Fatalf("Expected 1 lint error, got %d", l) + } + + err := linter.Messages[0].Err.(deprecatedAPIError) + if err.Deprecated != "apps/v1beta1 Deployment" { + t.Errorf("Surprised to learn that %q is deprecated", err.Deprecated) + } +} + +const manifest = `apiVersion: v1 +kind: ConfigMap +metadata: + name: foo +data: + myval1: {{default "val" .Values.mymap.key1 }} + myval2: {{default "val" .Values.mymap.key2 }} +` + +// TestSTrictTemplatePrasingMapError is a regression test. +// +// The template engine should not produce an error when a map in values.yaml does +// not contain all possible keys. +// +// See https://github.com/helm/helm/issues/7483 +func TestStrictTemplateParsingMapError(t *testing.T) { + + ch := chart.Chart{ + Metadata: &chart.Metadata{ + Name: "regression7483", + APIVersion: "v2", + Version: "0.1.0", + }, + Values: map[string]interface{}{ + "mymap": map[string]string{ + "key1": "val1", + }, + }, + Templates: []*chart.File{ + { + Name: "templates/configmap.yaml", + Data: []byte(manifest), + }, + }, + } + dir := ensure.TempDir(t) + defer os.RemoveAll(dir) + if err := chartutil.SaveDir(&ch, dir); err != nil { + t.Fatal(err) + } + linter := &support.Linter{ + ChartDir: filepath.Join(dir, ch.Metadata.Name), + } + Templates(linter, ch.Values, namespace, strict) + if len(linter.Messages) != 0 { + t.Errorf("expected zero messages, got %d", len(linter.Messages)) + for i, msg := range linter.Messages { + t.Logf("Message %d: %q", i, msg) + } + } +} diff --git a/pkg/lint/rules/testdata/goodone/templates/goodone.yaml b/pkg/lint/rules/testdata/goodone/templates/goodone.yaml index 0e77f46f2..cd46f62c7 100644 --- a/pkg/lint/rules/testdata/goodone/templates/goodone.yaml +++ b/pkg/lint/rules/testdata/goodone/templates/goodone.yaml @@ -1,2 +1,2 @@ metadata: - name: {{.Values.name | default "foo" | title}} + name: {{ .Values.name | default "foo" | lower }} diff --git a/pkg/lint/rules/testdata/goodone/values.yaml b/pkg/lint/rules/testdata/goodone/values.yaml index fe9abd983..92c3d9bb9 100644 --- a/pkg/lint/rules/testdata/goodone/values.yaml +++ b/pkg/lint/rules/testdata/goodone/values.yaml @@ -1 +1 @@ -name: "goodone here" +name: "goodone-here" diff --git a/pkg/plugin/plugin_test.go b/pkg/plugin/plugin_test.go index a869255c4..af0b61846 100644 --- a/pkg/plugin/plugin_test.go +++ b/pkg/plugin/plugin_test.go @@ -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") + } +}