Open
Conversation
hnousiainen
force-pushed
the
htn_cmk_initial_ops
branch
from
4f80f3b to
616e2fc
Compare
glenn-mcallister-aiven
previously requested changes
Nov 26, 2025
There was a problem hiding this comment.
I have to block this, for a couple of reason:
- Most of these commands are using optional style arguments (i.e.,
--cmk-id) for required arguments. That's a major UX no-no. - WAY MORE significantly, the current implementation is intended for xxxxxxx only, to the point where
- we are hiding access to the the API behind an ACL,
- we aren't documenting the API in our public API docs,
- xxxxxx does changes only through TF, meaning they need API access only.
Once these changes get published, customers are going to see that there are "customer managed keys" commands, which is 100% what we don't want currently.
hnousiainen
force-pushed
the
htn_cmk_initial_ops
branch
2 times, most recently
from
4fdb8fa to
bb5cd8e
Compare
hnousiainen
dismissed
glenn-mcallister-aiven’s stale review
We're getting ready to introduce this functionality at large, so I'm dismissing the blocker on customer concerns. There's nothing secret in us pursuing this functionality.
hnousiainen
force-pushed
the
htn_cmk_initial_ops
branch
from
bb5cd8e to
3ee3668
Compare
VadimPushtaev
requested changes
Apr 22, 2026
Contributor
VadimPushtaev
left a comment
VadimPushtaev
left a comment
There was a problem hiding this comment.
Some typos, but generally fine. --cmk-id being required pattern is a pretty common pattern for aiven-client anyway.
| @arg("--default-cmk", help="Mark CMK as default for all new services", choices=["true", "false"]) | ||
| @arg.json | ||
| def project__cmks__update(self) -> None: | ||
| """Define a project customer managed key (CMK)""" |
Contributor
There was a problem hiding this comment.
Copy-pasted message, probably should be "Change", not "Define".
| @arg("--cmk-id", help="CMK Identifier", required=True) | ||
| @arg.json | ||
| def project__cmks__get(self) -> None: | ||
| """Get details on a customer managed keys (CMKs)""" |
| @arg("--cmk-id", help="CMK Identifier", required=True) | ||
| @arg.json | ||
| def project__cmks__delete(self) -> None: | ||
| """Delete a customer managed keys (CMKs)""" |
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
3 participants
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.
Implement initial support for CMK operations.