From dae2173568d70c87e29c85cf106606ab77db2b92 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sat, 26 Feb 2022 14:52:12 +0100 Subject: [PATCH] registry: defaultService: use sync.RWMutex Most operations only require read access, so change this to use an RWMutex, and some minor refactoring in lookupV2Endpoints() so that we are not constructing tlsconfig multiple times in some cases. Signed-off-by: Sebastiaan van Stijn --- registry/registry.go | 1 + registry/service.go | 34 +++++++++++++++------------------- registry/service_v2.go | 10 ++++------ 3 files changed, 20 insertions(+), 25 deletions(-) diff --git a/registry/registry.go b/registry/registry.go index 983e4243bf..5ff39ce5e7 100644 --- a/registry/registry.go +++ b/registry/registry.go @@ -20,6 +20,7 @@ func HostCertsDir(hostname string) string { return filepath.Join(CertsDir(), cleanPath(hostname)) } +// newTLSConfig constructs a client TLS configuration based on server defaults func newTLSConfig(hostname string, isSecure bool) (*tls.Config, error) { // PreferredServerCipherSuites should have no effect tlsConfig := tlsconfig.ServerDefault() diff --git a/registry/service.go b/registry/service.go index d66736cb4f..cf53472c52 100644 --- a/registry/service.go +++ b/registry/service.go @@ -39,7 +39,7 @@ type Service interface { // of mirrors. type defaultService struct { config *serviceConfig - mu sync.Mutex + mu sync.RWMutex } // NewService returns a new instance of defaultService ready to be @@ -52,8 +52,8 @@ func NewService(options ServiceOptions) (Service, error) { // ServiceConfig returns the public registry service configuration. func (s *defaultService) ServiceConfig() *registry.ServiceConfig { - s.mu.Lock() - defer s.mu.Unlock() + s.mu.RLock() + defer s.mu.RUnlock() servConfig := registry.ServiceConfig{ AllowNondistributableArtifactsCIDRs: make([]*(registry.NetIPNet), 0), @@ -167,9 +167,9 @@ func (s *defaultService) Search(ctx context.Context, term string, limit int, aut indexName, remoteName := splitReposSearchTerm(term) // Search is a long-running operation, just lock s.config to avoid block others. - s.mu.Lock() + s.mu.RLock() index, err := newIndexInfo(s.config, indexName) - s.mu.Unlock() + s.mu.RUnlock() if err != nil { return nil, err @@ -226,8 +226,8 @@ func (s *defaultService) Search(ctx context.Context, term string, limit int, aut // ResolveRepository splits a repository name into its components // and configuration of the associated registry. func (s *defaultService) ResolveRepository(name reference.Named) (*RepositoryInfo, error) { - s.mu.Lock() - defer s.mu.Unlock() + s.mu.RLock() + defer s.mu.RUnlock() return newRepositoryInfo(s.config, name) } @@ -244,22 +244,18 @@ type APIEndpoint struct { // TLSConfig constructs a client TLS configuration based on server defaults func (s *defaultService) TLSConfig(hostname string) (*tls.Config, error) { - s.mu.Lock() - defer s.mu.Unlock() + s.mu.RLock() + secure := isSecureIndex(s.config, hostname) + s.mu.RUnlock() - return s.tlsConfig(hostname) -} - -// tlsConfig constructs a client TLS configuration based on server defaults -func (s *defaultService) tlsConfig(hostname string) (*tls.Config, error) { - return newTLSConfig(hostname, isSecureIndex(s.config, hostname)) + return newTLSConfig(hostname, secure) } // LookupPullEndpoints creates a list of v2 endpoints to try to pull from, in order of preference. // It gives preference to mirrors over the actual registry, and HTTPS over plain HTTP. func (s *defaultService) LookupPullEndpoints(hostname string) (endpoints []APIEndpoint, err error) { - s.mu.Lock() - defer s.mu.Unlock() + s.mu.RLock() + defer s.mu.RUnlock() return s.lookupV2Endpoints(hostname) } @@ -267,8 +263,8 @@ func (s *defaultService) LookupPullEndpoints(hostname string) (endpoints []APIEn // LookupPushEndpoints creates a list of v2 endpoints to try to push to, in order of preference. // It gives preference to HTTPS over plain HTTP. Mirrors are not included. func (s *defaultService) LookupPushEndpoints(hostname string) (endpoints []APIEndpoint, err error) { - s.mu.Lock() - defer s.mu.Unlock() + s.mu.RLock() + defer s.mu.RUnlock() allEndpoints, err := s.lookupV2Endpoints(hostname) if err == nil { diff --git a/registry/service_v2.go b/registry/service_v2.go index 11faf239e0..db164bbe90 100644 --- a/registry/service_v2.go +++ b/registry/service_v2.go @@ -8,7 +8,6 @@ import ( ) func (s *defaultService) lookupV2Endpoints(hostname string) (endpoints []APIEndpoint, err error) { - tlsConfig := tlsconfig.ServerDefault() if hostname == DefaultNamespace || hostname == IndexHostname { for _, mirror := range s.config.Mirrors { if !strings.HasPrefix(mirror, "http://") && !strings.HasPrefix(mirror, "https://") { @@ -18,7 +17,7 @@ func (s *defaultService) lookupV2Endpoints(hostname string) (endpoints []APIEndp if err != nil { return nil, invalidParam(err) } - mirrorTLSConfig, err := s.tlsConfig(mirrorURL.Host) + mirrorTLSConfig, err := newTLSConfig(mirrorURL.Host, isSecureIndex(s.config, mirrorURL.Host)) if err != nil { return nil, err } @@ -35,19 +34,18 @@ func (s *defaultService) lookupV2Endpoints(hostname string) (endpoints []APIEndp Version: APIVersion2, Official: true, TrimHostname: true, - TLSConfig: tlsConfig, + TLSConfig: tlsconfig.ServerDefault(), }) return endpoints, nil } - ana := allowNondistributableArtifacts(s.config, hostname) - - tlsConfig, err = s.tlsConfig(hostname) + tlsConfig, err := newTLSConfig(hostname, isSecureIndex(s.config, hostname)) if err != nil { return nil, err } + ana := allowNondistributableArtifacts(s.config, hostname) endpoints = []APIEndpoint{ { URL: &url.URL{