aws-apprunner-alpha: redesign Service construct to unify Source.fromEcr() and Source.fromEcrPublic() methods

[garysassano]

Describe the bug

The current design of the Service construct in aws-apprunner-alpha has significant inconsistencies, particularly between Source.fromEcr and Source.fromEcrPublic. This issue proposes a redesign to unify these methods into a single interface, dynamically determining the repository type (public or private) using regex. For context, see the related issue: #32974

Regression Issue

• Select this option if this issue appears to be a regression.

Last Known Working CDK Version

No response

Expected Behavior

see above

Current Behavior

see above

Reproduction Steps

see above

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

2.176.0

Framework Version

No response

Node.js Version

22.12.0

OS

Ubuntu 24.04.1

Language

TypeScript

Language Version

No response

Other information

No response

[pahud]

Hi @garysassano

I read your comment. The reason is ECR and ECR Public are actually two different services and I agree we should add validations when necessary. I am not sure if unifying them into a single method/interface would be a good idea as they are not just the difference in URI but ECR requires more authorization than ECR Public and absolutely we can programmatically control that. Given this is still an alpha module, we welcome any PRs or high level API samples that allows the team to get more insights to move this forward. wdyt? I am making this a FR.

[garysassano]

Hi @pahud

Thanks for the response and for acknowledging the need for validation. If the separation between ECR and ECR_PUBLIC must remain, I'd suggest setting this distinction directly at the Service level rather than at the Source level. By doing so, you could eliminate props like accessRole for ECR_PUBLIC entirely if they're not needed, rather than allowing them to be specified and then silently ignored. This would prevent confusion and improve the developer experience.

Even with the separation, the interface should be streamlined. It's inconsistent to use repository (an IRepository) + tagDigest (a string) for ECR repositories but imageIdentifier (a string) for ECR_PUBLIC repositories. A unified way to specifying the Docker image would simplify the construct significantly.

The approach could mirror how constructs like HealthCheck work. For example, you can define a health check using HealthCheck.http or HealthCheck.tcp:

healthCheck: HealthCheck.http({
  path: "/health",
  healthyThreshold: 1,
  unhealthyThreshold: 5,
}),

healthCheck: HealthCheck.tcp({
  healthyThreshold: 1,
  unhealthyThreshold: 5,
}),

Note that HealthCheck.http doesn't have a path prop.

In the same way, you could have Service.ecr and Service.ecr_public to make the API more intuitive and consistent.