Merge pull request #1938 from marbindrakon/allow-acme-public-certs

Allow ACME-issued certificates for public endpoint routes

Author: openshift-merge-bot[bot]

internal/openstack/common.go

@@ -602,7 +602,7 @@ func (ed *EndpointDetail) CreateRoute(
 		certSecret := &k8s_corev1.Secret{}
 
 		// if a custom cert secret was provided, check if it exist
-		// and has the required cert, key and cacert
+		// and has the required cert and key (cacert optional)
 		// Right now there is no check if certificate is valid for
 		// the hostname of the route. If the referenced secret is
 		// there and has the required files it is just being used.
@@ -616,13 +616,9 @@ func (ed *EndpointDetail) CreateRoute(
 				return ctrl.Result{}, err
 			}
 
-			// check if secret has the expected entries tls.crt, tls.key and ca.crt
-			if certSecret != nil {
-				for _, key := range []string{"tls.crt", "tls.key", "ca.crt"} {
-					if _, exist := certSecret.Data[key]; !exist {
-						return ctrl.Result{}, fmt.Errorf("certificate secret %s does not provide %s", *ed.Route.TLS.SecretName, key)
-					}
-				}
+			// check the secret has the required tls.crt and tls.key entries
+			if err := validateRouteCertSecret(certSecret, *ed.Route.TLS.SecretName); err != nil {
+				return ctrl.Result{}, err
 			}
 		}
 
@@ -659,9 +655,13 @@ func (ed *EndpointDetail) CreateRoute(
 			Termination:                   routev1.TLSTerminationEdge,
 			Certificate:                   string(certSecret.Data[tls.CertKey]),
 			Key:                           string(certSecret.Data[tls.PrivateKey]),
-			CACertificate:                 string(certSecret.Data[tls.CAKey]),
 			InsecureEdgeTerminationPolicy: routev1.InsecureEdgeTerminationPolicyRedirect,
 		}
+		// ca.crt is optional (absent for ACME-issued certs); only set the
+		// route CACertificate when the secret actually provides it.
+		if caCert, ok := certSecret.Data[tls.CAKey]; ok {
+			tlsConfig.CACertificate = string(caCert)
+		}
 
 		// for internal TLS (TLSE) use routev1.TLSTerminationReencrypt
 		if ed.Service.TLS.Enabled && (ed.Service.TLS.SecretName != nil || hasCertInOverrideSpec(ed.Route.OverrideSpec)) {
@@ -872,6 +872,23 @@ func hasCertInOverrideSpec(overrideSpec route.OverrideSpec) bool {
 		overrideSpec.Spec.TLS.Key != ""
 }
 
+// validateRouteCertSecret ensures a user-provided route TLS secret contains the
+// required tls.crt and tls.key entries. ca.crt is intentionally not required:
+// certificates issued by an ACME issuer (e.g. Let's Encrypt) do not populate
+// ca.crt, and the issuing chain is delivered via tls.crt instead. ca.crt is only
+// needed to advertise a custom CA on the route, which is optional.
+func validateRouteCertSecret(certSecret *k8s_corev1.Secret, secretName string) error {
+	if certSecret == nil {
+		return nil
+	}
+	for _, key := range []string{tls.CertKey, tls.PrivateKey} {
+		if _, exist := certSecret.Data[key]; !exist {
+			return fmt.Errorf("certificate secret %s does not provide %s", secretName, key)
+		}
+	}
+	return nil
+}
+
 func serviceExists(route string, services *k8s_corev1.ServiceList) bool {
 	for _, svc := range services.Items {
 		if svc.Name == route {

internal/openstack/common_test.go

@@ -572,3 +572,72 @@ func TestCheckRouteAdmissionStatus_UpdatedStatus(t *testing.T) {
 	err = checkRouteAdmissionStatus(ctx, h, "test-route", "test-namespace")
 	g.Expect(err).ToNot(HaveOccurred())
 }
+
+func TestValidateRouteCertSecret(t *testing.T) {
+	tests := []struct {
+		name      string
+		data      map[string][]byte
+		wantErr   bool
+		errSubstr string
+	}{
+		{
+			name: "cert, key and ca.crt present",
+			data: map[string][]byte{
+				"tls.crt": []byte("cert"),
+				"tls.key": []byte("key"),
+				"ca.crt":  []byte("ca"),
+			},
+			wantErr: false,
+		},
+		{
+			// ACME issuers (e.g. Let's Encrypt) do not populate ca.crt; the
+			// secret must still be accepted.
+			name: "cert and key present, ca.crt absent (ACME)",
+			data: map[string][]byte{
+				"tls.crt": []byte("cert"),
+				"tls.key": []byte("key"),
+			},
+			wantErr: false,
+		},
+		{
+			name: "tls.key missing",
+			data: map[string][]byte{
+				"tls.crt": []byte("cert"),
+			},
+			wantErr:   true,
+			errSubstr: "does not provide tls.key",
+		},
+		{
+			name: "tls.crt missing",
+			data: map[string][]byte{
+				"tls.key": []byte("key"),
+			},
+			wantErr:   true,
+			errSubstr: "does not provide tls.crt",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			g := NewWithT(t)
+			secret := &k8s_corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{Name: "custom-route-cert", Namespace: "test-namespace"},
+				Data:       tt.data,
+			}
+
+			err := validateRouteCertSecret(secret, secret.Name)
+			if tt.wantErr {
+				g.Expect(err).To(HaveOccurred())
+				g.Expect(err.Error()).To(ContainSubstring(tt.errSubstr))
+			} else {
+				g.Expect(err).ToNot(HaveOccurred())
+			}
+		})
+	}
+}
+
+func TestValidateRouteCertSecret_NilSecret(t *testing.T) {
+	g := NewWithT(t)
+	// A nil secret is tolerated (the caller may not have fetched one).
+	g.Expect(validateRouteCertSecret(nil, "custom-route-cert")).ToNot(HaveOccurred())
+}