3299-kms-v2-improvements #3302
3299-kms-v2-improvements #3302
Conversation
Signed-off-by: Rita Zhang <rita.z.zhang@gmail.com>
k8s-ci-robot
added
cncf-cla: yes
k8s-ci-robot
added
the
kind/kep
k8s-ci-robot
added
the
sig/auth
|
/assign @enj @smarterclayton @deads2k Mo and Clayton PTAL at the KEP details. David PTAL at the PRR details. |
Signed-off-by: Anish Ramasekar <anish.ramasekar@gmail.com>
chore: add sequence diagram for encrypt and decrypt request
k8s-ci-robot
added
size/XL
Signed-off-by: Rita Zhang <rita.z.zhang@gmail.com>
Signed-off-by: Rita Zhang <rita.z.zhang@gmail.com>
Signed-off-by: Anish Ramasekar <anish.ramasekar@gmail.com>
chore: use snake case for non-generated proto API
Signed-off-by: Rita Zhang <rita.z.zhang@gmail.com>
Signed-off-by: Anish Ramasekar <anish.ramasekar@gmail.com>
k8s-ci-robot
removed
the
lgtm
|
|
||
| ###### Can the feature be disabled once it has been enabled (i.e. can we roll back the enablement)? | ||
|
|
||
| Yes, via the `KMSv2` feature gate. Disabling this gate without first doing a storage migration to use a different encryption at rest mechanism will result in data loss. |
There was a problem hiding this comment.
It's more than data loss right, the server would not understand the encryption configuration and I would expect it to fail to start.
There was a problem hiding this comment.
We are adding a new APIVersion field in the KMS configuration which can be used to indicate v2 API. When the KMSv2 feature gate is disabled, any kms provider that is configured for v2 in the KMS configuration will not take effect because all the code is behind the feature gate. So, this would only result in data loss because existing encrypted data can't be decrypted but the KMS configuration would still load fine.
Signed-off-by: Anish Ramasekar <anish.ramasekar@gmail.com>
| https://github.com/kubernetes/kubernetes/pull/97058/files#diff-7826f7adbc1996a05ab52e3f5f02429e94b68ce6bce0dc534d1be636154fded3R246-R282 | ||
| --> | ||
|
|
||
| N/A. When the feature is disabled, data stored in etcd will no longer be encrypted using the external kms provider with v2 API |
There was a problem hiding this comment.
I think we need to know the failure mode of servers if the feature is disabled improperly and I think a manual test to confirm it does what is expected is appropriate.
|
|
||
| ###### Can the feature be disabled once it has been enabled (i.e. can we roll back the enablement)? | ||
|
|
||
| Yes, via the `KMSv2` feature gate. Disabling this gate without first doing a storage migration to use a different encryption at rest mechanism will result in data loss. |
There was a problem hiding this comment.
Can you expand this section with specifics about how to properly disable? I think they need to create a new KMS configuration, reload it, do a read/write cycle, then remove the v2 configuration. Missing any step results in either a process that doesn't start or a server that cannot read secrets, rigth?
Signed-off-by: Anish Ramasekar <anish.ramasekar@gmail.com>
add steps for disabling feature
|
The PRR should help someone reason about what will happen to their cluster if the KMS integration breaks for some reason (imagine a corrupted config you don't notice, followed by a crash. does it restart?). After that, the PRR lgtm. |
Signed-off-by: Anish Ramasekar <anish.ramasekar@gmail.com>
add more details for livez and readyz
|
the PRR and design lgtm /approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: deads2k, enj, ritazh The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
|
/lgtm |
k8s-ci-robot
removed
the
do-not-merge/hold
k8s-ci-robot
added
the
lgtm
Signed-off-by: Rita Zhang rita.z.zhang@gmail.com
One-line PR description: Introduce KMS v2alpha1 API to add performance, rotation, and observability improvements
Issue link: #3299
Other comments:
FYI @ibihim @aramase @dgrisonnet