From db4bce36d49b3dfc2377c62d11194e99cd28511a Mon Sep 17 00:00:00 2001 From: Tom Bamford Date: Tue, 30 Nov 2021 18:26:25 +0000 Subject: [PATCH] Support for v2 auth tokens (i.e. MSAL) - This is opt-in behaviour via the provider property `use_msal`, and environment variables `ARM_USE_MSAL` / `ARM_USE_MSGRAPH` (the latter for compatibility with the related backend property - When `use_msal` is true, do not make any API calls to a graph API (legacy or current). There are only 2 uses of this at present: - `data.azurerm_client_config`, which doesn't actually do anything with the result so this appears to be a vestige anyway - `azurerm_hdinsight_kafka_cluster`, the API for which requires both an AAD group ID and name to be specified (?) so currently this resource looks up the group name from the supplied ID. In future we'll require that both are specified (e.g. using `data.azuread_group` for any necessary lookup) - In v3.0, we'll remove support for graph clients in order to delegate any required usage to the AzureAD provider. - Also removes support for Azure Germany, which is now offline --- .gitignore | 1 + internal/clients/auth.go | 6 + internal/clients/builder.go | 141 ++++++++++++------ internal/common/client_options.go | 10 +- internal/provider/provider.go | 23 ++- .../services/authorization/client/client.go | 19 +-- .../client_config_data_source.go | 19 +-- internal/services/hdinsight/client/client.go | 11 ++ .../hdinsight_kafka_cluster_resource.go | 108 ++++++++++++-- .../hdinsight_kafka_cluster_resource_test.go | 77 ++++++++++ 10 files changed, 313 insertions(+), 102 deletions(-) diff --git a/.gitignore b/.gitignore index 2a7137d0435b..78dcda932f31 100644 --- a/.gitignore +++ b/.gitignore @@ -24,6 +24,7 @@ website/node_modules .idea *.iml *.test +.python-version .terraform.tfstate.lock.info .terraform.lock.hcl diff --git a/internal/clients/auth.go b/internal/clients/auth.go index d6315b009d28..13b8c20bf723 100644 --- a/internal/clients/auth.go +++ b/internal/clients/auth.go @@ -16,6 +16,9 @@ type ResourceManagerAccount struct { SkipResourceProviderRegistration bool SubscriptionId string TenantId string + + // TODO: remove in v3.0 + UseMSAL bool } func NewResourceManagerAccount(ctx context.Context, config authentication.Config, env azure.Environment, skipResourceProviderRegistration bool) (*ResourceManagerAccount, error) { @@ -38,6 +41,9 @@ func NewResourceManagerAccount(ctx context.Context, config authentication.Config TenantId: config.TenantID, SkipResourceProviderRegistration: skipResourceProviderRegistration, SubscriptionId: config.SubscriptionID, + + // TODO: remove in v3.0 + UseMSAL: config.UseMicrosoftGraph, } return &account, nil } diff --git a/internal/clients/builder.go b/internal/clients/builder.go index 09289374342c..a09f264abe0e 100644 --- a/internal/clients/builder.go +++ b/internal/clients/builder.go @@ -11,6 +11,8 @@ import ( "github.com/hashicorp/go-azure-helpers/authentication" "github.com/hashicorp/go-azure-helpers/resourcemanager/location" "github.com/hashicorp/go-azure-helpers/sender" + "github.com/manicminer/hamilton/environments" + "github.com/hashicorp/terraform-provider-azurerm/internal/common" "github.com/hashicorp/terraform-provider-azurerm/internal/features" "github.com/hashicorp/terraform-provider-azurerm/internal/resourceproviders" @@ -26,15 +28,16 @@ type ClientBuilder struct { StorageUseAzureAD bool TerraformVersion string Features features.UserFeatures + UseMSAL bool } const azureStackEnvironmentError = ` -The AzureRM Provider supports the different Azure Public Clouds - including China, Germany, -Public and US Government - however it does not support Azure Stack due to differences in -API and feature availability. +The AzureRM Provider supports the different Azure Public Clouds - including China, Public, +and US Government - however it does not support Azure Stack due to differences in API and +feature availability. Terraform instead offers a separate "azurestack" provider which supports the functionality -and API's available in Azure Stack via Azure Stack Profiles. +and APIs available in Azure Stack via Azure Stack Profiles. ` func Build(ctx context.Context, builder ClientBuilder) (*Client, error) { @@ -51,11 +54,18 @@ func Build(ctx context.Context, builder ClientBuilder) (*Client, error) { return nil, fmt.Errorf(azureStackEnvironmentError) } + // Autorest environment configuration env, err := authentication.AzureEnvironmentByNameFromEndpoint(ctx, builder.AuthConfig.MetadataHost, builder.AuthConfig.Environment) if err != nil { return nil, fmt.Errorf("unable to find environment %q from endpoint %q: %+v", builder.AuthConfig.Environment, builder.AuthConfig.MetadataHost, err) } + // Hamilton environment configuration + environment, err := environments.EnvironmentFromString(builder.AuthConfig.Environment) + if err != nil { + return nil, fmt.Errorf("unable to find environment %q from endpoint %q: %+v", builder.AuthConfig.Environment, builder.AuthConfig.MetadataHost, err) + } + // client declarations: account, err := NewResourceManagerAccount(ctx, *builder.AuthConfig, *env, builder.SkipProviderRegistration) if err != nil { @@ -78,44 +88,89 @@ func Build(ctx context.Context, builder ClientBuilder) (*Client, error) { sender := sender.BuildSender("AzureRM") - // Resource Manager endpoints - endpoint := env.ResourceManagerEndpoint - auth, err := builder.AuthConfig.GetADALToken(ctx, sender, oauthConfig, env.TokenAudience) - if err != nil { - return nil, fmt.Errorf("unable to get authorization token for resource manager: %+v", err) - } + // Authorizers, via autorest or hamilton/auth + var auth, storageAuth, synapseAuth, batchManagementAuth autorest.Authorizer + var keyVaultAuth *autorest.BearerAuthorizerCallback + var tokenFunc common.EndpointTokenFunc + var graphAuth autorest.Authorizer // TODO: remove in v3.0 - // Graph Endpoints - graphEndpoint := env.GraphEndpoint - graphAuth, err := builder.AuthConfig.GetADALToken(ctx, sender, oauthConfig, graphEndpoint) - if err != nil { - return nil, fmt.Errorf("unable to get authorization token for graph endpoints: %+v", err) - } + if builder.UseMSAL { + // TODO: remove UseMSAL toggle and make this the default behaviour in v3.0 + auth, err = builder.AuthConfig.GetMSALToken(ctx, environment.ResourceManager, sender, oauthConfig, string(environment.ResourceManager.Endpoint)) + if err != nil { + return nil, fmt.Errorf("unable to get MSAL authorization token for resource manager API: %+v", err) + } - // Storage Endpoints - storageAuth, err := builder.AuthConfig.GetADALToken(ctx, sender, oauthConfig, env.ResourceIdentifiers.Storage) - if err != nil { - return nil, fmt.Errorf("unable to get authorization token for storage endpoints: %+v", err) - } + storageAuth, err = builder.AuthConfig.GetMSALToken(ctx, environment.Storage, sender, oauthConfig, string(environment.Storage.Endpoint)) + if err != nil { + return nil, fmt.Errorf("unable to get MSAL authorization token for storage API: %+v", err) + } - // Synapse Endpoints - var synapseAuth autorest.Authorizer = nil - if env.ResourceIdentifiers.Synapse != azure.NotAvailable { - synapseAuth, err = builder.AuthConfig.GetADALToken(ctx, sender, oauthConfig, env.ResourceIdentifiers.Synapse) + if environment.Synapse.IsAvailable() { + synapseAuth, err = builder.AuthConfig.GetMSALToken(ctx, environment.Synapse, sender, oauthConfig, string(environment.Synapse.Endpoint)) + if err != nil { + return nil, fmt.Errorf("unable to get MSAL authorization token for synapse API: %+v", err) + } + } else { + log.Printf("[DEBUG] Skipping building the Synapse MSAL Authorizer since this is not supported in the current Azure Environment") + } + + batchManagementAuth, err = builder.AuthConfig.GetMSALToken(ctx, environment.BatchManagement, sender, oauthConfig, string(environment.BatchManagement.Endpoint)) if err != nil { - return nil, fmt.Errorf("unable to get authorization token for synapse endpoints: %+v", err) + return nil, fmt.Errorf("unable to get MSAL authorization token for batch management API: %+v", err) + } + + keyVaultAuth = builder.AuthConfig.MSALBearerAuthorizerCallback(ctx, environment.KeyVault, sender, oauthConfig, string(environment.KeyVault.Endpoint)) + + // Helper for obtaining endpoint-specific tokens + tokenFunc = func(endpoint string) (autorest.Authorizer, error) { + api := environments.Api{Endpoint: environments.ApiEndpoint(endpoint)} + authorizer, err := builder.AuthConfig.GetMSALToken(ctx, api, sender, oauthConfig, endpoint) + if err != nil { + return nil, fmt.Errorf("getting MSAL authorization token for endpoint %s: %+v", endpoint, err) + } + return authorizer, nil } } else { - log.Printf("[DEBUG] Skipping building the Synapse Authorizer since this is not supported in the current Azure Environment") - } + auth, err = builder.AuthConfig.GetADALToken(ctx, sender, oauthConfig, env.TokenAudience) + if err != nil { + return nil, fmt.Errorf("unable to get ADAL authorization token for resource manager API: %+v", err) + } - // Key Vault Endpoints - keyVaultAuth := builder.AuthConfig.ADALBearerAuthorizerCallback(ctx, sender, oauthConfig) + storageAuth, err = builder.AuthConfig.GetADALToken(ctx, sender, oauthConfig, env.ResourceIdentifiers.Storage) + if err != nil { + return nil, fmt.Errorf("unable to get ADAL authorization token for storage API: %+v", err) + } - // Batch Management Endpoints - batchManagementAuth, err := builder.AuthConfig.GetADALToken(ctx, sender, oauthConfig, env.BatchManagementEndpoint) - if err != nil { - return nil, fmt.Errorf("unable to get authorization token for batch management endpoint: %+v", err) + if env.ResourceIdentifiers.Synapse != azure.NotAvailable { + synapseAuth, err = builder.AuthConfig.GetADALToken(ctx, sender, oauthConfig, env.ResourceIdentifiers.Synapse) + if err != nil { + return nil, fmt.Errorf("unable to get ADAL authorization token for synapse API: %+v", err) + } + } else { + log.Printf("[DEBUG] Skipping building the Synapse ADAL Authorizer since this is not supported in the current Azure Environment") + } + + batchManagementAuth, err = builder.AuthConfig.GetADALToken(ctx, sender, oauthConfig, env.BatchManagementEndpoint) + if err != nil { + return nil, fmt.Errorf("unable to get ADAL authorization token for batch management API: %+v", err) + } + + keyVaultAuth = builder.AuthConfig.ADALBearerAuthorizerCallback(ctx, sender, oauthConfig) + + // Helper for obtaining endpoint-specific tokens + tokenFunc = func(endpoint string) (autorest.Authorizer, error) { + authorizer, err := builder.AuthConfig.GetADALToken(ctx, sender, oauthConfig, endpoint) + if err != nil { + return nil, fmt.Errorf("getting ADAL authorization token for endpoint %s: %+v", endpoint, err) + } + return authorizer, nil + } + + graphAuth, err = builder.AuthConfig.GetADALToken(ctx, sender, oauthConfig, env.GraphEndpoint) + if err != nil { + return nil, fmt.Errorf("unable to get ADAL authorization token for aadgraph API: %+v", err) + } } o := &common.ClientOptions{ @@ -123,11 +178,9 @@ func Build(ctx context.Context, builder ClientBuilder) (*Client, error) { TenantID: builder.AuthConfig.TenantID, PartnerId: builder.PartnerId, TerraformVersion: builder.TerraformVersion, - GraphAuthorizer: graphAuth, - GraphEndpoint: graphEndpoint, KeyVaultAuthorizer: keyVaultAuth, ResourceManagerAuthorizer: auth, - ResourceManagerEndpoint: endpoint, + ResourceManagerEndpoint: env.ResourceManagerEndpoint, StorageAuthorizer: storageAuth, SynapseAuthorizer: synapseAuth, BatchManagementAuthorizer: batchManagementAuth, @@ -138,13 +191,13 @@ func Build(ctx context.Context, builder ClientBuilder) (*Client, error) { Environment: *env, Features: builder.Features, StorageUseAzureAD: builder.StorageUseAzureAD, - TokenFunc: func(endpoint string) (autorest.Authorizer, error) { - authorizer, err := builder.AuthConfig.GetADALToken(ctx, sender, oauthConfig, endpoint) - if err != nil { - return nil, fmt.Errorf("getting authorization token for endpoint %s: %+v", endpoint, err) - } - return authorizer, nil - }, + TokenFunc: tokenFunc, + } + + // TODO: remove in v3.0 + if !builder.UseMSAL { + o.GraphEndpoint = env.GraphEndpoint + o.GraphAuthorizer = graphAuth } if err := client.Build(ctx, o); err != nil { diff --git a/internal/common/client_options.go b/internal/common/client_options.go index 76620c7345e9..e43622bc7bbc 100644 --- a/internal/common/client_options.go +++ b/internal/common/client_options.go @@ -13,14 +13,14 @@ import ( "github.com/hashicorp/terraform-provider-azurerm/version" ) +type EndpointTokenFunc func(endpoint string) (autorest.Authorizer, error) + type ClientOptions struct { SubscriptionId string TenantID string PartnerId string TerraformVersion string - GraphAuthorizer autorest.Authorizer - GraphEndpoint string KeyVaultAuthorizer autorest.Authorizer ResourceManagerAuthorizer autorest.Authorizer ResourceManagerEndpoint string @@ -37,7 +37,11 @@ type ClientOptions struct { StorageUseAzureAD bool // Some Dataplane APIs require a token scoped for a specific endpoint - TokenFunc func(endpoint string) (autorest.Authorizer, error) + TokenFunc EndpointTokenFunc + + // TODO: remove graph configuration in v3.0 + GraphAuthorizer autorest.Authorizer + GraphEndpoint string } func (o ClientOptions) ConfigureClient(c *autorest.Client, authorizer autorest.Authorizer) { diff --git a/internal/provider/provider.go b/internal/provider/provider.go index 669f8aa9b535..716a48c7e506 100644 --- a/internal/provider/provider.go +++ b/internal/provider/provider.go @@ -134,7 +134,7 @@ func azureProvider(supportLegacyTestSuite bool) *schema.Provider { Type: schema.TypeString, Required: true, DefaultFunc: schema.EnvDefaultFunc("ARM_ENVIRONMENT", "public"), - Description: "The Cloud Environment which should be used. Possible values are public, usgovernment, german, and china. Defaults to public.", + Description: "The Cloud Environment which should be used. Possible values are public, usgovernment, and china. Defaults to public.", }, "metadata_host": { @@ -228,6 +228,13 @@ func azureProvider(supportLegacyTestSuite bool) *schema.Provider { DefaultFunc: schema.EnvDefaultFunc("ARM_STORAGE_USE_AZUREAD", false), Description: "Should the AzureRM Provider use AzureAD to access the Storage Data Plane API's?", }, + + "use_msal": { + Type: schema.TypeBool, + Optional: true, + Description: "Should Terraform obtain MSAL auth tokens and no longer use Azure Active Directory Graph?", + DefaultFunc: schema.MultiEnvDefaultFunc([]string{"ARM_USE_MSAL", "ARM_USE_MSGRAPH"}, false), + }, }, DataSourcesMap: dataSources, @@ -258,7 +265,7 @@ func providerConfigure(p *schema.Provider) schema.ConfigureContextFunc { } if len(auxTenants) > 3 { - return nil, diag.FromErr(fmt.Errorf("The provider only supports 3 auxiliary tenant IDs")) + return nil, diag.Errorf("The provider only supports 3 auxiliary tenant IDs") } metadataHost := d.Get("metadata_host").(string) @@ -291,11 +298,14 @@ func providerConfigure(p *schema.Provider) schema.ConfigureContextFunc { // Doc Links ClientSecretDocsLink: "https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/guides/service_principal_client_secret", + + // MSAL opt-in + UseMicrosoftGraph: d.Get("use_msal").(bool), } config, err := builder.Build() if err != nil { - return nil, diag.FromErr(fmt.Errorf("building AzureRM Client: %s", err)) + return nil, diag.Errorf("building AzureRM Client: %s", err) } terraformVersion := p.TerraformVersion @@ -315,6 +325,7 @@ func providerConfigure(p *schema.Provider) schema.ConfigureContextFunc { DisableTerraformPartnerID: d.Get("disable_terraform_partner_id").(bool), Features: expandFeatures(d.Get("features").([]interface{})), StorageUseAzureAD: d.Get("storage_use_azuread").(bool), + UseMSAL: d.Get("use_msal").(bool), // this field is intentionally not exposed in the provider block, since it's only used for // platform level tracing @@ -339,16 +350,16 @@ func providerConfigure(p *schema.Provider) schema.ConfigureContextFunc { // requests. This also lets us check if the provider credentials are correct. providerList, err := client.Resource.ProvidersClient.List(ctx, nil, "") if err != nil { - return nil, diag.FromErr(fmt.Errorf("Unable to list provider registration status, it is possible that this is due to invalid "+ + return nil, diag.Errorf("Unable to list provider registration status, it is possible that this is due to invalid "+ "credentials or the service principal does not have permission to use the Resource Manager API, Azure "+ - "error: %s", err)) + "error: %s", err) } availableResourceProviders := providerList.Values() requiredResourceProviders := resourceproviders.Required() if err := resourceproviders.EnsureRegistered(ctx, *client.Resource.ProvidersClient, availableResourceProviders, requiredResourceProviders); err != nil { - return nil, diag.FromErr(fmt.Errorf(resourceProviderRegistrationErrorFmt, err)) + return nil, diag.Errorf(resourceProviderRegistrationErrorFmt, err) } } diff --git a/internal/services/authorization/client/client.go b/internal/services/authorization/client/client.go index ca8eb55b70fe..d79a77bf94a4 100644 --- a/internal/services/authorization/client/client.go +++ b/internal/services/authorization/client/client.go @@ -1,35 +1,24 @@ package client import ( - "github.com/Azure/azure-sdk-for-go/services/graphrbac/1.6/graphrbac" "github.com/Azure/azure-sdk-for-go/services/preview/authorization/mgmt/2020-04-01-preview/authorization" "github.com/hashicorp/terraform-provider-azurerm/internal/common" ) type Client struct { - GroupsClient *graphrbac.GroupsClient - RoleAssignmentsClient *authorization.RoleAssignmentsClient - RoleDefinitionsClient *authorization.RoleDefinitionsClient - ServicePrincipalsClient *graphrbac.ServicePrincipalsClient + RoleAssignmentsClient *authorization.RoleAssignmentsClient + RoleDefinitionsClient *authorization.RoleDefinitionsClient } func NewClient(o *common.ClientOptions) *Client { - groupsClient := graphrbac.NewGroupsClientWithBaseURI(o.GraphEndpoint, o.TenantID) - o.ConfigureClient(&groupsClient.Client, o.GraphAuthorizer) - roleAssignmentsClient := authorization.NewRoleAssignmentsClientWithBaseURI(o.ResourceManagerEndpoint, o.SubscriptionId) o.ConfigureClient(&roleAssignmentsClient.Client, o.ResourceManagerAuthorizer) roleDefinitionsClient := authorization.NewRoleDefinitionsClientWithBaseURI(o.ResourceManagerEndpoint, o.SubscriptionId) o.ConfigureClient(&roleDefinitionsClient.Client, o.ResourceManagerAuthorizer) - servicePrincipalsClient := graphrbac.NewServicePrincipalsClientWithBaseURI(o.GraphEndpoint, o.TenantID) - o.ConfigureClient(&servicePrincipalsClient.Client, o.GraphAuthorizer) - return &Client{ - GroupsClient: &groupsClient, - RoleAssignmentsClient: &roleAssignmentsClient, - RoleDefinitionsClient: &roleDefinitionsClient, - ServicePrincipalsClient: &servicePrincipalsClient, + RoleAssignmentsClient: &roleAssignmentsClient, + RoleDefinitionsClient: &roleDefinitionsClient, } } diff --git a/internal/services/authorization/client_config_data_source.go b/internal/services/authorization/client_config_data_source.go index f90f5c605e09..6b4e32ffec12 100644 --- a/internal/services/authorization/client_config_data_source.go +++ b/internal/services/authorization/client_config_data_source.go @@ -1,7 +1,6 @@ package authorization import ( - "fmt" "time" "github.com/hashicorp/terraform-provider-azurerm/internal/clients" @@ -43,25 +42,9 @@ func dataSourceArmClientConfig() *pluginsdk.Resource { func dataSourceArmClientConfigRead(d *pluginsdk.ResourceData, meta interface{}) error { client := meta.(*clients.Client) - ctx, cancel := timeouts.ForRead(meta.(*clients.Client).StopContext, d) + _, cancel := timeouts.ForRead(meta.(*clients.Client).StopContext, d) defer cancel() - if client.Account.AuthenticatedAsAServicePrincipal { - spClient := client.Authorization.ServicePrincipalsClient - // Application & Service Principal is 1:1 per tenant. Since we know the appId (client_id) - // here, we can query for the Service Principal whose appId matches. - filter := fmt.Sprintf("appId eq '%s'", client.Account.ClientId) - listResult, listErr := spClient.List(ctx, filter) - - if listErr != nil { - return fmt.Errorf("listing Service Principals: %#v", listErr) - } - - if listResult.Values() == nil || len(listResult.Values()) != 1 { - return fmt.Errorf("Unexpected Service Principal query result: %#v", listResult.Values()) - } - } - d.SetId(time.Now().UTC().String()) d.Set("client_id", client.Account.ClientId) d.Set("object_id", client.Account.ObjectId) diff --git a/internal/services/hdinsight/client/client.go b/internal/services/hdinsight/client/client.go index 688f4c1b4396..d49f6b169b0a 100644 --- a/internal/services/hdinsight/client/client.go +++ b/internal/services/hdinsight/client/client.go @@ -1,6 +1,7 @@ package client import ( + "github.com/Azure/azure-sdk-for-go/profiles/latest/graphrbac/graphrbac" "github.com/Azure/azure-sdk-for-go/services/hdinsight/mgmt/2018-06-01/hdinsight" "github.com/hashicorp/terraform-provider-azurerm/internal/common" ) @@ -10,6 +11,9 @@ type Client struct { ClustersClient *hdinsight.ClustersClient ConfigurationsClient *hdinsight.ConfigurationsClient ExtensionsClient *hdinsight.ExtensionsClient + + // TODO: remove graph client in v3.0 + GroupsClient *graphrbac.GroupsClient } func NewClient(o *common.ClientOptions) *Client { @@ -29,10 +33,17 @@ func NewClient(o *common.ClientOptions) *Client { ExtensionsClient := hdinsight.NewExtensionsClientWithBaseURI(opts.ResourceManagerEndpoint, opts.SubscriptionId) opts.ConfigureClient(&ExtensionsClient.Client, opts.ResourceManagerAuthorizer) + // TODO: remove graph client in v3.0 + groupsClient := graphrbac.NewGroupsClient(opts.TenantID) + opts.ConfigureClient(&groupsClient.Client, opts.GraphAuthorizer) + return &Client{ ApplicationsClient: &ApplicationsClient, ClustersClient: &ClustersClient, ConfigurationsClient: &ConfigurationsClient, ExtensionsClient: &ExtensionsClient, + + // TODO: remove graph client in v3.0 + GroupsClient: &groupsClient, } } diff --git a/internal/services/hdinsight/hdinsight_kafka_cluster_resource.go b/internal/services/hdinsight/hdinsight_kafka_cluster_resource.go index 41a64a5b5b63..f319eb567297 100644 --- a/internal/services/hdinsight/hdinsight_kafka_cluster_resource.go +++ b/internal/services/hdinsight/hdinsight_kafka_cluster_resource.go @@ -6,7 +6,6 @@ import ( "log" "time" - "github.com/Azure/azure-sdk-for-go/services/graphrbac/1.6/graphrbac" "github.com/Azure/azure-sdk-for-go/services/hdinsight/mgmt/2018-06-01/hdinsight" "github.com/hashicorp/terraform-provider-azurerm/helpers/azure" "github.com/hashicorp/terraform-provider-azurerm/helpers/tf" @@ -60,6 +59,9 @@ func resourceHDInsightKafkaCluster() *pluginsdk.Resource { // TODO: replace this with an importer which validates the ID during import Importer: pluginsdk.DefaultImporter(), + // TODO: won't be needed in v3.0 + CustomizeDiff: resourceHDInsightKafkaClusterCustomizeDiff, + Timeouts: &pluginsdk.ResourceTimeout{ Create: pluginsdk.DefaultTimeout(60 * time.Minute), Read: pluginsdk.DefaultTimeout(5 * time.Minute), @@ -140,6 +142,14 @@ func resourceHDInsightKafkaCluster() *pluginsdk.Resource { ForceNew: true, ValidateFunc: validation.IsUUID, }, + + "security_group_name": { + Type: pluginsdk.TypeString, + Optional: true, // TODO: make this Required in v3.0 + Computed: true, // TODO: remove Computed in v3.0 + ForceNew: true, + ValidateFunc: validation.StringIsNotEmpty, + }, }, }, RequiredWith: []string{"roles.0.kafka_management_node"}, @@ -167,11 +177,28 @@ func resourceHDInsightKafkaCluster() *pluginsdk.Resource { } } +func resourceHDInsightKafkaClusterCustomizeDiff(_ context.Context, diff *pluginsdk.ResourceDiff, meta interface{}) error { + // TODO: this conditional validation no longer needed in v3.0, `security_group_name` will be Required + if meta.(*clients.Client).Account.UseMSAL { + if v, ok := diff.GetOk("rest_proxy"); ok && len(v.([]interface{})) > 0 { + restProxy := v.([]interface{})[0].(map[string]interface{}) + if restProxy["security_group_name"].(string) == "" { + return fmt.Errorf("`rest_proxy.0.security_group_name` is required when the provider setting `use_msal` is true") + } + } + } + + return nil +} + func resourceHDInsightKafkaClusterCreate(d *pluginsdk.ResourceData, meta interface{}) error { - groupClient := meta.(*clients.Client).Authorization.GroupsClient client := meta.(*clients.Client).HDInsight.ClustersClient - subscriptionId := meta.(*clients.Client).Account.SubscriptionId extensionsClient := meta.(*clients.Client).HDInsight.ExtensionsClient + + // TODO: remove graph client in v3.0 + groupsClient := meta.(*clients.Client).HDInsight.GroupsClient + + subscriptionId := meta.(*clients.Client).Account.SubscriptionId ctx, cancel := timeouts.ForCreate(meta.(*clients.Client).StopContext, d) defer cancel() @@ -226,9 +253,22 @@ func resourceHDInsightKafkaClusterCreate(d *pluginsdk.ResourceData, meta interfa return tf.ImportAsExistsError("azurerm_hdinsight_kafka_cluster", *existing.ID) } - kafkaRestProperty, err := expandKafkaRestProxyProperty(ctx, groupClient, d.Get("rest_proxy").([]interface{})) - if err != nil { - return fmt.Errorf("expanding kafka rest proxy property") + // TODO: in v3.0, reduce this entire block to the following expandKafkaRestProxyProperty() function call + var kafkaRestProperty *hdinsight.KafkaRestProperties + if meta.(*clients.Client).Account.UseMSAL { + kafkaRestProperty = expandKafkaRestProxyProperty(d.Get("rest_proxy").([]interface{})) + } else { + kafkaRestProperty, err = expandKafkaRestProxyPropertyDeprecated(d.Get("rest_proxy").([]interface{}), func(groupId string) (*string, error) { + res, err := groupsClient.Get(ctx, groupId) + if err != nil { + return nil, fmt.Errorf("retrieving AAD group %q: %v", groupId, err) + } + + return res.DisplayName, nil + }) + if err != nil { + return fmt.Errorf("expanding kafka rest proxy property: %v", err) + } } params := hdinsight.ClusterCreateParametersExtended{ @@ -394,7 +434,8 @@ func resourceHDInsightKafkaClusterRead(d *pluginsdk.ResourceData, meta interface } d.Set("monitor", flattenHDInsightMonitoring(monitor)) - if err := d.Set("rest_proxy", flattenKafkaRestProxyProperty(props.KafkaRestProperties)); err != nil { + + if err = d.Set("rest_proxy", flattenKafkaRestProxyProperty(props.KafkaRestProperties)); err != nil { return fmt.Errorf(`failed setting "rest_proxy" for HDInsight Kafka Cluster %q (Resource Group %q): %+v`, name, resourceGroup, err) } @@ -430,7 +471,26 @@ func flattenHDInsightKafkaComponentVersion(input map[string]*string) []interface } } -func expandKafkaRestProxyProperty(ctx context.Context, client *graphrbac.GroupsClient, input []interface{}) (*hdinsight.KafkaRestProperties, error) { +func expandKafkaRestProxyProperty(input []interface{}) *hdinsight.KafkaRestProperties { + if len(input) == 0 || input[0] == nil { + return nil + } + + raw := input[0].(map[string]interface{}) + groupId := raw["security_group_id"].(string) + groupName := raw["security_group_name"].(string) + + return &hdinsight.KafkaRestProperties{ + ClientGroupInfo: &hdinsight.ClientGroupInfo{ + GroupID: &groupId, + GroupName: &groupName, + }, + } +} + +// TODO: remove expandKafkaRestProxyPropertyDeprecated in v3.0 +// nolint gocritic +func expandKafkaRestProxyPropertyDeprecated(input []interface{}, getGroupName func(string) (*string, error)) (*hdinsight.KafkaRestProperties, error) { if len(input) == 0 || input[0] == nil { return nil, nil } @@ -438,18 +498,27 @@ func expandKafkaRestProxyProperty(ctx context.Context, client *graphrbac.GroupsC raw := input[0].(map[string]interface{}) groupId := raw["security_group_id"].(string) - // Current API requires users further specify the "security_group_name" in the client group info of the kafka rest property, - // which is unnecessary as user already specify the "security_group_id". - // https://github.com/Azure/azure-rest-api-specs/issues/10667 - res, err := client.Get(ctx, groupId) - if err != nil { - return nil, fmt.Errorf("retrieving AAD gruop %s: %v", groupId, err) + groupName := "" + if v, ok := raw["security_group_name"]; ok && v.(string) != "" { + groupName = v.(string) + } else { + if getGroupName == nil { + return nil, fmt.Errorf("getGroupName function was nil") + } + displayName, err := getGroupName(groupId) + if err != nil { + return nil, err + } + if displayName == nil { + return nil, fmt.Errorf("retrieving group name for %q: displayName was nil", groupId) + } + groupName = *displayName } return &hdinsight.KafkaRestProperties{ ClientGroupInfo: &hdinsight.ClientGroupInfo{ GroupID: &groupId, - GroupName: res.DisplayName, + GroupName: &groupName, }, }, nil } @@ -460,14 +529,21 @@ func flattenKafkaRestProxyProperty(input *hdinsight.KafkaRestProperties) []inter } groupInfo := input.ClientGroupInfo + groupId := "" if groupInfo.GroupID != nil { groupId = *groupInfo.GroupID } + groupName := "" + if groupInfo.GroupName != nil { + groupName = *groupInfo.GroupName + } + return []interface{}{ map[string]interface{}{ - "security_group_id": groupId, + "security_group_id": groupId, + "security_group_name": groupName, }, } } diff --git a/internal/services/hdinsight/hdinsight_kafka_cluster_resource_test.go b/internal/services/hdinsight/hdinsight_kafka_cluster_resource_test.go index dd1a0cf4ca54..6865ea76c43c 100644 --- a/internal/services/hdinsight/hdinsight_kafka_cluster_resource_test.go +++ b/internal/services/hdinsight/hdinsight_kafka_cluster_resource_test.go @@ -388,6 +388,13 @@ func TestAccHDInsightKafkaCluster_restProxy(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_hdinsight_kafka_cluster", "test") r := HDInsightKafkaClusterResource{} data.ResourceTest(t, r, []acceptance.TestStep{ + { + Config: r.restProxyDeprecated(data), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + check.That(data.ResourceName).Key("kafka_rest_proxy_endpoint").Exists(), + ), + }, { Config: r.restProxy(data), Check: acceptance.ComposeTestCheckFunc( @@ -1293,6 +1300,76 @@ resource "azuread_group" "test" { security_enabled = true } +resource "azurerm_hdinsight_kafka_cluster" "test" { + name = "acctesthdi-%d" + resource_group_name = azurerm_resource_group.test.name + location = azurerm_resource_group.test.location + cluster_version = "4.0" + tier = "Standard" + + component_version { + kafka = "2.1" + } + + gateway { + enabled = true + username = "acctestusrgw" + password = "TerrAform123!" + } + + storage_account { + storage_container_id = azurerm_storage_container.test.id + storage_account_key = azurerm_storage_account.test.primary_access_key + is_default = true + } + + roles { + head_node { + vm_size = "Standard_D3_V2" + username = "acctestusrvm" + password = "AccTestvdSC4daf986!" + } + + worker_node { + vm_size = "Standard_D3_V2" + username = "acctestusrvm" + password = "AccTestvdSC4daf986!" + target_instance_count = 3 + number_of_disks_per_node = 2 + } + + zookeeper_node { + vm_size = "Standard_D3_V2" + username = "acctestusrvm" + password = "AccTestvdSC4daf986!" + } + + kafka_management_node { + vm_size = "Standard_D4_V2" + username = "acctestusrvm" + password = "AccTestvdSC4daf986!" + } + } + + rest_proxy { + security_group_id = azuread_group.test.id + security_group_name = azuread_group.test.display_name + } +} +`, r.template(data), data.RandomInteger, data.RandomInteger) +} + +func (r HDInsightKafkaClusterResource) restProxyDeprecated(data acceptance.TestData) string { + return fmt.Sprintf(` +%s + +provider "azuread" {} + +resource "azuread_group" "test" { + display_name = "acctesthdi-%d" + security_enabled = true +} + resource "azurerm_hdinsight_kafka_cluster" "test" { name = "acctesthdi-%d" resource_group_name = azurerm_resource_group.test.name