Skip to content

Comments

fix(batch): support repository credentials for private registries#37017

Open
matoom-nomu wants to merge 2 commits intoaws:mainfrom
matoom-nomu:fix/batch-repo-credentials
Open

fix(batch): support repository credentials for private registries#37017
matoom-nomu wants to merge 2 commits intoaws:mainfrom
matoom-nomu:fix/batch-repo-credentials

Conversation

@matoom-nomu
Copy link
Contributor

@matoom-nomu matoom-nomu commented Feb 18, 2026 •
edited
Loading

Issue

Closes #37003

Reason for this change

AWS Batch ECS container definitions do not support repository credentials for pulling images from private registries other than ECR. The credentials parameter passed to ContainerImage.fromRegistry() is currently ignored.

Description of changes

I modified EcsContainerDefinitionBase._renderContainerDefinition() to include repositoryCredentials property in the CloudFormation template

Describe any new or updated permissions being added

None

Description of how you validated changes

Add unit test

Checklist

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

@aws-cdk-automation aws-cdk-automation requested a review from a team February 18, 2026 07:03
@github-actions github-actions bot added effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2 valued-contributor [Pilot] contributed between 6-12 PRs to the CDK labels Feb 18, 2026
@matoom-nomu matoom-nomu changed the title respects repository credentials for private registries fix(batch): support repository credentials for private registries Feb 18, 2026
aws-cdk-automation
aws-cdk-automation previously requested changes Feb 18, 2026
View reviewed changes
Copy link
Collaborator

@aws-cdk-automation aws-cdk-automation left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(This review is outdated)

@matoom-nomu
Copy link
Contributor Author

matoom-nomu commented Feb 18, 2026

Exemption Request

Unit test verify cfn template has RepositoryCredentialsand and proper IAM permissions for the secret.

@aws-cdk-automation aws-cdk-automation added pr-linter/exemption-requested The contributor has requested an exemption to the PR Linter feedback. pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. labels Feb 18, 2026
lpizzinidev
lpizzinidev suggested changes Feb 18, 2026
View reviewed changes
Copy link
Contributor

@lpizzinidev lpizzinidev left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ty!

#37017 (comment) It would be nice to have integration test coverage for the scenario.

packages/aws-cdk-lib/aws-batch/lib/ecs-container-definition.ts
return {
image: this.imageConfig.imageName,
repositoryCredentials: this.imageConfig.repositoryCredentials ? {
credentialsParameter: this.imageConfig.repositoryCredentials.credentialsParameter!,
Copy link
Contributor

@lpizzinidev lpizzinidev Feb 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since credentialsParameter is required if repositoryCredentials is set (docs), I'd add validation to verify such requirement to prevent unhandled exceptions.

Copy link
Contributor Author

@matoom-nomu matoom-nomu Feb 19, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review!
I added validation to the constructor.

@aws-cdk-automation aws-cdk-automation removed the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Feb 18, 2026
@aws-cdk-automation aws-cdk-automation dismissed their stale review February 19, 2026 02:45

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 19, 2026 •
edited
Loading

⚠️ Experimental Feature: This security report is currently in experimental phase. Results may include false positives and the rules are being actively refined.
Please try merge from main to avoid findings unrelated to the PR.

TestsPassed ✅SkippedFailed
Security Guardian Results24 ran24 passed
TestResult
No test annotations available

@github-actions
Copy link
Contributor

github-actions bot commented Feb 19, 2026 •
edited
Loading

⚠️ Experimental Feature: This security report is currently in experimental phase. Results may include false positives and the rules are being actively refined.
Please try merge from main to avoid findings unrelated to the PR.

TestsPassed ✅SkippedFailed
Security Guardian Results with resolved templates24 ran24 passed
TestResult
No test annotations available

@matoom-nomu
Copy link
Contributor Author

matoom-nomu commented Feb 19, 2026

Ty!

#37017 (comment) It would be nice to have integration test coverage for the scenario.

I added integ-test and confirmed Credential is properly set in jobdfefinition cfn.

lpizzinidev
lpizzinidev approved these changes Feb 19, 2026
View reviewed changes
Copy link
Contributor

@lpizzinidev lpizzinidev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ty!

@aws-cdk-automation aws-cdk-automation added the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Feb 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aws-cdk-automation aws-cdk-automation aws-cdk-automation left review comments

+1 more reviewer

@lpizzinidev lpizzinidev lpizzinidev approved these changes

Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2 pr/needs-maintainer-review This PR needs a review from a Core Team Member pr-linter/exemption-requested The contributor has requested an exemption to the PR Linter feedback. valued-contributor [Pilot] contributed between 6-12 PRs to the CDK

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

(aws-batch): EcsFargateContainerDefinition does not respect ContainerImageConfig.repositoryCredentials to pull images from private registries

3 participants

@matoom-nomu @lpizzinidev @aws-cdk-automation