Plumb version info to validation, separate main/status/approval validation

pkg/apis/certificates/validation/validation.go

@@ -17,16 +17,65 @@ limitations under the License.
 package validation
 
 import (
+	"bytes"
+	"crypto/x509"
+	"encoding/pem"
 	"fmt"
 	"strings"
 
+	v1 "k8s.io/api/core/v1"
+	apiequality "k8s.io/apimachinery/pkg/api/equality"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apimachinery/pkg/util/diff"
+	"k8s.io/apimachinery/pkg/util/sets"
 	utilvalidation "k8s.io/apimachinery/pkg/util/validation"
 	"k8s.io/apimachinery/pkg/util/validation/field"
-
+	utilcert "k8s.io/client-go/util/cert"
 	"k8s.io/kubernetes/pkg/apis/certificates"
+	certificatesv1beta1 "k8s.io/kubernetes/pkg/apis/certificates/v1beta1"
 	apivalidation "k8s.io/kubernetes/pkg/apis/core/validation"
 )
 
+var (
+	// trueConditionTypes is the set of condition types which may only have a status of True if present
+	trueConditionTypes = sets.NewString(
+		string(certificates.CertificateApproved),
+		string(certificates.CertificateDenied),
+		string(certificates.CertificateFailed),
+	)
+
+	trueStatusOnly  = sets.NewString(string(v1.ConditionTrue))
+	allStatusValues = sets.NewString(string(v1.ConditionTrue), string(v1.ConditionFalse), string(v1.ConditionUnknown))
+)
+
+type certificateValidationOptions struct {
+	// The following allow modifications only permitted via certain update paths
+
+	// allow populating/modifying Approved/Denied conditions
+	allowSettingApprovalConditions bool
+	// allow populating status.certificate
+	allowSettingCertificate bool
+
+	// allow Approved and Denied conditions to be exist.
+	// we tolerate this when the problem is already present in the persisted object for compatibility.
+	allowBothApprovedAndDenied bool
+
+	// The following are bad things we tolerate for compatibility reasons:
+	// * in requests made via the v1beta1 API
+	// * in update requests where the problem is already present in the persisted object
+
+	// allow modifying status.certificate on an update where the old object has a different certificate
+	allowResettingCertificate bool
+	// allow the legacy-unknown signerName
+	allowLegacySignerName bool
+	// allow conditions with duplicate types
+	allowDuplicateConditionTypes bool
+	// allow conditions with "" types
+	allowEmptyConditionType bool
+	// allow arbitrary content in status.certificate
+	allowArbitraryCertificate bool
+}
+
 // validateCSR validates the signature and formatting of a base64-wrapped,
 // PEM-encoded PKCS#10 certificate signing request. If this is invalid, we must
 // not accept the CSR for further processing.
@@ -43,12 +92,55 @@ func validateCSR(obj *certificates.CertificateSigningRequest) error {
 	return nil
 }
 
+func validateCertificate(pemData []byte) error {
+	if len(pemData) == 0 {
+		return nil
+	}
+
+	blocks := 0
+	for {
+		block, remainingData := pem.Decode(pemData)
+		if block == nil {
+			break
+		}
+
+		if block.Type != utilcert.CertificateBlockType {
+			return fmt.Errorf("only CERTIFICATE PEM blocks are allowed, found %q", block.Type)
+		}
+		if len(block.Headers) != 0 {
+			return fmt.Errorf("no PEM block headers are permitted")
+		}
+		blocks++
+
+		certs, err := x509.ParseCertificates(block.Bytes)
+		if err != nil {
+			return err
+		}
+		if len(certs) == 0 {
+			return fmt.Errorf("found CERTIFICATE PEM block containing 0 certificates")
+		}
+
+		pemData = remainingData
+	}
+
+	if blocks == 0 {
+		return fmt.Errorf("must contain at least one CERTIFICATE PEM block")
+	}
+
+	return nil
+}
+
 // We don't care what you call your certificate requests.
 func ValidateCertificateRequestName(name string, prefix bool) []string {
 	return nil
 }
 
