Data Source: azurerm_virtual_network_peering - resolve issue (#27486)

[ning-kang]

Community Note

• Please vote on this PR by adding a 👍 reaction to the original PR to help the community and maintainers prioritize for review
• Please do not leave comments along the lines of "+1", "me too" or "any updates", they generate extra noise for PR followers and do not help prioritize for review

Description

This Pull Request resolves #27486 by adding a new data source for virtual network peering with corresponding tests and documentations.

PR Checklist

• I have followed the guidelines in our Contributing Documentation.
• I have checked to ensure there aren't other open Pull Requests for the same update/change.
• I have checked if my changes close any open issues. If so please include appropriate closing keywords below.
• I have updated/added Documentation as required written in a helpful and kind way to assist users that may be unfamiliar with the resource / data source.
• I have used a meaningful PR title to help maintainers and other users understand this change and help prevent duplicate work.
  For example: “resource_name_here - description of change e.g. adding property new_property_name_here”

Changes to existing Resource / Data Source

• I have added an explanation of what my changes do and why I'd like you to include them (This may be covered by linking to an issue above, but may benefit from additional explanation).
• I have written new tests for my resource or datasource changes & updated any relevent documentation.
• I have successfully run tests with my changes locally. If not, please provide details on testing challenges that prevented you running the tests.
• (For changes that include a state migration only). I have manually tested the migration path between relevant versions of the provider.

Testing

• My submission includes Test coverage as described in the Contribution Guide and the tests pass. (if this is not possible for any reason, please include details of why you did or could not add test coverage)

Change Log

Below please provide what should go into the changelog (if anything) conforming to the Changelog Format documented here.

• azurerm_virtual_network_peering - added the azurerm_virtual_network_peering data source.

This is a (please select all that apply):

• Bug Fix
• New Feature (ie adding a service, resource, or data source)
• Enhancement
• Breaking Change

Related Issue(s)

Fixes #27486

Note

If this PR changes meaningfully during the course of review please update the title and description as required.

[stephybun]

Thanks for this PR @ning-kang!

I see that this is adding a new untyped data source to the provider, which we currently no longer accept into the provider.

It would be greatly appreciated if you could re-write this as a typed data source. Our Contributor Docs have a guide on how to add a new typed data source to the provider to help get you started.

[ning-kang]

Thanks for this PR @ning-kang!

I see that this is adding a new untyped data source to the provider, which we currently no longer accept into the provider.

It would be greatly appreciated if you could re-write this as a typed data source. Our Contributor Docs have a guide on how to add a new typed data source to the provider to help get you started.

Thank you @stephybun for the review. I have refactored the PR to comply with the typed data source pattern. Please kindly have a review again. Much appreciate!

[stephybun]

Thanks for rewriting this as a typed data source @ning-kang!

It looks like the contributor docs for adding a new data source are slightly out of date, apologies for that. I left some further review comments that need to be fixed up before we can merge this.

[stephybun]

We can replace these two properties with the virtual_network_id since we can infer the resource group name after parsing the ID in the read function

Suggested change

	"resource_group_name": commonschema.ResourceGroupName(),
	"virtual_network_name": {
	Type: pluginsdk.TypeString,
	Required: true,
	ValidateFunc: validation.StringIsNotEmpty,
	},
	"virtual_network_id": {
	Type: pluginsdk.TypeString,
	Required: true,
	ValidateFunc: commonids.ValidateVirtualNetworkID,
	},

[stephybun]

We should define a struct Model for the data source and return it here. I left a comment further up with a suggestion on what the struct should look like since we typically define this at the top of the resource

Suggested change

	return nil
	return &VirtualNetworkPeeringDataSourceModel

[stephybun]

Defining a struct for the model means we can decode it and access properties via the fields of the struct

Suggested change

	subscriptionId := metadata.Client.Account.SubscriptionId
	name := metadata.ResourceData.Get("name").(string)
	resource_group_name := metadata.ResourceData.Get("resource_group_name").(string)
	virtual_network_name := metadata.ResourceData.Get("virtual_network_name").(string)
	id := virtualnetworkpeerings.NewVirtualNetworkPeeringID(subscriptionId, resource_group_name, virtual_network_name, name)
	subscriptionId := metadata.Client.Account.SubscriptionId
	var state VirtualNetworkPeeringDataSourceModel
	if err := metadata.Decode(&state); err != nil {
	return fmt.Errorf("decoding: %+v", err)
	}
	virtualNetworkId, err := commonids.ParseVirtualNetworkID(state.VirtualNetworkId)
	if err != nil {
	return err
	}
	id := virtualnetworkpeerings.NewVirtualNetworkPeeringID(subscriptionId, virtualNetworkId.ResourceGroupName, virtualNetworkId.VirtualNetworkName, state.Name)

[stephybun]

These don't need to be set back into state

Suggested change

	metadata.ResourceData.Set("name", id.VirtualNetworkPeeringName)
	metadata.ResourceData.Set("resource_group_name", id.ResourceGroupName)
	metadata.ResourceData.Set("virtual_network_name", id.VirtualNetworkName)

[stephybun]

Suggested change

	metadata.ResourceData.Set("allow_virtual_network_access", peer.AllowVirtualNetworkAccess)
	metadata.ResourceData.Set("allow_forwarded_traffic", peer.AllowForwardedTraffic)
	metadata.ResourceData.Set("allow_gateway_transit", peer.AllowGatewayTransit)
	metadata.ResourceData.Set("peer_complete_virtual_networks_enabled", peer.PeerCompleteVnets)
	metadata.ResourceData.Set("only_ipv6_peering_enabled", peer.EnableOnlyIPv6Peering)
	metadata.ResourceData.Set("use_remote_gateways", peer.UseRemoteGateways)
	state.AllowVirtualNetworkAccess = pointer.From(props.AllowVirtualNetworkAccess)
	state.AllowForwardedTraffic = pointer.From(props.AllowForwardedTraffic)
	... etc.

[stephybun]

Suggested change

	metadata.ResourceData.Set("remote_virtual_network_id", remoteVirtualNetworkId)
	state.RemoteVirtualNetworkId = remoteVirtualNetworkId

[stephybun]

Suggested change

	return nil
	return metadata.Encode(&state)

[stephybun]

Can we prefix these with acctest so that it's clear what resources were created by our acceptance tests and so that our automation can clean these up in case of any failures

Suggested change

	name = "peer1to2"
	resource_group_name = azurerm_resource_group.test.name
	virtual_network_name = azurerm_virtual_network.test-1.name
	remote_virtual_network_id = azurerm_virtual_network.test-2.id
	allow_virtual_network_access = true
	}
	resource "azurerm_virtual_network_peering" "test-2" {
	name = "peer2to1"
	resource_group_name = azurerm_resource_group.test.name
	virtual_network_name = azurerm_virtual_network.test-2.name
	remote_virtual_network_id = azurerm_virtual_network.test-1.id
	allow_virtual_network_access = true
	}
	data "azurerm_virtual_network_peering" "test-1" {
	name = "peer1to2"
	virtual_network_name = azurerm_virtual_network.test-1.name
	resource_group_name = azurerm_resource_group.test.name
	}
	data "azurerm_virtual_network_peering" "test-2" {
	name = "peer2to1"
	virtual_network_name = azurerm_virtual_network.test-2.name
	resource_group_name = azurerm_resource_group.test.name
	}
	`, data.RandomInteger, data.Locations.Primary)
	}
	name = "acctestpeer1to2"
	resource_group_name = azurerm_resource_group.test.name
	virtual_network_name = azurerm_virtual_network.test-1.name
	remote_virtual_network_id = azurerm_virtual_network.test-2.id
	allow_virtual_network_access = true
	}
	resource "azurerm_virtual_network_peering" "test-2" {
	name = "acctestpeer2to1"
	resource_group_name = azurerm_resource_group.test.name
	virtual_network_name = azurerm_virtual_network.test-2.name
	remote_virtual_network_id = azurerm_virtual_network.test-1.id
	allow_virtual_network_access = true
	}
	data "azurerm_virtual_network_peering" "test-1" {
	name = "acctestpeer1to2"
	virtual_network_name = azurerm_virtual_network.test-1.name
	resource_group_name = azurerm_resource_group.test.name
	}
	data "azurerm_virtual_network_peering" "test-2" {
	name = "acctestpeer2to1"
	virtual_network_name = azurerm_virtual_network.test-2.name
	resource_group_name = azurerm_resource_group.test.name
	}
	`, data.RandomInteger, data.Locations.Primary)
	}

