Fix circular reference in vnet module

[MSBrett]

🛠️ Description

Fix circular reference in vnet module which was causing private network enabled deployments to fail.

Fixes #1735

📷 Screenshots

📋 Checklist

🔬 How did you test this change?

• 🤏 Lint tests
• 🤞 PS -WhatIf / az validate
• 👍 Manually deployed + verified
• 💪 Unit tests
• 🙌 Integration tests

🙋‍♀️ Do any of the following that apply?

• 🚨 This is a breaking change.
• 🤏 The change is less than 20 lines of code.

📑 Did you update docs/changelog.md?

• ✅ Updated changelog (required for dev PRs)
• ➡️ Will add log in a future PR (feature branch PRs only)
• ❎ Log not needed (small/internal change)

📖 Did you update documentation?

• ✅ Public docs in docs (required for dev)
• ✅ Internal dev docs in src (required for dev)
• ➡️ Will add docs in a future PR (feature branch PRs only)
• ❎ Docs not needed (small/internal change)

[Copilot]

Pull Request Overview

This PR resolves a circular reference in the vNet module by switching name and ID lookups to the newly deployed vNet resource instead of the hub.routing outputs, ensuring private network–enabled deployments succeed.

• Use vNetName for the NSG name to remove dependency on hub.routing.networkName
• Swap hub.routing.networkId with vNet.id in private DNS zone resources
• Reference subnets in script storage and endpoint resources via vNet::… child resource IDs

Comments suppressed due to low confidence (3)

src/templates/finops-hub/modules/infrastructure.bicep:216

• Consider adding unit or integration tests to cover the updated private DNS zone attachment logic using vNet.id to prevent regressions when deploying with private networks.

        id: vNet.id

src/templates/finops-hub/modules/infrastructure.bicep:22

• [nitpick] Add an inline comment explaining why the NSG name has been switched to use vNetName instead of hub.routing.networkName, to clarify the refactoring intent.

var nsgName = '${vNetName}-nsg'

src/templates/finops-hub/modules/infrastructure.bicep:203

• [nitpick] There's a lot of duplicated code for private DNS zones (blob, dfs, queue, table). Consider parameterizing these definitions in a loop or extracting into a reusable module to reduce repetition and simplify future updates.

resource blobPrivateDnsZone 'Microsoft.Network/privateDnsZones@2024-06-01' = if (hub.options.privateRouting) {

[flanakin]

FYI - I was on the fence whether to use an on-demand reference or the ID that we have to maintain anyway. I ran into issues using the reference in some cases so I tended to use the original ID to work around bicep dependency limitations. I'm not overly opinionated, however. Just wanted to share the reasoning behind it being an ID rather than a reference.

[flanakin]

I wouldn't recommend using this approach. While an on-demand reference sounds nice, nobody else can use that unless we document the names and treat them as an API that never changes. If we use the variable in hub.routing.subnets, then we can tell everyone to use that forever without worrying about the name, even if we change it. The reason for us to use that same variable is because if we don't, then there might be a latent bug if the variable gets out of sync. When we rely on the same variable customers will use for extensibility, then we're much less likely to ship a bug.

Suggested change

	id: vNet::scriptSubnet.id
	id: hub.routing.subnets.scripts

[flanakin]

Same as above

Suggested change

	id: vNet::finopsHubSubnet.id
	id: hub.routing.subnets.storage

[flanakin]

I would not recommend this change. The hub.routing.networkName property is available to all apps. We should use that as the source of truth. If that's the same as vNetName here, then remove vNetName.

Suggested change

	var nsgName = '${vNetName}-nsg'
	var nsgName = '${hub.routing.networkName}-nsg'

[flanakin]

I couldn't make this part of the next suggestion, but this line is a duplicate. The source of truth is in hub-types.bicep.

Suggested change

var vNetName = '${safeHubName}-vnet-${hub.location}'

[microsoft-github-policy-service]

@@MSBrett: you have some new feedback!

Please review and resolve all comments and I'll let reviewers know by removing the Needs: Attention label. If I miss anything, just reply with #needs-review and I'll update the status.