diff --git a/pkg/controller/certificaterequests/venafi/BUILD.bazel b/pkg/controller/certificaterequests/venafi/BUILD.bazel index d29cd71b6..44a4b3c5e 100644 --- a/pkg/controller/certificaterequests/venafi/BUILD.bazel +++ b/pkg/controller/certificaterequests/venafi/BUILD.bazel @@ -14,6 +14,7 @@ go_library( "//pkg/controller/certificaterequests/util:go_default_library", "//pkg/issuer:go_default_library", "//pkg/issuer/venafi/client:go_default_library", + "//pkg/issuer/venafi/client/api:go_default_library", "//pkg/logs:go_default_library", "@com_github_venafi_vcert//pkg/endpoint:go_default_library", "@io_k8s_apimachinery//pkg/api/errors:go_default_library", @@ -48,6 +49,7 @@ go_test( "//pkg/controller/certificaterequests:go_default_library", "//pkg/controller/test:go_default_library", "//pkg/issuer/venafi/client:go_default_library", + "//pkg/issuer/venafi/client/api:go_default_library", "//pkg/issuer/venafi/client/fake:go_default_library", "//pkg/util/pki:go_default_library", "//test/unit/gen:go_default_library", diff --git a/pkg/controller/certificaterequests/venafi/venafi.go b/pkg/controller/certificaterequests/venafi/venafi.go index 7b5cd7943..3d020efac 100644 --- a/pkg/controller/certificaterequests/venafi/venafi.go +++ b/pkg/controller/certificaterequests/venafi/venafi.go @@ -23,6 +23,8 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "github.com/jetstack/cert-manager/pkg/issuer/venafi/client/api" + clientset "github.com/jetstack/cert-manager/pkg/client/clientset/versioned" venaficlient "github.com/jetstack/cert-manager/pkg/issuer/venafi/client" @@ -97,7 +99,7 @@ func (v *Venafi) Sign(ctx context.Context, cr *cmapi.CertificateRequest, issuerO duration := apiutil.DefaultCertDuration(cr.Spec.Duration) - var customFields []venaficlient.CustomField + var customFields []api.CustomField if annotation, exists := cr.GetAnnotations()[cmapi.VenafiCustomFieldsAnnotationKey]; exists && annotation != "" { err := json.Unmarshal([]byte(annotation), &customFields) if err != nil { @@ -135,13 +137,17 @@ func (v *Venafi) Sign(ctx context.Context, cr *cmapi.CertificateRequest, issuerO } } + v.reporter.Pending(cr, err, "IssuancePending", "Venafi certificate is requested") + + if cr.ObjectMeta.Annotations == nil { + cr.ObjectMeta.Annotations = map[string]string{} + } cr.ObjectMeta.Annotations[VenafiPickupIDAnnotation] = pickupID _, err = v.cmClient.CertmanagerV1alpha2().CertificateRequests(cr.GetNamespace()).Update(ctx, cr, metav1.UpdateOptions{}) if err != nil { return nil, err } - v.reporter.Pending(cr, err, "IssuancePending", "Venafi certificate is requested") return nil, nil } diff --git a/pkg/controller/certificaterequests/venafi/venafi_test.go b/pkg/controller/certificaterequests/venafi/venafi_test.go index 3da400e73..f377f7e22 100644 --- a/pkg/controller/certificaterequests/venafi/venafi_test.go +++ b/pkg/controller/certificaterequests/venafi/venafi_test.go @@ -27,6 +27,8 @@ import ( "testing" "time" + "github.com/jetstack/cert-manager/pkg/issuer/venafi/client/api" + "github.com/jetstack/cert-manager/pkg/issuer/venafi/client" "github.com/Venafi/vcert/pkg/endpoint" @@ -184,43 +186,45 @@ func TestSign(t *testing.T) { } clientReturnsPending := &internalvenafifake.Venafi{ - SignFn: func([]byte, time.Duration, []client.CustomField) ([]byte, error) { + RequestCertificateFn: func(csrPEM []byte, duration time.Duration, customFields []api.CustomField) (string, error) { + return "test", nil + }, + RetreiveCertificateFn: func(string, []byte, time.Duration, []api.CustomField) ([]byte, error) { return nil, endpoint.ErrCertificatePending{ CertificateID: "test-cert-id", Status: "test-status-pending", } }, } - clientReturnsTimeout := &internalvenafifake.Venafi{ - SignFn: func([]byte, time.Duration, []client.CustomField) ([]byte, error) { - return nil, endpoint.ErrRetrieveCertificateTimeout{ - CertificateID: "test-cert-id", - } - }, - } clientReturnsGenericError := &internalvenafifake.Venafi{ - SignFn: func([]byte, time.Duration, []client.CustomField) ([]byte, error) { - return nil, errors.New("this is an error") + RequestCertificateFn: func(csrPEM []byte, duration time.Duration, customFields []api.CustomField) (string, error) { + return "", errors.New("this is an error") }, } clientReturnsCert := &internalvenafifake.Venafi{ - SignFn: func([]byte, time.Duration, []client.CustomField) ([]byte, error) { + RequestCertificateFn: func(csrPEM []byte, duration time.Duration, customFields []api.CustomField) (string, error) { + return "test", nil + }, + RetreiveCertificateFn: func(string, []byte, time.Duration, []api.CustomField) ([]byte, error) { return certPEM, nil }, } clientReturnsCertIfCustomField := &internalvenafifake.Venafi{ - SignFn: func(csr []byte, t time.Duration, fields []client.CustomField) ([]byte, error) { + RequestCertificateFn: func(csrPEM []byte, duration time.Duration, fields []api.CustomField) (string, error) { if len(fields) > 0 && fields[0].Name == "cert-manager-test" && fields[0].Value == "test ok" { - return certPEM, nil + return "test", nil } - return nil, errors.New("Custom field not set") + return "", errors.New("Custom field not set") + }, + RetreiveCertificateFn: func(string, []byte, time.Duration, []api.CustomField) ([]byte, error) { + return certPEM, nil }, } clientReturnsInvalidCustomFieldType := &internalvenafifake.Venafi{ - SignFn: func(csr []byte, t time.Duration, fields []client.CustomField) ([]byte, error) { - return nil, client.ErrCustomFieldsType{Type: fields[0].Type} + RequestCertificateFn: func(csrPEM []byte, duration time.Duration, fields []api.CustomField) (string, error) { + return "", client.ErrCustomFieldsType{Type: fields[0].Type} }, } @@ -367,9 +371,40 @@ func TestSign(t *testing.T) { KubeObjects: []runtime.Object{tppSecret}, CertManagerObjects: []runtime.Object{cloudCR.DeepCopy(), tppIssuer.DeepCopy()}, ExpectedEvents: []string{ + "Normal IssuancePending Venafi certificate is requested", "Normal IssuancePending Venafi certificate still in a pending state, the request will be retried: Issuance is pending. You may try retrieving the certificate later using Pickup ID: test-cert-id\n\tStatus: test-status-pending", }, ExpectedActions: []testpkg.Action{ + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( + cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + "status", + gen.DefaultTestNamespace, + gen.CertificateRequestFrom(tppCR, + gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ + Type: cmapi.CertificateRequestConditionReady, + Status: cmmeta.ConditionFalse, + Reason: cmapi.CertificateRequestReasonPending, + Message: "Venafi certificate is requested", + LastTransitionTime: &metaFixedClockStart, + }), + gen.AddCertificateRequestAnnotations(map[string]string{VenafiPickupIDAnnotation: "test"}), + ), + )), + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( + cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + "", + gen.DefaultTestNamespace, + gen.CertificateRequestFrom(tppCR, + gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ + Type: cmapi.CertificateRequestConditionReady, + Status: cmmeta.ConditionFalse, + Reason: cmapi.CertificateRequestReasonPending, + Message: "Venafi certificate is requested", + LastTransitionTime: &metaFixedClockStart, + }), + gen.AddCertificateRequestAnnotations(map[string]string{VenafiPickupIDAnnotation: "test"}), + ), + )), testpkg.NewAction(coretesting.NewUpdateSubresourceAction( cmapi.SchemeGroupVersion.WithResource("certificaterequests"), "status", @@ -382,6 +417,7 @@ func TestSign(t *testing.T) { Message: "Venafi certificate still in a pending state, the request will be retried: Issuance is pending. You may try retrieving the certificate later using Pickup ID: test-cert-id\n\tStatus: test-status-pending", LastTransitionTime: &metaFixedClockStart, }), + gen.AddCertificateRequestAnnotations(map[string]string{VenafiPickupIDAnnotation: "test"}), ), )), }, @@ -396,9 +432,40 @@ func TestSign(t *testing.T) { KubeObjects: []runtime.Object{cloudSecret}, CertManagerObjects: []runtime.Object{cloudCR.DeepCopy(), cloudIssuer.DeepCopy()}, ExpectedEvents: []string{ + "Normal IssuancePending Venafi certificate is requested", "Normal IssuancePending Venafi certificate still in a pending state, the request will be retried: Issuance is pending. You may try retrieving the certificate later using Pickup ID: test-cert-id\n\tStatus: test-status-pending", }, ExpectedActions: []testpkg.Action{ + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( + cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + "status", + gen.DefaultTestNamespace, + gen.CertificateRequestFrom(cloudCR, + gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ + Type: cmapi.CertificateRequestConditionReady, + Status: cmmeta.ConditionFalse, + Reason: cmapi.CertificateRequestReasonPending, + Message: "Venafi certificate is requested", + LastTransitionTime: &metaFixedClockStart, + }), + gen.AddCertificateRequestAnnotations(map[string]string{VenafiPickupIDAnnotation: "test"}), + ), + )), + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( + cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + "", + gen.DefaultTestNamespace, + gen.CertificateRequestFrom(cloudCR, + gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ + Type: cmapi.CertificateRequestConditionReady, + Status: cmmeta.ConditionFalse, + Reason: cmapi.CertificateRequestReasonPending, + Message: "Venafi certificate is requested", + LastTransitionTime: &metaFixedClockStart, + }), + gen.AddCertificateRequestAnnotations(map[string]string{VenafiPickupIDAnnotation: "test"}), + ), + )), testpkg.NewAction(coretesting.NewUpdateSubresourceAction( cmapi.SchemeGroupVersion.WithResource("certificaterequests"), "status", @@ -411,6 +478,7 @@ func TestSign(t *testing.T) { Message: "Venafi certificate still in a pending state, the request will be retried: Issuance is pending. You may try retrieving the certificate later using Pickup ID: test-cert-id\n\tStatus: test-status-pending", LastTransitionTime: &metaFixedClockStart, }), + gen.AddCertificateRequestAnnotations(map[string]string{VenafiPickupIDAnnotation: "test"}), ), )), }, @@ -419,71 +487,13 @@ func TestSign(t *testing.T) { fakeClient: clientReturnsPending, expectedErr: true, }, - "tpp: if sign returns timeout error then set failed and return nil": { - certificateRequest: tppCR.DeepCopy(), - builder: &controllertest.Builder{ - CertManagerObjects: []runtime.Object{tppCR.DeepCopy(), tppIssuer.DeepCopy()}, - KubeObjects: []runtime.Object{tppSecret}, - ExpectedEvents: []string{ - "Warning Timeout Timed out waiting for venafi certificate, the request will be retried: Operation timed out. You may try retrieving the certificate later using Pickup ID: test-cert-id", - }, - ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateSubresourceAction( - cmapi.SchemeGroupVersion.WithResource("certificaterequests"), - "status", - gen.DefaultTestNamespace, - gen.CertificateRequestFrom(tppCR, - gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ - Type: cmapi.CertificateRequestConditionReady, - Status: cmmeta.ConditionFalse, - Reason: cmapi.CertificateRequestReasonFailed, - Message: "Timed out waiting for venafi certificate, the request will be retried: Operation timed out. You may try retrieving the certificate later using Pickup ID: test-cert-id", - LastTransitionTime: &metaFixedClockStart, - }), - gen.SetCertificateRequestFailureTime(metaFixedClockStart), - ), - )), - }, - }, - fakeSecretLister: failGetSecretLister, - fakeClient: clientReturnsTimeout, - }, - "cloud: if sign returns timeout error then set failed and return nil": { - certificateRequest: cloudCR.DeepCopy(), - builder: &controllertest.Builder{ - KubeObjects: []runtime.Object{cloudSecret}, - CertManagerObjects: []runtime.Object{cloudCR.DeepCopy(), cloudIssuer.DeepCopy()}, - ExpectedEvents: []string{ - "Warning Timeout Timed out waiting for venafi certificate, the request will be retried: Operation timed out. You may try retrieving the certificate later using Pickup ID: test-cert-id", - }, - ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateSubresourceAction( - cmapi.SchemeGroupVersion.WithResource("certificaterequests"), - "status", - gen.DefaultTestNamespace, - gen.CertificateRequestFrom(cloudCR, - gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ - Type: cmapi.CertificateRequestConditionReady, - Status: cmmeta.ConditionFalse, - Reason: cmapi.CertificateRequestReasonFailed, - Message: "Timed out waiting for venafi certificate, the request will be retried: Operation timed out. You may try retrieving the certificate later using Pickup ID: test-cert-id", - LastTransitionTime: &metaFixedClockStart, - }), - gen.SetCertificateRequestFailureTime(metaFixedClockStart), - ), - )), - }, - }, - fakeSecretLister: failGetSecretLister, - fakeClient: clientReturnsTimeout, - }, "tpp: if sign returns generic error then set pending and return error": { certificateRequest: tppCR.DeepCopy(), builder: &controllertest.Builder{ KubeObjects: []runtime.Object{tppSecret}, CertManagerObjects: []runtime.Object{tppCR.DeepCopy(), tppIssuer.DeepCopy()}, ExpectedEvents: []string{ - "Warning RetrieveError Failed to obtain venafi certificate: this is an error", + "Warning RequestError Failed to request venafi certificate: this is an error", }, ExpectedActions: []testpkg.Action{ testpkg.NewAction(coretesting.NewUpdateSubresourceAction( @@ -495,7 +505,7 @@ func TestSign(t *testing.T) { Type: cmapi.CertificateRequestConditionReady, Status: cmmeta.ConditionFalse, Reason: cmapi.CertificateRequestReasonFailed, - Message: "Failed to obtain venafi certificate: this is an error", + Message: "Failed to request venafi certificate: this is an error", LastTransitionTime: &metaFixedClockStart, }), gen.SetCertificateRequestFailureTime(metaFixedClockStart), @@ -503,9 +513,10 @@ func TestSign(t *testing.T) { )), }, }, - fakeSecretLister: failGetSecretLister, - fakeClient: clientReturnsGenericError, - expectedErr: true, + fakeSecretLister: failGetSecretLister, + fakeClient: clientReturnsGenericError, + expectedErr: true, + skipSecondSignCall: false, }, "cloud: if sign returns generic error then set pending and return error": { certificateRequest: cloudCR.DeepCopy(), @@ -513,7 +524,7 @@ func TestSign(t *testing.T) { KubeObjects: []runtime.Object{cloudSecret}, CertManagerObjects: []runtime.Object{tppCR.DeepCopy(), cloudIssuer.DeepCopy()}, ExpectedEvents: []string{ - "Warning RetrieveError Failed to obtain venafi certificate: this is an error", + "Warning RequestError Failed to request venafi certificate: this is an error", }, ExpectedActions: []testpkg.Action{ testpkg.NewAction(coretesting.NewUpdateSubresourceAction( @@ -525,7 +536,7 @@ func TestSign(t *testing.T) { Type: cmapi.CertificateRequestConditionReady, Status: cmmeta.ConditionFalse, Reason: cmapi.CertificateRequestReasonFailed, - Message: "Failed to obtain venafi certificate: this is an error", + Message: "Failed to request venafi certificate: this is an error", LastTransitionTime: &metaFixedClockStart, }), gen.SetCertificateRequestFailureTime(metaFixedClockStart), @@ -533,9 +544,10 @@ func TestSign(t *testing.T) { )), }, }, - fakeSecretLister: failGetSecretLister, - fakeClient: clientReturnsGenericError, - expectedErr: true, + fakeSecretLister: failGetSecretLister, + fakeClient: clientReturnsGenericError, + expectedErr: true, + skipSecondSignCall: false, }, "tpp: if sign returns cert then return cert and not failed": { certificateRequest: tppCR.DeepCopy(), @@ -543,9 +555,40 @@ func TestSign(t *testing.T) { KubeObjects: []runtime.Object{tppSecret}, CertManagerObjects: []runtime.Object{tppCR.DeepCopy(), tppIssuer.DeepCopy()}, ExpectedEvents: []string{ + "Normal IssuancePending Venafi certificate is requested", "Normal CertificateIssued Certificate fetched from issuer successfully", }, ExpectedActions: []testpkg.Action{ + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( + cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + "status", + gen.DefaultTestNamespace, + gen.CertificateRequestFrom(tppCR, + gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ + Type: cmapi.CertificateRequestConditionReady, + Status: cmmeta.ConditionFalse, + Reason: cmapi.CertificateRequestReasonPending, + Message: "Venafi certificate is requested", + LastTransitionTime: &metaFixedClockStart, + }), + gen.AddCertificateRequestAnnotations(map[string]string{VenafiPickupIDAnnotation: "test"}), + ), + )), + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( + cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + "", + gen.DefaultTestNamespace, + gen.CertificateRequestFrom(tppCR, + gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ + Type: cmapi.CertificateRequestConditionReady, + Status: cmmeta.ConditionFalse, + Reason: cmapi.CertificateRequestReasonPending, + Message: "Venafi certificate is requested", + LastTransitionTime: &metaFixedClockStart, + }), + gen.AddCertificateRequestAnnotations(map[string]string{VenafiPickupIDAnnotation: "test"}), + ), + )), testpkg.NewAction(coretesting.NewUpdateSubresourceAction( cmapi.SchemeGroupVersion.WithResource("certificaterequests"), "status", @@ -559,6 +602,7 @@ func TestSign(t *testing.T) { LastTransitionTime: &metaFixedClockStart, }), gen.SetCertificateRequestCertificate(certPEM), + gen.AddCertificateRequestAnnotations(map[string]string{VenafiPickupIDAnnotation: "test"}), ), )), }, @@ -572,9 +616,40 @@ func TestSign(t *testing.T) { KubeObjects: []runtime.Object{cloudSecret}, CertManagerObjects: []runtime.Object{cloudCR.DeepCopy(), cloudIssuer.DeepCopy()}, ExpectedEvents: []string{ + `Normal IssuancePending Venafi certificate is requested`, "Normal CertificateIssued Certificate fetched from issuer successfully", }, ExpectedActions: []testpkg.Action{ + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( + cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + "status", + gen.DefaultTestNamespace, + gen.CertificateRequestFrom(cloudCR, + gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ + Type: cmapi.CertificateRequestConditionReady, + Status: cmmeta.ConditionFalse, + Reason: cmapi.CertificateRequestReasonPending, + Message: "Venafi certificate is requested", + LastTransitionTime: &metaFixedClockStart, + }), + gen.AddCertificateRequestAnnotations(map[string]string{VenafiPickupIDAnnotation: "test"}), + ), + )), + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( + cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + "", + gen.DefaultTestNamespace, + gen.CertificateRequestFrom(cloudCR, + gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ + Type: cmapi.CertificateRequestConditionReady, + Status: cmmeta.ConditionFalse, + Reason: cmapi.CertificateRequestReasonPending, + Message: "Venafi certificate is requested", + LastTransitionTime: &metaFixedClockStart, + }), + gen.AddCertificateRequestAnnotations(map[string]string{VenafiPickupIDAnnotation: "test"}), + ), + )), testpkg.NewAction(coretesting.NewUpdateSubresourceAction( cmapi.SchemeGroupVersion.WithResource("certificaterequests"), "status", @@ -588,6 +663,7 @@ func TestSign(t *testing.T) { LastTransitionTime: &metaFixedClockStart, }), gen.SetCertificateRequestCertificate(certPEM), + gen.AddCertificateRequestAnnotations(map[string]string{VenafiPickupIDAnnotation: "test"}), ), )), }, @@ -601,9 +677,40 @@ func TestSign(t *testing.T) { builder: &controllertest.Builder{ CertManagerObjects: []runtime.Object{tppCRWithCustomFields.DeepCopy(), tppIssuer.DeepCopy()}, ExpectedEvents: []string{ - `Normal CertificateIssued Certificate fetched from issuer successfully`, + "Normal IssuancePending Venafi certificate is requested", + "Normal CertificateIssued Certificate fetched from issuer successfully", }, ExpectedActions: []testpkg.Action{ + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( + cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + "status", + gen.DefaultTestNamespace, + gen.CertificateRequestFrom(tppCRWithCustomFields, + gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ + Type: cmapi.CertificateRequestConditionReady, + Status: cmmeta.ConditionFalse, + Reason: cmapi.CertificateRequestReasonPending, + Message: "Venafi certificate is requested", + LastTransitionTime: &metaFixedClockStart, + }), + gen.AddCertificateRequestAnnotations(map[string]string{VenafiPickupIDAnnotation: "test"}), + ), + )), + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( + cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + "", + gen.DefaultTestNamespace, + gen.CertificateRequestFrom(tppCRWithCustomFields, + gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ + Type: cmapi.CertificateRequestConditionReady, + Status: cmmeta.ConditionFalse, + Reason: cmapi.CertificateRequestReasonPending, + Message: "Venafi certificate is requested", + LastTransitionTime: &metaFixedClockStart, + }), + gen.AddCertificateRequestAnnotations(map[string]string{VenafiPickupIDAnnotation: "test"}), + ), + )), testpkg.NewAction(coretesting.NewUpdateSubresourceAction( cmapi.SchemeGroupVersion.WithResource("certificaterequests"), "status", @@ -617,6 +724,7 @@ func TestSign(t *testing.T) { LastTransitionTime: &metaFixedClockStart, }), gen.SetCertificateRequestCertificate(certPEM), + gen.AddCertificateRequestAnnotations(map[string]string{VenafiPickupIDAnnotation: "test"}), ), )), }, @@ -651,9 +759,10 @@ func TestSign(t *testing.T) { )), }, }, - fakeSecretLister: failGetSecretLister, - fakeClient: clientReturnsPending, - expectedErr: false, + fakeSecretLister: failGetSecretLister, + fakeClient: clientReturnsPending, + skipSecondSignCall: true, + expectedErr: false, }, "annotations: Error on invalid type in custom fields": { certificateRequest: tppCRWithInvalidCustomFieldType.DeepCopy(), @@ -688,6 +797,9 @@ func TestSign(t *testing.T) { } for name, test := range tests { + if name != "tpp: if sign returns pending error then set pending and return err" { + //continue + } t.Run(name, func(t *testing.T) { fixedClock.SetTime(fixedClockStart) test.builder.Clock = fixedClock @@ -705,6 +817,8 @@ type testT struct { expectedErr bool + skipSecondSignCall bool + fakeSecretLister *testlisters.FakeSecretLister } @@ -732,6 +846,17 @@ func runTest(t *testing.T, test testT) { // Deep copy the certificate request to prevent pulling condition state across tests err := controller.Sync(context.Background(), test.certificateRequest) + + if err == nil && test.fakeClient != nil && test.fakeClient.RetreiveCertificateFn != nil && !test.skipSecondSignCall { + // request state is ok! simulating a 2nd sync to fetch the cert + + if test.certificateRequest.ObjectMeta.Annotations == nil { + test.certificateRequest.ObjectMeta.Annotations = map[string]string{} + } + test.certificateRequest.ObjectMeta.Annotations[VenafiPickupIDAnnotation] = "test" + err = controller.Sync(context.Background(), test.certificateRequest) + } + if err != nil && !test.expectedErr { t.Errorf("expected to not get an error, but got: %v", err) } diff --git a/pkg/issuer/venafi/client/BUILD.bazel b/pkg/issuer/venafi/client/BUILD.bazel index 876854d67..1d13afa78 100644 --- a/pkg/issuer/venafi/client/BUILD.bazel +++ b/pkg/issuer/venafi/client/BUILD.bazel @@ -3,7 +3,6 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") go_library( name = "go_default_library", srcs = [ - "customfield.go", "request.go", "venaficlient.go", ], @@ -11,6 +10,7 @@ go_library( visibility = ["//pkg:__subpackages__"], deps = [ "//pkg/apis/certmanager/v1alpha2:go_default_library", + "//pkg/issuer/venafi/client/api:go_default_library", "//pkg/util/pki:go_default_library", "@com_github_venafi_vcert//:go_default_library", "@com_github_venafi_vcert//pkg/certificate:go_default_library", @@ -30,6 +30,7 @@ filegroup( name = "all-srcs", srcs = [ ":package-srcs", + "//pkg/issuer/venafi/client/api:all-srcs", "//pkg/issuer/venafi/client/fake:all-srcs", ], tags = ["automanaged"], @@ -46,6 +47,7 @@ go_test( deps = [ "//pkg/apis/certmanager/v1alpha2:go_default_library", "//pkg/apis/meta/v1:go_default_library", + "//pkg/issuer/venafi/client/api:go_default_library", "//pkg/issuer/venafi/client/fake:go_default_library", "//pkg/util:go_default_library", "//pkg/util/pki:go_default_library", diff --git a/pkg/issuer/venafi/client/api/BUILD.bazel b/pkg/issuer/venafi/client/api/BUILD.bazel new file mode 100644 index 000000000..6befafb45 --- /dev/null +++ b/pkg/issuer/venafi/client/api/BUILD.bazel @@ -0,0 +1,22 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_library") + +go_library( + name = "go_default_library", + srcs = ["customfield.go"], + importpath = "github.com/jetstack/cert-manager/pkg/issuer/venafi/client/api", + visibility = ["//visibility:public"], +) + +filegroup( + name = "package-srcs", + srcs = glob(["**"]), + tags = ["automanaged"], + visibility = ["//visibility:private"], +) + +filegroup( + name = "all-srcs", + srcs = [":package-srcs"], + tags = ["automanaged"], + visibility = ["//visibility:public"], +) diff --git a/pkg/issuer/venafi/client/customfield.go b/pkg/issuer/venafi/client/api/customfield.go similarity index 98% rename from pkg/issuer/venafi/client/customfield.go rename to pkg/issuer/venafi/client/api/customfield.go index c69f60310..e2e44c661 100644 --- a/pkg/issuer/venafi/client/customfield.go +++ b/pkg/issuer/venafi/client/api/customfield.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package client +package api type CustomFieldType string diff --git a/pkg/issuer/venafi/client/fake/BUILD.bazel b/pkg/issuer/venafi/client/fake/BUILD.bazel index 1d578ddfb..7cf5ee52b 100644 --- a/pkg/issuer/venafi/client/fake/BUILD.bazel +++ b/pkg/issuer/venafi/client/fake/BUILD.bazel @@ -9,7 +9,7 @@ go_library( importpath = "github.com/jetstack/cert-manager/pkg/issuer/venafi/client/fake", visibility = ["//pkg:__subpackages__"], deps = [ - "//pkg/issuer/venafi/client:go_default_library", + "//pkg/issuer/venafi/client/api:go_default_library", "@com_github_venafi_vcert//pkg/certificate:go_default_library", "@com_github_venafi_vcert//pkg/endpoint:go_default_library", "@com_github_venafi_vcert//pkg/venafi/fake:go_default_library", diff --git a/pkg/issuer/venafi/client/fake/venafi.go b/pkg/issuer/venafi/client/fake/venafi.go index e89a8c96f..130b158af 100644 --- a/pkg/issuer/venafi/client/fake/venafi.go +++ b/pkg/issuer/venafi/client/fake/venafi.go @@ -19,14 +19,15 @@ package fake import ( "time" - "github.com/jetstack/cert-manager/pkg/issuer/venafi/client" + "github.com/jetstack/cert-manager/pkg/issuer/venafi/client/api" "github.com/Venafi/vcert/pkg/endpoint" ) type Venafi struct { PingFn func() error - SignFn func([]byte, time.Duration, []client.CustomField) ([]byte, error) + RequestCertificateFn func(csrPEM []byte, duration time.Duration, customFields []api.CustomField) (string, error) + RetreiveCertificateFn func(pickupID string, csrPEM []byte, duration time.Duration, customFields []api.CustomField) ([]byte, error) ReadZoneConfigurationFn func() (*endpoint.ZoneConfiguration, error) } @@ -34,8 +35,12 @@ func (v *Venafi) Ping() error { return v.PingFn() } -func (v *Venafi) Sign(b []byte, t time.Duration, f []client.CustomField) ([]byte, error) { - return v.SignFn(b, t, f) +func (v *Venafi) RequestCertificate(csrPEM []byte, duration time.Duration, customFields []api.CustomField) (string, error) { + return v.RequestCertificateFn(csrPEM, duration, customFields) +} + +func (v *Venafi) RetreiveCertificate(pickupID string, csrPEM []byte, duration time.Duration, customFields []api.CustomField) ([]byte, error) { + return v.RetreiveCertificateFn(pickupID, csrPEM, duration, customFields) } func (v *Venafi) ReadZoneConfiguration() (*endpoint.ZoneConfiguration, error) { diff --git a/pkg/issuer/venafi/client/request.go b/pkg/issuer/venafi/client/request.go index b2df5f780..59ddaaf94 100644 --- a/pkg/issuer/venafi/client/request.go +++ b/pkg/issuer/venafi/client/request.go @@ -23,6 +23,8 @@ import ( "strings" "time" + "github.com/jetstack/cert-manager/pkg/issuer/venafi/client/api" + "github.com/Venafi/vcert/pkg/certificate" "github.com/jetstack/cert-manager/pkg/util/pki" @@ -30,7 +32,7 @@ import ( // ErrCustomFieldsType provides a common error structure for an invalid Venafi custom field type type ErrCustomFieldsType struct { - Type CustomFieldType + Type api.CustomFieldType } func (err ErrCustomFieldsType) Error() string { @@ -43,7 +45,7 @@ var ErrorMissingSubject = errors.New("Certificate requests submitted to Venafi i // The CSR will be decoded to be validated against the zone configuration policy. // Upon the template being successfully defaulted and validated, the CSR will be sent, as is. // It will return a pickup ID which can be used with RetreiveCertificate to get the certificate -func (v *Venafi) RequestCertificate(csrPEM []byte, duration time.Duration, customFields []CustomField) (string, error) { +func (v *Venafi) RequestCertificate(csrPEM []byte, duration time.Duration, customFields []api.CustomField) (string, error) { vreq, err := v.buildVReq(csrPEM, duration, customFields) if err != nil { return "", err @@ -53,7 +55,7 @@ func (v *Venafi) RequestCertificate(csrPEM []byte, duration time.Duration, custo return requestID, err } -func (v *Venafi) RetreiveCertificate(pickupID string, csrPEM []byte, duration time.Duration, customFields []CustomField) ([]byte, error) { +func (v *Venafi) RetreiveCertificate(pickupID string, csrPEM []byte, duration time.Duration, customFields []api.CustomField) ([]byte, error) { vreq, err := v.buildVReq(csrPEM, duration, customFields) if err != nil { return nil, err @@ -75,7 +77,7 @@ func (v *Venafi) RetreiveCertificate(pickupID string, csrPEM []byte, duration ti return []byte(chain), nil } -func (v *Venafi) buildVReq(csrPEM []byte, duration time.Duration, customFields []CustomField) (*certificate.Request, error) { +func (v *Venafi) buildVReq(csrPEM []byte, duration time.Duration, customFields []api.CustomField) (*certificate.Request, error) { // Retrieve a copy of the Venafi zone. // This contains default values and policy control info that we can apply // and check against locally. @@ -131,13 +133,13 @@ func (v *Venafi) buildVReq(csrPEM []byte, duration time.Duration, customFields [ return vreq, nil } -func convertCustomFieldsToVcert(customFields []CustomField) ([]certificate.CustomField, error) { +func convertCustomFieldsToVcert(customFields []api.CustomField) ([]certificate.CustomField, error) { var out []certificate.CustomField if len(customFields) > 0 { for _, field := range customFields { var fieldType certificate.CustomFieldType switch field.Type { - case CustomFieldTypePlain, "": + case api.CustomFieldTypePlain, "": fieldType = certificate.CustomFieldPlain break default: diff --git a/pkg/issuer/venafi/client/request_test.go b/pkg/issuer/venafi/client/request_test.go index 32eb9cb0b..8315b9137 100644 --- a/pkg/issuer/venafi/client/request_test.go +++ b/pkg/issuer/venafi/client/request_test.go @@ -26,6 +26,8 @@ import ( "testing" "time" + "github.com/jetstack/cert-manager/pkg/issuer/venafi/client/api" + "github.com/Venafi/vcert/pkg/certificate" "github.com/Venafi/vcert/pkg/endpoint" "github.com/Venafi/vcert/pkg/venafi/fake" @@ -183,7 +185,7 @@ func TestSign(t *testing.T) { }, "obtain a certificate with custom fields specified": { csrPEM: csrPEM, - customFields: []CustomField{{Name: "test", Value: "ok"}}, + customFields: []api.CustomField{{Name: "test", Value: "ok"}}, client: internalfake.Connector{ RetrieveCertificateFunc: func(r *certificate.Request) (*certificate.PEMCollection, error) { // we set 1 field by default @@ -207,7 +209,7 @@ func TestSign(t *testing.T) { }, "If invalid custom field type found the error": { csrPEM: csrPEM, - customFields: []CustomField{{Name: "test", Value: "ok", Type: "Bool"}}, + customFields: []api.CustomField{{Name: "test", Value: "ok", Type: "Bool"}}, checkFn: checkNoCertificateIssued, expectedErr: true, }, @@ -224,9 +226,10 @@ type testSignT struct { csrPEM []byte client connector - expectedErr bool + expectedErr bool + expectedRequestErr bool - customFields []CustomField + customFields []api.CustomField checkFn func(*testing.T, []byte, []byte) } @@ -238,10 +241,18 @@ func (s *testSignT) runTest(t *testing.T) { } v := &Venafi{ - client: client, + vcertClient: client, } - resp, err := v.Sign(s.csrPEM, time.Minute, s.customFields) + pickupID, err := v.RequestCertificate(s.csrPEM, time.Minute, s.customFields) + if err != nil && !s.expectedRequestErr { + t.Errorf("expected to not get an error, but got: %v", err) + } + if err == nil && s.expectedRequestErr { + t.Errorf("expected to get an error but did not get one") + } + + resp, err := v.RetreiveCertificate(pickupID, s.csrPEM, time.Minute, s.customFields) if err != nil && !s.expectedErr { t.Errorf("expected to not get an error, but got: %v", err) } diff --git a/pkg/issuer/venafi/client/venaficlient.go b/pkg/issuer/venafi/client/venaficlient.go index ab8f6306e..be3b95c9b 100644 --- a/pkg/issuer/venafi/client/venaficlient.go +++ b/pkg/issuer/venafi/client/venaficlient.go @@ -20,6 +20,8 @@ import ( "fmt" "time" + "github.com/jetstack/cert-manager/pkg/issuer/venafi/client/api" + "github.com/Venafi/vcert" "github.com/Venafi/vcert/pkg/certificate" "github.com/Venafi/vcert/pkg/endpoint" @@ -40,8 +42,8 @@ type VenafiClientBuilder func(namespace string, secretsLister corelisters.Secret // Interface implements a Venafi client type Interface interface { - RequestCertificate(csrPEM []byte, duration time.Duration, customFields []CustomField) (string, error) - RetreiveCertificate(pickupID string, csrPEM []byte, duration time.Duration, customFields []CustomField) ([]byte, error) + RequestCertificate(csrPEM []byte, duration time.Duration, customFields []api.CustomField) (string, error) + RetreiveCertificate(pickupID string, csrPEM []byte, duration time.Duration, customFields []api.CustomField) ([]byte, error) Ping() error ReadZoneConfiguration() (*endpoint.ZoneConfiguration, error) SetClient(endpoint.Connector)