feat(bma): pass CDK managed role to BMA session - #2497
Open
nborges-aws wants to merge 1 commit into
Open
nborges-aws wants to merge 1 commit into
nborges-aws wants to merge 1 commit into
Conversation
Contributor
|
Claude Security Review: no high-confidence findings. (run) |
Contributor
There was a problem hiding this comment.
AgentCore Harness Review
Verdict: Looks good
Nice, well-scoped change. The schema addition, CDK output capture + state recording, graceful error for an outdated @aws/agentcore-cdk, and the simplified China-region gate are all tested. client.py correctly resolves the project root (parents[2] from app/<runtime>/client.py lands at the project root), the state-file path matches DEPLOYED_STATE_RELATIVE_PATH, and the removal path (bedrockManagedAgents: false) clearing bmaSession works because the shallow spread drops undefined keys at JSON.stringify time.
A couple of minor things worth being aware of (not blockers):
src/core/project/backends/cdk.ts~L374–402:updateTargetStateclearsbmaSessionbeforedescribeStack, so a transient CloudFormation error (as opposed to missing outputs) will leave the project with no recorded BMA role until the next successful deploy. Acceptable since the user retries, but you could narrow the window by only clearing whenbmaRuntimes.length === 0and otherwise overwriting in the single finalupdateTargetState.- Dropping the tag/policy heuristics in
manager.tsxmeans projects scaffolded by an older CLI that still have theagentcore:template=BedrockManagedAgentstag +bma-acr-policy.jsonbut nobedrockManagedAgents: truewill no longer be blocked from deploying to China and won't get a session role recorded. That seems intentional (the China deploy will fail at CFN time anyway), but worth confirming there's a migration note or that no in-the-wild projects fall into that gap.
Neither needs changes before merging.
aidandaly24
approved these changes
Oct 1, 2026
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Add
bedrockManagedAgentsto the runtime configuration and set it in newly scaffolded BMA projects. After deployment, capture the CDK-managed session role ARN and associated runtime ARNs in deployed state. This PR also updates the BMA client to select and pass the role for its runtime when creating a session. This client no longer creates or modifies the IAM role.Type of Change
Testing
How have you tested the change?
bun run test(3955 pass, 0 fail)npm run test:unitandnpm run test:integnpm run typechecknpm run lintsrc/assets/, I rannpm run test:update-snapshotsand committed the updated snapshotsChecklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the
terms of your choice.