DNAT rule support for both OriginalPort and TranslatedPort

[pasikarkkainen]

It seems edgegateway.go func AddNATMapping(nattype, externalIP, internalIP, port string) lack support for being able to specify separate OriginalPort and TranslatedPort. Currently "port" parameter is used for both:

//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",
        },
}

It'd be great to be able to add more flexible DNAT port mappings.. thoughts?

[pasikarkkainen]

What's the correct way of fixing this?

Should I perhaps rename the existing AddNATMapping() to something else, say AddNATMappingPorts(), then make it accept separate "originalPort" and "translatedPort" parameters, and then add AddNATMapping() again for backwards "compatibility", making it call the new AddNATMappingPorts() function giving the same "port" parameter as both "originalPort" and "translatedPort" ?

[robcoward]

Hi @pasikarkkainen, my suggested approach would be to try and keep backwards compatibility for other non-terraform users of the govcloudair library by renaming the current AddNATMapping() function to AddNATPortMapping() and extending the parameters to take src/dest ports, then creating a new wrapper function to replace AddNATMapping() than passed the port through to the new AddNATPortMapping() function as both the src & dst ports:

func (e *EdgeGateway) AddNATMapping(nattype, externalIP, internalIP, port string) (Task, error) {
    return e.AddNATPortMapping(nattype,externalIP,port,internalIP,port)
}

func (e *EdgeGateway) AddNATPortMapping(nattype, externalIP, externalPort string, internalIP, internalPort string) (Task, error) {
...
// Update to the original AddNATMapping() function
    natRule := &types.NatRule{
        RuleType:  nattype,
        IsEnabled: true,
        GatewayNatRule: &types.GatewayNatRule{
            Interface: &types.Reference{
                HREF: uplink.HREF,
            },
            OriginalIP:     externalIP,
            OriginalPort:   externalPort,
            TranslatedIP:   internalIP,
            TranslatedPort: internalPort,
            Protocol:       "tcp",
        },
    }

Please can you submit any PRs against the https://github.com/UKCloud/govcloudair fork of this vmware repo as we are in the process of enhancing the vcloud provider and have updated terraform to pull in our fork. We'll be more than happy to merge your changes. Any work done in our fork will be pushed back upstream to vmware's repo in due course.

[pasikarkkainen]

PR submitted here: https://github.com/UKCloud/govcloudair/pull/3

[pasikarkkainen]

@robcoward: I kept the backwards compatibility as you suggested, so existing AddNATMapping() and RemoveNATMapping() functions call the newly introduced AddNATPortMapping() and RemoveNATPortMapping().