New data source azurerm_ip_groups

[KyMidd]

azurerm_ip_group data source only permits fetching the info of one data source. This data intends to fetch the data from multiple IP Groups within a resource group by using the name attribute as a substring match.

Tests passing:

❯ make acctests SERVICE='network' TESTARGS='-run=TestAccDataSourceIPGroups_' TESTTIMEOUT='10m'
==> 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/network -run=TestAccDataSourceIPGroups_ -timeout 10m -ldflags="-X=github.com/hashicorp/terraform-provider-azurerm/version.ProviderVersion=acc"
=== RUN   TestAccDataSourceIPGroups_noResults
=== PAUSE TestAccDataSourceIPGroups_noResults
=== RUN   TestAccDataSourceIPGroups_single
=== PAUSE TestAccDataSourceIPGroups_single
=== RUN   TestAccDataSourceIPGroups_multiple
=== PAUSE TestAccDataSourceIPGroups_multiple
=== CONT  TestAccDataSourceIPGroups_noResults
=== CONT  TestAccDataSourceIPGroups_multiple
=== CONT  TestAccDataSourceIPGroups_single
--- PASS: TestAccDataSourceIPGroups_multiple (49.75s)
--- PASS: TestAccDataSourceIPGroups_noResults (112.59s)
--- PASS: TestAccDataSourceIPGroups_single (112.70s)
PASS
ok  	github.com/hashicorp/terraform-provider-azurerm/internal/services/network	116.525s

[tombuildsstuff]

hey @KyMidd

Thanks for this PR.

Data Sources (and Resources) are only written to the state when they've got a Resource ID associated with them - as in this case the Data Source isn't calling d.SetID( there's no Resource ID - meaning this isn't tracked in the state.

Since this Data Source is listing the IP Groups available within the specified Resource Group, it'd make sense to use the Resource Group ID as the ID for this Data Source - which is available in commonids as commonids.NewResourceGroupID - meaning that if you add:

subscriptionId := meta.(*clients.Client).Account.SubscriptionID
id := commonids.NewResourceGroupID(subscriptionId, resourceGroupName)
d.SetID(id.ID())

Then this Data Source should start being tracked in the state (and means that the tests will pass) - would you be able to take a look and see if that works for you?

Thanks!

[KyMidd]

Data Sources (and Resources) are only written to the state when they've got a Resource ID associated with them

Ooooooooohhhhhh! Thank you, that helps a ton! With that change, and a little cleanup of the tests, I'm able to compile the provider and this functionality works! 🥳

Interestingly, the checks are still failing, poking at that now. All tests are now working! PR is ready for review.

[stephybun]

Thanks for this PR @KyMidd.

Overall this looks fine, I left a minor comment on the use of sets in data sources. Once that's been addressed we can run the tests and if they run through this should be good to go!

Also just as an aside, we currently have a mix of untyped and typed resources/data sources in the provider with the goal being to move all untyped resource/data source to typed ones in preparation for the next major release of the provider.

Since (I think?) this is your first contribution to the provider I'm not going to ask you to convert this into a typed data source, but for future contributions I'd recommend checking out our Contributor Docs on adding new types resources/data sources 🙂

[KyMidd]

Since (I think?) this is your first contribution to the provider

It is my first contribution! 🎉

I'm not going to ask you to convert this into a typed data source, but for future contributions I'd recommend checking out our Contributor Docs on adding new types resources/data sources 🙂

I'm a n00b at Go, so I don't fully understand but I'll read the docs over and educate myself. Hopefully I can come back here later once I understand and fix this up to be better compliant.

Once that's been addressed we can run the tests and if they run through this should be good to go!

Done! Thanks so much for the info and suggested code blocks 😸 Ran the tests again, all passing after your suggested changes:

❯ make acctests SERVICE='network' TESTARGS='-run=TestAccDataSourceIPGroups_' TESTTIMEOUT='10m'

==> 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/network -run=TestAccDataSourceIPGroups_ -timeout 10m -ldflags="-X=github.com/hashicorp/terraform-provider-azurerm/version.ProviderVersion=acc"
=== RUN   TestAccDataSourceIPGroups_noResults
=== PAUSE TestAccDataSourceIPGroups_noResults
=== RUN   TestAccDataSourceIPGroups_single
=== PAUSE TestAccDataSourceIPGroups_single
=== RUN   TestAccDataSourceIPGroups_multiple
=== PAUSE TestAccDataSourceIPGroups_multiple
=== CONT  TestAccDataSourceIPGroups_noResults
=== CONT  TestAccDataSourceIPGroups_single
=== CONT  TestAccDataSourceIPGroups_multiple
--- PASS: TestAccDataSourceIPGroups_single (139.73s)
--- PASS: TestAccDataSourceIPGroups_noResults (142.29s)
--- PASS: TestAccDataSourceIPGroups_multiple (145.38s)
PASS
ok  	github.com/hashicorp/terraform-provider-azurerm/internal/services/network	149.550s

[tiwood]

If I'm understanding this correctly, this new DS is only returning the names and resource IDs of IP groups in a given RG.

This can already be accomplished using the azurerm_resources data source:

data "azurerm_resources" "example" {
  resource_group_name = "example-resources"
  type  = "Microsoft.Network/IpGroups"
}

[KyMidd]

This can already be accomplished using the azurerm_resources data source:

Oh, that's awesome! I didn't realize that existed. It doesn't appear to also offer name filtering, but that could for sure be done as a secondary action.

I think it's still useful to have data lookups like this, because they're so easy to consume and use. Defer to you and the team if you'd prefer to not use these bespoke data lookups. :)

FWIW, I've asked around with the teams I work with, and none were aware this data resource exists. Perhaps it could be better called out in documentation? I can look at that!

[KyMidd]

I mocked that locally, and it's not nearly as easy to use the azurerm_resources vs this resource type. I think it's still a great idea to move forward with these bespoke resources, and folks can use the azurerm_resources with sufficient knowledge or if the bespoke resources don't yet exist.

Here's an example of using this new resource:

data "azurerm_ip_groups" "example" {
  resource_group_name = "example-resources"
  name = "substring"
}
output "names" {
  value = data.azurerm_ip_groups.example.names
}

And here's the identical functionality using the azurerm_resources resource. Anytime the answer is "just use regex" I think we should consider simpler alternatives :)

data "azurerm_resources" "example" {
  resource_group_name = "example-resources"
  type  = "Microsoft.Network/IpGroups"
}
output "names" {
  [for r in data.azurerm_resources.example.resources : r.name if can(regex("substring", r.name))]
}

[stephybun]

Thanks for making those changes @KyMidd! The linter is still failing but it seems to be error'ing on something outside of this project, so I'm going to ignore it. LGTM 🚀

[github-actions]

This functionality has been released in v3.89.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]

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.