Skip to content

Commit

Permalink
network_watcher_flow_log - truncate name in case length larger than 80 (
Browse files Browse the repository at this point in the history
hashicorp#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 hashicorp#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 hashicorp#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 hashicorp#12460 (blocking an enterprise customer).
  • Loading branch information
magodo authored and yupwei68 committed Jul 26, 2021
1 parent 5f26dae commit c0e7458
Show file tree
Hide file tree
Showing 5 changed files with 105 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand Down Expand Up @@ -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)
}
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
12 changes: 11 additions & 1 deletion azurerm/internal/services/network/parse/flow_log.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
@@ -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
}

0 comments on commit c0e7458

Please sign in to comment.