diff --git a/spinifex/gateway/ecr_createdeleterepository_test.go b/spinifex/gateway/ecr_createdeleterepository_test.go index b7e2ebfce..3018f79c8 100644 --- a/spinifex/gateway/ecr_createdeleterepository_test.go +++ b/spinifex/gateway/ecr_createdeleterepository_test.go @@ -9,6 +9,7 @@ import ( "testing" "time" + "github.com/mulgadc/spinifex/spinifex/awserrors" handlers_ecr "github.com/mulgadc/spinifex/spinifex/handlers/ecr" "github.com/mulgadc/spinifex/spinifex/testutil" "github.com/nats-io/nats.go" @@ -53,12 +54,18 @@ func deleteRepo(t *testing.T, gw *GatewayConfig, body string) (*httptest.Respons type repoOut struct { Repository struct { - RepositoryName string `json:"repositoryName"` - RegistryID string `json:"registryId"` - RepositoryArn string `json:"repositoryArn"` - RepositoryURI string `json:"repositoryUri"` - ImageTagMutability string `json:"imageTagMutability"` - CreatedAt float64 `json:"createdAt"` + RepositoryName string `json:"repositoryName"` + RegistryID string `json:"registryId"` + RepositoryArn string `json:"repositoryArn"` + RepositoryURI string `json:"repositoryUri"` + ImageTagMutability string `json:"imageTagMutability"` + CreatedAt float64 `json:"createdAt"` + EncryptionConfiguration struct { + EncryptionType string `json:"encryptionType"` + } `json:"encryptionConfiguration"` + ImageScanningConfiguration struct { + ScanOnPush bool `json:"scanOnPush"` + } `json:"imageScanningConfiguration"` } `json:"repository"` } @@ -76,6 +83,64 @@ func TestCreateRepository_Happy(t *testing.T) { assert.Equal(t, ecrTestAccount+".dkr.ecr."+ecrTestRegion+"."+ecrTestSuffix+"/team/app", out.Repository.RepositoryURI) assert.Equal(t, "MUTABLE", out.Repository.ImageTagMutability) assert.Positive(t, out.Repository.CreatedAt) + assert.Equal(t, "AES256", out.Repository.EncryptionConfiguration.EncryptionType) + assert.False(t, out.Repository.ImageScanningConfiguration.ScanOnPush) +} + +// TestCreateRepository_Tags pins create-time tags surviving to the stored +// record. The wire keys are capitalized Key/Value because ecr.Tag carries no +// locationName; a lowercase spelling decodes to empty strings and is rejected. +func TestCreateRepository_Tags(t *testing.T) { + gw, nc := newRepoLifecycleGateway(t) + + _, err := createRepo(t, gw, `{"repositoryName":"team/tagged","tags":[{"Key":"env","Value":"prod"},{"Key":"team","Value":""}]}`) + require.NoError(t, err) + + meta, err := handlers_ecr.NewNATSMetaStore(nc).GetRepo(context.Background(), ecrTestAccount, "team/tagged") + require.NoError(t, err) + assert.Equal(t, map[string]string{"env": "prod", "team": ""}, meta.Tags) + + // An absent list must leave the map nil, so the record is byte-identical to + // one written before tags were accepted here. + _, err = createRepo(t, gw, `{"repositoryName":"team/untagged"}`) + require.NoError(t, err) + meta, err = handlers_ecr.NewNATSMetaStore(nc).GetRepo(context.Background(), ecrTestAccount, "team/untagged") + require.NoError(t, err) + assert.Nil(t, meta.Tags) + + _, err = createRepo(t, gw, `{"repositoryName":"team/badtag","tags":[{"Key":"","Value":"x"}]}`) + require.Error(t, err) + assert.Equal(t, "InvalidParameterValue", err.Error()) +} + +func TestCreateRepository_EncryptionAndScanningConfiguration(t *testing.T) { + gw, _ := newRepoLifecycleGateway(t) + + w, err := createRepo(t, gw, `{"repositoryName":"team/scanned","imageScanningConfiguration":{"scanOnPush":true}}`) + require.NoError(t, err) + var out repoOut + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &out)) + assert.Equal(t, "AES256", out.Repository.EncryptionConfiguration.EncryptionType) + assert.True(t, out.Repository.ImageScanningConfiguration.ScanOnPush) + + w, err = createRepo(t, gw, `{"repositoryName":"team/aes","encryptionConfiguration":{"encryptionType":"AES256"}}`) + require.NoError(t, err) + out = repoOut{} + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &out)) + assert.Equal(t, "AES256", out.Repository.EncryptionConfiguration.EncryptionType) + + // KMS carries a client-facing message (awserrors.Errorf), so the code is + // read via ResolveErrorDetail rather than a bare err.Error() comparison. + _, err = createRepo(t, gw, `{"repositoryName":"team/kms","encryptionConfiguration":{"encryptionType":"KMS"}}`) + require.Error(t, err) + code, message, ok := awserrors.ResolveErrorDetail(err) + require.True(t, ok) + assert.Equal(t, "InvalidParameterValue", code) + assert.NotEmpty(t, message) + + _, err = createRepo(t, gw, `{"repositoryName":"team/bad","encryptionConfiguration":{"encryptionType":"bogus"}}`) + require.Error(t, err) + assert.Equal(t, "InvalidParameterValue", err.Error()) } func TestCreateRepository_Errors(t *testing.T) { @@ -141,6 +206,7 @@ func TestDeleteRepository_Happy(t *testing.T) { var out repoOut require.NoError(t, json.Unmarshal(w.Body.Bytes(), &out)) assert.Equal(t, "team/app", out.Repository.RepositoryName) + assert.Equal(t, "AES256", out.Repository.EncryptionConfiguration.EncryptionType) // Gone afterwards. _, err = deleteRepo(t, gw, `{"repositoryName":"team/app"}`) diff --git a/spinifex/gateway/ecr_createrepository.go b/spinifex/gateway/ecr_createrepository.go index 9084c339b..1f54861c9 100644 --- a/spinifex/gateway/ecr_createrepository.go +++ b/spinifex/gateway/ecr_createrepository.go @@ -15,14 +15,29 @@ import ( handlers_ecr "github.com/mulgadc/spinifex/spinifex/handlers/ecr" ) -// createRepositoryRequest is the camelCase AWS JSON 1.1 input shape. The SDK -// input struct carries locationName tags rather than json tags, so the subset -// honored here is decoded explicitly. Tags, encryption, and scanning config are -// accepted-and-ignored in v1. +// createRepositoryRequest is the camelCase AWS JSON 1.1 input shape. ecr.Tag +// carries no locationName, so its wire keys are Key/Value (capitalized); +// reusing the SDK type keeps that spelling rather than a lowercase hand-roll. type createRepositoryRequest struct { - RepositoryName string `json:"repositoryName"` - RegistryID string `json:"registryId"` - ImageTagMutability string `json:"imageTagMutability"` + RepositoryName string `json:"repositoryName"` + RegistryID string `json:"registryId"` + ImageTagMutability string `json:"imageTagMutability"` + Tags []*ecr.Tag `json:"tags"` + EncryptionConfiguration *encryptionConfigurationInput `json:"encryptionConfiguration"` + ImageScanningConfiguration *imageScanningConfigurationInput `json:"imageScanningConfiguration"` +} + +// encryptionConfigurationInput is the camelCase input shape for +// encryptionConfiguration. kmsKey is not decoded: KMS is rejected outright, so +// any key material supplied alongside it is never read. +type encryptionConfigurationInput struct { + EncryptionType string `json:"encryptionType"` +} + +// imageScanningConfigurationInput is the camelCase input shape for +// imageScanningConfiguration. +type imageScanningConfigurationInput struct { + ScanOnPush bool `json:"scanOnPush"` } // handleCreateRepository provisions an empty repository in the caller account. @@ -56,6 +71,15 @@ func (gw *GatewayConfig) handleCreateRepository(w http.ResponseWriter, r *http.R if err != nil { return err } + encryptionType, err := normalizeEncryptionType(req.EncryptionConfiguration) + if err != nil { + return err + } + scanOnPush := req.ImageScanningConfiguration != nil && req.ImageScanningConfiguration.ScanOnPush + tags, err := tagMapFromInput(req.Tags) + if err != nil { + return err + } store := handlers_ecr.NewNATSMetaStore(gw.NATSConn) if _, err := store.GetRepo(ctx, accountID, req.RepositoryName); err == nil { @@ -69,6 +93,9 @@ func (gw *GatewayConfig) handleCreateRepository(w http.ResponseWriter, r *http.R Name: req.RepositoryName, CreatedAt: time.Now().UTC(), ImageTagMutability: mutability, + EncryptionType: encryptionType, + ScanOnPush: scanOnPush, + Tags: tags, } if err := store.PutRepo(ctx, accountID, meta); err != nil { slog.ErrorContext(ctx, "CreateRepository: put repo failed", "repo", req.RepositoryName, "err", err) @@ -76,18 +103,46 @@ func (gw *GatewayConfig) handleCreateRepository(w http.ResponseWriter, r *http.R } gateway_ecrapi.WriteJSONResponse(w, &ecr.CreateRepositoryOutput{ - Repository: &ecr.Repository{ - RegistryId: aws.String(accountID), - RepositoryName: aws.String(req.RepositoryName), - RepositoryArn: aws.String(gw.ecrRepositoryArn(accountID, req.RepositoryName)), - RepositoryUri: aws.String(gw.ecrRepositoryUri(accountID, req.RepositoryName)), - CreatedAt: aws.Time(meta.CreatedAt), - ImageTagMutability: aws.String(meta.TagMutability()), - }, + Repository: gw.buildRepository(accountID, req.RepositoryName, meta), }) return nil } +// tagMapFromInput folds the create-time tag list into the map RepoMeta stores, +// rejecting an empty key exactly as TagResource does. An absent list yields a +// nil map so the record round-trips identically to one created without tags. +func tagMapFromInput(in []*ecr.Tag) (map[string]string, error) { + if len(in) == 0 { + return nil, nil + } + out := make(map[string]string, len(in)) + for _, t := range in { + if t == nil || aws.StringValue(t.Key) == "" { + return nil, errors.New(awserrors.ErrorInvalidParameterValue) + } + out[aws.StringValue(t.Key)] = aws.StringValue(t.Value) + } + return out, nil +} + +// normalizeEncryptionType validates the requested encryption configuration, +// defaulting an absent one to AES256. KMS is rejected: no customer key +// material is plumbed anywhere for it to honor. +func normalizeEncryptionType(cfg *encryptionConfigurationInput) (string, error) { + if cfg == nil || cfg.EncryptionType == "" { + return handlers_ecr.EncryptionTypeAES256, nil + } + switch cfg.EncryptionType { + case handlers_ecr.EncryptionTypeAES256: + return cfg.EncryptionType, nil + case handlers_ecr.EncryptionTypeKMS: + return "", awserrors.Errorf(awserrors.ErrorInvalidParameterValue, + "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 "", errors.New(awserrors.ErrorInvalidParameterValue) + } +} + // normalizeTagMutability validates the requested mutability, defaulting an empty // value to MUTABLE. An unknown value is rejected with InvalidParameterValue. func normalizeTagMutability(v string) (string, error) { diff --git a/spinifex/gateway/ecr_deleterepository.go b/spinifex/gateway/ecr_deleterepository.go index f2edfe327..bcf39ee09 100644 --- a/spinifex/gateway/ecr_deleterepository.go +++ b/spinifex/gateway/ecr_deleterepository.go @@ -7,7 +7,6 @@ import ( "log/slog" "net/http" - "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ecr" "github.com/mulgadc/spinifex/spinifex/awserrors" gateway_ecrapi "github.com/mulgadc/spinifex/spinifex/gateway/ecrapi" @@ -80,14 +79,7 @@ func (gw *GatewayConfig) handleDeleteRepository(w http.ResponseWriter, r *http.R } gateway_ecrapi.WriteJSONResponse(w, &ecr.DeleteRepositoryOutput{ - Repository: &ecr.Repository{ - RegistryId: aws.String(accountID), - RepositoryName: aws.String(req.RepositoryName), - RepositoryArn: aws.String(gw.ecrRepositoryArn(accountID, req.RepositoryName)), - RepositoryUri: aws.String(gw.ecrRepositoryUri(accountID, req.RepositoryName)), - CreatedAt: aws.Time(meta.CreatedAt), - ImageTagMutability: aws.String(meta.TagMutability()), - }, + Repository: gw.buildRepository(accountID, req.RepositoryName, meta), }) return nil } diff --git a/spinifex/gateway/ecr_describerepositories.go b/spinifex/gateway/ecr_describerepositories.go index b9dc88067..47f29e72d 100644 --- a/spinifex/gateway/ecr_describerepositories.go +++ b/spinifex/gateway/ecr_describerepositories.go @@ -7,7 +7,6 @@ import ( "log/slog" "net/http" - "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ecr" "github.com/mulgadc/spinifex/spinifex/awserrors" gateway_ecrapi "github.com/mulgadc/spinifex/spinifex/gateway/ecrapi" @@ -69,14 +68,7 @@ func (gw *GatewayConfig) handleDescribeRepositories(w http.ResponseWriter, r *ht slog.ErrorContext(ctx, "DescribeRepositories: get repo failed", "repo", name, "err", err) return errors.New(awserrors.ErrorServerInternal) } - repos = append(repos, &ecr.Repository{ - RegistryId: aws.String(accountID), - RepositoryName: aws.String(name), - RepositoryArn: aws.String(gw.ecrRepositoryArn(accountID, name)), - RepositoryUri: aws.String(gw.ecrRepositoryUri(accountID, name)), - CreatedAt: aws.Time(meta.CreatedAt), - ImageTagMutability: aws.String(meta.TagMutability()), - }) + repos = append(repos, gw.buildRepository(accountID, name, meta)) } gateway_ecrapi.WriteJSONResponse(w, &ecr.DescribeRepositoriesOutput{Repositories: repos}) diff --git a/spinifex/gateway/ecr_describerepositories_test.go b/spinifex/gateway/ecr_describerepositories_test.go index 8f53766b0..0b0a5fd53 100644 --- a/spinifex/gateway/ecr_describerepositories_test.go +++ b/spinifex/gateway/ecr_describerepositories_test.go @@ -70,12 +70,18 @@ func describeReposRequest(t *testing.T, gw *GatewayConfig, body string) *httptes type describeReposOut struct { Repositories []struct { - RepositoryName string `json:"repositoryName"` - RegistryID string `json:"registryId"` - RepositoryArn string `json:"repositoryArn"` - RepositoryURI string `json:"repositoryUri"` - ImageTagMutability string `json:"imageTagMutability"` - CreatedAt float64 `json:"createdAt"` + RepositoryName string `json:"repositoryName"` + RegistryID string `json:"registryId"` + RepositoryArn string `json:"repositoryArn"` + RepositoryURI string `json:"repositoryUri"` + ImageTagMutability string `json:"imageTagMutability"` + CreatedAt float64 `json:"createdAt"` + EncryptionConfiguration struct { + EncryptionType string `json:"encryptionType"` + } `json:"encryptionConfiguration"` + ImageScanningConfiguration struct { + ScanOnPush bool `json:"scanOnPush"` + } `json:"imageScanningConfiguration"` } `json:"repositories"` } @@ -98,6 +104,8 @@ func TestDescribeRepositories_ListsAccountScoped(t *testing.T) { assert.Equal(t, ecrTestAccount+".dkr.ecr."+ecrTestRegion+"."+ecrTestSuffix+"/team/app", app.RepositoryURI) assert.Equal(t, "MUTABLE", app.ImageTagMutability) assert.Positive(t, app.CreatedAt) + assert.Equal(t, "AES256", app.EncryptionConfiguration.EncryptionType) + assert.False(t, app.ImageScanningConfiguration.ScanOnPush) } func TestECRRegistryHost_AppendsAdvertisedPort(t *testing.T) { diff --git a/spinifex/gateway/ecr_repository.go b/spinifex/gateway/ecr_repository.go new file mode 100644 index 000000000..615c1c9be --- /dev/null +++ b/spinifex/gateway/ecr_repository.go @@ -0,0 +1,27 @@ +package gateway + +import ( + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/ecr" + handlers_ecr "github.com/mulgadc/spinifex/spinifex/handlers/ecr" +) + +// buildRepository projects a RepoMeta record into the AWS *ecr.Repository +// shape shared by CreateRepository, DescribeRepositories, and +// DeleteRepository, so a new field is added here once rather than thrice. +func (gw *GatewayConfig) buildRepository(accountID, name string, meta handlers_ecr.RepoMeta) *ecr.Repository { + return &ecr.Repository{ + RegistryId: aws.String(accountID), + RepositoryName: aws.String(name), + RepositoryArn: aws.String(gw.ecrRepositoryArn(accountID, name)), + RepositoryUri: aws.String(gw.ecrRepositoryUri(accountID, name)), + CreatedAt: aws.Time(meta.CreatedAt), + ImageTagMutability: aws.String(meta.TagMutability()), + EncryptionConfiguration: &ecr.EncryptionConfiguration{ + EncryptionType: aws.String(meta.EncryptionTypeOrDefault()), + }, + ImageScanningConfiguration: &ecr.ImageScanningConfiguration{ + ScanOnPush: aws.Bool(meta.ScanOnPush), + }, + } +} diff --git a/spinifex/gateway/ecrapi/authz.go b/spinifex/gateway/ecrapi/authz.go index 7c3baa08b..b48857ee8 100644 --- a/spinifex/gateway/ecrapi/authz.go +++ b/spinifex/gateway/ecrapi/authz.go @@ -201,13 +201,24 @@ func repositoryARN(region, accountID, name string) string { // caller's account: the tag handler keys off the repository name alone and // operates in the caller's own account bucket. func tagARN(region, accountID, resourceARN string) string { - parts := strings.SplitN(resourceARN, ":", 6) - if len(parts) != 6 || parts[0] != "arn" || parts[2] != "ecr" { - return anyResource - } - kind, name, found := strings.Cut(parts[5], "/") - if !found || kind != repositoryResourceType || name == "" { + name, err := RepositoryNameFromResourceARN(resourceARN) + if err != nil { return anyResource } return repositoryARN(region, accountID, name) } + +// RepositoryNameFromResourceARN extracts the repository name from an ECR +// resource ARN (arn:aws:ecr:::repository/). tagARN and +// the tag handlers share this one parser for that shape. +func RepositoryNameFromResourceARN(resourceARN string) (string, error) { + parts := strings.SplitN(resourceARN, ":", 6) + if len(parts) != 6 || parts[0] != "arn" || parts[2] != "ecr" { + return "", errors.New(awserrors.ErrorInvalidParameterValue) + } + kind, name, found := strings.Cut(parts[5], "/") + if !found || kind != repositoryResourceType || name == "" { + return "", errors.New(awserrors.ErrorInvalidParameterValue) + } + return name, nil +} diff --git a/spinifex/gateway/ecrapi/handler.go b/spinifex/gateway/ecrapi/handler.go index 1c89ce014..17830fb68 100644 --- a/spinifex/gateway/ecrapi/handler.go +++ b/spinifex/gateway/ecrapi/handler.go @@ -81,9 +81,10 @@ var Actions = map[string]Handler{ "PutRegistryPolicy": NotImplemented, "DescribeRegistry": NotImplemented, - // Image scanning is unsupported (no scanner backend): the whole scan surface - // returns OperationNotSupportedException rather than NotImplemented. - "PutImageScanningConfiguration": ScanningNotSupported, + // Image scanning is unsupported (no scanner backend), except that + // PutImageScanningConfiguration accepts the scanOnPush=false no-op rather + // than rejecting the whole action unread. + "PutImageScanningConfiguration": PutImageScanningConfiguration, "GetImageScanningConfiguration": ScanningNotSupported, "StartImageScan": ScanningNotSupported, "DescribeImageScanFindings": ScanningNotSupported, @@ -91,7 +92,7 @@ var Actions = map[string]Handler{ "PutRegistryScanningConfiguration": ScanningNotSupported, "BatchGetRepositoryScanningConfiguration": ScanningNotSupported, - // Deferred-feature surface (lifecycle, replication, tagging). + // Deferred-feature surface (lifecycle, replication). "PutLifecyclePolicy": PutLifecyclePolicy, "GetLifecyclePolicy": GetLifecyclePolicy, "DeleteLifecyclePolicy": DeleteLifecyclePolicy, @@ -99,9 +100,11 @@ var Actions = map[string]Handler{ "GetLifecyclePolicyPreview": NotImplemented, "PutReplicationConfiguration": NotImplemented, "ReplicateImage": NotImplemented, - "TagResource": NotImplemented, - "UntagResource": NotImplemented, - "ListTagsForResource": ListTagsForResource, + + // Resource tags. + "TagResource": TagResource, + "UntagResource": UntagResource, + "ListTagsForResource": ListTagsForResource, } // StubbedActionNames returns the Actions entries still bound to NotImplemented. diff --git a/spinifex/gateway/ecrapi/handler_test.go b/spinifex/gateway/ecrapi/handler_test.go index e55de0d57..c80460b4f 100644 --- a/spinifex/gateway/ecrapi/handler_test.go +++ b/spinifex/gateway/ecrapi/handler_test.go @@ -43,9 +43,12 @@ func TestScanningNotSupported(t *testing.T) { assert.Equal(t, awserrors.ErrorOperationNotSupported, err.Error()) } +// TestActions_ScanSurfaceUnsupported covers actions that reject every call +// outright. PutImageScanningConfiguration is excluded: it inspects the body, +// covered separately in imagescanning_test.go. func TestActions_ScanSurfaceUnsupported(t *testing.T) { scan := []string{ - "PutImageScanningConfiguration", "GetImageScanningConfiguration", + "GetImageScanningConfiguration", "StartImageScan", "DescribeImageScanFindings", "GetRegistryScanningConfiguration", "PutRegistryScanningConfiguration", "BatchGetRepositoryScanningConfiguration", diff --git a/spinifex/gateway/ecrapi/imagescanning.go b/spinifex/gateway/ecrapi/imagescanning.go new file mode 100644 index 000000000..d72c8d298 --- /dev/null +++ b/spinifex/gateway/ecrapi/imagescanning.go @@ -0,0 +1,69 @@ +package gateway_ecrapi + +import ( + "context" + "encoding/json" + "errors" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/ecr" + "github.com/mulgadc/spinifex/spinifex/awserrors" + handlers_ecr "github.com/mulgadc/spinifex/spinifex/handlers/ecr" + "github.com/nats-io/nats.go" +) + +// putImageScanningConfigurationRequest is the camelCase AWS JSON 1.1 input +// shape. The SDK input struct carries locationName tags rather than json +// tags, so the subset honored here is decoded explicitly. +type putImageScanningConfigurationRequest struct { + RepositoryName string `json:"repositoryName"` + RegistryID string `json:"registryId"` + ImageScanningConfiguration *imageScanningConfigurationInput `json:"imageScanningConfiguration"` +} + +// imageScanningConfigurationInput is the camelCase input shape for +// imageScanningConfiguration. +type imageScanningConfigurationInput struct { + ScanOnPush bool `json:"scanOnPush"` +} + +// PutImageScanningConfiguration accepts the honest, already-true state — +// scanOnPush unset or false — as a no-op. scanOnPush=true asks for a scanner +// mulga does not run and is rejected with OperationNotSupportedException. +func PutImageScanningConfiguration(ctx context.Context, nc *nats.Conn, accountID string, body []byte) (any, error) { + var req putImageScanningConfigurationRequest + if len(body) > 0 { + if err := json.Unmarshal(body, &req); err != nil { + return nil, errors.New(awserrors.ErrorInvalidParameterValue) + } + } + if req.RepositoryName == "" || handlers_ecr.ValidateRepoName(req.RepositoryName) != nil { + return nil, errors.New(awserrors.ErrorInvalidParameterValue) + } + if req.RegistryID != "" && req.RegistryID != accountID { + return nil, errors.New(awserrors.ErrorAccessDenied) + } + if req.ImageScanningConfiguration != nil && req.ImageScanningConfiguration.ScanOnPush { + return nil, errors.New(awserrors.ErrorOperationNotSupported) + } + + store := handlers_ecr.NewNATSMetaStore(nc) + meta, err := store.GetRepo(ctx, accountID, req.RepositoryName) + if err != nil { + if errors.Is(err, handlers_ecr.ErrNotFound) { + return nil, errors.New(awserrors.ErrorRepositoryNotFound) + } + return nil, err + } + meta.ScanOnPush = false + if err := store.PutRepo(ctx, accountID, meta); err != nil { + return nil, err + } + return &ecr.PutImageScanningConfigurationOutput{ + RegistryId: aws.String(accountID), + RepositoryName: aws.String(req.RepositoryName), + ImageScanningConfiguration: &ecr.ImageScanningConfiguration{ + ScanOnPush: aws.Bool(false), + }, + }, nil +} diff --git a/spinifex/gateway/ecrapi/imagescanning_test.go b/spinifex/gateway/ecrapi/imagescanning_test.go new file mode 100644 index 000000000..28cabc3c0 --- /dev/null +++ b/spinifex/gateway/ecrapi/imagescanning_test.go @@ -0,0 +1,91 @@ +package gateway_ecrapi + +//test:in-package — the connection and repository seeding helpers these share +// with the rest of the package's tests are unexported, as is the request shape +// the handler decodes. + +import ( + "context" + "testing" + + "github.com/aws/aws-sdk-go/service/ecr" + "github.com/mulgadc/spinifex/spinifex/awserrors" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestPutImageScanningConfiguration_NoOpSucceeds covers the honest, +// already-true state: scanOnPush unset or explicitly false succeeds, unlike +// the rest of the scan surface, which rejects every call unread. +func TestPutImageScanningConfiguration_NoOpSucceeds(t *testing.T) { + nc := newPolicyTestConn(t) + seedRepo(t, nc, "team/app") + + cases := []struct { + name, body string + }{ + {"absent", `{"repositoryName":"team/app"}`}, + {"explicit false", `{"repositoryName":"team/app","imageScanningConfiguration":{"scanOnPush":false}}`}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + out, err := PutImageScanningConfiguration(context.Background(), nc, policyTestAccount, []byte(tc.body)) + require.NoError(t, err) + res, ok := out.(*ecr.PutImageScanningConfigurationOutput) + require.True(t, ok) + assert.Equal(t, "team/app", *res.RepositoryName) + assert.Equal(t, policyTestAccount, *res.RegistryId) + require.NotNil(t, res.ImageScanningConfiguration) + assert.False(t, *res.ImageScanningConfiguration.ScanOnPush) + }) + } +} + +// TestPutImageScanningConfiguration_ScanOnPushRejected covers the real, +// narrow defect: scanOnPush=true still asks for a scanner that does not +// exist, and is rejected exactly like the rest of the scan surface. +func TestPutImageScanningConfiguration_ScanOnPushRejected(t *testing.T) { + nc := newPolicyTestConn(t) + seedRepo(t, nc, "team/app") + + out, err := PutImageScanningConfiguration(context.Background(), nc, policyTestAccount, + []byte(`{"repositoryName":"team/app","imageScanningConfiguration":{"scanOnPush":true}}`)) + assert.Nil(t, out) + require.Error(t, err) + assert.Equal(t, awserrors.ErrorOperationNotSupported, err.Error()) +} + +func TestPutImageScanningConfiguration_Errors(t *testing.T) { + nc := newPolicyTestConn(t) + seedRepo(t, nc, "team/app") + + cases := []struct { + name, body, expect string + }{ + {"missing repo", `{"repositoryName":"team/ghost"}`, awserrors.ErrorRepositoryNotFound}, + {"empty name", `{}`, awserrors.ErrorInvalidParameterValue}, + {"invalid name", `{"repositoryName":"Team/App"}`, awserrors.ErrorInvalidParameterValue}, + {"cross-account", `{"repositoryName":"team/app","registryId":"999999999999"}`, awserrors.ErrorAccessDenied}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + _, err := PutImageScanningConfiguration(context.Background(), nc, policyTestAccount, []byte(tc.body)) + require.Error(t, err) + assert.Equal(t, tc.expect, err.Error()) + }) + } +} + +// TestActions_PutImageScanningConfigurationDispatched confirms the action +// table now routes to the real handler rather than ScanningNotSupported. +func TestActions_PutImageScanningConfigurationDispatched(t *testing.T) { + h, ok := Actions["PutImageScanningConfiguration"] + require.True(t, ok) + nc := newPolicyTestConn(t) + seedRepo(t, nc, "team/app") + + out, err := h(context.Background(), nc, policyTestAccount, []byte(`{"repositoryName":"team/app"}`)) + require.NoError(t, err) + _, ok = out.(*ecr.PutImageScanningConfigurationOutput) + assert.True(t, ok) +} diff --git a/spinifex/gateway/ecrapi/tags.go b/spinifex/gateway/ecrapi/tags.go index 021e49824..61057c177 100644 --- a/spinifex/gateway/ecrapi/tags.go +++ b/spinifex/gateway/ecrapi/tags.go @@ -2,16 +2,135 @@ package gateway_ecrapi import ( "context" + "encoding/json" + "errors" + "sort" + "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ecr" + "github.com/mulgadc/spinifex/spinifex/awserrors" + handlers_ecr "github.com/mulgadc/spinifex/spinifex/handlers/ecr" "github.com/nats-io/nats.go" ) -// ListTagsForResource returns the resource tags for an ECR repository. Resource -// tagging is a deferred feature (TagResource/UntagResource are not implemented), -// so this returns an empty tag set. AWS clients such as the Terraform provider -// call ListTagsForResource on every repository read, so a 200 with no tags keeps -// those reads working without claiming tag support. -func ListTagsForResource(_ context.Context, _ *nats.Conn, _ string, _ []byte) (any, error) { - return &ecr.ListTagsForResourceOutput{Tags: []*ecr.Tag{}}, nil +// tagResourceRequest is the TagResource input shape. ecr.Tag carries no +// locationName, so its wire keys are Key/Value (capitalized); reusing the SDK +// type keeps that spelling correct rather than hand-rolling a lowercase one. +type tagResourceRequest struct { + ResourceArn string `json:"resourceArn"` + Tags []*ecr.Tag `json:"tags"` +} + +// untagResourceRequest is the UntagResource input shape. +type untagResourceRequest struct { + ResourceArn string `json:"resourceArn"` + TagKeys []string `json:"tagKeys"` +} + +// listTagsForResourceRequest is the ListTagsForResource input shape. +type listTagsForResourceRequest struct { + ResourceArn string `json:"resourceArn"` +} + +// resolveTaggedRepo parses resourceArn and confirms the repository it names +// exists in the caller account, returning the repo name, its current meta, +// 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, errors.New(awserrors.ErrorInvalidParameterValue) + } + name, err := RepositoryNameFromResourceARN(resourceArn) + if err != nil { + return "", handlers_ecr.RepoMeta{}, nil, err + } + store := handlers_ecr.NewNATSMetaStore(nc) + meta, err := store.GetRepo(ctx, accountID, name) + if err != nil { + if errors.Is(err, handlers_ecr.ErrNotFound) { + return "", handlers_ecr.RepoMeta{}, nil, errors.New(awserrors.ErrorRepositoryNotFound) + } + return "", handlers_ecr.RepoMeta{}, nil, err + } + return name, meta, store, nil +} + +// TagResource merges the supplied tags onto the resourceArn-named repository's +// tag map, matching keys overwriting their prior value. +func TagResource(ctx context.Context, nc *nats.Conn, accountID string, body []byte) (any, error) { + var req tagResourceRequest + if len(body) > 0 { + if err := json.Unmarshal(body, &req); err != nil { + return nil, errors.New(awserrors.ErrorInvalidParameterValue) + } + } + if len(req.Tags) == 0 { + return nil, errors.New(awserrors.ErrorInvalidParameterValue) + } + _, meta, store, err := resolveTaggedRepo(ctx, nc, accountID, req.ResourceArn) + if err != nil { + return nil, err + } + if meta.Tags == nil { + meta.Tags = make(map[string]string, len(req.Tags)) + } + for _, t := range req.Tags { + if t == nil || aws.StringValue(t.Key) == "" { + return nil, errors.New(awserrors.ErrorInvalidParameterValue) + } + meta.Tags[aws.StringValue(t.Key)] = aws.StringValue(t.Value) + } + if err := store.PutRepo(ctx, accountID, meta); err != nil { + return nil, err + } + return &ecr.TagResourceOutput{}, nil +} + +// UntagResource deletes the named tag keys from the resourceArn-named +// repository's tag map. Removing an absent key is a no-op, matching AWS. +func UntagResource(ctx context.Context, nc *nats.Conn, accountID string, body []byte) (any, error) { + var req untagResourceRequest + if len(body) > 0 { + if err := json.Unmarshal(body, &req); err != nil { + return nil, errors.New(awserrors.ErrorInvalidParameterValue) + } + } + if len(req.TagKeys) == 0 { + return nil, errors.New(awserrors.ErrorInvalidParameterValue) + } + _, meta, store, err := resolveTaggedRepo(ctx, nc, accountID, req.ResourceArn) + if err != nil { + return nil, err + } + for _, k := range req.TagKeys { + delete(meta.Tags, k) + } + if err := store.PutRepo(ctx, accountID, meta); err != nil { + return nil, err + } + return &ecr.UntagResourceOutput{}, nil +} + +// ListTagsForResource returns the tags stored on the resourceArn-named +// repository, sorted by key so repeated reads are stable. +func ListTagsForResource(ctx context.Context, nc *nats.Conn, accountID string, body []byte) (any, error) { + var req listTagsForResourceRequest + if len(body) > 0 { + if err := json.Unmarshal(body, &req); err != nil { + return nil, errors.New(awserrors.ErrorInvalidParameterValue) + } + } + _, meta, _, err := resolveTaggedRepo(ctx, nc, accountID, req.ResourceArn) + if err != nil { + return nil, err + } + keys := make([]string, 0, len(meta.Tags)) + for k := range meta.Tags { + keys = append(keys, k) + } + sort.Strings(keys) + tags := make([]*ecr.Tag, 0, len(keys)) + for _, k := range keys { + tags = append(tags, &ecr.Tag{Key: aws.String(k), Value: aws.String(meta.Tags[k])}) + } + return &ecr.ListTagsForResourceOutput{Tags: tags}, nil } diff --git a/spinifex/gateway/ecrapi/tags_test.go b/spinifex/gateway/ecrapi/tags_test.go index 7bc00a9db..10483a931 100644 --- a/spinifex/gateway/ecrapi/tags_test.go +++ b/spinifex/gateway/ecrapi/tags_test.go @@ -2,16 +2,29 @@ package gateway_ecrapi import ( "context" - "testing" "github.com/aws/aws-sdk-go/service/ecr" + "github.com/mulgadc/spinifex/spinifex/awserrors" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) +const tagsTestRegion = "us-east-1" + +func tagResourceARN(repo string) string { + return "arn:aws:ecr:" + tagsTestRegion + ":" + policyTestAccount + ":repository/" + repo +} + +// TestListTagsForResource_ReturnsEmpty now seeds a real repository: it +// resolves the resourceArn against the caller's account rather than +// answering every call with an unconditional empty set. func TestListTagsForResource_ReturnsEmpty(t *testing.T) { - out, err := ListTagsForResource(context.Background(), nil, "123456789012", []byte(`{"resourceArn":"arn:aws:ecr:ap-southeast-2:123456789012:repository/demo"}`)) + nc := newPolicyTestConn(t) + seedRepo(t, nc, "demo") + + out, err := ListTagsForResource(context.Background(), nc, policyTestAccount, + []byte(`{"resourceArn":"`+tagResourceARN("demo")+`"}`)) require.NoError(t, err) resp, ok := out.(*ecr.ListTagsForResourceOutput) @@ -20,9 +33,127 @@ func TestListTagsForResource_ReturnsEmpty(t *testing.T) { assert.Empty(t, resp.Tags) } +// TestListTagsForResource_RegisteredNotStub now supplies a resourceArn: the +// real handler needs an identifier to resolve, unlike the old empty stub. func TestListTagsForResource_RegisteredNotStub(t *testing.T) { h, ok := Actions["ListTagsForResource"] require.True(t, ok) - _, err := h(context.Background(), nil, "123456789012", []byte("{}")) + nc := newPolicyTestConn(t) + seedRepo(t, nc, "demo") + + _, err := h(context.Background(), nc, policyTestAccount, []byte(`{"resourceArn":"`+tagResourceARN("demo")+`"}`)) assert.NoError(t, err, "ListTagsForResource should not resolve to the 501 stub") } + +// TestTagResource_DecodesRealAWSWireFormat proves the decode against the wire +// shape AWS clients actually send: ecr.Tag has no locationName, so its JSON +// keys are Key/Value (capitalized), unlike other fields in this package. +func TestTagResource_DecodesRealAWSWireFormat(t *testing.T) { + nc := newPolicyTestConn(t) + seedRepo(t, nc, "team/app") + + body := []byte(`{"resourceArn":"` + tagResourceARN("team/app") + `","tags":[{"Key":"env","Value":"prod"},{"Key":"team","Value":"platform"}]}`) + out, err := TagResource(context.Background(), nc, policyTestAccount, body) + require.NoError(t, err) + _, ok := out.(*ecr.TagResourceOutput) + require.True(t, ok) + + listed, err := ListTagsForResource(context.Background(), nc, policyTestAccount, + []byte(`{"resourceArn":"`+tagResourceARN("team/app")+`"}`)) + require.NoError(t, err) + res, ok := listed.(*ecr.ListTagsForResourceOutput) + require.True(t, ok) + require.Len(t, res.Tags, 2) + assert.Equal(t, "env", *res.Tags[0].Key) + assert.Equal(t, "prod", *res.Tags[0].Value) + assert.Equal(t, "team", *res.Tags[1].Key) + assert.Equal(t, "platform", *res.Tags[1].Value) +} + +func TestTagResource_MergeAndOverwrite(t *testing.T) { + nc := newPolicyTestConn(t) + seedRepo(t, nc, "team/app") + arn := tagResourceARN("team/app") + + _, err := TagResource(context.Background(), nc, policyTestAccount, + []byte(`{"resourceArn":"`+arn+`","tags":[{"Key":"env","Value":"dev"}]}`)) + require.NoError(t, err) + + // A second call overwrites the existing key and adds a new one. + _, err = TagResource(context.Background(), nc, policyTestAccount, + []byte(`{"resourceArn":"`+arn+`","tags":[{"Key":"env","Value":"prod"},{"Key":"owner","Value":"team"}]}`)) + require.NoError(t, err) + + out, err := ListTagsForResource(context.Background(), nc, policyTestAccount, []byte(`{"resourceArn":"`+arn+`"}`)) + require.NoError(t, err) + res := out.(*ecr.ListTagsForResourceOutput) + require.Len(t, res.Tags, 2) + got := map[string]string{} + for _, tag := range res.Tags { + got[*tag.Key] = *tag.Value + } + assert.Equal(t, "prod", got["env"]) + assert.Equal(t, "team", got["owner"]) +} + +func TestUntagResource_RemovesKeysAndIsIdempotent(t *testing.T) { + nc := newPolicyTestConn(t) + seedRepo(t, nc, "team/app") + arn := tagResourceARN("team/app") + + _, err := TagResource(context.Background(), nc, policyTestAccount, + []byte(`{"resourceArn":"`+arn+`","tags":[{"Key":"env","Value":"prod"},{"Key":"owner","Value":"team"}]}`)) + require.NoError(t, err) + + _, err = UntagResource(context.Background(), nc, policyTestAccount, + []byte(`{"resourceArn":"`+arn+`","tagKeys":["env","missing"]}`)) + require.NoError(t, err) + + out, err := ListTagsForResource(context.Background(), nc, policyTestAccount, []byte(`{"resourceArn":"`+arn+`"}`)) + require.NoError(t, err) + res := out.(*ecr.ListTagsForResourceOutput) + require.Len(t, res.Tags, 1) + assert.Equal(t, "owner", *res.Tags[0].Key) +} + +func TestTagResource_Errors(t *testing.T) { + nc := newPolicyTestConn(t) + seedRepo(t, nc, "team/app") + + t.Run("no tags", func(t *testing.T) { + _, err := TagResource(context.Background(), nc, policyTestAccount, + []byte(`{"resourceArn":"`+tagResourceARN("team/app")+`","tags":[]}`)) + require.Error(t, err) + assert.Equal(t, awserrors.ErrorInvalidParameterValue, err.Error()) + }) + + 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, err.Error()) + }) + + t.Run("repo not found", func(t *testing.T) { + _, err := TagResource(context.Background(), nc, policyTestAccount, + []byte(`{"resourceArn":"`+tagResourceARN("team/ghost")+`","tags":[{"Key":"env","Value":"prod"}]}`)) + require.Error(t, err) + assert.Equal(t, awserrors.ErrorRepositoryNotFound, err.Error()) + }) + + 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.ErrorInvalidParameterValue, err.Error()) + }) +} + +func TestUntagResource_Errors(t *testing.T) { + nc := newPolicyTestConn(t) + + _, err := UntagResource(context.Background(), nc, policyTestAccount, + []byte(`{"resourceArn":"`+tagResourceARN("team/app")+`","tagKeys":[]}`)) + require.Error(t, err) + assert.Equal(t, awserrors.ErrorInvalidParameterValue, err.Error()) +} diff --git a/spinifex/handlers/ecr/meta.go b/spinifex/handlers/ecr/meta.go index 09ad448b1..09a5515d4 100644 --- a/spinifex/handlers/ecr/meta.go +++ b/spinifex/handlers/ecr/meta.go @@ -29,12 +29,27 @@ const ( TagMutabilityImmutable = "IMMUTABLE" ) +// Repository encryption types. AES256 is the only value CreateRepository +// accepts: predastore seals every stored fragment with AES-256-GCM regardless +// of caller input. KMS is rejected: no customer key material is plumbed here. +const ( + EncryptionTypeAES256 = "AES256" + EncryptionTypeKMS = "KMS" +) + // RepoMeta is the per-repository metadata record. type RepoMeta struct { Name string `json:"name"` CreatedAt time.Time `json:"createdAt"` // ImageTagMutability is MUTABLE or IMMUTABLE; empty == MUTABLE. ImageTagMutability string `json:"imageTagMutability,omitempty"` + // EncryptionType is always AES256 in practice; empty == AES256. + EncryptionType string `json:"encryptionType,omitempty"` + // ScanOnPush mirrors imageScanningConfiguration.scanOnPush; unset == false, + // and no path in this codebase ever sets it true. + ScanOnPush bool `json:"scanOnPush,omitempty"` + // Tags is the resource tag map managed by TagResource/UntagResource. + Tags map[string]string `json:"tags,omitempty"` } // TagMutability returns the repo's effective mutability, defaulting an unset @@ -46,6 +61,15 @@ func (m RepoMeta) TagMutability() string { return m.ImageTagMutability } +// EncryptionTypeOrDefault returns the repo's effective encryption type, +// defaulting an unset value to AES256. +func (m RepoMeta) EncryptionTypeOrDefault() string { + if m.EncryptionType == "" { + return EncryptionTypeAES256 + } + return m.EncryptionType +} + // ManifestMeta records a stored manifest's properties for shallow validation // of image indexes and Docker-Content-Digest responses. type ManifestMeta struct { diff --git a/spinifex/handlers/ecr/meta_test.go b/spinifex/handlers/ecr/meta_test.go index 5c8ec3914..bbce0f7be 100644 --- a/spinifex/handlers/ecr/meta_test.go +++ b/spinifex/handlers/ecr/meta_test.go @@ -2,6 +2,7 @@ package ecr import ( "context" + "encoding/json" "strings" "testing" @@ -11,6 +12,26 @@ import ( "github.com/stretchr/testify/require" ) +// TestRepoMeta_DecodesZeroValueRecord proves a KV record written before +// EncryptionType, ScanOnPush, and Tags existed still decodes cleanly: every +// new field is omitempty, so an old record simply zero-values them. +func TestRepoMeta_DecodesZeroValueRecord(t *testing.T) { + const oldRecord = `{"name":"team/app","createdAt":"2024-01-01T00:00:00Z","imageTagMutability":"MUTABLE"}` + var m RepoMeta + require.NoError(t, json.Unmarshal([]byte(oldRecord), &m)) + assert.Equal(t, "team/app", m.Name) + assert.Equal(t, "MUTABLE", m.ImageTagMutability) + assert.Empty(t, m.EncryptionType) + assert.Equal(t, EncryptionTypeAES256, m.EncryptionTypeOrDefault()) + assert.False(t, m.ScanOnPush) + assert.Nil(t, m.Tags) +} + +func TestRepoMeta_EncryptionTypeOrDefault(t *testing.T) { + assert.Equal(t, EncryptionTypeAES256, RepoMeta{}.EncryptionTypeOrDefault()) + assert.Equal(t, EncryptionTypeAES256, RepoMeta{EncryptionType: EncryptionTypeAES256}.EncryptionTypeOrDefault()) +} + func TestValidateRepoName(t *testing.T) { valid := []string{"ab", "team/app", "a/b/c", "my-repo.name_1", "x0/y1/z2"} for _, n := range valid {