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

azurerm_eventgrid_event_subscription : Support delivery_property #13595

Merged
merged 10 commits into from
Oct 13, 2021

Conversation

dylanmorley
Copy link
Contributor

@dylanmorley dylanmorley commented Oct 3, 2021

As per #12145, supporting custom delivery properties for endpoints that support them.

  • Schema expansion, supporting multiple delivery_property definitions for a azurerm_eventgrid_event_subscription resource
  • Creating static or dynamic properties as configured
  • Added tests
  • Updated documentation
DylanM@WL-17770 /c/git/tf/terraform-provider-azurerm (feature/event-grid-subscription)
$ TF_ACC=1 go test -v ./internal/services/eventgrid -run=TestAccEventGridEventSubscription_ -timeout 180m -ldflags="-X=github.com/terraform-providers/terraform-provider-azurerm/version.ProviderVersion=acc"
=== RUN   TestAccEventGridEventSubscription_basic
=== PAUSE TestAccEventGridEventSubscription_basic
=== RUN   TestAccEventGridEventSubscription_requiresImport
=== PAUSE TestAccEventGridEventSubscription_requiresImport
=== RUN   TestAccEventGridEventSubscription_eventHubID
=== PAUSE TestAccEventGridEventSubscription_eventHubID
=== RUN   TestAccEventGridEventSubscription_serviceBusQueueID
=== PAUSE TestAccEventGridEventSubscription_serviceBusQueueID
=== RUN   TestAccEventGridEventSubscription_serviceBusTopicID
=== PAUSE TestAccEventGridEventSubscription_serviceBusTopicID
=== RUN   TestAccEventGridEventSubscription_update
=== PAUSE TestAccEventGridEventSubscription_update
=== RUN   TestAccEventGridEventSubscription_filter
=== PAUSE TestAccEventGridEventSubscription_filter
=== RUN   TestAccEventGridEventSubscription_advancedFilter
=== PAUSE TestAccEventGridEventSubscription_advancedFilter
=== RUN   TestAccEventGridEventSubscription_advancedFilterMaxItems
=== PAUSE TestAccEventGridEventSubscription_advancedFilterMaxItems
=== RUN   TestAccEventGridEventSubscription_deliveryPropertiesStatic
=== PAUSE TestAccEventGridEventSubscription_deliveryPropertiesStatic
=== RUN   TestAccEventGridEventSubscription_deliveryPropertiesMixed
=== PAUSE TestAccEventGridEventSubscription_deliveryPropertiesMixed
=== RUN   TestAccEventGridEventSubscription_deliveryPropertiesHybridRelay
=== PAUSE TestAccEventGridEventSubscription_deliveryPropertiesHybridRelay
=== RUN   TestAccEventGridEventSubscription_deliveryPropertiesForEventHubs
=== PAUSE TestAccEventGridEventSubscription_deliveryPropertiesForEventHubs
=== RUN   TestAccEventGridEventSubscription_identity
=== PAUSE TestAccEventGridEventSubscription_identity
=== CONT  TestAccEventGridEventSubscription_basic
=== CONT  TestAccEventGridEventSubscription_advancedFilterMaxItems
=== CONT  TestAccEventGridEventSubscription_serviceBusTopicID
=== CONT  TestAccEventGridEventSubscription_identity
--- PASS: TestAccEventGridEventSubscription_advancedFilterMaxItems (158.01s)
=== CONT  TestAccEventGridEventSubscription_filter
--- PASS: TestAccEventGridEventSubscription_basic (196.55s)
=== CONT  TestAccEventGridEventSubscription_advancedFilter
--- PASS: TestAccEventGridEventSubscription_advancedFilter (138.81s)
=== CONT  TestAccEventGridEventSubscription_deliveryPropertiesHybridRelay
--- PASS: TestAccEventGridEventSubscription_filter (178.94s)
=== CONT  TestAccEventGridEventSubscription_deliveryPropertiesForEventHubs
--- PASS: TestAccEventGridEventSubscription_identity (347.25s)
=== CONT  TestAccEventGridEventSubscription_update
--- PASS: TestAccEventGridEventSubscription_serviceBusTopicID (351.79s)
=== CONT  TestAccEventGridEventSubscription_eventHubID
--- PASS: TestAccEventGridEventSubscription_update (240.73s)
=== CONT  TestAccEventGridEventSubscription_serviceBusQueueID
--- PASS: TestAccEventGridEventSubscription_deliveryPropertiesHybridRelay (258.10s)
=== CONT  TestAccEventGridEventSubscription_requiresImport
--- PASS: TestAccEventGridEventSubscription_deliveryPropertiesForEventHubs (313.71s)
=== CONT  TestAccEventGridEventSubscription_deliveryPropertiesMixed
--- PASS: TestAccEventGridEventSubscription_eventHubID (318.37s)
=== CONT  TestAccEventGridEventSubscription_deliveryPropertiesStatic
--- PASS: TestAccEventGridEventSubscription_requiresImport (186.08s)
--- PASS: TestAccEventGridEventSubscription_serviceBusQueueID (324.83s)
--- PASS: TestAccEventGridEventSubscription_deliveryPropertiesStatic (295.95s)
--- PASS: TestAccEventGridEventSubscription_deliveryPropertiesMixed (345.93s)
PASS
ok      github.com/hashicorp/terraform-provider-azurerm/internal/services/eventgrid     999.407s

