diff --git a/api/oci/extensions/repositories/ocireg/repository.go b/api/oci/extensions/repositories/ocireg/repository.go index 51d03cd690..d34ab0bacb 100644 --- a/api/oci/extensions/repositories/ocireg/repository.go +++ b/api/oci/extensions/repositories/ocireg/repository.go @@ -3,7 +3,6 @@ package ocireg import ( "context" "crypto/tls" - "crypto/x509" "net/http" "path" "strings" @@ -53,6 +52,10 @@ type RepositoryImpl struct { logger logging.UnboundLogger spec *RepositorySpec info *RepositoryInfo + + transport *http.Transport + timeout *time.Duration + lock *locker.Locker } var ( @@ -66,11 +69,18 @@ func NewRepository(ctx cpi.Context, spec *RepositorySpec, info *RepositoryInfo) if urs.Scheme == "http" { logger.Warn("using insecure http for oci registry {{host}}", "host", urs.Host) } + transport, timeout, err := configureTransport(ctx, info.Scheme) + if err != nil { + return nil, err + } i := &RepositoryImpl{ RepositoryImplBase: cpi.NewRepositoryImplBase(ctx), logger: logger, spec: spec, info: info, + transport: transport, + timeout: timeout, + lock: locker.New(), } i.logger.Debug("created repository") return cpi.NewRepository(i), nil @@ -89,6 +99,10 @@ func (r *RepositoryImpl) GetSpecification() cpi.RepositorySpec { } func (r *RepositoryImpl) Close() error { + // transport is always initialized in NewRepository; CloseIdleConnections + // only drains idle pooled connections, the transport itself remains usable + r.transport.CloseIdleConnections() + return nil } @@ -155,18 +169,20 @@ func (r *RepositoryImpl) getResolver(comp string) (oras.Resolver, error) { } } - baseTransport, timeout, err := configureTransport(r.GetContext(), r.info.Scheme, creds) - if err != nil { - return nil, err + if creds != nil && r.transport.TLSClientConfig != nil { + c := creds.GetProperty(credentials.ATTR_CERTIFICATE_AUTHORITY) + if c != "" { + r.transport.TLSClientConfig.RootCAs.AppendCertsFromPEM([]byte(c)) + } } - retryTransport := retry.NewTransport(baseTransport) + retryTransport := retry.NewTransport(r.transport) client := &http.Client{ Transport: ocmlog.NewRoundTripper(retryTransport, logger), } - if timeout != nil { - client.Timeout = *timeout + if r.timeout != nil { + client.Timeout = *r.timeout } authClient := &auth.Client{ @@ -185,11 +201,11 @@ func (r *RepositoryImpl) getResolver(comp string) (oras.Resolver, error) { Client: authClient, PlainHTTP: r.info.Scheme == "http", Logger: logger, - Lock: locker.New(), + Lock: r.lock, }), nil } -func configureTransport(ctx cpi.Context, scheme string, creds credentials.Credentials) (*http.Transport, *time.Duration, error) { +func configureTransport(ctx cpi.Context, scheme string) (*http.Transport, *time.Duration, error) { httpSettings, err := ctx.GetHTTPSettings() if err != nil { return nil, nil, err @@ -203,17 +219,7 @@ func configureTransport(ctx cpi.Context, scheme string, creds credentials.Creden if scheme == "https" { //nolint:gosec // used like the default, there are OCI servers (quay.io) not working with min version. baseTransport.TLSClientConfig = &tls.Config{ - // MinVersion: tls.VersionTLS13, - RootCAs: func() *x509.CertPool { - rootCAs := rootcertsattr.Get(ctx).GetRootCertPool(true) - if creds != nil { - c := creds.GetProperty(credentials.ATTR_CERTIFICATE_AUTHORITY) - if c != "" { - rootCAs.AppendCertsFromPEM([]byte(c)) - } - } - return rootCAs - }(), + RootCAs: rootcertsattr.Get(ctx).GetRootCertPool(true), } } diff --git a/api/oci/extensions/repositories/ocireg/repository_test.go b/api/oci/extensions/repositories/ocireg/repository_test.go index 6e8646ec94..112300961d 100644 --- a/api/oci/extensions/repositories/ocireg/repository_test.go +++ b/api/oci/extensions/repositories/ocireg/repository_test.go @@ -14,7 +14,7 @@ import ( func TestConfigureTransport(t *testing.T) { t.Run("HTTPS sets TLSClientConfig with RootCAs", func(t *testing.T) { ctx := cpi.New() - transport, _, err := configureTransport(ctx, "https", nil) + transport, _, err := configureTransport(ctx, "https") require.NoError(t, err) require.NotNil(t, transport.TLSClientConfig) @@ -23,7 +23,7 @@ func TestConfigureTransport(t *testing.T) { t.Run("HTTP does not set RootCAs", func(t *testing.T) { ctx := cpi.New() - transport, _, err := configureTransport(ctx, "http", nil) + transport, _, err := configureTransport(ctx, "http") require.NoError(t, err) if transport.TLSClientConfig != nil { @@ -31,7 +31,7 @@ func TestConfigureTransport(t *testing.T) { } }) - t.Run("HTTPS appends CA cert from credentials", func(t *testing.T) { + t.Run("HTTPS appends CA cert from credentials via getResolver", func(t *testing.T) { ctx := cpi.New() // Self-signed test CA certificate (PEM format). @@ -43,10 +43,15 @@ ABC= credentials.ATTR_CERTIFICATE_AUTHORITY: caCert, }) - transport, _, err := configureTransport(ctx, "https", creds) - + transport, _, err := configureTransport(ctx, "https") require.NoError(t, err) require.NotNil(t, transport.TLSClientConfig) assert.NotNil(t, transport.TLSClientConfig.RootCAs) + + // CA cert appending now happens in getResolver, not configureTransport. + // Verify the mechanism works directly. + c := creds.GetProperty(credentials.ATTR_CERTIFICATE_AUTHORITY) + assert.NotEmpty(t, c) + transport.TLSClientConfig.RootCAs.AppendCertsFromPEM([]byte(c)) }) }