From 2045fab01fa33554d15b386a55776f57d3efab9a Mon Sep 17 00:00:00 2001 From: Matthew Fisher Date: Tue, 23 Jul 2019 14:33:23 -0700 Subject: [PATCH] ref(action): remove ParseReferenceWithChartDefaults Signed-off-by: Matthew Fisher --- cmd/helm/chart_save.go | 15 +++++++- pkg/action/action_test.go | 32 ++++++++++++++++- pkg/action/chart_save.go | 17 ++++----- pkg/action/chart_save_test.go | 63 ++++++++++++++++++++++++++++++++++ pkg/registry/reference.go | 18 ---------- pkg/registry/reference_test.go | 27 --------------- 6 files changed, 114 insertions(+), 58 deletions(-) create mode 100644 pkg/action/chart_save_test.go diff --git a/cmd/helm/chart_save.go b/cmd/helm/chart_save.go index c04bde9ba..1f8c915bb 100644 --- a/cmd/helm/chart_save.go +++ b/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) }, } } diff --git a/pkg/action/action_test.go b/pkg/action/action_test.go index cb966dcb8..83346ea58 100644 --- a/pkg/action/action_test.go +++ b/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(®istry.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{ - Name: "hello", + APIVersion: "v1", + Name: "hello", + Version: "0.1.0", }, // This adds a basic template and hooks. Templates: []*chart.File{ diff --git a/pkg/action/chart_save.go b/pkg/action/chart_save.go index 26a633725..3f8dcf533 100644 --- a/pkg/action/chart_save.go +++ b/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) } diff --git a/pkg/action/chart_save_test.go b/pkg/action/chart_save_test.go new file mode 100644 index 000000000..609fc79ed --- /dev/null +++ b/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) + } +} diff --git a/pkg/registry/reference.go b/pkg/registry/reference.go index b1887f054..7a136205f 100644 --- a/pkg/registry/reference.go +++ b/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 diff --git a/pkg/registry/reference_test.go b/pkg/registry/reference_test.go index 4bdb22c5d..7f0299341 100644 --- a/pkg/registry/reference_test.go +++ b/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) -}