From 7911f0b76ddccce947a937a7883001fd9b89d833 Mon Sep 17 00:00:00 2001 From: Laura Fitzgerald Date: Fri, 3 Nov 2023 14:55:42 +0000 Subject: [PATCH] check dnsnames on certificates rather than on get request --- pkg/apis/v1alpha1/tlspolicy_types.go | 4 +- test/e2e/gateway_single_spoke_test.go | 60 +++++++++++++++++---------- test/util/helper.go | 16 ------- test/util/test_types.go | 6 +-- 4 files changed, 43 insertions(+), 43 deletions(-) diff --git a/pkg/apis/v1alpha1/tlspolicy_types.go b/pkg/apis/v1alpha1/tlspolicy_types.go index bd664a56f..f1c6a27f5 100644 --- a/pkg/apis/v1alpha1/tlspolicy_types.go +++ b/pkg/apis/v1alpha1/tlspolicy_types.go @@ -20,7 +20,7 @@ import ( "fmt" certmanv1 "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1" - cmmeta "github.com/jetstack/cert-manager/pkg/apis/meta/v1" + certmanmetav1 "github.com/jetstack/cert-manager/pkg/apis/meta/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" @@ -46,7 +46,7 @@ type CertificateSpec struct { // If the `kind` field is set to `ClusterIssuer`, a ClusterIssuer with the // provided name will be used. // The `name` field in this stanza is required at all times. - IssuerRef cmmeta.ObjectReference `json:"issuerRef"` + IssuerRef certmanmetav1.ObjectReference `json:"issuerRef"` // CommonName is a common name to be used on the Certificate. // The CommonName should have a length of 64 characters or fewer to avoid diff --git a/test/e2e/gateway_single_spoke_test.go b/test/e2e/gateway_single_spoke_test.go index 22e1eff8b..77829b843 100644 --- a/test/e2e/gateway_single_spoke_test.go +++ b/test/e2e/gateway_single_spoke_test.go @@ -11,7 +11,8 @@ import ( "strings" "time" - cmmetav1 "github.com/jetstack/cert-manager/pkg/apis/meta/v1" + certmanv1 "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1" + certmanmetav1 "github.com/jetstack/cert-manager/pkg/apis/meta/v1" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" ocm_cluster_v1beta1 "open-cluster-management.io/api/cluster/v1beta1" @@ -93,7 +94,7 @@ var _ = Describe("Gateway single target cluster", func() { Namespace: Pointer(gatewayapi.Namespace(tconfig.HubNamespace())), }, CertificateSpec: mgcv1alpha1.CertificateSpec{ - IssuerRef: cmmetav1.ObjectReference{ + IssuerRef: certmanmetav1.ObjectReference{ Name: "glbc-ca", Kind: "ClusterIssuer", Group: "cert-manager.io", @@ -269,6 +270,14 @@ var _ = Describe("Gateway single target cluster", func() { } err = tconfig.HubClient().Delete(ctx, secret) Expect(client.IgnoreNotFound(err)).ToNot(HaveOccurred()) + cert := &certmanv1.Certificate{ + ObjectMeta: metav1.ObjectMeta{ + Name: strings.Join([]string{testID, tconfig.ManagedZone()}, "."), + Namespace: tconfig.HubNamespace(), + }, + } + err = tconfig.HubClient().Delete(ctx, cert) + Expect(client.IgnoreNotFound(err)).ToNot(HaveOccurred()) }) @@ -342,7 +351,17 @@ var _ = Describe("Gateway single target cluster", func() { GinkgoWriter.Printf("[debug] GET error: '%s'\n", err) return err } - return TestCertificate(string(hostname), resp) + if resp.TLS == nil { + return fmt.Errorf("expected tls configuration to be present on http request") + } + for _, cert := range resp.TLS.PeerCertificates { + for _, name := range cert.DNSNames { + if name == string(hostname) { + return nil + } + } + } + return fmt.Errorf("%s hostname not found in the certificate via get request", string(hostname)) }).WithTimeout(300 * time.Second).WithPolling(10 * time.Second).WithContext(ctx).ShouldNot(HaveOccurred()) defer resp.Body.Close() @@ -398,31 +417,28 @@ var _ = Describe("Gateway single target cluster", func() { }).WithContext(ctx).WithTimeout(180 * time.Second).WithPolling(2 * time.Second).ShouldNot(HaveOccurred()) } - By("checking a wildcard cert is present via get request") + By("checking tls certificate") { - dialer := &net.Dialer{Resolver: authoritativeResolver} - dialContext := func(ctx context.Context, network, addr string) (net.Conn, error) { - return dialer.DialContext(ctx, network, addr) - } - http.DefaultTransport.(*http.Transport).DialContext = dialContext - http.DefaultTransport.(*http.Transport).TLSClientConfig = &tls.Config{InsecureSkipVerify: true} - otherHostname = gatewayapi.Hostname(strings.Join([]string{"other", tconfig.ManagedZone()}, ".")) - var resp *http.Response + certList := &certmanv1.CertificateList{} Eventually(func(ctx SpecContext) error { - httpClient := &http.Client{} - resp, err = httpClient.Get("https://" + string(otherHostname)) + err = tconfig.HubClient().List(ctx, certList) if err != nil { - GinkgoWriter.Printf("[debug] GET error: '%s'\n", err) return err } - err = TestCertificate(string(wildcardHostname), resp) - if err != nil { - GinkgoWriter.Printf("[debug] Cert error: '%s'\n", err) - return err + if len(certList.Items) == 0 { + return fmt.Errorf("no certificate found") } - return nil - }).WithTimeout(600 * time.Second).WithPolling(10 * time.Second).WithContext(ctx).ShouldNot(HaveOccurred()) - defer resp.Body.Close() + for _, cert := range certList.Items { + if cert.Labels["gateway"] == testID { + for _, dnsName := range cert.Spec.DNSNames { + if dnsName == string(wildcardHostname) { + return nil + } + } + } + } + return fmt.Errorf("dns names for certificate not as expected") + }).WithContext(ctx).WithTimeout(180 * time.Second).WithPolling(2 * time.Second).ShouldNot(HaveOccurred()) } By("adding/removing listeners tls secrets are added/removed") { diff --git a/test/util/helper.go b/test/util/helper.go index 6fd6a9538..92b3b26d9 100644 --- a/test/util/helper.go +++ b/test/util/helper.go @@ -3,8 +3,6 @@ package testutil import ( - "fmt" - "net/http" "strings" "testing" @@ -136,17 +134,3 @@ func GetBasicScheme() *runtime.Scheme { _ = corev1.AddToScheme(scheme) return scheme } - -func TestCertificate(dnsName string, resp *http.Response) error { - if resp.TLS == nil { - return fmt.Errorf("expected tls configuration to be present on http request") - } - for _, cert := range resp.TLS.PeerCertificates { - for _, name := range cert.DNSNames { - if name == dnsName { - return nil - } - } - } - return fmt.Errorf("wildcard hostname not found in the certificate via get request") -} diff --git a/test/util/test_types.go b/test/util/test_types.go index 96134c7ec..97867c340 100644 --- a/test/util/test_types.go +++ b/test/util/test_types.go @@ -6,7 +6,7 @@ import ( "strings" certmanv1 "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1" - cmmeta "github.com/jetstack/cert-manager/pkg/apis/meta/v1" + certmanmetav1 "github.com/jetstack/cert-manager/pkg/apis/meta/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -152,13 +152,13 @@ func (t *TestTLSPolicy) WithTargetGateway(gwName string) *TestTLSPolicy { return t } -func (t *TestTLSPolicy) WithIssuerRef(issuerRef cmmeta.ObjectReference) *TestTLSPolicy { +func (t *TestTLSPolicy) WithIssuerRef(issuerRef certmanmetav1.ObjectReference) *TestTLSPolicy { t.Spec.IssuerRef = issuerRef return t } func (t *TestTLSPolicy) WithIssuer(name, kind, group string) *TestTLSPolicy { - t.WithIssuerRef(cmmeta.ObjectReference{ + t.WithIssuerRef(certmanmetav1.ObjectReference{ Name: name, Kind: kind, Group: group,