From 8ef52d3b1cb7266ce4f9b41847de58ed2447d3e1 Mon Sep 17 00:00:00 2001 From: rob Date: Tue, 10 Sep 2019 11:54:08 +0200 Subject: [PATCH 1/7] add virtual_network_subnet_id to network rules --- azurerm/resource_arm_container_registry.go | 58 +++++++++++++++++++--- 1 file changed, 51 insertions(+), 7 deletions(-) diff --git a/azurerm/resource_arm_container_registry.go b/azurerm/resource_arm_container_registry.go index 7e3596a7cd3d..c207ca0983a1 100644 --- a/azurerm/resource_arm_container_registry.go +++ b/azurerm/resource_arm_container_registry.go @@ -155,6 +155,27 @@ func resourceArmContainerRegistry() *schema.Resource { }, }, }, + + "virtual_network_subnet_id": { + Type: schema.TypeSet, + Optional: true, + ConfigMode: schema.SchemaConfigModeAttr, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "action": { + Type: schema.TypeString, + Required: true, + ValidateFunc: validation.StringInSlice([]string{ + string(containerregistry.Allow), + }, false), + }, + "id": { + Type: schema.TypeString, + Required: true, + }, + }, + }, + }, }, }, }, @@ -574,16 +595,28 @@ func expandNetworkRuleSet(profiles []interface{}) *containerregistry.NetworkRule ipRules := make([]containerregistry.IPRule, 0) for _, ipRuleInterface := range ipRuleConfigs { config := ipRuleInterface.(map[string]interface{}) - ipRules = - append(ipRules, containerregistry.IPRule{ - Action: containerregistry.Action(config["action"].(string)), - IPAddressOrRange: utils.String(config["ip_range"].(string)), - }) + newIpRule := containerregistry.IPRule{ + Action: containerregistry.Action(config["action"].(string)), + IPAddressOrRange: utils.String(config["ip_range"].(string)), + } + ipRules = append(ipRules, newIpRule) + } + + networkRuleConfigs := profile["virtual_network_subnet_id"].(*schema.Set).List() + virtualNetworkRules := make([]containerregistry.VirtualNetworkRule, 0) + for _, networkRuleInterface := range networkRuleConfigs { + config := networkRuleInterface.(map[string]interface{}) + newVirtualNetworkRule := containerregistry.VirtualNetworkRule{ + Action: containerregistry.Action(config["action"].(string)), + VirtualNetworkResourceID: utils.String(config["id"].(string)), + } + virtualNetworkRules = append(virtualNetworkRules, newVirtualNetworkRule) } networkRuleSet := containerregistry.NetworkRuleSet{ - DefaultAction: containerregistry.DefaultAction(profile["default_action"].(string)), - IPRules: &ipRules, + DefaultAction: containerregistry.DefaultAction(profile["default_action"].(string)), + IPRules: &ipRules, + VirtualNetworkRules: &virtualNetworkRules, } return &networkRuleSet } @@ -613,5 +646,16 @@ func flattenNetworkRuleSet(networkRuleSet *containerregistry.NetworkRuleSet) []i values["ip_rule"] = ipRules + virtualNetworkRules := make([]interface{}, 0) + for _, virtualNetworkRule := range *networkRuleSet.VirtualNetworkRules { + value := make(map[string]interface{}) + value["action"] = string(virtualNetworkRule.Action) + + value["id"] = virtualNetworkRule.VirtualNetworkResourceID + virtualNetworkRules = append(virtualNetworkRules, value) + } + + values["virtual_network_subnet_id"] = virtualNetworkRules + return []interface{}{values} } From 4ad47dab4e57f40de43027bb53b9454fa35477ce Mon Sep 17 00:00:00 2001 From: rob Date: Tue, 10 Sep 2019 13:31:09 +0200 Subject: [PATCH 2/7] add test for vnet ids --- .../resource_arm_container_registry_test.go | 73 ++++++++++++++++++- 1 file changed, 71 insertions(+), 2 deletions(-) diff --git a/azurerm/resource_arm_container_registry_test.go b/azurerm/resource_arm_container_registry_test.go index 6cab70951069..6e1c5d348464 100644 --- a/azurerm/resource_arm_container_registry_test.go +++ b/azurerm/resource_arm_container_registry_test.go @@ -333,7 +333,7 @@ func TestAccAzureRMContainerRegistry_geoReplication(t *testing.T) { }) } -func TestAccAzureRMContainerRegistry_networkAccessProfile_ip(t *testing.T) { +func TestAccAzureRMContainerRegistry_networkAccessProfileIp(t *testing.T) { rn := "azurerm_container_registry.test" ri := tf.AccRandTimeInt() l := testLocation() @@ -360,7 +360,7 @@ func TestAccAzureRMContainerRegistry_networkAccessProfile_ip(t *testing.T) { }) } -func TestAccAzureRMContainerRegistry_networkAccessProfile_update(t *testing.T) { +func TestAccAzureRMContainerRegistry_networkAccessProfileIp_update(t *testing.T) { rn := "azurerm_container_registry.test" ri := tf.AccRandTimeInt() l := testLocation() @@ -399,6 +399,33 @@ func TestAccAzureRMContainerRegistry_networkAccessProfile_update(t *testing.T) { }) } +func TestAccAzureRMContainerRegistry_networkAccessProfileVnet(t *testing.T) { + rn := "azurerm_container_registry.test" + ri := tf.AccRandTimeInt() + l := testLocation() + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testCheckAzureRMContainerRegistryDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAzureRMContainerRegistry_networkAccessProfile_vnet(ri, l), + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMContainerRegistryExists(rn), + resource.TestCheckResourceAttr(rn, "network_rule_set.0.default_action", "Deny"), + resource.TestCheckResourceAttr(rn, "network_rule_set.0.virtual_network_subnet_id.#", "1"), + ), + }, + { + ResourceName: rn, + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + func testCheckAzureRMContainerRegistryDestroy(s *terraform.State) error { conn := testAccProvider.Meta().(*ArmClient).containers.RegistriesClient ctx := testAccProvider.Meta().(*ArmClient).StopContext @@ -665,3 +692,45 @@ resource "azurerm_container_registry" "test" { } `, rInt, location, sku) } + +func testAccAzureRMContainerRegistry_networkAccessProfile_vnet(rInt int, location string) string { + return fmt.Sprintf(` +resource "azurerm_resource_group" "test" { + name = "testAccRg-%[1]d" + location = "%[2]s" +} + +resource "azurerm_virtual_network" "test" { + name = "virtualNetwork1" + location = "${azurerm_resource_group.test.location}" + resource_group_name = "${azurerm_resource_group.test.name}" + address_space = ["10.0.0.0/16"] +} + +resource "azurerm_subnet" "test" { + name = "testsubnet" + resource_group_name = "${azurerm_resource_group.test.name}" + virtual_network_name = "${azurerm_virtual_network.test.name}" + address_prefix = "10.0.1.0/24" + + service_endpoints = ["Microsoft.ContainerRegistry"] +} + +resource "azurerm_container_registry" "test" { + name = "testAccCr%[1]d" + resource_group_name = "${azurerm_resource_group.test.name}" + location = "${azurerm_resource_group.test.location}" + sku = "Premium" + admin_enabled = false + + network_rule_set { + default_action = "Deny" + + virtual_network_subnet_id { + action = "Allow" + id = "${azurerm_subnet.test.id}" + } + } +} +`, rInt, location) +} From d25810854b48d84b315091e42d16af3b7e1b778b Mon Sep 17 00:00:00 2001 From: rob Date: Tue, 10 Sep 2019 13:34:29 +0200 Subject: [PATCH 3/7] add documentation for subnet rules --- website/docs/r/container_registry.html.markdown | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/website/docs/r/container_registry.html.markdown b/website/docs/r/container_registry.html.markdown index 0027a0f71f0e..80cfd3bbb2ab 100644 --- a/website/docs/r/container_registry.html.markdown +++ b/website/docs/r/container_registry.html.markdown @@ -62,6 +62,8 @@ The following arguments are supported: * `ip_rule` - (Optional) One or more `ip_rule` blocks as defined below. +* `virtual_network_subnet_id` - (Optional) One or more `virtual_network_subnet_id` blocks as defined below. + ~> **NOTE:** `network_rule_set ` is only supported with the `Premium` SDK at this time. `ip_rule` supports the following: @@ -70,6 +72,12 @@ The following arguments are supported: * `ip_range` - (Required) The CIDR block from which requests will match the rule. +`virtual_network_subnet_id` supports the following: + +* `action` - (Required) The behaviour for requests matching this rule. At this time the only supported value is `Allow` + +* `id` - (Required) The subnet id from which requests will match the rule. + --- ## Attributes Reference From 427239186a37381b343701b3abe57a6cc7207c56 Mon Sep 17 00:00:00 2001 From: rob Date: Fri, 13 Sep 2019 06:54:17 +0200 Subject: [PATCH 4/7] fix tests to be removed by dalek --- .../resource_arm_container_registry_test.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/azurerm/resource_arm_container_registry_test.go b/azurerm/resource_arm_container_registry_test.go index 6e1c5d348464..38df23a0ae45 100644 --- a/azurerm/resource_arm_container_registry_test.go +++ b/azurerm/resource_arm_container_registry_test.go @@ -524,7 +524,7 @@ func testCheckAzureRMContainerRegistryGeoreplications(resourceName string, sku s func testAccAzureRMContainerRegistry_basic_basic(rInt int, location string) string { return fmt.Sprintf(` resource "azurerm_resource_group" "test" { - name = "acctestRg-%d" + name = "acctestrg-%d" location = "%s" } @@ -544,7 +544,7 @@ resource "azurerm_container_registry" "test" { func testAccAzureRMContainerRegistry_basicManaged(rInt int, location string, sku string) string { return fmt.Sprintf(` resource "azurerm_resource_group" "test" { - name = "acctestRg-%d" + name = "acctestrg-%d" location = "%s" } @@ -575,7 +575,7 @@ resource "azurerm_container_registry" "import" { func testAccAzureRMContainerRegistry_complete(rInt int, location string) string { return fmt.Sprintf(` resource "azurerm_resource_group" "test" { - name = "acctestRg-%d" + name = "acctestrg-%d" location = "%s" } @@ -596,7 +596,7 @@ resource "azurerm_container_registry" "test" { func testAccAzureRMContainerRegistry_completeUpdated(rInt int, location string) string { return fmt.Sprintf(` resource "azurerm_resource_group" "test" { - name = "acctestRg-%d" + name = "acctestrg-%d" location = "%s" } @@ -617,7 +617,7 @@ resource "azurerm_container_registry" "test" { func testAccAzureRMContainerRegistry_geoReplication(rInt int, location string, sku string, georeplicationLocations string) string { return fmt.Sprintf(` resource "azurerm_resource_group" "test" { - name = "testAccRg-%d" + name = "acctestrg-%d" location = "%s" } @@ -634,7 +634,7 @@ resource "azurerm_container_registry" "test" { func testAccAzureRMContainerRegistry_geoReplicationUpdateWithNoLocation(rInt int, location string, sku string) string { return fmt.Sprintf(` resource "azurerm_resource_group" "test" { - name = "testAccRg-%d" + name = "acctestrg-%d" location = "%s" } @@ -650,7 +650,7 @@ resource "azurerm_container_registry" "test" { func testAccAzureRMContainerRegistry_geoReplicationUpdateWithNoLocation_basic(rInt int, location string) string { return fmt.Sprintf(` resource "azurerm_resource_group" "test" { - name = "testAccRg-%d" + name = "acctestrg-%d" location = "%s" } @@ -670,7 +670,7 @@ resource "azurerm_container_registry" "test" { func testAccAzureRMContainerRegistry_networkAccessProfile_ip(rInt int, location string, sku string) string { return fmt.Sprintf(` resource "azurerm_resource_group" "test" { - name = "testAccRg-%[1]d" + name = "acctestrg-%[1]d" location = "%[2]s" } @@ -696,7 +696,7 @@ resource "azurerm_container_registry" "test" { func testAccAzureRMContainerRegistry_networkAccessProfile_vnet(rInt int, location string) string { return fmt.Sprintf(` resource "azurerm_resource_group" "test" { - name = "testAccRg-%[1]d" + name = "acctestrg-%[1]d" location = "%[2]s" } From 5609c59d877a275e8ae871b84728abece96f1202 Mon Sep 17 00:00:00 2001 From: rob Date: Fri, 13 Sep 2019 07:09:02 +0200 Subject: [PATCH 5/7] rename vnet rule fields; adjust test and documentation; validate subnet_id --- azurerm/resource_arm_container_registry.go | 28 +++++++++++-------- .../resource_arm_container_registry_test.go | 8 +++--- .../docs/r/container_registry.html.markdown | 4 +-- 3 files changed, 22 insertions(+), 18 deletions(-) diff --git a/azurerm/resource_arm_container_registry.go b/azurerm/resource_arm_container_registry.go index c207ca0983a1..eee61f3c3c53 100644 --- a/azurerm/resource_arm_container_registry.go +++ b/azurerm/resource_arm_container_registry.go @@ -156,7 +156,7 @@ func resourceArmContainerRegistry() *schema.Resource { }, }, - "virtual_network_subnet_id": { + "virtual_network": { Type: schema.TypeSet, Optional: true, ConfigMode: schema.SchemaConfigModeAttr, @@ -169,9 +169,10 @@ func resourceArmContainerRegistry() *schema.Resource { string(containerregistry.Allow), }, false), }, - "id": { - Type: schema.TypeString, - Required: true, + "subnet_id": { + Type: schema.TypeString, + Required: true, + ValidateFunc: azure.ValidateResourceID, }, }, }, @@ -602,13 +603,13 @@ func expandNetworkRuleSet(profiles []interface{}) *containerregistry.NetworkRule ipRules = append(ipRules, newIpRule) } - networkRuleConfigs := profile["virtual_network_subnet_id"].(*schema.Set).List() + networkRuleConfigs := profile["virtual_network"].(*schema.Set).List() virtualNetworkRules := make([]containerregistry.VirtualNetworkRule, 0) for _, networkRuleInterface := range networkRuleConfigs { config := networkRuleInterface.(map[string]interface{}) newVirtualNetworkRule := containerregistry.VirtualNetworkRule{ Action: containerregistry.Action(config["action"].(string)), - VirtualNetworkResourceID: utils.String(config["id"].(string)), + VirtualNetworkResourceID: utils.String(config["subnet_id"].(string)), } virtualNetworkRules = append(virtualNetworkRules, newVirtualNetworkRule) } @@ -647,15 +648,18 @@ func flattenNetworkRuleSet(networkRuleSet *containerregistry.NetworkRuleSet) []i values["ip_rule"] = ipRules virtualNetworkRules := make([]interface{}, 0) - for _, virtualNetworkRule := range *networkRuleSet.VirtualNetworkRules { - value := make(map[string]interface{}) - value["action"] = string(virtualNetworkRule.Action) - value["id"] = virtualNetworkRule.VirtualNetworkResourceID - virtualNetworkRules = append(virtualNetworkRules, value) + if networkRuleSet.VirtualNetworkRules != nil { + for _, virtualNetworkRule := range *networkRuleSet.VirtualNetworkRules { + value := make(map[string]interface{}) + value["action"] = string(virtualNetworkRule.Action) + + value["subnet_id"] = virtualNetworkRule.VirtualNetworkResourceID + virtualNetworkRules = append(virtualNetworkRules, value) + } } - values["virtual_network_subnet_id"] = virtualNetworkRules + values["virtual_network"] = virtualNetworkRules return []interface{}{values} } diff --git a/azurerm/resource_arm_container_registry_test.go b/azurerm/resource_arm_container_registry_test.go index 38df23a0ae45..89e6ae07227e 100644 --- a/azurerm/resource_arm_container_registry_test.go +++ b/azurerm/resource_arm_container_registry_test.go @@ -414,7 +414,7 @@ func TestAccAzureRMContainerRegistry_networkAccessProfileVnet(t *testing.T) { Check: resource.ComposeTestCheckFunc( testCheckAzureRMContainerRegistryExists(rn), resource.TestCheckResourceAttr(rn, "network_rule_set.0.default_action", "Deny"), - resource.TestCheckResourceAttr(rn, "network_rule_set.0.virtual_network_subnet_id.#", "1"), + resource.TestCheckResourceAttr(rn, "network_rule_set.0.virtual_network.#", "1"), ), }, { @@ -726,9 +726,9 @@ resource "azurerm_container_registry" "test" { network_rule_set { default_action = "Deny" - virtual_network_subnet_id { - action = "Allow" - id = "${azurerm_subnet.test.id}" + virtual_network { + action = "Allow" + subnet_id = "${azurerm_subnet.test.id}" } } } diff --git a/website/docs/r/container_registry.html.markdown b/website/docs/r/container_registry.html.markdown index 80cfd3bbb2ab..bc94a44dfaf3 100644 --- a/website/docs/r/container_registry.html.markdown +++ b/website/docs/r/container_registry.html.markdown @@ -72,11 +72,11 @@ The following arguments are supported: * `ip_range` - (Required) The CIDR block from which requests will match the rule. -`virtual_network_subnet_id` supports the following: +`virtual_network` supports the following: * `action` - (Required) The behaviour for requests matching this rule. At this time the only supported value is `Allow` -* `id` - (Required) The subnet id from which requests will match the rule. +* `subnet_id` - (Required) The subnet id from which requests will match the rule. --- From ccda7fc3b67d84219590c64998d199c7b14fe413 Mon Sep 17 00:00:00 2001 From: Tom Harvey Date: Fri, 13 Sep 2019 07:20:38 +0200 Subject: [PATCH 6/7] virtual_network_subnet_id -> virtual_network --- website/docs/r/container_registry.html.markdown | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/website/docs/r/container_registry.html.markdown b/website/docs/r/container_registry.html.markdown index bc94a44dfaf3..2e61e7fc0d5e 100644 --- a/website/docs/r/container_registry.html.markdown +++ b/website/docs/r/container_registry.html.markdown @@ -62,9 +62,9 @@ The following arguments are supported: * `ip_rule` - (Optional) One or more `ip_rule` blocks as defined below. -* `virtual_network_subnet_id` - (Optional) One or more `virtual_network_subnet_id` blocks as defined below. +* `virtual_network` - (Optional) One or more `virtual_network` blocks as defined below. -~> **NOTE:** `network_rule_set ` is only supported with the `Premium` SDK at this time. +~> **NOTE:** `network_rule_set ` is only supported with the `Premium` SKU at this time. `ip_rule` supports the following: From abdc199a2f9bf7e42c70b178628db7b60aefbf2d Mon Sep 17 00:00:00 2001 From: tombuildsstuff Date: Fri, 13 Sep 2019 10:52:56 +0200 Subject: [PATCH 7/7] r/container_registry: removing the name check since import handles this --- azurerm/resource_arm_container_registry_test.go | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/azurerm/resource_arm_container_registry_test.go b/azurerm/resource_arm_container_registry_test.go index 89e6ae07227e..8ee190b4623d 100644 --- a/azurerm/resource_arm_container_registry_test.go +++ b/azurerm/resource_arm_container_registry_test.go @@ -263,9 +263,6 @@ func TestAccAzureRMContainerRegistry_geoReplication(t *testing.T) { skuPremium := "Premium" skuBasic := "Basic" - containerRegistryName := fmt.Sprintf("testacccr%d", ri) - resourceGroupName := fmt.Sprintf("testAccRg-%d", ri) - resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, @@ -273,10 +270,9 @@ func TestAccAzureRMContainerRegistry_geoReplication(t *testing.T) { Steps: []resource.TestStep{ // first config creates an ACR with locations { + // TODO: fix this to use dynamic locations Config: testAccAzureRMContainerRegistry_geoReplication(ri, testLocation(), skuPremium, `eastus", "westus`), Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr(dsn, "name", containerRegistryName), - resource.TestCheckResourceAttr(dsn, "resource_group_name", resourceGroupName), resource.TestCheckResourceAttr(dsn, "sku", skuPremium), resource.TestCheckResourceAttr(dsn, "georeplication_locations.#", "2"), testCheckAzureRMContainerRegistryExists(dsn), @@ -287,8 +283,6 @@ func TestAccAzureRMContainerRegistry_geoReplication(t *testing.T) { { Config: testAccAzureRMContainerRegistry_geoReplication(ri, testLocation(), skuPremium, `centralus", "eastus`), Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr(dsn, "name", containerRegistryName), - resource.TestCheckResourceAttr(dsn, "resource_group_name", resourceGroupName), resource.TestCheckResourceAttr(dsn, "sku", skuPremium), resource.TestCheckResourceAttr(dsn, "georeplication_locations.#", "2"), testCheckAzureRMContainerRegistryExists(dsn), @@ -299,8 +293,6 @@ func TestAccAzureRMContainerRegistry_geoReplication(t *testing.T) { { Config: testAccAzureRMContainerRegistry_geoReplicationUpdateWithNoLocation(ri, testLocation(), skuPremium), Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr(dsn, "name", containerRegistryName), - resource.TestCheckResourceAttr(dsn, "resource_group_name", resourceGroupName), resource.TestCheckResourceAttr(dsn, "sku", skuPremium), testCheckAzureRMContainerRegistryExists(dsn), testCheckAzureRMContainerRegistryGeoreplications(dsn, skuPremium, nil), @@ -310,8 +302,6 @@ func TestAccAzureRMContainerRegistry_geoReplication(t *testing.T) { { Config: testAccAzureRMContainerRegistry_geoReplication(ri, testLocation(), skuPremium, `eastus", "westus`), Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr(dsn, "name", containerRegistryName), - resource.TestCheckResourceAttr(dsn, "resource_group_name", resourceGroupName), resource.TestCheckResourceAttr(dsn, "sku", skuPremium), resource.TestCheckResourceAttr(dsn, "georeplication_locations.#", "2"), testCheckAzureRMContainerRegistryExists(dsn), @@ -322,8 +312,6 @@ func TestAccAzureRMContainerRegistry_geoReplication(t *testing.T) { { Config: testAccAzureRMContainerRegistry_geoReplicationUpdateWithNoLocation_basic(ri, testLocation()), Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr(dsn, "name", containerRegistryName), - resource.TestCheckResourceAttr(dsn, "resource_group_name", resourceGroupName), resource.TestCheckResourceAttr(dsn, "sku", skuBasic), testCheckAzureRMContainerRegistryExists(dsn), testCheckAzureRMContainerRegistryGeoreplications(dsn, skuBasic, nil),