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

Private DNS Zone Virtual Network Link - Creation fails for cross-tenant #23238

Closed
1 task done
industrialzombie opened this issue Sep 11, 2023 · 15 comments · Fixed by #23420
Closed
1 task done

Private DNS Zone Virtual Network Link - Creation fails for cross-tenant #23238

industrialzombie opened this issue Sep 11, 2023 · 15 comments · Fixed by #23420

Comments

@industrialzombie
Copy link

Is there an existing issue for this?

  • I have searched the existing issues

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment and review the contribution guide to help.

Terraform Version

0.12.31

AzureRM Provider Version

3.72.0

Affected Resource(s)/Data Source(s)

azurerm_private_dns_zone_virtual_network_link

Terraform Configuration Files

terraform {
  required_version = "0.12.31"
  required_providers {
    azurerm = {
      source  = "hashicorp/azurerm"
      version = "3.72.0"
    }
  }
}

locals {
  // Fill these in with your own values
  tenant_id_a                    = ""
  tenant_id_b                    = ""
  subscription_id_a              = ""
  virtial_network_id_in_tenant_b = ""
  client_id                      = ""
  client_secret                  = ""
  domain_name                    = ""
}

provider "azurerm" {

  tenant_id       = local.tenant_id_a
  subscription_id = local.subscription_id_a
  client_id       = local.client_id
  client_secret   = local.client_secret

  auxiliary_tenant_ids = [local.tenant_id_b]

  features {
  }
}

resource "azurerm_resource_group" "this" {
  name     = "test_vnet_link_cross_tenant"
  location = "centralus"
}

resource "azurerm_private_dns_zone" "this" {
  name                = local.domain_name
  resource_group_name = azurerm_resource_group.this.name
}

resource "azurerm_private_dns_zone_virtual_network_link" "this" {
  name                  = local.domain_name
  resource_group_name   = azurerm_resource_group.this.name
  private_dns_zone_name = azurerm_private_dns_zone.this.name
  virtual_network_id    = local.virtial_network_id_in_tenant_b
  registration_enabled  = false
}

Debug Output/Panic Output

azurerm_private_dns_zone_virtual_network_link.this: Creating...

