Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cache credentials from AzureClusterIdentity #5283

Merged
merged 1 commit into from
Nov 19, 2024
Merged

cache credentials from AzureClusterIdentity #5283

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
13 changes: 7 additions & 6 deletions azure/scope/cluster.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,12 @@ import (
// ClusterScopeParams defines the input parameters used to create a new Scope.
type ClusterScopeParams struct {
AzureClients
Client client.Client
Cluster *clusterv1.Cluster
AzureCluster *infrav1.AzureCluster
Cache *ClusterCache
Timeouts azure.AsyncReconciler
Client client.Client
Cluster *clusterv1.Cluster
AzureCluster *infrav1.AzureCluster
Cache *ClusterCache
Timeouts azure.AsyncReconciler
CredentialCache azure.CredentialCache
}

// NewClusterScope creates a new Scope from the supplied parameters.
Expand All @@ -78,7 +79,7 @@ func NewClusterScope(ctx context.Context, params ClusterScopeParams) (*ClusterSc
return nil, errors.New("failed to generate new scope from nil AzureCluster")
}

credentialsProvider, err := NewAzureCredentialsProvider(ctx, params.Client, params.AzureCluster.Spec.IdentityRef, params.AzureCluster.Namespace)
credentialsProvider, err := NewAzureCredentialsProvider(ctx, params.CredentialCache, params.Client, params.AzureCluster.Spec.IdentityRef, params.AzureCluster.Namespace)
if err != nil {
return nil, errors.Wrap(err, "failed to init credentials provider")
}
Expand Down
7 changes: 4 additions & 3 deletions azure/scope/cluster_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,9 +116,10 @@ func TestNewClusterScope(t *testing.T) {
fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(initObjects...).Build()

_, err := NewClusterScope(context.TODO(), ClusterScopeParams{
Cluster: cluster,
AzureCluster: azureCluster,
Client: fakeClient,
Cluster: cluster,
AzureCluster: azureCluster,
Client: fakeClient,
CredentialCache: azure.NewCredentialCache(),
})
g.Expect(err).NotTo(HaveOccurred())
}
Expand Down
33 changes: 16 additions & 17 deletions azure/scope/identity.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"

infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1"
"sigs.k8s.io/cluster-api-provider-azure/azure"
"sigs.k8s.io/cluster-api-provider-azure/pkg/ot"
"sigs.k8s.io/cluster-api-provider-azure/util/tele"
)
Expand All @@ -52,10 +53,12 @@ type CredentialsProvider interface {
type AzureCredentialsProvider struct {
Client client.Client
Identity *infrav1.AzureClusterIdentity

cache azure.CredentialCache
}

// NewAzureCredentialsProvider creates a new AzureClusterCredentialsProvider from the supplied inputs.
func NewAzureCredentialsProvider(ctx context.Context, kubeClient client.Client, identityRef *corev1.ObjectReference, defaultNamespace string) (*AzureCredentialsProvider, error) {
func NewAzureCredentialsProvider(ctx context.Context, cache azure.CredentialCache, kubeClient client.Client, identityRef *corev1.ObjectReference, defaultNamespace string) (*AzureCredentialsProvider, error) {
if identityRef == nil {
return nil, errors.New("failed to generate new AzureClusterCredentialsProvider from empty identityName")
}
Expand All @@ -74,6 +77,7 @@ func NewAzureCredentialsProvider(ctx context.Context, kubeClient client.Client,
return &AzureCredentialsProvider{
Client: kubeClient,
Identity: identity,
cache: cache,
}, nil
}

Expand All @@ -93,15 +97,14 @@ func (p *AzureCredentialsProvider) GetTokenCredential(ctx context.Context, resou

switch p.Identity.Spec.Type {
case infrav1.WorkloadIdentity:
azwiCredOptions, err := NewWorkloadIdentityCredentialOptions().
WithTenantID(p.Identity.Spec.TenantID).
WithClientID(p.Identity.Spec.ClientID).
WithDefaults()
if err != nil {
return nil, errors.Wrapf(err, "failed to setup azwi options for identity %s", p.Identity.Name)
}
azwiCredOptions.ClientOptions.TracingProvider = tracingProvider
cred, authErr = NewWorkloadIdentityCredential(azwiCredOptions)
cred, authErr = p.cache.GetOrStoreWorkloadIdentity(&azidentity.WorkloadIdentityCredentialOptions{
ClientOptions: azcore.ClientOptions{
TracingProvider: tracingProvider,
},
TenantID: p.Identity.Spec.TenantID,
ClientID: p.Identity.Spec.ClientID,
TokenFilePath: GetProjectedTokenPath(),
})

case infrav1.ManualServicePrincipal:
log.Info("Identity type ManualServicePrincipal is deprecated and will be removed in a future release. See https://capz.sigs.k8s.io/topics/identities to find a supported identity type.")
Expand All @@ -125,7 +128,7 @@ func (p *AzureCredentialsProvider) GetTokenCredential(ctx context.Context, resou
},
},
}
cred, authErr = azidentity.NewClientSecretCredential(p.GetTenantID(), p.Identity.Spec.ClientID, clientSecret, &options)
cred, authErr = p.cache.GetOrStoreClientSecret(p.GetTenantID(), p.Identity.Spec.ClientID, clientSecret, &options)

case infrav1.ServicePrincipalCertificate:
var certsContent []byte
Expand All @@ -141,11 +144,7 @@ func (p *AzureCredentialsProvider) GetTokenCredential(ctx context.Context, resou
}
certsContent = []byte(clientSecret)
}
certs, key, err := azidentity.ParseCertificates(certsContent, nil)
if err != nil {
return nil, errors.Wrap(err, "failed to parse certificate data")
}
cred, authErr = azidentity.NewClientCertificateCredential(p.GetTenantID(), p.Identity.Spec.ClientID, certs, key, &azidentity.ClientCertificateCredentialOptions{
cred, authErr = p.cache.GetOrStoreClientCert(p.GetTenantID(), p.Identity.Spec.ClientID, certsContent, nil, &azidentity.ClientCertificateCredentialOptions{
ClientOptions: azcore.ClientOptions{
TracingProvider: tracingProvider,
},
Expand All @@ -158,7 +157,7 @@ func (p *AzureCredentialsProvider) GetTokenCredential(ctx context.Context, resou
},
ID: azidentity.ClientID(p.Identity.Spec.ClientID),
}
cred, authErr = azidentity.NewManagedIdentityCredential(&options)
cred, authErr = p.cache.GetOrStoreManagedIdentity(&options)

default:
return nil, errors.Errorf("identity type %s not supported", p.Identity.Spec.Type)
Expand Down
91 changes: 77 additions & 14 deletions azure/scope/identity_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,21 @@ package scope

import (
"context"
"encoding/base64"
"os"
"reflect"
"testing"

"github.com/Azure/azure-sdk-for-go/sdk/azcore/cloud"
"github.com/Azure/azure-sdk-for-go/sdk/azidentity"
. "github.com/onsi/gomega"
"go.uber.org/mock/gomock"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"sigs.k8s.io/controller-runtime/pkg/client/fake"

infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1"
"sigs.k8s.io/cluster-api-provider-azure/azure/mock_azure"
)

func TestAllowedNamespaces(t *testing.T) {
Expand Down Expand Up @@ -202,20 +207,15 @@ func TestHasClientSecret(t *testing.T) {
}

func TestGetTokenCredential(t *testing.T) {
g := NewWithT(t)

// Test cert data was generated with this command:
// openssl req -x509 -noenc -days 3650 -newkey rsa:2048 --keyout - -subj /CN=localhost | base64
encodedCertData := "LS0tLS1CRUdJTiBQUklWQVRFIEtFWS0tLS0tCk1JSUV2UUlCQURBTkJna3Foa2lHOXcwQkFRRUZBQVNDQktjd2dnU2pBZ0VBQW9JQkFRRGpyZEVyOVAwVGFVRVMKZHNwRTZjeW8yMk5VOHloUnJiWWxWOVZIMnZXdm5Qc1RoWGN4aG5kK2NVcWRORUJzd2h3Z0ZsVVFjZy9lU1Z4dwpyciszbmgrYkZUWldQY1krMUxRWXhmcEtHc3JDWFFmQjgyTERKSVpEWDRnSFlyV2YzWjI3MmpYTjFYZUZBS3RpCndES2dEWFh1UEg3cjVsSDd2QzNSWGVBZmZxTHdRSmhaZitOb0hOdHY5TUg5SWRVa1FmbURGWnRJL0NRekNyYjYKK3ZPUzZFbVVEL1EyRk5IQnpneENndUdxZ055QmNRYnhKOVFuZytaaklGdWhHWVhKbHN5UlV0ZXh5elRSNS92MApWTks4VXNaZ1JCRmhYcXJCdi9Sb0NDRyt4VkpZdG1kMFFzcnZOekRxRzZRbmpVQjIxelZYcXpLRWtXMmdSdGpYCmN3NHZZUWVoQWdNQkFBRUNnZ0VBUzZ4dGpnMG5Bb2trMGpTK1pPcEtsa01aQUZhemEzWnZ5SGlwa0hEejRQTXQKdGw3UmI1b1FaR3ZXVDJyYkVPcnhleTdCQmk3TEhHaEl1OEV4UXAvaFJHUG9CQUVUUDdYbHlDZ2hXUGtQdEV0RQpkVS9tWHhMb04wTnN6SHVmLzJzaTdwbUg4WXFHWjZRQjB0Z3IyMnV0NjBtYksrQUpGc0VFZjRhU3BCVXNwZXBKCjI4MDBzUUhzcVBFNkw2a1lrZloyR1JSWTFWOXZVcllFT0RLWnBXek1oTjNVQTluQUtIOVBCNnh2UDJPZHlNTmgKaEtnbVVVTU5JRnR3cjhwWmxKbjYwY2YwVXJXcmM1Q3ZxUUx1YUdZbHpEZ1VRR1Y0SkVWanFtOUY2bE1mRVBVdwplTjcwTVZlMXBjTGVMcTJyR0NWV1UzZ2FraC9IdkpxbFIvc2E1NDZIZ3dLQmdRRHlmMXZreVg0dzVzYm9pNkRKCmNsNWRNVUx0TU1ScEIxT2FNRlZPSmpJOWdaSjhtQ2RSanFYZFlvNWFTMktJcXhpZTh0R0c5K1NvaHhEQVdsNHQKbFNVdERzRTQ0ZlNtSUxxQzV6SWF3TlJRbm5rdjBYOEx3bVl1MFFkN1lBakpNbExUV3lEUnNqRDlYUnE0bnNSKwptSlZ3cnQ4NWlTcFM1VUZ5cnlFelBiRmowd0tCZ1FEd1d6cmFlTjBFY2NmMWlJWW1Rc1l5K3lNRUFsSE5SNXlpCmdQWHVBaFN5YnYySlJlUmhkVWIzOWhMci9Mdkt3MFplWGlMV1htWVVHcGJ5elB5WEltMHMrUEwzTFdsNjVHVEYKbCtjZlY1d2ZBZERrazZyQWRFUEVFMnB4Tjg1Q2h5YVBZUG9ZcjBvaG1WOTdWUWNZYzVGcVkrajF0TTZSMVJEdAovZldCU2E4aU93S0JnUUNwYTFkdFdXVERqNGdxVWRyc3d1MndtRWtVNDd4bFVJd1ZMbTE2NHU2NHovemk5WDZLCjJXbUNhV2ZoSjhmWWlnanlpOXpkT2ZYVDFFRmMwZ1g0UExvelo1cVJQalFwbUxZVjNLYkIwRFRGZW1KYWlUZ0UKcERXMXdhNURnUTNDVzFsSWR1TlAvZm1DR2ZrZ1FUUXc2ak9GL1hiUmdNWkVFZzJPclZJNXRZRm9wd0tCZ0VSOQppcWpFdGg1VkdlakNqWStMaVpUdmNVdnNLVWs0dGM2c3R1ZXFtaUU2ZFc3UGhzT3F1cDFmOW9aZWoxaTVDbTFMCm45dThMSlJmKzFHV3pnZDNIT3NxeVhsYjdHbkRlVi9BNkhCSzg4YjJLb05uL01rNG1ETGdZWDEvckh2U3JVOUEKRUNSR2x2WTZFVFpBeFhQWFFzR3hWS25uYXRHdGlGUjVBS05senMwUEFvR0FhNStYK0RVcUdoOWFFNUlEM3dydgpqa2p4UTJLTEZKQ05TcThmOUdTdXZwdmdYc3RIaDZ3S29NNnZNd0lTaGpnWHVVUkg4VWI0dWhSc1dueE1pbGRGCjdFRStRYVdVOWpuQ20ySFFZQXJmWHJBV3c2REJ1ZGlTa0JxZ0tjNkhqREh1bjVmWGxZVW84VWVzTk1RT3JnN2IKYnlkUVo1LzRWLzFvU1dQRVRrN2pTcjA9Ci0tLS0tRU5EIFBSSVZBVEUgS0VZLS0tLS0KLS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSURDVENDQWZHZ0F3SUJBZ0lVRlNudEVuK1R2NkhNMnhKUmVFQ0pwSmNDN2lVd0RRWUpLb1pJaHZjTkFRRUwKQlFBd0ZERVNNQkFHQTFVRUF3d0piRzlqWVd4b2IzTjBNQjRYRFRJME1ERXdPREU1TlRReE5Gb1hEVE0wTURFdwpOVEU1TlRReE5Gb3dGREVTTUJBR0ExVUVBd3dKYkc5allXeG9iM04wTUlJQklqQU5CZ2txaGtpRzl3MEJBUUVGCkFBT0NBUThBTUlJQkNnS0NBUUVBNDYzUksvVDlFMmxCRW5iS1JPbk1xTnRqVlBNb1VhMjJKVmZWUjlyMXI1ejcKRTRWM01ZWjNmbkZLblRSQWJNSWNJQlpWRUhJUDNrbGNjSzYvdDU0Zm14VTJWajNHUHRTMEdNWDZTaHJLd2wwSAp3Zk5pd3lTR1ExK0lCMksxbjkyZHU5bzF6ZFYzaFFDcllzQXlvQTExN2p4KzYrWlIrN3d0MFYzZ0gzNmk4RUNZCldYL2phQnpiYi9UQi9TSFZKRUg1Z3hXYlNQd2tNd3EyK3Zyemt1aEpsQS8wTmhUUndjNE1Rb0xocW9EY2dYRUcKOFNmVUo0UG1ZeUJib1JtRnlaYk1rVkxYc2NzMDBlZjc5RlRTdkZMR1lFUVJZVjZxd2IvMGFBZ2h2c1ZTV0xabgpkRUxLN3pjdzZodWtKNDFBZHRjMVY2c3loSkZ0b0ViWTEzTU9MMkVIb1FJREFRQUJvMU13VVRBZEJnTlZIUTRFCkZnUVVmcnkvS0R0YW13TWxSUXNGUGJCaHpkdjJVNWN3SHdZRFZSMGpCQmd3Rm9BVWZyeS9LRHRhbXdNbFJRc0YKUGJCaHpkdjJVNWN3RHdZRFZSMFRBUUgvQkFVd0F3RUIvekFOQmdrcWhraUc5dzBCQVFzRkFBT0NBUUVBeVlzdApWdmV3S1JScHVZUldjNFhHNlduWXBoVWR5WkxNb0lscTBzeVoxYWo2WWJxb0s5Tk1IQVlFbkN2U292NnpJWk9hCnRyaHVVY2Y5R0Z6NWUwaUoyeklsRGMzMTJJd3N2NDF4aUMvYnMxNmtFbjhZZi9TdWpFWGFzajd2bUEzSHJGV2YKd1pUSC95Rkw1YXpvL2YrbEExUTI4WXdxRnBIbWxlMHkwTzUzVXRoNHAwdG13bG51K0NyTzlmSHAza1RsYjdmRAo2bXFmazlOcnQ4dE9DNGFIWURvcXRZVWdaaHg1OHhzSE1PVGV0S2VSbHA4SE1GOW9ST3RyaXo0blltNkloVHdvCjVrMUExM1MzQmpheGtaQ3lQWENnWHNzdVhhZ05MYXNycjVRcStWZ2RiL25EaFZlaFY4K1o0SjBZbnp5OU1ac0UKSDFOMU5mTXRzQStQRXF0UFhBPT0KLS0tLS1FTkQgQ0VSVElGSUNBVEUtLS0tLQo="
certPEM, err := base64.StdEncoding.DecodeString(encodedCertData)
g.Expect(err).NotTo(HaveOccurred())
testCertPath := "../../test/setup/certificate"

tests := []struct {
name string
cluster *infrav1.AzureCluster
secret *corev1.Secret
identity *infrav1.AzureClusterIdentity
ActiveDirectoryAuthorityHost string
cacheExpect func(*mock_azure.MockCredentialCache)
}{
{
name: "workload identity",
Expand All @@ -235,6 +235,14 @@ func TestGetTokenCredential(t *testing.T) {
TenantID: fakeTenantID,
},
},
cacheExpect: func(cache *mock_azure.MockCredentialCache) {
cache.EXPECT().GetOrStoreWorkloadIdentity(gomock.Cond(func(opts *azidentity.WorkloadIdentityCredentialOptions) bool {
// ignore tracing provider
return opts.TenantID == fakeTenantID &&
opts.ClientID == fakeClientID &&
opts.TokenFilePath == GetProjectedTokenPath()
}))
},
},
{
name: "manual service principal",
Expand All @@ -251,6 +259,7 @@ func TestGetTokenCredential(t *testing.T) {
Spec: infrav1.AzureClusterIdentitySpec{
Type: infrav1.ManualServicePrincipal,
TenantID: fakeTenantID,
ClientID: fakeClientID,
ClientSecret: corev1.SecretReference{
Name: "test-identity-secret",
},
Expand All @@ -265,6 +274,20 @@ func TestGetTokenCredential(t *testing.T) {
},
},
ActiveDirectoryAuthorityHost: "https://login.microsoftonline.com",
cacheExpect: func(cache *mock_azure.MockCredentialCache) {
cache.EXPECT().GetOrStoreClientSecret(fakeTenantID, fakeClientID, "fooSecret", gomock.Cond(func(opts *azidentity.ClientSecretCredentialOptions) bool {
// ignore tracing provider
return reflect.DeepEqual(opts.ClientOptions.Cloud, cloud.Configuration{
ActiveDirectoryAuthorityHost: "https://login.microsoftonline.com",
Services: map[cloud.ServiceName]cloud.ServiceConfiguration{
cloud.ResourceManager: {
Audience: "",
Endpoint: "",
},
},
})
}))
},
},
{
name: "service principal",
Expand All @@ -281,6 +304,7 @@ func TestGetTokenCredential(t *testing.T) {
Spec: infrav1.AzureClusterIdentitySpec{
Type: infrav1.ServicePrincipal,
TenantID: fakeTenantID,
ClientID: fakeClientID,
ClientSecret: corev1.SecretReference{
Name: "test-identity-secret",
},
Expand All @@ -295,6 +319,20 @@ func TestGetTokenCredential(t *testing.T) {
},
},
ActiveDirectoryAuthorityHost: "https://login.microsoftonline.com",
cacheExpect: func(cache *mock_azure.MockCredentialCache) {
cache.EXPECT().GetOrStoreClientSecret(fakeTenantID, fakeClientID, "fooSecret", gomock.Cond(func(opts *azidentity.ClientSecretCredentialOptions) bool {
// ignore tracing provider
return reflect.DeepEqual(opts.ClientOptions.Cloud, cloud.Configuration{
ActiveDirectoryAuthorityHost: "https://login.microsoftonline.com",
Services: map[cloud.ServiceName]cloud.ServiceConfiguration{
cloud.ResourceManager: {
Audience: "",
Endpoint: "",
},
},
})
}))
},
},
{
name: "service principal certificate",
Expand All @@ -311,17 +349,23 @@ func TestGetTokenCredential(t *testing.T) {
Spec: infrav1.AzureClusterIdentitySpec{
Type: infrav1.ServicePrincipalCertificate,
TenantID: fakeTenantID,
CertPath: "../../test/setup/certificate",
ClientID: fakeClientID,
ClientSecret: corev1.SecretReference{
Name: "test-identity-secret",
},
},
},
secret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "test-identity-secret",
},
Data: map[string][]byte{
"clientSecret": certPEM,
"clientSecret": []byte("fooSecret"),
},
},
cacheExpect: func(cache *mock_azure.MockCredentialCache) {
cache.EXPECT().GetOrStoreClientCert(fakeTenantID, fakeClientID, []byte("fooSecret"), gomock.Nil(), gomock.Any())
},
},
{
name: "service principal certificate with certificate filepath",
Expand All @@ -338,9 +382,17 @@ func TestGetTokenCredential(t *testing.T) {
Spec: infrav1.AzureClusterIdentitySpec{
Type: infrav1.ServicePrincipalCertificate,
TenantID: fakeTenantID,
CertPath: "../../test/setup/certificate",
ClientID: fakeClientID,
CertPath: testCertPath,
},
},
cacheExpect: func(cache *mock_azure.MockCredentialCache) {
expectedCert, err := os.ReadFile(testCertPath)
if err != nil {
panic(err)
}
cache.EXPECT().GetOrStoreClientCert(fakeTenantID, fakeClientID, expectedCert, gomock.Nil(), gomock.Any())
},
},
{
name: "user-assigned identity",
Expand All @@ -357,8 +409,15 @@ func TestGetTokenCredential(t *testing.T) {
Spec: infrav1.AzureClusterIdentitySpec{
Type: infrav1.UserAssignedMSI,
TenantID: fakeTenantID,
ClientID: fakeClientID,
},
},
cacheExpect: func(cache *mock_azure.MockCredentialCache) {
cache.EXPECT().GetOrStoreManagedIdentity(gomock.Cond(func(opts *azidentity.ManagedIdentityCredentialOptions) bool {
// ignore tracing provider
return opts.ID == azidentity.ClientID(fakeClientID)
}))
},
},
}

Expand All @@ -377,11 +436,15 @@ func TestGetTokenCredential(t *testing.T) {
initObjects = append(initObjects, tt.secret)
}
fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(initObjects...).Build()
provider, err := NewAzureCredentialsProvider(context.Background(), fakeClient, tt.cluster.Spec.IdentityRef, "")

mockCtrl := gomock.NewController(t)
cache := mock_azure.NewMockCredentialCache(mockCtrl)
tt.cacheExpect(cache)

provider, err := NewAzureCredentialsProvider(context.Background(), cache, fakeClient, tt.cluster.Spec.IdentityRef, "")
g.Expect(err).NotTo(HaveOccurred())
cred, err := provider.GetTokenCredential(context.Background(), "", tt.ActiveDirectoryAuthorityHost, "")
_, err = provider.GetTokenCredential(context.Background(), "", tt.ActiveDirectoryAuthorityHost, "")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why aren't we asserting the first response object?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That value is returned by a mock defined by the test, so it doesn't really mean anything.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main validation of that is covered by verifying the arguments we pass to the p.cache.GetOrStore... calls in the cacheExpect of each test case.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see that GetTokenCredential returns non-nil every time err is nil, maybe I'm being extra-defensive and protecting against the possibility that GetTokenCredential changes and an undesirable outcome is identified only in the TokenCredential object.

Non-blocking.

g.Expect(err).NotTo(HaveOccurred())
g.Expect(cred).NotTo(BeNil())
})
}
}
3 changes: 2 additions & 1 deletion azure/scope/managedcontrolplane.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ type ManagedControlPlaneScopeParams struct {
ManagedMachinePools []ManagedMachinePool
Cache *ManagedControlPlaneCache
Timeouts azure.AsyncReconciler
CredentialCache azure.CredentialCache
}

// NewManagedControlPlaneScope creates a new Scope from the supplied parameters.
Expand All @@ -91,7 +92,7 @@ func NewManagedControlPlaneScope(ctx context.Context, params ManagedControlPlane
return nil, errors.New("failed to generate new scope from nil ControlPlane")
}

credentialsProvider, err := NewAzureCredentialsProvider(ctx, params.Client, params.ControlPlane.Spec.IdentityRef, params.ControlPlane.Namespace)
credentialsProvider, err := NewAzureCredentialsProvider(ctx, params.CredentialCache, params.Client, params.ControlPlane.Spec.IdentityRef, params.ControlPlane.Namespace)
if err != nil {
return nil, errors.Wrap(err, "failed to init credentials provider")
}
Expand Down
1 change: 1 addition & 0 deletions azure/scope/managedcontrolplane_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ func TestNewManagedControlPlaneScope(t *testing.T) {
},
},
},
CredentialCache: azure.NewCredentialCache(),
}
fakeIdentity := &infrav1.AzureClusterIdentity{
ObjectMeta: metav1.ObjectMeta{
Expand Down
Loading