Skip to content
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

rm deprecated parameter from forwarding_rule example #5758

Closed
wants to merge 1 commit into from

Conversation

rremer
Copy link
Contributor

@rremer rremer commented Feb 25, 2020

Documentation update.

This field 'allow_global_access' does not exist in any version of the provider I can find. Additionally, not addressed by this commit, is that there is some out-of-date (or generated?) description of this field here: https://www.terraform.io/docs/providers/google/r/compute_forwarding_rule.html#allow_global_access

@ghost ghost added the size/xs label Feb 25, 2020
@ghost ghost requested review from megan07 February 25, 2020 18:42
@ghost ghost added the documentation label Feb 25, 2020
@rremer
Copy link
Contributor Author

rremer commented Feb 25, 2020

added a reversion of 713012c, which broke ci - that commit is documenting a resource which doesn't exist yet (seems like @modular-magician automagically added this because the terraform-provider-google-beta has this resource).

@rremer rremer force-pushed the doc-allow_global_access branch from 713012c to f8d9708 Compare March 2, 2020 18:46
@rremer
Copy link
Contributor Author

rremer commented Mar 2, 2020

removed my second commit, which was done later by modular-magician in 1f9e0cb

Copy link
Contributor

@megan07 megan07 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your contribution! Let me know if you're having issues running the example and we can try and troubleshoot what the issue might be. I noticed that it is now available in the v1 API, so I think we could even propagate it to google rather than only google-beta if that is what you are looking for.

Thanks!

@@ -50,7 +50,6 @@ resource "google_compute_forwarding_rule" "default" {
load_balancing_scheme = "INTERNAL"
backend_service = "${google_compute_region_backend_service.backend.self_link}"
all_ports = true
allow_global_access = true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

allow_global_access is an option on the google-beta provider as you can see here: https://github.com/terraform-providers/terraform-provider-google-beta/blob/master/google-beta/resource_compute_forwarding_rule.go#L112

And it looks like it is available in the API as well, so I think it should be fine. https://cloud.google.com/compute/docs/reference/rest/beta/forwardingRules

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just beta, but if we're now documenting beta-only features on that page without specifying them as beta-only, that's fine I guess; just confusing.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HI @rremer, I'm sorry for the confusion! We try to label beta-only fields as (Optional, Beta), and it appears this field was labeled that way. However, if that didn't read as beta-only to you, I would love some feedback on what kind of documentation you would like for that. Thanks!

@rremer rremer closed this Mar 2, 2020
@ghost
Copy link

ghost commented Apr 2, 2020

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 feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 hashibot-feedback@hashicorp.com. Thanks!

@ghost ghost locked and limited conversation to collaborators Apr 2, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants