Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pkg/cluster/adminupdate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ func TestAdminUpdateSteps(t *testing.T) {
}

generalFixesSteps := []string{
"[Action ensureResourceGroup]",
"[AuthorizationRetryingAction ensureResourceGroup]",
"[Action createOrUpdateDenyAssignment]",
"[Action ensureServiceEndpoints]",
"[Action populateRegistryStorageAccountName]",
Expand Down
8 changes: 6 additions & 2 deletions pkg/cluster/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ type manager struct {
doc *api.OpenShiftClusterDocument
subscriptionDoc *api.SubscriptionDocument
fpAuthorizer refreshable.Authorizer
fpCredRefresher refreshable.Rebuilder
localFpAuthorizer autorest.Authorizer
metricsEmitter metrics.Emitter

Expand Down Expand Up @@ -158,7 +159,7 @@ func New(ctx context.Context, log *logrus.Entry, _env env.Interface, db database
return nil, err
}

fpCredClusterTenant, err := _env.FPNewClientCertificateCredential(subscriptionDoc.Subscription.Properties.TenantID, nil)
fpCredClusterTenant, err := refreshable.NewFPTokenCredential(_env, subscriptionDoc.Subscription.Properties.TenantID, nil)
if err != nil {
return nil, err
}
Expand All @@ -173,11 +174,13 @@ func New(ctx context.Context, log *logrus.Entry, _env env.Interface, db database
}
fpspID := tokenClaims.ObjectId

fpCredRPTenant, err := _env.FPNewClientCertificateCredential(_env.TenantID(), nil)
fpCredRPTenant, err := refreshable.NewFPTokenCredential(_env, _env.TenantID(), nil)
if err != nil {
return nil, err
}

fpCredRefresher := refreshable.NewMultiRebuilder(fpAuthorizer, fpCredClusterTenant, fpCredRPTenant)

msiCredential, err := _env.NewMSITokenCredential()
if err != nil {
return nil, err
Expand Down Expand Up @@ -283,6 +286,7 @@ func New(ctx context.Context, log *logrus.Entry, _env env.Interface, db database
doc: doc,
subscriptionDoc: subscriptionDoc,
fpAuthorizer: fpAuthorizer,
fpCredRefresher: fpCredRefresher,
localFpAuthorizer: localFPAuthorizer,
metricsEmitter: metricsEmitter,
disks: compute.NewDisksClient(_env.Environment(), r.SubscriptionID, fpAuthorizer),
Expand Down
54 changes: 27 additions & 27 deletions pkg/cluster/install.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,9 @@ func (m *manager) getZerothSteps() []steps.Step {
steps.Action(m.fixupClusterMsiTenantID),
steps.Action(m.ensureClusterMsiCertificate),
steps.Action(m.initializeClusterMsiClients),
steps.AuthorizationRetryingAction(m.fpAuthorizer, m.clusterIdentityIDs, m.managedResourceGroupName()),
steps.AuthorizationRetryingAction(m.fpAuthorizer, m.platformWorkloadIdentityIDs, m.managedResourceGroupName()),
steps.AuthorizationRetryingAction(m.fpAuthorizer, m.persistPlatformWorkloadIdentityIDs, m.managedResourceGroupName()),
steps.AuthorizationRetryingAction(m.fpCredRefresher, m.clusterIdentityIDs, m.managedResourceGroupName()),
steps.AuthorizationRetryingAction(m.fpCredRefresher, m.platformWorkloadIdentityIDs, m.managedResourceGroupName()),
steps.AuthorizationRetryingAction(m.fpCredRefresher, m.persistPlatformWorkloadIdentityIDs, m.managedResourceGroupName()),
}

bootstrap = append(bootstrap, managedIdentitySteps...)
Expand All @@ -122,16 +122,16 @@ func (m *manager) getEnsureAPIServerReadySteps() []steps.Step {

func (m *manager) getGeneralFixesSteps() []steps.Step {
stepsThatDontNeedAPIServer := []steps.Step{
steps.Action(m.ensureResourceGroup), // re-create RP RBAC if needed after tenant migration
steps.AuthorizationRetryingAction(m.fpCredRefresher, m.ensureResourceGroup, m.managedResourceGroupName()), // re-create RP RBAC if needed after tenant migration
steps.Action(m.createOrUpdateDenyAssignment),
steps.Action(m.ensureServiceEndpoints),
steps.Action(m.populateRegistryStorageAccountName), // must go before migrateStorageAccounts
steps.Action(m.migrateStorageAccounts),
steps.Action(m.fixSSH),
}
stepsThatNeedAPIServer := []steps.Step{
steps.AuthorizationRetryingAction(m.fpAuthorizer, m.fixSREKubeconfig, m.managedResourceGroupName()),
steps.AuthorizationRetryingAction(m.fpAuthorizer, m.fixUserAdminKubeconfig, m.managedResourceGroupName()),
steps.AuthorizationRetryingAction(m.fpCredRefresher, m.fixSREKubeconfig, m.managedResourceGroupName()),
steps.AuthorizationRetryingAction(m.fpCredRefresher, m.fixUserAdminKubeconfig, m.managedResourceGroupName()),
steps.Action(m.createOrUpdateRouterIPFromCluster),

steps.Action(m.ensureGatewayUpgrade),
Expand All @@ -152,7 +152,7 @@ func (m *manager) getCertificateRenewalSteps() []steps.Step {
s := []steps.Step{
steps.Action(m.populateDatabaseIntIP),
steps.Action(m.correctCertificateIssuer),
steps.AuthorizationRetryingAction(m.fpAuthorizer, m.fixMCSCert, m.managedResourceGroupName()),
steps.AuthorizationRetryingAction(m.fpCredRefresher, m.fixMCSCert, m.managedResourceGroupName()),
steps.Action(m.fixMCSUserData),
steps.Action(m.configureAPIServerCertificate),
steps.Action(m.configureIngressCertificate),
Expand Down Expand Up @@ -248,20 +248,20 @@ func (m *manager) Update(ctx context.Context) error {
)
}

s = append(s, steps.AuthorizationRetryingAction(m.fpAuthorizer, m.validateResources, m.managedResourceGroupName()))
s = append(s, steps.AuthorizationRetryingAction(m.fpCredRefresher, m.validateResources, m.managedResourceGroupName()))

if m.doc.OpenShiftCluster.UsesWorkloadIdentity() {
s = append(s,
steps.AuthorizationRetryingAction(m.fpAuthorizer, m.clusterIdentityIDs, m.managedResourceGroupName()),
steps.AuthorizationRetryingAction(m.fpAuthorizer, m.persistPlatformWorkloadIdentityIDs, m.managedResourceGroupName()),
steps.AuthorizationRetryingAction(m.fpCredRefresher, m.clusterIdentityIDs, m.managedResourceGroupName()),
steps.AuthorizationRetryingAction(m.fpCredRefresher, m.persistPlatformWorkloadIdentityIDs, m.managedResourceGroupName()),
steps.Action(m.ensurePlatformWorkloadIdentityRBAC),
steps.AuthorizationRetryingAction(nil, m.federateIdentityCredentials, m.managedResourceGroupName()),
)
} else {
s = append(s,
// Since ServicePrincipalProfile is now a pointer and our converters re-build the struct,
// our update path needs to enrich the doc with SPObjectID since it was overwritten by our API on put/patch.
steps.AuthorizationRetryingAction(m.fpAuthorizer, m.fixupClusterSPObjectID, m.managedResourceGroupName()),
steps.AuthorizationRetryingAction(m.fpCredRefresher, m.fixupClusterSPObjectID, m.managedResourceGroupName()),

// CSP credentials rotation flow steps
steps.Action(m.createOrUpdateClusterServicePrincipalRBAC))
Expand All @@ -277,7 +277,7 @@ func (m *manager) Update(ctx context.Context) error {
steps.Action(m.correctCertificateIssuer),
steps.Action(m.configureAPIServerCertificate),
steps.Action(m.configureIngressCertificate),
steps.AuthorizationRetryingAction(m.fpAuthorizer, m.fixUserAdminKubeconfig, m.managedResourceGroupName()),
steps.AuthorizationRetryingAction(m.fpCredRefresher, m.fixUserAdminKubeconfig, m.managedResourceGroupName()),
steps.Action(m.reconcileLoadBalancerProfile),
steps.Action(m.reconcileSoftwareDefinedNetwork),
steps.Action(m.ensureCredentialsRequest),
Expand Down Expand Up @@ -398,19 +398,19 @@ func (m *manager) bootstrap() []steps.Step {
}

s = append(s,
steps.AuthorizationRetryingAction(m.fpAuthorizer, m.validateResources, m.managedResourceGroupName()),
steps.AuthorizationRetryingAction(m.fpAuthorizer, m.validateZones, m.managedResourceGroupName()),
steps.AuthorizationRetryingAction(m.fpCredRefresher, m.validateResources, m.managedResourceGroupName()),
Comment thread
kimorris27 marked this conversation as resolved.
steps.AuthorizationRetryingAction(m.fpCredRefresher, m.validateZones, m.managedResourceGroupName()),
)

if m.doc.OpenShiftCluster.UsesWorkloadIdentity() {
s = append(s,
steps.AuthorizationRetryingAction(m.fpAuthorizer, m.clusterIdentityIDs, m.managedResourceGroupName()),
steps.AuthorizationRetryingAction(m.fpAuthorizer, m.persistPlatformWorkloadIdentityIDs, m.managedResourceGroupName()),
steps.AuthorizationRetryingAction(m.fpCredRefresher, m.clusterIdentityIDs, m.managedResourceGroupName()),
steps.AuthorizationRetryingAction(m.fpCredRefresher, m.persistPlatformWorkloadIdentityIDs, m.managedResourceGroupName()),
)
} else {
s = append(s,
steps.Action(m.initializeClusterSPClients),
steps.AuthorizationRetryingAction(m.fpAuthorizer, m.clusterSPObjectID, m.managedResourceGroupName()),
steps.AuthorizationRetryingAction(m.fpCredRefresher, m.clusterSPObjectID, m.managedResourceGroupName()),
)
}
s = append(s,
Expand All @@ -422,10 +422,10 @@ func (m *manager) bootstrap() []steps.Step {
steps.Action(m.populateMTUSize),
steps.Action(m.createDNS),
steps.Action(m.createOIDC),
steps.Action(m.ensureResourceGroup),
steps.AuthorizationRetryingAction(m.fpCredRefresher, m.ensureResourceGroup, m.managedResourceGroupName()),
steps.Action(m.ensureServiceEndpoints),
steps.Action(m.setMasterSubnetPolicies),
steps.AuthorizationRetryingAction(m.fpAuthorizer, m.deployBaseResourceTemplate, m.managedResourceGroupName()),
steps.AuthorizationRetryingAction(m.fpCredRefresher, m.deployBaseResourceTemplate, m.managedResourceGroupName()),
)

if m.doc.OpenShiftCluster.UsesWorkloadIdentity() {
Expand All @@ -435,12 +435,12 @@ func (m *manager) bootstrap() []steps.Step {
}

s = append(s,
steps.AuthorizationRetryingAction(m.fpAuthorizer, m.attachNSGs, m.managedResourceGroupName()),
steps.AuthorizationRetryingAction(m.fpCredRefresher, m.attachNSGs, m.managedResourceGroupName()),
// Allow extra time for ARM authorization to propagate to the internal load balancer.
steps.AuthorizationRetryingActionWithTimeout(m.fpAuthorizer, m.updateAPIIPEarly, m.managedResourceGroupName(), 15*time.Minute),
steps.AuthorizationRetryingAction(m.fpAuthorizer, m.createOrUpdateRouterIPEarly, m.managedResourceGroupName()),
steps.AuthorizationRetryingAction(m.fpAuthorizer, m.ensureGatewayCreate, m.managedResourceGroupName()),
steps.AuthorizationRetryingAction(m.fpAuthorizer, m.createAPIServerPrivateEndpoint, m.managedResourceGroupName()),
steps.AuthorizationRetryingActionWithTimeout(m.fpCredRefresher, m.updateAPIIPEarly, m.managedResourceGroupName(), 15*time.Minute),
steps.AuthorizationRetryingAction(m.fpCredRefresher, m.createOrUpdateRouterIPEarly, m.managedResourceGroupName()),
steps.AuthorizationRetryingAction(m.fpCredRefresher, m.ensureGatewayCreate, m.managedResourceGroupName()),
steps.AuthorizationRetryingAction(m.fpCredRefresher, m.createAPIServerPrivateEndpoint, m.managedResourceGroupName()),
steps.Action(m.createCertificates),
)

Expand All @@ -456,12 +456,12 @@ func (m *manager) bootstrap() []steps.Step {
// Give Hive 60 minutes to install the cluster, since this includes
// all of bootstrapping being complete
steps.Condition(m.hiveClusterInstallationComplete, 60*time.Minute, true),
steps.AuthorizationRetryingAction(m.fpAuthorizer, m.generateKubeconfigs, m.managedResourceGroupName()),
steps.AuthorizationRetryingAction(m.fpCredRefresher, m.generateKubeconfigs, m.managedResourceGroupName()),
)
} else {
s = append(s,
steps.Action(m.runPodmanInstaller),
steps.AuthorizationRetryingAction(m.fpAuthorizer, m.generateKubeconfigs, m.managedResourceGroupName()),
steps.AuthorizationRetryingAction(m.fpCredRefresher, m.generateKubeconfigs, m.managedResourceGroupName()),
)

if m.adoptViaHive {
Expand Down Expand Up @@ -517,7 +517,7 @@ func (m *manager) Install(ctx context.Context) error {
steps.Action(m.disableUpdates),
steps.Condition(m.clusterVersionReady, 30*time.Minute, true),
steps.Condition(m.aroDeploymentReady, 20*time.Minute, true),
steps.AuthorizationRetryingAction(m.fpAuthorizer, m.updateClusterData, m.managedResourceGroupName()),
steps.AuthorizationRetryingAction(m.fpCredRefresher, m.updateClusterData, m.managedResourceGroupName()),
steps.Action(m.configureIngressCertificate),
steps.Condition(m.ingressControllerReady, 30*time.Minute, true),
steps.Action(m.configureDefaultStorageClass),
Expand Down
45 changes: 45 additions & 0 deletions pkg/util/refreshable/authorizer.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
package refreshable

// Copyright (c) Microsoft Corporation.
// Licensed under the Apache License 2.0.

import (
"github.com/Azure/go-autorest/autorest"

"github.com/Azure/ARO-RP/pkg/env"
)

type Authorizer interface {
autorest.Authorizer
Rebuilder
}

type authorizer struct {
auth autorest.Authorizer
env env.Interface
tenantID string
}

func (a *authorizer) Rebuild() error {
auth, err := a.env.FPAuthorizer(a.tenantID, nil, a.env.Environment().ResourceManagerScope)
if err != nil {
return err
}
a.auth = auth
return nil
}

func (a *authorizer) WithAuthorization() autorest.PrepareDecorator {
return a.auth.WithAuthorization()
}

// NewAuthorizer creates an Authorizer that can be rebuilt when needed to force
// token recreation.
func NewAuthorizer(_env env.Interface, tenantID string) (Authorizer, error) {
a := &authorizer{
env: _env,
tenantID: tenantID,
}
err := a.Rebuild()
return a, err
}
46 changes: 15 additions & 31 deletions pkg/util/refreshable/refreshable.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,43 +3,27 @@ package refreshable
// Copyright (c) Microsoft Corporation.
// Licensed under the Apache License 2.0.

import (
"github.com/Azure/go-autorest/autorest"

"github.com/Azure/ARO-RP/pkg/env"
)

type Authorizer interface {
autorest.Authorizer
type Rebuilder interface {
Rebuild() error
}

type authorizer struct {
auth autorest.Authorizer
env env.Interface
tenantID string
}
type multiRebuilder []Rebuilder

func (m multiRebuilder) Rebuild() error {
for _, r := range m {
if r == nil {
continue
}

func (a *authorizer) Rebuild() error {
auth, err := a.env.FPAuthorizer(a.tenantID, nil, a.env.Environment().ResourceManagerScope)
if err != nil {
return err
err := r.Rebuild()
if err != nil {
return err
}
}
a.auth = auth
return nil
}

func (a *authorizer) WithAuthorization() autorest.PrepareDecorator {
return a.auth.WithAuthorization()
return nil
}

// NewAuthorizer creates an Authorizer that can be rebuilt when needed to force
// token recreation.
func NewAuthorizer(_env env.Interface, tenantID string) (Authorizer, error) {
a := &authorizer{
env: _env,
tenantID: tenantID,
}
err := a.Rebuild()
return a, err
func NewMultiRebuilder(rebuilders ...Rebuilder) Rebuilder {
return multiRebuilder(rebuilders)
}
55 changes: 55 additions & 0 deletions pkg/util/refreshable/refreshable_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
package refreshable

// Copyright (c) Microsoft Corporation.
// Licensed under the Apache License 2.0.

import (
"errors"
"testing"
)

type fakeRebuilder struct {
calls int
err error
}

func (r *fakeRebuilder) Rebuild() error {
r.calls++
return r.err
}

func TestMultiRebuilder(t *testing.T) {
wantErr := errors.New("boom")
Comment thread
rajdeepc2792 marked this conversation as resolved.

for _, tt := range []struct {
name string
first *fakeRebuilder
second *fakeRebuilder
wantErr error
wantCalls []int
}{
{
name: "rebuilds all",
first: &fakeRebuilder{},
second: &fakeRebuilder{},
wantCalls: []int{1, 1},
},
{
name: "stops at first error",
first: &fakeRebuilder{err: wantErr},
second: &fakeRebuilder{},
wantErr: wantErr,
wantCalls: []int{1, 0},
},
} {
t.Run(tt.name, func(t *testing.T) {
err := NewMultiRebuilder(tt.first, nil, tt.second).Rebuild()
if !errors.Is(err, tt.wantErr) {
t.Errorf("got %v, wanted %v", err, tt.wantErr)
}
if tt.first.calls != tt.wantCalls[0] || tt.second.calls != tt.wantCalls[1] {
t.Errorf("got calls %d,%d wanted %v", tt.first.calls, tt.second.calls, tt.wantCalls)
}
})
}
}
Loading
Loading