From dd78ac146ce43d44263b8316644c67315dc69bd6 Mon Sep 17 00:00:00 2001 From: Matei Pinzaru <201221140+mateipinzaru-parloa@users.noreply.github.com> Date: Fri, 15 May 2026 20:05:09 +0300 Subject: [PATCH 1/4] fix: preserve OCI registry identity tokens Signed-off-by: Matei Pinzaru <201221140+mateipinzaru-parloa@users.noreply.github.com> --- go.mod | 2 +- pkg/clients/helm/client.go | 97 +++++++- pkg/clients/helm/repocreds.go | 28 +++ pkg/clients/helm/repocreds_test.go | 310 ++++++++++++++++++++++++++ pkg/clients/registryauth/auth.go | 5 +- pkg/clients/registryauth/auth_test.go | 101 +++++++++ 6 files changed, 537 insertions(+), 6 deletions(-) create mode 100644 pkg/clients/helm/repocreds_test.go diff --git a/go.mod b/go.mod index 0babc76b..555d9782 100644 --- a/go.mod +++ b/go.mod @@ -21,6 +21,7 @@ require ( k8s.io/apiextensions-apiserver v0.33.3 k8s.io/apimachinery v0.34.2 k8s.io/client-go v0.34.2 + oras.land/oras-go/v2 v2.6.0 sigs.k8s.io/controller-runtime v0.19.1 sigs.k8s.io/controller-tools v0.18.0 sigs.k8s.io/kustomize/api v0.19.0 @@ -198,7 +199,6 @@ require ( k8s.io/kube-openapi v0.0.0-20250710124328-f3f2b991d03b // indirect k8s.io/kubectl v0.33.3 // indirect k8s.io/utils v0.0.0-20250604170112-4c0f3b243397 // indirect - oras.land/oras-go/v2 v2.6.0 // indirect sigs.k8s.io/json v0.0.0-20241014173422-cfa47c3a1cc8 // indirect sigs.k8s.io/randfill v1.0.0 // indirect sigs.k8s.io/structured-merge-diff/v6 v6.3.0 // indirect diff --git a/pkg/clients/helm/client.go b/pkg/clients/helm/client.go index 23d7f4a7..13984d94 100644 --- a/pkg/clients/helm/client.go +++ b/pkg/clients/helm/client.go @@ -17,7 +17,9 @@ limitations under the License. package helm import ( + "crypto/tls" "fmt" + "net/http" "net/url" "os" "path" @@ -34,6 +36,7 @@ import ( "helm.sh/helm/v3/pkg/registry" "helm.sh/helm/v3/pkg/release" "k8s.io/client-go/rest" + registryAuth "oras.land/oras-go/v2/registry/remote/auth" ktype "sigs.k8s.io/kustomize/api/types" clusterv1beta1 "github.com/crossplane-contrib/provider-helm/apis/cluster/release/v1beta1" @@ -76,6 +79,9 @@ type client struct { rollbackClient *action.Rollback uninstallClient *action.Uninstall loginClient *action.RegistryLogin + // registryClient is the default registry client. pullChart resets to this + // before each pull so an identity-token-scoped client does not leak. + registryClient *registry.Client } // ArgsApplier defines helm client arguments helper @@ -155,6 +161,7 @@ func NewClient(log logging.Logger, restConfig *rest.Config, argAppliers ...ArgsA rollbackClient: rb, uninstallClient: uic, loginClient: lc, + registryClient: rc, }, nil } @@ -221,12 +228,15 @@ func (hc *client) pullChart(chartUrl, chartName, chartVersion, chartRepo string, pc.Version = version chartRef = ociURL.String() } - pc.Username = creds.Username - pc.Password = creds.Password + configurePullCredentials(pc, chartUrl, chartRepo, creds) pc.DestDir = chartDir - if creds.Username != "" && creds.Password != "" { + if err := hc.configureRegistryClient(chartUrl, chartRepo, creds); err != nil { + return err + } + + if creds.hasBasicAuth() && !usesIdentityToken(chartUrl, chartRepo, creds) { err := hc.login(chartUrl, chartRepo, creds, pc.InsecureSkipTLSverify) if err != nil { return err @@ -241,6 +251,87 @@ func (hc *client) pullChart(chartUrl, chartName, chartVersion, chartRepo string, return nil } +func (hc *client) configureRegistryClient(chartUrl, chartRepo string, creds *RepoCreds) error { + hc.pullClient.SetRegistryClient(hc.registryClient) + + if !usesIdentityToken(chartUrl, chartRepo, creds) { + return nil + } + + parsedURL, err := url.Parse(ociURL(chartUrl, chartRepo)) + if err != nil { + return errors.Wrap(err, errFailedToParseURL) + } + + rc, err := identityTokenRegistryClient(parsedURL.Host, creds, hc.pullClient.InsecureSkipTLSverify, hc.pullClient.PlainHTTP) + if err != nil { + return err + } + hc.pullClient.SetRegistryClient(rc) + return nil +} + +func usesIdentityToken(chartUrl, chartRepo string, creds *RepoCreds) bool { + return creds.hasIdentityToken() && registry.IsOCI(ociURL(chartUrl, chartRepo)) +} + +func configurePullCredentials(pc *action.Pull, chartUrl, chartRepo string, creds *RepoCreds) { + pc.Username = "" + pc.Password = "" + if creds.hasBasicAuth() && !usesIdentityToken(chartUrl, chartRepo, creds) { + pc.Username = creds.Username + pc.Password = creds.Password + } +} + +func ociURL(chartUrl, chartRepo string) string { + if chartUrl != "" { + return chartUrl + } + return chartRepo +} + +func identityTokenRegistryClient(host string, creds *RepoCreds, insecureSkipTLSVerify, plainHTTP bool) (*registry.Client, error) { + httpClient := registryHTTPClient(insecureSkipTLSVerify) + authorizer := registryAuth.Client{ + Client: httpClient, + Credential: registryAuth.StaticCredential(host, creds.registryCredential()), + } + + opts := []registry.ClientOption{ + registry.ClientOptHTTPClient(httpClient), + registry.ClientOptAuthorizer(authorizer), + } + if plainHTTP { + opts = append(opts, registry.ClientOptPlainHTTP()) + } + + return registry.NewClient(opts...) +} + +func registryHTTPClient(insecureSkipTLSVerify bool) *http.Client { + transport := registry.NewTransport(false) + if !insecureSkipTLSVerify { + return &http.Client{Transport: transport} + } + + base, ok := transport.Base.(*http.Transport) + if !ok { + return &http.Client{Transport: transport} + } + + base = base.Clone() + if base.TLSClientConfig == nil { + base.TLSClientConfig = &tls.Config{} //nolint:gosec // This honors the Release's explicit insecureSkipTLSVerify setting. + } else { + base.TLSClientConfig = base.TLSClientConfig.Clone() + } + base.TLSClientConfig.InsecureSkipVerify = true //nolint:gosec // This honors the Release's explicit insecureSkipTLSVerify setting. + transport.Base = base + + return &http.Client{Transport: transport} +} + func (hc *client) login(chartUrl, chartRepo string, creds *RepoCreds, insecure bool) error { ociURL := chartUrl if chartUrl == "" { diff --git a/pkg/clients/helm/repocreds.go b/pkg/clients/helm/repocreds.go index a36c9bfd..d25ad1f8 100644 --- a/pkg/clients/helm/repocreds.go +++ b/pkg/clients/helm/repocreds.go @@ -1,7 +1,35 @@ package helm +import registryAuth "oras.land/oras-go/v2/registry/remote/auth" + // RepoCreds keeps auth information to access a Helm Chart type RepoCreds struct { Username string Password string + + // IdentityToken is a Docker credential-helper identity token. + // ORAS models this value as a refresh token. + // For OCI chart pulls, identity-token credentials take precedence over + // Helm's basic-auth login path. + IdentityToken string +} + +func (c *RepoCreds) hasBasicAuth() bool { + return c != nil && c.Username != "" && c.Password != "" +} + +func (c *RepoCreds) hasIdentityToken() bool { + return c != nil && c.IdentityToken != "" +} + +func (c *RepoCreds) registryCredential() registryAuth.Credential { + if c == nil { + return registryAuth.EmptyCredential + } + + return registryAuth.Credential{ + Username: c.Username, + Password: c.Password, + RefreshToken: c.IdentityToken, + } } diff --git a/pkg/clients/helm/repocreds_test.go b/pkg/clients/helm/repocreds_test.go new file mode 100644 index 00000000..d245e4d8 --- /dev/null +++ b/pkg/clients/helm/repocreds_test.go @@ -0,0 +1,310 @@ +/* +Copyright 2026 The Crossplane 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 helm + +import ( + "context" + "fmt" + "net/http" + "net/http/httptest" + "strings" + "sync/atomic" + "testing" + + "github.com/google/go-cmp/cmp" + "helm.sh/helm/v3/pkg/action" + "helm.sh/helm/v3/pkg/registry" + registryAuth "oras.land/oras-go/v2/registry/remote/auth" +) + +func TestRepoCredsRegistryCredential(t *testing.T) { + type want struct { + basicAuth bool + identityToken bool + registryCreds registryAuth.Credential + } + + cases := map[string]struct { + creds *RepoCreds + want want + }{ + "Nil": { + creds: nil, + want: want{ + registryCreds: registryAuth.EmptyCredential, + }, + }, + "UsernamePassword": { + creds: &RepoCreds{ + Username: "testuser", + Password: "testpass", + }, + want: want{ + basicAuth: true, + registryCreds: registryAuth.Credential{ + Username: "testuser", + Password: "testpass", + }, + }, + }, + "IdentityToken": { + creds: &RepoCreds{ + Username: "", + IdentityToken: "refresh-token", + }, + want: want{ + identityToken: true, + registryCreds: registryAuth.Credential{ + Username: "", + RefreshToken: "refresh-token", + }, + }, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + if got := tc.creds.hasBasicAuth(); got != tc.want.basicAuth { + t.Errorf("hasBasicAuth() = %v, want %v", got, tc.want.basicAuth) + } + + if got := tc.creds.hasIdentityToken(); got != tc.want.identityToken { + t.Errorf("hasIdentityToken() = %v, want %v", got, tc.want.identityToken) + } + + if diff := cmp.Diff(tc.want.registryCreds, tc.creds.registryCredential()); diff != "" { + t.Errorf("registryCredential(): -want, +got:\n%s", diff) + } + }) + } +} + +func TestRegistryHTTPClientInsecureSkipTLSVerify(t *testing.T) { + server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusNoContent) + })) + defer server.Close() + + cases := map[string]struct { + insecureSkipTLSVerify bool + wantErr bool + }{ + "DefaultTLSVerification": { + wantErr: true, + }, + "InsecureSkipTLSVerify": { + insecureSkipTLSVerify: true, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, server.URL, nil) + if err != nil { + t.Fatalf("http.NewRequestWithContext() error: %v", err) + } + + resp, err := registryHTTPClient(tc.insecureSkipTLSVerify).Do(req) + if resp != nil { + defer resp.Body.Close() + } + + if tc.wantErr { + if err == nil { + t.Fatal("registryHTTPClient().Do() error: want error, got nil") + } + return + } + + if err != nil { + t.Fatalf("registryHTTPClient().Do() error: %v", err) + } + }) + } +} + +func TestConfigurePullCredentials(t *testing.T) { + cases := map[string]struct { + chartURL string + chartRepo string + creds *RepoCreds + wantUser string + wantPass string + }{ + "Nil": {}, + "UsernamePassword": { + creds: &RepoCreds{ + Username: "testuser", + Password: "testpass", + }, + wantUser: "testuser", + wantPass: "testpass", + }, + "IdentityTokenOCI": { + chartRepo: "oci://registry.example.com/charts", + creds: &RepoCreds{ + Username: "", + IdentityToken: "refresh-token", + }, + }, + "IdentityTokenWithBasicAuthOCI": { + chartURL: "oci://registry.example.com/charts/my-chart:1.0.0", + creds: &RepoCreds{ + Username: "testuser", + Password: "testpass", + IdentityToken: "refresh-token", + }, + }, + "IdentityTokenWithBasicAuthNonOCI": { + chartRepo: "https://charts.example.com", + creds: &RepoCreds{ + Username: "testuser", + Password: "testpass", + IdentityToken: "refresh-token", + }, + wantUser: "testuser", + wantPass: "testpass", + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + pc := action.NewPull() + pc.Username = "stale-user" + pc.Password = "stale-pass" + + configurePullCredentials(pc, tc.chartURL, tc.chartRepo, tc.creds) + + if pc.Username != tc.wantUser { + t.Errorf("Username = %q, want %q", pc.Username, tc.wantUser) + } + if pc.Password != tc.wantPass { + t.Errorf("Password = %q, want %q", pc.Password, tc.wantPass) + } + }) + } +} + +func TestConfigureRegistryClientResetsDefault(t *testing.T) { + defaultClient, err := registry.NewClient() + if err != nil { + t.Fatalf("registry.NewClient() error: %v", err) + } + actionConfig := &action.Configuration{RegistryClient: defaultClient} + hc := &client{ + pullClient: action.NewPullWithOpts(action.WithConfig(actionConfig)), + registryClient: defaultClient, + } + + if err := hc.configureRegistryClient("oci://registry.example.com/charts", "", &RepoCreds{ + Username: "", + IdentityToken: "refresh-token", + }); err != nil { + t.Fatalf("configureRegistryClient() identity token error: %v", err) + } + identityTokenClient := actionConfig.RegistryClient + if identityTokenClient == defaultClient { + t.Fatal("configureRegistryClient() did not install identity-token registry client") + } + + if err := hc.configureRegistryClient("oci://registry.example.com/charts", "", &RepoCreds{ + Username: "testuser", + Password: "testpass", + }); err != nil { + t.Fatalf("configureRegistryClient() basic auth error: %v", err) + } + if actionConfig.RegistryClient != defaultClient { + t.Fatal("configureRegistryClient() did not reset to default registry client") + } +} + +func TestIdentityTokenRegistryClientUsesRefreshToken(t *testing.T) { + const ( + refreshToken = "refresh-token" + accessToken = "access-token" + service = "test-service" + scope = "repository:test:pull" + ) + + var tokenRequested atomic.Bool + var authorizedTagsRequested atomic.Bool + var server *httptest.Server + server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/v2/test/tags/list": + if got := r.Header.Get("Authorization"); got != "Bearer "+accessToken { + w.Header().Set("WWW-Authenticate", fmt.Sprintf(`Bearer realm="%s/token",service="%s",scope="%s"`, server.URL, service, scope)) + w.WriteHeader(http.StatusUnauthorized) + return + } + authorizedTagsRequested.Store(true) + w.Header().Set("Content-Type", "application/json") + fmt.Fprint(w, `{"name":"test","tags":["1.0.0"]}`) + case "/token": + tokenRequested.Store(true) + if r.Method != http.MethodPost { + t.Errorf("token request method = %s, want %s", r.Method, http.MethodPost) + } + if err := r.ParseForm(); err != nil { + t.Errorf("ParseForm() error: %v", err) + w.WriteHeader(http.StatusInternalServerError) + return + } + if got := r.PostForm.Get("grant_type"); got != "refresh_token" { + t.Errorf("grant_type = %q, want refresh_token", got) + } + if got := r.PostForm.Get("refresh_token"); got != refreshToken { + t.Errorf("refresh_token = %q, want %q", got, refreshToken) + } + if got := r.PostForm.Get("service"); got != service { + t.Errorf("service = %q, want %q", got, service) + } + if got := r.PostForm.Get("scope"); got != scope { + t.Errorf("scope = %q, want %q", got, scope) + } + w.Header().Set("Content-Type", "application/json") + fmt.Fprintf(w, `{"access_token":%q}`, accessToken) + default: + t.Errorf("unexpected request path: %s", r.URL.Path) + http.NotFound(w, r) + } + })) + defer server.Close() + + host := strings.TrimPrefix(server.URL, "http://") + rc, err := identityTokenRegistryClient(host, &RepoCreds{ + Username: "", + IdentityToken: refreshToken, + }, false, true) + if err != nil { + t.Fatalf("identityTokenRegistryClient() error: %v", err) + } + + tags, err := rc.Tags(host + "/test") + if err != nil { + t.Fatalf("Tags() error: %v", err) + } + if diff := cmp.Diff([]string{"1.0.0"}, tags); diff != "" { + t.Errorf("Tags(): -want, +got:\n%s", diff) + } + if !tokenRequested.Load() { + t.Fatal("token endpoint was not requested") + } + if !authorizedTagsRequested.Load() { + t.Fatal("authorized tags endpoint was not requested") + } +} diff --git a/pkg/clients/registryauth/auth.go b/pkg/clients/registryauth/auth.go index 4c8bdc52..9d028c17 100644 --- a/pkg/clients/registryauth/auth.go +++ b/pkg/clients/registryauth/auth.go @@ -210,7 +210,8 @@ func (r *Resolver) resolveCredentialsFromKeychain(ctx context.Context, keychain } return &helmClient.RepoCreds{ - Username: authConfig.Username, - Password: authConfig.Password, + Username: authConfig.Username, + Password: authConfig.Password, + IdentityToken: authConfig.IdentityToken, }, nil } diff --git a/pkg/clients/registryauth/auth_test.go b/pkg/clients/registryauth/auth_test.go index 90a3af00..85a383c3 100644 --- a/pkg/clients/registryauth/auth_test.go +++ b/pkg/clients/registryauth/auth_test.go @@ -21,6 +21,7 @@ import ( "testing" "github.com/google/go-cmp/cmp" + "github.com/google/go-containerregistry/pkg/authn" "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" kerrors "k8s.io/apimachinery/pkg/api/errors" @@ -341,3 +342,103 @@ func TestResolveCluster(t *testing.T) { }) } } + +func TestResolveCredentialsFromKeychain(t *testing.T) { + type args struct { + keychain authn.Keychain + } + type want struct { + creds *helmClient.RepoCreds + err error + } + + cases := map[string]struct { + args args + want want + }{ + "UsernamePassword": { + args: args{ + keychain: staticKeychain{ + authenticator: authn.FromConfig(authn.AuthConfig{ + Username: testUsername, + Password: testPassword, + }), + }, + }, + want: want{ + creds: &helmClient.RepoCreds{ + Username: testUsername, + Password: testPassword, + }, + }, + }, + "IdentityToken": { + args: args{ + keychain: staticKeychain{ + authenticator: authn.FromConfig(authn.AuthConfig{ + Username: "", + IdentityToken: "refresh-token", + }), + }, + }, + want: want{ + creds: &helmClient.RepoCreds{ + Username: "", + IdentityToken: "refresh-token", + }, + }, + }, + "ResolveErrorReturnsEmptyCredentials": { + args: args{ + keychain: staticKeychain{ + err: errBoom, + }, + }, + want: want{ + creds: &helmClient.RepoCreds{}, + }, + }, + "AuthorizationErrorReturnsEmptyCredentials": { + args: args{ + keychain: staticKeychain{ + authenticator: errorAuthenticator{err: errBoom}, + }, + }, + want: want{ + creds: &helmClient.RepoCreds{}, + }, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + resolver := NewResolver(&test.MockClient{}) + got, err := resolver.resolveCredentialsFromKeychain(context.Background(), tc.args.keychain, nil) + + if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" { + t.Errorf("resolveCredentialsFromKeychain() error: -want, +got:\n%s", diff) + } + + if diff := cmp.Diff(tc.want.creds, got); diff != "" { + t.Errorf("resolveCredentialsFromKeychain() creds: -want, +got:\n%s", diff) + } + }) + } +} + +type staticKeychain struct { + authenticator authn.Authenticator + err error +} + +func (s staticKeychain) Resolve(authn.Resource) (authn.Authenticator, error) { + return s.authenticator, s.err +} + +type errorAuthenticator struct { + err error +} + +func (e errorAuthenticator) Authorization() (*authn.AuthConfig, error) { + return nil, e.err +} From 81c827962db5c659b3a3432e4a96a064a8d1ed84 Mon Sep 17 00:00:00 2001 From: Matei Pinzaru <201221140+mateipinzaru-parloa@users.noreply.github.com> Date: Fri, 15 May 2026 22:25:52 +0300 Subject: [PATCH 2/4] fix: harden OCI identity token chart pulls Signed-off-by: Matei Pinzaru <201221140+mateipinzaru-parloa@users.noreply.github.com> --- pkg/clients/helm/client.go | 57 +++++++--- pkg/clients/helm/repocreds.go | 8 +- pkg/clients/helm/repocreds_test.go | 162 +++++++++++++++++++++++++++-- 3 files changed, 200 insertions(+), 27 deletions(-) diff --git a/pkg/clients/helm/client.go b/pkg/clients/helm/client.go index 13984d94..afbdcd4b 100644 --- a/pkg/clients/helm/client.go +++ b/pkg/clients/helm/client.go @@ -36,7 +36,8 @@ import ( "helm.sh/helm/v3/pkg/registry" "helm.sh/helm/v3/pkg/release" "k8s.io/client-go/rest" - registryAuth "oras.land/oras-go/v2/registry/remote/auth" + orasauth "oras.land/oras-go/v2/registry/remote/auth" + orasretry "oras.land/oras-go/v2/registry/remote/retry" ktype "sigs.k8s.io/kustomize/api/types" clusterv1beta1 "github.com/crossplane-contrib/provider-helm/apis/cluster/release/v1beta1" @@ -55,6 +56,9 @@ const ( errFailedToLoadChart = "failed to load chart" errUnexpectedDirContentTmpl = "expected 1 .tgz chart file, got [%s]" errFailedToParseURL = "failed to parse URL" + errMissingOCIRegistryHostTmpl = "missing OCI registry host in url [%s]" + errFailedToCreateRegistryClient = "failed to create identity-token registry client" + errUnexpectedRegistryTransportTmpl = "expected Helm registry transport base to be *http.Transport, got [%T]" errFailedToLogin = "failed to login to registry" errUnexpectedOCIUrlTmpl = "url not prefixed with oci://, got [%s]" devel = ">0.0.0-0" @@ -80,7 +84,7 @@ type client struct { uninstallClient *action.Uninstall loginClient *action.RegistryLogin // registryClient is the default registry client. pullChart resets to this - // before each pull so an identity-token-scoped client does not leak. + // before and after each pull so an identity-token-scoped client does not leak. registryClient *registry.Client } @@ -210,6 +214,7 @@ func (hc *client) pullLatestChartVersion(chartUrl, chartName, chartVersion, char func (hc *client) pullChart(chartUrl, chartName, chartVersion, chartRepo string, creds *RepoCreds, chartDir string) error { pc := hc.pullClient + resetPullState(pc) chartRef := chartUrl if chartUrl == "" { @@ -232,6 +237,7 @@ func (hc *client) pullChart(chartUrl, chartName, chartVersion, chartRepo string, pc.DestDir = chartDir + defer hc.pullClient.SetRegistryClient(hc.registryClient) if err := hc.configureRegistryClient(chartUrl, chartRepo, creds); err != nil { return err } @@ -262,6 +268,9 @@ func (hc *client) configureRegistryClient(chartUrl, chartRepo string, creds *Rep if err != nil { return errors.Wrap(err, errFailedToParseURL) } + if parsedURL.Host == "" { + return errors.Errorf(errMissingOCIRegistryHostTmpl, ociURL(chartUrl, chartRepo)) + } rc, err := identityTokenRegistryClient(parsedURL.Host, creds, hc.pullClient.InsecureSkipTLSverify, hc.pullClient.PlainHTTP) if err != nil { @@ -275,6 +284,13 @@ func usesIdentityToken(chartUrl, chartRepo string, creds *RepoCreds) bool { return creds.hasIdentityToken() && registry.IsOCI(ociURL(chartUrl, chartRepo)) } +func resetPullState(pc *action.Pull) { + pc.Username = "" + pc.Password = "" + pc.Version = "" + pc.RepoURL = "" +} + func configurePullCredentials(pc *action.Pull, chartUrl, chartRepo string, creds *RepoCreds) { pc.Username = "" pc.Password = "" @@ -292,10 +308,14 @@ func ociURL(chartUrl, chartRepo string) string { } func identityTokenRegistryClient(host string, creds *RepoCreds, insecureSkipTLSVerify, plainHTTP bool) (*registry.Client, error) { - httpClient := registryHTTPClient(insecureSkipTLSVerify) - authorizer := registryAuth.Client{ + httpClient, err := registryHTTPClient(insecureSkipTLSVerify) + if err != nil { + return nil, errors.Wrap(err, errFailedToCreateRegistryClient) + } + + authorizer := orasauth.Client{ Client: httpClient, - Credential: registryAuth.StaticCredential(host, creds.registryCredential()), + Credential: orasauth.StaticCredential(host, creds.registryCredential()), } opts := []registry.ClientOption{ @@ -306,18 +326,24 @@ func identityTokenRegistryClient(host string, creds *RepoCreds, insecureSkipTLSV opts = append(opts, registry.ClientOptPlainHTTP()) } - return registry.NewClient(opts...) + rc, err := registry.NewClient(opts...) + return rc, errors.Wrap(err, errFailedToCreateRegistryClient) +} + +func registryHTTPClient(insecureSkipTLSVerify bool) (*http.Client, error) { + // Keep Helm's normal retry transport. The flag disables Helm registry + // debug logging; it is unrelated to TLS verification. + return registryHTTPClientForTransport(registry.NewTransport(false), insecureSkipTLSVerify) } -func registryHTTPClient(insecureSkipTLSVerify bool) *http.Client { - transport := registry.NewTransport(false) +func registryHTTPClientForTransport(transport *orasretry.Transport, insecureSkipTLSVerify bool) (*http.Client, error) { if !insecureSkipTLSVerify { - return &http.Client{Transport: transport} + return &http.Client{Transport: transport}, nil } base, ok := transport.Base.(*http.Transport) if !ok { - return &http.Client{Transport: transport} + return nil, errors.Errorf(errUnexpectedRegistryTransportTmpl, transport.Base) } base = base.Clone() @@ -329,18 +355,15 @@ func registryHTTPClient(insecureSkipTLSVerify bool) *http.Client { base.TLSClientConfig.InsecureSkipVerify = true //nolint:gosec // This honors the Release's explicit insecureSkipTLSVerify setting. transport.Base = base - return &http.Client{Transport: transport} + return &http.Client{Transport: transport}, nil } func (hc *client) login(chartUrl, chartRepo string, creds *RepoCreds, insecure bool) error { - ociURL := chartUrl - if chartUrl == "" { - ociURL = chartRepo - } - if !registry.IsOCI(ociURL) { + registryURL := ociURL(chartUrl, chartRepo) + if !registry.IsOCI(registryURL) { return nil } - parsedURL, err := url.Parse(ociURL) + parsedURL, err := url.Parse(registryURL) if err != nil { return errors.Wrap(err, errFailedToParseURL) } diff --git a/pkg/clients/helm/repocreds.go b/pkg/clients/helm/repocreds.go index d25ad1f8..39ca5ef0 100644 --- a/pkg/clients/helm/repocreds.go +++ b/pkg/clients/helm/repocreds.go @@ -1,6 +1,6 @@ package helm -import registryAuth "oras.land/oras-go/v2/registry/remote/auth" +import orasauth "oras.land/oras-go/v2/registry/remote/auth" // RepoCreds keeps auth information to access a Helm Chart type RepoCreds struct { @@ -22,12 +22,12 @@ func (c *RepoCreds) hasIdentityToken() bool { return c != nil && c.IdentityToken != "" } -func (c *RepoCreds) registryCredential() registryAuth.Credential { +func (c *RepoCreds) registryCredential() orasauth.Credential { if c == nil { - return registryAuth.EmptyCredential + return orasauth.EmptyCredential } - return registryAuth.Credential{ + return orasauth.Credential{ Username: c.Username, Password: c.Password, RefreshToken: c.IdentityToken, diff --git a/pkg/clients/helm/repocreds_test.go b/pkg/clients/helm/repocreds_test.go index d245e4d8..196d5a64 100644 --- a/pkg/clients/helm/repocreds_test.go +++ b/pkg/clients/helm/repocreds_test.go @@ -21,21 +21,33 @@ import ( "fmt" "net/http" "net/http/httptest" + "os" "strings" "sync/atomic" "testing" + "github.com/crossplane/crossplane-runtime/v2/pkg/logging" "github.com/google/go-cmp/cmp" "helm.sh/helm/v3/pkg/action" + "helm.sh/helm/v3/pkg/chart" + "helm.sh/helm/v3/pkg/chartutil" + "helm.sh/helm/v3/pkg/cli" "helm.sh/helm/v3/pkg/registry" - registryAuth "oras.land/oras-go/v2/registry/remote/auth" + orasauth "oras.land/oras-go/v2/registry/remote/auth" + orasretry "oras.land/oras-go/v2/registry/remote/retry" ) +type roundTripperFunc func(*http.Request) (*http.Response, error) + +func (f roundTripperFunc) RoundTrip(req *http.Request) (*http.Response, error) { + return f(req) +} + func TestRepoCredsRegistryCredential(t *testing.T) { type want struct { basicAuth bool identityToken bool - registryCreds registryAuth.Credential + registryCreds orasauth.Credential } cases := map[string]struct { @@ -45,7 +57,7 @@ func TestRepoCredsRegistryCredential(t *testing.T) { "Nil": { creds: nil, want: want{ - registryCreds: registryAuth.EmptyCredential, + registryCreds: orasauth.EmptyCredential, }, }, "UsernamePassword": { @@ -55,7 +67,7 @@ func TestRepoCredsRegistryCredential(t *testing.T) { }, want: want{ basicAuth: true, - registryCreds: registryAuth.Credential{ + registryCreds: orasauth.Credential{ Username: "testuser", Password: "testpass", }, @@ -68,7 +80,7 @@ func TestRepoCredsRegistryCredential(t *testing.T) { }, want: want{ identityToken: true, - registryCreds: registryAuth.Credential{ + registryCreds: orasauth.Credential{ Username: "", RefreshToken: "refresh-token", }, @@ -118,7 +130,12 @@ func TestRegistryHTTPClientInsecureSkipTLSVerify(t *testing.T) { t.Fatalf("http.NewRequestWithContext() error: %v", err) } - resp, err := registryHTTPClient(tc.insecureSkipTLSVerify).Do(req) + httpClient, err := registryHTTPClient(tc.insecureSkipTLSVerify) + if err != nil { + t.Fatalf("registryHTTPClient() error: %v", err) + } + + resp, err := httpClient.Do(req) if resp != nil { defer resp.Body.Close() } @@ -137,6 +154,43 @@ func TestRegistryHTTPClientInsecureSkipTLSVerify(t *testing.T) { } } +func TestRegistryHTTPClientInsecureSkipTLSVerifyRequiresHTTPTransport(t *testing.T) { + transport := orasretry.NewTransport(roundTripperFunc(func(_ *http.Request) (*http.Response, error) { + return nil, nil + })) + + _, err := registryHTTPClientForTransport(transport, true) + if err == nil { + t.Fatal("registryHTTPClientForTransport() error: want error, got nil") + } + if !strings.Contains(err.Error(), "expected Helm registry transport base") { + t.Fatalf("registryHTTPClientForTransport() error = %q, want transport type context", err) + } +} + +func TestResetPullState(t *testing.T) { + pc := action.NewPull() + pc.Username = "stale-user" + pc.Password = "stale-pass" + pc.Version = "1.0.0" + pc.RepoURL = "https://charts.example.com" + + resetPullState(pc) + + if pc.Username != "" { + t.Errorf("Username = %q, want empty", pc.Username) + } + if pc.Password != "" { + t.Errorf("Password = %q, want empty", pc.Password) + } + if pc.Version != "" { + t.Errorf("Version = %q, want empty", pc.Version) + } + if pc.RepoURL != "" { + t.Errorf("RepoURL = %q, want empty", pc.RepoURL) + } +} + func TestConfigurePullCredentials(t *testing.T) { cases := map[string]struct { chartURL string @@ -199,6 +253,32 @@ func TestConfigurePullCredentials(t *testing.T) { } } +func TestConfigureRegistryClientRequiresOCIHost(t *testing.T) { + defaultClient, err := registry.NewClient() + if err != nil { + t.Fatalf("registry.NewClient() error: %v", err) + } + actionConfig := &action.Configuration{RegistryClient: defaultClient} + hc := &client{ + pullClient: action.NewPullWithOpts(action.WithConfig(actionConfig)), + registryClient: defaultClient, + } + + err = hc.configureRegistryClient("oci://", "", &RepoCreds{ + Username: "", + IdentityToken: "refresh-token", + }) + if err == nil { + t.Fatal("configureRegistryClient() error: want error, got nil") + } + if !strings.Contains(err.Error(), "missing OCI registry host") { + t.Fatalf("configureRegistryClient() error = %q, want missing host context", err) + } + if actionConfig.RegistryClient != defaultClient { + t.Fatal("configureRegistryClient() did not leave default registry client installed") + } +} + func TestConfigureRegistryClientResetsDefault(t *testing.T) { defaultClient, err := registry.NewClient() if err != nil { @@ -232,6 +312,76 @@ func TestConfigureRegistryClientResetsDefault(t *testing.T) { } } +func TestPullChartWithDirectURLClearsStalePullState(t *testing.T) { + archiveDir := t.TempDir() + archivePath, err := chartutil.Save(&chart.Chart{ + Metadata: &chart.Metadata{ + APIVersion: "v2", + Name: "test-chart", + Version: "1.0.0", + }, + }, archiveDir) + if err != nil { + t.Fatalf("chartutil.Save() error: %v", err) + } + archive, err := os.ReadFile(archivePath) + if err != nil { + t.Fatalf("os.ReadFile() error: %v", err) + } + + var requested atomic.Bool + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/test-chart-1.0.0.tgz" { + t.Errorf("request path = %s, want /test-chart-1.0.0.tgz", r.URL.Path) + } + requested.Store(true) + w.Header().Set("Content-Type", "application/gzip") + _, _ = w.Write(archive) + })) + defer server.Close() + + defaultClient, err := registry.NewClient() + if err != nil { + t.Fatalf("registry.NewClient() error: %v", err) + } + actionConfig := &action.Configuration{RegistryClient: defaultClient} + pc := action.NewPullWithOpts(action.WithConfig(actionConfig)) + pc.Settings = &cli.EnvSettings{} + pc.Username = "stale-user" + pc.Password = "stale-pass" + pc.Version = "9.9.9" + pc.RepoURL = "https://charts.example.com" + + hc := &client{ + log: logging.NewNopLogger(), + pullClient: pc, + registryClient: defaultClient, + } + + if err := hc.pullChart(server.URL+"/test-chart-1.0.0.tgz", "", "", "", nil, t.TempDir()); err != nil { + t.Fatalf("pullChart() error: %v", err) + } + + if !requested.Load() { + t.Fatal("chart archive was not requested") + } + if pc.Username != "" { + t.Errorf("Username = %q, want empty", pc.Username) + } + if pc.Password != "" { + t.Errorf("Password = %q, want empty", pc.Password) + } + if pc.Version != "" { + t.Errorf("Version = %q, want empty", pc.Version) + } + if pc.RepoURL != "" { + t.Errorf("RepoURL = %q, want empty", pc.RepoURL) + } + if actionConfig.RegistryClient != defaultClient { + t.Fatal("pullChart() did not leave default registry client installed") + } +} + func TestIdentityTokenRegistryClientUsesRefreshToken(t *testing.T) { const ( refreshToken = "refresh-token" From bbb6b469907ea87bffa7fbca064c10f4533840f9 Mon Sep 17 00:00:00 2001 From: Matei Pinzaru <201221140+mateipinzaru-parloa@users.noreply.github.com> Date: Sat, 16 May 2026 01:48:31 +0300 Subject: [PATCH 3/4] fix: simplify OCI identity-token chart pulls Signed-off-by: Matei Pinzaru <201221140+mateipinzaru-parloa@users.noreply.github.com> --- pkg/clients/helm/client.go | 85 +++++++++++---------------- pkg/clients/helm/repocreds_test.go | 93 ++++-------------------------- 2 files changed, 45 insertions(+), 133 deletions(-) diff --git a/pkg/clients/helm/client.go b/pkg/clients/helm/client.go index afbdcd4b..a61e8a83 100644 --- a/pkg/clients/helm/client.go +++ b/pkg/clients/helm/client.go @@ -51,17 +51,18 @@ const ( ) const ( - errFailedToCheckIfLocalChartExists = "failed to check if cached chart file exists" - errFailedToPullChart = "failed to pull chart" - errFailedToLoadChart = "failed to load chart" - errUnexpectedDirContentTmpl = "expected 1 .tgz chart file, got [%s]" - errFailedToParseURL = "failed to parse URL" - errMissingOCIRegistryHostTmpl = "missing OCI registry host in url [%s]" - errFailedToCreateRegistryClient = "failed to create identity-token registry client" - errUnexpectedRegistryTransportTmpl = "expected Helm registry transport base to be *http.Transport, got [%T]" - errFailedToLogin = "failed to login to registry" - errUnexpectedOCIUrlTmpl = "url not prefixed with oci://, got [%s]" - devel = ">0.0.0-0" + errFailedToCheckIfLocalChartExists = "failed to check if cached chart file exists" + errFailedToPullChart = "failed to pull chart" + errFailedToLoadChart = "failed to load chart" + errUnexpectedDirContentTmpl = "expected 1 .tgz chart file, got [%s]" + errFailedToParseURL = "failed to parse URL" + errMissingOCIRegistryHostTmpl = "missing OCI registry host in url [%s]" + errFailedToCreateRegistryHTTPClient = "failed to create identity-token registry HTTP client" + errFailedToCreateRegistryClient = "failed to create identity-token registry client" + errUnexpectedRegistryTransportTmpl = "expected Helm registry transport base to be *http.Transport, got [%T]" + errFailedToLogin = "failed to login to registry" + errUnexpectedOCIUrlTmpl = "url not prefixed with oci://, got [%s]" + devel = ">0.0.0-0" ) // Client is the interface to interact with Helm @@ -215,6 +216,11 @@ func (hc *client) pullLatestChartVersion(chartUrl, chartName, chartVersion, char func (hc *client) pullChart(chartUrl, chartName, chartVersion, chartRepo string, creds *RepoCreds, chartDir string) error { pc := hc.pullClient resetPullState(pc) + registryURL := chartUrl + if registryURL == "" { + registryURL = chartRepo + } + useIdentityToken := creds.hasIdentityToken() && registry.IsOCI(registryURL) chartRef := chartUrl if chartUrl == "" { @@ -226,24 +232,30 @@ func (hc *client) pullChart(chartUrl, chartName, chartVersion, chartRepo string, } pc.Version = chartVersion } else if registry.IsOCI(chartUrl) { - ociURL, version, err := resolveOCIChartVersion(chartUrl) + parsedURL, version, err := resolveOCIChartVersion(chartUrl) if err != nil { return err } pc.Version = version - chartRef = ociURL.String() + chartRef = parsedURL.String() + } + if creds.hasBasicAuth() && !useIdentityToken { + pc.Username = creds.Username + pc.Password = creds.Password } - configurePullCredentials(pc, chartUrl, chartRepo, creds) pc.DestDir = chartDir + hc.pullClient.SetRegistryClient(hc.registryClient) defer hc.pullClient.SetRegistryClient(hc.registryClient) - if err := hc.configureRegistryClient(chartUrl, chartRepo, creds); err != nil { - return err + if useIdentityToken { + if err := hc.configureIdentityTokenRegistryClient(registryURL, creds); err != nil { + return err + } } - if creds.hasBasicAuth() && !usesIdentityToken(chartUrl, chartRepo, creds) { - err := hc.login(chartUrl, chartRepo, creds, pc.InsecureSkipTLSverify) + if creds.hasBasicAuth() && !useIdentityToken { + err := hc.login(registryURL, creds, pc.InsecureSkipTLSverify) if err != nil { return err } @@ -257,19 +269,13 @@ func (hc *client) pullChart(chartUrl, chartName, chartVersion, chartRepo string, return nil } -func (hc *client) configureRegistryClient(chartUrl, chartRepo string, creds *RepoCreds) error { - hc.pullClient.SetRegistryClient(hc.registryClient) - - if !usesIdentityToken(chartUrl, chartRepo, creds) { - return nil - } - - parsedURL, err := url.Parse(ociURL(chartUrl, chartRepo)) +func (hc *client) configureIdentityTokenRegistryClient(registryURL string, creds *RepoCreds) error { + parsedURL, err := url.Parse(registryURL) if err != nil { return errors.Wrap(err, errFailedToParseURL) } if parsedURL.Host == "" { - return errors.Errorf(errMissingOCIRegistryHostTmpl, ociURL(chartUrl, chartRepo)) + return errors.Errorf(errMissingOCIRegistryHostTmpl, registryURL) } rc, err := identityTokenRegistryClient(parsedURL.Host, creds, hc.pullClient.InsecureSkipTLSverify, hc.pullClient.PlainHTTP) @@ -280,10 +286,6 @@ func (hc *client) configureRegistryClient(chartUrl, chartRepo string, creds *Rep return nil } -func usesIdentityToken(chartUrl, chartRepo string, creds *RepoCreds) bool { - return creds.hasIdentityToken() && registry.IsOCI(ociURL(chartUrl, chartRepo)) -} - func resetPullState(pc *action.Pull) { pc.Username = "" pc.Password = "" @@ -291,26 +293,10 @@ func resetPullState(pc *action.Pull) { pc.RepoURL = "" } -func configurePullCredentials(pc *action.Pull, chartUrl, chartRepo string, creds *RepoCreds) { - pc.Username = "" - pc.Password = "" - if creds.hasBasicAuth() && !usesIdentityToken(chartUrl, chartRepo, creds) { - pc.Username = creds.Username - pc.Password = creds.Password - } -} - -func ociURL(chartUrl, chartRepo string) string { - if chartUrl != "" { - return chartUrl - } - return chartRepo -} - func identityTokenRegistryClient(host string, creds *RepoCreds, insecureSkipTLSVerify, plainHTTP bool) (*registry.Client, error) { httpClient, err := registryHTTPClient(insecureSkipTLSVerify) if err != nil { - return nil, errors.Wrap(err, errFailedToCreateRegistryClient) + return nil, errors.Wrap(err, errFailedToCreateRegistryHTTPClient) } authorizer := orasauth.Client{ @@ -358,8 +344,7 @@ func registryHTTPClientForTransport(transport *orasretry.Transport, insecureSkip return &http.Client{Transport: transport}, nil } -func (hc *client) login(chartUrl, chartRepo string, creds *RepoCreds, insecure bool) error { - registryURL := ociURL(chartUrl, chartRepo) +func (hc *client) login(registryURL string, creds *RepoCreds, insecure bool) error { if !registry.IsOCI(registryURL) { return nil } diff --git a/pkg/clients/helm/repocreds_test.go b/pkg/clients/helm/repocreds_test.go index 196d5a64..944c58e8 100644 --- a/pkg/clients/helm/repocreds_test.go +++ b/pkg/clients/helm/repocreds_test.go @@ -191,69 +191,7 @@ func TestResetPullState(t *testing.T) { } } -func TestConfigurePullCredentials(t *testing.T) { - cases := map[string]struct { - chartURL string - chartRepo string - creds *RepoCreds - wantUser string - wantPass string - }{ - "Nil": {}, - "UsernamePassword": { - creds: &RepoCreds{ - Username: "testuser", - Password: "testpass", - }, - wantUser: "testuser", - wantPass: "testpass", - }, - "IdentityTokenOCI": { - chartRepo: "oci://registry.example.com/charts", - creds: &RepoCreds{ - Username: "", - IdentityToken: "refresh-token", - }, - }, - "IdentityTokenWithBasicAuthOCI": { - chartURL: "oci://registry.example.com/charts/my-chart:1.0.0", - creds: &RepoCreds{ - Username: "testuser", - Password: "testpass", - IdentityToken: "refresh-token", - }, - }, - "IdentityTokenWithBasicAuthNonOCI": { - chartRepo: "https://charts.example.com", - creds: &RepoCreds{ - Username: "testuser", - Password: "testpass", - IdentityToken: "refresh-token", - }, - wantUser: "testuser", - wantPass: "testpass", - }, - } - - for name, tc := range cases { - t.Run(name, func(t *testing.T) { - pc := action.NewPull() - pc.Username = "stale-user" - pc.Password = "stale-pass" - - configurePullCredentials(pc, tc.chartURL, tc.chartRepo, tc.creds) - - if pc.Username != tc.wantUser { - t.Errorf("Username = %q, want %q", pc.Username, tc.wantUser) - } - if pc.Password != tc.wantPass { - t.Errorf("Password = %q, want %q", pc.Password, tc.wantPass) - } - }) - } -} - -func TestConfigureRegistryClientRequiresOCIHost(t *testing.T) { +func TestConfigureIdentityTokenRegistryClientRequiresOCIHost(t *testing.T) { defaultClient, err := registry.NewClient() if err != nil { t.Fatalf("registry.NewClient() error: %v", err) @@ -264,22 +202,22 @@ func TestConfigureRegistryClientRequiresOCIHost(t *testing.T) { registryClient: defaultClient, } - err = hc.configureRegistryClient("oci://", "", &RepoCreds{ + err = hc.configureIdentityTokenRegistryClient("oci://", &RepoCreds{ Username: "", IdentityToken: "refresh-token", }) if err == nil { - t.Fatal("configureRegistryClient() error: want error, got nil") + t.Fatal("configureIdentityTokenRegistryClient() error: want error, got nil") } if !strings.Contains(err.Error(), "missing OCI registry host") { - t.Fatalf("configureRegistryClient() error = %q, want missing host context", err) + t.Fatalf("configureIdentityTokenRegistryClient() error = %q, want missing host context", err) } if actionConfig.RegistryClient != defaultClient { - t.Fatal("configureRegistryClient() did not leave default registry client installed") + t.Fatal("configureIdentityTokenRegistryClient() did not leave default registry client installed") } } -func TestConfigureRegistryClientResetsDefault(t *testing.T) { +func TestConfigureIdentityTokenRegistryClientInstallsClient(t *testing.T) { defaultClient, err := registry.NewClient() if err != nil { t.Fatalf("registry.NewClient() error: %v", err) @@ -290,25 +228,14 @@ func TestConfigureRegistryClientResetsDefault(t *testing.T) { registryClient: defaultClient, } - if err := hc.configureRegistryClient("oci://registry.example.com/charts", "", &RepoCreds{ + if err := hc.configureIdentityTokenRegistryClient("oci://registry.example.com/charts", &RepoCreds{ Username: "", IdentityToken: "refresh-token", }); err != nil { - t.Fatalf("configureRegistryClient() identity token error: %v", err) - } - identityTokenClient := actionConfig.RegistryClient - if identityTokenClient == defaultClient { - t.Fatal("configureRegistryClient() did not install identity-token registry client") + t.Fatalf("configureIdentityTokenRegistryClient() error: %v", err) } - - if err := hc.configureRegistryClient("oci://registry.example.com/charts", "", &RepoCreds{ - Username: "testuser", - Password: "testpass", - }); err != nil { - t.Fatalf("configureRegistryClient() basic auth error: %v", err) - } - if actionConfig.RegistryClient != defaultClient { - t.Fatal("configureRegistryClient() did not reset to default registry client") + if actionConfig.RegistryClient == defaultClient { + t.Fatal("configureIdentityTokenRegistryClient() did not install identity-token registry client") } } From 000417d0f84c23429b46e0d465b1eb2ef79c9419 Mon Sep 17 00:00:00 2001 From: Matei Pinzaru <201221140+mateipinzaru-parloa@users.noreply.github.com> Date: Sat, 16 May 2026 02:22:18 +0300 Subject: [PATCH 4/4] fix: trim OCI identity-token pull cleanup Signed-off-by: Matei Pinzaru <201221140+mateipinzaru-parloa@users.noreply.github.com> --- pkg/clients/helm/client.go | 22 ++++++++-------- pkg/clients/helm/repocreds_test.go | 40 ++++++------------------------ 2 files changed, 18 insertions(+), 44 deletions(-) diff --git a/pkg/clients/helm/client.go b/pkg/clients/helm/client.go index a61e8a83..c54b4d1a 100644 --- a/pkg/clients/helm/client.go +++ b/pkg/clients/helm/client.go @@ -37,7 +37,6 @@ import ( "helm.sh/helm/v3/pkg/release" "k8s.io/client-go/rest" orasauth "oras.land/oras-go/v2/registry/remote/auth" - orasretry "oras.land/oras-go/v2/registry/remote/retry" ktype "sigs.k8s.io/kustomize/api/types" clusterv1beta1 "github.com/crossplane-contrib/provider-helm/apis/cluster/release/v1beta1" @@ -246,10 +245,9 @@ func (hc *client) pullChart(chartUrl, chartName, chartVersion, chartRepo string, pc.DestDir = chartDir - hc.pullClient.SetRegistryClient(hc.registryClient) - defer hc.pullClient.SetRegistryClient(hc.registryClient) + defer pc.SetRegistryClient(hc.registryClient) if useIdentityToken { - if err := hc.configureIdentityTokenRegistryClient(registryURL, creds); err != nil { + if err := configureIdentityTokenRegistryClient(pc, registryURL, creds); err != nil { return err } } @@ -269,7 +267,7 @@ func (hc *client) pullChart(chartUrl, chartName, chartVersion, chartRepo string, return nil } -func (hc *client) configureIdentityTokenRegistryClient(registryURL string, creds *RepoCreds) error { +func configureIdentityTokenRegistryClient(pc *action.Pull, registryURL string, creds *RepoCreds) error { parsedURL, err := url.Parse(registryURL) if err != nil { return errors.Wrap(err, errFailedToParseURL) @@ -278,11 +276,11 @@ func (hc *client) configureIdentityTokenRegistryClient(registryURL string, creds return errors.Errorf(errMissingOCIRegistryHostTmpl, registryURL) } - rc, err := identityTokenRegistryClient(parsedURL.Host, creds, hc.pullClient.InsecureSkipTLSverify, hc.pullClient.PlainHTTP) + rc, err := identityTokenRegistryClient(parsedURL.Host, creds, pc.InsecureSkipTLSverify, pc.PlainHTTP) if err != nil { return err } - hc.pullClient.SetRegistryClient(rc) + pc.SetRegistryClient(rc) return nil } @@ -313,16 +311,16 @@ func identityTokenRegistryClient(host string, creds *RepoCreds, insecureSkipTLSV } rc, err := registry.NewClient(opts...) - return rc, errors.Wrap(err, errFailedToCreateRegistryClient) + if err != nil { + return nil, errors.Wrap(err, errFailedToCreateRegistryClient) + } + return rc, nil } func registryHTTPClient(insecureSkipTLSVerify bool) (*http.Client, error) { // Keep Helm's normal retry transport. The flag disables Helm registry // debug logging; it is unrelated to TLS verification. - return registryHTTPClientForTransport(registry.NewTransport(false), insecureSkipTLSVerify) -} - -func registryHTTPClientForTransport(transport *orasretry.Transport, insecureSkipTLSVerify bool) (*http.Client, error) { + transport := registry.NewTransport(false) if !insecureSkipTLSVerify { return &http.Client{Transport: transport}, nil } diff --git a/pkg/clients/helm/repocreds_test.go b/pkg/clients/helm/repocreds_test.go index 944c58e8..189261fb 100644 --- a/pkg/clients/helm/repocreds_test.go +++ b/pkg/clients/helm/repocreds_test.go @@ -18,6 +18,7 @@ package helm import ( "context" + "encoding/json" "fmt" "net/http" "net/http/httptest" @@ -34,15 +35,8 @@ import ( "helm.sh/helm/v3/pkg/cli" "helm.sh/helm/v3/pkg/registry" orasauth "oras.land/oras-go/v2/registry/remote/auth" - orasretry "oras.land/oras-go/v2/registry/remote/retry" ) -type roundTripperFunc func(*http.Request) (*http.Response, error) - -func (f roundTripperFunc) RoundTrip(req *http.Request) (*http.Response, error) { - return f(req) -} - func TestRepoCredsRegistryCredential(t *testing.T) { type want struct { basicAuth bool @@ -154,20 +148,6 @@ func TestRegistryHTTPClientInsecureSkipTLSVerify(t *testing.T) { } } -func TestRegistryHTTPClientInsecureSkipTLSVerifyRequiresHTTPTransport(t *testing.T) { - transport := orasretry.NewTransport(roundTripperFunc(func(_ *http.Request) (*http.Response, error) { - return nil, nil - })) - - _, err := registryHTTPClientForTransport(transport, true) - if err == nil { - t.Fatal("registryHTTPClientForTransport() error: want error, got nil") - } - if !strings.Contains(err.Error(), "expected Helm registry transport base") { - t.Fatalf("registryHTTPClientForTransport() error = %q, want transport type context", err) - } -} - func TestResetPullState(t *testing.T) { pc := action.NewPull() pc.Username = "stale-user" @@ -197,12 +177,9 @@ func TestConfigureIdentityTokenRegistryClientRequiresOCIHost(t *testing.T) { t.Fatalf("registry.NewClient() error: %v", err) } actionConfig := &action.Configuration{RegistryClient: defaultClient} - hc := &client{ - pullClient: action.NewPullWithOpts(action.WithConfig(actionConfig)), - registryClient: defaultClient, - } + pc := action.NewPullWithOpts(action.WithConfig(actionConfig)) - err = hc.configureIdentityTokenRegistryClient("oci://", &RepoCreds{ + err = configureIdentityTokenRegistryClient(pc, "oci://", &RepoCreds{ Username: "", IdentityToken: "refresh-token", }) @@ -223,12 +200,9 @@ func TestConfigureIdentityTokenRegistryClientInstallsClient(t *testing.T) { t.Fatalf("registry.NewClient() error: %v", err) } actionConfig := &action.Configuration{RegistryClient: defaultClient} - hc := &client{ - pullClient: action.NewPullWithOpts(action.WithConfig(actionConfig)), - registryClient: defaultClient, - } + pc := action.NewPullWithOpts(action.WithConfig(actionConfig)) - if err := hc.configureIdentityTokenRegistryClient("oci://registry.example.com/charts", &RepoCreds{ + if err := configureIdentityTokenRegistryClient(pc, "oci://registry.example.com/charts", &RepoCreds{ Username: "", IdentityToken: "refresh-token", }); err != nil { @@ -354,7 +328,9 @@ func TestIdentityTokenRegistryClientUsesRefreshToken(t *testing.T) { t.Errorf("scope = %q, want %q", got, scope) } w.Header().Set("Content-Type", "application/json") - fmt.Fprintf(w, `{"access_token":%q}`, accessToken) + if err := json.NewEncoder(w).Encode(map[string]string{"access_token": accessToken}); err != nil { + t.Errorf("Encode() error: %v", err) + } default: t.Errorf("unexpected request path: %s", r.URL.Path) http.NotFound(w, r)