From afab6bbc686912af27956134808248a308fdae20 Mon Sep 17 00:00:00 2001 From: Terry Howe Date: Sun, 12 Jul 2026 07:36:18 -0600 Subject: [PATCH 1/5] feat(registry): oras-go v3 full config stack integration Migrate helm.sh/helm/v4/pkg/registry from oras.land/oras-go/v2 to github.com/oras-project/oras-go/v3 (v3.0.2-dev) and wire in the full container ecosystem config stack: - Mechanical v2 -> v3 API migration (credentials, auth client, login/logout) - config.LoadConfigs(): Docker config.json, containers auth.json, registries.conf, certs.d, policy.json, registries.d - newRepository() uses RegistryProperties + NewRepositoryWithProperties for mirror resolution, per-registry TLS, and CLI flag overrides - policy.json enforcement via builder.PolicyEvaluator - registries.d + signedBy signature verification - LoadConfigsWithOptions override paths (ConfigOptions) - Login path Location rewrite from registries.conf - Namespaced authentication support for registry login - Set [pull,push] scope when pushing to token-auth registries Closes: https://github.com/helm/helm/pull/11771 Signed-off-by: Terry Howe --- go.mod | 2 +- go.sum | 4 +- pkg/registry/client.go | 400 +++++++++++++++++++++++++----- pkg/registry/client_http_test.go | 2 +- pkg/registry/client_scope_test.go | 76 +++--- pkg/registry/client_test.go | 287 +++++++++++++++++---- pkg/registry/generic.go | 41 +-- pkg/registry/reference.go | 8 +- pkg/registry/reference_test.go | 2 +- pkg/registry/registry_test.go | 6 +- pkg/registry/transport.go | 2 +- 11 files changed, 628 insertions(+), 202 deletions(-) diff --git a/go.mod b/go.mod index 9da20b09a..3ec31a475 100644 --- a/go.mod +++ b/go.mod @@ -28,6 +28,7 @@ require ( github.com/moby/term v0.5.2 github.com/opencontainers/go-digest v1.0.0 github.com/opencontainers/image-spec v1.1.1 + github.com/oras-project/oras-go/v3 v3.0.2-dev github.com/rubenv/sql-migrate v1.8.1 github.com/santhosh-tekuri/jsonschema/v6 v6.0.3 github.com/spf13/cobra v1.10.2 @@ -47,7 +48,6 @@ require ( k8s.io/client-go v0.36.4 k8s.io/klog/v2 v2.140.0 k8s.io/kubectl v0.36.4 - oras.land/oras-go/v2 v2.6.2 sigs.k8s.io/controller-runtime v0.24.1 sigs.k8s.io/kustomize/kyaml v0.21.1 sigs.k8s.io/yaml v1.6.0 diff --git a/go.sum b/go.sum index c4497aa58..c037fb865 100644 --- a/go.sum +++ b/go.sum @@ -241,6 +241,8 @@ github.com/opencontainers/go-digest v1.0.0 h1:apOUWs51W5PlhuyGyz9FCeeBIOUDA/6nW8 github.com/opencontainers/go-digest v1.0.0/go.mod h1:0JzlMkj0TRzQZfJkVvzbP0HBR3IKzErnv2BNG4W4MAM= github.com/opencontainers/image-spec v1.1.1 h1:y0fUlFfIZhPF1W537XOLg0/fcx6zcHCJwooC2xJA040= github.com/opencontainers/image-spec v1.1.1/go.mod h1:qpqAh3Dmcf36wStyyWU+kCeDgrGnAve2nCC8+7h8Q0M= +github.com/oras-project/oras-go/v3 v3.0.2-dev h1:FoSopdghExPGXRV+CvMjpXNPO74oZhquowB15OZL3l4= +github.com/oras-project/oras-go/v3 v3.0.2-dev/go.mod h1:bSbdCn421//d2i4+mATPCvbNcmpM+fI1G8no/mZBcD4= github.com/peterbourgon/diskv v2.0.1+incompatible h1:UBdAOUP5p4RWqPBg048CAvpKN+vxiaj6gdUUzhl4XmI= github.com/peterbourgon/diskv v2.0.1+incompatible/go.mod h1:uqqh8zWWbv1HBMNONnaR/tNboyR3/BZd58JJSHlUSCU= github.com/pkg/errors v0.8.0/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= @@ -512,8 +514,6 @@ k8s.io/kubectl v0.36.4 h1:xZd9g1bFBd7hpb1oKjK8lT9jRL18dtgr4DAPQG1Oksk= k8s.io/kubectl v0.36.4/go.mod h1:STWlr78cdEa1hHpr55wpcboaqchvfDueKRNDa1zOd1w= k8s.io/utils v0.0.0-20260507154919-ff6756f316d2 h1:wU4tMEhLGgIbLvXQb1cfN+EcM0wf7zC6CPF+C79jroc= k8s.io/utils v0.0.0-20260507154919-ff6756f316d2/go.mod h1:xDxuJ0whA3d0I4mf/C4ppKHxXynQ+fxnkmQH0vTHnuk= -oras.land/oras-go/v2 v2.6.2 h1:N04RXngAp1LJKTG6ifz3xHPipasEkWr+hFmInja5YKo= -oras.land/oras-go/v2 v2.6.2/go.mod h1:PlTtg4JTDJkDe8yVHpM2wz7/YDc00GVas+i4jAW2TZ4= sigs.k8s.io/controller-runtime v0.24.1 h1:miPEwrmirImAvgME1L9qebGHrOnGJoVmVdtOU9fRfo4= sigs.k8s.io/controller-runtime v0.24.1/go.mod h1:vFkfY5fGt5xAC/sKb8IBFKgWPNKG9OUG29dR8Y2wImw= sigs.k8s.io/json v0.0.0-20250730193827-2d320260d730 h1:IpInykpT6ceI+QxKBbEflcR5EXP7sU1kvOlxwZh5txg= diff --git a/pkg/registry/client.go b/pkg/registry/client.go index 786342c07..66a778e47 100644 --- a/pkg/registry/client.go +++ b/pkg/registry/client.go @@ -34,13 +34,16 @@ import ( "github.com/Masterminds/semver/v3" "github.com/opencontainers/image-spec/specs-go" ocispec "github.com/opencontainers/image-spec/specs-go/v1" - "oras.land/oras-go/v2" - "oras.land/oras-go/v2/content/memory" - "oras.land/oras-go/v2/registry" - "oras.land/oras-go/v2/registry/remote" - "oras.land/oras-go/v2/registry/remote/auth" - "oras.land/oras-go/v2/registry/remote/credentials" - "oras.land/oras-go/v2/registry/remote/retry" + "github.com/oras-project/oras-go/v3" + "github.com/oras-project/oras-go/v3/content/memory" + "github.com/oras-project/oras-go/v3/registry/remote" + "github.com/oras-project/oras-go/v3/registry/remote/auth" + remoteconfig "github.com/oras-project/oras-go/v3/registry/remote/config" + "github.com/oras-project/oras-go/v3/registry/remote/credentials" + "github.com/oras-project/oras-go/v3/registry/remote/policy" + "github.com/oras-project/oras-go/v3/registry/remote/properties" + "github.com/oras-project/oras-go/v3/registry/remote/retry" + "github.com/oras-project/oras-go/v3/registry/remote/signature" "helm.sh/helm/v4/internal/version" chart "helm.sh/helm/v4/pkg/chart/v2" @@ -54,6 +57,14 @@ storing semantic versions, Helm adopts the convention of changing plus (+) to an underscore (_) in chart version tags when pushing to a registry and back to a plus (+) when pulling from a registry.` +// ConfigOptions specifies override paths for container ecosystem config files. +type ConfigOptions struct { + RegistriesConfigPath string + PolicyConfigPath string + CertsDirPaths []string + ContainersAuthPath string +} + type ( // RemoteClient shadows the ORAS remote.Client interface // (hiding the ORAS type from Helm client visibility) @@ -76,6 +87,17 @@ type ( credentialsStore credentials.Store httpClient *http.Client plainHTTP bool + // v3 config-driven fields + configs *remoteconfig.Configs + builder *remote.ClientBuilder + policyEvaluator *policy.Evaluator + signatureVerification bool + configOptions ConfigOptions + insecure bool + certFile string + keyFile string + caFile string + customHTTPClient bool // true when ClientOptHTTPClient or ClientOptAuthorizer was used } // ClientOption allows specifying various settings configurable by the user for overriding the defaults @@ -102,22 +124,67 @@ func NewClient(options ...ClientOption) (*Client, error) { } storeOptions := credentials.StoreOptions{ - AllowPlaintextPut: true, - DetectDefaultNativeStore: true, + AllowPlaintextPut: true, } - store, err := credentials.NewStore(client.credentialsFile, storeOptions) + + // Primary credentials store (Helm's own file) — used for Login/Logout persistence. + helmStore, err := credentials.NewStore(client.credentialsFile, storeOptions) if err != nil { return nil, err } - dockerStore, err := credentials.NewStoreFromDocker(storeOptions) + client.credentialsStore = helmStore + + // Load the full container ecosystem config stack: Docker config.json, + // containers auth.json, registries.conf, policy.json, certs.d, registries.d. + // Missing files are silently skipped. + loaderOpts := remoteconfig.LoadConfigsOptions{ + RegistriesConfigPath: client.configOptions.RegistriesConfigPath, + PolicyConfigPath: client.configOptions.PolicyConfigPath, + CertsDirPaths: client.configOptions.CertsDirPaths, + ContainersAuthPath: client.configOptions.ContainersAuthPath, + } + configs, err := remoteconfig.LoadConfigsWithOptions(loaderOpts) if err != nil { - // should only fail if user home directory can't be determined - client.credentialsStore = store + return nil, fmt.Errorf("failed to load registry configurations: %w", err) + } + client.configs = configs + + // Only build from configs when not overridden by ClientOptPolicyEvaluator. + if configs.PolicyConfig != nil && client.policyEvaluator == nil { + evaluator, err := configs.PolicyEvaluator() + if err != nil { + return nil, fmt.Errorf("failed to build policy evaluator: %w", err) + } + client.policyEvaluator = evaluator + } + + // Build the combined credential store for read operations: + // 1. Helm's own credentials file (highest priority, stores login results) + // 2. Docker config.json + containers auth.json from the full config stack + var credStore credentials.Store + if configStore, err := configs.CredentialStore(storeOptions); err == nil { + credStore = credentials.NewStoreWithFallbacks(helmStore, configStore) } else { - // use Helm credentials with fallback to Docker - client.credentialsStore = credentials.NewStoreWithFallbacks(store, dockerStore) + credStore = helmStore + } + + // Build the ClientBuilder used by the config-driven repository creation path. + var logger *slog.Logger + if client.debug { + logger = slog.Default() } + builder := remote.NewClientBuilder() + builder.CredentialStore = credStore + builder.UserAgent = version.GetUserAgent() + builder.Logger = logger + builder.PolicyEvaluator = client.policyEvaluator + if !client.enableCache { + builder.CacheFactory = nil + } + client.builder = builder + // Build the legacy auth.Client used when a custom HTTP client or authorizer + // was provided (customHTTPClient=true path). if client.authorizer == nil { authorizer := auth.Client{ Client: client.httpClient, @@ -125,11 +192,11 @@ func NewClient(options ...ClientOption) (*Client, error) { authorizer.SetUserAgent(version.GetUserAgent()) if client.username != "" && client.password != "" { - authorizer.Credential = func(_ context.Context, _ string) (auth.Credential, error) { - return auth.Credential{Username: client.username, Password: client.password}, nil + authorizer.CredentialFunc = func(_ context.Context, _ string) (credentials.Credential, error) { + return credentials.Credential{Username: client.username, Password: client.password}, nil } } else { - authorizer.Credential = credentials.Credential(client.credentialsStore) + authorizer.CredentialFunc = remote.NewCredentialFunc(credStore) } if client.enableCache { @@ -141,6 +208,118 @@ func NewClient(options ...ClientOption) (*Client, error) { return client, nil } +// applyOverrides applies client-level CLI flag overrides to registry properties. +func (c *Client) applyOverrides(props *properties.Registry) { + if c.plainHTTP { + props.Transport.PlainHTTP = true + } + if c.insecure { + props.Transport.Insecure = true + } + if c.certFile != "" && c.keyFile != "" { + props.Transport.Cert = c.certFile + props.Transport.Key = c.keyFile + } + if c.caFile != "" { + props.Transport.CACerts = append(props.Transport.CACerts, c.caFile) + } + if c.username != "" && c.password != "" { + props.Credential = credentials.Credential{Username: c.username, Password: c.password} + } +} + +// newRepository creates a configured remote.Repository for the given reference. +// +// When a custom HTTP client or authorizer was provided, the legacy path is used +// (direct assignment of c.authorizer). Otherwise, the full config-driven path is +// used: registry properties are resolved from registries.conf and certs.d, CLI +// overrides are applied, and the repository is built via NewRepositoryWithProperties. +func (c *Client) newRepository(ref string) (*remote.Repository, error) { + if c.customHTTPClient { + // Legacy path: use c.authorizer directly (preserves custom TLS transport). + repo, err := remote.NewRepository(ref) + if err != nil { + return nil, err + } + repo.Registry.PlainHTTP = c.plainHTTP + if c.registryAuthorizer != nil { + repo.Registry.Client = c.registryAuthorizer + } else { + repo.Registry.Client = c.authorizer + } + repo.Registry.Policy = c.policyEvaluator + return repo, nil + } + // Config-driven path: resolve properties from registries.conf, certs.d, etc. + props, err := c.configs.RegistryProperties(ref) + if err != nil { + return nil, err + } + c.applyOverrides(props) + builder := c.builder + if props.Reference.Repository != "" && builder.CredentialStore != nil { + builderCopy := *builder + builderCopy.CredentialStore = &namespacedStore{ + inner: builder.CredentialStore, + repository: props.Reference.Repository, + } + builder = &builderCopy + } + repo, err := remote.NewRepositoryWithProperties(props, builder) + if err != nil { + return nil, err + } + if c.signatureVerification && c.configs.RegistriesDConfig != nil && c.policyEvaluator != nil { + scope := props.Reference.Registry + if props.Reference.Repository != "" { + scope += "/" + props.Reference.Repository + } + verifier := signature.NewSignedByVerifierFromConfig(c.configs.RegistriesDConfig, scope) + if verifier != nil { + scopedEval, err := c.configs.PolicyEvaluator(policy.WithSignedByVerifier(verifier)) + if err == nil && scopedEval != nil && repo.Registry.Policy == nil { + repo.Registry.Policy = scopedEval + } + } + } + return repo, nil +} + +// newRegistry creates a configured remote.Registry for the given host (used by Login). +func (c *Client) newRegistry(host string) (*remote.Registry, error) { + if c.customHTTPClient { + // Legacy path: use c.authorizer directly. + reg, err := remote.NewRegistry(host) + if err != nil { + return nil, err + } + reg.PlainHTTP = c.plainHTTP + if c.registryAuthorizer != nil { + reg.Client = c.registryAuthorizer + } else { + reg.Client = c.authorizer + } + return reg, nil + } + // Config-driven path: construct properties for a host-only reference + // (no repository path) and apply CLI overrides + certs.d. + props := properties.NewRegistryFromReference(properties.Reference{Registry: host}) + // Apply insecure setting from registries.conf if the host is found there. + if c.configs.RegistriesConfig != nil { + if reg := c.configs.RegistriesConfig.FindRegistry(host); reg != nil && reg.Insecure { + props.Transport.Insecure = true + } + } + // Apply per-host TLS certificates from certs.d. + if len(c.configs.CertsDirPaths) > 0 { + if certs, err := remoteconfig.LoadCertsDirFromPaths(host, c.configs.CertsDirPaths); err == nil && certs != nil { + certs.ApplyToTransport(&props.Transport) + } + } + c.applyOverrides(props) + return remote.NewRegistryWithProperties(props, c.builder) +} + // Generic returns a GenericClient for low-level OCI operations func (c *Client) Generic() *GenericClient { return NewGenericClient(c) @@ -182,6 +361,7 @@ func ClientOptWriter(out io.Writer) ClientOption { func ClientOptAuthorizer(authorizer auth.Client) ClientOption { return func(client *Client) { client.authorizer = &authorizer + client.customHTTPClient = true } } @@ -203,9 +383,12 @@ func ClientOptCredentialsFile(credentialsFile string) ClientOption { } // ClientOptHTTPClient returns a function that sets the HTTP client for the registry client. +// When a custom HTTP client is provided, the legacy repository creation path is used so +// that TLS configuration in the custom transport is preserved. func ClientOptHTTPClient(httpClient *http.Client) ClientOption { return func(client *Client) { client.httpClient = httpClient + client.customHTTPClient = true } } @@ -217,6 +400,28 @@ func ClientOptPlainHTTP() ClientOption { } } +// ClientOptPolicyEvaluator returns a function that sets a custom policy evaluator on the client. +func ClientOptPolicyEvaluator(e *policy.Evaluator) ClientOption { + return func(c *Client) { + c.policyEvaluator = e + } +} + +// ClientOptSignatureVerification returns a function that enables or disables +// GPG/simple-signing signature verification via registries.d lookaside storage. +func ClientOptSignatureVerification(enabled bool) ClientOption { + return func(c *Client) { + c.signatureVerification = enabled + } +} + +// ClientOptConfigOptions returns a function that overrides default config file paths. +func ClientOptConfigOptions(o ConfigOptions) ClientOption { + return func(c *Client) { + c.configOptions = o + } +} + type ( // LoginOption allows specifying various settings on login LoginOption func(*loginOperation) @@ -224,6 +429,7 @@ type ( loginOperation struct { host string client *Client + err error } ) @@ -238,37 +444,93 @@ func warnIfHostHasPath(host string) bool { return false } +// noopStore is a credentials.Store that performs no persistence. It is used +// during namespaced login to verify credentials with a registry ping without +// causing remote.Login to store credentials under the hostname-only key. +type noopStore struct{} + +func (noopStore) Get(_ context.Context, _ string) (credentials.Credential, error) { + return credentials.EmptyCredential, nil +} +func (noopStore) Put(_ context.Context, _ string, _ credentials.Credential) error { return nil } +func (noopStore) Delete(_ context.Context, _ string) error { return nil } + +// namespacedStore wraps a credentials.Store and performs hierarchical +// credential lookup: it tries "hostname/full/repo", then "hostname/partial", +// then falls back to "hostname" for each auth challenge. +type namespacedStore struct { + inner credentials.Store + repository string +} + +func (ns *namespacedStore) Get(ctx context.Context, serverAddress string) (credentials.Credential, error) { + parts := strings.Split(ns.repository, "/") + for i := len(parts); i > 0; i-- { + key := serverAddress + "/" + strings.Join(parts[:i], "/") + if cred, err := ns.inner.Get(ctx, key); err == nil && cred != credentials.EmptyCredential { + return cred, nil + } + } + return ns.inner.Get(ctx, serverAddress) +} + +func (ns *namespacedStore) Put(ctx context.Context, serverAddress string, cred credentials.Credential) error { + return ns.inner.Put(ctx, serverAddress, cred) +} + +func (ns *namespacedStore) Delete(ctx context.Context, serverAddress string) error { + return ns.inner.Delete(ctx, serverAddress) +} + // Login authenticates the client with a remote OCI registry using the provided host and options. func (c *Client) Login(host string, options ...LoginOption) error { + op := &loginOperation{host: host, client: c} for _, option := range options { - option(&loginOperation{host, c}) + option(op) } + if op.err != nil { + return op.err + } + + // Separate the registry hostname from any namespace path. + // e.g., "localhost:8000/myrepo" → registryHost="localhost:8000", namespacePath="myrepo" + registryHost, namespacePath, hasNamespace := strings.Cut(host, "/") - warnIfHostHasPath(host) + // Determine canonical host from registries.conf Location rewrite. + // We use the original (alias) host for newRegistry so its transport + // settings (Insecure, certs.d) are preserved, then redirect the + // authentication endpoint and credential key to the canonical host. + canonicalHost := registryHost + if c.configs != nil && c.configs.RegistriesConfig != nil { + if regCfg := c.configs.RegistriesConfig.FindRegistry(registryHost); regCfg != nil && regCfg.Location != "" { + canonicalHost = regCfg.Location + } + } - reg, err := remote.NewRegistry(host) + reg, err := c.newRegistry(registryHost) if err != nil { return err } - reg.PlainHTTP = c.plainHTTP - cred := auth.Credential{Username: c.username, Password: c.password} - c.authorizer.ForceAttemptOAuth2 = true - reg.Client = c.authorizer + reg.Reference.Registry = canonicalHost + cred := credentials.Credential{Username: c.username, Password: c.password} ctx := context.Background() - if err := reg.Ping(ctx); err != nil { - c.authorizer.ForceAttemptOAuth2 = false - if err := reg.Ping(ctx); err != nil { - return fmt.Errorf("authenticating to %q: %w", host, err) - } - } - // Always restore to false after probing, to avoid forcing POST to token endpoints like GHCR. - c.authorizer.ForceAttemptOAuth2 = false - key := credentials.ServerAddressFromRegistry(host) - key = credentials.ServerAddressFromHostname(key) - if err := c.credentialsStore.Put(ctx, key, cred); err != nil { - return err + if !hasNamespace { + // Standard hostname-only login: verify and store under the hostname. + if err := remote.Login(ctx, c.credentialsStore, reg, cred); err != nil { + return fmt.Errorf("authenticating to %q: %w", canonicalHost, err) + } + } else { + // Namespaced login: verify the credential against the hostname, + // then store under the namespaced key only. + if err := remote.Login(ctx, noopStore{}, reg, cred); err != nil { + return fmt.Errorf("authenticating to %q: %w", canonicalHost, err) + } + namespacedKey := canonicalHost + "/" + namespacePath + if err := c.credentialsStore.Put(ctx, namespacedKey, cred); err != nil { + return fmt.Errorf("storing credentials for %q: %w", namespacedKey, err) + } } _, _ = fmt.Fprintln(c.out, "Login Succeeded") @@ -280,7 +542,6 @@ func LoginOptBasicAuth(username, password string) LoginOption { return func(o *loginOperation) { o.client.username = username o.client.password = password - o.client.authorizer.Credential = auth.StaticCredential(o.host, auth.Credential{Username: username, Password: password}) } } @@ -333,11 +594,13 @@ func ensureTLSConfig(client *auth.Client, setConfig *tls.Config) (*tls.Config, e // LoginOptInsecure returns a function that sets the insecure setting on login func LoginOptInsecure(insecure bool) LoginOption { return func(o *loginOperation) { + o.client.insecure = insecure + // Also update the authorizer transport for the legacy path (customHTTPClient=true). tlsConfig, err := ensureTLSConfig(o.client.authorizer, nil) if err != nil { - panic(err) + o.err = err + return } - tlsConfig.InsecureSkipVerify = insecure } } @@ -348,11 +611,19 @@ func LoginOptTLSClientConfig(certFile, keyFile, caFile string) LoginOption { if (certFile == "" || keyFile == "") && caFile == "" { return } + // Set file path fields for the config-driven path. + if certFile != "" && keyFile != "" { + o.client.certFile = certFile + o.client.keyFile = keyFile + } + if caFile != "" { + o.client.caFile = caFile + } + // Also update the authorizer transport for the legacy path (customHTTPClient=true). tlsConfig, err := ensureTLSConfig(o.client.authorizer, nil) if err != nil { panic(err) } - if certFile != "" && keyFile != "" { authCert, err := tls.LoadX509KeyPair(certFile, keyFile) if err != nil { @@ -360,7 +631,6 @@ func LoginOptTLSClientConfig(certFile, keyFile, caFile string) LoginOption { } tlsConfig.Certificates = []tls.Certificate{authCert} } - if caFile != "" { certPool := x509.NewCertPool() ca, err := os.ReadFile(caFile) @@ -400,7 +670,21 @@ func (c *Client) Logout(host string, opts ...LogoutOption) error { opt(operation) } - if err := credentials.Logout(context.Background(), c.credentialsStore, host); err != nil { + // Extract registry hostname for Location rewrite lookup. + registryHost, namespacePath, hasNamespace := strings.Cut(host, "/") + canonicalHost := registryHost + if c.configs != nil && c.configs.RegistriesConfig != nil { + if regCfg := c.configs.RegistriesConfig.FindRegistry(registryHost); regCfg != nil && regCfg.Location != "" { + canonicalHost = regCfg.Location + } + } + if hasNamespace { + host = canonicalHost + "/" + namespacePath + } else { + host = canonicalHost + } + + if err := remote.Logout(context.Background(), c.credentialsStore, host); err != nil { return err } _, _ = fmt.Fprintf(c.out, "Removing login credentials for %s\n", host) @@ -712,12 +996,10 @@ func (c *Client) Push(data []byte, ref string, options ...PushOption) (*PushResu return nil, err } - repository, err := remote.NewRepository(parsedRef.String()) + repository, err := c.newRepository(parsedRef.String()) if err != nil { return nil, err } - repository.PlainHTTP = c.plainHTTP - repository.Client = c.authorizer ctx = withScopeHint(ctx, repository, auth.ActionPull, auth.ActionPush) @@ -752,7 +1034,7 @@ func (c *Client) Push(data []byte, ref string, options ...PushOption) (*PushResu } _, _ = fmt.Fprintf(c.out, "Pushed: %s\n", result.Ref) _, _ = fmt.Fprintf(c.out, "Digest: %s\n", result.Manifest.Digest) - if strings.Contains(parsedRef.orasReference.Reference, "_") { + if strings.Contains(parsedRef.Tag, "_") { _, _ = fmt.Fprintf(c.out, "%s contains an underscore.\n", result.Ref) _, _ = fmt.Fprint(c.out, registryUnderscoreMessage+"\n") } @@ -783,18 +1065,16 @@ func PushOptCreationTime(creationTime string) PushOption { // Tags provides a sorted list all semver compliant tags for a given repository func (c *Client) Tags(ref string) ([]string, error) { - parsedReference, err := registry.ParseReference(ref) + parsedReference, err := properties.NewReference(ref) if err != nil { return nil, err } ctx := context.Background() - repository, err := remote.NewRepository(parsedReference.String()) + repository, err := c.newRepository(parsedReference.String()) if err != nil { return nil, err } - repository.PlainHTTP = c.plainHTTP - repository.Client = c.authorizer var tagVersions []*semver.Version err = repository.Tags(ctx, "", func(tags []string) error { @@ -827,12 +1107,10 @@ func (c *Client) Tags(ref string) ([]string, error) { // Resolve a reference to a descriptor. func (c *Client) Resolve(ref string) (desc ocispec.Descriptor, err error) { - remoteRepository, err := remote.NewRepository(ref) + remoteRepository, err := c.newRepository(ref) if err != nil { return desc, err } - remoteRepository.PlainHTTP = c.plainHTTP - remoteRepository.Client = c.authorizer parsedReference, err := newReference(ref) if err != nil { @@ -931,13 +1209,11 @@ func (c *Client) tagManifest(ctx context.Context, memoryStore *memory.Store, manifestData, parsedRef.String()) } -// add actions when request a registry authentication token(jwt) -// example1. when we want to pull 'testrepo/local-subchart' we can send below url, and 'pull' is the action -// auth?scope=repository%3Atestrepo%2Flocal-subchart%3Apull&service=testservice -// example2. when we want to push 'testrepo/local-subchart' we can send below url, and 'pull%2Cpush' are the actions -// auth?scope=repository%3Atestrepo%2Flocal-subchart%3Apull%2Cpush&service=testservice -// we can set the actions like below -// example) ctx = withScopeHint(ctx, repository, auth.ActionPush, auth.ActionPull) +// withScopeHint hints the auth client to request a token covering all the given +// actions in a single request. Without this, pushing to a token-auth registry +// first requests a [pull] scope (which fails for a not-yet-existing repository +// path), making it hard to mint a valid token. Hinting [pull,push] up front +// produces a single correct token request. func withScopeHint(ctx context.Context, repo *remote.Repository, actions ...string) context.Context { - return auth.AppendRepositoryScope(ctx, repo.Reference, actions...) + return auth.AppendRepositoryScope(ctx, repo.Reference(), actions...) } diff --git a/pkg/registry/client_http_test.go b/pkg/registry/client_http_test.go index 0a51b3347..e23f55f30 100644 --- a/pkg/registry/client_http_test.go +++ b/pkg/registry/client_http_test.go @@ -20,8 +20,8 @@ import ( "os" "testing" + "github.com/oras-project/oras-go/v3/content" "github.com/stretchr/testify/suite" - "oras.land/oras-go/v2/content" ) type HTTPRegistryClientTestSuite struct { diff --git a/pkg/registry/client_scope_test.go b/pkg/registry/client_scope_test.go index 733ce980d..a308512b5 100644 --- a/pkg/registry/client_scope_test.go +++ b/pkg/registry/client_scope_test.go @@ -21,7 +21,6 @@ import ( "net" "net/http" "net/http/httptest" - "net/url" "os" "testing" "time" @@ -33,6 +32,28 @@ type RegistryScopeTestSuite struct { TestRegistry } +// authRequest captures the fields of a token request that the test auth server +// receives. The oras-go v3 auth client may issue the request either as a GET +// (query params) or as an OAuth2 POST (form body); reading the parsed form +// covers both. +type authRequest struct { + path string + service string + scope string +} + +func captureAuthRequest(requests chan<- authRequest) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + _ = r.ParseForm() + requests <- authRequest{ + path: r.URL.Path, + service: r.Form.Get("service"), + scope: r.Form.Get("scope"), + } + w.WriteHeader(http.StatusOK) + } +} + func (suite *RegistryScopeTestSuite) SetupSuite() { // Set up a plain-HTTP registry that uses token auth. The token realm is // served over http (see setup), so the registry must be contacted over @@ -47,22 +68,12 @@ func (suite *RegistryScopeTestSuite) TearDownSuite() { } func (suite *RegistryScopeTestSuite) Test_1_Check_Push_Request_Scope() { - requestURL := make(chan string, 1) - handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - // Capture only the first auth request; never block the handler if the - // client happens to retry, so the auth server always responds and the - // push/pull flow can't deadlock waiting on us. - select { - case requestURL <- r.URL.String(): - default: - } - w.WriteHeader(http.StatusOK) - }) + requests := make(chan authRequest, 1) lnCfg := net.ListenConfig{} listener, err := lnCfg.Listen(suite.T().Context(), "tcp", suite.AuthServerHost) suite.Require().NoError(err, "no error creating server listener") - ts := httptest.NewUnstartedServer(handler) + ts := httptest.NewUnstartedServer(captureAuthRequest(requests)) ts.Listener = listener ts.Start() defer ts.Close() @@ -79,41 +90,26 @@ func (suite *RegistryScopeTestSuite) Test_1_Check_Push_Request_Scope() { // check the url that authentication server received select { - case urlStr := <-requestURL: - u, err := url.Parse(urlStr) - suite.Require().NoError(err, "no error parsing requested URL") - - suite.Equal("/auth", u.Path) - suite.Equal("testservice", u.Query().Get("service")) - scope := u.Query().Get("scope") - suite.Contains(scope, "repository:testrepo/local-subchart:pull,push") + case req := <-requests: + suite.Equal("/auth", req.path) + suite.Equal("testservice", req.service) + suite.Contains(req.scope, "repository:testrepo/local-subchart:pull,push") case <-time.After(5 * time.Second): suite.T().Fatal("timeout waiting for auth request") } } func (suite *RegistryScopeTestSuite) Test_2_Check_Pull_Request_Scope() { - requestURL := make(chan string, 1) - handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - // Capture only the first auth request; never block the handler if the - // client happens to retry, so the auth server always responds and the - // push/pull flow can't deadlock waiting on us. - select { - case requestURL <- r.URL.String(): - default: - } - w.WriteHeader(http.StatusOK) - }) + requests := make(chan authRequest, 1) lnCfg := net.ListenConfig{} listener, err := lnCfg.Listen(suite.T().Context(), "tcp", suite.AuthServerHost) suite.Require().NoError(err, "no error creating server listener") - ts := httptest.NewUnstartedServer(handler) + ts := httptest.NewUnstartedServer(captureAuthRequest(requests)) ts.Listener = listener ts.Start() defer ts.Close() - // Load test chart (to build ref pushed in previous test) // Simple pull, chart only chartData, err := os.ReadFile("../downloader/testdata/local-subchart-0.1.0.tgz") suite.Require().NoError(err, "no error loading test chart") @@ -125,14 +121,10 @@ func (suite *RegistryScopeTestSuite) Test_2_Check_Pull_Request_Scope() { // check the url that authentication server received select { - case urlStr := <-requestURL: - u, err := url.Parse(urlStr) - suite.Require().NoError(err, "no error parsing requested URL") - - suite.Equal("/auth", u.Path) - suite.Equal("testservice", u.Query().Get("service")) - scope := u.Query().Get("scope") - suite.Contains(scope, "repository:testrepo/local-subchart:pull") + case req := <-requests: + suite.Equal("/auth", req.path) + suite.Equal("testservice", req.service) + suite.Contains(req.scope, "repository:testrepo/local-subchart:pull") case <-time.After(5 * time.Second): suite.T().Fatal("timeout waiting for auth request") } diff --git a/pkg/registry/client_test.go b/pkg/registry/client_test.go index 21604ce7e..9029ba906 100644 --- a/pkg/registry/client_test.go +++ b/pkg/registry/client_test.go @@ -17,17 +17,22 @@ limitations under the License. package registry import ( + "context" + "encoding/json" + "fmt" "io" "net/http" "net/http/httptest" + "os" "path/filepath" "strings" "testing" ocispec "github.com/opencontainers/image-spec/specs-go/v1" - "github.com/stretchr/testify/assert" + "github.com/oras-project/oras-go/v3/content/memory" + "github.com/oras-project/oras-go/v3/registry/remote/credentials" + "github.com/oras-project/oras-go/v3/registry/remote/policy" "github.com/stretchr/testify/require" - "oras.land/oras-go/v2/content/memory" ) // Inspired by oras test @@ -57,8 +62,8 @@ func TestTagManifestTransformsReferences(t *testing.T) { require.Error(t, err, "Should NOT find the reference with the original +") } -// Verifies that Login always restores ForceAttemptOAuth2 to false on success. -func TestLogin_ResetsForceAttemptOAuth2_OnSuccess(t *testing.T) { +// Verifies that the authorizer is set on a new client and Login succeeds against a reachable registry. +func TestLogin_AuthorizerSetAndSucceeds(t *testing.T) { t.Parallel() srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -80,17 +85,18 @@ func TestLogin_ResetsForceAttemptOAuth2_OnSuccess(t *testing.T) { ) require.NoError(t, err, "NewClient error") - require.NotNil(t, c.authorizer) - require.False(t, c.authorizer.ForceAttemptOAuth2, "expected ForceAttemptOAuth2 default to be false") + if c.authorizer == nil { + t.Fatal("expected authorizer to be set") + } // Call Login with plain HTTP against our test server - require.NoError(t, c.Login(host, LoginOptPlainText(true), LoginOptBasicAuth("u", "p")), "Login error") - - assert.False(t, c.authorizer.ForceAttemptOAuth2, "ForceAttemptOAuth2 should be false after successful Login") + if err := c.Login(host, LoginOptPlainText(true), LoginOptBasicAuth("u", "p")); err != nil { + t.Fatalf("Login error: %v", err) + } } -// Verifies that Login restores ForceAttemptOAuth2 to false even when ping fails. -func TestLogin_ResetsForceAttemptOAuth2_OnFailure(t *testing.T) { +// Verifies that Login returns an error when the registry is unreachable. +func TestLogin_FailsWhenUnreachable(t *testing.T) { t.Parallel() // Start and immediately close, so connections will fail @@ -105,51 +111,228 @@ func TestLogin_ResetsForceAttemptOAuth2_OnFailure(t *testing.T) { ) require.NoError(t, err, "NewClient error") - // Invoke Login, expect an error but ForceAttemptOAuth2 must end false - _ = c.Login(host, LoginOptPlainText(true), LoginOptBasicAuth("u", "p")) - - assert.False(t, c.authorizer.ForceAttemptOAuth2, "ForceAttemptOAuth2 should be false after failed Login") + // Invoke Login, expect an error since the server is closed + if err := c.Login(host, LoginOptPlainText(true), LoginOptBasicAuth("u", "p")); err == nil { + t.Error("expected Login to fail when server is unreachable") + } } -// TestWarnIfHostHasPath verifies that warnIfHostHasPath correctly detects path components. -func TestWarnIfHostHasPath(t *testing.T) { +func TestLogin_NamespacedAuth(t *testing.T) { t.Parallel() - tests := []struct { - name string - host string - wantWarn bool - }{ - { - name: "domain only", - host: "ghcr.io", - wantWarn: false, - }, - { - name: "domain with port", - host: "localhost:8000", - wantWarn: false, - }, - { - name: "domain with repository path", - host: "ghcr.io/terryhowe", - wantWarn: true, - }, - { - name: "domain with nested path", - host: "ghcr.io/terryhowe/myrepo", - wantWarn: true, - }, - { - name: "localhost with port and path", - host: "localhost:8000/myrepo", - wantWarn: true, - }, - } + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/v2/" { + w.WriteHeader(http.StatusOK) + return + } + http.NotFound(w, r) + })) + defer srv.Close() + + registryHost := strings.TrimPrefix(srv.URL, "http://") + namespacedHost := registryHost + "/myrepo" + + // Pre-create a config.json with a sentinel auths entry so that + // IsAuthConfigured() returns true and the credentials package does not + // detect a native platform store (e.g., osxkeychain on macOS). This + // ensures the credential is written into the plaintext config file + // where we can inspect the storage key directly. + credFile := filepath.Join(t.TempDir(), "config.json") + err := os.WriteFile(credFile, []byte(`{"auths":{"_sentinel_":{}}}`), 0o600) + require.NoError(t, err) + + c, err := NewClient( + ClientOptWriter(io.Discard), + ClientOptCredentialsFile(credFile), + ) + require.NoError(t, err) - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - assert.Equal(t, tt.wantWarn, warnIfHostHasPath(tt.host)) - }) + err = c.Login(namespacedHost, LoginOptPlainText(true), LoginOptBasicAuth("u", "p")) + require.NoError(t, err) + + ctx := context.Background() + + // Credential lookup by namespaced key succeeds. + cred, err := c.credentialsStore.Get(ctx, namespacedHost) + require.NoError(t, err) + require.Equal(t, "u", cred.Username) + + // Verify that the credential is stored on disk under the namespaced key, + // and NOT under the hostname-only key. We inspect the JSON file directly + // because the FileStore's Get() falls back to a hostname-based lookup, + // which would mask whether the storage key itself is hostname-only or + // namespaced. + data, err := os.ReadFile(credFile) + require.NoError(t, err) + var parsed struct { + Auths map[string]any `json:"auths"` } + require.NoError(t, json.Unmarshal(data, &parsed)) + require.Contains(t, parsed.Auths, namespacedHost, "credential should be stored under namespaced key") + require.NotContains(t, parsed.Auths, registryHost, "credential should not be stored under hostname-only key") +} + +func TestNamespacedStore_HierarchicalLookup(t *testing.T) { + t.Parallel() + + inner := &memCredStore{creds: map[string]credentials.Credential{}} + ctx := context.Background() + + // Store a credential under "localhost:5000/org". + _ = inner.Put(ctx, "localhost:5000/org", credentials.Credential{Username: "orguser"}) + + // Lookup for a deeper path should find it. + ns := &namespacedStore{inner: inner, repository: "org/repo"} + cred, err := ns.Get(ctx, "localhost:5000") + require.NoError(t, err) + require.Equal(t, "orguser", cred.Username) + + // Lookup for an unrelated path should NOT find it. + ns2 := &namespacedStore{inner: inner, repository: "other/repo"} + cred, err = ns2.Get(ctx, "localhost:5000") + require.NoError(t, err) + require.Empty(t, cred.Username) +} + +// memCredStore is a simple in-memory credentials.Store for testing. +type memCredStore struct { + creds map[string]credentials.Credential +} + +func (m *memCredStore) Get(_ context.Context, serverAddress string) (credentials.Credential, error) { + return m.creds[serverAddress], nil +} + +func (m *memCredStore) Put(_ context.Context, serverAddress string, cred credentials.Credential) error { + m.creds[serverAddress] = cred + return nil +} + +func (m *memCredStore) Delete(_ context.Context, serverAddress string) error { + delete(m.creds, serverAddress) + return nil +} + +func TestNewClient_WithDenyAllPolicy(t *testing.T) { + t.Parallel() + + denyPolicy := policy.NewRejectAllPolicy() + evaluator, err := policy.NewEvaluator(denyPolicy) + require.NoError(t, err) + + credFile := filepath.Join(t.TempDir(), "config.json") + c, err := NewClient( + ClientOptWriter(io.Discard), + ClientOptCredentialsFile(credFile), + ClientOptPolicyEvaluator(evaluator), + ) + require.NoError(t, err) + require.Same(t, evaluator, c.policyEvaluator) +} + +func TestNewClient_WithConfigOptions(t *testing.T) { + t.Parallel() + + credFile := filepath.Join(t.TempDir(), "config.json") + c, err := NewClient( + ClientOptWriter(io.Discard), + ClientOptCredentialsFile(credFile), + ClientOptConfigOptions(ConfigOptions{ + RegistriesConfigPath: "/nonexistent/registries.conf", + }), + ) + require.NoError(t, err) // nonexistent paths are silently skipped + require.NotNil(t, c) +} + +func TestLogin_LocationRewrite(t *testing.T) { + t.Parallel() + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/v2/" { + w.WriteHeader(http.StatusOK) + return + } + http.NotFound(w, r) + })) + defer srv.Close() + + canonicalHost := strings.TrimPrefix(srv.URL, "http://") + aliasHost := "registry.example.test" + + // Write a registries.conf that maps the alias to the canonical host. + registriesConf := filepath.Join(t.TempDir(), "registries.conf") + err := os.WriteFile(registriesConf, fmt.Appendf(nil, + "[[registry]]\nprefix = %q\nlocation = %q\n", aliasHost, canonicalHost, + ), 0o600) + require.NoError(t, err) + + credFile := filepath.Join(t.TempDir(), "config.json") + c, err := NewClient( + ClientOptWriter(io.Discard), + ClientOptCredentialsFile(credFile), + ClientOptConfigOptions(ConfigOptions{RegistriesConfigPath: registriesConf}), + ) + require.NoError(t, err) + + // Login via alias; the connection goes to the canonical host (plain HTTP). + require.NoError(t, c.Login(aliasHost, LoginOptPlainText(true), LoginOptBasicAuth("u", "p"))) + + // Credential must be stored under the canonical host, not the alias. + cred, err := c.credentialsStore.Get(context.Background(), canonicalHost) + require.NoError(t, err) + require.Equal(t, "u", cred.Username, "credential should be stored under canonical host") + + aliasCred, err := c.credentialsStore.Get(context.Background(), aliasHost) + require.NoError(t, err) + require.Empty(t, aliasCred.Username, "credential must not be stored under alias") +} + +// mockRemoteClient records whether Do was called, for use in registryAuthorizer tests. +type mockRemoteClient struct { + called bool + inner http.RoundTripper +} + +func (m *mockRemoteClient) Do(req *http.Request) (*http.Response, error) { + m.called = true + return m.inner.RoundTrip(req) +} + +func TestRegistryAuthorizer_UsedInLegacyPath(t *testing.T) { + t.Parallel() + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/v2/" { + w.WriteHeader(http.StatusOK) + return + } + // Return empty tag list for any tags request. + if strings.HasSuffix(r.URL.Path, "/tags/list") { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + w.Write([]byte(`{"name":"testchart","tags":[]}`)) + return + } + http.NotFound(w, r) + })) + defer srv.Close() + + host := strings.TrimPrefix(srv.URL, "http://") + mock := &mockRemoteClient{inner: http.DefaultTransport} + + credFile := filepath.Join(t.TempDir(), "config.json") + c, err := NewClient( + ClientOptWriter(io.Discard), + ClientOptCredentialsFile(credFile), + ClientOptHTTPClient(&http.Client{}), // triggers customHTTPClient=true (legacy path) + ClientOptRegistryAuthorizer(mock), + ClientOptPlainHTTP(), + ) + require.NoError(t, err) + + // Tags calls newRepository → legacy path → should use registryAuthorizer. + _, err = c.Tags(host + "/testchart") + require.NoError(t, err) + require.True(t, mock.called, "registryAuthorizer.Do should have been called in legacy path") } diff --git a/pkg/registry/generic.go b/pkg/registry/generic.go index b46133d91..b03c33276 100644 --- a/pkg/registry/generic.go +++ b/pkg/registry/generic.go @@ -19,33 +19,20 @@ package registry import ( "context" "io" - "net/http" "slices" "sort" "sync" ocispec "github.com/opencontainers/image-spec/specs-go/v1" - "oras.land/oras-go/v2" - "oras.land/oras-go/v2/content" - "oras.land/oras-go/v2/content/memory" - "oras.land/oras-go/v2/registry/remote" - "oras.land/oras-go/v2/registry/remote/auth" - "oras.land/oras-go/v2/registry/remote/credentials" + "github.com/oras-project/oras-go/v3" + "github.com/oras-project/oras-go/v3/content" + "github.com/oras-project/oras-go/v3/content/memory" ) // GenericClient provides low-level OCI operations without artifact-specific assumptions type GenericClient struct { - debug bool - enableCache bool - credentialsFile string - username string - password string - out io.Writer - authorizer *auth.Client - registryAuthorizer RemoteClient - credentialsStore credentials.Store - httpClient *http.Client - plainHTTP bool + client *Client + out io.Writer } // GenericPullOptions configures a generic pull operation @@ -69,17 +56,8 @@ type GenericPullResult struct { // NewGenericClient creates a new generic OCI client from an existing Client func NewGenericClient(client *Client) *GenericClient { return &GenericClient{ - debug: client.debug, - enableCache: client.enableCache, - credentialsFile: client.credentialsFile, - username: client.username, - password: client.password, - out: client.out, - authorizer: client.authorizer, - registryAuthorizer: client.registryAuthorizer, - credentialsStore: client.credentialsStore, - httpClient: client.httpClient, - plainHTTP: client.plainHTTP, + client: client, + out: client.out, } } @@ -93,13 +71,10 @@ func (c *GenericClient) PullGeneric(ref string, options GenericPullOptions) (*Ge memoryStore := memory.New() var descriptors []ocispec.Descriptor - // Set up a repository with authentication and configuration - repository, err := remote.NewRepository(parsedRef.String()) + repository, err := c.client.newRepository(parsedRef.String()) if err != nil { return nil, err } - repository.PlainHTTP = c.plainHTTP - repository.Client = c.authorizer ctx := context.Background() diff --git a/pkg/registry/reference.go b/pkg/registry/reference.go index 455792539..998b8f264 100644 --- a/pkg/registry/reference.go +++ b/pkg/registry/reference.go @@ -19,11 +19,11 @@ package registry import ( "strings" - "oras.land/oras-go/v2/registry" + "github.com/oras-project/oras-go/v3/registry/remote/properties" ) type reference struct { - orasReference registry.Reference + orasReference properties.Reference Registry string Repository string Tag string @@ -60,13 +60,13 @@ func newReference(raw string) (result reference, err error) { } } - result.orasReference, err = registry.ParseReference(raw) + result.orasReference, err = properties.NewReference(raw) if err != nil { return result, err } result.Registry = result.orasReference.Registry result.Repository = result.orasReference.Repository - result.Tag = result.orasReference.Reference + result.Tag = result.orasReference.Tag return result, nil } diff --git a/pkg/registry/reference_test.go b/pkg/registry/reference_test.go index 07987cccb..4342254ff 100644 --- a/pkg/registry/reference_test.go +++ b/pkg/registry/reference_test.go @@ -27,7 +27,7 @@ func verify(t *testing.T, actual reference, registry, repository, tag, digest st t.Helper() assert.Equal(t, registry, actual.orasReference.Registry, "Oras reference registry") assert.Equal(t, repository, actual.orasReference.Repository, "Oras reference repository") - assert.Equal(t, tag, actual.orasReference.Reference, "Oras reference reference") + assert.Equal(t, tag, actual.orasReference.Tag, "Oras reference tag") assert.Equal(t, registry, actual.Registry, "Registry") assert.Equal(t, repository, actual.Repository, "Repository") assert.Equal(t, tag, actual.Tag, "Tag") diff --git a/pkg/registry/registry_test.go b/pkg/registry/registry_test.go index 40ebe9687..ab6cb06a5 100644 --- a/pkg/registry/registry_test.go +++ b/pkg/registry/registry_test.go @@ -144,12 +144,12 @@ func setup(suite *TestRegistry, tlsEnabled, insecure bool, auth string) { config.Storage = map[string]configuration.Parameters{"inmemory": map[string]any{}} if auth == "token" { - ln, err := lnCfg.Listen(suite.T().Context(), "tcp", "127.0.0.1:0") + authLn, err := lnCfg.Listen(suite.T().Context(), "tcp", "127.0.0.1:0") suite.Require().NoError(err, "no error finding free port for test auth server") - defer ln.Close() + defer func() { _ = authLn.Close() }() // set test auth server host - suite.AuthServerHost = ln.Addr().String() + suite.AuthServerHost = authLn.Addr().String() config.Auth = configuration.Auth{ "token": configuration.Parameters{ diff --git a/pkg/registry/transport.go b/pkg/registry/transport.go index fa0103ac0..41446ca20 100644 --- a/pkg/registry/transport.go +++ b/pkg/registry/transport.go @@ -27,7 +27,7 @@ import ( "strings" "sync/atomic" - "oras.land/oras-go/v2/registry/remote/retry" + "github.com/oras-project/oras-go/v3/registry/remote/retry" ) var ( From 49d661945f4d61f7b021c6a004bf6b3fe00ab3b8 Mon Sep 17 00:00:00 2001 From: Terry Howe Date: Mon, 13 Jul 2026 12:13:04 -0600 Subject: [PATCH 2/5] refactor: drop unused registry config path overrides Only registries.conf was ever set (by tests); PolicyConfigPath, CertsDirPaths, and ContainersAuthPath were always empty and fell through to the container ecosystem default search locations. Remove the exported ConfigOptions struct and ClientOptConfigOptions option, and replace them with an unexported withRegistriesConfigPath test hook. No production behavior changes; avoids committing public API to the still-experimental oras-go v3 config surface. Signed-off-by: Terry Howe --- pkg/registry/client.go | 27 +++++++++++---------------- pkg/registry/client_test.go | 8 +++----- 2 files changed, 14 insertions(+), 21 deletions(-) diff --git a/pkg/registry/client.go b/pkg/registry/client.go index 66a778e47..871209e9d 100644 --- a/pkg/registry/client.go +++ b/pkg/registry/client.go @@ -57,14 +57,6 @@ storing semantic versions, Helm adopts the convention of changing plus (+) to an underscore (_) in chart version tags when pushing to a registry and back to a plus (+) when pulling from a registry.` -// ConfigOptions specifies override paths for container ecosystem config files. -type ConfigOptions struct { - RegistriesConfigPath string - PolicyConfigPath string - CertsDirPaths []string - ContainersAuthPath string -} - type ( // RemoteClient shadows the ORAS remote.Client interface // (hiding the ORAS type from Helm client visibility) @@ -92,7 +84,9 @@ type ( builder *remote.ClientBuilder policyEvaluator *policy.Evaluator signatureVerification bool - configOptions ConfigOptions + // registriesConfigPath overrides the registries.conf path; empty means + // the container ecosystem default search locations are used. Test-only. + registriesConfigPath string insecure bool certFile string keyFile string @@ -137,11 +131,10 @@ func NewClient(options ...ClientOption) (*Client, error) { // Load the full container ecosystem config stack: Docker config.json, // containers auth.json, registries.conf, policy.json, certs.d, registries.d. // Missing files are silently skipped. + // Only registries.conf is overridable (empty means default search); all + // other config files always resolve to their default locations. loaderOpts := remoteconfig.LoadConfigsOptions{ - RegistriesConfigPath: client.configOptions.RegistriesConfigPath, - PolicyConfigPath: client.configOptions.PolicyConfigPath, - CertsDirPaths: client.configOptions.CertsDirPaths, - ContainersAuthPath: client.configOptions.ContainersAuthPath, + RegistriesConfigPath: client.registriesConfigPath, } configs, err := remoteconfig.LoadConfigsWithOptions(loaderOpts) if err != nil { @@ -415,10 +408,12 @@ func ClientOptSignatureVerification(enabled bool) ClientOption { } } -// ClientOptConfigOptions returns a function that overrides default config file paths. -func ClientOptConfigOptions(o ConfigOptions) ClientOption { +// withRegistriesConfigPath overrides the registries.conf path. It is unexported +// because the only consumer is hermetic tests; production always uses the +// container ecosystem default search locations. +func withRegistriesConfigPath(path string) ClientOption { return func(c *Client) { - c.configOptions = o + c.registriesConfigPath = path } } diff --git a/pkg/registry/client_test.go b/pkg/registry/client_test.go index 9029ba906..4babf7132 100644 --- a/pkg/registry/client_test.go +++ b/pkg/registry/client_test.go @@ -230,16 +230,14 @@ func TestNewClient_WithDenyAllPolicy(t *testing.T) { require.Same(t, evaluator, c.policyEvaluator) } -func TestNewClient_WithConfigOptions(t *testing.T) { +func TestNewClient_WithRegistriesConfigPath(t *testing.T) { t.Parallel() credFile := filepath.Join(t.TempDir(), "config.json") c, err := NewClient( ClientOptWriter(io.Discard), ClientOptCredentialsFile(credFile), - ClientOptConfigOptions(ConfigOptions{ - RegistriesConfigPath: "/nonexistent/registries.conf", - }), + withRegistriesConfigPath("/nonexistent/registries.conf"), ) require.NoError(t, err) // nonexistent paths are silently skipped require.NotNil(t, c) @@ -271,7 +269,7 @@ func TestLogin_LocationRewrite(t *testing.T) { c, err := NewClient( ClientOptWriter(io.Discard), ClientOptCredentialsFile(credFile), - ClientOptConfigOptions(ConfigOptions{RegistriesConfigPath: registriesConf}), + withRegistriesConfigPath(registriesConf), ) require.NoError(t, err) From 1777fc22a418b82f12ff6a6cfefeb54baaa79ac3 Mon Sep 17 00:00:00 2001 From: Terry Howe Date: Mon, 13 Jul 2026 12:28:51 -0600 Subject: [PATCH 3/5] style: gofmt struct field alignment in registry client Signed-off-by: Terry Howe --- pkg/registry/client.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/registry/client.go b/pkg/registry/client.go index 871209e9d..11dba0162 100644 --- a/pkg/registry/client.go +++ b/pkg/registry/client.go @@ -87,11 +87,11 @@ type ( // registriesConfigPath overrides the registries.conf path; empty means // the container ecosystem default search locations are used. Test-only. registriesConfigPath string - insecure bool - certFile string - keyFile string - caFile string - customHTTPClient bool // true when ClientOptHTTPClient or ClientOptAuthorizer was used + insecure bool + certFile string + keyFile string + caFile string + customHTTPClient bool // true when ClientOptHTTPClient or ClientOptAuthorizer was used } // ClientOption allows specifying various settings configurable by the user for overriding the defaults From 0674d9f4e470220062fefdb0200073059f7b3601 Mon Sep 17 00:00:00 2001 From: Terry Howe Date: Tue, 1 Sep 2026 15:08:19 -0600 Subject: [PATCH 4/5] refactor(registry): drop host-path login warning superseded by namespaced login warnIfHostHasPath warned that registry login only supports a hostname and not a repository path. Namespaced login now supports exactly that, so the warning is no longer accurate and the helper has no call site. Signed-off-by: Terry Howe --- pkg/registry/client.go | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/pkg/registry/client.go b/pkg/registry/client.go index 11dba0162..3954712e2 100644 --- a/pkg/registry/client.go +++ b/pkg/registry/client.go @@ -428,17 +428,6 @@ type ( } ) -// warnIfHostHasPath checks if the host contains a repository path and logs a warning if it does. -// Returns true if the host contains a path component (i.e., contains a '/'). -func warnIfHostHasPath(host string) bool { - if strings.Contains(host, "/") { - registryHost, _, _ := strings.Cut(host, "/") - slog.Warn("registry login currently only supports registry hostname, not a repository path", "host", host, "suggested", registryHost) - return true - } - return false -} - // noopStore is a credentials.Store that performs no persistence. It is used // during namespaced login to verify credentials with a registry ping without // causing remote.Login to store credentials under the hostname-only key. From e43d580b70def213d1df23c8bc6e5f6219e0e5f1 Mon Sep 17 00:00:00 2001 From: Terry Howe Date: Tue, 1 Sep 2026 15:08:24 -0600 Subject: [PATCH 5/5] build(deps): bump oras-go to v3.0.3-dev Signed-off-by: Terry Howe --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 3ec31a475..5848f0996 100644 --- a/go.mod +++ b/go.mod @@ -28,7 +28,7 @@ require ( github.com/moby/term v0.5.2 github.com/opencontainers/go-digest v1.0.0 github.com/opencontainers/image-spec v1.1.1 - github.com/oras-project/oras-go/v3 v3.0.2-dev + github.com/oras-project/oras-go/v3 v3.0.3-dev github.com/rubenv/sql-migrate v1.8.1 github.com/santhosh-tekuri/jsonschema/v6 v6.0.3 github.com/spf13/cobra v1.10.2 diff --git a/go.sum b/go.sum index c037fb865..c230fa482 100644 --- a/go.sum +++ b/go.sum @@ -241,8 +241,8 @@ github.com/opencontainers/go-digest v1.0.0 h1:apOUWs51W5PlhuyGyz9FCeeBIOUDA/6nW8 github.com/opencontainers/go-digest v1.0.0/go.mod h1:0JzlMkj0TRzQZfJkVvzbP0HBR3IKzErnv2BNG4W4MAM= github.com/opencontainers/image-spec v1.1.1 h1:y0fUlFfIZhPF1W537XOLg0/fcx6zcHCJwooC2xJA040= github.com/opencontainers/image-spec v1.1.1/go.mod h1:qpqAh3Dmcf36wStyyWU+kCeDgrGnAve2nCC8+7h8Q0M= -github.com/oras-project/oras-go/v3 v3.0.2-dev h1:FoSopdghExPGXRV+CvMjpXNPO74oZhquowB15OZL3l4= -github.com/oras-project/oras-go/v3 v3.0.2-dev/go.mod h1:bSbdCn421//d2i4+mATPCvbNcmpM+fI1G8no/mZBcD4= +github.com/oras-project/oras-go/v3 v3.0.3-dev h1:1tpitwyCDXMai5TfI/tJEOtBl2Av6fAu5CvGfpq2Xj4= +github.com/oras-project/oras-go/v3 v3.0.3-dev/go.mod h1:8eV2J5RW//YjOcI7SZKhqz2o0ivP3300wBB6Fgf2CIY= github.com/peterbourgon/diskv v2.0.1+incompatible h1:UBdAOUP5p4RWqPBg048CAvpKN+vxiaj6gdUUzhl4XmI= github.com/peterbourgon/diskv v2.0.1+incompatible/go.mod h1:uqqh8zWWbv1HBMNONnaR/tNboyR3/BZd58JJSHlUSCU= github.com/pkg/errors v0.8.0/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=