Added Cloud Storage as a PubSub subscription

[DanielRieske]

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).
• 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).
• Generated Terraform providers, and ran make test and make lint in the generated providers to ensure it passes unit and linter tests.
• Ran relevant acceptance tests using my own Google Cloud project and credentials (If the acceptance tests do not yet pass or you are unable to run them, please let your reviewer know).
• Read Write release notes before writing my release note below.

Release Note Template for Downstream PRs (will be copied)

pubsub: added `CloudStorageConfig` field to `google_pubsub_subscription` resource

Closes: hashicorp/terraform-provider-google#15352

[modular-magician]

Hello! I am a robot. It looks like you are a community contributor. @c2thorn, a repository maintainer, has been assigned to review your changes. If you have not received review feedback within 2 business days, please leave a comment on this PR asking them to take a look.

You can help make sure that review is quick by doing a self-review and by running impacted tests locally.

[DanielRieske]

I am a little unsure on how to interpret the following fields, as far as I can see textConfig doesn't have to be implemented and will default to if avroConfig isn't used.

Is this assumption correct?

// Union field output_format can be only one of the following:
  "textConfig": {
    object ([TextConfig](https://cloud.google.com/pubsub/docs/reference/rest/v1/projects.subscriptions#TextConfig))
  },
  "avroConfig": {
    object ([AvroConfig](https://cloud.google.com/pubsub/docs/reference/rest/v1/projects.subscriptions#AvroConfig))
  }
  // End of list of possible types for union field output_format.

https://cloud.google.com/pubsub/docs/reference/rest/v1/projects.subscriptions#CloudStorageConfig

[c2thorn]

I am a little unsure on how to interpret the following fields, as far as I can see textConfig doesn't have to be implemented and will default to if avroConfig isn't used.

Is this assumption correct?

// Union field output_format can be only one of the following:
  "textConfig": {
    object ([TextConfig](https://cloud.google.com/pubsub/docs/reference/rest/v1/projects.subscriptions#TextConfig))
  },
  "avroConfig": {
    object ([AvroConfig](https://cloud.google.com/pubsub/docs/reference/rest/v1/projects.subscriptions#AvroConfig))
  }
  // End of list of possible types for union field output_format.

cloud.google.com/pubsub/docs/reference/rest/v1/projects.subscriptions#CloudStorageConfig

That looks like a correct assumption to me. Looks like there is an inner field writeMetadata that is still configurable within textConfig, so implementing it (with default_from_api: true since it is an API default) wouldn't be useless.

[c2thorn]

not reviewing for now since this is set to draft. If you think it is ready for review, then feel free to message me here or switch the PR to ready

[DanielRieske]

I implemented the field textConfig as you suggested, the PR should be ready for review now.

[modular-magician]

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

Terraform GA: Diff ( 3 files changed, 506 insertions(+), 6 deletions(-))
Terraform Beta: Diff ( 3 files changed, 506 insertions(+), 6 deletions(-))
TF Conversion: Diff ( 3 files changed, 147 insertions(+), 3 deletions(-))
TF OiCS: Diff ( 4 files changed, 135 insertions(+))

Missing test report

Your PR includes resource fields which are not covered by any test.

Resource: google_pubsub_subscription (16 total tests)
Please add an acceptance test which includes these fields. The test should include the following:

resource "google_pubsub_subscription" "primary" {
  cloud_storage_config {
    avro_config {
      write_metadata = # value needed
    }
    filename_prefix = # value needed
    filename_suffix = # value needed
    max_bytes       = # value needed
    max_duration    = # value needed
    text_config {
      write_metadata = # value needed
    }
  }
}

[modular-magician]

Tests analytics

Total tests: 2918
Passed tests 2615
Skipped tests: 302
Affected tests: 1

Action taken

Found 1 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests

TestAccPubsubSubscription_pubsubSubscriptionPushCloudstorageExample

Get to know how VCR tests work

[modular-magician]

$\textcolor{red}{\textsf{Tests failed during RECORDING mode:}}$
TestAccPubsubSubscription_pubsubSubscriptionPushCloudstorageExample[Error message] [Debug log]

$\textcolor{red}{\textsf{Please fix these to complete your PR.}}$
View the build log or the debug log for each test

[c2thorn]

Please verify that all added fields are somehow included in an example or handwritten test.

For the current failing test:

    vcr_utils.go:152: Step 1/2 error: Error running apply: exit status 1
        
        Error: Error creating Subscription: googleapi: Error 400: Cloud Pub/Sub did not have the necessary permissions configured to access the provided bucket tf-test-example-bucket<suffix> (or the bucket may not exist). Please verify that the service account service-<id>@gcp-sa-pubsub.iam.gserviceaccount.com was granted the Storage Legacy Bucket Reader and Storage Object Creator roles for the provided bucket.
        
          with google_pubsub_subscription.example,
          on terraform_plugin_test.tf line 11, in resource "google_pubsub_subscription" "example":
          11: resource "google_pubsub_subscription" "example" {

[DanielRieske]

Added additional test coverage for the other fields, and the test failed due to a race condition with creating the subscription resource and the IAM permissions.

This should all be fixed now, can we run the tests again?

[modular-magician]

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

Terraform GA: Diff ( 3 files changed, 656 insertions(+), 6 deletions(-))
Terraform Beta: Diff ( 3 files changed, 656 insertions(+), 6 deletions(-))
TF Conversion: Diff ( 3 files changed, 147 insertions(+), 3 deletions(-))
TF OiCS: Diff ( 8 files changed, 290 insertions(+))

Missing test report

Your PR includes resource fields which are not covered by any test.

Resource: google_pubsub_subscription (17 total tests)
Please add an acceptance test which includes these fields. The test should include the following:

resource "google_pubsub_subscription" "primary" {
  cloud_storage_config {
    max_duration = # value needed
    text_config {
      write_metadata = # value needed
    }
  }
}

[modular-magician]

Tests analytics

Total tests: 2926
Passed tests 2621
Skipped tests: 302
Affected tests: 3

Action taken

Found 3 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests

TestAccPubsubSubscription_pubsubSubscriptionPushCloudstorageAvroExample|TestAccBigQueryDataTable_bigtable|TestAccPubsubSubscription_pubsubSubscriptionPushCloudstorageExample

Get to know how VCR tests work

[modular-magician]

$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$
TestAccPubsubSubscription_pubsubSubscriptionPushCloudstorageAvroExample[Debug log]
TestAccBigQueryDataTable_bigtable[Debug log]
TestAccPubsubSubscription_pubsubSubscriptionPushCloudstorageExample[Debug log]

Rerun these tests in REPLAYING mode to catch issues

$\textcolor{red}{\textsf{Tests failed when rerunning REPLAYING mode:}}$
TestAccPubsubSubscription_pubsubSubscriptionPushCloudstorageAvroExample[Error message] [Debug log]
TestAccPubsubSubscription_pubsubSubscriptionPushCloudstorageExample[Error message] [Debug log]

Tests failed due to non-determinism or randomness when the VCR replayed the response after the HTTP request was made.

Please fix these to complete your PR. If you believe these test failures to be incorrect or unrelated to your change, or if you have any questions, please raise the concern with your reviewer.

---

$\textcolor{red}{\textsf{Errors occurred during RECORDING mode. Please fix them to complete your PR.}}$
View the build log or the debug log for each test

[c2thorn]

Error: Error applying IAM policy for storage bucket "b/tf-test-example-bucket<suffix>": Error setting IAM policy for storage bucket "b/tf-test-example-bucket<suffix>": Put "https://storage.googleapis.com/storage/v1/b/tf-test-example-bucket<suffix>/iam?alt=json": Requested interaction not found

seems to be the same error for both

[DanielRieske]

@c2thorn Do you by any chance have any more information?

I ran them on my own account and I couldn't repoduce this, I think that either the bucket doesn't exist yet at that point or there is some sort of organisational policy prohibiting me from doing this.

terraform-provider-google git:(main) ✗ make testacc TEST=./google TESTARGS='-run=TestAccPubsubSubscription_'
TF_ACC=1 TF_SCHEMA_PANIC_ON_ERROR=1 go test ./google -v -run=TestAccPubsubSubscription_ -timeout 240m -ldflags="-X=github.com/hashicorp/terraform-provider-google/version.ProviderVersion=acc"
=== RUN   TestAccPubsubSubscription_pubsubSubscriptionPushExample
=== PAUSE TestAccPubsubSubscription_pubsubSubscriptionPushExample
=== RUN   TestAccPubsubSubscription_pubsubSubscriptionPullExample
=== PAUSE TestAccPubsubSubscription_pubsubSubscriptionPullExample
=== RUN   TestAccPubsubSubscription_pubsubSubscriptionDeadLetterExample
=== PAUSE TestAccPubsubSubscription_pubsubSubscriptionDeadLetterExample
=== RUN   TestAccPubsubSubscription_pubsubSubscriptionPushBqExample
=== PAUSE TestAccPubsubSubscription_pubsubSubscriptionPushBqExample
=== RUN   TestAccPubsubSubscription_pubsubSubscriptionPushCloudstorageExample
=== PAUSE TestAccPubsubSubscription_pubsubSubscriptionPushCloudstorageExample
=== RUN   TestAccPubsubSubscription_pubsubSubscriptionPushCloudstorageAvroExample
=== PAUSE TestAccPubsubSubscription_pubsubSubscriptionPushCloudstorageAvroExample
=== RUN   TestAccPubsubSubscription_emptyTTL
=== PAUSE TestAccPubsubSubscription_emptyTTL
=== RUN   TestAccPubsubSubscription_basic
=== PAUSE TestAccPubsubSubscription_basic
=== RUN   TestAccPubsubSubscription_update
=== PAUSE TestAccPubsubSubscription_update
=== RUN   TestAccPubsubSubscription_push
=== PAUSE TestAccPubsubSubscription_push
=== RUN   TestAccPubsubSubscription_pushNoWrapper
=== PAUSE TestAccPubsubSubscription_pushNoWrapper
=== RUN   TestAccPubsubSubscription_pollOnCreate
=== PAUSE TestAccPubsubSubscription_pollOnCreate
=== CONT  TestAccPubsubSubscription_pubsubSubscriptionPushExample
=== CONT  TestAccPubsubSubscription_emptyTTL
=== CONT  TestAccPubsubSubscription_pubsubSubscriptionPushBqExample
=== CONT  TestAccPubsubSubscription_pubsubSubscriptionPushCloudstorageAvroExample
=== CONT  TestAccPubsubSubscription_pubsubSubscriptionPushCloudstorageExample
=== CONT  TestAccPubsubSubscription_push
=== CONT  TestAccPubsubSubscription_pollOnCreate
=== CONT  TestAccPubsubSubscription_pushNoWrapper
=== CONT  TestAccPubsubSubscription_update
=== CONT  TestAccPubsubSubscription_basic
--- PASS: TestAccPubsubSubscription_basic (30.16s)
=== CONT  TestAccPubsubSubscription_pubsubSubscriptionDeadLetterExample
--- PASS: TestAccPubsubSubscription_emptyTTL (31.38s)
=== CONT  TestAccPubsubSubscription_pubsubSubscriptionPullExample
--- PASS: TestAccPubsubSubscription_pubsubSubscriptionPushExample (33.97s)
--- PASS: TestAccPubsubSubscription_pollOnCreate (35.27s)
--- PASS: TestAccPubsubSubscription_pushNoWrapper (38.43s)
--- PASS: TestAccPubsubSubscription_push (39.05s)
--- PASS: TestAccPubsubSubscription_update (43.48s)
--- PASS: TestAccPubsubSubscription_pubsubSubscriptionPushBqExample (47.15s)
--- PASS: TestAccPubsubSubscription_pubsubSubscriptionPushCloudstorageAvroExample (49.55s)
--- PASS: TestAccPubsubSubscription_pubsubSubscriptionPushCloudstorageExample (51.07s)
--- PASS: TestAccPubsubSubscription_pubsubSubscriptionDeadLetterExample (32.74s)
--- PASS: TestAccPubsubSubscription_pubsubSubscriptionPullExample (152.57s)
PASS
ok      github.com/hashicorp/terraform-provider-google/google   184.642s

Also I don't quite understand why modular-magician asks to cover the field text_config as this is a default from the api and also marked as such.

    text_config {
      write_metadata = # value needed
    }

[c2thorn]

@c2thorn Do you by any chance have any more information?

I ran them on my own account and I couldn't repoduce this, I think that either the bucket doesn't exist yet at that point or there is some sort of organisational policy prohibiting me from doing this.

Don't have a great answer here, my guess is that the bucket had some eventual consistency? I'm running the test again in a separate build.

Also I don't quite understand why modular-magician asks to cover the field text_config as this is a default from the api and also marked as such.

    text_config {
      write_metadata = # value needed
    }

Terraform will default to the API's value when not configured, but if it is a settable field, we should have a test attempting to set it. Only a field that is completely output-only would not need a test.

[c2thorn]

The avro example failed this time. Posted some suggestions to hopefully fix the consistency

[DanielRieske]

I have taken another look at the text_config field and I am unsure if we really have to implement this field.

Looking at the response body of a GET request of this resource it looks like this field has nothing in it.

{
  "name": "projects/projext-x/subscriptions/tf-test-example-subscription4fvru7y71f",
  "topic": "projects/project-x/tf-test-example-topic4fvru7y71f",
  "pushConfig": {},
  "ackDeadlineSeconds": 300,
  "messageRetentionDuration": "604800s",
  "expirationPolicy": {
    "ttl": "2678400s"
  },
  "state": "ACTIVE",
  "cloudStorageConfig": {
    "bucket": "tf-test-example-bucket4fvru7y71f",
    "filenamePrefix": "pre-",
    "filenameSuffix": "-4fvru7y71f",
    "textConfig": {},
    "maxDuration": "300s",
    "maxBytes": "1000",
    "state": "ACTIVE"
  }
}

[c2thorn]

@DanielRieske assuming that the default value of writeMetadata is set for that resource, it is not unheard of to have the default value not return by an API. If the API did not return an non-default value, that is a problem.

However, there is no need to implement textConfig now so I'm fine with omitting it.

[c2thorn]

committing minor whitespace fix

[modular-magician]

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

Terraform GA: Diff ( 3 files changed, 616 insertions(+), 6 deletions(-))
Terraform Beta: Diff ( 3 files changed, 616 insertions(+), 6 deletions(-))
TF Conversion: Diff ( 3 files changed, 117 insertions(+), 3 deletions(-))
TF OiCS: Diff ( 8 files changed, 308 insertions(+))

[modular-magician]

Tests analytics

Total tests: 2933
Passed tests 2629
Skipped tests: 302
Affected tests: 2

Action taken

Found 2 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests

TestAccPubsubSubscription_pubsubSubscriptionPushCloudstorageAvroExample|TestAccPubsubSubscription_pubsubSubscriptionPushCloudstorageExample

Get to know how VCR tests work

[modular-magician]

$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$
TestAccPubsubSubscription_pubsubSubscriptionPushCloudstorageAvroExample[Debug log]
TestAccPubsubSubscription_pubsubSubscriptionPushCloudstorageExample[Debug log]

Rerun these tests in REPLAYING mode to catch issues

$\textcolor{red}{\textsf{Tests failed when rerunning REPLAYING mode:}}$
TestAccPubsubSubscription_pubsubSubscriptionPushCloudstorageAvroExample[Error message] [Debug log]
TestAccPubsubSubscription_pubsubSubscriptionPushCloudstorageExample[Error message] [Debug log]

Tests failed due to non-determinism or randomness when the VCR replayed the response after the HTTP request was made.

Please fix these to complete your PR. If you believe these test failures to be incorrect or unrelated to your change, or if you have any questions, please raise the concern with your reviewer.

---

$\textcolor{red}{\textsf{Errors occurred during RECORDING mode. Please fix them to complete your PR.}}$
View the build log or the debug log for each test

[DanielRieske]

I assume the build failure is because of the same thing as before, at this point the only thing I can try is to use admin rights. I changed them and hopefully this solves the inconsistency between projects.

[modular-magician]

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

Terraform GA: Diff ( 3 files changed, 560 insertions(+), 6 deletions(-))
Terraform Beta: Diff ( 3 files changed, 560 insertions(+), 6 deletions(-))
TF Conversion: Diff ( 3 files changed, 117 insertions(+), 3 deletions(-))
TF OiCS: Diff ( 8 files changed, 280 insertions(+))

[modular-magician]

Tests analytics

Total tests: 2933
Passed tests 2629
Skipped tests: 302
Affected tests: 2

Action taken

Found 2 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests

TestAccPubsubSubscription_pubsubSubscriptionPushCloudstorageAvroExample|TestAccPubsubSubscription_pubsubSubscriptionPushCloudstorageExample

Get to know how VCR tests work

[modular-magician]

$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$
TestAccPubsubSubscription_pubsubSubscriptionPushCloudstorageAvroExample[Debug log]
TestAccPubsubSubscription_pubsubSubscriptionPushCloudstorageExample[Debug log]

Rerun these tests in REPLAYING mode to catch issues

$\textcolor{green}{\textsf{No issues found for passed tests after REPLAYING rerun.}}$

---

$\textcolor{green}{\textsf{All tests passed!}}
View the build log or the debug log for each test