From c0e74585d187f6b6f6ff94ee564590fbaf620112 Mon Sep 17 00:00:00 2001 From: magodo Date: Tue, 13 Jul 2021 04:58:54 +0800 Subject: [PATCH] network_watcher_flow_log - truncate name in case length larger than 80 (#12533) This PR "silently" truncate the name of the network watcher flow log (which is now constructed by combining the resource group name and the NSG name) to be less than 80 in length, which is required by the flow log API. This might be needed since otherwise the users will have to tune the length of the resource group name or the NSG name, in order to make the flow log API happy, which is kind of weired. However, there are some further concerns: If the name pattern of the resource group name or the NSG name doesn't conform to the flow log pattern, there is nothing can be done in the provider code to work around that Before the refactoring in Refactor azurerm_network_watcher_flow_log and add supports for location and tags #11670, the configureFlowLog endpoint is used to create the flow log. The created flow log CAN has name longer than 80 in length. This means if we merge this PR, it will break the users who created a long name flow log prior to Refactor azurerm_network_watcher_flow_log and add supports for location and tags #11670, and wants to use the latest provider to import that resource. Once we are in v3, we can remove all these hairy code and expose the name property, adding any constraint (length, pattern) on the name. Fix #12460 (blocking an enterprise customer). --- .../network_watcher_flow_log_resource.go | 9 +++ .../network_watcher_flow_log_resource_test.go | 69 +++++++++++++++++++ .../network/network_watcher_resource_test.go | 1 + .../services/network/parse/flow_log.go | 12 +++- .../validate/network_watcher_flow_log_name.go | 15 ++++ 5 files changed, 105 insertions(+), 1 deletion(-) create mode 100644 azurerm/internal/services/network/validate/network_watcher_flow_log_name.go diff --git a/azurerm/internal/services/network/network_watcher_flow_log_resource.go b/azurerm/internal/services/network/network_watcher_flow_log_resource.go index 237fb624131b..37e0fb574429 100644 --- a/azurerm/internal/services/network/network_watcher_flow_log_resource.go +++ b/azurerm/internal/services/network/network_watcher_flow_log_resource.go @@ -47,6 +47,14 @@ func resourceNetworkWatcherFlowLog() *pluginsdk.Resource { "resource_group_name": azure.SchemaResourceGroupName(), + "name": { + Type: pluginsdk.TypeString, + Computed: true, + // TODO 3.0: Make this required, and remove computed. + //Required: true, + //ValidateFunc: validate.NetworkWatcherFlowLogName, + }, + "network_security_group_id": { Type: pluginsdk.TypeString, Required: true, @@ -250,6 +258,7 @@ func resourceNetworkWatcherFlowLogRead(d *pluginsdk.ResourceData, meta interface d.Set("resource_group_name", id.ResourceGroupName) d.Set("network_security_group_id", id.NetworkSecurityGroupID()) d.Set("location", location.NormalizeNilable(resp.Location)) + d.Set("name", resp.Name) if prop := resp.FlowLogPropertiesFormat; prop != nil { if err := d.Set("traffic_analytics", flattenAzureRmNetworkWatcherFlowLogTrafficAnalytics(prop.FlowAnalyticsConfiguration)); err != nil { diff --git a/azurerm/internal/services/network/network_watcher_flow_log_resource_test.go b/azurerm/internal/services/network/network_watcher_flow_log_resource_test.go index fa6d3dd76708..11db8c6a0753 100644 --- a/azurerm/internal/services/network/network_watcher_flow_log_resource_test.go +++ b/azurerm/internal/services/network/network_watcher_flow_log_resource_test.go @@ -270,6 +270,23 @@ func testAccNetworkWatcherFlowLog_trafficAnalytics(t *testing.T) { }) } +// TODO 3.0: remove this test as we will validate the length for the `name` property, rather than truncate the name for the users. +func testAccNetworkWatcherFlowLog_longName(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_network_watcher_flow_log", "test") + r := NetworkWatcherFlowLogResource{} + + data.ResourceSequentialTest(t, r, []acceptance.TestStep{ + { + Config: r.longName(data), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + check.That(data.ResourceName).Key("name").HasValue("Microsoft.NetworkacctestRG-watcher-01234567890123456789012345678901acctestNSG012"), + ), + }, + data.ImportStep(), + }) +} + func testAccNetworkWatcherFlowLog_version(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_network_watcher_flow_log", "test") r := NetworkWatcherFlowLogResource{} @@ -654,3 +671,55 @@ resource "azurerm_network_watcher_flow_log" "test" { } `, r.prerequisites(data), v) } + +func (r NetworkWatcherFlowLogResource) longName(data acceptance.TestData) string { + return fmt.Sprintf(` +provider "azurerm" { + features {} +} + +resource "azurerm_resource_group" "test" { + # 01234567890123456789012345678901234567890123456789 = 40 + name = "acctestRG-watcher-01234567890123456789012345678901" + location = "%s" +} + +resource "azurerm_network_security_group" "test" { + # 01234567890123456789012345678901234567890123456789 = 40 + name = "acctestNSG0123456789012345678901234567890123456789" + location = azurerm_resource_group.test.location + resource_group_name = azurerm_resource_group.test.name +} + +resource "azurerm_network_watcher" "test" { + name = "acctest-NW-%d" + location = azurerm_resource_group.test.location + resource_group_name = azurerm_resource_group.test.name +} + +resource "azurerm_storage_account" "test" { + name = "acctestsa%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" + enable_https_traffic_only = true +} + +resource "azurerm_network_watcher_flow_log" "test" { + network_watcher_name = azurerm_network_watcher.test.name + resource_group_name = azurerm_resource_group.test.name + + 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 + } +} +`, data.Locations.Primary, data.RandomInteger, data.RandomInteger%1000000) +} diff --git a/azurerm/internal/services/network/network_watcher_resource_test.go b/azurerm/internal/services/network/network_watcher_resource_test.go index 8f4d15f7d8d0..65891fcf01cc 100644 --- a/azurerm/internal/services/network/network_watcher_resource_test.go +++ b/azurerm/internal/services/network/network_watcher_resource_test.go @@ -74,6 +74,7 @@ func TestAccNetworkWatcher(t *testing.T) { "retentionPolicy": testAccNetworkWatcherFlowLog_retentionPolicy, "updateStorageAccount": testAccNetworkWatcherFlowLog_updateStorageAccount, "trafficAnalytics": testAccNetworkWatcherFlowLog_trafficAnalytics, + "long_name": testAccNetworkWatcherFlowLog_longName, "version": testAccNetworkWatcherFlowLog_version, "location": testAccNetworkWatcherFlowLog_location, "tags": testAccNetworkWatcherFlowLog_tags, diff --git a/azurerm/internal/services/network/parse/flow_log.go b/azurerm/internal/services/network/parse/flow_log.go index 6204fe8e0a8e..7f438bf68ffe 100644 --- a/azurerm/internal/services/network/parse/flow_log.go +++ b/azurerm/internal/services/network/parse/flow_log.go @@ -40,7 +40,17 @@ func (id FlowLogId) Name() string { // The flow log name generated by the "configureFlowLog" endpoint is in below format: // Microsoft.Network{nsg rg name}{nsg name} // We follow this rule to ensure backward compatibility. - return fmt.Sprintf("Microsoft.Network%s%s", id.nsgId.ResourceGroup, id.nsgId.Name) + name := fmt.Sprintf("Microsoft.Network%s%s", id.nsgId.ResourceGroup, id.nsgId.Name) + + // TODO 3.0: remove below code and generate the ID parser via go generate + // Check whether the name is Longer than 80, which is the maximum allowed length for the flow log dedicated endpoint. + // If exceeded, we will truncate the name. + // This is needed since we are constructing the name from resource group name and NSG name. It makes no sense to ask + // users to constraint those names in order to make the flow log work. Also the Portal does that behind the scenes. + if len(name) > 80 { + name = name[:80] + } + return name } func (id FlowLogId) NetworkSecurityGroupID() string { diff --git a/azurerm/internal/services/network/validate/network_watcher_flow_log_name.go b/azurerm/internal/services/network/validate/network_watcher_flow_log_name.go new file mode 100644 index 000000000000..8ffe8309e57f --- /dev/null +++ b/azurerm/internal/services/network/validate/network_watcher_flow_log_name.go @@ -0,0 +1,15 @@ +package validate + +import ( + "fmt" + "regexp" +) + +func NetworkWatcherFlowLogName(v interface{}, k string) (warnings []string, errors []error) { + value := v.(string) + if !regexp.MustCompile(`^[^\W_]([\w]{0,79}$|[\w]{0,78}[\w\-.]$)`).MatchString(value) { + errors = append(errors, fmt.Errorf("the name can be up to 80 characters long. It must begin with a word character, and it must end with a word character or with '_'. The name may contain word characters or '.', '-', '_'. %q: %q", k, value)) + } + + return warnings, errors +}