-
Notifications
You must be signed in to change notification settings - Fork 1.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add new resource_compute_router_nat to Terraform provider #907
Add new resource_compute_router_nat to Terraform provider #907
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resource looks good. Looking forward to seeing the tests/docs :)
third_party/terraform/tests/resource_compute_router_nat_test.go
Outdated
Show resolved
Hide resolved
Added documentation and tests, PTAL |
third_party/terraform/tests/resource_compute_router_nat_test.go
Outdated
Show resolved
Hide resolved
third_party/terraform/website/docs/r/compute_router_nat.html.markdown
Outdated
Show resolved
Hide resolved
third_party/terraform/website/docs/r/compute_router_nat.html.markdown
Outdated
Show resolved
Hide resolved
routersService := config.clientCompute.Routers | ||
|
||
for _, rs := range s.RootModule().Resources { | ||
if rs.Type != "google_compute_router" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this supposed to be router or router nat?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it can be either at this line, but you ultimately have to check that the Router has been deleted. I think the idea is that since the Nat is just a block within a Router, you need to check that the Router has been deleted at the end of the test.
third_party/terraform/website/docs/r/compute_router_nat.html.markdown
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I responded to all your questions. Let me know once this is ready to review again!
PTAL. |
I am a robot that works on MagicModules PRs! I built this PR into one or more PRs on other repositories, and when those are closed, this PR will also be merged and closed. |
subnetworkToNat.SecondaryIpRangeNames = convertStringSet(v.(*schema.Set)) | ||
} | ||
result = append(result, &subnetworkToNat) | ||
} | ||
|
||
return result | ||
} | ||
|
||
func flattenRouterNatSubnetworkToNatBeta(subnetworksToNat []*computeBeta.RouterNatSubnetworkToNat) []map[string]interface{} { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI - I had to re-add this because directly assigning the block didn't work:
2018/11/30 00:36:07 [DEBUG] Unlocking "router/us-central1/router-nat-test-74df4tnpuu"
2018/11/30 00:36:07 [DEBUG] Unlocked "router/us-central1/router-nat-test-74df4tnpuu"
panic: Invalid address to set: []string{"subnetwork", "0", "SourceIpRangesToNat"}
goroutine 378 [running]:
github.com/terraform-providers/terraform-provider-google-beta/vendor/github.com/hashicorp/terraform/helper/schema.(*ResourceData).Set(0xc0001eeb60, 0x1ebb8ed, 0xa, 0x19adc40, 0xc0005750e0, 0x0, 0x0)
/usr/local/google/home/shikhman/go/src/github.com/terraform-providers/terraform-provider-google-beta/vendor/github.com/hashicorp/terraform/helper/schema/resource_data.go:191 +0x334
github.com/terraform-providers/terraform-provider-google-beta/google-beta.resourceComputeRouterNatRead(0xc0001eeb60, 0x1aff700, 0xc000280780, 0xf, 0x1ec1847)
/usr/local/google/home/shikhman/go/src/github.com/terraform-providers/terraform-provider-google-beta/google-beta/resource_compute_router_nat.go:253 +0x8fb
I am (still) a robot that works on MagicModules PRs! I just wanted to let you know that your changes (as of commit 064636d) have been included in your existing downstream PRs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks @cornmander!
The GCP API is OK with empty fields so we can unconditionally set most fields. Also fixes some indentation in the documentation.
Tracked submodules are build/terraform-beta build/terraform build/ansible build/inspec.
e3b98ed
to
cfc4d97
Compare
Add new resource_compute_router_nat to beta Terraform provider
[all]
[terraform]
[terraform-beta]
Add support for Cloud NAT
[ansible]
[inspec]