Firewall network field now supports self_link in addition of name #477
Conversation
| @@ -299,7 +331,7 @@ func testAccComputeFirewall_update(network, firewall string) string { | |||
| resource "google_compute_firewall" "foobar" { | |||
| name = "firewall-test-%s" | |||
| description = "Resource created for Terraform acceptance testing" | |||
| network = "${google_compute_network.foobar.name}" | |||
| network = "${google_compute_network.foobar.self_link}" | |||
There was a problem hiding this comment.
Note: Other tests are referencing the network by name.
| // - projects/myproject/global/networks/my-network | ||
| // - global/networks/my-network (default project is used) | ||
| // - my-network (default project is used) | ||
| func ParseNetworkFieldValue(network string, config *Config) *NetworkFieldValue { |
There was a problem hiding this comment.
Brainstorming outloud here; none of these are action items:
I wonder if there's a way we can eventually merge all the url parsing code; unfortunately it doesn't seem obvious how since each has their own path scheme. Right now the code is relatively short and simple so the cost of having it isn't much.
Maybe at some point we should combine them all into a single namespace? urltools or resourcetools or something?
There was a problem hiding this comment.
Agree. I think once we have created a few of these FieldValue classes, we will see patterns emerging and we can create common utils to make it easy to create these FieldValue classes.
google/field_helpers.go
Outdated
|
|
||
| const networkLinkTemplate = "projects/%s/global/networks/%s" | ||
|
|
||
| var networkLinkRegex = regexp.MustCompile(fmt.Sprintf(networkLinkTemplate, "(.+)", "(.+)")) |
There was a problem hiding this comment.
This made me quite uncomfortable; something about printing regex chunks into another regex made me squirm a bit
How do you feel about just inlining the regex here?
google/field_helpers.go
Outdated
| } | ||
| } | ||
|
|
||
| func (f *NetworkFieldValue) RelativeLink() string { |
There was a problem hiding this comment.
Minor quibble; if the struct is small it's nice to not use a ptr here for the receiving struct; that way its obvious that your method doesn't mutate the struct its called on (there's runtime benefits as well but I don't think we should be concerned with that)
| @@ -405,12 +404,6 @@ func resourceComputeFirewallDelete(d *schema.ResourceData, meta interface{}) err | |||
|
|
|||
| func resourceFirewall(d *schema.ResourceData, meta interface{}, computeApiVersion ComputeApiVersion) (*computeBeta.Firewall, error) { | |||
| config := meta.(*Config) | |||
| project, _ := getProject(d, config) | |||
|
|
|||
| network, err := config.clientCompute.Networks.Get(project, d.Get("network").(string)).Do() | |||
There was a problem hiding this comment.
nice, you eliminated an API call with this change :)
There was a problem hiding this comment.
Yup. And the other good thing is that sometimes, this call was failing if the user didn't have the iam permission for reading the network! It was kind of unexpected since we are setting up a firewall.
google/field_helpers.go
Outdated
|
|
||
| const networkLinkTemplate = "projects/%s/global/networks/%s" | ||
|
|
||
| var networkLinkRegex = regexp.MustCompile(fmt.Sprintf(networkLinkTemplate, "(.+)", "(.+)")) |
| // - projects/myproject/global/networks/my-network | ||
| // - global/networks/my-network (default project is used) | ||
| // - my-network (default project is used) | ||
| func ParseNetworkFieldValue(network string, config *Config) *NetworkFieldValue { |
There was a problem hiding this comment.
Agree. I think once we have created a few of these FieldValue classes, we will see patterns emerging and we can create common utils to make it easy to create these FieldValue classes.
| @@ -405,12 +404,6 @@ func resourceComputeFirewallDelete(d *schema.ResourceData, meta interface{}) err | |||
|
|
|||
| func resourceFirewall(d *schema.ResourceData, meta interface{}, computeApiVersion ComputeApiVersion) (*computeBeta.Firewall, error) { | |||
| config := meta.(*Config) | |||
| project, _ := getProject(d, config) | |||
|
|
|||
| network, err := config.clientCompute.Networks.Get(project, d.Get("network").(string)).Do() | |||
There was a problem hiding this comment.
Yup. And the other good thing is that sometimes, this call was failing if the user didn't have the iam permission for reading the network! It was kind of unexpected since we are setting up a firewall.
google/field_helpers.go
Outdated
| } | ||
| } | ||
|
|
||
| func (f *NetworkFieldValue) RelativeLink() string { |
Autogen for region read, cloud scheduler examples
|
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 👉 [email protected]. Thanks! |
ghost
locked and limited conversation to collaborators
Fixes #377.
networkfield for other resources will be able to reuseNetworkFieldValue. #431 is listing the other resources with a network field that is not properly supported.