Fix for protocol upper/lower case handling

CHANGELOG.md

@@ -18,18 +18,23 @@ FEATURES:
 * `resource/vcd_org_vdc` added Org VDC update and full state read - [GH-275]
 * `resource/vcd_org_vdc` added Org VDC metadata support - [GH-276]
 * `resource/vcd_snat` added ability to choose network name and type. [GH-282] 
-* `resource/vcd_dnat` added ability to choose network name and type. [GH-282]
+* `resource/vcd_dnat` added ability to choose network name and type. [GH-282, GH-292, GH-293]
 
 IMPROVEMENTS:
 * `resource/vcd_org_vdc`: Fix ignoring of resource guarantee values - [GH-265]
 * `resource/vcd_org_vdc`: Org VDC state ID changed from name to vCD ID - [GH-275]
 * Change resource handling to use locking mechanism when resource parallel handling is not supported by vCD. [GH-255] 
 * Fix issue when vApp is power cycled during member VM deletion. [GH-261]
+* `resource/vcd_dnat`, `resource/vcd_snat` has got full read functionality. This means that on the next `plan/apply` it will detect if configuration has changed in vCD and propose to update it.
 
 BUG FIXES:
 
 * `resource/vcd_dnat and resource/vcd_snat` - fix resource destroy as it would still leave NAT rule in edge gateway. Fix works if network_name and network_type is used. [GH-282]
 
+NOTES:
+* `resource/vcd_dnat` `protocol` requires lower case values to be consistent with the underlying NSX API. This may result in invalid configuration if upper case was used previously!
+* `resource/vcd_dnat` default value for `protocol` field changed from upper case `TCP` to lower case `tcp`, which may result in a single update when running `plan` on a configuration with a state file from an older version.
+
 ## 2.3.0 (May 29, 2019)
 
 IMPROVEMENTS:

go.mod

@@ -4,5 +4,5 @@ go 1.12
 
 require (
 	github.com/hashicorp/terraform v0.12.0
-	github.com/vmware/go-vcloud-director/v2 v2.3.0-rc.2
+	github.com/vmware/go-vcloud-director/v2 v2.3.0
 )

go.sum

@@ -301,8 +301,8 @@ github.com/ulikunitz/xz v0.5.5/go.mod h1:2bypXElzHzzJZwzH67Y6wb67pO62Rzfn7BSiF4A
 github.com/vmihailenco/msgpack v3.3.3+incompatible/go.mod h1:fy3FlTQTDXWkZ7Bh6AcGMlsjHatGryHQYUTf1ShIgkk=
 github.com/vmihailenco/msgpack v4.0.1+incompatible h1:RMF1enSPeKTlXrXdOcqjFUElywVZjjC6pqse21bKbEU=
 github.com/vmihailenco/msgpack v4.0.1+incompatible/go.mod h1:fy3FlTQTDXWkZ7Bh6AcGMlsjHatGryHQYUTf1ShIgkk=
-github.com/vmware/go-vcloud-director/v2 v2.3.0-rc.2 h1:8Uh2iKTlsr8aBjgRCoFSYLBVN+oSMEpoye2lr9vP3u0=
-github.com/vmware/go-vcloud-director/v2 v2.3.0-rc.2/go.mod h1:+Hq7ryFfgZqsO6mXH29RQFnpIMSujCOMI57otHoXHhQ=
+github.com/vmware/go-vcloud-director/v2 v2.3.0 h1:o2iZxqZB4j1V/9oEaQYYwG5H+3/t8iqkTBziP05XpuE=
+github.com/vmware/go-vcloud-director/v2 v2.3.0/go.mod h1:+Hq7ryFfgZqsO6mXH29RQFnpIMSujCOMI57otHoXHhQ=
 github.com/xanzy/ssh-agent v0.2.1/go.mod h1:mLlQY/MoOhWBj+gOGMQkOeiEvkx+8pJSI+0Bx9h2kr4=
 github.com/xiang90/probing v0.0.0-20160813154853-07dd2e8dfe18/go.mod h1:UETIi67q53MR2AWcXfiuqkDkRtnGDLqkBTpCHuJHxtU=
 github.com/xlab/treeprint v0.0.0-20161029104018-1d6e34225557/go.mod h1:ce1O1j6UtZfjr22oyGxGLbauSBp2YVXpARAosm7dHBg=

