Add ability to set priority on compute_firewall

[selmanj]

Closes #342

[rosbo]

The import test is failing.

You can either update the configuration to set explicitly the priority on the configuration to import or use ImportStateVerifyIgnore in the TestStep.

make testacc TESTARGS="-run TestAccComputeFirewall_" TEST="./google"
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./google -v -run TestAccComputeFirewall_ -timeout 120m
=== RUN   TestAccComputeFirewall_importBasic
--- FAIL: TestAccComputeFirewall_importBasic (58.64s)
	testing.go:435: Step 1 error: ImportStateVerify attributes not equivalent. Difference is shown below. Top is actual, bottom is expected.

		(map[string]string) (len=1) {
		 (string) (len=8) "priority": (string) (len=1) "0"
		}


		(map[string]string) (len=1) {
		 (string) (len=8) "priority": (string) (len=4) "1000"
		}

=== RUN   TestAccComputeFirewall_basic
--- PASS: TestAccComputeFirewall_basic (66.31s)
=== RUN   TestAccComputeFirewall_update
--- PASS: TestAccComputeFirewall_update (73.46s)
=== RUN   TestAccComputeFirewall_priority
--- PASS: TestAccComputeFirewall_priority (52.01s)
=== RUN   TestAccComputeFirewall_noSource
--- PASS: TestAccComputeFirewall_noSource (52.18s)
=== RUN   TestAccComputeFirewall_denied
--- PASS: TestAccComputeFirewall_denied (59.78s)
=== RUN   TestAccComputeFirewall_egress
--- PASS: TestAccComputeFirewall_egress (59.78s)
FAIL
exit status 1
FAIL	github.com/terraform-providers/terraform-provider-google/google	422.298s
make: *** [testacc] Error 1

[selmanj]

Good catch, I forgot to run the import test.

Interesting why this is failing. I believe its due to resourceComputeFirewallRead making a choice about api version to use; as this is an import, there's no v0beta features in use, so it chooses v1. Then it converts this resource to a v0beta resource by dumping to json and reparsing. This means it never sees priority (and annoyingly, the default value for int64 is 0, which is a valid value for priority).

When a user imports a firewall, which version should we use to read? The code currently decides which version based on what's set in the schema, but when importing should we instead look at the remote api and see if any features are in use?

[selmanj]

Spoke to @paddycarver, we decided that the current behavior is correct (when importing, default to non-beta apis). To fix the issue I decided to explicitly set Priority to the default when converting the struct.

[ghost]

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!