use correct common name when correcting cert issuer

Azure · Oct 10, 2024 · 9960dea · 9960dea
1 parent fe94d32
commit 9960dea
Show file tree

Hide file tree

Showing 2 changed files with 51 additions and 24 deletions.
diff --git a/pkg/cluster/correct_cert_issuer.go b/pkg/cluster/correct_cert_issuer.go
@@ -5,6 +5,8 @@ package cluster
 
 import (
 	"context"
+	"errors"
+	"fmt"
 	"strings"
 
 	"github.com/Azure/ARO-RP/pkg/util/keyvault"
@@ -16,18 +18,29 @@ import (
 // signing algorithm in use by DigiCert
 func (m *manager) correctCertificateIssuer(ctx context.Context) error {
 	if !strings.Contains(m.doc.OpenShiftCluster.Properties.ClusterProfile.Domain, ".") {
-		for _, certName := range []string{m.doc.ID + "-apiserver", m.doc.ID + "-ingress"} {
-			err := m.ensureCertificateIssuer(ctx, certName, "OneCertV2-PublicCA")
-			if err != nil {
-				return err
-			}
+		apiCertName := m.doc.ID + "-apiserver"
+		apiHostname := strings.Split(strings.TrimPrefix(m.doc.OpenShiftCluster.Properties.APIServerProfile.URL, "https://"), ":")[0]
+		err := m.ensureCertificateIssuer(ctx, apiCertName, apiHostname, "OneCertV2-PublicCA")
+		if err != nil {
+			return err
+		}
+
+		ingressCertName := m.doc.ID + "-ingress"
+		ingressHostname := "*" + strings.TrimSuffix(strings.TrimPrefix(m.doc.OpenShiftCluster.Properties.ConsoleProfile.URL, "https://console-openshift-console"), "/")
+		err = m.ensureCertificateIssuer(ctx, ingressCertName, ingressHostname, "OneCertV2-PublicCA")
+		if err != nil {
+			return err
 		}
 	}
 
 	return nil
 }
 
-func (m *manager) ensureCertificateIssuer(ctx context.Context, certificateName, issuerName string) error {
+func (m *manager) ensureCertificateIssuer(ctx context.Context, certificateName, dnsName, issuerName string) error {
+	if strings.Count(dnsName, ".") < 2 {
+		return errors.New(fmt.Sprintf("%s is not a valid DNS name", dnsName))
+	}
+
 	clusterKeyvault := m.env.ClusterKeyvault()
 
 	bundle, err := clusterKeyvault.GetCertificate(ctx, certificateName)
@@ -47,7 +60,7 @@ func (m *manager) ensureCertificateIssuer(ctx context.Context, certificateName,
 			return err
 		}
 
-		err = clusterKeyvault.CreateSignedCertificate(ctx, issuerName, certificateName, certificateName, keyvault.EkuServerAuth)
+		err = clusterKeyvault.CreateSignedCertificate(ctx, issuerName, certificateName, dnsName, keyvault.EkuServerAuth)
 		if err != nil {
 			return err
 		}

diff --git a/pkg/cluster/correct_cert_issuer_test.go b/pkg/cluster/correct_cert_issuer_test.go
@@ -18,22 +18,33 @@ func TestEnsureCertificateIssuer(t *testing.T) {
 	tests := []struct {
 		Name              string
 		CertificateName   string
+		DNSName           string
 		CurrentIssuerName string
 		NewIssuerName     string
 		ExpectError       bool
 	}{
 		{
 			Name:              "current issuer matches new issuer",
 			CertificateName:   "testCert",
+			DNSName:           "*.apps.test.asdf.tld",
 			CurrentIssuerName: "fakeIssuer",
 			NewIssuerName:     "fakeIssuer",
 		},
 		{
 			Name:              "current issuer different from new issuer",
 			CertificateName:   "testCert",
+			DNSName:           "*.apps.test.asdf.tld",
 			CurrentIssuerName: "OldFakeIssuer",
 			NewIssuerName:     "NewFakeIssuer",
 		},
+		{
+			Name:              "malformed dns data",
+			CertificateName:   "testCert",
+			DNSName:           "f898896d-c9ce-4d2b-b218-95a1f858df3a-something",
+			CurrentIssuerName: "OldFakeIssuer",
+			NewIssuerName:     "NewFakeIssuer",
+			ExpectError:       true,
+		},
 	}
 
 	for _, test := range tests {
@@ -42,33 +53,36 @@ func TestEnsureCertificateIssuer(t *testing.T) {
 			defer controller.Finish()
 
 			clusterKeyvault := mock_keyvault.NewMockManager(controller)
-			clusterKeyvault.EXPECT().GetCertificate(gomock.Any(), test.CertificateName).Return(azkeyvault.CertificateBundle{
-				Policy: &azkeyvault.CertificatePolicy{
-					IssuerParameters: &azkeyvault.IssuerParameters{
-						Name: &test.CurrentIssuerName,
-					},
-				},
-			}, nil)
+			env := mock_env.NewMockInterface(controller)
 
-			if test.CurrentIssuerName != test.NewIssuerName {
-				clusterKeyvault.EXPECT().GetCertificatePolicy(gomock.Any(), test.CertificateName).Return(azkeyvault.CertificatePolicy{
-					IssuerParameters: &azkeyvault.IssuerParameters{
-						Name: &test.CurrentIssuerName,
+			if !test.ExpectError {
+				clusterKeyvault.EXPECT().GetCertificate(gomock.Any(), test.CertificateName).Return(azkeyvault.CertificateBundle{
+					Policy: &azkeyvault.CertificatePolicy{
+						IssuerParameters: &azkeyvault.IssuerParameters{
+							Name: &test.CurrentIssuerName,
+						},
 					},
 				}, nil)
 
-				clusterKeyvault.EXPECT().UpdateCertificatePolicy(gomock.Any(), test.CertificateName, gomock.Any()).Return(nil)
-				clusterKeyvault.EXPECT().CreateSignedCertificate(gomock.Any(), test.NewIssuerName, test.CertificateName, test.CertificateName, gomock.Any()).Return(nil)
-			}
+				if test.CurrentIssuerName != test.NewIssuerName {
+					clusterKeyvault.EXPECT().GetCertificatePolicy(gomock.Any(), test.CertificateName).Return(azkeyvault.CertificatePolicy{
+						IssuerParameters: &azkeyvault.IssuerParameters{
+							Name: &test.CurrentIssuerName,
+						},
+					}, nil)
 
-			env := mock_env.NewMockInterface(controller)
-			env.EXPECT().ClusterKeyvault().AnyTimes().Return(clusterKeyvault)
+					clusterKeyvault.EXPECT().UpdateCertificatePolicy(gomock.Any(), test.CertificateName, gomock.Any()).Return(nil)
+					clusterKeyvault.EXPECT().CreateSignedCertificate(gomock.Any(), test.NewIssuerName, test.CertificateName, test.DNSName, gomock.Any()).Return(nil)
+				}
+
+				env.EXPECT().ClusterKeyvault().AnyTimes().Return(clusterKeyvault)
+			}
 
 			m := &manager{
 				env: env,
 			}
 
-			err := m.ensureCertificateIssuer(context.TODO(), test.CertificateName, test.NewIssuerName)
+			err := m.ensureCertificateIssuer(context.TODO(), test.CertificateName, test.DNSName, test.NewIssuerName)
 			if test.ExpectError == (err == nil) {
 				t.Errorf("TestCorrectCertificateIssuer() %s: ExpectError: %t, actual error: %s\n", test.Name, test.ExpectError, err)
 			}