From b1d344db44d8b5358bed4feb53b2cefbb54baa3e Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Mon, 9 Aug 2021 12:37:34 -0400 Subject: [PATCH] Drop legacy validation logic for certificates API --- .../certificates/validation/validation.go | 63 ++-- .../validation/validation_test.go | 313 +++++++----------- .../certificates/certificates/strategy.go | 37 +-- .../certificates/strategy_test.go | 80 ++--- 4 files changed, 175 insertions(+), 318 deletions(-) diff --git a/pkg/apis/certificates/validation/validation.go b/pkg/apis/certificates/validation/validation.go index b019ac57cfd..a4e21860361 100644 --- a/pkg/apis/certificates/validation/validation.go +++ b/pkg/apis/certificates/validation/validation.go @@ -25,14 +25,12 @@ import ( 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" ) @@ -139,8 +137,8 @@ func ValidateCertificateRequestName(name string, prefix bool) []string { return nil } -func ValidateCertificateSigningRequestCreate(csr *certificates.CertificateSigningRequest, version schema.GroupVersion) field.ErrorList { - opts := getValidationOptions(version, csr, nil) +func ValidateCertificateSigningRequestCreate(csr *certificates.CertificateSigningRequest) field.ErrorList { + opts := getValidationOptions(csr, nil) return validateCertificateSigningRequest(csr, opts) } @@ -347,19 +345,19 @@ func ValidateCertificateSigningRequestSignerName(fldPath *field.Path, signerName return el } -func ValidateCertificateSigningRequestUpdate(newCSR, oldCSR *certificates.CertificateSigningRequest, version schema.GroupVersion) field.ErrorList { - opts := getValidationOptions(version, newCSR, oldCSR) +func ValidateCertificateSigningRequestUpdate(newCSR, oldCSR *certificates.CertificateSigningRequest) field.ErrorList { + opts := getValidationOptions(newCSR, oldCSR) return validateCertificateSigningRequestUpdate(newCSR, oldCSR, opts) } -func ValidateCertificateSigningRequestStatusUpdate(newCSR, oldCSR *certificates.CertificateSigningRequest, version schema.GroupVersion) field.ErrorList { - opts := getValidationOptions(version, newCSR, oldCSR) +func ValidateCertificateSigningRequestStatusUpdate(newCSR, oldCSR *certificates.CertificateSigningRequest) field.ErrorList { + opts := getValidationOptions(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) +func ValidateCertificateSigningRequestApprovalUpdate(newCSR, oldCSR *certificates.CertificateSigningRequest) field.ErrorList { + opts := getValidationOptions(newCSR, oldCSR) opts.allowSettingApprovalConditions = true return validateCertificateSigningRequestUpdate(newCSR, oldCSR, opts) } @@ -420,24 +418,19 @@ func findConditions(csr *certificates.CertificateSigningRequest, conditionType c // 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 { +func getValidationOptions(newCSR, oldCSR *certificates.CertificateSigningRequest) certificateValidationOptions { return certificateValidationOptions{ - allowResettingCertificate: allowResettingCertificate(version), + allowResettingCertificate: false, allowBothApprovedAndDenied: allowBothApprovedAndDenied(oldCSR), - allowLegacySignerName: allowLegacySignerName(version, oldCSR), - allowDuplicateConditionTypes: allowDuplicateConditionTypes(version, oldCSR), - allowEmptyConditionType: allowEmptyConditionType(version, oldCSR), - allowArbitraryCertificate: allowArbitraryCertificate(version, newCSR, oldCSR), - allowDuplicateUsages: allowDuplicateUsages(version, oldCSR), - allowUnknownUsages: allowUnknownUsages(version, oldCSR), + allowLegacySignerName: allowLegacySignerName(oldCSR), + allowDuplicateConditionTypes: allowDuplicateConditionTypes(oldCSR), + allowEmptyConditionType: allowEmptyConditionType(oldCSR), + allowArbitraryCertificate: allowArbitraryCertificate(newCSR, oldCSR), + allowDuplicateUsages: allowDuplicateUsages(oldCSR), + allowUnknownUsages: allowUnknownUsages(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 @@ -455,10 +448,8 @@ func allowBothApprovedAndDenied(oldCSR *certificates.CertificateSigningRequest) return approved && denied } -func allowLegacySignerName(version schema.GroupVersion, oldCSR *certificates.CertificateSigningRequest) bool { +func allowLegacySignerName(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: @@ -466,10 +457,8 @@ func allowLegacySignerName(version schema.GroupVersion, oldCSR *certificates.Cer } } -func allowDuplicateConditionTypes(version schema.GroupVersion, oldCSR *certificates.CertificateSigningRequest) bool { +func allowDuplicateConditionTypes(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: @@ -487,10 +476,8 @@ func hasDuplicateConditionTypes(csr *certificates.CertificateSigningRequest) boo return false } -func allowEmptyConditionType(version schema.GroupVersion, oldCSR *certificates.CertificateSigningRequest) bool { +func allowEmptyConditionType(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: @@ -506,10 +493,8 @@ func hasEmptyConditionType(csr *certificates.CertificateSigningRequest) bool { return false } -func allowArbitraryCertificate(version schema.GroupVersion, newCSR, oldCSR *certificates.CertificateSigningRequest) bool { +func allowArbitraryCertificate(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: @@ -519,10 +504,8 @@ func allowArbitraryCertificate(version schema.GroupVersion, newCSR, oldCSR *cert } } -func allowUnknownUsages(version schema.GroupVersion, oldCSR *certificates.CertificateSigningRequest) bool { +func allowUnknownUsages(oldCSR *certificates.CertificateSigningRequest) bool { switch { - case version == certificatesv1beta1.SchemeGroupVersion: - return true // compatibility with v1beta1 case oldCSR != nil && hasUnknownUsage(oldCSR.Spec.Usages): return true // compatibility with existing data default: @@ -539,10 +522,8 @@ func hasUnknownUsage(usages []certificates.KeyUsage) bool { return false } -func allowDuplicateUsages(version schema.GroupVersion, oldCSR *certificates.CertificateSigningRequest) bool { +func allowDuplicateUsages(oldCSR *certificates.CertificateSigningRequest) bool { switch { - case version == certificatesv1beta1.SchemeGroupVersion: - return true // compatibility with v1beta1 case oldCSR != nil && hasDuplicateUsage(oldCSR.Spec.Usages): return true // compatibility with existing data default: diff --git a/pkg/apis/certificates/validation/validation_test.go b/pkg/apis/certificates/validation/validation_test.go index 32cade47070..8ef876d895c 100644 --- a/pkg/apis/certificates/validation/validation_test.go +++ b/pkg/apis/certificates/validation/validation_test.go @@ -29,12 +29,10 @@ import ( "time" 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/client-go/util/certificate/csr" capi "k8s.io/kubernetes/pkg/apis/certificates" - capiv1beta1 "k8s.io/kubernetes/pkg/apis/certificates/v1beta1" "k8s.io/kubernetes/pkg/apis/core" "k8s.io/utils/pointer" ) @@ -54,7 +52,6 @@ func TestValidateCertificateSigningRequestCreate(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": { @@ -342,19 +339,7 @@ func TestValidateCertificateSigningRequestCreate(t *testing.T) { field.Required(specPath.Child("usages"), "usages must be provided"), }, }, - "unknown and duplicate usages - v1beta1": { - gv: schema.GroupVersion{Group: capi.SchemeGroupVersion.Group, Version: "v1beta1"}, - csr: capi.CertificateSigningRequest{ - ObjectMeta: validObjectMeta, - Spec: capi.CertificateSigningRequestSpec{ - Usages: []capi.KeyUsage{"unknown", "unknown"}, - Request: newCSRPEM(t), - SignerName: validSignerName, - }, - }, - }, - "unknown and duplicate usages - v1": { - gv: schema.GroupVersion{Group: capi.SchemeGroupVersion.Group, Version: "v1"}, + "unknown and duplicate usages": { csr: capi.CertificateSigningRequest{ ObjectMeta: validObjectMeta, Spec: capi.CertificateSigningRequestSpec{ @@ -372,7 +357,7 @@ func TestValidateCertificateSigningRequestCreate(t *testing.T) { } for name, test := range tests { t.Run(name, func(t *testing.T) { - el := ValidateCertificateSigningRequestCreate(&test.csr, test.gv) + el := ValidateCertificateSigningRequestCreate(&test.csr) if !reflect.DeepEqual(el, test.errs) { t.Errorf("returned and expected errors did not match - expected\n%v\nbut got\n%v", test.errs.ToAggregate(), el.ToAggregate()) } @@ -420,59 +405,23 @@ func newCSRPEM(t *testing.T) []byte { func Test_getValidationOptions(t *testing.T) { tests := []struct { - name string - version schema.GroupVersion - newCSR *capi.CertificateSigningRequest - oldCSR *capi.CertificateSigningRequest - want certificateValidationOptions + name string + newCSR *capi.CertificateSigningRequest + oldCSR *capi.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, - allowUnknownUsages: true, - allowDuplicateUsages: true, - }, + name: "strict create", + oldCSR: nil, + want: certificateValidationOptions{}, }, { - name: "v1 strict create", - version: schema.GroupVersion{Group: "certificates.k8s.io", Version: "v1"}, - oldCSR: nil, - want: certificateValidationOptions{}, + name: "strict update", + oldCSR: &capi.CertificateSigningRequest{}, + 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, - allowUnknownUsages: true, - allowDuplicateUsages: 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"}, + name: "compatible update, approved+denied", oldCSR: &capi.CertificateSigningRequest{Status: capi.CertificateSigningRequestStatus{ Conditions: []capi.CertificateSigningRequestCondition{{Type: capi.CertificateApproved}, {Type: capi.CertificateDenied}}, }}, @@ -481,16 +430,14 @@ func Test_getValidationOptions(t *testing.T) { }, }, { - name: "v1 compatible update, legacy signerName", - version: schema.GroupVersion{Group: "certificates.k8s.io", Version: "v1"}, - oldCSR: &capi.CertificateSigningRequest{Spec: capi.CertificateSigningRequestSpec{SignerName: capi.LegacyUnknownSignerName}}, + name: "compatible update, legacy signerName", + 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"}, + name: "compatible update, duplicate condition types", oldCSR: &capi.CertificateSigningRequest{Status: capi.CertificateSigningRequestStatus{ Conditions: []capi.CertificateSigningRequestCondition{{Type: capi.CertificateApproved}, {Type: capi.CertificateApproved}}, }}, @@ -499,8 +446,7 @@ func Test_getValidationOptions(t *testing.T) { }, }, { - name: "v1 compatible update, empty condition types", - version: schema.GroupVersion{Group: "certificates.k8s.io", Version: "v1"}, + name: "compatible update, empty condition types", oldCSR: &capi.CertificateSigningRequest{Status: capi.CertificateSigningRequestStatus{ Conditions: []capi.CertificateSigningRequestCondition{{}}, }}, @@ -509,8 +455,7 @@ func Test_getValidationOptions(t *testing.T) { }, }, { - name: "v1 compatible update, no diff to certificate", - version: schema.GroupVersion{Group: "certificates.k8s.io", Version: "v1"}, + name: "compatible update, no diff to certificate", newCSR: &capi.CertificateSigningRequest{Status: capi.CertificateSigningRequestStatus{ Certificate: validCertificate, }}, @@ -522,8 +467,7 @@ func Test_getValidationOptions(t *testing.T) { }, }, { - name: "v1 compatible update, existing invalid certificate", - version: schema.GroupVersion{Group: "certificates.k8s.io", Version: "v1"}, + name: "compatible update, existing invalid certificate", newCSR: &capi.CertificateSigningRequest{Status: capi.CertificateSigningRequestStatus{ Certificate: []byte(`new - no PEM blocks`), }}, @@ -535,17 +479,15 @@ func Test_getValidationOptions(t *testing.T) { }, }, { - name: "v1 compatible update, existing unknown usages", - version: schema.GroupVersion{Group: "certificates.k8s.io", Version: "v1"}, - oldCSR: &capi.CertificateSigningRequest{Spec: capi.CertificateSigningRequestSpec{Usages: []capi.KeyUsage{"unknown"}}}, + name: "compatible update, existing unknown usages", + oldCSR: &capi.CertificateSigningRequest{Spec: capi.CertificateSigningRequestSpec{Usages: []capi.KeyUsage{"unknown"}}}, want: certificateValidationOptions{ allowUnknownUsages: true, }, }, { - name: "v1 compatible update, existing duplicate usages", - version: schema.GroupVersion{Group: "certificates.k8s.io", Version: "v1"}, - oldCSR: &capi.CertificateSigningRequest{Spec: capi.CertificateSigningRequestSpec{Usages: []capi.KeyUsage{"any", "any"}}}, + name: "compatible update, existing duplicate usages", + oldCSR: &capi.CertificateSigningRequest{Spec: capi.CertificateSigningRequestSpec{Usages: []capi.KeyUsage{"any", "any"}}}, want: certificateValidationOptions{ allowDuplicateUsages: true, }, @@ -553,7 +495,7 @@ func Test_getValidationOptions(t *testing.T) { } 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) { + if got := getValidationOptions(tt.newCSR, tt.oldCSR); !reflect.DeepEqual(got, tt.want) { t.Errorf("got %#v\nwant %#v", got, tt.want) } }) @@ -574,10 +516,10 @@ func TestValidateCertificateSigningRequestUpdate(t *testing.T) { } tests := []struct { - name string - newCSR *capi.CertificateSigningRequest - oldCSR *capi.CertificateSigningRequest - versionErrs map[string][]string + name string + newCSR *capi.CertificateSigningRequest + oldCSR *capi.CertificateSigningRequest + errs []string }{ { name: "no-op", @@ -595,9 +537,8 @@ func TestValidateCertificateSigningRequestUpdate(t *testing.T) { 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"`}, + errs: []string{ + `status.conditions: Forbidden: updates may not add a condition of type "Approved"`, }, }, { @@ -606,9 +547,8 @@ func TestValidateCertificateSigningRequestUpdate(t *testing.T) { 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"`}, + errs: []string{ + `status.conditions: Forbidden: updates may not remove a condition of type "Approved"`, }, }, { @@ -617,9 +557,8 @@ func TestValidateCertificateSigningRequestUpdate(t *testing.T) { 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"`}, + errs: []string{ + `status.conditions: Forbidden: updates may not add a condition of type "Denied"`, }, }, { @@ -628,9 +567,8 @@ func TestValidateCertificateSigningRequestUpdate(t *testing.T) { 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"`}, + errs: []string{ + `status.conditions: Forbidden: updates may not remove a condition of type "Denied"`, }, }, { @@ -638,8 +576,8 @@ func TestValidateCertificateSigningRequestUpdate(t *testing.T) { 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{}, + oldCSR: &capi.CertificateSigningRequest{ObjectMeta: validUpdateMetaWithFinalizers, Spec: validSpec}, + errs: []string{}, }, { name: "remove Failed condition", @@ -647,9 +585,8 @@ func TestValidateCertificateSigningRequestUpdate(t *testing.T) { 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"`}, + errs: []string{ + `status.conditions: Forbidden: updates may not remove a condition of type "Failed"`, }, }, { @@ -658,29 +595,26 @@ func TestValidateCertificateSigningRequestUpdate(t *testing.T) { 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`}, + errs: []string{ + `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: capi.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) - } - }) - } + t.Run(tt.name, func(t *testing.T) { + gotErrs := sets.NewString() + for _, err := range ValidateCertificateSigningRequestUpdate(tt.newCSR, tt.oldCSR) { + gotErrs.Insert(err.Error()) + } + wantErrs := sets.NewString(tt.errs...) + 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) + } + }) } } @@ -698,10 +632,10 @@ func TestValidateCertificateSigningRequestStatusUpdate(t *testing.T) { } tests := []struct { - name string - newCSR *capi.CertificateSigningRequest - oldCSR *capi.CertificateSigningRequest - versionErrs map[string][]string + name string + newCSR *capi.CertificateSigningRequest + oldCSR *capi.CertificateSigningRequest + errs []string }{ { name: "no-op", @@ -732,9 +666,8 @@ func TestValidateCertificateSigningRequestStatusUpdate(t *testing.T) { 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"`}, + errs: []string{ + `status.conditions: Forbidden: updates may not add a condition of type "Approved"`, }, }, { @@ -743,9 +676,8 @@ func TestValidateCertificateSigningRequestStatusUpdate(t *testing.T) { 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"`}, + errs: []string{ + `status.conditions: Forbidden: updates may not remove a condition of type "Approved"`, }, }, { @@ -754,9 +686,8 @@ func TestValidateCertificateSigningRequestStatusUpdate(t *testing.T) { 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"`}, + errs: []string{ + `status.conditions: Forbidden: updates may not add a condition of type "Denied"`, }, }, { @@ -765,9 +696,8 @@ func TestValidateCertificateSigningRequestStatusUpdate(t *testing.T) { 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"`}, + errs: []string{ + `status.conditions: Forbidden: updates may not remove a condition of type "Denied"`, }, }, { @@ -775,8 +705,8 @@ func TestValidateCertificateSigningRequestStatusUpdate(t *testing.T) { 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{}, + oldCSR: &capi.CertificateSigningRequest{ObjectMeta: validUpdateMetaWithFinalizers, Spec: validSpec}, + errs: []string{}, }, { name: "remove Failed condition", @@ -784,9 +714,8 @@ func TestValidateCertificateSigningRequestStatusUpdate(t *testing.T) { 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"`}, + errs: []string{ + `status.conditions: Forbidden: updates may not remove a condition of type "Failed"`, }, }, { @@ -794,8 +723,8 @@ func TestValidateCertificateSigningRequestStatusUpdate(t *testing.T) { newCSR: &capi.CertificateSigningRequest{ObjectMeta: validUpdateMeta, Spec: validSpec, Status: capi.CertificateSigningRequestStatus{ Certificate: validCertificate, }}, - oldCSR: &capi.CertificateSigningRequest{ObjectMeta: validUpdateMetaWithFinalizers, Spec: validSpec}, - versionErrs: map[string][]string{}, + oldCSR: &capi.CertificateSigningRequest{ObjectMeta: validUpdateMetaWithFinalizers, Spec: validSpec}, + errs: []string{}, }, { name: "set invalid certificate", @@ -803,8 +732,8 @@ func TestValidateCertificateSigningRequestStatusUpdate(t *testing.T) { 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`}, + errs: []string{ + `status.certificate: Invalid value: "": must contain at least one CERTIFICATE PEM block`, }, }, { @@ -815,28 +744,26 @@ func TestValidateCertificateSigningRequestStatusUpdate(t *testing.T) { 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`}, + errs: []string{ + `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: capi.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) - } - }) - } + t.Run(tt.name, func(t *testing.T) { + gotErrs := sets.NewString() + for _, err := range ValidateCertificateSigningRequestStatusUpdate(tt.newCSR, tt.oldCSR) { + gotErrs.Insert(err.Error()) + } + wantErrs := sets.NewString(tt.errs...) + 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) + } + }) } } @@ -854,10 +781,10 @@ func TestValidateCertificateSigningRequestApprovalUpdate(t *testing.T) { } tests := []struct { - name string - newCSR *capi.CertificateSigningRequest - oldCSR *capi.CertificateSigningRequest - versionErrs map[string][]string + name string + newCSR *capi.CertificateSigningRequest + oldCSR *capi.CertificateSigningRequest + errs []string }{ { name: "no-op", @@ -882,9 +809,8 @@ func TestValidateCertificateSigningRequestApprovalUpdate(t *testing.T) { 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"`}, + errs: []string{ + `status.conditions: Forbidden: updates may not remove a condition of type "Approved"`, }, }, { @@ -900,9 +826,8 @@ func TestValidateCertificateSigningRequestApprovalUpdate(t *testing.T) { 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"`}, + errs: []string{ + `status.conditions: Forbidden: updates may not remove a condition of type "Denied"`, }, }, { @@ -910,8 +835,8 @@ func TestValidateCertificateSigningRequestApprovalUpdate(t *testing.T) { 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{}, + oldCSR: &capi.CertificateSigningRequest{ObjectMeta: validUpdateMetaWithFinalizers, Spec: validSpec}, + errs: []string{}, }, { name: "remove Failed condition", @@ -919,9 +844,8 @@ func TestValidateCertificateSigningRequestApprovalUpdate(t *testing.T) { 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"`}, + errs: []string{ + `status.conditions: Forbidden: updates may not remove a condition of type "Failed"`, }, }, { @@ -930,29 +854,26 @@ func TestValidateCertificateSigningRequestApprovalUpdate(t *testing.T) { 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`}, + errs: []string{ + `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: capi.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) - } - }) - } + t.Run(tt.name, func(t *testing.T) { + gotErrs := sets.NewString() + for _, err := range ValidateCertificateSigningRequestApprovalUpdate(tt.newCSR, tt.oldCSR) { + gotErrs.Insert(err.Error()) + } + wantErrs := sets.NewString(tt.errs...) + 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) + } + }) } } diff --git a/pkg/registry/certificates/certificates/strategy.go b/pkg/registry/certificates/certificates/strategy.go index 41281ee177d..95450358e6a 100644 --- a/pkg/registry/certificates/certificates/strategy.go +++ b/pkg/registry/certificates/certificates/strategy.go @@ -20,12 +20,10 @@ 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" @@ -120,7 +118,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.ValidateCertificateSigningRequestCreate(csr, requestGroupVersion(ctx)) + return validation.ValidateCertificateSigningRequestCreate(csr) } // WarningsOnCreate returns warnings for the creation of the given object. @@ -133,7 +131,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, requestGroupVersion(ctx)) + return validation.ValidateCertificateSigningRequestUpdate(newCSR, oldCSR) } // WarningsOnUpdate returns warnings for the given update. @@ -181,20 +179,11 @@ func (csrStatusStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime. // Updating /status should not modify spec newCSR.Spec = oldCSR.Spec - 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) - } + // 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) } @@ -255,7 +244,7 @@ func populateConditionTimestamps(newCSR, oldCSR *certificates.CertificateSigning } func (csrStatusStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) field.ErrorList { - return validation.ValidateCertificateSigningRequestStatusUpdate(obj.(*certificates.CertificateSigningRequest), old.(*certificates.CertificateSigningRequest), requestGroupVersion(ctx)) + return validation.ValidateCertificateSigningRequestStatusUpdate(obj.(*certificates.CertificateSigningRequest), old.(*certificates.CertificateSigningRequest)) } // WarningsOnUpdate returns warnings for the given update. @@ -307,7 +296,7 @@ func (csrApprovalStrategy) PrepareForUpdate(ctx context.Context, obj, old runtim } func (csrApprovalStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) field.ErrorList { - return validation.ValidateCertificateSigningRequestApprovalUpdate(obj.(*certificates.CertificateSigningRequest), old.(*certificates.CertificateSigningRequest), requestGroupVersion(ctx)) + return validation.ValidateCertificateSigningRequestApprovalUpdate(obj.(*certificates.CertificateSigningRequest), old.(*certificates.CertificateSigningRequest)) } // WarningsOnUpdate returns warnings for the given update. @@ -332,11 +321,3 @@ 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 fb1318d3ea5..c34c0af9b31 100644 --- a/pkg/registry/certificates/certificates/strategy_test.go +++ b/pkg/registry/certificates/certificates/strategy_test.go @@ -174,19 +174,16 @@ func TestStatusUpdate(t *testing.T) { }() tests := []struct { - name string - newObj *certapi.CertificateSigningRequest - oldObj *certapi.CertificateSigningRequest - expectedObjs map[string]*certapi.CertificateSigningRequest + name string + newObj *certapi.CertificateSigningRequest + oldObj *certapi.CertificateSigningRequest + expectedObj *certapi.CertificateSigningRequest }{ { - name: "no-op", - newObj: &certapi.CertificateSigningRequest{}, - oldObj: &certapi.CertificateSigningRequest{}, - expectedObjs: map[string]*certapi.CertificateSigningRequest{ - "v1": {}, - "v1beta1": {}, - }, + name: "no-op", + newObj: &certapi.CertificateSigningRequest{}, + oldObj: &certapi.CertificateSigningRequest{}, + expectedObj: &certapi.CertificateSigningRequest{}, }, { name: "adding failed condition to existing approved/denied conditions", @@ -211,29 +208,15 @@ func TestStatusUpdate(t *testing.T) { }, }, }, - expectedObjs: map[string]*certapi.CertificateSigningRequest{ + expectedObj: &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"}, - }, + 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"}, }, }, }, @@ -248,33 +231,24 @@ func TestStatusUpdate(t *testing.T) { }, }, oldObj: &certapi.CertificateSigningRequest{}, - expectedObjs: map[string]*certapi.CertificateSigningRequest{ + expectedObj: &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}, - }, + 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])) - } - }) - } + t.Run(tt.name, func(t *testing.T) { + obj := tt.newObj.DeepCopy() + StatusStrategy.PrepareForUpdate(context.TODO(), obj, tt.oldObj.DeepCopy()) + if !reflect.DeepEqual(obj, tt.expectedObj) { + t.Errorf("object diff: %s", diff.ObjectDiff(obj, tt.expectedObj)) + } + }) } }