Repository: Azure/ARO-RP
Pull request #1183 (2020-11-19 01:26:02)

remove certificate policy upgrade code, all certs are upgraded
I've validated manually that all certs are upgraded


Patch:
diff --git a/pkg/cluster/deploystorage.go b/pkg/cluster/deploystorage.go
index 3f6181cd3..a6929930d 100644
--- a/pkg/cluster/deploystorage.go
+++ b/pkg/cluster/deploystorage.go
@@ -236,6 +236,7 @@ func (m *manager) deployStorageTemplate(ctx context.Context, installConfig *inst
 var extraDenyAssignmentExclusions = map[string][]string{
 	"Microsoft.RedHatOpenShift/RedHatEngineering": {
 		"Microsoft.Network/networkInterfaces/effectiveRouteTable/action",
+		"Microsoft.Resources/subscriptions/resourceGroups/write", // enable resource group tagging
 	},
 }
 
diff --git a/pkg/cluster/install.go b/pkg/cluster/install.go
index c2f52d2bd..434f25470 100644
--- a/pkg/cluster/install.go
+++ b/pkg/cluster/install.go
@@ -36,7 +36,6 @@ func (m *manager) AdminUpgrade(ctx context.Context) error {
 		steps.Action(m.fixPullSecret),       // TODO(mj): Remove when operator deployed
 		steps.Action(m.ensureAROOperator),
 		steps.Condition(m.aroDeploymentReady, 20*time.Minute),
-		steps.Action(m.upgradeCertificates),
 		steps.Action(m.configureAPIServerCertificate),
 		steps.Action(m.configureIngressCertificate),
 		steps.Action(m.updateProvisionedBy), // Run this last so we capture the resource provider only once the upgrade has been fully performed
diff --git a/pkg/cluster/tls.go b/pkg/cluster/tls.go
index 669e939de..9e0bbc755 100644
--- a/pkg/cluster/tls.go
+++ b/pkg/cluster/tls.go
@@ -68,31 +68,6 @@ func (m *manager) createCertificates(ctx context.Context) error {
 	return nil
 }
 
-func (m *manager) upgradeCertificates(ctx context.Context) error {
-	if m.env.DeploymentMode() == deployment.Development {
-		return nil
-	}
-
-	managedDomain, err := dns.ManagedDomain(m.env, m.doc.OpenShiftCluster.Properties.ClusterProfile.Domain)
-	if err != nil {
-		return err
-	}
-
-	if managedDomain == "" {
-		return nil
-	}
-
-	for _, c := range []string{m.doc.ID + "-apiserver", m.doc.ID + "-ingress"} {
-		m.log.Printf("upgrading certificate %s", c)
-		err = m.keyvault.UpgradeCertificatePolicy(ctx, c)
-		if err != nil {
-			return err
-		}
-	}
-
-	return nil
-}
-
 func (m *manager) ensureSecret(ctx context.Context, secrets coreclient.SecretInterface, certificateName string) error {
 	bundle, err := m.keyvault.GetSecret(ctx, certificateName)
 	if err != nil {
diff --git a/pkg/util/azureclient/keyvault/base.go b/pkg/util/azureclient/keyvault/base.go
index 4067e771c..c719f134f 100644
--- a/pkg/util/azureclient/keyvault/base.go
+++ b/pkg/util/azureclient/keyvault/base.go
@@ -18,8 +18,6 @@ type BaseClient interface {
 	GetSecret(ctx context.Context, vaultBaseURL string, secretName string, secretVersion string) (result keyvault.SecretBundle, err error)
 	GetCertificates(ctx context.Context, vaultBaseURL string, maxresults *int32, includePending *bool) (result keyvault.CertificateListResultPage, err error)
 	SetSecret(ctx context.Context, vaultBaseURL string, secretName string, parameters keyvault.SecretSetParameters) (result keyvault.SecretBundle, err error)
-	GetCertificatePolicy(ctx context.Context, vaultBaseURL string, certificateName string) (result keyvault.CertificatePolicy, err error)
-	UpdateCertificatePolicy(ctx context.Context, vaultBaseURL string, certificateName string, certificatePolicy keyvault.CertificatePolicy) (result keyvault.CertificatePolicy, err error)
 	BaseClientAddons
 }
 
diff --git a/pkg/util/keyvault/keyvault.go b/pkg/util/keyvault/keyvault.go
index 9d5fd2e42..de21a9ae8 100644
--- a/pkg/util/keyvault/keyvault.go
+++ b/pkg/util/keyvault/keyvault.go
@@ -10,7 +10,6 @@ import (
 	"crypto/x509/pkix"
 	"encoding/base64"
 	"fmt"
-	"reflect"
 	"strings"
 	"time"
 
@@ -45,7 +44,6 @@ type Manager interface {
 	GetSecret(context.Context, string) (keyvault.SecretBundle, error)
 	GetSecrets(context.Context) ([]keyvault.SecretItem, error)
 	SetSecret(context.Context, string, keyvault.SecretSetParameters) error
-	UpgradeCertificatePolicy(context.Context, string) error
 	WaitForCertificateOperation(context.Context, string) error
 }
 
@@ -198,33 +196,6 @@ func (m *manager) WaitForCertificateOperation(ctx context.Context, certificateNa
 	return err
 }
 
-func (m *manager) UpgradeCertificatePolicy(ctx context.Context, certificateName string) error {
-	policy, err := m.kv.GetCertificatePolicy(ctx, m.keyvaultURI, certificateName)
-	if err != nil {
-		return err
-	}
-
-	lifetimeActions := &[]keyvault.LifetimeAction{
-		{
-			Trigger: &keyvault.Trigger{
-				DaysBeforeExpiry: to.Int32Ptr(365 - 90),
-			},
-			Action: &keyvault.Action{
-				ActionType: keyvault.AutoRenew,
-			},
-		},
-	}
-
-	if reflect.DeepEqual(policy.LifetimeActions, lifetimeActions) {
-		return nil
-	}
-
-	policy.LifetimeActions = lifetimeActions
-
-	_, err = m.kv.UpdateCertificatePolicy(ctx, m.keyvaultURI, certificateName, policy)
-	return err
-}
-
 func keyvaultError(err *keyvault.Error) string {
 	if err == nil {
 		return ""
diff --git a/pkg/util/mocks/azureclient/keyvault/keyvault.go b/pkg/util/mocks/azureclient/keyvault/keyvault.go
index 6035ca9a9..2045db382 100644
--- a/pkg/util/mocks/azureclient/keyvault/keyvault.go
+++ b/pkg/util/mocks/azureclient/keyvault/keyvault.go
@@ -80,21 +80,6 @@ func (mr *MockBaseClientMockRecorder) GetCertificateOperation(arg0, arg1, arg2 i
 	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetCertificateOperation", reflect.TypeOf((*MockBaseClient)(nil).GetCertificateOperation), arg0, arg1, arg2)
 }
 
-// GetCertificatePolicy mocks base method
-func (m *MockBaseClient) GetCertificatePolicy(arg0 context.Context, arg1, arg2 string) (keyvault.CertificatePolicy, error) {
-	m.ctrl.T.Helper()
-	ret := m.ctrl.Call(m, "GetCertificatePolicy", arg0, arg1, arg2)
-	ret0, _ := ret[0].(keyvault.CertificatePolicy)
-	ret1, _ := ret[1].(error)
-	return ret0, ret1
-}
-
-// GetCertificatePolicy indicates an expected call of GetCertificatePolicy
-func (mr *MockBaseClientMockRecorder) GetCertificatePolicy(arg0, arg1, arg2 interface{}) *gomock.Call {
-	mr.mock.ctrl.T.Helper()
-	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetCertificatePolicy", reflect.TypeOf((*MockBaseClient)(nil).GetCertificatePolicy), arg0, arg1, arg2)
-}
-
 // GetCertificates mocks base method
 func (m *MockBaseClient) GetCertificates(arg0 context.Context, arg1 string, arg2 *int32, arg3 *bool) (keyvault.CertificateListResultPage, error) {
 	m.ctrl.T.Helper()
@@ -154,18 +139,3 @@ func (mr *MockBaseClientMockRecorder) SetSecret(arg0, arg1, arg2, arg3 interface
 	mr.mock.ctrl.T.Helper()
 	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetSecret", reflect.TypeOf((*MockBaseClient)(nil).SetSecret), arg0, arg1, arg2, arg3)
 }
-
-// UpdateCertificatePolicy mocks base method
-func (m *MockBaseClient) UpdateCertificatePolicy(arg0 context.Context, arg1, arg2 string, arg3 keyvault.CertificatePolicy) (keyvault.CertificatePolicy, error) {
-	m.ctrl.T.Helper()
-	ret := m.ctrl.Call(m, "UpdateCertificatePolicy", arg0, arg1, arg2, arg3)
-	ret0, _ := ret[0].(keyvault.CertificatePolicy)
-	ret1, _ := ret[1].(error)
-	return ret0, ret1
-}
-
-// UpdateCertificatePolicy indicates an expected call of UpdateCertificatePolicy
-func (mr *MockBaseClientMockRecorder) UpdateCertificatePolicy(arg0, arg1, arg2, arg3 interface{}) *gomock.Call {
-	mr.mock.ctrl.T.Helper()
-	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateCertificatePolicy", reflect.TypeOf((*MockBaseClient)(nil).UpdateCertificatePolicy), arg0, arg1, arg2, arg3)
-}
diff --git a/pkg/util/mocks/keyvault/keyvault.go b/pkg/util/mocks/keyvault/keyvault.go
index 65dff11a5..3f6849490 100644
--- a/pkg/util/mocks/keyvault/keyvault.go
+++ b/pkg/util/mocks/keyvault/keyvault.go
@@ -142,20 +142,6 @@ func (mr *MockManagerMockRecorder) SetSecret(arg0, arg1, arg2 interface{}) *gomo
 	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetSecret", reflect.TypeOf((*MockManager)(nil).SetSecret), arg0, arg1, arg2)
 }
 
-// UpgradeCertificatePolicy mocks base method
-func (m *MockManager) UpgradeCertificatePolicy(arg0 context.Context, arg1 string) error {
-	m.ctrl.T.Helper()
-	ret := m.ctrl.Call(m, "UpgradeCertificatePolicy", arg0, arg1)
-	ret0, _ := ret[0].(error)
-	return ret0
-}
-
-// UpgradeCertificatePolicy indicates an expected call of UpgradeCertificatePolicy
-func (mr *MockManagerMockRecorder) UpgradeCertificatePolicy(arg0, arg1 interface{}) *gomock.Call {
-	mr.mock.ctrl.T.Helper()
-	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpgradeCertificatePolicy", reflect.TypeOf((*MockManager)(nil).UpgradeCertificatePolicy), arg0, arg1)
-}
-
 // WaitForCertificateOperation mocks base method
 func (m *MockManager) WaitForCertificateOperation(arg0 context.Context, arg1 string) error {
 	m.ctrl.T.Helper()


Test patch:
diff --git a/pkg/cluster/deploystorage_test.go b/pkg/cluster/deploystorage_test.go
index 837f0754e..39d2bcd28 100644
--- a/pkg/cluster/deploystorage_test.go
+++ b/pkg/cluster/deploystorage_test.go
@@ -44,6 +44,7 @@ var daTestCases = []struct {
 			"Microsoft.Compute/snapshots/write",
 			"Microsoft.Compute/snapshots/delete",
 			"Microsoft.Network/networkInterfaces/effectiveRouteTable/action",
+			"Microsoft.Resources/subscriptions/resourceGroups/write",
 		},
 	},
 }