diff --git a/api/exec/common.go b/api/exec/common.go index 5d2b96078f..593b2042cb 100644 --- a/api/exec/common.go +++ b/api/exec/common.go @@ -38,7 +38,7 @@ func fetchEndpointProxy(proxyManager *proxy.Manager, endpoint *portainer.Endpoin // portainerRegistriesToAuthConfigs converts registries to Docker auth configs. // Callers must ensure ECR tokens are valid before calling this function (e.g. via -// registryutils.ValidateRegistriesECRTokens with a real DataStoreTx). This function +// registryutils.RefreshAndPersistECRTokens with a real DataStoreTx). This function // intentionally performs no DB writes to avoid write-lock contention when called inside // an active BoltDB write transaction. func portainerRegistriesToAuthConfigs(registries []portainer.Registry) []types.AuthConfig { diff --git a/api/internal/registryutils/ecr_reg_token.go b/api/internal/registryutils/ecr_reg_token.go index 107204d293..a4387a8e79 100644 --- a/api/internal/registryutils/ecr_reg_token.go +++ b/api/internal/registryutils/ecr_reg_token.go @@ -38,9 +38,9 @@ func doGetRegToken(tx dataservices.DataStoreTx, registry *portainer.Registry) er return tx.Registry().Update(registry.ID, registry) } -// ValidateRegistriesECRTokens refreshes and persists ECR tokens for all registries that need it. +// RefreshAndPersistECRTokens refreshes and persists ECR tokens for all registries that need it. // Must be called with a real DataStoreTx (not a top-level DataStore) to avoid write-lock contention. -func ValidateRegistriesECRTokens(tx dataservices.DataStoreTx, registries []portainer.Registry) error { +func RefreshAndPersistECRTokens(tx dataservices.DataStoreTx, registries []portainer.Registry) { for i := range registries { reg := ®istries[i] if reg.Type != portainer.EcrRegistry { @@ -50,11 +50,12 @@ func ValidateRegistriesECRTokens(tx dataservices.DataStoreTx, registries []porta continue } if err := doGetRegToken(tx, reg); err != nil { - return fmt.Errorf("ECR registry %q credentials are invalid or expired. Error: %w", reg.Name, err) + log.Warn(). + Err(err). + Str("RegistryName", reg.Name). + Msg("Failed to get valid ECR registry token. Skip logging with this registry.") } } - - return nil } func EnsureRegTokenValid(tx dataservices.DataStoreTx, registry *portainer.Registry) error { diff --git a/api/internal/registryutils/ecr_reg_token_test.go b/api/internal/registryutils/ecr_reg_token_test.go index 70a8ce1b8e..2c9b9a3030 100644 --- a/api/internal/registryutils/ecr_reg_token_test.go +++ b/api/internal/registryutils/ecr_reg_token_test.go @@ -1,6 +1,8 @@ package registryutils_test import ( + "io" + "strings" "testing" "time" @@ -8,6 +10,7 @@ import ( "github.com/portainer/portainer/api/dataservices" "github.com/portainer/portainer/api/datastore" "github.com/portainer/portainer/api/internal/registryutils" + zerolog "github.com/rs/zerolog/log" "github.com/stretchr/testify/require" ) @@ -24,75 +27,40 @@ func newECRRegistry(id portainer.RegistryID, accessToken string, expiry int64) p } } -func TestValidateRegistriesECRTokens(t *testing.T) { - t.Parallel() - - t.Run("skips non-ECR registries without error", func(t *testing.T) { - t.Parallel() - _, ds := datastore.MustNewTestStore(t, true, false) - registries := []portainer.Registry{ - {ID: 1, Type: portainer.DockerHubRegistry, Name: "dockerhub"}, - {ID: 2, Type: portainer.CustomRegistry, Name: "custom"}, - } - require.NoError(t, ds.UpdateTx(func(tx dataservices.DataStoreTx) error { - return registryutils.ValidateRegistriesECRTokens(tx, registries) - })) - }) - - t.Run("skips ECR registries with valid tokens", func(t *testing.T) { - t.Parallel() +func TestRefreshAndPersistECRTokens(t *testing.T) { + t.Run("does not modify token for ECR registry with valid token", func(t *testing.T) { _, ds := datastore.MustNewTestStore(t, true, false) reg := newECRRegistry(1, "valid-token", time.Now().Add(time.Hour).Unix()) require.NoError(t, ds.UpdateTx(func(tx dataservices.DataStoreTx) error { - return registryutils.ValidateRegistriesECRTokens(tx, []portainer.Registry{reg}) + registryutils.RefreshAndPersistECRTokens(tx, []portainer.Registry{reg}) + return nil })) + require.Equal(t, "valid-token", reg.AccessToken) }) - t.Run("returns nil for empty registry list", func(t *testing.T) { - t.Parallel() - _, ds := datastore.MustNewTestStore(t, true, false) - require.NoError(t, ds.UpdateTx(func(tx dataservices.DataStoreTx) error { - return registryutils.ValidateRegistriesECRTokens(tx, []portainer.Registry{}) - })) - }) + t.Run("does not block and leaves token empty when ECR token refresh fails", func(t *testing.T) { + var logOutput strings.Builder + setupLogOutput(t, &logOutput) - t.Run("returns error for ECR registry with missing token", func(t *testing.T) { - t.Parallel() _, ds := datastore.MustNewTestStore(t, true, false) reg := newECRRegistry(1, "", 0) require.NoError(t, ds.UpdateTx(func(tx dataservices.DataStoreTx) error { - return tx.Registry().Create(®) - })) - - var validateErr error - _ = ds.UpdateTx(func(tx dataservices.DataStoreTx) error { - validateErr = registryutils.ValidateRegistriesECRTokens(tx, []portainer.Registry{reg}) + registryutils.RefreshAndPersistECRTokens(tx, []portainer.Registry{reg}) return nil - }) - require.Error(t, validateErr) - require.Contains(t, validateErr.Error(), "test-ecr") + })) + require.Empty(t, reg.AccessToken) + require.Contains(t, logOutput.String(), "test-ecr") + require.Contains(t, logOutput.String(), "Failed to get valid ECR registry token") }) +} - t.Run("stops on first invalid ECR registry and includes its name in error", func(t *testing.T) { - t.Parallel() - _, ds := datastore.MustNewTestStore(t, true, false) +func setupLogOutput(t *testing.T, w io.Writer) { + t.Helper() - validECR := newECRRegistry(1, "valid-token", time.Now().Add(time.Hour).Unix()) - invalidECR := newECRRegistry(2, "", 0) - invalidECR.Name = "invalid-ecr" - nonECR := portainer.Registry{ID: 3, Type: portainer.DockerHubRegistry} - - require.NoError(t, ds.UpdateTx(func(tx dataservices.DataStoreTx) error { - return tx.Registry().Create(&invalidECR) - })) - - var validateErr error - _ = ds.UpdateTx(func(tx dataservices.DataStoreTx) error { - validateErr = registryutils.ValidateRegistriesECRTokens(tx, []portainer.Registry{validECR, invalidECR, nonECR}) - return nil - }) - require.Error(t, validateErr) - require.Contains(t, validateErr.Error(), "invalid-ecr") + oldLogger := zerolog.Logger + zerolog.Logger = zerolog.Output(w) + t.Cleanup(func() { + zerolog.Logger = oldLogger }) } diff --git a/api/stacks/deployments/deployment_compose_config.go b/api/stacks/deployments/deployment_compose_config.go index 14f6b000ff..85f5e163c5 100644 --- a/api/stacks/deployments/deployment_compose_config.go +++ b/api/stacks/deployments/deployment_compose_config.go @@ -40,9 +40,7 @@ func CreateComposeStackDeploymentConfigTx(tx dataservices.DataStoreTx, securityC filteredRegistries := security.FilterRegistries(registries, user, securityContext.UserMemberships, endpoint.ID) - if err := registryutils.ValidateRegistriesECRTokens(tx, filteredRegistries); err != nil { - return nil, err - } + registryutils.RefreshAndPersistECRTokens(tx, filteredRegistries) config := &ComposeStackDeploymentConfig{ stack: stack, diff --git a/api/stacks/deployments/deployment_swarm_config.go b/api/stacks/deployments/deployment_swarm_config.go index 8f70d0e2a9..d99dc83d5d 100644 --- a/api/stacks/deployments/deployment_swarm_config.go +++ b/api/stacks/deployments/deployment_swarm_config.go @@ -38,9 +38,7 @@ func CreateSwarmStackDeploymentConfigTx(tx dataservices.DataStoreTx, securityCon filteredRegistries := security.FilterRegistries(registries, user, securityContext.UserMemberships, endpoint.ID) - if err := registryutils.ValidateRegistriesECRTokens(tx, filteredRegistries); err != nil { - return nil, err - } + registryutils.RefreshAndPersistECRTokens(tx, filteredRegistries) config := &SwarmStackDeploymentConfig{ stack: stack,