-
Notifications
You must be signed in to change notification settings - Fork 194
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
refactor(common,cli): kms deployer gets keyId from environment #2760
Merged
Merged
Changes from all commits
Commits
Show all changes
20 commits
Select commit
Hold shift + click to select a range
53ea173
refactor(common,cli): kms deployer gets keyId from environment
yonadaaa edb762b
docs: with instead of wtih
yonadaaa e173987
fix: empty object
yonadaaa 5ecee2e
chore: description
yonadaaa 5793e83
refactor: get address arg type
yonadaaa 3906037
refactor: default to environment variable
yonadaaa dffe56d
fix: cast
yonadaaa ad55b82
fix: dev contracts flag
yonadaaa 6a67071
refactor: kms flag
yonadaaa 168600d
refactor: if env variable undefined
yonadaaa 99b287d
chore: changeset
yonadaaa 8d5a78a
chore: tweak changeset
yonadaaa b02fb29
Merge remote-tracking branch 'origin/main' into yonadaaa/kms-bool-flag
yonadaaa 3ef20b9
fix: throw if no keyId
yonadaaa 55f00b2
refactor: dont need environment variable in rundeploy
yonadaaa 2c82434
refactor: throw in runDeploy
yonadaaa cf81602
chore: error message
yonadaaa e00a273
chore: changeset
yonadaaa 5b8808b
chore: changeset doesnt need common
yonadaaa 507f66a
chore: prettier
yonadaaa File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
--- | ||
"@latticexyz/cli": patch | ||
--- | ||
|
||
The key ID for deploying via KMS signer is now set via an `AWS_KMS_KEY_ID` environment variable to better align with Foundry tooling. To enable KMS signing with this environment variable, use the `--kms` flag. | ||
|
||
```diff | ||
-mud deploy --awsKmsKeyId [key ID] | ||
+AWS_KMS_KEY_ID=[key ID] mud deploy --kms | ||
``` |
This file contains 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
This file contains 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
This file contains 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
Oops, something went wrong.
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.
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.
ooc what does this type resolve to?
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.
It's
string | undefined
(same asSignCommandInput
actually). I don't know why they make it optional! https://docs.aws.amazon.com/AWSJavaScriptSDK/v3/latest/Package/-aws-sdk-client-kms/Interface/GetPublicKeyCommandInput/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.
oh, should we just make it a regular string and require it?
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.
We can, i figured we can pipe it through the same way they do. I copied that approach from the ethers signer https://github.com/rumblefishdev/eth-signer-kms/blob/master/src/kms.ts#L16
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.
also this change is totally orthogonal to the point of this PR, it's just a small consistency refactor I noticed and threw in
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.
I thought we had it as
string
before, not sure when it changed