diff --git a/design/20190708.certificate-request-crd.md b/design/20190708.certificate-request-crd.md index f05a753ed..63fa49711 100644 --- a/design/20190708.certificate-request-crd.md +++ b/design/20190708.certificate-request-crd.md @@ -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