-
Notifications
You must be signed in to change notification settings - Fork 4.3k
feat(bedrock-agentcore): add fromImageUri method to AgentRuntimeArtifact #36263
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(This review is outdated)
|
Exemption Request This doesn't need an integration test. It's adding another way to pass a container URI to the same CloudFormation property that The method is a simple pass-through. It takes a string and sets Unit tests cover the behavior (URI reference, CloudFormation tokens, no auto-permissions). |
packages/@aws-cdk/aws-bedrock-agentcore-alpha/lib/runtime/runtime-artifact.ts
Outdated
Show resolved
Hide resolved
packages/@aws-cdk/aws-bedrock-agentcore-alpha/lib/runtime/runtime-artifact.ts
Show resolved
Hide resolved
packages/@aws-cdk/aws-bedrock-agentcore-alpha/lib/runtime/runtime-artifact.ts
Show resolved
Hide resolved
|
@alvazjor Thanks for the feedback. Reading the container uri pattern requirement shows it only accepts ECR which means that external repos unfortunately can't be used. Maybe it's worthwhile to eventually enable this from the Bedrock agentcore side. At least for my use case, I'm still happy to have this change in. Updated the documentation and comments to reflect ECR only containers are supported. |
✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.
|
@kimsbrian there is a linting issue in the PR, that is why the build keeps failing |
Pull request has been modified.
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
|
Comments on closed issues and PRs are hard for our team to see. |
Issue # (if applicable)
N/A
Reason for this change
Users currently need ECR repository constructs (
fromEcrRepository) or local Docker assets (fromAsset) to reference container images in AgentCore runtimes. This creates friction when:Description of changes
Added
fromImageUri()static method toAgentRuntimeArtifactclass:ImageUriArtifactclass extendingAgentRuntimeArtifactEcrImageandAssetImageclassesThis provides a fourth deployment option alongside
fromEcrRepository(),fromAsset(), andfromS3().Description of how you validated changes
Unit tests added:
Manual testing:
bash
cd packages/@aws-cdk/aws-bedrock-agentcore-alpha
yarn build
yarn test
All tests pass.
Production usage:
This change has been built locally and used successfully in my application to reference container images via CloudFormation parameters.
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license