From 4b404031e797e56086f6e1dd43f2c1432f41b828 Mon Sep 17 00:00:00 2001 From: Tom Bamford Date: Mon, 21 Sep 2020 15:33:47 +0100 Subject: [PATCH 1/6] azurerm_ip_group new resource and data source --- .../services/network/client/client.go | 5 + azurerm/internal/services/network/ip_group.go | 33 +++ .../services/network/ip_group_data_source.go | 82 ++++++ .../services/network/ip_group_resource.go | 178 ++++++++++++ .../network_security_group_data_source.go | 1 + .../internal/services/network/registration.go | 2 + .../tests/ip_group_data_source_test.go | 71 +++++ .../network/tests/ip_group_resource_test.go | 265 ++++++++++++++++++ .../services/network/tests/ip_group_test.go | 60 ++++ website/azurerm.erb | 7 + website/docs/d/ip_group.html.markdown | 48 ++++ website/docs/r/ip_group.html.markdown | 71 +++++ 12 files changed, 823 insertions(+) create mode 100644 azurerm/internal/services/network/ip_group.go create mode 100644 azurerm/internal/services/network/ip_group_data_source.go create mode 100644 azurerm/internal/services/network/ip_group_resource.go create mode 100644 azurerm/internal/services/network/tests/ip_group_data_source_test.go create mode 100644 azurerm/internal/services/network/tests/ip_group_resource_test.go create mode 100644 azurerm/internal/services/network/tests/ip_group_test.go create mode 100644 website/docs/d/ip_group.html.markdown create mode 100644 website/docs/r/ip_group.html.markdown diff --git a/azurerm/internal/services/network/client/client.go b/azurerm/internal/services/network/client/client.go index 7b18cadda36a..621295da8f9f 100644 --- a/azurerm/internal/services/network/client/client.go +++ b/azurerm/internal/services/network/client/client.go @@ -19,6 +19,7 @@ type Client struct { ExpressRoutePeeringsClient *network.ExpressRouteCircuitPeeringsClient FirewallPolicyClient *network.FirewallPoliciesClient InterfacesClient *network.InterfacesClient + IPGroupsClient *network.IPGroupsClient LoadBalancersClient *networkLegacy.LoadBalancersClient LoadBalancerLoadBalancingRulesClient *networkLegacy.LoadBalancerLoadBalancingRulesClient LocalNetworkGatewaysClient *network.LocalNetworkGatewaysClient @@ -89,6 +90,9 @@ func NewClient(o *common.ClientOptions) *Client { InterfacesClient := network.NewInterfacesClientWithBaseURI(o.ResourceManagerEndpoint, o.SubscriptionId) o.ConfigureClient(&InterfacesClient.Client, o.ResourceManagerAuthorizer) + IpGroupsClient := network.NewIPGroupsClientWithBaseURI(o.ResourceManagerEndpoint, o.SubscriptionId) + o.ConfigureClient(&IpGroupsClient.Client, o.ResourceManagerAuthorizer) + LoadBalancersClient := networkLegacy.NewLoadBalancersClientWithBaseURI(o.ResourceManagerEndpoint, o.SubscriptionId) o.ConfigureClient(&LoadBalancersClient.Client, o.ResourceManagerAuthorizer) @@ -195,6 +199,7 @@ func NewClient(o *common.ClientOptions) *Client { ExpressRoutePeeringsClient: &ExpressRoutePeeringsClient, FirewallPolicyClient: &FirewallPolicyClient, InterfacesClient: &InterfacesClient, + IPGroupsClient: &IpGroupsClient, LoadBalancersClient: &LoadBalancersClient, LoadBalancerLoadBalancingRulesClient: &LoadBalancerLoadBalancingRulesClient, LocalNetworkGatewaysClient: &LocalNetworkGatewaysClient, diff --git a/azurerm/internal/services/network/ip_group.go b/azurerm/internal/services/network/ip_group.go new file mode 100644 index 000000000000..70019da79f46 --- /dev/null +++ b/azurerm/internal/services/network/ip_group.go @@ -0,0 +1,33 @@ +package network + +import ( + "fmt" + + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/azure" +) + +type IpGroupResourceID struct { + ResourceGroup string + Name string +} + +func ParseIpGroupID(input string) (*IpGroupResourceID, error) { + id, err := azure.ParseAzureResourceID(input) + if err != nil { + return nil, fmt.Errorf("[ERROR] Unable to parse IP Group ID %q: %+v", input, err) + } + + ipGroup := IpGroupResourceID{ + ResourceGroup: id.ResourceGroup, + } + + if ipGroup.Name, err = id.PopSegment("ipGroups"); err != nil { + return nil, err + } + + if err := id.ValidateNoEmptySegments(input); err != nil { + return nil, err + } + + return &ipGroup, nil +} diff --git a/azurerm/internal/services/network/ip_group_data_source.go b/azurerm/internal/services/network/ip_group_data_source.go new file mode 100644 index 000000000000..d148406c50ec --- /dev/null +++ b/azurerm/internal/services/network/ip_group_data_source.go @@ -0,0 +1,82 @@ +package network + +import ( + "fmt" + "time" + + "github.com/hashicorp/terraform-plugin-sdk/helper/schema" + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/azure" + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/clients" + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/tags" + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/timeouts" + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils" +) + +func dataSourceArmIpGroup() *schema.Resource { + return &schema.Resource{ + Read: dataSourceArmIpGroupRead, + + Timeouts: &schema.ResourceTimeout{ + Read: schema.DefaultTimeout(5 * time.Minute), + }, + + Schema: map[string]*schema.Schema{ + "name": { + Type: schema.TypeString, + Required: true, + }, + + "resource_group_name": azure.SchemaResourceGroupNameForDataSource(), + + "location": azure.SchemaLocationForDataSource(), + + "cidrs": { + Type: schema.TypeSet, + Computed: true, + Elem: &schema.Schema{Type: schema.TypeString}, + Set: schema.HashString, + }, + + "tags": tags.SchemaDataSource(), + }, + } +} + +func dataSourceArmIpGroupRead(d *schema.ResourceData, meta interface{}) error { + client := meta.(*clients.Client).Network.IPGroupsClient + ctx, cancel := timeouts.ForRead(meta.(*clients.Client).StopContext, d) + defer cancel() + + resourceGroup := d.Get("resource_group_name").(string) + name := d.Get("name").(string) + + resp, err := client.Get(ctx, resourceGroup, name, "") + if err != nil { + if utils.ResponseWasNotFound(resp.Response) { + return fmt.Errorf("IP Group %q (Resource Group %q) was not found", name, resourceGroup) + } + return fmt.Errorf("making Read request on IP Group %q (Resource Group %q): %+v", name, resourceGroup, err) + } + + if resp.ID == nil || *resp.ID == "" { + return fmt.Errorf("reading request on IP Group %q (Resource Group %q): %+v", name, resourceGroup, err) + } + d.SetId(*resp.ID) + + d.Set("name", resp.Name) + d.Set("resource_group_name", resourceGroup) + if location := resp.Location; location != nil { + d.Set("location", azure.NormalizeLocation(*location)) + } + + if props := resp.IPGroupPropertiesFormat; props != nil { + if props.IPAddresses == nil { + return fmt.Errorf("list of ipAddresses returned is nil") + } + if err := d.Set("cidrs", props.IPAddresses); err != nil { + return fmt.Errorf("setting `cidrs`: %+v", err) + } + } + + return tags.FlattenAndSet(d, resp.Tags) +} diff --git a/azurerm/internal/services/network/ip_group_resource.go b/azurerm/internal/services/network/ip_group_resource.go new file mode 100644 index 000000000000..15bbf478110e --- /dev/null +++ b/azurerm/internal/services/network/ip_group_resource.go @@ -0,0 +1,178 @@ +package network + +import ( + "fmt" + "time" + + "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2020-05-01/network" + "github.com/hashicorp/terraform-plugin-sdk/helper/schema" + "github.com/hashicorp/terraform-plugin-sdk/helper/validation" + + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/azure" + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/tf" + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/clients" + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/tags" + azSchema "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/tf/schema" + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/timeouts" + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils" +) + +func resourceArmIpGroup() *schema.Resource { + return &schema.Resource{ + Create: resourceArmIpGroupCreateUpdate, + Read: resourceArmIpGroupRead, + Update: resourceArmIpGroupCreateUpdate, + Delete: resourceArmIpGroupDelete, + + Importer: azSchema.ValidateResourceIDPriorToImport(func(id string) error { + _, err := ParseIpGroupID(id) + return err + }), + + Timeouts: &schema.ResourceTimeout{ + Create: schema.DefaultTimeout(30 * time.Minute), + Read: schema.DefaultTimeout(5 * time.Minute), + Update: schema.DefaultTimeout(30 * time.Minute), + Delete: schema.DefaultTimeout(30 * time.Minute), + }, + + Schema: map[string]*schema.Schema{ + "name": { + Type: schema.TypeString, + Required: true, + ForceNew: true, + }, + + "location": azure.SchemaLocation(), + + "resource_group_name": azure.SchemaResourceGroupName(), + + "cidrs": { + Type: schema.TypeSet, + Optional: true, + Elem: &schema.Schema{ + Type: schema.TypeString, + ValidateFunc: validation.StringIsNotEmpty, + }, + Set: schema.HashString, + }, + + "tags": tags.Schema(), + }, + } +} + +func resourceArmIpGroupCreateUpdate(d *schema.ResourceData, meta interface{}) error { + client := meta.(*clients.Client).Network.IPGroupsClient + ctx, cancel := timeouts.ForCreateUpdate(meta.(*clients.Client).StopContext, d) + defer cancel() + + name := d.Get("name").(string) + resGroup := d.Get("resource_group_name").(string) + + if d.IsNewResource() { + existing, err := client.Get(ctx, resGroup, name, "") + if err != nil { + if !utils.ResponseWasNotFound(existing.Response) { + return fmt.Errorf("checking for presence of existing IP Group %q (Resource Group %q): %s", name, resGroup, err) + } + } + + if existing.ID != nil && *existing.ID != "" { + return tf.ImportAsExistsError("azurerm_ip_group", *existing.ID) + } + } + + location := azure.NormalizeLocation(d.Get("location").(string)) + t := d.Get("tags").(map[string]interface{}) + ipAddresses := d.Get("cidrs").(*schema.Set).List() + + sg := network.IPGroup{ + Name: &name, + Location: &location, + IPGroupPropertiesFormat: &network.IPGroupPropertiesFormat{ + IPAddresses: utils.ExpandStringSlice(ipAddresses), + }, + Tags: tags.Expand(t), + } + + future, err := client.CreateOrUpdate(ctx, resGroup, name, sg) + if err != nil { + return fmt.Errorf("creating/updating IP Group %q (Resource Group %q): %+v", name, resGroup, err) + } + + if err = future.WaitForCompletionRef(ctx, client.Client); err != nil { + return fmt.Errorf("waiting for the completion of IP Group %q (Resource Group %q): %+v", name, resGroup, err) + } + + read, err := client.Get(ctx, resGroup, name, "") + if err != nil { + return err + } + if read.ID == nil { + return fmt.Errorf("cannot read IP Group %q (resource group %q) ID", name, resGroup) + } + + d.SetId(*read.ID) + + return resourceArmIpGroupRead(d, meta) +} + +func resourceArmIpGroupRead(d *schema.ResourceData, meta interface{}) error { + client := meta.(*clients.Client).Network.IPGroupsClient + ctx, cancel := timeouts.ForRead(meta.(*clients.Client).StopContext, d) + defer cancel() + + id, err := ParseIpGroupID(d.Id()) + if err != nil { + return err + } + + resp, err := client.Get(ctx, id.ResourceGroup, id.Name, "") + if err != nil { + if utils.ResponseWasNotFound(resp.Response) { + d.SetId("") + return nil + } + return fmt.Errorf("making Read request on IP Group %q (Resource Group %q): %+v", id.Name, id.ResourceGroup, err) + } + + d.Set("name", resp.Name) + d.Set("resource_group_name", id.ResourceGroup) + if location := resp.Location; location != nil { + d.Set("location", azure.NormalizeLocation(*location)) + } + + if props := resp.IPGroupPropertiesFormat; props != nil { + if props.IPAddresses == nil { + return fmt.Errorf("list of ipAddresses returned is nil") + } + if err := d.Set("cidrs", props.IPAddresses); err != nil { + return fmt.Errorf("setting `cidrs`: %+v", err) + } + } + + return tags.FlattenAndSet(d, resp.Tags) +} + +func resourceArmIpGroupDelete(d *schema.ResourceData, meta interface{}) error { + client := meta.(*clients.Client).Network.IPGroupsClient + ctx, cancel := timeouts.ForDelete(meta.(*clients.Client).StopContext, d) + defer cancel() + + id, err := ParseIpGroupID(d.Id()) + if err != nil { + return err + } + + future, err := client.Delete(ctx, id.ResourceGroup, id.Name) + if err != nil { + return fmt.Errorf("deleting IP Group %q (Resource Group %q): %+v", id.Name, id.ResourceGroup, err) + } + + if err = future.WaitForCompletionRef(ctx, client.Client); err != nil { + return fmt.Errorf("deleting IP Group %q (Resource Group %q): %+v", id.Name, id.ResourceGroup, err) + } + + return err +} diff --git a/azurerm/internal/services/network/network_security_group_data_source.go b/azurerm/internal/services/network/network_security_group_data_source.go index f4ab5881e981..bcae04ecba78 100644 --- a/azurerm/internal/services/network/network_security_group_data_source.go +++ b/azurerm/internal/services/network/network_security_group_data_source.go @@ -5,6 +5,7 @@ import ( "time" "github.com/hashicorp/terraform-plugin-sdk/helper/schema" + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/azure" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/clients" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/tags" diff --git a/azurerm/internal/services/network/registration.go b/azurerm/internal/services/network/registration.go index 97506263a3f5..316288e8303b 100644 --- a/azurerm/internal/services/network/registration.go +++ b/azurerm/internal/services/network/registration.go @@ -26,6 +26,7 @@ func (r Registration) SupportedDataSources() map[string]*schema.Resource { "azurerm_express_route_circuit": dataSourceArmExpressRouteCircuit(), "azurerm_firewall": dataSourceArmFirewall(), "azurerm_firewall_policy": dataSourceArmFirewallPolicy(), + "azurerm_ip_group": dataSourceArmIpGroup(), "azurerm_lb": dataSourceArmLoadBalancer(), "azurerm_lb_backend_address_pool": dataSourceArmLoadBalancerBackendAddressPool(), "azurerm_lb_rule": dataSourceArmLoadBalancerRule(), @@ -67,6 +68,7 @@ func (r Registration) SupportedResources() map[string]*schema.Resource { "azurerm_firewall_nat_rule_collection": resourceArmFirewallNatRuleCollection(), "azurerm_firewall_network_rule_collection": resourceArmFirewallNetworkRuleCollection(), "azurerm_firewall": resourceArmFirewall(), + "azurerm_ip_group": resourceArmIpGroup(), "azurerm_local_network_gateway": resourceArmLocalNetworkGateway(), "azurerm_lb_backend_address_pool": resourceArmLoadBalancerBackendAddressPool(), "azurerm_lb_nat_pool": resourceArmLoadBalancerNatPool(), diff --git a/azurerm/internal/services/network/tests/ip_group_data_source_test.go b/azurerm/internal/services/network/tests/ip_group_data_source_test.go new file mode 100644 index 000000000000..a38a58f30f87 --- /dev/null +++ b/azurerm/internal/services/network/tests/ip_group_data_source_test.go @@ -0,0 +1,71 @@ +package tests + +import ( + "fmt" + "testing" + + "github.com/hashicorp/terraform-plugin-sdk/helper/resource" + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/acceptance" +) + +func TestAccDataSourceAzureRMIPGroup_basic(t *testing.T) { + data := acceptance.BuildTestData(t, "data.azurerm_ip_group", "test") + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acceptance.PreCheck(t) }, + Providers: acceptance.SupportedProviders, + CheckDestroy: testCheckAzureRMIpGroupDestroy, + Steps: []resource.TestStep{ + { + Config: testAccDataSourceAzureRMIpGroup_basic(data), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttrSet(data.ResourceName, "location"), + resource.TestCheckResourceAttr(data.ResourceName, "cidrs.#", "0"), + resource.TestCheckResourceAttr(data.ResourceName, "tags.%", "0"), + ), + }, + }, + }) +} + +func TestAccDataSourceAzureRMIpGroup_complete(t *testing.T) { + data := acceptance.BuildTestData(t, "data.azurerm_ip_group", "test") + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acceptance.PreCheck(t) }, + Providers: acceptance.SupportedProviders, + CheckDestroy: testCheckAzureRMIpGroupDestroy, + Steps: []resource.TestStep{ + { + Config: testAccDataSourceAzureRMIpGroup_complete(data), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttrSet(data.ResourceName, "location"), + resource.TestCheckResourceAttr(data.ResourceName, "cidrs.#", "3"), + resource.TestCheckResourceAttr(data.ResourceName, "tags.%", "2"), + ), + }, + }, + }) +} + +func testAccDataSourceAzureRMIpGroup_basic(data acceptance.TestData) string { + return fmt.Sprintf(` +%s + +data "azurerm_ip_group" "test" { + name = azurerm_ip_group.test.name + resource_group_name = azurerm_resource_group.test.name +} +`, testAccAzureRMIpGroup_basic(data)) +} + +func testAccDataSourceAzureRMIpGroup_complete(data acceptance.TestData) string { + return fmt.Sprintf(` +%s + +data "azurerm_ip_group" "test" { + name = azurerm_ip_group.test.name + resource_group_name = azurerm_resource_group.test.name +} +`, testAccAzureRMIpGroup_complete(data)) +} diff --git a/azurerm/internal/services/network/tests/ip_group_resource_test.go b/azurerm/internal/services/network/tests/ip_group_resource_test.go new file mode 100644 index 000000000000..a2068c3ac9a3 --- /dev/null +++ b/azurerm/internal/services/network/tests/ip_group_resource_test.go @@ -0,0 +1,265 @@ +package tests + +import ( + "fmt" + "testing" + + "github.com/hashicorp/terraform-plugin-sdk/helper/resource" + "github.com/hashicorp/terraform-plugin-sdk/terraform" + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/acceptance" + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/clients" + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils" +) + +func TestAccAzureRMIpGroup_basic(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_ip_group", "test") + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acceptance.PreCheck(t) }, + Providers: acceptance.SupportedProviders, + CheckDestroy: testCheckAzureRMIpGroupDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAzureRMIpGroup_basic(data), + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMIpGroupExists(data.ResourceName), + ), + }, + data.ImportStep(), + }, + }) +} + +func TestAccAzureRMIpGroup_requiresImport(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_ip_group", "test") + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acceptance.PreCheck(t) }, + Providers: acceptance.SupportedProviders, + CheckDestroy: testCheckAzureRMIpGroupDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAzureRMIpGroup_basic(data), + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMIpGroupExists(data.ResourceName), + ), + }, + { + Config: testAccAzureRMIpGroup_requiresImport(data), + ExpectError: acceptance.RequiresImportError("azurerm_ip_group"), + }, + }, + }) +} + +func TestAccAzureRMIpGroup_complete(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_ip_group", "test") + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acceptance.PreCheck(t) }, + Providers: acceptance.SupportedProviders, + CheckDestroy: testCheckAzureRMIpGroupDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAzureRMIpGroup_complete(data), + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMIpGroupExists(data.ResourceName), + ), + }, + data.ImportStep(), + }, + }) +} + +func TestAccAzureRMIpGroup_update(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_ip_group", "test") + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acceptance.PreCheck(t) }, + Providers: acceptance.SupportedProviders, + CheckDestroy: testCheckAzureRMIpGroupDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAzureRMIpGroup_basic(data), + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMIpGroupExists(data.ResourceName), + resource.TestCheckResourceAttr(data.ResourceName, "cidrs.#", "0"), + resource.TestCheckResourceAttr(data.ResourceName, "tags.%", "0"), + ), + }, + data.ImportStep(), + { + Config: testAccAzureRMIpGroup_complete(data), + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMIpGroupExists(data.ResourceName), + resource.TestCheckResourceAttr(data.ResourceName, "cidrs.#", "3"), + resource.TestCheckResourceAttr(data.ResourceName, "tags.%", "2"), + ), + }, + data.ImportStep(), + { + Config: testAccAzureRMIpGroup_completeUpdate(data), + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMIpGroupExists(data.ResourceName), + resource.TestCheckResourceAttr(data.ResourceName, "cidrs.#", "1"), + resource.TestCheckResourceAttr(data.ResourceName, "tags.%", "2"), + ), + }, + data.ImportStep(), + { + Config: testAccAzureRMIpGroup_complete(data), + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMIpGroupExists(data.ResourceName), + resource.TestCheckResourceAttr(data.ResourceName, "cidrs.#", "3"), + resource.TestCheckResourceAttr(data.ResourceName, "tags.%", "2"), + ), + }, + data.ImportStep(), + { + Config: testAccAzureRMIpGroup_basic(data), + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMIpGroupExists(data.ResourceName), + resource.TestCheckResourceAttr(data.ResourceName, "cidrs.#", "0"), + resource.TestCheckResourceAttr(data.ResourceName, "tags.%", "0"), + ), + }, + data.ImportStep(), + }, + }) +} + +func testCheckAzureRMIpGroupExists(resourceName string) resource.TestCheckFunc { + return func(s *terraform.State) error { + client := acceptance.AzureProvider.Meta().(*clients.Client).Network.IPGroupsClient + ctx := acceptance.AzureProvider.Meta().(*clients.Client).StopContext + + rs, ok := s.RootModule().Resources[resourceName] + if !ok { + return fmt.Errorf("Not found: %q", resourceName) + } + + ipGroupName := rs.Primary.Attributes["name"] + resourceGroup, hasResourceGroup := rs.Primary.Attributes["resource_group_name"] + if !hasResourceGroup { + return fmt.Errorf("Bad: no resource group found in state for IP Group: %q", ipGroupName) + } + + resp, err := client.Get(ctx, resourceGroup, ipGroupName, "") + if err != nil { + if utils.ResponseWasNotFound(resp.Response) { + return fmt.Errorf("Bad: IP Group %q (resource group: %q) does not exist", ipGroupName, resourceGroup) + } + + return fmt.Errorf("Bad: Get on IPGroupsClient: %+v", err) + } + + return nil + } +} + +func testCheckAzureRMIpGroupDestroy(s *terraform.State) error { + client := acceptance.AzureProvider.Meta().(*clients.Client).Network.IPGroupsClient + ctx := acceptance.AzureProvider.Meta().(*clients.Client).StopContext + + for _, rs := range s.RootModule().Resources { + if rs.Type != "azurerm_ip_group" { + continue + } + + name := rs.Primary.Attributes["name"] + resourceGroup := rs.Primary.Attributes["resource_group_name"] + + resp, err := client.Get(ctx, resourceGroup, name, "") + + if err != nil { + if utils.ResponseWasNotFound(resp.Response) { + return nil + } + return err + } + + return fmt.Errorf("IP Group still exists:\n%#v", resp.IPGroupPropertiesFormat) + } + + return nil +} + +func testAccAzureRMIpGroup_basic(data acceptance.TestData) string { + return fmt.Sprintf(` +provider "azurerm" { + features {} +} + +resource "azurerm_resource_group" "test" { + name = "acctestRG-%d" + location = "%s" +} + +resource "azurerm_ip_group" "test" { + name = "acceptanceTestIpGroup1" + location = azurerm_resource_group.test.location + resource_group_name = azurerm_resource_group.test.name +} +`, data.RandomInteger, data.Locations.Primary) +} + +func testAccAzureRMIpGroup_requiresImport(data acceptance.TestData) string { + template := testAccAzureRMIpGroup_basic(data) + return fmt.Sprintf(` +%s + +resource "azurerm_ip_group" "import" { + name = azurerm_ip_group.test.name + location = azurerm_ip_group.test.location + resource_group_name = azurerm_ip_group.test.resource_group_name +} +`, template) +} + +func testAccAzureRMIpGroup_complete(data acceptance.TestData) string { + return fmt.Sprintf(` +provider "azurerm" { + features {} +} + +resource "azurerm_resource_group" "test" { + name = "acctestRG-%d" + location = "%s" +} + +resource "azurerm_ip_group" "test" { + name = "acceptanceTestIpGroup1" + location = azurerm_resource_group.test.location + resource_group_name = azurerm_resource_group.test.name + + cidrs = ["192.168.0.1", "172.16.240.0/20", "10.48.0.0/12"] + + tags = { + environment = "Production" + cost_center = "MSFT" + } +} +`, data.RandomInteger, data.Locations.Primary) +} + +func testAccAzureRMIpGroup_completeUpdate(data acceptance.TestData) string { + return fmt.Sprintf(` +provider "azurerm" { + features {} +} + +resource "azurerm_resource_group" "test" { + name = "acctestRG-%d" + location = "%s" +} + +resource "azurerm_ip_group" "test" { + name = "acceptanceTestIpGroup1" + location = azurerm_resource_group.test.location + resource_group_name = azurerm_resource_group.test.name + + cidrs = ["172.16.240.0/20"] + + tags = { + environment = "Production" + cost_center = "MSFT" + } +} +`, data.RandomInteger, data.Locations.Primary) +} diff --git a/azurerm/internal/services/network/tests/ip_group_test.go b/azurerm/internal/services/network/tests/ip_group_test.go new file mode 100644 index 000000000000..fc038ea56c58 --- /dev/null +++ b/azurerm/internal/services/network/tests/ip_group_test.go @@ -0,0 +1,60 @@ +package tests + +import ( + "testing" + + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/network" +) + +func TestParseIpGroup(t *testing.T) { + testData := []struct { + Name string + Input string + Expected *network.IpGroupResourceID + }{ + { + Name: "Empty", + Input: "", + Expected: nil, + }, + { + Name: "No IP Groups Segment", + Input: "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/foo", + Expected: nil, + }, + { + Name: "No IP Groups Value", + Input: "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/foo/ipGroups/", + Expected: nil, + }, + { + Name: "Completed", + Input: "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/foo/ipGroups/example", + Expected: &network.IpGroupResourceID{ + Name: "example", + ResourceGroup: "foo", + }, + }, + } + + for _, v := range testData { + t.Logf("[DEBUG] Testing %q", v.Name) + + actual, err := network.ParseIpGroupID(v.Input) + if err != nil { + if v.Expected == nil { + continue + } + + t.Fatalf("Expected a value but got an error: %s", err) + } + + if actual.Name != v.Expected.Name { + t.Fatalf("Expected %q but got %q for Name", v.Expected.Name, actual.Name) + } + + if actual.ResourceGroup != v.Expected.ResourceGroup { + t.Fatalf("Expected %q but got %q for ResourceGroup", v.Expected.ResourceGroup, actual.ResourceGroup) + } + } +} diff --git a/website/azurerm.erb b/website/azurerm.erb index 5e1fd7e6de87..20189fe4e764 100644 --- a/website/azurerm.erb +++ b/website/azurerm.erb @@ -313,6 +313,10 @@ azurerm_image +
  • + azurerm_ip_group +
    • +
  • azurerm_key_vault
    • @@ -2313,6 +2317,9 @@ azurerm_firewall_policy +
  • + azurerm_ip_group +
  • azurerm_local_network_gateway diff --git a/website/docs/d/ip_group.html.markdown b/website/docs/d/ip_group.html.markdown new file mode 100644 index 000000000000..cb8ca5054e5b --- /dev/null +++ b/website/docs/d/ip_group.html.markdown @@ -0,0 +1,48 @@ +--- +subcategory: "Network" +layout: "azurerm" +page_title: "Azure Resource Manager: azurerm_ip_group" +description: |- + Gets information about an existing IP Group. +--- + +# Data Source: azurerm_ip_group + +Use this data source to access information about an existing IP Group. + +## Example Usage + +```hcl +data "azurerm_ip_group" "example" { + name = "example1-ipgroup" + resource_group_name = "example-rg" +} + +output "cidrs" { + value = data.azurerm_ip_group.example.cidrs +} +``` + +## Argument Reference + +* `name` - Specifies the Name of the IP Group. + +* `resource_group_name` - Specifies the Name of the Resource Group within which the IP Group exists + + +## Attributes Reference + +* `id` - The ID of the IP Group. + +* `location` - The supported Azure location where the resource exists. + +* `cidrs` - A list of CIDRs or IP addresses. + +* `tags` - A mapping of tags assigned to the resource. + + +## Timeouts + +The `timeouts` block allows you to specify [timeouts](https://www.terraform.io/docs/configuration/resources.html#timeouts) for certain actions: + +* `read` - (Defaults to 5 minutes) Used when retrieving the IP Group. diff --git a/website/docs/r/ip_group.html.markdown b/website/docs/r/ip_group.html.markdown new file mode 100644 index 000000000000..d43024b2f26b --- /dev/null +++ b/website/docs/r/ip_group.html.markdown @@ -0,0 +1,71 @@ +--- +subcategory: "Network" +layout: "azurerm" +page_title: "Azure Resource Manager: azurerm_ip_group" +description: |- + Manages an IP group which contains a list of CIDRs and/or IP addresses. + +--- + +# azurerm_ip_group + +Manages an IP group that contains a list of CIDRs and/or IP addresses. + +## Example Usage + +```hcl +resource "azurerm_resource_group" "example" { + name = "example-rg" + location = "westus" +} + +resource "azurerm_ip_group" "example" { + name = "example1-ipgroup" + location = azurerm_resource_group.example.location + resource_group_name = azurerm_resource_group.example.name + + cidrs = ["192.168.0.1", "172.16.240.0/20", "10.48.0.0/12"] + + tags = { + environment = "Production" + } +} +``` + +## Argument Reference + +The following arguments are supported: + +* `name` - (Required) Specifies the name of the IP group. Changing this forces a new resource to be created. + +* `resource_group_name` - (Required) The name of the resource group in which to create the IP group. Changing this forces a new resource to be created. + +* `location` - (Required) Specifies the supported Azure location where the resource exists. Changing this forces a new resource to be created. + +* `cidrs` - (Optional) A list of CIDRs or IP addresses. + +* `tags` - (Optional) A mapping of tags to assign to the resource. + + +## Attributes Reference + +The following attributes are exported: + +* `id` - The ID of the IP Group. + +## Timeouts + +The `timeouts` block allows you to specify [timeouts](https://www.terraform.io/docs/configuration/resources.html#timeouts) for certain actions: + +* `create` - (Defaults to 30 minutes) Used when creating the IP Group. +* `update` - (Defaults to 30 minutes) Used when updating the IP Group. +* `read` - (Defaults to 5 minutes) Used when retrieving the IP Group. +* `delete` - (Defaults to 30 minutes) Used when deleting the IP Group. + +## Import + +IP Groups can be imported using the `resource id`, e.g. + +```shell +terraform import azurerm_ip_group.ipgroup1 /subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/mygroup1/providers/Microsoft.Network/ipGroups/myIpGroup +``` From 4ad9da6ce66b781ad5c43b428e51b9927695ba51 Mon Sep 17 00:00:00 2001 From: Tom Bamford Date: Mon, 21 Sep 2020 22:29:17 +0100 Subject: [PATCH 2/6] azurerm_firewall_application_rule_collection: support source_ip_groups for rules --- ...ll_application_rule_collection_resource.go | 60 ++++++---- ...plication_rule_collection_resource_test.go | 104 ++++++++++++++++++ ..._application_rule_collection.html.markdown | 6 +- 3 files changed, 147 insertions(+), 23 deletions(-) diff --git a/azurerm/internal/services/network/firewall_application_rule_collection_resource.go b/azurerm/internal/services/network/firewall_application_rule_collection_resource.go index 09814707699f..ed496c1ea359 100644 --- a/azurerm/internal/services/network/firewall_application_rule_collection_resource.go +++ b/azurerm/internal/services/network/firewall_application_rule_collection_resource.go @@ -8,6 +8,7 @@ import ( "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2020-05-01/network" "github.com/hashicorp/terraform-plugin-sdk/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/helper/validation" + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/azure" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/set" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/tf" @@ -80,7 +81,13 @@ func resourceArmFirewallApplicationRuleCollection() *schema.Resource { }, "source_addresses": { Type: schema.TypeSet, - Required: true, + Optional: true, + Elem: &schema.Schema{Type: schema.TypeString}, + Set: schema.HashString, + }, + "source_ip_groups": { + Type: schema.TypeSet, + Optional: true, Elem: &schema.Schema{Type: schema.TypeString}, Set: schema.HashString, }, @@ -140,7 +147,7 @@ func resourceArmFirewallApplicationRuleCollectionCreateUpdate(d *schema.Resource resourceGroup := d.Get("resource_group_name").(string) applicationRules, err := expandArmFirewallApplicationRules(d.Get("rule").([]interface{})) if err != nil { - return fmt.Errorf("Error expanding Firewall Application Rules: %+v", err) + return fmt.Errorf("expanding Firewall Application Rules: %+v", err) } locks.ByName(firewallName, azureFirewallResourceName) @@ -148,16 +155,16 @@ func resourceArmFirewallApplicationRuleCollectionCreateUpdate(d *schema.Resource firewall, err := client.Get(ctx, resourceGroup, firewallName) if err != nil { - return fmt.Errorf("Error retrieving Firewall %q (Resource Group %q): %+v", firewallName, resourceGroup, err) + return fmt.Errorf("retrieving Firewall %q (Resource Group %q): %+v", firewallName, resourceGroup, err) } if firewall.AzureFirewallPropertiesFormat == nil { - return fmt.Errorf("Error retrieving Application Rule Collections (Firewall %q / Resource Group %q): `properties` was nil", firewallName, resourceGroup) + return fmt.Errorf("retrieving Application Rule Collections (Firewall %q / Resource Group %q): `properties` was nil", firewallName, resourceGroup) } props := *firewall.AzureFirewallPropertiesFormat if props.ApplicationRuleCollections == nil { - return fmt.Errorf("Error retrieving Application Rule Collections (Firewall %q / Resource Group %q): `properties.ApplicationRuleCollections` was nil", firewallName, resourceGroup) + return fmt.Errorf("retrieving Application Rule Collections (Firewall %q / Resource Group %q): `properties.ApplicationRuleCollections` was nil", firewallName, resourceGroup) } ruleCollections := *props.ApplicationRuleCollections @@ -169,7 +176,7 @@ func resourceArmFirewallApplicationRuleCollectionCreateUpdate(d *schema.Resource Type: network.AzureFirewallRCActionType(d.Get("action").(string)), }, Priority: utils.Int32(int32(priority)), - Rules: &applicationRules, + Rules: applicationRules, }, } @@ -189,7 +196,7 @@ func resourceArmFirewallApplicationRuleCollectionCreateUpdate(d *schema.Resource if !d.IsNewResource() { if index == -1 { - return fmt.Errorf("Error locating Application Rule Collection %q (Firewall %q / Resource Group %q)", name, firewallName, resourceGroup) + return fmt.Errorf("locating Application Rule Collection %q (Firewall %q / Resource Group %q)", name, firewallName, resourceGroup) } ruleCollections[index] = newRuleCollection @@ -205,16 +212,16 @@ func resourceArmFirewallApplicationRuleCollectionCreateUpdate(d *schema.Resource future, err := client.CreateOrUpdate(ctx, resourceGroup, firewallName, firewall) if err != nil { - return fmt.Errorf("Error creating/updating Application Rule Collection %q in Firewall %q (Resource Group %q): %+v", name, firewallName, resourceGroup, err) + return fmt.Errorf("creating/updating Application Rule Collection %q in Firewall %q (Resource Group %q): %+v", name, firewallName, resourceGroup, err) } if err = future.WaitForCompletionRef(ctx, client.Client); err != nil { - return fmt.Errorf("Error waiting for creation/update of Application Rule Collection %q of Firewall %q (Resource Group %q): %+v", name, firewallName, resourceGroup, err) + return fmt.Errorf("waiting for creation/update of Application Rule Collection %q of Firewall %q (Resource Group %q): %+v", name, firewallName, resourceGroup, err) } read, err := client.Get(ctx, resourceGroup, firewallName) if err != nil { - return fmt.Errorf("Error retrieving Firewall %q (Resource Group %q): %+v", firewallName, resourceGroup, err) + return fmt.Errorf("retrieving Firewall %q (Resource Group %q): %+v", firewallName, resourceGroup, err) } var collectionID string @@ -262,16 +269,16 @@ func resourceArmFirewallApplicationRuleCollectionRead(d *schema.ResourceData, me d.SetId("") return nil } - return fmt.Errorf("Error retrieving Azure Firewall %q (Resource Group %q): %+v", name, resourceGroup, err) + return fmt.Errorf("retrieving Azure Firewall %q (Resource Group %q): %+v", name, resourceGroup, err) } if read.AzureFirewallPropertiesFormat == nil { - return fmt.Errorf("Error retrieving Application Rule Collection %q (Firewall %q / Resource Group %q): `props` was nil", name, firewallName, resourceGroup) + return fmt.Errorf("retrieving Application Rule Collection %q (Firewall %q / Resource Group %q): `props` was nil", name, firewallName, resourceGroup) } props := *read.AzureFirewallPropertiesFormat if props.ApplicationRuleCollections == nil { - return fmt.Errorf("Error retrieving Application Rule Collection %q (Firewall %q / Resource Group %q): `props.ApplicationRuleCollections` was nil", name, firewallName, resourceGroup) + return fmt.Errorf("retrieving Application Rule Collection %q (Firewall %q / Resource Group %q): `props.ApplicationRuleCollections` was nil", name, firewallName, resourceGroup) } var rule *network.AzureFirewallApplicationRuleCollection @@ -307,7 +314,7 @@ func resourceArmFirewallApplicationRuleCollectionRead(d *schema.ResourceData, me flattenedRules := flattenFirewallApplicationRuleCollectionRules(props.Rules) if err := d.Set("rule", flattenedRules); err != nil { - return fmt.Errorf("Error setting `rule`: %+v", err) + return fmt.Errorf("setting `rule`: %+v", err) } } @@ -338,15 +345,15 @@ func resourceArmFirewallApplicationRuleCollectionDelete(d *schema.ResourceData, return nil } - return fmt.Errorf("Error making Read request on Azure Firewall %q (Resource Group %q): %+v", firewallName, resourceGroup, err) + return fmt.Errorf("making Read request on Azure Firewall %q (Resource Group %q): %+v", firewallName, resourceGroup, err) } props := firewall.AzureFirewallPropertiesFormat if props == nil { - return fmt.Errorf("Error retrieving Application Rule Collection %q (Firewall %q / Resource Group %q): `props` was nil", name, firewallName, resourceGroup) + return fmt.Errorf("retrieving Application Rule Collection %q (Firewall %q / Resource Group %q): `props` was nil", name, firewallName, resourceGroup) } if props.ApplicationRuleCollections == nil { - return fmt.Errorf("Error retrieving Application Rule Collection %q (Firewall %q / Resource Group %q): `props.ApplicationRuleCollections` was nil", name, firewallName, resourceGroup) + return fmt.Errorf("retrieving Application Rule Collection %q (Firewall %q / Resource Group %q): `props.ApplicationRuleCollections` was nil", name, firewallName, resourceGroup) } applicationRules := make([]network.AzureFirewallApplicationRuleCollection, 0) @@ -363,17 +370,17 @@ func resourceArmFirewallApplicationRuleCollectionDelete(d *schema.ResourceData, future, err := client.CreateOrUpdate(ctx, resourceGroup, firewallName, firewall) if err != nil { - return fmt.Errorf("Error deleting Application Rule Collection %q from Firewall %q (Resource Group %q): %+v", name, firewallName, resourceGroup, err) + return fmt.Errorf("deleting Application Rule Collection %q from Firewall %q (Resource Group %q): %+v", name, firewallName, resourceGroup, err) } if err = future.WaitForCompletionRef(ctx, client.Client); err != nil { - return fmt.Errorf("Error waiting for deletion of Application Rule Collection %q from Firewall %q (Resource Group %q): %+v", name, firewallName, resourceGroup, err) + return fmt.Errorf("waiting for deletion of Application Rule Collection %q from Firewall %q (Resource Group %q): %+v", name, firewallName, resourceGroup, err) } return nil } -func expandArmFirewallApplicationRules(inputs []interface{}) ([]network.AzureFirewallApplicationRule, error) { +func expandArmFirewallApplicationRules(inputs []interface{}) (*[]network.AzureFirewallApplicationRule, error) { outputs := make([]network.AzureFirewallApplicationRule, 0) for _, input := range inputs { @@ -382,6 +389,7 @@ func expandArmFirewallApplicationRules(inputs []interface{}) ([]network.AzureFir ruleName := rule["name"].(string) ruleDescription := rule["description"].(string) ruleSourceAddresses := rule["source_addresses"].(*schema.Set).List() + ruleSourceIpGroups := rule["source_ip_groups"].(*schema.Set).List() ruleFqdnTags := rule["fqdn_tags"].(*schema.Set).List() ruleTargetFqdns := rule["target_fqdns"].(*schema.Set).List() @@ -389,6 +397,7 @@ func expandArmFirewallApplicationRules(inputs []interface{}) ([]network.AzureFir Name: utils.String(ruleName), Description: utils.String(ruleDescription), SourceAddresses: utils.ExpandStringSlice(ruleSourceAddresses), + SourceIPGroups: utils.ExpandStringSlice(ruleSourceIpGroups), FqdnTags: utils.ExpandStringSlice(ruleFqdnTags), TargetFqdns: utils.ExpandStringSlice(ruleTargetFqdns), } @@ -407,13 +416,17 @@ func expandArmFirewallApplicationRules(inputs []interface{}) ([]network.AzureFir output.Protocols = &ruleProtocols if len(*output.FqdnTags) > 0 { if len(*output.TargetFqdns) > 0 || len(*output.Protocols) > 0 { - return outputs, fmt.Errorf("`fqdn_tags` cannot be used with `target_fqdns` or `protocol`") + return nil, fmt.Errorf("`fqdn_tags` cannot be used with `target_fqdns` or `protocol`") } } + + if len(*output.SourceAddresses) == 0 && len(*output.SourceIPGroups) == 0 { + return nil, fmt.Errorf("at least one of %q and %q must be specified for each rule", "source_addresses", "source_ip_groups") + } outputs = append(outputs, output) } - return outputs, nil + return &outputs, nil } func flattenFirewallApplicationRuleCollectionRules(rules *[]network.AzureFirewallApplicationRule) []map[string]interface{} { @@ -433,6 +446,9 @@ func flattenFirewallApplicationRuleCollectionRules(rules *[]network.AzureFirewal if ruleSourceAddresses := rule.SourceAddresses; ruleSourceAddresses != nil { output["source_addresses"] = set.FromStringSlice(*ruleSourceAddresses) } + if ruleSourceIpGroups := rule.SourceIPGroups; ruleSourceIpGroups != nil { + output["source_ip_groups"] = set.FromStringSlice(*ruleSourceIpGroups) + } if ruleFqdnTags := rule.FqdnTags; ruleFqdnTags != nil { output["fqdn_tags"] = set.FromStringSlice(*ruleFqdnTags) } diff --git a/azurerm/internal/services/network/tests/firewall_application_rule_collection_resource_test.go b/azurerm/internal/services/network/tests/firewall_application_rule_collection_resource_test.go index d3195d6c7d3f..0c20474fce41 100644 --- a/azurerm/internal/services/network/tests/firewall_application_rule_collection_resource_test.go +++ b/azurerm/internal/services/network/tests/firewall_application_rule_collection_resource_test.go @@ -2,6 +2,7 @@ package tests import ( "fmt" + "regexp" "testing" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/acceptance" @@ -375,6 +376,42 @@ func TestAccAzureRMFirewallApplicationRuleCollection_updateFirewallTags(t *testi }) } +func TestAccAzureRMFirewallApplicationRuleCollection_ipGroups(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_firewall_application_rule_collection", "test") + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acceptance.PreCheck(t) }, + Providers: acceptance.SupportedProviders, + CheckDestroy: testCheckAzureRMFirewallDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAzureRMFirewallApplicationRuleCollection_ipGroups(data), + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMFirewallApplicationRuleCollectionExists(data.ResourceName), + resource.TestCheckResourceAttr(data.ResourceName, "rule.#", "1"), + ), + }, + data.ImportStep(), + }, + }) +} + +func TestAccAzureRMFirewallApplicationRuleCollection_noSource(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_firewall_application_rule_collection", "test") + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acceptance.PreCheck(t) }, + Providers: acceptance.SupportedProviders, + CheckDestroy: testCheckAzureRMFirewallDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAzureRMFirewallApplicationRuleCollection_noSource(data), + ExpectError: regexp.MustCompile(fmt.Sprintf("at least one of %q and %q must be specified", "source_addresses", "source_ip_groups")), + }, + }, + }) +} + func testCheckAzureRMFirewallApplicationRuleCollectionExists(resourceName string) resource.TestCheckFunc { return func(s *terraform.State) error { client := acceptance.AzureProvider.Meta().(*clients.Client).Network.AzureFirewallsClient @@ -846,3 +883,70 @@ resource "azurerm_firewall_application_rule_collection" "test" { } `, template) } + +func testAccAzureRMFirewallApplicationRuleCollection_ipGroups(data acceptance.TestData) string { + template := testAccAzureRMFirewall_basic(data) + return fmt.Sprintf(` +%s + +resource "azurerm_ip_group" "test" { + name = "acctestIpGroupForFirewallAppRules" + location = azurerm_resource_group.test.location + resource_group_name = azurerm_resource_group.test.name + cidrs = ["192.168.0.0/25", "192.168.0.192/26"] +} + +resource "azurerm_firewall_application_rule_collection" "test" { + name = "acctestarc" + azure_firewall_name = azurerm_firewall.test.name + resource_group_name = azurerm_resource_group.test.name + priority = 100 + action = "Allow" + + rule { + name = "rule1" + + source_ip_groups = [ + azurerm_ip_group.test.id, + ] + + target_fqdns = [ + "*.google.com", + ] + + protocol { + port = 443 + type = "Https" + } + } +} +`, template) +} + +func testAccAzureRMFirewallApplicationRuleCollection_noSource(data acceptance.TestData) string { + template := testAccAzureRMFirewall_basic(data) + return fmt.Sprintf(` +%s + +resource "azurerm_firewall_application_rule_collection" "test" { + name = "acctestarc" + azure_firewall_name = azurerm_firewall.test.name + resource_group_name = azurerm_resource_group.test.name + priority = 100 + action = "Allow" + + rule { + name = "rule1" + + target_fqdns = [ + "*.google.com", + ] + + protocol { + port = 443 + type = "Https" + } + } +} +`, template) +} diff --git a/website/docs/r/firewall_application_rule_collection.html.markdown b/website/docs/r/firewall_application_rule_collection.html.markdown index 5bf29ac3aeb8..a9fdd41c72e5 100644 --- a/website/docs/r/firewall_application_rule_collection.html.markdown +++ b/website/docs/r/firewall_application_rule_collection.html.markdown @@ -103,7 +103,11 @@ A `rule` block supports the following: * `description` - (Optional) Specifies a description for the rule. -* `source_addresses` - (Required) A list of source IP addresses and/or IP ranges. +* `source_addresses` - (Optional) A list of source IP addresses and/or IP ranges. + +* `source_ip_groups` - (Optional) A list of source IP Group IDs for the rule. + +-> **NOTE** At least one of `source_addresses` and `source_ip_groups` must be specified for a rule. * `fqdn_tags` - (Optional) A list of FQDN tags. Possible values are `AppServiceEnvironment`, `AzureBackup`, `AzureKubernetesService`, `HDInsight`, `MicrosoftActiveProtectionService`, `WindowsDiagnostics`, `WindowsUpdate` and `WindowsVirtualDesktop`. From 04f203c803db96372811afffdd2625bacb59913c Mon Sep 17 00:00:00 2001 From: Tom Bamford Date: Mon, 21 Sep 2020 22:30:04 +0100 Subject: [PATCH 3/6] azurerm_firewall_nat_rule_collection: support source_ip_groups for rules --- .../firewall_nat_rule_collection_resource.go | 65 ++++++---- ...ewall_nat_rule_collection_resource_test.go | 115 ++++++++++++++++++ ...firewall_nat_rule_collection.html.markdown | 6 +- 3 files changed, 164 insertions(+), 22 deletions(-) diff --git a/azurerm/internal/services/network/firewall_nat_rule_collection_resource.go b/azurerm/internal/services/network/firewall_nat_rule_collection_resource.go index 4624599bc4a8..5269144482e6 100644 --- a/azurerm/internal/services/network/firewall_nat_rule_collection_resource.go +++ b/azurerm/internal/services/network/firewall_nat_rule_collection_resource.go @@ -8,6 +8,7 @@ import ( "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2020-05-01/network" "github.com/hashicorp/terraform-plugin-sdk/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/helper/validation" + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/azure" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/set" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/tf" @@ -91,7 +92,13 @@ func resourceArmFirewallNatRuleCollection() *schema.Resource { }, "source_addresses": { Type: schema.TypeSet, - Required: true, + Optional: true, + Elem: &schema.Schema{Type: schema.TypeString}, + Set: schema.HashString, + }, + "source_ip_groups": { + Type: schema.TypeSet, + Optional: true, Elem: &schema.Schema{Type: schema.TypeString}, Set: schema.HashString, }, @@ -142,20 +149,23 @@ func resourceArmFirewallNatRuleCollectionCreateUpdate(d *schema.ResourceData, me firewall, err := client.Get(ctx, resourceGroup, firewallName) if err != nil { - return fmt.Errorf("Error retrieving Firewall %q (Resource Group %q): %+v", firewallName, resourceGroup, err) + return fmt.Errorf("retrieving Firewall %q (Resource Group %q): %+v", firewallName, resourceGroup, err) } if firewall.AzureFirewallPropertiesFormat == nil { - return fmt.Errorf("Error expanding Firewall %q (Resource Group %q): `properties` was nil.", firewallName, resourceGroup) + return fmt.Errorf("expanding Firewall %q (Resource Group %q): `properties` was nil.", firewallName, resourceGroup) } props := *firewall.AzureFirewallPropertiesFormat if props.NatRuleCollections == nil { - return fmt.Errorf("Error expanding Firewall %q (Resource Group %q): `properties.NatRuleCollections` was nil.", firewallName, resourceGroup) + return fmt.Errorf("expanding Firewall %q (Resource Group %q): `properties.NatRuleCollections` was nil.", firewallName, resourceGroup) } ruleCollections := *props.NatRuleCollections - natRules := expandArmFirewallNatRules(d.Get("rule").(*schema.Set)) + natRules, err := expandArmFirewallNatRules(d.Get("rule").(*schema.Set)) + if err != nil { + return fmt.Errorf("expanding Firewall NAT Rules: %+v", err) + } priority := d.Get("priority").(int) newRuleCollection := network.AzureFirewallNatRuleCollection{ Name: utils.String(name), @@ -164,7 +174,7 @@ func resourceArmFirewallNatRuleCollectionCreateUpdate(d *schema.ResourceData, me Type: network.AzureFirewallNatRCActionType(d.Get("action").(string)), }, Priority: utils.Int32(int32(priority)), - Rules: &natRules, + Rules: natRules, }, } @@ -185,7 +195,7 @@ func resourceArmFirewallNatRuleCollectionCreateUpdate(d *schema.ResourceData, me if !d.IsNewResource() { if index == -1 { - return fmt.Errorf("Error locating NAT Rule Collection %q (Firewall %q / Resource Group %q)", name, firewallName, resourceGroup) + return fmt.Errorf("locating NAT Rule Collection %q (Firewall %q / Resource Group %q)", name, firewallName, resourceGroup) } ruleCollections[index] = newRuleCollection @@ -201,16 +211,16 @@ func resourceArmFirewallNatRuleCollectionCreateUpdate(d *schema.ResourceData, me firewall.AzureFirewallPropertiesFormat.NatRuleCollections = &ruleCollections future, err := client.CreateOrUpdate(ctx, resourceGroup, firewallName, firewall) if err != nil { - return fmt.Errorf("Error creating/updating NAT Rule Collection %q in Firewall %q (Resource Group %q): %+v", name, firewallName, resourceGroup, err) + return fmt.Errorf("creating/updating NAT Rule Collection %q in Firewall %q (Resource Group %q): %+v", name, firewallName, resourceGroup, err) } if err = future.WaitForCompletionRef(ctx, client.Client); err != nil { - return fmt.Errorf("Error waiting for creation/update of NAT Rule Collection %q of Firewall %q (Resource Group %q): %+v", name, firewallName, resourceGroup, err) + return fmt.Errorf("waiting for creation/update of NAT Rule Collection %q of Firewall %q (Resource Group %q): %+v", name, firewallName, resourceGroup, err) } read, err := client.Get(ctx, resourceGroup, firewallName) if err != nil { - return fmt.Errorf("Error retrieving Firewall %q (Resource Group %q): %+v", firewallName, resourceGroup, err) + return fmt.Errorf("retrieving Firewall %q (Resource Group %q): %+v", firewallName, resourceGroup, err) } var collectionID string @@ -258,16 +268,16 @@ func resourceArmFirewallNatRuleCollectionRead(d *schema.ResourceData, meta inter d.SetId("") return nil } - return fmt.Errorf("Error retrieving Azure Firewall %q (Resource Group %q): %+v", name, resourceGroup, err) + return fmt.Errorf("retrieving Azure Firewall %q (Resource Group %q): %+v", name, resourceGroup, err) } if read.AzureFirewallPropertiesFormat == nil { - return fmt.Errorf("Error retrieving NAT Rule Collection %q (Firewall %q / Resource Group %q): `props` was nil", name, firewallName, resourceGroup) + return fmt.Errorf("retrieving NAT Rule Collection %q (Firewall %q / Resource Group %q): `props` was nil", name, firewallName, resourceGroup) } props := *read.AzureFirewallPropertiesFormat if props.NatRuleCollections == nil { - return fmt.Errorf("Error retrieving NAT Rule Collection %q (Firewall %q / Resource Group %q): `props.NetworkRuleCollections` was nil", name, firewallName, resourceGroup) + return fmt.Errorf("retrieving NAT Rule Collection %q (Firewall %q / Resource Group %q): `props.NetworkRuleCollections` was nil", name, firewallName, resourceGroup) } var rule *network.AzureFirewallNatRuleCollection @@ -303,7 +313,7 @@ func resourceArmFirewallNatRuleCollectionRead(d *schema.ResourceData, meta inter flattenedRules := flattenFirewallNatRuleCollectionRules(props.Rules) if err := d.Set("rule", flattenedRules); err != nil { - return fmt.Errorf("Error setting `rule`: %+v", err) + return fmt.Errorf("setting `rule`: %+v", err) } } @@ -334,15 +344,15 @@ func resourceArmFirewallNatRuleCollectionDelete(d *schema.ResourceData, meta int return nil } - return fmt.Errorf("Error making Read request on Azure Firewall %q (Resource Group %q): %+v", firewallName, resourceGroup, err) + return fmt.Errorf("making Read request on Azure Firewall %q (Resource Group %q): %+v", firewallName, resourceGroup, err) } props := firewall.AzureFirewallPropertiesFormat if props == nil { - return fmt.Errorf("Error retrieving NAT Rule Collection %q (Firewall %q / Resource Group %q): `props` was nil", name, firewallName, resourceGroup) + return fmt.Errorf("retrieving NAT Rule Collection %q (Firewall %q / Resource Group %q): `props` was nil", name, firewallName, resourceGroup) } if props.NetworkRuleCollections == nil { - return fmt.Errorf("Error retrieving NAT Rule Collection %q (Firewall %q / Resource Group %q): `props.NatRuleCollections` was nil", name, firewallName, resourceGroup) + return fmt.Errorf("retrieving NAT Rule Collection %q (Firewall %q / Resource Group %q): `props.NatRuleCollections` was nil", name, firewallName, resourceGroup) } natRules := make([]network.AzureFirewallNatRuleCollection, 0) @@ -359,17 +369,17 @@ func resourceArmFirewallNatRuleCollectionDelete(d *schema.ResourceData, meta int future, err := client.CreateOrUpdate(ctx, resourceGroup, firewallName, firewall) if err != nil { - return fmt.Errorf("Error deleting NAT Rule Collection %q from Firewall %q (Resource Group %q): %+v", name, firewallName, resourceGroup, err) + return fmt.Errorf("deleting NAT Rule Collection %q from Firewall %q (Resource Group %q): %+v", name, firewallName, resourceGroup, err) } if err = future.WaitForCompletionRef(ctx, client.Client); err != nil { - return fmt.Errorf("Error waiting for deletion of NAT Rule Collection %q from Firewall %q (Resource Group %q): %+v", name, firewallName, resourceGroup, err) + return fmt.Errorf("waiting for deletion of NAT Rule Collection %q from Firewall %q (Resource Group %q): %+v", name, firewallName, resourceGroup, err) } return nil } -func expandArmFirewallNatRules(input *schema.Set) []network.AzureFirewallNatRule { +func expandArmFirewallNatRules(input *schema.Set) (*[]network.AzureFirewallNatRule, error) { nwRules := input.List() rules := make([]network.AzureFirewallNatRule, 0) @@ -384,6 +394,15 @@ func expandArmFirewallNatRules(input *schema.Set) []network.AzureFirewallNatRule sourceAddresses = append(sourceAddresses, v.(string)) } + sourceIpGroups := make([]string, 0) + for _, v := range rule["source_ip_groups"].(*schema.Set).List() { + sourceIpGroups = append(sourceIpGroups, v.(string)) + } + + if len(sourceAddresses) == 0 && len(sourceIpGroups) == 0 { + return nil, fmt.Errorf("at least one of %q and %q must be specified for each rule", "source_addresses", "source_ip_groups") + } + destinationAddresses := make([]string, 0) for _, v := range rule["destination_addresses"].(*schema.Set).List() { destinationAddresses = append(destinationAddresses, v.(string)) @@ -401,6 +420,7 @@ func expandArmFirewallNatRules(input *schema.Set) []network.AzureFirewallNatRule Name: utils.String(name), Description: utils.String(description), SourceAddresses: &sourceAddresses, + SourceIPGroups: &sourceIpGroups, DestinationAddresses: &destinationAddresses, DestinationPorts: &destinationPorts, TranslatedAddress: &translatedAddress, @@ -417,7 +437,7 @@ func expandArmFirewallNatRules(input *schema.Set) []network.AzureFirewallNatRule rules = append(rules, ruleToAdd) } - return rules + return &rules, nil } func flattenFirewallNatRuleCollectionRules(rules *[]network.AzureFirewallNatRule) []map[string]interface{} { @@ -443,6 +463,9 @@ func flattenFirewallNatRuleCollectionRules(rules *[]network.AzureFirewallNatRule if rule.SourceAddresses != nil { output["source_addresses"] = set.FromStringSlice(*rule.SourceAddresses) } + if rule.SourceIPGroups != nil { + output["source_ip_groups"] = set.FromStringSlice(*rule.SourceIPGroups) + } if rule.DestinationAddresses != nil { output["destination_addresses"] = set.FromStringSlice(*rule.DestinationAddresses) } diff --git a/azurerm/internal/services/network/tests/firewall_nat_rule_collection_resource_test.go b/azurerm/internal/services/network/tests/firewall_nat_rule_collection_resource_test.go index 5163d98daa8d..4a07f72e749b 100644 --- a/azurerm/internal/services/network/tests/firewall_nat_rule_collection_resource_test.go +++ b/azurerm/internal/services/network/tests/firewall_nat_rule_collection_resource_test.go @@ -2,6 +2,7 @@ package tests import ( "fmt" + "regexp" "testing" "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2020-05-01/network" @@ -222,6 +223,41 @@ func TestAccAzureRMFirewallNatRuleCollection_updateFirewallTags(t *testing.T) { }) } +func TestAccAzureRMFirewallNatRuleCollection_ipGroup(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_firewall_nat_rule_collection", "test") + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acceptance.PreCheck(t) }, + Providers: acceptance.SupportedProviders, + CheckDestroy: testCheckAzureRMFirewallDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAzureRMFirewallNatRuleCollection_ipGroup(data), + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMFirewallNatRuleCollectionExists(data.ResourceName), + ), + }, + data.ImportStep(), + }, + }) +} + +func TestAccAzureRMFirewallNatRuleCollection_noSource(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_firewall_nat_rule_collection", "test") + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acceptance.PreCheck(t) }, + Providers: acceptance.SupportedProviders, + CheckDestroy: testCheckAzureRMFirewallDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAzureRMFirewallNatRuleCollection_noSource(data), + ExpectError: regexp.MustCompile(fmt.Sprintf("at least one of %q and %q must be specified", "source_addresses", "source_ip_groups")), + }, + }, + }) +} + func testCheckAzureRMFirewallNatRuleCollectionExists(resourceName string) resource.TestCheckFunc { return func(s *terraform.State) error { client := acceptance.AzureProvider.Meta().(*clients.Client).Network.AzureFirewallsClient @@ -680,3 +716,82 @@ resource "azurerm_firewall_nat_rule_collection" "test" { } `, template, data.RandomInteger) } + +func testAccAzureRMFirewallNatRuleCollection_ipGroup(data acceptance.TestData) string { + template := testAccAzureRMFirewall_basic(data) + return fmt.Sprintf(` +%s + +resource "azurerm_ip_group" "test" { + name = "acctestIpGroupForFirewallNatRules" + location = azurerm_resource_group.test.location + resource_group_name = azurerm_resource_group.test.name + cidrs = ["192.168.0.0/25", "192.168.0.192/26"] +} + +resource "azurerm_firewall_nat_rule_collection" "test" { + name = "acctestnrc-%d" + azure_firewall_name = azurerm_firewall.test.name + resource_group_name = azurerm_resource_group.test.name + priority = 100 + action = "Dnat" + + rule { + name = "rule1" + + source_ip_groups = [ + azurerm_ip_group.test.id, + ] + + destination_ports = [ + "53", + ] + + destination_addresses = [ + azurerm_public_ip.test.ip_address, + ] + + protocols = [ + "Any", + ] + + translated_port = 53 + translated_address = "8.8.8.8" + } +} +`, template, data.RandomInteger) +} + +func testAccAzureRMFirewallNatRuleCollection_noSource(data acceptance.TestData) string { + template := testAccAzureRMFirewall_basic(data) + return fmt.Sprintf(` +%s + +resource "azurerm_firewall_nat_rule_collection" "test" { + name = "acctestnrc-%d" + azure_firewall_name = azurerm_firewall.test.name + resource_group_name = azurerm_resource_group.test.name + priority = 100 + action = "Dnat" + + rule { + name = "rule1" + + destination_ports = [ + "53", + ] + + destination_addresses = [ + azurerm_public_ip.test.ip_address, + ] + + protocols = [ + "Any", + ] + + translated_port = 53 + translated_address = "8.8.8.8" + } +} +`, template, data.RandomInteger) +} diff --git a/website/docs/r/firewall_nat_rule_collection.html.markdown b/website/docs/r/firewall_nat_rule_collection.html.markdown index eb124679da6d..24717e747ef9 100644 --- a/website/docs/r/firewall_nat_rule_collection.html.markdown +++ b/website/docs/r/firewall_nat_rule_collection.html.markdown @@ -117,7 +117,11 @@ A `rule` block supports the following: * `protocols` - (Required) A list of protocols. Possible values are `Any`, `ICMP`, `TCP` and `UDP`. If `action` is `Dnat`, protocols can only be `TCP` and `UDP`. -* `source_addresses` - (Required) A list of source IP addresses and/or IP ranges. +* `source_addresses` - (Optional) A list of source IP addresses and/or IP ranges. + +* `source_ip_groups` - (Optional) A list of source IP Group IDs for the rule. + +-> **NOTE** At least one of `source_addresses` and `source_ip_groups` must be specified for a rule. * `translated_address` - (Required) The address of the service behind the Firewall. From a2f346ea439e53d7b03316b16c9bed674aef7c8d Mon Sep 17 00:00:00 2001 From: Tom Bamford Date: Mon, 21 Sep 2020 22:30:53 +0100 Subject: [PATCH 4/6] azurerm_firewall_network_rule_collection: support destination_ip_groups and source_ip_groups for rules --- ...rewall_network_rule_collection_resource.go | 86 ++++++--- ...l_network_rule_collection_resource_test.go | 164 ++++++++++++++++++ ...wall_network_rule_collection.html.markdown | 10 +- 3 files changed, 237 insertions(+), 23 deletions(-) diff --git a/azurerm/internal/services/network/firewall_network_rule_collection_resource.go b/azurerm/internal/services/network/firewall_network_rule_collection_resource.go index bf7cb17fc667..4dfdef9e3d8a 100644 --- a/azurerm/internal/services/network/firewall_network_rule_collection_resource.go +++ b/azurerm/internal/services/network/firewall_network_rule_collection_resource.go @@ -8,6 +8,7 @@ import ( "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2020-05-01/network" "github.com/hashicorp/terraform-plugin-sdk/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/helper/validation" + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/azure" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/set" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/tf" @@ -83,13 +84,25 @@ func resourceArmFirewallNetworkRuleCollection() *schema.Resource { }, "source_addresses": { Type: schema.TypeSet, - Required: true, + Optional: true, + Elem: &schema.Schema{Type: schema.TypeString}, + Set: schema.HashString, + }, + "source_ip_groups": { + Type: schema.TypeSet, + Optional: true, Elem: &schema.Schema{Type: schema.TypeString}, Set: schema.HashString, }, "destination_addresses": { Type: schema.TypeSet, - Required: true, + Optional: true, + Elem: &schema.Schema{Type: schema.TypeString}, + Set: schema.HashString, + }, + "destination_ip_groups": { + Type: schema.TypeSet, + Optional: true, Elem: &schema.Schema{Type: schema.TypeString}, Set: schema.HashString, }, @@ -134,20 +147,23 @@ func resourceArmFirewallNetworkRuleCollectionCreateUpdate(d *schema.ResourceData firewall, err := client.Get(ctx, resourceGroup, firewallName) if err != nil { - return fmt.Errorf("Error retrieving Firewall %q (Resource Group %q): %+v", firewallName, resourceGroup, err) + return fmt.Errorf("retrieving Firewall %q (Resource Group %q): %+v", firewallName, resourceGroup, err) } if firewall.AzureFirewallPropertiesFormat == nil { - return fmt.Errorf("Error expanding Firewall %q (Resource Group %q): `properties` was nil.", firewallName, resourceGroup) + return fmt.Errorf("expanding Firewall %q (Resource Group %q): `properties` was nil.", firewallName, resourceGroup) } props := *firewall.AzureFirewallPropertiesFormat if props.NetworkRuleCollections == nil { - return fmt.Errorf("Error expanding Firewall %q (Resource Group %q): `properties.NetworkRuleCollections` was nil.", firewallName, resourceGroup) + return fmt.Errorf("expanding Firewall %q (Resource Group %q): `properties.NetworkRuleCollections` was nil.", firewallName, resourceGroup) } ruleCollections := *props.NetworkRuleCollections - networkRules := expandArmFirewallNetworkRules(d.Get("rule").(*schema.Set)) + networkRules, err := expandArmFirewallNetworkRules(d.Get("rule").(*schema.Set)) + if err != nil { + return fmt.Errorf("expanding Firewall Network Rules: %+v", err) + } priority := d.Get("priority").(int) newRuleCollection := network.AzureFirewallNetworkRuleCollection{ Name: utils.String(name), @@ -156,7 +172,7 @@ func resourceArmFirewallNetworkRuleCollectionCreateUpdate(d *schema.ResourceData Type: network.AzureFirewallRCActionType(d.Get("action").(string)), }, Priority: utils.Int32(int32(priority)), - Rules: &networkRules, + Rules: networkRules, }, } @@ -177,7 +193,7 @@ func resourceArmFirewallNetworkRuleCollectionCreateUpdate(d *schema.ResourceData if !d.IsNewResource() { if index == -1 { - return fmt.Errorf("Error locating Network Rule Collection %q (Firewall %q / Resource Group %q)", name, firewallName, resourceGroup) + return fmt.Errorf("locating Network Rule Collection %q (Firewall %q / Resource Group %q)", name, firewallName, resourceGroup) } ruleCollections[index] = newRuleCollection @@ -194,16 +210,16 @@ func resourceArmFirewallNetworkRuleCollectionCreateUpdate(d *schema.ResourceData future, err := client.CreateOrUpdate(ctx, resourceGroup, firewallName, firewall) if err != nil { - return fmt.Errorf("Error creating/updating Network Rule Collection %q in Firewall %q (Resource Group %q): %+v", name, firewallName, resourceGroup, err) + return fmt.Errorf("creating/updating Network Rule Collection %q in Firewall %q (Resource Group %q): %+v", name, firewallName, resourceGroup, err) } if err = future.WaitForCompletionRef(ctx, client.Client); err != nil { - return fmt.Errorf("Error waiting for creation/update of Network Rule Collection %q of Firewall %q (Resource Group %q): %+v", name, firewallName, resourceGroup, err) + return fmt.Errorf("waiting for creation/update of Network Rule Collection %q of Firewall %q (Resource Group %q): %+v", name, firewallName, resourceGroup, err) } read, err := client.Get(ctx, resourceGroup, firewallName) if err != nil { - return fmt.Errorf("Error retrieving Firewall %q (Resource Group %q): %+v", firewallName, resourceGroup, err) + return fmt.Errorf("retrieving Firewall %q (Resource Group %q): %+v", firewallName, resourceGroup, err) } var collectionID string @@ -251,16 +267,16 @@ func resourceArmFirewallNetworkRuleCollectionRead(d *schema.ResourceData, meta i d.SetId("") return nil } - return fmt.Errorf("Error retrieving Azure Firewall %q (Resource Group %q): %+v", name, resourceGroup, err) + return fmt.Errorf("retrieving Azure Firewall %q (Resource Group %q): %+v", name, resourceGroup, err) } if read.AzureFirewallPropertiesFormat == nil { - return fmt.Errorf("Error retrieving Network Rule Collection %q (Firewall %q / Resource Group %q): `props` was nil", name, firewallName, resourceGroup) + return fmt.Errorf("retrieving Network Rule Collection %q (Firewall %q / Resource Group %q): `props` was nil", name, firewallName, resourceGroup) } props := *read.AzureFirewallPropertiesFormat if props.NetworkRuleCollections == nil { - return fmt.Errorf("Error retrieving Network Rule Collection %q (Firewall %q / Resource Group %q): `props.NetworkRuleCollections` was nil", name, firewallName, resourceGroup) + return fmt.Errorf("retrieving Network Rule Collection %q (Firewall %q / Resource Group %q): `props.NetworkRuleCollections` was nil", name, firewallName, resourceGroup) } var rule *network.AzureFirewallNetworkRuleCollection @@ -296,7 +312,7 @@ func resourceArmFirewallNetworkRuleCollectionRead(d *schema.ResourceData, meta i flattenedRules := flattenFirewallNetworkRuleCollectionRules(props.Rules) if err := d.Set("rule", flattenedRules); err != nil { - return fmt.Errorf("Error setting `rule`: %+v", err) + return fmt.Errorf("setting `rule`: %+v", err) } } @@ -327,15 +343,15 @@ func resourceArmFirewallNetworkRuleCollectionDelete(d *schema.ResourceData, meta return nil } - return fmt.Errorf("Error making Read request on Azure Firewall %q (Resource Group %q): %+v", firewallName, resourceGroup, err) + return fmt.Errorf("making Read request on Azure Firewall %q (Resource Group %q): %+v", firewallName, resourceGroup, err) } props := firewall.AzureFirewallPropertiesFormat if props == nil { - return fmt.Errorf("Error retrieving Network Rule Collection %q (Firewall %q / Resource Group %q): `props` was nil", name, firewallName, resourceGroup) + return fmt.Errorf("retrieving Network Rule Collection %q (Firewall %q / Resource Group %q): `props` was nil", name, firewallName, resourceGroup) } if props.NetworkRuleCollections == nil { - return fmt.Errorf("Error retrieving Network Rule Collection %q (Firewall %q / Resource Group %q): `props.NetworkRuleCollections` was nil", name, firewallName, resourceGroup) + return fmt.Errorf("retrieving Network Rule Collection %q (Firewall %q / Resource Group %q): `props.NetworkRuleCollections` was nil", name, firewallName, resourceGroup) } networkRules := make([]network.AzureFirewallNetworkRuleCollection, 0) @@ -352,17 +368,17 @@ func resourceArmFirewallNetworkRuleCollectionDelete(d *schema.ResourceData, meta future, err := client.CreateOrUpdate(ctx, resourceGroup, firewallName, firewall) if err != nil { - return fmt.Errorf("Error deleting Network Rule Collection %q from Firewall %q (Resource Group %q): %+v", name, firewallName, resourceGroup, err) + return fmt.Errorf("deleting Network Rule Collection %q from Firewall %q (Resource Group %q): %+v", name, firewallName, resourceGroup, err) } if err = future.WaitForCompletionRef(ctx, client.Client); err != nil { - return fmt.Errorf("Error waiting for deletion of Network Rule Collection %q from Firewall %q (Resource Group %q): %+v", name, firewallName, resourceGroup, err) + return fmt.Errorf("waiting for deletion of Network Rule Collection %q from Firewall %q (Resource Group %q): %+v", name, firewallName, resourceGroup, err) } return nil } -func expandArmFirewallNetworkRules(input *schema.Set) []network.AzureFirewallNetworkRule { +func expandArmFirewallNetworkRules(input *schema.Set) (*[]network.AzureFirewallNetworkRule, error) { nwRules := input.List() rules := make([]network.AzureFirewallNetworkRule, 0) @@ -377,11 +393,29 @@ func expandArmFirewallNetworkRules(input *schema.Set) []network.AzureFirewallNet sourceAddresses = append(sourceAddresses, v.(string)) } + sourceIpGroups := make([]string, 0) + for _, v := range rule["source_ip_groups"].(*schema.Set).List() { + sourceIpGroups = append(sourceIpGroups, v.(string)) + } + + if len(sourceAddresses) == 0 && len(sourceIpGroups) == 0 { + return nil, fmt.Errorf("at least one of %q and %q must be specified for each rule", "source_addresses", "source_ip_groups") + } + destinationAddresses := make([]string, 0) for _, v := range rule["destination_addresses"].(*schema.Set).List() { destinationAddresses = append(destinationAddresses, v.(string)) } + destinationIpGroups := make([]string, 0) + for _, v := range rule["destination_ip_groups"].(*schema.Set).List() { + destinationIpGroups = append(destinationIpGroups, v.(string)) + } + + if len(destinationAddresses) == 0 && len(destinationIpGroups) == 0 { + return nil, fmt.Errorf("at least one of %q and %q must be specified for each rule", "destination_addresses", "destination_ip_groups") + } + destinationPorts := make([]string, 0) for _, v := range rule["destination_ports"].(*schema.Set).List() { destinationPorts = append(destinationPorts, v.(string)) @@ -391,7 +425,9 @@ func expandArmFirewallNetworkRules(input *schema.Set) []network.AzureFirewallNet Name: utils.String(name), Description: utils.String(description), SourceAddresses: &sourceAddresses, + SourceIPGroups: &sourceIpGroups, DestinationAddresses: &destinationAddresses, + DestinationIPGroups: &destinationIpGroups, DestinationPorts: &destinationPorts, } @@ -405,7 +441,7 @@ func expandArmFirewallNetworkRules(input *schema.Set) []network.AzureFirewallNet rules = append(rules, ruleToAdd) } - return rules + return &rules, nil } func flattenFirewallNetworkRuleCollectionRules(rules *[]network.AzureFirewallNetworkRule) []map[string]interface{} { @@ -425,9 +461,15 @@ func flattenFirewallNetworkRuleCollectionRules(rules *[]network.AzureFirewallNet if rule.SourceAddresses != nil { output["source_addresses"] = set.FromStringSlice(*rule.SourceAddresses) } + if rule.SourceIPGroups != nil { + output["source_ip_groups"] = set.FromStringSlice(*rule.SourceIPGroups) + } if rule.DestinationAddresses != nil { output["destination_addresses"] = set.FromStringSlice(*rule.DestinationAddresses) } + if rule.DestinationIPGroups != nil { + output["destination_ip_groups"] = set.FromStringSlice(*rule.DestinationIPGroups) + } if rule.DestinationPorts != nil { output["destination_ports"] = set.FromStringSlice(*rule.DestinationPorts) } diff --git a/azurerm/internal/services/network/tests/firewall_network_rule_collection_resource_test.go b/azurerm/internal/services/network/tests/firewall_network_rule_collection_resource_test.go index 73aeb5150c1b..2cf989a34da8 100644 --- a/azurerm/internal/services/network/tests/firewall_network_rule_collection_resource_test.go +++ b/azurerm/internal/services/network/tests/firewall_network_rule_collection_resource_test.go @@ -2,6 +2,7 @@ package tests import ( "fmt" + "regexp" "testing" "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2020-05-01/network" @@ -303,6 +304,58 @@ func TestAccAzureRMFirewallNetworkRuleCollection_serviceTag(t *testing.T) { }) } +func TestAccAzureRMFirewallNetworkRuleCollection_ipGroup(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_firewall_network_rule_collection", "test") + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acceptance.PreCheck(t) }, + Providers: acceptance.SupportedProviders, + CheckDestroy: testCheckAzureRMFirewallDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAzureRMFirewallNetworkRuleCollection_ipGroup(data), + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMFirewallNetworkRuleCollectionExists(data.ResourceName), + resource.TestCheckResourceAttr(data.ResourceName, "rule.#", "1"), + ), + }, + data.ImportStep(), + }, + }) +} + +func TestAccAzureRMFirewallNetworkRuleCollection_noSource(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_firewall_network_rule_collection", "test") + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acceptance.PreCheck(t) }, + Providers: acceptance.SupportedProviders, + CheckDestroy: testCheckAzureRMFirewallDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAzureRMFirewallNetworkRuleCollection_noSource(data), + ExpectError: regexp.MustCompile(fmt.Sprintf("at least one of %q and %q must be specified", "source_addresses", "source_ip_groups")), + }, + }, + }) +} + +func TestAccAzureRMFirewallNetworkRuleCollection_noDestination(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_firewall_network_rule_collection", "test") + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acceptance.PreCheck(t) }, + Providers: acceptance.SupportedProviders, + CheckDestroy: testCheckAzureRMFirewallDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAzureRMFirewallNetworkRuleCollection_noDestination(data), + ExpectError: regexp.MustCompile(fmt.Sprintf("at least one of %q and %q must be specified", "destination_addresses", "destination_ip_groups")), + }, + }, + }) +} + func testCheckAzureRMFirewallNetworkRuleCollectionExists(resourceName string) resource.TestCheckFunc { return func(s *terraform.State) error { client := acceptance.AzureProvider.Meta().(*clients.Client).Network.AzureFirewallsClient @@ -766,3 +819,114 @@ resource "azurerm_firewall_network_rule_collection" "test" { } `, template) } + +func testAccAzureRMFirewallNetworkRuleCollection_ipGroup(data acceptance.TestData) string { + template := testAccAzureRMFirewall_basic(data) + return fmt.Sprintf(` +%s + +resource "azurerm_ip_group" "test_source" { + name = "acctestIpGroupForFirewallNetworkRulesSource" + location = azurerm_resource_group.test.location + resource_group_name = azurerm_resource_group.test.name + cidrs = ["1.2.3.4/32", "12.34.56.0/24"] +} + +resource "azurerm_ip_group" "test_destination" { + name = "acctestIpGroupForFirewallNetworkRulesDestination" + location = azurerm_resource_group.test.location + resource_group_name = azurerm_resource_group.test.name + cidrs = ["192.168.0.0/25", "192.168.0.192/26"] +} + +resource "azurerm_firewall_network_rule_collection" "test" { + name = "acctestnrc" + azure_firewall_name = azurerm_firewall.test.name + resource_group_name = azurerm_resource_group.test.name + priority = 100 + action = "Allow" + + rule { + name = "rule1" + + source_ip_groups = [ + azurerm_ip_group.test_source.id, + ] + + destination_ports = [ + "53", + ] + + destination_ip_groups = [ + azurerm_ip_group.test_destination.id, + ] + + protocols = [ + "Any", + ] + } +} +`, template) +} + +func testAccAzureRMFirewallNetworkRuleCollection_noSource(data acceptance.TestData) string { + template := testAccAzureRMFirewall_basic(data) + return fmt.Sprintf(` +%s + +resource "azurerm_firewall_network_rule_collection" "test" { + name = "acctestnrc" + azure_firewall_name = azurerm_firewall.test.name + resource_group_name = azurerm_resource_group.test.name + priority = 100 + action = "Allow" + + rule { + name = "rule1" + + destination_ports = [ + "53", + ] + + destination_addresses = [ + "8.8.8.8", + ] + + protocols = [ + "Any", + ] + } +} +`, template) +} + +func testAccAzureRMFirewallNetworkRuleCollection_noDestination(data acceptance.TestData) string { + template := testAccAzureRMFirewall_basic(data) + return fmt.Sprintf(` +%s + +resource "azurerm_firewall_network_rule_collection" "test" { + name = "acctestnrc" + azure_firewall_name = azurerm_firewall.test.name + resource_group_name = azurerm_resource_group.test.name + priority = 100 + action = "Allow" + + rule { + name = "rule1" + + source_addresses = [ + "10.0.0.0/16", + ] + + destination_ports = [ + "53", + ] + + protocols = [ + "Any", + ] + } +} +`, template) +} diff --git a/website/docs/r/firewall_network_rule_collection.html.markdown b/website/docs/r/firewall_network_rule_collection.html.markdown index 004f07905a1c..5fd0fc7e9064 100644 --- a/website/docs/r/firewall_network_rule_collection.html.markdown +++ b/website/docs/r/firewall_network_rule_collection.html.markdown @@ -110,7 +110,15 @@ A `rule` block supports the following: * `source_addresses` - (Required) A list of source IP addresses and/or IP ranges. -* `destination_addresses` - (Required) Either a list of destination IP addresses and/or IP ranges, or a list of destination [Service Tags](https://docs.microsoft.com/en-us/azure/virtual-network/service-tags-overview#available-service-tags). +* `source_ip_groups` - (Optional) A list of IP Group IDs for the rule. + +-> **NOTE** At least one of `source_addresses` and `source_ip_groups` must be specified for a rule. + +* `destination_addresses` - (Optional) Either a list of destination IP addresses and/or IP ranges, or a list of destination [Service Tags](https://docs.microsoft.com/en-us/azure/virtual-network/service-tags-overview#available-service-tags). + +* `destination_ip_groups` - (Optional) A list of destination IP Group IDs for the rule. + +-> **NOTE** At least one of `destination_addresses` and `destination_ip_groups` must be specified for a rule. * `destination_ports` - (Required) A list of destination ports. From 7b5221c00f0416d761f67e531024e2a89a33bdbe Mon Sep 17 00:00:00 2001 From: Tom Bamford Date: Mon, 21 Sep 2020 22:31:24 +0100 Subject: [PATCH 5/6] Fix deprecation warnings in Firewall tests --- .../network/tests/firewall_resource_test.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/azurerm/internal/services/network/tests/firewall_resource_test.go b/azurerm/internal/services/network/tests/firewall_resource_test.go index ab7719336889..6bb1a781bb12 100644 --- a/azurerm/internal/services/network/tests/firewall_resource_test.go +++ b/azurerm/internal/services/network/tests/firewall_resource_test.go @@ -347,7 +347,7 @@ resource "azurerm_subnet" "test" { name = "AzureFirewallSubnet" resource_group_name = azurerm_resource_group.test.name virtual_network_name = azurerm_virtual_network.test.name - address_prefix = "10.0.1.0/24" + address_prefixes = ["10.0.1.0/24"] } resource "azurerm_public_ip" "test" { @@ -395,7 +395,7 @@ resource "azurerm_subnet" "test" { name = "AzureFirewallSubnet" resource_group_name = azurerm_resource_group.test.name virtual_network_name = azurerm_virtual_network.test.name - address_prefix = "10.0.1.0/24" + address_prefixes = ["10.0.1.0/24"] } resource "azurerm_public_ip" "test" { @@ -410,7 +410,7 @@ resource "azurerm_subnet" "test_mgmt" { name = "AzureFirewallManagementSubnet" resource_group_name = azurerm_resource_group.test.name virtual_network_name = azurerm_virtual_network.test.name - address_prefix = "10.0.2.0/24" + address_prefixes = ["10.0.2.0/24"] } resource "azurerm_public_ip" "test_mgmt" { @@ -465,7 +465,7 @@ resource "azurerm_subnet" "test" { name = "AzureFirewallSubnet" resource_group_name = azurerm_resource_group.test.name virtual_network_name = azurerm_virtual_network.test.name - address_prefix = "10.0.1.0/24" + address_prefixes = ["10.0.1.0/24"] } resource "azurerm_public_ip" "test" { @@ -545,7 +545,7 @@ resource "azurerm_subnet" "test" { name = "AzureFirewallSubnet" resource_group_name = azurerm_resource_group.test.name virtual_network_name = azurerm_virtual_network.test.name - address_prefix = "10.0.1.0/24" + address_prefixes = ["10.0.1.0/24"] } resource "azurerm_public_ip" "test" { @@ -597,7 +597,7 @@ resource "azurerm_subnet" "test" { name = "AzureFirewallSubnet" resource_group_name = azurerm_resource_group.test.name virtual_network_name = azurerm_virtual_network.test.name - address_prefix = "10.0.1.0/24" + address_prefixes = ["10.0.1.0/24"] } resource "azurerm_public_ip" "test" { @@ -649,7 +649,7 @@ resource "azurerm_subnet" "test" { name = "AzureFirewallSubnet" resource_group_name = azurerm_resource_group.test.name virtual_network_name = azurerm_virtual_network.test.name - address_prefix = "10.0.1.0/24" + address_prefixes = ["10.0.1.0/24"] } resource "azurerm_public_ip" "test" { @@ -698,7 +698,7 @@ resource "azurerm_subnet" "test" { name = "AzureFirewallSubnet" resource_group_name = azurerm_resource_group.test.name virtual_network_name = azurerm_virtual_network.test.name - address_prefix = "10.0.1.0/24" + address_prefixes = ["10.0.1.0/24"] } resource "azurerm_public_ip" "test" { From da673aabb7135482da523b9a9c46191386d2da21 Mon Sep 17 00:00:00 2001 From: Tom Bamford Date: Thu, 24 Sep 2020 11:49:30 +0100 Subject: [PATCH 6/6] Address review; better resource group name --- .../services/network/tests/ip_group_resource_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/azurerm/internal/services/network/tests/ip_group_resource_test.go b/azurerm/internal/services/network/tests/ip_group_resource_test.go index a2068c3ac9a3..60d4ff8607d2 100644 --- a/azurerm/internal/services/network/tests/ip_group_resource_test.go +++ b/azurerm/internal/services/network/tests/ip_group_resource_test.go @@ -187,7 +187,7 @@ provider "azurerm" { } resource "azurerm_resource_group" "test" { - name = "acctestRG-%d" + name = "acctestRG-network-%d" location = "%s" } @@ -219,7 +219,7 @@ provider "azurerm" { } resource "azurerm_resource_group" "test" { - name = "acctestRG-%d" + name = "acctestRG-network-%d" location = "%s" } @@ -245,7 +245,7 @@ provider "azurerm" { } resource "azurerm_resource_group" "test" { - name = "acctestRG-%d" + name = "acctestRG-network-%d" location = "%s" }