Skip to content

Commit 62eefc2

Browse files
committed
chore: fix the transport leak in getResolver
On-behalf-of: Gergely Brautigam <gergely.brautigam@sap.com> Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>
1 parent baefd59 commit 62eefc2

1 file changed

Lines changed: 46 additions & 21 deletions

File tree

api/oci/extensions/repositories/ocireg/repository.go

Lines changed: 46 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,10 @@ package ocireg
33
import (
44
"context"
55
"crypto/tls"
6-
"crypto/x509"
76
"net/http"
87
"path"
98
"strings"
9+
"sync"
1010
"time"
1111

1212
"github.com/containerd/errdefs"
@@ -53,6 +53,11 @@ type RepositoryImpl struct {
5353
logger logging.UnboundLogger
5454
spec *RepositorySpec
5555
info *RepositoryInfo
56+
57+
mu sync.Mutex
58+
transport *http.Transport
59+
timeout *time.Duration
60+
lock *locker.Locker
5661
}
5762

5863
var (
@@ -89,6 +94,14 @@ func (r *RepositoryImpl) GetSpecification() cpi.RepositorySpec {
8994
}
9095

9196
func (r *RepositoryImpl) Close() error {
97+
r.mu.Lock()
98+
defer r.mu.Unlock()
99+
100+
if r.transport != nil {
101+
r.transport.CloseIdleConnections()
102+
r.transport = nil
103+
}
104+
92105
return nil
93106
}
94107

@@ -118,6 +131,26 @@ func (r *RepositoryImpl) getCreds(comp string) (credentials.Credentials, error)
118131
return identity.GetCredentials(r.GetContext(), r.info.Locator, comp)
119132
}
120133

134+
func (r *RepositoryImpl) getOrCreateTransport() (*http.Transport, *time.Duration, error) {
135+
r.mu.Lock()
136+
defer r.mu.Unlock()
137+
138+
if r.transport == nil {
139+
transport, timeout, err := configureTransport(r.GetContext(), r.info.Scheme, nil)
140+
if err != nil {
141+
return nil, nil, err
142+
}
143+
r.transport = transport
144+
r.timeout = timeout
145+
}
146+
147+
if r.lock == nil {
148+
r.lock = locker.New()
149+
}
150+
151+
return r.transport, r.timeout, nil
152+
}
153+
121154
func (r *RepositoryImpl) getResolver(comp string) (oras.Resolver, error) {
122155
creds, err := r.getCreds(comp)
123156
if err != nil {
@@ -136,8 +169,6 @@ func (r *RepositoryImpl) getResolver(comp string) (oras.Resolver, error) {
136169
password := creds.GetProperty(credentials.ATTR_PASSWORD)
137170
token := creds.GetProperty(credentials.ATTR_IDENTITY_TOKEN)
138171

139-
// If ATTR_PASSWORD was not set but there IS a username defined we do have an ATTR_IDENTITY_TOKEN set,
140-
// we have to provide that token through the `Password` field for authentication.
141172
if password == "" && token != "" && username != "" {
142173
password = token
143174
}
@@ -147,20 +178,24 @@ func (r *RepositoryImpl) getResolver(comp string) (oras.Resolver, error) {
147178
Password: password,
148179
}
149180

150-
// If there was NO username set ( for example, docker login, azure login, etc... ) but the token
151-
// IS set we are dealing with a RefreshToken. RefreshTokens CANNOT be used together with a username.
152-
// There are checks for that resulting in a "The operation is unsupported" error.
153181
if token != "" && username == "" {
154182
authCreds.RefreshToken = token
155183
}
156184
}
157185

158-
baseTransport, timeout, err := configureTransport(r.GetContext(), r.info.Scheme, creds)
186+
transport, timeout, err := r.getOrCreateTransport()
159187
if err != nil {
160188
return nil, err
161189
}
162190

163-
retryTransport := retry.NewTransport(baseTransport)
191+
if creds != nil && transport.TLSClientConfig != nil {
192+
c := creds.GetProperty(credentials.ATTR_CERTIFICATE_AUTHORITY)
193+
if c != "" {
194+
transport.TLSClientConfig.RootCAs.AppendCertsFromPEM([]byte(c))
195+
}
196+
}
197+
198+
retryTransport := retry.NewTransport(transport)
164199

165200
client := &http.Client{
166201
Transport: ocmlog.NewRoundTripper(retryTransport, logger),
@@ -185,11 +220,11 @@ func (r *RepositoryImpl) getResolver(comp string) (oras.Resolver, error) {
185220
Client: authClient,
186221
PlainHTTP: r.info.Scheme == "http",
187222
Logger: logger,
188-
Lock: locker.New(),
223+
Lock: r.lock,
189224
}), nil
190225
}
191226

192-
func configureTransport(ctx cpi.Context, scheme string, creds credentials.Credentials) (*http.Transport, *time.Duration, error) {
227+
func configureTransport(ctx cpi.Context, scheme string, _ credentials.Credentials) (*http.Transport, *time.Duration, error) {
193228
httpSettings, err := ctx.GetHTTPSettings()
194229
if err != nil {
195230
return nil, nil, err
@@ -203,17 +238,7 @@ func configureTransport(ctx cpi.Context, scheme string, creds credentials.Creden
203238
if scheme == "https" {
204239
//nolint:gosec // used like the default, there are OCI servers (quay.io) not working with min version.
205240
baseTransport.TLSClientConfig = &tls.Config{
206-
// MinVersion: tls.VersionTLS13,
207-
RootCAs: func() *x509.CertPool {
208-
rootCAs := rootcertsattr.Get(ctx).GetRootCertPool(true)
209-
if creds != nil {
210-
c := creds.GetProperty(credentials.ATTR_CERTIFICATE_AUTHORITY)
211-
if c != "" {
212-
rootCAs.AppendCertsFromPEM([]byte(c))
213-
}
214-
}
215-
return rootCAs
216-
}(),
241+
RootCAs: rootcertsattr.Get(ctx).GetRootCertPool(true),
217242
}
218243
}
219244

0 commit comments

Comments
 (0)