fix(registry): suppress ecr token pre-validation error with warning log [BE-13059] (#2827)

This commit is contained in:
Oscar Zhou
2026-06-09 10:58:50 +12:00
committed by GitHub
parent df7a4b5d6f
commit 5cdd0023d7
5 changed files with 32 additions and 67 deletions
+1 -1
View File
@@ -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 {
+6 -5
View File
@@ -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 := &registries[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 {
@@ -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(&reg)
}))
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
})
}
@@ -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,
@@ -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,