create util function to ensure certs have the right issuer

pkg/cluster/adminupdate_test.go

@@ -62,7 +62,7 @@ func TestAdminUpdateSteps(t *testing.T) {
 		"[Action startVMs]",
 		"[Condition apiServersReady, timeout 30m0s]",
 		"[Action populateDatabaseIntIP]",
-		"[Action replaceDigicert]",
+		"[Action correctCertificateIssuer]",
 		"[Action fixMCSCert]",
 		"[Action fixMCSUserData]",
 		"[Action configureAPIServerCertificate]",

pkg/cluster/correct_cert_issuer.go

@@ -0,0 +1,56 @@
+package cluster
+
+// Copyright (c) Microsoft Corporation.
+// Licensed under the Apache License 2.0.
+
+import (
+	"context"
+	"strings"
+
+	"github.com/Azure/ARO-RP/pkg/util/keyvault"
+)
+
+// if the cluster isn't using a managed domain and has a DigiCert-issued
+// certificate, replace the certificate with one issued by OneCert. This
+// ensures that clusters upgrading to 4.16 aren't blocked due to the SHA-1
+// 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
+			}
+		}
+	}
+
+	return nil
+}
+
+func (m *manager) ensureCertificateIssuer(ctx context.Context, certificateName, issuerName string) error {
+	clusterKeyvault := m.env.ClusterKeyvault()
+
+	bundle, err := clusterKeyvault.GetCertificate(ctx, certificateName)
+	if err != nil {
+		return err
+	}
+
+	if *bundle.Policy.IssuerParameters.Name != issuerName {
+		policy, err := clusterKeyvault.GetCertificatePolicy(ctx, certificateName)
+		if err != nil {
+			return err
+		}
+
+		policy.IssuerParameters.Name = &issuerName
+		err = clusterKeyvault.UpdateCertificatePolicy(ctx, certificateName, policy)
+		if err != nil {
+			return err
+		}
+
+		err = clusterKeyvault.CreateSignedCertificate(ctx, issuerName, certificateName, certificateName, keyvault.EkuServerAuth)
+		if err != nil {
+			return err
+		}
+	}
+	return nil
+}

pkg/cluster/correct_cert_issuer_test.go

@@ -0,0 +1,77 @@
+package cluster
+
+// Copyright (c) Microsoft Corporation.
+// Licensed under the Apache License 2.0.
+
+import (
+	"context"
+	"testing"
+
+	azkeyvault "github.com/Azure/azure-sdk-for-go/services/keyvault/v7.0/keyvault"
+	"go.uber.org/mock/gomock"
+
+	mock_env "github.com/Azure/ARO-RP/pkg/util/mocks/env"
+	mock_keyvault "github.com/Azure/ARO-RP/pkg/util/mocks/keyvault"
+)
+
+func TestEnsureCertificateIssuer(t *testing.T) {
+	tests := []struct {
+		Name              string
+		CertificateName   string
+		CurrentIssuerName string
+		NewIssuerName     string
+		ExpectError       bool
+	}{
+		{
+			Name:              "current issuer matches new issuer",
+			CertificateName:   "testCert",
+			CurrentIssuerName: "fakeIssuer",
+			NewIssuerName:     "fakeIssuer",
+		},
+		{
+			Name:              "current issuer different from new issuer",
+			CertificateName:   "testCert",
+			CurrentIssuerName: "OldFakeIssuer",
+			NewIssuerName:     "NewFakeIssuer",
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.Name, func(t *testing.T) {
+			controller := gomock.NewController(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)
+
+			if test.CurrentIssuerName != test.NewIssuerName {
+				clusterKeyvault.EXPECT().GetCertificatePolicy(gomock.Any(), test.CertificateName).Return(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)
+			}
+
+			env := mock_env.NewMockInterface(controller)
+			env.EXPECT().ClusterKeyvault().AnyTimes().Return(clusterKeyvault)
+
+			m := &manager{
+				env: env,
+			}
+
+			err := m.ensureCertificateIssuer(context.TODO(), test.CertificateName, test.NewIssuerName)
+			if test.ExpectError == (err == nil) {
+				t.Errorf("TestCorrectCertificateIssuer() %s: ExpectError: %t, actual error: %s\n", test.Name, test.ExpectError, err)
+			}
+		})
+	}
+}

pkg/cluster/install.go

@@ -138,7 +138,7 @@ func (m *manager) getGeneralFixesSteps() []steps.Step {
 func (m *manager) getCertificateRenewalSteps() []steps.Step {
 	steps := []steps.Step{
 		steps.Action(m.populateDatabaseIntIP),
-		steps.Action(m.replaceDigicert),
+		steps.Action(m.correctCertificateIssuer),
 		steps.Action(m.fixMCSCert),
 		steps.Action(m.fixMCSUserData),
 		steps.Action(m.configureAPIServerCertificate),
@@ -224,7 +224,7 @@ func (m *manager) Update(ctx context.Context) error {
 		steps.Action(m.startVMs),
 		steps.Condition(m.apiServersReady, 30*time.Minute, true),
 		steps.Action(m.rotateACRTokenPassword),
-		steps.Action(m.replaceDigicert),
+		steps.Action(m.correctCertificateIssuer),
 		steps.Action(m.configureAPIServerCertificate),
 		steps.Action(m.configureIngressCertificate),
 		steps.Action(m.renewMDSDCertificate),

pkg/cluster/replacedigicert.go

@@ -1,47 +0,0 @@
-package cluster
-
-// Copyright (c) Microsoft Corporation.
-// Licensed under the Apache License 2.0.
-
-import (
-	"context"
-	"strings"
-
-	"github.com/Azure/ARO-RP/pkg/util/keyvault"
-)
-
-// if the cluster is using a managed domain and has a DigiCert-issued
-// certificate, replace the certificate with one issued by OneCert. This
-// ensures that clusters upgrading to 4.16 aren't blocked due to the SHA-1
-// signing algorithm in use by DigiCert
-func (m *manager) replaceDigicert(ctx context.Context) error {
-	if strings.Contains(m.doc.OpenShiftCluster.Properties.ClusterProfile.Domain, ".") {
-		oneCertIssuerName := "OneCertV2-PublicCA"
-
-		for _, certName := range []string{m.doc.ID + "-apiserver", m.doc.ID + "-ingress"} {
-			clusterKeyvault := m.env.ClusterKeyvault()
-
-			bundle, err := clusterKeyvault.GetCertificate(ctx, certName)
-			if err != nil {
-				return err
-			}
-
-			if strings.Contains(*bundle.Policy.IssuerParameters.Name, "DigiCert") {
-				policy, err := clusterKeyvault.GetCertificatePolicy(ctx, certName)
-				if err != nil {
-					return err
-				}
-
-				policy.IssuerParameters.Name = &oneCertIssuerName
-				err = clusterKeyvault.UpdateCertificatePolicy(ctx, certName, policy)
-				if err != nil {
-					return err
-				}
-
-				m.env.ClusterKeyvault().CreateSignedCertificate(ctx, oneCertIssuerName, certName, certName, keyvault.EkuServerAuth)
-			}
-		}
-	}
-
-	return nil
-}