Pub/Sub Topic CMEK/KMS support #1982

drebes · 2019-06-25T23:51:20Z

Release Note for Downstream PRs (will be copied)

pubsub: `google_pubsub_topic` support for `kms_key_name` for customer-managed encryption keys (KMS)

drebes · 2019-06-25T23:56:59Z

Acceptance test is currently failing:

$ make testacc TEST=./google TESTARGS='-run=TestAccPubsubTopic_cmek' 
==> Checking source code against gofmt...
==> Checking that code complies with gofmt requirements...
TF_ACC=1 TF_SCHEMA_PANIC_ON_ERROR=1 go test ./google -v -run=TestAccPubsubTopic_cmek -timeout 240m -ldflags="-X=github.com/terraform-providers/terraform-provider-google/version.ProviderVersion=acc"
=== RUN   TestAccPubsubTopic_cmek
=== PAUSE TestAccPubsubTopic_cmek
=== CONT  TestAccPubsubTopic_cmek
--- FAIL: TestAccPubsubTopic_cmek (60.39s)
    testing.go:568: Step 0 error: errors during apply:
        
        Error: Error creating Topic: googleapi: Error 400: The topic projects/terraform-nvu1vl4xx1/topics/tf-test-99unm4hfsf cannot be created or updated due to an organization policy specified at the project or organization level. The organization policy does not allow message storage in any GCP region where Cloud Pub/Sub is present. The organization policy may be updated by your project or organization administrator. See https://console.cloud.google.com/iam-admin/orgpolicies/gcp-resourceLocations?project=terraform-nvu1vl4xx1. If the topic's project was recently created, you may need to wait a few minutes for the project's organization policy to be properly initialized, and then retry this operation.
        
          on /var/folders/zp/nbv834pd1dz433q33085ldv80000gp/T/tf-test111227742/main.tf line 35:
          (source code not available)
        
        
FAIL
FAIL	github.com/terraform-providers/terraform-provider-google/google	60.440s
make: *** [testacc] Error 1

I confirm that this is related to the project being created too recently and the gcp-resourceLocations message is unrelated: attempting to create the Topic with a KMS CryptoKey project after a couple of minutes succeeds. I'm not sure how to force a delay in the acceptance tests.

modular-magician · 2019-06-26T00:02:54Z

Hi! I'm the modular magician, I work on Magic Modules.
This PR seems not to have generated downstream PRs before, as of 28d7d07.

Pull request statuses

New Pull Requests

I built this PR into one or more new PRs on other repositories, and when those are closed, this PR will also be merged and closed.
depends: hashicorp/terraform-provider-google-beta#894
depends: GoogleCloudPlatform/terraform-google-conversion#119
depends: hashicorp/terraform-provider-google#3925
depends: modular-magician/ansible#291
depends: modular-magician/inspec-gcp#168

third_party/terraform/tests/resource_pubsub_topic_test.go

drebes · 2019-06-26T20:51:27Z

Acceptance pass now:

$ make testacc TEST=./google TESTARGS='-run=TestAccPubsubTopic_cmek' 
==> Checking source code against gofmt...
==> Checking that code complies with gofmt requirements...
TF_ACC=1 TF_SCHEMA_PANIC_ON_ERROR=1 go test ./google -v -run=TestAccPubsubTopic_cmek -timeout 240m -ldflags="-X=github.com/terraform-providers/terraform-provider-google/version.ProviderVersion=acc"
=== RUN   TestAccPubsubTopic_cmek
=== PAUSE TestAccPubsubTopic_cmek
=== CONT  TestAccPubsubTopic_cmek
[...]
--- PASS: TestAccPubsubTopic_cmek (16.60s)
PASS
ok  	github.com/terraform-providers/terraform-provider-google/google	16.634s

modular-magician · 2019-06-26T20:57:23Z

Hi! I'm the modular magician, I work on Magic Modules.
I see that this PR has already had some downstream PRs generated. Any open downstreams are already updated to your most recent commit, 0132d70.

Pull request statuses

terraform-provider-google-beta already has an open PR.
terraform-google-conversion already has an open PR.
terraform-provider-google already has an open PR.
Ansible already has an open PR.
InSpec already has an open PR.

New Pull Requests

I didn't open any new pull requests because of this PR.

rileykarson

I got a test failure in CI because of a required permission on the project's PubSub service account. For the test, wdyt about requiring it be preconfigured?

rileykarson · 2019-06-26T22:19:41Z

third_party/terraform/tests/resource_pubsub_topic_test.go

+  project_id = "%s"
+}
+
+resource "google_project_iam_member" "kms-project-binding" {


As-is, there's a race condition between the topic and this permission. While we could add this dependency with depends_on, I'd rather remove it altogether and just rely on the user having configured it in their project. Alternatively, we could add the permission during the kmsKeyBootstrap function but that isn't necessary.

Since I create projects specifically for the acceptance test to not leave the KMS keys behind, I would prefer to leave the dependency in. I just pushed a change that adds an implicit dependency making the topic project be the IAM member binding, which seems to address the race condition for me. Can we keep this, or does it still trigger the race condition for you (due to the IAM change propagation)?

Our CI system is a single project so there's a risk of a race condition here, but I don't think the risk is enough to block on. It'll be resolved on next run anyways, so if we actually encounter this flake we can resolve the issue.

rileykarson · 2019-06-26T22:21:20Z

third_party/terraform/tests/resource_pubsub_topic_test.go

@@ -40,6 +40,29 @@ func TestAccPubsubTopic_update(t *testing.T) {
 	})
 }