@dylanmorley
Copy link
Contributor Author

Bit of guidance required - what's the correct approach to take when the Azure API doesn't return a value, because it's marked as sensitive/secret?

In this PR, you can create 'Static' properties and set 'Secret' to true - if you do this, then subsequent calls to the API will just return you a value that says 'Hidden'. This caused issues as described in #6688

The implementation linked from issue 6688 seems to be 'don't return the value if it's a secret, then ignore it in the ImportSteps' - which I've followed, but I still experienced failing tests until I marked the test as ExpectNonEmptyPlan: true (which feels like cheating :) )

Concerned the current state would cause constant changes on updates when using 'Secret' properties due to Diffs - if anyone could have a look at this and let me know if I've missed something, or a better approach to handle this.

@katbyte
Copy link
Collaborator

katbyte commented Oct 6, 2021

@dylanmorley - when azure does not return a secret value what we do is just pull it across from the config - ie in the read we do a d.Get() to get the old value and set that.

@dylanmorley
Copy link
Contributor Author

Thanks @katbyte, updated to follow suggested approach - amended + added a test for scenario.

Copy link
Member

@catriona-m catriona-m left a comment

Choose a reason for hiding this comment

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

Thanks @dylanmorley this looks good so far - left a couple of comments inline.

It might be also be a good idea to include an update test for this.

Thanks!

@dylanmorley
Copy link
Contributor Author

Added update test specifically for delvery property changes, these are all grouped under deliveryProperties to run these subset of tests, e.g.

TF_ACC=1 go test -v ./internal/services/eventgrid -run=TestAccEventGridEventSubscription_deliveryProperties -timeout 180m -ldflags="-X=github.com/terraform-providers/terraform-provider-azurerm/version.ProviderVersion=acc"
=== RUN   TestAccEventGridEventSubscription_deliveryPropertiesStatic
=== PAUSE TestAccEventGridEventSubscription_deliveryPropertiesStatic
=== RUN   TestAccEventGridEventSubscription_deliveryPropertiesMixed
=== PAUSE TestAccEventGridEventSubscription_deliveryPropertiesMixed
=== RUN   TestAccEventGridEventSubscription_deliveryPropertiesSecret
=== PAUSE TestAccEventGridEventSubscription_deliveryPropertiesSecret
=== RUN   TestAccEventGridEventSubscription_deliveryPropertiesHybridRelay
=== PAUSE TestAccEventGridEventSubscription_deliveryPropertiesHybridRelay
=== RUN   TestAccEventGridEventSubscription_deliveryPropertiesForEventHubs
=== PAUSE TestAccEventGridEventSubscription_deliveryPropertiesForEventHubs
=== RUN   TestAccEventGridEventSubscription_deliveryPropertiesUpdate
=== PAUSE TestAccEventGridEventSubscription_deliveryPropertiesUpdate
=== CONT  TestAccEventGridEventSubscription_deliveryPropertiesStatic
=== CONT  TestAccEventGridEventSubscription_deliveryPropertiesForEventHubs
=== CONT  TestAccEventGridEventSubscription_deliveryPropertiesSecret
=== CONT  TestAccEventGridEventSubscription_deliveryPropertiesHybridRelay
--- PASS: TestAccEventGridEventSubscription_deliveryPropertiesHybridRelay (294.74s)
=== CONT  TestAccEventGridEventSubscription_deliveryPropertiesUpdate
=== CONT  TestAccEventGridEventSubscription_deliveryPropertiesMixed
--- PASS: TestAccEventGridEventSubscription_deliveryPropertiesForEventHubs (303.11s)
--- PASS: TestAccEventGridEventSubscription_deliveryPropertiesStatic (327.93s)
--- PASS: TestAccEventGridEventSubscription_deliveryPropertiesSecret (337.95s)
--- PASS: TestAccEventGridEventSubscription_deliveryPropertiesMixed (291.17s)
--- PASS: TestAccEventGridEventSubscription_deliveryPropertiesUpdate (545.16s)
PASS
ok      github.com/hashicorp/terraform-provider-azurerm/internal/services/eventgrid     839.928s

@dylanmorley dylanmorley force-pushed the feature/event-grid-subscription branch from 0986fe8 to 010aa80 Compare October 12, 2021 18:52
@katbyte katbyte added the service/event-grid EventGrid label Oct 13, 2021
@katbyte katbyte added this to the v2.81.0 milestone Oct 13, 2021
Copy link
Collaborator

@katbyte katbyte left a comment

Choose a reason for hiding this comment

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

Thanks @dylanmorley - LGTM 🙌

@katbyte katbyte changed the title azurerm_eventgrid_event_subscription : Support delivery properties azurerm_eventgrid_event_subscription : Support delivery_property Oct 13, 2021
@katbyte katbyte merged commit 063aa81 into hashicorp:main Oct 13, 2021
katbyte added a commit that referenced this pull request Oct 13, 2021
@dylanmorley dylanmorley deleted the feature/event-grid-subscription branch October 14, 2021 11:45
@github-actions
Copy link

This functionality has been released in v2.81.0 of the Terraform Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants