terraform-google-modules / terraform-google-cloud-nat

Creates and configures Cloud NAT
https://registry.terraform.io/modules/terraform-google-modules/cloud-nat/google
Apache License 2.0
81 stars 68 forks source link

feat(removes nat_ip_allocate_option)!: add dynamic port mapping #73

Closed nikhilmakhijani closed 1 year ago

nikhilmakhijani commented 2 years ago

This MR includes the following changes: 1) Adding dynamic port mapping functionality 2) Remove nat_ip_allocation_option variable

github-actions[bot] commented 2 years ago

This PR is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 7 days

nikhilmakhijani commented 2 years ago

Please review @bharathkkb

comment-bot-dev commented 1 year ago

@nikhilmakhijani Thanks for the PR! 🚀
✅ Lint checks have passed.

github-actions[bot] commented 1 year ago

This PR is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 7 days

gnubibi33 commented 1 year ago

Any news ?

apeabody commented 1 year ago

/gcbrun

apeabody commented 1 year ago

Thanks for the contribution @nikhilmakhijani! I've triggered the CI, in the meantime can all the remaining comment threads be closed?

apeabody commented 1 year ago

Hi @nikhilmakhijani, relevant output from the INT cI:

    subnetworks_test.go:45: 
            Error Trace:    /workspace/test/integration/subnetworks/subnetworks_test.go:45
                                        /builder/home/go/pkg/mod/github.com/!google!cloud!platform/cloud-foundation-toolkit/infra/blueprint-test@v0.5.0/pkg/tft/terraform.go:412
                                        /builder/home/go/pkg/mod/github.com/!google!cloud!platform/cloud-foundation-toolkit/infra/blueprint-test@v0.5.0/pkg/tft/terraform.go:432
                                        /builder/home/go/pkg/mod/github.com/!google!cloud!platform/cloud-foundation-toolkit/infra/blueprint-test@v0.5.0/pkg/utils/stages.go:31
                                        /builder/home/go/pkg/mod/github.com/!google!cloud!platform/cloud-foundation-toolkit/infra/blueprint-test@v0.5.0/pkg/tft/terraform.go:432
            Error:          Not equal: 
                            expected: map[string]interface {}{"enableEndpointIndependentMapping":true, "endpointTypes":[]interface {}{"ENDPOINT_TYPE_VM"}, "icmpIdleTimeoutSec":15, "minPortsPerVm":128, "name":"my-cloud-nat-cft-cloud-nat-test-RANDOM_ID", "natIpAllocateOption":"MANUAL_ONLY", "sourceSubnetworkIpRangesToNat":"LIST_OF_SUBNETWORKS", "subnetworks":[]interface {}{map[string]interface {}{"name":"https://www.googleapis.com/compute/v1/projects/PROJECT_ID/regions/us-east4/subnetworks/cft-cloud-nat-test-RANDOM_ID-a", "sourceIpRangesToNat":[]interface {}{"ALL_IP_RANGES"}}}, "tcpEstablishedIdleTimeoutSec":600, "tcpTimeWaitTimeoutSec":120, "tcpTransitoryIdleTimeoutSec":15, "udpIdleTimeoutSec":15}
                            actual  : map[string]interface {}{"enableDynamicPortAllocation":false, "enableEndpointIndependentMapping":true, "endpointTypes":[]interface {}{"ENDPOINT_TYPE_VM"}, "icmpIdleTimeoutSec":15, "minPortsPerVm":128, "name":"my-cloud-nat-cft-cloud-nat-test-RANDOM_ID", "natIpAllocateOption":"MANUAL_ONLY", "sourceSubnetworkIpRangesToNat":"LIST_OF_SUBNETWORKS", "subnetworks":[]interface {}{map[string]interface {}{"name":"https://www.googleapis.com/compute/v1/projects/PROJECT_ID/regions/us-east4/subnetworks/cft-cloud-nat-test-RANDOM_ID-a", "sourceIpRangesToNat":[]interface {}{"ALL_IP_RANGES"}}}, "tcpEstablishedIdleTimeoutSec":600, "tcpTimeWaitTimeoutSec":120, "tcpTransitoryIdleTimeoutSec":15, "udpIdleTimeoutSec":15}

                            Diff:
                            --- Expected
                            +++ Actual
                            @@ -1,2 +1,3 @@
                            -(map[string]interface {}) (len=12) {
                            +(map[string]interface {}) (len=13) {
                            + (string) (len=27) "enableDynamicPortAllocation": (bool) false,
                              (string) (len=32) "enableEndpointIndependentMapping": (bool) true,
            Test:           TestSubnetworks
apeabody commented 1 year ago

/gcbrun

apeabody commented 1 year ago

/gcbrun

sclausson commented 1 year ago

@apeabody any updates on this one? Would be great to be able to add dynamic port mapping to the module.

apeabody commented 1 year ago

/gcbrun

apeabody commented 1 year ago

@apeabody any updates on this one? Would be great to be able to add dynamic port mapping to the module.

Hi @sclausson - I've retriggered the CI, but I suspect this is still relevant https://github.com/terraform-google-modules/terraform-google-cloud-nat/pull/73#issuecomment-1483077913

nikhilmakhijani commented 1 year ago

/gcbrun

nikhilmakhijani commented 1 year ago

@apeabody any updates on this one? Would be great to be able to add dynamic port mapping to the module.

Hi @sclausson - I've retriggered the CI, but I suspect this is still relevant #73 (comment)

Hi @apeabody I have updated the test case. Could you please check again?

apeabody commented 1 year ago

/gcbrun

apeabody commented 1 year ago

Hi @nikhilmakhijani here is the current INT output:

TestAdvanced 2023-05-11T16:34:02Z command.go:185: "cloud-foundation-cicd"
    advanced_test.go:45: 
            Error Trace:    /workspace/test/integration/advanced/advanced_test.go:45
                                        /builder/home/go/pkg/mod/github.com/!google!cloud!platform/cloud-foundation-toolkit/infra/blueprint-test@v0.5.1/pkg/tft/terraform.go:408
                                        /builder/home/go/pkg/mod/github.com/!google!cloud!platform/cloud-foundation-toolkit/infra/blueprint-test@v0.5.1/pkg/tft/terraform.go:428
                                        /builder/home/go/pkg/mod/github.com/!google!cloud!platform/cloud-foundation-toolkit/infra/blueprint-test@v0.5.1/pkg/utils/stages.go:31
                                        /builder/home/go/pkg/mod/github.com/!google!cloud!platform/cloud-foundation-toolkit/infra/blueprint-test@v0.5.1/pkg/tft/terraform.go:428
            Error:          Not equal: 
                            expected: map[string]interface {}{"enableEndpointIndependentMapping":true, "endpointTypes":[]interface {}{"ENDPOINT_TYPE_VM"}, "icmpIdleTimeoutSec":15, "minPortsPerVm":128, "name":"my-cloud-nat-cft-cloud-nat-test-RANDOM_ID", "natIpAllocateOption":"MANUAL_ONLY", "sourceSubnetworkIpRangesToNat":"ALL_SUBNETWORKS_ALL_IP_RANGES", "tcpEstablishedIdleTimeoutSec":600, "tcpTimeWaitTimeoutSec":240, "tcpTransitoryIdleTimeoutSec":15, "udpIdleTimeoutSec":15}
                            actual  : map[string]interface {}{"enableDynamicPortAllocation":false, "enableEndpointIndependentMapping":true, "endpointTypes":[]interface {}{"ENDPOINT_TYPE_VM"}, "icmpIdleTimeoutSec":15, "minPortsPerVm":128, "name":"my-cloud-nat-cft-cloud-nat-test-RANDOM_ID", "natIpAllocateOption":"MANUAL_ONLY", "sourceSubnetworkIpRangesToNat":"ALL_SUBNETWORKS_ALL_IP_RANGES", "tcpEstablishedIdleTimeoutSec":600, "tcpTimeWaitTimeoutSec":240, "tcpTransitoryIdleTimeoutSec":15, "udpIdleTimeoutSec":15}

                            Diff:
                            --- Expected
                            +++ Actual
                            @@ -1,2 +1,3 @@
                            -(map[string]interface {}) (len=11) {
                            +(map[string]interface {}) (len=12) {
                            + (string) (len=27) "enableDynamicPortAllocation": (bool) false,
                              (string) (len=32) "enableEndpointIndependentMapping": (bool) true,
            Test:           TestAdvanced
apeabody commented 1 year ago

/gcbrun

apeabody commented 1 year ago

/gcbrun