Merge pull request #1736 from terraform-providers/lb_nat_rule-updates

Update azurerm_lb_nat_rule with azurestasck PR review requests

azurerm/helpers/validate/network.go

@@ -45,3 +45,17 @@ func MACAddress(i interface{}, k string) (_ []string, errors []error) {
 
 	return
 }
+
+func PortNumber(i interface{}, k string) (_ []string, errors []error) {
+	v, ok := i.(int)
+	if !ok {
+		errors = append(errors, fmt.Errorf("expected type of %q to be int", k))
+		return
+	}
+
+	if v < 0 || 65535 < v {
+		errors = append(errors, fmt.Errorf("%q is not a valid port number: %q", k, i))
+	}
+
+	return
+}

azurerm/helpers/validate/network_test.go

@@ -1,6 +1,9 @@
 package validate
 
-import "testing"
+import (
+	"strconv"
+	"testing"
+)
 
 func TestIPv4Address(t *testing.T) {
 	cases := []struct {
@@ -147,3 +150,49 @@ func TestMACAddress(t *testing.T) {
 		})
 	}
 }
+
+func TestPortNumber(t *testing.T) {
+	cases := []struct {
+		Port   int
+		Errors int
+	}{
+		{
+			Port:   -1,
+			Errors: 1,
+		},
+		{
+			Port:   0,
+			Errors: 0,
+		},
+		{
+			Port:   1,
+			Errors: 0,
+		},
+		{
+			Port:   8477,
+			Errors: 0,
+		},
+		{
+			Port:   65535,
+			Errors: 0,
+		},
+		{
+			Port:   65536,
+			Errors: 1,
+		},
+		{
+			Port:   7000000,
+			Errors: 1,
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(strconv.Itoa(tc.Port), func(t *testing.T) {
+			_, errors := PortNumber(tc.Port, "test")
+
+			if len(errors) != tc.Errors {
+				t.Fatalf("Expected PortNumber to return %d error(s) not %d", len(errors), tc.Errors)
+			}
+		})
+	}
+}

azurerm/resource_arm_loadbalancer_nat_rule.go

@@ -9,41 +9,52 @@ import (
 	"github.com/hashicorp/errwrap"
 	"github.com/hashicorp/terraform/helper/resource"
 	"github.com/hashicorp/terraform/helper/schema"
+	"github.com/hashicorp/terraform/helper/validation"
+	"github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/azure"
+	"github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/validate"
 	"github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils"
 )
 
 func resourceArmLoadBalancerNatRule() *schema.Resource {
 	return &schema.Resource{
-		Create: resourceArmLoadBalancerNatRuleCreate,
+		Create: resourceArmLoadBalancerNatRuleCreateUpdate,
 		Read:   resourceArmLoadBalancerNatRuleRead,
-		Update: resourceArmLoadBalancerNatRuleCreate,
+		Update: resourceArmLoadBalancerNatRuleCreateUpdate,
 		Delete: resourceArmLoadBalancerNatRuleDelete,
+
 		Importer: &schema.ResourceImporter{
 			State: loadBalancerSubResourceStateImporter,
 		},
 
 		Schema: map[string]*schema.Schema{
 			"name": {
-				Type:     schema.TypeString,
-				Required: true,
-				ForceNew: true,
+				Type:         schema.TypeString,
+				Required:     true,
+				ForceNew:     true,
+				ValidateFunc: validation.NoZeroValues,
 			},
 
 			"location": deprecatedLocationSchema(),
 
 			"resource_group_name": resourceGroupNameSchema(),
 
 			"loadbalancer_id": {
-				Type:     schema.TypeString,
-				Required: true,
-				ForceNew: true,
+				Type:         schema.TypeString,
+				Required:     true,
+				ForceNew:     true,
+				ValidateFunc: azure.ValidateResourceID,
 			},
 
 			"protocol": {
 				Type:             schema.TypeString,
 				Required:         true,
 				StateFunc:        ignoreCaseStateFunc,
 				DiffSuppressFunc: ignoreCaseDiffSuppressFunc,
+				ValidateFunc: validation.StringInSlice([]string{
+					string(network.TransportProtocolAll),
+					string(network.TransportProtocolTCP),
+					string(network.TransportProtocolUDP),
+				}, true),
 			},
 
 			"enable_floating_ip": {
@@ -53,18 +64,21 @@ func resourceArmLoadBalancerNatRule() *schema.Resource {
 			},
 
 			"frontend_port": {
-				Type:     schema.TypeInt,
-				Required: true,
+				Type:         schema.TypeInt,
+				Required:     true,
+				ValidateFunc: validate.PortNumber,
 			},
 
 			"backend_port": {
-				Type:     schema.TypeInt,
-				Required: true,
+				Type:         schema.TypeInt,
+				Required:     true,
+				ValidateFunc: validate.PortNumber,
 			},
 
 			"frontend_ip_configuration_name": {
-				Type:     schema.TypeString,
-				Required: true,
+				Type:         schema.TypeString,
+				Required:     true,
+				ValidateFunc: validation.NoZeroValues,
 			},
 
 			"frontend_ip_configuration_id": {
@@ -80,7 +94,7 @@ func resourceArmLoadBalancerNatRule() *schema.Resource {
 	}
 }
 
-func resourceArmLoadBalancerNatRuleCreate(d *schema.ResourceData, meta interface{}) error {
+func resourceArmLoadBalancerNatRuleCreateUpdate(d *schema.ResourceData, meta interface{}) error {
 	client := meta.(*ArmClient).loadBalancerClient
 	ctx := meta.(*ArmClient).StopContext
 
@@ -108,7 +122,7 @@ func resourceArmLoadBalancerNatRuleCreate(d *schema.ResourceData, meta interface
 	existingNatRule, existingNatRuleIndex, exists := findLoadBalancerNatRuleByName(loadBalancer, d.Get("name").(string))
 	if exists {
 		if d.Get("name").(string) == *existingNatRule.Name {
-			// this probe is being updated/reapplied remove old copy from the slice
+			// this nat rule is being updated/reapplied remove old copy from the slice
 			natRules = append(natRules[:existingNatRuleIndex], natRules[existingNatRuleIndex+1:]...)
 		}
 	}
@@ -278,8 +292,7 @@ func expandAzureRmLoadBalancerNatRule(d *schema.ResourceData, lb *network.LoadBa
 	}
 
 	if v, ok := d.GetOk("enable_floating_ip"); ok {
-		enableFloatingIP := v.(bool)
-		properties.EnableFloatingIP = utils.Bool(enableFloatingIP)
+		properties.EnableFloatingIP = utils.Bool(v.(bool))
 	}
 
 	if v := d.Get("frontend_ip_configuration_name").(string); v != "" {
@@ -288,11 +301,9 @@ func expandAzureRmLoadBalancerNatRule(d *schema.ResourceData, lb *network.LoadBa
 			return nil, fmt.Errorf("[ERROR] Cannot find FrontEnd IP Configuration with the name %s", v)
 		}
 
-		feip := network.SubResource{
+		properties.FrontendIPConfiguration = &network.SubResource{
 			ID: rule.ID,
 		}
-
-		properties.FrontendIPConfiguration = &feip
 	}
 
 	natRule := network.InboundNatRule{

website/docs/r/loadbalancer_nat_rule.html.markdown

@@ -8,7 +8,7 @@ description: |-
 
 # azurerm_lb_nat_rule
 
-Create a LoadBalancer NAT Rule.
+Manages a LoadBalancer NAT Rule.
 
 ~> **NOTE** When using this resource, the LoadBalancer needs to have a FrontEnd IP Configuration Attached
 
@@ -57,7 +57,7 @@ The following arguments are supported:
 * `resource_group_name` - (Required) The name of the resource group in which to create the resource.
 * `loadbalancer_id` - (Required) The ID of the LoadBalancer in which to create the NAT Rule.
 * `frontend_ip_configuration_name` - (Required) The name of the frontend IP configuration exposing this rule.
-* `protocol` - (Required) The transport protocol for the external endpoint. Possible values are `Udp` or `Tcp`.
+* `protocol` - (Required) The transport protocol for the external endpoint. Possible values are `Udp`, `Tcp` or `All`.
 * `frontend_port` - (Required) The port for the external endpoint. Port numbers for each Rule must be unique within the Load Balancer. Possible values range between 1 and 65534, inclusive.
 * `backend_port` - (Required) The port used for internal connections on the endpoint. Possible values range between 1 and 65535, inclusive.
 * `enable_floating_ip` - (Optional) Enables the Floating IP Capacity, required to configure a SQL AlwaysOn Availability Group.