From 98a3f2b72130106edc8c8bc9cc6dfce54510042d Mon Sep 17 00:00:00 2001 From: Julian Sommer <115516918+juliansommer@users.noreply.github.com> Date: Wed, 7 Oct 2026 12:10:08 +1100 Subject: [PATCH] fix: answer ECR validation refusals with InvalidParameterException (#1184) --- spinifex/awserrors/awserrors.go | 3 +++ spinifex/gateway/ecr/registry_test.go | 7 +++++ spinifex/gateway/ecr_authz_test.go | 2 +- .../ecr_createdeleterepository_test.go | 16 ++++++------ spinifex/gateway/ecr_createrepository.go | 4 +-- spinifex/gateway/ecr_image_test.go | 6 ++--- spinifex/gateway/ecr_lifecyclepreview_test.go | 2 +- spinifex/gateway/ecr_tagmutability_test.go | 10 +++---- spinifex/gateway/ecrapi/authz.go | 2 +- spinifex/gateway/ecrapi/authz_test.go | 2 +- spinifex/gateway/ecrapi/imagescanning_test.go | 4 +-- spinifex/gateway/ecrapi/lifecyclepolicy.go | 2 +- .../gateway/ecrapi/lifecyclepolicy_test.go | 6 ++--- .../gateway/ecrapi/repositorypolicy_test.go | 4 +-- spinifex/gateway/ecrapi/tags_test.go | 8 +++--- spinifex/gateway/ecrapi/validation.go | 26 +++++++------------ spinifex/gateway/ecrapi/validation_test.go | 12 ++++++--- spinifex/handlers/ecr/lifecycle.go | 2 +- spinifex/handlers/ecr/meta_test.go | 4 +-- spinifex/handlers/ecr/storage.go | 11 +++++--- .../integration/conformance-known-findings.md | 2 +- tests/integration/ecr_test.go | 9 +++++++ 22 files changed, 83 insertions(+), 61 deletions(-) diff --git a/spinifex/awserrors/awserrors.go b/spinifex/awserrors/awserrors.go index e529983aa..cd9aeafdd 100644 --- a/spinifex/awserrors/awserrors.go +++ b/spinifex/awserrors/awserrors.go @@ -388,6 +388,9 @@ var ( // real ECS both use the restJson1 wire code "InvalidParameterException", // so this gives EKS call sites their own name for the same registered code. ErrorEKSInvalidParameter = ErrorECSInvalidParameter + // ErrorECRInvalidParameter aliases ErrorECSInvalidParameter: every ECR + // operation declares the AWS JSON 1.1 wire code "InvalidParameterException". + ErrorECRInvalidParameter = ErrorECSInvalidParameter // ErrorACMResourceInUse aliases ErrorEKSResourceInUse: real ACM's // DeleteCertificate and real EKS's CreateCluster both legitimately use the // wire code "ResourceInUseException", so this is a distinct, self-documenting diff --git a/spinifex/gateway/ecr/registry_test.go b/spinifex/gateway/ecr/registry_test.go index 999560f5e..267733b0a 100644 --- a/spinifex/gateway/ecr/registry_test.go +++ b/spinifex/gateway/ecr/registry_test.go @@ -460,6 +460,13 @@ func TestDispatch_InvalidRepoName(t *testing.T) { assertCode(t, w, "NAME_INVALID") } +func TestDispatch_RepeatedSeparatorRepoName(t *testing.T) { + reg := newTestRegistry() + dg := pushBlob(t, reg, "team/a__b--c", []byte("layer")) + w := do(reg, http.MethodHead, "/v2/team/a__b--c/blobs/"+dg, nil, nil) + assert.Equal(t, http.StatusOK, w.Code) +} + func TestDispatch_UnknownPath(t *testing.T) { reg := newTestRegistry() w := do(reg, http.MethodGet, "/v2/team/app/frobnicate", nil, nil) diff --git a/spinifex/gateway/ecr_authz_test.go b/spinifex/gateway/ecr_authz_test.go index 9d3cdf30b..5fc095759 100644 --- a/spinifex/gateway/ecr_authz_test.go +++ b/spinifex/gateway/ecr_authz_test.go @@ -66,7 +66,7 @@ func TestECRRequest_FieldSpelledTwoWaysIsRejected(t *testing.T) { err := dispatchECR(t, gw, "BatchCheckLayerAvailability", `{"repositoryName":"dev","RepositoryName":"prod"}`) require.Error(t, err) - assert.Equal(t, awserrors.ErrorInvalidParameterValue, awserrors.ValidErrorCodeFromError(err)) + assert.Equal(t, awserrors.ErrorECRInvalidParameter, awserrors.ValidErrorCodeFromError(err)) } } diff --git a/spinifex/gateway/ecr_createdeleterepository_test.go b/spinifex/gateway/ecr_createdeleterepository_test.go index 85ff04813..7314cee77 100644 --- a/spinifex/gateway/ecr_createdeleterepository_test.go +++ b/spinifex/gateway/ecr_createdeleterepository_test.go @@ -110,7 +110,7 @@ func TestCreateRepository_Tags(t *testing.T) { _, err = createRepo(t, gw, `{"repositoryName":"team/badtag","tags":[{"Key":"","Value":"x"}]}`) require.Error(t, err) - assert.Equal(t, "InvalidParameterValue", awserrors.ValidErrorCodeFromError(err)) + assert.Equal(t, "InvalidParameterException", awserrors.ValidErrorCodeFromError(err)) } func TestCreateRepository_EncryptionAndScanningConfiguration(t *testing.T) { @@ -135,12 +135,12 @@ func TestCreateRepository_EncryptionAndScanningConfiguration(t *testing.T) { require.Error(t, err) code, message, ok := awserrors.ResolveErrorDetail(err) require.True(t, ok) - assert.Equal(t, "InvalidParameterValue", code) + assert.Equal(t, "InvalidParameterException", code) assert.NotEmpty(t, message) _, err = createRepo(t, gw, `{"repositoryName":"team/bad","encryptionConfiguration":{"encryptionType":"bogus"}}`) require.Error(t, err) - assert.Equal(t, "InvalidParameterValue", awserrors.ValidErrorCodeFromError(err)) + assert.Equal(t, "InvalidParameterException", awserrors.ValidErrorCodeFromError(err)) } func TestCreateRepository_Errors(t *testing.T) { @@ -152,8 +152,8 @@ func TestCreateRepository_Errors(t *testing.T) { name, body, expect string }{ {"already exists", `{"repositoryName":"team/app"}`, "RepositoryAlreadyExistsException"}, - {"invalid name", `{"repositoryName":"Team/App"}`, "InvalidParameterValue"}, - {"empty name", `{}`, "InvalidParameterValue"}, + {"invalid name", `{"repositoryName":"Team/App"}`, "InvalidParameterException"}, + {"empty name", `{}`, "InvalidParameterException"}, {"cross-account", `{"repositoryName":"team/x","registryId":"999999999999"}`, "AccessDenied"}, } for _, tc := range cases { @@ -172,7 +172,7 @@ func TestCreateRepository_BadNameCarriesAWSMessage(t *testing.T) { require.Error(t, err) code, message, ok := awserrors.ResolveErrorDetail(err) require.True(t, ok) - assert.Equal(t, awserrors.ErrorInvalidParameterValue, code) + assert.Equal(t, awserrors.ErrorECRInvalidParameter, code) assert.Equal(t, `Invalid parameter at 'repositoryName' failed to satisfy constraint: 'must satisfy regular expression '[a-z0-9]+((\.|_|__|-+)[a-z0-9]+)*(/[a-z0-9]+((\.|_|__|-+)[a-z0-9]+)*)*''`, message) } @@ -191,7 +191,7 @@ func TestCreateRepository_NoAccountAndMalformed(t *testing.T) { _, err = createRepo(t, gw, `{`) require.Error(t, err) - assert.Equal(t, "InvalidParameterValue", awserrors.ValidErrorCodeFromError(err)) + assert.Equal(t, "InvalidParameterException", awserrors.ValidErrorCodeFromError(err)) } func TestDeleteRepository_NoAccountAndMalformed(t *testing.T) { @@ -203,7 +203,7 @@ func TestDeleteRepository_NoAccountAndMalformed(t *testing.T) { _, err = deleteRepo(t, gw, `{`) require.Error(t, err) - assert.Equal(t, "InvalidParameterValue", awserrors.ValidErrorCodeFromError(err)) + assert.Equal(t, "InvalidParameterException", awserrors.ValidErrorCodeFromError(err)) } func TestDeleteRepository_Happy(t *testing.T) { diff --git a/spinifex/gateway/ecr_createrepository.go b/spinifex/gateway/ecr_createrepository.go index 313b532c4..a3959e967 100644 --- a/spinifex/gateway/ecr_createrepository.go +++ b/spinifex/gateway/ecr_createrepository.go @@ -136,7 +136,7 @@ func normalizeEncryptionType(cfg *encryptionConfigurationInput) (string, error) case handlers_ecr.EncryptionTypeAES256: return cfg.EncryptionType, nil case handlers_ecr.EncryptionTypeKMS: - return "", awserrors.Errorf(awserrors.ErrorInvalidParameterValue, + return "", awserrors.Errorf(awserrors.ErrorECRInvalidParameter, "encryptionType KMS is not supported: no customer-managed key is used, and repositories are already encrypted at rest under a server-managed AES-256 key") default: return "", gateway_ecrapi.EnumValueError("encryptionConfiguration.encryptionType", cfg.EncryptionType, @@ -145,7 +145,7 @@ func normalizeEncryptionType(cfg *encryptionConfigurationInput) (string, error) } // normalizeTagMutability validates the requested mutability, defaulting an empty -// value to MUTABLE. An unknown value is rejected with InvalidParameterValue. +// value to MUTABLE. An unknown value is rejected with InvalidParameterException. func normalizeTagMutability(v string) (string, error) { switch v { case "": diff --git a/spinifex/gateway/ecr_image_test.go b/spinifex/gateway/ecr_image_test.go index 2b5be3017..05d3d9fac 100644 --- a/spinifex/gateway/ecr_image_test.go +++ b/spinifex/gateway/ecr_image_test.go @@ -147,7 +147,7 @@ func TestBatchGetImage_CapExceeded(t *testing.T) { body := `{"repositoryName":"team/app","imageIds":[` + strings.Join(ids, ",") + `]}` _, err := callImage(t, gw, (*GatewayConfig).handleBatchGetImage, body) require.Error(t, err) - assert.Equal(t, "InvalidParameterValue", awserrors.ValidErrorCodeFromError(err)) + assert.Equal(t, "InvalidParameterException", awserrors.ValidErrorCodeFromError(err)) } func TestPutImage_HappyAndMissingManifest(t *testing.T) { @@ -175,10 +175,10 @@ func TestPutImage_HappyAndMissingManifest(t *testing.T) { require.NoError(t, json.Unmarshal(w.Body.Bytes(), &list)) assert.Len(t, list.ImageIds, 1) - // Missing manifest -> InvalidParameterValue. + // Missing manifest -> InvalidParameterException. _, err = callImage(t, gw, (*GatewayConfig).handlePutImage, `{"repositoryName":"team/app","imageTag":"v2"}`) require.Error(t, err) - assert.Equal(t, "InvalidParameterValue", awserrors.ValidErrorCodeFromError(err)) + assert.Equal(t, "InvalidParameterException", awserrors.ValidErrorCodeFromError(err)) } // TestPutImage_RepoNotCreated asserts the JSON PutImage path rejects an uncreated diff --git a/spinifex/gateway/ecr_lifecyclepreview_test.go b/spinifex/gateway/ecr_lifecyclepreview_test.go index dddc125a6..d541dd246 100644 --- a/spinifex/gateway/ecr_lifecyclepreview_test.go +++ b/spinifex/gateway/ecr_lifecyclepreview_test.go @@ -81,7 +81,7 @@ func TestLifecyclePreview_Errors(t *testing.T) { // Malformed override policy. _, err = callImage(t, gw, (*GatewayConfig).handleGetLifecyclePolicyPreview, previewBody("team/app", "not-json")) require.Error(t, err) - assert.Equal(t, "InvalidParameterValue", awserrors.ValidErrorCodeFromError(err)) + assert.Equal(t, "InvalidParameterException", awserrors.ValidErrorCodeFromError(err)) // Cross-account. _, err = callImage(t, gw, (*GatewayConfig).handleStartLifecyclePolicyPreview, `{"repositoryName":"team/app","registryId":"999999999999","lifecyclePolicyText":`+strconvQuotePreview(previewExpireOldest)+`}`) diff --git a/spinifex/gateway/ecr_tagmutability_test.go b/spinifex/gateway/ecr_tagmutability_test.go index 4bfa2cc66..e803e1f3c 100644 --- a/spinifex/gateway/ecr_tagmutability_test.go +++ b/spinifex/gateway/ecr_tagmutability_test.go @@ -44,7 +44,7 @@ func TestCreateRepository_InvalidMutability(t *testing.T) { gw, _ := newRepoLifecycleGateway(t) _, err := createRepo(t, gw, `{"repositoryName":"team/app","imageTagMutability":"SOMETIMES"}`) require.Error(t, err) - assert.Equal(t, "InvalidParameterValue", awserrors.ValidErrorCodeFromError(err)) + assert.Equal(t, "InvalidParameterException", awserrors.ValidErrorCodeFromError(err)) } func TestPutImageTagMutability_Happy(t *testing.T) { @@ -82,9 +82,9 @@ func TestPutImageTagMutability_Errors(t *testing.T) { cases := []struct { name, body, expect string }{ - {"missing value", `{"repositoryName":"team/app"}`, "InvalidParameterValue"}, - {"invalid value", `{"repositoryName":"team/app","imageTagMutability":"NOPE"}`, "InvalidParameterValue"}, - {"invalid name", `{"repositoryName":"Team/App","imageTagMutability":"IMMUTABLE"}`, "InvalidParameterValue"}, + {"missing value", `{"repositoryName":"team/app"}`, "InvalidParameterException"}, + {"invalid value", `{"repositoryName":"team/app","imageTagMutability":"NOPE"}`, "InvalidParameterException"}, + {"invalid name", `{"repositoryName":"Team/App","imageTagMutability":"IMMUTABLE"}`, "InvalidParameterException"}, {"cross-account", `{"repositoryName":"team/app","registryId":"999999999999","imageTagMutability":"IMMUTABLE"}`, "AccessDenied"}, {"not found", `{"repositoryName":"team/ghost","imageTagMutability":"IMMUTABLE"}`, "RepositoryNotFoundException"}, } @@ -106,7 +106,7 @@ func TestPutImageTagMutability_NoAccountAndMalformed(t *testing.T) { _, err = putTagMutability(t, gw, `{`) require.Error(t, err) - assert.Equal(t, "InvalidParameterValue", awserrors.ValidErrorCodeFromError(err)) + assert.Equal(t, "InvalidParameterException", awserrors.ValidErrorCodeFromError(err)) } func TestECRRequest_PutImageTagMutabilityDispatched(t *testing.T) { diff --git a/spinifex/gateway/ecrapi/authz.go b/spinifex/gateway/ecrapi/authz.go index 7930d60fb..496a72c22 100644 --- a/spinifex/gateway/ecrapi/authz.go +++ b/spinifex/gateway/ecrapi/authz.go @@ -124,7 +124,7 @@ func ResourceARNs(action, region, accountID string, body []byte) ([]string, erro // Parse fails only on a field spelled two ways, which it cannot scope. scope, err := bodyscope.Parse(action, body) if err != nil { - return nil, awserrors.Errorf(awserrors.ErrorInvalidParameterValue, + return nil, awserrors.Errorf(awserrors.ErrorECRInvalidParameter, "The request body names the same field more than once in different letter case") } diff --git a/spinifex/gateway/ecrapi/authz_test.go b/spinifex/gateway/ecrapi/authz_test.go index d6e9b3a70..7f236299e 100644 --- a/spinifex/gateway/ecrapi/authz_test.go +++ b/spinifex/gateway/ecrapi/authz_test.go @@ -129,6 +129,6 @@ func TestResourceARNs_FieldSpelledTwoWaysIsRejected(t *testing.T) { for range 50 { _, err := gateway_ecrapi.ResourceARNs("DeleteRepository", testRegion, testAccountID, []byte(`{"repositoryName":"dev","RepositoryName":"prod"}`)) - assert.Equal(t, awserrors.ErrorInvalidParameterValue, awserrors.ValidErrorCodeFromError(err)) + assert.Equal(t, awserrors.ErrorECRInvalidParameter, awserrors.ValidErrorCodeFromError(err)) } } diff --git a/spinifex/gateway/ecrapi/imagescanning_test.go b/spinifex/gateway/ecrapi/imagescanning_test.go index a07acac4b..6e3cf0907 100644 --- a/spinifex/gateway/ecrapi/imagescanning_test.go +++ b/spinifex/gateway/ecrapi/imagescanning_test.go @@ -63,8 +63,8 @@ func TestPutImageScanningConfiguration_Errors(t *testing.T) { name, body, expect string }{ {"missing repo", `{"repositoryName":"team/ghost"}`, awserrors.ErrorRepositoryNotFound}, - {"empty name", `{}`, awserrors.ErrorInvalidParameterValue}, - {"invalid name", `{"repositoryName":"Team/App"}`, awserrors.ErrorInvalidParameterValue}, + {"empty name", `{}`, awserrors.ErrorECRInvalidParameter}, + {"invalid name", `{"repositoryName":"Team/App"}`, awserrors.ErrorECRInvalidParameter}, {"cross-account", `{"repositoryName":"team/app","registryId":"999999999999"}`, awserrors.ErrorAccessDenied}, } for _, tc := range cases { diff --git a/spinifex/gateway/ecrapi/lifecyclepolicy.go b/spinifex/gateway/ecrapi/lifecyclepolicy.go index 4424bd341..cbe58ea0e 100644 --- a/spinifex/gateway/ecrapi/lifecyclepolicy.go +++ b/spinifex/gateway/ecrapi/lifecyclepolicy.go @@ -49,7 +49,7 @@ func resolveLifecycleRepo(ctx context.Context, nc *nats.Conn, accountID string, // PutLifecyclePolicy validates and stores the lifecycle-policy document for a // repository. The document is parsed by the evaluation engine; a malformed or -// unsupported rule is rejected with InvalidParameterValue. +// unsupported rule is rejected with InvalidParameterException. func PutLifecyclePolicy(ctx context.Context, nc *nats.Conn, accountID string, body []byte) (any, error) { req, store, err := resolveLifecycleRepo(ctx, nc, accountID, body) if err != nil { diff --git a/spinifex/gateway/ecrapi/lifecyclepolicy_test.go b/spinifex/gateway/ecrapi/lifecyclepolicy_test.go index 130a508c8..8e44141bb 100644 --- a/spinifex/gateway/ecrapi/lifecyclepolicy_test.go +++ b/spinifex/gateway/ecrapi/lifecyclepolicy_test.go @@ -72,9 +72,9 @@ func TestLifecyclePolicy_Errors(t *testing.T) { expect string }{ {"put missing repo", PutLifecyclePolicy, `{"repositoryName":"team/ghost","lifecyclePolicyText":` + strconvQuote(validLifecyclePolicy) + `}`, awserrors.ErrorRepositoryNotFound}, - {"put invalid json", PutLifecyclePolicy, `{"repositoryName":"team/app","lifecyclePolicyText":"not-json"}`, awserrors.ErrorInvalidParameterValue}, - {"put bad rule", PutLifecyclePolicy, `{"repositoryName":"team/app","lifecyclePolicyText":"{\"rules\":[{\"rulePriority\":1,\"selection\":{\"tagStatus\":\"tagged\",\"countType\":\"imageCountMoreThan\",\"countNumber\":1},\"action\":{\"type\":\"expire\"}}]}"}`, awserrors.ErrorInvalidParameterValue}, - {"put empty name", PutLifecyclePolicy, `{"lifecyclePolicyText":` + strconvQuote(validLifecyclePolicy) + `}`, awserrors.ErrorInvalidParameterValue}, + {"put invalid json", PutLifecyclePolicy, `{"repositoryName":"team/app","lifecyclePolicyText":"not-json"}`, awserrors.ErrorECRInvalidParameter}, + {"put bad rule", PutLifecyclePolicy, `{"repositoryName":"team/app","lifecyclePolicyText":"{\"rules\":[{\"rulePriority\":1,\"selection\":{\"tagStatus\":\"tagged\",\"countType\":\"imageCountMoreThan\",\"countNumber\":1},\"action\":{\"type\":\"expire\"}}]}"}`, awserrors.ErrorECRInvalidParameter}, + {"put empty name", PutLifecyclePolicy, `{"lifecyclePolicyText":` + strconvQuote(validLifecyclePolicy) + `}`, awserrors.ErrorECRInvalidParameter}, {"put cross-account", PutLifecyclePolicy, `{"repositoryName":"team/app","registryId":"999999999999","lifecyclePolicyText":` + strconvQuote(validLifecyclePolicy) + `}`, awserrors.ErrorAccessDenied}, {"get no policy", GetLifecyclePolicy, `{"repositoryName":"team/app"}`, awserrors.ErrorLifecyclePolicyNotFound}, {"delete no policy", DeleteLifecyclePolicy, `{"repositoryName":"team/app"}`, awserrors.ErrorLifecyclePolicyNotFound}, diff --git a/spinifex/gateway/ecrapi/repositorypolicy_test.go b/spinifex/gateway/ecrapi/repositorypolicy_test.go index ed93e687a..9df863c2a 100644 --- a/spinifex/gateway/ecrapi/repositorypolicy_test.go +++ b/spinifex/gateway/ecrapi/repositorypolicy_test.go @@ -105,8 +105,8 @@ func TestRepositoryPolicy_Errors(t *testing.T) { expect string }{ {"set missing repo", SetRepositoryPolicy, `{"repositoryName":"team/ghost","policyText":"{}"}`, awserrors.ErrorRepositoryNotFound}, - {"set invalid json policy", SetRepositoryPolicy, `{"repositoryName":"team/app","policyText":"not-json"}`, awserrors.ErrorInvalidParameterValue}, - {"set empty name", SetRepositoryPolicy, `{"policyText":"{}"}`, awserrors.ErrorInvalidParameterValue}, + {"set invalid json policy", SetRepositoryPolicy, `{"repositoryName":"team/app","policyText":"not-json"}`, awserrors.ErrorECRInvalidParameter}, + {"set empty name", SetRepositoryPolicy, `{"policyText":"{}"}`, awserrors.ErrorECRInvalidParameter}, {"set cross-account", SetRepositoryPolicy, `{"repositoryName":"team/app","registryId":"999999999999","policyText":"{}"}`, awserrors.ErrorAccessDenied}, {"get no policy", GetRepositoryPolicy, `{"repositoryName":"team/app"}`, awserrors.ErrorRepositoryPolicyNotFound}, {"delete no policy", DeleteRepositoryPolicy, `{"repositoryName":"team/app"}`, awserrors.ErrorRepositoryPolicyNotFound}, diff --git a/spinifex/gateway/ecrapi/tags_test.go b/spinifex/gateway/ecrapi/tags_test.go index f9026b551..c8756b1c5 100644 --- a/spinifex/gateway/ecrapi/tags_test.go +++ b/spinifex/gateway/ecrapi/tags_test.go @@ -124,14 +124,14 @@ func TestTagResource_Errors(t *testing.T) { _, err := TagResource(context.Background(), nc, policyTestAccount, []byte(`{"resourceArn":"`+tagResourceARN("team/app")+`","tags":[]}`)) require.Error(t, err) - assert.Equal(t, awserrors.ErrorInvalidParameterValue, awserrors.ValidErrorCodeFromError(err)) + assert.Equal(t, awserrors.ErrorECRInvalidParameter, awserrors.ValidErrorCodeFromError(err)) }) t.Run("malformed arn", func(t *testing.T) { _, err := TagResource(context.Background(), nc, policyTestAccount, []byte(`{"resourceArn":"not-an-arn","tags":[{"Key":"env","Value":"prod"}]}`)) require.Error(t, err) - assert.Equal(t, awserrors.ErrorInvalidParameterValue, awserrors.ValidErrorCodeFromError(err)) + assert.Equal(t, awserrors.ErrorECRInvalidParameter, awserrors.ValidErrorCodeFromError(err)) }) t.Run("repo not found", func(t *testing.T) { @@ -145,7 +145,7 @@ func TestTagResource_Errors(t *testing.T) { _, err := TagResource(context.Background(), nc, policyTestAccount, []byte(`{"resourceArn":"`+tagResourceARN("team/app")+`","tags":[{"Key":"","Value":"prod"}]}`)) require.Error(t, err) - assert.Equal(t, awserrors.ErrorInvalidParameterValue, awserrors.ValidErrorCodeFromError(err)) + assert.Equal(t, awserrors.ErrorECRInvalidParameter, awserrors.ValidErrorCodeFromError(err)) }) } @@ -155,5 +155,5 @@ func TestUntagResource_Errors(t *testing.T) { _, err := UntagResource(context.Background(), nc, policyTestAccount, []byte(`{"resourceArn":"`+tagResourceARN("team/app")+`","tagKeys":[]}`)) require.Error(t, err) - assert.Equal(t, awserrors.ErrorInvalidParameterValue, awserrors.ValidErrorCodeFromError(err)) + assert.Equal(t, awserrors.ErrorECRInvalidParameter, awserrors.ValidErrorCodeFromError(err)) } diff --git a/spinifex/gateway/ecrapi/validation.go b/spinifex/gateway/ecrapi/validation.go index 158592bdc..083608f33 100644 --- a/spinifex/gateway/ecrapi/validation.go +++ b/spinifex/gateway/ecrapi/validation.go @@ -10,64 +10,58 @@ import ( handlers_ecr "github.com/mulgadc/spinifex/spinifex/handlers/ecr" ) -// repositoryNamePattern is the expression AWS names when it refuses a -// repositoryName. -const repositoryNamePattern = `[a-z0-9]+((\.|_|__|-+)[a-z0-9]+)*(/[a-z0-9]+((\.|_|__|-+)[a-z0-9]+)*)*` - const ( minRepositoryNameLength = 2 maxRepositoryNameLength = 256 ) -// ValidateRepositoryName returns InvalidParameterValue naming the first +// ValidateRepositoryName returns InvalidParameterException naming the first // constraint name fails, or nil. func ValidateRepositoryName(name string) error { switch { case name == "": return RequiredParameterError("repositoryName") case len(name) < minRepositoryNameLength: - return awserrors.Errorf(awserrors.ErrorInvalidParameterValue, + return awserrors.Errorf(awserrors.ErrorECRInvalidParameter, "1 validation error detected: Value '%s' at 'repositoryName' failed to satisfy constraint: Member must have length greater than or equal to %d", name, minRepositoryNameLength) case len(name) > maxRepositoryNameLength: - return awserrors.Errorf(awserrors.ErrorInvalidParameterValue, - "1 validation error detected: Value '%s' at 'repositoryName' failed to satisfy constraint: Member must have length less than or equal to %d", - name, maxRepositoryNameLength) + return ConstraintError("repositoryName", fmt.Sprintf("must have length less than or equal to %d", maxRepositoryNameLength)) case handlers_ecr.ValidateRepoName(name) != nil: - return ConstraintError("repositoryName", "must satisfy regular expression '"+repositoryNamePattern+"'") + return ConstraintError("repositoryName", "must satisfy regular expression '"+handlers_ecr.RepoNamePattern+"'") } return nil } // RequiredParameterError refuses a request that omits field. func RequiredParameterError(field string) error { - return awserrors.Errorf(awserrors.ErrorInvalidParameterValue, + return awserrors.Errorf(awserrors.ErrorECRInvalidParameter, "1 validation error detected: Value null at '%s' failed to satisfy constraint: Member must not be null", field) } // EnumValueError refuses value for field, listing the values accepted. func EnumValueError(field, value string, allowed ...string) error { - return awserrors.Errorf(awserrors.ErrorInvalidParameterValue, + return awserrors.Errorf(awserrors.ErrorECRInvalidParameter, "1 validation error detected: Value '%s' at '%s' failed to satisfy constraint: Member must satisfy enum value set: [%s]", value, field, strings.Join(allowed, ", ")) } // MaxItemsError refuses a list field carrying more than limit members. func MaxItemsError(field string, limit int) error { - return awserrors.Errorf(awserrors.ErrorInvalidParameterValue, + return awserrors.Errorf(awserrors.ErrorECRInvalidParameter, "1 validation error detected: Value at '%s' failed to satisfy constraint: Member must have length less than or equal to %d", field, limit) } // ConstraintError refuses field for failing constraint. func ConstraintError(field, constraint string) error { - return awserrors.Errorf(awserrors.ErrorInvalidParameterValue, + return awserrors.Errorf(awserrors.ErrorECRInvalidParameter, "Invalid parameter at '%s' failed to satisfy constraint: '%s'", field, constraint) } // MalformedBodyError refuses a body that does not decode into the action's // input shape. func MalformedBodyError() error { - return awserrors.Errorf(awserrors.ErrorInvalidParameterValue, + return awserrors.Errorf(awserrors.ErrorECRInvalidParameter, "The request body is not valid JSON or does not match the input shape of the action") } @@ -79,7 +73,7 @@ func ValidateTags(tags []*ecr.Tag) error { return RequiredParameterError(field) } if aws.StringValue(t.Key) == "" { - return awserrors.Errorf(awserrors.ErrorInvalidParameterValue, + return awserrors.Errorf(awserrors.ErrorECRInvalidParameter, "1 validation error detected: Value '' at '%s' failed to satisfy constraint: Member must have length greater than or equal to 1", field) } } diff --git a/spinifex/gateway/ecrapi/validation_test.go b/spinifex/gateway/ecrapi/validation_test.go index d9c0fa397..088c0d49f 100644 --- a/spinifex/gateway/ecrapi/validation_test.go +++ b/spinifex/gateway/ecrapi/validation_test.go @@ -21,7 +21,7 @@ func requireInvalidParameter(t *testing.T, err error, wantMessage string) { require.Error(t, err) code, message, ok := awserrors.ResolveErrorDetail(err) require.True(t, ok) - assert.Equal(t, awserrors.ErrorInvalidParameterValue, code) + assert.Equal(t, awserrors.ErrorECRInvalidParameter, code) assert.Equal(t, wantMessage, message) } @@ -33,14 +33,20 @@ func TestValidateRepositoryName(t *testing.T) { {"uppercase", "Team/App", awsRepositoryNameMessage}, {"missing", "", "1 validation error detected: Value null at 'repositoryName' failed to satisfy constraint: Member must not be null"}, {"too short", "a", "1 validation error detected: Value 'a' at 'repositoryName' failed to satisfy constraint: Member must have length greater than or equal to 2"}, - {"too long", strings.Repeat("a", 257), "1 validation error detected: Value '" + strings.Repeat("a", 257) + "' at 'repositoryName' failed to satisfy constraint: Member must have length less than or equal to 256"}, + {"too long", strings.Repeat("a", 257), "Invalid parameter at 'repositoryName' failed to satisfy constraint: 'must have length less than or equal to 256'"}, + {"triple underscore", "a___b", awsRepositoryNameMessage}, + {"underscore then hyphen", "a_-b", awsRepositoryNameMessage}, + {"leading separator", "_a", awsRepositoryNameMessage}, + {"trailing separator", "a_", awsRepositoryNameMessage}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { requireInvalidParameter(t, gateway_ecrapi.ValidateRepositoryName(tc.input), tc.want) }) } - assert.NoError(t, gateway_ecrapi.ValidateRepositoryName("team/app")) + for _, name := range []string{"team/app", "a__b", "a--b", "a---b", "a/b__c", strings.Repeat("a", 256)} { + assert.NoError(t, gateway_ecrapi.ValidateRepositoryName(name), "expected %q valid", name) + } } func TestValidateTags(t *testing.T) { diff --git a/spinifex/handlers/ecr/lifecycle.go b/spinifex/handlers/ecr/lifecycle.go index 671dc9f75..ff87ff6f1 100644 --- a/spinifex/handlers/ecr/lifecycle.go +++ b/spinifex/handlers/ecr/lifecycle.go @@ -10,7 +10,7 @@ import ( // ErrInvalidLifecyclePolicy is returned when a lifecycle-policy document is // malformed or violates the supported rule schema. The gateway maps it to -// InvalidParameterValue. +// InvalidParameterException. var ErrInvalidLifecyclePolicy = errors.New("ecr: invalid lifecycle policy") // Lifecycle tag-status selectors and count types (AWS lifecycle-policy schema). diff --git a/spinifex/handlers/ecr/meta_test.go b/spinifex/handlers/ecr/meta_test.go index bbce0f7be..3395fa37b 100644 --- a/spinifex/handlers/ecr/meta_test.go +++ b/spinifex/handlers/ecr/meta_test.go @@ -33,11 +33,11 @@ func TestRepoMeta_EncryptionTypeOrDefault(t *testing.T) { } func TestValidateRepoName(t *testing.T) { - valid := []string{"ab", "team/app", "a/b/c", "my-repo.name_1", "x0/y1/z2"} + valid := []string{"ab", "team/app", "a/b/c", "my-repo.name_1", "x0/y1/z2", "a__b", "a--b", "a---b", "a/b__c"} for _, n := range valid { assert.NoError(t, ValidateRepoName(n), "expected %q valid", n) } - invalid := []string{"A", "Team/App", "a", "/leading", "trailing/", "a//b", strings.Repeat("a", 257)} + invalid := []string{"A", "Team/App", "a", "/leading", "trailing/", "a//b", strings.Repeat("a", 257), "a___b", "a_-b", "_a", "a_", "a..b", "Bad_Name!"} for _, n := range invalid { assert.Error(t, ValidateRepoName(n), "expected %q invalid", n) } diff --git a/spinifex/handlers/ecr/storage.go b/spinifex/handlers/ecr/storage.go index bb76dcd88..b81b1a70b 100644 --- a/spinifex/handlers/ecr/storage.go +++ b/spinifex/handlers/ecr/storage.go @@ -139,11 +139,14 @@ func DigestToken(digest string) string { return strings.ReplaceAll(digest, ":", "-") } -// repoNameRe is the OCI Distribution repository-name grammar. Compiled once. -var repoNameRe = regexp.MustCompile(`^(?:[a-z0-9]+(?:[._-][a-z0-9]+)*/)*[a-z0-9]+(?:[._-][a-z0-9]+)*$`) +// RepoNamePattern is the repository-name expression AWS ECR enforces, which is +// also the OCI Distribution grammar: components split by '.', '_', '__' or '-' +// runs, joined by '/'. +const RepoNamePattern = `[a-z0-9]+((\.|_|__|-+)[a-z0-9]+)*(/[a-z0-9]+((\.|_|__|-+)[a-z0-9]+)*)*` -// ValidateRepoName enforces the OCI repository-name grammar and 2-256 length -// bound. It returns nil for a valid name and a descriptive error otherwise. +var repoNameRe = regexp.MustCompile(`^(?:` + RepoNamePattern + `)$`) + +// ValidateRepoName enforces RepoNamePattern and the 2-256 length bound. It returns nil for a valid name and a descriptive error otherwise. func ValidateRepoName(name string) error { if len(name) < 2 || len(name) > 256 { return errors.New("repository name must be 2-256 characters") diff --git a/tests/integration/conformance-known-findings.md b/tests/integration/conformance-known-findings.md index 062470f88..589664c99 100644 --- a/tests/integration/conformance-known-findings.md +++ b/tests/integration/conformance-known-findings.md @@ -41,7 +41,7 @@ The 2026-09-29 baseline sent 4047 requests across 371 operations: 980 pass, 506 |---|---:|---| | A request breaking a model constraint is accepted | 420 | ECS 116, IAM 112, ACM 56, ELBv2 48, ECR 47, EC2 22, RDS 7, STS 6, EKS 6. Examples: ACM and ELBv2 accept a tag without `Key`, as IAM did; EC2 network `Describe*` accepted out-of-range `MaxResults`; ECS `CreateCapacityProvider` accepts every enum and range break inside `autoScalingGroupProvider` and `managedInstancesProvider`; ACM `RequestCertificate` accepts every break inside `DomainValidationOptions`; STS `GetSessionToken` accepted out-of-range `DurationSeconds` and a `SerialNumber` and `TokenCode` below their minimum length, and still accepts an out-of-range `MinimumSessionTokenSize`. | | A model-valid request is refused as invalid | 74 | RDS 42, ECS 10, STS 10, EC2 8, EKS 2, IAM 2. Mostly deliberate limitations that say so: RDS `CreateDBInstance`, `ModifyDBInstance` and `RestoreDBInstanceFromDBSnapshot` members, STS session policies, tags and MFA, ECS `availabilityZoneRebalancing`. Others return a generic message whose cause is unresolved (ECS task and task-definition operations, EC2 `Modify*Attribute`), or add a member next to a seeded one that AWS may refuse in combination, such as EC2 `CreateCapacityReservation` `AvailabilityZoneId` beside `AvailabilityZone`. | -| A refusal uses an undeclared error code | 96 | ECR (90) and EKS (2) answer with `InvalidParameterValueException`; both models declare `InvalidParameterException`. IAM (4) answered `CreateGroup` and `Put*Policy` name-length breaks with `InvalidInput`, which those operations do not declare; they now answer `ValidationError`, as AWS does. | +| A refusal uses an undeclared error code | 96 | EKS (2) answers with `InvalidParameterValueException`; its model declares `InvalidParameterException`. ECR (90) answered with `InvalidParameterValueException` too; it now answers `InvalidParameterException`, as AWS's `CreateRepository` does for a name that breaks the pattern. IAM (4) answered `CreateGroup` and `Put*Policy` name-length breaks with `InvalidInput`, which those operations do not declare; they now answer `ValidationError`, as AWS does. | How the sweep reaches its targets: diff --git a/tests/integration/ecr_test.go b/tests/integration/ecr_test.go index 30db31a96..4f1a9be9e 100644 --- a/tests/integration/ecr_test.go +++ b/tests/integration/ecr_test.go @@ -9,6 +9,7 @@ import ( "testing" "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/aws/awserr" "github.com/aws/aws-sdk-go/service/ecr" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -33,6 +34,14 @@ func TestECRControlPlane(t *testing.T) { _, err := c.CreateRepository(&ecr.CreateRepositoryInput{RepositoryName: aws.String(repo)}) require.NoError(t, err, "create-repository") + t.Run("an invalid name is refused with InvalidParameterException", func(t *testing.T) { + _, err := c.CreateRepository(&ecr.CreateRepositoryInput{RepositoryName: aws.String("Bad_Name!")}) + var aerr awserr.Error + require.ErrorAs(t, err, &aerr) + assert.Equal(t, ecr.ErrCodeInvalidParameterException, aerr.Code()) + assert.Equal(t, `Invalid parameter at 'repositoryName' failed to satisfy constraint: 'must satisfy regular expression '[a-z0-9]+((\.|_|__|-+)[a-z0-9]+)*(/[a-z0-9]+((\.|_|__|-+)[a-z0-9]+)*)*''`, aerr.Message()) + }) + t.Run("DescribeRepositories lists the new repo", func(t *testing.T) { out, err := c.DescribeRepositories(&ecr.DescribeRepositoriesInput{ RepositoryNames: []*string{aws.String(repo)},