[stephybun]

Suggested change

	* `resource_group_name` - (Required) The name of the resource group where the virtual network peering exists.
	* `virtual_network_name` - (Required) The name of the local virtual network.
	* `virtual_network_id` - (Required) The resource ID of the virtual network.

[ning-kang]

Acceptance test is still failing:

------- Stdout: -------
=== RUN   TestAccDataSourceVirtualNetworkPeering_basic
=== PAUSE TestAccDataSourceVirtualNetworkPeering_basic
=== CONT  TestAccDataSourceVirtualNetworkPeering_basic
    testcase.go:173: Step 1/1 error: Error running apply: exit status 1
        Error: Virtual Network Peering (Subscription: "*******"
        Resource Group Name: "acctestRg-241029073552743580"
        Virtual Network Name: "acctestvnet1-241029073552743580"
        Virtual Network Peering Name: "acctestpeer1to2") was not found
          with data.azurerm_virtual_network_peering.test-1,
          on terraform_plugin_test.tf line 66, in data "azurerm_virtual_network_peering" "test-1":
          66: data "azurerm_virtual_network_peering" "test-1" {
        Virtual Network Peering (Subscription: "*******"
        Resource Group Name: "acctestRg-241029073552743580"
        Virtual Network Name: "acctestvnet1-241029073552743580"
        Virtual Network Peering Name: "acctestpeer1to2") was not found
        Error: Virtual Network Peering (Subscription: "*******"
        Resource Group Name: "acctestRg-241029073552743580"
        Virtual Network Name: "acctestvnet2-241029073552743580"
        Virtual Network Peering Name: "acctestpeer2to1") was not found
          with data.azurerm_virtual_network_peering.test-2,
          on terraform_plugin_test.tf line 71, in data "azurerm_virtual_network_peering" "test-2":
          71: data "azurerm_virtual_network_peering" "test-2" {
        Virtual Network Peering (Subscription: "*******"
        Resource Group Name: "acctestRg-241029073552743580"
        Virtual Network Name: "acctestvnet2-241029073552743580"
        Virtual Network Peering Name: "acctestpeer2to1") was not found
--- FAIL: TestAccDataSourceVirtualNetworkPeering_basic (205.14s)
FAIL

@stephybun Sorry to be a pain. I think I have found the issue and fixed the tests. Have also uplifted the sdk version due to a vendor package update. Please kindly have a review again.

[stephybun]

Different failure now:

------- Stdout: -------
=== RUN   TestAccDataSourceVirtualNetworkPeering_basic
=== PAUSE TestAccDataSourceVirtualNetworkPeering_basic
=== CONT  TestAccDataSourceVirtualNetworkPeering_basic
    testcase.go:173: Step 1/1 error: Check failed: Check 2/12 error: data.azurerm_virtual_network_peering.test-1: Attribute 'allow_forwarded_traffic' expected "true", got "false"
--- FAIL: TestAccDataSourceVirtualNetworkPeering_basic (207.40s)
FAIL

[stephybun]

Thanks @ning-kang, test is passing now. LGTM 🚀

[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.