Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(push): Add Pub/Sub Subscription support for push payload unwrapping (no wrapper) #141

Closed
wants to merge 11 commits into from
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ module "pubsub" {
x-goog-version = "v1beta1" // optional
oidc_service_account_email = "[email protected]" // optional
audience = "example" // optional
no_wrapper = false // optional
Copy link
Contributor

@apeabody apeabody Aug 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If possible, please add any new variables to an example so they are exercised in the Integration CI: https://github.com/terraform-google-modules/terraform-google-pubsub/tree/master/examples

no_wrapper_write_metadata = false // optional
expiration_policy = "1209600s" // optional
dead_letter_topic = "projects/my-pubsub-project/topics/example-dl-topic" // optional
max_delivery_attempts = 5 // optional
Expand Down
7 changes: 7 additions & 0 deletions examples/subscriptions_only/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,13 @@ module "pubsub" {
ack_deadline_seconds = 20
expiration_policy = "1209600s" // two weeks
},
{
name = "push_payload_unwrapping"
push_endpoint = "https://${var.project_id}.appspot.com/"
no_wrapper = { write_metadata = true }

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need something like

      no_wrapper                 = true
      no_wrapper_write_metadata  = true

ack_deadline_seconds = 20
expiration_policy = "1209600s" // two weeks
},
]

}
7 changes: 7 additions & 0 deletions main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,13 @@ resource "google_pubsub_subscription" "push_subscriptions" {
audience = lookup(each.value, "audience", "")
}
}

dynamic "no_wrapper" {
for_each = lookup(each.value, "no_wrapper", "") ? [true] : []
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than a bool, would it make more sense to use a nested block no_wrapper = {write_metadata: false} with a default of null? That would more closely follow the provider and allow for easy addition of additional keys in the future.

content {
write_metadata = lookup(each.value, "no_wrapper_write_metadata", null)
}
}
}
depends_on = [
google_pubsub_topic.topic,
Expand Down
2 changes: 1 addition & 1 deletion versions.tf
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ terraform {

google = {
source = "hashicorp/google"
version = ">= 4.32, < 5.0"
version = ">= 4.77, < 5.0"
apeabody marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand Down