diff --git a/Makefile b/Makefile index 3d7d1064..9406b540 100644 --- a/Makefile +++ b/Makefile @@ -62,6 +62,9 @@ mod-vendor: test: go test -coverprofile coverage.out `go list ./... | egrep -v '(test|mocks|ext/)'` +test-race: + go test -race -coverprofile coverage.out `go list ./... | egrep -v '(test|mocks|ext/)'` + .PHONY: prereq prereq: mkdir -p dist diff --git a/pkg/argocd/update_test.go b/pkg/argocd/update_test.go index e31f1eda..ff8abf86 100644 --- a/pkg/argocd/update_test.go +++ b/pkg/argocd/update_test.go @@ -839,11 +839,11 @@ func Test_UpdateApplication(t *testing.T) { assert.Equal(t, 0, res.NumImagesUpdated) }) - t.Run("Error - unknown registry", func(t *testing.T) { + t.Run("Update from infered registry", func(t *testing.T) { mockClientFn := func(endpoint *registry.RegistryEndpoint, username, password string) (registry.RegistryClient, error) { regMock := regmock.RegistryClient{} regMock.On("NewRepository", mock.Anything).Return(nil) - regMock.On("Tags", mock.Anything).Return([]string{"1.0.1"}, nil) + regMock.On("Tags", mock.Anything).Return([]string{"1.0.1", "1.0.2"}, nil) return ®Mock, nil } @@ -863,7 +863,7 @@ func Test_UpdateApplication(t *testing.T) { Source: v1alpha1.ApplicationSource{ Kustomize: &v1alpha1.ApplicationSourceKustomize{ Images: v1alpha1.KustomizeImages{ - "example.io/jannfis/foobar:1.0.1", + "example.io/jannfis/example:1.0.1", }, }, }, @@ -872,13 +872,13 @@ func Test_UpdateApplication(t *testing.T) { SourceType: v1alpha1.ApplicationSourceTypeKustomize, Summary: v1alpha1.ApplicationSummary{ Images: []string{ - "example.io/jannfis/foobar:1.0.1", + "example.io/jannfis/example:1.0.1", }, }, }, }, Images: image.ContainerImageList{ - image.NewFromIdentifier("example.io/jannfis/foobar:1.0.1"), + image.NewFromIdentifier("example.io/jannfis/example:1.0.x"), }, } res := UpdateApplication(&UpdateConfiguration{ @@ -888,11 +888,11 @@ func Test_UpdateApplication(t *testing.T) { UpdateApp: appImages, DryRun: false, }, NewSyncIterationState()) - assert.Equal(t, 1, res.NumErrors) + assert.Equal(t, 0, res.NumErrors) assert.Equal(t, 0, res.NumSkipped) assert.Equal(t, 1, res.NumApplicationsProcessed) assert.Equal(t, 1, res.NumImagesConsidered) - assert.Equal(t, 0, res.NumImagesUpdated) + assert.Equal(t, 1, res.NumImagesUpdated) }) t.Run("Test error on generic registry client failure", func(t *testing.T) { diff --git a/pkg/registry/config.go b/pkg/registry/config.go index 72168678..05926484 100644 --- a/pkg/registry/config.go +++ b/pkg/registry/config.go @@ -23,6 +23,7 @@ type RegistryConfiguration struct { Insecure bool `yaml:"insecure,omitempty"` DefaultNS string `yaml:"defaultns,omitempty"` Limit int `yaml:"limit,omitempty"` + IsDefault bool `yaml:"default,omitempty"` } // RegistryList contains multiple RegistryConfiguration items @@ -30,6 +31,12 @@ type RegistryList struct { Items []RegistryConfiguration `yaml:"registries"` } +func clearRegistries() { + registryLock.Lock() + registries = make(map[string]*RegistryEndpoint) + registryLock.Unlock() +} + // LoadRegistryConfiguration loads a YAML-formatted registry configuration from // a given file at path. func LoadRegistryConfiguration(path string, clear bool) error { @@ -43,20 +50,35 @@ func LoadRegistryConfiguration(path string, clear bool) error { } if clear { - registryLock.Lock() - registries = make(map[string]*RegistryEndpoint) - registryLock.Unlock() + clearRegistries() } + haveDefault := false + for _, reg := range registryList.Items { tagSortMode := TagListSortFromString(reg.TagSortMode) if tagSortMode != TagListSortUnsorted { log.Warnf("Registry %s has tag sort mode set to %s, meta data retrieval will be disabled for this registry.", reg.ApiURL, tagSortMode) } - err = AddRegistryEndpoint(reg.Prefix, reg.Name, reg.ApiURL, reg.Credentials, reg.DefaultNS, reg.Insecure, tagSortMode, reg.Limit, reg.CredsExpire) - if err != nil { + ep := NewRegistryEndpoint(reg.Prefix, reg.Name, reg.ApiURL, reg.Credentials, reg.DefaultNS, reg.Insecure, tagSortMode, reg.Limit, reg.CredsExpire) + if reg.IsDefault { + if haveDefault { + dep := GetDefaultRegistry() + if dep == nil { + panic("unexpected: default registry should be set, but is not") + } + return fmt.Errorf("cannot set registry %s as default - only one default registry allowed, currently set to %s", ep.RegistryPrefix, dep.RegistryPrefix) + } + } + + if err := AddRegistryEndpoint(ep); err != nil { return err } + + if reg.IsDefault { + SetDefaultRegistry(ep) + haveDefault = true + } } log.Infof("Loaded %d registry configurations from %s", len(registryList.Items), path) @@ -106,8 +128,12 @@ func ParseRegistryConfiguration(yamlSource string) (RegistryList, error) { func RestoreDefaultRegistryConfiguration() { registryLock.Lock() defer registryLock.Unlock() + defaultRegistry = nil registries = make(map[string]*RegistryEndpoint) - for k, v := range defaultRegistries { + for k, v := range registryTweaks { registries[k] = v.DeepCopy() + if v.IsDefault { + SetDefaultRegistry(registries[k]) + } } } diff --git a/pkg/registry/config_test.go b/pkg/registry/config_test.go index 1162583a..080dd004 100644 --- a/pkg/registry/config_test.go +++ b/pkg/registry/config_test.go @@ -75,10 +75,12 @@ registries: } func Test_LoadRegistryConfiguration(t *testing.T) { + RestoreDefaultRegistryConfiguration() + t.Run("Load from valid location", func(t *testing.T) { - err := LoadRegistryConfiguration("../../config/example-config.yaml", false) + err := LoadRegistryConfiguration("../../config/example-config.yaml", true) require.NoError(t, err) - assert.Len(t, registries, 5) + assert.Len(t, registries, 4) reg, err := GetRegistryEndpoint("gcr.io") require.NoError(t, err) assert.Equal(t, "pullsecret:foo/bar", reg.Credentials) @@ -91,4 +93,18 @@ func Test_LoadRegistryConfiguration(t *testing.T) { require.NoError(t, err) assert.Equal(t, "", reg.Credentials) }) + + t.Run("Load from invalid location", func(t *testing.T) { + err := LoadRegistryConfiguration("../../test/testdata/registry/config/does-not-exist.yaml", true) + require.Error(t, err) + require.Contains(t, err.Error(), "no such file or directory") + }) + + t.Run("Two defaults defined in same config", func(t *testing.T) { + err := LoadRegistryConfiguration("../../test/testdata/registry/config/two-defaults.yaml", true) + require.Error(t, err) + require.Contains(t, err.Error(), "cannot set registry") + }) + + RestoreDefaultRegistryConfiguration() } diff --git a/pkg/registry/endpoints.go b/pkg/registry/endpoints.go index 04f8227f..4230b3d4 100644 --- a/pkg/registry/endpoints.go +++ b/pkg/registry/endpoints.go @@ -85,76 +85,50 @@ type RegistryEndpoint struct { TagListSort TagListSort Cache cache.ImageTagCache Limiter ratelimit.Limiter + IsDefault bool lock sync.RWMutex + limit int } -// Map of configured registries, pre-filled with some well-known registries -var defaultRegistries map[string]*RegistryEndpoint = map[string]*RegistryEndpoint{ - "": { +// registryTweaks should contain a list of registries whose settings cannot be +// infered by just looking at the image prefix. Prominent example here is the +// Docker Hub registry, which is refered to as docker.io from the image, but +// its API endpoint is https://registry-1.docker.io (and not https://docker.io) +var registryTweaks map[string]*RegistryEndpoint = map[string]*RegistryEndpoint{ + "docker.io": { RegistryName: "Docker Hub", - RegistryPrefix: "", + RegistryPrefix: "docker.io", RegistryAPI: "https://registry-1.docker.io", Ping: true, Insecure: false, DefaultNS: "library", Cache: cache.NewMemCache(), Limiter: ratelimit.New(RateLimitDefault), - }, - "gcr.io": { - RegistryName: "Google Container Registry", - RegistryPrefix: "gcr.io", - RegistryAPI: "https://gcr.io", - Ping: false, - Insecure: false, - Cache: cache.NewMemCache(), - Limiter: ratelimit.New(RateLimitDefault), - }, - "quay.io": { - RegistryName: "RedHat Quay", - RegistryPrefix: "quay.io", - RegistryAPI: "https://quay.io", - Ping: false, - Insecure: false, - Cache: cache.NewMemCache(), - Limiter: ratelimit.New(RateLimitDefault), - }, - "docker.pkg.github.com": { - RegistryName: "GitHub packages", - RegistryPrefix: "docker.pkg.github.com", - RegistryAPI: "https://docker.pkg.github.com", - Ping: false, - Insecure: false, - Cache: cache.NewMemCache(), - Limiter: ratelimit.New(RateLimitDefault), - }, - "ghcr.io": { - RegistryName: "GitHub Container Registry", - RegistryPrefix: "ghcr.io", - RegistryAPI: "https://ghcr.io", - Ping: false, - Insecure: false, - Cache: cache.NewMemCache(), - Limiter: ratelimit.New(RateLimitDefault), + IsDefault: true, }, } var registries map[string]*RegistryEndpoint = make(map[string]*RegistryEndpoint) +// Default registry points to the registry that is to be used as the default, +// e.g. when no registry prefix is given for a certain image. +var defaultRegistry *RegistryEndpoint + // Simple RW mutex for concurrent access to registries map var registryLock sync.RWMutex func AddRegistryEndpointFromConfig(epc RegistryConfiguration) error { - return AddRegistryEndpoint(epc.Prefix, epc.Name, epc.ApiURL, epc.Credentials, epc.DefaultNS, epc.Insecure, TagListSortFromString(epc.TagSortMode), epc.Limit, epc.CredsExpire) + ep := NewRegistryEndpoint(epc.Prefix, epc.Name, epc.ApiURL, epc.Credentials, epc.DefaultNS, epc.Insecure, TagListSortFromString(epc.TagSortMode), epc.Limit, epc.CredsExpire) + return AddRegistryEndpoint(ep) } -// AddRegistryEndpoint adds registry endpoint information with the given details -func AddRegistryEndpoint(prefix, name, apiUrl, credentials, defaultNS string, insecure bool, tagListSort TagListSort, limit int, credsExpire time.Duration) error { - registryLock.Lock() - defer registryLock.Unlock() +// NewRegistryEndpoint returns an endpoint object with the given configuration +// pre-populated and a fresh cache. +func NewRegistryEndpoint(prefix, name, apiUrl, credentials, defaultNS string, insecure bool, tagListSort TagListSort, limit int, credsExpire time.Duration) *RegistryEndpoint { if limit <= 0 { limit = RateLimitNone } - registries[prefix] = &RegistryEndpoint{ + ep := &RegistryEndpoint{ RegistryName: name, RegistryPrefix: prefix, RegistryAPI: strings.TrimSuffix(apiUrl, "/"), @@ -165,28 +139,97 @@ func AddRegistryEndpoint(prefix, name, apiUrl, credentials, defaultNS string, in DefaultNS: defaultNS, TagListSort: tagListSort, Limiter: ratelimit.New(limit), + limit: limit, } + return ep +} + +// AddRegistryEndpoint adds registry endpoint information with the given details +func AddRegistryEndpoint(ep *RegistryEndpoint) error { + prefix := ep.RegistryPrefix + + registryLock.Lock() + // If the endpoint is supposed to be the default endpoint, make sure that + // any previously set default endpoint is unset. + if ep.IsDefault { + if dep := GetDefaultRegistry(); dep != nil { + dep.IsDefault = false + } + SetDefaultRegistry(ep) + } + registries[prefix] = ep + registryLock.Unlock() logCtx := log.WithContext() - logCtx.AddField("registry", registries[prefix].RegistryAPI) - logCtx.AddField("prefix", registries[prefix].RegistryPrefix) - if limit != RateLimitNone { - logCtx.Debugf("setting rate limit to %d requests per second", limit) + logCtx.AddField("registry", ep.RegistryAPI) + logCtx.AddField("prefix", ep.RegistryPrefix) + if ep.limit != RateLimitNone { + logCtx.Debugf("setting rate limit to %d requests per second", ep.limit) } else { logCtx.Debugf("rate limiting is disabled") } return nil } +// inferRegistryEndpointFromPrefix returns a registry endpoint with the API +// URL infered from the prefix and adds it to the list of the configured +// registries. +func inferRegistryEndpointFromPrefix(prefix string) *RegistryEndpoint { + apiURL := "https://" + prefix + return NewRegistryEndpoint(prefix, prefix, apiURL, "", "", false, TagListSortUnsorted, 20, 0) +} + // GetRegistryEndpoint retrieves the endpoint information for the given prefix func GetRegistryEndpoint(prefix string) (*RegistryEndpoint, error) { + if prefix == "" { + if defaultRegistry == nil { + return nil, fmt.Errorf("no default endpoint configured") + } else { + return defaultRegistry, nil + } + } + registryLock.RLock() - defer registryLock.RUnlock() - if registry, ok := registries[prefix]; ok { + registry, ok := registries[prefix] + registryLock.RUnlock() + + if ok { return registry, nil } else { - return nil, fmt.Errorf("no registry with prefix '%s' configured", prefix) + var err error + ep := inferRegistryEndpointFromPrefix(prefix) + if ep != nil { + err = AddRegistryEndpoint(ep) + } else { + err = fmt.Errorf("could not infer registry configuration from prefix %s", prefix) + } + if err == nil { + log.Debugf("Infered registry from prefix %s to use API %s", prefix, ep.RegistryAPI) + } + return ep, err + } +} + +// SetDefaultRegistry sets a given registry endpoint as the default +func SetDefaultRegistry(ep *RegistryEndpoint) { + log.Debugf("Setting default registry endpoint to %s", ep.RegistryPrefix) + ep.IsDefault = true + if defaultRegistry != nil { + log.Debugf("Previous default registry was %s", defaultRegistry.RegistryPrefix) + defaultRegistry.IsDefault = false + } + defaultRegistry = ep +} + +// GetDefaultRegistry returns the registry endpoint that is set as default, +// or nil if no default registry endpoint is set +func GetDefaultRegistry() *RegistryEndpoint { + if defaultRegistry != nil { + log.Debugf("Getting default registry endpoint: %s", defaultRegistry.RegistryPrefix) + } else { + log.Debugf("No default registry defined.") } + return defaultRegistry } // SetRegistryEndpointCredentials allows to change the credentials used for @@ -229,6 +272,8 @@ func (ep *RegistryEndpoint) DeepCopy() *RegistryEndpoint { newEp.Limiter = ep.Limiter newEp.CredsExpire = ep.CredsExpire newEp.CredsUpdated = ep.CredsUpdated + newEp.IsDefault = ep.IsDefault + newEp.limit = ep.limit ep.lock.RUnlock() return newEp } @@ -245,8 +290,16 @@ func (ep *RegistryEndpoint) GetTransport() *http.Transport { } } +// init initializes the registry configuration func init() { - for k, v := range defaultRegistries { + for k, v := range registryTweaks { registries[k] = v.DeepCopy() + if v.IsDefault { + if defaultRegistry == nil { + defaultRegistry = v + } else { + panic("only one default registry can be configured") + } + } } } diff --git a/pkg/registry/endpoints_test.go b/pkg/registry/endpoints_test.go index 8cc61245..9a309b6f 100644 --- a/pkg/registry/endpoints_test.go +++ b/pkg/registry/endpoints_test.go @@ -2,6 +2,7 @@ package registry import ( "fmt" + "sync" "testing" "github.com/stretchr/testify/assert" @@ -9,12 +10,15 @@ import ( ) func Test_GetEndpoints(t *testing.T) { + RestoreDefaultRegistryConfiguration() + t.Run("Get default endpoint", func(t *testing.T) { ep, err := GetRegistryEndpoint("") require.NoError(t, err) require.NotNil(t, ep) - assert.Equal(t, ep.RegistryPrefix, "") + assert.Equal(t, "docker.io", ep.RegistryPrefix) }) + t.Run("Get GCR endpoint", func(t *testing.T) { ep, err := GetRegistryEndpoint("gcr.io") require.NoError(t, err) @@ -22,16 +26,20 @@ func Test_GetEndpoints(t *testing.T) { assert.Equal(t, ep.RegistryPrefix, "gcr.io") }) - t.Run("Get non-existing endpoint", func(t *testing.T) { + t.Run("Infer endpoint", func(t *testing.T) { ep, err := GetRegistryEndpoint("foobar.com") - assert.Error(t, err) - assert.Nil(t, ep) + require.NoError(t, err) + require.NotNil(t, ep) + assert.Equal(t, "foobar.com", ep.RegistryPrefix) + assert.Equal(t, "https://foobar.com", ep.RegistryAPI) }) } func Test_AddEndpoint(t *testing.T) { + RestoreDefaultRegistryConfiguration() + t.Run("Add new endpoint", func(t *testing.T) { - err := AddRegistryEndpoint("example.com", "Example", "https://example.com", "", "", false, TagListSortUnsorted, 5, 0) + err := AddRegistryEndpoint(NewRegistryEndpoint("example.com", "Example", "https://example.com", "", "", false, TagListSortUnsorted, 5, 0)) require.NoError(t, err) }) t.Run("Get example.com endpoint", func(t *testing.T) { @@ -46,7 +54,7 @@ func Test_AddEndpoint(t *testing.T) { assert.Equal(t, ep.TagListSort, TagListSortUnsorted) }) t.Run("Change existing endpoint", func(t *testing.T) { - err := AddRegistryEndpoint("example.com", "Example", "https://example.com", "", "library", true, TagListSortLatestFirst, 5, 0) + err := AddRegistryEndpoint(NewRegistryEndpoint("example.com", "Example", "https://example.com", "", "library", true, TagListSortLatestFirst, 5, 0)) require.NoError(t, err) ep, err := GetRegistryEndpoint("example.com") require.NoError(t, err) @@ -58,6 +66,8 @@ func Test_AddEndpoint(t *testing.T) { } func Test_SetEndpointCredentials(t *testing.T) { + RestoreDefaultRegistryConfiguration() + t.Run("Set credentials on default registry", func(t *testing.T) { err := SetRegistryEndpointCredentials("", "env:FOOBAR") require.NoError(t, err) @@ -78,19 +88,28 @@ func Test_SetEndpointCredentials(t *testing.T) { } func Test_EndpointConcurrentAccess(t *testing.T) { + RestoreDefaultRegistryConfiguration() + const numRuns = 50 // Make sure we're not deadlocking on read t.Run("Concurrent read access", func(t *testing.T) { - for i := 0; i < 50; i++ { + var wg sync.WaitGroup + wg.Add(numRuns) + for i := 0; i < numRuns; i++ { go func() { ep, err := GetRegistryEndpoint("gcr.io") require.NoError(t, err) require.NotNil(t, ep) + wg.Done() }() } + wg.Wait() }) + // Make sure we're not deadlocking on write t.Run("Concurrent write access", func(t *testing.T) { - for i := 0; i < 50; i++ { + var wg sync.WaitGroup + wg.Add(numRuns) + for i := 0; i < numRuns; i++ { go func(i int) { creds := fmt.Sprintf("secret:foo/secret-%d", i) err := SetRegistryEndpointCredentials("", creds) @@ -98,11 +117,33 @@ func Test_EndpointConcurrentAccess(t *testing.T) { ep, err := GetRegistryEndpoint("") require.NoError(t, err) require.NotNil(t, ep) + wg.Done() }(i) } + wg.Wait() }) } +func Test_SetDefault(t *testing.T) { + RestoreDefaultRegistryConfiguration() + + dep := GetDefaultRegistry() + require.NotNil(t, dep) + assert.Equal(t, "docker.io", dep.RegistryPrefix) + assert.True(t, dep.IsDefault) + + ep, err := GetRegistryEndpoint("ghcr.io") + require.NoError(t, err) + require.NotNil(t, ep) + require.False(t, ep.IsDefault) + + SetDefaultRegistry(ep) + assert.True(t, ep.IsDefault) + assert.False(t, dep.IsDefault) + require.NotNil(t, GetDefaultRegistry()) + assert.Equal(t, ep.RegistryPrefix, GetDefaultRegistry().RegistryPrefix) +} + func Test_DeepCopy(t *testing.T) { t.Run("DeepCopy endpoint object", func(t *testing.T) { ep, err := GetRegistryEndpoint("docker.pkg.github.com") diff --git a/test/testdata/registry/config/two-defaults.yaml b/test/testdata/registry/config/two-defaults.yaml new file mode 100644 index 00000000..21254cde --- /dev/null +++ b/test/testdata/registry/config/two-defaults.yaml @@ -0,0 +1,9 @@ +registries: +- name: "Reg One" + prefix: regone.io + api_url: "https://regone.io" + default: true +- name: "Reg Two" + prefix: regtwo.io + api_url: "https://regtwo.io" + default: true \ No newline at end of file