mirror of
https://github.com/wahyd4/cert-manager.git
synced 2026-08-09 05:06:38 +10:00
Add a section as to _why_ I am proposing this approach
Signed-off-by: joshvanl <vleeuwenjoshua@gmail.com>
This commit is contained in:
@@ -22,6 +22,7 @@ status: provisional
|
||||
- [Motivation](#motivation)
|
||||
- [Goals](#goals)
|
||||
- [Non-Goals](#non-goals)
|
||||
- [Reasons For Approach](#reasons-for-approach)
|
||||
- [Proposal](#proposal)
|
||||
- [Signers](#signers)
|
||||
- [API Changes](#api-changes)
|
||||
@@ -62,11 +63,45 @@ third-party project.
|
||||
- Make changes to upstream Kubernetes to implement controllers in cert-manager
|
||||
|
||||
|
||||
## Reasons for Approach
|
||||
|
||||
Below are a summary of properties of the `CertificateSigningRequest`, and
|
||||
explanations as to why other approaches to integrate the resource with
|
||||
cert-manager aren't possible (i.e. completely switching the `CertificateRequest`
|
||||
resource to `CertificateSigningRequest`:
|
||||
|
||||
1. `CertificateSigningRequests` are cluster scoped resources. This means that
|
||||
the concept of namespaced `Issuers` doesn't fit with this resource, and any
|
||||
attempt to integrate it would open up the ability of privilege escalation
|
||||
(i.e. referencing Secrets in another tenants Namespace). Switching to
|
||||
`CertificateSigningRequests` would mean the removal of the `Issuer` resource.
|
||||
|
||||
1. As `CertificateSigningRequests` are cluster scoped, they do not provide the
|
||||
same kind of isolation that `CertificateRequests` provide- preventing tenants
|
||||
reading others requests, namespace specific issuer authentication (e.g. Vault
|
||||
Kubernetes auth), namespacing cert-manager.
|
||||
|
||||
1. `CertificateSigningRequests` do not include a duration field. This means that
|
||||
any attempt to "proxy", or copy `CertificateRequests`, would result in a
|
||||
regression of this feature. A feature which many users of cert-manager rely
|
||||
on.
|
||||
|
||||
1. cert-manager has a large number of [external
|
||||
issuers](https://cert-manager.io/docs/configuration/external/) created by
|
||||
the community that operate over the `CertificateRequest` resource. No longer
|
||||
supporting these resources would require these authors to completely re-write
|
||||
these issuers to continue to be supported by cert-manager.
|
||||
|
||||
Supporting both resource types, but making `CertificateRequests` the "priority"
|
||||
resource (i.e. the resource created by the `Certificates` controller), means
|
||||
that we don't have any regression in the features cert-manager offers, whilst
|
||||
maintaining the same security properties.
|
||||
|
||||
## Proposal
|
||||
|
||||
Instead of the concept of an `IssuerRef` for `CertificateRequest`s,
|
||||
`CertificateSigningRequest`s have the concept of a `SignerName`. Since
|
||||
`CertificateSigningRequest`s are cluster scoped resources, the signer name can
|
||||
Instead of the concept of an `IssuerRef` for `CertificateRequests`,
|
||||
`CertificateSigningRequests` have the concept of a `SignerName`. Since
|
||||
`CertificateSigningRequests` are cluster scoped resources, the signer name can
|
||||
be directly mapped to the `ClusterIssuer` resource. `ClusterIssuers` will be
|
||||
referenced in the following format:
|
||||
|
||||
@@ -79,26 +114,26 @@ existing `CertificateRequest` resource, by getting the referenced
|
||||
`ClusterIssuer`, and attempting to sign. If the `ClusterIssuer` type is not
|
||||
managed by this controller, do nothing, else sign.
|
||||
|
||||
Each `CertificateSigningRequest` controller will set the
|
||||
Each `CertificateSigningRequest` controller will automatically set the
|
||||
`CertificateSigningRequest` `RequestCondition` to `Approved` if the request is
|
||||
for their managed `ClusterIssuer` type, and the `ClusterIssuer` is ready. If
|
||||
the controller does manage that `ClusterIssuer` type, but the `ClusterIssuer`
|
||||
doesn't exist or is not ready, the controller will set the condition to
|
||||
`Failed`.
|
||||
for their managed `ClusterIssuer` type. If the controller manages that
|
||||
`ClusterIssuer` type, but the `ClusterIssuer` doesn't exist or is not ready, the
|
||||
controller will set the condition to `Pending`. If signing fails during
|
||||
processing, it will set the `CertificateSigningRequest` condition to `Failed`.
|
||||
|
||||
|
||||
### Signers
|
||||
|
||||
Some special cases for some `ClusterIssuer`s that need to be addressed:
|
||||
Some special cases for some `ClusterIssuers` that need to be addressed:
|
||||
|
||||
- SelfSigned: Makes use of annotations, and so these annotations should also be
|
||||
present on `CertificateSigningRequest`s.
|
||||
present on `CertificateSigningRequests`.
|
||||
|
||||
- Venafi: Makes use of annotations, and so these annotations should also be
|
||||
present on `CertificateSigningRequest`s.
|
||||
present on `CertificateSigningRequests`.
|
||||
|
||||
- ACME: The ACME controller creates sub-resources (`Orders`). Since
|
||||
`CertificateSigningRequest`s are cluster scoped resources, we should create
|
||||
`CertificateSigningRequests` are cluster scoped resources, we should create
|
||||
`Orders` in the `Cluster Resource Namespace` (default `cert-manager`).
|
||||
|
||||
|
||||
@@ -115,8 +150,8 @@ No effect to upgrades as only additional controllers added. No API changes.
|
||||
### Risks and Mitigations
|
||||
|
||||
The controllers need to be aware if the `certificates.k8s.io/v1`
|
||||
`CertificateSigningRequest` resource exists (pre v1.19 cluster). If they don't
|
||||
exist, they should gracefully never start. If the Kubernetes API server were to
|
||||
be upgraded to a version that does support this resource, cert-manager will need
|
||||
to be restarted to make use of these controllers. This is acceptable- worker
|
||||
nodes are typically always restarted during a cluster upgrade.
|
||||
`CertificateSigningRequest` resource exists (e.g. pre v1.19 cluster). If they
|
||||
don't exist, they should gracefully never start. If the Kubernetes API server
|
||||
were to be upgraded to a version that does support this resource, cert-manager
|
||||
will need to be restarted to make use of these controllers. This is acceptable-
|
||||
worker nodes are typically always restarted during a cluster upgrade.
|
||||
|
||||
Reference in New Issue
Block a user