ref(action): remove ParseReferenceWithChartDefaults

Signed-off-by: Matthew Fisher <matt.fisher@microsoft.com>

cmd/helm/chart_save.go

@@ -18,11 +18,13 @@ package main
 
 import (
 	"io"
+	"path/filepath"
 
 	"github.com/spf13/cobra"
 
 	"helm.sh/helm/cmd/helm/require"
 	"helm.sh/helm/pkg/action"
+	"helm.sh/helm/pkg/chart/loader"
 )
 
 const chartSaveDesc = `
@@ -41,7 +43,18 @@ func newChartSaveCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
 		RunE: func(cmd *cobra.Command, args []string) error {
 			path := args[0]
 			ref := args[1]
-			return action.NewChartSave(cfg).Run(out, path, ref)
+
+			path, err := filepath.Abs(path)
+			if err != nil {
+				return err
+			}
+
+			ch, err := loader.LoadDir(path)
+			if err != nil {
+				return err
+			}
+
+			return action.NewChartSave(cfg).Run(out, ch, ref)
 		},
 	}
 }

pkg/action/action_test.go

@@ -16,16 +16,19 @@ limitations under the License.
 package action
 
 import (
+	"context"
 	"flag"
 	"io/ioutil"
 	"testing"
 	"time"
 
+	dockerauth "github.com/deislabs/oras/pkg/auth/docker"
 	fakeclientset "k8s.io/client-go/kubernetes/fake"
 
 	"helm.sh/helm/pkg/chart"
 	"helm.sh/helm/pkg/chartutil"
 	kubefake "helm.sh/helm/pkg/kube/fake"
+	"helm.sh/helm/pkg/registry"
 	"helm.sh/helm/pkg/release"
 	"helm.sh/helm/pkg/storage"
 	"helm.sh/helm/pkg/storage/driver"
@@ -36,10 +39,35 @@ var verbose = flag.Bool("test.log", false, "enable test logging")
 func actionConfigFixture(t *testing.T) *Configuration {
 	t.Helper()
 
+	client, err := dockerauth.NewClient()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	resolver, err := client.Resolver(context.Background())
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	tdir, err := ioutil.TempDir("", "helm-action-test")
+	if err != nil {
+		t.Fatal(err)
+	}
+
 	return &Configuration{
 		Releases:     storage.Init(driver.NewMemory()),
 		KubeClient:   &kubefake.FailingKubeClient{PrintingKubeClient: kubefake.PrintingKubeClient{Out: ioutil.Discard}},
 		Capabilities: chartutil.DefaultCapabilities,
+		RegistryClient: registry.NewClient(&registry.ClientOptions{
+			Out: ioutil.Discard,
+			Authorizer: registry.Authorizer{
+				Client: client,
+			},
+			Resolver: registry.Resolver{
+				Resolver: resolver,
+			},
+			CacheRootDir: tdir,
+		}),
 		Log: func(format string, v ...interface{}) {
 			t.Helper()
 			if *verbose {
@@ -106,7 +134,9 @@ func buildChart(opts ...chartOption) *chart.Chart {
 		Chart: &chart.Chart{
 			// TODO: This should be more complete.
 			Metadata: &chart.Metadata{
+				APIVersion: "v1",
 				Name:       "hello",
+				Version:    "0.1.0",
 			},
 			// This adds a basic template and hooks.
 			Templates: []*chart.File{

pkg/action/chart_save.go

@@ -18,9 +18,8 @@ package action
 
 import (
 	"io"
-	"path/filepath"
 
-	"helm.sh/helm/pkg/chart/loader"
+	"helm.sh/helm/pkg/chart"
 	"helm.sh/helm/pkg/registry"
 )
 
@@ -37,20 +36,16 @@ func NewChartSave(cfg *Configuration) *ChartSave {
 }
 
 // Run executes the chart save operation
-func (a *ChartSave) Run(out io.Writer, path, ref string) error {
-	path, err := filepath.Abs(path)
+func (a *ChartSave) Run(out io.Writer, ch *chart.Chart, ref string) error {
+	r, err := registry.ParseReference(ref)
 	if err != nil {
 		return err
 	}
 
-	ch, err := loader.LoadDir(path)
-	if err != nil {
-		return err
+	// If no tag is present, use the chart version
+	if r.Tag == "" {
+		r.Tag = ch.Metadata.Version
 	}
 
-	r, err := registry.ParseReferenceWithChartDefaults(ref, ch)
-	if err != nil {
-		return err
-	}
 	return a.cfg.RegistryClient.SaveChart(ch, r)
 }

pkg/action/chart_save_test.go

@@ -0,0 +1,63 @@
+/*
+Copyright The Helm Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package action
+
+import (
+	"io/ioutil"
+	"testing"
+
+	"helm.sh/helm/pkg/registry"
+)
+
+func chartSaveAction(t *testing.T) *ChartSave {
+	t.Helper()
+	config := actionConfigFixture(t)
+	action := NewChartSave(config)
+	return action
+}
+
+func TestChartSave(t *testing.T) {
+	action := chartSaveAction(t)
+
+	input := buildChart()
+	if err := action.Run(ioutil.Discard, input, "localhost:5000/test:0.2.0"); err != nil {
+		t.Error(err)
+	}
+
+	ref, err := registry.ParseReference("localhost:5000/test:0.2.0")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if _, err := action.cfg.RegistryClient.LoadChart(ref); err != nil {
+		t.Error(err)
+	}
+
+	// now let's check if `helm chart save` can use the chart version when the tag is not present
+	if err := action.Run(ioutil.Discard, input, "localhost:5000/test"); err != nil {
+		t.Error(err)
+	}
+
+	ref, err = registry.ParseReference("localhost:5000/test:0.1.0")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if _, err := action.cfg.RegistryClient.LoadChart(ref); err != nil {
+		t.Error(err)
+	}
+}

pkg/registry/reference.go

@@ -22,8 +22,6 @@ import (
 	"strings"
 
 	"github.com/containerd/containerd/reference"
-
-	"helm.sh/helm/pkg/chart"
 )
 
 var (
@@ -61,22 +59,6 @@ func ParseReference(s string) (*Reference, error) {
 	return &ref, nil
 }
 
-// ParseReferenceWithChartDefaults converts a string to a Reference,
-// using values from a given chart as defaults
-func ParseReferenceWithChartDefaults(s string, ch *chart.Chart) (*Reference, error) {
-	ref, err := ParseReference(s)
-	if err != nil {
-		return nil, err
-	}
-
-	// If no tag is present, use the chart version
-	if ref.Tag == "" {
-		ref.Tag = ch.Metadata.Version
-	}
-
-	return ref, nil
-}
-
 // setExtraFields adds the Repo and Tag fields to a Reference
 func (ref *Reference) setExtraFields() {
 	ref.Tag = ref.Object

pkg/registry/reference_test.go

@@ -20,8 +20,6 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
-
-	"helm.sh/helm/pkg/chart"
 )
 
 func TestParseReference(t *testing.T) {
@@ -89,28 +87,3 @@ func TestParseReference(t *testing.T) {
 	is.Equal("my.host.com/my/nested/repo", ref.Repo)
 	is.Equal("1.2.3", ref.Tag)
 }
-
-func TestParseReferenceWithChartDefaults(t *testing.T) {
-	is := assert.New(t)
-
-	ch := &chart.Chart{
-		Metadata: &chart.Metadata{
-			Name:    "mychart",
-			Version: "1.5.0",
-		},
-	}
-
-	// If tag provided, use tag (1.2.3)
-	s := "my.host.com/my/nested/repo:1.2.3"
-	ref, err := ParseReferenceWithChartDefaults(s, ch)
-	is.NoError(err)
-	is.Equal("my.host.com/my/nested/repo", ref.Repo)
-	is.Equal("1.2.3", ref.Tag)
-
-	// If tag NOT provided, use version from chart (1.5.0)
-	s = "my.host.com/my/nested/repo"
-	ref, err = ParseReferenceWithChartDefaults(s, ch)
-	is.NoError(err)
-	is.Equal("my.host.com/my/nested/repo", ref.Repo)
-	is.Equal("1.5.0", ref.Tag)
-}