From db97a5a101a23ea93a7e7e05c66b0d6a9d0b6575 Mon Sep 17 00:00:00 2001 From: Julian Sommer <115516918+juliansommer@users.noreply.github.com> Date: Wed, 7 Oct 2026 11:34:15 +1000 Subject: [PATCH] fix: answer ECR constraint breaks with AWS's messages and InvalidTagParameterException --- spinifex/awserrors/awserrors.go | 2 ++ .../ecr_createdeleterepository_test.go | 2 +- spinifex/gateway/ecr_createrepository.go | 7 ++-- spinifex/gateway/ecr_tagmutability.go | 3 +- spinifex/gateway/ecrapi/repositorypolicy.go | 2 +- .../gateway/ecrapi/repositorypolicy_test.go | 12 +++++++ spinifex/gateway/ecrapi/tags.go | 2 +- spinifex/gateway/ecrapi/tags_test.go | 21 +++++++++--- spinifex/gateway/ecrapi/validation.go | 34 +++++++++---------- spinifex/gateway/ecrapi/validation_test.go | 25 ++++++++++---- 10 files changed, 74 insertions(+), 36 deletions(-) diff --git a/spinifex/awserrors/awserrors.go b/spinifex/awserrors/awserrors.go index cd9aeafdd..2e7a4b59d 100644 --- a/spinifex/awserrors/awserrors.go +++ b/spinifex/awserrors/awserrors.go @@ -520,6 +520,7 @@ var ( ErrorLayersNotFound = "LayersNotFoundException" ErrorReferencedImagesNotFound = "ReferencedImagesNotFoundException" ErrorTooManyTags = "TooManyTagsException" + ErrorInvalidTagParameter = "InvalidTagParameterException" ErrorOperationNotSupported = "OperationNotSupportedException" // ELBv2-specific error codes. @@ -1286,6 +1287,7 @@ var ErrorLookup = map[string]ErrorMessage{ ErrorImageDigestDoesNotMatch: {HTTPCode: 400, Message: "The specified image digest does not match the digest of the supplied image manifest."}, ErrorLayersNotFound: {HTTPCode: 400, Message: "The specified layers could not be found, or the specified layer is not valid for this repository."}, ErrorReferencedImagesNotFound: {HTTPCode: 400, Message: "The manifest list is referencing an image that does not exist."}, + ErrorInvalidTagParameter: {HTTPCode: 400, Message: "Tag parameters are invalid"}, ErrorTooManyTags: {HTTPCode: 400, Message: "The list of tags on the repository is over the limit. The maximum number of tags that can be applied to a repository is 50."}, ErrorOperationNotSupported: {HTTPCode: 400, Message: "The specified operation is not supported in this registry."}, diff --git a/spinifex/gateway/ecr_createdeleterepository_test.go b/spinifex/gateway/ecr_createdeleterepository_test.go index ebe1190b0..56490c18f 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, "InvalidParameterException", awserrors.ValidErrorCodeFromError(err)) + assert.Equal(t, "InvalidTagParameterException", awserrors.ValidErrorCodeFromError(err)) } func TestCreateRepository_EncryptionAndScanningConfiguration(t *testing.T) { diff --git a/spinifex/gateway/ecr_createrepository.go b/spinifex/gateway/ecr_createrepository.go index 789044b84..9043394e3 100644 --- a/spinifex/gateway/ecr_createrepository.go +++ b/spinifex/gateway/ecr_createrepository.go @@ -139,8 +139,8 @@ func normalizeEncryptionType(cfg *encryptionConfigurationInput) (string, error) 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, - handlers_ecr.EncryptionTypeAES256, handlers_ecr.EncryptionTypeKMS) + return "", gateway_ecrapi.EnumValueError("encryptionConfiguration.encryptionType", + handlers_ecr.EncryptionTypeAES256, "KMS_DSSE", handlers_ecr.EncryptionTypeKMS) } } @@ -153,7 +153,6 @@ func normalizeTagMutability(v string) (string, error) { case handlers_ecr.TagMutabilityMutable, handlers_ecr.TagMutabilityImmutable: return v, nil default: - return "", gateway_ecrapi.EnumValueError("imageTagMutability", v, - handlers_ecr.TagMutabilityMutable, handlers_ecr.TagMutabilityImmutable) + return "", gateway_ecrapi.EnumValueError("imageTagMutability", gateway_ecrapi.ImageTagMutabilityValues...) } } diff --git a/spinifex/gateway/ecr_tagmutability.go b/spinifex/gateway/ecr_tagmutability.go index 45385a37b..0038a9594 100644 --- a/spinifex/gateway/ecr_tagmutability.go +++ b/spinifex/gateway/ecr_tagmutability.go @@ -53,8 +53,7 @@ func (gw *GatewayConfig) handlePutImageTagMutability(w http.ResponseWriter, r *h case "": return gateway_ecrapi.RequiredParameterError("imageTagMutability") default: - return gateway_ecrapi.EnumValueError("imageTagMutability", req.ImageTagMutability, - handlers_ecr.TagMutabilityMutable, handlers_ecr.TagMutabilityImmutable) + return gateway_ecrapi.EnumValueError("imageTagMutability", gateway_ecrapi.ImageTagMutabilityValues...) } store := handlers_ecr.NewNATSMetaStore(gw.NATSConn) diff --git a/spinifex/gateway/ecrapi/repositorypolicy.go b/spinifex/gateway/ecrapi/repositorypolicy.go index 136f5bf5f..0c418600c 100644 --- a/spinifex/gateway/ecrapi/repositorypolicy.go +++ b/spinifex/gateway/ecrapi/repositorypolicy.go @@ -59,7 +59,7 @@ func SetRepositoryPolicy(ctx context.Context, nc *nats.Conn, accountID string, b return nil, err } if req.PolicyText == "" { - return nil, RequiredParameterError("policyText") + return nil, ConstraintError("PolicyText", "Cannot be null") } if !json.Valid([]byte(req.PolicyText)) { return nil, ConstraintError("policyText", "Invalid repository policy provided") diff --git a/spinifex/gateway/ecrapi/repositorypolicy_test.go b/spinifex/gateway/ecrapi/repositorypolicy_test.go index bfb47a468..431198310 100644 --- a/spinifex/gateway/ecrapi/repositorypolicy_test.go +++ b/spinifex/gateway/ecrapi/repositorypolicy_test.go @@ -124,6 +124,18 @@ func TestRepositoryPolicy_Errors(t *testing.T) { } } +// AWS names the member PolicyText, capitalised, when SetRepositoryPolicy omits it. +func TestSetRepositoryPolicy_MissingPolicyTextMessage(t *testing.T) { + nc := newPolicyTestConn(t) + seedRepo(t, nc, "team/app") + _, err := SetRepositoryPolicy(context.Background(), nc, policyTestAccount, []byte(`{"repositoryName":"team/app"}`)) + require.Error(t, err) + code, message, ok := awserrors.ResolveErrorDetail(err) + require.True(t, ok) + assert.Equal(t, awserrors.ErrorECRInvalidParameter, code) + assert.Equal(t, "Invalid parameter at 'PolicyText' failed to satisfy constraint: 'Cannot be null'", message) +} + // strconvQuote JSON-quotes a string for inline test bodies. func strconvQuote(s string) string { b, _ := json.Marshal(s) diff --git a/spinifex/gateway/ecrapi/tags.go b/spinifex/gateway/ecrapi/tags.go index 8d290875f..94bbe6938 100644 --- a/spinifex/gateway/ecrapi/tags.go +++ b/spinifex/gateway/ecrapi/tags.go @@ -37,7 +37,7 @@ type listTagsForResourceRequest struct { // and the NATS-backed MetaStore for a follow-on read-modify-write. func resolveTaggedRepo(ctx context.Context, nc *nats.Conn, accountID, resourceArn string) (string, handlers_ecr.RepoMeta, *handlers_ecr.NATSMetaStore, error) { if resourceArn == "" { - return "", handlers_ecr.RepoMeta{}, nil, RequiredParameterError("resourceArn") + return "", handlers_ecr.RepoMeta{}, nil, invalidResourceARNError() } name, err := RepositoryNameFromResourceARN(resourceArn) if err != nil { diff --git a/spinifex/gateway/ecrapi/tags_test.go b/spinifex/gateway/ecrapi/tags_test.go index c8756b1c5..1489c17f2 100644 --- a/spinifex/gateway/ecrapi/tags_test.go +++ b/spinifex/gateway/ecrapi/tags_test.go @@ -130,8 +130,12 @@ func TestTagResource_Errors(t *testing.T) { 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.ErrorECRInvalidParameter, awserrors.ValidErrorCodeFromError(err)) + requireTagError(t, err, awserrors.ErrorECRInvalidParameter, "Invalid parameter at 'resourceArn' failed to satisfy constraint: 'Invalid ARN'") + }) + + t.Run("missing arn", func(t *testing.T) { + _, err := TagResource(context.Background(), nc, policyTestAccount, []byte(`{"tags":[{"Key":"env","Value":"prod"}]}`)) + requireTagError(t, err, awserrors.ErrorECRInvalidParameter, "Invalid parameter at 'resourceArn' failed to satisfy constraint: 'Invalid ARN'") }) t.Run("repo not found", func(t *testing.T) { @@ -144,11 +148,20 @@ func TestTagResource_Errors(t *testing.T) { t.Run("empty key rejected", func(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.ErrorECRInvalidParameter, awserrors.ValidErrorCodeFromError(err)) + requireTagError(t, err, awserrors.ErrorInvalidTagParameter, "Tag parameters are invalid") }) } +// requireTagError asserts err resolves to AWS's code and message. +func requireTagError(t *testing.T, err error, wantCode, wantMessage string) { + t.Helper() + require.Error(t, err) + code, message, ok := awserrors.ResolveErrorDetail(err) + require.True(t, ok) + assert.Equal(t, wantCode, code) + assert.Equal(t, wantMessage, message) +} + func TestUntagResource_Errors(t *testing.T) { nc := newPolicyTestConn(t) diff --git a/spinifex/gateway/ecrapi/validation.go b/spinifex/gateway/ecrapi/validation.go index 083608f33..b908413e5 100644 --- a/spinifex/gateway/ecrapi/validation.go +++ b/spinifex/gateway/ecrapi/validation.go @@ -20,11 +20,9 @@ const ( func ValidateRepositoryName(name string) error { switch { case name == "": - return RequiredParameterError("repositoryName") + return ConstraintError("repositoryName", "must not be null") case len(name) < minRepositoryNameLength: - 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) + return ConstraintError("repositoryName", fmt.Sprintf("must have length greater than or equal to %d", minRepositoryNameLength)) case len(name) > maxRepositoryNameLength: return ConstraintError("repositoryName", fmt.Sprintf("must have length less than or equal to %d", maxRepositoryNameLength)) case handlers_ecr.ValidateRepoName(name) != nil: @@ -35,21 +33,18 @@ func ValidateRepositoryName(name string) error { // RequiredParameterError refuses a request that omits field. func RequiredParameterError(field string) error { - return awserrors.Errorf(awserrors.ErrorECRInvalidParameter, - "1 validation error detected: Value null at '%s' failed to satisfy constraint: Member must not be null", field) + return ConstraintError(field, "Member must not be null") } -// EnumValueError refuses value for field, listing the values accepted. -func EnumValueError(field, value string, allowed ...string) error { - 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, ", ")) +// EnumValueError refuses a value of field outside allowed, which lists the +// values AWS's model declares, in the order AWS lists them. +func EnumValueError(field string, allowed ...string) error { + return ConstraintError(field, "Member must satisfy enum value set: ["+strings.Join(allowed, ", ")+"]") } // MaxItemsError refuses a list field carrying more than limit members. func MaxItemsError(field string, limit int) error { - 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) + return ConstraintError(field, fmt.Sprintf("Member must have length less than or equal to %d", limit)) } // ConstraintError refuses field for failing constraint. @@ -65,7 +60,8 @@ func MalformedBodyError() error { "The request body is not valid JSON or does not match the input shape of the action") } -// ValidateTags refuses a tag list with a missing or empty key. +// ValidateTags refuses a tag list with a missing or empty key. AWS answers a +// null key as a parameter error and an empty one as a tag error. func ValidateTags(tags []*ecr.Tag) error { for i, t := range tags { field := fmt.Sprintf("tags.%d.member.key", i+1) @@ -73,8 +69,7 @@ func ValidateTags(tags []*ecr.Tag) error { return RequiredParameterError(field) } if aws.StringValue(t.Key) == "" { - 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) + return awserrors.Errorf(awserrors.ErrorInvalidTagParameter, "Tag parameters are invalid") } } return nil @@ -87,5 +82,10 @@ func InvalidLifecyclePolicyError() error { } func invalidResourceARNError() error { - return ConstraintError("resourceArn", "must be a repository ARN of the form arn:aws:ecr:::repository/") + return ConstraintError("resourceArn", "Invalid ARN") } + +// ImageTagMutabilityValues is the imageTagMutability enum AWS's model declares, +// in the order AWS lists it when refusing a value. The exclusion values are +// listed but not accepted. +var ImageTagMutabilityValues = []string{"IMMUTABLE", "MUTABLE", "MUTABLE_WITH_EXCLUSION", "IMMUTABLE_WITH_EXCLUSION"} diff --git a/spinifex/gateway/ecrapi/validation_test.go b/spinifex/gateway/ecrapi/validation_test.go index 088c0d49f..a365b2a6f 100644 --- a/spinifex/gateway/ecrapi/validation_test.go +++ b/spinifex/gateway/ecrapi/validation_test.go @@ -31,8 +31,8 @@ func TestValidateRepositoryName(t *testing.T) { }{ {"pattern", "Bad_Name!", awsRepositoryNameMessage}, {"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"}, + {"missing", "", "Invalid parameter at 'repositoryName' failed to satisfy constraint: 'must not be null'"}, + {"too short", "a", "Invalid parameter at 'repositoryName' failed to satisfy constraint: 'must have length greater than or equal to 2'"}, {"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}, @@ -52,13 +52,26 @@ func TestValidateRepositoryName(t *testing.T) { func TestValidateTags(t *testing.T) { requireInvalidParameter(t, gateway_ecrapi.ValidateTags([]*ecr.Tag{{Key: aws.String("env")}, {Value: aws.String("prod")}}), - "1 validation error detected: Value null at 'tags.2.member.key' failed to satisfy constraint: Member must not be null") - requireInvalidParameter(t, - gateway_ecrapi.ValidateTags([]*ecr.Tag{{Key: aws.String("")}}), - "1 validation error detected: Value '' at 'tags.1.member.key' failed to satisfy constraint: Member must have length greater than or equal to 1") + "Invalid parameter at 'tags.2.member.key' failed to satisfy constraint: 'Member must not be null'") + err := gateway_ecrapi.ValidateTags([]*ecr.Tag{{Key: aws.String("")}}) + require.Error(t, err) + code, message, ok := awserrors.ResolveErrorDetail(err) + require.True(t, ok) + assert.Equal(t, "InvalidTagParameterException", code) + assert.Equal(t, "Tag parameters are invalid", message) assert.NoError(t, gateway_ecrapi.ValidateTags([]*ecr.Tag{{Key: aws.String("env"), Value: aws.String("")}})) } +// The messages below are AWS's, captured from real ECR refusals. +func TestConstraintHelpers_AWSMessages(t *testing.T) { + requireInvalidParameter(t, gateway_ecrapi.RequiredParameterError("imageTagMutability"), + "Invalid parameter at 'imageTagMutability' failed to satisfy constraint: 'Member must not be null'") + requireInvalidParameter(t, gateway_ecrapi.EnumValueError("imageTagMutability", gateway_ecrapi.ImageTagMutabilityValues...), + "Invalid parameter at 'imageTagMutability' failed to satisfy constraint: 'Member must satisfy enum value set: [IMMUTABLE, MUTABLE, MUTABLE_WITH_EXCLUSION, IMMUTABLE_WITH_EXCLUSION]'") + requireInvalidParameter(t, gateway_ecrapi.MaxItemsError("imageIds", 100), + "Invalid parameter at 'imageIds' failed to satisfy constraint: 'Member must have length less than or equal to 100'") +} + func TestResourceARNs_AmbiguousBodyNamesTheFault(t *testing.T) { _, err := gateway_ecrapi.ResourceARNs("DeleteRepository", "ap-southeast-2", "123456789012", []byte(`{"repositoryName":"dev","RepositoryName":"prod"}`))