From 3ad08f3ea9c09d16ddf6519d65f3f6f2ceee2c37 Mon Sep 17 00:00:00 2001 From: Peter Engelbert Date: Thu, 1 Oct 2020 16:37:44 -0500 Subject: [PATCH 1/4] Implement `helm pull` for OCI registries * Implement `helm dep update` for oci dependencies * New unit tests * Remove `helm chart pull` command * New `helm pull` does not depend on registry cache Signed-off-by: Peter Engelbert --- cmd/helm/dependency.go | 4 +- cmd/helm/dependency_update.go | 3 +- cmd/helm/dependency_update_test.go | 46 +++++ cmd/helm/pull.go | 13 +- cmd/helm/pull_test.go | 58 +++++- cmd/helm/root.go | 23 ++- .../testcharts/oci-dependent-chart-0.1.0.tgz | Bin 0 -> 3599 bytes internal/experimental/registry/client.go | 61 +++++- internal/experimental/registry/client_test.go | 6 +- internal/resolver/resolver.go | 39 +++- pkg/action/chart_pull.go | 2 +- pkg/action/pull.go | 17 +- pkg/downloader/chart_downloader.go | 6 + pkg/downloader/manager.go | 60 +++++- pkg/getter/getter.go | 29 ++- pkg/getter/getter_test.go | 2 +- pkg/getter/ocigetter.go | 69 +++++++ pkg/repo/repotest/server.go | 192 +++++++++++++++++- 18 files changed, 585 insertions(+), 45 deletions(-) create mode 100644 cmd/helm/testdata/testcharts/oci-dependent-chart-0.1.0.tgz create mode 100644 pkg/getter/ocigetter.go diff --git a/cmd/helm/dependency.go b/cmd/helm/dependency.go index 2cc4c5045..3de3ef014 100644 --- a/cmd/helm/dependency.go +++ b/cmd/helm/dependency.go @@ -82,7 +82,7 @@ the contents of a chart. This will produce an error if the chart cannot be loaded. ` -func newDependencyCmd(out io.Writer) *cobra.Command { +func newDependencyCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { cmd := &cobra.Command{ Use: "dependency update|build|list", Aliases: []string{"dep", "dependencies"}, @@ -92,7 +92,7 @@ func newDependencyCmd(out io.Writer) *cobra.Command { } cmd.AddCommand(newDependencyListCmd(out)) - cmd.AddCommand(newDependencyUpdateCmd(out)) + cmd.AddCommand(newDependencyUpdateCmd(cfg, out)) cmd.AddCommand(newDependencyBuildCmd(out)) return cmd diff --git a/cmd/helm/dependency_update.go b/cmd/helm/dependency_update.go index 9855afb92..ad0188f17 100644 --- a/cmd/helm/dependency_update.go +++ b/cmd/helm/dependency_update.go @@ -43,7 +43,7 @@ in the Chart.yaml file, but (b) at the wrong version. ` // newDependencyUpdateCmd creates a new dependency update command. -func newDependencyUpdateCmd(out io.Writer) *cobra.Command { +func newDependencyUpdateCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { client := action.NewDependency() cmd := &cobra.Command{ @@ -63,6 +63,7 @@ func newDependencyUpdateCmd(out io.Writer) *cobra.Command { Keyring: client.Keyring, SkipUpdate: client.SkipRefresh, Getters: getter.All(settings), + RegistryClient: cfg.RegistryClient, RepositoryConfig: settings.RepositoryConfig, RepositoryCache: settings.RepositoryCache, Debug: settings.Debug, diff --git a/cmd/helm/dependency_update_test.go b/cmd/helm/dependency_update_test.go index bf27c7b6c..ce93e5c41 100644 --- a/cmd/helm/dependency_update_test.go +++ b/cmd/helm/dependency_update_test.go @@ -40,6 +40,23 @@ func TestDependencyUpdateCmd(t *testing.T) { defer srv.Stop() t.Logf("Listening on directory %s", srv.Root()) + ociSrv, err := repotest.NewOCIServer(t, srv.Root()) + if err != nil { + t.Fatal(err) + } + + ociChartName := "oci-depending-chart" + c := createTestingMetadataForOCI(ociChartName, ociSrv.RegistryURL) + if err := chartutil.SaveDir(c, ociSrv.Dir); err != nil { + t.Fatal(err) + } + ociSrv.Run(t, repotest.WithDependingChart(c)) + + err = os.Setenv("HELM_EXPERIMENTAL_OCI", "1") + if err != nil { + t.Fatal("failed to set environment variable enabling OCI support") + } + if err := srv.LinkIndices(); err != nil { t.Fatal(err) } @@ -115,6 +132,22 @@ func TestDependencyUpdateCmd(t *testing.T) { if _, err := os.Stat(unexpected); err == nil { t.Fatalf("Unexpected %q", unexpected) } + + // test for OCI charts + cmd := fmt.Sprintf("dependency update '%s' --repository-config %s --repository-cache %s --registry-config %s/config.json", + dir(ociChartName), + dir("repositories.yaml"), + dir(), + dir()) + _, out, err = executeActionCommand(cmd) + if err != nil { + t.Logf("Output: %s", out) + t.Fatal(err) + } + expect = dir(ociChartName, "charts/oci-dependent-chart") + if _, err := os.Stat(expect); err != nil { + t.Fatal(err) + } } func TestDependencyUpdateCmd_DontDeleteOldChartsOnError(t *testing.T) { @@ -193,6 +226,19 @@ func createTestingMetadata(name, baseURL string) *chart.Chart { } } +func createTestingMetadataForOCI(name, registryURL string) *chart.Chart { + return &chart.Chart{ + Metadata: &chart.Metadata{ + APIVersion: chart.APIVersionV2, + Name: name, + Version: "1.2.3", + Dependencies: []*chart.Dependency{ + {Name: "oci-dependent-chart", Version: "0.1.0", Repository: fmt.Sprintf("oci://%s/u/ocitestuser", registryURL)}, + }, + }, + } +} + // createTestingChart creates a basic chart that depends on reqtest-0.1.0 // // The baseURL can be used to point to a particular repository server. diff --git a/cmd/helm/pull.go b/cmd/helm/pull.go index 3f62bf0c7..ded0609e5 100644 --- a/cmd/helm/pull.go +++ b/cmd/helm/pull.go @@ -20,6 +20,7 @@ import ( "fmt" "io" "log" + "strings" "github.com/spf13/cobra" @@ -42,8 +43,8 @@ file, and MUST pass the verification process. Failure in any part of this will result in an error, and the chart will not be saved locally. ` -func newPullCmd(out io.Writer) *cobra.Command { - client := action.NewPull() +func newPullCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { + client := action.NewPull(cfg) cmd := &cobra.Command{ Use: "pull [chart URL | repo/chartname] [...]", @@ -64,6 +65,14 @@ func newPullCmd(out io.Writer) *cobra.Command { client.Version = ">0.0.0-0" } + if strings.HasPrefix(args[0], "oci://") { + if !FeatureGateOCI.IsEnabled() { + return FeatureGateOCI.Error() + } + + client.OCI = true + } + for i := 0; i < len(args); i++ { output, err := client.Run(args[i]) if err != nil { diff --git a/cmd/helm/pull_test.go b/cmd/helm/pull_test.go index 1d439e873..51cdfdfa4 100644 --- a/cmd/helm/pull_test.go +++ b/cmd/helm/pull_test.go @@ -32,6 +32,13 @@ func TestPullCmd(t *testing.T) { } defer srv.Stop() + os.Setenv("HELM_EXPERIMENTAL_OCI", "1") + ociSrv, err := repotest.NewOCIServer(t, srv.Root()) + if err != nil { + t.Fatal(err) + } + ociSrv.Run(t) + if err := srv.LinkIndices(); err != nil { t.Fatal(err) } @@ -139,23 +146,70 @@ func TestPullCmd(t *testing.T) { failExpect: "Failed to fetch chart version", wantError: true, }, + { + name: "Fetch OCI Chart", + args: fmt.Sprintf("oci://%s/u/ocitestuser/oci-dependent-chart --version 0.1.0", ociSrv.RegistryURL), + expectFile: "./oci-dependent-chart-0.1.0.tgz", + }, + { + name: "Fetch OCI Chart with untar", + args: fmt.Sprintf("oci://%s/u/ocitestuser/oci-dependent-chart --version 0.1.0 --untar", ociSrv.RegistryURL), + expectFile: "./oci-dependent-chart", + expectDir: true, + }, + { + name: "Fetch OCI Chart with untar and untardir", + args: fmt.Sprintf("oci://%s/u/ocitestuser/oci-dependent-chart --version 0.1.0 --untar --untardir ocitest2", ociSrv.RegistryURL), + expectFile: "./ocitest2", + expectDir: true, + }, + { + name: "OCI Fetch untar when dir with same name existed", + args: fmt.Sprintf("oci-test-chart oci://%s/u/ocitestuser/oci-dependent-chart --version 0.1.0 --untar --untardir ocitest2 --untar --untardir ocitest2", ociSrv.RegistryURL), + wantError: true, + wantErrorMsg: fmt.Sprintf("failed to untar: a file or directory with the name %s already exists", filepath.Join(srv.Root(), "ocitest2")), + }, + { + name: "Fail fetching non-existent OCI chart", + args: fmt.Sprintf("oci://%s/u/ocitestuser/nosuchthing --version 0.1.0", ociSrv.RegistryURL), + failExpect: "Failed to fetch", + wantError: true, + }, + { + name: "Fail fetching OCI chart without version specified", + args: fmt.Sprintf("oci://%s/u/ocitestuser/nosuchthing", ociSrv.RegistryURL), + wantErrorMsg: "Error: --version flag is explicitly required for OCI registries", + wantError: true, + }, + { + name: "Fail fetching OCI chart without version specified", + args: fmt.Sprintf("oci://%s/u/ocitestuser/oci-dependent-chart:0.1.0", ociSrv.RegistryURL), + wantErrorMsg: "Error: --version flag is explicitly required for OCI registries", + wantError: true, + }, + { + name: "Fail fetching OCI chart without version specified", + args: fmt.Sprintf("oci://%s/u/ocitestuser/oci-dependent-chart:0.1.0 --version 0.1.0", ociSrv.RegistryURL), + wantError: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { outdir := srv.Root() - cmd := fmt.Sprintf("fetch %s -d '%s' --repository-config %s --repository-cache %s ", + cmd := fmt.Sprintf("fetch %s -d '%s' --repository-config %s --repository-cache %s --registry-config %s", tt.args, outdir, filepath.Join(outdir, "repositories.yaml"), outdir, + filepath.Join(outdir, "config.json"), ) // Create file or Dir before helm pull --untar, see: https://github.com/helm/helm/issues/7182 if tt.existFile != "" { file := filepath.Join(outdir, tt.existFile) _, err := os.Create(file) if err != nil { - t.Fatal("err") + t.Fatal(err) } } if tt.existDir != "" { diff --git a/cmd/helm/root.go b/cmd/helm/root.go index f2be0b5a9..8025a9ddf 100644 --- a/cmd/helm/root.go +++ b/cmd/helm/root.go @@ -153,12 +153,22 @@ func newRootCmd(actionConfig *action.Configuration, out io.Writer, args []string flags.ParseErrorsWhitelist.UnknownFlags = true flags.Parse(args) + registryClient, err := registry.NewClient( + registry.ClientOptDebug(settings.Debug), + registry.ClientOptWriter(out), + registry.ClientOptCredentialsFile(settings.RegistryConfig), + ) + if err != nil { + return nil, err + } + actionConfig.RegistryClient = registryClient + // Add subcommands cmd.AddCommand( // chart commands newCreateCmd(out), - newDependencyCmd(out), - newPullCmd(out), + newDependencyCmd(actionConfig, out), + newPullCmd(actionConfig, out), newShowCmd(out), newLintCmd(out), newPackageCmd(out), @@ -188,15 +198,6 @@ func newRootCmd(actionConfig *action.Configuration, out io.Writer, args []string ) // Add *experimental* subcommands - registryClient, err := registry.NewClient( - registry.ClientOptDebug(settings.Debug), - registry.ClientOptWriter(out), - registry.ClientOptCredentialsFile(settings.RegistryConfig), - ) - if err != nil { - return nil, err - } - actionConfig.RegistryClient = registryClient cmd.AddCommand( newRegistryCmd(actionConfig, out), newChartCmd(actionConfig, out), diff --git a/cmd/helm/testdata/testcharts/oci-dependent-chart-0.1.0.tgz b/cmd/helm/testdata/testcharts/oci-dependent-chart-0.1.0.tgz new file mode 100644 index 0000000000000000000000000000000000000000..7b4cbeccc1c06dd27ff6376bcc6607ad03fa3839 GIT binary patch literal 3599 zcmV+q4)F0GiwG0|00000|0w_~VMtOiV@ORlOnEsqVl!4SWK%V1T2nbTPgYhoO;>Dc zVQyr3R8em|NM&qo0PH+#Z`(Mw{j6Vcu9HO{x0Yq+(Lz8kkel|-2HU1a(&=I_7!s#+V?Fk?U?WB~tf}&8NR0 zGxp?OMi2zS;r_n;8w5e~Z!icBpY-=$?H%+7d;R|4NzgwCg259A?uY#Pkt$8(li<#@ zn!^1>21)2=l!^)-!hGP7Bq_f3{r;gJcrmI-(nQ;PNAP!KGqCFf#zMkB(h*9I8kNV% z3`1yHP@Y~S7y?NWMk8VndGnk|;P?H&`_WqX&mC>{KPN0jb$yPooQzL}9!gZmwFj^RALl3~jSbx?g2e-xOyF`V6hfQ{O5J1U!%Bze zdtiV==yWn3hACs`7)jJBgkOKN4lXBQ!Nw_LD>prba!w;WiXtnLj^LxSXP% zq67jN91sTxYAR9|8+}C*iN@H2>?#B;Q?!VVI2YRbP^(-L$L5cbr-9A`ASG-F%WM1o zrzwJA8N|5lErTpo=v&y6F>s+lp$5X^j2Ejunc_Ni6s8W+2TwH{eP69S}2XPik@Z4kJPE)2B3NSXN59^e#VN`pP}HQ+%Zd) zMtL$ci&aP+!t22ED4$4FmMP@MG~y7(geh#DCPV3>2&_I8z3hEIVnnIZ8dd={Q(Y2S zH5;Zo9>7#6Z7CZCm@lDQ`(3;uvxK3~L`0Z<4v-K%b*mjfv;1nuysE4zoZ308A?RwR zG7Xo+b{xKLD=kl=5~+-^T$ukvNL5N0TY$t!%x1`AyZp2OWvypXSa9*SU6)z_Lo!Fu z#y=6`pCZ9kL`CY@il)LWapT#{%1jkX-#IhFlmN=j%2ucht2}alrB-ILL2y7mr&914 za;@N!>M1a)HOm%6&dN(rX*4zDKTuB1r1-{w79xdMz$M`|Nh+_U+)1mQ*$BqOCFK)~ zol$N;Nc?-M?DLr+z%fFlh+Mq1@=KfvD?LJ#O16NJBvv=`}H@^BjYjU zR4?q~A03k)a6!Pel**aWbTL?+`1Yy^N1PI@(K-*AZ zU;#>K-v%IecdB)=TpB{V&3833FlxC36DF>&!(MN>bfhd^xV~O4!7s5pFt2&Z6dL@I z;F?gbpmQo>915vB1-n^<_f2?r`0&aEb~yd`_T87^$FomwcApu4A87^?)X?pkJVzxY zLNH96Av58{KP~QFUqe^G?@DkZ?VHZhH*vu?SkC4JQ+5aG$hnFu}U@0ESE|XohqNiv6&LI_p&e79m_y86?PKCUT-&JBit?2 zcEb$37bN~)!b~KV>t+D}jC%^1Atu$?UdF8KDoQvRX1fwHCzP25 z>u0%-9lwYaBEsR4=xFLvWGkmhm@5|X^wu-3<`Hx+Z#j=o%XvJ1clP<@xMeCW;%zv% z9ck!x^FRQd<+8I}W+wWK@))lmpMO?SZf>6tJhv9;TQhV9*ST(46{S$2VY@NeR_+^3 zwaB!$u+`zmw_i?=KD=wFE)nszM!=v$lEklP+vdeLo77sW^yMlB%9S>%d()(BGni5D^O-8D z^C|BSUipC^`2Fy8@H)u$o31KQ2|#Awl5@Tl$A*jUSjbBv;|lwRMz~v7N;L@l=Fd^-V5*DRbn^NjoMIF_b`3wS$~%M=xV_XzJiu1kKl06UeeilIv&$2=yW@cbj`Dz9=jS8DJYnj zwSYfT!swV{Y1RWc2ilrfWcq{{HAOs7&?Db-M^_$ z1D7MZ{ZebctS!LG%o@wd+F52+?d)bt#X&JLLbl}$+s|@(n_^Wp?yj@qHGBn$2{Xa4 z^WPoleCgcYw8U&CxZ_rB$E|~P>`sx*d^PRaC*j6iGZWNztgMGsS?%V1M$WIStV|6@ zMH$~bTTQcyexpesR$loEf22ZVLoKUVW`vUo#@*2(tYsqSSrNTWt$E?;R*G#(+-+sY zmD9IRKU+}b>1Vi-S*=_B%|OxKMA%l;YPaO{Cht}U^Rr$=BiuaOc6;;uCcL~VdBwlv z6gt_(%9f)Q!HN(0(b0Z&tk3^qOqf{Akl*Pz$j1C{Kd9$_gP=dyd(8hHVpPw6kR(yP zdEdLBJPzS)G521+-3*H!-t9^W9%SCnb)l?juX~#nj{YlfYM;Qq!DAAxqWZ{%F08Q9 zbf{J#Z4Wu?EL7?X)s}O^LIHB$>v)|J9Zla?nBRi^)p%8kap;KTCYBMY=-1)5ZKj;v zdb6~se$^U>MG4Eit;H{7#%gq5=daadHG|cJyQ6po88=fU#+pFQ?t010O5c5JxmqM! z4K14&Zd*9=t0?Q8=T`R0X>0Ve+xafbDJ8mYX=l}>uXtxIFE{X`Ze*rEsAF~~FrZU0 z8GFkwPh`OLb-40alL>rn?hk6IS*J;5SUJzJg0DR6QRmN=oa*xC3Y%)3)E-pyJIveh z_2-EcT10|{@acG1dB*4*IVv@jVuW>ShRRRaP`PWKhtO-@wPYKNNhC4-j@a{_@1{jj zw$j^|!;0-zo2eyOt;Kp5n_Z!ocV;VYFYndfE1gP9xP+?qiNB57kB;w~#`^qknvi=q z2Dl;r>mMElEB)X8{$u|45Tmipx*?CMWr-z7Z$22ICV9sAcOmJY#@Vu=h>tQbl>ct& zBY@@WlRZ3ghGU1dEfF^NSCV1t?!HZm+R>Lvw5w{p0+YQ|Jsxw@OC?P95^6~!tFSdQ zy#AO??$5f}$+ojBRCx6mQ#wYcl_*JzJC4B793`1xg~*M9Y{OB_mv5AA&FP?T6wsY_ zx~q&E`FfZN!g~LoAGf$$4A|iR2f<#`Y*`EssCr(Z6k*(C#J%+BfU34~Aq*)tS zsSDhYvF@)pnmj<>uqxcCtL$WOfjK+4brWoU>bQ|XXKz&wfw|GOZnIjFufEmT-v7!QSRem$C;eUHe{k48Sc(6^WBva@ zM(y{X`9?15Xa4(b++3dCa#FB7@K>n~cC+Hxnv;dq6n@yOq_q_WYgNX2tKd$B^Zg&> zShuqOtgrj6ZeWxBAMCC4|6UzD)_)#kY}ICOrpQf4k%51MzgW|5`EGf0L&kedpZ|h6 z(t7*XsI>d+MuKyUj8<;#1~$h3y~gi<27|%iasU4yqt&d3B5twe&h3JKx3O$G_h{2A zCfdo5pG}3h=!9Ts7g2G-hN=}ry zc{_cq`GJGQEYYk)oi{^IagOr)yOB2g_l+{?gbRuHLSu9MZ|468gU9~Q!;Bpm60K3X ze<$E39WJNHVU$u9Q$B%&L>FX&s`wWWDot#b4Qh&v!GwvCJ10Z=&U+=I5s5C+#QqZt<-PFLW#YZ?N94kL z;p>^X7Lrc97ys|=z-J<k>YoZwCB?pNzZ{x@w?~GRU~5U{biDX$MHBG V$A9GbR{#J2|NqD05>x Date: Mon, 5 Oct 2020 12:20:04 -0500 Subject: [PATCH 2/4] Clean up imports and add doc comments Additionally, revert `NewPull()` to its existing signature. Signed-off-by: Peter Engelbert --- cmd/helm/pull.go | 2 +- go.mod | 1 + internal/experimental/registry/client.go | 12 ++++--- internal/experimental/registry/client_test.go | 20 ++--------- internal/resolver/resolver.go | 6 ++-- pkg/action/pull.go | 24 +++++++++++-- pkg/repo/repotest/server.go | 35 +++++-------------- 7 files changed, 44 insertions(+), 56 deletions(-) diff --git a/cmd/helm/pull.go b/cmd/helm/pull.go index ded0609e5..b094db6c3 100644 --- a/cmd/helm/pull.go +++ b/cmd/helm/pull.go @@ -44,7 +44,7 @@ result in an error, and the chart will not be saved locally. ` func newPullCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { - client := action.NewPull(cfg) + client := action.NewPullWithOpts(action.WithConfig(cfg)) cmd := &cobra.Command{ Use: "pull [chart URL | repo/chartname] [...]", diff --git a/go.mod b/go.mod index 971f4949d..8b182f6d3 100644 --- a/go.mod +++ b/go.mod @@ -27,6 +27,7 @@ require ( github.com/mitchellh/copystructure v1.0.0 github.com/opencontainers/go-digest v1.0.0 github.com/opencontainers/image-spec v1.0.1 + github.com/phayes/freeport v0.0.0-20180830031419-95f893ade6f2 github.com/pkg/errors v0.9.1 github.com/rubenv/sql-migrate v0.0.0-20200616145509-8d140a17f351 github.com/sirupsen/logrus v1.7.0 diff --git a/internal/experimental/registry/client.go b/internal/experimental/registry/client.go index 55b34d68f..c889ee913 100644 --- a/internal/experimental/registry/client.go +++ b/internal/experimental/registry/client.go @@ -25,16 +25,15 @@ import ( "net/http" "sort" - "helm.sh/helm/v3/pkg/chart" - "helm.sh/helm/v3/pkg/helmpath" - - "github.com/deislabs/oras/pkg/content" - auth "github.com/deislabs/oras/pkg/auth/docker" + "github.com/deislabs/oras/pkg/content" "github.com/deislabs/oras/pkg/oras" "github.com/gosuri/uitable" ocispec "github.com/opencontainers/image-spec/specs-go/v1" "github.com/pkg/errors" + + "helm.sh/helm/v3/pkg/chart" + "helm.sh/helm/v3/pkg/helmpath" ) const ( @@ -197,6 +196,9 @@ func (c *Client) PullChart(ref *Reference) (*bytes.Buffer, error) { return buf, nil } +// PullChartToCache pulls a chart from an OCI Registry to the Registry Cache. +// This function is needed for `helm chart pull`, which is experimental and will be deprecated soon. +// Likewise, the Registry cache will soon be deprecated as will this function. func (c *Client) PullChartToCache(ref *Reference) error { if ref.Tag == "" { return errors.New("tag explicitly required") diff --git a/internal/experimental/registry/client_test.go b/internal/experimental/registry/client_test.go index 0d5d508d5..a9936ba13 100644 --- a/internal/experimental/registry/client_test.go +++ b/internal/experimental/registry/client_test.go @@ -22,7 +22,6 @@ import ( "fmt" "io" "io/ioutil" - "net" "net/http" "net/http/httptest" "net/url" @@ -33,12 +32,12 @@ import ( "time" "github.com/containerd/containerd/errdefs" - auth "github.com/deislabs/oras/pkg/auth/docker" "github.com/docker/distribution/configuration" "github.com/docker/distribution/registry" _ "github.com/docker/distribution/registry/auth/htpasswd" _ "github.com/docker/distribution/registry/storage/driver/inmemory" + "github.com/phayes/freeport" "github.com/stretchr/testify/suite" "golang.org/x/crypto/bcrypt" @@ -107,7 +106,7 @@ func (suite *RegistryClientTestSuite) SetupSuite() { // Registry config config := &configuration.Configuration{} - port, err := getFreePort() + port, err := freeport.GetFreePort() suite.Nil(err, "no error finding free port for test registry") suite.DockerRegistryHost = fmt.Sprintf("localhost:%d", port) config.HTTP.Addr = fmt.Sprintf(":%d", port) @@ -254,21 +253,6 @@ func TestRegistryClientTestSuite(t *testing.T) { suite.Run(t, new(RegistryClientTestSuite)) } -// borrowed from https://github.com/phayes/freeport -func getFreePort() (int, error) { - addr, err := net.ResolveTCPAddr("tcp", "localhost:0") - if err != nil { - return 0, err - } - - l, err := net.ListenTCP("tcp", addr) - if err != nil { - return 0, err - } - defer l.Close() - return l.Addr().(*net.TCPAddr).Port, nil -} - func initCompromisedRegistryTestServer() string { s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if strings.Contains(r.URL.Path, "manifests") { diff --git a/internal/resolver/resolver.go b/internal/resolver/resolver.go index 6692942a1..de0634093 100644 --- a/internal/resolver/resolver.go +++ b/internal/resolver/resolver.go @@ -23,15 +23,15 @@ import ( "strings" "time" + "github.com/Masterminds/semver/v3" + "github.com/pkg/errors" + "helm.sh/helm/v3/pkg/chart" "helm.sh/helm/v3/pkg/chart/loader" "helm.sh/helm/v3/pkg/gates" "helm.sh/helm/v3/pkg/helmpath" "helm.sh/helm/v3/pkg/provenance" "helm.sh/helm/v3/pkg/repo" - - "github.com/Masterminds/semver/v3" - "github.com/pkg/errors" ) const FeatureGateOCI = gates.Gate("HELM_EXPERIMENTAL_OCI") diff --git a/pkg/action/pull.go b/pkg/action/pull.go index 258685441..a7f69c4bc 100644 --- a/pkg/action/pull.go +++ b/pkg/action/pull.go @@ -49,9 +49,27 @@ type Pull struct { cfg *Configuration } -// NewPull creates a new Pull object with the given configuration. -func NewPull(cfg *Configuration) *Pull { - return &Pull{cfg: cfg} +type PullOpt func(*Pull) + +func WithConfig(cfg *Configuration) PullOpt { + return func(p *Pull) { + p.cfg = cfg + } +} + +// NewPull creates a new Pull object. +func NewPull() *Pull { + return NewPullWithOpts() +} + +// NewPull creates a new pull, with configuration options. +func NewPullWithOpts(opts ...PullOpt) *Pull { + p := &Pull{} + for _, fn := range opts { + fn(p) + } + + return p } // Run executes 'helm pull' against the given release. diff --git a/pkg/repo/repotest/server.go b/pkg/repo/repotest/server.go index 7dc60e948..94b5ce7f9 100644 --- a/pkg/repo/repotest/server.go +++ b/pkg/repo/repotest/server.go @@ -19,7 +19,6 @@ import ( "context" "fmt" "io/ioutil" - "net" "net/http" "net/http/httptest" "os" @@ -27,23 +26,21 @@ import ( "testing" "time" - "helm.sh/helm/v3/internal/tlsutil" - "helm.sh/helm/v3/pkg/chart" - "helm.sh/helm/v3/pkg/chart/loader" - "helm.sh/helm/v3/pkg/chartutil" - "helm.sh/helm/v3/pkg/repo" - - "sigs.k8s.io/yaml" - auth "github.com/deislabs/oras/pkg/auth/docker" "github.com/docker/distribution/configuration" "github.com/docker/distribution/registry" _ "github.com/docker/distribution/registry/auth/htpasswd" // used for docker test registry _ "github.com/docker/distribution/registry/storage/driver/inmemory" // used for docker test registry + "github.com/phayes/freeport" + "golang.org/x/crypto/bcrypt" + "sigs.k8s.io/yaml" ociRegistry "helm.sh/helm/v3/internal/experimental/registry" - - "golang.org/x/crypto/bcrypt" + "helm.sh/helm/v3/internal/tlsutil" + "helm.sh/helm/v3/pkg/chart" + "helm.sh/helm/v3/pkg/chart/loader" + "helm.sh/helm/v3/pkg/chartutil" + "helm.sh/helm/v3/pkg/repo" ) // NewTempServerWithCleanup creates a server inside of a temp dir. @@ -96,7 +93,7 @@ func NewOCIServer(t *testing.T, dir string) (*OCIServer, error) { // Registry config config := &configuration.Configuration{} - port, err := getFreePort() + port, err := freeport.GetFreePort() if err != nil { t.Fatalf("error finding free port for test registry") } @@ -404,17 +401,3 @@ func setTestingRepository(url, fname string) error { }) return r.WriteFile(fname, 0644) } - -func getFreePort() (int, error) { - addr, err := net.ResolveTCPAddr("tcp", "localhost:0") - if err != nil { - return 0, err - } - - l, err := net.ListenTCP("tcp", addr) - if err != nil { - return 0, err - } - defer l.Close() - return l.Addr().(*net.TCPAddr).Port, nil -} From f666fceb30a1033f3309a4e47bedb6193791619e Mon Sep 17 00:00:00 2001 From: Peter Engelbert Date: Thu, 8 Oct 2020 10:26:55 -0500 Subject: [PATCH 3/4] Remove OCI boolean from struct Signed-off-by: Peter Engelbert --- cmd/helm/pull.go | 2 -- pkg/action/pull.go | 3 +-- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/cmd/helm/pull.go b/cmd/helm/pull.go index b094db6c3..7711320f1 100644 --- a/cmd/helm/pull.go +++ b/cmd/helm/pull.go @@ -69,8 +69,6 @@ func newPullCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { if !FeatureGateOCI.IsEnabled() { return FeatureGateOCI.Error() } - - client.OCI = true } for i := 0; i < len(args); i++ { diff --git a/pkg/action/pull.go b/pkg/action/pull.go index a7f69c4bc..acd39bf05 100644 --- a/pkg/action/pull.go +++ b/pkg/action/pull.go @@ -43,7 +43,6 @@ type Pull struct { Devel bool Untar bool VerifyLater bool - OCI bool UntarDir string DestDir string cfg *Configuration @@ -90,7 +89,7 @@ func (p *Pull) Run(chartRef string) (string, error) { RepositoryCache: p.Settings.RepositoryCache, } - if p.OCI { + if strings.HasPrefix(chartRef, "oci://") { if p.Version == "" { return out.String(), errors.Errorf("--version flag is explicitly required for OCI registries") } From beda5e1e2be460543c32e2267982d6a7333be483 Mon Sep 17 00:00:00 2001 From: Peter Engelbert Date: Fri, 18 Dec 2020 16:29:15 -0600 Subject: [PATCH 4/4] Address error on deletion of old dependencies Signed-off-by: Peter Engelbert --- cmd/helm/dependency_update_test.go | 2 +- pkg/downloader/manager.go | 16 ++-------------- pkg/getter/getter_test.go | 2 +- 3 files changed, 4 insertions(+), 16 deletions(-) diff --git a/cmd/helm/dependency_update_test.go b/cmd/helm/dependency_update_test.go index ce93e5c41..896018735 100644 --- a/cmd/helm/dependency_update_test.go +++ b/cmd/helm/dependency_update_test.go @@ -144,7 +144,7 @@ func TestDependencyUpdateCmd(t *testing.T) { t.Logf("Output: %s", out) t.Fatal(err) } - expect = dir(ociChartName, "charts/oci-dependent-chart") + expect = dir(ociChartName, "charts/oci-dependent-chart-0.1.0.tgz") if _, err := os.Stat(expect); err != nil { t.Fatal(err) } diff --git a/pkg/downloader/manager.go b/pkg/downloader/manager.go index f2945fdb6..38ed04279 100644 --- a/pkg/downloader/manager.go +++ b/pkg/downloader/manager.go @@ -335,7 +335,7 @@ func (m *Manager) downloadAll(deps []*chart.Dependency) error { }, } - untar, version := false, "" + version := "" if strings.HasPrefix(churl, "oci://") { if !resolver.FeatureGateOCI.IsEnabled() { return errors.Wrapf(resolver.FeatureGateOCI.Error(), @@ -346,29 +346,17 @@ func (m *Manager) downloadAll(deps []*chart.Dependency) error { if err != nil { return errors.Wrapf(err, "could not parse OCI reference") } - untar = true dl.Options = append(dl.Options, getter.WithRegistryClient(m.RegistryClient), getter.WithTagName(version)) } - destFile, _, err := dl.DownloadTo(churl, version, destPath) + _, _, err = dl.DownloadTo(churl, version, destPath) if err != nil { saveError = errors.Wrapf(err, "could not download %s", churl) break } - if untar { - err = chartutil.ExpandFile(destPath, destFile) - if err != nil { - return errors.Wrapf(err, "could not open %s to untar", destFile) - } - err = os.RemoveAll(destFile) - if err != nil { - return errors.Wrapf(err, "chart was downloaded and untarred, but was unable to remove the tarball: %s", destFile) - } - } - churls[churl] = struct{}{} } diff --git a/pkg/getter/getter_test.go b/pkg/getter/getter_test.go index 95d309c16..ab14784ab 100644 --- a/pkg/getter/getter_test.go +++ b/pkg/getter/getter_test.go @@ -58,7 +58,7 @@ func TestAll(t *testing.T) { all := All(env) if len(all) != 4 { - t.Errorf("expected 3 providers (default plus two plugins), got %d", len(all)) + t.Errorf("expected 4 providers (default plus three plugins), got %d", len(all)) } if _, err := all.ByScheme("test2"); err != nil {