diff --git a/cmd/helm/create.go b/cmd/helm/create.go index b79752efb..52acb6670 100644 --- a/cmd/helm/create.go +++ b/cmd/helm/create.go @@ -21,10 +21,10 @@ import ( "io" "path/filepath" + "github.com/pkg/errors" "github.com/spf13/cobra" "helm.sh/helm/v3/cmd/helm/require" - "helm.sh/helm/v3/pkg/chart" "helm.sh/helm/v3/pkg/chartutil" "helm.sh/helm/v3/pkg/helmpath" ) @@ -51,9 +51,10 @@ will be overwritten, but other files will be left alone. ` type createOptions struct { - starter string // --starter - name string - starterDir string + starter string // --starter + name string + starterDir string + keepMetadata bool } func newCreateCmd(out io.Writer) *cobra.Command { @@ -73,6 +74,13 @@ func newCreateCmd(out io.Writer) *cobra.Command { // No more completions, so disable file completion return nil, cobra.ShellCompDirectiveNoFileComp }, + PreRunE: func(cmd *cobra.Command, args []string) error { + if len(o.starter) == 0 && o.keepMetadata { + return errors.New("the -k/--keep-metadata flag can only be specified when using a starter") + } + + return nil + }, RunE: func(_ *cobra.Command, args []string) error { o.name = args[0] o.starterDir = helmpath.DataPath("starters") @@ -81,6 +89,7 @@ func newCreateCmd(out io.Writer) *cobra.Command { } cmd.Flags().StringVarP(&o.starter, "starter", "p", "", "the name or absolute path to Helm starter scaffold") + cmd.Flags().BoolVarP(&o.keepMetadata, "keep-metadata", "k", false, "if specified, does not override the starter's Chart.yaml") return cmd } @@ -88,14 +97,6 @@ func (o *createOptions) run(out io.Writer) error { fmt.Fprintf(out, "Creating %s\n", o.name) chartname := filepath.Base(o.name) - cfile := &chart.Metadata{ - Name: chartname, - Description: "A Helm chart for Kubernetes", - Type: "application", - Version: "0.1.0", - AppVersion: "0.1.0", - APIVersion: chart.APIVersionV2, - } if o.starter != "" { // Create from the starter @@ -104,7 +105,7 @@ func (o *createOptions) run(out io.Writer) error { if filepath.IsAbs(o.starter) { lstarter = o.starter } - return chartutil.CreateFrom(cfile, filepath.Dir(o.name), lstarter) + return chartutil.CreateFrom(chartname, filepath.Dir(chartname), lstarter, o.keepMetadata) } chartutil.Stderr = out diff --git a/pkg/chartutil/create.go b/pkg/chartutil/create.go index 50212f9d5..6f1c65437 100644 --- a/pkg/chartutil/create.go +++ b/pkg/chartutil/create.go @@ -25,6 +25,7 @@ import ( "strings" "github.com/pkg/errors" + "k8s.io/utils/strings/slices" "sigs.k8s.io/yaml" "helm.sh/helm/v3/pkg/chart" @@ -547,13 +548,15 @@ spec: var Stderr io.Writer = os.Stderr // CreateFrom creates a new chart, but scaffolds it from the src chart. -func CreateFrom(chartfile *chart.Metadata, dest, src string) error { +func CreateFrom(name, dest, src string, keepMetadata bool) error { schart, err := loader.Load(src) if err != nil { return errors.Wrapf(err, "could not load %s", src) } - schart.Metadata = chartfile + if err = setMetadata(schart, name, keepMetadata); err != nil { + return errors.Wrap(err, "could not set metadata") + } var updatedTemplates []*chart.File @@ -574,12 +577,12 @@ func CreateFrom(chartfile *chart.Metadata, dest, src string) error { } schart.Values = m - // SaveDir looks for the file values.yaml when saving rather than the values - // key in order to preserve the comments in the YAML. The name placeholder - // needs to be replaced on that file. + // SaveDir looks for the files values.yaml and Chart.yaml when saving rather than the values + // and metadata keys in order to preserve the comments in the YAML. The name placeholder + // needs to be replaced on these files. for _, f := range schart.Raw { - if f.Name == ValuesfileName { - f.Data = transform(string(f.Data), schart.Name()) + if slices.Contains([]string{ValuesfileName, ChartfileName}, f.Name) { + f.Data = transform(string(f.Data), name) } } @@ -721,3 +724,23 @@ func validateChartName(name string) error { } return nil } + +func setMetadata(schart *chart.Chart, name string, keepMetadata bool) error { + for _, f := range schart.Raw { + if f.Name == ChartfileName { + // Overwrite Metadata only if flag was not set by user to keep backwards compatibility. + if !keepMetadata { + f.Data = []byte(fmt.Sprintf(defaultChartfile, name)) + } + + // Deserialize Metadata and check for errors + var m chart.Metadata + if err := yaml.Unmarshal(transform(string(f.Data), name), &m); err != nil { + return errors.Wrap(err, "transforming charts file") + } + schart.Metadata = &m + } + } + + return nil +} diff --git a/pkg/chartutil/create_test.go b/pkg/chartutil/create_test.go index 1697c4218..22e490c91 100644 --- a/pkg/chartutil/create_test.go +++ b/pkg/chartutil/create_test.go @@ -20,9 +20,9 @@ import ( "bytes" "os" "path/filepath" + "strings" "testing" - "helm.sh/helm/v3/pkg/chart" "helm.sh/helm/v3/pkg/chart/loader" ) @@ -67,19 +67,15 @@ func TestCreate(t *testing.T) { func TestCreateFrom(t *testing.T) { tdir := t.TempDir() - cf := &chart.Metadata{ - APIVersion: chart.APIVersionV1, - Name: "foo", - Version: "0.1.0", - } + chartname := "foo" srcdir := "./testdata/frobnitz/charts/mariner" - if err := CreateFrom(cf, tdir, srcdir); err != nil { + if err := CreateFrom(chartname, tdir, srcdir, false); err != nil { t.Fatal(err) } dir := filepath.Join(tdir, "foo") - c := filepath.Join(tdir, cf.Name) + c := filepath.Join(tdir, chartname) mychart, err := loader.LoadDir(c) if err != nil { t.Fatalf("Failed to load newly created chart %q: %s", c, err) @@ -109,6 +105,58 @@ func TestCreateFrom(t *testing.T) { } } +func TestCreateFromKeepMetadata(t *testing.T) { + tdir := t.TempDir() + + chartname := "foo" + srcdir := "./testdata/frobnitz/charts/custom" + srcMetadata := filepath.Join(srcdir, ChartfileName) + rawMetadata, err := ioutil.ReadFile(srcMetadata) + if err != nil { + t.Errorf("Unable to read file %s: %s", srcMetadata, err) + } + + if err := CreateFrom(chartname, tdir, srcdir, true); err != nil { + t.Fatal(err) + } + + dir := filepath.Join(tdir, chartname) + mychart, err := loader.LoadDir(dir) + if err != nil { + t.Fatalf("Failed to load newly created chart %q: %s", dir, err) + } + + if mychart.Name() != "foo" { + t.Errorf("Expected name to be 'foo', got %q", mychart.Name()) + } + + expectedMetadata := strings.ReplaceAll(string(rawMetadata), "", chartname) + for _, f := range []string{ + ChartfileName, + ValuesfileName, + filepath.Join(TemplatesDir, "deployment.yaml"), + } { + if _, err := os.Stat(filepath.Join(dir, f)); err != nil { + t.Errorf("Expected %s file: %s", f, err) + } + + // Check each file to make sure has been replaced + b, err := ioutil.ReadFile(filepath.Join(dir, f)) + if err != nil { + t.Errorf("Unable to read file %s: %s", f, err) + } + if bytes.Contains(b, []byte("")) { + t.Errorf("File %s contains ", f) + } + + if f == ChartfileName { + if string(b) != expectedMetadata { + t.Errorf("%s does not contain expected content", ChartfileName) + } + } + } +} + // TestCreate_Overwrite is a regression test for making sure that files are overwritten. func TestCreate_Overwrite(t *testing.T) { tdir := t.TempDir() diff --git a/pkg/chartutil/save.go b/pkg/chartutil/save.go index 4ee90709c..8e3753549 100644 --- a/pkg/chartutil/save.go +++ b/pkg/chartutil/save.go @@ -26,6 +26,7 @@ import ( "time" "github.com/pkg/errors" + "k8s.io/utils/strings/slices" "sigs.k8s.io/yaml" "helm.sh/helm/v3/pkg/chart" @@ -51,15 +52,28 @@ func SaveDir(c *chart.Chart, dest string) error { return err } - // Save the chart file. - if err := SaveChartfile(filepath.Join(outdir, ChartfileName), c.Metadata); err != nil { - return err + // Check if Chart has raw metadata stored + hasRawMetadata := false + for _, f := range c.Raw { + if f.Name == ChartfileName { + hasRawMetadata = true + } } - // Save values.yaml + // This ensures that the Chart always has raw metadata stored, since some tests + // call SaveDir without adding raw metadata to the Chart. + if !hasRawMetadata { + b, err := yaml.Marshal(c.Metadata) + if err != nil { + return errors.Wrap(err, "reading charts file") + } + c.Raw = append(c.Raw, &chart.File{Name: ChartfileName, Data: b}) + } + + // Save values.yaml and Chart.yaml for _, f := range c.Raw { - if f.Name == ValuesfileName { - vf := filepath.Join(outdir, ValuesfileName) + if slices.Contains([]string{ValuesfileName, ChartfileName}, f.Name) { + vf := filepath.Join(outdir, f.Name) if err := writeFile(vf, f.Data); err != nil { return err } diff --git a/pkg/chartutil/testdata/frobnitz/charts/custom/Chart.yaml b/pkg/chartutil/testdata/frobnitz/charts/custom/Chart.yaml new file mode 100644 index 000000000..8371ebefc --- /dev/null +++ b/pkg/chartutil/testdata/frobnitz/charts/custom/Chart.yaml @@ -0,0 +1,29 @@ +apiVersion: v2 +name: +description: A Helm chart for Kubernetes + +# A chart can be either an 'application' or a 'library' chart. +# +# Application charts are a collection of templates that can be packaged into versioned archives +# to be deployed. +# +# Library charts provide useful utilities or functions for the chart developer. They're included as +# a dependency of application charts to inject those utilities and functions into the rendering +# pipeline. Library charts do not define any templates and therefore cannot be deployed. +type: application + +# This is the chart version. This version number should be incremented each time you make changes +# to the chart and its templates, including the app version. +# Versions are expected to follow Semantic Versioning (https://semver.org/) +version: 0.1.0 + +# This is the version number of the application being deployed. This version number should be +# incremented each time you make changes to the application. Versions are not expected to +# follow Semantic Versioning. They should reflect the version the application is using. +# It is recommended to use it with quotes. +appVersion: "1.16.0" + +dependencies: + - name: albatross + repository: https://example.com/mariner/charts + version: "0.1.0" diff --git a/pkg/chartutil/testdata/frobnitz/charts/custom/templates/deployment.yaml b/pkg/chartutil/testdata/frobnitz/charts/custom/templates/deployment.yaml new file mode 100644 index 000000000..2de3173fe --- /dev/null +++ b/pkg/chartutil/testdata/frobnitz/charts/custom/templates/deployment.yaml @@ -0,0 +1,24 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: {{ include ".fullname" . }} + labels: + {{- include ".labels" . | nindent 4 }} +spec: + replicas: {{ .Values.replicaCount }} + selector: + matchLabels: + {{- include ".selectorLabels" . | nindent 6 }} + template: + metadata: + labels: + {{- include ".selectorLabels" . | nindent 8 }} + spec: + containers: + - name: {{ .Chart.Name }} + image: "{{ .Values.image.repository }}:{{ .Values.image.tag | default .Chart.AppVersion }}" + imagePullPolicy: {{ .Values.image.pullPolicy }} + ports: + - name: http + containerPort: {{ .Values.service.port }} + protocol: TCP diff --git a/pkg/chartutil/testdata/frobnitz/charts/custom/values.yaml b/pkg/chartutil/testdata/frobnitz/charts/custom/values.yaml new file mode 100644 index 000000000..63975d498 --- /dev/null +++ b/pkg/chartutil/testdata/frobnitz/charts/custom/values.yaml @@ -0,0 +1,22 @@ +# Default values for . +# This is a YAML-formatted file. https://github.com/toml-lang/toml +# Declare name/value pairs to be passed into your templates. +# name: "value" + +: + test: true + +replicaCount: 1 + +image: + repository: "" + pullPolicy: IfNotPresent + tag: latest + +imagePullSecrets: [] +nameOverride: "" +fullnameOverride: "" + +service: + type: ClusterIP + port: 80