+func TestAccPubsubTopic_cmek(t *testing.T) {


Suggested change

func TestAccPubsubTopic_cmek(t *testing.T) {

// This test requires your project's PubSub service account (service-{{PROJECT_NUMBER}}@gcp-sa-pubsub.iam.gserviceaccount.com)

// to have `roles/cloudkms.cryptoKeyEncrypterDecrypter`.

func TestAccPubsubTopic_cmek(t *testing.T) {

products/pubsub/api.yaml

modular-magician · 2019-06-29T21:50:17Z

Hi! I'm the modular magician, I work on Magic Modules.
I see that this PR has already had some downstream PRs generated. Any open downstreams are already updated to your most recent commit, 0f5b697.

Pull request statuses

terraform-provider-google-beta already has an open PR.
terraform-google-conversion already has an open PR.
terraform-provider-google already has an open PR.
Ansible already has an open PR.
InSpec already has an open PR.

New Pull Requests

I didn't open any new pull requests because of this PR.

modular-magician · 2019-06-29T21:54:12Z

Hi! I'm the modular magician, I work on Magic Modules.
I see that this PR has already had some downstream PRs generated. Any open downstreams are already updated to your most recent commit, 17cf443.

Pull request statuses

terraform-provider-google-beta already has an open PR.
terraform-google-conversion already has an open PR.
terraform-provider-google already has an open PR.
Ansible already has an open PR.
InSpec already has an open PR.

New Pull Requests

I didn't open any new pull requests because of this PR.

rileykarson

Thanks @drebes!

rileykarson · 2019-07-01T19:54:28Z

third_party/terraform/tests/resource_pubsub_topic_test.go

+  project_id = "%s"
+}
+
+resource "google_project_iam_member" "kms-project-binding" {


Our CI system is a single project so there's a risk of a race condition here, but I don't think the risk is enough to block on. It'll be resolved on next run anyways, so if we actually encounter this flake we can resolve the issue.

…MEK. Co-Authored-By: Riley Karson <[email protected]>

googlebot added the cla: yes label Jun 25, 2019

modular-magician added downstream-generated ansible inspec terraform-google-conversion terraform-provider-google-beta terraform-provider-google labels Jun 26, 2019

rileykarson self-requested a review June 26, 2019 17:47

rileykarson reviewed Jun 26, 2019

View reviewed changes

third_party/terraform/tests/resource_pubsub_topic_test.go Show resolved Hide resolved

rileykarson reviewed Jun 26, 2019

View reviewed changes

rileykarson approved these changes Jul 1, 2019

View reviewed changes

modular-magician added the automerged label Jul 2, 2019

drebes and others added 4 commits July 2, 2019 21:20

Pub/Sub Topic CMEK/KMS support

a25aa96

Use BootstrapKMSKey in Pub/Sub Topic KMS acceptance test

1cac135

add dependency between iam_binding and pubsub_topic for acceptance test

bb27c84

Make explicit the Pub/Sub service account KMS role requirements for C…

9ace532

…MEK. Co-Authored-By: Riley Karson <[email protected]>

modular-magician force-pushed the pubsub-cmek branch from 17cf443 to 9ace532 Compare July 2, 2019 21:20

modular-magician merged commit cd0c3ca into GoogleCloudPlatform:master Jul 2, 2019

drebes deleted the pubsub-cmek branch July 19, 2019 20:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pub/Sub Topic CMEK/KMS support #1982

Pub/Sub Topic CMEK/KMS support #1982

drebes commented Jun 25, 2019

drebes commented Jun 25, 2019 •

edited

Loading

modular-magician commented Jun 26, 2019

drebes commented Jun 26, 2019

modular-magician commented Jun 26, 2019

rileykarson left a comment

rileykarson Jun 26, 2019

drebes Jun 29, 2019

rileykarson Jul 1, 2019

rileykarson Jun 26, 2019

modular-magician commented Jun 29, 2019

modular-magician commented Jun 29, 2019

rileykarson left a comment

rileykarson Jul 1, 2019

Pub/Sub Topic CMEK/KMS support #1982

Pub/Sub Topic CMEK/KMS support #1982

Conversation

drebes commented Jun 25, 2019

Release Note for Downstream PRs (will be copied)

drebes commented Jun 25, 2019 • edited Loading

modular-magician commented Jun 26, 2019

Pull request statuses

New Pull Requests

drebes commented Jun 26, 2019

modular-magician commented Jun 26, 2019

Pull request statuses

New Pull Requests

rileykarson left a comment

Choose a reason for hiding this comment

rileykarson Jun 26, 2019

Choose a reason for hiding this comment

drebes Jun 29, 2019

Choose a reason for hiding this comment

rileykarson Jul 1, 2019

Choose a reason for hiding this comment

rileykarson Jun 26, 2019

Choose a reason for hiding this comment

modular-magician commented Jun 29, 2019

Pull request statuses

New Pull Requests

modular-magician commented Jun 29, 2019

Pull request statuses

New Pull Requests

rileykarson left a comment

Choose a reason for hiding this comment

rileykarson Jul 1, 2019

Choose a reason for hiding this comment

drebes commented Jun 25, 2019 •

edited

Loading