diff --git a/pkg/cluster/adminupdate_test.go b/pkg/cluster/adminupdate_test.go index 072a27e96..a84ab13f7 100644 --- a/pkg/cluster/adminupdate_test.go +++ b/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]", diff --git a/pkg/cluster/correct_cert_issuer.go b/pkg/cluster/correct_cert_issuer.go new file mode 100644 index 000000000..255628ae1 --- /dev/null +++ b/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 +} diff --git a/pkg/cluster/correct_cert_issuer_test.go b/pkg/cluster/correct_cert_issuer_test.go new file mode 100644 index 000000000..8d9733441 --- /dev/null +++ b/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) + } + }) + } +} diff --git a/pkg/cluster/install.go b/pkg/cluster/install.go index 1ac06e502..3e4297ec5 100644 --- a/pkg/cluster/install.go +++ b/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), diff --git a/pkg/cluster/replacedigicert.go b/pkg/cluster/replacedigicert.go deleted file mode 100644 index 95e0df319..000000000 --- a/pkg/cluster/replacedigicert.go +++ /dev/null @@ -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 -}