From b3c7364002cde4def6353d8440d9dc4c50563e3b Mon Sep 17 00:00:00 2001 From: Steph Date: Thu, 7 Apr 2022 16:24:55 +0200 Subject: [PATCH] rename public_network_access_enabled to better reflect that this applied to behind a vnet --- .../machine_learning_workspace_resource.go | 93 ++++++++++++------- ...achine_learning_workspace_resource_test.go | 89 +++++++++++++++--- .../machine_learning_workspace.html.markdown | 4 + 3 files changed, 140 insertions(+), 46 deletions(-) diff --git a/internal/services/machinelearning/machine_learning_workspace_resource.go b/internal/services/machinelearning/machine_learning_workspace_resource.go index 2d35897d9bad..373991e5a23a 100644 --- a/internal/services/machinelearning/machine_learning_workspace_resource.go +++ b/internal/services/machinelearning/machine_learning_workspace_resource.go @@ -4,19 +4,21 @@ import ( "fmt" "time" - "github.com/hashicorp/go-azure-helpers/resourcemanager/commonids" - "github.com/Azure/azure-sdk-for-go/services/machinelearningservices/mgmt/2021-07-01/machinelearningservices" + "github.com/hashicorp/go-azure-helpers/resourcemanager/commonids" "github.com/hashicorp/go-azure-helpers/resourcemanager/commonschema" "github.com/hashicorp/go-azure-helpers/resourcemanager/identity" "github.com/hashicorp/terraform-provider-azurerm/helpers/azure" "github.com/hashicorp/terraform-provider-azurerm/helpers/tf" "github.com/hashicorp/terraform-provider-azurerm/internal/clients" "github.com/hashicorp/terraform-provider-azurerm/internal/features" + appInsightsValidate "github.com/hashicorp/terraform-provider-azurerm/internal/services/applicationinsights/validate" + containerValidate "github.com/hashicorp/terraform-provider-azurerm/internal/services/containers/validate" keyVaultValidate "github.com/hashicorp/terraform-provider-azurerm/internal/services/keyvault/validate" "github.com/hashicorp/terraform-provider-azurerm/internal/services/machinelearning/parse" "github.com/hashicorp/terraform-provider-azurerm/internal/services/machinelearning/validate" msiValidate "github.com/hashicorp/terraform-provider-azurerm/internal/services/msi/validate" + storageValidate "github.com/hashicorp/terraform-provider-azurerm/internal/services/storage/validate" "github.com/hashicorp/terraform-provider-azurerm/internal/tags" "github.com/hashicorp/terraform-provider-azurerm/internal/tf/pluginsdk" "github.com/hashicorp/terraform-provider-azurerm/internal/tf/suppress" @@ -32,7 +34,7 @@ const ( ) func resourceMachineLearningWorkspace() *pluginsdk.Resource { - return &pluginsdk.Resource{ + resource := &pluginsdk.Resource{ Create: resourceMachineLearningWorkspaceCreate, Read: resourceMachineLearningWorkspaceRead, Update: resourceMachineLearningWorkspaceUpdate, @@ -63,11 +65,10 @@ func resourceMachineLearningWorkspace() *pluginsdk.Resource { "resource_group_name": azure.SchemaResourceGroupName(), "application_insights_id": { - Type: pluginsdk.TypeString, - Required: true, - ForceNew: true, - // TODO -- use the custom validation function of application insights - ValidateFunc: azure.ValidateResourceID, + Type: pluginsdk.TypeString, + Required: true, + ForceNew: true, + ValidateFunc: appInsightsValidate.ComponentID, // TODO -- remove when issue https://github.com/Azure/azure-rest-api-specs/issues/8323 is addressed DiffSuppressFunc: suppress.CaseDifference, }, @@ -82,11 +83,10 @@ func resourceMachineLearningWorkspace() *pluginsdk.Resource { }, "storage_account_id": { - Type: pluginsdk.TypeString, - Required: true, - ForceNew: true, - // TODO -- use the custom validation function of storage account, when issue https://github.com/Azure/azure-rest-api-specs/issues/8323 is addressed - ValidateFunc: azure.ValidateResourceID, + Type: pluginsdk.TypeString, + Required: true, + ForceNew: true, + ValidateFunc: storageValidate.StorageAccountID, // TODO -- remove when issue https://github.com/Azure/azure-rest-api-specs/issues/8323 is addressed DiffSuppressFunc: suppress.CaseDifference, }, @@ -100,20 +100,25 @@ func resourceMachineLearningWorkspace() *pluginsdk.Resource { }, "container_registry_id": { - Type: pluginsdk.TypeString, - Optional: true, - ForceNew: true, - // TODO -- use the custom validation function of container registry - ValidateFunc: azure.ValidateResourceID, + Type: pluginsdk.TypeString, + Optional: true, + ForceNew: true, + ValidateFunc: containerValidate.RegistryID, // TODO -- remove when issue https://github.com/Azure/azure-rest-api-specs/issues/8323 is addressed DiffSuppressFunc: suppress.CaseDifference, }, - "public_network_access_enabled": { + "public_access_behind_virtual_network_enabled": { Type: pluginsdk.TypeBool, Optional: true, + Computed: !features.FourPointOhBeta(), ForceNew: true, - Default: false, + ConflictsWith: func() []string { + if !features.FourPointOhBeta() { + return []string{"public_network_access_enabled"} + } + return []string{} + }(), }, "image_build_compute_name": { @@ -165,16 +170,10 @@ func resourceMachineLearningWorkspace() *pluginsdk.Resource { }, "sku_name": { - Type: pluginsdk.TypeString, - Optional: true, - Default: string(Basic), - ValidateFunc: validation.StringInSlice(func() []string { - out := []string{string(Basic)} - if !features.ThreePointOhBeta() { - out = append(out, "Enterprise") - } - return out - }(), !features.ThreePointOhBeta()), + Type: pluginsdk.TypeString, + Optional: true, + Default: string(Basic), + ValidateFunc: validation.StringInSlice([]string{string(Basic)}, false), DiffSuppressFunc: suppress.CaseDifferenceV2Only, }, @@ -186,6 +185,22 @@ func resourceMachineLearningWorkspace() *pluginsdk.Resource { "tags": tags.Schema(), }, } + + if !features.FourPointOhBeta() { + // For the time being we should just deprecate and remove this property since it's broken in the API - it doesn't + // actually set the property and also isn't returned by the API. Once https://github.com/Azure/azure-rest-api-specs/issues/18340 + // is fixed we can reassess how to deal with this field. + resource.Schema["public_network_access_enabled"] = &pluginsdk.Schema{ + Type: pluginsdk.TypeBool, + Optional: true, + Computed: true, + ForceNew: true, + Deprecated: "`public_network_access_enabled` will be removed in favour of the property `public_access_behind_virtual_network_enabled` in version 4.0 of the AzureRM Provider.", + ConflictsWith: []string{"public_access_behind_virtual_network_enabled"}, + } + } + + return resource } func resourceMachineLearningWorkspaceCreate(d *pluginsdk.ResourceData, meta interface{}) error { @@ -212,6 +227,16 @@ func resourceMachineLearningWorkspaceCreate(d *pluginsdk.ResourceData, meta inte expandedEncryption := expandMachineLearningWorkspaceEncryption(d.Get("encryption").([]interface{})) + networkAccessBehindVnetEnabled := false + if !features.FourPointOhBeta() { + if v, ok := d.GetOkExists("public_network_access_enabled"); ok { + networkAccessBehindVnetEnabled = v.(bool) + } + } + if v, ok := d.GetOkExists("public_access_behind_virtual_network_enabled"); ok { + networkAccessBehindVnetEnabled = v.(bool) + } + workspace := machinelearningservices.Workspace{ Name: utils.String(id.Name), Location: utils.String(azure.NormalizeLocation(d.Get("location").(string))), @@ -226,7 +251,7 @@ func resourceMachineLearningWorkspaceCreate(d *pluginsdk.ResourceData, meta inte StorageAccount: utils.String(d.Get("storage_account_id").(string)), ApplicationInsights: utils.String(d.Get("application_insights_id").(string)), KeyVault: utils.String(d.Get("key_vault_id").(string)), - AllowPublicAccessWhenBehindVnet: utils.Bool(d.Get("public_network_access_enabled").(bool)), + AllowPublicAccessWhenBehindVnet: utils.Bool(networkAccessBehindVnetEnabled), }, } @@ -305,10 +330,14 @@ func resourceMachineLearningWorkspaceRead(d *pluginsdk.ResourceData, meta interf d.Set("description", props.Description) d.Set("friendly_name", props.FriendlyName) d.Set("high_business_impact", props.HbiWorkspace) - d.Set("public_network_access_enabled", props.AllowPublicAccessWhenBehindVnet) d.Set("image_build_compute_name", props.ImageBuildCompute) d.Set("discovery_url", props.DiscoveryURL) d.Set("primary_user_assigned_identity", props.PrimaryUserAssignedIdentity) + d.Set("public_access_behind_virtual_network_enabled", props.AllowPublicAccessWhenBehindVnet) + + if !features.FourPointOhBeta() { + d.Set("public_network_access_enabled", props.AllowPublicAccessWhenBehindVnet) + } } flattenedIdentity, err := flattenMachineLearningWorkspaceIdentity(resp.Identity) diff --git a/internal/services/machinelearning/machine_learning_workspace_resource_test.go b/internal/services/machinelearning/machine_learning_workspace_resource_test.go index 118668a9c57d..12164b372d53 100644 --- a/internal/services/machinelearning/machine_learning_workspace_resource_test.go +++ b/internal/services/machinelearning/machine_learning_workspace_resource_test.go @@ -8,6 +8,7 @@ import ( "github.com/hashicorp/terraform-provider-azurerm/internal/acceptance" "github.com/hashicorp/terraform-provider-azurerm/internal/acceptance/check" "github.com/hashicorp/terraform-provider-azurerm/internal/clients" + "github.com/hashicorp/terraform-provider-azurerm/internal/features" "github.com/hashicorp/terraform-provider-azurerm/internal/services/machinelearning/parse" "github.com/hashicorp/terraform-provider-azurerm/internal/tf/pluginsdk" "github.com/hashicorp/terraform-provider-azurerm/utils" @@ -301,7 +302,8 @@ resource "azurerm_machine_learning_workspace" "test" { func (r WorkspaceResource) complete(data acceptance.TestData) string { template := r.template(data) - return fmt.Sprintf(` + if !features.FourPointOhBeta() { + return fmt.Sprintf(` %[1]s resource "azurerm_container_registry" "test" { @@ -358,6 +360,65 @@ resource "azurerm_machine_learning_workspace" "test" { ENV = "Test" } } +`, template, data.RandomIntOfLength(16)) + } + return fmt.Sprintf(` +%[1]s + +resource "azurerm_container_registry" "test" { + name = "acctestacr%[2]d" + resource_group_name = azurerm_resource_group.test.name + location = azurerm_resource_group.test.location + sku = "Standard" + admin_enabled = true +} + +resource "azurerm_key_vault_key" "test" { + name = "acctest-kv-key-%[2]d" + key_vault_id = azurerm_key_vault.test.id + key_type = "RSA" + key_size = 2048 + + key_opts = [ + "decrypt", + "encrypt", + "sign", + "unwrapKey", + "verify", + "wrapKey", + ] + + depends_on = [azurerm_key_vault.test, azurerm_key_vault_access_policy.test] +} + +resource "azurerm_machine_learning_workspace" "test" { + name = "acctest-MLW-%[2]d" + location = azurerm_resource_group.test.location + resource_group_name = azurerm_resource_group.test.name + friendly_name = "test-workspace" + description = "Test machine learning workspace" + application_insights_id = azurerm_application_insights.test.id + key_vault_id = azurerm_key_vault.test.id + storage_account_id = azurerm_storage_account.test.id + container_registry_id = azurerm_container_registry.test.id + sku_name = "Basic" + high_business_impact = true + public_access_behind_virtual_network_enabled = true + image_build_compute_name = "terraformCompute" + + identity { + type = "SystemAssigned" + } + + encryption { + key_vault_id = azurerm_key_vault.test.id + key_id = azurerm_key_vault_key.test.id + } + + tags = { + ENV = "Test" + } +} `, template, data.RandomIntOfLength(16)) } @@ -393,19 +454,19 @@ resource "azurerm_key_vault_key" "test" { } resource "azurerm_machine_learning_workspace" "test" { - name = "acctest-MLW-%[2]d" - location = azurerm_resource_group.test.location - resource_group_name = azurerm_resource_group.test.name - friendly_name = "test-workspace-updated" - description = "Test machine learning workspace update" - application_insights_id = azurerm_application_insights.test.id - key_vault_id = azurerm_key_vault.test.id - storage_account_id = azurerm_storage_account.test.id - container_registry_id = azurerm_container_registry.test.id - sku_name = "Basic" - high_business_impact = true - public_network_access_enabled = true - image_build_compute_name = "terraformCompute" + name = "acctest-MLW-%[2]d" + location = azurerm_resource_group.test.location + resource_group_name = azurerm_resource_group.test.name + friendly_name = "test-workspace-updated" + description = "Test machine learning workspace update" + application_insights_id = azurerm_application_insights.test.id + key_vault_id = azurerm_key_vault.test.id + storage_account_id = azurerm_storage_account.test.id + container_registry_id = azurerm_container_registry.test.id + sku_name = "Basic" + high_business_impact = true + public_access_behind_virtual_network_enabled = true + image_build_compute_name = "terraformCompute" identity { type = "SystemAssigned" diff --git a/website/docs/r/machine_learning_workspace.html.markdown b/website/docs/r/machine_learning_workspace.html.markdown index 78f38f84bd0d..d94e44d9d79d 100644 --- a/website/docs/r/machine_learning_workspace.html.markdown +++ b/website/docs/r/machine_learning_workspace.html.markdown @@ -360,8 +360,12 @@ The following arguments are supported: -> **NOTE:** The `admin_enabled` should be `true` in order to associate the Container Registry to this Machine Learning Workspace. +* `public_access_behind_virtual_network_enabled` - (Optional) Enable public access when this Machine Learning Workspace is behind a VNet. + * `public_network_access_enabled` - (Optional) Enable public access when this Machine Learning Workspace is behind VNet. +~> **NOTE:** `public_network_access_enabled` is deprecated and will be removed in favour of the property `public_access_behind_virtual_network_enabled` in version 4.0 of the AzureRM Provider. + * `image_build_compute_name` - (Optional) The compute name for image build of the Machine Learning Workspace. * `description` - (Optional) The description of this Machine Learning Workspace.