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

Support for System and User Assigned Managed Identities in azurerm_eventgrid_domain, azurerm_eventgrid_topic and azurerm_eventgrid_system_topic #12951

Conversation

jrauschenbusch
Copy link
Contributor

Reopened PR for #9906

Issue: #9525

@mbfrahry Please review

@jrauschenbusch jrauschenbusch changed the title Event grid topic system managed identity Support for System and User Assigned Managed Identities in azurerm_eventgrid_domain and azurerm_eventgrid_topic Aug 12, 2021
Copy link
Member

@stephybun stephybun left a comment

Choose a reason for hiding this comment

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

Thanks for reopening this PR and for fixing most of the comments left from #9906 @jrauschenbusch. The tests are passing, once the comments left on on the docs are fixed then this LGTM 👍🏼.

website/docs/r/eventgrid_topic.html.markdown Outdated Show resolved Hide resolved
website/docs/r/eventgrid_topic.html.markdown Outdated Show resolved Hide resolved
website/docs/r/eventgrid_topic.html.markdown Outdated Show resolved Hide resolved
website/docs/r/eventgrid_topic.html.markdown Outdated Show resolved Hide resolved
website/docs/r/eventgrid_topic.html.markdown Outdated Show resolved Hide resolved
website/docs/r/eventgrid_topic.html.markdown Outdated Show resolved Hide resolved
website/docs/r/eventgrid_topic.html.markdown Outdated Show resolved Hide resolved
@jrauschenbusch
Copy link
Contributor Author

@stephybun Done

@katbyte katbyte added this to the v2.76.0 milestone Sep 3, 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 both! LGTM 🚀

@katbyte
Copy link
Collaborator

katbyte commented Sep 3, 2021

@jrauschenbusch - this is ready to go once the merge conflicts are resolved!

@jrauschenbusch
Copy link
Contributor Author

jrauschenbusch commented Sep 3, 2021

@katbyte Could you please fix it? I'm currently in vacation with mobile phone access only.

UPDATE: Regained and back at home. I'll bring this PR to a mergeable state.

@javier-moreno-h
Copy link

Thanks a lot for reopening this PR for fixing #9925. What about azurerm_eventgrid_system_topic ? Will it also support identities ? King Regards.

stephybun
stephybun previously approved these changes Sep 6, 2021
@jrauschenbusch
Copy link
Contributor Author

@javier-moreno-h Until now this PR just comprised the identity support for azurerm_eventgrid_domain and azurerm_eventgrid_topic. I've now added also support for azurerm_eventgrid_system_topic. I'll do a few tests and give an update here if the state is mergable. Basically it was the really same code changes as needed for the topic resource. Hence a further PR review should be just a comparison of those two resources.

@stephybun stephybun dismissed their stale review September 6, 2021 08:14

Additional functionality to be reviewed

@stephybun
Copy link
Member

@jrauschenbusch please re-request review when ready.

@javier-moreno-h
Copy link

@javier-moreno-h Until now this PR just comprised the identity support for azurerm_eventgrid_domain and azurerm_eventgrid_topic. I've now added also support for azurerm_eventgrid_system_topic. I'll do a few tests and give an update here if the state is mergable. Basically it was the really same code changes as needed for the topic resource. Hence a further PR review should be just a comparison of those two resources.

@jrauschenbusch thanks a lot. I really appreciate it. I will be watching this thread. Regards.

@jrauschenbusch
Copy link
Contributor Author