Error: creating/updating Virtual Network Link (Subscription: "[redacted]"
Resource Group Name: "test_vnet_link_cross_tenant"
Private Dns Zone Name: "[redacted]"
Virtual Network Link Name: "[redacted]"): performing CreateOrUpdate: unexpected status 403 with error: LinkedAuthorizationFailed: The client has permission to perform action 'Microsoft.Network/virtualNetworks/join/action' on scope '/subscriptions/[redacted]/resourceGroups/test_vnet_link_cross_tenant/providers/Microsoft.Network/privateDnsZones/[redacted]/virtualNetworkLinks/[redacted]', however the current tenant '[redacted=tenant a]' is not authorized to access linked subscription '[redacted=subscription id in tenant b]'.

  on main.tf line 44, in resource "azurerm_private_dns_zone_virtual_network_link" "this":
  44: resource "azurerm_private_dns_zone_virtual_network_link" "this" {

Expected Behaviour

Private DNS Zone VNet Link should be created successfully.

Actual Behaviour

Using version 3.66.0, this works as expected.

All versions since 3.67.0 (up to 3.72.0 as of now) give the above error and the VNet link is not created.

Steps to Reproduce

Arrange:

  1. create a subscription in tenant A
  2. create a subscription in tenant B
  3. create a VNet in subscription B
  4. create an app registration with client secret in tenant A
  5. grant admin consent to enterprise application for app registration in tenant B

Act:

  1. configure locals with details from setup, specifically app registration client secret from step 4
  2. terraform apply

Important Factoids

Admin consent was already granted to the enterprise application and this was working as of version 3.66.0

References

No response

@magodo
Copy link
Collaborator

magodo commented Sep 12, 2023

This probably relates to the upgrading of the underlying transport layer to the github.com/hashicorp/go-azure-sdk since v3.67.0 (as is stated here).

But since I didn't find any other complaint about the cross tenant functionality not work on the new transport layer, would you mind to double check your configurations, e.g. retry with the v3.66.0, or with other tools like azure CLI, to make sure it indeed is the provider's bug.

Meanwhile, if you can setup some mitmproxy, you shall be able to see the ARM request, where you can check whether the access tokens for the auxiliary tenant is indeed sent to Azure.

@industrialzombie
Copy link
Author

Hi @magodo,

Using the az cli I am able to successfully create the resource, so this does appear to be a Terraform issue (or the underlying azure SDK as you mentioned).

I ran the requests through a mitm proxy as suggested.

  • The PUT request made by the az cli correctly includes the "x-ms-authorization-auxiliary" header.
  • The PUT request made by the Terraform provider does not contain this header.

@magodo
Copy link
Collaborator

magodo commented Sep 13, 2023

Hi @manicminer, do you know whether there is any issue with the cross-tenant scenario when using the go-azure-sdk transport layer?

I've checked with the provider setting, and noticed it has correctly specified the auxiliary tenants:

authConfig := &auth.Credentials{
Environment: *env,
ClientID: *clientId,
TenantID: d.Get("tenant_id").(string),
AuxiliaryTenantIDs: auxTenants,

@industrialzombie
Copy link
Author

Confirmed this is still an issue in 3.73.0 released yesterday.

@mdanylyuk
Copy link

I have the same problem with 3.68.0 provider version

@industrialzombie
Copy link
Author

Hey,
I had some more time today to go digging into the code.

If I understand correctly, the change here was to switch the networks service from using the autorest transport to using the new resource manager client from the go-azure-sdk.

Looking at the codebase, I can see that autorest client gets decorated to call the "AuxiliaryTokens" function of the authorizer. I don't see any equivalent call in the new resource manager client:

It calls the authorizer's "Token" function, but not the "AuxiliaryTokens" function.

https://github.com/hashicorp/go-azure-sdk/blob/main/sdk/client/client.go#L333

https://github.com/hashicorp/go-azure-sdk/blob/main/sdk/auth/autorest/auth.go#L43

This aligns with what I see in the calls being made by the provider in my example; the calls to interact with the resource group are using the autorest client and are correctly have the extra HTTP header, whereas the calls using the new resource manager client do not.

Lastly, I'm pretty new to golang and this is not my codebase so this is all speculation. Please feel free to correct me if I'm way off base here!

@industrialzombie
Copy link
Author

Also, if I'm right, please can we get the following change reverted until the resource manager client supports auxiliary tokens?

https://github.com/hashicorp/terraform-provider-azurerm/pull/22681/files#diff-71f78de5d72d9452156b0c94b0a4972380457c9cc846db9fc94eb7f1d11009f1

@manicminer
Copy link
Contributor

@industrialzombie Thanks for digging into this. Ideally we'd prefer to roll forward and fix the SDK issue rather than roll back to the legacy SDK. I'll look into it and try to issue a fix.

@manicminer manicminer self-assigned this Sep 21, 2023
@manicminer
Copy link
Contributor

Hi @manicminer, do you know whether there is any issue with the cross-tenant scenario when using the go-azure-sdk transport layer?

@magodo I suspect there may be a bug in the setup or handling of multi-tenant auth, am looking into it.

@manicminer
Copy link
Contributor

Thanks again for reporting this authorization issue. We should have a fix out soon.

@industrialzombie
Copy link
Author

Hey @katbyte

I can see that a test was added for this, but I don't see the underlying issue is resolved yet.
hashicorp/go-azure-sdk#665

If I'm right, please can you re-open this issue?

Also, this has been open now for a while and is still causing problems for my organization.
I appreciate the work that @manicminer has put in on the azure sdk, but is it possible to reconsider the approach of rolling back the regression rather than waiting for rolling forwards?

Thanks

@manicminer
Copy link
Contributor

@industrialzombie Sorry about that, I'll reopen this issue. I'm going to try and get the SDK fix in this week to resolve this.

@manicminer manicminer reopened this Nov 27, 2023
@manicminer manicminer added this to the v3.83.0 milestone Nov 27, 2023
@manicminer
Copy link
Contributor

manicminer commented Nov 27, 2023

Note to self: SDK update to include hashicorp/go-azure-sdk#665, and changelog entry are needed to close this

manicminer added a commit that referenced this issue Nov 29, 2023
@manicminer
Copy link
Contributor

The fix for this was merged in #24063 and this should now be resolved in the next release!

Copy link

github-actions bot commented May 1, 2024

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems similar to this, 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 May 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.