diff --git a/spinifex/domains/ecr/awsapi/batch_delete_image.go b/spinifex/domains/ecr/awsapi/batch_delete_image.go new file mode 100644 index 000000000..572ee6570 --- /dev/null +++ b/spinifex/domains/ecr/awsapi/batch_delete_image.go @@ -0,0 +1,66 @@ +package awsapi + +import ( + "context" + "encoding/json" + "errors" + "log/slog" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/ecr" + ecrregistry "github.com/mulgadc/spinifex/spinifex/domains/ecr/registry" + "github.com/mulgadc/spinifex/spinifex/foundation/aws/errors" +) + +type batchDeleteImageRequest struct { + RepositoryName string `json:"repositoryName"` + RegistryID string `json:"registryId"` + ImageIDs []imageIdentifier `json:"imageIds"` +} + +// BatchDeleteImage removes tag pointers or manifests by digest. A tag removes +// only that pointer; a digest removes its manifest and all tags resolving to +// it. Per ECR's batch contract, individual misses are structured failures in a +// successful response rather than a failed request. +func BatchDeleteImage(ctx context.Context, deleter ImageDeleter, accountID string, body []byte) (*ecr.BatchDeleteImageOutput, error) { + if deleter == nil { + return nil, errors.New(awserrors.ErrorServerInternal) + } + var req batchDeleteImageRequest + if err := json.Unmarshal(body, &req); err != nil { + return nil, MalformedBodyError() + } + if err := ValidateRepositoryScope(req.RepositoryName, req.RegistryID, accountID); err != nil { + return nil, err + } + if len(req.ImageIDs) > maxImageBatch { + return nil, MaxItemsError("imageIds", maxImageBatch) + } + + deleted := make([]*ecr.ImageIdentifier, 0, len(req.ImageIDs)) + failures := make([]*ecr.ImageFailure, 0) + for _, imageID := range req.ImageIDs { + if imageID.ImageDigest == "" && imageID.ImageTag == "" { + failures = append(failures, imageFailure(imageID, ecr.ImageFailureCodeMissingDigestAndTag, "no digest or tag specified")) + continue + } + + digest, err := deleter.DeleteImage(ctx, accountID, req.RepositoryName, imageID.ImageTag, imageID.ImageDigest) + if errors.Is(err, ecrregistry.ErrImageNotFound) { + failures = append(failures, imageFailure(imageID, ecr.ImageFailureCodeImageNotFound, "image not found")) + continue + } + if err != nil { + slog.ErrorContext(ctx, "ECR BatchDeleteImage: delete image failed", "repository", req.RepositoryName, "err", err) + return nil, errors.New(awserrors.ErrorServerInternal) + } + + deletedImage := &ecr.ImageIdentifier{ImageDigest: aws.String(digest)} + if imageID.ImageTag != "" { + deletedImage.ImageTag = aws.String(imageID.ImageTag) + } + deleted = append(deleted, deletedImage) + } + + return &ecr.BatchDeleteImageOutput{ImageIds: deleted, Failures: failures}, nil +} diff --git a/spinifex/domains/ecr/awsapi/batch_delete_image_test.go b/spinifex/domains/ecr/awsapi/batch_delete_image_test.go new file mode 100644 index 000000000..dcac4c563 --- /dev/null +++ b/spinifex/domains/ecr/awsapi/batch_delete_image_test.go @@ -0,0 +1,72 @@ +package awsapi + +import ( + "context" + "errors" + "testing" + + "github.com/aws/aws-sdk-go/service/ecr" + ecrregistry "github.com/mulgadc/spinifex/spinifex/domains/ecr/registry" + "github.com/mulgadc/spinifex/spinifex/foundation/aws/errors" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +type fakeImageDeleter struct { + results map[string]fakeDeleteResult +} + +type fakeDeleteResult struct { + digest string + err error +} + +func (d fakeImageDeleter) DeleteImage(_ context.Context, _ string, _ string, tag, digest string) (string, error) { + reference := digest + if reference == "" { + reference = tag + } + result, ok := d.results[reference] + if !ok { + return "", ecrregistry.ErrImageNotFound + } + return result.digest, result.err +} + +func TestBatchDeleteImage_ReturnsDeletedImagesAndPerItemFailures(t *testing.T) { + deleter := fakeImageDeleter{results: map[string]fakeDeleteResult{ + "sha256:present": {digest: "sha256:present"}, + }} + body := []byte(`{"repositoryName":"team/app","imageIds":[{"imageDigest":"sha256:present"},{"imageTag":"missing"},{}]}`) + + out, err := BatchDeleteImage(context.Background(), deleter, "123456789012", body) + require.NoError(t, err) + require.Len(t, out.ImageIds, 1) + assert.Equal(t, "sha256:present", *out.ImageIds[0].ImageDigest) + require.Len(t, out.Failures, 2) + assert.Equal(t, ecr.ImageFailureCodeImageNotFound, *out.Failures[0].FailureCode) + assert.Equal(t, ecr.ImageFailureCodeMissingDigestAndTag, *out.Failures[1].FailureCode) +} + +func TestBatchDeleteImage_ValidatesRequestAndDeleterFailure(t *testing.T) { + tooMany := `{"repositoryName":"team/app","imageIds":[` + repeatImageIdentifier(101) + `]}` + cases := []struct { + name string + deleter ImageDeleter + body string + code string + }{ + {"missing deleter", nil, `{"repositoryName":"team/app"}`, awserrors.ErrorServerInternal}, + {"malformed", fakeImageDeleter{}, `{`, awserrors.ErrorInvalidParameterValue}, + {"cross account", fakeImageDeleter{}, `{"repositoryName":"team/app","registryId":"999999999999"}`, awserrors.ErrorAccessDenied}, + {"cap exceeded", fakeImageDeleter{}, tooMany, awserrors.ErrorInvalidParameterValue}, + {"backend failure", fakeImageDeleter{results: map[string]fakeDeleteResult{"present": {err: errors.New("unavailable")}}}, `{"repositoryName":"team/app","imageIds":[{"imageTag":"present"}]}`, awserrors.ErrorServerInternal}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + _, err := BatchDeleteImage(context.Background(), tc.deleter, "123456789012", []byte(tc.body)) + require.Error(t, err) + assert.Equal(t, tc.code, awserrors.ValidErrorCodeFromError(err)) + }) + } +} diff --git a/spinifex/domains/ecr/awsapi/batch_get_image.go b/spinifex/domains/ecr/awsapi/batch_get_image.go index c08b10baa..545134a7e 100644 --- a/spinifex/domains/ecr/awsapi/batch_get_image.go +++ b/spinifex/domains/ecr/awsapi/batch_get_image.go @@ -33,7 +33,7 @@ func BatchGetImage(ctx context.Context, reader ManifestReader, accountID string, if err := json.Unmarshal(body, &req); err != nil { return nil, MalformedBodyError() } - if err := validateRepositoryScope(req.RepositoryName, req.RegistryID, accountID); err != nil { + if err := ValidateRepositoryScope(req.RepositoryName, req.RegistryID, accountID); err != nil { return nil, err } if len(req.ImageIDs) > maxImageBatch { diff --git a/spinifex/domains/ecr/awsapi/describe_images.go b/spinifex/domains/ecr/awsapi/describe_images.go index ba35cd732..64c6d7bdd 100644 --- a/spinifex/domains/ecr/awsapi/describe_images.go +++ b/spinifex/domains/ecr/awsapi/describe_images.go @@ -36,7 +36,7 @@ func DescribeImages(ctx context.Context, catalog ImageCatalog, accountID string, if err := json.Unmarshal(body, &req); err != nil { return nil, MalformedBodyError() } - if err := validateRepositoryScope(req.RepositoryName, req.RegistryID, accountID); err != nil { + if err := ValidateRepositoryScope(req.RepositoryName, req.RegistryID, accountID); err != nil { return nil, err } diff --git a/spinifex/domains/ecr/awsapi/image_catalog.go b/spinifex/domains/ecr/awsapi/image_catalog.go index 8c33cb649..ba754bb06 100644 --- a/spinifex/domains/ecr/awsapi/image_catalog.go +++ b/spinifex/domains/ecr/awsapi/image_catalog.go @@ -31,7 +31,17 @@ type ManifestWriter interface { StoreManifest(ctx context.Context, account, repository, reference, contentType string, body []byte) (digest string, err error) } -func validateRepositoryScope(repositoryName, registryID, accountID string) error { +// ImageDeleter is the registry capability used by BatchDeleteImage. The +// registry owns tag-versus-digest deletion and storage reclamation; this action +// owns ECR's batch response shape. +type ImageDeleter interface { + DeleteImage(ctx context.Context, account, repository, tag, digest string) (resolvedDigest string, err error) +} + +// ValidateRepositoryScope validates a repository name and refuses a registryId +// that names another account. AWS ECR action adapters and gateway-resident +// transitional actions share this account-boundary rule. +func ValidateRepositoryScope(repositoryName, registryID, accountID string) error { if err := ValidateRepositoryName(repositoryName); err != nil { return err } diff --git a/spinifex/domains/ecr/awsapi/list_images.go b/spinifex/domains/ecr/awsapi/list_images.go index a6257306f..ae23700eb 100644 --- a/spinifex/domains/ecr/awsapi/list_images.go +++ b/spinifex/domains/ecr/awsapi/list_images.go @@ -26,7 +26,7 @@ func ListImages(ctx context.Context, catalog ImageCatalog, accountID string, bod if err := json.Unmarshal(body, &req); err != nil { return nil, MalformedBodyError() } - if err := validateRepositoryScope(req.RepositoryName, req.RegistryID, accountID); err != nil { + if err := ValidateRepositoryScope(req.RepositoryName, req.RegistryID, accountID); err != nil { return nil, err } diff --git a/spinifex/domains/ecr/awsapi/put_image.go b/spinifex/domains/ecr/awsapi/put_image.go index 3023b7326..ccbb1c746 100644 --- a/spinifex/domains/ecr/awsapi/put_image.go +++ b/spinifex/domains/ecr/awsapi/put_image.go @@ -33,7 +33,7 @@ func PutImage(ctx context.Context, writer ManifestWriter, accountID string, body if err := json.Unmarshal(body, &req); err != nil { return nil, MalformedBodyError() } - if err := validateRepositoryScope(req.RepositoryName, req.RegistryID, accountID); err != nil { + if err := ValidateRepositoryScope(req.RepositoryName, req.RegistryID, accountID); err != nil { return nil, err } if req.ImageManifest == "" { diff --git a/spinifex/gateway/ecr_image.go b/spinifex/gateway/ecr_image.go index 3e8757b5e..9a3171c8c 100644 --- a/spinifex/gateway/ecr_image.go +++ b/spinifex/gateway/ecr_image.go @@ -1,39 +1,17 @@ package gateway import ( - "encoding/json" "errors" "io" "log/slog" "net/http" - "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/service/ecr" awsapi "github.com/mulgadc/spinifex/spinifex/domains/ecr/awsapi" - ecrregistry "github.com/mulgadc/spinifex/spinifex/domains/ecr/registry" "github.com/mulgadc/spinifex/spinifex/foundation/aws/errors" ) -// maxImageBatch is the per-call cap on imageIds for the batch image actions. -const maxImageBatch = 100 - -// imageIdentifier is the AWS JSON 1.1 {imageDigest, imageTag} pair used by -// the remaining gateway image-action adapters. It will leave with those -// actions; DescribeImages has its domain-local input shape already. -type imageIdentifier struct { - ImageDigest string `json:"imageDigest"` - ImageTag string `json:"imageTag"` -} - -type batchDeleteImageRequest struct { - RepositoryName string `json:"repositoryName"` - RegistryID string `json:"registryId"` - ImageIds []imageIdentifier `json:"imageIds"` -} - -// ecrImageAccount reads the auth-context account and requires the OCI registry -// to be wired. Image actions need the gateway-side predastore Store, so a nil -// registry is a server fault. +// ecrImageAccount reads the auth-context account and requires the composed OCI +// registry capability. A missing registry is a deployment/composition fault. func (gw *GatewayConfig) ecrImageAccount(r *http.Request) (string, error) { accountID, _ := r.Context().Value(ctxAccountID).(string) if accountID == "" { @@ -47,29 +25,6 @@ func (gw *GatewayConfig) ecrImageAccount(r *http.Request) (string, error) { return accountID, nil } -// validateRepoAndRegistry rejects a malformed repository name and a registryId -// targeting another account. -func validateRepoAndRegistry(name, registryID, accountID string) error { - if err := awsapi.ValidateRepositoryName(name); err != nil { - return err - } - if registryID != "" && registryID != accountID { - return errors.New(awserrors.ErrorAccessDenied) - } - return nil -} - -func decodeJSONBody(r *http.Request, dst any) error { - body, err := io.ReadAll(r.Body) - if err != nil { - return awsapi.MalformedBodyError() - } - if err := json.Unmarshal(body, dst); err != nil { - return awsapi.MalformedBodyError() - } - return nil -} - // handleListImages adapts the authenticated HTTP request to the ECR ListImages // action. The ECR AWS adapter owns request and response semantics; gateway // supplies the configured OCI registry capability. @@ -154,66 +109,23 @@ func (gw *GatewayConfig) handlePutImage(w http.ResponseWriter, r *http.Request) return nil } -// handleBatchDeleteImage deletes images by tag or digest. A tag removes only -// that tag pointer; a digest removes the manifest and every tag pointing at it. -// Partial failures are reported in the failures array with HTTP 200. +// handleBatchDeleteImage adapts the authenticated HTTP request to the ECR AWS +// action. The ECR adapter owns batch partial-failure semantics; gateway +// supplies the configured OCI registry capability. func (gw *GatewayConfig) handleBatchDeleteImage(w http.ResponseWriter, r *http.Request) error { ctx := r.Context() accountID, err := gw.ecrImageAccount(r) if err != nil { return err } - var req batchDeleteImageRequest - if err := decodeJSONBody(r, &req); err != nil { + body, err := io.ReadAll(r.Body) + if err != nil { + return awsapi.MalformedBodyError() + } + output, err := awsapi.BatchDeleteImage(ctx, gw.ECRRegistry, accountID, body) + if err != nil { return err } - if err := validateRepoAndRegistry(req.RepositoryName, req.RegistryID, accountID); err != nil { - return err - } - if len(req.ImageIds) > maxImageBatch { - return awsapi.MaxItemsError("imageIds", maxImageBatch) - } - - var deleted []*ecr.ImageIdentifier - var failures []*ecr.ImageFailure - for _, id := range req.ImageIds { - if id.ImageDigest == "" && id.ImageTag == "" { - failures = append(failures, imageFailure(id, ecr.ImageFailureCodeMissingDigestAndTag, "no digest or tag specified")) - continue - } - - digest, dErr := gw.ECRRegistry.DeleteImage(ctx, accountID, req.RepositoryName, id.ImageTag, id.ImageDigest) - if errors.Is(dErr, ecrregistry.ErrImageNotFound) { - failures = append(failures, imageFailure(id, ecr.ImageFailureCodeImageNotFound, "image not found")) - continue - } - if dErr != nil { - slog.ErrorContext(ctx, "BatchDeleteImage: delete failed", "repo", req.RepositoryName, "err", dErr) - return errors.New(awserrors.ErrorServerInternal) - } - - out := &ecr.ImageIdentifier{ImageDigest: aws.String(digest)} - if id.ImageTag != "" { - out.ImageTag = aws.String(id.ImageTag) - } - deleted = append(deleted, out) - } - - awsapi.WriteJSONResponse(w, &ecr.BatchDeleteImageOutput{ImageIds: deleted, Failures: failures}) + awsapi.WriteJSONResponse(w, output) return nil } - -func imageFailure(id imageIdentifier, code, reason string) *ecr.ImageFailure { - failure := &ecr.ImageFailure{ - FailureCode: aws.String(code), - FailureReason: aws.String(reason), - ImageId: &ecr.ImageIdentifier{}, - } - if id.ImageDigest != "" { - failure.ImageId.ImageDigest = aws.String(id.ImageDigest) - } - if id.ImageTag != "" { - failure.ImageId.ImageTag = aws.String(id.ImageTag) - } - return failure -} diff --git a/spinifex/gateway/ecr_lifecyclepreview.go b/spinifex/gateway/ecr_lifecyclepreview.go index 3740e5185..33e218b3e 100644 --- a/spinifex/gateway/ecr_lifecyclepreview.go +++ b/spinifex/gateway/ecr_lifecyclepreview.go @@ -34,7 +34,7 @@ func (gw *GatewayConfig) evaluateLifecyclePreview(r *http.Request) (string, []ha if err := decodeJSONBody(r, &req); err != nil { return "", nil, nil, err } - if err := validateRepoAndRegistry(req.RepositoryName, req.RegistryID, accountID); err != nil { + if err := awsapi.ValidateRepositoryScope(req.RepositoryName, req.RegistryID, accountID); err != nil { return "", nil, nil, err } diff --git a/spinifex/gateway/ecr_request.go b/spinifex/gateway/ecr_request.go new file mode 100644 index 000000000..68f6de7a1 --- /dev/null +++ b/spinifex/gateway/ecr_request.go @@ -0,0 +1,24 @@ +package gateway + +import ( + "encoding/json" + "io" + "net/http" + + awsapi "github.com/mulgadc/spinifex/spinifex/domains/ecr/awsapi" +) + +// decodeJSONBody is the generic gateway HTTP adapter for ECR JSON 1.1 action +// wrappers that are not yet direct awsapi handlers. ECR request semantics stay +// in the receiving action; this helper only turns a readable HTTP body into a +// Go request shape. +func decodeJSONBody(r *http.Request, dst any) error { + body, err := io.ReadAll(r.Body) + if err != nil { + return awsapi.MalformedBodyError() + } + if err := json.Unmarshal(body, dst); err != nil { + return awsapi.MalformedBodyError() + } + return nil +}