Add import-only to KMS key

[bdhess]

Similar to #5131, and probably also required for the same customer.
Apologies for the oversight.

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)

kms: added support for `import_only` to `google_kms_crypto_key`

[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.

@melinath, 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 ( 3 files changed, 94 insertions(+))
Terraform Beta: Diff ( 3 files changed, 94 insertions(+))
TF Conversion: Diff ( 1 file changed, 10 insertions(+))

[melinath]

/gcbrun

[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, 94 insertions(+))
Terraform Beta: Diff ( 3 files changed, 94 insertions(+))
TF Conversion: Diff ( 1 file changed, 10 insertions(+))

[modular-magician]

I have triggered VCR tests in RECORDING mode for the following tests that failed during VCR: TestAccComputeInstanceFromMachineImage_basic|TestAccComputeInstanceFromMachineImage_overrideMetadataDotStartupScript|TestAccKmsCryptoKey_importOnly|TestAccPrivatecaCaPool_privatecaCapoolEmptyBaseline|TestAccPrivatecaCertificate_privatecaCertificateWithTemplateExample|TestAccTags You can view the result here: https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=203007

[modular-magician]

Tests failed during RECORDING mode: TestAccPrivatecaCaPool_privatecaCapoolEmptyBaseline|TestAccKmsCryptoKey_importOnly|TestAccTags Please fix these to complete your PR

[melinath]

Thanks for contributing! It looks like the new test is currently failing with the following error:

Error: Error creating CryptoKey: googleapi: Error 400: Import-only keys must skip initial version creation.

You might be able to get more information by generating the provider locally and then running the new test

I recommend setting TF_LOG=TRACE to get more verbose output.

Let me know how it goes & if you run into any problems you can't solve.

[bdhess]

Oops, that's my oversight, and a quick fix.

[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, 95 insertions(+))
Terraform Beta: Diff ( 4 files changed, 96 insertions(+), 1 deletion(-))
TF Conversion: Diff ( 1 file changed, 10 insertions(+))

[melinath]

/gcbrun

[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, 95 insertions(+))
Terraform Beta: Diff ( 5 files changed, 97 insertions(+), 2 deletions(-))
TF Conversion: Diff ( 1 file changed, 10 insertions(+))

[bdhess]

Friendly ping on this. We have an internal thread that suggests that the present failure is just a flake.

[sshcherbakov]

Hello Everybody,
We are interested in the getting this PR through as soon as possible.
How can we help?

[melinath]

Hey all - sorry for the delay on this; I lost track of this PR. The best thing you can do if this happens again on a PR in the future is to press the "Re-request review" button to make sure this is in the reviewer's actionable queue.

/gcbrun because the last one failed due to CI issues rather than something wrong with the PR.

[melinath]

We have an internal thread that suggests that the present failure is just a flake.

@bdhess the failure was definitely unrelated to this PR; do you mean that the test is also flakey?

[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, 95 insertions(+))
Terraform Beta: Diff ( 3 files changed, 95 insertions(+))
TF Conversion: Diff ( 1 file changed, 10 insertions(+))

[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, 95 insertions(+))
Terraform Beta: Diff ( 4 files changed, 96 insertions(+), 1 deletion(-))
TF Conversion: Diff ( 1 file changed, 10 insertions(+))

[modular-magician]

I have triggered VCR tests in RECORDING mode for the following tests that failed during VCR: TestAccBigtableAppProfile_bigtableAppProfileSingleclusterExample|TestAccKmsCryptoKey_importOnly You can view the result here: https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=207021

[bdhess]

@melinath I mean flaky per the definition at go/flaky -- it fails nondeterministically with respect to the test inputs.

[melinath]

@bdhess great, thanks for confirming. We generally would prefer not to merge new flakey tests; is it at all possible to fix the test to not be flakey?

[bdhess]

@melinath and I discussed offline that I don't think any new flakiness is introduced with this PR

[melinath]

At least we're not aware of flakiness yet. :-) It looks like the new test failed in CI, but it could be a CI issue again. I'll do a rerun of just that test: https://ci-oss.hashicorp.engineering/buildConfiguration/GoogleCloudBeta_ProviderGoogleCloudBetaMmUpstream/207210

[drebes]

I cloned your fork and tried to run your test, but it first failed:

$ make testacc TEST=./google TESTARGS='-run=TestAccKmsCryptoKey_importOnly'
==> Checking source code against gofmt...
==> Checking that code complies with gofmt requirements...
go generate  ./...
TF_ACC=1 TF_SCHEMA_PANIC_ON_ERROR=1 go test ./google -v -run=TestAccKmsCryptoKey_importOnly -timeout 240m -ldflags="-X=github.com/hashicorp/terraform-provider-google/version.ProviderVersion=acc"
=== RUN   TestAccKmsCryptoKey_importOnly
=== PAUSE TestAccKmsCryptoKey_importOnly
=== CONT  TestAccKmsCryptoKey_importOnly
    provider_test.go:270: ImportStateVerify attributes not equivalent. Difference is shown below. Top is actual, bottom is expected.
       
        (map[string]string) (len=1) {
         (string) (len=29) "skip_initial_version_creation": (string) (len=5) "false"
        }
       
       
        (map[string]string) (len=1) {
         (string) (len=29) "skip_initial_version_creation": (string) (len=4) "true"
        }
--- FAIL: TestAccKmsCryptoKey_importOnly (86.85s)
FAIL
FAIL github.com/hashicorp/terraform-provider-google/google 87.364s
FAIL
make: *** [testacc] Error 1

This is actually expected because when we import the state to verify the resource was created correctly, "skip_initial_version" is always set to false since its value cannot be inferred from the object read from the API and we assume a safe default. But it's actually easy to fix, you just need to add this change to your test:

diff --git a/mmv1/third_party/terraform/tests/resource_kms_crypto_key_test.go b/mmv1/third_party/terraform/tests/resource_kms_crypto_key_test.go
index 251ea357f..52f6fa205 100644
--- a/mmv1/third_party/terraform/tests/resource_kms_crypto_key_test.go
+++ b/mmv1/third_party/terraform/tests/resource_kms_crypto_key_test.go
@@ -353,9 +353,10 @@ func TestAccKmsCryptoKey_importOnly(t *testing.T) {
                                Config: testGoogleKmsCryptoKey_importOnly(projectId, projectOrg, projectBillingAccount, keyRingName, cryptoKeyName),
                        },
                        {
-                               ResourceName:      "google_kms_crypto_key.crypto_key",
-                               ImportState:       true,
-                               ImportStateVerify: true,
+                               ResourceName:            "google_kms_crypto_key.crypto_key",
+                               ImportState:             true,
+                               ImportStateVerify:       true,
+                               ImportStateVerifyIgnore: []string{"skip_initial_version_creation"},
                        },
                        // Use a separate TestStep rather than a CheckDestroy because we need the project to still exist.
                        {

With that change, your importOnly and all other CryptoKey tests successfully pass for me:

$ make testacc TEST=./google TESTARGS='-run=TestAccKmsCryptoKey'
==> Checking source code against gofmt...
==> Checking that code complies with gofmt requirements...
go generate  ./...
TF_ACC=1 TF_SCHEMA_PANIC_ON_ERROR=1 go test ./google -v -run=TestAccKmsCryptoKey -timeout 240m -ldflags="-X=github.com/hashicorp/terraform-provider-google/version.ProviderVersion=acc"
=== RUN   TestAccKmsCryptoKeyIamBinding
=== PAUSE TestAccKmsCryptoKeyIamBinding
=== RUN   TestAccKmsCryptoKeyIamMember
=== PAUSE TestAccKmsCryptoKeyIamMember
=== RUN   TestAccKmsCryptoKeyIamPolicy
=== PAUSE TestAccKmsCryptoKeyIamPolicy
=== RUN   TestAccKmsCryptoKey_basic
=== PAUSE TestAccKmsCryptoKey_basic
=== RUN   TestAccKmsCryptoKey_rotation
=== PAUSE TestAccKmsCryptoKey_rotation
=== RUN   TestAccKmsCryptoKey_template
=== PAUSE TestAccKmsCryptoKey_template
=== RUN   TestAccKmsCryptoKey_destroyDuration
=== PAUSE TestAccKmsCryptoKey_destroyDuration
=== RUN   TestAccKmsCryptoKey_importOnly
=== PAUSE TestAccKmsCryptoKey_importOnly
=== CONT  TestAccKmsCryptoKeyIamBinding
=== CONT  TestAccKmsCryptoKey_template
=== CONT  TestAccKmsCryptoKey_basic
=== CONT  TestAccKmsCryptoKey_rotation
=== CONT  TestAccKmsCryptoKeyIamPolicy
=== CONT  TestAccKmsCryptoKey_destroyDuration
=== CONT  TestAccKmsCryptoKey_importOnly
=== CONT  TestAccKmsCryptoKeyIamMember
--- PASS: TestAccKmsCryptoKey_importOnly (105.83s)
--- PASS: TestAccKmsCryptoKey_basic (106.83s)
--- PASS: TestAccKmsCryptoKey_destroyDuration (107.13s)
--- PASS: TestAccKmsCryptoKeyIamPolicy (126.04s)
--- PASS: TestAccKmsCryptoKey_template (127.44s)
--- PASS: TestAccKmsCryptoKeyIamMember (140.07s)
--- PASS: TestAccKmsCryptoKey_rotation (144.87s)
--- PASS: TestAccKmsCryptoKeyIamBinding (177.08s)
PASS
ok  	github.com/hashicorp/terraform-provider-google/google	177.601s

[melinath]

I can confirm that (now that CI was able to run the test) we're seeing that same error in CI.

Given that we're not able to set skip_initial_version_creation accurately, this seems like a reasonable fix.

[bdhess]

Thanks a bunch @drebes for the analysis!

[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, 96 insertions(+))
Terraform Beta: Diff ( 3 files changed, 96 insertions(+))
TF Conversion: Diff ( 1 file changed, 10 insertions(+))

[bdhess]

@melinath I don't see a re-request review button sadly. Is that possibly limited to maintainers?

[melinath]

@bdhess I've already re-requested a review for myself, so the button is not available to me right now either. :-) you could check for it once I get a chance to do a review again.

[modular-magician]

I have triggered VCR tests in RECORDING mode for the following tests that failed during VCR: TestAccKmsCryptoKey_importOnly You can view the result here: https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=207236

[melinath]

LGTM