-
Notifications
You must be signed in to change notification settings - Fork 107
fix: handle unauthorized kms keys in the account #3071
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
base: main
Are you sure you want to change the base?
Conversation
|
Hey @satra, if I understand your use case correctly, this scenario only occurs if you are using pre-existing KMS keys associated with your account, right? The reason for my confusion is that, in theory, all keys used by Nebari should have been created and managed by Terraform during the initial deployment; thus, the ownership should have been maintained correctly. |
|
@viniciusdc - we use our main account for all kinds of projects through various iam users and roles. so there are keys already in place from different deployments, not just via nebari. however, this code was iterating over existing keys whether or not the running account had privileges for it. if it found a key it would try to use it, not create one. in our current deployment we created a key manually and passed it on. i don't know if the code is specific enough to check if a key exists for a specific deployment. (for example we have a production and sandbox deployment). |
|
Thanks for the extra context. I will try to review this whenever I have the chance, though I can't promise it will be ready for this month's release. |
Annotation for our "manual patch queue": PR: nebari-dev#3071
c04e775 to
6307c4c
Compare
|
@satra I rebased , added pointer to this PR into description, and force-pushed |
Annotation for our "manual patch queue": PR: nebari-dev#3071
Annotation for our "manual patch queue": PR: nebari-dev#3071
If an account has unauthorized KMS keys, then this section of code fails (
describe_keyspecifically). This merely traps the failure and allows proceeding in those situations.What does this implement/fix?
Put a
xin the boxes that applyDocumentation
Testing
How to test this PR?
Any other comments?