Skip to content

add check for kms key in token vault before create one#339

Open
jesseturner21 wants to merge 1 commit intomainfrom
kmskeytokenvaultcheck
Open

add check for kms key in token vault before create one#339
jesseturner21 wants to merge 1 commit intomainfrom
kmskeytokenvaultcheck

Conversation

@jesseturner21
Copy link
Contributor

@jesseturner21 jesseturner21 commented Feb 18, 2026

Description

There is a bug in the deploy command.

Every time we deploy it creates a new kms key for the token vault in identity. This is very bad as it leaves a orphaned kms keys in the users account costing $1 a month per key.

This checks if there is a kms key in token vault already and uses the one already there

Related Issue

#337

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update
  • Other (please describe):

Testing

How have you tested the change?

  • I ran npm run test:unit and npm run test:integ
  • I ran npm run typecheck
  • I ran npm run lint
  • If I modified src/assets/, I ran npm run test:update-snapshots and committed the updated snapshots

Checklist

  • I have read the CONTRIBUTING document
  • I have added any necessary tests that prove my fix is effective or my feature works
  • I have updated the documentation accordingly
  • I have added an appropriate example to the documentation to outline the feature, or no new docs are needed
  • My changes generate no new warnings
  • Any dependent changes have been merged and published

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the
terms of your choice.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 18, 2026

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 41.66% 2683 / 6439
🔵 Statements 41.09% 2812 / 6842
🔵 Functions 43.67% 573 / 1312
🔵 Branches 47.03% 1775 / 3774
Generated in workflow #427 for commit 1ee642f by the Vitest Coverage Report Action

Comment on lines +97 to +98
const controlClient = new BedrockAgentCoreControlClient({ region, credentials });

Copy link
Contributor

Choose a reason for hiding this comment

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

NIT - Can we create a single BedrockAgentCoreControl client in https://github.com/aws/agentcore-cli/blob/main/src/cli/aws/agentcore-control.ts and move this API call there. We don't want many client initializations peppered through the codebase. Makes it easier to make config changes or debug if its all in one place

Copy link
Contributor

Choose a reason for hiding this comment

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

Same ^

Otherwise, good bug catch and thanks for the fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Comments