-func ValidateCertificateSigningRequest(csr *certificates.CertificateSigningRequest) field.ErrorList {
+func ValidateCertificateSigningRequestCreate(csr *certificates.CertificateSigningRequest, version schema.GroupVersion) field.ErrorList {
+	opts := getValidationOptions(version, csr, nil)
+	return validateCertificateSigningRequest(csr, opts)
+}
+
+func validateCertificateSigningRequest(csr *certificates.CertificateSigningRequest, opts certificateValidationOptions) field.ErrorList {
 	isNamespaced := false
 	allErrs := apivalidation.ValidateObjectMeta(&csr.ObjectMeta, isNamespaced, ValidateCertificateRequestName, field.NewPath("metadata"))
 
@@ -60,7 +152,71 @@ func ValidateCertificateSigningRequest(csr *certificates.CertificateSigningReque
 	if len(csr.Spec.Usages) == 0 {
 		allErrs = append(allErrs, field.Required(specPath.Child("usages"), "usages must be provided"))
 	}
-	allErrs = append(allErrs, ValidateCertificateSigningRequestSignerName(specPath.Child("signerName"), csr.Spec.SignerName)...)
+	if !opts.allowLegacySignerName && csr.Spec.SignerName == certificates.LegacyUnknownSignerName {
+		allErrs = append(allErrs, field.Invalid(specPath.Child("signerName"), csr.Spec.SignerName, "the legacy signerName is not allowed via this API version"))
+	} else {
+		allErrs = append(allErrs, ValidateCertificateSigningRequestSignerName(specPath.Child("signerName"), csr.Spec.SignerName)...)
+	}
+	allErrs = append(allErrs, validateConditions(field.NewPath("status", "conditions"), csr, opts)...)
+
+	if !opts.allowArbitraryCertificate {
+		if err := validateCertificate(csr.Status.Certificate); err != nil {
+			allErrs = append(allErrs, field.Invalid(field.NewPath("status", "certificate"), "<certificate data>", err.Error()))
+		}
+	}
+
+	return allErrs
+}
+
+func validateConditions(fldPath *field.Path, csr *certificates.CertificateSigningRequest, opts certificateValidationOptions) field.ErrorList {
+	allErrs := field.ErrorList{}
+
+	seenTypes := map[certificates.RequestConditionType]bool{}
+	hasApproved := false
+	hasDenied := false
+
+	for i, c := range csr.Status.Conditions {
+
+		if !opts.allowEmptyConditionType {
+			if len(c.Type) == 0 {
+				allErrs = append(allErrs, field.Required(fldPath.Index(i).Child("type"), ""))
+			}
+		}
+
+		allowedStatusValues := allStatusValues
+		if trueConditionTypes.Has(string(c.Type)) {
+			allowedStatusValues = trueStatusOnly
+		}
+		switch {
+		case c.Status == "":
+			allErrs = append(allErrs, field.Required(fldPath.Index(i).Child("status"), ""))
+		case !allowedStatusValues.Has(string(c.Status)):
+			allErrs = append(allErrs, field.NotSupported(fldPath.Index(i).Child("status"), c.Status, trueConditionTypes.List()))
+		}
+
+		if !opts.allowBothApprovedAndDenied {
+			switch c.Type {
+			case certificates.CertificateApproved:
+				hasApproved = true
+				if hasDenied {
+					allErrs = append(allErrs, field.Invalid(fldPath.Index(i).Child("type"), c.Type, "Approved and Denied conditions are mutually exclusive"))
+				}
+			case certificates.CertificateDenied:
+				hasDenied = true
+				if hasApproved {
+					allErrs = append(allErrs, field.Invalid(fldPath.Index(i).Child("type"), c.Type, "Approved and Denied conditions are mutually exclusive"))
+				}
+			}
+		}
+
+		if !opts.allowDuplicateConditionTypes {
+			if seenTypes[c.Type] {
+				allErrs = append(allErrs, field.Duplicate(fldPath.Index(i).Child("type"), c.Type))
+			}
+			seenTypes[c.Type] = true
+		}
+	}
+
 	return allErrs
 }
 
@@ -140,8 +296,172 @@ func ValidateCertificateSigningRequestSignerName(fldPath *field.Path, signerName
 	return el
 }
 
-func ValidateCertificateSigningRequestUpdate(newCSR, oldCSR *certificates.CertificateSigningRequest) field.ErrorList {
+func ValidateCertificateSigningRequestUpdate(newCSR, oldCSR *certificates.CertificateSigningRequest, version schema.GroupVersion) field.ErrorList {
-	validationErrorList := ValidateCertificateSigningRequest(newCSR)
+	opts := getValidationOptions(version, newCSR, oldCSR)
+	return validateCertificateSigningRequestUpdate(newCSR, oldCSR, opts)
+}
+
+func ValidateCertificateSigningRequestStatusUpdate(newCSR, oldCSR *certificates.CertificateSigningRequest, version schema.GroupVersion) field.ErrorList {
+	opts := getValidationOptions(version, newCSR, oldCSR)
+	opts.allowSettingCertificate = true
+	return validateCertificateSigningRequestUpdate(newCSR, oldCSR, opts)
+}
+
+func ValidateCertificateSigningRequestApprovalUpdate(newCSR, oldCSR *certificates.CertificateSigningRequest, version schema.GroupVersion) field.ErrorList {
+	opts := getValidationOptions(version, newCSR, oldCSR)
+	opts.allowSettingApprovalConditions = true
+	return validateCertificateSigningRequestUpdate(newCSR, oldCSR, opts)
+}
+
+func validateCertificateSigningRequestUpdate(newCSR, oldCSR *certificates.CertificateSigningRequest, opts certificateValidationOptions) field.ErrorList {
+	validationErrorList := validateCertificateSigningRequest(newCSR, opts)
 	metaUpdateErrorList := apivalidation.ValidateObjectMetaUpdate(&newCSR.ObjectMeta, &oldCSR.ObjectMeta, field.NewPath("metadata"))
+
+	// prevent removal of existing Approved/Denied/Failed conditions
+	for _, t := range []certificates.RequestConditionType{certificates.CertificateApproved, certificates.CertificateDenied, certificates.CertificateFailed} {
+		oldConditions := findConditions(oldCSR, t)
+		newConditions := findConditions(newCSR, t)
+		if len(newConditions) < len(oldConditions) {
+			validationErrorList = append(validationErrorList, field.Forbidden(field.NewPath("status", "conditions"), fmt.Sprintf("updates may not remove a condition of type %q", t)))
+		}
+	}
+
+	if !opts.allowSettingApprovalConditions {
+		// prevent addition/removal/modification of Approved/Denied conditions
+		for _, t := range []certificates.RequestConditionType{certificates.CertificateApproved, certificates.CertificateDenied} {
+			oldConditions := findConditions(oldCSR, t)
+			newConditions := findConditions(newCSR, t)
+			switch {
+			case len(newConditions) < len(oldConditions):
+				// removals are prevented above
+			case len(newConditions) > len(oldConditions):
+				validationErrorList = append(validationErrorList, field.Forbidden(field.NewPath("status", "conditions"), fmt.Sprintf("updates may not add a condition of type %q", t)))
+			case !apiequality.Semantic.DeepEqual(oldConditions, newConditions):
+				conditionDiff := diff.ObjectDiff(oldConditions, newConditions)
+				validationErrorList = append(validationErrorList, field.Forbidden(field.NewPath("status", "conditions"), fmt.Sprintf("updates may not modify a condition of type %q\n%v", t, conditionDiff)))
+			}
+		}
+	}
+
+	if !bytes.Equal(newCSR.Status.Certificate, oldCSR.Status.Certificate) {
+		if !opts.allowSettingCertificate {
+			validationErrorList = append(validationErrorList, field.Forbidden(field.NewPath("status", "certificate"), "updates may not set certificate content"))
+		} else if !opts.allowResettingCertificate && len(oldCSR.Status.Certificate) > 0 {
+			validationErrorList = append(validationErrorList, field.Forbidden(field.NewPath("status", "certificate"), "updates may not modify existing certificate content"))
+		}
+	}
+
 	return append(validationErrorList, metaUpdateErrorList...)
 }
+
+// findConditions returns all instances of conditions of the specified type
+func findConditions(csr *certificates.CertificateSigningRequest, conditionType certificates.RequestConditionType) []certificates.CertificateSigningRequestCondition {
+	var retval []certificates.CertificateSigningRequestCondition
+	for i, c := range csr.Status.Conditions {
+		if c.Type == conditionType {
+			retval = append(retval, csr.Status.Conditions[i])
+		}
+	}
+	return retval
+}
+
+// getValidationOptions returns the validation options to be
+// compatible with the specified version and existing CSR.
+// oldCSR may be nil if this is a create request.
+// validation options related to subresource-specific capabilities are set to false.
+func getValidationOptions(version schema.GroupVersion, newCSR, oldCSR *certificates.CertificateSigningRequest) certificateValidationOptions {
+	return certificateValidationOptions{
+		allowResettingCertificate:    allowResettingCertificate(version),
+		allowBothApprovedAndDenied:   allowBothApprovedAndDenied(oldCSR),
+		allowLegacySignerName:        allowLegacySignerName(version, oldCSR),
+		allowDuplicateConditionTypes: allowDuplicateConditionTypes(version, oldCSR),
+		allowEmptyConditionType:      allowEmptyConditionType(version, oldCSR),
+		allowArbitraryCertificate:    allowArbitraryCertificate(version, newCSR, oldCSR),
+	}
+}
+
+func allowResettingCertificate(version schema.GroupVersion) bool {
+	// compatibility with v1beta1
+	return version == certificatesv1beta1.SchemeGroupVersion
+}
+
+func allowBothApprovedAndDenied(oldCSR *certificates.CertificateSigningRequest) bool {
+	if oldCSR == nil {
+		return false
+	}
+	approved := false
+	denied := false
+	for _, c := range oldCSR.Status.Conditions {
+		if c.Type == certificates.CertificateApproved {
+			approved = true
+		} else if c.Type == certificates.CertificateDenied {
+			denied = true
+		}
+	}
+	// compatibility with existing data
+	return approved && denied
+}
+
+func allowLegacySignerName(version schema.GroupVersion, oldCSR *certificates.CertificateSigningRequest) bool {
+	switch {
+	case version == certificatesv1beta1.SchemeGroupVersion:
+		return true // compatibility with v1beta1
+	case oldCSR != nil && oldCSR.Spec.SignerName == certificates.LegacyUnknownSignerName:
+		return true // compatibility with existing data
+	default:
+		return false
+	}
+}
+
+func allowDuplicateConditionTypes(version schema.GroupVersion, oldCSR *certificates.CertificateSigningRequest) bool {
+	switch {
+	case version == certificatesv1beta1.SchemeGroupVersion:
+		return true // compatibility with v1beta1
+	case oldCSR != nil && hasDuplicateConditionTypes(oldCSR):
+		return true // compatibility with existing data
+	default:
+		return false
+	}
+}
+func hasDuplicateConditionTypes(csr *certificates.CertificateSigningRequest) bool {
+	seen := map[certificates.RequestConditionType]bool{}
+	for _, c := range csr.Status.Conditions {
+		if seen[c.Type] {
+			return true
+		}
+		seen[c.Type] = true
+	}
+	return false
+}
+
+func allowEmptyConditionType(version schema.GroupVersion, oldCSR *certificates.CertificateSigningRequest) bool {
+	switch {
+	case version == certificatesv1beta1.SchemeGroupVersion:
+		return true // compatibility with v1beta1
+	case oldCSR != nil && hasEmptyConditionType(oldCSR):
+		return true // compatibility with existing data
+	default:
+		return false
+	}
+}
+func hasEmptyConditionType(csr *certificates.CertificateSigningRequest) bool {
+	for _, c := range csr.Status.Conditions {
+		if len(c.Type) == 0 {
+			return true
+		}
+	}
+	return false
+}
+
+func allowArbitraryCertificate(version schema.GroupVersion, newCSR, oldCSR *certificates.CertificateSigningRequest) bool {
+	switch {
+	case version == certificatesv1beta1.SchemeGroupVersion:
+		return true // compatibility with v1beta1
+	case newCSR != nil && oldCSR != nil && bytes.Equal(newCSR.Status.Certificate, oldCSR.Status.Certificate):
+		return true // tolerate updates that don't touch status.certificate
+	case oldCSR != nil && validateCertificate(oldCSR.Status.Certificate) != nil:
+		return true // compatibility with existing data
+	default:
+		return false
+	}
+}

pkg/apis/certificates/validation/validation_test.go

@@ -28,9 +28,13 @@ import (
 	"testing"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apimachinery/pkg/util/validation/field"
-
+	"k8s.io/kubernetes/pkg/apis/certificates"
 	capi "k8s.io/kubernetes/pkg/apis/certificates"
+	capiv1beta1 "k8s.io/kubernetes/pkg/apis/certificates/v1beta1"
+	"k8s.io/kubernetes/pkg/apis/core"
 )
 
 var (
@@ -39,7 +43,7 @@ var (
 	validUsages     = []capi.KeyUsage{capi.UsageKeyEncipherment}
 )
 
-func TestValidateCertificateSigningRequest(t *testing.T) {
+func TestValidateCertificateSigningRequestCreate(t *testing.T) {
 	specPath := field.NewPath("spec")
 	// maxLengthSignerName is a signerName that is of maximum length, utilising
 	// the max length specifications defined in validation.go.
@@ -48,6 +52,7 @@ func TestValidateCertificateSigningRequest(t *testing.T) {
 	maxLengthSignerName := fmt.Sprintf("%s/%s.%s", maxLengthFQDN, repeatString("a", 63), repeatString("a", 253))
 	tests := map[string]struct {
 		csr  capi.CertificateSigningRequest
+		gv   schema.GroupVersion
 		errs field.ErrorList
 	}{
 		"CSR with empty request data should fail": {
@@ -261,7 +266,7 @@ func TestValidateCertificateSigningRequest(t *testing.T) {
 	}
 	for name, test := range tests {
 		t.Run(name, func(t *testing.T) {
-			el := ValidateCertificateSigningRequest(&test.csr)
+			el := ValidateCertificateSigningRequestCreate(&test.csr, test.gv)
 			if !reflect.DeepEqual(el, test.errs) {
 				t.Errorf("returned and expected errors did not match - expected %v but got %v", test.errs.ToAggregate(), el.ToAggregate())
 			}
@@ -306,3 +311,812 @@ func newCSRPEM(t *testing.T) []byte {
 
 	return p
 }
+
+func Test_getValidationOptions(t *testing.T) {
+	tests := []struct {
+		name    string
+		version schema.GroupVersion
+		newCSR  *certificates.CertificateSigningRequest
+		oldCSR  *certificates.CertificateSigningRequest
+		want    certificateValidationOptions
+	}{
+		{
+			name:    "v1beta1 compatible create",
+			version: capiv1beta1.SchemeGroupVersion,
+			oldCSR:  nil,
+			want: certificateValidationOptions{
+				allowResettingCertificate:    true,
+				allowBothApprovedAndDenied:   false,
+				allowLegacySignerName:        true,
+				allowDuplicateConditionTypes: true,
+				allowEmptyConditionType:      true,
+				allowArbitraryCertificate:    true,
+			},
+		},
+		{
+			name:    "v1 strict create",
+			version: schema.GroupVersion{Group: "certificates.k8s.io", Version: "v1"},
+			oldCSR:  nil,
+			want:    certificateValidationOptions{},
+		},
+		{
+			name:    "v1beta1 compatible update",
+			version: capiv1beta1.SchemeGroupVersion,
+			oldCSR: &capi.CertificateSigningRequest{Status: capi.CertificateSigningRequestStatus{
+				Conditions: []capi.CertificateSigningRequestCondition{{Type: capi.CertificateApproved}, {Type: capi.CertificateDenied}},
+			}},
+			want: certificateValidationOptions{
+				allowResettingCertificate:    true,
+				allowBothApprovedAndDenied:   true, // existing object has both approved and denied
+				allowLegacySignerName:        true,
+				allowDuplicateConditionTypes: true,
+				allowEmptyConditionType:      true,
+				allowArbitraryCertificate:    true,
+			},
+		},
+		{
+			name:    "v1 strict update",
+			version: schema.GroupVersion{Group: "certificates.k8s.io", Version: "v1"},
+			oldCSR:  &capi.CertificateSigningRequest{},
+			want:    certificateValidationOptions{},
+		},
+		{
+			name:    "v1 compatible update, approved+denied",
+			version: schema.GroupVersion{Group: "certificates.k8s.io", Version: "v1"},
+			oldCSR: &capi.CertificateSigningRequest{Status: capi.CertificateSigningRequestStatus{
+				Conditions: []capi.CertificateSigningRequestCondition{{Type: capi.CertificateApproved}, {Type: capi.CertificateDenied}},
+			}},
+			want: certificateValidationOptions{
+				allowBothApprovedAndDenied: true,
+			},
+		},
+		{
+			name:    "v1 compatible update, legacy signerName",
+			version: schema.GroupVersion{Group: "certificates.k8s.io", Version: "v1"},
+			oldCSR:  &capi.CertificateSigningRequest{Spec: capi.CertificateSigningRequestSpec{SignerName: capi.LegacyUnknownSignerName}},
+			want: certificateValidationOptions{
+				allowLegacySignerName: true,
+			},
+		},
+		{
+			name:    "v1 compatible update, duplicate condition types",
+			version: schema.GroupVersion{Group: "certificates.k8s.io", Version: "v1"},
+			oldCSR: &capi.CertificateSigningRequest{Status: capi.CertificateSigningRequestStatus{
+				Conditions: []capi.CertificateSigningRequestCondition{{Type: capi.CertificateApproved}, {Type: capi.CertificateApproved}},
+			}},
+			want: certificateValidationOptions{
+				allowDuplicateConditionTypes: true,
+			},
+		},
+		{
+			name:    "v1 compatible update, empty condition types",
+			version: schema.GroupVersion{Group: "certificates.k8s.io", Version: "v1"},
+			oldCSR: &capi.CertificateSigningRequest{Status: capi.CertificateSigningRequestStatus{
+				Conditions: []capi.CertificateSigningRequestCondition{{}},
+			}},
+			want: certificateValidationOptions{
+				allowEmptyConditionType: true,
+			},
+		},
+		{
+			name:    "v1 compatible update, no diff to certificate",
+			version: schema.GroupVersion{Group: "certificates.k8s.io", Version: "v1"},
+			newCSR: &capi.CertificateSigningRequest{Status: capi.CertificateSigningRequestStatus{
+				Certificate: validCertificate,
+			}},
+			oldCSR: &capi.CertificateSigningRequest{Status: capi.CertificateSigningRequestStatus{
+				Certificate: validCertificate,
+			}},
+			want: certificateValidationOptions{
+				allowArbitraryCertificate: true,
+			},
+		},
+		{
+			name:    "v1 compatible update, existing invalid certificate",
+			version: schema.GroupVersion{Group: "certificates.k8s.io", Version: "v1"},
+			newCSR: &capi.CertificateSigningRequest{Status: capi.CertificateSigningRequestStatus{
+				Certificate: []byte(`new - no PEM blocks`),
+			}},
+			oldCSR: &capi.CertificateSigningRequest{Status: capi.CertificateSigningRequestStatus{
+				Certificate: []byte(`old - no PEM blocks`),
+			}},
+			want: certificateValidationOptions{
+				allowArbitraryCertificate: true,
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := getValidationOptions(tt.version, tt.newCSR, tt.oldCSR); !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("got  %#v\nwant %#v", got, tt.want)
+			}
+		})
+	}
+}
+
+func TestValidateCertificateSigningRequestUpdate(t *testing.T) {
+	validUpdateMeta := validObjectMeta
+	validUpdateMeta.ResourceVersion = "1"
+
+	validUpdateMetaWithFinalizers := validUpdateMeta
+	validUpdateMetaWithFinalizers.Finalizers = []string{"foo"}
+
+	validSpec := capi.CertificateSigningRequestSpec{
+		Usages:     validUsages,
+		Request:    newCSRPEM(t),
+		SignerName: "example.com/something",
+	}
+
+	tests := []struct {
+		name        string
+		newCSR      *certificates.CertificateSigningRequest
+		oldCSR      *certificates.CertificateSigningRequest
+		versionErrs map[string][]string
+	}{
+		{
+			name:   "no-op",
+			newCSR: &capi.CertificateSigningRequest{ObjectMeta: validUpdateMeta, Spec: validSpec},
+			oldCSR: &capi.CertificateSigningRequest{ObjectMeta: validUpdateMeta, Spec: validSpec},
+		},
+		{
+			name:   "finalizer change with invalid status",
+			newCSR: &capi.CertificateSigningRequest{ObjectMeta: validUpdateMeta, Spec: validSpec, Status: capi.CertificateSigningRequestStatus{Certificate: invalidCertificateNoPEM}},
+			oldCSR: &capi.CertificateSigningRequest{ObjectMeta: validUpdateMetaWithFinalizers, Spec: validSpec, Status: capi.CertificateSigningRequestStatus{Certificate: invalidCertificateNoPEM}},
+		},
+		{
+			name: "add Approved condition",
+			newCSR: &capi.CertificateSigningRequest{ObjectMeta: validUpdateMeta, Spec: validSpec, Status: capi.CertificateSigningRequestStatus{
+				Conditions: []capi.CertificateSigningRequestCondition{{Type: capi.CertificateApproved, Status: core.ConditionTrue}},
+			}},
+			oldCSR: &capi.CertificateSigningRequest{ObjectMeta: validUpdateMetaWithFinalizers, Spec: validSpec},
+			versionErrs: map[string][]string{
+				"v1":      {`status.conditions: Forbidden: updates may not add a condition of type "Approved"`},
+				"v1beta1": {`status.conditions: Forbidden: updates may not add a condition of type "Approved"`},
+			},
+		},
+		{
+			name:   "remove Approved condition",
+			newCSR: &capi.CertificateSigningRequest{ObjectMeta: validUpdateMeta, Spec: validSpec},
+			oldCSR: &capi.CertificateSigningRequest{ObjectMeta: validUpdateMetaWithFinalizers, Spec: validSpec, Status: capi.CertificateSigningRequestStatus{
+				Conditions: []capi.CertificateSigningRequestCondition{{Type: capi.CertificateApproved, Status: core.ConditionTrue}},
+			}},
+			versionErrs: map[string][]string{
+				"v1":      {`status.conditions: Forbidden: updates may not remove a condition of type "Approved"`},
+				"v1beta1": {`status.conditions: Forbidden: updates may not remove a condition of type "Approved"`},
+			},
+		},
+		{
+			name: "add Denied condition",
+			newCSR: &capi.CertificateSigningRequest{ObjectMeta: validUpdateMeta, Spec: validSpec, Status: capi.CertificateSigningRequestStatus{
+				Conditions: []capi.CertificateSigningRequestCondition{{Type: capi.CertificateDenied, Status: core.ConditionTrue}},
+			}},
+			oldCSR: &capi.CertificateSigningRequest{ObjectMeta: validUpdateMetaWithFinalizers, Spec: validSpec},
+			versionErrs: map[string][]string{
+				"v1":      {`status.conditions: Forbidden: updates may not add a condition of type "Denied"`},
+				"v1beta1": {`status.conditions: Forbidden: updates may not add a condition of type "Denied"`},
+			},
+		},
+		{
+			name:   "remove Denied condition",
+			newCSR: &capi.CertificateSigningRequest{ObjectMeta: validUpdateMeta, Spec: validSpec},
+			oldCSR: &capi.CertificateSigningRequest{ObjectMeta: validUpdateMetaWithFinalizers, Spec: validSpec, Status: capi.CertificateSigningRequestStatus{
+				Conditions: []capi.CertificateSigningRequestCondition{{Type: capi.CertificateDenied, Status: core.ConditionTrue}},
+			}},
+			versionErrs: map[string][]string{
+				"v1":      {`status.conditions: Forbidden: updates may not remove a condition of type "Denied"`},
+				"v1beta1": {`status.conditions: Forbidden: updates may not remove a condition of type "Denied"`},
+			},
+		},
+		{
+			name: "add Failed condition",
+			newCSR: &capi.CertificateSigningRequest{ObjectMeta: validUpdateMeta, Spec: validSpec, Status: capi.CertificateSigningRequestStatus{
+				Conditions: []capi.CertificateSigningRequestCondition{{Type: capi.CertificateFailed, Status: core.ConditionTrue}},
+			}},
+			oldCSR:      &capi.CertificateSigningRequest{ObjectMeta: validUpdateMetaWithFinalizers, Spec: validSpec},
+			versionErrs: map[string][]string{},
+		},
+		{
+			name:   "remove Failed condition",
+			newCSR: &capi.CertificateSigningRequest{ObjectMeta: validUpdateMeta, Spec: validSpec},
+			oldCSR: &capi.CertificateSigningRequest{ObjectMeta: validUpdateMetaWithFinalizers, Spec: validSpec, Status: capi.CertificateSigningRequestStatus{
+				Conditions: []capi.CertificateSigningRequestCondition{{Type: capi.CertificateFailed, Status: core.ConditionTrue}},
+			}},
+			versionErrs: map[string][]string{
+				"v1":      {`status.conditions: Forbidden: updates may not remove a condition of type "Failed"`},
+				"v1beta1": {`status.conditions: Forbidden: updates may not remove a condition of type "Failed"`},
+			},
+		},
+		{
+			name: "set certificate",
+			newCSR: &capi.CertificateSigningRequest{ObjectMeta: validUpdateMeta, Spec: validSpec, Status: capi.CertificateSigningRequestStatus{
+				Certificate: validCertificate,
+			}},
+			oldCSR: &capi.CertificateSigningRequest{ObjectMeta: validUpdateMetaWithFinalizers, Spec: validSpec},
+			versionErrs: map[string][]string{
+				"v1":      {`status.certificate: Forbidden: updates may not set certificate content`},
+				"v1beta1": {`status.certificate: Forbidden: updates may not set certificate content`},
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		for _, version := range []string{"v1", "v1beta1"} {
+			t.Run(tt.name+"_"+version, func(t *testing.T) {
+				gotErrs := sets.NewString()
+				for _, err := range ValidateCertificateSigningRequestUpdate(tt.newCSR, tt.oldCSR, schema.GroupVersion{Group: certificates.GroupName, Version: version}) {
+					gotErrs.Insert(err.Error())
+				}
+				wantErrs := sets.NewString(tt.versionErrs[version]...)
+				for _, missing := range wantErrs.Difference(gotErrs).List() {
+					t.Errorf("missing expected error: %s", missing)
+				}
+				for _, unexpected := range gotErrs.Difference(wantErrs).List() {
+					t.Errorf("unexpected error: %s", unexpected)
+				}
+			})
+		}
+	}
+}
+
+func TestValidateCertificateSigningRequestStatusUpdate(t *testing.T) {
+	validUpdateMeta := validObjectMeta
+	validUpdateMeta.ResourceVersion = "1"
+
+	validUpdateMetaWithFinalizers := validUpdateMeta
+	validUpdateMetaWithFinalizers.Finalizers = []string{"foo"}
+
+	validSpec := capi.CertificateSigningRequestSpec{
+		Usages:     validUsages,
+		Request:    newCSRPEM(t),
+		SignerName: "example.com/something",
+	}
+
+	tests := []struct {
+		name        string
+		newCSR      *certificates.CertificateSigningRequest
+		oldCSR      *certificates.CertificateSigningRequest
+		versionErrs map[string][]string
+	}{
+		{
+			name:   "no-op",
+			newCSR: &capi.CertificateSigningRequest{ObjectMeta: validUpdateMeta, Spec: validSpec},
+			oldCSR: &capi.CertificateSigningRequest{ObjectMeta: validUpdateMeta, Spec: validSpec},
+		},
+		{
+			name:   "finalizer change with invalid status",
+			newCSR: &capi.CertificateSigningRequest{ObjectMeta: validUpdateMeta, Spec: validSpec, Status: capi.CertificateSigningRequestStatus{Certificate: invalidCertificateNoPEM}},
+			oldCSR: &capi.CertificateSigningRequest{ObjectMeta: validUpdateMetaWithFinalizers, Spec: validSpec, Status: capi.CertificateSigningRequestStatus{Certificate: invalidCertificateNoPEM}},
+		},
+		{
+			name: "add Approved condition",
+			newCSR: &capi.CertificateSigningRequest{ObjectMeta: validUpdateMeta, Spec: validSpec, Status: capi.CertificateSigningRequestStatus{
+				Conditions: []capi.CertificateSigningRequestCondition{{Type: capi.CertificateApproved, Status: core.ConditionTrue}},
+			}},
+			oldCSR: &capi.CertificateSigningRequest{ObjectMeta: validUpdateMetaWithFinalizers, Spec: validSpec},
+			versionErrs: map[string][]string{
+				"v1":      {`status.conditions: Forbidden: updates may not add a condition of type "Approved"`},
+				"v1beta1": {`status.conditions: Forbidden: updates may not add a condition of type "Approved"`},
+			},
+		},
+		{
+			name:   "remove Approved condition",
+			newCSR: &capi.CertificateSigningRequest{ObjectMeta: validUpdateMeta, Spec: validSpec},
+			oldCSR: &capi.CertificateSigningRequest{ObjectMeta: validUpdateMetaWithFinalizers, Spec: validSpec, Status: capi.CertificateSigningRequestStatus{
+				Conditions: []capi.CertificateSigningRequestCondition{{Type: capi.CertificateApproved, Status: core.ConditionTrue}},
+			}},
+			versionErrs: map[string][]string{
+				"v1":      {`status.conditions: Forbidden: updates may not remove a condition of type "Approved"`},
+				"v1beta1": {`status.conditions: Forbidden: updates may not remove a condition of type "Approved"`},
+			},
+		},
+		{
+			name: "add Denied condition",
+			newCSR: &capi.CertificateSigningRequest{ObjectMeta: validUpdateMeta, Spec: validSpec, Status: capi.CertificateSigningRequestStatus{
+				Conditions: []capi.CertificateSigningRequestCondition{{Type: capi.CertificateDenied, Status: core.ConditionTrue}},
+			}},
+			oldCSR: &capi.CertificateSigningRequest{ObjectMeta: validUpdateMetaWithFinalizers, Spec: validSpec},
+			versionErrs: map[string][]string{
+				"v1":      {`status.conditions: Forbidden: updates may not add a condition of type "Denied"`},
+				"v1beta1": {`status.conditions: Forbidden: updates may not add a condition of type "Denied"`},
+			},
+		},
+		{
+			name:   "remove Denied condition",
+			newCSR: &capi.CertificateSigningRequest{ObjectMeta: validUpdateMeta, Spec: validSpec},
+			oldCSR: &capi.CertificateSigningRequest{ObjectMeta: validUpdateMetaWithFinalizers, Spec: validSpec, Status: capi.CertificateSigningRequestStatus{
+				Conditions: []capi.CertificateSigningRequestCondition{{Type: capi.CertificateDenied, Status: core.ConditionTrue}},
+			}},
+			versionErrs: map[string][]string{
+				"v1":      {`status.conditions: Forbidden: updates may not remove a condition of type "Denied"`},
+				"v1beta1": {`status.conditions: Forbidden: updates may not remove a condition of type "Denied"`},
+			},
+		},
+		{
+			name: "add Failed condition",
+			newCSR: &capi.CertificateSigningRequest{ObjectMeta: validUpdateMeta, Spec: validSpec, Status: capi.CertificateSigningRequestStatus{
+				Conditions: []capi.CertificateSigningRequestCondition{{Type: capi.CertificateFailed, Status: core.ConditionTrue}},
+			}},
+			oldCSR:      &capi.CertificateSigningRequest{ObjectMeta: validUpdateMetaWithFinalizers, Spec: validSpec},
+			versionErrs: map[string][]string{},
+		},
+		{
+			name:   "remove Failed condition",
+			newCSR: &capi.CertificateSigningRequest{ObjectMeta: validUpdateMeta, Spec: validSpec},
+			oldCSR: &capi.CertificateSigningRequest{ObjectMeta: validUpdateMetaWithFinalizers, Spec: validSpec, Status: capi.CertificateSigningRequestStatus{
+				Conditions: []capi.CertificateSigningRequestCondition{{Type: capi.CertificateFailed, Status: core.ConditionTrue}},
+			}},
+			versionErrs: map[string][]string{
+				"v1":      {`status.conditions: Forbidden: updates may not remove a condition of type "Failed"`},
+				"v1beta1": {`status.conditions: Forbidden: updates may not remove a condition of type "Failed"`},
+			},
+		},
+		{
+			name: "set valid certificate",
+			newCSR: &capi.CertificateSigningRequest{ObjectMeta: validUpdateMeta, Spec: validSpec, Status: capi.CertificateSigningRequestStatus{
+				Certificate: validCertificate,
+			}},
+			oldCSR:      &capi.CertificateSigningRequest{ObjectMeta: validUpdateMetaWithFinalizers, Spec: validSpec},
+			versionErrs: map[string][]string{},
+		},
+		{
+			name: "set invalid certificate",
+			newCSR: &capi.CertificateSigningRequest{ObjectMeta: validUpdateMeta, Spec: validSpec, Status: capi.CertificateSigningRequestStatus{
+				Certificate: invalidCertificateNoPEM,
+			}},
+			oldCSR: &capi.CertificateSigningRequest{ObjectMeta: validUpdateMetaWithFinalizers, Spec: validSpec},
+			versionErrs: map[string][]string{
+				"v1": {`status.certificate: Invalid value: "<certificate data>": must contain at least one CERTIFICATE PEM block`},
+			},
+		},
+		{
+			name: "reset certificate",
+			newCSR: &capi.CertificateSigningRequest{ObjectMeta: validUpdateMeta, Spec: validSpec, Status: capi.CertificateSigningRequestStatus{
+				Certificate: invalidCertificateNonCertificatePEM,
+			}},
+			oldCSR: &capi.CertificateSigningRequest{ObjectMeta: validUpdateMetaWithFinalizers, Spec: validSpec, Status: capi.CertificateSigningRequestStatus{
+				Certificate: invalidCertificateNoPEM,
+			}},
+			versionErrs: map[string][]string{
+				"v1": {`status.certificate: Forbidden: updates may not modify existing certificate content`},
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		for _, version := range []string{"v1", "v1beta1"} {
+			t.Run(tt.name+"_"+version, func(t *testing.T) {
+				gotErrs := sets.NewString()
+				for _, err := range ValidateCertificateSigningRequestStatusUpdate(tt.newCSR, tt.oldCSR, schema.GroupVersion{Group: certificates.GroupName, Version: version}) {
+					gotErrs.Insert(err.Error())
+				}
+				wantErrs := sets.NewString(tt.versionErrs[version]...)
+				for _, missing := range wantErrs.Difference(gotErrs).List() {
+					t.Errorf("missing expected error: %s", missing)
+				}
+				for _, unexpected := range gotErrs.Difference(wantErrs).List() {
+					t.Errorf("unexpected error: %s", unexpected)
+				}
+			})
+		}
+	}
+}
+
+func TestValidateCertificateSigningRequestApprovalUpdate(t *testing.T) {
+	validUpdateMeta := validObjectMeta
+	validUpdateMeta.ResourceVersion = "1"
+
+	validUpdateMetaWithFinalizers := validUpdateMeta
+	validUpdateMetaWithFinalizers.Finalizers = []string{"foo"}
+
+	validSpec := capi.CertificateSigningRequestSpec{
+		Usages:     validUsages,
+		Request:    newCSRPEM(t),
+		SignerName: "example.com/something",
+	}
+
+	tests := []struct {
+		name        string
+		newCSR      *certificates.CertificateSigningRequest
+		oldCSR      *certificates.CertificateSigningRequest
+		versionErrs map[string][]string
+	}{
+		{
+			name:   "no-op",
+			newCSR: &capi.CertificateSigningRequest{ObjectMeta: validUpdateMeta, Spec: validSpec},
+			oldCSR: &capi.CertificateSigningRequest{ObjectMeta: validUpdateMeta, Spec: validSpec},
+		},
+		{
+			name:   "finalizer change with invalid certificate",
+			newCSR: &capi.CertificateSigningRequest{ObjectMeta: validUpdateMeta, Spec: validSpec, Status: capi.CertificateSigningRequestStatus{Certificate: invalidCertificateNoPEM}},
+			oldCSR: &capi.CertificateSigningRequest{ObjectMeta: validUpdateMetaWithFinalizers, Spec: validSpec, Status: capi.CertificateSigningRequestStatus{Certificate: invalidCertificateNoPEM}},
+		},
+		{
+			name: "add Approved condition",
+			newCSR: &capi.CertificateSigningRequest{ObjectMeta: validUpdateMeta, Spec: validSpec, Status: capi.CertificateSigningRequestStatus{
+				Conditions: []capi.CertificateSigningRequestCondition{{Type: capi.CertificateApproved, Status: core.ConditionTrue}},
+			}},
+			oldCSR: &capi.CertificateSigningRequest{ObjectMeta: validUpdateMetaWithFinalizers, Spec: validSpec},
+		},
+		{
+			name:   "remove Approved condition",
+			newCSR: &capi.CertificateSigningRequest{ObjectMeta: validUpdateMeta, Spec: validSpec},
+			oldCSR: &capi.CertificateSigningRequest{ObjectMeta: validUpdateMetaWithFinalizers, Spec: validSpec, Status: capi.CertificateSigningRequestStatus{
+				Conditions: []capi.CertificateSigningRequestCondition{{Type: capi.CertificateApproved, Status: core.ConditionTrue}},
+			}},
+			versionErrs: map[string][]string{
+				"v1":      {`status.conditions: Forbidden: updates may not remove a condition of type "Approved"`},
+				"v1beta1": {`status.conditions: Forbidden: updates may not remove a condition of type "Approved"`},
+			},
+		},
+		{
+			name: "add Denied condition",
+			newCSR: &capi.CertificateSigningRequest{ObjectMeta: validUpdateMeta, Spec: validSpec, Status: capi.CertificateSigningRequestStatus{
+				Conditions: []capi.CertificateSigningRequestCondition{{Type: capi.CertificateDenied, Status: core.ConditionTrue}},
+			}},
+			oldCSR: &capi.CertificateSigningRequest{ObjectMeta: validUpdateMetaWithFinalizers, Spec: validSpec},
+		},
+		{
+			name:   "remove Denied condition",
+			newCSR: &capi.CertificateSigningRequest{ObjectMeta: validUpdateMeta, Spec: validSpec},
+			oldCSR: &capi.CertificateSigningRequest{ObjectMeta: validUpdateMetaWithFinalizers, Spec: validSpec, Status: capi.CertificateSigningRequestStatus{
+				Conditions: []capi.CertificateSigningRequestCondition{{Type: capi.CertificateDenied, Status: core.ConditionTrue}},
+			}},
+			versionErrs: map[string][]string{
+				"v1":      {`status.conditions: Forbidden: updates may not remove a condition of type "Denied"`},
+				"v1beta1": {`status.conditions: Forbidden: updates may not remove a condition of type "Denied"`},
+			},
+		},
+		{
+			name: "add Failed condition",
+			newCSR: &capi.CertificateSigningRequest{ObjectMeta: validUpdateMeta, Spec: validSpec, Status: capi.CertificateSigningRequestStatus{
+				Conditions: []capi.CertificateSigningRequestCondition{{Type: capi.CertificateFailed, Status: core.ConditionTrue}},
+			}},
+			oldCSR:      &capi.CertificateSigningRequest{ObjectMeta: validUpdateMetaWithFinalizers, Spec: validSpec},
+			versionErrs: map[string][]string{},
+		},
+		{
+			name:   "remove Failed condition",
+			newCSR: &capi.CertificateSigningRequest{ObjectMeta: validUpdateMeta, Spec: validSpec},
+			oldCSR: &capi.CertificateSigningRequest{ObjectMeta: validUpdateMetaWithFinalizers, Spec: validSpec, Status: capi.CertificateSigningRequestStatus{
+				Conditions: []capi.CertificateSigningRequestCondition{{Type: capi.CertificateFailed, Status: core.ConditionTrue}},
+			}},
+			versionErrs: map[string][]string{
+				"v1":      {`status.conditions: Forbidden: updates may not remove a condition of type "Failed"`},
+				"v1beta1": {`status.conditions: Forbidden: updates may not remove a condition of type "Failed"`},
+			},
+		},
+		{
+			name: "set certificate",
+			newCSR: &capi.CertificateSigningRequest{ObjectMeta: validUpdateMeta, Spec: validSpec, Status: capi.CertificateSigningRequestStatus{
+				Certificate: validCertificate,
+			}},
+			oldCSR: &capi.CertificateSigningRequest{ObjectMeta: validUpdateMetaWithFinalizers, Spec: validSpec},
+			versionErrs: map[string][]string{
+				"v1":      {`status.certificate: Forbidden: updates may not set certificate content`},
+				"v1beta1": {`status.certificate: Forbidden: updates may not set certificate content`},
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		for _, version := range []string{"v1", "v1beta1"} {
+			t.Run(tt.name+"_"+version, func(t *testing.T) {
+				gotErrs := sets.NewString()
+				for _, err := range ValidateCertificateSigningRequestApprovalUpdate(tt.newCSR, tt.oldCSR, schema.GroupVersion{Group: certificates.GroupName, Version: version}) {
+					gotErrs.Insert(err.Error())
+				}
+				wantErrs := sets.NewString(tt.versionErrs[version]...)
+				for _, missing := range wantErrs.Difference(gotErrs).List() {
+					t.Errorf("missing expected error: %s", missing)
+				}
+				for _, unexpected := range gotErrs.Difference(wantErrs).List() {
+					t.Errorf("unexpected error: %s", unexpected)
+				}
+			})
+		}
+	}
+}
+
+// Test_validateCertificateSigningRequestOptions verifies validation options are effective in tolerating specific aspects of CSRs
+func Test_validateCertificateSigningRequestOptions(t *testing.T) {
+	validSpec := capi.CertificateSigningRequestSpec{
+		Usages:     validUsages,
+		Request:    newCSRPEM(t),
+		SignerName: "example.com/something",
+	}
+
+	tests := []struct {
+		// testcase name
+		name string
+
+		// csr being validated
+		csr *certificates.CertificateSigningRequest
+
+		// options that allow the csr to pass validation
+		lenientOpts certificateValidationOptions
+
+		// expected errors when validating strictly
+		strictErrs []string
+	}{
+		// valid strict cases
+		{
+			name: "no status",
+			csr:  &capi.CertificateSigningRequest{ObjectMeta: validObjectMeta, Spec: validSpec},
+		},
+		{
+			name: "approved condition",
+			csr: &capi.CertificateSigningRequest{ObjectMeta: validObjectMeta, Spec: validSpec,
+				Status: capi.CertificateSigningRequestStatus{
+					Conditions: []capi.CertificateSigningRequestCondition{{Type: capi.CertificateApproved, Status: core.ConditionTrue}},
+				},
+			},
+		},
+		{
+			name: "denied condition",
+			csr: &capi.CertificateSigningRequest{ObjectMeta: validObjectMeta, Spec: validSpec,
+				Status: capi.CertificateSigningRequestStatus{
+					Conditions: []capi.CertificateSigningRequestCondition{{Type: capi.CertificateDenied, Status: core.ConditionTrue}},
+				},
+			},
+		},
+		{
+			name: "failed condition",
+			csr: &capi.CertificateSigningRequest{ObjectMeta: validObjectMeta, Spec: validSpec,
+				Status: capi.CertificateSigningRequestStatus{
+					Conditions: []capi.CertificateSigningRequestCondition{{Type: capi.CertificateFailed, Status: core.ConditionTrue}},
+				},
+			},
+		},
+		{
+			name: "approved+issued",
+			csr: &capi.CertificateSigningRequest{ObjectMeta: validObjectMeta, Spec: validSpec,
+				Status: capi.CertificateSigningRequestStatus{
+					Conditions:  []capi.CertificateSigningRequestCondition{{Type: capi.CertificateApproved, Status: core.ConditionTrue}},
+					Certificate: validCertificate,
+				},
+			},
+		},
+
+		// legacy signer
+		{
+			name: "legacy signer",
+			csr: &capi.CertificateSigningRequest{
+				ObjectMeta: validObjectMeta,
+				Spec: func() capi.CertificateSigningRequestSpec {
+					specCopy := validSpec
+					specCopy.SignerName = capi.LegacyUnknownSignerName
+					return specCopy
+				}(),
+			},
+			lenientOpts: certificateValidationOptions{allowLegacySignerName: true},
+			strictErrs:  []string{`spec.signerName: Invalid value: "kubernetes.io/legacy-unknown": the legacy signerName is not allowed via this API version`},
+		},
+
+		// invalid condition cases
+		{
+			name: "empty condition type",
+			csr: &capi.CertificateSigningRequest{ObjectMeta: validObjectMeta, Spec: validSpec,
+				Status: capi.CertificateSigningRequestStatus{
+					Conditions: []capi.CertificateSigningRequestCondition{{Status: core.ConditionTrue}},
+				},
+			},
+			lenientOpts: certificateValidationOptions{allowEmptyConditionType: true},
+			strictErrs:  []string{`status.conditions[0].type: Required value`},
+		},
+		{
+			name: "approved and denied",
+			csr: &capi.CertificateSigningRequest{ObjectMeta: validObjectMeta, Spec: validSpec,
+				Status: capi.CertificateSigningRequestStatus{
+					Conditions: []capi.CertificateSigningRequestCondition{{Type: capi.CertificateApproved, Status: core.ConditionTrue}, {Type: capi.CertificateDenied, Status: core.ConditionTrue}},
+				},
+			},
+			lenientOpts: certificateValidationOptions{allowBothApprovedAndDenied: true},
+			strictErrs:  []string{`status.conditions[1].type: Invalid value: "Denied": Approved and Denied conditions are mutually exclusive`},
+		},
+		{
+			name: "duplicate condition",
+			csr: &capi.CertificateSigningRequest{ObjectMeta: validObjectMeta, Spec: validSpec,
+				Status: capi.CertificateSigningRequestStatus{
+					Conditions: []capi.CertificateSigningRequestCondition{{Type: capi.CertificateApproved, Status: core.ConditionTrue}, {Type: capi.CertificateApproved, Status: core.ConditionTrue}},
+				},
+			},
+			lenientOpts: certificateValidationOptions{allowDuplicateConditionTypes: true},
+			strictErrs:  []string{`status.conditions[1].type: Duplicate value: "Approved"`},
+		},
+
+		// invalid allowArbitraryCertificate cases
+		{
+			name: "status.certificate, no PEM",
+			csr: &capi.CertificateSigningRequest{ObjectMeta: validObjectMeta, Spec: validSpec,
+				Status: capi.CertificateSigningRequestStatus{
+					Conditions:  []capi.CertificateSigningRequestCondition{{Type: capi.CertificateApproved, Status: core.ConditionTrue}},
+					Certificate: invalidCertificateNoPEM,
+				},
+			},
+			lenientOpts: certificateValidationOptions{allowArbitraryCertificate: true},
+			strictErrs:  []string{`status.certificate: Invalid value: "<certificate data>": must contain at least one CERTIFICATE PEM block`},
+		},
+		{
+			name: "status.certificate, non-CERTIFICATE PEM",
+			csr: &capi.CertificateSigningRequest{ObjectMeta: validObjectMeta, Spec: validSpec,
+				Status: capi.CertificateSigningRequestStatus{
+					Conditions:  []capi.CertificateSigningRequestCondition{{Type: capi.CertificateApproved, Status: core.ConditionTrue}},
+					Certificate: invalidCertificateNonCertificatePEM,
+				},
+			},
+			lenientOpts: certificateValidationOptions{allowArbitraryCertificate: true},
+			strictErrs:  []string{`status.certificate: Invalid value: "<certificate data>": only CERTIFICATE PEM blocks are allowed, found "CERTIFICATE1"`},
+		},
+		{
+			name: "status.certificate, PEM headers",
+			csr: &capi.CertificateSigningRequest{ObjectMeta: validObjectMeta, Spec: validSpec,
+				Status: capi.CertificateSigningRequestStatus{
+					Conditions:  []capi.CertificateSigningRequestCondition{{Type: capi.CertificateApproved, Status: core.ConditionTrue}},
+					Certificate: invalidCertificatePEMHeaders,
+				},
+			},
+			lenientOpts: certificateValidationOptions{allowArbitraryCertificate: true},
+			strictErrs:  []string{`status.certificate: Invalid value: "<certificate data>": no PEM block headers are permitted`},
+		},
+		{
+			name: "status.certificate, non-base64 PEM",
+			csr: &capi.CertificateSigningRequest{ObjectMeta: validObjectMeta, Spec: validSpec,
+				Status: capi.CertificateSigningRequestStatus{
+					Conditions:  []capi.CertificateSigningRequestCondition{{Type: capi.CertificateApproved, Status: core.ConditionTrue}},
+					Certificate: invalidCertificateNonBase64PEM,
+				},
+			},
+			lenientOpts: certificateValidationOptions{allowArbitraryCertificate: true},
+			strictErrs:  []string{`status.certificate: Invalid value: "<certificate data>": must contain at least one CERTIFICATE PEM block`},
+		},
+		{
+			name: "status.certificate, empty PEM block",
+			csr: &capi.CertificateSigningRequest{ObjectMeta: validObjectMeta, Spec: validSpec,
+				Status: capi.CertificateSigningRequestStatus{
+					Conditions:  []capi.CertificateSigningRequestCondition{{Type: capi.CertificateApproved, Status: core.ConditionTrue}},
+					Certificate: invalidCertificateEmptyPEM,
+				},
+			},
+			lenientOpts: certificateValidationOptions{allowArbitraryCertificate: true},
+			strictErrs:  []string{`status.certificate: Invalid value: "<certificate data>": found CERTIFICATE PEM block containing 0 certificates`},
+		},
+		{
+			name: "status.certificate, non-ASN1 data",
+			csr: &capi.CertificateSigningRequest{ObjectMeta: validObjectMeta, Spec: validSpec,
+				Status: capi.CertificateSigningRequestStatus{
+					Conditions:  []capi.CertificateSigningRequestCondition{{Type: capi.CertificateApproved, Status: core.ConditionTrue}},
+					Certificate: invalidCertificateNonASN1Data,
+				},
+			},
+			lenientOpts: certificateValidationOptions{allowArbitraryCertificate: true},
+			strictErrs:  []string{`status.certificate: Invalid value: "<certificate data>": asn1: structure error: sequence tag mismatch`},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			// make sure the lenient options validate with no errors
+			for _, err := range validateCertificateSigningRequest(tt.csr, tt.lenientOpts) {
+				t.Errorf("unexpected error with lenient options: %s", err.Error())
+			}
+
+			// make sure the strict options produce the expected errors
+			gotErrs := sets.NewString()
+			for _, err := range validateCertificateSigningRequest(tt.csr, certificateValidationOptions{}) {
+				gotErrs.Insert(err.Error())
+			}
+			wantErrs := sets.NewString(tt.strictErrs...)
+			for _, missing := range wantErrs.Difference(gotErrs).List() {
+				t.Errorf("missing expected strict error: %s", missing)
+			}
+			for _, unexpected := range gotErrs.Difference(wantErrs).List() {
+				t.Errorf("unexpected strict error: %s", unexpected)
+			}
+		})
+	}
+}
+
+var (
+	validCertificate = []byte(`
+Leading non-PEM content
+-----BEGIN CERTIFICATE-----
+MIIBqDCCAU2gAwIBAgIUfbqeieihh/oERbfvRm38XvS/xHAwCgYIKoZIzj0EAwIw
+GjEYMBYGA1UEAxMPSW50ZXJtZWRpYXRlLUNBMCAXDTE2MTAxMTA1MDYwMFoYDzIx
+MTYwOTE3MDUwNjAwWjAUMRIwEAYDVQQDEwlNeSBDbGllbnQwWTATBgcqhkjOPQIB
+BggqhkjOPQMBBwNCAARv6N4R/sjMR65iMFGNLN1GC/vd7WhDW6J4X/iAjkRLLnNb
+KbRG/AtOUZ+7upJ3BWIRKYbOabbQGQe2BbKFiap4o3UwczAOBgNVHQ8BAf8EBAMC
+BaAwEwYDVR0lBAwwCgYIKwYBBQUHAwIwDAYDVR0TAQH/BAIwADAdBgNVHQ4EFgQU
+K/pZOWpNcYai6eHFpmJEeFpeQlEwHwYDVR0jBBgwFoAUX6nQlxjfWnP6aM1meO/Q
+a6b3a9kwCgYIKoZIzj0EAwIDSQAwRgIhAIWTKw/sjJITqeuNzJDAKU4xo1zL+xJ5
+MnVCuBwfwDXCAiEAw/1TA+CjPq9JC5ek1ifR0FybTURjeQqYkKpve1dveps=
+-----END CERTIFICATE-----
+Intermediate non-PEM content
+-----BEGIN CERTIFICATE-----
+MIIBqDCCAU6gAwIBAgIUfqZtjoFgczZ+oQZbEC/BDSS2J6wwCgYIKoZIzj0EAwIw
+EjEQMA4GA1UEAxMHUm9vdC1DQTAgFw0xNjEwMTEwNTA2MDBaGA8yMTE2MDkxNzA1
+MDYwMFowGjEYMBYGA1UEAxMPSW50ZXJtZWRpYXRlLUNBMFkwEwYHKoZIzj0CAQYI
+KoZIzj0DAQcDQgAEyWHEMMCctJg8Xa5YWLqaCPbk3MjB+uvXac42JM9pj4k9jedD
+kpUJRkWIPzgJI8Zk/3cSzluUTixP6JBSDKtwwaN4MHYwDgYDVR0PAQH/BAQDAgGm
+MBMGA1UdJQQMMAoGCCsGAQUFBwMCMA8GA1UdEwEB/wQFMAMBAf8wHQYDVR0OBBYE
+FF+p0JcY31pz+mjNZnjv0Gum92vZMB8GA1UdIwQYMBaAFB7P6+i4/pfNjqZgJv/b
+dgA7Fe4tMAoGCCqGSM49BAMCA0gAMEUCIQCTT1YWQZaAqfQ2oBxzOkJE2BqLFxhz
+3smQlrZ5gCHddwIgcvT7puhYOzAgcvMn9+SZ1JOyZ7edODjshCVCRnuHK2c=
+-----END CERTIFICATE-----
+Trailing non-PEM content
+`)
+
+	invalidCertificateNoPEM = []byte(`no PEM content`)
+
+	invalidCertificateNonCertificatePEM = []byte(`
+Leading non-PEM content
+-----BEGIN CERTIFICATE1-----
+MIIBqDCCAU2gAwIBAgIUfbqeieihh/oERbfvRm38XvS/xHAwCgYIKoZIzj0EAwIw
+GjEYMBYGA1UEAxMPSW50ZXJtZWRpYXRlLUNBMCAXDTE2MTAxMTA1MDYwMFoYDzIx
+MTYwOTE3MDUwNjAwWjAUMRIwEAYDVQQDEwlNeSBDbGllbnQwWTATBgcqhkjOPQIB
+BggqhkjOPQMBBwNCAARv6N4R/sjMR65iMFGNLN1GC/vd7WhDW6J4X/iAjkRLLnNb
+KbRG/AtOUZ+7upJ3BWIRKYbOabbQGQe2BbKFiap4o3UwczAOBgNVHQ8BAf8EBAMC
+BaAwEwYDVR0lBAwwCgYIKwYBBQUHAwIwDAYDVR0TAQH/BAIwADAdBgNVHQ4EFgQU
+K/pZOWpNcYai6eHFpmJEeFpeQlEwHwYDVR0jBBgwFoAUX6nQlxjfWnP6aM1meO/Q
+a6b3a9kwCgYIKoZIzj0EAwIDSQAwRgIhAIWTKw/sjJITqeuNzJDAKU4xo1zL+xJ5
+MnVCuBwfwDXCAiEAw/1TA+CjPq9JC5ek1ifR0FybTURjeQqYkKpve1dveps=
+-----END CERTIFICATE1-----
+Trailing non-PEM content
+`)
+
+	invalidCertificatePEMHeaders = []byte(`
+Leading non-PEM content
+-----BEGIN CERTIFICATE-----
+Some-Header: Some-Value
+MIIBqDCCAU2gAwIBAgIUfbqeieihh/oERbfvRm38XvS/xHAwCgYIKoZIzj0EAwIw
+GjEYMBYGA1UEAxMPSW50ZXJtZWRpYXRlLUNBMCAXDTE2MTAxMTA1MDYwMFoYDzIx
+MTYwOTE3MDUwNjAwWjAUMRIwEAYDVQQDEwlNeSBDbGllbnQwWTATBgcqhkjOPQIB
+BggqhkjOPQMBBwNCAARv6N4R/sjMR65iMFGNLN1GC/vd7WhDW6J4X/iAjkRLLnNb
+KbRG/AtOUZ+7upJ3BWIRKYbOabbQGQe2BbKFiap4o3UwczAOBgNVHQ8BAf8EBAMC
+BaAwEwYDVR0lBAwwCgYIKwYBBQUHAwIwDAYDVR0TAQH/BAIwADAdBgNVHQ4EFgQU
+K/pZOWpNcYai6eHFpmJEeFpeQlEwHwYDVR0jBBgwFoAUX6nQlxjfWnP6aM1meO/Q
+a6b3a9kwCgYIKoZIzj0EAwIDSQAwRgIhAIWTKw/sjJITqeuNzJDAKU4xo1zL+xJ5
+MnVCuBwfwDXCAiEAw/1TA+CjPq9JC5ek1ifR0FybTURjeQqYkKpve1dveps=
+-----END CERTIFICATE-----
+Trailing non-PEM content
+`)
+
+	invalidCertificateNonBase64PEM = []byte(`
+Leading non-PEM content
+-----BEGIN CERTIFICATE-----
+MIIBqDCCAU2gAwIBAgIUfbqeieihh/oERbfvRm38XvS/xHAwCgYIKoZIzj0EAwIw
+GjEYMBYGA1UEAxMPSW50ZXJtZWRpYXRlLUNBMCAXDTE2MTAxMTA1MDYwMFoYDzIx
+MTYwOTE3MDUwNjAwWjAUMRIwEAYDVQQDEwlNeSBDbGllbnQwWTATBgcqhkjOPQIB
+BggqhkjOPQMBBwNCAARv6N4R/sjMR65iMFGNLN1GC/vd7WhDW6J4X/iAjkRLLnNb
+KbRG/AtOUZ+7upJ3BWIRKYbOabbQGQe2BbKFiap4o3UwczAOBgNVHQ8BAf8EBAMC
+BaAwEwYDVR0lBAwwCgYIKwYBBQUHAwIwDAYDVR0TAQH/BAIwADAdBgNVHQ4EFgQU
+K/pZOWpNcYai6eHFpmJEeFpeQlEwHwYDVR0jBBgwFoAUX6nQlxjfWnP6aM1meO/Q
+a6b3a9kwCgYIKoZIzj0EAwIDSQAwRgIhAIWTKw/sjJITqeuNzJDAKU4xo1zL+xJ5
+MnVCuBwfwDXCAiEAw/1TA+CjPq9JC5ek1ifR0FybTURjeQqYkKpve1d?????????
+-----END CERTIFICATE-----
+Trailing non-PEM content
+`)
+
+	invalidCertificateEmptyPEM = []byte(`
+Leading non-PEM content
+-----BEGIN CERTIFICATE-----
+-----END CERTIFICATE-----
+Trailing non-PEM content
+`)
+
+	// first character is invalid
+	invalidCertificateNonASN1Data = []byte(`
+Leading non-PEM content
+-----BEGIN CERTIFICATE-----
+MIIBqDCCAU2gAwIBAgIUfbqeieihh/oERbfvRm38XvS/xHAwCgYIKoZIzj0EAwIw
+GjEYMBYGA1UEAxMPSW50ZXJtZWRpYXRlLUNBMCAXDTE2MTAxMTA1MDYwMFoYDzIx
+MTYwOTE3MDUwNjAwWjAUNRIwEAYDVQQDEwlNeSBDbGllbnQwWTATBgcqhkjOPQIB
+BggqhkjOPQMBBwNCAARv6N4R/sjMR65iMFGNLN1GC/vd7WhDW6J4X/iAjkRLLnNb
+KbRG/AtOUZ+7upJ3BWIRKYbOabbQGQe2BbKFiap4o3UwczAOBgNVHQ8BAf8EBAMC
+BaAwEwYDVR0lBAwwCgYIKwYBBQUHAwIwDAYDVR0TAQH/BAIwADAdBgNVHQ4EFgQU
+K/pZOWpNcYai6eHFpmJEeFpeQlEwHwYDVR0jBBgwFoAUX6nQlxjfWnP6aM1meO/Q
+a6b3a9kwCgYIKoZIzj0EAwIDSQAwRgIhAIWTKw/sjJITqeuNzJDAKU4xo1zL+xJ5
+MnVCuBwfwDXCAiEAw/1TA+CjPq9JC5ek1ifR0FybTURjeQqYkKpve1dveps=
+-----END CERTIFICATE-----
+Trailing non-PEM content
+`)
+)

pkg/registry/certificates/certificates/strategy.go

@@ -20,10 +20,12 @@ import (
 	"context"
 	"fmt"
 
+	certificatesv1beta1 "k8s.io/api/certificates/v1beta1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/fields"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
 	"k8s.io/apiserver/pkg/registry/generic"
@@ -94,7 +96,7 @@ func (csrStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Object
 // Validate validates a new CSR. Validation must check for a correct signature.
 func (csrStrategy) Validate(ctx context.Context, obj runtime.Object) field.ErrorList {
 	csr := obj.(*certificates.CertificateSigningRequest)
-	return validation.ValidateCertificateSigningRequest(csr)
+	return validation.ValidateCertificateSigningRequestCreate(csr, requestGroupVersion(ctx))
 }
 
 // Canonicalize normalizes the object after validation (which includes a signature check).
@@ -104,7 +106,7 @@ func (csrStrategy) Canonicalize(obj runtime.Object) {}
 func (csrStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) field.ErrorList {
 	oldCSR := old.(*certificates.CertificateSigningRequest)
 	newCSR := obj.(*certificates.CertificateSigningRequest)
-	return validation.ValidateCertificateSigningRequestUpdate(newCSR, oldCSR)
+	return validation.ValidateCertificateSigningRequestUpdate(newCSR, oldCSR, requestGroupVersion(ctx))
 }
 
 // If AllowUnconditionalUpdate() is true and the object specified by
@@ -142,20 +144,84 @@ func (csrStatusStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.
 	newCSR := obj.(*certificates.CertificateSigningRequest)
 	oldCSR := old.(*certificates.CertificateSigningRequest)
 
-	// Updating the Status should only update the Status and not the spec
+	// Updating /status should not modify spec
-	// or approval conditions. The intent is to separate the concerns of
-	// approval and certificate issuance.
 	newCSR.Spec = oldCSR.Spec
-	newCSR.Status.Conditions = oldCSR.Status.Conditions
+
+	switch requestGroupVersion(ctx) {
+	case certificatesv1beta1.SchemeGroupVersion:
+		// Specifically preserve existing Approved/Denied conditions.
+		// If we cannot (if the status update attempted to add/remove Approved/Denied conditions), revert to old conditions for backwards compatibility.
+		if !preserveConditionInstances(newCSR, oldCSR, certificates.CertificateApproved) || !preserveConditionInstances(newCSR, oldCSR, certificates.CertificateDenied) {
+			newCSR.Status.Conditions = oldCSR.Status.Conditions
+		}
+	default:
+		// Specifically preserve existing Approved/Denied conditions.
+		// Adding/removing Approved/Denied conditions will cause these to fail,
+		// and the change in Approved/Denied conditions will produce a validation error
+		preserveConditionInstances(newCSR, oldCSR, certificates.CertificateApproved)
+		preserveConditionInstances(newCSR, oldCSR, certificates.CertificateDenied)
+	}
+
+	populateConditionTimestamps(newCSR, oldCSR)
+}
+
+// preserveConditionInstances copies instances of the the specified condition type from oldCSR to newCSR.
+// or returns false if the newCSR added or removed instances
+func preserveConditionInstances(newCSR, oldCSR *certificates.CertificateSigningRequest, conditionType certificates.RequestConditionType) bool {
+	oldIndices := findConditionIndices(oldCSR, conditionType)
+	newIndices := findConditionIndices(newCSR, conditionType)
+	if len(oldIndices) != len(newIndices) {
+		// instances were added or removed, we cannot preserve the existing values
+		return false
+	}
+	// preserve the old condition values
+	for i, oldIndex := range oldIndices {
+		newCSR.Status.Conditions[newIndices[i]] = oldCSR.Status.Conditions[oldIndex]
+	}
+	return true
+}
+
+// findConditionIndices returns the indices of instances of the specified condition type
+func findConditionIndices(csr *certificates.CertificateSigningRequest, conditionType certificates.RequestConditionType) []int {
+	var retval []int
+	for i, c := range csr.Status.Conditions {
+		if c.Type == conditionType {
+			retval = append(retval, i)
+		}
+	}
+	return retval
+}
+
+// nowFunc allows overriding for unit tests
+var nowFunc = metav1.Now
+
+// populateConditionTimestamps sets LastUpdateTime and LastTransitionTime in newCSR if missing
+func populateConditionTimestamps(newCSR, oldCSR *certificates.CertificateSigningRequest) {
+	now := nowFunc()
 	for i := range newCSR.Status.Conditions {
 		if newCSR.Status.Conditions[i].LastUpdateTime.IsZero() {
-			newCSR.Status.Conditions[i].LastUpdateTime = metav1.Now()
+			newCSR.Status.Conditions[i].LastUpdateTime = now
+		}
+
+		// preserve existing lastTransitionTime if the condition with this type/status already exists,
+		// otherwise set to now.
+		if newCSR.Status.Conditions[i].LastTransitionTime.IsZero() {
+			lastTransition := now
+			for _, oldCondition := range oldCSR.Status.Conditions {
+				if oldCondition.Type == newCSR.Status.Conditions[i].Type &&
+					oldCondition.Status == newCSR.Status.Conditions[i].Status &&
+					!oldCondition.LastTransitionTime.IsZero() {
+					lastTransition = oldCondition.LastTransitionTime
+					break
+				}
+			}
+			newCSR.Status.Conditions[i].LastTransitionTime = lastTransition
 		}
 	}
 }
 
 func (csrStatusStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) field.ErrorList {
-	return validation.ValidateCertificateSigningRequestUpdate(obj.(*certificates.CertificateSigningRequest), old.(*certificates.CertificateSigningRequest))
+	return validation.ValidateCertificateSigningRequestStatusUpdate(obj.(*certificates.CertificateSigningRequest), old.(*certificates.CertificateSigningRequest), requestGroupVersion(ctx))
 }
 
 // Canonicalize normalizes the object after validation.
@@ -170,28 +236,22 @@ type csrApprovalStrategy struct {
 var ApprovalStrategy = csrApprovalStrategy{Strategy}
 
 // PrepareForUpdate prepares the new certificate signing request by limiting
-// the data that is updated to only the conditions. Also, if there is no
+// the data that is updated to only the conditions and populating condition timestamps
-// existing LastUpdateTime on a condition, the current date/time will be set.
 func (csrApprovalStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Object) {
 	newCSR := obj.(*certificates.CertificateSigningRequest)
 	oldCSR := old.(*certificates.CertificateSigningRequest)
 
+	populateConditionTimestamps(newCSR, oldCSR)
+	newConditions := newCSR.Status.Conditions
+
 	// Updating the approval should only update the conditions.
 	newCSR.Spec = oldCSR.Spec
-	oldCSR.Status.Conditions = newCSR.Status.Conditions
-	for i := range newCSR.Status.Conditions {
-		// The Conditions are an array of values, some of which may be
-		// pre-existing and unaltered by this update, so a LastUpdateTime is
-		// added only if one isn't already set.
-		if newCSR.Status.Conditions[i].LastUpdateTime.IsZero() {
-			newCSR.Status.Conditions[i].LastUpdateTime = metav1.Now()
-		}
-	}
 	newCSR.Status = oldCSR.Status
+	newCSR.Status.Conditions = newConditions
 }
 
 func (csrApprovalStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) field.ErrorList {
-	return validation.ValidateCertificateSigningRequestUpdate(obj.(*certificates.CertificateSigningRequest), old.(*certificates.CertificateSigningRequest))
+	return validation.ValidateCertificateSigningRequestApprovalUpdate(obj.(*certificates.CertificateSigningRequest), old.(*certificates.CertificateSigningRequest), requestGroupVersion(ctx))
 }
 
 // GetAttrs returns labels and fields of a given object for filtering purposes.
@@ -211,3 +271,11 @@ func SelectableFields(obj *certificates.CertificateSigningRequest) fields.Set {
 	}
 	return generic.MergeFieldsSets(objectMetaFieldsSet, csrSpecificFieldsSet)
 }
+
+// requestGroupVersion returns the group/version associated with the given context, or a zero-value group/version
+func requestGroupVersion(ctx context.Context) schema.GroupVersion {
+	if requestInfo, found := genericapirequest.RequestInfoFrom(ctx); found {
+		return schema.GroupVersion{Group: requestInfo.APIGroup, Version: requestInfo.APIVersion}
+	}
+	return schema.GroupVersion{}
+}

pkg/registry/certificates/certificates/strategy_test.go

@@ -20,7 +20,9 @@ import (
 	"context"
 	"reflect"
 	"testing"
+	"time"
 
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/diff"
 	"k8s.io/apiserver/pkg/authentication/user"
@@ -121,3 +123,117 @@ func TestStrategyCreate(t *testing.T) {
 		}
 	}
 }
+
+func TestStatusUpdate(t *testing.T) {
+	now := metav1.Now()
+	later := metav1.NewTime(now.Add(time.Hour))
+	nowFunc = func() metav1.Time { return now }
+	defer func() {
+		nowFunc = metav1.Now
+	}()
+
+	tests := []struct {
+		name         string
+		newObj       *certapi.CertificateSigningRequest
+		oldObj       *certapi.CertificateSigningRequest
+		expectedObjs map[string]*certapi.CertificateSigningRequest
+	}{
+		{
+			name:   "no-op",
+			newObj: &certapi.CertificateSigningRequest{},
+			oldObj: &certapi.CertificateSigningRequest{},
+			expectedObjs: map[string]*certapi.CertificateSigningRequest{
+				"v1":      {},
+				"v1beta1": {},
+			},
+		},
+		{
+			name: "adding failed condition to existing approved/denied conditions",
+			newObj: &certapi.CertificateSigningRequest{
+				Status: certapi.CertificateSigningRequestStatus{
+					Conditions: []certapi.CertificateSigningRequestCondition{
+						{Type: certapi.CertificateFailed},
+						{Type: certapi.CertificateDenied},
+						{Type: certapi.CertificateApproved},
+						{Type: certapi.CertificateDenied},
+						{Type: certapi.CertificateApproved},
+					},
+				},
+			},
+			oldObj: &certapi.CertificateSigningRequest{
+				Status: certapi.CertificateSigningRequestStatus{
+					Conditions: []certapi.CertificateSigningRequestCondition{
+						{Type: certapi.CertificateDenied, Reason: "because1"},
+						{Type: certapi.CertificateApproved, Reason: "because2"},
+						{Type: certapi.CertificateDenied, Reason: "because3", LastUpdateTime: later, LastTransitionTime: later},
+						{Type: certapi.CertificateApproved, Reason: "because4", LastUpdateTime: later, LastTransitionTime: later},
+					},
+				},
+			},
+			expectedObjs: map[string]*certapi.CertificateSigningRequest{
+				// preserve existing Approved/Denied conditions
+				"v1": {
+					Status: certapi.CertificateSigningRequestStatus{
+						Conditions: []certapi.CertificateSigningRequestCondition{
+							{Type: certapi.CertificateFailed, LastUpdateTime: now, LastTransitionTime: now},
+							{Type: certapi.CertificateDenied, LastUpdateTime: now, LastTransitionTime: later, Reason: "because1"},
+							{Type: certapi.CertificateApproved, LastUpdateTime: now, LastTransitionTime: later, Reason: "because2"},
+							{Type: certapi.CertificateDenied, LastUpdateTime: later, LastTransitionTime: later, Reason: "because3"},
+							{Type: certapi.CertificateApproved, LastUpdateTime: later, LastTransitionTime: later, Reason: "because4"},
+						},
+					},
+				},
+				// preserve existing Approved/Denied conditions
+				"v1beta1": {
+					Status: certapi.CertificateSigningRequestStatus{
+						Conditions: []certapi.CertificateSigningRequestCondition{
+							{Type: certapi.CertificateFailed, LastUpdateTime: now, LastTransitionTime: now},
+							{Type: certapi.CertificateDenied, LastUpdateTime: now, LastTransitionTime: later, Reason: "because1"},
+							{Type: certapi.CertificateApproved, LastUpdateTime: now, LastTransitionTime: later, Reason: "because2"},
+							{Type: certapi.CertificateDenied, LastUpdateTime: later, LastTransitionTime: later, Reason: "because3"},
+							{Type: certapi.CertificateApproved, LastUpdateTime: later, LastTransitionTime: later, Reason: "because4"},
+						},
+					},
+				},
+			},
+		},
+		{
+			name: "add approved condition",
+			newObj: &certapi.CertificateSigningRequest{
+				Status: certapi.CertificateSigningRequestStatus{
+					Conditions: []certapi.CertificateSigningRequestCondition{
+						{Type: certapi.CertificateApproved},
+					},
+				},
+			},
+			oldObj: &certapi.CertificateSigningRequest{},
+			expectedObjs: map[string]*certapi.CertificateSigningRequest{
+				// preserved submitted conditions if existing Approved/Denied conditions could not be copied over (will fail validation)
+				"v1": {
+					Status: certapi.CertificateSigningRequestStatus{
+						Conditions: []certapi.CertificateSigningRequestCondition{
+							{Type: certapi.CertificateApproved, LastUpdateTime: now, LastTransitionTime: now},
+						},
+					},
+				},
+				// reset conditions to existing conditions if Approved/Denied conditions could not be copied over
+				"v1beta1": {
+					Status: certapi.CertificateSigningRequestStatus{},
+				},
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		for _, version := range []string{"v1", "v1beta1"} {
+			t.Run(tt.name+"_"+version, func(t *testing.T) {
+				ctx := genericapirequest.WithRequestInfo(context.TODO(), &genericapirequest.RequestInfo{APIGroup: "certificates.k8s.io", APIVersion: version})
+				obj := tt.newObj.DeepCopy()
+				StatusStrategy.PrepareForUpdate(ctx, obj, tt.oldObj.DeepCopy())
+				if !reflect.DeepEqual(obj, tt.expectedObjs[version]) {
+					t.Errorf("object diff: %s", diff.ObjectDiff(obj, tt.expectedObjs[version]))
+				}
+			})
+		}
+	}
+}