diff --git a/pkg/apis/certificates/validation/validation.go b/pkg/apis/certificates/validation/validation.go index d24446355af..20d84a3395a 100644 --- a/pkg/apis/certificates/validation/validation.go +++ b/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"), "", 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 { - validationErrorList := ValidateCertificateSigningRequest(newCSR) +func ValidateCertificateSigningRequestUpdate(newCSR, oldCSR *certificates.CertificateSigningRequest, version schema.GroupVersion) field.ErrorList { + 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 + } +} diff --git a/pkg/apis/certificates/validation/validation_test.go b/pkg/apis/certificates/validation/validation_test.go index 12857c3fbd0..78675b664e9 100644 --- a/pkg/apis/certificates/validation/validation_test.go +++ b/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: "": 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: "": 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: "": 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: "": 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: "": 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: "": 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: "": 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 +`) +) diff --git a/pkg/registry/certificates/certificates/strategy.go b/pkg/registry/certificates/certificates/strategy.go index 1bbebdedde9..fc35c33acfa 100644 --- a/pkg/registry/certificates/certificates/strategy.go +++ b/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 - // or approval conditions. The intent is to separate the concerns of - // approval and certificate issuance. + // Updating /status should not modify spec 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 -// existing LastUpdateTime on a condition, the current date/time will be set. +// the data that is updated to only the conditions and populating condition timestamps 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{} +} diff --git a/pkg/registry/certificates/certificates/strategy_test.go b/pkg/registry/certificates/certificates/strategy_test.go index b1b660ba068..0c940b296f2 100644 --- a/pkg/registry/certificates/certificates/strategy_test.go +++ b/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])) + } + }) + } + } +}