Adds some minor changes to clarity, grammar and spelling

Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>
This commit is contained in:
JoshVanL 2019-07-09 11:37:26 +01:00
parent a768624a14
commit 2d9ced2b55

View File

@ -3,7 +3,6 @@ title: Certificate Request CRD
authors:
- "@joshvanl"
- "@munnerz"
owning-sig: cert-manager TODO: << ?? :shrug:
reviewers:
- "@joshvanl"
- "@munnerz"
@ -50,7 +49,7 @@ This proposal adds a new custom resource `CertificateRequest` that contains an
x509 certificate signing request, a target issuer, and other metadata about the
request. Each issuer will have their own `CertificateRequest` controller to
watch for resources that are referencing them. The `Certificate` controller will
then rely on creating `CertificateRequest`s to resolve it's own Spec.
then rely on creating `CertificateRequest`s to resolve its own Spec.
## Motivation
@ -94,9 +93,12 @@ same code base and repository.
- This proposal does not document or explore possible or planned integrations
using this new functionality.
- This proposal will not investigate possible alignment or merging with the
Kubernetes internal `CertificateSigningRequest` resource.
TODO: maybe we do want to talk about this as a possibility @munnerz
Kubernetes internal `CertificateSigningRequest` resource. Although is is of
interest, the motivation is mostly in order to get a built-in approval workflow
for CertificateRequests. The feasibility of being able to implement a solution
using the built-in type in the near future however is small, so we'd rather
'trail-blaze' here and then try and fold our changes back upstream at a later
date.
## Proposal
@ -170,13 +172,14 @@ enables namespacing of references to different issuers of external API groups.
### Controller Behaviour
The philociphy for the `CertificateRequest` controllers are planned to be as
minimal as possible in that the single goal of them is to enable it's owning
The philosophy for the `CertificateRequest` controllers are planned to be as
minimal as possible in that the single goal of them is to enable its owning
`Issuer` to create the resulting certificate. Once a sync on a
`CertificateRequest` has been observed, the general flow is as follows:
- Check the group belongs to the owning `Issuer`, exit if not.
- Check if `CertificateRequest` is in a failed state, exit if true.
- Check if `CertificateRequest` is in a failed state, exit if true. TODO: more
tightly define what a 'failed state' exactly is.
- Check the `Issuer` type is of the same type, exit if not.
- Verify the Spec of the `CertificateRequest`.
- If a certificate exits then update the status if needed and exit.
@ -195,21 +198,26 @@ implementation details TBD.
### Internal API Resource Behaviour
The group name of `IssuerRef` inside `CertificateRequest`s are to be defaulted
The group name of `IssuerRef` inside `CertificateRequest`s is to be defaulted
to "certmanager.k8s.io" if the field is empty, using a mutating webhook. This
means that if unspecified, `CertificateRequest` objects will be put into the
ownership of the default pool of issuers in the cert-manager project.
Until the mutating webhook is fully implemented, we will handle defaulting
internally in the controller.
### Test Plan
Standard unit and end-to-end tests will be used to verify new behaviour, as used
by cert-manager currently.
by cert-manager currently. Current end-to-end tests for `Certificate` resources
will also give a good signal for `CertificateRequest`s once the controller has
migrated its implementation.
### Risks and Mitigations
The introduction and consequently the reliance on this core resource for all
cert-manager functions means it poses a high risk to bugs or unexpected behaviour
appearing accross the whole codebase. With this, it is key to ensure the change
appearing across the whole codebase. With this, it is key to ensure the change
happens in incremental roll-outs and proper care is taken during testing.
The new resource could be potentially confusing for current cert-manager