Support the `gcp` option in `cast wallet list` by moricho · Pull Request #8232 · foundry-rs/foundry (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation7 Commits9 Checks22 Files changed

Conversation

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

[ Show hidden characters]({{ revealButtonHref }})

Motivation

To list an account from GCP KMS as with the other options.
Related to this comment in the PR.

Closes: #10221

Solution

Adds a gcp flag to the MultiWallet and to the ListArgs of the cast wallet.
NOTE: The current implementation only supports a single key, but multiple keys should be supported in the next step

Choose a reason for hiding this comment

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

I don't have strong feelings about this feature and idk if gcloud keys are even widely used, but I don't mind adding this

Comment on lines 56 to 57

| .aws(self.aws || self.all) | | --------------------------- | | .gcp(self.gcp || self.all) |

Choose a reason for hiding this comment

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

this makes me realize that if --all is set, the command will always fail if the env vars are missing, same for aws, which is probably not great UX.
we could make gcp infallible if the env vars are missing, same for aws.

Choose a reason for hiding this comment

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

@mattsse That's true. I created a helper function gcp_env_vars_set to check that all GCP environment variables are set and fixed the above part to .gcp(self.gcp || (self.all && gcp_env_vars_set())).

e2f5814