-
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 purpose field to google compute address #2284
Add purpose field to google compute address #2284
Conversation
Hi! I'm the modular magician, I work on Magic Modules. Pull request statusesNo diff detected in Inspec. New Pull RequestsI built this PR into one or more new PRs on other repositories, and when those are closed, this PR will also be merged and closed. |
As I'm looking over the downstreams, maybe the example isn't necessary for the documentation. Should I create a separate test for this, or is it minor enough that it wouldn't need a specific acceptance test? |
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 don't see the harm in adding the test as an example! I find one of our most common user asks is for more examples. + I'd rather we generate the test than not. If we end up with an unwieldy number of examples on resources, we can add a flag to omit certain examples from the docs in the future.
I think your findings about the purpose are correct, and agree with what you think of network
on hashicorp/terraform-provider-google#4166.
@@ -0,0 +1,14 @@ | |||
// All internal addresses default to a purpose of `GCE_ENDPOINT`, but it is not | |||
// allowed on external addresses, so we can't set the default on the field. | |||
func addressDiffSuppress(k, old, new string, d *schema.ResourceData) bool { |
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.
We could implement this as a CustomizeDiff
as well I think, setting the new
value to the default when address_type
is internal. It's little nicer than a DSF imo, because Terraform shows what it will do in plan, but isn't necessary. (this is very much a stretch thing, and feel free to ignore it. Historically we've been a little squeamish about CD, so we don't have great examples of using it so I over-suggest it on PRs sometimes.)
Hi! I'm the modular magician, I work on Magic Modules. Pull request statusesterraform-provider-google-beta already has an open PR. New Pull RequestsI didn't open any new pull requests because of this PR. |
Fixes hashicorp/terraform-provider-google#4167
The API documentation for both
address
andglobal address
list the same available values forpurpose
-GCE_ENDPOINT
,DNS_RESOLVER
,VPC_PEERING
andNAT_AUTO
. I tried adding these as options here, however, while testing I received the error thatDNS_RESOLVER
needs to be created by Cloud DNS,NAT_AUTO
cannot be created manually, but is automatically created when a NAT is configured, andVPC_PEERING
is only available on global addresses. So I went to look at thegoogle_compute_global_address
resource, and although the GCP API documentation shows all 4 options there as well, the Terraform resource only allowsVPC_PEERING
, so I decided to only allowGCE_ENDPOINT
on thegoogle_compute_address
resource.If I am correct in my assumption, I think hashicorp/terraform-provider-google#4166 can only be implemented with a global address (purpose
VPC_PEERING
) and can be closed as well.Release Note for Downstream PRs (will be copied)