From c86cf31aaf63e5aee7a84ca2b184a17a6c035cfe Mon Sep 17 00:00:00 2001 From: joshvanl Date: Wed, 24 Mar 2021 13:19:52 +0000 Subject: [PATCH] Merges the approved/denied condition design with the master CertificateRequest design doc Signed-off-by: joshvanl --- design/20190708.certificate-request-crd.md | 199 ++++++++++++++++-- ...210324.certificate-request-approve-deny.md | 191 ----------------- 2 files changed, 187 insertions(+), 203 deletions(-) delete mode 100644 design/20210324.certificate-request-approve-deny.md diff --git a/design/20190708.certificate-request-crd.md b/design/20190708.certificate-request-crd.md index 18699cdec..1282f022b 100644 --- a/design/20190708.certificate-request-crd.md +++ b/design/20190708.certificate-request-crd.md @@ -11,7 +11,7 @@ approvers: - "@munnerz" editor: "@joshvanl" creation-date: 2019-07-08 -last-updated: 2019-08-12 +last-updated: 2021-03-24 status: implementable --- @@ -22,20 +22,33 @@ status: implementable - [Summary](#summary) - [Motivation](#motivation) - - [Goals](#goals) - - [Non-Goals](#non-goals) + * [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) + * [API Changes](#api-changes) + + [Approved and Denied Conditions](#approved-and-denied-conditions) + - [Behaviour](#behaviour) + - [RBAC](#rbac) + * [Scope](#scope) + * [API Group](#api-group) + * [Resource](#resource) + * [Verbs](#verbs) + * [Resource Names](#resource-names) + - [cert-manager Approver](#cert-manager-approver) + * [Controller Behaviour](#controller-behaviour) + + [ACME](#acme) + + [Issuing Controller](#issuing-controller) + * [Failure](#failure) + * [Internal API Resource Behaviour](#internal-api-resource-behaviour) + * [CertificateRequest Annotations](#certificaterequest-annotations) + * [Test Plan](#test-plan) + * [Risks and Mitigations](#risks-and-mitigations) + * [Graduation Criteria](#graduation-criteria) - [Alpha](#alpha) - - [Alpha -> Beta Graduation](#alpha---beta-graduation) - - [Beta -> GA Graduation](#beta---ga-graduation) + - [Alpha -> Beta Graduation](#alpha---beta-graduation) + - [Beta -> GA Graduation](#beta---ga-graduation) - [Removing a deprecated flag](#removing-a-deprecated-flag) - - [Version Skew Strategy](#version-skew-strategy) + * [Version Skew Strategy](#version-skew-strategy) ## Summary @@ -174,6 +187,136 @@ type ObjectReference struct { 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. +#### Approved and Denied Conditions + +The CertificateRequest resource has [Approved and Denied +conditions](https://github.com/jetstack/cert-manager/blob/7204284063702358164713379b07b73642ea9bec/pkg/apis/certmanager/v1/types_certificaterequest.go#L194). +These conditions are based upon the [certificates.k8s.io +CertificateSigningRequest](https://github.com/kubernetes/api/blob/3a2d6b5bb7f84daa2125131865eb81b151448d5b/certificates/v1/types.go#L198) +conditions of the same name. The purpose of these conditions is so that users, +or "approvers", are able to mark a request as either Approved or Denied. This +condition gates an Issuer from signing the request. An Issuer waits for a +CertificateRequest to have an Approved condition before signing. A +CertificateRequest with a Denied condition will never be signed. + +##### Behaviour + +The Approved and Denied conditions are two distinct condition types on the +CertificateRequest. These conditions must always have the status of True, and +are mutually exclusive (i.e. a CertificateRequest cannot have an Approved and +Denied condition simultaneously). This behaviour is enforced in the cert-manager +Webhook component. + +An "approver" is an entity that is responsible for setting the Approved/Denied +conditions. It is up to the approver's implementation as to what +CertificateRequests are managed by that approver. + +The Reason field of the Approved/Denied condition should be set to *who* set the +condition. Who can be interpreted however makes sense to the approver +implementation. For example, it may include the API group of an approving policy +controller, or the client agent of a manual request. + +The Message field of the Approved/Denied condition should be set to *why* the +condition is set. Again, why can be interpreted however makes sense to the +implementation of the approver. For example, the name of the resource that +approves this request, the violations which caused the request to be denied, or +the team to who manually approved the request. + + +##### RBAC + +Setting the Approved or Denied conditions are restricted by the approver having +sufficient RBAC permissions. These permissions are based upon the request +itself- specifically the request's IssuerRef: + +```yaml +apiGroups: ["cert-manager.io"] +resources: ["signers"] +verbs: ["approve"] +resourceNames: ["./"] +``` + +An example ClusterRole that would grant the permissions to set the Approve and +Denied conditions of CertificateRequests that reference the `MyIssuer` external +issuer, in the group `my-example.io`, with the name `MyApp`: + +``` +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + name: my-example-io-my-issuer-myapp-approver +rules: + - apiGroups: ["cert-manager.io"] + resources: ["signers"] + verbs: ["approve"] + resourceNames: ["myissuer.my-example.io/MyApp"] +``` + +These permissions are enforced using a +[SubjectAccessReview](https://github.com/kubernetes/kubernetes/blob/b11d0fbdd58394a62622787b38e98a620df82750/pkg/apis/authorization/types.go#L27) +that is performed in the cert-manager Webhook component when a user attempts to +Approve or Deny a request. The user of the approver is tested along with the +IssuerRef of the CertificateRequest. + +###### Scope +The RBAC permissions *must* be granted at the cluster scope (i.e. +ClusterRoleBinding). This matches the same scoping behaviour as +[certificates.k8s.io +CertificateSigningRequests](https://kubernetes.io/docs/reference/access-authn-authz/certificate-signing-requests/#authorization). + +###### API Group +The apiGroup must *always* be `cert-manage.io`, as all CertificateRequests are +in the `cert-manager.io` group. + +###### Resource +The resource must *always* be `signers`. This matches the same resource as +[required by certificates.k8s.io +CertificateSigningRequests](https://kubernetes.io/docs/reference/access-authn-authz/certificate-signing-requests/#authorization). + +###### Verbs +The verb must *always* be `approve`. This verb grants the approver the +permissions to set *both* Approved and Denied conditions. + +###### Resource Names +The resource name is made up of 3 parts: the issuer kind, issuer group, and the +issuer name. + +The issuer kind must be lower case, i.e. "ClusterIssuer" would become +"clusterissuer". + +The issuer group should be the same string of the group of issuer. + +The name may be either a wild card '*' denoting all issuer names, or instead a +string which must match the issuer name as it appears in the IssuerRef. + +An example of signing all `MyIssuer` issuers, and `ClusterMyIssuers` with the name +`MyApp`, in the `my-example.io` group: + +```yaml + resourceNames: ["myissuer.my-example.io/*", "clustermyissuer.my-example.io/MyApp"] +``` + + + +##### cert-manager Approver + +The cert-manager controller is deployed with an internal approver controller. +This controller will attempt to Approve *all* CertificateRequests that are +created, regardless of its contents or IssuerRef. + +By default, this approver is given the permission to sign all internal issuers, +that is `Issuer` and `ClusterIssuer` `cert-manager.io` issuers. External issuers +may consider whether they wish to install further RBAC that would allow the +cert-manager approver controller to also approve CertificateRequests that +reference these external issuers. + +The default cert-manager approver controller may be disabled by adding the +`--controllers=-certificaterequests-approver` argument to the +`cert-manager-container` component. This allows for other approvers to make +decisions about CertificateRequests, without racing against the internal +approver controller. + ### Controller Behaviour The philosophy for the `CertificateRequest` controllers are planned to be as @@ -210,6 +353,38 @@ non-networking or other non-transient issue then the `Order` is marked as failed further processing will take place by the `CertificateRequest` controller on this resource. +#### Issuing Controller + +Since external issuers have been built before the addition of Approved and +Denied conditions, the issuing controller needs to be permissive. An external +issuer may not honour an Approved condition and will sign and set a +CertificateRequest as being Ready, before the request has been approved. The +issuing controller must mark issuance as being successful in this case. In +practice, this means that the issuing controller is never concerned with +Approved conditions. + +External issuers that do not honour Denied conditions will sign +CertificateRequests, even if they have a Denied condition set. In this case, the +issuing controller will successfully complete the issuance of the Certificate. + +External Issuers and internal issuer that honour the Denied condition will never +sign CertificateRequests with the Denied condition set, and thus never set the +Ready condition. In this case, the issuing controller will consider this +CertificateRequest as failed, set the Issuing condition to False, and set the +last failure time. This will cause the issuance to be retried at a later date (1 +hour). This is done for a number of reasons: + +- The Certificate is clearly reported as Failed to users who may miss the + Denied request from a cursory view. +- The Spec may genuinely be violating the policy, and so can be changed by the + user. This will cause an immediate reissue. +- The policy may be misconfigured, and as such, the Certificate will be retired + later with no user intervention. +- The [manual renew + command](https://cert-manager.io/docs/usage/kubectl-plugin/#renew) relies on + the Issuing condition. In the case of policy being misconfigured, the user + is able to immediately retry the request using the CLI plugin. + ### Failure The `CertificateRequest` resource has a `FailureTime` field in its Status. If diff --git a/design/20210324.certificate-request-approve-deny.md b/design/20210324.certificate-request-approve-deny.md deleted file mode 100644 index 3b7f965b7..000000000 --- a/design/20210324.certificate-request-approve-deny.md +++ /dev/null @@ -1,191 +0,0 @@ ---- -title: CertificateRequest Approve and Denied Conditions -authors: - - "@joshvanl" -editor: "@joshvanl" -creation-date: 2021-02-03 -last-updated: 2021-02-03 -status: implementable ---- - -# Approved and Denied Conditions - -## Table of Contents - - -- [Summary](#summary) -- [API Behaviour](#api-behaviour) -- [RBAC](#rbac) - * [Scope](#scope) - * [API Group](#api-group) - * [Resource](#resource) - * [Verbs](#verbs) - * [Resource Names](#resource-names) -- [cert-manager Approver](#cert-manager-approver) -- [Certificates Controller Changes](#certificates-controller-changes) - * [Issuing Controller](#issuing-controller) - - -## Summary - -The CertificateRequest resource has [Approved and Denied -conditions](https://github.com/jetstack/cert-manager/blob/7204284063702358164713379b07b73642ea9bec/pkg/apis/certmanager/v1/types_certificaterequest.go#L194). -These conditions are based upon the [certificates.k8s.io -CertificateSigningRequest](https://github.com/kubernetes/api/blob/3a2d6b5bb7f84daa2125131865eb81b151448d5b/certificates/v1/types.go#L198) -conditions of the same name. The purpose of these conditions is so that users, -or "approvers", are able to mark a request as either Approved or Denied. This -condition gates an Issuer from signing the request. An Issuer waits for a -CertificateRequest to have an Approved condition before signing. A -CertificateRequest with a Denied condition will never be signed. - - -## API Behaviour - -The Approved and Denied conditions are two distinct condition types on the -CertificateRequest. These conditions must always have the status of True, and -are mutually exclusive (i.e. a CertificateRequest cannot have an Approved and -Denied condition simultaneously). This behaviour is enforced in the cert-manager -Webhook component. - -An "approver" is an entity that is responsible for setting the Approved/Denied -conditions. It is up to the approver's implementation as to what -CertificateRequests are managed by that approver. - -The Reason field of the Approved/Denied condition should be set to *who* set the -condition. Who can be interpreted however makes sense to the approver -implementation. For example, it may include the API group of an approving policy -controller, or the client agent of a manual request. - -The Message field of the Approved/Denied condition should be set to *why* the -condition is set. Again, why can be interpreted however makes sense to the -implementation of the approver. For example, the name of the resource that -approves this request, the violations which caused the request to be denied, or -the team to who manually approved the request. - - -## RBAC - -Setting the Approved or Denied conditions are restricted by the approver having -sufficient RBAC permissions. These permissions are based upon the request -itself- specifically the request's IssuerRef: - -```yaml -apiGroups: ["cert-manager.io"] -resources: ["signers"] -verbs: ["approve"] -resourceNames: ["./"] -``` - -An example ClusterRole that would grant the permissions to set the Approve and -Denied conditions of CertificateRequests that reference the `MyIssuer` external -issuer, in the group `my-example.io`, with the name `MyApp`: - -``` -apiVersion: rbac.authorization.k8s.io/v1 -kind: ClusterRole -metadata: - name: my-example-io-my-issuer-myapp-approver -rules: - - apiGroups: ["cert-manager.io"] - resources: ["signers"] - verbs: ["approve"] - resourceNames: ["myissuer.my-example.io/MyApp"] -``` - -These permissions are enforced using a -[SubjectAccessReview](https://github.com/kubernetes/kubernetes/blob/b11d0fbdd58394a62622787b38e98a620df82750/pkg/apis/authorization/types.go#L27) -that is performed in the cert-manager Webhook component when a user attempts to -Approve or Deny a request. The user of the approver is tested along with the -IssuerRef of the CertificateRequest. - -### Scope -The RBAC permissions *must* be granted at the cluster scope (i.e. -ClusterRoleBinding). This matches the same scoping behaviour as -[certificates.k8s.io -CertificateSigningRequests](https://kubernetes.io/docs/reference/access-authn-authz/certificate-signing-requests/#authorization). - -### API Group -The apiGroup must *always* be `cert-manage.io`, as all CertificateRequests are -in the `cert-manager.io` group. - -### Resource -The resource must *always* be `signers`. This matches the same resource as -[required by certificates.k8s.io -CertificateSigningRequests](https://kubernetes.io/docs/reference/access-authn-authz/certificate-signing-requests/#authorization). - -### Verbs -The verb must *always* be `approve`. This verb grants the approver the -permissions to set *both* Approved and Denied conditions. - -### Resource Names -The resource name is made up of 3 parts: the issuer kind, issuer group, and the -issuer name. - -The issuer kind must be lower case, i.e. "ClusterIssuer" would become -"clusterissuer". - -The issuer group should be the same string of the group of issuer. - -The name may be either a wild card '*' denoting all issuer names, or instead a -string which must match the issuer name as it appears in the IssuerRef. - -An example of signing all `MyIssuer` issuers, and `ClusterMyIssuers` with the name -`MyApp`, in the `my-example.io` group: - -```yaml - resourceNames: ["myissuer.my-example.io/*", "clustermyissuer.my-example.io/MyApp"] -``` - -## cert-manager Approver - -The cert-manager controller is deployed with an internal approver controller. -This controller will attempt to Approve *all* CertificateRequests that are -created, regardless of its contents or IssuerRef. - -By default, this approver is given the permission to sign all internal issuers, -that is `Issuer` and `ClusterIssuer` `cert-manager.io` issuers. External issuers -may consider whether they wish to install further RBAC that would allow the -cert-manager approver controller to also approve CertificateRequests that -reference these external issuers. - -The default cert-manager approver controller may be disabled by adding the -`--controllers=-certificaterequests-approver` argument to the -`cert-manager-container` component. This allows for other approvers to make -decisions about CertificateRequests, without racing against the internal -approver controller. - - -## Certificates Controller Changes - -### Issuing Controller - -Since external issuers have been built before the addition of Approved and -Denied conditions, the issuing controller needs to be permissive. An external -issuer may not honour an Approved condition and will sign and set a -CertificateRequest as being Ready, before the request has been approved. The -issuing controller must mark issuance as being successful in this case. In -practice, this means that the issuing controller is never concerned with -Approved conditions. - -External issuers that do not honour Denied conditions will sign -CertificateRequests, even if they have a Denied condition set. In this case, the -issuing controller will successfully complete the issuance of the Certificate. - -External Issuers and internal issuer that honour the Denied condition will never -sign CertificateRequests with the Denied condition set, and thus never set the -Ready condition. In this case, the issuing controller will consider this -CertificateRequest as failed, set the Issuing condition to False, and set the -last failure time. This will cause the issuance to be retried at a later date (1 -hour). This is done for a number of reasons: - -- The Certificate is clearly reported as Failed to users who may miss the - Denied request from a cursory view. -- The Spec may genuinely be violating the policy, and so can be changed by the - user. This will cause an immediate reissue. -- The policy may be misconfigured, and as such, the Certificate will be retired - later with no user intervention. -- The [manual renew - command](https://cert-manager.io/docs/usage/kubectl-plugin/#renew) relies on - the Issuing condition. In the case of policy being misconfigured, the user - is able to immediately retry the request using the CLI plugin.