feat(starter): enable preserving custom Chart.yaml using --keep-metadata flag

Fixes helm/helm#4745
Fixes helm/helm#4060
Fixes helm/helm#7566
Fixes helm/helm#9551
Fixes helm/helm#10057

Signed-off-by: Filip Krakowski <krakowski@hhu.de>

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

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
+}

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>", 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 <CHARTNAME> 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("<CHARTNAME>")) {
+			t.Errorf("File %s contains <CHARTNAME>", 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()

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
 			}

pkg/chartutil/testdata/frobnitz/charts/custom/Chart.yaml

@@ -0,0 +1,29 @@
+apiVersion: v2
+name: <CHARTNAME>
+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"

pkg/chartutil/testdata/frobnitz/charts/custom/templates/deployment.yaml

@@ -0,0 +1,24 @@
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: {{ include "<CHARTNAME>.fullname" . }}
+  labels:
+    {{- include "<CHARTNAME>.labels" . | nindent 4 }}
+spec:
+  replicas: {{ .Values.replicaCount }}
+  selector:
+    matchLabels:
+      {{- include "<CHARTNAME>.selectorLabels" . | nindent 6 }}
+  template:
+    metadata:
+      labels:
+        {{- include "<CHARTNAME>.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

pkg/chartutil/testdata/frobnitz/charts/custom/values.yaml

@@ -0,0 +1,22 @@
+# Default values for <CHARTNAME>.
+# This is a YAML-formatted file. https://github.com/toml-lang/toml
+# Declare name/value pairs to be passed into your templates.
+# name: "value"
+
+<CHARTNAME>:
+  test: true
+
+replicaCount: 1
+
+image:
+  repository: ""
+  pullPolicy: IfNotPresent
+  tag: latest
+
+imagePullSecrets: []
+nameOverride: ""
+fullnameOverride: ""
+
+service:
+  type: ClusterIP
+  port: 80