vcd/resource_vcd_dnat.go

@@ -66,9 +66,10 @@ func resourceVcdDNAT() *schema.Resource {
 				Required: true,
 			},
 			"protocol": &schema.Schema{
-				Type:     schema.TypeString,
-				Optional: true,
-				Default:  "TCP", // keep back compatibility as was hardcoded previously
+				Type:         schema.TypeString,
+				Optional:     true,
+				Default:      "tcp", // keep back compatibility as was hardcoded previously
+				ValidateFunc: validateCase("lower"),
 			},
 			"icmp_sub_type": &schema.Schema{
 				Type:     schema.TypeString,
@@ -130,6 +131,10 @@ func resourceVcdDNATCreate(d *schema.ResourceData, meta interface{}) error {
 			return fmt.Errorf("error creating DNAT rule: %#v", err)
 		}
 	} else if networkName != "" && networkType == "ext" {
+		if !vcdClient.Client.IsSysAdmin {
+			return fmt.Errorf("functionality requires system administrator privileges")
+		}
+
 		externalNetwork, err := govcd.GetExternalNetwork(vcdClient.VCDClient, networkName)
 		if err != nil {
 			return fmt.Errorf("unable to find external network: %s, err: %s", networkName, err)
@@ -204,22 +209,29 @@ func resourceVcdDNATRead(d *schema.ResourceData, meta interface{}) error {
 		d.Set("icmp_sub_type", natRule.GatewayNatRule.IcmpSubType)
 		d.Set("network_name", natRule.GatewayNatRule.Interface.Name)
 
-		orgVdcNetwork, _ := getOrgVdcNetwork(d, vcdClient, natRule.GatewayNatRule.Interface.Name)
+		orgVdcNetwork, err := getOrgVdcNetwork(d, vcdClient, natRule.GatewayNatRule.Interface.Name)
 		if orgVdcNetwork != nil {
 			d.Set("network_type", "org")
-			found = true
+		} else {
+			log.Printf("[DEBUG] didn't find org VDC network with name: %s, %#v", natRule.GatewayNatRule.Interface.Name, err)
 		}
+
 		_, extNetwErr := govcd.GetExternalNetwork(vcdClient.VCDClient, natRule.GatewayNatRule.Interface.Name)
 		if extNetwErr == nil {
 			d.Set("network_type", "ext")
-			found = true
 		} else {
 			log.Printf("[DEBUG] didn't find external network with name: %s, %#v", natRule.GatewayNatRule.Interface.Name, extNetwErr)
-			found = false
 		}
+
 		if orgVdcNetwork != nil && extNetwErr == nil {
 			return fmt.Errorf("found external network or org VCD network with same name: %s", natRule.GatewayNatRule.Interface.Name)
 		}
+
+		if orgVdcNetwork == nil && extNetwErr != nil {
+			return fmt.Errorf("issue updating resource state. Didn't find external network or org VCD network with name: %s", natRule.GatewayNatRule.Interface.Name)
+		}
+
+		return nil
 	} else {
 		// TODO remove when major release is done
 		for _, r := range edgeGateway.EdgeGateway.Configuration.EdgeGatewayServiceConfiguration.NatService.NatRule {
@@ -233,6 +245,7 @@ func resourceVcdDNATRead(d *schema.ResourceData, meta interface{}) error {
 	}
 
 	if !found {
+		log.Printf("[INFO] Removing from state.")
 		d.SetId("")
 	}
 

vcd/resource_vcd_dnat_test.go

@@ -4,7 +4,6 @@ package vcd
 
 import (
 	"fmt"
-	"regexp"
 	"testing"
 
 	"github.com/hashicorp/terraform/helper/resource"
@@ -49,8 +48,7 @@ func TestAccVcdDNAT_WithOrgNetw(t *testing.T) {
 		CheckDestroy: testAccCheckVcdDNATDestroy,
 		Steps: []resource.TestStep{
 			resource.TestStep{
-				Config:      configText,
-				ExpectError: regexp.MustCompile(`After applying this step and refreshing, the plan was not empty:`),
+				Config: configText,
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckVcdDNATExists("vcd_dnat."+dnatName, &e),
 					resource.TestCheckResourceAttr(
@@ -61,6 +59,8 @@ func TestAccVcdDNAT_WithOrgNetw(t *testing.T) {
 						"vcd_dnat."+dnatName, "external_ip", testConfig.Networking.ExternalIp),
 					resource.TestCheckResourceAttr(
 						"vcd_dnat."+dnatName, "port", "7777"),
+					resource.TestCheckResourceAttr(
+						"vcd_dnat."+dnatName, "protocol", "tcp"),
 					resource.TestCheckResourceAttr(
 						"vcd_dnat."+dnatName, "internal_ip", "10.10.102.60"),
 					resource.TestCheckResourceAttr(
@@ -108,8 +108,7 @@ func TestAccVcdDNAT_WithExtNetw(t *testing.T) {
 		CheckDestroy: testAccCheckVcdDNATDestroy,
 		Steps: []resource.TestStep{
 			resource.TestStep{
-				Config:      configText,
-				ExpectError: regexp.MustCompile(`After applying this step and refreshing, the plan was not empty:`),
+				Config: configText,
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckVcdDNATExists("vcd_dnat."+dnatName, &e),
 					resource.TestCheckResourceAttr(
@@ -120,6 +119,8 @@ func TestAccVcdDNAT_WithExtNetw(t *testing.T) {
 						"vcd_dnat."+dnatName, "external_ip", testConfig.Networking.ExternalIp),
 					resource.TestCheckResourceAttr(
 						"vcd_dnat."+dnatName, "port", "7777"),
+					resource.TestCheckResourceAttr(
+						"vcd_dnat."+dnatName, "protocol", "tcp"),
 					resource.TestCheckResourceAttr(
 						"vcd_dnat."+dnatName, "internal_ip", "10.10.102.60"),
 					resource.TestCheckResourceAttr(
@@ -129,8 +130,7 @@ func TestAccVcdDNAT_WithExtNetw(t *testing.T) {
 				),
 			},
 			resource.TestStep{
-				Config:      updateText,
-				ExpectError: regexp.MustCompile(`After applying this step and refreshing, the plan was not empty:`),
+				Config: updateText,
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckVcdDNATExists("vcd_dnat."+dnatName, &e),
 					resource.TestCheckResourceAttr(
@@ -141,6 +141,8 @@ func TestAccVcdDNAT_WithExtNetw(t *testing.T) {
 						"vcd_dnat."+dnatName, "external_ip", testConfig.Networking.ExternalIp),
 					resource.TestCheckResourceAttr(
 						"vcd_dnat."+dnatName, "port", "8888"),
+					resource.TestCheckResourceAttr(
+						"vcd_dnat."+dnatName, "protocol", "udp"),
 					resource.TestCheckResourceAttr(
 						"vcd_dnat."+dnatName, "internal_ip", "10.10.102.80"),
 					resource.TestCheckResourceAttr(
@@ -379,6 +381,7 @@ resource "vcd_dnat" "{{.DnatName}}" {
   edge_gateway    = "{{.EdgeGateway}}"
   external_ip     = "{{.ExternalIp}}"
   port            = 7777
+  protocol        = "tcp"
   internal_ip     = "10.10.102.60"
   translated_port = 77
   description     = "{{.Description}}"
@@ -393,6 +396,7 @@ resource "vcd_dnat" "{{.DnatName}}" {
   network_type    = "ext"
   edge_gateway    = "{{.EdgeGateway}}"
   external_ip     = "{{.ExternalIp}}"
+  protocol        = "udp"
   port            = 8888
   internal_ip     = "10.10.102.80"
   translated_port = 88

vendor/modules.txt

@@ -216,7 +216,7 @@ github.com/ulikunitz/xz/internal/hash
 # github.com/vmihailenco/msgpack v4.0.1+incompatible
 github.com/vmihailenco/msgpack
 github.com/vmihailenco/msgpack/codes
-# github.com/vmware/go-vcloud-director/v2 v2.3.0-rc.2
+# github.com/vmware/go-vcloud-director/v2 v2.3.0
 github.com/vmware/go-vcloud-director/v2/govcd
 github.com/vmware/go-vcloud-director/v2/types/v56
 github.com/vmware/go-vcloud-director/v2/util

website/docs/index.html.markdown

@@ -12,7 +12,7 @@ The VMware vCloud Director provider is used to interact with the resources suppo
 
 Use the navigation to the left to read about the available resources.
 
-~> **NOTE:** The VMware vCloud Director Provider went through a refresh at the beginning of 2019 and some semantic changes were made compared to the previously available initial version. Please check docs for *v2.0+*, *v2.1+*, *v2.2+* labels and your existing .tf configuration files carefully when shifting to this new version. 
+~> **NOTE:** The VMware vCloud Director Provider went through a refresh at the beginning of 2019 and some semantic changes were made compared to the previously available initial version. Please check docs for *v2.0+*, *v2.1+*, *v2.2+*, *v2.4+* labels and your existing .tf configuration files carefully when shifting to this new version. 
 
 ## Supported vCD Versions
 

website/docs/r/dnat.html.markdown

@@ -11,7 +11,9 @@ description: |-
 Provides a vCloud Director DNAT resource. This can be used to create, modify,
 and delete destination NATs to map an external IP/port to an internal IP/port.
 
-!> **Warning:** When advanced edge gateway is used and the rule is updated using UI, then Id mapping will be lost and terraform won't find the rule anymore and remove it from state.  
+~> **Note:** From v2.4+ `protocol` requires lower case values. This may result in invalid configuration if upper case was used previously.
+
+!> **Warning:** When advanced edge gateway is used and the rule is updated using UI, then ID mapping will be lost and Terraform won't find the rule anymore and remove it from state.  
 
 ## Example Usage
 
@@ -38,7 +40,7 @@ resource "vcd_dnat" "forIcmp" {
   external_ip   = "78.101.10.20"
   port          = -1                    # "-1" == "any"
   internal_ip   = "10.10.0.5"
-  protocol      = "ICMP"
+  protocol      = "icmp"
   icmp_sub_type = "router-solicitation"
 }
 ```
@@ -52,9 +54,10 @@ The following arguments are supported:
 * `port` - (Required) The port number to map. -1 translates to "any"
 * `translated_port` - (Optional) The port number to map
 * `internal_ip` - (Required) The IP of the VM to map to
-* `protocol` - (Optional; *v2.0+*) The protocol type. Possible values are TCP, UDP, TCPUDP, ICMP, ANY. TCP is default to be backward compatible with previous version
+* `protocol` - (Optional; *v2.0+*) The protocol type. Possible values are `tcp`, `udp`, `tcpupd`, `icmp`, `any`. `tcp` is default to be backward compatible with previous version
 * `icmp_sub_type` - (Optional; *v2.0+*) The name of ICMP type. Possible values are   address-mask-request, destination-unreachable, echo-request, echo-reply, parameter-problem, redirect, router-advertisement, router-solicitation, source-quench, time-exceeded, timestamp-request, timestamp-reply, any
-* `network_type` - (Optional; *v2.4+*) Type of the network on which to apply the NAT rule. Possible values org or ext. *`network_type` will be a required field in the next major version.*
+* `network_type` - (Optional; *v2.4+*) Type of the network on which to apply the NAT rule. Possible values `org` or `ext`. `ext` requires system administrator privileges. *`network_type` will be a required field in the next major version.*
 * `network_name` - (Optional; *v2.4+*) The name of the network on which to apply the SNAT. *`network_name` will be a required field in the next major version.*
 * `org` - (Optional; *v2.0+*) The name of organization to use, optional if defined at provider level. Useful when connected as sysadmin working across different organisations
-* `vdc` - (Optional; *v2.0+*) The name of VDC to use, optional if defined at provider level
+* `vdc` - (Optional; *v2.0+*) The name of VDC to use, optional if defined at provider level
+* `description` - (Optional; *v2.4+*) - Description of item