-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
KEP-3299: Rough details about rotation #3486
Conversation
Signed-off-by: Monis Khan <mok@microsoft.com>
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: enj 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 |
|
||
// this field would be populated by scanning all KMS v2 encrypted data and recording all observed | ||
// key IDs. this would be an expensive operation and is a bit weird to be per API server. | ||
InUseKeyIDHashes []Hash `json:"inUseKeyIDHashes" protobuf:"bytes,4,opt,name=inUseKeyIDHashes"` |
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.
Reminder: this needs a way to indicate that non-kms v2 encryption methods are in use as well as when unencrypted data is present.
Signed-off-by: Monis Khan <mok@microsoft.com>
/cc |
**Question: should we have a status field with the Kubernetes version to prevent migration during differences in API server version?** | ||
|
||
```go | ||
// metadata.name is the ID of the API server |
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.
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.
Sadly no because it is alpha and has not made any progress in ~2 years. If it ever does go GA, we could start using it since the values just need to be unique and are otherwise opaque.
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.
could we add this as a note here?
// an empty string from the hash value with a non-empty updated time means that | ||
// a non-KMS v2 write key is in use. This can be used with the `EncryptionConfigurationHash` | ||
// to determine when the API servers are at the same state. | ||
WriteKeyIDHash Hash `json:"writeKeyIDHash" protobuf:"bytes,2,opt,name=writeKeyIDHash"` |
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.
My memory of the EncryptionResponse is that it includes the key_id, so a server would only know this value as-of the last time it wrote a particular resource and how that value is determined by the KMS plugin is opaque to the apiserver. If that memory is correct how does an apiserver know that there is a single key_id (why would differing by set of namespaces be invalid for instance?) and how does an apiserver know it is writing the most current key_id (maybe it hasn't observed a write recently)
// covered by the `EncryptionConfigurationHash`. KMS v2 can dynamically change keys | ||
// at runtime without any change to the `EncryptionConfigurationHash` and thus needs | ||
// its own hash tracking mechanism. | ||
ReadKeyIDHashes []Hash `json:"readKeyIDHashes" protobuf:"bytes,3,opt,name=readKeyIDHashes"` |
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 the full set of key_ids required for decryption would be consistent for all kube-apiservers since it is logically the list of all known key_ids on the envelope. If this is supposed to reflect the key_id that the KMS plugin can interpret, I don't remember an API to the kms plugin that gives us that list.
The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs. This bot triages issues and PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle stale |
The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs. This bot triages issues and PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle rotten |
Signed-off-by: Monis Khan mok@microsoft.com