Zero out upload url before update

[slevenick]

Fixes: hashicorp/terraform-provider-google#7921

If this PR is for Terraform, I acknowledge that I have:

• Searched through the issue tracker for an open issue that this either resolves or contributes to, commented on it to claim it, and written "fixes {url}" or "part of {url}" in this PR description. If there were no relevant open issues, I opened one and commented that I would like to work on it (not necessary for very small changes).
• Generated Terraform, and ran make test and make lint to ensure it passes unit and linter tests.
• Ensured that all new fields I added that can be set by a user appear in at least one example (for generated resources) or third_party test (for handwritten resources or update tests).
• Ran relevant acceptance tests (If the acceptance tests do not yet pass or you are unable to run them, please let your reviewer know).
• Read the Release Notes Guide before writing my release note below.

Release Note Template for Downstream PRs (will be copied)

cloudfunctions: fixed a bug where `google_cloudfunctions_function` would sometimes fail to update after being imported from gcloud

[modular-magician]

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 1 file changed, 4 insertions(+))
Terraform Beta: Diff ( 1 file changed, 4 insertions(+))

[modular-magician]

I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=161965"

[modular-magician]

I have triggered VCR tests in RECORDING mode for the following tests that failed during VCR: TestAccDataSourceComposerEnvironment_basic|TestAccDataSourceSpannerInstance_basic|TestAccActiveDirectoryDomainTrust_activeDirectoryDomainTrustBasicExample|TestAccBigqueryDataTransferConfig|TestAccCloudSchedulerJob_schedulerJobHttpExample|TestAccComposerEnvironment_withUpdateOnCreate|TestAccFilestoreInstance_filestoreInstanceBasicExample|TestAccFilestoreInstance_update|TestAccOSLoginSSHPublicKey_osLoginSshKeyExpiry You can view the result here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=162049"

[melinath]

Is there a use case we need to support that requires SourceUploadUrl?

i.e. it looks like source_archive_* and source_repository are both optional (though they conflict.) Could a user reasonably create a cloudfunctions function with neither one set and expect it to use the source they uploaded, instead?

Would it be at all possible to make sure there's a test that covers this use case? It's sort of a weird situation...

[slevenick]

Is there a use case we need to support that requires SourceUploadUrl?

i.e. it looks like source_archive_* and source_repository are both optional (though they conflict.) Could a user reasonably create a cloudfunctions function with neither one set and expect it to use the source they uploaded, instead?

Would it be at all possible to make sure there's a test that covers this use case? It's sort of a weird situation...

I believe that either of source_repository or both of source_archive_* must be specified: https://github.com/GoogleCloudPlatform/magic-modules/blob/master/third_party/terraform/resources/resource_cloudfunctions_function.go#L372

This is only enforced on create, but I believe it makes sense for Terraform to effectively require one of those.

I think that the behavior on import for unsupported fields on resources is somewhat undefined. For fields that are marked as Computed in the schema we would not overwrite existing values, but for non Computed fields we may change them if they are not specified in the users config. Because it's not specified and we require the user specify a source when the function is created via TF I'm not particularly worried about this change

We could theoretically support SourceUploadUrl via https://cloud.google.com/functions/docs/reference/rest/v1/projects.locations.functions/generateUploadUrl but it looks like more trouble than it would be worth when the combination of bucket+object handles the .zip file use case.

I don't believe it's possible to write a test for this use case using Terraform. It may be possible using a local exec provider that would call gcloud directly to create the resource to be imported, but shelling out to other tools is troublesome

[melinath]

I'm sorry for the delay on this review; this slipped through the cracks somehow. I generally try to respond within 24 hours - feel free to ping me any time but especially past that point!

LGTM