Updates design document from comments
Signed-off-by: joshvanl <vleeuwenjoshua@gmail.com>
This commit is contained in:
parent
d94880e693
commit
aeda0f23c0
@ -55,11 +55,11 @@ anti-pattern to make runtime decisions on historical audit logs.
|
||||
|
||||
The upstream [Kubernetes certificates
|
||||
`CertificateSigningRequest`](https://github.com/kubernetes/api/blob/master/certificates/v1/types.go#L41)
|
||||
API has identity as part of the resource. As the project intends to align and
|
||||
likely make this resource compatible with cert-manager in some way, the
|
||||
`CertificateRequest` should expose identical identity details. This means
|
||||
extensions or additions to cert-manager (policy, auditing) can have the same
|
||||
guarantees about what information is available from both resources.
|
||||
API has the identity of the requesting Kubernetes user. As the project intends
|
||||
to transition to this resource as part of the project, the `CertificateRequest`
|
||||
should strive to match 1:1 wherever possible. This means extensions or additions
|
||||
to cert-manager (policy, auditing) can have the same guarantees about what
|
||||
information is available from both resources, while this transition takes place.
|
||||
|
||||
|
||||
### Goals
|
||||
@ -73,37 +73,49 @@ guarantees about what information is available from both resources.
|
||||
|
||||
### Non-Goals
|
||||
|
||||
- Discuss how this identity can be used (policy, auditing)
|
||||
- Make changes to upstream Kubernetes to implement identiy in cert-manager
|
||||
- Although considered below, "passed down" identity is not part of this design
|
||||
- Dictate any kind of means through which identity or policy evaluation should
|
||||
be performed (rather, only the building blocks to enable others to build
|
||||
evaluation systems is a goal)
|
||||
- Make changes to upstream Kubernetes to implement identity in cert-manager
|
||||
- Although considered below, ["passed down"](#certificate-identity-pass-down)
|
||||
identity is not part of this design
|
||||
|
||||
## Proposal
|
||||
|
||||
Kubernetes does [not currently support immutable
|
||||
fields](https://github.com/kubernetes/enhancements/blob/8b9b994136371f1bc938aabf012f4c45535d684c/keps/sig-api-machinery/20190603-immutable-fields.md)
|
||||
for CRDs. The cert-manager webhook will be responsible for populating and
|
||||
enforcing identity fields which are present on the spec of `CertificateRequest`
|
||||
enforcing user info fields which are present on the spec of `CertificateRequest`
|
||||
resources.
|
||||
|
||||
The webhook will be responsible for enforcing the following during a CREATE
|
||||
operation:
|
||||
- No identity fields have been set by the user creating the resource
|
||||
- Set identity fields to exactly what is received from the API server in the
|
||||
operation. We will not reject requests which populate these fields, but instead
|
||||
simply override them.
|
||||
- No user info fields have been set by the [user creating the resource](
|
||||
https://github.com/kubernetes/kubernetes/blob/7a94debba5f8c21bbf8b42b2a7f1d5e974ddb837/pkg/registry/certificates/certificates/strategy.go#L63-L79)
|
||||
- Set user info fields to exactly what is received from the API server in the
|
||||
[UserInfo](https://github.com/kubernetes/api/blob/master/authentication/v1/types.go#L102)
|
||||
|
||||
The webhook will also responsible for enforcing the following during an UPDATE
|
||||
operation:
|
||||
- No changes to identity fields are allowed to be made
|
||||
operation. Any attempt to changes these fields will result in a rejected
|
||||
request.
|
||||
- No changes to user info fields are allowed to be made
|
||||
|
||||
### API Changes
|
||||
|
||||
In order to expose the identity of who created `CertificateRequest` resources,
|
||||
In order to expose the user info of who created `CertificateRequest` resources,
|
||||
these resources must be updated to have parity with the upstream Kubernetes
|
||||
certificates API.
|
||||
This means that the `CertificateRequest` API type be updated to include the
|
||||
following fields in spec, for all API versions:
|
||||
|
||||
```go
|
||||
type CertificateRequestSpec {
|
||||
// EXISTING FIELDS
|
||||
// ...
|
||||
|
||||
// NEW FIELDS
|
||||
|
||||
// Username contains the name of the user that created the CertificateRequest.
|
||||
// Populated by the cert-manager webhook on creation and immutable.
|
||||
// +optional
|
||||
@ -123,15 +135,23 @@ following fields in spec, for all API versions:
|
||||
Extra map[string][]string `json:"extra,omitempty"`
|
||||
```
|
||||
|
||||
All new fields here are marked as optional. It is likely for a number of them to
|
||||
be empty for a given request, depending on the requester's identity. It is up to
|
||||
any component consuming the user info fields to make appropriate decisions about
|
||||
fields which are not populated.
|
||||
|
||||
### Upgrading
|
||||
|
||||
Any `CertificateRequest`s that are present in the cluster at the time of
|
||||
upgrade, won't and will never have their identity fields populated. This should
|
||||
upgrade, won't and will never have their user info fields populated. This should
|
||||
be acceptable.
|
||||
|
||||
All subsequent `CertificateRequest`s created after this upgrade will have their
|
||||
identities populated.
|
||||
|
||||
When the CRDs are upgraded, there will be a brief period where the user info
|
||||
fields will not be populated until the webhook is also upgraded.
|
||||
|
||||
### Test Plan
|
||||
|
||||
Unit tests will be created is ensure that the [properties](#proposal) are
|
||||
@ -142,19 +162,23 @@ resources have their respective identities populated in the fields.
|
||||
|
||||
### Risks and Mitigations
|
||||
|
||||
There are large security implications if the identity fields that other
|
||||
There are large security implications if the user info fields that other
|
||||
components rely on are wrong, or could be made fraudulent. Special care when
|
||||
testing needs to be given to ensure the properties described above are correct
|
||||
and enforced.
|
||||
|
||||
If the validating or mutating webhooks were not installed properly, either by
|
||||
being out of date or deleted, then this would invalidate the security guarantees
|
||||
of the user info fields.
|
||||
|
||||
### Certificate Identity Pass Down
|
||||
|
||||
`CertificateRequest`s created and managed via `Certificate` resources will have
|
||||
the identity of the cert-manager controller. Though this design doesn't provide
|
||||
a solution to components that need to traverse the identity to the original
|
||||
the user info of the cert-manager controller. Though this design doesn't provide
|
||||
a solution to components that need to traverse the user info to the original
|
||||
user who created the `Certificate` resource, some possible further design
|
||||
options are;
|
||||
- cert-manager would override the identity fields with the of the identity that
|
||||
- cert-manager would override the user info fields with the of the identity that
|
||||
created the `Certificate` resource, if cert-manager is the creator.
|
||||
- Consumers of the identity will need to be aware that the `CertificateRequest`
|
||||
may be managed by a `Certificate` resource if it is created by cert-manager,
|
||||
|
||||
Loading…
Reference in New Issue
Block a user