Add Beta support & Beta feature deny to google_compute_firewall #282
Conversation
| @@ -5,6 +5,25 @@ import ( | |||
| "google.golang.org/api/compute/v1" | |||
| ) | |||
|
|
|||
| func computeSharedOperationWait(config *Config, op interface{}, project string, activity string) error { | |||
There was a problem hiding this comment.
Don't have to do this in this PR, but I'd like to see the beta operations get simplified like the v1 ones did
There was a problem hiding this comment.
For sure - what I'd like to do is convert the Operation object to v1 and call the v1 waits, like in #187
Otherwise, we can just copy compute_operation.go again.
| if err != nil { | ||
| return fmt.Errorf("Error deleting firewall: %s", err) | ||
| var op interface{} | ||
| switch computeApiVersion { |
There was a problem hiding this comment.
Hmm I wonder- do deletes actually need to be done with a particular version of the api?
There was a problem hiding this comment.
I'm honestly not sure; for now, for completeness for now, I'd like to perform all operations at the resource's API version; we'll be able to bring deletes of versioned resources either down to v1 or up to v0beta every time later, unless we adopt a multiversioned service generated by #240 which makes this mostly a non-issue.
| @@ -302,11 +457,12 @@ func resourceFirewall( | |||
| } | |||
|
|
|||
| // Build the firewall parameter | |||
| return &compute.Firewall{ | |||
| return &computeBeta.Firewall{ | |||
| Name: d.Get("name").(string), | |||
| Description: d.Get("description").(string), | |||
| Network: network.SelfLink, | |||
There was a problem hiding this comment.
Does this self link need to be converted to v1? Also, do we actually need to read the network at both versions or can we just read it at v1 since all we need is the self link?
There was a problem hiding this comment.
This variable never gets stored in state, since we do that in Read.
We can just read at v1; done.
| resource.TestStep{ | ||
| Config: testAccComputeFirewall_denied(networkName, firewallName), | ||
| Check: resource.ComposeTestCheckFunc( | ||
| testAccCheckComputeFirewallExists("google_compute_firewall.foobar", &firewall), |
There was a problem hiding this comment.
Do you want to check that the firewall also has the deny properties we expect?
There was a problem hiding this comment.
make testacc TEST=./google TESTARGS='-run=TestAccComputeFirewall'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./google -v -run=TestAccComputeFirewall -timeout 120m
=== RUN TestAccComputeFirewall_importBasic
--- PASS: TestAccComputeFirewall_importBasic (53.67s)
=== RUN TestAccComputeFirewall_basic
--- PASS: TestAccComputeFirewall_basic (50.55s)
=== RUN TestAccComputeFirewall_update
--- PASS: TestAccComputeFirewall_update (85.31s)
=== RUN TestAccComputeFirewall_noSource
--- PASS: TestAccComputeFirewall_noSource (58.25s)
=== RUN TestAccComputeFirewall_denied
--- PASS: TestAccComputeFirewall_denied (50.27s)
PASS
ok github.com/terraform-providers/terraform-provider-google/google 298.205s
…icorp#282) * Add versioned Beta support to google_compute_firewall. * Add Beta support for deny to google_compute_firewall. * remove extra line: * make fmt * Add missing ForceNew fields. * Respond to review comments testing functionality + reducing network GET to v1
…icorp#282) * Add versioned Beta support to google_compute_firewall. * Add Beta support for deny to google_compute_firewall. * remove extra line: * make fmt * Add missing ForceNew fields. * Respond to review comments testing functionality + reducing network GET to v1
…icorp#282) * Add versioned Beta support to google_compute_firewall. * Add Beta support for deny to google_compute_firewall. * remove extra line: * make fmt * Add missing ForceNew fields. * Respond to review comments testing functionality + reducing network GET to v1
<!-- This change is generated by MagicModules. --> /cc @chrisst
|
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
locked and limited conversation to collaborators
google_compute_firewallto a versioned resource with Beta support.denytogoogle_compute_firewalldenyis not able to be updated by the API yet.I'm not sure what to do here docs-wise where a Beta feature makes a GA field no longer Required; one of
allowordenyis, so I keptallowrequired in docs and saiddenycan replace it. Let me know your thoughts.Fixes #148.
Related to #93