From adc7cd0f0662166822ee1294236cfc0505bb371c Mon Sep 17 00:00:00 2001 From: Sankalp Yengaldas Date: Wed, 24 Apr 2024 10:14:31 -0400 Subject: [PATCH] add testcases and generate deepcopy methods Signed-off-by: Sankalp Yengaldas --- deploy/crds/crd-clusterissuers.yaml | 22 ++ deploy/crds/crd-issuers.yaml | 22 ++ .../certmanager/v1/zz_generated.conversion.go | 18 ++ .../v1alpha2/zz_generated.conversion.go | 18 ++ .../v1alpha2/zz_generated.deepcopy.go | 5 + .../v1alpha3/zz_generated.conversion.go | 18 ++ .../v1alpha3/zz_generated.deepcopy.go | 5 + .../v1beta1/zz_generated.conversion.go | 18 ++ .../v1beta1/zz_generated.deepcopy.go | 5 + .../apis/certmanager/zz_generated.deepcopy.go | 5 + pkg/apis/certmanager/v1/types_issuer.go | 8 + .../certmanager/v1/zz_generated.deepcopy.go | 5 + pkg/controller/clusterissuers/checks.go | 8 +- pkg/controller/issuers/checks.go | 8 +- pkg/issuer/venafi/client/venaficlient.go | 49 +++- pkg/issuer/venafi/client/venaficlient_test.go | 255 ++++++++++++++++++ 16 files changed, 465 insertions(+), 4 deletions(-) diff --git a/deploy/crds/crd-clusterissuers.yaml b/deploy/crds/crd-clusterissuers.yaml index bebebfdeb..1ec9ca137 100644 --- a/deploy/crds/crd-clusterissuers.yaml +++ b/deploy/crds/crd-clusterissuers.yaml @@ -2247,6 +2247,28 @@ spec: is used to validate the chain. type: string format: byte + caBundleSecretRef: + description: |- + Reference to a Secret containing a base64-encoded bundle of PEM CAs + which will be used to validate the certificate chain presented by the TPP server. + Only used if using HTTPS; ignored for HTTP. Mutually exclusive with CABundle. + If neither CABundle nor CABundleSecretRef is defined, the certificate bundle in + the cert-manager controller container is used to validate the TLS connection. + type: object + required: + - name + properties: + key: + description: |- + The key of the entry in the Secret resource's `data` field to be used. + Some instances of this field may be defaulted, in others it may be + required. + type: string + name: + description: |- + Name of the resource being referred to. + More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + type: string credentialsRef: description: |- CredentialsRef is a reference to a Secret containing the username and diff --git a/deploy/crds/crd-issuers.yaml b/deploy/crds/crd-issuers.yaml index af9f71ee1..80ce56b54 100644 --- a/deploy/crds/crd-issuers.yaml +++ b/deploy/crds/crd-issuers.yaml @@ -2247,6 +2247,28 @@ spec: is used to validate the chain. type: string format: byte + caBundleSecretRef: + description: |- + Reference to a Secret containing a base64-encoded bundle of PEM CAs + which will be used to validate the certificate chain presented by the TPP server. + Only used if using HTTPS; ignored for HTTP. Mutually exclusive with CABundle. + If neither CABundle nor CABundleSecretRef is defined, the certificate bundle in + the cert-manager controller container is used to validate the TLS connection. + type: object + required: + - name + properties: + key: + description: |- + The key of the entry in the Secret resource's `data` field to be used. + Some instances of this field may be defaulted, in others it may be + required. + type: string + name: + description: |- + Name of the resource being referred to. + More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + type: string credentialsRef: description: |- CredentialsRef is a reference to a Secret containing the username and diff --git a/internal/apis/certmanager/v1/zz_generated.conversion.go b/internal/apis/certmanager/v1/zz_generated.conversion.go index ed4d3b3c1..6a2447f14 100644 --- a/internal/apis/certmanager/v1/zz_generated.conversion.go +++ b/internal/apis/certmanager/v1/zz_generated.conversion.go @@ -1735,6 +1735,15 @@ func autoConvert_v1_VenafiTPP_To_certmanager_VenafiTPP(in *v1.VenafiTPP, out *ce return err } out.CABundle = *(*[]byte)(unsafe.Pointer(&in.CABundle)) + if in.CABundleSecretRef != nil { + in, out := &in.CABundleSecretRef, &out.CABundleSecretRef + *out = new(meta.SecretKeySelector) + if err := internalapismetav1.Convert_v1_SecretKeySelector_To_meta_SecretKeySelector(*in, *out, s); err != nil { + return err + } + } else { + out.CABundleSecretRef = nil + } return nil } @@ -1749,6 +1758,15 @@ func autoConvert_certmanager_VenafiTPP_To_v1_VenafiTPP(in *certmanager.VenafiTPP return err } out.CABundle = *(*[]byte)(unsafe.Pointer(&in.CABundle)) + if in.CABundleSecretRef != nil { + in, out := &in.CABundleSecretRef, &out.CABundleSecretRef + *out = new(apismetav1.SecretKeySelector) + if err := internalapismetav1.Convert_meta_SecretKeySelector_To_v1_SecretKeySelector(*in, *out, s); err != nil { + return err + } + } else { + out.CABundleSecretRef = nil + } return nil } diff --git a/internal/apis/certmanager/v1alpha2/zz_generated.conversion.go b/internal/apis/certmanager/v1alpha2/zz_generated.conversion.go index b07a368cb..49eb80871 100644 --- a/internal/apis/certmanager/v1alpha2/zz_generated.conversion.go +++ b/internal/apis/certmanager/v1alpha2/zz_generated.conversion.go @@ -1736,6 +1736,15 @@ func autoConvert_v1alpha2_VenafiTPP_To_certmanager_VenafiTPP(in *VenafiTPP, out return err } out.CABundle = *(*[]byte)(unsafe.Pointer(&in.CABundle)) + if in.CABundleSecretRef != nil { + in, out := &in.CABundleSecretRef, &out.CABundleSecretRef + *out = new(meta.SecretKeySelector) + if err := apismetav1.Convert_v1_SecretKeySelector_To_meta_SecretKeySelector(*in, *out, s); err != nil { + return err + } + } else { + out.CABundleSecretRef = nil + } return nil } @@ -1750,6 +1759,15 @@ func autoConvert_certmanager_VenafiTPP_To_v1alpha2_VenafiTPP(in *certmanager.Ven return err } out.CABundle = *(*[]byte)(unsafe.Pointer(&in.CABundle)) + if in.CABundleSecretRef != nil { + in, out := &in.CABundleSecretRef, &out.CABundleSecretRef + *out = new(metav1.SecretKeySelector) + if err := apismetav1.Convert_meta_SecretKeySelector_To_v1_SecretKeySelector(*in, *out, s); err != nil { + return err + } + } else { + out.CABundleSecretRef = nil + } return nil } diff --git a/internal/apis/certmanager/v1alpha2/zz_generated.deepcopy.go b/internal/apis/certmanager/v1alpha2/zz_generated.deepcopy.go index 37c3397ca..070d64519 100644 --- a/internal/apis/certmanager/v1alpha2/zz_generated.deepcopy.go +++ b/internal/apis/certmanager/v1alpha2/zz_generated.deepcopy.go @@ -1100,6 +1100,11 @@ func (in *VenafiTPP) DeepCopyInto(out *VenafiTPP) { *out = make([]byte, len(*in)) copy(*out, *in) } + if in.CABundleSecretRef != nil { + in, out := &in.CABundleSecretRef, &out.CABundleSecretRef + *out = new(metav1.SecretKeySelector) + **out = **in + } return } diff --git a/internal/apis/certmanager/v1alpha3/zz_generated.conversion.go b/internal/apis/certmanager/v1alpha3/zz_generated.conversion.go index c0d2b8aeb..6d8661bf3 100644 --- a/internal/apis/certmanager/v1alpha3/zz_generated.conversion.go +++ b/internal/apis/certmanager/v1alpha3/zz_generated.conversion.go @@ -1735,6 +1735,15 @@ func autoConvert_v1alpha3_VenafiTPP_To_certmanager_VenafiTPP(in *VenafiTPP, out return err } out.CABundle = *(*[]byte)(unsafe.Pointer(&in.CABundle)) + if in.CABundleSecretRef != nil { + in, out := &in.CABundleSecretRef, &out.CABundleSecretRef + *out = new(meta.SecretKeySelector) + if err := apismetav1.Convert_v1_SecretKeySelector_To_meta_SecretKeySelector(*in, *out, s); err != nil { + return err + } + } else { + out.CABundleSecretRef = nil + } return nil } @@ -1749,6 +1758,15 @@ func autoConvert_certmanager_VenafiTPP_To_v1alpha3_VenafiTPP(in *certmanager.Ven return err } out.CABundle = *(*[]byte)(unsafe.Pointer(&in.CABundle)) + if in.CABundleSecretRef != nil { + in, out := &in.CABundleSecretRef, &out.CABundleSecretRef + *out = new(metav1.SecretKeySelector) + if err := apismetav1.Convert_meta_SecretKeySelector_To_v1_SecretKeySelector(*in, *out, s); err != nil { + return err + } + } else { + out.CABundleSecretRef = nil + } return nil } diff --git a/internal/apis/certmanager/v1alpha3/zz_generated.deepcopy.go b/internal/apis/certmanager/v1alpha3/zz_generated.deepcopy.go index 3e02b1843..96224d32d 100644 --- a/internal/apis/certmanager/v1alpha3/zz_generated.deepcopy.go +++ b/internal/apis/certmanager/v1alpha3/zz_generated.deepcopy.go @@ -1095,6 +1095,11 @@ func (in *VenafiTPP) DeepCopyInto(out *VenafiTPP) { *out = make([]byte, len(*in)) copy(*out, *in) } + if in.CABundleSecretRef != nil { + in, out := &in.CABundleSecretRef, &out.CABundleSecretRef + *out = new(metav1.SecretKeySelector) + **out = **in + } return } diff --git a/internal/apis/certmanager/v1beta1/zz_generated.conversion.go b/internal/apis/certmanager/v1beta1/zz_generated.conversion.go index 8df77513b..883e0bfab 100644 --- a/internal/apis/certmanager/v1beta1/zz_generated.conversion.go +++ b/internal/apis/certmanager/v1beta1/zz_generated.conversion.go @@ -1723,6 +1723,15 @@ func autoConvert_v1beta1_VenafiTPP_To_certmanager_VenafiTPP(in *VenafiTPP, out * return err } out.CABundle = *(*[]byte)(unsafe.Pointer(&in.CABundle)) + if in.CABundleSecretRef != nil { + in, out := &in.CABundleSecretRef, &out.CABundleSecretRef + *out = new(meta.SecretKeySelector) + if err := apismetav1.Convert_v1_SecretKeySelector_To_meta_SecretKeySelector(*in, *out, s); err != nil { + return err + } + } else { + out.CABundleSecretRef = nil + } return nil } @@ -1737,6 +1746,15 @@ func autoConvert_certmanager_VenafiTPP_To_v1beta1_VenafiTPP(in *certmanager.Vena return err } out.CABundle = *(*[]byte)(unsafe.Pointer(&in.CABundle)) + if in.CABundleSecretRef != nil { + in, out := &in.CABundleSecretRef, &out.CABundleSecretRef + *out = new(metav1.SecretKeySelector) + if err := apismetav1.Convert_meta_SecretKeySelector_To_v1_SecretKeySelector(*in, *out, s); err != nil { + return err + } + } else { + out.CABundleSecretRef = nil + } return nil } diff --git a/internal/apis/certmanager/v1beta1/zz_generated.deepcopy.go b/internal/apis/certmanager/v1beta1/zz_generated.deepcopy.go index 59492f6ae..d71b07875 100644 --- a/internal/apis/certmanager/v1beta1/zz_generated.deepcopy.go +++ b/internal/apis/certmanager/v1beta1/zz_generated.deepcopy.go @@ -1095,6 +1095,11 @@ func (in *VenafiTPP) DeepCopyInto(out *VenafiTPP) { *out = make([]byte, len(*in)) copy(*out, *in) } + if in.CABundleSecretRef != nil { + in, out := &in.CABundleSecretRef, &out.CABundleSecretRef + *out = new(metav1.SecretKeySelector) + **out = **in + } return } diff --git a/internal/apis/certmanager/zz_generated.deepcopy.go b/internal/apis/certmanager/zz_generated.deepcopy.go index 49380ee89..0a698cb6d 100644 --- a/internal/apis/certmanager/zz_generated.deepcopy.go +++ b/internal/apis/certmanager/zz_generated.deepcopy.go @@ -1095,6 +1095,11 @@ func (in *VenafiTPP) DeepCopyInto(out *VenafiTPP) { *out = make([]byte, len(*in)) copy(*out, *in) } + if in.CABundleSecretRef != nil { + in, out := &in.CABundleSecretRef, &out.CABundleSecretRef + *out = new(meta.SecretKeySelector) + **out = **in + } return } diff --git a/pkg/apis/certmanager/v1/types_issuer.go b/pkg/apis/certmanager/v1/types_issuer.go index 57645c5c0..714cf1f9e 100644 --- a/pkg/apis/certmanager/v1/types_issuer.go +++ b/pkg/apis/certmanager/v1/types_issuer.go @@ -160,6 +160,14 @@ type VenafiTPP struct { // is used to validate the chain. // +optional CABundle []byte `json:"caBundle,omitempty"` + + // Reference to a Secret containing a base64-encoded bundle of PEM CAs + // which will be used to validate the certificate chain presented by the TPP server. + // Only used if using HTTPS; ignored for HTTP. Mutually exclusive with CABundle. + // If neither CABundle nor CABundleSecretRef is defined, the certificate bundle in + // the cert-manager controller container is used to validate the TLS connection. + // +optional + CABundleSecretRef *cmmeta.SecretKeySelector `json:"caBundleSecretRef,omitempty"` } // VenafiCloud defines connection configuration details for Venafi Cloud diff --git a/pkg/apis/certmanager/v1/zz_generated.deepcopy.go b/pkg/apis/certmanager/v1/zz_generated.deepcopy.go index 27331ba59..31bc04984 100644 --- a/pkg/apis/certmanager/v1/zz_generated.deepcopy.go +++ b/pkg/apis/certmanager/v1/zz_generated.deepcopy.go @@ -1095,6 +1095,11 @@ func (in *VenafiTPP) DeepCopyInto(out *VenafiTPP) { *out = make([]byte, len(*in)) copy(*out, *in) } + if in.CABundleSecretRef != nil { + in, out := &in.CABundleSecretRef, &out.CABundleSecretRef + *out = new(apismetav1.SecretKeySelector) + **out = **in + } return } diff --git a/pkg/controller/clusterissuers/checks.go b/pkg/controller/clusterissuers/checks.go index 705b2b7ae..808162560 100644 --- a/pkg/controller/clusterissuers/checks.go +++ b/pkg/controller/clusterissuers/checks.go @@ -28,7 +28,7 @@ func (c *controller) issuersForSecret(secret *corev1.Secret) ([]*v1.ClusterIssue issuers, err := c.clusterIssuerLister.List(labels.NewSelector()) if err != nil { - return nil, fmt.Errorf("error listing certificates: %s", err.Error()) + return nil, fmt.Errorf("error listing issuers: %s", err.Error()) } var affected []*v1.ClusterIssuer @@ -60,6 +60,12 @@ func (c *controller) issuersForSecret(secret *corev1.Secret) ([]*v1.ClusterIssue continue } } + if iss.Spec.Venafi.TPP.CABundleSecretRef != nil { + if iss.Spec.Venafi.TPP.CABundleSecretRef.Name == secret.Name { + affected = append(affected, iss) + continue + } + } if iss.Spec.Venafi.Cloud != nil { if iss.Spec.Venafi.Cloud.APITokenSecretRef.Name == secret.Name { affected = append(affected, iss) diff --git a/pkg/controller/issuers/checks.go b/pkg/controller/issuers/checks.go index 02c3dcf81..c19970f14 100644 --- a/pkg/controller/issuers/checks.go +++ b/pkg/controller/issuers/checks.go @@ -28,7 +28,7 @@ func (c *controller) issuersForSecret(secret *corev1.Secret) ([]*v1.Issuer, erro issuers, err := c.issuerLister.List(labels.NewSelector()) if err != nil { - return nil, fmt.Errorf("error listing certificates: %s", err.Error()) + return nil, fmt.Errorf("error listing issuers: %s", err.Error()) } var affected []*v1.Issuer @@ -62,6 +62,12 @@ func (c *controller) issuersForSecret(secret *corev1.Secret) ([]*v1.Issuer, erro continue } } + if iss.Spec.Venafi.TPP.CABundleSecretRef != nil { + if iss.Spec.Venafi.TPP.CABundleSecretRef.Name == secret.Name { + affected = append(affected, iss) + continue + } + } if iss.Spec.Venafi.Cloud != nil { if iss.Spec.Venafi.Cloud.APITokenSecretRef.Name == secret.Name { affected = append(affected, iss) diff --git a/pkg/issuer/venafi/client/venaficlient.go b/pkg/issuer/venafi/client/venaficlient.go index be42f4a61..da622f5d1 100644 --- a/pkg/issuer/venafi/client/venaficlient.go +++ b/pkg/issuer/venafi/client/venaficlient.go @@ -34,6 +34,7 @@ import ( internalinformers "github.com/cert-manager/cert-manager/internal/informers" cmapi "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" + cmmeta "github.com/cert-manager/cert-manager/pkg/apis/meta/v1" "github.com/cert-manager/cert-manager/pkg/issuer/venafi/client/api" "github.com/cert-manager/cert-manager/pkg/metrics" "github.com/cert-manager/cert-manager/pkg/util" @@ -139,6 +140,8 @@ func configForIssuer(iss cmapi.GenericIssuer, secretsLister internalinformers.Se return nil, err } + caBundle, err := caBundleForVcertTPP(tpp, secretsLister, namespace) + username := string(tppSecret.Data[tppUsernameKey]) password := string(tppSecret.Data[tppPasswordKey]) accessToken := string(tppSecret.Data[tppAccessTokenKey]) @@ -156,7 +159,7 @@ func configForIssuer(iss cmapi.GenericIssuer, secretsLister internalinformers.Se // below. But we want to retain the CA bundle validation errors that // were returned in previous versions of this code. // https://github.com/Venafi/vcert/blob/89645a7710a7b529765274cb60dc5e28066217a1/client.go#L55-L61 - ConnectionTrust: string(tpp.CABundle), + ConnectionTrust: string(caBundle), Credentials: &endpoint.Authentication{ User: username, Password: password, @@ -164,7 +167,7 @@ func configForIssuer(iss cmapi.GenericIssuer, secretsLister internalinformers.Se }, Client: httpClientForVcert(&httpClientForVcertOptions{ UserAgent: ptr.To(userAgent), - CABundle: tpp.CABundle, + CABundle: caBundle, TLSRenegotiationSupport: ptr.To(tls.RenegotiateOnceAsClient), }), }, nil @@ -312,6 +315,48 @@ func httpClientForVcert(options *httpClientForVcertOptions) *http.Client { } } +// caBundleForVcertTPP is used to by ConnectionTrust and Client fields of vcert.Config. +// This function sets appropriate CA based on provided bundle or kubernetes secret +// If no custom CA bundle is configured, an empty byte slice is returned. +// Assumes exactly one of the in-line/Secret CA bundles are defined. +// If the `key` of the Secret CA bundle is not defined, its value defaults to +// `ca.crt`. +func caBundleForVcertTPP(tpp *cmapi.VenafiTPP, secretsLister internalinformers.SecretLister, namespace string) (caBundle []byte, err error) { + if len(tpp.CABundle) > 0 { + return tpp.CABundle, nil + } + + secretRef := tpp.CABundleSecretRef + if secretRef == nil { + return nil, nil + } + + var certBytes []byte + var ok bool + + if secretRef != nil { + secret, err := secretsLister.Secrets(namespace).Get(secretRef.Name) + if err != nil { + return nil, fmt.Errorf("could not access secret '%s/%s': %s", namespace, secretRef.Name, err) + } + + var key string + if secretRef.Key != "" { + key = secretRef.Key + } else { + key = cmmeta.TLSCAKey + } + + certBytes, ok = secret.Data[key] + if !ok { + return nil, fmt.Errorf("no data for %q in secret '%s/%s'", key, namespace, secretRef.Name) + } + + } + + return certBytes, nil +} + func (v *Venafi) Ping() error { return v.vcertClient.Ping() } diff --git a/pkg/issuer/venafi/client/venaficlient_test.go b/pkg/issuer/venafi/client/venaficlient_test.go index 9a44ea042..93bec50e1 100644 --- a/pkg/issuer/venafi/client/venaficlient_test.go +++ b/pkg/issuer/venafi/client/venaficlient_test.go @@ -31,6 +31,50 @@ import ( testlisters "github.com/cert-manager/cert-manager/test/unit/listers" ) +const ( + zone = "test-zone" + username = "test-username" + password = "test-password" + accessToken = "KT2EEVTIjWM/37L78dqJAg==" + apiKey = "test-api-key" + customKey = "test-custom-key" + defaultCaKey = "ca.crt" + customCaKey = "custom-ca-key" + tppUrl = "https://tpp.example.com/vedsdk" + customCaSecretName = "custom-ca-secret" + testLeafCertificate = `-----BEGIN CERTIFICATE----- +MIIFFTCCAv2gAwIBAgICEAAwDQYJKoZIhvcNAQELBQAwRjELMAkGA1UEBhMCVVMx +CzAJBgNVBAgMAkNBMRQwEgYDVQQKDAtDRVJUTUFOQUdFUjEUMBIGA1UEAwwLZm9v +LmJhci5pbnQwHhcNMjAxMDAyMTQ1NzMwWhcNMjExMDEyMTQ1NzMwWjBKMQswCQYD +VQQGEwJVUzELMAkGA1UECAwCQ0ExFDASBgNVBAoMC0NFUlRNQU5BR0VSMRgwFgYD +VQQDDA9leGFtcGxlLmZvby5iYXIwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEK +AoIBAQC8yTGzYIX3OoRma11vewbNf8dgKHc9GgvJJ29SVjaNwRAJjKOXokGOwcyQ +7Ieb1puYQ5KdSPC1IxyUx77URovIvd3Wql+J1gIxyrdN3om3uQdJ2ck6xatBZ8BI +Y3Z+6WpUQ2067Wk4KpUGfMrbGg5zVcesh6zc8J9yEiItUENeR+6GyEf+B8IJ0xqe +5lps2LaxZp6I6vaKeMELjj17Nb9r81Rjyk8BN7yX74tFE1mUGX9o75tsODU9IrYW +nqSl5gr2PO9Zb/bd6zhoncLJr9kj2tk6cLRPht+JOPoA2LAP6D0aEdC3a2XWuj2E +EsUYJR9e5C/X49VQaak0VdNnhO6RAgMBAAGjggEHMIIBAzAJBgNVHRMEAjAAMBEG +CWCGSAGG+EIBAQQEAwIGQDAzBglghkgBhvhCAQ0EJhYkT3BlblNTTCBHZW5lcmF0 +ZWQgU2VydmVyIENlcnRpZmljYXRlMB0GA1UdDgQWBBQ41U/GiA2rQtuMz6tNL55C +o4pnBDBqBgNVHSMEYzBhgBSfus9cb7UA/PCfHJAGtL6ot2EpLKFFpEMwQTEPMA0G +A1UEAwwGYmFyLmNhMQswCQYDVQQGEwJVUzELMAkGA1UECAwCQ0ExFDASBgNVBAoM +C0NFUlRNQU5BR0VSggIQADAOBgNVHQ8BAf8EBAMCBaAwEwYDVR0lBAwwCgYIKwYB +BQUHAwEwDQYJKoZIhvcNAQELBQADggIBAFFTJNqKSkkJNWWt+R7WFNIEKoaPcFH5 +yupCQRYX9LK2cXdBQpF458/PFxREyt5jKFUcDyzQhOglFYq0hfcoAc2EB3Vw8Ww9 +c4QCiCU6ehJVMRt7MzZ9uUVGCRVOA+Fa1tIFfL3dKlI+4pTSbDhNHRqDtFhfWOZK +bgtruQEUOW1lQR61AsidOF1iwDBU6ckpVY9Lc2SHEAfQFs0MoXmJ8B4MqFptF4+H +al+IAeQ1bC/2EccFYg3tq9+YKHDCyghHf8qeKJR9tZslvkHrAzuX56e0MHxM3AD6 +D0L8nG3DsrHcjK0MlVUWmq0QFnY5t+78iocLoQZzpILZYuZn3p+XNlUdW4lcqSBn +y5fUwQ3RIuvN66GBhTeDV4vzYPa7g3i9PoBFoG50Ayr6VtIVn08rnl03lgp57Edv +A5oRrSHcd8Hd8/lk0Y9BpFTnZEg7RLhFhh9nazVp1/pjwaGx449uHIGEoxREQoPq +9Q+KLGMJR2IqiNI6+U1z2j8BChTOPkuAvsnSuAXyotu4BXBL5zbDzfDoggEk1ps1 +bfHWnmdelE0WP7h7B0PSA0EXn0pdg2VQIQsknV6y3MCzFQCCSAog/OSguokXG1PG +l6fctDJ3+AF07EjtgArOBkUn7Nt3/CgMN8I1rnBZ1Vmd8yrHEP0E3yRXBL7cDj5j +Fqmd89NQLlGs +-----END CERTIFICATE----- +` +) + func checkNoConfigReturned(t *testing.T, cnf *vcert.Config) { if cnf != nil { t.Errorf("expected no config to be returned, got=%+v", cnf) @@ -48,6 +92,28 @@ func checkZone(t *testing.T, zone string, cnf *vcert.Config) { } } +func checkTppUrl(t *testing.T, tppUrl string, cnf *vcert.Config) { + if cnf == nil { + t.Errorf("expected config but got: %+v", cnf) + } + + if tppUrl != cnf.BaseUrl { + t.Errorf("got unexpected BaseUrl set, exp=%s got=%s", + tppUrl, cnf.BaseUrl) + } +} + +func checkTppCa(t *testing.T, ca string, cnf *vcert.Config) { + if cnf == nil { + t.Errorf("expected config but got: %+v", cnf) + } + + if ca != cnf.ConnectionTrust { + t.Errorf("got unexpected CA as trust, exp=%s got=%s", + ca, cnf.ConnectionTrust) + } +} + func generateSecretLister(s *corev1.Secret, err error) internalinformers.SecretLister { return &testlisters.FakeSecretLister{ SecretsFn: func(string) corelisters.SecretNamespaceLister { @@ -79,6 +145,36 @@ func TestConfigForIssuerT(t *testing.T) { }), ) + tppIssuerWithoutCA := gen.IssuerFrom(baseIssuer, + gen.SetIssuerVenafi(cmapi.VenafiIssuer{ + Zone: zone, + TPP: &cmapi.VenafiTPP{ + URL: tppUrl, + }, + }), + ) + + tppIssuerWithCABundle := gen.IssuerFrom(tppIssuerWithoutCA, + gen.SetIssuerVenafi(cmapi.VenafiIssuer{ + TPP: &cmapi.VenafiTPP{ + CABundle: []byte(testLeafCertificate), + }, + }), + ) + + tppIssuerWithCABundleSecretRef := gen.IssuerFrom(tppIssuer, + gen.SetIssuerVenafi(cmapi.VenafiIssuer{ + TPP: &cmapi.VenafiTPP{ + CABundleSecretRef: &cmmeta.SecretKeySelector{ + Key: customCaKey, + LocalObjectReference: cmmeta.LocalObjectReference{ + Name: customCaSecretName, + }, + }, + }, + }), + ) + cloudIssuer := gen.IssuerFrom(baseIssuer, gen.SetIssuerVenafi(cmapi.VenafiIssuer{ Zone: zone, @@ -109,6 +205,22 @@ func TestConfigForIssuerT(t *testing.T) { CheckFn: checkNoConfigReturned, expectedErr: true, }, + "if TPP and neither caBundle nor caBundleSecretRef is specified, CA bundle is not set in vcert config": { + iss: tppIssuerWithoutCA, + secretsLister: generateSecretLister(&corev1.Secret{ + Data: map[string][]byte{ + tppUsernameKey: []byte(username), + tppPasswordKey: []byte(password), + }, + }, nil), + CheckFn: func(t *testing.T, cnf *vcert.Config) { + if trust := cnf.ConnectionTrust; trust != "" { + t.Errorf("got unexpected CA bundle: %s", trust) + } + checkTppUrl(t, tppUrl, cnf) + }, + expectedErr: false, + }, "if TPP and secret returns user/pass, should return config with those credentials": { iss: tppIssuer, secretsLister: generateSecretLister(&corev1.Secret{ @@ -143,6 +255,32 @@ func TestConfigForIssuerT(t *testing.T) { }, expectedErr: false, }, + // NOTE: Below scenarios assume valid TPP CAs, the scenarios with invalid TPP CAs are run part of TestCaBundleForVcertTPP test + "if TPP and a good caBundle specified, CA bundle should be added to ConnectionTrust and Client in vcert config": { + iss: tppIssuerWithCABundle, + secretsLister: generateSecretLister(&corev1.Secret{ + Data: map[string][]byte{ + tppAccessTokenKey: []byte(accessToken), + }, + }, nil), + CheckFn: func(t *testing.T, cnf *vcert.Config) { + checkTppCa(t, testLeafCertificate, cnf) + }, + expectedErr: false, + }, + "if TPP and a good caBundleSecretRef specified, CA bundle should be added to ConnectionTrust and Client in vcert config": { + iss: tppIssuerWithCABundleSecretRef, + // tppAccessTokenKey secret lister is not passed as we only have single secretsLister in testConfigForIssuerT struck + secretsLister: generateSecretLister(&corev1.Secret{ + Data: map[string][]byte{ + customCaKey: []byte(testLeafCertificate), + }, + }, nil), + CheckFn: func(t *testing.T, cnf *vcert.Config) { + checkTppCa(t, testLeafCertificate, cnf) + }, + expectedErr: false, + }, "if Cloud but getting secret fails, should error": { iss: cloudIssuer, secretsLister: generateSecretLister(nil, errors.New("this is a network error")), @@ -213,9 +351,108 @@ func TestConfigForIssuerT(t *testing.T) { } } +func TestCaBundleForVcertTPP(t *testing.T) { + baseIssuer := gen.Issuer("non-venafi-issue", + gen.SetIssuerVenafi(cmapi.VenafiIssuer{}), + ) + + tppIssuer := gen.IssuerFrom(baseIssuer, + gen.SetIssuerVenafi(cmapi.VenafiIssuer{ + Zone: zone, + TPP: &cmapi.VenafiTPP{}, + }), + ) + + tppIssuerWithCABundle := gen.IssuerFrom(tppIssuer, + gen.SetIssuerVenafi(cmapi.VenafiIssuer{ + TPP: &cmapi.VenafiTPP{ + CABundle: []byte(testLeafCertificate), + }, + }), + ) + + tppIssuerWithCABundleSecretRefNoKey := gen.IssuerFrom(tppIssuer, + gen.SetIssuerVenafi(cmapi.VenafiIssuer{ + TPP: &cmapi.VenafiTPP{ + CABundleSecretRef: &cmmeta.SecretKeySelector{ + LocalObjectReference: cmmeta.LocalObjectReference{ + Name: customCaSecretName, + }, + }, + }, + }), + ) + + tppIssuerWithCABundleSecretRef := gen.IssuerFrom(tppIssuer, + gen.SetIssuerVenafi(cmapi.VenafiIssuer{ + TPP: &cmapi.VenafiTPP{ + CABundleSecretRef: &cmmeta.SecretKeySelector{ + Key: customCaKey, + LocalObjectReference: cmmeta.LocalObjectReference{ + Name: customCaSecretName, + }, + }, + }, + }), + ) + + tests := map[string]testConfigForIssuerT{ + "if TPP and neither of caBundle nor caBundleSecretRef is specified, CA bundle is not returned": { + iss: tppIssuer, + caBundle: "", + expectedErr: false, + }, + "if TPP and caBundle is specified, correct CA bundle from CABundle should be returned": { + iss: tppIssuerWithCABundle, + caBundle: testLeafCertificate, + expectedErr: false, + }, + "if TPP and caBundleSecretRef is specified, correct CA bundle from CABundleSecretRef should be returned": { + iss: tppIssuerWithCABundleSecretRef, + caBundle: testLeafCertificate, + secretsLister: generateSecretLister(&corev1.Secret{ + Data: map[string][]byte{ + customCaKey: []byte(testLeafCertificate), + }, + }, nil), + expectedErr: false, + }, + "if TPP and caBundleSecretRef is specified without `key`, correct CA bundle from CABundleSecretRef with default key should be returned": { + iss: tppIssuerWithCABundleSecretRefNoKey, + caBundle: testLeafCertificate, + secretsLister: generateSecretLister(&corev1.Secret{ + Data: map[string][]byte{ + defaultCaKey: []byte(testLeafCertificate), + }, + }, nil), + expectedErr: false, + }, + "if TPP and caBundleSecretRef is specified, but getting secret fails should error": { + iss: tppIssuerWithCABundleSecretRef, + caBundle: testLeafCertificate, + secretsLister: generateSecretLister(nil, errors.New("this is a network error")), + expectedErr: true, + }, + // TODO: write test cases where bad CA is passed. + // above TODO can be ignored if the checks are added to issuer validations per below link + // https://github.com/cert-manager/cert-manager/blob/v1.14.4/internal/apis/certmanager/validation/issuer.go#L354 + // eventhough we are not prevalidating, vcert http.Client would anyway fail when using invalid CA + // 2 scenarios with bad CAs: + // "if TPP and caBundle is specified, a bad bundle from CABundle should error" + // "if TPP and caBundleSecretRef is specified, a bad bundle from a CABundleSecretRef should error" + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + test.runTppCaTest(t) + }) + } +} + type testConfigForIssuerT struct { iss cmapi.GenericIssuer secretsLister internalinformers.SecretLister + caBundle string expectedErr bool @@ -235,3 +472,21 @@ func (c *testConfigForIssuerT) runTest(t *testing.T) { c.CheckFn(t, resp) } } + +func (c *testConfigForIssuerT) runTppCaTest(t *testing.T) { + caResp, err := caBundleForVcertTPP(c.iss.GetSpec().Venafi.TPP, c.secretsLister, "test-namespace") + + if err != nil && !c.expectedErr { + t.Errorf("expected to not get an error, but got: %v", err) + } + if err == nil && c.expectedErr { + t.Errorf("expected to get an error but did not get one") + } + + if !c.expectedErr { + if c.caBundle != string(caResp) { + t.Errorf("got unexpected CA bundle, exp=%s got=%s", + c.caBundle, caResp) + } + } +}