Merge pull request #3735 from JoshVanL/certificate-request-approve-condition

Certificate request approved condition

cmd/controller/app/options/BUILD.bazel

@@ -10,6 +10,7 @@ go_library(
         "//pkg/controller/acmechallenges:go_default_library",
         "//pkg/controller/acmeorders:go_default_library",
         "//pkg/controller/certificaterequests/acme:go_default_library",
+        "//pkg/controller/certificaterequests/approver:go_default_library",
         "//pkg/controller/certificaterequests/ca:go_default_library",
         "//pkg/controller/certificaterequests/selfsigned:go_default_library",
         "//pkg/controller/certificaterequests/vault:go_default_library",

cmd/controller/app/options/options.go

@@ -27,6 +27,7 @@ import (
 	challengescontroller "github.com/jetstack/cert-manager/pkg/controller/acmechallenges"
 	orderscontroller "github.com/jetstack/cert-manager/pkg/controller/acmeorders"
 	cracmecontroller "github.com/jetstack/cert-manager/pkg/controller/certificaterequests/acme"
+	crapprovercontroller "github.com/jetstack/cert-manager/pkg/controller/certificaterequests/approver"
 	crcacontroller "github.com/jetstack/cert-manager/pkg/controller/certificaterequests/ca"
 	crselfsignedcontroller "github.com/jetstack/cert-manager/pkg/controller/certificaterequests/selfsigned"
 	crvaultcontroller "github.com/jetstack/cert-manager/pkg/controller/certificaterequests/vault"
@@ -144,6 +145,7 @@ var (
 		orderscontroller.ControllerName,
 		challengescontroller.ControllerName,
 		cracmecontroller.CRControllerName,
+		crapprovercontroller.ControllerName,
 		crcacontroller.CRControllerName,
 		crselfsignedcontroller.CRControllerName,
 		crvaultcontroller.CRControllerName,

deploy/crds/crd-certificaterequests.yaml

@@ -39,6 +39,12 @@ spec:
       subresources:
         status: {}
       additionalPrinterColumns:
+        - jsonPath: .status.conditions[?(@.type=="Approved")].status
+          name: Approved
+          type: string
+        - jsonPath: .status.conditions[?(@.type=="Denied")].status
+          name: Denied
+          type: string
         - jsonPath: .status.conditions[?(@.type=="Ready")].status
           name: Ready
           type: string
@@ -190,7 +196,7 @@ spec:
                           - "False"
                           - Unknown
                       type:
-                        description: Type of the condition, known values are (`Ready`, `InvalidRequest`).
+                        description: Type of the condition, known values are (`Ready`, `InvalidRequest`, `Approved`, `Denied`).
                         type: string
                 failureTime:
                   description: FailureTime stores the time that this CertificateRequest failed. This is used to influence garbage collection and back-off.
@@ -202,6 +208,12 @@ spec:
       subresources:
         status: {}
       additionalPrinterColumns:
+        - jsonPath: .status.conditions[?(@.type=="Approved")].status
+          name: Approved
+          type: string
+        - jsonPath: .status.conditions[?(@.type=="Denied")].status
+          name: Denied
+          type: string
         - jsonPath: .status.conditions[?(@.type=="Ready")].status
           name: Ready
           type: string
@@ -353,7 +365,7 @@ spec:
                           - "False"
                           - Unknown
                       type:
-                        description: Type of the condition, known values are (`Ready`, `InvalidRequest`).
+                        description: Type of the condition, known values are (`Ready`, `InvalidRequest`, `Approved`, `Denied`).
                         type: string
                 failureTime:
                   description: FailureTime stores the time that this CertificateRequest failed. This is used to influence garbage collection and back-off.
@@ -365,6 +377,12 @@ spec:
       subresources:
         status: {}
       additionalPrinterColumns:
+        - jsonPath: .status.conditions[?(@.type=="Approved")].status
+          name: Approved
+          type: string
+        - jsonPath: .status.conditions[?(@.type=="Denied")].status
+          name: Denied
+          type: string
         - jsonPath: .status.conditions[?(@.type=="Ready")].status
           name: Ready
           type: string
@@ -518,7 +536,7 @@ spec:
                           - "False"
                           - Unknown
                       type:
-                        description: Type of the condition, known values are (`Ready`, `InvalidRequest`).
+                        description: Type of the condition, known values are (`Ready`, `InvalidRequest`, `Approved`, `Denied`).
                         type: string
                 failureTime:
                   description: FailureTime stores the time that this CertificateRequest failed. This is used to influence garbage collection and back-off.
@@ -530,6 +548,12 @@ spec:
       subresources:
         status: {}
       additionalPrinterColumns:
+        - jsonPath: .status.conditions[?(@.type=="Approved")].status
+          name: Approved
+          type: string
+        - jsonPath: .status.conditions[?(@.type=="Denied")].status
+          name: Denied
+          type: string
         - jsonPath: .status.conditions[?(@.type=="Ready")].status
           name: Ready
           type: string
@@ -683,7 +707,7 @@ spec:
                           - "False"
                           - Unknown
                       type:
-                        description: Type of the condition, known values are (`Ready`, `InvalidRequest`).
+                        description: Type of the condition, known values are (`Ready`, `InvalidRequest`, `Approved`, `Denied`).
                         type: string
                 failureTime:
                   description: FailureTime stores the time that this CertificateRequest failed. This is used to influence garbage collection and back-off.

pkg/api/util/conditions.go

@@ -315,3 +315,38 @@ func CertificateRequestHasInvalidRequest(cr *cmapi.CertificateRequest) bool {
 
 	return false
 }
+
+// CertificateRequestIsApproved returns true if the CertificateRequest is
+// approved via an Approved condition of status `True`, returns false
+// otherwise.
+func CertificateRequestIsApproved(cr *cmapi.CertificateRequest) bool {
+	if cr == nil {
+		return false
+	}
+
+	for _, con := range cr.Status.Conditions {
+		if con.Type == cmapi.CertificateRequestConditionApproved &&
+			con.Status == cmmeta.ConditionTrue {
+			return true
+		}
+	}
+
+	return false
+}
+
+// CertificateRequestIsDenied returns true if the CertificateRequest is denied
+// via a Denied condition of status `True`, returns false otherwise.
+func CertificateRequestIsDenied(cr *cmapi.CertificateRequest) bool {
+	if cr == nil {
+		return false
+	}
+
+	for _, con := range cr.Status.Conditions {
+		if con.Type == cmapi.CertificateRequestConditionDenied &&
+			con.Status == cmmeta.ConditionTrue {
+			return true
+		}
+	}
+
+	return false
+}

pkg/apis/certmanager/v1/types_certificaterequest.go

@@ -153,7 +153,8 @@ type CertificateRequestStatus struct {
 
 // CertificateRequestCondition contains condition information for a CertificateRequest.
 type CertificateRequestCondition struct {
-	// Type of the condition, known values are (`Ready`, `InvalidRequest`).
+	// Type of the condition, known values are (`Ready`, `InvalidRequest`,
+	// `Approved`, `Denied`).
 	Type CertificateRequestConditionType `json:"type"`
 
 	// Status of the condition, one of (`True`, `False`, `Unknown`).
@@ -189,4 +190,16 @@ const (
 	// parameters being invalid. Additional information about why the request
 	// was rejected can be found in the `reason` and `message` fields.
 	CertificateRequestConditionInvalidRequest CertificateRequestConditionType = "InvalidRequest"
+
+	// CertificateRequestConditionApproved indicates that a certificate request
+	// is approved and ready for signing. Condition must never have a status of
+	// `False`, and cannot be modified once set. Cannot be set alongside
+	// `Denied`.
+	CertificateRequestConditionApproved CertificateRequestConditionType = "Approved"
+
+	// CertificateRequestConditionDenied indicates that a certificate request is
+	// denied, and must never be signed. Condition must never have a status of
+	// `False`, and cannot be modified once set. Cannot be set alongside
+	// `Approved`.
+	CertificateRequestConditionDenied CertificateRequestConditionType = "Denied"
 )

pkg/apis/certmanager/v1alpha2/types_certificaterequest.go

@@ -150,7 +150,8 @@ type CertificateRequestStatus struct {
 
 // CertificateRequestCondition contains condition information for a CertificateRequest.
 type CertificateRequestCondition struct {
-	// Type of the condition, known values are (`Ready`, `InvalidRequest`).
+	// Type of the condition, known values are (`Ready`,
+	// `InvalidRequest`, `Approved`, `Denied`).
 	Type CertificateRequestConditionType `json:"type"`
 
 	// Status of the condition, one of (`True`, `False`, `Unknown`).
@@ -186,4 +187,16 @@ const (
 	// parameters being invalid. Additional information about why the request
 	// was rejected can be found in the `reason` and `message` fields.
 	CertificateRequestConditionInvalidRequest CertificateRequestConditionType = "InvalidRequest"
+
+	// CertificateRequestConditionApproved indicates that a certificate request
+	// is approved and ready for signing. Condition must never have a status of
+	// `False`, and cannot be modified once set. Cannot be set alongside
+	// `Denied`.
+	CertificateRequestConditionApproved CertificateRequestConditionType = "Approved"
+
+	// CertificateRequestConditionDenied indicates that a certificate request is
+	// denied, and must never be signed. Condition must never have a status of
+	// `False`, and cannot be modified once set. Cannot be set alongside
+	// `Approved`.
+	CertificateRequestConditionDenied CertificateRequestConditionType = "Denied"
 )

pkg/apis/certmanager/v1alpha3/types_certificaterequest.go

@@ -150,7 +150,8 @@ type CertificateRequestStatus struct {
 
 // CertificateRequestCondition contains condition information for a CertificateRequest.
 type CertificateRequestCondition struct {
-	// Type of the condition, known values are (`Ready`, `InvalidRequest`).
+	// Type of the condition, known values are (`Ready`,
+	// `InvalidRequest`, `Approved`, `Denied`).
 	Type CertificateRequestConditionType `json:"type"`
 
 	// Status of the condition, one of (`True`, `False`, `Unknown`).
@@ -186,4 +187,14 @@ const (
 	// parameters being invalid. Additional information about why the request
 	// was rejected can be found in the `reason` and `message` fields.
 	CertificateRequestConditionInvalidRequest CertificateRequestConditionType = "InvalidRequest"
+
+	// CertificateRequestConditionApproved indicates that a certificate request
+	// is approved and ready for signing. Condition must never have a status of
+	// `False`, and cannot be modified once set.
+	CertificateRequestConditionApproved CertificateRequestConditionType = "Approved"
+
+	// CertificateRequestConditionDenied indicates that a certificate request is
+	// denied, and must never be signed. Condition must never have a status of
+	// `False`, and cannot be modified once set.
+	CertificateRequestConditionDenied CertificateRequestConditionType = "Denied"
 )

pkg/apis/certmanager/v1beta1/types_certificaterequest.go

@@ -151,7 +151,8 @@ type CertificateRequestStatus struct {
 
 // CertificateRequestCondition contains condition information for a CertificateRequest.
 type CertificateRequestCondition struct {
-	// Type of the condition, known values are (`Ready`, `InvalidRequest`).
+	// Type of the condition, known values are (`Ready`,
+	// `InvalidRequest`, `Approved`, `Denied`).
 	Type CertificateRequestConditionType `json:"type"`
 
 	// Status of the condition, one of (`True`, `False`, `Unknown`).
@@ -187,4 +188,16 @@ const (
 	// parameters being invalid. Additional information about why the request
 	// was rejected can be found in the `reason` and `message` fields.
 	CertificateRequestConditionInvalidRequest CertificateRequestConditionType = "InvalidRequest"
+
+	// CertificateRequestConditionApproved indicates that a certificate request
+	// is approved and ready for signing. Condition must never have a status of
+	// `False`, and cannot be modified once set. Cannot be set alongside
+	// `Denied`.
+	CertificateRequestConditionApproved CertificateRequestConditionType = "Approved"
+
+	// CertificateRequestConditionDenied indicates that a certificate request is
+	// denied, and must never be signed. Condition must never have a status of
+	// `False`, and cannot be modified once set. Cannot be set alongside
+	// `Approved`.
+	CertificateRequestConditionDenied CertificateRequestConditionType = "Denied"
 )

pkg/controller/certificaterequests/BUILD.bazel

@@ -68,6 +68,7 @@ filegroup(
     srcs = [
         ":package-srcs",
         "//pkg/controller/certificaterequests/acme:all-srcs",
+        "//pkg/controller/certificaterequests/approver:all-srcs",
         "//pkg/controller/certificaterequests/ca:all-srcs",
         "//pkg/controller/certificaterequests/fake:all-srcs",
         "//pkg/controller/certificaterequests/selfsigned:all-srcs",

pkg/controller/certificaterequests/acme/acme_test.go

@@ -102,6 +102,7 @@ func generateCSRWithIPs(t *testing.T, secretKey crypto.Signer, commonName string
 }
 
 func TestSign(t *testing.T) {
+	metaFixedClockStart := metav1.NewTime(fixedClockStart)
 	baseIssuer := gen.Issuer("test-issuer",
 		gen.SetIssuerACME(cmacme.ACMEIssuer{}),
 		gen.AddIssuerCondition(cmapi.IssuerCondition{
@@ -118,7 +119,7 @@ func TestSign(t *testing.T) {
 	csrPEM := generateCSR(t, sk, "example.com", "example.com", "foo.com")
 	csrPEMExampleNotPresent := generateCSR(t, sk, "example.com", "foo.com")
 
-	baseCR := gen.CertificateRequest("test-cr",
+	baseCRNotApproved := gen.CertificateRequest("test-cr",
 		gen.SetCertificateRequestCSR(csrPEM),
 		gen.SetCertificateRequestIsCA(false),
 		gen.SetCertificateRequestDuration(&metav1.Duration{Duration: time.Hour * 24 * 60}),
@@ -128,6 +129,24 @@ func TestSign(t *testing.T) {
 			Kind:  "Issuer",
 		}),
 	)
+	baseCRDenied := gen.CertificateRequestFrom(baseCRNotApproved,
+		gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{
+			Type:               cmapi.CertificateRequestConditionDenied,
+			Status:             cmmeta.ConditionTrue,
+			Reason:             "Foo",
+			Message:            "Certificate request has been denied by cert-manager.io",
+			LastTransitionTime: &metaFixedClockStart,
+		}),
+	)
+	baseCR := gen.CertificateRequestFrom(baseCRNotApproved,
+		gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{
+			Type:               cmapi.CertificateRequestConditionApproved,
+			Status:             cmmeta.ConditionTrue,
+			Reason:             "cert-manager.io",
+			Message:            "Certificate request has been approved by cert-manager.io",
+			LastTransitionTime: &metaFixedClockStart,
+		}),
+	)
 
 	csr, err := pki.DecodeX509CertificateRequestBytes(csrPEM)
 	if err != nil {
@@ -175,8 +194,51 @@ func TestSign(t *testing.T) {
 		t.Fatalf("failed to build order during testing: %s", err)
 	}
 
-	metaFixedClockStart := metav1.NewTime(fixedClockStart)
 	tests := map[string]testT{
+		"a CertificateRequest without an approved condition should do nothing": {
+			certificateRequest: baseCRNotApproved.DeepCopy(),
+			builder: &testpkg.Builder{
+				KubeObjects:        []runtime.Object{},
+				CertManagerObjects: []runtime.Object{baseCRNotApproved.DeepCopy(), baseIssuer.DeepCopy()},
+			},
+		},
+		"a CertificateRequest with a denied condition should do nothing": {
+			certificateRequest: baseCRDenied.DeepCopy(),
+			builder: &testpkg.Builder{
+				KubeObjects:        []runtime.Object{},
+				CertManagerObjects: []runtime.Object{baseCRDenied.DeepCopy(), baseIssuer.DeepCopy()},
+			},
+		},
+		"a badly formed CSR should report failure": {
+			certificateRequest: gen.CertificateRequestFrom(baseCR,
+				gen.SetCertificateRequestCSR([]byte("a bad csr")),
+			),
+			builder: &testpkg.Builder{
+				KubeObjects:        []runtime.Object{},
+				CertManagerObjects: []runtime.Object{baseCR.DeepCopy(), baseIssuer.DeepCopy()},
+				ExpectedEvents: []string{
+					"Warning RequestParsingError Failed to decode CSR in spec.request: error decoding certificate request PEM block",
+				},
+				ExpectedActions: []testpkg.Action{
+					testpkg.NewAction(coretesting.NewUpdateSubresourceAction(
+						cmapi.SchemeGroupVersion.WithResource("certificaterequests"),
+						"status",
+						gen.DefaultTestNamespace,
+						gen.CertificateRequestFrom(baseCR,
+							gen.SetCertificateRequestCSR([]byte("a bad csr")),
+							gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{
+								Type:               cmapi.CertificateRequestConditionReady,
+								Status:             cmmeta.ConditionFalse,
+								Reason:             cmapi.CertificateRequestReasonFailed,
+								Message:            "Failed to decode CSR in spec.request: error decoding certificate request PEM block",
+								LastTransitionTime: &metaFixedClockStart,
+							}),
+							gen.SetCertificateRequestFailureTime(metaFixedClockStart),
+						),
+					)),
+				},
+			},
+		},
 		"if the common name is not present in the DNS names then should hard fail": {
 			certificateRequest: gen.CertificateRequestFrom(baseCR,
 				gen.SetCertificateRequestCSR(csrPEMExampleNotPresent),

pkg/controller/certificaterequests/approver/BUILD.bazel

@@ -0,0 +1,56 @@
+load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
+
+go_library(
+    name = "go_default_library",
+    srcs = [
+        "approver.go",
+        "sync.go",
+    ],
+    importpath = "github.com/jetstack/cert-manager/pkg/controller/certificaterequests/approver",
+    visibility = ["//visibility:public"],
+    deps = [
+        "//pkg/api/util:go_default_library",
+        "//pkg/apis/certmanager/v1:go_default_library",
+        "//pkg/apis/meta/v1:go_default_library",
+        "//pkg/client/clientset/versioned:go_default_library",
+        "//pkg/client/listers/certmanager/v1:go_default_library",
+        "//pkg/controller:go_default_library",
+        "//pkg/logs:go_default_library",
+        "@com_github_go_logr_logr//:go_default_library",
+        "@io_k8s_api//core/v1:go_default_library",
+        "@io_k8s_apimachinery//pkg/api/errors:go_default_library",
+        "@io_k8s_apimachinery//pkg/apis/meta/v1:go_default_library",
+        "@io_k8s_client_go//tools/cache:go_default_library",
+        "@io_k8s_client_go//tools/record:go_default_library",
+        "@io_k8s_client_go//util/workqueue:go_default_library",
+    ],
+)
+
+filegroup(
+    name = "package-srcs",
+    srcs = glob(["**"]),
+    tags = ["automanaged"],
+    visibility = ["//visibility:private"],
+)
+
+filegroup(
+    name = "all-srcs",
+    srcs = [":package-srcs"],
+    tags = ["automanaged"],
+    visibility = ["//visibility:public"],
+)
+
+go_test(
+    name = "go_default_test",
+    srcs = ["approver_test.go"],
+    embed = [":go_default_library"],
+    deps = [
+        "//pkg/apis/certmanager/v1:go_default_library",
+        "//pkg/apis/meta/v1:go_default_library",
+        "//pkg/controller:go_default_library",
+        "//pkg/controller/test:go_default_library",
+        "@io_k8s_apimachinery//pkg/apis/meta/v1:go_default_library",
+        "@io_k8s_client_go//testing:go_default_library",
+        "@io_k8s_utils//clock/testing:go_default_library",
+    ],
+)

pkg/controller/certificaterequests/approver/approver.go

@@ -0,0 +1,106 @@
+/*
+Copyright 2021 The cert-manager Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package approver
+
+import (
+	"context"
+	"fmt"
+
+	"github.com/go-logr/logr"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/client-go/tools/cache"
+	"k8s.io/client-go/tools/record"
+	"k8s.io/client-go/util/workqueue"
+
+	cmclient "github.com/jetstack/cert-manager/pkg/client/clientset/versioned"
+	cmlisters "github.com/jetstack/cert-manager/pkg/client/listers/certmanager/v1"
+	controllerpkg "github.com/jetstack/cert-manager/pkg/controller"
+	logf "github.com/jetstack/cert-manager/pkg/logs"
+)
+
+const (
+	ControllerName = "certificaterequests-approver"
+)
+
+// Controller is a CertificateRequest controller which manages the "Approved"
+// condition. In the absence of any automated policy engine, this controller
+// will _always_ set the "Approved" condition to True. All CertificateRequest
+// signing controllers should wait until the "Approved" condition is set to
+// True before processing.
+type Controller struct {
+	// logger to be used by this controller
+	log logr.Logger
+
+	certificateRequestLister cmlisters.CertificateRequestLister
+	cmClient                 cmclient.Interface
+
+	recorder record.EventRecorder
+
+	queue workqueue.RateLimitingInterface
+}
+
+func init() {
+	// create certificate request approver controller
+	controllerpkg.Register(ControllerName, func(ctx *controllerpkg.Context) (controllerpkg.Interface, error) {
+		return controllerpkg.NewBuilder(ctx, ControllerName).
+			For(new(Controller)).Complete()
+	})
+}
+
+// Register registers and constructs the controller using the provided context.
+// It returns the workqueue to be used to enqueue items, a list of
+// InformerSynced functions that must be synced, or an error.
+func (c *Controller) Register(ctx *controllerpkg.Context) (workqueue.RateLimitingInterface, []cache.InformerSynced, error) {
+	c.log = logf.FromContext(ctx.RootContext, ControllerName)
+	c.queue = workqueue.NewNamedRateLimitingQueue(controllerpkg.DefaultItemBasedRateLimiter(), ControllerName)
+
+	certificateRequestInformer := ctx.SharedInformerFactory.Certmanager().V1().CertificateRequests()
+	mustSync := append([]cache.InformerSynced{certificateRequestInformer.Informer().HasSynced})
+	certificateRequestInformer.Informer().AddEventHandler(&controllerpkg.QueuingEventHandler{Queue: c.queue})
+
+	c.certificateRequestLister = certificateRequestInformer.Lister()
+	c.cmClient = ctx.CMClient
+	c.recorder = ctx.Recorder
+
+	c.log.V(logf.DebugLevel).Info("certificate request approver controller registered")
+
+	return c.queue, mustSync, nil
+}
+
+func (c *Controller) ProcessItem(ctx context.Context, key string) error {
+	log := logf.FromContext(ctx)
+	dbg := log.V(logf.DebugLevel)
+
+	namespace, name, err := cache.SplitMetaNamespaceKey(key)
+	if err != nil {
+		log.Error(err, "invalid resource key")
+		return nil
+	}
+
+	cr, err := c.certificateRequestLister.CertificateRequests(namespace).Get(name)
+	if apierrors.IsNotFound(err) {
+		dbg.Info(fmt.Sprintf("certificate request in work queue no longer exists: %s", err))
+		return nil
+	}
+
+	if err != nil {
+		return err
+	}
+
+	ctx = logf.NewContext(ctx, logf.WithResource(log, cr))
+	return c.Sync(ctx, cr)
+}

pkg/controller/certificaterequests/approver/approver_test.go

@@ -0,0 +1,240 @@
+/*
+Copyright 2021 The cert-manager Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package approver
+
+import (
+	"context"
+	"testing"
+	"time"
+
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	coretesting "k8s.io/client-go/testing"
+	fakeclock "k8s.io/utils/clock/testing"
+
+	cmapi "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1"
+	cmmeta "github.com/jetstack/cert-manager/pkg/apis/meta/v1"
+	controllerpkg "github.com/jetstack/cert-manager/pkg/controller"
+	testpkg "github.com/jetstack/cert-manager/pkg/controller/test"
+)
+
+func TestProcessItem(t *testing.T) {
+	// now time is the current time at the start of the test (the clock is fixed)
+	now := time.Now()
+	metaNow := metav1.NewTime(now)
+	tests := map[string]struct {
+		// key that should be passed to ProcessItem.
+		// if not set, the 'namespace/name' of the 'CertificateRequest' field will be used.
+		// if neither is set, the key will be ""
+		key string
+
+		// CertificateRequest to be synced for the test.
+		// if not set, the 'key' will be passed to ProcessItem instead.
+		request *cmapi.CertificateRequest
+
+		// expectedEvent, if set, is an 'event string' that is expected to be fired.
+		expectedEvent string
+
+		// expectedConditions is the expected set of conditions on the
+		// CertificateRequest resource if an Update is made.
+		// If nil, no update is expected.
+		// If empty, an update to the empty set/nil is expected.
+		expectedConditions []cmapi.CertificateRequestCondition
+
+		// err is the expected error text returned by the controller, if any.
+		err string
+	}{
+		"do nothing if an empty 'key' is used": {},
+		"do nothing if an invalid 'key' is used": {
+			key: "abc/def/ghi",
+		},
+		"do nothing if a key references a Certificate that does not exist": {
+			key: "namespace/name",
+		},
+		"do nothing if CertificateRequest already has 'Approved' True condition": {
+			request: &cmapi.CertificateRequest{
+				ObjectMeta: metav1.ObjectMeta{Namespace: "testns", Name: "test"},
+				Status: cmapi.CertificateRequestStatus{
+					Conditions: []cmapi.CertificateRequestCondition{
+						{
+							Type:   cmapi.CertificateRequestConditionApproved,
+							Status: cmmeta.ConditionTrue,
+						},
+					},
+				},
+			},
+		},
+		"do nothing if CertificateRequest already has 'Denied' True condition": {
+			request: &cmapi.CertificateRequest{
+				ObjectMeta: metav1.ObjectMeta{Namespace: "testns", Name: "test"},
+				Status: cmapi.CertificateRequestStatus{
+					Conditions: []cmapi.CertificateRequestCondition{
+						{
+							Type:   cmapi.CertificateRequestConditionDenied,
+							Status: cmmeta.ConditionTrue,
+						},
+					},
+				},
+			},
+		},
+		"do nothing if CertificateRequest already has 'Ready' Failed condition": {
+			request: &cmapi.CertificateRequest{
+				ObjectMeta: metav1.ObjectMeta{Namespace: "testns", Name: "test"},
+				Status: cmapi.CertificateRequestStatus{
+					Conditions: []cmapi.CertificateRequestCondition{
+						{
+							Type:   cmapi.CertificateRequestConditionReady,
+							Status: cmmeta.ConditionFalse,
+							Reason: cmapi.CertificateRequestReasonFailed,
+						},
+					},
+				},
+			},
+		},
+		"do nothing if CertificateRequest already has 'Ready' Issued condition": {
+			request: &cmapi.CertificateRequest{
+				ObjectMeta: metav1.ObjectMeta{Namespace: "testns", Name: "test"},
+				Status: cmapi.CertificateRequestStatus{
+					Conditions: []cmapi.CertificateRequestCondition{
+						{
+							Type:   cmapi.CertificateRequestConditionReady,
+							Status: cmmeta.ConditionTrue,
+							Reason: cmapi.CertificateRequestReasonIssued,
+						},
+					},
+				},
+			},
+		},
+		"approve CertificateRequest if no condition": {
+			request: &cmapi.CertificateRequest{
+				ObjectMeta: metav1.ObjectMeta{Namespace: "testns", Name: "test"},
+				Status: cmapi.CertificateRequestStatus{
+					Conditions: []cmapi.CertificateRequestCondition{},
+				},
+			},
+			expectedConditions: []cmapi.CertificateRequestCondition{
+				{
+					Type:               cmapi.CertificateRequestConditionApproved,
+					Status:             cmmeta.ConditionTrue,
+					Reason:             "cert-manager.io",
+					Message:            ApprovedMessage,
+					LastTransitionTime: &metaNow,
+				},
+			},
+			expectedEvent: "Normal cert-manager.io Certificate request has been approved by cert-manager.io",
+		},
+		"approve CertificateRequest has 'Ready' Pending condition": {
+			request: &cmapi.CertificateRequest{
+				ObjectMeta: metav1.ObjectMeta{Namespace: "testns", Name: "test"},
+				Status: cmapi.CertificateRequestStatus{
+					Conditions: []cmapi.CertificateRequestCondition{
+						{
+							Type:   cmapi.CertificateRequestConditionReady,
+							Status: cmmeta.ConditionFalse,
+							Reason: cmapi.CertificateRequestReasonPending,
+						},
+					},
+				},
+			},
+			expectedConditions: []cmapi.CertificateRequestCondition{
+				{
+					Type:   cmapi.CertificateRequestConditionReady,
+					Status: cmmeta.ConditionFalse,
+					Reason: cmapi.CertificateRequestReasonPending,
+				},
+				{
+					Type:               cmapi.CertificateRequestConditionApproved,
+					Status:             cmmeta.ConditionTrue,
+					Reason:             "cert-manager.io",
+					Message:            ApprovedMessage,
+					LastTransitionTime: &metaNow,
+				},
+			},
+			expectedEvent: "Normal cert-manager.io Certificate request has been approved by cert-manager.io",
+		},
+	}
+	for name, test := range tests {
+		t.Run(name, func(t *testing.T) {
+			// Create and initialise a new unit test builder
+			builder := &testpkg.Builder{
+				T:     t,
+				Clock: fakeclock.NewFakeClock(now),
+			}
+			if test.request != nil {
+				builder.CertManagerObjects = append(builder.CertManagerObjects, test.request)
+			}
+			builder.Init()
+
+			c := new(Controller)
+			_, _, err := c.Register(builder.Context)
+			if err != nil {
+				t.Fatal(err)
+			}
+			if test.expectedConditions != nil {
+				if test.request == nil {
+					t.Fatal("cannot expect an Update operation if test.request is nil")
+				}
+				expectedRequest := test.request.DeepCopy()
+				expectedRequest.Status.Conditions = test.expectedConditions
+				builder.ExpectedActions = append(builder.ExpectedActions,
+					testpkg.NewAction(coretesting.NewUpdateSubresourceAction(
+						cmapi.SchemeGroupVersion.WithResource("certificaterequests"),
+						"status",
+						test.request.Namespace,
+						expectedRequest,
+					)),
+				)
+			}
+			if test.expectedEvent != "" {
+				builder.ExpectedEvents = []string{test.expectedEvent}
+			}
+			// Start the informers and begin processing updates
+			builder.Start()
+			defer builder.Stop()
+
+			key := test.key
+			if key == "" && test.request != nil {
+				key, err = controllerpkg.KeyFunc(test.request)
+				if err != nil {
+					t.Fatal(err)
+				}
+			}
+
+			// Call ProcessItem
+			err = c.ProcessItem(context.Background(), key)
+			switch {
+			case err != nil:
+				if test.err != err.Error() {
+					t.Errorf("error text did not match, got=%s, exp=%s", err.Error(), test.err)
+				}
+			default:
+				if test.err != "" {
+					t.Errorf("got no error but expected: %s", test.err)
+				}
+			}
+
+			if err := builder.AllEventsCalled(); err != nil {
+				builder.T.Error(err)
+			}
+			if err := builder.AllActionsExecuted(); err != nil {
+				builder.T.Error(err)
+			}
+			if err := builder.AllReactorsCalled(); err != nil {
+				builder.T.Error(err)
+			}
+		})
+	}
+}

pkg/controller/certificaterequests/approver/sync.go

@@ -0,0 +1,73 @@
+/*
+Copyright 2021 The cert-manager Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package approver
+
+import (
+	"context"
+
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	apiutil "github.com/jetstack/cert-manager/pkg/api/util"
+	cmapi "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1"
+	cmmeta "github.com/jetstack/cert-manager/pkg/apis/meta/v1"
+	logf "github.com/jetstack/cert-manager/pkg/logs"
+)
+
+const (
+	ApprovedMessage = "Certificate request has been approved by cert-manager.io"
+)
+
+// Sync will set the "Approved" condition to True on synced
+// CertificateRequests. If the "Denied", "Approved" or "Ready" condition
+// already exists, exit early.
+func (c *Controller) Sync(ctx context.Context, cr *cmapi.CertificateRequest) (err error) {
+	log := logf.FromContext(ctx, "approver")
+
+	switch {
+	case
+		// If the CertificateRequest has already been approved, exit early.
+		apiutil.CertificateRequestIsApproved(cr),
+
+		// If the CertificateRequest has already been denied, exit early.
+		apiutil.CertificateRequestIsDenied(cr),
+
+		// If the CertificateRequest is "Issued" or "Failed", exit early.
+		apiutil.CertificateRequestReadyReason(cr) == cmapi.CertificateRequestReasonFailed,
+		apiutil.CertificateRequestReadyReason(cr) == cmapi.CertificateRequestReasonIssued:
+		return nil
+	}
+
+	// Update the CertificateRequest approved condition to true.
+	apiutil.SetCertificateRequestCondition(cr,
+		cmapi.CertificateRequestConditionApproved,
+		cmmeta.ConditionTrue,
+		"cert-manager.io",
+		ApprovedMessage,
+	)
+
+	// Update CertificateRequest with
+	_, err = c.cmClient.CertmanagerV1().CertificateRequests(cr.Namespace).UpdateStatus(ctx, cr, metav1.UpdateOptions{})
+	if err != nil {
+		return err
+	}
+	c.recorder.Event(cr, corev1.EventTypeNormal, "cert-manager.io", ApprovedMessage)
+
+	log.V(logf.DebugLevel).Info("approved certificate request")
+
+	return nil
+}

pkg/controller/certificaterequests/ca/ca_test.go

@@ -105,6 +105,8 @@ func generateSelfSignedCertFromCR(t *testing.T, cr *cmapi.CertificateRequest, ke
 }
 
 func TestSign(t *testing.T) {
+	metaFixedClockStart := metav1.NewTime(fixedClockStart)
+
 	baseIssuer := gen.Issuer("test-issuer",
 		gen.SetIssuerCA(cmapi.CAIssuer{SecretName: "root-ca-secret"}),
 		gen.AddIssuerCondition(cmapi.IssuerCondition{
@@ -123,7 +125,7 @@ func TestSign(t *testing.T) {
 	skRSAPEM := pki.EncodePKCS1PrivateKey(skRSA)
 	rsaCSR := generateCSR(t, skRSA)
 
-	baseCR := gen.CertificateRequest("test-cr",
+	baseCRNotApproved := gen.CertificateRequest("test-cr",
 		gen.SetCertificateRequestIsCA(true),
 		gen.SetCertificateRequestCSR(rsaCSR),
 		gen.SetCertificateRequestIssuer(cmmeta.ObjectReference{
@@ -133,6 +135,24 @@ func TestSign(t *testing.T) {
 		}),
 		gen.SetCertificateRequestDuration(&metav1.Duration{Duration: time.Hour * 24 * 60}),
 	)
+	baseCRDenied := gen.CertificateRequestFrom(baseCRNotApproved,
+		gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{
+			Type:               cmapi.CertificateRequestConditionDenied,
+			Status:             cmmeta.ConditionTrue,
+			Reason:             "Foo",
+			Message:            "Certificate request has been denied by cert-manager.io",
+			LastTransitionTime: &metaFixedClockStart,
+		}),
+	)
+	baseCR := gen.CertificateRequestFrom(baseCRNotApproved,
+		gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{
+			Type:               cmapi.CertificateRequestConditionApproved,
+			Status:             cmmeta.ConditionTrue,
+			Reason:             "cert-manager.io",
+			Message:            "Certificate request has been approved by cert-manager.io",
+			LastTransitionTime: &metaFixedClockStart,
+		}),
+	)
 
 	// generate a self signed root ca valid for 60d
 	_, rsaPEMCert := generateSelfSignedCertFromCR(t, baseCR, skRSA, time.Hour*24*60)
@@ -161,8 +181,21 @@ func TestSign(t *testing.T) {
 		t.FailNow()
 	}
 
-	metaFixedClockStart := metav1.NewTime(fixedClockStart)
 	tests := map[string]testT{
+		"a CertificateRequest without an approved condition should do nothing": {
+			certificateRequest: baseCRNotApproved.DeepCopy(),
+			builder: &testpkg.Builder{
+				KubeObjects:        []runtime.Object{},
+				CertManagerObjects: []runtime.Object{baseCRNotApproved.DeepCopy(), baseIssuer.DeepCopy()},
+			},
+		},
+		"a CertificateRequest with a denied condition should do nothing": {
+			certificateRequest: baseCRDenied.DeepCopy(),
+			builder: &testpkg.Builder{
+				KubeObjects:        []runtime.Object{},
+				CertManagerObjects: []runtime.Object{baseCRDenied.DeepCopy(), baseIssuer.DeepCopy()},
+			},
+		},
 		"a missing CA key pair should set the condition to pending and wait for a re-sync": {
 			certificateRequest: baseCR.DeepCopy(),
 			builder: &testpkg.Builder{

pkg/controller/certificaterequests/selfsigned/selfsigned_test.go

@@ -129,7 +129,7 @@ func TestSign(t *testing.T) {
 	}
 	csrECPEM := generateCSR(t, skEC, x509.ECDSAWithSHA256)
 
-	baseCR := gen.CertificateRequest("test-cr",
+	baseCRNotApproved := gen.CertificateRequest("test-cr",
 		gen.SetCertificateRequestAnnotations(
 			map[string]string{
 				cmapi.CertificateRequestPrivateKeyAnnotationKey: rsaKeySecret.Name,
@@ -142,6 +142,24 @@ func TestSign(t *testing.T) {
 			Kind:  "Issuer",
 		}),
 	)
+	baseCRDenied := gen.CertificateRequestFrom(baseCRNotApproved,
+		gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{
+			Type:               cmapi.CertificateRequestConditionDenied,
+			Status:             cmmeta.ConditionTrue,
+			Reason:             "Foo",
+			Message:            "Certificate request has been denied by cert-manager.io",
+			LastTransitionTime: &metaFixedClockStart,
+		}),
+	)
+	baseCR := gen.CertificateRequestFrom(baseCRNotApproved,
+		gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{
+			Type:               cmapi.CertificateRequestConditionApproved,
+			Status:             cmmeta.ConditionTrue,
+			Reason:             "cert-manager.io",
+			Message:            "Certificate request has been approved by cert-manager.io",
+			LastTransitionTime: &metaFixedClockStart,
+		}),
+	)
 	ecCR := gen.CertificateRequestFrom(baseCR,
 		gen.SetCertificateRequestCSR(csrECPEM),
 	)
@@ -169,6 +187,20 @@ func TestSign(t *testing.T) {
 	}
 
 	tests := map[string]testT{
+		"a CertificateRequest without an approved condition should do nothing": {
+			certificateRequest: baseCRNotApproved.DeepCopy(),
+			builder: &testpkg.Builder{
+				KubeObjects:        []runtime.Object{},
+				CertManagerObjects: []runtime.Object{baseCRNotApproved.DeepCopy(), baseIssuer.DeepCopy()},
+			},
+		},
+		"a CertificateRequest with a denied condition should do nothing": {
+			certificateRequest: baseCRDenied.DeepCopy(),
+			builder: &testpkg.Builder{
+				KubeObjects:        []runtime.Object{},
+				CertManagerObjects: []runtime.Object{baseCRDenied.DeepCopy(), baseIssuer.DeepCopy()},
+			},
+		},
 		"a CertificateRequest with no cert-manager.io/selfsigned-private-key annotation should fail": {
 			certificateRequest: gen.CertificateRequestFrom(baseCR,
 				// no annotation

pkg/controller/certificaterequests/sync.go

@@ -29,17 +29,17 @@ import (
 
 	apiutil "github.com/jetstack/cert-manager/pkg/api/util"
 	"github.com/jetstack/cert-manager/pkg/apis/certmanager"
-	v1 "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1"
+	cmapi "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1"
 	cmmeta "github.com/jetstack/cert-manager/pkg/apis/meta/v1"
 	logf "github.com/jetstack/cert-manager/pkg/logs"
 	"github.com/jetstack/cert-manager/pkg/util/pki"
 )
 
 var (
-	certificateRequestGvk = v1.SchemeGroupVersion.WithKind(v1.CertificateRequestKind)
+	certificateRequestGvk = cmapi.SchemeGroupVersion.WithKind(cmapi.CertificateRequestKind)
 )
 
-func (c *Controller) Sync(ctx context.Context, cr *v1.CertificateRequest) (err error) {
+func (c *Controller) Sync(ctx context.Context, cr *cmapi.CertificateRequest) (err error) {
 	log := logf.FromContext(ctx)
 	dbg := log.V(logf.DebugLevel)
 
@@ -48,12 +48,18 @@ func (c *Controller) Sync(ctx context.Context, cr *v1.CertificateRequest) (err e
 		return nil
 	}
 
+	// If CertificateRequest has not been approved or is denied, exit early.
+	if !apiutil.CertificateRequestIsApproved(cr) || apiutil.CertificateRequestIsDenied(cr) {
+		dbg.Info("certificate request has not been approved")
+		return nil
+	}
+
 	switch apiutil.CertificateRequestReadyReason(cr) {
-	case v1.CertificateRequestReasonFailed:
+	case cmapi.CertificateRequestReasonFailed:
 		dbg.Info("certificate request Ready condition failed so skipping processing")
 		return
 
-	case v1.CertificateRequestReasonIssued:
+	case cmapi.CertificateRequestReasonIssued:
 		dbg.Info("certificate request Ready condition true so skipping processing")
 		return
 	}
@@ -99,8 +105,8 @@ func (c *Controller) Sync(ctx context.Context, cr *v1.CertificateRequest) (err e
 	}
 
 	// check ready condition
-	if !apiutil.IssuerHasCondition(issuerObj, v1.IssuerCondition{
-		Type:   v1.IssuerConditionReady,
+	if !apiutil.IssuerHasCondition(issuerObj, cmapi.IssuerCondition{
+		Type:   cmapi.IssuerConditionReady,
 		Status: cmmeta.ConditionTrue,
 	}) {
 		c.reporter.Pending(crCopy, nil, "IssuerNotReady",
@@ -148,7 +154,7 @@ func (c *Controller) Sync(ctx context.Context, cr *v1.CertificateRequest) (err e
 	return nil
 }
 
-func (c *Controller) updateCertificateRequestStatusAndAnnotations(ctx context.Context, old, new *v1.CertificateRequest) (*v1.CertificateRequest, error) {
+func (c *Controller) updateCertificateRequestStatusAndAnnotations(ctx context.Context, old, new *cmapi.CertificateRequest) (*cmapi.CertificateRequest, error) {
 	log := logf.FromContext(ctx, "updateStatus")
 
 	// if annotations changed we have to call .Update() and not .UpdateStatus()

pkg/controller/certificaterequests/sync_test.go

@@ -122,7 +122,7 @@ func TestSync(t *testing.T) {
 		}),
 	)
 
-	baseCR := gen.CertificateRequest("test-cr",
+	baseCRNotApproved := gen.CertificateRequest("test-cr",
 		gen.SetCertificateRequestIsCA(false),
 		gen.SetCertificateRequestCSR(csrRSAPEM),
 		gen.SetCertificateRequestIssuer(cmmeta.ObjectReference{
@@ -131,6 +131,16 @@ func TestSync(t *testing.T) {
 		}),
 	)
 
+	baseCR := gen.CertificateRequestFrom(baseCRNotApproved,
+		gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{
+			Type:               cmapi.CertificateRequestConditionApproved,
+			Status:             cmmeta.ConditionTrue,
+			Reason:             "cert-manager.io",
+			Message:            "Certificate request has been approved by cert-manager.io",
+			LastTransitionTime: &nowMetaTime,
+		}),
+	)
+
 	certRSAPEM := generateSelfSignedCert(t, baseCR, skRSA, fixedClockStart, fixedClockStart.Add(time.Hour*12))
 	certRSAPEMExpired := generateSelfSignedCert(t, baseCR, skRSA, fixedClockStart.Add(-time.Hour*13), fixedClockStart.Add(-time.Hour*12))
 
@@ -150,6 +160,46 @@ func TestSync(t *testing.T) {
 				ExpectedActions:    []testpkg.Action{},
 			},
 		},
+		"should return nil (no action) if certificate request is not approved": {
+			certificateRequest: gen.CertificateRequestFrom(baseCRNotApproved),
+			builder: &testpkg.Builder{
+				CertManagerObjects: []runtime.Object{baseIssuer, baseCR},
+				ExpectedEvents:     []string{},
+				ExpectedActions:    []testpkg.Action{},
+			},
+		},
+		"should return nil (no action) if certificate request is denied": {
+			certificateRequest: gen.CertificateRequestFrom(baseCRNotApproved,
+				gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{
+					Type:               cmapi.CertificateRequestConditionDenied,
+					Status:             cmmeta.ConditionTrue,
+					Reason:             "Foo",
+					Message:            "Certificate request has been denied by cert-manager.io",
+					LastTransitionTime: &nowMetaTime,
+				}),
+			),
+			builder: &testpkg.Builder{
+				CertManagerObjects: []runtime.Object{baseIssuer, baseCR},
+				ExpectedEvents:     []string{},
+				ExpectedActions:    []testpkg.Action{},
+			},
+		},
+		"should return nil (no action) if certificate request approved is set to false": {
+			certificateRequest: gen.CertificateRequestFrom(baseCRNotApproved,
+				gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{
+					Type:               cmapi.CertificateRequestConditionApproved,
+					Status:             cmmeta.ConditionFalse,
+					Reason:             "cert-manager.io",
+					Message:            "Certificate request has not been approved",
+					LastTransitionTime: &nowMetaTime,
+				}),
+			),
+			builder: &testpkg.Builder{
+				CertManagerObjects: []runtime.Object{baseIssuer, baseCR},
+				ExpectedEvents:     []string{},
+				ExpectedActions:    []testpkg.Action{},
+			},
+		},
 		"should return nil (no action) if certificate request is ready and reason Issued": {
 			certificateRequest: gen.CertificateRequestFrom(baseCR,
 				gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{

pkg/controller/certificaterequests/vault/vault_test.go

@@ -113,7 +113,7 @@ func TestSign(t *testing.T) {
 
 	csrPEM := generateCSR(t, rsaSK)
 
-	baseCR := gen.CertificateRequest("test-cr",
+	baseCRNotApproved := gen.CertificateRequest("test-cr",
 		gen.SetCertificateRequestIsCA(true),
 		gen.SetCertificateRequestCSR(csrPEM),
 		gen.SetCertificateRequestDuration(&metav1.Duration{Duration: time.Hour * 24 * 60}),
@@ -123,6 +123,24 @@ func TestSign(t *testing.T) {
 			Kind:  baseIssuer.Kind,
 		}),
 	)
+	baseCRDenied := gen.CertificateRequestFrom(baseCRNotApproved,
+		gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{
+			Type:               cmapi.CertificateRequestConditionDenied,
+			Status:             cmmeta.ConditionTrue,
+			Reason:             "Foo",
+			Message:            "Certificate request has been denied by cert-manager.io",
+			LastTransitionTime: &metaFixedClockStart,
+		}),
+	)
+	baseCR := gen.CertificateRequestFrom(baseCRNotApproved,
+		gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{
+			Type:               cmapi.CertificateRequestConditionApproved,
+			Status:             cmmeta.ConditionTrue,
+			Reason:             "cert-manager.io",
+			Message:            "Certificate request has been approved by cert-manager.io",
+			LastTransitionTime: &metaFixedClockStart,
+		}),
+	)
 
 	rsaPEMCert, err := generateSelfSignedCertFromCR(baseCR, rsaSK, time.Hour*24*60)
 	if err != nil {
@@ -151,6 +169,20 @@ func TestSign(t *testing.T) {
 	}
 
 	tests := map[string]testT{
+		"a CertificateRequest without an approved condition should do nothing": {
+			certificateRequest: baseCRNotApproved.DeepCopy(),
+			builder: &testpkg.Builder{
+				KubeObjects:        []runtime.Object{},
+				CertManagerObjects: []runtime.Object{baseCRNotApproved.DeepCopy(), baseIssuer.DeepCopy()},
+			},
+		},
+		"a CertificateRequest with a denied condition should do nothing": {
+			certificateRequest: baseCRDenied.DeepCopy(),
+			builder: &testpkg.Builder{
+				KubeObjects:        []runtime.Object{},
+				CertManagerObjects: []runtime.Object{baseCRDenied.DeepCopy(), baseIssuer.DeepCopy()},
+			},
+		},
 		"no token, app role secret or kubernetes auth reference should report pending": {
 			certificateRequest: baseCR.DeepCopy(),
 			builder: &testpkg.Builder{

pkg/controller/certificaterequests/venafi/venafi_test.go

@@ -78,6 +78,7 @@ func generateCSR(t *testing.T, secretKey crypto.Signer, alg x509.SignatureAlgori
 }
 
 func TestSign(t *testing.T) {
+	metaFixedClockStart := metav1.NewTime(fixedClockStart)
 	rsaSK, err := pki.GenerateRSAPrivateKey(2048)
 	if err != nil {
 		t.Error(err)
@@ -135,9 +136,27 @@ func TestSign(t *testing.T) {
 		}),
 	)
 
-	baseCR := gen.CertificateRequest("test-cr",
+	baseCRNotApproved := gen.CertificateRequest("test-cr",
 		gen.SetCertificateRequestCSR(csrPEM),
 	)
+	baseCRDenied := gen.CertificateRequestFrom(baseCRNotApproved,
+		gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{
+			Type:               cmapi.CertificateRequestConditionDenied,
+			Status:             cmmeta.ConditionTrue,
+			Reason:             "Foo",
+			Message:            "Certificate request has been denied by cert-manager.io",
+			LastTransitionTime: &metaFixedClockStart,
+		}),
+	)
+	baseCR := gen.CertificateRequestFrom(baseCRNotApproved,
+		gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{
+			Type:               cmapi.CertificateRequestConditionApproved,
+			Status:             cmmeta.ConditionTrue,
+			Reason:             "cert-manager.io",
+			Message:            "Certificate request has been approved by cert-manager.io",
+			LastTransitionTime: &metaFixedClockStart,
+		}),
+	)
 
 	tppCR := gen.CertificateRequestFrom(baseCR,
 		gen.SetCertificateRequestIssuer(cmmeta.ObjectReference{
@@ -226,8 +245,21 @@ func TestSign(t *testing.T) {
 		},
 	}
 
-	metaFixedClockStart := metav1.NewTime(fixedClockStart)
 	tests := map[string]testT{
+		"a CertificateRequest without an approved condition should do nothing": {
+			certificateRequest: baseCRNotApproved.DeepCopy(),
+			builder: &testpkg.Builder{
+				KubeObjects:        []runtime.Object{},
+				CertManagerObjects: []runtime.Object{baseCRNotApproved.DeepCopy(), baseIssuer.DeepCopy()},
+			},
+		},
+		"a CertificateRequest with a denied condition should do nothing": {
+			certificateRequest: baseCRDenied.DeepCopy(),
+			builder: &testpkg.Builder{
+				KubeObjects:        []runtime.Object{},
+				CertManagerObjects: []runtime.Object{baseCRDenied.DeepCopy(), baseIssuer.DeepCopy()},
+			},
+		},
 		"tpp: if fail to build client based on missing secret then return nil and hard fail": {
 			certificateRequest: tppCR.DeepCopy(),
 			builder: &controllertest.Builder{

pkg/internal/apis/certmanager/types_certificaterequest.go

@@ -136,7 +136,8 @@ type CertificateRequestStatus struct {
 
 // CertificateRequestCondition contains condition information for a CertificateRequest.
 type CertificateRequestCondition struct {
-	// Type of the condition, known values are (`Ready`, `InvalidRequest`).
+	// Type of the condition, known values are (`Ready`,
+	// `InvalidRequest`, `Approved`, `Denied`).
 	Type CertificateRequestConditionType
 
 	// Status of the condition, one of (`True`, `False`, `Unknown`).
@@ -169,4 +170,14 @@ const (
 	// parameters being invalid. Additional information about why the request
 	// was rejected can be found in the `reason` and `message` fields.
 	CertificateRequestConditionInvalidRequest CertificateRequestConditionType = "InvalidRequest"
+
+	// CertificateRequestConditionApproved indicates that a certificate request
+	// is approved and ready for signing. Condition must never have a status of
+	// `False`, and cannot be modified once set.
+	CertificateRequestConditionApproved CertificateRequestConditionType = "Approved"
+
+	// CertificateRequestConditionDenied indicates that a certificate request is
+	// denied, and must never be signed. Condition must never have a status of
+	// `False`, and cannot be modified once set.
+	CertificateRequestConditionDenied CertificateRequestConditionType = "Denied"
 )

pkg/internal/apis/certmanager/validation/certificaterequest.go

@@ -31,6 +31,7 @@ import (
 	"github.com/jetstack/cert-manager/pkg/apis/acme"
 	"github.com/jetstack/cert-manager/pkg/apis/certmanager"
 	cmapi "github.com/jetstack/cert-manager/pkg/internal/apis/certmanager"
+	cmmeta "github.com/jetstack/cert-manager/pkg/internal/apis/meta"
 	"github.com/jetstack/cert-manager/pkg/util"
 	"github.com/jetstack/cert-manager/pkg/util/pki"
 )
@@ -40,6 +41,9 @@ var defaultInternalKeyUsages = []cmapi.KeyUsage{cmapi.UsageDigitalSignature, cma
 func ValidateCertificateRequest(_ *admissionv1.AdmissionRequest, obj runtime.Object) field.ErrorList {
 	cr := obj.(*cmapi.CertificateRequest)
 	allErrs := ValidateCertificateRequestSpec(&cr.Spec, field.NewPath("spec"), true)
+	allErrs = append(allErrs,
+		ValidateCertificateRequestApprovalCondition(cr.Status.Conditions, field.NewPath("status", "conditions"))...)
+
 	return allErrs
 }
 
@@ -55,6 +59,8 @@ func ValidateUpdateCertificateRequest(_ *admissionv1.AdmissionRequest, oldObj, n
 	annotationField := field.NewPath("metadata", "annotations")
 	el = append(el, validateCertificateRequestAnnotations(oldCR, newCR, annotationField)...)
 	el = append(el, validateCertificateRequestAnnotations(newCR, oldCR, annotationField)...)
+	el = append(el,
+		ValidateUpdateCertificateRequestApprovalCondition(oldCR.Status.Conditions, newCR.Status.Conditions, field.NewPath("status", "conditions"))...)
 
 	if !reflect.DeepEqual(oldCR.Spec, newCR.Spec) {
 		el = append(el, field.Forbidden(field.NewPath("spec"), "cannot change spec after creation"))
@@ -107,6 +113,80 @@ func ValidateCertificateRequestSpec(crSpec *cmapi.CertificateRequestSpec, fldPat
 	return el
 }
 
+// ValidateCertificateRequestApprovalCondition will ensure that only a single
+// 'Approved' or 'Denied' condition may exist, and that they are set to True.
+func ValidateCertificateRequestApprovalCondition(crConds []cmapi.CertificateRequestCondition, fldPath *field.Path) field.ErrorList {
+	var (
+		approvedConditions []cmapi.CertificateRequestCondition
+		deniedConditions   []cmapi.CertificateRequestCondition
+		el                 = field.ErrorList{}
+	)
+
+	for _, cond := range crConds {
+		if cond.Type == cmapi.CertificateRequestConditionApproved {
+			approvedConditions = append(approvedConditions, cond)
+		}
+
+		if cond.Type == cmapi.CertificateRequestConditionDenied {
+			deniedConditions = append(deniedConditions, cond)
+		}
+	}
+
+	for _, cond := range []struct {
+		condType   cmapi.CertificateRequestConditionType
+		conditions []cmapi.CertificateRequestCondition
+	}{
+		{cmapi.CertificateRequestConditionApproved, approvedConditions},
+		{cmapi.CertificateRequestConditionDenied, deniedConditions},
+	} {
+		switch len(cond.conditions) {
+		case 0:
+			break
+		case 1:
+			if condition := cond.conditions[0]; condition.Status != cmmeta.ConditionTrue {
+				el = append(el, field.Invalid(fldPath.Child(string(condition.Type)), condition.Status,
+					fmt.Sprintf("%q condition may only be set to True", cond.condType)))
+			}
+		default:
+			el = append(el, field.Forbidden(fldPath, fmt.Sprintf("multiple %q conditions present", cond.condType)))
+		}
+	}
+
+	if len(deniedConditions) > 0 && len(approvedConditions) > 0 {
+		el = append(el, field.Forbidden(fldPath, "both 'Denied' and 'Approved' conditions cannot coexist"))
+	}
+
+	return el
+}
+
+// ValidateUpdateCertificateRequestApprovalCondition will ensure that the
+// 'Approved' and 'Denied' conditions may not be changed once set, i.e. if they
+// exist, they are not modified in the updated resource. Also runs the base
+// approval validation on the updated CertificateRequest conditions.
+func ValidateUpdateCertificateRequestApprovalCondition(oldCRConds, newCRConds []cmapi.CertificateRequestCondition, fldPath *field.Path) field.ErrorList {
+	var (
+		el            = field.ErrorList{}
+		oldCRDenied   = getCertificateRequestCondition(oldCRConds, cmapi.CertificateRequestConditionDenied)
+		oldCRApproved = getCertificateRequestCondition(oldCRConds, cmapi.CertificateRequestConditionApproved)
+	)
+
+	// If the approval condition has been set, ensure it hasn't been modified.
+	if oldCRApproved != nil && !reflect.DeepEqual(oldCRApproved,
+		getCertificateRequestCondition(newCRConds, cmapi.CertificateRequestConditionApproved),
+	) {
+		el = append(el, field.Forbidden(fldPath, "'Approved' condition may not be modified once set"))
+	}
+
+	// If the denied condition has been set, ensure it hasn't been modified.
+	if oldCRDenied != nil && !reflect.DeepEqual(oldCRDenied,
+		getCertificateRequestCondition(newCRConds, cmapi.CertificateRequestConditionDenied),
+	) {
+		el = append(el, field.Forbidden(fldPath, "'Denied' condition may not be modified once set"))
+	}
+
+	return append(el, ValidateCertificateRequestApprovalCondition(newCRConds, fldPath)...)
+}
+
 func getCSRKeyUsage(crSpec *cmapi.CertificateRequestSpec, fldPath *field.Path, csr *x509.CertificateRequest, el field.ErrorList) ([]cmapi.KeyUsage, field.ErrorList) {
 	var ekus []x509.ExtKeyUsage
 	var ku x509.KeyUsage
@@ -200,3 +280,12 @@ func ensureCertSignIsSet(list []cmapi.KeyUsage) []cmapi.KeyUsage {
 
 	return append(list, cmapi.UsageCertSign)
 }
+
+func getCertificateRequestCondition(conds []cmapi.CertificateRequestCondition, conditionType cmapi.CertificateRequestConditionType) *cmapi.CertificateRequestCondition {
+	for _, cond := range conds {
+		if cond.Type == conditionType {
+			return &cond
+		}
+	}
+	return nil
+}

pkg/internal/apis/certmanager/validation/certificaterequest_test.go

@@ -27,12 +27,17 @@ import (
 
 	cmapi "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1"
 	cminternal "github.com/jetstack/cert-manager/pkg/internal/apis/certmanager"
+	cminternalmeta "github.com/jetstack/cert-manager/pkg/internal/apis/meta"
 	"github.com/jetstack/cert-manager/pkg/util/pki"
 	utilpki "github.com/jetstack/cert-manager/pkg/util/pki"
 	"github.com/jetstack/cert-manager/test/unit/gen"
 )
 
 func TestValidateCertificateRequestUpdate(t *testing.T) {
+	fldPathConditions := field.NewPath("status", "conditions")
+
+	baseRequest := mustGenerateCSR(t, gen.Certificate("test", gen.SetCertificateDNSNames("example.com")))
+
 	baseCR := &cminternal.CertificateRequest{
 		ObjectMeta: metav1.ObjectMeta{
 			Annotations: map[string]string{
@@ -42,7 +47,7 @@ func TestValidateCertificateRequestUpdate(t *testing.T) {
 			},
 		},
 		Spec: cminternal.CertificateRequestSpec{
-			Request:   mustGenerateCSR(t, gen.Certificate("test", gen.SetCertificateDNSNames("example.com"))),
+			Request:   baseRequest,
 			IssuerRef: validIssuerRef,
 			Usages:    nil,
 			UID:       "abc",
@@ -100,136 +105,651 @@ func TestValidateCertificateRequestUpdate(t *testing.T) {
 			newCR: baseCR.DeepCopy(),
 			want:  nil,
 		},
+		"CertificateRequest with single Approved=true condition that doesn't change, shouldn't error": {
+			oldCR: &cminternal.CertificateRequest{
+				Spec: cminternal.CertificateRequestSpec{
+					Request:   baseRequest,
+					IssuerRef: validIssuerRef,
+				},
+				Status: cminternal.CertificateRequestStatus{
+					Conditions: []cminternal.CertificateRequestCondition{
+						{
+							Type:   cminternal.CertificateRequestConditionApproved,
+							Status: cminternalmeta.ConditionTrue,
+						},
+					},
+				},
+			},
+			newCR: &cminternal.CertificateRequest{
+				Spec: cminternal.CertificateRequestSpec{
+					Request:   baseRequest,
+					IssuerRef: validIssuerRef,
+				},
+				Status: cminternal.CertificateRequestStatus{
+					Conditions: []cminternal.CertificateRequestCondition{
+						{
+							Type:   cminternal.CertificateRequestConditionApproved,
+							Status: cminternalmeta.ConditionTrue,
+						},
+					},
+				},
+			},
+			want: nil,
+		},
+		"CertificateRequest with single Denied=true condition that doesn't change, shouldn't error": {
+			oldCR: &cminternal.CertificateRequest{
+				Spec: cminternal.CertificateRequestSpec{
+					Request:   baseRequest,
+					IssuerRef: validIssuerRef,
+				},
+				Status: cminternal.CertificateRequestStatus{
+					Conditions: []cminternal.CertificateRequestCondition{
+						{
+							Type:   cminternal.CertificateRequestConditionDenied,
+							Status: cminternalmeta.ConditionTrue,
+						},
+					},
+				},
+			},
+			newCR: &cminternal.CertificateRequest{
+				Spec: cminternal.CertificateRequestSpec{
+					Request:   baseRequest,
+					IssuerRef: validIssuerRef,
+				},
+				Status: cminternal.CertificateRequestStatus{
+					Conditions: []cminternal.CertificateRequestCondition{
+						{
+							Type:   cminternal.CertificateRequestConditionDenied,
+							Status: cminternalmeta.ConditionTrue,
+						},
+					},
+				},
+			},
+			want: nil,
+		},
+		"CertificateRequest with single Approved=false condition that changes, should error": {
+			oldCR: &cminternal.CertificateRequest{
+				Spec: cminternal.CertificateRequestSpec{
+					Request:   baseRequest,
+					IssuerRef: validIssuerRef,
+				},
+				Status: cminternal.CertificateRequestStatus{
+					Conditions: []cminternal.CertificateRequestCondition{
+						{
+							Type:   cminternal.CertificateRequestConditionApproved,
+							Status: cminternalmeta.ConditionFalse,
+							Reason: "Foo",
+						},
+					},
+				},
+			},
+			newCR: &cminternal.CertificateRequest{
+				Spec: cminternal.CertificateRequestSpec{
+					Request:   baseRequest,
+					IssuerRef: validIssuerRef,
+				},
+				Status: cminternal.CertificateRequestStatus{
+					Conditions: []cminternal.CertificateRequestCondition{
+						{
+							Type:   cminternal.CertificateRequestConditionApproved,
+							Status: cminternalmeta.ConditionTrue,
+							Reason: "cert-manager.io",
+						},
+					},
+				},
+			},
+			want: []*field.Error{
+				field.Forbidden(fldPathConditions, "'Approved' condition may not be modified once set"),
+			},
+		},
+		"CertificateRequest with single Denied=false condition that changes, should error": {
+			oldCR: &cminternal.CertificateRequest{
+				Spec: cminternal.CertificateRequestSpec{
+					Request:   baseRequest,
+					IssuerRef: validIssuerRef,
+				},
+				Status: cminternal.CertificateRequestStatus{
+					Conditions: []cminternal.CertificateRequestCondition{
+						{
+							Type:   cminternal.CertificateRequestConditionDenied,
+							Status: cminternalmeta.ConditionTrue,
+							Reason: "Foo",
+						},
+					},
+				},
+			},
+			newCR: &cminternal.CertificateRequest{
+				Spec: cminternal.CertificateRequestSpec{
+					Request:   baseRequest,
+					IssuerRef: validIssuerRef,
+				},
+				Status: cminternal.CertificateRequestStatus{
+					Conditions: []cminternal.CertificateRequestCondition{
+						{
+							Type:   cminternal.CertificateRequestConditionDenied,
+							Status: cminternalmeta.ConditionFalse,
+							Reason: "test",
+						},
+					},
+				},
+			},
+			want: []*field.Error{
+				field.Forbidden(fldPathConditions, "'Denied' condition may not be modified once set"),
+				field.Invalid(fldPathConditions.Child("Denied"), nil, `"Denied" condition may only be set to True`),
+			},
+		},
+		"CertificateRequest with single Denied=true condition that changes to Approve=true, should error": {
+			oldCR: &cminternal.CertificateRequest{
+				Spec: cminternal.CertificateRequestSpec{
+					Request:   baseRequest,
+					IssuerRef: validIssuerRef,
+				},
+				Status: cminternal.CertificateRequestStatus{
+					Conditions: []cminternal.CertificateRequestCondition{
+						{
+							Type:   cminternal.CertificateRequestConditionDenied,
+							Status: cminternalmeta.ConditionTrue,
+							Reason: "Foo",
+						},
+					},
+				},
+			},
+			newCR: &cminternal.CertificateRequest{
+				Spec: cminternal.CertificateRequestSpec{
+					Request:   baseRequest,
+					IssuerRef: validIssuerRef,
+				},
+				Status: cminternal.CertificateRequestStatus{
+					Conditions: []cminternal.CertificateRequestCondition{
+						{
+							Type:   cminternal.CertificateRequestConditionApproved,
+							Status: cminternalmeta.ConditionTrue,
+							Reason: "cert-manager.io",
+						},
+					},
+				},
+			},
+			want: []*field.Error{
+				field.Forbidden(fldPathConditions, "'Denied' condition may not be modified once set"),
+			},
+		},
+		"CertificateRequest with single Approved=true condition that changes to Denied=true, should error": {
+			oldCR: &cminternal.CertificateRequest{
+				Spec: cminternal.CertificateRequestSpec{
+					Request:   baseRequest,
+					IssuerRef: validIssuerRef,
+				},
+				Status: cminternal.CertificateRequestStatus{
+					Conditions: []cminternal.CertificateRequestCondition{
+						{
+							Type:   cminternal.CertificateRequestConditionApproved,
+							Status: cminternalmeta.ConditionTrue,
+							Reason: "cert-manager.io",
+						},
+					},
+				},
+			},
+			newCR: &cminternal.CertificateRequest{
+				Spec: cminternal.CertificateRequestSpec{
+					Request:   baseRequest,
+					IssuerRef: validIssuerRef,
+				},
+				Status: cminternal.CertificateRequestStatus{
+					Conditions: []cminternal.CertificateRequestCondition{
+						{
+							Type:   cminternal.CertificateRequestConditionDenied,
+							Status: cminternalmeta.ConditionTrue,
+							Reason: "Foo",
+						},
+					},
+				},
+			},
+			want: []*field.Error{
+				field.Forbidden(fldPathConditions, "'Approved' condition may not be modified once set"),
+			},
+		},
+		"CertificateRequest with no condition that changes to Approve=true, shouldn't error": {
+			oldCR: &cminternal.CertificateRequest{
+				Spec: cminternal.CertificateRequestSpec{
+					Request:   baseRequest,
+					IssuerRef: validIssuerRef,
+				},
+				Status: cminternal.CertificateRequestStatus{
+					Conditions: []cminternal.CertificateRequestCondition{},
+				},
+			},
+			newCR: &cminternal.CertificateRequest{
+				Spec: cminternal.CertificateRequestSpec{
+					Request:   baseRequest,
+					IssuerRef: validIssuerRef,
+				},
+				Status: cminternal.CertificateRequestStatus{
+					Conditions: []cminternal.CertificateRequestCondition{
+						{
+							Type:   cminternal.CertificateRequestConditionApproved,
+							Status: cminternalmeta.ConditionTrue,
+							Reason: "cert-manager.io",
+						},
+					},
+				},
+			},
+			want: nil,
+		},
+		"CertificateRequest with no condition that changes to Denied=true, shouldn't error": {
+			oldCR: &cminternal.CertificateRequest{
+				Spec: cminternal.CertificateRequestSpec{
+					Request:   baseRequest,
+					IssuerRef: validIssuerRef,
+				},
+				Status: cminternal.CertificateRequestStatus{
+					Conditions: []cminternal.CertificateRequestCondition{},
+				},
+			},
+			newCR: &cminternal.CertificateRequest{
+				Spec: cminternal.CertificateRequestSpec{
+					Request:   baseRequest,
+					IssuerRef: validIssuerRef,
+				},
+				Status: cminternal.CertificateRequestStatus{
+					Conditions: []cminternal.CertificateRequestCondition{
+						{
+							Type:   cminternal.CertificateRequestConditionDenied,
+							Status: cminternalmeta.ConditionTrue,
+							Reason: "Foo",
+						},
+					},
+				},
+			},
+			want: nil,
+		},
+		"CertificateRequest with single Approved=true condition that is removed, should error": {
+			oldCR: &cminternal.CertificateRequest{
+				Spec: cminternal.CertificateRequestSpec{
+					Request:   baseRequest,
+					IssuerRef: validIssuerRef,
+				},
+				Status: cminternal.CertificateRequestStatus{
+					Conditions: []cminternal.CertificateRequestCondition{
+						{
+							Type:   cminternal.CertificateRequestConditionApproved,
+							Status: cminternalmeta.ConditionTrue,
+						},
+					},
+				},
+			},
+			newCR: &cminternal.CertificateRequest{
+				Spec: cminternal.CertificateRequestSpec{
+					Request:   baseRequest,
+					IssuerRef: validIssuerRef,
+				},
+				Status: cminternal.CertificateRequestStatus{
+					Conditions: []cminternal.CertificateRequestCondition{},
+				},
+			},
+			want: []*field.Error{
+				field.Forbidden(fldPathConditions, "'Approved' condition may not be modified once set"),
+			},
+		},
+		"CertificateRequest with single Denied=true condition that is removed, should error": {
+			oldCR: &cminternal.CertificateRequest{
+				Spec: cminternal.CertificateRequestSpec{
+					Request:   baseRequest,
+					IssuerRef: validIssuerRef,
+				},
+				Status: cminternal.CertificateRequestStatus{
+					Conditions: []cminternal.CertificateRequestCondition{
+						{
+							Type:   cminternal.CertificateRequestConditionDenied,
+							Status: cminternalmeta.ConditionTrue,
+						},
+					},
+				},
+			},
+			newCR: &cminternal.CertificateRequest{
+				Spec: cminternal.CertificateRequestSpec{
+					Request:   baseRequest,
+					IssuerRef: validIssuerRef,
+				},
+				Status: cminternal.CertificateRequestStatus{
+					Conditions: []cminternal.CertificateRequestCondition{},
+				},
+			},
+			want: []*field.Error{
+				field.Forbidden(fldPathConditions, "'Denied' condition may not be modified once set"),
+			},
+		},
 	}
 
 	for name, test := range tests {
 		t.Run(name, func(t *testing.T) {
-			err := ValidateUpdateCertificateRequest(nil, test.oldCR, test.newCR)
-			if !reflect.DeepEqual(err, test.want) {
-				t.Errorf("got unexpected error response, exp=%v got=%v",
-					test.want, err)
+			got := ValidateUpdateCertificateRequest(nil, test.oldCR, test.newCR)
+			for i := range got {
+				if got[i].Type != field.ErrorTypeForbidden {
+					// filter out the value so it does not print the full CSR in tests
+					got[i].BadValue = nil
+				}
+			}
+
+			if !reflect.DeepEqual(got, test.want) {
+				t.Errorf("ValidateUpdateCertificateRequest() = %v, want %v", got, test.want)
 			}
 		})
 	}
 }
 
-func TestValidateCertificateRequestSpec(t *testing.T) {
-	fldPath := field.NewPath("test")
+func TestValidateCertificateRequest(t *testing.T) {
+	fldPath := field.NewPath("spec")
+	fldPathConditions := field.NewPath("status", "conditions")
 
-	tests := []struct {
-		name   string
-		crSpec *cminternal.CertificateRequestSpec
-		want   field.ErrorList
+	tests := map[string]struct {
+		cr   *cminternal.CertificateRequest
+		want field.ErrorList
 	}{
-		{
-			name: "Test csr with no usages",
-			crSpec: &cminternal.CertificateRequestSpec{
-				Request:   mustGenerateCSR(t, gen.Certificate("test", gen.SetCertificateDNSNames("example.com"))),
-				IssuerRef: validIssuerRef,
-				Usages:    nil,
+		"Test csr with no usages": {
+			cr: &cminternal.CertificateRequest{
+				Spec: cminternal.CertificateRequestSpec{
+					Request:   mustGenerateCSR(t, gen.Certificate("test", gen.SetCertificateDNSNames("example.com"))),
+					IssuerRef: validIssuerRef,
+					Usages:    nil,
+				},
 			},
 			want: []*field.Error{},
 		},
-		{
-			name: "Test csr with double signature usages",
-			crSpec: &cminternal.CertificateRequestSpec{
-				Request:   mustGenerateCSR(t, gen.Certificate("test", gen.SetCertificateDNSNames("example.com"), gen.SetCertificateKeyUsages(cmapi.UsageSigning, cmapi.UsageDigitalSignature, cmapi.UsageKeyEncipherment))),
-				IssuerRef: validIssuerRef,
-				Usages:    []cminternal.KeyUsage{cminternal.UsageSigning, cminternal.UsageKeyEncipherment},
+		"Test csr with double signature usages": {
+			cr: &cminternal.CertificateRequest{
+				Spec: cminternal.CertificateRequestSpec{
+					Request:   mustGenerateCSR(t, gen.Certificate("test", gen.SetCertificateDNSNames("example.com"), gen.SetCertificateKeyUsages(cmapi.UsageSigning, cmapi.UsageDigitalSignature, cmapi.UsageKeyEncipherment))),
+					IssuerRef: validIssuerRef,
+					Usages:    []cminternal.KeyUsage{cminternal.UsageSigning, cminternal.UsageKeyEncipherment},
+				},
 			},
 			want: []*field.Error{},
 		},
-		{
-			name: "Test csr with double extended usages",
-			crSpec: &cminternal.CertificateRequestSpec{
-				Request:   mustGenerateCSR(t, gen.Certificate("test", gen.SetCertificateDNSNames("example.com"), gen.SetCertificateKeyUsages(cmapi.UsageDigitalSignature, cmapi.UsageKeyEncipherment, cmapi.UsageServerAuth, cmapi.UsageClientAuth))),
-				IssuerRef: validIssuerRef,
-				Usages:    []cminternal.KeyUsage{cminternal.UsageSigning, cminternal.UsageKeyEncipherment, cminternal.UsageServerAuth, cminternal.UsageClientAuth},
+		"Test csr with double extended usages": {
+			cr: &cminternal.CertificateRequest{
+				Spec: cminternal.CertificateRequestSpec{
+					Request:   mustGenerateCSR(t, gen.Certificate("test", gen.SetCertificateDNSNames("example.com"), gen.SetCertificateKeyUsages(cmapi.UsageDigitalSignature, cmapi.UsageKeyEncipherment, cmapi.UsageServerAuth, cmapi.UsageClientAuth))),
+					IssuerRef: validIssuerRef,
+					Usages:    []cminternal.KeyUsage{cminternal.UsageSigning, cminternal.UsageKeyEncipherment, cminternal.UsageServerAuth, cminternal.UsageClientAuth},
+				},
 			},
 			want: []*field.Error{},
 		},
-		{
-			name: "Test csr with reordered usages",
-			crSpec: &cminternal.CertificateRequestSpec{
-				Request:   mustGenerateCSR(t, gen.Certificate("test", gen.SetCertificateDNSNames("example.com"), gen.SetCertificateKeyUsages(cmapi.UsageDigitalSignature, cmapi.UsageKeyEncipherment, cmapi.UsageServerAuth, cmapi.UsageClientAuth))),
-				IssuerRef: validIssuerRef,
-				Usages:    []cminternal.KeyUsage{cminternal.UsageServerAuth, cminternal.UsageClientAuth, cminternal.UsageKeyEncipherment, cminternal.UsageDigitalSignature},
+		"Test csr with reordered usages": {
+			cr: &cminternal.CertificateRequest{
+				Spec: cminternal.CertificateRequestSpec{
+					Request:   mustGenerateCSR(t, gen.Certificate("test", gen.SetCertificateDNSNames("example.com"), gen.SetCertificateKeyUsages(cmapi.UsageDigitalSignature, cmapi.UsageKeyEncipherment, cmapi.UsageServerAuth, cmapi.UsageClientAuth))),
+					IssuerRef: validIssuerRef,
+					Usages:    []cminternal.KeyUsage{cminternal.UsageServerAuth, cminternal.UsageClientAuth, cminternal.UsageKeyEncipherment, cminternal.UsageDigitalSignature},
+				},
 			},
 			want: []*field.Error{},
 		},
-		{
-			name: "Test csr that is CA with usages set",
-			crSpec: &cminternal.CertificateRequestSpec{
-				Request:   mustGenerateCSR(t, gen.Certificate("test", gen.SetCertificateDNSNames("example.com"), gen.SetCertificateKeyUsages(cmapi.UsageAny, cmapi.UsageDigitalSignature, cmapi.UsageKeyEncipherment, cmapi.UsageCertSign), gen.SetCertificateIsCA(true))),
-				IssuerRef: validIssuerRef,
-				IsCA:      true,
-				Usages:    []cminternal.KeyUsage{cminternal.UsageAny, cminternal.UsageDigitalSignature, cminternal.UsageKeyEncipherment, cminternal.UsageCertSign},
+		"Test csr that is CA with usages set": {
+			cr: &cminternal.CertificateRequest{
+				Spec: cminternal.CertificateRequestSpec{
+					Request:   mustGenerateCSR(t, gen.Certificate("test", gen.SetCertificateDNSNames("example.com"), gen.SetCertificateKeyUsages(cmapi.UsageAny, cmapi.UsageDigitalSignature, cmapi.UsageKeyEncipherment, cmapi.UsageCertSign), gen.SetCertificateIsCA(true))),
+					IssuerRef: validIssuerRef,
+					IsCA:      true,
+					Usages:    []cminternal.KeyUsage{cminternal.UsageAny, cminternal.UsageDigitalSignature, cminternal.UsageKeyEncipherment, cminternal.UsageCertSign},
+				},
 			},
 			want: []*field.Error{},
 		},
-		{
-			name: "Test csr that is CA but no cert sign in usages",
-			crSpec: &cminternal.CertificateRequestSpec{
-				Request:   mustGenerateCSR(t, gen.Certificate("test", gen.SetCertificateDNSNames("example.com"), gen.SetCertificateKeyUsages(cmapi.UsageAny, cmapi.UsageDigitalSignature, cmapi.UsageKeyEncipherment, cmapi.UsageClientAuth, cmapi.UsageServerAuth), gen.SetCertificateIsCA(true))),
-				IssuerRef: validIssuerRef,
-				IsCA:      true,
-				Usages:    []cminternal.KeyUsage{cminternal.UsageAny, cminternal.UsageDigitalSignature, cminternal.UsageKeyEncipherment, cminternal.UsageClientAuth, cminternal.UsageServerAuth},
+		"Test csr that is CA but no cert sign in usages": {
+			cr: &cminternal.CertificateRequest{
+				Spec: cminternal.CertificateRequestSpec{
+					Request:   mustGenerateCSR(t, gen.Certificate("test", gen.SetCertificateDNSNames("example.com"), gen.SetCertificateKeyUsages(cmapi.UsageAny, cmapi.UsageDigitalSignature, cmapi.UsageKeyEncipherment, cmapi.UsageClientAuth, cmapi.UsageServerAuth), gen.SetCertificateIsCA(true))),
+					IssuerRef: validIssuerRef,
+					IsCA:      true,
+					Usages:    []cminternal.KeyUsage{cminternal.UsageAny, cminternal.UsageDigitalSignature, cminternal.UsageKeyEncipherment, cminternal.UsageClientAuth, cminternal.UsageServerAuth},
+				},
 			},
 			want: []*field.Error{},
 		},
-		{
-			name: "Error on csr not having all usages",
-			crSpec: &cminternal.CertificateRequestSpec{
-				Request:   mustGenerateCSR(t, gen.Certificate("test", gen.SetCertificateDNSNames("example.com"), gen.SetCertificateKeyUsages(cmapi.UsageDigitalSignature, cmapi.UsageKeyEncipherment, cmapi.UsageServerAuth))),
-				IssuerRef: validIssuerRef,
-				Usages:    []cminternal.KeyUsage{cminternal.UsageSigning, cminternal.UsageKeyEncipherment, cminternal.UsageServerAuth, cminternal.UsageClientAuth},
+		"Error on csr not having all usages": {
+			cr: &cminternal.CertificateRequest{
+				Spec: cminternal.CertificateRequestSpec{
+					Request:   mustGenerateCSR(t, gen.Certificate("test", gen.SetCertificateDNSNames("example.com"), gen.SetCertificateKeyUsages(cmapi.UsageDigitalSignature, cmapi.UsageKeyEncipherment, cmapi.UsageServerAuth))),
+					IssuerRef: validIssuerRef,
+					Usages:    []cminternal.KeyUsage{cminternal.UsageSigning, cminternal.UsageKeyEncipherment, cminternal.UsageServerAuth, cminternal.UsageClientAuth},
+				},
 			},
 			want: []*field.Error{
 				field.Invalid(fldPath.Child("request"), nil, "csr key usages do not match specified usages, these should match if both are set: [[]certmanager.KeyUsage[3] != []certmanager.KeyUsage[4]]"),
 			},
 		},
-		{
-			name: "Error on cr not having all usages",
-			crSpec: &cminternal.CertificateRequestSpec{
-				Request:   mustGenerateCSR(t, gen.Certificate("test", gen.SetCertificateDNSNames("example.com"), gen.SetCertificateKeyUsages(cmapi.UsageDigitalSignature, cmapi.UsageKeyEncipherment, cmapi.UsageServerAuth, cmapi.UsageClientAuth))),
-				IssuerRef: validIssuerRef,
-				Usages:    []cminternal.KeyUsage{cminternal.UsageSigning, cminternal.UsageKeyEncipherment},
+		"Error on cr not having all usages": {
+			cr: &cminternal.CertificateRequest{
+				Spec: cminternal.CertificateRequestSpec{
+					Request:   mustGenerateCSR(t, gen.Certificate("test", gen.SetCertificateDNSNames("example.com"), gen.SetCertificateKeyUsages(cmapi.UsageDigitalSignature, cmapi.UsageKeyEncipherment, cmapi.UsageServerAuth, cmapi.UsageClientAuth))),
+					IssuerRef: validIssuerRef,
+					Usages:    []cminternal.KeyUsage{cminternal.UsageSigning, cminternal.UsageKeyEncipherment},
+				},
 			},
 			want: []*field.Error{
 				field.Invalid(fldPath.Child("request"), nil, "csr key usages do not match specified usages, these should match if both are set: [[]certmanager.KeyUsage[4] != []certmanager.KeyUsage[2]]"),
 			},
 		},
-		{
-			name: "Error on cr not having all usages",
-			crSpec: &cminternal.CertificateRequestSpec{
-				Request:   mustGenerateCSR(t, gen.Certificate("test", gen.SetCertificateDNSNames("example.com"), gen.SetCertificateKeyUsages(cmapi.UsageDigitalSignature, cmapi.UsageKeyEncipherment, cmapi.UsageServerAuth, cmapi.UsageClientAuth))),
-				IssuerRef: validIssuerRef,
-				Usages:    []cminternal.KeyUsage{cminternal.UsageAny, cminternal.UsageSigning},
-			},
-			want: []*field.Error{
-				field.Invalid(fldPath.Child("request"), nil, "csr key usages do not match specified usages, these should match if both are set: [[]certmanager.KeyUsage[4] != []certmanager.KeyUsage[2]]"),
-			},
-		},
-		{
-			name: "Test csr with any, signing, digital signature, key encipherment, server and client auth",
-			crSpec: &cminternal.CertificateRequestSpec{
-				Request:   mustGenerateCSR(t, gen.Certificate("test", gen.SetCertificateDNSNames("example.com"), gen.SetCertificateKeyUsages(cmapi.UsageAny, cmapi.UsageSigning, cmapi.UsageKeyEncipherment, cmapi.UsageClientAuth, cmapi.UsageServerAuth), gen.SetCertificateIsCA(true))),
-				IssuerRef: validIssuerRef,
-				IsCA:      true,
-				Usages:    []cminternal.KeyUsage{cminternal.UsageAny, cminternal.UsageSigning, cminternal.UsageKeyEncipherment, cminternal.UsageClientAuth, cminternal.UsageServerAuth},
+		"Test csr with any, signing, digital signature, key encipherment, server and client auth": {
+			cr: &cminternal.CertificateRequest{
+				Spec: cminternal.CertificateRequestSpec{
+					Request:   mustGenerateCSR(t, gen.Certificate("test", gen.SetCertificateDNSNames("example.com"), gen.SetCertificateKeyUsages(cmapi.UsageAny, cmapi.UsageSigning, cmapi.UsageKeyEncipherment, cmapi.UsageClientAuth, cmapi.UsageServerAuth), gen.SetCertificateIsCA(true))),
+					IssuerRef: validIssuerRef,
+					IsCA:      true,
+					Usages:    []cminternal.KeyUsage{cminternal.UsageAny, cminternal.UsageSigning, cminternal.UsageKeyEncipherment, cminternal.UsageClientAuth, cminternal.UsageServerAuth},
+				},
 			},
 			want: []*field.Error{},
 		},
+		"CertificateRequest with single Approved=true condition, shouldn't error": {
+			cr: &cminternal.CertificateRequest{
+				Spec: cminternal.CertificateRequestSpec{
+					Request:   mustGenerateCSR(t, gen.Certificate("spec", gen.SetCertificateDNSNames("example.com"), gen.SetCertificateKeyUsages(cmapi.UsageAny), gen.SetCertificateIsCA(true))),
+					IssuerRef: validIssuerRef,
+					IsCA:      true,
+					Usages:    []cminternal.KeyUsage{cminternal.UsageAny},
+				},
+				Status: cminternal.CertificateRequestStatus{
+					Conditions: []cminternal.CertificateRequestCondition{
+						{
+							Type:   cminternal.CertificateRequestConditionApproved,
+							Status: cminternalmeta.ConditionTrue,
+						},
+					},
+				},
+			},
+			want: []*field.Error{},
+		},
+		"CertificateRequest with single Denied=true condition, shouldn't error": {
+			cr: &cminternal.CertificateRequest{
+				Spec: cminternal.CertificateRequestSpec{
+					Request:   mustGenerateCSR(t, gen.Certificate("spec", gen.SetCertificateDNSNames("example.com"), gen.SetCertificateKeyUsages(cmapi.UsageAny), gen.SetCertificateIsCA(true))),
+					IssuerRef: validIssuerRef,
+					IsCA:      true,
+					Usages:    []cminternal.KeyUsage{cminternal.UsageAny},
+				},
+				Status: cminternal.CertificateRequestStatus{
+					Conditions: []cminternal.CertificateRequestCondition{
+						{
+							Type:   cminternal.CertificateRequestConditionDenied,
+							Status: cminternalmeta.ConditionTrue,
+						},
+					},
+				},
+			},
+			want: []*field.Error{},
+		},
+		"CertificateRequest with single Approved=false condition, should error": {
+			cr: &cminternal.CertificateRequest{
+				Spec: cminternal.CertificateRequestSpec{
+					Request:   mustGenerateCSR(t, gen.Certificate("spec", gen.SetCertificateDNSNames("example.com"), gen.SetCertificateKeyUsages(cmapi.UsageAny), gen.SetCertificateIsCA(true))),
+					IssuerRef: validIssuerRef,
+					IsCA:      true,
+					Usages:    []cminternal.KeyUsage{cminternal.UsageAny},
+				},
+				Status: cminternal.CertificateRequestStatus{
+					Conditions: []cminternal.CertificateRequestCondition{
+						{
+							Type:   cminternal.CertificateRequestConditionApproved,
+							Status: cminternalmeta.ConditionFalse,
+							Reason: "cert-manager.io",
+						},
+					},
+				},
+			},
+			want: []*field.Error{
+				field.Invalid(fldPathConditions.Child("Approved"), nil,
+					`"Approved" condition may only be set to True`),
+			},
+		},
+		"CertificateRequest with single Denied=false condition, should error": {
+			cr: &cminternal.CertificateRequest{
+				Spec: cminternal.CertificateRequestSpec{
+					Request:   mustGenerateCSR(t, gen.Certificate("spec", gen.SetCertificateDNSNames("example.com"), gen.SetCertificateKeyUsages(cmapi.UsageAny), gen.SetCertificateIsCA(true))),
+					IssuerRef: validIssuerRef,
+					IsCA:      true,
+					Usages:    []cminternal.KeyUsage{cminternal.UsageAny},
+				},
+				Status: cminternal.CertificateRequestStatus{
+					Conditions: []cminternal.CertificateRequestCondition{
+						{
+							Type:   cminternal.CertificateRequestConditionDenied,
+							Status: cminternalmeta.ConditionFalse,
+							Reason: "Foo",
+						},
+					},
+				},
+			},
+			want: []*field.Error{
+				field.Invalid(fldPathConditions.Child("Denied"), nil,
+					`"Denied" condition may only be set to True`),
+			},
+		},
+		"CertificateRequest with both Denied=false and Approved=false conditions, should error": {
+			cr: &cminternal.CertificateRequest{
+				Spec: cminternal.CertificateRequestSpec{
+					Request:   mustGenerateCSR(t, gen.Certificate("spec", gen.SetCertificateDNSNames("example.com"), gen.SetCertificateKeyUsages(cmapi.UsageAny), gen.SetCertificateIsCA(true))),
+					IssuerRef: validIssuerRef,
+					IsCA:      true,
+					Usages:    []cminternal.KeyUsage{cminternal.UsageAny},
+				},
+				Status: cminternal.CertificateRequestStatus{
+					Conditions: []cminternal.CertificateRequestCondition{
+						{
+							Type:   cminternal.CertificateRequestConditionApproved,
+							Status: cminternalmeta.ConditionFalse,
+							Reason: "cert-manager.io",
+						},
+						{
+							Type:   cminternal.CertificateRequestConditionDenied,
+							Status: cminternalmeta.ConditionFalse,
+							Reason: "Foo",
+						},
+					},
+				},
+			},
+			want: []*field.Error{
+				field.Invalid(field.NewPath("status", "conditions", "Approved"), nil,
+					`"Approved" condition may only be set to True`),
+				field.Invalid(field.NewPath("status", "conditions", "Denied"), nil,
+					`"Denied" condition may only be set to True`),
+				field.Forbidden(fldPathConditions, "both 'Denied' and 'Approved' conditions cannot coexist"),
+			},
+		},
+		"CertificateRequest with both Denied=true and Approved=true conditions, should error": {
+			cr: &cminternal.CertificateRequest{
+				Spec: cminternal.CertificateRequestSpec{
+					Request:   mustGenerateCSR(t, gen.Certificate("spec", gen.SetCertificateDNSNames("example.com"), gen.SetCertificateKeyUsages(cmapi.UsageAny), gen.SetCertificateIsCA(true))),
+					IssuerRef: validIssuerRef,
+					IsCA:      true,
+					Usages:    []cminternal.KeyUsage{cminternal.UsageAny},
+				},
+				Status: cminternal.CertificateRequestStatus{
+					Conditions: []cminternal.CertificateRequestCondition{
+						{
+							Type:   cminternal.CertificateRequestConditionApproved,
+							Status: cminternalmeta.ConditionTrue,
+							Reason: "cert-manager.io",
+						},
+						{
+							Type:   cminternal.CertificateRequestConditionDenied,
+							Status: cminternalmeta.ConditionTrue,
+							Reason: "Foo",
+						},
+					},
+				},
+			},
+			want: []*field.Error{
+				field.Forbidden(fldPathConditions, "both 'Denied' and 'Approved' conditions cannot coexist"),
+			},
+		},
+		"CertificateRequest with multiple Approved conditions, should error": {
+			cr: &cminternal.CertificateRequest{
+				Spec: cminternal.CertificateRequestSpec{
+					Request:   mustGenerateCSR(t, gen.Certificate("spec", gen.SetCertificateDNSNames("example.com"), gen.SetCertificateKeyUsages(cmapi.UsageAny), gen.SetCertificateIsCA(true))),
+					IssuerRef: validIssuerRef,
+					IsCA:      true,
+					Usages:    []cminternal.KeyUsage{cminternal.UsageAny},
+				},
+				Status: cminternal.CertificateRequestStatus{
+					Conditions: []cminternal.CertificateRequestCondition{
+						{
+							Type:   cminternal.CertificateRequestConditionApproved,
+							Status: cminternalmeta.ConditionTrue,
+							Reason: "cert-manager.io",
+						},
+						{
+							Type:   cminternal.CertificateRequestConditionApproved,
+							Status: cminternalmeta.ConditionFalse,
+							Reason: "foo",
+						},
+					},
+				},
+			},
+			want: []*field.Error{
+				field.Forbidden(fldPathConditions, `multiple "Approved" conditions present`),
+			},
+		},
+		"CertificateRequest with multiple Denied conditions, should error": {
+			cr: &cminternal.CertificateRequest{
+				Spec: cminternal.CertificateRequestSpec{
+					Request:   mustGenerateCSR(t, gen.Certificate("spec", gen.SetCertificateDNSNames("example.com"), gen.SetCertificateKeyUsages(cmapi.UsageAny), gen.SetCertificateIsCA(true))),
+					IssuerRef: validIssuerRef,
+					IsCA:      true,
+					Usages:    []cminternal.KeyUsage{cminternal.UsageAny},
+				},
+				Status: cminternal.CertificateRequestStatus{
+					Conditions: []cminternal.CertificateRequestCondition{
+						{
+							Type:   cminternal.CertificateRequestConditionDenied,
+							Status: cminternalmeta.ConditionTrue,
+							Reason: "Foo",
+						},
+						{
+							Type:   cminternal.CertificateRequestConditionDenied,
+							Status: cminternalmeta.ConditionFalse,
+							Reason: "Foo",
+						},
+					},
+				},
+			},
+			want: []*field.Error{
+				field.Forbidden(fldPathConditions, `multiple "Denied" conditions present`),
+			},
+		},
 	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			got := ValidateCertificateRequestSpec(tt.crSpec, field.NewPath("test"), true)
+	for name, test := range tests {
+		t.Run(name, func(t *testing.T) {
+			got := ValidateCertificateRequest(nil, test.cr)
 			for i := range got {
-				// filter out the value so it does not print the full CSR in tests
-				got[i].BadValue = nil
+				if got[i].Type != field.ErrorTypeForbidden {
+					// filter out the value so it does not print the full CSR in tests
+					got[i].BadValue = nil
+				}
 			}
-			if !reflect.DeepEqual(got, tt.want) {
-				t.Errorf("ValidateCertificateRequestSpec() = %v, want %v", got, tt.want)
+			if !reflect.DeepEqual(got, test.want) {
+				t.Errorf("ValidateCertificateRequest() = %v, want %v", got, test.want)
 			}
 		})
 	}

test/e2e/framework/helper/certificaterequests.go

@@ -186,6 +186,13 @@ func (h *Helper) ValidateIssuedCertificateRequest(cr *cmapi.CertificateRequest,
 		}
 	}
 
+	if !apiutil.CertificateRequestIsApproved(cr) {
+		return nil, fmt.Errorf("CertificateRequest does not have an Approved condition set to True: %+v", cr.Status.Conditions)
+	}
+	if apiutil.CertificateRequestIsDenied(cr) {
+		return nil, fmt.Errorf("CertificateRequest has a Denied conditon set to True: %+v", cr.Status.Conditions)
+	}
+
 	return cert, nil
 }
 

test/unit/gen/certificaterequest.go

@@ -96,6 +96,12 @@ func SetCertificateRequestStatusCondition(c v1.CertificateRequestCondition) Cert
 	}
 }
 
+func AddCertificateRequestStatusCondition(c v1.CertificateRequestCondition) CertificateRequestModifier {
+	return func(cr *v1.CertificateRequest) {
+		cr.Status.Conditions = append(cr.Status.Conditions, c)
+	}
+}
+
 func SetCertificateRequestNamespace(namespace string) CertificateRequestModifier {
 	return func(cr *v1.CertificateRequest) {
 		cr.ObjectMeta.Namespace = namespace