From c90718d1acbdc9ba66dfe0c88158245e3f44cff9 Mon Sep 17 00:00:00 2001 From: James Nugent Date: Thu, 21 Apr 2016 16:50:47 -0700 Subject: [PATCH] provider/azurerm: Error on bad creds and speed++ (#6290) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit uses Riviera to register the Microsoft.Compute provider as a canary for whether or not the Azure account credentials are set up. It used to use the MS client, but that appeared to panic internally if the credentials were bad. It's possible that we were using it wrong, but there are no docs so ¯\_(ツ)_/¯. As part of this, we parellelise the registration of the other providers. This shaves the latency of each provider request times the number of providers minus 1 off the "startup" time of the AzureRM provider. The result is quite noticeable. --- builtin/providers/azurerm/config.go | 18 +++++--- builtin/providers/azurerm/provider.go | 64 +++++++++++++++++++-------- 2 files changed, 59 insertions(+), 23 deletions(-) diff --git a/builtin/providers/azurerm/config.go b/builtin/providers/azurerm/config.go index 73f0f612064b..eb6b0d4dc0c5 100644 --- a/builtin/providers/azurerm/config.go +++ b/builtin/providers/azurerm/config.go @@ -107,11 +107,6 @@ func setUserAgent(client *autorest.Client) { // getArmClient is a helper method which returns a fully instantiated // *ArmClient based on the Config's current settings. func (c *Config) getArmClient() (*ArmClient, error) { - spt, err := azure.NewServicePrincipalToken(c.ClientID, c.ClientSecret, c.TenantID, azure.AzureResourceManagerScope) - if err != nil { - return nil, err - } - // client declarations: client := ArmClient{} @@ -125,8 +120,21 @@ func (c *Config) getArmClient() (*ArmClient, error) { return nil, fmt.Errorf("Error creating Riviera client: %s", err) } + // validate that the credentials are correct using Riviera. Note that this must be + // done _before_ using the Microsoft SDK, because Riviera handles errors. Using a + // namespace registration instead of a simple OAuth token refresh guarantees that + // service delegation is correct. This has the effect of registering Microsoft.Compute + // which is neccessary anyway. + if err := registerProviderWithSubscription("Microsoft.Compute", rivieraClient); err != nil { + return nil, err + } client.rivieraClient = rivieraClient + spt, err := azure.NewServicePrincipalToken(c.ClientID, c.ClientSecret, c.TenantID, azure.AzureResourceManagerScope) + if err != nil { + return nil, err + } + // NOTE: these declarations should be left separate for clarity should the // clients be wished to be configured with custom Responders/PollingModess etc... asc := compute.NewAvailabilitySetsClient(c.SubscriptionID) diff --git a/builtin/providers/azurerm/provider.go b/builtin/providers/azurerm/provider.go index 179c2a2d24a2..f0852ee8af86 100644 --- a/builtin/providers/azurerm/provider.go +++ b/builtin/providers/azurerm/provider.go @@ -14,6 +14,7 @@ import ( "github.com/hashicorp/terraform/helper/schema" "github.com/hashicorp/terraform/terraform" riviera "github.com/jen20/riviera/azure" + "sync" ) // Provider returns a terraform.ResourceProvider. @@ -91,9 +92,11 @@ type Config struct { ClientID string ClientSecret string TenantID string + + validateCredentialsOnce sync.Once } -func (c Config) validate() error { +func (c *Config) validate() error { var err *multierror.Error if c.SubscriptionID == "" { @@ -113,7 +116,7 @@ func (c Config) validate() error { } func providerConfigure(d *schema.ResourceData) (interface{}, error) { - config := Config{ + config := &Config{ SubscriptionID: d.Get("subscription_id").(string), ClientID: d.Get("client_id").(string), ClientSecret: d.Get("client_secret").(string), @@ -129,7 +132,7 @@ func providerConfigure(d *schema.ResourceData) (interface{}, error) { return nil, err } - err = registerAzureResourceProvidersWithSubscription(&config, client) + err = registerAzureResourceProvidersWithSubscription(client.rivieraClient) if err != nil { return nil, err } @@ -137,27 +140,52 @@ func providerConfigure(d *schema.ResourceData) (interface{}, error) { return client, nil } +func registerProviderWithSubscription(providerName string, client *riviera.Client) error { + request := client.NewRequest() + request.Command = riviera.RegisterResourceProvider{ + Namespace: providerName, + } + + response, err := request.Execute() + if err != nil { + return fmt.Errorf("Cannot request provider registration for Azure Resource Manager: %s.", err) + } + + if !response.IsSuccessful() { + return fmt.Errorf("Credentials for acessing the Azure Resource Manager API are likely " + + "to be incorrect, or\n the service principal does not have permission to use " + + "the Azure Service Management\n API.") + } + + return nil +} + +var providerRegistrationOnce sync.Once + // registerAzureResourceProvidersWithSubscription uses the providers client to register // all Azure resource providers which the Terraform provider may require (regardless of // whether they are actually used by the configuration or not). It was confirmed by Microsoft // that this is the approach their own internal tools also take. -func registerAzureResourceProvidersWithSubscription(config *Config, client *ArmClient) error { - providerClient := client.providers - - providers := []string{"Microsoft.Network", "Microsoft.Compute", "Microsoft.Cdn", "Microsoft.Storage", "Microsoft.Sql", "Microsoft.Search", "Microsoft.Resources"} - - for _, v := range providers { - res, err := providerClient.Register(v) - if err != nil { - return err +func registerAzureResourceProvidersWithSubscription(client *riviera.Client) error { + var err error + providerRegistrationOnce.Do(func() { + // We register Microsoft.Compute during client initialization + providers := []string{"Microsoft.Network", "Microsoft.Cdn", "Microsoft.Storage", "Microsoft.Sql", "Microsoft.Search", "Microsoft.Resources"} + + var wg sync.WaitGroup + wg.Add(len(providers)) + for _, providerName := range providers { + go func(p string) { + defer wg.Done() + if innerErr := registerProviderWithSubscription(p, client); err != nil { + err = innerErr + } + }(providerName) } + wg.Wait() + }) - if res.StatusCode != http.StatusOK { - return fmt.Errorf("Error registering provider %q with subscription %q", v, config.SubscriptionID) - } - } - - return nil + return err } // azureRMNormalizeLocation is a function which normalises human-readable region/location