CertificateRequest "KEP"

design/20190708.certificate-request-crd.md

@@ -0,0 +1,242 @@
+---
+title: Certificate Request CRD
+authors:
+  - "@joshvanl"
+  - "@munnerz"
+owning-sig: cert-manager TODO: << ?? :shrug:
+reviewers:
+  - "@joshvanl"
+  - "@munnerz"
+approvers:
+  - "@joshvanl"
+  - "@munnerz"
+editor: "@joshvanl"
+creation-date: 2019-07-08
+last-updated: 2019-07-08
+status: implementable
+---
+
+# Certificate Request CRD
+
+## Table of Contents
+
+<!-- toc -->
+- [Summary](#summary)
+- [Motivation](#motivation)
+  - [Goals](#goals)
+  - [Non-Goals](#non-goals)
+- [Proposal](#proposal)
+  - [API Changes](#api-changes)
+  - [Controller Behaviour](#controller-behaviour)
+  - [Internal API Resource Behaviour](#internal-api-resource-behaviour)
+  - [Test Plan](#test-plan)
+  - [Risks and Mitigations](#risks-and-mitigations)
+  - [Graduation Criteria](#graduation-criteria)
+      - [Alpha](#alpha)
+      - [Alpha -&gt; Beta Graduation](#alpha---beta-graduation)
+      - [Beta -&gt; GA Graduation](#beta---ga-graduation)
+      - [Removing a deprecated flag](#removing-a-deprecated-flag)
+  - [Version Skew Strategy](#version-skew-strategy)
+<!-- /toc -->
+
+## Summary
+
+Currently, certificates issued via cert-manager rely on the `Certificate`
+resource being reconciled by the `Certificate` controller. This resource imposes
+limitations on what issuers are able to honour the `Certificate` resource as
+well as other opinionated implementation details.
+
+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.
+
+## Motivation
+
+Currently the required use of the `Certificate` resource means that users are
+forced to:
+
+- expose the signed certificate's private key to the API server
+- be limited to the finite set of issuers as implemented into the cert-manager
+  project, or, "in-tree"
+- adhere to the `Certificate` controller's opinionated implementation, limiting
+  scope for integrations with other projects
+- rely on developers of the cert-manager project for reviews and approval of new
+  issuers
+
+Due to these issues, cert-manager can be often unsuitable for some use
+cases/integrations or users are unsatisfied with some behaviour. Lack of
+exposure of options that a raw x509 certificate signing request provides can
+also be a source of frustration.
+
+With cert-manger maintainers ensuring that all issuers are always fully
+supported and tested, it becomes difficult for new issuers to become
+accepted. Some developers of new issuers would be happy to maintain these
+issuers themselves however is not possible with all issuers belonging in the
+same code base and repository.
+
+### Goals
+
+- Introduce the `CertificateRequest` resource.
+- Create a `CertificateRequest` controller for each in-tree issuer to resolve
+  `CertificateRequest`.
+- Change the implementation of the `Certificate` controller to rely on the
+  `CertificateRequest` resource to resolve the request.
+- Update documentation detailing this new behaviour and how it can be used to
+  develop out-of-tree implantations of an issuer `CertificateRequest`
+  controller.
+- Create a boilerplate/scaffolding example code to help quick start developers
+  on creating a controller with best practices.
+
+### Non-Goals
+
+- 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
+
+## Proposal
+
+### API Changes
+
+This proposal will create the following new API types in the
+`certmanager.k8s.io` group;
+
+```
+// CertificateRequestSpec defines the desired state of CertificateRequest
+type CertificateRequestSpec struct {
+	// Requested certificate default Duration
+	// +optional
+	Duration *metav1.Duration `json:"duration,omitempty"`
+
+	// IssuerRef is a reference to the issuer for this CertificateRequest.  If
+	// the 'kind' field is not set, or set to 'Issuer', an Issuer resource with
+	// the given name in the same namespace as the CertificateRequest will be
+	// used.  If the 'kind' field is set to 'ClusterIssuer', a ClusterIssuer with
+	// the provided name will be used. The 'name' field in this stanza is
+	// required at all times. The group field refers to the API group of the
+	// issuer which defaults to 'certmanager.k8s.io' if empty.
+	IssuerRef ObjectReference `json:"issuerRef"`
+
+	// Byte slice containing the PEM encoded CertificateSigningRequest
+	// +optional
+	CSRPEM []byte `json:"csr,omitempty"`
+
+	// IsCA will mark the resulting certificate as valid for signing. This
+	// implies that the 'signing' usage is set
+	// +optional
+	IsCA bool `json:"isCA,omitempty"`
+}
+
+// CertificateStatus defines the observed state of CertificateRequest and
+// resulting signed certificate.
+type CertificateRequestStatus struct {
+	// +optional
+	Conditions []CertificateRequestCondition `json:"conditions,omitempty"`
+
+	// Byte slice containing a PEM encoded signed certificate resulting from the
+	// given certificate signing request.
+	// +optional
+	Certificate []byte `json:"certificate,omitempty"`
+
+	// Byte slice containing the PEM encoded certificate authority of the signed
+	// certificate.
+	// +optional
+	CA []byte `json:"ca,omitempty"`
+}
+```
+
+The `CertificateRequestCondition` resembles much the same of the
+`CertificateRequestCondition`.
+
+The `ObjectReference` field type has had a new field `Group` added as follows:
+
+```
+// ObjectReference is a reference to an object with a given name, kind and group.
+type ObjectReference struct {
+	Name string `json:"name"`
+	// +optional
+	Kind string `json:"kind,omitempty"`
+	// +optional
+	Group string `json:"group,omitempty"`
+}
+```
+
+The group refers to the API group that the target Issuer belongs to. This
+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
+`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 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.
+- Sign the certificate via the Issuer using the contents of Spec.
+
+It is worth noting that whether the certificate is invalid, out-of-date or
+failed then the controller should take no further action on the resource. It is
+the responsibility of a higher level controller such as the `Certificate`
+controller to take further action to retry the certificate issuance through
+managing the life cycle of the `CertificateRequest` resources.
+
+With all `Issuer`s updated with `CertificateRequest` controllers, the
+`Certificate` controller will be migrated to begin to use and manage the life
+cycle `CertificateRequest`s to resolve it's Spec. Further concrete
+implementation details TBD.
+
+### Internal API Resource Behaviour
+
+The group name of `IssuerRef` inside `CertificateRequest`s are 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.
+
+### Test Plan
+
+Standard unit and end-to-end tests will be used to verify new behaviour, as used
+by cert-manager currently.
+
+### 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
+happens in incremental roll-outs and proper care is taken during testing.
+
+The new resource could be potentially confusing for current cert-manager
+users. To mitigate this, proper documentation should be created to explain the
+changes. It should also be made clear that the resource is typically only to be
+consumed or managed by a more complex controller or system, not necessarily a
+human user.
+
+### Graduation Criteria
+
+##### Alpha
+
+- Creation of `CertificateRequest` resource
+- A CA issuer `CertificateRequest` controller
+- Exposing the single controller via a feature gated flag
+
+##### Alpha -> Beta Graduation
+
+- All issuers have a `CertificateRequest` controller
+- All controllers are enabled by default
+
+##### Beta -> GA Graduation
+
+- The `CertificateRequest` API resource should be considered stable
+- The `Certificate` resource make use of the `CertificateRequest` resource to
+  resolve certificates.
+
+##### Removing a deprecated flag
+
+### Version Skew Strategy