Add Snapshot location to compute snapshot

[upodroid]

Fixes: hashicorp/terraform-provider-google#6941

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)

compute: added storage_locations field to `google_compute_snapshot`

compute: added `kms_key_service_account`, `kms_key_self_link ` fields to `snapshot_encryption_key` field in `google_compute_snapshot`

compute: added `source_disk_encryption_key.kms_key_service_account` field to `google_compute_snapshot`

[modular-magician]

Hello! I am a robot who works on Magic Modules PRs.

I have detected that you are a community contributor, so your PR will be assigned to someone with a commit-bit on this repo for initial review.

Thanks for your contribution! A human will be with you soon.

@megan07, please review this PR or find an appropriate assignee.

[modular-magician]

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

Diff report:

Terraform GA: Diff ( 2 files changed, 70 insertions(+))
Terraform Beta: Diff ( 2 files changed, 70 insertions(+))
Ansible: Diff ( 2 files changed, 81 insertions(+), 6 deletions(-))
TF Conversion: Diff ( 1 file changed, 22 insertions(+))
Inspec: Diff ( 6 files changed, 17 insertions(+))

[megan07]

Hi @upodroid! Thanks for your contribution! Would you be able to add some tests/examples with these please?

[upodroid]

I didn't write a test for the KMS fields. I don't currently see one using CMEK, there is a CSEK test though.

[modular-magician]

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

Diff report:

Terraform GA: Diff ( 3 files changed, 84 insertions(+))
Terraform Beta: Diff ( 3 files changed, 84 insertions(+))
Ansible: Diff ( 2 files changed, 89 insertions(+), 6 deletions(-))
TF Conversion: Diff ( 1 file changed, 32 insertions(+))
TF OiCS: Diff ( 1 file changed, 1 insertion(+))
Inspec: Diff ( 6 files changed, 17 insertions(+))

[megan07]

Hi @upodroid! Thanks again for this contribution - and I apologize for my delay in responding, I wanted to make sure I had ample time to review it.

As for tests, I think you can probably add tests to resource_compute_snapshot_test.go - for examples on how to use the kms key, take a look at TestAccComputeDisk_encryptionKMS maybe?

Also, I was playing around with it a little and I think if storage_locations is not set, there is a default returned by the API, so we might want to make that Optional + Computed.

Let me know if you have more questions! Thanks! And sorry again for the delay!

[upodroid]

@megan07 The tests are kind of passing now.

The APIs are being a bit dodgy. The service account was used to encrypt the disk first and then create the snapshot. The CMEK works for the disk creation but fails the snapshot creation even thought it happens after the disk is created.

I tried it manually via the APIs and it works correctly. Terraform is a bit flaky.

 REDACTED  MCW0CDP3YY  ~  go  …  github.com  hashicorp  terraform-provider-google-beta   master  29⬇  8✎  8+  USAGE  $    make testacc TEST=./google-beta TESTARGS='-run=TestAccComputeSnapshot_encryptionCMEK'
==> Checking source code against gofmt...
==> Checking that code complies with gofmt requirements...
TF_ACC=1 TF_SCHEMA_PANIC_ON_ERROR=1 go test ./google-beta -v -run=TestAccComputeSnapshot_encryptionCMEK -timeout 240m -ldflags="-X=github.com/hashicorp/terraform-provider-google-beta/version.ProviderVersion=acc"
=== RUN   TestAccComputeSnapshot_encryptionCMEK
=== PAUSE TestAccComputeSnapshot_encryptionCMEK
=== CONT  TestAccComputeSnapshot_encryptionCMEK
    TestAccComputeSnapshot_encryptionCMEK: testing.go:674: Step 0 error: errors during apply:
        
        Error: Error creating Snapshot: googleapi: Error 400: Cloud KMS error when using key projects/REDACTED/locations/us-central1/keyRings/tf-test-tuc2a1pnxq/cryptoKeys/tf-test-tuc2a1pnxq/cryptoKeyVersions/1: Permission 'cloudkms.cryptoKeyVersions.useToDecrypt' denied on resource 'projects/REDACTED/locations/us-central1/keyRings/tf-test-tuc2a1pnxq/cryptoKeys/tf-test-tuc2a1pnxq' (or it may not exist)., kmsPermissionDenied
        
          on /var/folders/b1/dthn83bs2qbcrg38qszm22440000gn/T/tf-test534309074/main.tf line 39:
          (source code not available)
        
        
