From a2c2dec18b2fc62570a6954c77aaebf4df478bc5 Mon Sep 17 00:00:00 2001 From: Ryan Fredette Date: Tue, 4 Jun 2024 22:36:47 -0400 Subject: [PATCH] Parallelize CRL Tests Modify TestMTLSWithCRLs and TestCRLUpdate so that the subtests of each test can be run in parallel. --- test/e2e/client_tls_test.go | 219 ++++++++++++++++++------------------ test/e2e/util_test.go | 6 + 2 files changed, 115 insertions(+), 110 deletions(-) diff --git a/test/e2e/client_tls_test.go b/test/e2e/client_tls_test.go index e171e9eb51..0319f0bd63 100644 --- a/test/e2e/client_tls_test.go +++ b/test/e2e/client_tls_test.go @@ -376,20 +376,12 @@ type TestMTLSWithCRLsCerts struct { // TestMTLSWithCRLs verifies that mTLS works when the client auth chain includes certificate revocation lists (CRLs). func TestMTLSWithCRLs(t *testing.T) { t.Parallel() - namespaceName := names.SimpleNameGenerator.GenerateName("mtls-with-crls") - crlHostName := types.NamespacedName{ - Name: "crl-host", - Namespace: namespaceName, - } - // When generating certificates, the CRL distribution points need to be specified by URL. - crlHostServiceName := "crl-host-service" - crlHostURL := crlHostServiceName + "." + crlHostName.Namespace + ".svc" testCases := []struct { // Name is the name of the test case. Name string // CreateCerts generates the certificates for the test case. Certificates and CRLs must not have expired at the // time of the run, so they must be generated at runtime. - CreateCerts func() TestMTLSWithCRLsCerts + CreateCerts func(string) TestMTLSWithCRLsCerts }{ { // This test case has CA certificates including a CRL distribution point (CDP) for the CRL that they @@ -422,7 +414,7 @@ func TestMTLSWithCRLs(t *testing.T) { // - Self signed. // - Should be rejected because it's not signed by any trusted CA. Name: "certificate-distributes-its-own-crl", - CreateCerts: func() TestMTLSWithCRLsCerts { + CreateCerts: func(crlHostURL string) TestMTLSWithCRLsCerts { rootCDP := "http://" + crlHostURL + "/root/root.crl" intermediateCDP := "http://" + crlHostURL + "/intermediate/intermediate.crl" @@ -502,7 +494,7 @@ func TestMTLSWithCRLs(t *testing.T) { // - Self signed. // - Should be rejected because it's not signed by any trusted CA (SSL error "unknown ca"). Name: "certificate-distributes-its-signers-crl", - CreateCerts: func() TestMTLSWithCRLsCerts { + CreateCerts: func(crlHostURL string) TestMTLSWithCRLsCerts { rootCDP := "http://" + crlHostURL + "/root/root.crl" intermediateCDP := "http://" + crlHostURL + "/intermediate/intermediate.crl" @@ -587,7 +579,7 @@ func TestMTLSWithCRLs(t *testing.T) { // - Self signed. // - Should be rejected because it's not signed by any trusted CA (SSL error "unknown ca"). Name: "certificate-distributes-its-signers-crl-with-workaround", - CreateCerts: func() TestMTLSWithCRLsCerts { + CreateCerts: func(crlHostURL string) TestMTLSWithCRLsCerts { rootCDP := "http://" + crlHostURL + "/root/root.crl" intermediateCDP := "http://" + crlHostURL + "/intermediate/intermediate.crl" @@ -633,7 +625,7 @@ func TestMTLSWithCRLs(t *testing.T) { { // large-crl verifies that CRLs larger than 1MB can be used. This tests the fix for OCPBUGS-6661. Name: "large-crl", - CreateCerts: func() TestMTLSWithCRLsCerts { + CreateCerts: func(crlHostURL string) TestMTLSWithCRLsCerts { maxDummyRevokedSerialNumber := 25000 rootCDP := "http://" + crlHostURL + "/root/root.crl" intermediateCDP := "http://" + crlHostURL + "/intermediate/intermediate.crl" @@ -720,7 +712,7 @@ func TestMTLSWithCRLs(t *testing.T) { { // multiple-intermediate-ca tests that more than 2 CAs can be used. Each CA lists its own CRL's distribution point. Name: "multiple-intermediate-ca", - CreateCerts: func() TestMTLSWithCRLsCerts { + CreateCerts: func(crlHostURL string) TestMTLSWithCRLsCerts { CANames := []string{ "root", "foo", @@ -774,10 +766,19 @@ func TestMTLSWithCRLs(t *testing.T) { }, } + namespaceName := names.SimpleNameGenerator.GenerateName("mtls-with-crls-") namespace := createNamespace(t, namespaceName) for _, tc := range testCases { t.Run(tc.Name, func(t *testing.T) { - tcCerts := tc.CreateCerts() + t.Parallel() + crlHostName := types.NamespacedName{ + Name: names.SimpleNameGenerator.GenerateName("crl-host-"), + Namespace: namespaceName, + } + // When generating certificates, the CRL distribution points need to be specified by URL + crlHostServiceName := names.SimpleNameGenerator.GenerateName("crl-host-service-") + crlHostURL := crlHostServiceName + "." + crlHostName.Namespace + ".svc" + tcCerts := tc.CreateCerts(crlHostURL) // Get the URL path of one of the CRLs to use in the CRL host pod's readiness probe. readinessProbePath := "" for crlName := range tcCerts.CRLs { @@ -815,7 +816,7 @@ func TestMTLSWithCRLs(t *testing.T) { }, } for name, crl := range tcCerts.CRLs { - crlConfigMapName := name + "-crl" + crlConfigMapName := names.SimpleNameGenerator.GenerateName(name + "-crl-") crlConfigMap := corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ Name: crlConfigMapName, @@ -857,21 +858,7 @@ func TestMTLSWithCRLs(t *testing.T) { // case comes through and creates a new one, but if that stops being true in the future, their assertDeleted // calls may need to be replaced by the slower assertDeletedWaitForCleanup option. t.Cleanup(func() { assertDeletedWaitForCleanup(t, kclient, &crlHostPod) }) - crlHostService := corev1.Service{ - ObjectMeta: metav1.ObjectMeta{ - Name: crlHostServiceName, - Namespace: namespace.Name, - }, - Spec: corev1.ServiceSpec{ - Selector: map[string]string{"app": crlHostName.Name}, - Ports: []corev1.ServicePort{{ - Name: "http", - Port: 80, - Protocol: corev1.ProtocolTCP, - TargetPort: intstr.FromString("http-svc"), - }}, - }, - } + crlHostService := buildCRLHostService(crlHostServiceName, namespace.Name, crlHostName.Name) if err := kclient.Create(context.TODO(), &crlHostService); err != nil { t.Fatalf("Failed to create service %q: %v", crlHostService.Name, err) } @@ -889,8 +876,8 @@ func TestMTLSWithCRLs(t *testing.T) { } return false, nil }) - // Create CA cert bundle. - clientCAConfigmapName := "client-ca-cm-" + namespace.Name + // Create CA cert bundle + clientCAConfigmapName := names.SimpleNameGenerator.GenerateName("client-ca-cm-") clientCAConfigmap := corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ Name: clientCAConfigmapName, @@ -905,7 +892,7 @@ func TestMTLSWithCRLs(t *testing.T) { } t.Cleanup(func() { assertDeleted(t, kclient, &clientCAConfigmap) }) icName := types.NamespacedName{ - Name: "mtls-with-crls", + Name: names.SimpleNameGenerator.GenerateName("mtls-with-crls"), Namespace: operatorNamespace, } icDomain := icName.Name + "." + dnsConfig.Spec.BaseDomain @@ -929,7 +916,7 @@ func TestMTLSWithCRLs(t *testing.T) { // certificates and keys, and mount that to the client pod. clientCertsConfigmap := corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ - Name: "client-certificates", + Name: names.SimpleNameGenerator.GenerateName("client-certificates-"), Namespace: namespace.Name, }, Data: map[string]string{}, @@ -954,7 +941,7 @@ func TestMTLSWithCRLs(t *testing.T) { t.Fatalf("failed to get routerDeployment %q: %v", routerDeploymentName, err) } - podName := "mtls-with-crls-client" + podName := names.SimpleNameGenerator.GenerateName("mtls-with-crls-client-") image := routerDeployment.Spec.Template.Spec.Containers[0].Image clientPod := buildExecPod(podName, namespace.Name, image) clientPod.Spec.Volumes = []corev1.Volume{{ @@ -1112,36 +1099,18 @@ func TestMTLSWithCRLs(t *testing.T) { // TestCRLUpdate verifies that CRLs are updated when they expire. func TestCRLUpdate(t *testing.T) { t.Parallel() - testName := names.SimpleNameGenerator.GenerateName("crl-update") - crlHostName := types.NamespacedName{ - Name: "crl-host", - Namespace: testName, - } - // When generating certificates, the CRL distribution points need to be specified by URL. - crlHostServiceName := "crl-host-service" - crlHostURL := crlHostServiceName + "." + crlHostName.Namespace + ".svc" - namespace := corev1.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - Name: testName, - }, - } - if err := kclient.Create(context.TODO(), &namespace); err != nil { - t.Fatalf("Failed to create namespace %q: %v", namespace.Name, err) - } - t.Cleanup(func() { assertDeleted(t, kclient, &namespace) }) testCases := []struct { - // Test case name Name string // Function to generate the necessary certificates. These will be put into the ingresscontroller's client CA // bundle, and will be used to generate CRLs during the test. - CreateCerts func() map[string]KeyCert + CreateCerts func(string) map[string]KeyCert // The names of the CAs whose CRLs are expected to be downloaded by the router pod. Only the certs with the // corresponding names will be used to generate CRLs. ExpectedCRLs []string }{ { Name: "certificate-distributes-its-own-crl", - CreateCerts: func() map[string]KeyCert { + CreateCerts: func(crlHostURL string) map[string]KeyCert { rootCDP := "http://" + crlHostURL + "/root.crl" intermediateCDP := "http://" + crlHostURL + "/intermediate.crl" @@ -1159,7 +1128,7 @@ func TestCRLUpdate(t *testing.T) { }, { Name: "certificate-distributes-its-signers-crl", - CreateCerts: func() map[string]KeyCert { + CreateCerts: func(crlHostURL string) map[string]KeyCert { rootCDP := "http://" + crlHostURL + "/root.crl" rootCA := MustCreateTLSKeyCert("testing root CA", time.Now(), time.Now().Add(24*time.Hour), true, []string{}, nil) @@ -1175,7 +1144,7 @@ func TestCRLUpdate(t *testing.T) { }, { Name: "certificate-distributes-its-signers-crl-with-workaround", - CreateCerts: func() map[string]KeyCert { + CreateCerts: func(crlHostURL string) map[string]KeyCert { rootCDP := "http://" + crlHostURL + "/root.crl" intermediateCDP := "http://" + crlHostURL + "/intermediate.crl" @@ -1195,7 +1164,7 @@ func TestCRLUpdate(t *testing.T) { }, { Name: "many-CAs-with-signers-crl-workaround", - CreateCerts: func() map[string]KeyCert { + CreateCerts: func(crlHostURL string) map[string]KeyCert { rootCDP := "http://" + crlHostURL + "/root.crl" intermediateCDP := "http://" + crlHostURL + "/intermediate.crl" fooCDP := "http://" + crlHostURL + "/foo.crl" @@ -1222,15 +1191,33 @@ func TestCRLUpdate(t *testing.T) { }, }, } - for _, tc := range testCases { + namespaceName := names.SimpleNameGenerator.GenerateName("crl-update-") + namespace := corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: namespaceName, + }, + } + if err := kclient.Create(context.TODO(), &namespace); err != nil { + t.Fatalf("Failed to create namespace %q: %v", namespace.Name, err) + } + t.Cleanup(func() { assertDeleted(t, kclient, &namespace) }) + for i := range testCases { + tc := testCases[i] t.Run(tc.Name, func(t *testing.T) { - caCerts := tc.CreateCerts() + t.Parallel() + crlHostName := types.NamespacedName{ + Name: names.SimpleNameGenerator.GenerateName("crl-host-"), + Namespace: namespaceName, + } + // When generating certificates, the CRL distribution points need to be specified by URL. + crlHostServiceName := names.SimpleNameGenerator.GenerateName("crl-host-service") + crlHostURL := crlHostServiceName + "." + crlHostName.Namespace + ".svc" + caCerts := tc.CreateCerts(crlHostURL) // Generate CRLs. Offset the expiration times by 1 minute each so that we can verify that only the correct CRLs get updated at each expiration. currentCRLs := map[string]*x509.RevocationList{} crlPems := map[string]string{} caBundle := []string{} validTime := 3 * time.Minute - //expirations := []time.Time{} for _, caName := range tc.ExpectedCRLs { currentCRLs[caName], crlPems[caName+".crl"] = MustCreateCRL(nil, caCerts[caName], time.Now(), time.Now().Add(validTime), nil) validTime += time.Minute @@ -1239,7 +1226,7 @@ func TestCRLUpdate(t *testing.T) { caBundle = append(caBundle, caCert.CertPem) } // Create a pod which will host the CRLs. - crlConfigMapName := crlHostName.Name + "-configmap" + crlConfigMapName := names.SimpleNameGenerator.GenerateName(crlHostName.Name + "-configmap-") crlConfigMap := corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ Name: crlConfigMapName, @@ -1251,50 +1238,7 @@ func TestCRLUpdate(t *testing.T) { t.Fatalf("Failed to create configmap %q: %v", crlConfigMap.Name, err) } t.Cleanup(func() { assertDeleted(t, kclient, &crlConfigMap) }) - crlHostPod := corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: crlHostName.Name, - Namespace: namespace.Name, - Labels: map[string]string{"app": crlHostName.Name}, - }, - Spec: corev1.PodSpec{ - Containers: []corev1.Container{{ - Name: "httpd", - Image: "quay.io/centos7/httpd-24-centos7:centos7", - Ports: []corev1.ContainerPort{{ - ContainerPort: 8080, - Name: "http-svc", - }}, - VolumeMounts: []corev1.VolumeMount{{ - Name: "data", - MountPath: "/var/www/html", - ReadOnly: true, - }}, - SecurityContext: generateUnprivilegedSecurityContext(), - ReadinessProbe: &corev1.Probe{ - ProbeHandler: corev1.ProbeHandler{ - HTTPGet: &corev1.HTTPGetAction{ - Path: fmt.Sprintf("/%s.crl", tc.ExpectedCRLs[0]), - Port: intstr.IntOrString{ - Type: intstr.String, - StrVal: "http-svc", - }, - }, - }, - }, - }}, - Volumes: []corev1.Volume{{ - Name: "data", - VolumeSource: corev1.VolumeSource{ - ConfigMap: &corev1.ConfigMapVolumeSource{ - LocalObjectReference: corev1.LocalObjectReference{ - Name: crlConfigMap.Name, - }, - }, - }, - }}, - }, - } + crlHostPod := buildCRLHostPod(crlHostName.Name, namespace.Name, crlConfigMap.Name) if err := kclient.Create(context.TODO(), &crlHostPod); err != nil { t.Fatalf("Failed to create pod %q: %v", crlHostPod.Name, err) @@ -1339,7 +1283,7 @@ func TestCRLUpdate(t *testing.T) { return false, nil }) // Create CA cert bundle. - clientCAConfigmapName := "client-ca-cm-" + namespace.Name + clientCAConfigmapName := names.SimpleNameGenerator.GenerateName("client-ca-cm-") clientCAConfigmap := corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ Name: clientCAConfigmapName, @@ -1354,7 +1298,7 @@ func TestCRLUpdate(t *testing.T) { } t.Cleanup(func() { assertDeleted(t, kclient, &clientCAConfigmap) }) icName := types.NamespacedName{ - Name: testName, + Name: names.SimpleNameGenerator.GenerateName("crl-update-"), Namespace: operatorNamespace, } icDomain := icName.Name + "." + dnsConfig.Spec.BaseDomain @@ -1441,7 +1385,7 @@ func verifyCRLs(t *testing.T, pod *corev1.Pod, expectedCRLs map[string]*x509.Rev if err != nil { return false, err } - if len(activeCRLs) == 0 { + if len(activeCRLs) == 0 && len(expectedCRLs) != 0 { // 0 CRLs probably means the router hasn't completed startup yet. Retry. return false, nil } @@ -1479,6 +1423,7 @@ func verifyCRLs(t *testing.T, pod *corev1.Pod, expectedCRLs map[string]*x509.Rev } return false, fmt.Errorf("found %d CRLs, but only %d matched", len(activeCRLs), matchingCRLs) } + t.Log("all CRLs are up to date") return true, nil } @@ -1670,3 +1615,57 @@ func curlGetStatusCode(t *testing.T, clientPod *corev1.Pod, certName, endpoint, return httpStatusCodeInt, nil } } + +func buildCRLHostPod(podName, namespaceName, crlConfigMapName string) corev1.Pod { + return corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: podName, + Namespace: namespaceName, + Labels: map[string]string{"app": podName}, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Name: "httpd", + Image: "quay.io/openshifttest/httpd-24:multi", + Ports: []corev1.ContainerPort{{ + ContainerPort: 8080, + Name: "http-svc", + }}, + VolumeMounts: []corev1.VolumeMount{{ + Name: "data", + MountPath: "/var/www/html", + ReadOnly: true, + }}, + SecurityContext: generateUnprivilegedSecurityContext(), + }}, + Volumes: []corev1.Volume{{ + Name: "data", + VolumeSource: corev1.VolumeSource{ + ConfigMap: &corev1.ConfigMapVolumeSource{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: crlConfigMapName, + }, + }, + }, + }}, + }, + } +} + +func buildCRLHostService(serviceName, namespaceName, podName string) corev1.Service { + return corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: serviceName, + Namespace: namespaceName, + }, + Spec: corev1.ServiceSpec{ + Selector: map[string]string{"app": podName}, + Ports: []corev1.ServicePort{{ + Name: "http", + Port: 80, + Protocol: corev1.ProtocolTCP, + TargetPort: intstr.FromString("http-svc"), + }}, + }, + } +} diff --git a/test/e2e/util_test.go b/test/e2e/util_test.go index d196ab827b..165872214e 100644 --- a/test/e2e/util_test.go +++ b/test/e2e/util_test.go @@ -796,6 +796,12 @@ func verifyInternalIngressController(t *testing.T, name types.NamespacedName, ho } } +func assertCreated(t *testing.T, cl client.Client, thing client.Object) { + if err := cl.Create(context.TODO(), thing); err != nil { + t.Fatalf("Failed to create %s: %v", thing.GetName(), err) + } +} + // assertDeleted tries to delete a cluster resource, and causes test failure if the delete fails. func assertDeleted(t *testing.T, cl client.Client, thing client.Object) { t.Helper()