From 8b0ac045e26e013e6e10a7797df18191ff9de096 Mon Sep 17 00:00:00 2001 From: andyzhangx Date: Sun, 12 Jun 2022 04:45:59 +0000 Subject: [PATCH] fix image pulling failure when IMDS is unavailalbe in kubelet startup fix test failure --- .../azure/azure_credentials.go | 57 +++++-------------- .../azure/azure_credentials_test.go | 33 +---------- 2 files changed, 17 insertions(+), 73 deletions(-) diff --git a/pkg/credentialprovider/azure/azure_credentials.go b/pkg/credentialprovider/azure/azure_credentials.go index 1443f997f1e..8302ebb47e8 100644 --- a/pkg/credentialprovider/azure/azure_credentials.go +++ b/pkg/credentialprovider/azure/azure_credentials.go @@ -31,7 +31,6 @@ import ( "time" "github.com/Azure/azure-sdk-for-go/services/containerregistry/mgmt/2019-05-01/containerregistry" - "github.com/Azure/go-autorest/autorest" "github.com/Azure/go-autorest/autorest/adal" "github.com/Azure/go-autorest/autorest/azure" "github.com/spf13/pflag" @@ -91,39 +90,6 @@ type RegistriesClient interface { List(ctx context.Context) ([]containerregistry.Registry, error) } -// azRegistriesClient implements RegistriesClient. -type azRegistriesClient struct { - client containerregistry.RegistriesClient -} - -func newAzRegistriesClient(subscriptionID, endpoint string, token *adal.ServicePrincipalToken) *azRegistriesClient { - registryClient := containerregistry.NewRegistriesClient(subscriptionID) - registryClient.BaseURI = endpoint - registryClient.Authorizer = autorest.NewBearerAuthorizer(token) - - return &azRegistriesClient{ - client: registryClient, - } -} - -func (az *azRegistriesClient) List(ctx context.Context) ([]containerregistry.Registry, error) { - iterator, err := az.client.ListComplete(ctx) - if err != nil { - return nil, err - } - - result := make([]containerregistry.Registry, 0) - for ; iterator.NotDone(); err = iterator.Next() { - if err != nil { - return nil, err - } - - result = append(result, iterator.Value()) - } - - return result, nil -} - // NewACRProvider parses the specified configFile and returns a DockerConfigProvider func NewACRProvider(configFile *string) credentialprovider.DockerConfigProvider { return &acrProvider{ @@ -136,7 +102,6 @@ type acrProvider struct { file *string config *auth.AzureAuthConfig environment *azure.Environment - registryClient RegistriesClient servicePrincipalToken *adal.ServicePrincipalToken cache cache.Store } @@ -209,11 +174,7 @@ func (a *acrProvider) Enabled() bool { a.servicePrincipalToken, err = auth.GetServicePrincipalToken(a.config, a.environment) if err != nil { klog.Errorf("Failed to create service principal token: %v", err) - return false } - - a.registryClient = newAzRegistriesClient(a.config.SubscriptionID, a.environment.ResourceManagerEndpoint, a.servicePrincipalToken) - return true } @@ -324,11 +285,21 @@ func getLoginServer(registry containerregistry.Registry) string { } func getACRDockerEntryFromARMToken(a *acrProvider, loginServer string) (*credentialprovider.DockerConfigEntry, error) { - // Run EnsureFresh to make sure the token is valid and does not expire - if err := a.servicePrincipalToken.EnsureFresh(); err != nil { - klog.Errorf("Failed to ensure fresh service principal token: %v", err) - return nil, err + if a.servicePrincipalToken == nil { + token, err := auth.GetServicePrincipalToken(a.config, a.environment) + if err != nil { + klog.Errorf("Failed to create service principal token: %v", err) + return nil, err + } + a.servicePrincipalToken = token + } else { + // Run EnsureFresh to make sure the token is valid and does not expire + if err := a.servicePrincipalToken.EnsureFresh(); err != nil { + klog.Errorf("Failed to ensure fresh service principal token: %v", err) + return nil, err + } } + armAccessToken := a.servicePrincipalToken.OAuthToken() klog.V(4).Infof("discovering auth redirects for: %s", loginServer) diff --git a/pkg/credentialprovider/azure/azure_credentials_test.go b/pkg/credentialprovider/azure/azure_credentials_test.go index 80a21b79fa9..a2a25a6cd75 100644 --- a/pkg/credentialprovider/azure/azure_credentials_test.go +++ b/pkg/credentialprovider/azure/azure_credentials_test.go @@ -21,7 +21,6 @@ package azure import ( "bytes" - "context" "testing" "github.com/Azure/azure-sdk-for-go/services/containerregistry/mgmt/2019-05-01/containerregistry" @@ -32,14 +31,6 @@ import ( "github.com/stretchr/testify/assert" ) -type fakeClient struct { - results []containerregistry.Registry -} - -func (f *fakeClient) List(ctx context.Context) ([]containerregistry.Registry, error) { - return f.results, nil -} - func Test(t *testing.T) { configStr := ` { @@ -72,13 +63,9 @@ func Test(t *testing.T) { }, }, } - fakeClient := &fakeClient{ - results: result, - } provider := &acrProvider{ - registryClient: fakeClient, - cache: cache.NewExpirationStore(stringKeyFunc, &acrExpirationPolicy{}), + cache: cache.NewExpirationStore(stringKeyFunc, &acrExpirationPolicy{}), } provider.loadConfig(bytes.NewBufferString(configStr)) @@ -133,8 +120,7 @@ func TestProvide(t *testing.T) { for i, test := range testCases { provider := &acrProvider{ - registryClient: &fakeClient{}, - cache: cache.NewExpirationStore(stringKeyFunc, &acrExpirationPolicy{}), + cache: cache.NewExpirationStore(stringKeyFunc, &acrExpirationPolicy{}), } provider.loadConfig(bytes.NewBufferString(test.configStr)) @@ -149,21 +135,8 @@ func TestParseACRLoginServerFromImage(t *testing.T) { "aadClientId": "foo", "aadClientSecret": "bar" }` - result := []containerregistry.Registry{ - { - Name: to.StringPtr("foo"), - RegistryProperties: &containerregistry.RegistryProperties{ - LoginServer: to.StringPtr("*.azurecr.io"), - }, - }, - } - fakeClient := &fakeClient{ - results: result, - } - provider := &acrProvider{ - registryClient: fakeClient, - } + provider := &acrProvider{} provider.loadConfig(bytes.NewBufferString(configStr)) provider.environment = &azure.Environment{ ContainerRegistryDNSSuffix: ".azurecr.my.cloud",