vcd_dnat missing separate original and translated port in vCloud Director provider

[pasikarkkainen]

Currently Terraform vCloud Director provider allows defining dnat rule like this:

resource "vcd_dnat" "web" {
edge_gateway = "Edge Gateway Name"
external_ip = "78.101.10.20"
port = 80
internal_ip = "10.10.0.5"
}

The obvious problem here is that we only have one definition of "port", and thus the "original port" and "translated port" end up being the same value. This isn't optimal.

Quite often one needs to be able to create dnat rules like these:
external port 2001 --> internal IP1 port 22.
external port 2002 --> internal IP2 port 22.
external port 2003 --> internal IP3 port 22.

Currently it's not possible to define such dnat rules with vCloud Director provider in terraform.

This limitation seems to come from the underlying vcd go library used: vmware-govcd edgegateway.go func AddNATMapping(nattype, externalIP, internalIP, port string) :

//add rule
natRule := &types.NatRule{
        RuleType:  nattype,
        IsEnabled: true,
        GatewayNatRule: &types.GatewayNatRule{
                Interface: &types.Reference{
                        HREF: uplink.HREF,
                },
                OriginalIP:     externalIP,
                OriginalPort:   port,
                TranslatedIP:   internalIP,
                TranslatedPort: port,
                Protocol:       "tcp",
        },
}

so "port" is used for both OriginalPort and TranslatedPort.. that needs to be fixed / improved first in the govcd library, and then in terraform vcd provider.

[pasikarkkainen]

Upstream govcloudair issue: vmware-archive/govcloudair#37

[robcoward]

Hi @pasikarkkainen as per my comment on vmware-archive/govcloudair#37 I would try to maintain backwards compatibility in the govcloudair library by renaming the current function to a new AddNATPortMapping() function and adding your enhancements, with a new wrapper AddNATMapping() function just passing through the old format parameters. Then you can update terraform to use the new AddNATPortMapping() function.

Again, to maintain backwards compatibility, my suggested approach would be to leave the existing port attribute in the vcd_dnat resource but making it optional / mutually exclusive with 2 new internal_port and external_port attributes.

The documentation could then show that these two options are equivalent, and in time depricate the original port attribute:

resource "vcd_dnat" "web" {
  edge_gateway = "Edge Gateway Name"
  external_ip = "78.101.10.20"
  port = 80
  internal_ip = "10.10.0.5"
}

resource "vcd_dnat" "web" {
  edge_gateway = "Edge Gateway Name"
  external_ip = "78.101.10.20"
  external_port = 80
  internal_ip = "10.10.0.5"
  internal_port = 80
}

[pasikarkkainen]

Makes sense. I'm hoping to get to this next week. Thanks!

[pasikarkkainen]

@robcoward : It seems govcloudair library NATPortMapping PR has been merged to UKCloud fork of govcloudair.

For the terraform vcd_provider parts.. should I send a PR against upstream terraform, or the UKCloud fork? What's your preference? Thanks!

[robcoward]

Hi @pasikarkkainen, my colleague @tlawrence beat me to hitting the merge button. Please submit any PR for the vcd_provider to the upstream hashicorp/terraform repo.

[Boran]

Hi, Could someone help me to understand? The issue need to be fixed in govcloud, but how and when is that pulled into terraform. Can one track that? Does each terraform release automatically pull in the latest govcloud?

[robcoward]

HI @Boran, I suggest you have a read of the documentation, specifically https://github.com/hashicorp/terraform/#dependencies - it tells you all you need.
To summarise for those who can't be bothered though:

• Fork and branch your dependency (in this case govcloudair from UKCloud's fork), making whatever updates you require.
• Raise a PR to get your change merged back into master for the dependency.
• Fork and branch the terraform code.
• After your PR for the dependency has been merged, use govendor to update the vendor/ directories, committing changes to your branch.
• Raise a PR to get the vendor updates merged into terraform's master
• Wait for Hashicorp to release the next build of Terraform.

[pasikarkkainen]

Hey @Boran . Yes, there is now support for separate original_port and translated_port in the UKCloud fork of govcloud library, but the Terraform parts to use that new feature haven't been done yet. I've been busy with other stuff. I expect to get the Terraform parts done some day in the future..

Are you interested in contributing/helping?

[Boran]

Thanks @robcoward.
@pasikarkkainen: I'll see what I can do, once I've understand the build/test and PR process and found out where to look in the code. I found vendor/github.com/vmware/govmomi too. If I have questions regarding the build process if there a place I can ask without annoying people?
I have access to a Vmware Vcenter and vcloud so at least I run tests properly.

Update: So my understand is that I develop as follows..
Have forked and checked out TF, did "make plugin-dev PLUGIN=provider-vcd" to build with that provider only. Then started playing with changes in builtin/providers/vcd/resource_vcd_dnat.go
Make then gives the error "edgeGateway.AddNATPortMapping undefined", as expected since that lib call is new. So, next wipe "vendor/github.com/ukcloud" and "govendor fetch github.com/UKCloud/govcloudair" to get the latest, and make again. No errors, make creates gocode/bin/terraform-provider-vcd.
Next create a .terraformrc with providers { vcd = "/Users/foo/gocode/bin/terraform-provider-vcd }
And finally do a "terraform apply" to start using the new changes.

Moved work to a PR, if either of you would like to give me dev tips, that probably a better place to do it than here.

[Boran]

So, PR ready, feedback appreciated.

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.