From b0ecb210563ccf0441020e8658c7f7e492c6fb0a Mon Sep 17 00:00:00 2001 From: Soule BA Date: Tue, 21 Jun 2022 13:30:41 +0200 Subject: [PATCH 1/6] Enable custom certificates option for OCI If implemented, users will be able to use custom certificates and CA to while interacting with OCI registries. Signed-off-by: Soule BA --- cmd/helm/install.go | 2 +- cmd/helm/pull.go | 2 +- cmd/helm/push.go | 16 +- cmd/helm/registry_login.go | 29 +- cmd/helm/root.go | 21 +- cmd/helm/show.go | 14 +- cmd/helm/upgrade.go | 2 +- pkg/action/install.go | 18 +- pkg/action/pull.go | 19 ++ pkg/action/push.go | 37 ++- pkg/action/registry_login.go | 41 ++- pkg/getter/ocigetter.go | 94 +++++- pkg/getter/ocigetter_test.go | 121 ++++++- pkg/pusher/ocipusher.go | 68 +++- pkg/pusher/ocipusher_test.go | 64 +++- pkg/pusher/pusher.go | 12 + pkg/registry/client.go | 25 ++ pkg/registry/client_test.go | 286 +---------------- pkg/registry/client_tls_test.go | 81 +++++ pkg/registry/testdata/tls/ca-cert.pem | 21 ++ pkg/registry/testdata/tls/client-cert.pem | 22 ++ pkg/registry/testdata/tls/client-key.pem | 28 ++ pkg/registry/testdata/tls/server-cert.pem | 22 ++ pkg/registry/testdata/tls/server-key.pem | 28 ++ pkg/registry/util.go | 26 ++ pkg/registry/utils_test.go | 371 ++++++++++++++++++++++ 26 files changed, 1117 insertions(+), 353 deletions(-) create mode 100644 pkg/registry/client_tls_test.go create mode 100644 pkg/registry/testdata/tls/ca-cert.pem create mode 100644 pkg/registry/testdata/tls/client-cert.pem create mode 100644 pkg/registry/testdata/tls/client-key.pem create mode 100644 pkg/registry/testdata/tls/server-cert.pem create mode 100644 pkg/registry/testdata/tls/server-key.pem create mode 100644 pkg/registry/utils_test.go diff --git a/cmd/helm/install.go b/cmd/helm/install.go index 976ce0a29..dc7018089 100644 --- a/cmd/helm/install.go +++ b/cmd/helm/install.go @@ -203,7 +203,7 @@ func runInstall(args []string, client *action.Install, valueOpts *values.Options } client.ReleaseName = name - cp, err := client.ChartPathOptions.LocateChart(chart, settings) + cp, err := client.ChartPathOptions.LocateChart(chart, out, settings) if err != nil { return nil, err } diff --git a/cmd/helm/pull.go b/cmd/helm/pull.go index 378301196..238550250 100644 --- a/cmd/helm/pull.go +++ b/cmd/helm/pull.go @@ -43,7 +43,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.NewPullWithOpts(action.WithConfig(cfg)) + client := action.NewPullWithOpts(action.WithConfig(cfg), action.WithPullOptWriter(out)) cmd := &cobra.Command{ Use: "pull [chart URL | repo/chartname] [...]", diff --git a/cmd/helm/push.go b/cmd/helm/push.go index d2cf2693e..ac6659fa3 100644 --- a/cmd/helm/push.go +++ b/cmd/helm/push.go @@ -34,8 +34,14 @@ If the chart has an associated provenance file, it will also be uploaded. ` +type registryPushOptions struct { + certFile string + keyFile string + caFile string +} + func newPushCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { - client := action.NewPushWithOpts(action.WithPushConfig(cfg)) + o := ®istryPushOptions{} cmd := &cobra.Command{ Use: "push [chart] [remote]", @@ -62,6 +68,9 @@ func newPushCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { RunE: func(cmd *cobra.Command, args []string) error { chartRef := args[0] remote := args[1] + client := action.NewPushWithOpts(action.WithPushConfig(cfg), + action.WithTLSClientConfig(o.certFile, o.keyFile, o.caFile), + action.WithPushOptWriter(out)) client.Settings = settings output, err := client.Run(chartRef, remote) if err != nil { @@ -72,5 +81,10 @@ func newPushCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { }, } + f := cmd.Flags() + f.StringVar(&o.certFile, "cert-file", "", "identify registry client using this SSL certificate file") + f.StringVar(&o.keyFile, "key-file", "", "identify registry client using this SSL key file") + f.StringVar(&o.caFile, "ca-file", "", "verify certificates of HTTPS-enabled servers using this CA bundle") + return cmd } diff --git a/cmd/helm/registry_login.go b/cmd/helm/registry_login.go index 6b1fed589..98d31bddc 100644 --- a/cmd/helm/registry_login.go +++ b/cmd/helm/registry_login.go @@ -36,9 +36,17 @@ const registryLoginDesc = ` Authenticate to a remote registry. ` +type registryLoginOptions struct { + username string + password string + passwordFromStdinOpt bool + certFile string + keyFile string + caFile string +} + func newRegistryLoginCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { - var usernameOpt, passwordOpt string - var passwordFromStdinOpt, insecureOpt bool + o := ®istryLoginOptions{} cmd := &cobra.Command{ Use: "login [host]", @@ -49,20 +57,25 @@ func newRegistryLoginCmd(cfg *action.Configuration, out io.Writer) *cobra.Comman RunE: func(cmd *cobra.Command, args []string) error { hostname := args[0] - username, password, err := getUsernamePassword(usernameOpt, passwordOpt, passwordFromStdinOpt) + username, password, err := getUsernamePassword(o.username, o.password, o.passwordFromStdinOpt) if err != nil { return err } - return action.NewRegistryLogin(cfg).Run(out, hostname, username, password, insecureOpt) + return action.NewRegistryLogin(cfg).Run(out, hostname, username, password, + action.WithCertFile(o.certFile), + action.WithKeyFile(o.keyFile), + action.WithCAFile(o.caFile)) }, } f := cmd.Flags() - f.StringVarP(&usernameOpt, "username", "u", "", "registry username") - f.StringVarP(&passwordOpt, "password", "p", "", "registry password or identity token") - f.BoolVarP(&passwordFromStdinOpt, "password-stdin", "", false, "read password or identity token from stdin") - f.BoolVarP(&insecureOpt, "insecure", "", false, "allow connections to TLS registry without certs") + f.StringVarP(&o.username, "username", "u", "", "registry username") + f.StringVarP(&o.password, "password", "p", "", "registry password or identity token") + f.BoolVarP(&o.passwordFromStdinOpt, "password-stdin", "", false, "read password or identity token from stdin") + f.StringVar(&o.certFile, "cert-file", "", "identify registry client using this SSL certificate file") + f.StringVar(&o.keyFile, "key-file", "", "identify registry client using this SSL key file") + f.StringVar(&o.caFile, "ca-file", "", "verify certificates of HTTPS-enabled servers using this CA bundle") return cmd } diff --git a/cmd/helm/root.go b/cmd/helm/root.go index 7da57c6aa..c4f439511 100644 --- a/cmd/helm/root.go +++ b/cmd/helm/root.go @@ -152,12 +152,7 @@ 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.ClientOptEnableCache(true), - registry.ClientOptWriter(os.Stderr), - registry.ClientOptCredentialsFile(settings.RegistryConfig), - ) + registryClient, err := newRegistryClient(out) if err != nil { return nil, err } @@ -261,3 +256,17 @@ func checkForExpiredRepos(repofile string) { } } + +func newRegistryClient(out io.Writer) (*registry.Client, error) { + // Create a new registry client + registryClient, err := registry.NewClient( + registry.ClientOptDebug(settings.Debug), + registry.ClientOptEnableCache(true), + registry.ClientOptWriter(os.Stderr), + registry.ClientOptCredentialsFile(settings.RegistryConfig), + ) + if err != nil { + return nil, err + } + return registryClient, nil +} diff --git a/cmd/helm/show.go b/cmd/helm/show.go index 718d716a0..e9b17a8cb 100644 --- a/cmd/helm/show.go +++ b/cmd/helm/show.go @@ -84,7 +84,7 @@ func newShowCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { ValidArgsFunction: validArgsFunc, RunE: func(cmd *cobra.Command, args []string) error { client.OutputFormat = action.ShowAll - output, err := runShow(args, client) + output, err := runShow(args, client, out) if err != nil { return err } @@ -101,7 +101,7 @@ func newShowCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { ValidArgsFunction: validArgsFunc, RunE: func(cmd *cobra.Command, args []string) error { client.OutputFormat = action.ShowValues - output, err := runShow(args, client) + output, err := runShow(args, client, out) if err != nil { return err } @@ -118,7 +118,7 @@ func newShowCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { ValidArgsFunction: validArgsFunc, RunE: func(cmd *cobra.Command, args []string) error { client.OutputFormat = action.ShowChart - output, err := runShow(args, client) + output, err := runShow(args, client, out) if err != nil { return err } @@ -135,7 +135,7 @@ func newShowCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { ValidArgsFunction: validArgsFunc, RunE: func(cmd *cobra.Command, args []string) error { client.OutputFormat = action.ShowReadme - output, err := runShow(args, client) + output, err := runShow(args, client, out) if err != nil { return err } @@ -152,7 +152,7 @@ func newShowCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { ValidArgsFunction: validArgsFunc, RunE: func(cmd *cobra.Command, args []string) error { client.OutputFormat = action.ShowCRDs - output, err := runShow(args, client) + output, err := runShow(args, client, out) if err != nil { return err } @@ -191,14 +191,14 @@ func addShowFlags(subCmd *cobra.Command, client *action.Show) { } } -func runShow(args []string, client *action.Show) (string, error) { +func runShow(args []string, client *action.Show, out io.Writer) (string, error) { debug("Original chart version: %q", client.Version) if client.Version == "" && client.Devel { debug("setting version to >0.0.0-0") client.Version = ">0.0.0-0" } - cp, err := client.ChartPathOptions.LocateChart(args[0], settings) + cp, err := client.ChartPathOptions.LocateChart(args[0], out, settings) if err != nil { return "", err } diff --git a/cmd/helm/upgrade.go b/cmd/helm/upgrade.go index 3302da12c..bf9b30ffb 100644 --- a/cmd/helm/upgrade.go +++ b/cmd/helm/upgrade.go @@ -136,7 +136,7 @@ func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { client.Version = ">0.0.0-0" } - chartPath, err := client.ChartPathOptions.LocateChart(args[1], settings) + chartPath, err := client.ChartPathOptions.LocateChart(args[1], out, settings) if err != nil { return err } diff --git a/pkg/action/install.go b/pkg/action/install.go index 4658c9be8..4d716bec7 100644 --- a/pkg/action/install.go +++ b/pkg/action/install.go @@ -20,6 +20,7 @@ import ( "bytes" "context" "fmt" + "io" "io/ioutil" "net/url" "os" @@ -675,13 +676,22 @@ OUTER: // - URL // // If 'verify' was set on ChartPathOptions, this will attempt to also verify the chart. -func (c *ChartPathOptions) LocateChart(name string, settings *cli.EnvSettings) (string, error) { +func (c *ChartPathOptions) LocateChart(name string, out io.Writer, settings *cli.EnvSettings) (string, error) { // If there is no registry client and the name is in an OCI registry return // an error and a lookup will not occur. - if registry.IsOCI(name) && c.registryClient == nil { - return "", fmt.Errorf("unable to lookup chart %q, missing registry client", name) + if registry.IsOCI(name) { + if (c.CertFile != "" && c.KeyFile != "") || c.CaFile != "" { + registryClient, err := registry.NewRegistryClientWithTLS(out, c.CertFile, c.KeyFile, c.CaFile, + settings.RegistryConfig, settings.Debug) + if err != nil { + return "", err + } + c.registryClient = registryClient + } + if c.registryClient == nil { + return "", fmt.Errorf("unable to lookup chart %q, missing registry client", name) + } } - name = strings.TrimSpace(name) version := strings.TrimSpace(c.Version) diff --git a/pkg/action/pull.go b/pkg/action/pull.go index b4018869e..bcd93705d 100644 --- a/pkg/action/pull.go +++ b/pkg/action/pull.go @@ -18,6 +18,7 @@ package action import ( "fmt" + "io" "io/ioutil" "os" "path/filepath" @@ -47,6 +48,7 @@ type Pull struct { UntarDir string DestDir string cfg *Configuration + out io.Writer } type PullOpt func(*Pull) @@ -57,6 +59,13 @@ func WithConfig(cfg *Configuration) PullOpt { } } +// WithOptWriter sets the registryOut field on the push configuration object. +func WithPullOptWriter(out io.Writer) PullOpt { + return func(p *Pull) { + p.out = out + } +} + // NewPull creates a new Pull object. func NewPull() *Pull { return NewPullWithOpts() @@ -93,6 +102,16 @@ func (p *Pull) Run(chartRef string) (string, error) { } if registry.IsOCI(chartRef) { + // Provide a tls enabled client for the pull command if the user has + // specified the cert file or key file or ca file. + if (p.ChartPathOptions.CertFile != "" && p.ChartPathOptions.KeyFile != "") || p.ChartPathOptions.CaFile != "" { + registryClient, err := registry.NewRegistryClientWithTLS(p.out, p.ChartPathOptions.CertFile, p.ChartPathOptions.KeyFile, p.ChartPathOptions.CaFile, + p.Settings.RegistryConfig, p.Settings.Debug) + if err != nil { + return out.String(), err + } + p.cfg.RegistryClient = registryClient + } c.Options = append(c.Options, getter.WithRegistryClient(p.cfg.RegistryClient)) } diff --git a/pkg/action/push.go b/pkg/action/push.go index 99d1beadc..f6e7a6aaa 100644 --- a/pkg/action/push.go +++ b/pkg/action/push.go @@ -17,6 +17,7 @@ limitations under the License. package action import ( + "io" "strings" "helm.sh/helm/v3/pkg/cli" @@ -31,6 +32,10 @@ import ( type Push struct { Settings *cli.EnvSettings cfg *Configuration + certFile string + keyFile string + caFile string + out io.Writer } // PushOpt is a type of function that sets options for a push action. @@ -43,6 +48,22 @@ func WithPushConfig(cfg *Configuration) PushOpt { } } +// WithTLSClientConfig sets the certFile, keyFile, and caFile fields on the push configuration object. +func WithTLSClientConfig(certFile, keyFile, caFile string) PushOpt { + return func(p *Push) { + p.certFile = certFile + p.keyFile = keyFile + p.caFile = caFile + } +} + +// WithOptWriter sets the registryOut field on the push configuration object. +func WithPushOptWriter(out io.Writer) PushOpt { + return func(p *Push) { + p.out = out + } +} + // NewPushWithOpts creates a new push, with configuration options. func NewPushWithOpts(opts ...PushOpt) *Push { p := &Push{} @@ -59,10 +80,24 @@ func (p *Push) Run(chartRef string, remote string) (string, error) { c := uploader.ChartUploader{ Out: &out, Pushers: pusher.All(p.Settings), - Options: []pusher.Option{}, + Options: []pusher.Option{ + pusher.WithTLSClientConfig(p.certFile, p.keyFile, p.caFile), + }, } if registry.IsOCI(remote) { + // Provide a tls enabled client for the pull command if the user has + // specified the cert file or key file or ca file. + if (p.certFile != "" && p.keyFile != "") || p.caFile != "" { + registryClient, err := registry.NewRegistryClientWithTLS(p.out, p.certFile, p.keyFile, p.caFile, + p.Settings.RegistryConfig, p.Settings.Debug) + if err != nil { + return out.String(), err + } + p.cfg.RegistryClient = registryClient + } + + // Don't use the default registry client if tls options are set. c.Options = append(c.Options, pusher.WithRegistryClient(p.cfg.RegistryClient)) } diff --git a/pkg/action/registry_login.go b/pkg/action/registry_login.go index 68bcc7442..3c9bd0bc5 100644 --- a/pkg/action/registry_login.go +++ b/pkg/action/registry_login.go @@ -24,7 +24,36 @@ import ( // RegistryLogin performs a registry login operation. type RegistryLogin struct { - cfg *Configuration + cfg *Configuration + certFile string + keyFile string + caFile string +} + +type RegistryLoginOpt func(*RegistryLogin) error + +// WithCertFile specifies the path to the certificate file to use for TLS. +func WithCertFile(certFile string) RegistryLoginOpt { + return func(r *RegistryLogin) error { + r.certFile = certFile + return nil + } +} + +// WithKeyFile specifies the path to the key file to use for TLS. +func WithKeyFile(keyFile string) RegistryLoginOpt { + return func(r *RegistryLogin) error { + r.keyFile = keyFile + return nil + } +} + +// WithCAFile specifies the path to the CA file to use for TLS. +func WithCAFile(caFile string) RegistryLoginOpt { + return func(r *RegistryLogin) error { + r.caFile = caFile + return nil + } } // NewRegistryLogin creates a new RegistryLogin object with the given configuration. @@ -35,9 +64,15 @@ func NewRegistryLogin(cfg *Configuration) *RegistryLogin { } // Run executes the registry login operation -func (a *RegistryLogin) Run(out io.Writer, hostname string, username string, password string, insecure bool) error { +func (a *RegistryLogin) Run(out io.Writer, hostname string, username string, password string, opts ...RegistryLoginOpt) error { + for _, opt := range opts { + if err := opt(a); err != nil { + return err + } + } + return a.cfg.RegistryClient.Login( hostname, registry.LoginOptBasicAuth(username, password), - registry.LoginOptInsecure(insecure)) + registry.LoginOptTLSClientConfig(a.certFile, a.keyFile, a.caFile)) } diff --git a/pkg/getter/ocigetter.go b/pkg/getter/ocigetter.go index 14f5cb3ec..d64ca0fb2 100644 --- a/pkg/getter/ocigetter.go +++ b/pkg/getter/ocigetter.go @@ -18,14 +18,22 @@ package getter import ( "bytes" "fmt" + "net" + "net/http" "strings" + "sync" + "time" + "helm.sh/helm/v3/internal/tlsutil" + "helm.sh/helm/v3/internal/urlutil" "helm.sh/helm/v3/pkg/registry" ) // OCIGetter is the default HTTP(/S) backend handler type OCIGetter struct { - opts options + opts options + transport *http.Transport + once sync.Once } // Get performs a Get from repo.Getter and returns the body. @@ -38,6 +46,15 @@ func (g *OCIGetter) Get(href string, options ...Option) (*bytes.Buffer, error) { func (g *OCIGetter) get(href string) (*bytes.Buffer, error) { client := g.opts.registryClient + // if the user has already provided a configured registry client, use it, + // this is particularly true when user has his own way of handling the client credentials. + if client == nil { + c, err := g.newRegistryClient() + if err != nil { + return nil, err + } + client = c + } ref := strings.TrimPrefix(href, fmt.Sprintf("%s://", registry.OCIScheme)) @@ -63,18 +80,7 @@ func (g *OCIGetter) get(href string) (*bytes.Buffer, error) { // NewOCIGetter constructs a valid http/https client as a Getter func NewOCIGetter(ops ...Option) (Getter, error) { - registryClient, err := registry.NewClient( - registry.ClientOptEnableCache(true), - ) - if err != nil { - return nil, err - } - - client := OCIGetter{ - opts: options{ - registryClient: registryClient, - }, - } + var client OCIGetter for _, opt := range ops { opt(&client.opts) @@ -82,3 +88,65 @@ func NewOCIGetter(ops ...Option) (Getter, error) { return &client, nil } + +func (g *OCIGetter) newRegistryClient() (*registry.Client, error) { + if g.opts.transport != nil { + client, err := registry.NewClient( + registry.ClientOptHTTPClient(&http.Client{ + Transport: g.opts.transport, + Timeout: g.opts.timeout, + }), + ) + if err != nil { + return nil, err + } + return client, nil + } + + g.once.Do(func() { + g.transport = &http.Transport{ + // From https://github.com/google/go-containerregistry/blob/31786c6cbb82d6ec4fb8eb79cd9387905130534e/pkg/v1/remote/options.go#L87 + DisableCompression: true, + DialContext: (&net.Dialer{ + // By default we wrap the transport in retries, so reduce the + // default dial timeout to 5s to avoid 5x 30s of connection + // timeouts when doing the "ping" on certain http registries. + Timeout: 5 * time.Second, + KeepAlive: 30 * time.Second, + }).DialContext, + ForceAttemptHTTP2: true, + MaxIdleConns: 100, + IdleConnTimeout: 90 * time.Second, + TLSHandshakeTimeout: 10 * time.Second, + ExpectContinueTimeout: 1 * time.Second, + } + }) + + if (g.opts.certFile != "" && g.opts.keyFile != "") || g.opts.caFile != "" { + tlsConf, err := tlsutil.NewClientTLS(g.opts.certFile, g.opts.keyFile, g.opts.caFile) + if err != nil { + return nil, fmt.Errorf("can't create TLS config for client: %w", err) + } + + sni, err := urlutil.ExtractHostname(g.opts.url) + if err != nil { + return nil, err + } + tlsConf.ServerName = sni + + g.transport.TLSClientConfig = tlsConf + } + + client, err := registry.NewClient( + registry.ClientOptHTTPClient(&http.Client{ + Transport: g.transport, + Timeout: g.opts.timeout, + }), + ) + + if err != nil { + return nil, err + } + + return client, nil +} diff --git a/pkg/getter/ocigetter_test.go b/pkg/getter/ocigetter_test.go index fc548b7a6..86f54b153 100644 --- a/pkg/getter/ocigetter_test.go +++ b/pkg/getter/ocigetter_test.go @@ -16,15 +16,124 @@ limitations under the License. package getter import ( + "net/http" + "path/filepath" "testing" + "time" + + "helm.sh/helm/v3/pkg/registry" ) -func TestNewOCIGetter(t *testing.T) { - testfn := func(ops *options) { - if ops.registryClient == nil { - t.Fatalf("the OCIGetter's registryClient should not be null") - } +func TestOCIGetter(t *testing.T) { + g, err := NewOCIGetter(WithURL("oci://example.com")) + if err != nil { + t.Fatal(err) + } + + if _, ok := g.(*OCIGetter); !ok { + t.Fatal("Expected NewOCIGetter to produce an *OCIGetter") + } + + cd := "../../testdata" + join := filepath.Join + ca, pub, priv := join(cd, "rootca.crt"), join(cd, "crt.pem"), join(cd, "key.pem") + timeout := time.Second * 5 + transport := &http.Transport{} + + // Test with options + g, err = NewOCIGetter( + WithBasicAuth("I", "Am"), + WithTLSClientConfig(pub, priv, ca), + WithTimeout(timeout), + WithTransport(transport), + ) + if err != nil { + t.Fatal(err) + } + + og, ok := g.(*OCIGetter) + if !ok { + t.Fatal("expected NewOCIGetter to produce an *OCIGetter") + } + + if og.opts.username != "I" { + t.Errorf("Expected NewOCIGetter to contain %q as the username, got %q", "I", og.opts.username) + } + + if og.opts.password != "Am" { + t.Errorf("Expected NewOCIGetter to contain %q as the password, got %q", "Am", og.opts.password) + } + + if og.opts.certFile != pub { + t.Errorf("Expected NewOCIGetter to contain %q as the public key file, got %q", pub, og.opts.certFile) + } + + if og.opts.keyFile != priv { + t.Errorf("Expected NewOCIGetter to contain %q as the private key file, got %q", priv, og.opts.keyFile) + } + + if og.opts.caFile != ca { + t.Errorf("Expected NewOCIGetter to contain %q as the CA file, got %q", ca, og.opts.caFile) + } + + if og.opts.timeout != timeout { + t.Errorf("Expected NewOCIGetter to contain %s as Timeout flag, got %s", timeout, og.opts.timeout) + } + + if og.opts.transport != transport { + t.Errorf("Expected NewOCIGetter to contain %p as Transport, got %p", transport, og.opts.transport) } - NewOCIGetter(testfn) + // Test if setting registryClient is being passed to the ops + registryClient, err := registry.NewClient() + if err != nil { + t.Fatal(err) + } + + g, err = NewOCIGetter( + WithRegistryClient(registryClient), + ) + if err != nil { + t.Fatal(err) + } + og, ok = g.(*OCIGetter) + if !ok { + t.Fatal("expected NewOCIGetter to produce an *OCIGetter") + } + + if og.opts.registryClient != registryClient { + t.Errorf("Expected NewOCIGetter to contain %p as RegistryClient, got %p", registryClient, og.opts.registryClient) + } +} + +func TestOCIHTTPTransportReuse(t *testing.T) { + g := OCIGetter{} + + _, err := g.newRegistryClient() + + if err != nil { + t.Fatal(err) + } + + if g.transport == nil { + t.Fatalf("Expected non nil value for transport") + } + + transport1 := g.transport + + _, err = g.newRegistryClient() + + if err != nil { + t.Fatal(err) + } + + if g.transport == nil { + t.Fatalf("Expected non nil value for transport") + } + + transport2 := g.transport + + if transport1 != transport2 { + t.Fatalf("Expected default transport to be reused") + } } diff --git a/pkg/pusher/ocipusher.go b/pkg/pusher/ocipusher.go index 7c90e85a4..a431577af 100644 --- a/pkg/pusher/ocipusher.go +++ b/pkg/pusher/ocipusher.go @@ -18,12 +18,16 @@ package pusher import ( "fmt" "io/ioutil" + "net" + "net/http" "os" "path" "strings" + "time" "github.com/pkg/errors" + "helm.sh/helm/v3/internal/tlsutil" "helm.sh/helm/v3/pkg/chart/loader" "helm.sh/helm/v3/pkg/registry" ) @@ -59,6 +63,13 @@ func (pusher *OCIPusher) push(chartRef, href string) error { } client := pusher.opts.registryClient + if client == nil { + c, err := pusher.newRegistryClient() + if err != nil { + return err + } + client = c + } chartBytes, err := ioutil.ReadFile(chartRef) if err != nil { @@ -85,18 +96,7 @@ func (pusher *OCIPusher) push(chartRef, href string) error { // NewOCIPusher constructs a valid OCI client as a Pusher func NewOCIPusher(ops ...Option) (Pusher, error) { - registryClient, err := registry.NewClient( - registry.ClientOptEnableCache(true), - ) - if err != nil { - return nil, err - } - - client := OCIPusher{ - opts: options{ - registryClient: registryClient, - }, - } + var client OCIPusher for _, opt := range ops { opt(&client.opts) @@ -104,3 +104,47 @@ func NewOCIPusher(ops ...Option) (Pusher, error) { return &client, nil } + +func (pusher *OCIPusher) newRegistryClient() (*registry.Client, error) { + if (pusher.opts.certFile != "" && pusher.opts.keyFile != "") || pusher.opts.caFile != "" { + tlsConf, err := tlsutil.NewClientTLS(pusher.opts.certFile, pusher.opts.keyFile, pusher.opts.caFile) + if err != nil { + return nil, errors.Wrap(err, "can't create TLS config for client") + } + + registryClient, err := registry.NewClient( + registry.ClientOptHTTPClient(&http.Client{ + // From https://github.com/google/go-containerregistry/blob/31786c6cbb82d6ec4fb8eb79cd9387905130534e/pkg/v1/remote/options.go#L87 + Transport: &http.Transport{ + Proxy: http.ProxyFromEnvironment, + DialContext: (&net.Dialer{ + // By default we wrap the transport in retries, so reduce the + // default dial timeout to 5s to avoid 5x 30s of connection + // timeouts when doing the "ping" on certain http registries. + Timeout: 5 * time.Second, + KeepAlive: 30 * time.Second, + }).DialContext, + ForceAttemptHTTP2: true, + MaxIdleConns: 100, + IdleConnTimeout: 90 * time.Second, + TLSHandshakeTimeout: 10 * time.Second, + ExpectContinueTimeout: 1 * time.Second, + TLSClientConfig: tlsConf, + }, + }), + registry.ClientOptEnableCache(true), + ) + if err != nil { + return nil, err + } + return registryClient, nil + } + + registryClient, err := registry.NewClient( + registry.ClientOptEnableCache(true), + ) + if err != nil { + return nil, err + } + return registryClient, nil +} diff --git a/pkg/pusher/ocipusher_test.go b/pkg/pusher/ocipusher_test.go index 27be15b5d..d718cec6d 100644 --- a/pkg/pusher/ocipusher_test.go +++ b/pkg/pusher/ocipusher_test.go @@ -16,21 +16,69 @@ limitations under the License. package pusher import ( + "path/filepath" "testing" + + "helm.sh/helm/v3/pkg/registry" ) func TestNewOCIPusher(t *testing.T) { - testfn := func(ops *options) { - if ops.registryClient == nil { - t.Fatalf("the OCIPusher's registryClient should not be null") - } + p, err := NewOCIPusher() + if err != nil { + t.Fatal(err) } - p, err := NewOCIPusher(testfn) - if p == nil { - t.Error("NewOCIPusher returned nil") + if _, ok := p.(*OCIPusher); !ok { + t.Fatal("Expected NewOCIPusher to produce an *OCIPusher") } + + cd := "../../testdata" + join := filepath.Join + ca, pub, priv := join(cd, "rootca.crt"), join(cd, "crt.pem"), join(cd, "key.pem") + + // Test with options + p, err = NewOCIPusher( + WithTLSClientConfig(pub, priv, ca), + ) if err != nil { - t.Error(err) + t.Fatal(err) + } + + op, ok := p.(*OCIPusher) + if !ok { + t.Fatal("Expected NewOCIPusher to produce an *OCIPusher") + } + + if op.opts.certFile != pub { + t.Errorf("Expected NewOCIPusher to contain %q as the public key file, got %q", pub, op.opts.certFile) + } + + if op.opts.keyFile != priv { + t.Errorf("Expected NewOCIPusher to contain %q as the private key file, got %q", priv, op.opts.keyFile) + } + + if op.opts.caFile != ca { + t.Errorf("Expected NewOCIPusher to contain %q as the CA file, got %q", ca, op.opts.caFile) + } + + // Test if setting registryClient is being passed to the ops + registryClient, err := registry.NewClient() + if err != nil { + t.Fatal(err) + } + + p, err = NewOCIPusher( + WithRegistryClient(registryClient), + ) + if err != nil { + t.Fatal(err) + } + op, ok = p.(*OCIPusher) + if !ok { + t.Fatal("expected NewOCIPusher to produce an *OCIPusher") + } + + if op.opts.registryClient != registryClient { + t.Errorf("Expected NewOCIPusher to contain %p as RegistryClient, got %p", registryClient, op.opts.registryClient) } } diff --git a/pkg/pusher/pusher.go b/pkg/pusher/pusher.go index 30c6af97c..86a5ebf90 100644 --- a/pkg/pusher/pusher.go +++ b/pkg/pusher/pusher.go @@ -28,6 +28,9 @@ import ( // Pushers may or may not ignore these parameters as they are passed in. type options struct { registryClient *registry.Client + certFile string + keyFile string + caFile string } // Option allows specifying various settings configurable by the user for overriding the defaults @@ -41,6 +44,15 @@ func WithRegistryClient(client *registry.Client) Option { } } +// WithTLSClientConfig sets the client auth with the provided credentials. +func WithTLSClientConfig(certFile, keyFile, caFile string) Option { + return func(opts *options) { + opts.certFile = certFile + opts.keyFile = keyFile + opts.caFile = caFile + } +} + // Pusher is an interface to support upload to the specified URL. type Pusher interface { // Push file content by url string diff --git a/pkg/registry/client.go b/pkg/registry/client.go index c1004f956..6505dadd2 100644 --- a/pkg/registry/client.go +++ b/pkg/registry/client.go @@ -61,6 +61,7 @@ type ( authorizer auth.Client registryAuthorizer *registryauth.Client resolver remotes.Resolver + httpClient *http.Client } // ClientOption allows specifying various settings configurable by the user for overriding the defaults @@ -90,6 +91,9 @@ func NewClient(options ...ClientOption) (*Client, error) { headers := http.Header{} headers.Set("User-Agent", version.GetUserAgent()) opts := []auth.ResolverOption{auth.WithResolverHeaders(headers)} + if client.httpClient != nil { + opts = append(opts, auth.WithResolverClient(client.httpClient)) + } resolver, err := client.authorizer.ResolverWithOpts(opts...) if err != nil { return nil, err @@ -104,6 +108,7 @@ func NewClient(options ...ClientOption) (*Client, error) { } if client.registryAuthorizer == nil { client.registryAuthorizer = ®istryauth.Client{ + Client: client.httpClient, Header: http.Header{ "User-Agent": {version.GetUserAgent()}, }, @@ -166,6 +171,13 @@ func ClientOptCredentialsFile(credentialsFile string) ClientOption { } } +// ClientOptHTTPClient returns a function that sets the httpClient setting on a client options set +func ClientOptHTTPClient(httpClient *http.Client) ClientOption { + return func(client *Client) { + client.httpClient = httpClient + } +} + type ( // LoginOption allows specifying various settings on login LoginOption func(*loginOperation) @@ -174,6 +186,9 @@ type ( username string password string insecure bool + certFile string + keyFile string + caFile string } ) @@ -189,6 +204,7 @@ func (c *Client) Login(host string, options ...LoginOption) error { auth.WithLoginUsername(operation.username), auth.WithLoginSecret(operation.password), auth.WithLoginUserAgent(version.GetUserAgent()), + auth.WithLoginTLS(operation.certFile, operation.keyFile, operation.caFile), } if operation.insecure { authorizerLoginOpts = append(authorizerLoginOpts, auth.WithLoginInsecure()) @@ -215,6 +231,15 @@ func LoginOptInsecure(insecure bool) LoginOption { } } +// LoginOptTLSClientConfig returns a function that sets the TLS settings on login. +func LoginOptTLSClientConfig(certFile, keyFile, caFile string) LoginOption { + return func(operation *loginOperation) { + operation.certFile = certFile + operation.keyFile = keyFile + operation.caFile = caFile + } +} + type ( // LogoutOption allows specifying various settings on logout LogoutOption func(*logoutOperation) diff --git a/pkg/registry/client_test.go b/pkg/registry/client_test.go index 138dd4245..4be6a1cd4 100644 --- a/pkg/registry/client_test.go +++ b/pkg/registry/client_test.go @@ -17,90 +17,21 @@ limitations under the License. package registry import ( - "bytes" - "context" "fmt" - "io" - "io/ioutil" - "net/http" - "net/http/httptest" - "net/url" "os" - "path/filepath" - "strings" "testing" - "time" "github.com/containerd/containerd/errdefs" - "github.com/distribution/distribution/v3/configuration" - "github.com/distribution/distribution/v3/registry" - _ "github.com/distribution/distribution/v3/registry/auth/htpasswd" - _ "github.com/distribution/distribution/v3/registry/storage/driver/inmemory" - "github.com/phayes/freeport" "github.com/stretchr/testify/suite" - "golang.org/x/crypto/bcrypt" -) - -var ( - testWorkspaceDir = "helm-registry-test" - testHtpasswdFileBasename = "authtest.htpasswd" - testUsername = "myuser" - testPassword = "mypass" ) type RegistryClientTestSuite struct { - suite.Suite - Out io.Writer - DockerRegistryHost string - CompromisedRegistryHost string - WorkspaceDir string - RegistryClient *Client + TestSuite } func (suite *RegistryClientTestSuite) SetupSuite() { - suite.WorkspaceDir = testWorkspaceDir - os.RemoveAll(suite.WorkspaceDir) - os.Mkdir(suite.WorkspaceDir, 0700) - - var out bytes.Buffer - suite.Out = &out - credentialsFile := filepath.Join(suite.WorkspaceDir, CredentialsFileBasename) - // init test client - var err error - suite.RegistryClient, err = NewClient( - ClientOptDebug(true), - ClientOptEnableCache(true), - ClientOptWriter(suite.Out), - ClientOptCredentialsFile(credentialsFile), - ) - suite.Nil(err, "no error creating registry client") - - // create htpasswd file (w BCrypt, which is required) - pwBytes, err := bcrypt.GenerateFromPassword([]byte(testPassword), bcrypt.DefaultCost) - suite.Nil(err, "no error generating bcrypt password for test htpasswd file") - htpasswdPath := filepath.Join(suite.WorkspaceDir, testHtpasswdFileBasename) - err = ioutil.WriteFile(htpasswdPath, []byte(fmt.Sprintf("%s:%s\n", testUsername, string(pwBytes))), 0644) - suite.Nil(err, "no error creating test htpasswd file") - - // Registry config - config := &configuration.Configuration{} - 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("127.0.0.1:%d", port) - config.HTTP.DrainTimeout = time.Duration(10) * time.Second - config.Storage = map[string]configuration.Parameters{"inmemory": map[string]interface{}{}} - config.Auth = configuration.Auth{ - "htpasswd": configuration.Parameters{ - "realm": "localhost", - "path": htpasswdPath, - }, - } - dockerRegistry, err := registry.NewRegistry(context.Background(), config) - suite.Nil(err, "no error creating test registry") - - suite.CompromisedRegistryHost = initCompromisedRegistryTestServer() + dockerRegistry := setup(&suite.TestSuite, false) // Start Docker registry go dockerRegistry.ListenAndServe() @@ -133,182 +64,15 @@ func (suite *RegistryClientTestSuite) Test_0_Login() { } func (suite *RegistryClientTestSuite) Test_1_Push() { - // Bad bytes - ref := fmt.Sprintf("%s/testrepo/testchart:1.2.3", suite.DockerRegistryHost) - _, err := suite.RegistryClient.Push([]byte("hello"), ref) - suite.NotNil(err, "error pushing non-chart bytes") - - // Load a test chart - chartData, err := ioutil.ReadFile("../repo/repotest/testdata/examplechart-0.1.0.tgz") - suite.Nil(err, "no error loading test chart") - meta, err := extractChartMeta(chartData) - suite.Nil(err, "no error extracting chart meta") - - // non-strict ref (chart name) - ref = fmt.Sprintf("%s/testrepo/boop:%s", suite.DockerRegistryHost, meta.Version) - _, err = suite.RegistryClient.Push(chartData, ref) - suite.NotNil(err, "error pushing non-strict ref (bad basename)") - - // non-strict ref (chart name), with strict mode disabled - _, err = suite.RegistryClient.Push(chartData, ref, PushOptStrictMode(false)) - suite.Nil(err, "no error pushing non-strict ref (bad basename), with strict mode disabled") - - // non-strict ref (chart version) - ref = fmt.Sprintf("%s/testrepo/%s:latest", suite.DockerRegistryHost, meta.Name) - _, err = suite.RegistryClient.Push(chartData, ref) - suite.NotNil(err, "error pushing non-strict ref (bad tag)") - - // non-strict ref (chart version), with strict mode disabled - _, err = suite.RegistryClient.Push(chartData, ref, PushOptStrictMode(false)) - suite.Nil(err, "no error pushing non-strict ref (bad tag), with strict mode disabled") - - // basic push, good ref - chartData, err = ioutil.ReadFile("../downloader/testdata/local-subchart-0.1.0.tgz") - suite.Nil(err, "no error loading test chart") - meta, err = extractChartMeta(chartData) - suite.Nil(err, "no error extracting chart meta") - ref = fmt.Sprintf("%s/testrepo/%s:%s", suite.DockerRegistryHost, meta.Name, meta.Version) - _, err = suite.RegistryClient.Push(chartData, ref) - suite.Nil(err, "no error pushing good ref") - - _, err = suite.RegistryClient.Pull(ref) - suite.Nil(err, "no error pulling a simple chart") - - // Load another test chart - chartData, err = ioutil.ReadFile("../downloader/testdata/signtest-0.1.0.tgz") - suite.Nil(err, "no error loading test chart") - meta, err = extractChartMeta(chartData) - suite.Nil(err, "no error extracting chart meta") - - // Load prov file - provData, err := ioutil.ReadFile("../downloader/testdata/signtest-0.1.0.tgz.prov") - suite.Nil(err, "no error loading test prov") - - // push with prov - ref = fmt.Sprintf("%s/testrepo/%s:%s", suite.DockerRegistryHost, meta.Name, meta.Version) - result, err := suite.RegistryClient.Push(chartData, ref, PushOptProvData(provData)) - suite.Nil(err, "no error pushing good ref with prov") - - _, err = suite.RegistryClient.Pull(ref) - suite.Nil(err, "no error pulling a simple chart") - - // Validate the output - // Note: these digests/sizes etc may change if the test chart/prov files are modified, - // or if the format of the OCI manifest changes - suite.Equal(ref, result.Ref) - suite.Equal(meta.Name, result.Chart.Meta.Name) - suite.Equal(meta.Version, result.Chart.Meta.Version) - suite.Equal(int64(512), result.Manifest.Size) - suite.Equal(int64(99), result.Config.Size) - suite.Equal(int64(973), result.Chart.Size) - suite.Equal(int64(695), result.Prov.Size) - suite.Equal( - "sha256:af4c20a1df1431495e673c14ecfa3a2ba24839a7784349d6787cd67957392e83", - result.Manifest.Digest) - suite.Equal( - "sha256:8d17cb6bf6ccd8c29aace9a658495cbd5e2e87fc267876e86117c7db681c9580", - result.Config.Digest) - suite.Equal( - "sha256:e5ef611620fb97704d8751c16bab17fedb68883bfb0edc76f78a70e9173f9b55", - result.Chart.Digest) - suite.Equal( - "sha256:b0a02b7412f78ae93324d48df8fcc316d8482e5ad7827b5b238657a29a22f256", - result.Prov.Digest) + testPush(&suite.TestSuite) } func (suite *RegistryClientTestSuite) Test_2_Pull() { - // bad/missing ref - ref := fmt.Sprintf("%s/testrepo/no-existy:1.2.3", suite.DockerRegistryHost) - _, err := suite.RegistryClient.Pull(ref) - suite.NotNil(err, "error on bad/missing ref") - - // Load test chart (to build ref pushed in previous test) - chartData, err := ioutil.ReadFile("../downloader/testdata/local-subchart-0.1.0.tgz") - suite.Nil(err, "no error loading test chart") - meta, err := extractChartMeta(chartData) - suite.Nil(err, "no error extracting chart meta") - ref = fmt.Sprintf("%s/testrepo/%s:%s", suite.DockerRegistryHost, meta.Name, meta.Version) - - // Simple pull, chart only - _, err = suite.RegistryClient.Pull(ref) - suite.Nil(err, "no error pulling a simple chart") - - // Simple pull with prov (no prov uploaded) - _, err = suite.RegistryClient.Pull(ref, PullOptWithProv(true)) - suite.NotNil(err, "error pulling a chart with prov when no prov exists") - - // Simple pull with prov, ignoring missing prov - _, err = suite.RegistryClient.Pull(ref, - PullOptWithProv(true), - PullOptIgnoreMissingProv(true)) - suite.Nil(err, - "no error pulling a chart with prov when no prov exists, ignoring missing") - - // Load test chart (to build ref pushed in previous test) - chartData, err = ioutil.ReadFile("../downloader/testdata/signtest-0.1.0.tgz") - suite.Nil(err, "no error loading test chart") - meta, err = extractChartMeta(chartData) - suite.Nil(err, "no error extracting chart meta") - ref = fmt.Sprintf("%s/testrepo/%s:%s", suite.DockerRegistryHost, meta.Name, meta.Version) - - // Load prov file - provData, err := ioutil.ReadFile("../downloader/testdata/signtest-0.1.0.tgz.prov") - suite.Nil(err, "no error loading test prov") - - // no chart and no prov causes error - _, err = suite.RegistryClient.Pull(ref, - PullOptWithChart(false), - PullOptWithProv(false)) - suite.NotNil(err, "error on both no chart and no prov") - - // full pull with chart and prov - result, err := suite.RegistryClient.Pull(ref, PullOptWithProv(true)) - suite.Nil(err, "no error pulling a chart with prov") - - // Validate the output - // Note: these digests/sizes etc may change if the test chart/prov files are modified, - // or if the format of the OCI manifest changes - suite.Equal(ref, result.Ref) - suite.Equal(meta.Name, result.Chart.Meta.Name) - suite.Equal(meta.Version, result.Chart.Meta.Version) - suite.Equal(int64(512), result.Manifest.Size) - suite.Equal(int64(99), result.Config.Size) - suite.Equal(int64(973), result.Chart.Size) - suite.Equal(int64(695), result.Prov.Size) - suite.Equal( - "sha256:af4c20a1df1431495e673c14ecfa3a2ba24839a7784349d6787cd67957392e83", - result.Manifest.Digest) - suite.Equal( - "sha256:8d17cb6bf6ccd8c29aace9a658495cbd5e2e87fc267876e86117c7db681c9580", - result.Config.Digest) - suite.Equal( - "sha256:e5ef611620fb97704d8751c16bab17fedb68883bfb0edc76f78a70e9173f9b55", - result.Chart.Digest) - suite.Equal( - "sha256:b0a02b7412f78ae93324d48df8fcc316d8482e5ad7827b5b238657a29a22f256", - result.Prov.Digest) - suite.Equal("{\"schemaVersion\":2,\"config\":{\"mediaType\":\"application/vnd.cncf.helm.config.v1+json\",\"digest\":\"sha256:8d17cb6bf6ccd8c29aace9a658495cbd5e2e87fc267876e86117c7db681c9580\",\"size\":99},\"layers\":[{\"mediaType\":\"application/vnd.cncf.helm.chart.provenance.v1.prov\",\"digest\":\"sha256:b0a02b7412f78ae93324d48df8fcc316d8482e5ad7827b5b238657a29a22f256\",\"size\":695},{\"mediaType\":\"application/vnd.cncf.helm.chart.content.v1.tar+gzip\",\"digest\":\"sha256:e5ef611620fb97704d8751c16bab17fedb68883bfb0edc76f78a70e9173f9b55\",\"size\":973}]}", - string(result.Manifest.Data)) - suite.Equal("{\"name\":\"signtest\",\"version\":\"0.1.0\",\"description\":\"A Helm chart for Kubernetes\",\"apiVersion\":\"v1\"}", - string(result.Config.Data)) - suite.Equal(chartData, result.Chart.Data) - suite.Equal(provData, result.Prov.Data) + testPull(&suite.TestSuite) } func (suite *RegistryClientTestSuite) Test_3_Tags() { - - // Load test chart (to build ref pushed in previous test) - chartData, err := ioutil.ReadFile("../downloader/testdata/local-subchart-0.1.0.tgz") - suite.Nil(err, "no error loading test chart") - meta, err := extractChartMeta(chartData) - suite.Nil(err, "no error extracting chart meta") - ref := fmt.Sprintf("%s/testrepo/%s", suite.DockerRegistryHost, meta.Name) - - // Query for tags and validate length - tags, err := suite.RegistryClient.Tags(ref) - suite.Nil(err, "no error retrieving tags") - suite.Equal(1, len(tags)) - + testTags(&suite.TestSuite) } func (suite *RegistryClientTestSuite) Test_4_Logout() { @@ -331,43 +95,3 @@ func (suite *RegistryClientTestSuite) Test_5_ManInTheMiddle() { func TestRegistryClientTestSuite(t *testing.T) { suite.Run(t, new(RegistryClientTestSuite)) } - -func initCompromisedRegistryTestServer() string { - s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if strings.Contains(r.URL.Path, "manifests") { - w.Header().Set("Content-Type", "application/vnd.oci.image.manifest.v1+json") - w.WriteHeader(200) - - // layers[0] is the blob []byte("a") - w.Write([]byte( - fmt.Sprintf(`{ "schemaVersion": 2, "config": { - "mediaType": "%s", - "digest": "sha256:a705ee2789ab50a5ba20930f246dbd5cc01ff9712825bb98f57ee8414377f133", - "size": 181 - }, - "layers": [ - { - "mediaType": "%s", - "digest": "sha256:ca978112ca1bbdcafac231b39a23dc4da786eff8147c4e72b9807785afee48bb", - "size": 1 - } - ] -}`, ConfigMediaType, ChartLayerMediaType))) - } else if r.URL.Path == "/v2/testrepo/supposedlysafechart/blobs/sha256:a705ee2789ab50a5ba20930f246dbd5cc01ff9712825bb98f57ee8414377f133" { - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(200) - w.Write([]byte("{\"name\":\"mychart\",\"version\":\"0.1.0\",\"description\":\"A Helm chart for Kubernetes\\n" + - "an 'application' or a 'library' chart.\",\"apiVersion\":\"v2\",\"appVersion\":\"1.16.0\",\"type\":" + - "\"application\"}")) - } else if r.URL.Path == "/v2/testrepo/supposedlysafechart/blobs/sha256:ca978112ca1bbdcafac231b39a23dc4da786eff8147c4e72b9807785afee48bb" { - w.Header().Set("Content-Type", ChartLayerMediaType) - w.WriteHeader(200) - w.Write([]byte("b")) - } else { - w.WriteHeader(500) - } - })) - - u, _ := url.Parse(s.URL) - return fmt.Sprintf("localhost:%s", u.Port()) -} diff --git a/pkg/registry/client_tls_test.go b/pkg/registry/client_tls_test.go new file mode 100644 index 000000000..3be0419bf --- /dev/null +++ b/pkg/registry/client_tls_test.go @@ -0,0 +1,81 @@ +/* +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 registry + +import ( + "os" + "testing" + + "github.com/stretchr/testify/suite" +) + +type TLSRegistryClientTestSuite struct { + TestSuite +} + +func (suite *TLSRegistryClientTestSuite) SetupSuite() { + // init test client + dockerRegistry := setup(&suite.TestSuite, true) + + // Start Docker registry + go dockerRegistry.ListenAndServe() +} + +func (suite *TLSRegistryClientTestSuite) TearDownSuite() { + os.RemoveAll(suite.WorkspaceDir) +} + +func (suite *TLSRegistryClientTestSuite) Test_0_Login() { + err := suite.RegistryClient.Login(suite.DockerRegistryHost, + LoginOptBasicAuth("badverybad", "ohsobad"), + LoginOptTLSClientConfig(tlsCert, tlsKey, tlsCA)) + suite.NotNil(err, "error logging into registry with bad credentials") + + err = suite.RegistryClient.Login(suite.DockerRegistryHost, + LoginOptBasicAuth(testUsername, testPassword), + LoginOptTLSClientConfig(tlsCert, tlsKey, tlsCA)) + suite.Nil(err, "no error logging into registry with good credentials") + + err = suite.RegistryClient.Login(suite.DockerRegistryHost, + LoginOptBasicAuth(testUsername, testPassword), + LoginOptTLSClientConfig(tlsCert, tlsKey, tlsCA)) + suite.Nil(err, "no error logging into registry with good credentials, insecure mode") +} + +func (suite *TLSRegistryClientTestSuite) Test_1_Push() { + testPush(&suite.TestSuite) +} + +func (suite *TLSRegistryClientTestSuite) Test_2_Pull() { + testPull(&suite.TestSuite) +} + +func (suite *TLSRegistryClientTestSuite) Test_3_Tags() { + testTags(&suite.TestSuite) +} + +func (suite *TLSRegistryClientTestSuite) Test_4_Logout() { + err := suite.RegistryClient.Logout("this-host-aint-real:5000") + suite.NotNil(err, "error logging out of registry that has no entry") + + err = suite.RegistryClient.Logout(suite.DockerRegistryHost) + suite.Nil(err, "no error logging out of registry") +} + +func TestTLSRegistryClientTestSuite(t *testing.T) { + suite.Run(t, new(TLSRegistryClientTestSuite)) +} diff --git a/pkg/registry/testdata/tls/ca-cert.pem b/pkg/registry/testdata/tls/ca-cert.pem new file mode 100644 index 000000000..b2f4fe107 --- /dev/null +++ b/pkg/registry/testdata/tls/ca-cert.pem @@ -0,0 +1,21 @@ +-----BEGIN CERTIFICATE----- +MIIDhzCCAm+gAwIBAgIUdI/ees1mQ4N++1jpF5xI5fq6TSUwDQYJKoZIhvcNAQEL +BQAwUjELMAkGA1UEBhMCVVMxCzAJBgNVBAgMAkNBMQswCQYDVQQHDAJTRjENMAsG +A1UECgwEaGVsbTEaMBgGA1UEAwwRcmVnaXN0cnktdGVzdC5jb20wIBcNMjIwOTIw +MDgyMDQ2WhgPMzAyMjAxMjEwODIwNDZaMFIxCzAJBgNVBAYTAlVTMQswCQYDVQQI +DAJDQTELMAkGA1UEBwwCU0YxDTALBgNVBAoMBGhlbG0xGjAYBgNVBAMMEXJlZ2lz +dHJ5LXRlc3QuY29tMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA0mxP +WVkpDo3PnXalJhy9rSYuK8OIxcO1kBroEnILYrNWn5zpKioaBXZEYcaU6crc5N4j +wQRC16wucyQAQh/d3ty7j5Wyy79CgH5AAKDbCacii4BgGUJ2xY6UXuKvwdsROAXN +wEtXT5f3yO8bVboYrZRxJ4UuTUFndtuz2b230JFs2FzTv4QdLaPHo/S4FTW5xRn5 +Irhmcmkns+XY4AduscYtzydvIuuOS3CVmB8/sClo62F5DpBl68b+/WFwqLrkX5Sn +ZWKx/fJPIxln5SavPXHEEcI14ZGNUhsv+4+sABHzVjBPK8oKjoNo8QmxDWdeWPgR +sPj/H2oldE6KfgyoQQIDAQABo1MwUTAdBgNVHQ4EFgQUkkmPK6SIj4PY8YOw+Yer +hKCOS7owHwYDVR0jBBgwFoAUkkmPK6SIj4PY8YOw+YerhKCOS7owDwYDVR0TAQH/ +BAUwAwEB/zANBgkqhkiG9w0BAQsFAAOCAQEADSz9s8rcObLrUo8DpVRptWUxK3NH +hvD7bYGQ9eJO9B4ojKSBKJRchP0m5kpVLorMRZDRw17T2GouKQn3g+Wcy+8CygxW +1JDO/1iCZ8QX3vfwIfHTaKuY6eYcJyVmxL58bRI3qQNRZIU4s18tKFIazBluxS3g +5Wp8kOCBssttsM+lEgC/cj7skl9CBKhUFupHPzXzha+1upJUK51Egc7M7nsrnpaZ +2SY+PBEhSY5Wcuzb5m9tw7PJnkdRDS/dUOY6kSzJXgNMVV0GnN+Smucqmvrez0M5 +vHFMiQjlRxViVLJDNOCJYIjWNygAOvhJyRU2cTodIhZ/jbYqpNGAPc5Eyg== +-----END CERTIFICATE----- diff --git a/pkg/registry/testdata/tls/client-cert.pem b/pkg/registry/testdata/tls/client-cert.pem new file mode 100644 index 000000000..f541fcd54 --- /dev/null +++ b/pkg/registry/testdata/tls/client-cert.pem @@ -0,0 +1,22 @@ +-----BEGIN CERTIFICATE----- +MIIDsTCCApmgAwIBAgIBATANBgkqhkiG9w0BAQsFADBSMQswCQYDVQQGEwJVUzEL +MAkGA1UECAwCQ0ExCzAJBgNVBAcMAlNGMQ0wCwYDVQQKDARoZWxtMRowGAYDVQQD +DBFyZWdpc3RyeS10ZXN0LmNvbTAgFw0yMjA5MjAwODI4MzBaGA8yMTIyMDgyNzA4 +MjgzMFowWTELMAkGA1UEBhMCVVMxCzAJBgNVBAgMAkNBMQswCQYDVQQHDAJTRjEN +MAsGA1UECgwEaGVsbTEhMB8GA1UEAwwYY2xpZW50LnJlZ2lzdHJ5LXRlc3QuY29t +MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAnvxfrJn8PeerlHJLnMVo +p1yOT/kvFAoNhObhtDUosDLjQBt+vICfjWoTNIabIiBRTwkVt5CdGvx1oKsbH3iT +VErL6N6MagIJdnOfBjxtlTL/TFtJ7U/VSUSxZwa+SV6HS4cmIntC/FV3MHjBlFJn +klSdDXa5YdYE2xuSPse+zlGRfmPTNmHsiNWphGC54U6WZ1UI0G22+L/yO8BuEkSq +47iCN6ZIw8ds+azl/woIEDJsVSgEapNsanBrJFnBUJBXh4lwpMB37U+6Ds1kUUuz +GXhVWz1pmRBt+vXWN802MqRg2RnCjTb2gWbmg7En4uFCTzx/GhRlJiV47O15n0g+ +tQIDAQABo4GIMIGFMB8GA1UdIwQYMBaAFJJJjyukiI+D2PGDsPmHq4Sgjku6MAkG +A1UdEwQCMAAwCwYDVR0PBAQDAgTwMCsGA1UdEQQkMCKCCWxvY2FsaG9zdIIKMHg3 +ZjAwMDAwMYIJMTI3LjAuMC4xMB0GA1UdDgQWBBT+cCGLyj5wOIMG7TVqPyxPQsBi ++DANBgkqhkiG9w0BAQsFAAOCAQEATIDXr3LmD1S+13lVG263rn21cDT3m4VycQCu +oGNDuxtFwd/Zn/XnZLk2r1msz6YXWUqErJ8C7Ea7fFdimoJR5V3m7LYrYRPeLYVn +aVqyNN4LD48Su3VO5sjTyFxXJJJ9C5HX8LU/Pw/517qzLOFrmsO/fXN/XE52erBE ++K6vX4lyxnZyPfl3A/X/33G2tsGtHFK1uBILpn29fpeC/Pgm3Nj8ZqQ8rtcLZbog +heqdKkHKWdL3i1deplwxT7xVnqsWszU6Znzm/C/VQSB4Isn4puQDKqVPwGobHgxY +1zZr5mueot8mX9Qmg8IcWOVZ2u7nz8lw6+wpabkyjjdTC6iizg== +-----END CERTIFICATE----- diff --git a/pkg/registry/testdata/tls/client-key.pem b/pkg/registry/testdata/tls/client-key.pem new file mode 100644 index 000000000..7e7ace54f --- /dev/null +++ b/pkg/registry/testdata/tls/client-key.pem @@ -0,0 +1,28 @@ +-----BEGIN PRIVATE KEY----- +MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQCe/F+smfw956uU +ckucxWinXI5P+S8UCg2E5uG0NSiwMuNAG368gJ+NahM0hpsiIFFPCRW3kJ0a/HWg +qxsfeJNUSsvo3oxqAgl2c58GPG2VMv9MW0ntT9VJRLFnBr5JXodLhyYie0L8VXcw +eMGUUmeSVJ0Ndrlh1gTbG5I+x77OUZF+Y9M2YeyI1amEYLnhTpZnVQjQbbb4v/I7 +wG4SRKrjuII3pkjDx2z5rOX/CggQMmxVKARqk2xqcGskWcFQkFeHiXCkwHftT7oO +zWRRS7MZeFVbPWmZEG369dY3zTYypGDZGcKNNvaBZuaDsSfi4UJPPH8aFGUmJXjs +7XmfSD61AgMBAAECggEAKYp/5TWG9xXlezAyGZBrO++vL65IYtANoEBDkTainwds +4X9NqithhS3GPt89Abm4BRK2nfQnWLnGcmjC+YIj3M5+YSZlQf2uQ0kKsDJx354n +nufrdRp6/F36jJTye3E7oLx7dl8GrbAXKI8k5YByl4WMU8xFvA6TzjxyBf1jGb1E +8JBZpnqwSHgtH0zGPqgcIsqmQjiMJ+wHNZxdvtjPPC8exy/yLL9Hhj2UaqZSMMRi +afaAFXBLNvJ6Y/SUjRaL9liAyTQ0kJ+xR6TMDJ7ix0toGlylsK/3YesXEgAyui6c +UC3dmSC4UDJW+fGLrj/hVBLdpMRpgrWzwXnRyr0RMQKBgQDDnJqAtULhlo0W4E29 +Oo7XYFEcilzxB3hxEQSmts53GeQZHo1gI4wthyMzAgY3uOCIUtB2lPkNLV+dU86A +Cy1WTRL2vbwdM1qHz2tls4LNa+k+XTMWX7aqfCzOydBpV3Yehmnzb4NvFn9+QHjp +5omwwOaG7dhJCVet3CUJctoeOwKBgQDQETAVd4xfwQ/cBbKgoQhrkHOr+gTWcKYP +WD86EFDbRVboYDevU/dAj5Vwm5763zRsBFyL6/ZVUr9Wa1HHy0paE5YfdewMrRje +LhHeTbrLJ4Q3I0ix3bawv/04B66hw+Yaom0bQV3gBrNk+Cn8VFAo6IKNy7A0pK3i +KQmwoO+XzwKBgC3EqInQ33M07JIbrVTHLMDL8m6BGTn0C4Q4/SOcxjYrwqj18xI5 +fwTwB5ZZtOa4xSBgcBIuzQ7+PM7s2vYup073/aXpwuf6KgZ4y6IiHErAIvTKjbeA +cZb2Mu23XqInKqX9wTCKOPB3DSGXKDNiE3ldyRJs+BwuqWsuhSPu0YYdAoGADjd+ +b5kRkGFisgf5opweNStTnAajWfusfRPsjg0bWUAtpgcdBu/XzyOAdIdNn5qsvEy3 +/h+LX10eEcuXdO1hETKRaWjnTh5tupCvS99HyiXTFOlmSDD8EKuto6xytD7sdBlx +FxGqVmpey6FhTQp9x63LbeDjE1XFQ9TGArmcZWUCgYEAprSfhSemz9tP5tKKdYTc +LM5eWqK0aB1sN/hCZVx86VcNBxRbV+POEASTYO9AyVMjthGRe6UnCjwdXKTJ/ToX +KdtXINYeeK3hzANeCvtqg81qxi+8nmNLimtcjvFsB5g44LOFYyXqAD5FeQYTog1n +t/TLHYY+S8BbJ9cXfObXqyE= +-----END PRIVATE KEY----- diff --git a/pkg/registry/testdata/tls/server-cert.pem b/pkg/registry/testdata/tls/server-cert.pem new file mode 100644 index 000000000..8d2eda528 --- /dev/null +++ b/pkg/registry/testdata/tls/server-cert.pem @@ -0,0 +1,22 @@ +-----BEGIN CERTIFICATE----- +MIIDsTCCApmgAwIBAgIBATANBgkqhkiG9w0BAQsFADBSMQswCQYDVQQGEwJVUzEL +MAkGA1UECAwCQ0ExCzAJBgNVBAcMAlNGMQ0wCwYDVQQKDARoZWxtMRowGAYDVQQD +DBFyZWdpc3RyeS10ZXN0LmNvbTAgFw0yMjA5MjAwODI3NDZaGA8yMTIyMDgyNzA4 +Mjc0NlowWTELMAkGA1UEBhMCVVMxCzAJBgNVBAgMAkNBMQswCQYDVQQHDAJTRjEN +MAsGA1UECgwEaGVsbTEhMB8GA1UEAwwYc2VydmVyLnJlZ2lzdHJ5LXRlc3QuY29t +MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAxve7spJ44uC/f6BCUEKQ +PA9Sqc+ulTXyptZROLa90o7GK9P1WW8hcDRIYaIU3Rh+o6E0QYwBwvspoEAKYP0q +kp16pD1Ezf5VTikVElq20qvYOaAjvxFltIAmrxoCokkwEIsgEY6RYHZedimKWtdg +kG7R0aNnwgognoz6j4GD/Z/HejCY54jckQczDdaxWrcbBdQ0h/WNjLwHmlids4H9 +ni4cas4An5TZ3cOA9ah+8PSRNYgSLFR34KuydLd8xx5E2fG8OuU5zCNaDQ4puYKP +u+D6GNCdwi+w+Ac/3MTAX8ORLrB/8BCIMwnYi7g7En4a47ck21VqhfE+CH10AR07 +nQIDAQABo4GIMIGFMB8GA1UdIwQYMBaAFJJJjyukiI+D2PGDsPmHq4Sgjku6MAkG +A1UdEwQCMAAwCwYDVR0PBAQDAgTwMCsGA1UdEQQkMCKCCWxvY2FsaG9zdIIKMHg3 +ZjAwMDAwMYIJMTI3LjAuMC4xMB0GA1UdDgQWBBRoIiJ5S3EJmcNUmjT+dxWO+14k +ADANBgkqhkiG9w0BAQsFAAOCAQEAb6UOBss8IA3uT76LIK9TSNSyn6BoYlTFGwgx +O2Cp4kqyKb370qAWV1QVVefQP1uftXpsdqhtwEL4jUptYO5yP4Udtg0QV0SsyMsg +jXgaeuC7589lcJpmTvPj/XlnAZE6vmTrVPG4c1wEC+qCTSHAu3EBRN8hHKZFmLON +254/6x2HlSTqwKzzJY5YEL8pP1kAIww40YMd5G5gFqCNdcg2FKB3ZWo9cFzCU3VK +HoeOUG286GuEN6AG/YT2DIFAZpP+SUgjY8mj1CxoIv9LMNyF1Tm8kzQDU0IA2dfW +1AY0edoHL2kLoUUKet/d7tayP9gnt0sOUrY2oZXrp+TvSHVTlw== +-----END CERTIFICATE----- diff --git a/pkg/registry/testdata/tls/server-key.pem b/pkg/registry/testdata/tls/server-key.pem new file mode 100644 index 000000000..28bcbe214 --- /dev/null +++ b/pkg/registry/testdata/tls/server-key.pem @@ -0,0 +1,28 @@ +-----BEGIN PRIVATE KEY----- +MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQDG97uyknji4L9/ +oEJQQpA8D1Kpz66VNfKm1lE4tr3SjsYr0/VZbyFwNEhhohTdGH6joTRBjAHC+ymg +QApg/SqSnXqkPUTN/lVOKRUSWrbSq9g5oCO/EWW0gCavGgKiSTAQiyARjpFgdl52 +KYpa12CQbtHRo2fCCiCejPqPgYP9n8d6MJjniNyRBzMN1rFatxsF1DSH9Y2MvAea +WJ2zgf2eLhxqzgCflNndw4D1qH7w9JE1iBIsVHfgq7J0t3zHHkTZ8bw65TnMI1oN +Dim5go+74PoY0J3CL7D4Bz/cxMBfw5EusH/wEIgzCdiLuDsSfhrjtyTbVWqF8T4I +fXQBHTudAgMBAAECggEAD13Tr7tzPaZ487znUjaJ2DGgwz+obpqvhmYX+MbYSzo+ +oOTqVoFoNje7fVrcvKSnJzEMjaFoA2yNbvRzOMFkt9UUwzl+JmClqvcuSvAZnZSr +CuxMxnVsAvBAzJY4LNt1LFnqXKDDpo0Nx5d2uYRXz1/XsZaqrUhF86jUsx+gF4bM +LYe6SjXWtf1sumgE1gbil8NDLbqHPMvimQhLu1WgVxiarlye2NMyHxk6MTqwYOX3 +iinf3cuRFYuFyD1IHorreVAdOH0zuYvqLFylBbRqEfeOozVytX73yKfRK4lPobc+ +Q1n/mPzwyc9aVWKRo4WId0mA2rhP8sL7BvMFRwYnSwKBgQDdUqlel4/Fj2WfcsKa +SMjmqM66tFDxH27Vp55RoS/Fr+RZSVYda7cdbMJaGVswbZevwsCS46l2BJJdJXHt +UE1viKkKiIxGJzpH9Q1vyUEf+21eESnkr7HKoUrSpopwqOlc1dYPvn47aJukcGee +vwMkiaG5IUaR5MCfLA8xQ89UPwKBgQDmJGWtrwcUIdEvRI1wg8Unj0chAyz+/KIR +9jkVIyu4SUfThQp6GsCHsvc5TGN6yieGLIfrVb7qb8F2gDPdg8L/13zqAorpcK6E +AagYLDgKWV4O2oGT4AGQrcz/66BYAfeD868r442bhyEkD7zLqZSbHlPTpy8bPKuC +nen88JGJIwKBgD/OawHYVByywKt9XFk6jqDhHeh5v7QkScHS9zO1cp5dnUmYePk2 +aq5TAp0THlUR419KmFZAyEQ8AS5Vc0jlk82J6qIcx8QZ3xWLsnn93Yao59jsvdUu +SeWPJpEgbl0YdV7MT1BurNnXyLdZqKX9j5xjCXrj+wJonpfFDgQ39nflAoGAd1bo +YuggA5CFqL0jmvS5h4oEmFnNO2xFnorPjuZuBWH6nPSgOjElJTjoeg3iiAnL9Qei +c6ZDGc5Zw9k3C+cHdyOG4tHutp534Hv7bo1/gd5Vp94m00eViDCX3R2SSBC9CO+U +Jm4ZQE0SImEGxZVqOgW/8kD/bGBJj7HTZBZbYYECgYEAoGwLnE2TiMLfXIKXsmII +h9+rZrPfFyDCM27+QIADpCv7Ae2cIGanqSbyPJrFWD4CRXBv+92L2LyG7yA9C498 +uyMJ98DVp4SAaNWFha+JCz5TO6KCXOuwGrQTSUitqxQ2rMv2WpXnO2T8puvXW8dD +mxfiHuvNMNHfA9Bd4tsbbPE= +-----END PRIVATE KEY----- diff --git a/pkg/registry/util.go b/pkg/registry/util.go index 47eed267f..025b145ba 100644 --- a/pkg/registry/util.go +++ b/pkg/registry/util.go @@ -21,6 +21,7 @@ import ( "context" "fmt" "io" + "net/http" "strings" "github.com/Masterminds/semver/v3" @@ -29,6 +30,7 @@ import ( orascontext "oras.land/oras-go/pkg/context" "oras.land/oras-go/pkg/registry" + "helm.sh/helm/v3/internal/tlsutil" "helm.sh/helm/v3/pkg/chart" "helm.sh/helm/v3/pkg/chart/loader" ) @@ -129,3 +131,27 @@ func parseReference(raw string) (registry.Reference, error) { return registry.ParseReference(raw) } + +// NewRegistryClientWithTLS is a helper function to create a new registry client with TLS enabled. +func NewRegistryClientWithTLS(out io.Writer, certFile, keyFile, caFile string, registryConfig string, debug bool) (*Client, error) { + tlsConf, err := tlsutil.NewClientTLS(certFile, keyFile, caFile) + if err != nil { + return nil, fmt.Errorf("can't create TLS config for client: %s", err) + } + // Create a new registry client + registryClient, err := NewClient( + ClientOptDebug(debug), + ClientOptEnableCache(true), + ClientOptWriter(out), + ClientOptCredentialsFile(registryConfig), + ClientOptHTTPClient(&http.Client{ + Transport: &http.Transport{ + TLSClientConfig: tlsConf, + }, + }), + ) + if err != nil { + return nil, err + } + return registryClient, nil +} diff --git a/pkg/registry/utils_test.go b/pkg/registry/utils_test.go new file mode 100644 index 000000000..abe9d649f --- /dev/null +++ b/pkg/registry/utils_test.go @@ -0,0 +1,371 @@ +/* +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 registry + +import ( + "bytes" + "context" + "crypto/tls" + "fmt" + "io" + "io/ioutil" + "net/http" + "net/http/httptest" + "net/url" + "os" + "path/filepath" + "strings" + "time" + + "github.com/distribution/distribution/v3/configuration" + "github.com/distribution/distribution/v3/registry" + _ "github.com/distribution/distribution/v3/registry/auth/htpasswd" + _ "github.com/distribution/distribution/v3/registry/storage/driver/inmemory" + "github.com/phayes/freeport" + "github.com/stretchr/testify/suite" + "golang.org/x/crypto/bcrypt" + + "helm.sh/helm/v3/internal/tlsutil" +) + +const ( + tlsServerKey = "./testdata/tls/server-key.pem" + tlsServerCert = "./testdata/tls/server-cert.pem" + tlsCA = "./testdata/tls/ca-cert.pem" + tlsKey = "./testdata/tls/client-key.pem" + tlsCert = "./testdata/tls/client-cert.pem" +) + +var ( + testWorkspaceDir = "helm-registry-test" + testHtpasswdFileBasename = "authtest.htpasswd" + testUsername = "myuser" + testPassword = "mypass" +) + +type TestSuite struct { + suite.Suite + Out io.Writer + DockerRegistryHost string + CompromisedRegistryHost string + WorkspaceDir string + RegistryClient *Client +} + +func setup(suite *TestSuite, secure bool) *registry.Registry { + suite.WorkspaceDir = testWorkspaceDir + os.RemoveAll(suite.WorkspaceDir) + os.Mkdir(suite.WorkspaceDir, 0700) + + var ( + out bytes.Buffer + err error + ) + suite.Out = &out + credentialsFile := filepath.Join(suite.WorkspaceDir, CredentialsFileBasename) + + // init test client + if secure { + var tlsConf *tls.Config + tlsConf, err = tlsutil.NewClientTLS(tlsCert, tlsKey, tlsCA) + httpClient := &http.Client{ + Transport: &http.Transport{ + TLSClientConfig: tlsConf, + }, + } + suite.Nil(err, "no error loading tlsconfog") + suite.RegistryClient, err = NewClient( + ClientOptDebug(true), + ClientOptEnableCache(true), + ClientOptWriter(suite.Out), + ClientOptCredentialsFile(credentialsFile), + ClientOptHTTPClient(httpClient), + ) + } else { + suite.RegistryClient, err = NewClient( + ClientOptDebug(true), + ClientOptEnableCache(true), + ClientOptWriter(suite.Out), + ClientOptCredentialsFile(credentialsFile), + ) + } + + suite.Nil(err, "no error creating registry client") + + // create htpasswd file (w BCrypt, which is required) + pwBytes, err := bcrypt.GenerateFromPassword([]byte(testPassword), bcrypt.DefaultCost) + suite.Nil(err, "no error generating bcrypt password for test htpasswd file") + htpasswdPath := filepath.Join(suite.WorkspaceDir, testHtpasswdFileBasename) + err = ioutil.WriteFile(htpasswdPath, []byte(fmt.Sprintf("%s:%s\n", testUsername, string(pwBytes))), 0644) + suite.Nil(err, "no error creating test htpasswd file") + + // Registry config + config := &configuration.Configuration{} + port, err := freeport.GetFreePort() + suite.Nil(err, "no error finding free port for test registry") + if secure { + // docker has "MatchLocalhost is a host match function which returns true for + // localhost, and is used to enforce http for localhost requests." + // That function does not handle matching of ip addresses in octal, + // decimal or hex form. + suite.DockerRegistryHost = fmt.Sprintf("0x7f000001:%d", port) + } else { + suite.DockerRegistryHost = fmt.Sprintf("localhost:%d", port) + } + config.HTTP.Addr = fmt.Sprintf(":%d", port) + // config.HTTP.Addr = fmt.Sprintf("127.0.0.1:%d", port) + config.HTTP.DrainTimeout = time.Duration(10) * time.Second + config.Storage = map[string]configuration.Parameters{"inmemory": map[string]interface{}{}} + config.Auth = configuration.Auth{ + "htpasswd": configuration.Parameters{ + "realm": "localhost", + "path": htpasswdPath, + }, + } + + // config tls + if secure { + // TLS config + // this set tlsConf.ClientAuth = tls.RequireAndVerifyClientCert in the + // server tls config + config.HTTP.TLS.Certificate = tlsServerCert + config.HTTP.TLS.Key = tlsServerKey + config.HTTP.TLS.ClientCAs = []string{tlsCA} + } + dockerRegistry, err := registry.NewRegistry(context.Background(), config) + suite.Nil(err, "no error creating test registry") + + suite.CompromisedRegistryHost = initCompromisedRegistryTestServer() + return dockerRegistry +} + +func initCompromisedRegistryTestServer() string { + s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if strings.Contains(r.URL.Path, "manifests") { + w.Header().Set("Content-Type", "application/vnd.oci.image.manifest.v1+json") + w.WriteHeader(200) + + // layers[0] is the blob []byte("a") + w.Write([]byte( + fmt.Sprintf(`{ "schemaVersion": 2, "config": { + "mediaType": "%s", + "digest": "sha256:a705ee2789ab50a5ba20930f246dbd5cc01ff9712825bb98f57ee8414377f133", + "size": 181 + }, + "layers": [ + { + "mediaType": "%s", + "digest": "sha256:ca978112ca1bbdcafac231b39a23dc4da786eff8147c4e72b9807785afee48bb", + "size": 1 + } + ] +}`, ConfigMediaType, ChartLayerMediaType))) + } else if r.URL.Path == "/v2/testrepo/supposedlysafechart/blobs/sha256:a705ee2789ab50a5ba20930f246dbd5cc01ff9712825bb98f57ee8414377f133" { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(200) + w.Write([]byte("{\"name\":\"mychart\",\"version\":\"0.1.0\",\"description\":\"A Helm chart for Kubernetes\\n" + + "an 'application' or a 'library' chart.\",\"apiVersion\":\"v2\",\"appVersion\":\"1.16.0\",\"type\":" + + "\"application\"}")) + } else if r.URL.Path == "/v2/testrepo/supposedlysafechart/blobs/sha256:ca978112ca1bbdcafac231b39a23dc4da786eff8147c4e72b9807785afee48bb" { + w.Header().Set("Content-Type", ChartLayerMediaType) + w.WriteHeader(200) + w.Write([]byte("b")) + } else { + w.WriteHeader(500) + } + })) + + u, _ := url.Parse(s.URL) + return fmt.Sprintf("localhost:%s", u.Port()) +} + +func testPush(suite *TestSuite) { + // Bad bytes + ref := fmt.Sprintf("%s/testrepo/testchart:1.2.3", suite.DockerRegistryHost) + _, err := suite.RegistryClient.Push([]byte("hello"), ref) + suite.NotNil(err, "error pushing non-chart bytes") + + // Load a test chart + chartData, err := ioutil.ReadFile("../repo/repotest/testdata/examplechart-0.1.0.tgz") + suite.Nil(err, "no error loading test chart") + meta, err := extractChartMeta(chartData) + suite.Nil(err, "no error extracting chart meta") + + // non-strict ref (chart name) + ref = fmt.Sprintf("%s/testrepo/boop:%s", suite.DockerRegistryHost, meta.Version) + _, err = suite.RegistryClient.Push(chartData, ref) + suite.NotNil(err, "error pushing non-strict ref (bad basename)") + + // non-strict ref (chart name), with strict mode disabled + _, err = suite.RegistryClient.Push(chartData, ref, PushOptStrictMode(false)) + suite.Nil(err, "no error pushing non-strict ref (bad basename), with strict mode disabled") + + // non-strict ref (chart version) + ref = fmt.Sprintf("%s/testrepo/%s:latest", suite.DockerRegistryHost, meta.Name) + _, err = suite.RegistryClient.Push(chartData, ref) + suite.NotNil(err, "error pushing non-strict ref (bad tag)") + + // non-strict ref (chart version), with strict mode disabled + _, err = suite.RegistryClient.Push(chartData, ref, PushOptStrictMode(false)) + suite.Nil(err, "no error pushing non-strict ref (bad tag), with strict mode disabled") + + // basic push, good ref + chartData, err = ioutil.ReadFile("../downloader/testdata/local-subchart-0.1.0.tgz") + suite.Nil(err, "no error loading test chart") + meta, err = extractChartMeta(chartData) + suite.Nil(err, "no error extracting chart meta") + ref = fmt.Sprintf("%s/testrepo/%s:%s", suite.DockerRegistryHost, meta.Name, meta.Version) + _, err = suite.RegistryClient.Push(chartData, ref) + suite.Nil(err, "no error pushing good ref") + + _, err = suite.RegistryClient.Pull(ref) + suite.Nil(err, "no error pulling a simple chart") + + // Load another test chart + chartData, err = ioutil.ReadFile("../downloader/testdata/signtest-0.1.0.tgz") + suite.Nil(err, "no error loading test chart") + meta, err = extractChartMeta(chartData) + suite.Nil(err, "no error extracting chart meta") + + // Load prov file + provData, err := ioutil.ReadFile("../downloader/testdata/signtest-0.1.0.tgz.prov") + suite.Nil(err, "no error loading test prov") + + // push with prov + ref = fmt.Sprintf("%s/testrepo/%s:%s", suite.DockerRegistryHost, meta.Name, meta.Version) + result, err := suite.RegistryClient.Push(chartData, ref, PushOptProvData(provData)) + suite.Nil(err, "no error pushing good ref with prov") + + _, err = suite.RegistryClient.Pull(ref) + suite.Nil(err, "no error pulling a simple chart") + + // Validate the output + // Note: these digests/sizes etc may change if the test chart/prov files are modified, + // or if the format of the OCI manifest changes + suite.Equal(ref, result.Ref) + suite.Equal(meta.Name, result.Chart.Meta.Name) + suite.Equal(meta.Version, result.Chart.Meta.Version) + suite.Equal(int64(512), result.Manifest.Size) + suite.Equal(int64(99), result.Config.Size) + suite.Equal(int64(973), result.Chart.Size) + suite.Equal(int64(695), result.Prov.Size) + suite.Equal( + "sha256:af4c20a1df1431495e673c14ecfa3a2ba24839a7784349d6787cd67957392e83", + result.Manifest.Digest) + suite.Equal( + "sha256:8d17cb6bf6ccd8c29aace9a658495cbd5e2e87fc267876e86117c7db681c9580", + result.Config.Digest) + suite.Equal( + "sha256:e5ef611620fb97704d8751c16bab17fedb68883bfb0edc76f78a70e9173f9b55", + result.Chart.Digest) + suite.Equal( + "sha256:b0a02b7412f78ae93324d48df8fcc316d8482e5ad7827b5b238657a29a22f256", + result.Prov.Digest) +} + +func testPull(suite *TestSuite) { + // bad/missing ref + ref := fmt.Sprintf("%s/testrepo/no-existy:1.2.3", suite.DockerRegistryHost) + _, err := suite.RegistryClient.Pull(ref) + suite.NotNil(err, "error on bad/missing ref") + + // Load test chart (to build ref pushed in previous test) + chartData, err := ioutil.ReadFile("../downloader/testdata/local-subchart-0.1.0.tgz") + suite.Nil(err, "no error loading test chart") + meta, err := extractChartMeta(chartData) + suite.Nil(err, "no error extracting chart meta") + ref = fmt.Sprintf("%s/testrepo/%s:%s", suite.DockerRegistryHost, meta.Name, meta.Version) + + // Simple pull, chart only + _, err = suite.RegistryClient.Pull(ref) + suite.Nil(err, "no error pulling a simple chart") + + // Simple pull with prov (no prov uploaded) + _, err = suite.RegistryClient.Pull(ref, PullOptWithProv(true)) + suite.NotNil(err, "error pulling a chart with prov when no prov exists") + + // Simple pull with prov, ignoring missing prov + _, err = suite.RegistryClient.Pull(ref, + PullOptWithProv(true), + PullOptIgnoreMissingProv(true)) + suite.Nil(err, + "no error pulling a chart with prov when no prov exists, ignoring missing") + + // Load test chart (to build ref pushed in previous test) + chartData, err = ioutil.ReadFile("../downloader/testdata/signtest-0.1.0.tgz") + suite.Nil(err, "no error loading test chart") + meta, err = extractChartMeta(chartData) + suite.Nil(err, "no error extracting chart meta") + ref = fmt.Sprintf("%s/testrepo/%s:%s", suite.DockerRegistryHost, meta.Name, meta.Version) + + // Load prov file + provData, err := ioutil.ReadFile("../downloader/testdata/signtest-0.1.0.tgz.prov") + suite.Nil(err, "no error loading test prov") + + // no chart and no prov causes error + _, err = suite.RegistryClient.Pull(ref, + PullOptWithChart(false), + PullOptWithProv(false)) + suite.NotNil(err, "error on both no chart and no prov") + + // full pull with chart and prov + result, err := suite.RegistryClient.Pull(ref, PullOptWithProv(true)) + suite.Nil(err, "no error pulling a chart with prov") + + // Validate the output + // Note: these digests/sizes etc may change if the test chart/prov files are modified, + // or if the format of the OCI manifest changes + suite.Equal(ref, result.Ref) + suite.Equal(meta.Name, result.Chart.Meta.Name) + suite.Equal(meta.Version, result.Chart.Meta.Version) + suite.Equal(int64(512), result.Manifest.Size) + suite.Equal(int64(99), result.Config.Size) + suite.Equal(int64(973), result.Chart.Size) + suite.Equal(int64(695), result.Prov.Size) + suite.Equal( + "sha256:af4c20a1df1431495e673c14ecfa3a2ba24839a7784349d6787cd67957392e83", + result.Manifest.Digest) + suite.Equal( + "sha256:8d17cb6bf6ccd8c29aace9a658495cbd5e2e87fc267876e86117c7db681c9580", + result.Config.Digest) + suite.Equal( + "sha256:e5ef611620fb97704d8751c16bab17fedb68883bfb0edc76f78a70e9173f9b55", + result.Chart.Digest) + suite.Equal( + "sha256:b0a02b7412f78ae93324d48df8fcc316d8482e5ad7827b5b238657a29a22f256", + result.Prov.Digest) + suite.Equal("{\"schemaVersion\":2,\"config\":{\"mediaType\":\"application/vnd.cncf.helm.config.v1+json\",\"digest\":\"sha256:8d17cb6bf6ccd8c29aace9a658495cbd5e2e87fc267876e86117c7db681c9580\",\"size\":99},\"layers\":[{\"mediaType\":\"application/vnd.cncf.helm.chart.provenance.v1.prov\",\"digest\":\"sha256:b0a02b7412f78ae93324d48df8fcc316d8482e5ad7827b5b238657a29a22f256\",\"size\":695},{\"mediaType\":\"application/vnd.cncf.helm.chart.content.v1.tar+gzip\",\"digest\":\"sha256:e5ef611620fb97704d8751c16bab17fedb68883bfb0edc76f78a70e9173f9b55\",\"size\":973}]}", + string(result.Manifest.Data)) + suite.Equal("{\"name\":\"signtest\",\"version\":\"0.1.0\",\"description\":\"A Helm chart for Kubernetes\",\"apiVersion\":\"v1\"}", + string(result.Config.Data)) + suite.Equal(chartData, result.Chart.Data) + suite.Equal(provData, result.Prov.Data) +} + +func testTags(suite *TestSuite) { + // Load test chart (to build ref pushed in previous test) + chartData, err := ioutil.ReadFile("../downloader/testdata/local-subchart-0.1.0.tgz") + suite.Nil(err, "no error loading test chart") + meta, err := extractChartMeta(chartData) + suite.Nil(err, "no error extracting chart meta") + ref := fmt.Sprintf("%s/testrepo/%s", suite.DockerRegistryHost, meta.Name) + + // Query for tags and validate length + tags, err := suite.RegistryClient.Tags(ref) + suite.Nil(err, "no error retrieving tags") + suite.Equal(1, len(tags)) +} From 08593c8dd6e4b05b1855296f6d7c6247de1d9e6a Mon Sep 17 00:00:00 2001 From: Andrew Block Date: Mon, 21 Nov 2022 11:21:55 -0600 Subject: [PATCH 2/6] Added support for insecure OCI registries Signed-off-by: Andrew Block --- cmd/helm/push.go | 9 ++++++--- internal/tlsutil/tls.go | 6 ++++-- internal/tlsutil/tlsutil_test.go | 7 ++++--- pkg/action/install.go | 4 ++-- pkg/action/pull.go | 5 +++-- pkg/action/push.go | 24 ++++++++++++++++-------- pkg/getter/httpgetter.go | 4 ++-- pkg/getter/httpgetter_test.go | 3 ++- pkg/getter/ocigetter.go | 4 ++-- pkg/pusher/ocipusher.go | 4 ++-- pkg/pusher/ocipusher_test.go | 2 ++ pkg/pusher/pusher.go | 16 ++++++++++++---- pkg/registry/client_test.go | 2 +- pkg/registry/client_tls_test.go | 2 +- pkg/registry/util.go | 4 ++-- pkg/registry/utils_test.go | 10 +++++----- pkg/repo/repotest/server.go | 4 +++- 17 files changed, 69 insertions(+), 41 deletions(-) diff --git a/cmd/helm/push.go b/cmd/helm/push.go index ac6659fa3..dffe8f4ea 100644 --- a/cmd/helm/push.go +++ b/cmd/helm/push.go @@ -35,9 +35,10 @@ it will also be uploaded. ` type registryPushOptions struct { - certFile string - keyFile string - caFile string + certFile string + keyFile string + caFile string + insecureSkipTLSverify bool } func newPushCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { @@ -70,6 +71,7 @@ func newPushCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { remote := args[1] client := action.NewPushWithOpts(action.WithPushConfig(cfg), action.WithTLSClientConfig(o.certFile, o.keyFile, o.caFile), + action.WithInsecureSkipTLSVerify(o.insecureSkipTLSverify), action.WithPushOptWriter(out)) client.Settings = settings output, err := client.Run(chartRef, remote) @@ -85,6 +87,7 @@ func newPushCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { f.StringVar(&o.certFile, "cert-file", "", "identify registry client using this SSL certificate file") f.StringVar(&o.keyFile, "key-file", "", "identify registry client using this SSL key file") f.StringVar(&o.caFile, "ca-file", "", "verify certificates of HTTPS-enabled servers using this CA bundle") + f.BoolVar(&o.insecureSkipTLSverify, "insecure-skip-tls-verify", false, "skip tls certificate checks for the chart upload") return cmd } diff --git a/internal/tlsutil/tls.go b/internal/tlsutil/tls.go index ed7795dbe..28c70f9db 100644 --- a/internal/tlsutil/tls.go +++ b/internal/tlsutil/tls.go @@ -25,8 +25,10 @@ import ( ) // NewClientTLS returns tls.Config appropriate for client auth. -func NewClientTLS(certFile, keyFile, caFile string) (*tls.Config, error) { - config := tls.Config{} +func NewClientTLS(certFile, keyFile, caFile string, insecureSkipTLSverify bool) (*tls.Config, error) { + config := tls.Config{ + InsecureSkipVerify: insecureSkipTLSverify, + } if certFile != "" && keyFile != "" { cert, err := CertFromFilePair(certFile, keyFile) diff --git a/internal/tlsutil/tlsutil_test.go b/internal/tlsutil/tlsutil_test.go index e660c030c..e31a873d3 100644 --- a/internal/tlsutil/tlsutil_test.go +++ b/internal/tlsutil/tlsutil_test.go @@ -65,8 +65,9 @@ func TestNewClientTLS(t *testing.T) { certFile := testfile(t, testCertFile) keyFile := testfile(t, testKeyFile) caCertFile := testfile(t, testCaCertFile) + insecureSkipTLSverify := false - cfg, err := NewClientTLS(certFile, keyFile, caCertFile) + cfg, err := NewClientTLS(certFile, keyFile, caCertFile, insecureSkipTLSverify) if err != nil { t.Error(err) } @@ -81,7 +82,7 @@ func TestNewClientTLS(t *testing.T) { t.Fatalf("mismatch tls RootCAs, expecting non-nil") } - cfg, err = NewClientTLS("", "", caCertFile) + cfg, err = NewClientTLS("", "", caCertFile, insecureSkipTLSverify) if err != nil { t.Error(err) } @@ -96,7 +97,7 @@ func TestNewClientTLS(t *testing.T) { t.Fatalf("mismatch tls RootCAs, expecting non-nil") } - cfg, err = NewClientTLS(certFile, keyFile, "") + cfg, err = NewClientTLS(certFile, keyFile, "", insecureSkipTLSverify) if err != nil { t.Error(err) } diff --git a/pkg/action/install.go b/pkg/action/install.go index 4d716bec7..bc228e0c0 100644 --- a/pkg/action/install.go +++ b/pkg/action/install.go @@ -680,9 +680,9 @@ func (c *ChartPathOptions) LocateChart(name string, out io.Writer, settings *cli // If there is no registry client and the name is in an OCI registry return // an error and a lookup will not occur. if registry.IsOCI(name) { - if (c.CertFile != "" && c.KeyFile != "") || c.CaFile != "" { + if (c.CertFile != "" && c.KeyFile != "") || c.CaFile != "" || c.InsecureSkipTLSverify { registryClient, err := registry.NewRegistryClientWithTLS(out, c.CertFile, c.KeyFile, c.CaFile, - settings.RegistryConfig, settings.Debug) + c.InsecureSkipTLSverify, settings.RegistryConfig, settings.Debug) if err != nil { return "", err } diff --git a/pkg/action/pull.go b/pkg/action/pull.go index bcd93705d..102e4a588 100644 --- a/pkg/action/pull.go +++ b/pkg/action/pull.go @@ -104,9 +104,9 @@ func (p *Pull) Run(chartRef string) (string, error) { if registry.IsOCI(chartRef) { // Provide a tls enabled client for the pull command if the user has // specified the cert file or key file or ca file. - if (p.ChartPathOptions.CertFile != "" && p.ChartPathOptions.KeyFile != "") || p.ChartPathOptions.CaFile != "" { + if (p.ChartPathOptions.CertFile != "" && p.ChartPathOptions.KeyFile != "") || p.ChartPathOptions.CaFile != "" || p.ChartPathOptions.InsecureSkipTLSverify { registryClient, err := registry.NewRegistryClientWithTLS(p.out, p.ChartPathOptions.CertFile, p.ChartPathOptions.KeyFile, p.ChartPathOptions.CaFile, - p.Settings.RegistryConfig, p.Settings.Debug) + p.ChartPathOptions.InsecureSkipTLSverify, p.Settings.RegistryConfig, p.Settings.Debug) if err != nil { return out.String(), err } @@ -114,6 +114,7 @@ func (p *Pull) Run(chartRef string) (string, error) { } c.Options = append(c.Options, getter.WithRegistryClient(p.cfg.RegistryClient)) + c.RegistryClient = p.cfg.RegistryClient } if p.Verify { diff --git a/pkg/action/push.go b/pkg/action/push.go index f6e7a6aaa..21f6fb947 100644 --- a/pkg/action/push.go +++ b/pkg/action/push.go @@ -30,12 +30,13 @@ import ( // // It provides the implementation of 'helm push'. type Push struct { - Settings *cli.EnvSettings - cfg *Configuration - certFile string - keyFile string - caFile string - out io.Writer + Settings *cli.EnvSettings + cfg *Configuration + certFile string + keyFile string + caFile string + insecureSkipTLSverify bool + out io.Writer } // PushOpt is a type of function that sets options for a push action. @@ -57,6 +58,13 @@ func WithTLSClientConfig(certFile, keyFile, caFile string) PushOpt { } } +// WithInsecureSkipTLSVerify determines if a TLS Certificate will be checked +func WithInsecureSkipTLSVerify(insecureSkipTLSVerify bool) PushOpt { + return func(p *Push) { + p.insecureSkipTLSverify = insecureSkipTLSVerify + } +} + // WithOptWriter sets the registryOut field on the push configuration object. func WithPushOptWriter(out io.Writer) PushOpt { return func(p *Push) { @@ -88,9 +96,9 @@ func (p *Push) Run(chartRef string, remote string) (string, error) { if registry.IsOCI(remote) { // Provide a tls enabled client for the pull command if the user has // specified the cert file or key file or ca file. - if (p.certFile != "" && p.keyFile != "") || p.caFile != "" { + if (p.certFile != "" && p.keyFile != "") || p.caFile != "" || p.insecureSkipTLSverify { registryClient, err := registry.NewRegistryClientWithTLS(p.out, p.certFile, p.keyFile, p.caFile, - p.Settings.RegistryConfig, p.Settings.Debug) + p.insecureSkipTLSverify, p.Settings.RegistryConfig, p.Settings.Debug) if err != nil { return out.String(), err } diff --git a/pkg/getter/httpgetter.go b/pkg/getter/httpgetter.go index 081bf059e..b53e558e3 100644 --- a/pkg/getter/httpgetter.go +++ b/pkg/getter/httpgetter.go @@ -123,8 +123,8 @@ func (g *HTTPGetter) httpClient() (*http.Client, error) { } }) - if (g.opts.certFile != "" && g.opts.keyFile != "") || g.opts.caFile != "" { - tlsConf, err := tlsutil.NewClientTLS(g.opts.certFile, g.opts.keyFile, g.opts.caFile) + if (g.opts.certFile != "" && g.opts.keyFile != "") || g.opts.caFile != "" || g.opts.insecureSkipVerifyTLS { + tlsConf, err := tlsutil.NewClientTLS(g.opts.certFile, g.opts.keyFile, g.opts.caFile, g.opts.insecureSkipVerifyTLS) if err != nil { return nil, errors.Wrap(err, "can't create TLS config for client") } diff --git a/pkg/getter/httpgetter_test.go b/pkg/getter/httpgetter_test.go index 94baef3b8..472763511 100644 --- a/pkg/getter/httpgetter_test.go +++ b/pkg/getter/httpgetter_test.go @@ -285,9 +285,10 @@ func TestDownload(t *testing.T) { func TestDownloadTLS(t *testing.T) { cd := "../../testdata" ca, pub, priv := filepath.Join(cd, "rootca.crt"), filepath.Join(cd, "crt.pem"), filepath.Join(cd, "key.pem") + insecureSkipTLSverify := false tlsSrv := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})) - tlsConf, err := tlsutil.NewClientTLS(pub, priv, ca) + tlsConf, err := tlsutil.NewClientTLS(pub, priv, ca, insecureSkipTLSverify) if err != nil { t.Fatal(errors.Wrap(err, "can't create TLS config for client")) } diff --git a/pkg/getter/ocigetter.go b/pkg/getter/ocigetter.go index d64ca0fb2..1705fca91 100644 --- a/pkg/getter/ocigetter.go +++ b/pkg/getter/ocigetter.go @@ -122,8 +122,8 @@ func (g *OCIGetter) newRegistryClient() (*registry.Client, error) { } }) - if (g.opts.certFile != "" && g.opts.keyFile != "") || g.opts.caFile != "" { - tlsConf, err := tlsutil.NewClientTLS(g.opts.certFile, g.opts.keyFile, g.opts.caFile) + if (g.opts.certFile != "" && g.opts.keyFile != "") || g.opts.caFile != "" || g.opts.insecureSkipVerifyTLS { + tlsConf, err := tlsutil.NewClientTLS(g.opts.certFile, g.opts.keyFile, g.opts.caFile, g.opts.insecureSkipVerifyTLS) if err != nil { return nil, fmt.Errorf("can't create TLS config for client: %w", err) } diff --git a/pkg/pusher/ocipusher.go b/pkg/pusher/ocipusher.go index a431577af..614141698 100644 --- a/pkg/pusher/ocipusher.go +++ b/pkg/pusher/ocipusher.go @@ -106,8 +106,8 @@ func NewOCIPusher(ops ...Option) (Pusher, error) { } func (pusher *OCIPusher) newRegistryClient() (*registry.Client, error) { - if (pusher.opts.certFile != "" && pusher.opts.keyFile != "") || pusher.opts.caFile != "" { - tlsConf, err := tlsutil.NewClientTLS(pusher.opts.certFile, pusher.opts.keyFile, pusher.opts.caFile) + if (pusher.opts.certFile != "" && pusher.opts.keyFile != "") || pusher.opts.caFile != "" || pusher.opts.insecureSkipTLSverify { + tlsConf, err := tlsutil.NewClientTLS(pusher.opts.certFile, pusher.opts.keyFile, pusher.opts.caFile, pusher.opts.insecureSkipTLSverify) if err != nil { return nil, errors.Wrap(err, "can't create TLS config for client") } diff --git a/pkg/pusher/ocipusher_test.go b/pkg/pusher/ocipusher_test.go index d718cec6d..9390710a0 100644 --- a/pkg/pusher/ocipusher_test.go +++ b/pkg/pusher/ocipusher_test.go @@ -35,10 +35,12 @@ func TestNewOCIPusher(t *testing.T) { cd := "../../testdata" join := filepath.Join ca, pub, priv := join(cd, "rootca.crt"), join(cd, "crt.pem"), join(cd, "key.pem") + insecureSkipTLSverify := false // Test with options p, err = NewOCIPusher( WithTLSClientConfig(pub, priv, ca), + WithInsecureSkipTLSVerify(insecureSkipTLSverify), ) if err != nil { t.Fatal(err) diff --git a/pkg/pusher/pusher.go b/pkg/pusher/pusher.go index 86a5ebf90..e325ce498 100644 --- a/pkg/pusher/pusher.go +++ b/pkg/pusher/pusher.go @@ -27,10 +27,11 @@ import ( // // Pushers may or may not ignore these parameters as they are passed in. type options struct { - registryClient *registry.Client - certFile string - keyFile string - caFile string + registryClient *registry.Client + certFile string + keyFile string + caFile string + insecureSkipTLSverify bool } // Option allows specifying various settings configurable by the user for overriding the defaults @@ -53,6 +54,13 @@ func WithTLSClientConfig(certFile, keyFile, caFile string) Option { } } +// WithInsecureSkipTLSVerify determines if a TLS Certificate will be checked +func WithInsecureSkipTLSVerify(insecureSkipTLSVerify bool) Option { + return func(opts *options) { + opts.insecureSkipTLSverify = insecureSkipTLSVerify + } +} + // Pusher is an interface to support upload to the specified URL. type Pusher interface { // Push file content by url string diff --git a/pkg/registry/client_test.go b/pkg/registry/client_test.go index 4be6a1cd4..3bb4a991b 100644 --- a/pkg/registry/client_test.go +++ b/pkg/registry/client_test.go @@ -31,7 +31,7 @@ type RegistryClientTestSuite struct { func (suite *RegistryClientTestSuite) SetupSuite() { // init test client - dockerRegistry := setup(&suite.TestSuite, false) + dockerRegistry := setup(&suite.TestSuite, false, false) // Start Docker registry go dockerRegistry.ListenAndServe() diff --git a/pkg/registry/client_tls_test.go b/pkg/registry/client_tls_test.go index 3be0419bf..c4bb31c25 100644 --- a/pkg/registry/client_tls_test.go +++ b/pkg/registry/client_tls_test.go @@ -29,7 +29,7 @@ type TLSRegistryClientTestSuite struct { func (suite *TLSRegistryClientTestSuite) SetupSuite() { // init test client - dockerRegistry := setup(&suite.TestSuite, true) + dockerRegistry := setup(&suite.TestSuite, true, false) // Start Docker registry go dockerRegistry.ListenAndServe() diff --git a/pkg/registry/util.go b/pkg/registry/util.go index 025b145ba..2a2754378 100644 --- a/pkg/registry/util.go +++ b/pkg/registry/util.go @@ -133,8 +133,8 @@ func parseReference(raw string) (registry.Reference, error) { } // NewRegistryClientWithTLS is a helper function to create a new registry client with TLS enabled. -func NewRegistryClientWithTLS(out io.Writer, certFile, keyFile, caFile string, registryConfig string, debug bool) (*Client, error) { - tlsConf, err := tlsutil.NewClientTLS(certFile, keyFile, caFile) +func NewRegistryClientWithTLS(out io.Writer, certFile, keyFile, caFile string, insecureSkipTLSverify bool, registryConfig string, debug bool) (*Client, error) { + tlsConf, err := tlsutil.NewClientTLS(certFile, keyFile, caFile, insecureSkipTLSverify) if err != nil { return nil, fmt.Errorf("can't create TLS config for client: %s", err) } diff --git a/pkg/registry/utils_test.go b/pkg/registry/utils_test.go index abe9d649f..7cd338793 100644 --- a/pkg/registry/utils_test.go +++ b/pkg/registry/utils_test.go @@ -66,7 +66,7 @@ type TestSuite struct { RegistryClient *Client } -func setup(suite *TestSuite, secure bool) *registry.Registry { +func setup(suite *TestSuite, tlsEnabled bool, insecure bool) *registry.Registry { suite.WorkspaceDir = testWorkspaceDir os.RemoveAll(suite.WorkspaceDir) os.Mkdir(suite.WorkspaceDir, 0700) @@ -79,9 +79,9 @@ func setup(suite *TestSuite, secure bool) *registry.Registry { credentialsFile := filepath.Join(suite.WorkspaceDir, CredentialsFileBasename) // init test client - if secure { + if tlsEnabled { var tlsConf *tls.Config - tlsConf, err = tlsutil.NewClientTLS(tlsCert, tlsKey, tlsCA) + tlsConf, err = tlsutil.NewClientTLS(tlsCert, tlsKey, tlsCA, insecure) httpClient := &http.Client{ Transport: &http.Transport{ TLSClientConfig: tlsConf, @@ -117,7 +117,7 @@ func setup(suite *TestSuite, secure bool) *registry.Registry { config := &configuration.Configuration{} port, err := freeport.GetFreePort() suite.Nil(err, "no error finding free port for test registry") - if secure { + if tlsEnabled { // docker has "MatchLocalhost is a host match function which returns true for // localhost, and is used to enforce http for localhost requests." // That function does not handle matching of ip addresses in octal, @@ -138,7 +138,7 @@ func setup(suite *TestSuite, secure bool) *registry.Registry { } // config tls - if secure { + if tlsEnabled { // TLS config // this set tlsConf.ClientAuth = tls.RequireAndVerifyClientCert in the // server tls config diff --git a/pkg/repo/repotest/server.go b/pkg/repo/repotest/server.go index 90ad3d856..5753dbeb9 100644 --- a/pkg/repo/repotest/server.go +++ b/pkg/repo/repotest/server.go @@ -360,6 +360,7 @@ func (s *Server) Start() { func (s *Server) StartTLS() { cd := "../../testdata" ca, pub, priv := filepath.Join(cd, "rootca.crt"), filepath.Join(cd, "crt.pem"), filepath.Join(cd, "key.pem") + insecure := false s.srv = httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if s.middleware != nil { @@ -367,7 +368,7 @@ func (s *Server) StartTLS() { } http.FileServer(http.Dir(s.Root())).ServeHTTP(w, r) })) - tlsConf, err := tlsutil.NewClientTLS(pub, priv, ca) + tlsConf, err := tlsutil.NewClientTLS(pub, priv, ca, insecure) if err != nil { panic(err) } @@ -400,6 +401,7 @@ func (s *Server) Stop() { // URL returns the URL of the server. // // Example: +// // http://localhost:1776 func (s *Server) URL() string { return s.srv.URL From 154f37efec5b66cf26b88e018805deb7df2f2029 Mon Sep 17 00:00:00 2001 From: Andrew Block Date: Fri, 9 Dec 2022 08:28:52 -0600 Subject: [PATCH 3/6] Added insecure option to login subcommand Signed-off-by: Andrew Block --- cmd/helm/registry_login.go | 5 ++++- pkg/action/registry_login.go | 10 ++++++++++ pkg/getter/ocigetter_test.go | 2 ++ 3 files changed, 16 insertions(+), 1 deletion(-) diff --git a/cmd/helm/registry_login.go b/cmd/helm/registry_login.go index 98d31bddc..0a268c4bf 100644 --- a/cmd/helm/registry_login.go +++ b/cmd/helm/registry_login.go @@ -43,6 +43,7 @@ type registryLoginOptions struct { certFile string keyFile string caFile string + insecure bool } func newRegistryLoginCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { @@ -65,7 +66,8 @@ func newRegistryLoginCmd(cfg *action.Configuration, out io.Writer) *cobra.Comman return action.NewRegistryLogin(cfg).Run(out, hostname, username, password, action.WithCertFile(o.certFile), action.WithKeyFile(o.keyFile), - action.WithCAFile(o.caFile)) + action.WithCAFile(o.caFile), + action.WithInsecure(o.insecure)) }, } @@ -73,6 +75,7 @@ func newRegistryLoginCmd(cfg *action.Configuration, out io.Writer) *cobra.Comman f.StringVarP(&o.username, "username", "u", "", "registry username") f.StringVarP(&o.password, "password", "p", "", "registry password or identity token") f.BoolVarP(&o.passwordFromStdinOpt, "password-stdin", "", false, "read password or identity token from stdin") + f.BoolVarP(&o.insecure, "insecure", "", false, "allow connections to TLS registry without certs") f.StringVar(&o.certFile, "cert-file", "", "identify registry client using this SSL certificate file") f.StringVar(&o.keyFile, "key-file", "", "identify registry client using this SSL key file") f.StringVar(&o.caFile, "ca-file", "", "verify certificates of HTTPS-enabled servers using this CA bundle") diff --git a/pkg/action/registry_login.go b/pkg/action/registry_login.go index 3c9bd0bc5..a55f2de58 100644 --- a/pkg/action/registry_login.go +++ b/pkg/action/registry_login.go @@ -28,6 +28,7 @@ type RegistryLogin struct { certFile string keyFile string caFile string + insecure bool } type RegistryLoginOpt func(*RegistryLogin) error @@ -40,6 +41,14 @@ func WithCertFile(certFile string) RegistryLoginOpt { } } +// WithKeyFile specifies whether to very certificates when communicating. +func WithInsecure(insecure bool) RegistryLoginOpt { + return func(r *RegistryLogin) error { + r.insecure = insecure + return nil + } +} + // WithKeyFile specifies the path to the key file to use for TLS. func WithKeyFile(keyFile string) RegistryLoginOpt { return func(r *RegistryLogin) error { @@ -74,5 +83,6 @@ func (a *RegistryLogin) Run(out io.Writer, hostname string, username string, pas return a.cfg.RegistryClient.Login( hostname, registry.LoginOptBasicAuth(username, password), + registry.LoginOptInsecure(a.insecure), registry.LoginOptTLSClientConfig(a.certFile, a.keyFile, a.caFile)) } diff --git a/pkg/getter/ocigetter_test.go b/pkg/getter/ocigetter_test.go index 86f54b153..fa2fa67a5 100644 --- a/pkg/getter/ocigetter_test.go +++ b/pkg/getter/ocigetter_test.go @@ -39,6 +39,7 @@ func TestOCIGetter(t *testing.T) { ca, pub, priv := join(cd, "rootca.crt"), join(cd, "crt.pem"), join(cd, "key.pem") timeout := time.Second * 5 transport := &http.Transport{} + insecureSkipTLSverify := false // Test with options g, err = NewOCIGetter( @@ -46,6 +47,7 @@ func TestOCIGetter(t *testing.T) { WithTLSClientConfig(pub, priv, ca), WithTimeout(timeout), WithTransport(transport), + WithInsecureSkipVerifyTLS(insecureSkipTLSverify), ) if err != nil { t.Fatal(err) From ec5e29e8012ec062db8a022bb1e8f85c95e38815 Mon Sep 17 00:00:00 2001 From: Andrew Block Date: Sun, 11 Dec 2022 15:42:50 -0600 Subject: [PATCH 4/6] Removed conditional Signed-off-by: Andrew Block --- pkg/getter/httpgetter.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/getter/httpgetter.go b/pkg/getter/httpgetter.go index b53e558e3..fc4a1b8ef 100644 --- a/pkg/getter/httpgetter.go +++ b/pkg/getter/httpgetter.go @@ -123,7 +123,7 @@ func (g *HTTPGetter) httpClient() (*http.Client, error) { } }) - if (g.opts.certFile != "" && g.opts.keyFile != "") || g.opts.caFile != "" || g.opts.insecureSkipVerifyTLS { + if (g.opts.certFile != "" && g.opts.keyFile != "") || g.opts.caFile != "" { tlsConf, err := tlsutil.NewClientTLS(g.opts.certFile, g.opts.keyFile, g.opts.caFile, g.opts.insecureSkipVerifyTLS) if err != nil { return nil, errors.Wrap(err, "can't create TLS config for client") From c94306f75d73a84a4e81b93ecfbe70ef4ca79998 Mon Sep 17 00:00:00 2001 From: Andrew Block Date: Mon, 12 Dec 2022 10:40:38 -0600 Subject: [PATCH 5/6] Reimplemented change in httpgetter for insecure TLS option Signed-off-by: Andrew Block --- pkg/getter/httpgetter.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/getter/httpgetter.go b/pkg/getter/httpgetter.go index fc4a1b8ef..b53e558e3 100644 --- a/pkg/getter/httpgetter.go +++ b/pkg/getter/httpgetter.go @@ -123,7 +123,7 @@ func (g *HTTPGetter) httpClient() (*http.Client, error) { } }) - if (g.opts.certFile != "" && g.opts.keyFile != "") || g.opts.caFile != "" { + if (g.opts.certFile != "" && g.opts.keyFile != "") || g.opts.caFile != "" || g.opts.insecureSkipVerifyTLS { tlsConf, err := tlsutil.NewClientTLS(g.opts.certFile, g.opts.keyFile, g.opts.caFile, g.opts.insecureSkipVerifyTLS) if err != nil { return nil, errors.Wrap(err, "can't create TLS config for client") From 11738dde51447c7bfd1ef0c97cd2bd8fb5e3bfa1 Mon Sep 17 00:00:00 2001 From: Soule BA Date: Mon, 19 Dec 2022 22:52:20 +0100 Subject: [PATCH 6/6] Provide a helper to set the registryClient in cmd If enabled the registryClient is set using a helper that accepts the TLS flags. This keeps the client creation consistent accross the different commands. Signed-off-by: Soule BA --- cmd/helm/install.go | 8 +++++++- cmd/helm/pull.go | 8 +++++++- cmd/helm/push.go | 5 +++++ cmd/helm/root.go | 30 ++++++++++++++++++++++++++++-- cmd/helm/show.go | 43 ++++++++++++++++++++++++++++++++++++------- cmd/helm/template.go | 6 ++++++ cmd/helm/upgrade.go | 8 +++++++- pkg/action/install.go | 25 +++++++++---------------- pkg/action/pull.go | 24 +++++------------------- pkg/action/push.go | 12 +----------- pkg/action/show.go | 6 ++++++ pkg/action/upgrade.go | 6 ++++++ 12 files changed, 123 insertions(+), 58 deletions(-) diff --git a/cmd/helm/install.go b/cmd/helm/install.go index dc7018089..13c674066 100644 --- a/cmd/helm/install.go +++ b/cmd/helm/install.go @@ -136,6 +136,12 @@ func newInstallCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { return compInstall(args, toComplete, client) }, RunE: func(_ *cobra.Command, args []string) error { + registryClient, err := newRegistryClient(client.CertFile, client.KeyFile, client.CaFile, client.InsecureSkipTLSverify) + if err != nil { + return fmt.Errorf("missing registry client: %w", err) + } + client.SetRegistryClient(registryClient) + rel, err := runInstall(args, client, valueOpts, out) if err != nil { return errors.Wrap(err, "INSTALLATION FAILED") @@ -203,7 +209,7 @@ func runInstall(args []string, client *action.Install, valueOpts *values.Options } client.ReleaseName = name - cp, err := client.ChartPathOptions.LocateChart(chart, out, settings) + cp, err := client.ChartPathOptions.LocateChart(chart, settings) if err != nil { return nil, err } diff --git a/cmd/helm/pull.go b/cmd/helm/pull.go index 238550250..2d3747f28 100644 --- a/cmd/helm/pull.go +++ b/cmd/helm/pull.go @@ -43,7 +43,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.NewPullWithOpts(action.WithConfig(cfg), action.WithPullOptWriter(out)) + client := action.NewPullWithOpts(action.WithConfig(cfg)) cmd := &cobra.Command{ Use: "pull [chart URL | repo/chartname] [...]", @@ -64,6 +64,12 @@ func newPullCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { client.Version = ">0.0.0-0" } + registryClient, err := newRegistryClient(client.CertFile, client.KeyFile, client.CaFile, client.InsecureSkipTLSverify) + if err != nil { + return fmt.Errorf("missing registry client: %w", err) + } + client.SetRegistryClient(registryClient) + for i := 0; i < len(args); i++ { output, err := client.Run(args[i]) if err != nil { diff --git a/cmd/helm/push.go b/cmd/helm/push.go index dffe8f4ea..b1e3e60af 100644 --- a/cmd/helm/push.go +++ b/cmd/helm/push.go @@ -67,6 +67,11 @@ func newPushCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { return nil, cobra.ShellCompDirectiveNoFileComp }, RunE: func(cmd *cobra.Command, args []string) error { + registryClient, err := newRegistryClient(o.certFile, o.keyFile, o.caFile, o.insecureSkipTLSverify) + if err != nil { + return fmt.Errorf("missing registry client: %w", err) + } + cfg.RegistryClient = registryClient chartRef := args[0] remote := args[1] client := action.NewPushWithOpts(action.WithPushConfig(cfg), diff --git a/cmd/helm/root.go b/cmd/helm/root.go index c4f439511..5bccdf5bf 100644 --- a/cmd/helm/root.go +++ b/cmd/helm/root.go @@ -152,7 +152,7 @@ func newRootCmd(actionConfig *action.Configuration, out io.Writer, args []string flags.ParseErrorsWhitelist.UnknownFlags = true flags.Parse(args) - registryClient, err := newRegistryClient(out) + registryClient, err := newDefaultRegistryClient() if err != nil { return nil, err } @@ -257,7 +257,22 @@ func checkForExpiredRepos(repofile string) { } -func newRegistryClient(out io.Writer) (*registry.Client, error) { +func newRegistryClient(certFile, keyFile, caFile string, insecureSkipTLSverify bool) (*registry.Client, error) { + if certFile != "" && keyFile != "" || caFile != "" || insecureSkipTLSverify { + registryClient, err := newRegistryClientWithTLS(certFile, keyFile, caFile, insecureSkipTLSverify) + if err != nil { + return nil, err + } + return registryClient, nil + } + registryClient, err := newDefaultRegistryClient() + if err != nil { + return nil, err + } + return registryClient, nil +} + +func newDefaultRegistryClient() (*registry.Client, error) { // Create a new registry client registryClient, err := registry.NewClient( registry.ClientOptDebug(settings.Debug), @@ -270,3 +285,14 @@ func newRegistryClient(out io.Writer) (*registry.Client, error) { } return registryClient, nil } + +func newRegistryClientWithTLS(certFile, keyFile, caFile string, insecureSkipTLSverify bool) (*registry.Client, error) { + // Create a new registry client + registryClient, err := registry.NewRegistryClientWithTLS(os.Stderr, certFile, keyFile, caFile, insecureSkipTLSverify, + settings.RegistryConfig, settings.Debug, + ) + if err != nil { + return nil, err + } + return registryClient, nil +} diff --git a/cmd/helm/show.go b/cmd/helm/show.go index e9b17a8cb..a2edd1931 100644 --- a/cmd/helm/show.go +++ b/cmd/helm/show.go @@ -84,7 +84,11 @@ func newShowCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { ValidArgsFunction: validArgsFunc, RunE: func(cmd *cobra.Command, args []string) error { client.OutputFormat = action.ShowAll - output, err := runShow(args, client, out) + err := addRegistryClient(client) + if err != nil { + return err + } + output, err := runShow(args, client) if err != nil { return err } @@ -101,7 +105,11 @@ func newShowCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { ValidArgsFunction: validArgsFunc, RunE: func(cmd *cobra.Command, args []string) error { client.OutputFormat = action.ShowValues - output, err := runShow(args, client, out) + err := addRegistryClient(client) + if err != nil { + return err + } + output, err := runShow(args, client) if err != nil { return err } @@ -118,7 +126,11 @@ func newShowCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { ValidArgsFunction: validArgsFunc, RunE: func(cmd *cobra.Command, args []string) error { client.OutputFormat = action.ShowChart - output, err := runShow(args, client, out) + err := addRegistryClient(client) + if err != nil { + return err + } + output, err := runShow(args, client) if err != nil { return err } @@ -135,7 +147,11 @@ func newShowCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { ValidArgsFunction: validArgsFunc, RunE: func(cmd *cobra.Command, args []string) error { client.OutputFormat = action.ShowReadme - output, err := runShow(args, client, out) + err := addRegistryClient(client) + if err != nil { + return err + } + output, err := runShow(args, client) if err != nil { return err } @@ -152,7 +168,11 @@ func newShowCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { ValidArgsFunction: validArgsFunc, RunE: func(cmd *cobra.Command, args []string) error { client.OutputFormat = action.ShowCRDs - output, err := runShow(args, client, out) + err := addRegistryClient(client) + if err != nil { + return err + } + output, err := runShow(args, client) if err != nil { return err } @@ -191,16 +211,25 @@ func addShowFlags(subCmd *cobra.Command, client *action.Show) { } } -func runShow(args []string, client *action.Show, out io.Writer) (string, error) { +func runShow(args []string, client *action.Show) (string, error) { debug("Original chart version: %q", client.Version) if client.Version == "" && client.Devel { debug("setting version to >0.0.0-0") client.Version = ">0.0.0-0" } - cp, err := client.ChartPathOptions.LocateChart(args[0], out, settings) + cp, err := client.ChartPathOptions.LocateChart(args[0], settings) if err != nil { return "", err } return client.Run(cp) } + +func addRegistryClient(client *action.Show) error { + registryClient, err := newRegistryClient(client.CertFile, client.KeyFile, client.CaFile, client.InsecureSkipTLSverify) + if err != nil { + return fmt.Errorf("missing registry client: %w", err) + } + client.SetRegistryClient(registryClient) + return nil +} diff --git a/cmd/helm/template.go b/cmd/helm/template.go index 0ddd0c551..3bc70f995 100644 --- a/cmd/helm/template.go +++ b/cmd/helm/template.go @@ -73,6 +73,12 @@ func newTemplateCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { client.KubeVersion = parsedKubeVersion } + registryClient, err := newRegistryClient(client.CertFile, client.KeyFile, client.CaFile, client.InsecureSkipTLSverify) + if err != nil { + return fmt.Errorf("missing registry client: %w", err) + } + client.SetRegistryClient(registryClient) + client.DryRun = true client.ReleaseName = "release-name" client.Replace = true // Skip the name check diff --git a/cmd/helm/upgrade.go b/cmd/helm/upgrade.go index bf9b30ffb..145d342b7 100644 --- a/cmd/helm/upgrade.go +++ b/cmd/helm/upgrade.go @@ -90,6 +90,12 @@ func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { RunE: func(cmd *cobra.Command, args []string) error { client.Namespace = settings.Namespace() + registryClient, err := newRegistryClient(client.CertFile, client.KeyFile, client.CaFile, client.InsecureSkipTLSverify) + if err != nil { + return fmt.Errorf("missing registry client: %w", err) + } + client.SetRegistryClient(registryClient) + // Fixes #7002 - Support reading values from STDIN for `upgrade` command // Must load values AFTER determining if we have to call install so that values loaded from stdin are are not read twice if client.Install { @@ -136,7 +142,7 @@ func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { client.Version = ">0.0.0-0" } - chartPath, err := client.ChartPathOptions.LocateChart(args[1], out, settings) + chartPath, err := client.ChartPathOptions.LocateChart(args[1], settings) if err != nil { return err } diff --git a/pkg/action/install.go b/pkg/action/install.go index bc228e0c0..5af61b932 100644 --- a/pkg/action/install.go +++ b/pkg/action/install.go @@ -20,7 +20,6 @@ import ( "bytes" "context" "fmt" - "io" "io/ioutil" "net/url" "os" @@ -137,6 +136,11 @@ func NewInstall(cfg *Configuration) *Install { return in } +// SetRegistryClient sets the registry client for the install action +func (i *Install) SetRegistryClient(registryClient *registry.Client) { + i.ChartPathOptions.registryClient = registryClient +} + func (i *Install) installCRDs(crds []chart.CRD) error { // We do these one file at a time in the order they were read. totalItems := []*resource.Info{} @@ -676,22 +680,11 @@ OUTER: // - URL // // If 'verify' was set on ChartPathOptions, this will attempt to also verify the chart. -func (c *ChartPathOptions) LocateChart(name string, out io.Writer, settings *cli.EnvSettings) (string, error) { - // If there is no registry client and the name is in an OCI registry return - // an error and a lookup will not occur. - if registry.IsOCI(name) { - if (c.CertFile != "" && c.KeyFile != "") || c.CaFile != "" || c.InsecureSkipTLSverify { - registryClient, err := registry.NewRegistryClientWithTLS(out, c.CertFile, c.KeyFile, c.CaFile, - c.InsecureSkipTLSverify, settings.RegistryConfig, settings.Debug) - if err != nil { - return "", err - } - c.registryClient = registryClient - } - if c.registryClient == nil { - return "", fmt.Errorf("unable to lookup chart %q, missing registry client", name) - } +func (c *ChartPathOptions) LocateChart(name string, settings *cli.EnvSettings) (string, error) { + if registry.IsOCI(name) && c.registryClient == nil { + return "", fmt.Errorf("unable to lookup chart %q, missing registry client", name) } + name = strings.TrimSpace(name) version := strings.TrimSpace(c.Version) diff --git a/pkg/action/pull.go b/pkg/action/pull.go index 102e4a588..9952fb2ed 100644 --- a/pkg/action/pull.go +++ b/pkg/action/pull.go @@ -18,7 +18,6 @@ package action import ( "fmt" - "io" "io/ioutil" "os" "path/filepath" @@ -48,7 +47,6 @@ type Pull struct { UntarDir string DestDir string cfg *Configuration - out io.Writer } type PullOpt func(*Pull) @@ -59,13 +57,6 @@ func WithConfig(cfg *Configuration) PullOpt { } } -// WithOptWriter sets the registryOut field on the push configuration object. -func WithPullOptWriter(out io.Writer) PullOpt { - return func(p *Pull) { - p.out = out - } -} - // NewPull creates a new Pull object. func NewPull() *Pull { return NewPullWithOpts() @@ -81,6 +72,11 @@ func NewPullWithOpts(opts ...PullOpt) *Pull { return p } +// SetRegistryClient sets the registry client on the pull configuration object. +func (p *Pull) SetRegistryClient(client *registry.Client) { + p.cfg.RegistryClient = client +} + // Run executes 'helm pull' against the given release. func (p *Pull) Run(chartRef string) (string, error) { var out strings.Builder @@ -102,16 +98,6 @@ func (p *Pull) Run(chartRef string) (string, error) { } if registry.IsOCI(chartRef) { - // Provide a tls enabled client for the pull command if the user has - // specified the cert file or key file or ca file. - if (p.ChartPathOptions.CertFile != "" && p.ChartPathOptions.KeyFile != "") || p.ChartPathOptions.CaFile != "" || p.ChartPathOptions.InsecureSkipTLSverify { - registryClient, err := registry.NewRegistryClientWithTLS(p.out, p.ChartPathOptions.CertFile, p.ChartPathOptions.KeyFile, p.ChartPathOptions.CaFile, - p.ChartPathOptions.InsecureSkipTLSverify, p.Settings.RegistryConfig, p.Settings.Debug) - if err != nil { - return out.String(), err - } - p.cfg.RegistryClient = registryClient - } c.Options = append(c.Options, getter.WithRegistryClient(p.cfg.RegistryClient)) c.RegistryClient = p.cfg.RegistryClient diff --git a/pkg/action/push.go b/pkg/action/push.go index 21f6fb947..892006406 100644 --- a/pkg/action/push.go +++ b/pkg/action/push.go @@ -90,21 +90,11 @@ func (p *Push) Run(chartRef string, remote string) (string, error) { Pushers: pusher.All(p.Settings), Options: []pusher.Option{ pusher.WithTLSClientConfig(p.certFile, p.keyFile, p.caFile), + pusher.WithInsecureSkipTLSVerify(p.insecureSkipTLSverify), }, } if registry.IsOCI(remote) { - // Provide a tls enabled client for the pull command if the user has - // specified the cert file or key file or ca file. - if (p.certFile != "" && p.keyFile != "") || p.caFile != "" || p.insecureSkipTLSverify { - registryClient, err := registry.NewRegistryClientWithTLS(p.out, p.certFile, p.keyFile, p.caFile, - p.insecureSkipTLSverify, p.Settings.RegistryConfig, p.Settings.Debug) - if err != nil { - return out.String(), err - } - p.cfg.RegistryClient = registryClient - } - // Don't use the default registry client if tls options are set. c.Options = append(c.Options, pusher.WithRegistryClient(p.cfg.RegistryClient)) } diff --git a/pkg/action/show.go b/pkg/action/show.go index 9ba85234d..8cf231593 100644 --- a/pkg/action/show.go +++ b/pkg/action/show.go @@ -28,6 +28,7 @@ import ( "helm.sh/helm/v3/pkg/chart" "helm.sh/helm/v3/pkg/chart/loader" "helm.sh/helm/v3/pkg/chartutil" + "helm.sh/helm/v3/pkg/registry" ) // ShowOutputFormat is the format of the output of `helm show` @@ -82,6 +83,11 @@ func NewShowWithConfig(output ShowOutputFormat, cfg *Configuration) *Show { return sh } +// SetRegistryClient sets the registry client to use when pulling a chart from a registry. +func (s *Show) SetRegistryClient(client *registry.Client) { + s.ChartPathOptions.registryClient = client +} + // Run executes 'helm show' against the given release. func (s *Show) Run(chartpath string) (string, error) { if s.chart == nil { diff --git a/pkg/action/upgrade.go b/pkg/action/upgrade.go index 8fb895df0..829be51df 100644 --- a/pkg/action/upgrade.go +++ b/pkg/action/upgrade.go @@ -32,6 +32,7 @@ import ( "helm.sh/helm/v3/pkg/chartutil" "helm.sh/helm/v3/pkg/kube" "helm.sh/helm/v3/pkg/postrender" + "helm.sh/helm/v3/pkg/registry" "helm.sh/helm/v3/pkg/release" "helm.sh/helm/v3/pkg/releaseutil" "helm.sh/helm/v3/pkg/storage/driver" @@ -122,6 +123,11 @@ func NewUpgrade(cfg *Configuration) *Upgrade { return up } +// SetRegistryClient sets the registry client to use when fetching charts. +func (u *Upgrade) SetRegistryClient(client *registry.Client) { + u.ChartPathOptions.registryClient = client +} + // Run executes the upgrade on the given release. func (u *Upgrade) Run(name string, chart *chart.Chart, vals map[string]interface{}) (*release.Release, error) { ctx := context.Background()