From c43d388b8348ce2a5b1f69aa968760e0fddeb971 Mon Sep 17 00:00:00 2001 From: Vladimir Lazarenko Date: Fri, 17 May 2024 23:32:07 +0200 Subject: [PATCH 01/17] `azurerm_network_watcher_flow_log`: Property changes `network_security_group_id` is superseded by `target_resource_id` Fixes #25982 --- .../network_watcher_flow_log_resource.go | 73 ++++++++++--------- .../r/network_watcher_flow_log.html.markdown | 4 + 2 files changed, 43 insertions(+), 34 deletions(-) diff --git a/internal/services/network/network_watcher_flow_log_resource.go b/internal/services/network/network_watcher_flow_log_resource.go index 77452c84818c..b1189f72b72d 100644 --- a/internal/services/network/network_watcher_flow_log_resource.go +++ b/internal/services/network/network_watcher_flow_log_resource.go @@ -14,7 +14,6 @@ import ( "github.com/hashicorp/go-azure-helpers/resourcemanager/commonschema" "github.com/hashicorp/go-azure-helpers/resourcemanager/location" "github.com/hashicorp/go-azure-helpers/resourcemanager/tags" - "github.com/hashicorp/go-azure-sdk/resource-manager/network/2023-11-01/networksecuritygroups" "github.com/hashicorp/go-azure-sdk/resource-manager/network/2024-03-01/flowlogs" "github.com/hashicorp/go-azure-sdk/resource-manager/network/2024-03-01/networkwatchers" "github.com/hashicorp/terraform-provider-azurerm/helpers/azure" @@ -72,11 +71,13 @@ func resourceNetworkWatcherFlowLog() *pluginsdk.Resource { ValidateFunc: validate.NetworkWatcherFlowLogName, }, - "network_security_group_id": { - Type: pluginsdk.TypeString, - Required: true, - ForceNew: true, - ValidateFunc: networksecuritygroups.ValidateNetworkSecurityGroupID, + "target_resource_id": { + Type: pluginsdk.TypeString, + Required: true, + ForceNew: true, + ValidateFunc: validation.Any( + validate.VirtualNetworkID, + ), }, "storage_account_id": { @@ -172,12 +173,13 @@ func resourceNetworkWatcherFlowLog() *pluginsdk.Resource { }, } - if !features.FourPointOhBeta() { - resource.Schema["version"] = &pluginsdk.Schema{ - Type: pluginsdk.TypeInt, - Optional: true, - Computed: true, - ValidateFunc: validation.IntBetween(1, 2), + if !features.FivePointOh() { + resource.Schema["network_security_group_id"] = &pluginsdk.Schema{ + Required: true, + ForceNew: true, + ValidateFunc: azure.ValidateResourceID, + Deprecated: "The property `network_security_group_id` has been superseded by `target_resource_id` and will be removed in version 5.0 of the AzureRM Provider.", + ConflictsWith: []string{"target_resource_id"}, } } @@ -203,10 +205,14 @@ func resourceNetworkWatcherFlowLogCreate(d *pluginsdk.ResourceData, meta interfa defer cancel() id := flowlogs.NewFlowLogID(subscriptionId, d.Get("resource_group_name").(string), d.Get("network_watcher_name").(string), d.Get("name").(string)) - nsgId, err := networksecuritygroups.ParseNetworkSecurityGroupID(d.Get("network_security_group_id").(string)) - if err != nil { - return err + + var targetResourceId string + if !features.FivePointOh() { + if v, ok := d.GetOk("network_security_group_id"); ok { + targetResourceId = v.(string) + } } + targetResourceId = d.Get("target_resource_id").(string) // For newly created resources, the "name" is required, it is set as Optional and Computed is merely for the existing ones for the sake of backward compatibility. if id.NetworkWatcherName == "" { @@ -224,8 +230,8 @@ func resourceNetworkWatcherFlowLogCreate(d *pluginsdk.ResourceData, meta interfa return tf.ImportAsExistsError("azurerm_network_watcher_flow_log", id.ID()) } - locks.ByID(nsgId.ID()) - defer locks.UnlockByID(nsgId.ID()) + locks.ByID(targetResourceId) + defer locks.UnlockByID(targetResourceId) loc := d.Get("location").(string) if loc == "" { @@ -244,7 +250,7 @@ func resourceNetworkWatcherFlowLogCreate(d *pluginsdk.ResourceData, meta interfa parameters := flowlogs.FlowLog{ Location: utils.String(location.Normalize(loc)), Properties: &flowlogs.FlowLogPropertiesFormat{ - TargetResourceId: nsgId.ID(), + TargetResourceId: targetResourceId, StorageId: d.Get("storage_account_id").(string), Enabled: pointer.To(d.Get("enabled").(bool)), RetentionPolicy: expandNetworkWatcherFlowLogRetentionPolicy(d.Get("retention_policy").([]interface{})), @@ -297,12 +303,14 @@ func resourceNetworkWatcherFlowLogUpdate(d *pluginsdk.ResourceData, meta interfa payload := existing.Model - nsgId, err := networksecuritygroups.ParseNetworkSecurityGroupID(d.Get("network_security_group_id").(string)) - if err != nil { - return err + var targetResourceId string + if !features.FivePointOh() { + targetResourceId = d.Get("network_security_group_id").(string) } - locks.ByID(nsgId.ID()) - defer locks.UnlockByID(nsgId.ID()) + targetResourceId = d.Get("target_resource_id").(string) + + locks.ByID(targetResourceId) + defer locks.UnlockByID(targetResourceId) if d.HasChange("storage_account_id") { payload.Properties.StorageId = d.Get("storage_account_id").(string) @@ -389,12 +397,12 @@ func resourceNetworkWatcherFlowLogRead(d *pluginsdk.ResourceData, meta interface d.Set("storage_account_id", props.StorageId) } - networkSecurityGroupId := "" - nsgId, err := networksecuritygroups.ParseNetworkSecurityGroupIDInsensitively(props.TargetResourceId) - if err == nil { - networkSecurityGroupId = nsgId.ID() + targetResourceId := "" + if !features.FourPointOhBeta() { + d.Set("network_security_group_id", targetResourceId) } - d.Set("network_security_group_id", networkSecurityGroupId) + + d.Set("target_resource_id", targetResourceId) if err := d.Set("retention_policy", flattenNetworkWatcherFlowLogRetentionPolicy(props.RetentionPolicy)); err != nil { return fmt.Errorf("setting `retention_policy`: %+v", err) @@ -425,13 +433,10 @@ func resourceNetworkWatcherFlowLogDelete(d *pluginsdk.ResourceData, meta interfa return fmt.Errorf("retreiving %s: `properties` or `properties.TargetResourceID` was nil", id) } - networkSecurityGroupId, err := networksecuritygroups.ParseNetworkSecurityGroupIDInsensitively(resp.Model.Properties.TargetResourceId) - if err != nil { - return fmt.Errorf("parsing %q as a Network Security Group ID: %+v", resp.Model.Properties.TargetResourceId, err) - } + targetResourceId := resp.Model.Properties.TargetResourceId - locks.ByID(networkSecurityGroupId.ID()) - defer locks.UnlockByID(networkSecurityGroupId.ID()) + locks.ByID(targetResourceId) + defer locks.UnlockByID(targetResourceId) if err := client.DeleteThenPoll(ctx, *id); err != nil { return fmt.Errorf("deleting %s: %v", id, err) diff --git a/website/docs/r/network_watcher_flow_log.html.markdown b/website/docs/r/network_watcher_flow_log.html.markdown index 2241e12a3e0a..1dd0e153e474 100644 --- a/website/docs/r/network_watcher_flow_log.html.markdown +++ b/website/docs/r/network_watcher_flow_log.html.markdown @@ -87,6 +87,10 @@ The following arguments are supported: * `network_security_group_id` - (Required) The ID of the Network Security Group for which to enable flow logs for. Changing this forces a new resource to be created. +~> **NOTE:** `network_security_group_id` is deprecated and will be removed in favour of the property `target_resource_id` in version 4.0 of the AzureRM Provider. + +* `target_resource_id` - (Required) The ID of the Resource for which to enable flow logs for. Changing this forces a new resource to be created. + * `storage_account_id` - (Required) The ID of the Storage Account where flow logs are stored. * `enabled` - (Required) Should Network Flow Logging be Enabled? From 2cb96866160d81f97f3f2e4e0e5282957a28c1e7 Mon Sep 17 00:00:00 2001 From: Vladimir Lazarenko Date: Fri, 17 May 2024 23:47:45 +0200 Subject: [PATCH 02/17] Fix schema errors --- .../network/network_watcher_flow_log_resource.go | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/internal/services/network/network_watcher_flow_log_resource.go b/internal/services/network/network_watcher_flow_log_resource.go index b1189f72b72d..23610e76f587 100644 --- a/internal/services/network/network_watcher_flow_log_resource.go +++ b/internal/services/network/network_watcher_flow_log_resource.go @@ -72,12 +72,10 @@ func resourceNetworkWatcherFlowLog() *pluginsdk.Resource { }, "target_resource_id": { - Type: pluginsdk.TypeString, - Required: true, - ForceNew: true, - ValidateFunc: validation.Any( - validate.VirtualNetworkID, - ), + Type: pluginsdk.TypeString, + Required: true, + ForceNew: true, + ValidateFunc: azure.ValidateResourceID, }, "storage_account_id": { @@ -175,6 +173,7 @@ func resourceNetworkWatcherFlowLog() *pluginsdk.Resource { if !features.FivePointOh() { resource.Schema["network_security_group_id"] = &pluginsdk.Schema{ + Type: pluginsdk.TypeString, Required: true, ForceNew: true, ValidateFunc: azure.ValidateResourceID, From 86b89be9e2f764a0ebd4983905ef73b249687628 Mon Sep 17 00:00:00 2001 From: Vladimir Lazarenko Date: Fri, 17 May 2024 23:50:41 +0200 Subject: [PATCH 03/17] Fix schema errors --- .../network_watcher_flow_log_resource.go | 20 ++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/internal/services/network/network_watcher_flow_log_resource.go b/internal/services/network/network_watcher_flow_log_resource.go index 23610e76f587..8b70f4f108b3 100644 --- a/internal/services/network/network_watcher_flow_log_resource.go +++ b/internal/services/network/network_watcher_flow_log_resource.go @@ -76,6 +76,7 @@ func resourceNetworkWatcherFlowLog() *pluginsdk.Resource { Required: true, ForceNew: true, ValidateFunc: azure.ValidateResourceID, + ExactlyOneOf: []string{"network_security_group_id", "target_resource_id"}, }, "storage_account_id": { @@ -173,12 +174,12 @@ func resourceNetworkWatcherFlowLog() *pluginsdk.Resource { if !features.FivePointOh() { resource.Schema["network_security_group_id"] = &pluginsdk.Schema{ - Type: pluginsdk.TypeString, - Required: true, - ForceNew: true, - ValidateFunc: azure.ValidateResourceID, - Deprecated: "The property `network_security_group_id` has been superseded by `target_resource_id` and will be removed in version 5.0 of the AzureRM Provider.", - ConflictsWith: []string{"target_resource_id"}, + Type: pluginsdk.TypeString, + Required: true, + ForceNew: true, + ValidateFunc: azure.ValidateResourceID, + Deprecated: "The property `network_security_group_id` has been superseded by `target_resource_id` and will be removed in version 5.0 of the AzureRM Provider.", + ExactlyOneOf: []string{"network_security_group_id", "target_resource_id"}, } } @@ -205,11 +206,16 @@ func resourceNetworkWatcherFlowLogCreate(d *pluginsdk.ResourceData, meta interfa id := flowlogs.NewFlowLogID(subscriptionId, d.Get("resource_group_name").(string), d.Get("network_watcher_name").(string), d.Get("name").(string)) - var targetResourceId string + targetResourceId := "" + if !features.FivePointOh() { if v, ok := d.GetOk("network_security_group_id"); ok { targetResourceId = v.(string) } + } else { + if v, ok := d.GetOk("target_resource_id"); ok { + targetResourceId = v.(string) + } } targetResourceId = d.Get("target_resource_id").(string) From 9a839881004c471d4de34fc25de49e110e20b8db Mon Sep 17 00:00:00 2001 From: Vladimir Lazarenko Date: Fri, 17 May 2024 23:56:29 +0200 Subject: [PATCH 04/17] Fix lint --- internal/services/network/network_watcher_flow_log_resource.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/internal/services/network/network_watcher_flow_log_resource.go b/internal/services/network/network_watcher_flow_log_resource.go index 8b70f4f108b3..71f844183e4d 100644 --- a/internal/services/network/network_watcher_flow_log_resource.go +++ b/internal/services/network/network_watcher_flow_log_resource.go @@ -311,6 +311,8 @@ func resourceNetworkWatcherFlowLogUpdate(d *pluginsdk.ResourceData, meta interfa var targetResourceId string if !features.FivePointOh() { targetResourceId = d.Get("network_security_group_id").(string) + } else { + targetResourceId = d.Get("target_resource_id").(string) } targetResourceId = d.Get("target_resource_id").(string) From 0f68ccc8b999309977ae6bbba0bc981d125e8c2f Mon Sep 17 00:00:00 2001 From: Vladimir Lazarenko Date: Wed, 22 May 2024 19:49:32 +0200 Subject: [PATCH 05/17] WIP --- internal/services/network/network_watcher_flow_log_resource.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/internal/services/network/network_watcher_flow_log_resource.go b/internal/services/network/network_watcher_flow_log_resource.go index 71f844183e4d..b4de3c35bc11 100644 --- a/internal/services/network/network_watcher_flow_log_resource.go +++ b/internal/services/network/network_watcher_flow_log_resource.go @@ -73,7 +73,6 @@ func resourceNetworkWatcherFlowLog() *pluginsdk.Resource { "target_resource_id": { Type: pluginsdk.TypeString, - Required: true, ForceNew: true, ValidateFunc: azure.ValidateResourceID, ExactlyOneOf: []string{"network_security_group_id", "target_resource_id"}, @@ -175,7 +174,6 @@ func resourceNetworkWatcherFlowLog() *pluginsdk.Resource { if !features.FivePointOh() { resource.Schema["network_security_group_id"] = &pluginsdk.Schema{ Type: pluginsdk.TypeString, - Required: true, ForceNew: true, ValidateFunc: azure.ValidateResourceID, Deprecated: "The property `network_security_group_id` has been superseded by `target_resource_id` and will be removed in version 5.0 of the AzureRM Provider.", From a6b68f7b398e37e2180d62c04b880a80d8be5c8a Mon Sep 17 00:00:00 2001 From: Vladimir Lazarenko Date: Sat, 25 May 2024 19:42:11 +0200 Subject: [PATCH 06/17] Add acceptance tests --- .../network_watcher_flow_log_resource_test.go | 141 ++++++++++++++---- .../network/network_watcher_resource_test.go | 22 +-- 2 files changed, 122 insertions(+), 41 deletions(-) diff --git a/internal/services/network/network_watcher_flow_log_resource_test.go b/internal/services/network/network_watcher_flow_log_resource_test.go index c63236b7b221..bc44a8278b19 100644 --- a/internal/services/network/network_watcher_flow_log_resource_test.go +++ b/internal/services/network/network_watcher_flow_log_resource_test.go @@ -33,6 +33,36 @@ func testAccNetworkWatcherFlowLog_basic(t *testing.T) { }) } +func testAccNetworkWatcherFlowLog_basicWithVirtualNetwork(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_network_watcher_flow_log", "test") + r := NetworkWatcherFlowLogResource{} + + data.ResourceSequentialTest(t, r, []acceptance.TestStep{ + { + Config: r.basicConfigWithVirtualNetwork(data), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), + }, + data.ImportStep(), + }) +} + +func testAccNetworkWatcherFlowLog_basicOldNSGProperty(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_network_watcher_flow_log", "test") + r := NetworkWatcherFlowLogResource{} + + data.ResourceSequentialTest(t, r, []acceptance.TestStep{ + { + Config: r.basicConfigOldNSGProperty(data), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), + }, + data.ImportStep(), + }) +} + func testAccNetworkWatcherFlowLog_requiresImport(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_network_watcher_flow_log", "test") r := NetworkWatcherFlowLogResource{} @@ -288,6 +318,55 @@ func (r NetworkWatcherFlowLogResource) basicConfig(data acceptance.TestData) str return fmt.Sprintf(` %s +resource "azurerm_network_watcher_flow_log" "test" { + network_watcher_name = azurerm_network_watcher.test.name + resource_group_name = azurerm_resource_group.test.name + name = "flowlog-%d" + + target_resource_id = azurerm_network_security_group.test.id + storage_account_id = azurerm_storage_account.test.id + enabled = true + + retention_policy { + enabled = false + days = 0 + } +} +`, r.prerequisites(data), data.RandomInteger) +} + +func (r NetworkWatcherFlowLogResource) basicConfigWithVirtualNetwork(data acceptance.TestData) string { + return fmt.Sprintf(` +%s + +resource "azurerm_virtual_network" "test" { + name = "acctestvn-%d" + address_space = ["10.0.0.0/16"] + location = azurerm_resource_group.test.location + resource_group_name = azurerm_resource_group.test.name +} + +resource "azurerm_network_watcher_flow_log" "test" { + network_watcher_name = azurerm_network_watcher.test.name + resource_group_name = azurerm_resource_group.test.name + name = "flowlog-%d" + + target_resource_id = azurerm_virtual_network.test.id + storage_account_id = azurerm_storage_account.test.id + enabled = true + + retention_policy { + enabled = false + days = 0 + } +} +`, r.prerequisites(data), data.RandomInteger, data.RandomInteger) +} + +func (r NetworkWatcherFlowLogResource) basicConfigOldNSGProperty(data acceptance.TestData) string { + return fmt.Sprintf(` +%s + resource "azurerm_network_watcher_flow_log" "test" { network_watcher_name = azurerm_network_watcher.test.name resource_group_name = azurerm_resource_group.test.name @@ -314,9 +393,9 @@ resource "azurerm_network_watcher_flow_log" "import" { resource_group_name = azurerm_network_watcher_flow_log.test.resource_group_name name = azurerm_network_watcher_flow_log.test.name - network_security_group_id = azurerm_network_watcher_flow_log.test.network_security_group_id - storage_account_id = azurerm_network_watcher_flow_log.test.storage_account_id - enabled = azurerm_network_watcher_flow_log.test.enabled + target_resource_id = azurerm_network_watcher_flow_log.test.target_resource_id + storage_account_id = azurerm_network_watcher_flow_log.test.storage_account_id + enabled = azurerm_network_watcher_flow_log.test.enabled retention_policy { enabled = false @@ -335,9 +414,9 @@ resource "azurerm_network_watcher_flow_log" "test" { resource_group_name = azurerm_resource_group.test.name name = "flowlog-%d" - network_security_group_id = azurerm_network_security_group.test.id - storage_account_id = azurerm_storage_account.test.id - enabled = true + target_resource_id = azurerm_network_security_group.test.id + storage_account_id = azurerm_storage_account.test.id + enabled = true retention_policy { enabled = true @@ -367,9 +446,9 @@ resource "azurerm_network_watcher_flow_log" "test" { resource_group_name = azurerm_resource_group.test.name name = "flowlog-%d" - network_security_group_id = azurerm_network_security_group.test.id - storage_account_id = azurerm_storage_account.testb.id - enabled = true + target_resource_id = azurerm_network_security_group.test.id + storage_account_id = azurerm_storage_account.testb.id + enabled = true retention_policy { enabled = true @@ -389,9 +468,9 @@ resource "azurerm_network_watcher_flow_log" "test" { name = "flowlog-%d" location = azurerm_network_watcher.test.location - network_security_group_id = azurerm_network_security_group.test.id - storage_account_id = azurerm_storage_account.test.id - enabled = false + target_resource_id = azurerm_network_security_group.test.id + storage_account_id = azurerm_storage_account.test.id + enabled = false retention_policy { enabled = true @@ -418,9 +497,9 @@ resource "azurerm_network_watcher_flow_log" "test" { name = "flowlog-%d" location = azurerm_network_watcher.test.location - network_security_group_id = azurerm_network_security_group.test.id - storage_account_id = azurerm_storage_account.test.id - enabled = true + target_resource_id = azurerm_network_security_group.test.id + storage_account_id = azurerm_storage_account.test.id + enabled = true retention_policy { enabled = true @@ -454,9 +533,9 @@ resource "azurerm_network_watcher_flow_log" "test" { name = "flowlog-%d" location = azurerm_network_watcher.test.location - network_security_group_id = azurerm_network_security_group.test.id - storage_account_id = azurerm_storage_account.test.id - enabled = true + target_resource_id = azurerm_network_security_group.test.id + storage_account_id = azurerm_storage_account.test.id + enabled = true retention_policy { enabled = true @@ -491,9 +570,9 @@ resource "azurerm_network_watcher_flow_log" "test" { name = "flowlog-%d" location = azurerm_network_watcher.test.location - network_security_group_id = azurerm_network_security_group.test.id - storage_account_id = azurerm_storage_account.test.id - enabled = true + target_resource_id = azurerm_network_security_group.test.id + storage_account_id = azurerm_storage_account.test.id + enabled = true retention_policy { enabled = true @@ -527,10 +606,10 @@ resource "azurerm_network_watcher_flow_log" "test" { name = "flowlog-%d" location = azurerm_network_watcher.test.location - network_security_group_id = azurerm_network_security_group.test.id - storage_account_id = azurerm_storage_account.test.id - enabled = true - version = %d + target_resource_id = azurerm_network_security_group.test.id + storage_account_id = azurerm_storage_account.test.id + enabled = true + version = %d retention_policy { enabled = true @@ -557,9 +636,9 @@ resource "azurerm_network_watcher_flow_log" "test" { name = "flowlog-%d" location = azurerm_resource_group.test.location - network_security_group_id = azurerm_network_security_group.test.id - storage_account_id = azurerm_storage_account.test.id - enabled = true + target_resource_id = azurerm_network_security_group.test.id + storage_account_id = azurerm_storage_account.test.id + enabled = true retention_policy { enabled = false @@ -578,9 +657,9 @@ resource "azurerm_network_watcher_flow_log" "test" { resource_group_name = azurerm_resource_group.test.name name = "flowlog-%d" - network_security_group_id = azurerm_network_security_group.test.id - storage_account_id = azurerm_storage_account.test.id - enabled = true + target_resource_id = azurerm_network_security_group.test.id + storage_account_id = azurerm_storage_account.test.id + enabled = true retention_policy { enabled = false diff --git a/internal/services/network/network_watcher_resource_test.go b/internal/services/network/network_watcher_resource_test.go index 57b9766f220b..be77d047e115 100644 --- a/internal/services/network/network_watcher_resource_test.go +++ b/internal/services/network/network_watcher_resource_test.go @@ -77,16 +77,18 @@ func TestAccNetworkWatcher(t *testing.T) { "machineScope": testAccVirtualMachineScaleSetPacketCapture_machineScope, }, "FlowLog": { - "basic": testAccNetworkWatcherFlowLog_basic, - "requiresImport": testAccNetworkWatcherFlowLog_requiresImport, - "disabled": testAccNetworkWatcherFlowLog_disabled, - "reenabled": testAccNetworkWatcherFlowLog_reenabled, - "retentionPolicy": testAccNetworkWatcherFlowLog_retentionPolicy, - "updateStorageAccount": testAccNetworkWatcherFlowLog_updateStorageAccount, - "trafficAnalytics": testAccNetworkWatcherFlowLog_trafficAnalytics, - "version": testAccNetworkWatcherFlowLog_version, - "location": testAccNetworkWatcherFlowLog_location, - "tags": testAccNetworkWatcherFlowLog_tags, + "basic": testAccNetworkWatcherFlowLog_basic, + "basicWithVirtualNetwork": testAccNetworkWatcherFlowLog_basicWithVirtualNetwork, + "basicOldNSGProperty": testAccNetworkWatcherFlowLog_basicOldNSGProperty, + "requiresImport": testAccNetworkWatcherFlowLog_requiresImport, + "disabled": testAccNetworkWatcherFlowLog_disabled, + "reenabled": testAccNetworkWatcherFlowLog_reenabled, + "retentionPolicy": testAccNetworkWatcherFlowLog_retentionPolicy, + "updateStorageAccount": testAccNetworkWatcherFlowLog_updateStorageAccount, + "trafficAnalytics": testAccNetworkWatcherFlowLog_trafficAnalytics, + "version": testAccNetworkWatcherFlowLog_version, + "location": testAccNetworkWatcherFlowLog_location, + "tags": testAccNetworkWatcherFlowLog_tags, }, } From 0b6ba2d264e971596adf5af5cc6928d2412f8de1 Mon Sep 17 00:00:00 2001 From: Vladimir Lazarenko Date: Sun, 26 May 2024 14:08:29 +0200 Subject: [PATCH 07/17] D'oh --- .../network/network_watcher_flow_log_resource.go | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/internal/services/network/network_watcher_flow_log_resource.go b/internal/services/network/network_watcher_flow_log_resource.go index b4de3c35bc11..665bb2c6ab55 100644 --- a/internal/services/network/network_watcher_flow_log_resource.go +++ b/internal/services/network/network_watcher_flow_log_resource.go @@ -210,10 +210,10 @@ func resourceNetworkWatcherFlowLogCreate(d *pluginsdk.ResourceData, meta interfa if v, ok := d.GetOk("network_security_group_id"); ok { targetResourceId = v.(string) } - } else { - if v, ok := d.GetOk("target_resource_id"); ok { - targetResourceId = v.(string) - } + } + + if v, ok := d.GetOk("target_resource_id"); ok { + targetResourceId = v.(string) } targetResourceId = d.Get("target_resource_id").(string) @@ -402,12 +402,11 @@ func resourceNetworkWatcherFlowLogRead(d *pluginsdk.ResourceData, meta interface d.Set("storage_account_id", props.StorageId) } - targetResourceId := "" if !features.FourPointOhBeta() { - d.Set("network_security_group_id", targetResourceId) + d.Set("network_security_group_id", props.TargetResourceId) } - d.Set("target_resource_id", targetResourceId) + d.Set("target_resource_id", props.TargetResourceId) if err := d.Set("retention_policy", flattenNetworkWatcherFlowLogRetentionPolicy(props.RetentionPolicy)); err != nil { return fmt.Errorf("setting `retention_policy`: %+v", err) From e53423bcbe02bc5c331e5c4042a0694629951c27 Mon Sep 17 00:00:00 2001 From: aristosvo <8375124+aristosvo@users.noreply.github.com> Date: Mon, 26 Aug 2024 21:45:06 +0200 Subject: [PATCH 08/17] fix based on the comments --- .../network_watcher_flow_log_resource.go | 52 +++++++++++++------ .../r/network_watcher_flow_log.html.markdown | 4 -- 2 files changed, 35 insertions(+), 21 deletions(-) diff --git a/internal/services/network/network_watcher_flow_log_resource.go b/internal/services/network/network_watcher_flow_log_resource.go index 665bb2c6ab55..9c7119e7632b 100644 --- a/internal/services/network/network_watcher_flow_log_resource.go +++ b/internal/services/network/network_watcher_flow_log_resource.go @@ -14,8 +14,10 @@ import ( "github.com/hashicorp/go-azure-helpers/resourcemanager/commonschema" "github.com/hashicorp/go-azure-helpers/resourcemanager/location" "github.com/hashicorp/go-azure-helpers/resourcemanager/tags" + "github.com/hashicorp/go-azure-sdk/resource-manager/network/2024-03-01/flowlogs" "github.com/hashicorp/go-azure-sdk/resource-manager/network/2024-03-01/networkwatchers" + "github.com/hashicorp/terraform-provider-azurerm/helpers/azure" "github.com/hashicorp/terraform-provider-azurerm/helpers/tf" "github.com/hashicorp/terraform-provider-azurerm/internal/clients" @@ -72,10 +74,13 @@ func resourceNetworkWatcherFlowLog() *pluginsdk.Resource { }, "target_resource_id": { - Type: pluginsdk.TypeString, - ForceNew: true, - ValidateFunc: azure.ValidateResourceID, - ExactlyOneOf: []string{"network_security_group_id", "target_resource_id"}, + Type: pluginsdk.TypeString, + ForceNew: true, + Required: true, + ValidateFunc: validation.Any( + networksecuritygroups.ValidateNetworkSecurityGroupID, + commonids.ValidateVirtualNetworkID, + ), }, "storage_account_id": { @@ -171,14 +176,18 @@ func resourceNetworkWatcherFlowLog() *pluginsdk.Resource { }, } - if !features.FivePointOh() { + if !features.FivePointOhBeta() { resource.Schema["network_security_group_id"] = &pluginsdk.Schema{ Type: pluginsdk.TypeString, + Optional: true, ForceNew: true, - ValidateFunc: azure.ValidateResourceID, + ValidateFunc: networksecuritygroups.ValidateNetworkSecurityGroupID, Deprecated: "The property `network_security_group_id` has been superseded by `target_resource_id` and will be removed in version 5.0 of the AzureRM Provider.", ExactlyOneOf: []string{"network_security_group_id", "target_resource_id"}, } + resource.Schema["target_resource_id"].Required = false + resource.Schema["target_resource_id"].Optional = true + resource.Schema["target_resource_id"].ExactlyOneOf = []string{"network_security_group_id", "target_resource_id"} } return resource @@ -206,16 +215,15 @@ func resourceNetworkWatcherFlowLogCreate(d *pluginsdk.ResourceData, meta interfa targetResourceId := "" - if !features.FivePointOh() { - if v, ok := d.GetOk("network_security_group_id"); ok { + if !features.FivePointOhBeta() { + if v, ok := d.GetOk("network_security_group_id"); ok && v.(string) != "" { targetResourceId = v.(string) } } - if v, ok := d.GetOk("target_resource_id"); ok { + if v, ok := d.GetOk("target_resource_id"); ok && v.(string) != "" { targetResourceId = v.(string) } - targetResourceId = d.Get("target_resource_id").(string) // For newly created resources, the "name" is required, it is set as Optional and Computed is merely for the existing ones for the sake of backward compatibility. if id.NetworkWatcherName == "" { @@ -305,14 +313,12 @@ func resourceNetworkWatcherFlowLogUpdate(d *pluginsdk.ResourceData, meta interfa } payload := existing.Model - var targetResourceId string - if !features.FivePointOh() { + if !features.FivePointOhBeta() { targetResourceId = d.Get("network_security_group_id").(string) } else { targetResourceId = d.Get("target_resource_id").(string) } - targetResourceId = d.Get("target_resource_id").(string) locks.ByID(targetResourceId) defer locks.UnlockByID(targetResourceId) @@ -402,11 +408,18 @@ func resourceNetworkWatcherFlowLogRead(d *pluginsdk.ResourceData, meta interface d.Set("storage_account_id", props.StorageId) } - if !features.FourPointOhBeta() { - d.Set("network_security_group_id", props.TargetResourceId) + targetResourceId := props.TargetResourceId + if nsgId, err := networksecuritygroups.ParseNetworkSecurityGroupIDInsensitively(props.TargetResourceId); err == nil { + targetResourceId = nsgId.ID() + } else if vnetId, err := commonids.ParseVirtualNetworkIDInsensitively(props.TargetResourceId); err == nil { + targetResourceId = vnetId.ID() + } + + if !features.FivePointOhBeta() { + d.Set("network_security_group_id", targetResourceId) } - d.Set("target_resource_id", props.TargetResourceId) + d.Set("target_resource_id", targetResourceId) if err := d.Set("retention_policy", flattenNetworkWatcherFlowLogRetentionPolicy(props.RetentionPolicy)); err != nil { return fmt.Errorf("setting `retention_policy`: %+v", err) @@ -434,10 +447,15 @@ func resourceNetworkWatcherFlowLogDelete(d *pluginsdk.ResourceData, meta interfa return fmt.Errorf("retrieving %s: %+v", id, err) } if resp.Model == nil || resp.Model.Properties == nil || resp.Model.Properties.TargetResourceId == "" { - return fmt.Errorf("retreiving %s: `properties` or `properties.TargetResourceID` was nil", id) + return fmt.Errorf("retrieving %s: `properties` or `properties.TargetResourceID` was nil", id) } targetResourceId := resp.Model.Properties.TargetResourceId + if nsgId, err := networksecuritygroups.ParseNetworkSecurityGroupIDInsensitively(resp.Model.Properties.TargetResourceId); err == nil { + targetResourceId = nsgId.ID() + } else if vnetId, err := commonids.ParseVirtualNetworkIDInsensitively(resp.Model.Properties.TargetResourceId); err == nil { + targetResourceId = vnetId.ID() + } locks.ByID(targetResourceId) defer locks.UnlockByID(targetResourceId) diff --git a/website/docs/r/network_watcher_flow_log.html.markdown b/website/docs/r/network_watcher_flow_log.html.markdown index 1dd0e153e474..b6814eb543e9 100644 --- a/website/docs/r/network_watcher_flow_log.html.markdown +++ b/website/docs/r/network_watcher_flow_log.html.markdown @@ -85,10 +85,6 @@ The following arguments are supported: * `resource_group_name` - (Required) The name of the resource group in which the Network Watcher was deployed. Changing this forces a new resource to be created. -* `network_security_group_id` - (Required) The ID of the Network Security Group for which to enable flow logs for. Changing this forces a new resource to be created. - -~> **NOTE:** `network_security_group_id` is deprecated and will be removed in favour of the property `target_resource_id` in version 4.0 of the AzureRM Provider. - * `target_resource_id` - (Required) The ID of the Resource for which to enable flow logs for. Changing this forces a new resource to be created. * `storage_account_id` - (Required) The ID of the Storage Account where flow logs are stored. From 7b63021bef6a8f12a36977cfbc5725a34f144d30 Mon Sep 17 00:00:00 2001 From: Vladimir Lazarenko Date: Thu, 14 Nov 2024 18:44:03 +0100 Subject: [PATCH 09/17] Rebase --- internal/services/network/network_watcher_flow_log_resource.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/internal/services/network/network_watcher_flow_log_resource.go b/internal/services/network/network_watcher_flow_log_resource.go index 9c7119e7632b..3e8415fcaf94 100644 --- a/internal/services/network/network_watcher_flow_log_resource.go +++ b/internal/services/network/network_watcher_flow_log_resource.go @@ -14,10 +14,9 @@ import ( "github.com/hashicorp/go-azure-helpers/resourcemanager/commonschema" "github.com/hashicorp/go-azure-helpers/resourcemanager/location" "github.com/hashicorp/go-azure-helpers/resourcemanager/tags" - "github.com/hashicorp/go-azure-sdk/resource-manager/network/2024-03-01/flowlogs" + "github.com/hashicorp/go-azure-sdk/resource-manager/network/2024-03-01/networksecuritygroups" "github.com/hashicorp/go-azure-sdk/resource-manager/network/2024-03-01/networkwatchers" - "github.com/hashicorp/terraform-provider-azurerm/helpers/azure" "github.com/hashicorp/terraform-provider-azurerm/helpers/tf" "github.com/hashicorp/terraform-provider-azurerm/internal/clients" From fb5a8f4c9c59a00c3a1cb5ce139f5b8775a6a160 Mon Sep 17 00:00:00 2001 From: Vladimir Lazarenko Date: Thu, 14 Nov 2024 19:11:42 +0100 Subject: [PATCH 10/17] Fix update hopefully --- .../network/network_watcher_flow_log_resource.go | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/internal/services/network/network_watcher_flow_log_resource.go b/internal/services/network/network_watcher_flow_log_resource.go index 3e8415fcaf94..f20407e3f7ce 100644 --- a/internal/services/network/network_watcher_flow_log_resource.go +++ b/internal/services/network/network_watcher_flow_log_resource.go @@ -180,12 +180,14 @@ func resourceNetworkWatcherFlowLog() *pluginsdk.Resource { Type: pluginsdk.TypeString, Optional: true, ForceNew: true, + Computed: true, ValidateFunc: networksecuritygroups.ValidateNetworkSecurityGroupID, Deprecated: "The property `network_security_group_id` has been superseded by `target_resource_id` and will be removed in version 5.0 of the AzureRM Provider.", ExactlyOneOf: []string{"network_security_group_id", "target_resource_id"}, } resource.Schema["target_resource_id"].Required = false resource.Schema["target_resource_id"].Optional = true + resource.Schema["target_resource_id"].Computed = true resource.Schema["target_resource_id"].ExactlyOneOf = []string{"network_security_group_id", "target_resource_id"} } @@ -312,11 +314,17 @@ func resourceNetworkWatcherFlowLogUpdate(d *pluginsdk.ResourceData, meta interfa } payload := existing.Model - var targetResourceId string + + targetResourceId := "" + if !features.FivePointOhBeta() { - targetResourceId = d.Get("network_security_group_id").(string) - } else { - targetResourceId = d.Get("target_resource_id").(string) + if v, ok := d.GetOk("network_security_group_id"); ok && v.(string) != "" { + targetResourceId = v.(string) + } + } + + if v, ok := d.GetOk("target_resource_id"); ok && v.(string) != "" { + targetResourceId = v.(string) } locks.ByID(targetResourceId) From 575c713e208dd4bb02f587332af264e3f652cfcc Mon Sep 17 00:00:00 2001 From: Vladimir Lazarenko Date: Thu, 14 Nov 2024 19:29:18 +0100 Subject: [PATCH 11/17] How about this one then --- .../network_watcher_flow_log_resource.go | 2 +- .../network_watcher_flow_log_resource_test.go | 345 ++++++++++++++++-- 2 files changed, 322 insertions(+), 25 deletions(-) diff --git a/internal/services/network/network_watcher_flow_log_resource.go b/internal/services/network/network_watcher_flow_log_resource.go index f20407e3f7ce..a4502c3b080c 100644 --- a/internal/services/network/network_watcher_flow_log_resource.go +++ b/internal/services/network/network_watcher_flow_log_resource.go @@ -188,7 +188,7 @@ func resourceNetworkWatcherFlowLog() *pluginsdk.Resource { resource.Schema["target_resource_id"].Required = false resource.Schema["target_resource_id"].Optional = true resource.Schema["target_resource_id"].Computed = true - resource.Schema["target_resource_id"].ExactlyOneOf = []string{"network_security_group_id", "target_resource_id"} + resource.Schema["target_resource_id"].ConflictsWith = []string{"network_security_group_id"} } return resource diff --git a/internal/services/network/network_watcher_flow_log_resource_test.go b/internal/services/network/network_watcher_flow_log_resource_test.go index bc44a8278b19..a6e75447df56 100644 --- a/internal/services/network/network_watcher_flow_log_resource_test.go +++ b/internal/services/network/network_watcher_flow_log_resource_test.go @@ -13,6 +13,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/tf/pluginsdk" ) @@ -48,21 +49,6 @@ func testAccNetworkWatcherFlowLog_basicWithVirtualNetwork(t *testing.T) { }) } -func testAccNetworkWatcherFlowLog_basicOldNSGProperty(t *testing.T) { - data := acceptance.BuildTestData(t, "azurerm_network_watcher_flow_log", "test") - r := NetworkWatcherFlowLogResource{} - - data.ResourceSequentialTest(t, r, []acceptance.TestStep{ - { - Config: r.basicConfigOldNSGProperty(data), - Check: acceptance.ComposeTestCheckFunc( - check.That(data.ResourceName).ExistsInAzure(r), - ), - }, - data.ImportStep(), - }) -} - func testAccNetworkWatcherFlowLog_requiresImport(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_network_watcher_flow_log", "test") r := NetworkWatcherFlowLogResource{} @@ -315,6 +301,26 @@ resource "azurerm_storage_account" "test" { } func (r NetworkWatcherFlowLogResource) basicConfig(data acceptance.TestData) string { + if !features.FivePointOhBeta() { + return fmt.Sprintf(` +%s + +resource "azurerm_network_watcher_flow_log" "test" { + network_watcher_name = azurerm_network_watcher.test.name + resource_group_name = azurerm_resource_group.test.name + name = "flowlog-%d" + + network_security_group_id = azurerm_network_security_group.test.id + storage_account_id = azurerm_storage_account.test.id + enabled = true + + retention_policy { + enabled = false + days = 0 + } +} +`, r.prerequisites(data), data.RandomInteger) + } return fmt.Sprintf(` %s @@ -336,7 +342,8 @@ resource "azurerm_network_watcher_flow_log" "test" { } func (r NetworkWatcherFlowLogResource) basicConfigWithVirtualNetwork(data acceptance.TestData) string { - return fmt.Sprintf(` + if !features.FivePointOhBeta() { + return fmt.Sprintf(` %s resource "azurerm_virtual_network" "test" { @@ -351,7 +358,7 @@ resource "azurerm_network_watcher_flow_log" "test" { resource_group_name = azurerm_resource_group.test.name name = "flowlog-%d" - target_resource_id = azurerm_virtual_network.test.id + network_security_group_id = azurerm_virtual_network.test.id storage_account_id = azurerm_storage_account.test.id enabled = true @@ -361,30 +368,55 @@ resource "azurerm_network_watcher_flow_log" "test" { } } `, r.prerequisites(data), data.RandomInteger, data.RandomInteger) -} - -func (r NetworkWatcherFlowLogResource) basicConfigOldNSGProperty(data acceptance.TestData) string { + } return fmt.Sprintf(` %s +resource "azurerm_virtual_network" "test" { + name = "acctestvn-%d" + address_space = ["10.0.0.0/16"] + location = azurerm_resource_group.test.location + resource_group_name = azurerm_resource_group.test.name +} + resource "azurerm_network_watcher_flow_log" "test" { network_watcher_name = azurerm_network_watcher.test.name resource_group_name = azurerm_resource_group.test.name name = "flowlog-%d" - network_security_group_id = azurerm_network_security_group.test.id - storage_account_id = azurerm_storage_account.test.id - enabled = true + target_resource_id = azurerm_virtual_network.test.id + storage_account_id = azurerm_storage_account.test.id + enabled = true retention_policy { enabled = false days = 0 } } -`, r.prerequisites(data), data.RandomInteger) +`, r.prerequisites(data), data.RandomInteger, data.RandomInteger) } func (r NetworkWatcherFlowLogResource) requiresImport(data acceptance.TestData) string { + if !features.FivePointOhBeta() { + return fmt.Sprintf(` +%s + +resource "azurerm_network_watcher_flow_log" "import" { + network_watcher_name = azurerm_network_watcher_flow_log.test.network_watcher_name + resource_group_name = azurerm_network_watcher_flow_log.test.resource_group_name + name = azurerm_network_watcher_flow_log.test.name + + network_security_group_id = azurerm_network_watcher_flow_log.test.target_resource_id + storage_account_id = azurerm_network_watcher_flow_log.test.storage_account_id + enabled = azurerm_network_watcher_flow_log.test.enabled + + retention_policy { + enabled = false + days = 0 + } +} +`, r.basicConfig(data)) + } return fmt.Sprintf(` %s @@ -406,6 +438,26 @@ resource "azurerm_network_watcher_flow_log" "import" { } func (r NetworkWatcherFlowLogResource) retentionPolicyConfig(data acceptance.TestData) string { + if !features.FivePointOhBeta() { + return fmt.Sprintf(` +%s + +resource "azurerm_network_watcher_flow_log" "test" { + network_watcher_name = azurerm_network_watcher.test.name + resource_group_name = azurerm_resource_group.test.name + name = "flowlog-%d" + + network_security_group_id = azurerm_network_security_group.test.id + storage_account_id = azurerm_storage_account.test.id + enabled = true + + retention_policy { + enabled = true + days = 7 + } +} +`, r.prerequisites(data), data.RandomInteger) + } return fmt.Sprintf(` %s @@ -427,6 +479,37 @@ resource "azurerm_network_watcher_flow_log" "test" { } func (r NetworkWatcherFlowLogResource) retentionPolicyConfigUpdateStorageAccount(data acceptance.TestData) string { + if !features.FivePointOhBeta() { + return fmt.Sprintf(` +%s + +resource "azurerm_storage_account" "testb" { + name = "acctestsab%d" + resource_group_name = azurerm_resource_group.test.name + location = azurerm_resource_group.test.location + + account_tier = "Standard" + account_kind = "StorageV2" + account_replication_type = "LRS" + https_traffic_only_enabled = true +} + +resource "azurerm_network_watcher_flow_log" "test" { + network_watcher_name = azurerm_network_watcher.test.name + resource_group_name = azurerm_resource_group.test.name + name = "flowlog-%d" + + network_security_group_id = azurerm_network_security_group.test.id + storage_account_id = azurerm_storage_account.testb.id + enabled = true + + retention_policy { + enabled = true + days = 7 + } +} +`, r.prerequisites(data), data.RandomInteger%1000000+1, data.RandomInteger) + } return fmt.Sprintf(` %s @@ -459,6 +542,27 @@ resource "azurerm_network_watcher_flow_log" "test" { } func (r NetworkWatcherFlowLogResource) disabledConfig(data acceptance.TestData) string { + if !features.FivePointOhBeta() { + return fmt.Sprintf(` +%s + +resource "azurerm_network_watcher_flow_log" "test" { + network_watcher_name = azurerm_network_watcher.test.name + resource_group_name = azurerm_resource_group.test.name + name = "flowlog-%d" + location = azurerm_network_watcher.test.location + + network_security_group_id = azurerm_network_security_group.test.id + storage_account_id = azurerm_storage_account.test.id + enabled = false + + retention_policy { + enabled = true + days = 7 + } +} +`, r.prerequisites(data), data.RandomInteger) + } return fmt.Sprintf(` %s @@ -481,6 +585,41 @@ resource "azurerm_network_watcher_flow_log" "test" { } func (r NetworkWatcherFlowLogResource) TrafficAnalyticsEnabledConfig(data acceptance.TestData) string { + if !features.FivePointOhBeta() { + return fmt.Sprintf(` +%s + +resource "azurerm_log_analytics_workspace" "test" { + name = "acctestLAW-%d" + location = azurerm_resource_group.test.location + resource_group_name = azurerm_resource_group.test.name + sku = "PerGB2018" +} + +resource "azurerm_network_watcher_flow_log" "test" { + network_watcher_name = azurerm_network_watcher.test.name + resource_group_name = azurerm_resource_group.test.name + name = "flowlog-%d" + location = azurerm_network_watcher.test.location + + network_security_group_id = azurerm_network_security_group.test.id + storage_account_id = azurerm_storage_account.test.id + enabled = true + + retention_policy { + enabled = true + days = 7 + } + + traffic_analytics { + enabled = true + workspace_id = azurerm_log_analytics_workspace.test.workspace_id + workspace_region = azurerm_log_analytics_workspace.test.location + workspace_resource_id = azurerm_log_analytics_workspace.test.id + } +} +`, r.prerequisites(data), data.RandomInteger, data.RandomInteger) + } return fmt.Sprintf(` %s @@ -517,6 +656,42 @@ resource "azurerm_network_watcher_flow_log" "test" { } func (r NetworkWatcherFlowLogResource) TrafficAnalyticsUpdateInterval(data acceptance.TestData) string { + if !features.FivePointOhBeta() { + return fmt.Sprintf(` +%s + +resource "azurerm_log_analytics_workspace" "test" { + name = "acctestLAW-%d" + location = azurerm_resource_group.test.location + resource_group_name = azurerm_resource_group.test.name + sku = "PerGB2018" +} + +resource "azurerm_network_watcher_flow_log" "test" { + network_watcher_name = azurerm_network_watcher.test.name + resource_group_name = azurerm_resource_group.test.name + name = "flowlog-%d" + location = azurerm_network_watcher.test.location + + network_security_group_id = azurerm_network_security_group.test.id + storage_account_id = azurerm_storage_account.test.id + enabled = true + + retention_policy { + enabled = true + days = 7 + } + + traffic_analytics { + enabled = true + workspace_id = azurerm_log_analytics_workspace.test.workspace_id + workspace_region = azurerm_log_analytics_workspace.test.location + workspace_resource_id = azurerm_log_analytics_workspace.test.id + interval_in_minutes = 10 + } +} +`, r.prerequisites(data), data.RandomInteger, data.RandomInteger) + } return fmt.Sprintf(` %s @@ -554,6 +729,43 @@ resource "azurerm_network_watcher_flow_log" "test" { } func (r NetworkWatcherFlowLogResource) TrafficAnalyticsDisabledConfig(data acceptance.TestData) string { + if !features.FivePointOhBeta() { + + return fmt.Sprintf(` +%s + +resource "azurerm_log_analytics_workspace" "test" { + name = "acctestLAW-%d" + location = azurerm_resource_group.test.location + resource_group_name = azurerm_resource_group.test.name + sku = "PerGB2018" +} + +resource "azurerm_network_watcher_flow_log" "test" { + network_watcher_name = azurerm_network_watcher.test.name + resource_group_name = azurerm_resource_group.test.name + name = "flowlog-%d" + location = azurerm_network_watcher.test.location + + network_security_group_id = azurerm_network_security_group.test.id + storage_account_id = azurerm_storage_account.test.id + enabled = true + + retention_policy { + enabled = true + days = 7 + } + + traffic_analytics { + enabled = false + workspace_id = azurerm_log_analytics_workspace.test.workspace_id + workspace_region = azurerm_log_analytics_workspace.test.location + workspace_resource_id = azurerm_log_analytics_workspace.test.id + } +} +`, r.prerequisites(data), data.RandomInteger, data.RandomInteger) + } + return fmt.Sprintf(` %s @@ -590,6 +802,44 @@ resource "azurerm_network_watcher_flow_log" "test" { } func (r NetworkWatcherFlowLogResource) versionConfig(data acceptance.TestData, version int) string { + if !features.FivePointOhBeta() { + + return fmt.Sprintf(` +%s + +resource "azurerm_log_analytics_workspace" "test" { + name = "acctestLAW-%d" + location = azurerm_resource_group.test.location + resource_group_name = azurerm_resource_group.test.name + sku = "PerGB2018" +} + +resource "azurerm_network_watcher_flow_log" "test" { + network_watcher_name = azurerm_network_watcher.test.name + resource_group_name = azurerm_resource_group.test.name + name = "flowlog-%d" + location = azurerm_network_watcher.test.location + + network_security_group_id = azurerm_network_security_group.test.id + storage_account_id = azurerm_storage_account.test.id + enabled = true + version = %d + + retention_policy { + enabled = true + days = 7 + } + + traffic_analytics { + enabled = true + workspace_id = azurerm_log_analytics_workspace.test.workspace_id + workspace_region = azurerm_log_analytics_workspace.test.location + workspace_resource_id = azurerm_log_analytics_workspace.test.id + } +} +`, r.prerequisites(data), data.RandomInteger, data.RandomInteger, version) + } + return fmt.Sprintf(` %s @@ -627,6 +877,28 @@ resource "azurerm_network_watcher_flow_log" "test" { } func (r NetworkWatcherFlowLogResource) location(data acceptance.TestData) string { + if !features.FivePointOhBeta() { + + return fmt.Sprintf(` +%s + +resource "azurerm_network_watcher_flow_log" "test" { + network_watcher_name = azurerm_network_watcher.test.name + resource_group_name = azurerm_resource_group.test.name + name = "flowlog-%d" + location = azurerm_resource_group.test.location + + network_security_group_id = azurerm_network_security_group.test.id + storage_account_id = azurerm_storage_account.test.id + enabled = true + + retention_policy { + enabled = false + days = 0 + } +} +`, r.prerequisites(data), data.RandomInteger) + } return fmt.Sprintf(` %s @@ -649,6 +921,31 @@ resource "azurerm_network_watcher_flow_log" "test" { } func (r NetworkWatcherFlowLogResource) tags(data acceptance.TestData, v string) string { + if !features.FivePointOhBeta() { + + return fmt.Sprintf(` +%s + +resource "azurerm_network_watcher_flow_log" "test" { + network_watcher_name = azurerm_network_watcher.test.name + resource_group_name = azurerm_resource_group.test.name + name = "flowlog-%d" + + network_security_group_id = azurerm_network_security_group.test.id + storage_account_id = azurerm_storage_account.test.id + enabled = true + + retention_policy { + enabled = false + days = 0 + } + + tags = { + env = "%s" + } +} +`, r.prerequisites(data), data.RandomInteger, v) + } return fmt.Sprintf(` %s From 65c9535600ebfbee333d6b98e4c86c35fdb0b107 Mon Sep 17 00:00:00 2001 From: Vladimir Lazarenko Date: Thu, 14 Nov 2024 19:50:05 +0100 Subject: [PATCH 12/17] terrafmt --- .../network_watcher_flow_log_resource_test.go | 46 +++++++++---------- ...age_account_queue_properties.html.markdown | 2 +- ...orage_account_static_website.html.markdown | 2 +- 3 files changed, 25 insertions(+), 25 deletions(-) diff --git a/internal/services/network/network_watcher_flow_log_resource_test.go b/internal/services/network/network_watcher_flow_log_resource_test.go index a6e75447df56..e119fb9525d2 100644 --- a/internal/services/network/network_watcher_flow_log_resource_test.go +++ b/internal/services/network/network_watcher_flow_log_resource_test.go @@ -359,8 +359,8 @@ resource "azurerm_network_watcher_flow_log" "test" { name = "flowlog-%d" network_security_group_id = azurerm_virtual_network.test.id - storage_account_id = azurerm_storage_account.test.id - enabled = true + storage_account_id = azurerm_storage_account.test.id + enabled = true retention_policy { enabled = false @@ -407,8 +407,8 @@ resource "azurerm_network_watcher_flow_log" "import" { name = azurerm_network_watcher_flow_log.test.name network_security_group_id = azurerm_network_watcher_flow_log.test.target_resource_id - storage_account_id = azurerm_network_watcher_flow_log.test.storage_account_id - enabled = azurerm_network_watcher_flow_log.test.enabled + storage_account_id = azurerm_network_watcher_flow_log.test.storage_account_id + enabled = azurerm_network_watcher_flow_log.test.enabled retention_policy { enabled = false @@ -448,8 +448,8 @@ resource "azurerm_network_watcher_flow_log" "test" { name = "flowlog-%d" network_security_group_id = azurerm_network_security_group.test.id - storage_account_id = azurerm_storage_account.test.id - enabled = true + storage_account_id = azurerm_storage_account.test.id + enabled = true retention_policy { enabled = true @@ -500,8 +500,8 @@ resource "azurerm_network_watcher_flow_log" "test" { name = "flowlog-%d" network_security_group_id = azurerm_network_security_group.test.id - storage_account_id = azurerm_storage_account.testb.id - enabled = true + storage_account_id = azurerm_storage_account.testb.id + enabled = true retention_policy { enabled = true @@ -553,8 +553,8 @@ resource "azurerm_network_watcher_flow_log" "test" { location = azurerm_network_watcher.test.location network_security_group_id = azurerm_network_security_group.test.id - storage_account_id = azurerm_storage_account.test.id - enabled = false + storage_account_id = azurerm_storage_account.test.id + enabled = false retention_policy { enabled = true @@ -603,8 +603,8 @@ resource "azurerm_network_watcher_flow_log" "test" { location = azurerm_network_watcher.test.location network_security_group_id = azurerm_network_security_group.test.id - storage_account_id = azurerm_storage_account.test.id - enabled = true + storage_account_id = azurerm_storage_account.test.id + enabled = true retention_policy { enabled = true @@ -674,8 +674,8 @@ resource "azurerm_network_watcher_flow_log" "test" { location = azurerm_network_watcher.test.location network_security_group_id = azurerm_network_security_group.test.id - storage_account_id = azurerm_storage_account.test.id - enabled = true + storage_account_id = azurerm_storage_account.test.id + enabled = true retention_policy { enabled = true @@ -748,8 +748,8 @@ resource "azurerm_network_watcher_flow_log" "test" { location = azurerm_network_watcher.test.location network_security_group_id = azurerm_network_security_group.test.id - storage_account_id = azurerm_storage_account.test.id - enabled = true + storage_account_id = azurerm_storage_account.test.id + enabled = true retention_policy { enabled = true @@ -821,9 +821,9 @@ resource "azurerm_network_watcher_flow_log" "test" { location = azurerm_network_watcher.test.location network_security_group_id = azurerm_network_security_group.test.id - storage_account_id = azurerm_storage_account.test.id - enabled = true - version = %d + storage_account_id = azurerm_storage_account.test.id + enabled = true + version = %d retention_policy { enabled = true @@ -889,8 +889,8 @@ resource "azurerm_network_watcher_flow_log" "test" { location = azurerm_resource_group.test.location network_security_group_id = azurerm_network_security_group.test.id - storage_account_id = azurerm_storage_account.test.id - enabled = true + storage_account_id = azurerm_storage_account.test.id + enabled = true retention_policy { enabled = false @@ -932,8 +932,8 @@ resource "azurerm_network_watcher_flow_log" "test" { name = "flowlog-%d" network_security_group_id = azurerm_network_security_group.test.id - storage_account_id = azurerm_storage_account.test.id - enabled = true + storage_account_id = azurerm_storage_account.test.id + enabled = true retention_policy { enabled = false diff --git a/website/docs/r/storage_account_queue_properties.html.markdown b/website/docs/r/storage_account_queue_properties.html.markdown index d149b95a0274..a5ed5768a8b0 100644 --- a/website/docs/r/storage_account_queue_properties.html.markdown +++ b/website/docs/r/storage_account_queue_properties.html.markdown @@ -141,4 +141,4 @@ Storage Account Queue Properties can be imported using the `resource id`, e.g. ```shell terraform import azurerm_storage_account_queue_properties.queueprops /subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/myresourcegroup/providers/Microsoft.Storage/storageAccounts/myaccount -``` \ No newline at end of file +``` diff --git a/website/docs/r/storage_account_static_website.html.markdown b/website/docs/r/storage_account_static_website.html.markdown index 8e654ea4aadf..435517541ab2 100644 --- a/website/docs/r/storage_account_static_website.html.markdown +++ b/website/docs/r/storage_account_static_website.html.markdown @@ -63,4 +63,4 @@ Storage Account Static Websites can be imported using the `resource id`, e.g. ```shell terraform import azurerm_storage_account_static_website.mysite /subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/myresourcegroup/providers/Microsoft.Storage/storageAccounts/myaccount -``` \ No newline at end of file +``` From 120cd1e9c80fdcdecb7a489db362ff7411566a50 Mon Sep 17 00:00:00 2001 From: Vladimir Lazarenko Date: Thu, 14 Nov 2024 19:53:37 +0100 Subject: [PATCH 13/17] Remove test ref --- internal/services/network/network_watcher_resource_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/services/network/network_watcher_resource_test.go b/internal/services/network/network_watcher_resource_test.go index be77d047e115..01f4ec4eba96 100644 --- a/internal/services/network/network_watcher_resource_test.go +++ b/internal/services/network/network_watcher_resource_test.go @@ -79,7 +79,6 @@ func TestAccNetworkWatcher(t *testing.T) { "FlowLog": { "basic": testAccNetworkWatcherFlowLog_basic, "basicWithVirtualNetwork": testAccNetworkWatcherFlowLog_basicWithVirtualNetwork, - "basicOldNSGProperty": testAccNetworkWatcherFlowLog_basicOldNSGProperty, "requiresImport": testAccNetworkWatcherFlowLog_requiresImport, "disabled": testAccNetworkWatcherFlowLog_disabled, "reenabled": testAccNetworkWatcherFlowLog_reenabled, From 6350d3a269f3457e3026e7b462ebd29b8cd7bc6f Mon Sep 17 00:00:00 2001 From: Vladimir Lazarenko Date: Thu, 14 Nov 2024 20:01:54 +0100 Subject: [PATCH 14/17] lint --- .../network/network_watcher_flow_log_resource_test.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/internal/services/network/network_watcher_flow_log_resource_test.go b/internal/services/network/network_watcher_flow_log_resource_test.go index e119fb9525d2..4b08c3f0b8f2 100644 --- a/internal/services/network/network_watcher_flow_log_resource_test.go +++ b/internal/services/network/network_watcher_flow_log_resource_test.go @@ -730,7 +730,6 @@ resource "azurerm_network_watcher_flow_log" "test" { func (r NetworkWatcherFlowLogResource) TrafficAnalyticsDisabledConfig(data acceptance.TestData) string { if !features.FivePointOhBeta() { - return fmt.Sprintf(` %s @@ -803,7 +802,6 @@ resource "azurerm_network_watcher_flow_log" "test" { func (r NetworkWatcherFlowLogResource) versionConfig(data acceptance.TestData, version int) string { if !features.FivePointOhBeta() { - return fmt.Sprintf(` %s @@ -839,7 +837,6 @@ resource "azurerm_network_watcher_flow_log" "test" { } `, r.prerequisites(data), data.RandomInteger, data.RandomInteger, version) } - return fmt.Sprintf(` %s @@ -878,7 +875,6 @@ resource "azurerm_network_watcher_flow_log" "test" { func (r NetworkWatcherFlowLogResource) location(data acceptance.TestData) string { if !features.FivePointOhBeta() { - return fmt.Sprintf(` %s @@ -922,7 +918,6 @@ resource "azurerm_network_watcher_flow_log" "test" { func (r NetworkWatcherFlowLogResource) tags(data acceptance.TestData, v string) string { if !features.FivePointOhBeta() { - return fmt.Sprintf(` %s From 6c545b62b04ae49e10d905e245249cd65150f98f Mon Sep 17 00:00:00 2001 From: Vladimir Lazarenko Date: Thu, 14 Nov 2024 21:03:56 +0100 Subject: [PATCH 15/17] Fixup test --- .../services/network/network_watcher_flow_log_resource_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/services/network/network_watcher_flow_log_resource_test.go b/internal/services/network/network_watcher_flow_log_resource_test.go index 4b08c3f0b8f2..879e94a958c0 100644 --- a/internal/services/network/network_watcher_flow_log_resource_test.go +++ b/internal/services/network/network_watcher_flow_log_resource_test.go @@ -358,7 +358,7 @@ resource "azurerm_network_watcher_flow_log" "test" { resource_group_name = azurerm_resource_group.test.name name = "flowlog-%d" - network_security_group_id = azurerm_virtual_network.test.id + target_resource_id = azurerm_virtual_network.test.id storage_account_id = azurerm_storage_account.test.id enabled = true From 1ca0e4adfd2ae0ff45069c318033b6272d48cd93 Mon Sep 17 00:00:00 2001 From: Vladimir Lazarenko Date: Thu, 14 Nov 2024 21:29:30 +0100 Subject: [PATCH 16/17] terrafmt --- .../network/network_watcher_flow_log_resource_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/services/network/network_watcher_flow_log_resource_test.go b/internal/services/network/network_watcher_flow_log_resource_test.go index 879e94a958c0..8fda8ef2ee43 100644 --- a/internal/services/network/network_watcher_flow_log_resource_test.go +++ b/internal/services/network/network_watcher_flow_log_resource_test.go @@ -358,9 +358,9 @@ resource "azurerm_network_watcher_flow_log" "test" { resource_group_name = azurerm_resource_group.test.name name = "flowlog-%d" - target_resource_id = azurerm_virtual_network.test.id - storage_account_id = azurerm_storage_account.test.id - enabled = true + target_resource_id = azurerm_virtual_network.test.id + storage_account_id = azurerm_storage_account.test.id + enabled = true retention_policy { enabled = false From 1bc777ffb5db476ff9938185fbc54c585a54d0f0 Mon Sep 17 00:00:00 2001 From: Vladimir Lazarenko Date: Wed, 20 Nov 2024 18:30:03 +0100 Subject: [PATCH 17/17] Address review feedback --- .../services/network/network_watcher_flow_log_resource.go | 8 +++++--- website/docs/5.0-upgrade-guide.html.markdown | 5 +++++ 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/internal/services/network/network_watcher_flow_log_resource.go b/internal/services/network/network_watcher_flow_log_resource.go index a4502c3b080c..cf755d3105c9 100644 --- a/internal/services/network/network_watcher_flow_log_resource.go +++ b/internal/services/network/network_watcher_flow_log_resource.go @@ -179,7 +179,6 @@ func resourceNetworkWatcherFlowLog() *pluginsdk.Resource { resource.Schema["network_security_group_id"] = &pluginsdk.Schema{ Type: pluginsdk.TypeString, Optional: true, - ForceNew: true, Computed: true, ValidateFunc: networksecuritygroups.ValidateNetworkSecurityGroupID, Deprecated: "The property `network_security_group_id` has been superseded by `target_resource_id` and will be removed in version 5.0 of the AzureRM Provider.", @@ -188,7 +187,8 @@ func resourceNetworkWatcherFlowLog() *pluginsdk.Resource { resource.Schema["target_resource_id"].Required = false resource.Schema["target_resource_id"].Optional = true resource.Schema["target_resource_id"].Computed = true - resource.Schema["target_resource_id"].ConflictsWith = []string{"network_security_group_id"} + resource.Schema["target_resource_id"].ForceNew = false + resource.Schema["target_resource_id"].ExactlyOneOf = []string{"network_security_group_id", "target_resource_id"} } return resource @@ -416,13 +416,15 @@ func resourceNetworkWatcherFlowLogRead(d *pluginsdk.ResourceData, meta interface } targetResourceId := props.TargetResourceId + targetIsNSG := false if nsgId, err := networksecuritygroups.ParseNetworkSecurityGroupIDInsensitively(props.TargetResourceId); err == nil { targetResourceId = nsgId.ID() + targetIsNSG = true } else if vnetId, err := commonids.ParseVirtualNetworkIDInsensitively(props.TargetResourceId); err == nil { targetResourceId = vnetId.ID() } - if !features.FivePointOhBeta() { + if !features.FivePointOhBeta() && targetIsNSG { d.Set("network_security_group_id", targetResourceId) } diff --git a/website/docs/5.0-upgrade-guide.html.markdown b/website/docs/5.0-upgrade-guide.html.markdown index 7ff47e2f1c62..1149e6db4926 100644 --- a/website/docs/5.0-upgrade-guide.html.markdown +++ b/website/docs/5.0-upgrade-guide.html.markdown @@ -79,6 +79,11 @@ Please follow the format in the example below for listing breaking changes in re ### `azurerm_cdn_frontdoor_custom_domain` * The `tls.minimum_tls_version` property no longer accepts `TLS10` as a value. + +## `azurerm_network_watcher_flow_log` + +* The deprecated `network_security_group_id` property has been removed in favour of the `target_resource_id` property. + ## Breaking Changes in Data Sources