From 4452fbd9a8c8cfe114218a66ad11481808b6b681 Mon Sep 17 00:00:00 2001 From: Maartje Eyskens Date: Tue, 4 Feb 2020 13:46:22 +0100 Subject: [PATCH 01/20] Add venafi.cert-manager.io/custom-fields annnotation This adds the venafi.cert-manager.io/custom-fields annotation to CertificateRequest. The JSON decoded value of this annotation will be passed to the Venafi customfields on signing. Signed-off-by: Maartje Eyskens --- go.mod | 4 ++-- go.sum | 6 ++++++ .../certificaterequests/venafi/venafi.go | 17 ++++++++++++++++- pkg/internal/venafi/fake/venafi.go | 7 ++++--- pkg/internal/venafi/sign.go | 3 ++- pkg/internal/venafi/sign_test.go | 2 +- pkg/internal/venafi/venafi.go | 2 +- 7 files changed, 32 insertions(+), 9 deletions(-) diff --git a/go.mod b/go.mod index 4f6c3d736..3b143304b 100644 --- a/go.mod +++ b/go.mod @@ -13,7 +13,7 @@ require ( github.com/Azure/go-autorest/autorest/adal v0.5.0 github.com/Azure/go-autorest/autorest/to v0.3.0 github.com/Azure/go-autorest/autorest/validation v0.2.0 // indirect - github.com/Venafi/vcert v0.0.0-20190613103158-62139eb19b25 + github.com/Venafi/vcert v0.0.0-20200120162047-d4d4df569eec github.com/aws/aws-sdk-go v1.24.1 github.com/cloudflare/cloudflare-go v0.8.5 github.com/cpu/goacmedns v0.0.0-20180701200144-565ecf2a84df @@ -41,7 +41,7 @@ require ( golang.org/x/net v0.0.0-20191004110552-13f9640d40b9 golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45 google.golang.org/api v0.4.0 - gopkg.in/ini.v1 v1.42.0 // indirect + gopkg.in/ini.v1 v1.52.0 // indirect k8s.io/api v0.17.0 k8s.io/apiextensions-apiserver v0.17.0 k8s.io/apimachinery v0.17.0 diff --git a/go.sum b/go.sum index c03c92906..ec6ced371 100644 --- a/go.sum +++ b/go.sum @@ -38,6 +38,10 @@ github.com/PuerkitoBio/urlesc v0.0.0-20170810143723-de5bf2ad4578 h1:d+Bc7a5rLufV github.com/PuerkitoBio/urlesc v0.0.0-20170810143723-de5bf2ad4578/go.mod h1:uGdkoq3SwY9Y+13GIhn11/XLaGBb4BfwItxLd5jeuXE= github.com/Venafi/vcert v0.0.0-20190613103158-62139eb19b25 h1:Vy4WHFF6SULSubt7vuyJRJ5AqwjJXzJQWdF37VcsyGo= github.com/Venafi/vcert v0.0.0-20190613103158-62139eb19b25/go.mod h1:3sXw16DKVded/kLVDma2veqEUQC7O37h98ims7cIvN4= +github.com/Venafi/vcert v0.0.0-20200120162047-d4d4df569eec h1:QcsUgs0gP2qltiFPlVXXqlnmRthKPv6WRxCGd7ANutk= +github.com/Venafi/vcert v0.0.0-20200120162047-d4d4df569eec/go.mod h1:5T4bFPhcgGXbdz8nVVRuE2gXSRDlZVL+9T5CwZZ3Yk4= +github.com/Venafi/vcert v3.18.4+incompatible h1:mDXSjd+EpXa8YEkEo9Oad19E270aiPJJMhjoKs63b+8= +github.com/Venafi/vcert v3.18.4+incompatible/go.mod h1:3dpfrCI+31cDZosD+1UX8GFziVFORaegByXtzT1dwNo= github.com/agnivade/levenshtein v1.0.1/go.mod h1:CURSv5d9Uaml+FovSIICkLbAUZ9S4RqaHDIsdSBg7lM= github.com/alecthomas/template v0.0.0-20160405071501-a0175ee3bccc/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc= github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0= @@ -640,6 +644,8 @@ gopkg.in/inf.v0 v0.9.1/go.mod h1:cWUDdTG/fYaXco+Dcufb5Vnc6Gp2YChqWtbxRZE0mXw= gopkg.in/ini.v1 v1.38.2/go.mod h1:pNLf8WUiyNEtQjuu5G5vTm06TEv9tsIgeAvK8hOrP4k= gopkg.in/ini.v1 v1.42.0 h1:7N3gPTt50s8GuLortA00n8AqRTk75qOP98+mTPpgzRk= gopkg.in/ini.v1 v1.42.0/go.mod h1:pNLf8WUiyNEtQjuu5G5vTm06TEv9tsIgeAvK8hOrP4k= +gopkg.in/ini.v1 v1.52.0 h1:j+Lt/M1oPPejkniCg1TkWE2J3Eh1oZTsHSXzMTzUXn4= +gopkg.in/ini.v1 v1.52.0/go.mod h1:pNLf8WUiyNEtQjuu5G5vTm06TEv9tsIgeAvK8hOrP4k= gopkg.in/natefinch/lumberjack.v2 v2.0.0 h1:1Lc07Kr7qY4U2YPouBjpCLxpiyxIVoxqXgkXLknAOE8= gopkg.in/natefinch/lumberjack.v2 v2.0.0/go.mod h1:l0ndWWf7gzL7RNwBG7wST/UCcT4T24xpD6X8LsfU/+k= gopkg.in/resty.v1 v1.12.0/go.mod h1:mDo4pnntr5jdWRML875a/NmxYqAlA73dVijT2AXvQQo= diff --git a/pkg/controller/certificaterequests/venafi/venafi.go b/pkg/controller/certificaterequests/venafi/venafi.go index 3bd8875e4..808ef288d 100644 --- a/pkg/controller/certificaterequests/venafi/venafi.go +++ b/pkg/controller/certificaterequests/venafi/venafi.go @@ -18,7 +18,9 @@ package venafi import ( "context" + "encoding/json" + "github.com/Venafi/vcert/pkg/certificate" "github.com/Venafi/vcert/pkg/endpoint" k8sErrors "k8s.io/apimachinery/pkg/api/errors" corelisters "k8s.io/client-go/listers/core/v1" @@ -88,7 +90,20 @@ func (v *Venafi) Sign(ctx context.Context, cr *cmapi.CertificateRequest, issuerO duration := apiutil.DefaultCertDuration(cr.Spec.Duration) - certPem, err := client.Sign(cr.Spec.CSRPEM, duration) + customFields := []certificate.CustomField{} + if annotation, exists := cr.GetAnnotations()["venafi.cert-manager.io/custom-fields"]; exists && annotation != "" { + err := json.Unmarshal([]byte(annotation), &customFields) + if err != nil { + message := "Failed to parse venafi.cert-manager.io/custom-fields anotation" + + v.reporter.Pending(cr, err, "VenafiCustomFieldsParseError", message) + log.Error(err, message) + + return nil, err + } + } + + certPem, err := client.Sign(cr.Spec.CSRPEM, duration, customFields) // Check some known error types if err != nil { diff --git a/pkg/internal/venafi/fake/venafi.go b/pkg/internal/venafi/fake/venafi.go index 94163758f..8f391148a 100644 --- a/pkg/internal/venafi/fake/venafi.go +++ b/pkg/internal/venafi/fake/venafi.go @@ -19,12 +19,13 @@ package fake import ( "time" + "github.com/Venafi/vcert/pkg/certificate" "github.com/Venafi/vcert/pkg/endpoint" ) type Venafi struct { PingFn func() error - SignFn func([]byte, time.Duration) ([]byte, error) + SignFn func([]byte, time.Duration, []certificate.CustomField) ([]byte, error) ReadZoneConfigurationFn func() (*endpoint.ZoneConfiguration, error) } @@ -32,8 +33,8 @@ func (v *Venafi) Ping() error { return v.PingFn() } -func (v *Venafi) Sign(b []byte, t time.Duration) ([]byte, error) { - return v.SignFn(b, t) +func (v *Venafi) Sign(b []byte, t time.Duration, f []certificate.CustomField) ([]byte, error) { + return v.SignFn(b, t, f) } func (v *Venafi) ReadZoneConfiguration() (*endpoint.ZoneConfiguration, error) { diff --git a/pkg/internal/venafi/sign.go b/pkg/internal/venafi/sign.go index e67150bac..d1e0e91c0 100644 --- a/pkg/internal/venafi/sign.go +++ b/pkg/internal/venafi/sign.go @@ -30,7 +30,7 @@ import ( // This function sends a request to Venafi to for a signed certificate. // 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. -func (v *Venafi) Sign(csrPEM []byte, duration time.Duration) (cert []byte, err error) { +func (v *Venafi) Sign(csrPEM []byte, duration time.Duration, customFields []certificate.CustomField) (cert []byte, err error) { // Retrieve a copy of the Venafi zone. // This contains default values and policy control info that we can apply // and check against locally. @@ -46,6 +46,7 @@ func (v *Venafi) Sign(csrPEM []byte, duration time.Duration) (cert []byte, err e // Create a vcert Request structure vreq := newVRequest(tmpl) + vreq.CustomFields = customFields // Apply default values from the Venafi zone zoneCfg.UpdateCertificateRequest(vreq) diff --git a/pkg/internal/venafi/sign_test.go b/pkg/internal/venafi/sign_test.go index 63f35763d..8d0081c2e 100644 --- a/pkg/internal/venafi/sign_test.go +++ b/pkg/internal/venafi/sign_test.go @@ -201,7 +201,7 @@ func (s *testSignT) runTest(t *testing.T) { client: client, } - resp, err := v.Sign(s.csrPEM, time.Minute) + resp, err := v.Sign(s.csrPEM, time.Minute, []certificate.CustomField{}) if err != nil && !s.expectedErr { t.Errorf("expected to not get an error, but got: %v", err) } diff --git a/pkg/internal/venafi/venafi.go b/pkg/internal/venafi/venafi.go index 0204c3066..c23e3c3b5 100644 --- a/pkg/internal/venafi/venafi.go +++ b/pkg/internal/venafi/venafi.go @@ -39,7 +39,7 @@ type VenafiClientBuilder func(namespace string, secretsLister corelisters.Secret issuer cmapi.GenericIssuer) (Interface, error) type Interface interface { - Sign(csrPEM []byte, duration time.Duration) (cert []byte, err error) + Sign(csrPEM []byte, duration time.Duration, customFields []certificate.CustomField) (cert []byte, err error) Ping() error ReadZoneConfiguration() (*endpoint.ZoneConfiguration, error) SetClient(endpoint.Connector) From 6ecc07ba26cdf4082b723313c038376b2335a014 Mon Sep 17 00:00:00 2001 From: Maartje Eyskens Date: Tue, 4 Feb 2020 15:37:40 +0100 Subject: [PATCH 02/20] Update bazel Signed-off-by: Maartje Eyskens --- devel/BUILD.bazel | 1 + pkg/controller/certificaterequests/venafi/BUILD.bazel | 1 + 2 files changed, 2 insertions(+) diff --git a/devel/BUILD.bazel b/devel/BUILD.bazel index 710dad719..992e11844 100644 --- a/devel/BUILD.bazel +++ b/devel/BUILD.bazel @@ -14,6 +14,7 @@ filegroup( "//devel/addon/pebble:all-srcs", "//devel/addon/samplewebhook:all-srcs", "//devel/addon/vault:all-srcs", + "//devel/bin:all-srcs", ], tags = ["automanaged"], visibility = ["//visibility:public"], diff --git a/pkg/controller/certificaterequests/venafi/BUILD.bazel b/pkg/controller/certificaterequests/venafi/BUILD.bazel index 355aa496f..551da23fb 100644 --- a/pkg/controller/certificaterequests/venafi/BUILD.bazel +++ b/pkg/controller/certificaterequests/venafi/BUILD.bazel @@ -14,6 +14,7 @@ go_library( "//pkg/internal/venafi:go_default_library", "//pkg/issuer:go_default_library", "//pkg/logs:go_default_library", + "@com_github_venafi_vcert//pkg/certificate:go_default_library", "@com_github_venafi_vcert//pkg/endpoint:go_default_library", "@io_k8s_apimachinery//pkg/api/errors:go_default_library", "@io_k8s_client_go//listers/core/v1:go_default_library", From 623c932f2f63c334aa7f1d9160ced8b32c3873f8 Mon Sep 17 00:00:00 2001 From: Maartje Eyskens Date: Tue, 4 Feb 2020 17:57:04 +0100 Subject: [PATCH 03/20] Update deps Signed-off-by: Maartje Eyskens --- go.sum | 6 ------ hack/build/repos.bzl | 8 ++++---- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/go.sum b/go.sum index ec6ced371..b55ea2238 100644 --- a/go.sum +++ b/go.sum @@ -36,12 +36,8 @@ github.com/PuerkitoBio/purell v1.1.1/go.mod h1:c11w/QuzBsJSee3cPx9rAFu61PvFxuPbt github.com/PuerkitoBio/urlesc v0.0.0-20160726150825-5bd2802263f2/go.mod h1:uGdkoq3SwY9Y+13GIhn11/XLaGBb4BfwItxLd5jeuXE= github.com/PuerkitoBio/urlesc v0.0.0-20170810143723-de5bf2ad4578 h1:d+Bc7a5rLufV/sSk/8dngufqelfh6jnri85riMAaF/M= github.com/PuerkitoBio/urlesc v0.0.0-20170810143723-de5bf2ad4578/go.mod h1:uGdkoq3SwY9Y+13GIhn11/XLaGBb4BfwItxLd5jeuXE= -github.com/Venafi/vcert v0.0.0-20190613103158-62139eb19b25 h1:Vy4WHFF6SULSubt7vuyJRJ5AqwjJXzJQWdF37VcsyGo= -github.com/Venafi/vcert v0.0.0-20190613103158-62139eb19b25/go.mod h1:3sXw16DKVded/kLVDma2veqEUQC7O37h98ims7cIvN4= github.com/Venafi/vcert v0.0.0-20200120162047-d4d4df569eec h1:QcsUgs0gP2qltiFPlVXXqlnmRthKPv6WRxCGd7ANutk= github.com/Venafi/vcert v0.0.0-20200120162047-d4d4df569eec/go.mod h1:5T4bFPhcgGXbdz8nVVRuE2gXSRDlZVL+9T5CwZZ3Yk4= -github.com/Venafi/vcert v3.18.4+incompatible h1:mDXSjd+EpXa8YEkEo9Oad19E270aiPJJMhjoKs63b+8= -github.com/Venafi/vcert v3.18.4+incompatible/go.mod h1:3dpfrCI+31cDZosD+1UX8GFziVFORaegByXtzT1dwNo= github.com/agnivade/levenshtein v1.0.1/go.mod h1:CURSv5d9Uaml+FovSIICkLbAUZ9S4RqaHDIsdSBg7lM= github.com/alecthomas/template v0.0.0-20160405071501-a0175ee3bccc/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc= github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0= @@ -642,8 +638,6 @@ gopkg.in/inf.v0 v0.9.0/go.mod h1:cWUDdTG/fYaXco+Dcufb5Vnc6Gp2YChqWtbxRZE0mXw= gopkg.in/inf.v0 v0.9.1 h1:73M5CoZyi3ZLMOyDlQh031Cx6N9NDJ2Vvfl76EDAgDc= gopkg.in/inf.v0 v0.9.1/go.mod h1:cWUDdTG/fYaXco+Dcufb5Vnc6Gp2YChqWtbxRZE0mXw= gopkg.in/ini.v1 v1.38.2/go.mod h1:pNLf8WUiyNEtQjuu5G5vTm06TEv9tsIgeAvK8hOrP4k= -gopkg.in/ini.v1 v1.42.0 h1:7N3gPTt50s8GuLortA00n8AqRTk75qOP98+mTPpgzRk= -gopkg.in/ini.v1 v1.42.0/go.mod h1:pNLf8WUiyNEtQjuu5G5vTm06TEv9tsIgeAvK8hOrP4k= gopkg.in/ini.v1 v1.52.0 h1:j+Lt/M1oPPejkniCg1TkWE2J3Eh1oZTsHSXzMTzUXn4= gopkg.in/ini.v1 v1.52.0/go.mod h1:pNLf8WUiyNEtQjuu5G5vTm06TEv9tsIgeAvK8hOrP4k= gopkg.in/natefinch/lumberjack.v2 v2.0.0 h1:1Lc07Kr7qY4U2YPouBjpCLxpiyxIVoxqXgkXLknAOE8= diff --git a/hack/build/repos.bzl b/hack/build/repos.bzl index 703de977f..9aca363a8 100644 --- a/hack/build/repos.bzl +++ b/hack/build/repos.bzl @@ -1647,8 +1647,8 @@ def go_repositories(): build_file_generation = "on", build_file_proto_mode = "disable", importpath = "github.com/Venafi/vcert", - sum = "h1:Vy4WHFF6SULSubt7vuyJRJ5AqwjJXzJQWdF37VcsyGo=", - version = "v0.0.0-20190613103158-62139eb19b25", + sum = "h1:QcsUgs0gP2qltiFPlVXXqlnmRthKPv6WRxCGd7ANutk=", + version = "v0.0.0-20200120162047-d4d4df569eec", ) go_repository( name = "com_github_xiang90_probing", @@ -1719,8 +1719,8 @@ def go_repositories(): build_file_generation = "on", build_file_proto_mode = "disable", importpath = "gopkg.in/ini.v1", - sum = "h1:7N3gPTt50s8GuLortA00n8AqRTk75qOP98+mTPpgzRk=", - version = "v1.42.0", + sum = "h1:j+Lt/M1oPPejkniCg1TkWE2J3Eh1oZTsHSXzMTzUXn4=", + version = "v1.52.0", ) go_repository( name = "in_gopkg_mgo_v2", From 42f3bca6ef9e9228a1a48c1109b6561fd5ef9d98 Mon Sep 17 00:00:00 2001 From: Maartje Eyskens Date: Wed, 5 Feb 2020 14:08:41 +0100 Subject: [PATCH 04/20] Move annotation to apis Signed-off-by: Maartje Eyskens --- pkg/apis/certmanager/v1alpha2/types.go | 6 ++++++ pkg/apis/certmanager/v1alpha3/types.go | 6 ++++++ pkg/controller/certificaterequests/venafi/venafi.go | 4 ++-- 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/pkg/apis/certmanager/v1alpha2/types.go b/pkg/apis/certmanager/v1alpha2/types.go index 062d4e785..eaa85e63e 100644 --- a/pkg/apis/certmanager/v1alpha2/types.go +++ b/pkg/apis/certmanager/v1alpha2/types.go @@ -98,6 +98,12 @@ const ( AllowsInjectionFromSecretAnnotation = "cert-manager.io/allow-direct-injection" ) +// Issuer specific Annotations +const ( + // VenafiCustomFields is the annotation that passes on JSON encoded custom fields to the Venafi issuer + VenafiCustomFields = "venafi.cert-manager.io/custom-fields" +) + // KeyUsage specifies valid usage contexts for keys. // See: https://tools.ietf.org/html/rfc5280#section-4.2.1.3 // https://tools.ietf.org/html/rfc5280#section-4.2.1.12 diff --git a/pkg/apis/certmanager/v1alpha3/types.go b/pkg/apis/certmanager/v1alpha3/types.go index 4eec7b05c..5fbd09339 100644 --- a/pkg/apis/certmanager/v1alpha3/types.go +++ b/pkg/apis/certmanager/v1alpha3/types.go @@ -98,6 +98,12 @@ const ( AllowsInjectionFromSecretAnnotation = "cert-manager.io/allow-direct-injection" ) +// Issuer specific Annotations +const ( + // VenafiCustomFields is the annotation that passes on JSON encoded custom fields to the Venafi issuer + VenafiCustomFields = "venafi.cert-manager.io/custom-fields" +) + // KeyUsage specifies valid usage contexts for keys. // See: https://tools.ietf.org/html/rfc5280#section-4.2.1.3 // https://tools.ietf.org/html/rfc5280#section-4.2.1.12 diff --git a/pkg/controller/certificaterequests/venafi/venafi.go b/pkg/controller/certificaterequests/venafi/venafi.go index 808ef288d..58b130543 100644 --- a/pkg/controller/certificaterequests/venafi/venafi.go +++ b/pkg/controller/certificaterequests/venafi/venafi.go @@ -90,8 +90,8 @@ func (v *Venafi) Sign(ctx context.Context, cr *cmapi.CertificateRequest, issuerO duration := apiutil.DefaultCertDuration(cr.Spec.Duration) - customFields := []certificate.CustomField{} - if annotation, exists := cr.GetAnnotations()["venafi.cert-manager.io/custom-fields"]; exists && annotation != "" { + customFields := []venafiinternal.CustomField{} + if annotation, exists := cr.GetAnnotations()[cmapi.VenafiCustomFields]; exists && annotation != "" { err := json.Unmarshal([]byte(annotation), &customFields) if err != nil { message := "Failed to parse venafi.cert-manager.io/custom-fields anotation" From 93f0cfa7170ff03ffc562c9331c41608e8c5f443 Mon Sep 17 00:00:00 2001 From: Maartje Eyskens Date: Wed, 5 Feb 2020 14:12:16 +0100 Subject: [PATCH 05/20] Use internal CustomField to add json tags Signed-off-by: Maartje Eyskens --- pkg/controller/certificaterequests/venafi/venafi.go | 5 ++--- .../certificaterequests/venafi/venafi_test.go | 8 ++++---- pkg/internal/venafi/fake/venafi.go | 6 +++--- pkg/internal/venafi/sign.go | 11 +++++++++-- pkg/internal/venafi/venafi.go | 9 ++++++++- 5 files changed, 26 insertions(+), 13 deletions(-) diff --git a/pkg/controller/certificaterequests/venafi/venafi.go b/pkg/controller/certificaterequests/venafi/venafi.go index 58b130543..4827eb90c 100644 --- a/pkg/controller/certificaterequests/venafi/venafi.go +++ b/pkg/controller/certificaterequests/venafi/venafi.go @@ -20,7 +20,6 @@ import ( "context" "encoding/json" - "github.com/Venafi/vcert/pkg/certificate" "github.com/Venafi/vcert/pkg/endpoint" k8sErrors "k8s.io/apimachinery/pkg/api/errors" corelisters "k8s.io/client-go/listers/core/v1" @@ -94,9 +93,9 @@ func (v *Venafi) Sign(ctx context.Context, cr *cmapi.CertificateRequest, issuerO if annotation, exists := cr.GetAnnotations()[cmapi.VenafiCustomFields]; exists && annotation != "" { err := json.Unmarshal([]byte(annotation), &customFields) if err != nil { - message := "Failed to parse venafi.cert-manager.io/custom-fields anotation" + message := "Failed to parse venafi.cert-manager.io/custom-fields annotation" - v.reporter.Pending(cr, err, "VenafiCustomFieldsParseError", message) + v.reporter.Failed(cr, err, "CustomFieldsError", message) log.Error(err, message) return nil, err diff --git a/pkg/controller/certificaterequests/venafi/venafi_test.go b/pkg/controller/certificaterequests/venafi/venafi_test.go index db990d089..da8e40e36 100644 --- a/pkg/controller/certificaterequests/venafi/venafi_test.go +++ b/pkg/controller/certificaterequests/venafi/venafi_test.go @@ -177,7 +177,7 @@ func TestSign(t *testing.T) { } clientReturnsPending := &internalvenafifake.Venafi{ - SignFn: func([]byte, time.Duration) ([]byte, error) { + SignFn: func([]byte, time.Duration, []internalvenafi.CustomField) ([]byte, error) { return nil, endpoint.ErrCertificatePending{ CertificateID: "test-cert-id", Status: "test-status-pending", @@ -185,19 +185,19 @@ func TestSign(t *testing.T) { }, } clientReturnsTimeout := &internalvenafifake.Venafi{ - SignFn: func([]byte, time.Duration) ([]byte, error) { + SignFn: func([]byte, time.Duration, []internalvenafi.CustomField) ([]byte, error) { return nil, endpoint.ErrRetrieveCertificateTimeout{ CertificateID: "test-cert-id", } }, } clientReturnsGenericError := &internalvenafifake.Venafi{ - SignFn: func([]byte, time.Duration) ([]byte, error) { + SignFn: func([]byte, time.Duration, []internalvenafi.CustomField) ([]byte, error) { return nil, errors.New("this is an error") }, } clientReturnsCert := &internalvenafifake.Venafi{ - SignFn: func([]byte, time.Duration) ([]byte, error) { + SignFn: func([]byte, time.Duration, []internalvenafi.CustomField) ([]byte, error) { return certPEM, nil }, } diff --git a/pkg/internal/venafi/fake/venafi.go b/pkg/internal/venafi/fake/venafi.go index 8f391148a..aa5927baf 100644 --- a/pkg/internal/venafi/fake/venafi.go +++ b/pkg/internal/venafi/fake/venafi.go @@ -19,13 +19,13 @@ package fake import ( "time" - "github.com/Venafi/vcert/pkg/certificate" "github.com/Venafi/vcert/pkg/endpoint" + venafiinternal "github.com/jetstack/cert-manager/pkg/internal/venafi" ) type Venafi struct { PingFn func() error - SignFn func([]byte, time.Duration, []certificate.CustomField) ([]byte, error) + SignFn func([]byte, time.Duration, []venafiinternal.CustomField) ([]byte, error) ReadZoneConfigurationFn func() (*endpoint.ZoneConfiguration, error) } @@ -33,7 +33,7 @@ func (v *Venafi) Ping() error { return v.PingFn() } -func (v *Venafi) Sign(b []byte, t time.Duration, f []certificate.CustomField) ([]byte, error) { +func (v *Venafi) Sign(b []byte, t time.Duration, f []venafiinternal.CustomField) ([]byte, error) { return v.SignFn(b, t, f) } diff --git a/pkg/internal/venafi/sign.go b/pkg/internal/venafi/sign.go index d1e0e91c0..25fc0447b 100644 --- a/pkg/internal/venafi/sign.go +++ b/pkg/internal/venafi/sign.go @@ -30,7 +30,7 @@ import ( // This function sends a request to Venafi to for a signed certificate. // 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. -func (v *Venafi) Sign(csrPEM []byte, duration time.Duration, customFields []certificate.CustomField) (cert []byte, err error) { +func (v *Venafi) Sign(csrPEM []byte, duration time.Duration, customFields []CustomField) (cert []byte, err error) { // Retrieve a copy of the Venafi zone. // This contains default values and policy control info that we can apply // and check against locally. @@ -46,7 +46,14 @@ func (v *Venafi) Sign(csrPEM []byte, duration time.Duration, customFields []cert // Create a vcert Request structure vreq := newVRequest(tmpl) - vreq.CustomFields = customFields + + // Convert over custom fields from our struct type to venafi's + if len(customFields) > 0 { + vreq.CustomFields = []certificate.CustomField{} + for _, field := range customFields { + vreq.CustomFields = append(vreq.CustomFields, certificate.CustomField(field)) + } + } // Apply default values from the Venafi zone zoneCfg.UpdateCertificateRequest(vreq) diff --git a/pkg/internal/venafi/venafi.go b/pkg/internal/venafi/venafi.go index c23e3c3b5..75134d242 100644 --- a/pkg/internal/venafi/venafi.go +++ b/pkg/internal/venafi/venafi.go @@ -39,7 +39,7 @@ type VenafiClientBuilder func(namespace string, secretsLister corelisters.Secret issuer cmapi.GenericIssuer) (Interface, error) type Interface interface { - Sign(csrPEM []byte, duration time.Duration, customFields []certificate.CustomField) (cert []byte, err error) + Sign(csrPEM []byte, duration time.Duration, customFields []CustomField) (cert []byte, err error) Ping() error ReadZoneConfiguration() (*endpoint.ZoneConfiguration, error) SetClient(endpoint.Connector) @@ -68,6 +68,13 @@ type connector interface { RenewCertificate(req *certificate.RenewalRequest) (requestID string, err error) } +// CustomField defines a custom field to be passed to Venafi +type CustomField struct { + Type certificate.CustomFieldType `json:"type,omitempty"` + Name string `json:"name"` + Value string `json:"value"` +} + func New(namespace string, secretsLister corelisters.SecretLister, issuer cmapi.GenericIssuer) (Interface, error) { From ae742c588e85261b0e1b65e754488ff9663c16da Mon Sep 17 00:00:00 2001 From: Maartje Eyskens Date: Wed, 5 Feb 2020 15:36:30 +0100 Subject: [PATCH 06/20] Add tests for custom fields inside certificaterequest controller Signed-off-by: Maartje Eyskens --- .../certificaterequests/venafi/venafi.go | 2 +- .../certificaterequests/venafi/venafi_test.go | 73 +++++++++++++++++++ test/unit/gen/certificaterequest.go | 6 ++ 3 files changed, 80 insertions(+), 1 deletion(-) diff --git a/pkg/controller/certificaterequests/venafi/venafi.go b/pkg/controller/certificaterequests/venafi/venafi.go index 4827eb90c..935b20f36 100644 --- a/pkg/controller/certificaterequests/venafi/venafi.go +++ b/pkg/controller/certificaterequests/venafi/venafi.go @@ -89,7 +89,7 @@ func (v *Venafi) Sign(ctx context.Context, cr *cmapi.CertificateRequest, issuerO duration := apiutil.DefaultCertDuration(cr.Spec.Duration) - customFields := []venafiinternal.CustomField{} + var customFields []venafiinternal.CustomField if annotation, exists := cr.GetAnnotations()[cmapi.VenafiCustomFields]; exists && annotation != "" { err := json.Unmarshal([]byte(annotation), &customFields) if err != nil { diff --git a/pkg/controller/certificaterequests/venafi/venafi_test.go b/pkg/controller/certificaterequests/venafi/venafi_test.go index da8e40e36..a354a507f 100644 --- a/pkg/controller/certificaterequests/venafi/venafi_test.go +++ b/pkg/controller/certificaterequests/venafi/venafi_test.go @@ -146,6 +146,10 @@ func TestSign(t *testing.T) { }), ) + tppCRWithCustomFields := gen.CertificateRequestFrom(tppCR, gen.SetAnnotations(map[string]string{"venafi.cert-manager.io/custom-fields": `[{"name": "cert-manager-test", "value": "test ok"}]`})) + + tppCRWithInvalidCustomFields := gen.CertificateRequestFrom(tppCR, gen.SetAnnotations(map[string]string{"venafi.cert-manager.io/custom-fields": `[{"name": cert-manager-test}]`})) + cloudCR := gen.CertificateRequestFrom(baseCR, gen.SetCertificateRequestIssuer(cmmeta.ObjectReference{ Group: certmanager.GroupName, @@ -202,6 +206,15 @@ func TestSign(t *testing.T) { }, } + clientReturnsCertIfCustomField := &internalvenafifake.Venafi{ + SignFn: func(csr []byte, t time.Duration, fields []internalvenafi.CustomField) ([]byte, error) { + if len(fields) > 0 && fields[0].Name == "cert-manager-test" && fields[0].Value == "test ok" { + return certPEM, nil + } + return nil, errors.New("Custom field not set") + }, + } + metaFixedClockStart := metav1.NewTime(fixedClockStart) tests := map[string]testT{ "tpp: if fail to build client based on missing secret then return nil and hard fail": { @@ -573,6 +586,66 @@ func TestSign(t *testing.T) { fakeSecretLister: failGetSecretLister, fakeClient: clientReturnsCert, }, + "annotations: Custom Fields": { + certificateRequest: tppCRWithCustomFields.DeepCopy(), + issuer: tppIssuer, + builder: &controllertest.Builder{ + CertManagerObjects: []runtime.Object{tppCRWithCustomFields.DeepCopy(), tppIssuer.DeepCopy()}, + ExpectedEvents: []string{ + `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.ConditionTrue, + Reason: cmapi.CertificateRequestReasonIssued, + Message: "Certificate fetched from issuer successfully", + LastTransitionTime: &metaFixedClockStart, + }), + gen.SetCertificateRequestCertificate(certPEM), + ), + )), + }, + }, + fakeSecretLister: failGetSecretLister, + fakeClient: clientReturnsCertIfCustomField, + expectedErr: false, + }, + "annotations: Error on invalid JSON in custom fields": { + certificateRequest: tppCRWithInvalidCustomFields.DeepCopy(), + issuer: tppIssuer, + builder: &controllertest.Builder{ + CertManagerObjects: []runtime.Object{tppCRWithInvalidCustomFields.DeepCopy(), tppIssuer.DeepCopy()}, + ExpectedEvents: []string{ + `Warning CustomFieldsError Failed to parse venafi.cert-manager.io/custom-fields annotation: invalid character 'c' looking for beginning of value`, + }, + ExpectedActions: []testpkg.Action{ + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( + cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + "status", + gen.DefaultTestNamespace, + gen.CertificateRequestFrom(tppCRWithInvalidCustomFields, + gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ + Type: cmapi.CertificateRequestConditionReady, + Status: cmmeta.ConditionFalse, + Reason: cmapi.CertificateRequestReasonFailed, + Message: "Failed to parse venafi.cert-manager.io/custom-fields annotation: invalid character 'c' looking for beginning of value", + LastTransitionTime: &metaFixedClockStart, + }), + gen.SetCertificateRequestFailureTime(metaFixedClockStart), + ), + )), + }, + }, + fakeSecretLister: failGetSecretLister, + fakeClient: clientReturnsPending, + expectedErr: true, + }, } for name, test := range tests { diff --git a/test/unit/gen/certificaterequest.go b/test/unit/gen/certificaterequest.go index 3ebc8d4b0..cb8075ab6 100644 --- a/test/unit/gen/certificaterequest.go +++ b/test/unit/gen/certificaterequest.go @@ -139,3 +139,9 @@ func SetCertificateRequestFailureTime(p metav1.Time) CertificateRequestModifier cr.Status.FailureTime = &p } } + +func SetAnnotations(annotations map[string]string) CertificateRequestModifier { + return func(cr *v1alpha2.CertificateRequest) { + cr.SetAnnotations(annotations) + } +} From 15b9a940e727f2712b72b7afb1ab6b442aecea65 Mon Sep 17 00:00:00 2001 From: Maartje Eyskens Date: Wed, 5 Feb 2020 16:14:49 +0100 Subject: [PATCH 07/20] Add tests for custom fields inside internal/venafi Signed-off-by: Maartje Eyskens --- .../certificaterequests/venafi/BUILD.bazel | 1 - pkg/internal/venafi/fake/venafi.go | 6 +++--- pkg/internal/venafi/sign_test.go | 21 ++++++++++++++++++- 3 files changed, 23 insertions(+), 5 deletions(-) diff --git a/pkg/controller/certificaterequests/venafi/BUILD.bazel b/pkg/controller/certificaterequests/venafi/BUILD.bazel index 551da23fb..355aa496f 100644 --- a/pkg/controller/certificaterequests/venafi/BUILD.bazel +++ b/pkg/controller/certificaterequests/venafi/BUILD.bazel @@ -14,7 +14,6 @@ go_library( "//pkg/internal/venafi:go_default_library", "//pkg/issuer:go_default_library", "//pkg/logs:go_default_library", - "@com_github_venafi_vcert//pkg/certificate:go_default_library", "@com_github_venafi_vcert//pkg/endpoint:go_default_library", "@io_k8s_apimachinery//pkg/api/errors:go_default_library", "@io_k8s_client_go//listers/core/v1:go_default_library", diff --git a/pkg/internal/venafi/fake/venafi.go b/pkg/internal/venafi/fake/venafi.go index aa5927baf..1eaaa8b36 100644 --- a/pkg/internal/venafi/fake/venafi.go +++ b/pkg/internal/venafi/fake/venafi.go @@ -17,15 +17,15 @@ limitations under the License. package fake import ( + "github.com/Venafi/vcert/pkg/certificate" "time" "github.com/Venafi/vcert/pkg/endpoint" - venafiinternal "github.com/jetstack/cert-manager/pkg/internal/venafi" ) type Venafi struct { PingFn func() error - SignFn func([]byte, time.Duration, []venafiinternal.CustomField) ([]byte, error) + SignFn func([]byte, time.Duration, []certificate.CustomField) ([]byte, error) ReadZoneConfigurationFn func() (*endpoint.ZoneConfiguration, error) } @@ -33,7 +33,7 @@ func (v *Venafi) Ping() error { return v.PingFn() } -func (v *Venafi) Sign(b []byte, t time.Duration, f []venafiinternal.CustomField) ([]byte, error) { +func (v *Venafi) Sign(b []byte, t time.Duration, f []certificate.CustomField) ([]byte, error) { return v.SignFn(b, t, f) } diff --git a/pkg/internal/venafi/sign_test.go b/pkg/internal/venafi/sign_test.go index 8d0081c2e..f7d07bb2b 100644 --- a/pkg/internal/venafi/sign_test.go +++ b/pkg/internal/venafi/sign_test.go @@ -173,6 +173,23 @@ func TestSign(t *testing.T) { checkFn: checkCertificateIssued, expectedErr: false, }, + "obtain a certificate with custom fields specified": { + csrPEM: csrPEM, + customFields: []CustomField{{Name: "test", Value: "ok"}}, + client: internalfake.Connector{ + RetrieveCertificateFunc: func(r *certificate.Request) (*certificate.PEMCollection, error) { + if len(r.CustomFields) == 0 { + return nil, errors.New("custom fields not set") + } + if r.CustomFields[0].Name != "test" || r.CustomFields[0].Value != "ok" { + return nil, errors.New("custom fields content not correct") + } + return internalfake.Connector{}.Default().RetrieveCertificate(r) // hack to return to normal + }, + }.Default(), + checkFn: checkCertificateIssued, + expectedErr: false, + }, } for name, test := range tests { @@ -188,6 +205,8 @@ type testSignT struct { expectedErr bool + customFields []CustomField + checkFn func(*testing.T, []byte, []byte) } @@ -201,7 +220,7 @@ func (s *testSignT) runTest(t *testing.T) { client: client, } - resp, err := v.Sign(s.csrPEM, time.Minute, []certificate.CustomField{}) + resp, err := v.Sign(s.csrPEM, time.Minute, s.customFields) if err != nil && !s.expectedErr { t.Errorf("expected to not get an error, but got: %v", err) } From e040d4f2840cbc9b4a5dd6d8bd1cc9df7a890c0a Mon Sep 17 00:00:00 2001 From: Maartje Eyskens Date: Thu, 6 Feb 2020 10:32:06 +0100 Subject: [PATCH 08/20] Implement feedback Signed-off-by: Maartje Eyskens --- pkg/apis/certmanager/v1alpha2/types.go | 4 +++- pkg/apis/certmanager/v1alpha3/types.go | 4 +++- pkg/controller/certificaterequests/venafi/venafi.go | 7 ++++--- pkg/controller/certificaterequests/venafi/venafi_test.go | 4 ++-- test/unit/gen/certificaterequest.go | 6 ------ 5 files changed, 12 insertions(+), 13 deletions(-) diff --git a/pkg/apis/certmanager/v1alpha2/types.go b/pkg/apis/certmanager/v1alpha2/types.go index eaa85e63e..c6d621e86 100644 --- a/pkg/apis/certmanager/v1alpha2/types.go +++ b/pkg/apis/certmanager/v1alpha2/types.go @@ -101,7 +101,9 @@ const ( // Issuer specific Annotations const ( // VenafiCustomFields is the annotation that passes on JSON encoded custom fields to the Venafi issuer - VenafiCustomFields = "venafi.cert-manager.io/custom-fields" + // The value is an array with objetcs containing the name and value keys + // for example: `[{"name": "custom-field", "value": "custom-value"}]` + VenafiCustomFieldsAnnotationKey = "venafi.cert-manager.io/custom-fields" ) // KeyUsage specifies valid usage contexts for keys. diff --git a/pkg/apis/certmanager/v1alpha3/types.go b/pkg/apis/certmanager/v1alpha3/types.go index 5fbd09339..ec79bcc1c 100644 --- a/pkg/apis/certmanager/v1alpha3/types.go +++ b/pkg/apis/certmanager/v1alpha3/types.go @@ -101,7 +101,9 @@ const ( // Issuer specific Annotations const ( // VenafiCustomFields is the annotation that passes on JSON encoded custom fields to the Venafi issuer - VenafiCustomFields = "venafi.cert-manager.io/custom-fields" + // The value is an array with objetcs containing the name and value keys + // for example: `[{"name": "custom-field", "value": "custom-value"}]` + VenafiCustomFieldsAnnotationKey = "venafi.cert-manager.io/custom-fields" ) // KeyUsage specifies valid usage contexts for keys. diff --git a/pkg/controller/certificaterequests/venafi/venafi.go b/pkg/controller/certificaterequests/venafi/venafi.go index 935b20f36..1850126a6 100644 --- a/pkg/controller/certificaterequests/venafi/venafi.go +++ b/pkg/controller/certificaterequests/venafi/venafi.go @@ -19,6 +19,7 @@ package venafi import ( "context" "encoding/json" + "fmt" "github.com/Venafi/vcert/pkg/endpoint" k8sErrors "k8s.io/apimachinery/pkg/api/errors" @@ -90,15 +91,15 @@ func (v *Venafi) Sign(ctx context.Context, cr *cmapi.CertificateRequest, issuerO duration := apiutil.DefaultCertDuration(cr.Spec.Duration) var customFields []venafiinternal.CustomField - if annotation, exists := cr.GetAnnotations()[cmapi.VenafiCustomFields]; exists && annotation != "" { + if annotation, exists := cr.GetAnnotations()[cmapi.VenafiCustomFieldsAnnotationKey]; exists && annotation != "" { err := json.Unmarshal([]byte(annotation), &customFields) if err != nil { - message := "Failed to parse venafi.cert-manager.io/custom-fields annotation" + message := fmt.Sprintf("Failed to parse %q annotation", cmapi.VenafiCustomFieldsAnnotationKey) v.reporter.Failed(cr, err, "CustomFieldsError", message) log.Error(err, message) - return nil, err + return nil, nil } } diff --git a/pkg/controller/certificaterequests/venafi/venafi_test.go b/pkg/controller/certificaterequests/venafi/venafi_test.go index a354a507f..ba05febbe 100644 --- a/pkg/controller/certificaterequests/venafi/venafi_test.go +++ b/pkg/controller/certificaterequests/venafi/venafi_test.go @@ -146,9 +146,9 @@ func TestSign(t *testing.T) { }), ) - tppCRWithCustomFields := gen.CertificateRequestFrom(tppCR, gen.SetAnnotations(map[string]string{"venafi.cert-manager.io/custom-fields": `[{"name": "cert-manager-test", "value": "test ok"}]`})) + tppCRWithCustomFields := gen.CertificateRequestFrom(tppCR, gen.SetCertificateRequestAnnotations(map[string]string{"venafi.cert-manager.io/custom-fields": `[{"name": "cert-manager-test", "value": "test ok"}]`})) - tppCRWithInvalidCustomFields := gen.CertificateRequestFrom(tppCR, gen.SetAnnotations(map[string]string{"venafi.cert-manager.io/custom-fields": `[{"name": cert-manager-test}]`})) + tppCRWithInvalidCustomFields := gen.CertificateRequestFrom(tppCR, gen.SetCertificateRequestAnnotations(map[string]string{"venafi.cert-manager.io/custom-fields": `[{"name": cert-manager-test}]`})) cloudCR := gen.CertificateRequestFrom(baseCR, gen.SetCertificateRequestIssuer(cmmeta.ObjectReference{ diff --git a/test/unit/gen/certificaterequest.go b/test/unit/gen/certificaterequest.go index cb8075ab6..3ebc8d4b0 100644 --- a/test/unit/gen/certificaterequest.go +++ b/test/unit/gen/certificaterequest.go @@ -139,9 +139,3 @@ func SetCertificateRequestFailureTime(p metav1.Time) CertificateRequestModifier cr.Status.FailureTime = &p } } - -func SetAnnotations(annotations map[string]string) CertificateRequestModifier { - return func(cr *v1alpha2.CertificateRequest) { - cr.SetAnnotations(annotations) - } -} From 1eb4fc6846e7b7e32ca576ab0826d494eaad7785 Mon Sep 17 00:00:00 2001 From: Maartje Eyskens Date: Thu, 6 Feb 2020 11:11:37 +0100 Subject: [PATCH 09/20] Create internalvanafiapi to prevent cyclic imports Signed-off-by: Maartje Eyskens --- .../certificaterequests/venafi/venafi.go | 3 ++- .../certificaterequests/venafi/venafi_test.go | 17 +++++++++-------- pkg/internal/venafi/api/api.go | 12 ++++++++++++ pkg/internal/venafi/fake/venafi.go | 6 +++--- pkg/internal/venafi/sign.go | 3 ++- pkg/internal/venafi/sign_test.go | 5 +++-- pkg/internal/venafi/venafi.go | 10 ++-------- 7 files changed, 33 insertions(+), 23 deletions(-) create mode 100644 pkg/internal/venafi/api/api.go diff --git a/pkg/controller/certificaterequests/venafi/venafi.go b/pkg/controller/certificaterequests/venafi/venafi.go index 1850126a6..5aa28563a 100644 --- a/pkg/controller/certificaterequests/venafi/venafi.go +++ b/pkg/controller/certificaterequests/venafi/venafi.go @@ -20,6 +20,7 @@ import ( "context" "encoding/json" "fmt" + internalvanafiapi "github.com/jetstack/cert-manager/pkg/internal/venafi/api" "github.com/Venafi/vcert/pkg/endpoint" k8sErrors "k8s.io/apimachinery/pkg/api/errors" @@ -90,7 +91,7 @@ func (v *Venafi) Sign(ctx context.Context, cr *cmapi.CertificateRequest, issuerO duration := apiutil.DefaultCertDuration(cr.Spec.Duration) - var customFields []venafiinternal.CustomField + var customFields []internalvanafiapi.CustomField if annotation, exists := cr.GetAnnotations()[cmapi.VenafiCustomFieldsAnnotationKey]; exists && annotation != "" { err := json.Unmarshal([]byte(annotation), &customFields) if err != nil { diff --git a/pkg/controller/certificaterequests/venafi/venafi_test.go b/pkg/controller/certificaterequests/venafi/venafi_test.go index ba05febbe..de9854f41 100644 --- a/pkg/controller/certificaterequests/venafi/venafi_test.go +++ b/pkg/controller/certificaterequests/venafi/venafi_test.go @@ -24,6 +24,7 @@ import ( "crypto/x509/pkix" "encoding/pem" "errors" + internalvanafiapi "github.com/jetstack/cert-manager/pkg/internal/venafi/api" "testing" "time" @@ -181,7 +182,7 @@ func TestSign(t *testing.T) { } clientReturnsPending := &internalvenafifake.Venafi{ - SignFn: func([]byte, time.Duration, []internalvenafi.CustomField) ([]byte, error) { + SignFn: func([]byte, time.Duration, []internalvanafiapi.CustomField) ([]byte, error) { return nil, endpoint.ErrCertificatePending{ CertificateID: "test-cert-id", Status: "test-status-pending", @@ -189,25 +190,25 @@ func TestSign(t *testing.T) { }, } clientReturnsTimeout := &internalvenafifake.Venafi{ - SignFn: func([]byte, time.Duration, []internalvenafi.CustomField) ([]byte, error) { + SignFn: func([]byte, time.Duration, []internalvanafiapi.CustomField) ([]byte, error) { return nil, endpoint.ErrRetrieveCertificateTimeout{ CertificateID: "test-cert-id", } }, } clientReturnsGenericError := &internalvenafifake.Venafi{ - SignFn: func([]byte, time.Duration, []internalvenafi.CustomField) ([]byte, error) { + SignFn: func([]byte, time.Duration, []internalvanafiapi.CustomField) ([]byte, error) { return nil, errors.New("this is an error") }, } clientReturnsCert := &internalvenafifake.Venafi{ - SignFn: func([]byte, time.Duration, []internalvenafi.CustomField) ([]byte, error) { + SignFn: func([]byte, time.Duration, []internalvanafiapi.CustomField) ([]byte, error) { return certPEM, nil }, } clientReturnsCertIfCustomField := &internalvenafifake.Venafi{ - SignFn: func(csr []byte, t time.Duration, fields []internalvenafi.CustomField) ([]byte, error) { + SignFn: func(csr []byte, t time.Duration, fields []internalvanafiapi.CustomField) ([]byte, error) { if len(fields) > 0 && fields[0].Name == "cert-manager-test" && fields[0].Value == "test ok" { return certPEM, nil } @@ -622,7 +623,7 @@ func TestSign(t *testing.T) { builder: &controllertest.Builder{ CertManagerObjects: []runtime.Object{tppCRWithInvalidCustomFields.DeepCopy(), tppIssuer.DeepCopy()}, ExpectedEvents: []string{ - `Warning CustomFieldsError Failed to parse venafi.cert-manager.io/custom-fields annotation: invalid character 'c' looking for beginning of value`, + `Warning CustomFieldsError Failed to parse "venafi.cert-manager.io/custom-fields" annotation: invalid character 'c' looking for beginning of value`, }, ExpectedActions: []testpkg.Action{ testpkg.NewAction(coretesting.NewUpdateSubresourceAction( @@ -634,7 +635,7 @@ func TestSign(t *testing.T) { Type: cmapi.CertificateRequestConditionReady, Status: cmmeta.ConditionFalse, Reason: cmapi.CertificateRequestReasonFailed, - Message: "Failed to parse venafi.cert-manager.io/custom-fields annotation: invalid character 'c' looking for beginning of value", + Message: "Failed to parse \"venafi.cert-manager.io/custom-fields\" annotation: invalid character 'c' looking for beginning of value", LastTransitionTime: &metaFixedClockStart, }), gen.SetCertificateRequestFailureTime(metaFixedClockStart), @@ -644,7 +645,7 @@ func TestSign(t *testing.T) { }, fakeSecretLister: failGetSecretLister, fakeClient: clientReturnsPending, - expectedErr: true, + expectedErr: false, }, } diff --git a/pkg/internal/venafi/api/api.go b/pkg/internal/venafi/api/api.go new file mode 100644 index 000000000..c8d0be001 --- /dev/null +++ b/pkg/internal/venafi/api/api.go @@ -0,0 +1,12 @@ +package api + +import ( + "github.com/Venafi/vcert/pkg/certificate" +) + +// CustomField defines a custom field to be passed to Venafi +type CustomField struct { + Type certificate.CustomFieldType `json:"type,omitempty"` + Name string `json:"name"` + Value string `json:"value"` +} diff --git a/pkg/internal/venafi/fake/venafi.go b/pkg/internal/venafi/fake/venafi.go index 1eaaa8b36..ff58615ec 100644 --- a/pkg/internal/venafi/fake/venafi.go +++ b/pkg/internal/venafi/fake/venafi.go @@ -17,15 +17,15 @@ limitations under the License. package fake import ( - "github.com/Venafi/vcert/pkg/certificate" "time" "github.com/Venafi/vcert/pkg/endpoint" + internalvanafiapi "github.com/jetstack/cert-manager/pkg/internal/venafi/api" ) type Venafi struct { PingFn func() error - SignFn func([]byte, time.Duration, []certificate.CustomField) ([]byte, error) + SignFn func([]byte, time.Duration, []internalvanafiapi.CustomField) ([]byte, error) ReadZoneConfigurationFn func() (*endpoint.ZoneConfiguration, error) } @@ -33,7 +33,7 @@ func (v *Venafi) Ping() error { return v.PingFn() } -func (v *Venafi) Sign(b []byte, t time.Duration, f []certificate.CustomField) ([]byte, error) { +func (v *Venafi) Sign(b []byte, t time.Duration, f []internalvanafiapi.CustomField) ([]byte, error) { return v.SignFn(b, t, f) } diff --git a/pkg/internal/venafi/sign.go b/pkg/internal/venafi/sign.go index 25fc0447b..abd19b1a8 100644 --- a/pkg/internal/venafi/sign.go +++ b/pkg/internal/venafi/sign.go @@ -24,13 +24,14 @@ import ( "github.com/Venafi/vcert/pkg/certificate" + internalvanafiapi "github.com/jetstack/cert-manager/pkg/internal/venafi/api" "github.com/jetstack/cert-manager/pkg/util/pki" ) // This function sends a request to Venafi to for a signed certificate. // 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. -func (v *Venafi) Sign(csrPEM []byte, duration time.Duration, customFields []CustomField) (cert []byte, err error) { +func (v *Venafi) Sign(csrPEM []byte, duration time.Duration, customFields []internalvanafiapi.CustomField) (cert []byte, err error) { // Retrieve a copy of the Venafi zone. // This contains default values and policy control info that we can apply // and check against locally. diff --git a/pkg/internal/venafi/sign_test.go b/pkg/internal/venafi/sign_test.go index f7d07bb2b..f5ed82568 100644 --- a/pkg/internal/venafi/sign_test.go +++ b/pkg/internal/venafi/sign_test.go @@ -23,6 +23,7 @@ import ( "crypto/x509/pkix" "encoding/pem" "errors" + internalvanafiapi "github.com/jetstack/cert-manager/pkg/internal/venafi/api" "testing" "time" @@ -175,7 +176,7 @@ func TestSign(t *testing.T) { }, "obtain a certificate with custom fields specified": { csrPEM: csrPEM, - customFields: []CustomField{{Name: "test", Value: "ok"}}, + customFields: []internalvanafiapi.CustomField{{Name: "test", Value: "ok"}}, client: internalfake.Connector{ RetrieveCertificateFunc: func(r *certificate.Request) (*certificate.PEMCollection, error) { if len(r.CustomFields) == 0 { @@ -205,7 +206,7 @@ type testSignT struct { expectedErr bool - customFields []CustomField + customFields []internalvanafiapi.CustomField checkFn func(*testing.T, []byte, []byte) } diff --git a/pkg/internal/venafi/venafi.go b/pkg/internal/venafi/venafi.go index 75134d242..e5a0994fa 100644 --- a/pkg/internal/venafi/venafi.go +++ b/pkg/internal/venafi/venafi.go @@ -18,6 +18,7 @@ package venafi import ( "fmt" + internalvanafiapi "github.com/jetstack/cert-manager/pkg/internal/venafi/api" "time" "github.com/Venafi/vcert" @@ -39,7 +40,7 @@ type VenafiClientBuilder func(namespace string, secretsLister corelisters.Secret issuer cmapi.GenericIssuer) (Interface, error) type Interface interface { - Sign(csrPEM []byte, duration time.Duration, customFields []CustomField) (cert []byte, err error) + Sign(csrPEM []byte, duration time.Duration, customFields []internalvanafiapi.CustomField) (cert []byte, err error) Ping() error ReadZoneConfiguration() (*endpoint.ZoneConfiguration, error) SetClient(endpoint.Connector) @@ -68,13 +69,6 @@ type connector interface { RenewCertificate(req *certificate.RenewalRequest) (requestID string, err error) } -// CustomField defines a custom field to be passed to Venafi -type CustomField struct { - Type certificate.CustomFieldType `json:"type,omitempty"` - Name string `json:"name"` - Value string `json:"value"` -} - func New(namespace string, secretsLister corelisters.SecretLister, issuer cmapi.GenericIssuer) (Interface, error) { From 292d7f1e61d22744bb89a8981ed30fcf448c0e48 Mon Sep 17 00:00:00 2001 From: Maartje Eyskens Date: Fri, 7 Feb 2020 09:19:24 +0100 Subject: [PATCH 10/20] Update vcert Signed-off-by: Maartje Eyskens --- go.mod | 2 +- go.sum | 4 ++-- hack/build/repos.bzl | 4 ++-- pkg/controller/certificaterequests/venafi/BUILD.bazel | 2 ++ pkg/internal/venafi/BUILD.bazel | 3 +++ pkg/internal/venafi/fake/BUILD.bazel | 1 + 6 files changed, 11 insertions(+), 5 deletions(-) diff --git a/go.mod b/go.mod index 3b143304b..8526c83a5 100644 --- a/go.mod +++ b/go.mod @@ -13,7 +13,7 @@ require ( github.com/Azure/go-autorest/autorest/adal v0.5.0 github.com/Azure/go-autorest/autorest/to v0.3.0 github.com/Azure/go-autorest/autorest/validation v0.2.0 // indirect - github.com/Venafi/vcert v0.0.0-20200120162047-d4d4df569eec + github.com/Venafi/vcert v0.0.0-20200207035730-5a915d73be5d github.com/aws/aws-sdk-go v1.24.1 github.com/cloudflare/cloudflare-go v0.8.5 github.com/cpu/goacmedns v0.0.0-20180701200144-565ecf2a84df diff --git a/go.sum b/go.sum index b55ea2238..f43658e3c 100644 --- a/go.sum +++ b/go.sum @@ -36,8 +36,8 @@ github.com/PuerkitoBio/purell v1.1.1/go.mod h1:c11w/QuzBsJSee3cPx9rAFu61PvFxuPbt github.com/PuerkitoBio/urlesc v0.0.0-20160726150825-5bd2802263f2/go.mod h1:uGdkoq3SwY9Y+13GIhn11/XLaGBb4BfwItxLd5jeuXE= github.com/PuerkitoBio/urlesc v0.0.0-20170810143723-de5bf2ad4578 h1:d+Bc7a5rLufV/sSk/8dngufqelfh6jnri85riMAaF/M= github.com/PuerkitoBio/urlesc v0.0.0-20170810143723-de5bf2ad4578/go.mod h1:uGdkoq3SwY9Y+13GIhn11/XLaGBb4BfwItxLd5jeuXE= -github.com/Venafi/vcert v0.0.0-20200120162047-d4d4df569eec h1:QcsUgs0gP2qltiFPlVXXqlnmRthKPv6WRxCGd7ANutk= -github.com/Venafi/vcert v0.0.0-20200120162047-d4d4df569eec/go.mod h1:5T4bFPhcgGXbdz8nVVRuE2gXSRDlZVL+9T5CwZZ3Yk4= +github.com/Venafi/vcert v0.0.0-20200207035730-5a915d73be5d h1:oFQa1HzZX7Kcnjnosx9wEdVi8ADTDvs+2Tz96HIXrEU= +github.com/Venafi/vcert v0.0.0-20200207035730-5a915d73be5d/go.mod h1:5T4bFPhcgGXbdz8nVVRuE2gXSRDlZVL+9T5CwZZ3Yk4= github.com/agnivade/levenshtein v1.0.1/go.mod h1:CURSv5d9Uaml+FovSIICkLbAUZ9S4RqaHDIsdSBg7lM= github.com/alecthomas/template v0.0.0-20160405071501-a0175ee3bccc/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc= github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0= diff --git a/hack/build/repos.bzl b/hack/build/repos.bzl index 9aca363a8..7357682a6 100644 --- a/hack/build/repos.bzl +++ b/hack/build/repos.bzl @@ -1647,8 +1647,8 @@ def go_repositories(): build_file_generation = "on", build_file_proto_mode = "disable", importpath = "github.com/Venafi/vcert", - sum = "h1:QcsUgs0gP2qltiFPlVXXqlnmRthKPv6WRxCGd7ANutk=", - version = "v0.0.0-20200120162047-d4d4df569eec", + sum = "h1:oFQa1HzZX7Kcnjnosx9wEdVi8ADTDvs+2Tz96HIXrEU=", + version = "v0.0.0-20200207035730-5a915d73be5d", ) go_repository( name = "com_github_xiang90_probing", diff --git a/pkg/controller/certificaterequests/venafi/BUILD.bazel b/pkg/controller/certificaterequests/venafi/BUILD.bazel index 355aa496f..cea41fcec 100644 --- a/pkg/controller/certificaterequests/venafi/BUILD.bazel +++ b/pkg/controller/certificaterequests/venafi/BUILD.bazel @@ -12,6 +12,7 @@ go_library( "//pkg/controller/certificaterequests:go_default_library", "//pkg/controller/certificaterequests/util:go_default_library", "//pkg/internal/venafi:go_default_library", + "//pkg/internal/venafi/api:go_default_library", "//pkg/issuer:go_default_library", "//pkg/logs:go_default_library", "@com_github_venafi_vcert//pkg/endpoint:go_default_library", @@ -46,6 +47,7 @@ go_test( "//pkg/controller/certificaterequests:go_default_library", "//pkg/controller/test:go_default_library", "//pkg/internal/venafi:go_default_library", + "//pkg/internal/venafi/api:go_default_library", "//pkg/internal/venafi/fake:go_default_library", "//pkg/util/pki:go_default_library", "//test/unit/gen:go_default_library", diff --git a/pkg/internal/venafi/BUILD.bazel b/pkg/internal/venafi/BUILD.bazel index 8abfae3bc..7ab70866e 100644 --- a/pkg/internal/venafi/BUILD.bazel +++ b/pkg/internal/venafi/BUILD.bazel @@ -10,6 +10,7 @@ go_library( visibility = ["//pkg:__subpackages__"], deps = [ "//pkg/apis/certmanager/v1alpha2:go_default_library", + "//pkg/internal/venafi/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", @@ -29,6 +30,7 @@ filegroup( name = "all-srcs", srcs = [ ":package-srcs", + "//pkg/internal/venafi/api:all-srcs", "//pkg/internal/venafi/fake:all-srcs", ], tags = ["automanaged"], @@ -45,6 +47,7 @@ go_test( deps = [ "//pkg/apis/certmanager/v1alpha2:go_default_library", "//pkg/apis/meta/v1:go_default_library", + "//pkg/internal/venafi/api:go_default_library", "//pkg/internal/venafi/fake:go_default_library", "//pkg/util:go_default_library", "//pkg/util/pki:go_default_library", diff --git a/pkg/internal/venafi/fake/BUILD.bazel b/pkg/internal/venafi/fake/BUILD.bazel index cf3d07f5e..c1025b6bd 100644 --- a/pkg/internal/venafi/fake/BUILD.bazel +++ b/pkg/internal/venafi/fake/BUILD.bazel @@ -9,6 +9,7 @@ go_library( importpath = "github.com/jetstack/cert-manager/pkg/internal/venafi/fake", visibility = ["//pkg:__subpackages__"], deps = [ + "//pkg/internal/venafi/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", From 5d4054067e446b396af98a73487a5a6b6f5de70a Mon Sep 17 00:00:00 2001 From: Maartje Eyskens Date: Fri, 7 Feb 2020 10:07:46 +0100 Subject: [PATCH 11/20] Add missing bazel file Signed-off-by: Maartje Eyskens --- pkg/internal/venafi/api/BUILD.bazel | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 pkg/internal/venafi/api/BUILD.bazel diff --git a/pkg/internal/venafi/api/BUILD.bazel b/pkg/internal/venafi/api/BUILD.bazel new file mode 100644 index 000000000..6df04e38c --- /dev/null +++ b/pkg/internal/venafi/api/BUILD.bazel @@ -0,0 +1,13 @@ +filegroup( + name = "package-srcs", + srcs = glob(["**"]), + tags = ["automanaged"], + visibility = ["//visibility:private"], +) + +filegroup( + name = "all-srcs", + srcs = [":package-srcs"], + tags = ["automanaged"], + visibility = ["//visibility:public"], +) From b780d78f7545705376b688e062ce1d0a226072d1 Mon Sep 17 00:00:00 2001 From: Maartje Eyskens Date: Fri, 7 Feb 2020 10:55:49 +0100 Subject: [PATCH 12/20] Update Bazel Signed-off-by: Maartje Eyskens --- pkg/internal/venafi/api/BUILD.bazel | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/pkg/internal/venafi/api/BUILD.bazel b/pkg/internal/venafi/api/BUILD.bazel index 6df04e38c..2eb1a194e 100644 --- a/pkg/internal/venafi/api/BUILD.bazel +++ b/pkg/internal/venafi/api/BUILD.bazel @@ -1,3 +1,5 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_library") + filegroup( name = "package-srcs", srcs = glob(["**"]), @@ -11,3 +13,11 @@ filegroup( tags = ["automanaged"], visibility = ["//visibility:public"], ) + +go_library( + name = "go_default_library", + srcs = ["api.go"], + importpath = "github.com/jetstack/cert-manager/pkg/internal/venafi/api", + visibility = ["//pkg:__subpackages__"], + deps = ["@com_github_venafi_vcert//pkg/certificate:go_default_library"], +) From 47368f719b3cffbe40595c96a3d6eeb514b8662d Mon Sep 17 00:00:00 2001 From: Maartje Eyskens Date: Fri, 7 Feb 2020 11:33:59 +0100 Subject: [PATCH 13/20] Add copyright Signed-off-by: Maartje Eyskens --- pkg/internal/venafi/api/api.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/pkg/internal/venafi/api/api.go b/pkg/internal/venafi/api/api.go index c8d0be001..cc574494f 100644 --- a/pkg/internal/venafi/api/api.go +++ b/pkg/internal/venafi/api/api.go @@ -1,3 +1,19 @@ +/* +Copyright 2020 The Jetstack cert-manager contributors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + package api import ( From 1ebc9ef56b7b7d7dd6e29eb6cd3d5c90caadbf60 Mon Sep 17 00:00:00 2001 From: Maartje Eyskens Date: Wed, 12 Feb 2020 10:29:10 +0100 Subject: [PATCH 14/20] Properly sort imports Signed-off-by: Maartje Eyskens --- pkg/controller/certificaterequests/venafi/venafi.go | 2 +- pkg/controller/certificaterequests/venafi/venafi_test.go | 2 +- pkg/internal/venafi/fake/venafi.go | 1 + pkg/internal/venafi/sign_test.go | 2 +- pkg/internal/venafi/venafi.go | 2 +- 5 files changed, 5 insertions(+), 4 deletions(-) diff --git a/pkg/controller/certificaterequests/venafi/venafi.go b/pkg/controller/certificaterequests/venafi/venafi.go index 5aa28563a..40a62d5e0 100644 --- a/pkg/controller/certificaterequests/venafi/venafi.go +++ b/pkg/controller/certificaterequests/venafi/venafi.go @@ -20,7 +20,6 @@ import ( "context" "encoding/json" "fmt" - internalvanafiapi "github.com/jetstack/cert-manager/pkg/internal/venafi/api" "github.com/Venafi/vcert/pkg/endpoint" k8sErrors "k8s.io/apimachinery/pkg/api/errors" @@ -32,6 +31,7 @@ import ( "github.com/jetstack/cert-manager/pkg/controller/certificaterequests" crutil "github.com/jetstack/cert-manager/pkg/controller/certificaterequests/util" venafiinternal "github.com/jetstack/cert-manager/pkg/internal/venafi" + internalvanafiapi "github.com/jetstack/cert-manager/pkg/internal/venafi/api" issuerpkg "github.com/jetstack/cert-manager/pkg/issuer" logf "github.com/jetstack/cert-manager/pkg/logs" ) diff --git a/pkg/controller/certificaterequests/venafi/venafi_test.go b/pkg/controller/certificaterequests/venafi/venafi_test.go index de9854f41..643b248b1 100644 --- a/pkg/controller/certificaterequests/venafi/venafi_test.go +++ b/pkg/controller/certificaterequests/venafi/venafi_test.go @@ -24,7 +24,6 @@ import ( "crypto/x509/pkix" "encoding/pem" "errors" - internalvanafiapi "github.com/jetstack/cert-manager/pkg/internal/venafi/api" "testing" "time" @@ -44,6 +43,7 @@ import ( controllertest "github.com/jetstack/cert-manager/pkg/controller/test" testpkg "github.com/jetstack/cert-manager/pkg/controller/test" internalvenafi "github.com/jetstack/cert-manager/pkg/internal/venafi" + internalvanafiapi "github.com/jetstack/cert-manager/pkg/internal/venafi/api" internalvenafifake "github.com/jetstack/cert-manager/pkg/internal/venafi/fake" "github.com/jetstack/cert-manager/pkg/util/pki" "github.com/jetstack/cert-manager/test/unit/gen" diff --git a/pkg/internal/venafi/fake/venafi.go b/pkg/internal/venafi/fake/venafi.go index ff58615ec..f2f333c1d 100644 --- a/pkg/internal/venafi/fake/venafi.go +++ b/pkg/internal/venafi/fake/venafi.go @@ -20,6 +20,7 @@ import ( "time" "github.com/Venafi/vcert/pkg/endpoint" + internalvanafiapi "github.com/jetstack/cert-manager/pkg/internal/venafi/api" ) diff --git a/pkg/internal/venafi/sign_test.go b/pkg/internal/venafi/sign_test.go index f5ed82568..b0afdca64 100644 --- a/pkg/internal/venafi/sign_test.go +++ b/pkg/internal/venafi/sign_test.go @@ -23,7 +23,6 @@ import ( "crypto/x509/pkix" "encoding/pem" "errors" - internalvanafiapi "github.com/jetstack/cert-manager/pkg/internal/venafi/api" "testing" "time" @@ -31,6 +30,7 @@ import ( "github.com/Venafi/vcert/pkg/endpoint" "github.com/Venafi/vcert/pkg/venafi/fake" + internalvanafiapi "github.com/jetstack/cert-manager/pkg/internal/venafi/api" internalfake "github.com/jetstack/cert-manager/pkg/internal/venafi/fake" "github.com/jetstack/cert-manager/pkg/util" "github.com/jetstack/cert-manager/pkg/util/pki" diff --git a/pkg/internal/venafi/venafi.go b/pkg/internal/venafi/venafi.go index e5a0994fa..b43d5f35d 100644 --- a/pkg/internal/venafi/venafi.go +++ b/pkg/internal/venafi/venafi.go @@ -18,7 +18,6 @@ package venafi import ( "fmt" - internalvanafiapi "github.com/jetstack/cert-manager/pkg/internal/venafi/api" "time" "github.com/Venafi/vcert" @@ -27,6 +26,7 @@ import ( corelisters "k8s.io/client-go/listers/core/v1" cmapi "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1alpha2" + internalvanafiapi "github.com/jetstack/cert-manager/pkg/internal/venafi/api" ) const ( From b3c4dd2ba85c457ccb98280b4d860d2a1fff108e Mon Sep 17 00:00:00 2001 From: Maartje Eyskens Date: Wed, 12 Feb 2020 10:41:19 +0100 Subject: [PATCH 15/20] Implement our own `CustomFieldType` Signed-off-by: Maartje Eyskens --- pkg/internal/venafi/api/api.go | 12 +++++++----- pkg/internal/venafi/sign.go | 15 ++++++++++++++- 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/pkg/internal/venafi/api/api.go b/pkg/internal/venafi/api/api.go index cc574494f..e2e44c661 100644 --- a/pkg/internal/venafi/api/api.go +++ b/pkg/internal/venafi/api/api.go @@ -16,13 +16,15 @@ limitations under the License. package api -import ( - "github.com/Venafi/vcert/pkg/certificate" +type CustomFieldType string + +const ( + CustomFieldTypePlain CustomFieldType = "Plain" ) // CustomField defines a custom field to be passed to Venafi type CustomField struct { - Type certificate.CustomFieldType `json:"type,omitempty"` - Name string `json:"name"` - Value string `json:"value"` + Type CustomFieldType `json:"type,omitempty"` + Name string `json:"name"` + Value string `json:"value"` } diff --git a/pkg/internal/venafi/sign.go b/pkg/internal/venafi/sign.go index abd19b1a8..dd8152825 100644 --- a/pkg/internal/venafi/sign.go +++ b/pkg/internal/venafi/sign.go @@ -52,7 +52,20 @@ func (v *Venafi) Sign(csrPEM []byte, duration time.Duration, customFields []inte if len(customFields) > 0 { vreq.CustomFields = []certificate.CustomField{} for _, field := range customFields { - vreq.CustomFields = append(vreq.CustomFields, certificate.CustomField(field)) + var fieldType certificate.CustomFieldType + switch field.Type { + case internalvanafiapi.CustomFieldTypePlain: + fieldType = certificate.CustomFieldPlain + break + default: + fieldType = certificate.CustomFieldPlain + } + + vreq.CustomFields = append(vreq.CustomFields, certificate.CustomField{ + Type: fieldType, + Name: field.Name, + Value: field.Value, + }) } } From 460189c26aa0b512729949ca796fb56400713fc6 Mon Sep 17 00:00:00 2001 From: Maartje Eyskens Date: Wed, 12 Feb 2020 10:43:21 +0100 Subject: [PATCH 16/20] Fix comment Signed-off-by: Maartje Eyskens --- pkg/apis/certmanager/v1alpha2/types.go | 2 +- pkg/apis/certmanager/v1alpha3/types.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/apis/certmanager/v1alpha2/types.go b/pkg/apis/certmanager/v1alpha2/types.go index c6d621e86..fa5b6864f 100644 --- a/pkg/apis/certmanager/v1alpha2/types.go +++ b/pkg/apis/certmanager/v1alpha2/types.go @@ -100,7 +100,7 @@ const ( // Issuer specific Annotations const ( - // VenafiCustomFields is the annotation that passes on JSON encoded custom fields to the Venafi issuer + // VenafiCustomFieldsAnnotationKey is the annotation that passes on JSON encoded custom fields to the Venafi issuer // The value is an array with objetcs containing the name and value keys // for example: `[{"name": "custom-field", "value": "custom-value"}]` VenafiCustomFieldsAnnotationKey = "venafi.cert-manager.io/custom-fields" diff --git a/pkg/apis/certmanager/v1alpha3/types.go b/pkg/apis/certmanager/v1alpha3/types.go index ec79bcc1c..fffa1bb51 100644 --- a/pkg/apis/certmanager/v1alpha3/types.go +++ b/pkg/apis/certmanager/v1alpha3/types.go @@ -100,7 +100,7 @@ const ( // Issuer specific Annotations const ( - // VenafiCustomFields is the annotation that passes on JSON encoded custom fields to the Venafi issuer + // VenafiCustomFieldsAnnotationKey is the annotation that passes on JSON encoded custom fields to the Venafi issuer // The value is an array with objetcs containing the name and value keys // for example: `[{"name": "custom-field", "value": "custom-value"}]` VenafiCustomFieldsAnnotationKey = "venafi.cert-manager.io/custom-fields" From 09d45e1d281974e977e72145b506fa93b54dbe72 Mon Sep 17 00:00:00 2001 From: Maartje Eyskens Date: Wed, 12 Feb 2020 10:52:47 +0100 Subject: [PATCH 17/20] Error on an invalid custom field type Signed-off-by: Maartje Eyskens --- pkg/internal/venafi/sign.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pkg/internal/venafi/sign.go b/pkg/internal/venafi/sign.go index dd8152825..287a282bb 100644 --- a/pkg/internal/venafi/sign.go +++ b/pkg/internal/venafi/sign.go @@ -57,8 +57,11 @@ func (v *Venafi) Sign(csrPEM []byte, duration time.Duration, customFields []inte case internalvanafiapi.CustomFieldTypePlain: fieldType = certificate.CustomFieldPlain break - default: + case "": fieldType = certificate.CustomFieldPlain + break + default: + return nil, errors.New("certificate request contains an invalid Venafi custom fields type") } vreq.CustomFields = append(vreq.CustomFields, certificate.CustomField{ From d40f0101800d8ce94161a92acd7355daa0cb1f2a Mon Sep 17 00:00:00 2001 From: Maartje Eyskens Date: Wed, 12 Feb 2020 11:34:28 +0100 Subject: [PATCH 18/20] Error on an invalid custom field type Signed-off-by: Maartje Eyskens --- .../certificaterequests/venafi/venafi.go | 6 +++ .../certificaterequests/venafi/venafi_test.go | 38 +++++++++++++++++++ pkg/internal/venafi/sign.go | 12 +++++- pkg/internal/venafi/sign_test.go | 6 +++ 4 files changed, 61 insertions(+), 1 deletion(-) diff --git a/pkg/controller/certificaterequests/venafi/venafi.go b/pkg/controller/certificaterequests/venafi/venafi.go index 40a62d5e0..aff04d131 100644 --- a/pkg/controller/certificaterequests/venafi/venafi.go +++ b/pkg/controller/certificaterequests/venafi/venafi.go @@ -110,6 +110,12 @@ func (v *Venafi) Sign(ctx context.Context, cr *cmapi.CertificateRequest, issuerO if err != nil { switch err.(type) { + case venafiinternal.ErrCustomFieldsType: + v.reporter.Failed(cr, err, "CustomFieldsError", err.Error()) + log.Error(err, err.Error()) + + return nil, nil + case endpoint.ErrCertificatePending: message := "Venafi certificate still in a pending state, the request will be retried" diff --git a/pkg/controller/certificaterequests/venafi/venafi_test.go b/pkg/controller/certificaterequests/venafi/venafi_test.go index 643b248b1..e0ce578e8 100644 --- a/pkg/controller/certificaterequests/venafi/venafi_test.go +++ b/pkg/controller/certificaterequests/venafi/venafi_test.go @@ -151,6 +151,8 @@ func TestSign(t *testing.T) { tppCRWithInvalidCustomFields := gen.CertificateRequestFrom(tppCR, gen.SetCertificateRequestAnnotations(map[string]string{"venafi.cert-manager.io/custom-fields": `[{"name": cert-manager-test}]`})) + tppCRWithInvalidCustomFieldType := gen.CertificateRequestFrom(tppCR, gen.SetCertificateRequestAnnotations(map[string]string{"venafi.cert-manager.io/custom-fields": `[{"name": "cert-manager-test", "value": "test ok", "type": "Bool"}]`})) + cloudCR := gen.CertificateRequestFrom(baseCR, gen.SetCertificateRequestIssuer(cmmeta.ObjectReference{ Group: certmanager.GroupName, @@ -216,6 +218,12 @@ func TestSign(t *testing.T) { }, } + clientReturnsInvalidCustomFieldType := &internalvenafifake.Venafi{ + SignFn: func(csr []byte, t time.Duration, fields []internalvanafiapi.CustomField) ([]byte, error) { + return nil, internalvenafi.ErrCustomFieldsType{Type: fields[0].Type} + }, + } + metaFixedClockStart := metav1.NewTime(fixedClockStart) tests := map[string]testT{ "tpp: if fail to build client based on missing secret then return nil and hard fail": { @@ -647,6 +655,36 @@ func TestSign(t *testing.T) { fakeClient: clientReturnsPending, expectedErr: false, }, + "annotations: Error on invalid type in custom fields": { + certificateRequest: tppCRWithInvalidCustomFieldType.DeepCopy(), + issuer: tppIssuer, + builder: &controllertest.Builder{ + CertManagerObjects: []runtime.Object{tppCRWithInvalidCustomFieldType.DeepCopy(), tppIssuer.DeepCopy()}, + ExpectedEvents: []string{ + `Warning CustomFieldsError certificate request contains an invalid Venafi custom fields type: "Bool": certificate request contains an invalid Venafi custom fields type: "Bool"`, + }, + ExpectedActions: []testpkg.Action{ + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( + cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + "status", + gen.DefaultTestNamespace, + gen.CertificateRequestFrom(tppCRWithInvalidCustomFieldType, + gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ + Type: cmapi.CertificateRequestConditionReady, + Status: cmmeta.ConditionFalse, + Reason: cmapi.CertificateRequestReasonFailed, + Message: "certificate request contains an invalid Venafi custom fields type: \"Bool\": certificate request contains an invalid Venafi custom fields type: \"Bool\"", + LastTransitionTime: &metaFixedClockStart, + }), + gen.SetCertificateRequestFailureTime(metaFixedClockStart), + ), + )), + }, + }, + fakeSecretLister: failGetSecretLister, + fakeClient: clientReturnsInvalidCustomFieldType, + expectedErr: false, + }, } for name, test := range tests { diff --git a/pkg/internal/venafi/sign.go b/pkg/internal/venafi/sign.go index 287a282bb..8015ef890 100644 --- a/pkg/internal/venafi/sign.go +++ b/pkg/internal/venafi/sign.go @@ -19,6 +19,7 @@ package venafi import ( "crypto/x509" "errors" + "fmt" "strings" "time" @@ -28,6 +29,15 @@ import ( "github.com/jetstack/cert-manager/pkg/util/pki" ) +// ErrCustomFieldsType provides a common error structure for a fivalid custom field types +type ErrCustomFieldsType struct { + Type internalvanafiapi.CustomFieldType +} + +func (err ErrCustomFieldsType) Error() string { + return fmt.Sprintf("certificate request contains an invalid Venafi custom fields type: %q", err.Type) +} + // This function sends a request to Venafi to for a signed certificate. // 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. @@ -61,7 +71,7 @@ func (v *Venafi) Sign(csrPEM []byte, duration time.Duration, customFields []inte fieldType = certificate.CustomFieldPlain break default: - return nil, errors.New("certificate request contains an invalid Venafi custom fields type") + return nil, ErrCustomFieldsType{Type: field.Type} } vreq.CustomFields = append(vreq.CustomFields, certificate.CustomField{ diff --git a/pkg/internal/venafi/sign_test.go b/pkg/internal/venafi/sign_test.go index b0afdca64..27e514d65 100644 --- a/pkg/internal/venafi/sign_test.go +++ b/pkg/internal/venafi/sign_test.go @@ -191,6 +191,12 @@ func TestSign(t *testing.T) { checkFn: checkCertificateIssued, expectedErr: false, }, + "If invalid custom field type found the error": { + csrPEM: csrPEM, + customFields: []internalvanafiapi.CustomField{{Name: "test", Value: "ok", Type: "Bool"}}, + checkFn: checkNoCetificateIssued, + expectedErr: true, + }, } for name, test := range tests { From 02fb35bbc92118c95fbecd715f2c653ff5eaad9d Mon Sep 17 00:00:00 2001 From: Maartje Eyskens Date: Wed, 12 Feb 2020 11:37:56 +0100 Subject: [PATCH 19/20] Update bazel Signed-off-by: Maartje Eyskens --- pkg/internal/venafi/api/BUILD.bazel | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/internal/venafi/api/BUILD.bazel b/pkg/internal/venafi/api/BUILD.bazel index 2eb1a194e..4f4e15082 100644 --- a/pkg/internal/venafi/api/BUILD.bazel +++ b/pkg/internal/venafi/api/BUILD.bazel @@ -19,5 +19,4 @@ go_library( srcs = ["api.go"], importpath = "github.com/jetstack/cert-manager/pkg/internal/venafi/api", visibility = ["//pkg:__subpackages__"], - deps = ["@com_github_venafi_vcert//pkg/certificate:go_default_library"], ) From bd45eb2acd6223933282385935028651e65fd65f Mon Sep 17 00:00:00 2001 From: Maartje Eyskens Date: Thu, 13 Feb 2020 08:21:08 +0100 Subject: [PATCH 20/20] Add a note on minimum TPP version Signed-off-by: Maartje Eyskens --- pkg/apis/certmanager/v1alpha2/types.go | 1 + pkg/apis/certmanager/v1alpha3/types.go | 1 + 2 files changed, 2 insertions(+) diff --git a/pkg/apis/certmanager/v1alpha2/types.go b/pkg/apis/certmanager/v1alpha2/types.go index fa5b6864f..da6acc720 100644 --- a/pkg/apis/certmanager/v1alpha2/types.go +++ b/pkg/apis/certmanager/v1alpha2/types.go @@ -101,6 +101,7 @@ const ( // Issuer specific Annotations const ( // VenafiCustomFieldsAnnotationKey is the annotation that passes on JSON encoded custom fields to the Venafi issuer + // This will only work with Venafi TPP v19.3 and higher // The value is an array with objetcs containing the name and value keys // for example: `[{"name": "custom-field", "value": "custom-value"}]` VenafiCustomFieldsAnnotationKey = "venafi.cert-manager.io/custom-fields" diff --git a/pkg/apis/certmanager/v1alpha3/types.go b/pkg/apis/certmanager/v1alpha3/types.go index fffa1bb51..19e499d34 100644 --- a/pkg/apis/certmanager/v1alpha3/types.go +++ b/pkg/apis/certmanager/v1alpha3/types.go @@ -101,6 +101,7 @@ const ( // Issuer specific Annotations const ( // VenafiCustomFieldsAnnotationKey is the annotation that passes on JSON encoded custom fields to the Venafi issuer + // This will only work with Venafi TPP v19.3 and higher // The value is an array with objetcs containing the name and value keys // for example: `[{"name": "custom-field", "value": "custom-value"}]` VenafiCustomFieldsAnnotationKey = "venafi.cert-manager.io/custom-fields"