@stephybun PR is now ready to be re-reviewed. I had to adjust the expand/flatten function names due to collisions with another PR which was merged before. Basically there is some misalignment of the identity implementation in the Azure SDK. Therefore we need multiple functions for this job within the event grid module. This was already addressed by @tombuildsstuff within the azure-rest-api-specs repository (see Azure/azure-rest-api-specs#13340). Also i had to adjust a few parts of the system topic tests, due to source resource restrictions (allowed lengths of storage accounts).

Below you'll find the output of my acceptance test execution. I ran the whole event grid acceptance test collection.

Console Output
make acctests SERVICE='eventgrid' TESTARGS='-run=TestAccEventGrid' TESTTIMEOUT='60m' 
==> Checking that code complies with gofmt requirements...
==> Checking that Custom Timeouts are used...
==> Checking that acceptance test packages are used...
TF_ACC=1 go test -v ./internal/services/eventgrid -run=TestAccEventGrid -timeout 60m -ldflags="-X=github.com/hashicorp/terraform-provider-azurerm/version.ProviderVersion=acc"
=== RUN   TestAccEventGridDomainDataSource_basic
=== PAUSE TestAccEventGridDomainDataSource_basic
=== RUN   TestAccEventGridDomain_basic
=== PAUSE TestAccEventGridDomain_basic
=== RUN   TestAccEventGridDomain_requiresImport
=== PAUSE TestAccEventGridDomain_requiresImport
=== RUN   TestAccEventGridDomain_mapping
=== PAUSE TestAccEventGridDomain_mapping
=== RUN   TestAccEventGridDomain_basicWithTags
=== PAUSE TestAccEventGridDomain_basicWithTags
=== RUN   TestAccEventGridDomain_inboundIPRules
=== PAUSE TestAccEventGridDomain_inboundIPRules
=== RUN   TestAccEventGridDomain_basicWithSystemManagedIdentity
=== PAUSE TestAccEventGridDomain_basicWithSystemManagedIdentity
=== RUN   TestAccEventGridDomain_basicWithUserAssignedManagedIdentity
=== PAUSE TestAccEventGridDomain_basicWithUserAssignedManagedIdentity
=== RUN   TestAccEventGridDomainTopicDataSource_basic
=== PAUSE TestAccEventGridDomainTopicDataSource_basic
=== RUN   TestAccEventGridDomainTopic_basic
=== PAUSE TestAccEventGridDomainTopic_basic
=== RUN   TestAccEventGridDomainTopic_requiresImport
=== PAUSE TestAccEventGridDomainTopic_requiresImport
=== 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_identity
=== PAUSE TestAccEventGridEventSubscription_identity
=== RUN   TestAccEventGridSystemTopicEventSubscription_basic
=== PAUSE TestAccEventGridSystemTopicEventSubscription_basic
=== RUN   TestAccEventGridSystemTopicEventSubscription_requiresImport
=== PAUSE TestAccEventGridSystemTopicEventSubscription_requiresImport
=== RUN   TestAccEventGridSystemTopicEventSubscription_eventHubID
=== PAUSE TestAccEventGridSystemTopicEventSubscription_eventHubID
=== RUN   TestAccEventGridSystemTopicEventSubscription_serviceBusQueueID
=== PAUSE TestAccEventGridSystemTopicEventSubscription_serviceBusQueueID
=== RUN   TestAccEventGridSystemTopicEventSubscription_serviceBusTopicID
=== PAUSE TestAccEventGridSystemTopicEventSubscription_serviceBusTopicID
=== RUN   TestAccEventGridSystemTopicEventSubscription_update
=== PAUSE TestAccEventGridSystemTopicEventSubscription_update
=== RUN   TestAccEventGridSystemTopicEventSubscription_filter
=== PAUSE TestAccEventGridSystemTopicEventSubscription_filter
=== RUN   TestAccEventGridSystemTopicEventSubscription_advancedFilter
=== PAUSE TestAccEventGridSystemTopicEventSubscription_advancedFilter
=== RUN   TestAccEventGridSystemTopicEventSubscription_advancedFilterMaxItems
=== PAUSE TestAccEventGridSystemTopicEventSubscription_advancedFilterMaxItems
=== RUN   TestAccEventGridSystemTopicEventSubscription_identity
=== PAUSE TestAccEventGridSystemTopicEventSubscription_identity
=== RUN   TestAccEventGridSystemTopic_basic
=== PAUSE TestAccEventGridSystemTopic_basic
=== RUN   TestAccEventGridSystemTopic_policyStates
=== PAUSE TestAccEventGridSystemTopic_policyStates
=== RUN   TestAccEventGridSystemTopic_requiresImport
=== PAUSE TestAccEventGridSystemTopic_requiresImport
=== RUN   TestAccEventGridSystemTopic_complete
=== PAUSE TestAccEventGridSystemTopic_complete
=== RUN   TestAccEventGridSystemTopic_basicWithSystemManagedIdentity
=== PAUSE TestAccEventGridSystemTopic_basicWithSystemManagedIdentity
=== RUN   TestAccEventGridSystemTopic_basicWithUserAssignedManagedIdentity
=== PAUSE TestAccEventGridSystemTopic_basicWithUserAssignedManagedIdentity
=== RUN   TestAccEventGridTopicDataSource_basic
=== PAUSE TestAccEventGridTopicDataSource_basic
=== RUN   TestAccEventGridTopic_basic
=== PAUSE TestAccEventGridTopic_basic
=== RUN   TestAccEventGridTopic_requiresImport
=== PAUSE TestAccEventGridTopic_requiresImport
=== RUN   TestAccEventGridTopic_mapping
=== PAUSE TestAccEventGridTopic_mapping
=== RUN   TestAccEventGridTopic_basicWithTags
=== PAUSE TestAccEventGridTopic_basicWithTags
=== RUN   TestAccEventGridTopic_inboundIPRules
=== PAUSE TestAccEventGridTopic_inboundIPRules
=== RUN   TestAccEventGridTopic_basicWithSystemManagedIdentity
=== PAUSE TestAccEventGridTopic_basicWithSystemManagedIdentity
=== RUN   TestAccEventGridTopic_basicWithUserAssignedManagedIdentity
=== PAUSE TestAccEventGridTopic_basicWithUserAssignedManagedIdentity
=== CONT  TestAccEventGridDomainDataSource_basic
=== CONT  TestAccEventGridSystemTopicEventSubscription_eventHubID
=== CONT  TestAccEventGridEventSubscription_requiresImport
=== CONT  TestAccEventGridSystemTopicEventSubscription_requiresImport
=== CONT  TestAccEventGridSystemTopicEventSubscription_basic
=== CONT  TestAccEventGridEventSubscription_identity
=== CONT  TestAccEventGridEventSubscription_advancedFilterMaxItems
=== CONT  TestAccEventGridEventSubscription_advancedFilter
=== CONT  TestAccEventGridEventSubscription_filter
=== CONT  TestAccEventGridEventSubscription_update
=== CONT  TestAccEventGridEventSubscription_serviceBusTopicID
=== CONT  TestAccEventGridEventSubscription_serviceBusQueueID
--- PASS: TestAccEventGridEventSubscription_advancedFilterMaxItems (131.63s)
=== CONT  TestAccEventGridSystemTopic_basicWithSystemManagedIdentity
--- PASS: TestAccEventGridEventSubscription_advancedFilter (131.92s)
=== CONT  TestAccEventGridEventSubscription_eventHubID
--- PASS: TestAccEventGridEventSubscription_filter (138.49s)
=== CONT  TestAccEventGridDomain_basicWithSystemManagedIdentity
--- PASS: TestAccEventGridDomainDataSource_basic (149.60s)
=== CONT  TestAccEventGridTopic_basicWithUserAssignedManagedIdentity
--- PASS: TestAccEventGridSystemTopicEventSubscription_basic (152.47s)
=== CONT  TestAccEventGridEventSubscription_basic
--- PASS: TestAccEventGridSystemTopicEventSubscription_requiresImport (166.92s)
=== CONT  TestAccEventGridTopic_basicWithSystemManagedIdentity
--- PASS: TestAccEventGridEventSubscription_requiresImport (189.39s)
=== CONT  TestAccEventGridDomainTopic_requiresImport
--- PASS: TestAccEventGridEventSubscription_update (240.98s)
=== CONT  TestAccEventGridTopic_inboundIPRules
--- PASS: TestAccEventGridTopic_basicWithUserAssignedManagedIdentity (100.95s)
=== CONT  TestAccEventGridDomainTopic_basic
--- PASS: TestAccEventGridSystemTopic_basicWithSystemManagedIdentity (124.37s)
=== CONT  TestAccEventGridDomainTopicDataSource_basic
--- PASS: TestAccEventGridTopic_basicWithSystemManagedIdentity (97.10s)
=== CONT  TestAccEventGridDomain_basicWithUserAssignedManagedIdentity
--- PASS: TestAccEventGridSystemTopicEventSubscription_eventHubID (284.47s)
=== CONT  TestAccEventGridTopic_basicWithTags
=== CONT  TestAccEventGridTopic_basic
--- PASS: TestAccEventGridEventSubscription_identity (284.60s)
--- PASS: TestAccEventGridEventSubscription_basic (134.24s)
=== CONT  TestAccEventGridTopicDataSource_basic
--- PASS: TestAccEventGridEventSubscription_serviceBusTopicID (292.68s)
=== CONT  TestAccEventGridTopic_mapping
--- PASS: TestAccEventGridDomain_basicWithSystemManagedIdentity (160.88s)
=== CONT  TestAccEventGridSystemTopic_basicWithUserAssignedManagedIdentity
--- PASS: TestAccEventGridEventSubscription_serviceBusQueueID (311.46s)
=== CONT  TestAccEventGridTopic_requiresImport
--- PASS: TestAccEventGridDomain_basicWithUserAssignedManagedIdentity (102.08s)
=== CONT  TestAccEventGridDomain_mapping
--- PASS: TestAccEventGridDomainTopic_requiresImport (185.88s)
=== CONT  TestAccEventGridDomain_inboundIPRules
--- PASS: TestAccEventGridTopic_basicWithTags (98.64s)
=== CONT  TestAccEventGridDomain_basicWithTags
--- PASS: TestAccEventGridTopic_mapping (98.45s)
=== CONT  TestAccEventGridSystemTopicEventSubscription_update
--- PASS: TestAccEventGridTopic_inboundIPRules (158.30s)
=== CONT  TestAccEventGridSystemTopic_policyStates
--- PASS: TestAccEventGridEventSubscription_eventHubID (281.64s)
=== CONT  TestAccEventGridSystemTopicEventSubscription_advancedFilter
--- PASS: TestAccEventGridSystemTopic_basicWithUserAssignedManagedIdentity (126.34s)
=== CONT  TestAccEventGridSystemTopic_complete
--- PASS: TestAccEventGridDomainTopic_basic (180.45s)
=== CONT  TestAccEventGridSystemTopicEventSubscription_filter
--- PASS: TestAccEventGridDomainTopicDataSource_basic (175.96s)
=== CONT  TestAccEventGridSystemTopic_requiresImport
--- PASS: TestAccEventGridTopicDataSource_basic (165.40s)
=== CONT  TestAccEventGridSystemTopicEventSubscription_serviceBusTopicID
--- PASS: TestAccEventGridTopic_basic (183.44s)
=== CONT  TestAccEventGridSystemTopicEventSubscription_serviceBusQueueID
--- PASS: TestAccEventGridDomain_inboundIPRules (98.33s)
=== CONT  TestAccEventGridDomain_requiresImport
--- PASS: TestAccEventGridDomain_basicWithTags (100.91s)
=== CONT  TestAccEventGridSystemTopic_basic
--- PASS: TestAccEventGridTopic_requiresImport (197.18s)
=== CONT  TestAccEventGridSystemTopicEventSubscription_identity
--- PASS: TestAccEventGridDomain_mapping (159.34s)
=== CONT  TestAccEventGridDomain_basic
--- PASS: TestAccEventGridSystemTopic_complete (115.32s)
=== CONT  TestAccEventGridSystemTopicEventSubscription_advancedFilterMaxItems
--- PASS: TestAccEventGridSystemTopic_policyStates (154.36s)
--- PASS: TestAccEventGridSystemTopic_requiresImport (127.01s)
--- PASS: TestAccEventGridSystemTopicEventSubscription_filter (146.12s)
--- PASS: TestAccEventGridSystemTopicEventSubscription_update (193.78s)
--- PASS: TestAccEventGridSystemTopic_basic (112.54s)
--- PASS: TestAccEventGridDomain_requiresImport (171.41s)
--- PASS: TestAccEventGridSystemTopicEventSubscription_advancedFilter (234.56s)
--- PASS: TestAccEventGridSystemTopicEventSubscription_advancedFilterMaxItems (141.83s)
--- PASS: TestAccEventGridDomain_basic (159.84s)
--- PASS: TestAccEventGridSystemTopicEventSubscription_serviceBusQueueID (248.56s)
--- PASS: TestAccEventGridSystemTopicEventSubscription_serviceBusTopicID (282.77s)
--- PASS: TestAccEventGridSystemTopicEventSubscription_identity (339.07s)
PASS
ok      github.com/hashicorp/terraform-provider-azurerm/internal/services/eventgrid     849.227s

@jrauschenbusch jrauschenbusch changed the title Support for System and User Assigned Managed Identities in azurerm_eventgrid_domain and azurerm_eventgrid_topic Support for System and User Assigned Managed Identities in azurerm_eventgrid_domain, azurerm_eventgrid_topic and azurerm_eventgrid_system_topic Sep 6, 2021
Copy link
Member

@stephybun stephybun left a comment

Choose a reason for hiding this comment

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

Thanks @jrauschenbusch LGTM 🥳

@stephybun stephybun merged commit 8d46cd0 into hashicorp:main Sep 6, 2021
stephybun added a commit that referenced this pull request Sep 6, 2021
@github-actions
Copy link

This functionality has been released in v2.76.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 Oct 10, 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.

4 participants