From e10e4ad4c9b16193f43969b6d199e9f2a6bb5169 Mon Sep 17 00:00:00 2001 From: Sander van Harmelen Date: Thu, 1 Feb 2018 14:56:50 +0100 Subject: [PATCH] Add support for augmented security rules MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This support was already added to the `azurerm_network_security_rule` resource in PR #692, but it wasn’t added to the `azurerm_network_security_group` resource at that time. It did needed a bit of additional logic to make it work properly in the `azurerm_network_security_group` resource as well, but it all worked out nicely. All related unit- and acceptance tests were successfully run before creating this PR. --- azurerm/network_security_rule.go | 20 -- azurerm/network_security_rule_test.go | 43 ---- .../resource_arm_network_security_group.go | 234 ++++++++++++++---- ...esource_arm_network_security_group_test.go | 46 ++++ azurerm/resource_arm_network_security_rule.go | 16 +- .../r/network_security_group.html.markdown | 16 +- 6 files changed, 245 insertions(+), 130 deletions(-) delete mode 100644 azurerm/network_security_rule.go delete mode 100644 azurerm/network_security_rule_test.go diff --git a/azurerm/network_security_rule.go b/azurerm/network_security_rule.go deleted file mode 100644 index c9df9ee516f1..000000000000 --- a/azurerm/network_security_rule.go +++ /dev/null @@ -1,20 +0,0 @@ -package azurerm - -import ( - "fmt" - "strings" -) - -func validateNetworkSecurityRuleProtocol(v interface{}, k string) (ws []string, errors []error) { - value := strings.ToLower(v.(string)) - protocols := map[string]bool{ - "tcp": true, - "udp": true, - "*": true, - } - - if !protocols[value] { - errors = append(errors, fmt.Errorf("Network Security Rule Protocol can only be Tcp, Udp or *")) - } - return -} diff --git a/azurerm/network_security_rule_test.go b/azurerm/network_security_rule_test.go deleted file mode 100644 index 8b9f21ecc4b2..000000000000 --- a/azurerm/network_security_rule_test.go +++ /dev/null @@ -1,43 +0,0 @@ -package azurerm - -import "testing" - -func TestResourceAzureRMNetworkSecurityRuleProtocol_validation(t *testing.T) { - cases := []struct { - Value string - ErrCount int - }{ - { - Value: "Random", - ErrCount: 1, - }, - { - Value: "tcp", - ErrCount: 0, - }, - { - Value: "TCP", - ErrCount: 0, - }, - { - Value: "*", - ErrCount: 0, - }, - { - Value: "Udp", - ErrCount: 0, - }, - { - Value: "Tcp", - ErrCount: 0, - }, - } - - for _, tc := range cases { - _, errors := validateNetworkSecurityRuleProtocol(tc.Value, "azurerm_network_security_rule") - - if len(errors) != tc.ErrCount { - t.Fatalf("Expected the Azure RM Network Security Rule protocol to trigger a validation error") - } - } -} diff --git a/azurerm/resource_arm_network_security_group.go b/azurerm/resource_arm_network_security_group.go index 6bbdc6a5d809..f2066a9f1b9d 100644 --- a/azurerm/resource_arm_network_security_group.go +++ b/azurerm/resource_arm_network_security_group.go @@ -4,6 +4,7 @@ import ( "fmt" "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2017-09-01/network" + multierror "github.com/hashicorp/go-multierror" "github.com/hashicorp/terraform/helper/schema" "github.com/hashicorp/terraform/helper/validation" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils" @@ -33,7 +34,7 @@ func resourceArmNetworkSecurityGroup() *schema.Resource { "resource_group_name": resourceGroupNameSchema(), "security_rule": { - Type: schema.TypeList, + Type: schema.TypeSet, Optional: true, Computed: true, Elem: &schema.Resource{ @@ -50,30 +51,62 @@ func resourceArmNetworkSecurityGroup() *schema.Resource { }, "protocol": { - Type: schema.TypeString, - Required: true, - ValidateFunc: validateNetworkSecurityRuleProtocol, - StateFunc: ignoreCaseStateFunc, + Type: schema.TypeString, + Required: true, + ValidateFunc: validation.StringInSlice([]string{ + string(network.SecurityRuleProtocolAsterisk), + string(network.SecurityRuleProtocolTCP), + string(network.SecurityRuleProtocolUDP), + }, true), + DiffSuppressFunc: ignoreCaseDiffSuppressFunc, }, "source_port_range": { Type: schema.TypeString, - Required: true, + Optional: true, + }, + + "source_port_ranges": { + Type: schema.TypeSet, + Optional: true, + Elem: &schema.Schema{Type: schema.TypeString}, + Set: schema.HashString, }, "destination_port_range": { Type: schema.TypeString, - Required: true, + Optional: true, + }, + + "destination_port_ranges": { + Type: schema.TypeSet, + Optional: true, + Elem: &schema.Schema{Type: schema.TypeString}, + Set: schema.HashString, }, "source_address_prefix": { Type: schema.TypeString, - Required: true, + Optional: true, + }, + + "source_address_prefixes": { + Type: schema.TypeSet, + Optional: true, + Elem: &schema.Schema{Type: schema.TypeString}, + Set: schema.HashString, }, "destination_address_prefix": { Type: schema.TypeString, - Required: true, + Optional: true, + }, + + "destination_address_prefixes": { + Type: schema.TypeSet, + Optional: true, + Elem: &schema.Schema{Type: schema.TypeString}, + Set: schema.HashString, }, "access": { @@ -216,8 +249,89 @@ func resourceArmNetworkSecurityGroupDelete(d *schema.ResourceData, meta interfac return err } -func flattenNetworkSecurityRules(rules *[]network.SecurityRule) []interface{} { - result := make([]interface{}, 0) +func expandAzureRmSecurityRules(d *schema.ResourceData) ([]network.SecurityRule, error) { + sgRules := d.Get("security_rule").(*schema.Set) + rules := make([]network.SecurityRule, 0) + + for _, sgRaw := range sgRules.List() { + sgRule := sgRaw.(map[string]interface{}) + + if err := validateSecurityRule(sgRule); err != nil { + return nil, err + } + + name := sgRule["name"].(string) + source_port_range := sgRule["source_port_range"].(string) + destination_port_range := sgRule["destination_port_range"].(string) + source_address_prefix := sgRule["source_address_prefix"].(string) + destination_address_prefix := sgRule["destination_address_prefix"].(string) + priority := int32(sgRule["priority"].(int)) + access := sgRule["access"].(string) + direction := sgRule["direction"].(string) + protocol := sgRule["protocol"].(string) + + properties := network.SecurityRulePropertiesFormat{ + SourcePortRange: &source_port_range, + DestinationPortRange: &destination_port_range, + SourceAddressPrefix: &source_address_prefix, + DestinationAddressPrefix: &destination_address_prefix, + Priority: &priority, + Access: network.SecurityRuleAccess(access), + Direction: network.SecurityRuleDirection(direction), + Protocol: network.SecurityRuleProtocol(protocol), + } + + if v := sgRule["description"].(string); v != "" { + properties.Description = &v + } + + if r, ok := sgRule["source_port_ranges"].(*schema.Set); ok && r.Len() > 0 { + var sourcePortRanges []string + for _, v := range r.List() { + s := v.(string) + sourcePortRanges = append(sourcePortRanges, s) + } + properties.SourcePortRanges = &sourcePortRanges + } + + if r, ok := sgRule["destination_port_ranges"].(*schema.Set); ok && r.Len() > 0 { + var destinationPortRanges []string + for _, v := range r.List() { + s := v.(string) + destinationPortRanges = append(destinationPortRanges, s) + } + properties.DestinationPortRanges = &destinationPortRanges + } + + if r, ok := sgRule["source_address_prefixes"].(*schema.Set); ok && r.Len() > 0 { + var sourceAddressPrefixes []string + for _, v := range r.List() { + s := v.(string) + sourceAddressPrefixes = append(sourceAddressPrefixes, s) + } + properties.SourceAddressPrefixes = &sourceAddressPrefixes + } + + if r, ok := sgRule["destination_address_prefixes"].(*schema.Set); ok && r.Len() > 0 { + var destinationAddressPrefixes []string + for _, v := range r.List() { + s := v.(string) + destinationAddressPrefixes = append(destinationAddressPrefixes, s) + } + properties.DestinationAddressPrefixes = &destinationAddressPrefixes + } + + rules = append(rules, network.SecurityRule{ + Name: &name, + SecurityRulePropertiesFormat: &properties, + }) + } + + return rules, nil +} + +func flattenNetworkSecurityRules(rules *[]network.SecurityRule) *schema.Set { + result := resourceArmNetworkSecurityGroup().Schema["security_rule"].ZeroValue().(*schema.Set) if rules != nil { for _, rule := range *rules { @@ -225,74 +339,84 @@ func flattenNetworkSecurityRules(rules *[]network.SecurityRule) []interface{} { sgRule["name"] = *rule.Name if props := rule.SecurityRulePropertiesFormat; props != nil { + if props.Description != nil { + sgRule["description"] = *props.Description + } + if props.DestinationAddressPrefix != nil { sgRule["destination_address_prefix"] = *props.DestinationAddressPrefix } + if props.DestinationAddressPrefixes != nil { + sgRule["destination_address_prefixes"] = sliceToSet(*props.DestinationAddressPrefixes) + } if props.DestinationPortRange != nil { sgRule["destination_port_range"] = *props.DestinationPortRange } + if props.DestinationPortRanges != nil { + sgRule["destination_port_ranges"] = sliceToSet(*props.DestinationPortRanges) + } if props.SourceAddressPrefix != nil { sgRule["source_address_prefix"] = *props.SourceAddressPrefix } + if props.SourceAddressPrefixes != nil { + sgRule["source_address_prefixes"] = sliceToSet(*props.SourceAddressPrefixes) + } if props.SourcePortRange != nil { sgRule["source_port_range"] = *props.SourcePortRange } + if props.SourcePortRanges != nil { + sgRule["source_port_ranges"] = sliceToSet(*props.SourcePortRanges) + } + + sgRule["protocol"] = string(props.Protocol) sgRule["priority"] = int(*props.Priority) sgRule["access"] = string(props.Access) sgRule["direction"] = string(props.Direction) - sgRule["protocol"] = string(props.Protocol) - - if props.Description != nil { - sgRule["description"] = *props.Description - } } - result = append(result, sgRule) + result.Add(sgRule) } } return result } -func expandAzureRmSecurityRules(d *schema.ResourceData) ([]network.SecurityRule, error) { - sgRules := d.Get("security_rule").([]interface{}) - rules := make([]network.SecurityRule, 0) - - for _, sgRaw := range sgRules { - data := sgRaw.(map[string]interface{}) - - name := data["name"].(string) - source_port_range := data["source_port_range"].(string) - destination_port_range := data["destination_port_range"].(string) - source_address_prefix := data["source_address_prefix"].(string) - destination_address_prefix := data["destination_address_prefix"].(string) - priority := int32(data["priority"].(int)) - access := data["access"].(string) - direction := data["direction"].(string) - protocol := data["protocol"].(string) - - properties := network.SecurityRulePropertiesFormat{ - SourcePortRange: &source_port_range, - DestinationPortRange: &destination_port_range, - SourceAddressPrefix: &source_address_prefix, - DestinationAddressPrefix: &destination_address_prefix, - Priority: &priority, - Access: network.SecurityRuleAccess(access), - Direction: network.SecurityRuleDirection(direction), - Protocol: network.SecurityRuleProtocol(protocol), - } - - if v := data["description"].(string); v != "" { - properties.Description = &v - } - - rule := network.SecurityRule{ - Name: &name, - SecurityRulePropertiesFormat: &properties, - } +func sliceToSet(slice []string) *schema.Set { + set := &schema.Set{F: schema.HashString} + for _, v := range slice { + set.Add(v) + } + return set +} - rules = append(rules, rule) +func validateSecurityRule(sgRule map[string]interface{}) error { + var err *multierror.Error + + sourcePortRange := sgRule["source_port_range"].(string) + sourcePortRanges := sgRule["source_port_ranges"].(*schema.Set) + destinationPortRange := sgRule["destination_port_range"].(string) + destinationPortRanges := sgRule["destination_port_ranges"].(*schema.Set) + sourceAddressPrefix := sgRule["source_address_prefix"].(string) + sourceAddressPrefixes := sgRule["source_address_prefixes"].(*schema.Set) + destinationAddressPrefix := sgRule["destination_address_prefix"].(string) + destinationAddressPrefixes := sgRule["destination_address_prefixes"].(*schema.Set) + + if sourcePortRange != "" && sourcePortRanges.Len() > 0 { + err = multierror.Append(err, fmt.Errorf( + "only one of \"source_port_range\" and \"source_port_ranges\" can be used per security rule")) + } + if destinationPortRange != "" && destinationPortRanges.Len() > 0 { + err = multierror.Append(err, fmt.Errorf( + "only one of \"destination_port_range\" and \"destination_port_ranges\" can be used per security rule")) + } + if sourceAddressPrefix != "" && sourceAddressPrefixes.Len() > 0 { + err = multierror.Append(err, fmt.Errorf( + "only one of \"source_address_prefix\" and \"source_address_prefixes\" can be used per security rule")) + } + if destinationAddressPrefix != "" && destinationAddressPrefixes.Len() > 0 { + err = multierror.Append(err, fmt.Errorf( + "only one of \"destination_address_prefix\" and \"destination_address_prefixes\" can be used per security rule")) } - return rules, nil + return err.ErrorOrNil() } diff --git a/azurerm/resource_arm_network_security_group_test.go b/azurerm/resource_arm_network_security_group_test.go index 4d40b31990c0..e473b0ed9415 100644 --- a/azurerm/resource_arm_network_security_group_test.go +++ b/azurerm/resource_arm_network_security_group_test.go @@ -149,6 +149,25 @@ func TestAccAzureRMNetworkSecurityGroup_addingExtraRules(t *testing.T) { }) } +func TestAccAzureRMNetworkSecurityGroup_augmented(t *testing.T) { + resourceName := "azurerm_network_security_group.test" + rInt := acctest.RandInt() + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testCheckAzureRMNetworkSecurityGroupDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAzureRMNetworkSecurityGroup_augmented(rInt, testLocation()), + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMNetworkSecurityGroupExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "security_rule.#", "1"), + ), + }, + }, + }) +} + func testCheckAzureRMNetworkSecurityGroupExists(name string) resource.TestCheckFunc { return func(s *terraform.State) error { @@ -377,3 +396,30 @@ resource "azurerm_network_security_group" "test" { `, rInt, location) } + +func testAccAzureRMNetworkSecurityGroup_augmented(rInt int, location string) string { + return fmt.Sprintf(` +resource "azurerm_resource_group" "test" { + name = "acctestRG-%d" + location = "%s" +} + +resource "azurerm_network_security_group" "test" { + name = "acceptanceTestSecurityGroup1" + location = "${azurerm_resource_group.test.location}" + resource_group_name = "${azurerm_resource_group.test.name}" + + security_rule { + name = "test123" + priority = 100 + direction = "Inbound" + access = "Allow" + protocol = "Tcp" + source_port_ranges = [ "10000-40000" ] + destination_port_ranges = [ "80", "443", "8080", "8190" ] + source_address_prefixes = [ "10.0.0.0/8", "192.168.0.0/16" ] + destination_address_prefixes = [ "172.16.0.0/20", "8.8.8.8" ] + } +} +`, rInt, location) +} diff --git a/azurerm/resource_arm_network_security_rule.go b/azurerm/resource_arm_network_security_rule.go index df4f42fe49b4..71af8b0aee02 100644 --- a/azurerm/resource_arm_network_security_rule.go +++ b/azurerm/resource_arm_network_security_rule.go @@ -263,19 +263,19 @@ func resourceArmNetworkSecurityRuleRead(d *schema.ResourceData, meta interface{} d.Set("resource_group_name", resGroup) if props := resp.SecurityRulePropertiesFormat; props != nil { - d.Set("access", string(props.Access)) - d.Set("destination_address_prefix", props.DestinationAddressPrefix) - d.Set("destination_port_range", props.DestinationPortRange) - d.Set("direction", string(props.Direction)) d.Set("description", props.Description) - d.Set("priority", int(*props.Priority)) d.Set("protocol", string(props.Protocol)) + d.Set("destination_address_prefix", props.DestinationAddressPrefix) + d.Set("destination_address_prefixes", props.DestinationAddressPrefixes) + d.Set("destination_port_range", props.DestinationPortRange) + d.Set("destination_port_ranges", props.DestinationPortRanges) d.Set("source_address_prefix", props.SourceAddressPrefix) - d.Set("source_port_range", props.SourcePortRange) d.Set("source_address_prefixes", props.SourceAddressPrefixes) - d.Set("destination_address_prefixes", props.DestinationAddressPrefixes) + d.Set("source_port_range", props.SourcePortRange) d.Set("source_port_ranges", props.SourcePortRanges) - d.Set("destination_port_ranges", props.DestinationPortRanges) + d.Set("access", string(props.Access)) + d.Set("priority", int(*props.Priority)) + d.Set("direction", string(props.Direction)) } return nil diff --git a/website/docs/r/network_security_group.html.markdown b/website/docs/r/network_security_group.html.markdown index 99e12676c085..1f0fb61aaf49 100644 --- a/website/docs/r/network_security_group.html.markdown +++ b/website/docs/r/network_security_group.html.markdown @@ -69,13 +69,21 @@ The `security_rule` block supports: * `protocol` - (Required) Network protocol this rule applies to. Can be `Tcp`, `Udp` or `*` to match both. -* `source_port_range` - (Required) Source Port or Range. Integer or range between `0` and `65535` or `*` to match any. +* `source_port_range` - (Optional) Source Port or Range. Integer or range between `0` and `65535` or `*` to match any. This is required if `source_port_ranges` is not specified. -* `destination_port_range` - (Required) Destination Port or Range. Integer or range between `0` and `65535` or `*` to match any. +* `source_port_ranges` - (Optional) List of source ports or port ranges. This is required if `source_port_range` is not specified. -* `source_address_prefix` - (Required) CIDR or source IP range or * to match any IP. Tags such as `VirtualNetwork`, `AzureLoadBalancer` and `Internet` can also be used. +* `destination_port_range` - (Optional) Destination Port or Range. Integer or range between `0` and `65535` or `*` to match any. This is required if `destination_port_ranges` is not specified. -* `destination_address_prefix` - (Required) CIDR or destination IP range or * to match any IP. Tags such as `VirtualNetwork`, `AzureLoadBalancer` and `Internet` can also be used. +* `destination_port_ranges` - (Optional) List of destination ports or port ranges. This is required if `destination_port_range` is not specified. + +* `source_address_prefix` - (Optional) CIDR or source IP range or * to match any IP. Tags such as ‘VirtualNetwork’, ‘AzureLoadBalancer’ and ‘Internet’ can also be used. This is required if `source_address_prefixes` is not specified. + +* `source_address_prefixes` - (Optional) List of source address prefixes. Tags may not be used. This is required if `source_address_prefix` is not specified. + +* `destination_address_prefix` - (Optional) CIDR or destination IP range or * to match any IP. Tags such as ‘VirtualNetwork’, ‘AzureLoadBalancer’ and ‘Internet’ can also be used. This is required if `destination_address_prefixes` is not specified. + +* `destination_address_prefixes` - (Optional) List of destination address prefixes. Tags may not be used. This is required if `destination_address_prefix` is not specified. * `access` - (Required) Specifies whether network traffic is allowed or denied. Possible values are `Allow` and `Deny`.