From e4c70a37c2a4315228e077715a5ed7342e0f9146 Mon Sep 17 00:00:00 2001 From: Tom Bamford Date: Tue, 26 Sep 2023 00:06:32 +0100 Subject: [PATCH 1/3] provider: work around RP unavailability in some non-public clouds when validating RP registrations --- internal/provider/provider.go | 2 +- internal/resourceproviders/registration.go | 5 +++-- .../resourceproviders/requiring_registration.go | 14 +++++++++++--- 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/internal/provider/provider.go b/internal/provider/provider.go index 50fd34f0f4b5..adae8385a4c6 100644 --- a/internal/provider/provider.go +++ b/internal/provider/provider.go @@ -489,7 +489,7 @@ func buildClient(ctx context.Context, p *schema.Provider, d *schema.ResourceData ctx2, cancel := context.WithTimeout(ctx, 30*time.Minute) defer cancel() - if err := resourceproviders.EnsureRegistered(ctx2, client.Resource.ResourceProvidersClient, subscriptionId, requiredResourceProviders); err != nil { + if err := resourceproviders.EnsureRegistered(ctx2, client.Resource.ResourceProvidersClient, subscriptionId, requiredResourceProviders, authConfig.Environment); err != nil { return nil, diag.Errorf(resourceProviderRegistrationErrorFmt, err) } } diff --git a/internal/resourceproviders/registration.go b/internal/resourceproviders/registration.go index de0fdb4f35e9..2cca657c1977 100644 --- a/internal/resourceproviders/registration.go +++ b/internal/resourceproviders/registration.go @@ -14,10 +14,11 @@ import ( "github.com/hashicorp/go-azure-helpers/resourcemanager/commonids" "github.com/hashicorp/go-azure-sdk/resource-manager/resources/2022-09-01/providers" "github.com/hashicorp/go-azure-sdk/sdk/client/pollers" + "github.com/hashicorp/go-azure-sdk/sdk/environments" "github.com/hashicorp/terraform-provider-azurerm/internal/resourceproviders/custompollers" ) -func EnsureRegistered(ctx context.Context, client *providers.ProvidersClient, subscriptionId commonids.SubscriptionId, requiredRPs map[string]struct{}) error { +func EnsureRegistered(ctx context.Context, client *providers.ProvidersClient, subscriptionId commonids.SubscriptionId, requiredRPs map[string]struct{}, environment environments.Environment) error { if cachedResourceProviders == nil || registeredResourceProviders == nil || unregisteredResourceProviders == nil { if err := populateCache(ctx, client, subscriptionId); err != nil { return fmt.Errorf("populating Resource Provider cache: %+v", err) @@ -25,7 +26,7 @@ func EnsureRegistered(ctx context.Context, client *providers.ProvidersClient, su } log.Printf("[DEBUG] Determining which Resource Providers require Registration") - providersToRegister, err := DetermineWhichRequiredResourceProvidersRequireRegistration(requiredRPs) + providersToRegister, err := DetermineWhichRequiredResourceProvidersRequireRegistration(requiredRPs, environment) if err != nil { return fmt.Errorf("determining which Required Resource Providers require registration: %+v", err) } diff --git a/internal/resourceproviders/requiring_registration.go b/internal/resourceproviders/requiring_registration.go index 6fca6a0b497c..9be4bffa5720 100644 --- a/internal/resourceproviders/requiring_registration.go +++ b/internal/resourceproviders/requiring_registration.go @@ -5,10 +5,13 @@ package resourceproviders import ( "fmt" + "log" + + "github.com/hashicorp/go-azure-sdk/sdk/environments" ) // DetermineWhichRequiredResourceProvidersRequireRegistration determines which Resource Providers require registration to be able to be used -func DetermineWhichRequiredResourceProvidersRequireRegistration(requiredResourceProviders map[string]struct{}) (*[]string, error) { +func DetermineWhichRequiredResourceProvidersRequireRegistration(requiredResourceProviders map[string]struct{}, environment environments.Environment) (*[]string, error) { if registeredResourceProviders == nil || unregisteredResourceProviders == nil { return nil, fmt.Errorf("internal-error: the registered/unregistered Resource Provider cache isn't populated") } @@ -20,8 +23,13 @@ func DetermineWhichRequiredResourceProvidersRequireRegistration(requiredResource } if _, isUnregistered := (*unregisteredResourceProviders)[providerName]; !isUnregistered { - // this is likely a typo in the Required Resource Providers list, so we should surface this - return nil, fmt.Errorf("the required Resource Provider %q wasn't returned from the Azure API", providerName) + if environment.Name == environments.AzurePublicCloud { + // this is likely a typo in the Required Resource Providers list, so we should surface this + return nil, fmt.Errorf("the required Resource Provider %q wasn't returned from the Azure API", providerName) + } + + // some RPs may not exist in some non-public clouds, so we'll log a warning here instead of raising an error + log.Printf("[WARN] The required Resource Provider %q wasn't returned from the Azure API", providerName) } requiringRegistration = append(requiringRegistration, providerName) From 63a3e43c42337427e7e7f00836e064bc1cec4296 Mon Sep 17 00:00:00 2001 From: Matthew Date: Thu, 2 Nov 2023 10:32:43 -0700 Subject: [PATCH 2/3] Address review --- internal/acceptance/required_test.go | 2 +- internal/resourceproviders/registration.go | 2 +- internal/resourceproviders/requiring_registration.go | 9 +-------- 3 files changed, 3 insertions(+), 10 deletions(-) diff --git a/internal/acceptance/required_test.go b/internal/acceptance/required_test.go index 25af6564c83d..6321e4119c57 100644 --- a/internal/acceptance/required_test.go +++ b/internal/acceptance/required_test.go @@ -44,7 +44,7 @@ func TestAccEnsureRequiredResourceProvidersAreRegistered(t *testing.T) { requiredResourceProviders := resourceproviders.Required() subscriptionId := commonids.NewSubscriptionID(armClient.Account.SubscriptionId) - if err = resourceproviders.EnsureRegistered(ctx, client, subscriptionId, requiredResourceProviders); err != nil { + if err = resourceproviders.EnsureRegistered(ctx, client, subscriptionId, requiredResourceProviders, config.Environment); err != nil { t.Fatalf("Error registering Resource Providers: %+v", err) } diff --git a/internal/resourceproviders/registration.go b/internal/resourceproviders/registration.go index 2cca657c1977..ba4311e2a96a 100644 --- a/internal/resourceproviders/registration.go +++ b/internal/resourceproviders/registration.go @@ -26,7 +26,7 @@ func EnsureRegistered(ctx context.Context, client *providers.ProvidersClient, su } log.Printf("[DEBUG] Determining which Resource Providers require Registration") - providersToRegister, err := DetermineWhichRequiredResourceProvidersRequireRegistration(requiredRPs, environment) + providersToRegister, err := DetermineWhichRequiredResourceProvidersRequireRegistration(requiredRPs) if err != nil { return fmt.Errorf("determining which Required Resource Providers require registration: %+v", err) } diff --git a/internal/resourceproviders/requiring_registration.go b/internal/resourceproviders/requiring_registration.go index 9be4bffa5720..2a082ceb0654 100644 --- a/internal/resourceproviders/requiring_registration.go +++ b/internal/resourceproviders/requiring_registration.go @@ -6,12 +6,10 @@ package resourceproviders import ( "fmt" "log" - - "github.com/hashicorp/go-azure-sdk/sdk/environments" ) // DetermineWhichRequiredResourceProvidersRequireRegistration determines which Resource Providers require registration to be able to be used -func DetermineWhichRequiredResourceProvidersRequireRegistration(requiredResourceProviders map[string]struct{}, environment environments.Environment) (*[]string, error) { +func DetermineWhichRequiredResourceProvidersRequireRegistration(requiredResourceProviders map[string]struct{}) (*[]string, error) { if registeredResourceProviders == nil || unregisteredResourceProviders == nil { return nil, fmt.Errorf("internal-error: the registered/unregistered Resource Provider cache isn't populated") } @@ -23,11 +21,6 @@ func DetermineWhichRequiredResourceProvidersRequireRegistration(requiredResource } if _, isUnregistered := (*unregisteredResourceProviders)[providerName]; !isUnregistered { - if environment.Name == environments.AzurePublicCloud { - // this is likely a typo in the Required Resource Providers list, so we should surface this - return nil, fmt.Errorf("the required Resource Provider %q wasn't returned from the Azure API", providerName) - } - // some RPs may not exist in some non-public clouds, so we'll log a warning here instead of raising an error log.Printf("[WARN] The required Resource Provider %q wasn't returned from the Azure API", providerName) } From 3ae1e3a3e0896577d942cd2e44f60386eb77c26d Mon Sep 17 00:00:00 2001 From: Matthew Date: Thu, 2 Nov 2023 10:37:33 -0700 Subject: [PATCH 3/3] Remove unneeded environment atrribute from method call --- internal/acceptance/required_test.go | 2 +- internal/provider/provider.go | 2 +- internal/resourceproviders/registration.go | 3 +-- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/internal/acceptance/required_test.go b/internal/acceptance/required_test.go index 6321e4119c57..25af6564c83d 100644 --- a/internal/acceptance/required_test.go +++ b/internal/acceptance/required_test.go @@ -44,7 +44,7 @@ func TestAccEnsureRequiredResourceProvidersAreRegistered(t *testing.T) { requiredResourceProviders := resourceproviders.Required() subscriptionId := commonids.NewSubscriptionID(armClient.Account.SubscriptionId) - if err = resourceproviders.EnsureRegistered(ctx, client, subscriptionId, requiredResourceProviders, config.Environment); err != nil { + if err = resourceproviders.EnsureRegistered(ctx, client, subscriptionId, requiredResourceProviders); err != nil { t.Fatalf("Error registering Resource Providers: %+v", err) } diff --git a/internal/provider/provider.go b/internal/provider/provider.go index adae8385a4c6..50fd34f0f4b5 100644 --- a/internal/provider/provider.go +++ b/internal/provider/provider.go @@ -489,7 +489,7 @@ func buildClient(ctx context.Context, p *schema.Provider, d *schema.ResourceData ctx2, cancel := context.WithTimeout(ctx, 30*time.Minute) defer cancel() - if err := resourceproviders.EnsureRegistered(ctx2, client.Resource.ResourceProvidersClient, subscriptionId, requiredResourceProviders, authConfig.Environment); err != nil { + if err := resourceproviders.EnsureRegistered(ctx2, client.Resource.ResourceProvidersClient, subscriptionId, requiredResourceProviders); err != nil { return nil, diag.Errorf(resourceProviderRegistrationErrorFmt, err) } } diff --git a/internal/resourceproviders/registration.go b/internal/resourceproviders/registration.go index ba4311e2a96a..de0fdb4f35e9 100644 --- a/internal/resourceproviders/registration.go +++ b/internal/resourceproviders/registration.go @@ -14,11 +14,10 @@ import ( "github.com/hashicorp/go-azure-helpers/resourcemanager/commonids" "github.com/hashicorp/go-azure-sdk/resource-manager/resources/2022-09-01/providers" "github.com/hashicorp/go-azure-sdk/sdk/client/pollers" - "github.com/hashicorp/go-azure-sdk/sdk/environments" "github.com/hashicorp/terraform-provider-azurerm/internal/resourceproviders/custompollers" ) -func EnsureRegistered(ctx context.Context, client *providers.ProvidersClient, subscriptionId commonids.SubscriptionId, requiredRPs map[string]struct{}, environment environments.Environment) error { +func EnsureRegistered(ctx context.Context, client *providers.ProvidersClient, subscriptionId commonids.SubscriptionId, requiredRPs map[string]struct{}) error { if cachedResourceProviders == nil || registeredResourceProviders == nil || unregisteredResourceProviders == nil { if err := populateCache(ctx, client, subscriptionId); err != nil { return fmt.Errorf("populating Resource Provider cache: %+v", err)