--- FAIL: TestAccComputeSnapshot_encryptionCMEK (56.02s)
FAIL
FAIL    github.com/hashicorp/terraform-provider-google-beta/google-beta 60.536s
FAIL
make: *** [testacc] Error 1

[modular-magician]

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

Diff report:

Terraform GA: Diff ( 6 files changed, 217 insertions(+), 2 deletions(-))
Terraform Beta: Diff ( 6 files changed, 217 insertions(+), 2 deletions(-))
Ansible: Diff ( 4 files changed, 103 insertions(+), 18 deletions(-))
TF Conversion: Diff ( 2 files changed, 54 insertions(+))
TF OiCS: Diff ( 1 file changed, 1 insertion(+))
Inspec: Diff ( 8 files changed, 22 insertions(+))

[megan07]

After making a few of the suggested changes, I ran the tests locally and ran into a couple other things - the Disk resource has a decoder (templates/terraform/decoders/disk.erb) that decodes the encryption keys, the child attributes are hard-coded in that decoder, so now that we've added kmsKeyServiceAccount to it's children, we'll need to make sure it's added there as well.

Along the same lines, you'll see in that decoder that one of the things decoded is that the kmsKeyName actually returns the version, so we handle that in there - and we need to do the same thing in a decoder for Snapshot now as well.

Let me know if you have any questions, but I think this should get us close! Thanks for all of your work put in here!

[megan07]

One other thing, as I was looking into the code locally, I saw some extra whitespace at the end of lines, would you mind cleaning that up a bit too, please? Thanks!

[upodroid]

This is working now. That decoder hint gave me ideas on how to fix PR 3939.

REDACTED  MCW0CDP3YY  ~  go  …  github.com  hashicorp  terraform-provider-google-beta   master  37⬇  10✎  USAGE  $    make testacc TEST=./google-beta TESTARGS='-run=TestAccComputeSnapshot_encryptionCMEK'
==> Checking source code against gofmt...
==> Checking that code complies with gofmt requirements...
TF_ACC=1 TF_SCHEMA_PANIC_ON_ERROR=1 go test ./google-beta -v -run=TestAccComputeSnapshot_encryptionCMEK -timeout 240m -ldflags="-X=github.com/hashicorp/terraform-provider-google-beta/version.ProviderVersion=acc"
=== RUN   TestAccComputeSnapshot_encryptionCMEK
=== PAUSE TestAccComputeSnapshot_encryptionCMEK
=== CONT  TestAccComputeSnapshot_encryptionCMEK
--- PASS: TestAccComputeSnapshot_encryptionCMEK (91.82s)
PASS
ok      github.com/hashicorp/terraform-provider-google-beta/google-beta 97.315s

[modular-magician]

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

Diff report:

Terraform GA: Diff ( 7 files changed, 289 insertions(+), 6 deletions(-))
Terraform Beta: Diff ( 7 files changed, 289 insertions(+), 6 deletions(-))
Ansible: Diff ( 4 files changed, 113 insertions(+), 24 deletions(-))
TF Conversion: Diff ( 2 files changed, 54 insertions(+))
TF OiCS: Diff ( 1 file changed, 1 insertion(+))
Inspec: Diff ( 8 files changed, 24 insertions(+), 2 deletions(-))

[modular-magician]

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

Diff report:

Terraform GA: Diff ( 7 files changed, 260 insertions(+), 6 deletions(-))
Terraform Beta: Diff ( 7 files changed, 260 insertions(+), 6 deletions(-))
Ansible: Diff ( 4 files changed, 99 insertions(+), 12 deletions(-))
TF Conversion: Diff ( 1 file changed, 43 insertions(+))
TF OiCS: Diff ( 1 file changed, 1 insertion(+))
Inspec: Diff ( 7 files changed, 19 insertions(+), 2 deletions(-))

[megan07]

Looks good! Thank you!