Azure / arm-template-whatif

A repository to track issues related to what-if noise suppression
MIT License
85 stars 13 forks source link

what-if reporting modify to VNet #233

Open srininz77 opened 2 years ago

srininz77 commented 2 years ago

I created a VNet using: resource vnet 'Microsoft.Network/virtualNetworks@2020-08-01'

And I created couple of Subnets in this VNet using: resource subnet 'Microsoft.Network/virtualNetworks/subnets@2021-02-01'

I deployed the bicep template, it sucessfully created VNet and SubNets.

I then manually created a peering between the VNet and the Hub VNet.

I ran what-if on the same template and I was expecting it to report "No" changes but it reported "1 to modify"

Output from what-if: { "after": { "apiVersion": "2020-08-01", "id": "/subscriptions/subA/resourceGroups/rgA/providers/Microsoft.Network/virtualNetworks/vnetA", "location": "australiaeast", "name": "vnetA", "properties": { "addressSpace": { "addressPrefixes": [ "x.x.x.x/x" ] }, "enableDdosProtection": false }, "resourceGroup": "rgA", "type": "Microsoft.Network/virtualNetworks" }, "before": { "apiVersion": "2020-08-01", "id": "/subscriptions/subA/resourceGroups/rgA/providers/Microsoft.Network/virtualNetworks/vnetA", "location": "australiaeast", "name": "vnetA", "properties": { "addressSpace": { "addressPrefixes": [ "x.x.x.x/x" ] }, "enableDdosProtection": false, "subnets": [ { "name": "snetA", "properties": { "addressPrefix": "x.x.x.x/x", "networkSecurityGroup": { "id": "/subscriptions/subA/resourceGroups/rgA/providers/Microsoft.Network/networkSecurityGroups/nsgA", "resourceGroup": "rgA" }, "privateEndpointNetworkPolicies": "Enabled", "privateLinkServiceNetworkPolicies": "Enabled", "routeTable": { "id": "/subscriptions/subA/resourceGroups/rgA/providers/Microsoft.Network/routeTables/rtA", "resourceGroup": "rgA" } } }, { "name": "snetB", "properties": { "addressPrefix": "x.x.x.x/x", "networkSecurityGroup": { "id": "/subscriptions/subA/resourceGroups/rgA/providers/Microsoft.Network/networkSecurityGroups/nsgA", "resourceGroup": "rgA" }, "privateEndpointNetworkPolicies": "Enabled", "privateLinkServiceNetworkPolicies": "Enabled", "routeTable": { "id": "/subscriptions/subA/resourceGroups/rgA/providers/Microsoft.Network/routeTables/rtA", "resourceGroup": "rgA" } } } ], "virtualNetworkPeerings": [ { "name": "vnetA_to_vnetHub", "properties": { "allowForwardedTraffic": true, "allowGatewayTransit": false, "allowVirtualNetworkAccess": true, "doNotVerifyRemoteGateways": false, "remoteAddressSpace": { "addressPrefixes": [ "x.x.x.x/x" ] }, "remoteVirtualNetwork": { "id": "/subscriptions/subB/resourceGroups/rgB/providers/Microsoft.Network/virtualNetworks/vnetHub", "resourceGroup": "rgB" }, "useRemoteGateways": false } } ] }, "resourceGroup": "rgA", "type": "Microsoft.Network/virtualNetworks" }, "changeType": "Modify", "delta": [ { "after": null, "before": [ { "name": "snetA", "properties": { "addressPrefix": "x.x.x.x/x", "networkSecurityGroup": { "id": "/subscriptions/subA/resourceGroups/rgA/providers/Microsoft.Network/networkSecurityGroups/nsgA", "resourceGroup": "rgA" }, "privateEndpointNetworkPolicies": "Enabled", "privateLinkServiceNetworkPolicies": "Enabled", "routeTable": { "id": "/subscriptions/subA/resourceGroups/rgA/providers/Microsoft.Network/routeTables/rtA", "resourceGroup": "rgA" } } }, { "name": "snetB", "properties": { "addressPrefix": "x.x.x.x/x", "networkSecurityGroup": { "id": "/subscriptions/subA/resourceGroups/rgA/providers/Microsoft.Network/networkSecurityGroups/nsgA", "resourceGroup": "rgA" }, "privateEndpointNetworkPolicies": "Enabled", "privateLinkServiceNetworkPolicies": "Enabled", "routeTable": { "id": "/subscriptions/subA/resourceGroups/rgA/providers/Microsoft.Network/routeTables/rtA", "resourceGroup": "rgA" } } } ], "children": null, "path": "properties.subnets", "propertyChangeType": "Delete" }, { "after": null, "before": [ { "name": "vnetA_to_vnetHub", "properties": { "allowForwardedTraffic": true, "allowGatewayTransit": false, "allowVirtualNetworkAccess": true, "doNotVerifyRemoteGateways": false, "remoteAddressSpace": { "addressPrefixes": [ "x.x.x.x/x" ] }, "remoteVirtualNetwork": { "id": "/subscriptions/subB/resourceGroups/rgB/providers/Microsoft.Network/virtualNetworks/vnetHub", "resourceGroup": "rgB" }, "useRemoteGateways": false } } ], "children": null, "path": "properties.virtualNetworkPeerings", "propertyChangeType": "Delete" } ], "resourceId": "/subscriptions/subA/resourceGroups/rgA/providers/Microsoft.Network/virtualNetworks/vnetA" }

I ignored the what-if report and went ahead to deploy the template to check if it actually deletes the subnets and vnet peering and it didn't delete them which is what I was expecting.

johndowns commented 2 years ago

Hey @srininz77, technically the what-if command is actually reporting correctly here. I'll explain why, and then give you a suggestion for how you might be able to work around this.

Explanation

Subnets can be defined in VNets in two different ways when you use ARM templates/Bicep.

Approach 1, which it looks like you're using, is to define the VNet as one resource, and the subnet as a separate resource:

resource virtualNetwork 'Microsoft.Network/virtualNetworks@2021-02-01' = {
  name: 'name'
  location: resourceGroup().location
  properties: {
    addressSpace: {
      addressPrefixes: [
        '10.0.0.0/16'
      ]
    }
  }
}

resource subnet 'Microsoft.Network/virtualNetworks/subnets@2021-02-01' = {
  parent: virtualNetwork
  name: 'Subnet-1'
  properties: {
    addressPrefix: '10.0.0.0/24'
  }
}

Approach 2 is to configure a single VNet resource and use the subnets property to define the subnets:

resource virtualNetwork 'Microsoft.Network/virtualNetworks@2021-02-01' = {
  name: 'name'
  location: resourceGroup().location
  properties: {
    addressSpace: {
      addressPrefixes: [
        '10.0.0.0/16'
      ]
    }
    subnets: [
      {
        name: 'Subnet-1'
        properties: {
          addressPrefix: '10.0.0.0/24'
        }
      }
    ]
  }
}

It looks like you're using approach 1. Unfortunately, the way ARM works means that there are some significant (and non-obvious) downsides to this approach.

How ARM works

When you submit a deployment, the ARM engine analyses the dependencies between all the resources you've defined and then plans a deployment strategy to parallelise the deployments as much as possible. But ARM works based on individual resources, and because the subnet and the VNet are two separate resources, ARM has to submit the VNet deployment first, and then after it's completed, submit the subnet deployment.

In approach 1, the definition of the VNet resource doesn't include any subnets. The underlying VNet API interprets this as "there are no subnets in this VNet". So, this means that when the ARM deployment engine submits the VNet deployment, it's effectively saying to ARM "please deploy a VNet with no subnets", which is also effectively saying "please delete any subnets on the VNet". Then as soon as that's finished, it says "now please add this subnet".

Impact

When you're deploying a VNet for the first time, the behaviour I just outlined doesn't really matter. But for redeployments, this behaviour has several side effects, including:

  1. If the subnet already exists and is in use, the VNet deployment is likely to fail. This is because you can't usually delete a subnet that's in use.
  2. Even if the VNet deployment succeeds (and there are some situations where you can), you may experience some downtime for the resources that are deployed to the VNet, because the subnet will disappear briefly and then reappear.
  3. The what-if operation looks at the all of the changes that will be applied, and just as you've identified in your issue, it tells you that the VNet will have its subnets removed. This is actually correct for the reasons I've just outlined.

Workaround

To avoid this issue when you redeploy your templates, you should use approach 2 from above - i.e. define the subnets in the VNet resource.

We are aware of the issue and are trying to fix it, but it will likely take some time.

cc @DanielLarsenNZ

srininz77 commented 2 years ago

Cool. I had the same issue with my route table routes where I used approach 2 as a work around. I will use approach 2 for subnets too.

srininz77 commented 2 years ago

@johndowns I tried the workaround but we now have a new issue. We have included subnets as part of the vnet resource definition and what-if is reporting "1 to ignore" when I was expecting "No change". Here is the what-if output.

{
      "after": {
        "id": "/subscriptions/xxx/resourceGroups/rg1/providers/Microsoft.Network/virtualNetworks/vnet1",
        "location": "australiaeast",
        "name": "vnet1",
        "resourceGroup": "rg1",       
        "type": "Microsoft.Network/virtualNetworks"
      },
      "before": {
        "id": "/subscriptions/xxx/resourceGroups/rg1/providers/Microsoft.Network/virtualNetworks/vnet1",
        "location": "australiaeast",
        "name": "vnet1",
        "resourceGroup": "rg1",
        "type": "Microsoft.Network/virtualNetworks"
      },
      "changeType": "Ignore",
      "delta": null,
      "resourceId": "/subscriptions/xxx/resourceGroups/rg1/providers/Microsoft.Network/virtualNetworks/vnet1"
    }

The interesting thing is it's not displaying the subnets either.

BICEP template:

param vNetSettings object

resource vnet 'Microsoft.Network/virtualNetworks@2020-08-01' = {
  name: vNetSettings.name
  location: resourceGroup().location
  tags: vNetSettings.tags
  properties: {
    addressSpace: {
      addressPrefixes: [
        vNetSettings.addressPrefix
      ]
    }
    enableDdosProtection: false
    subnets:[for subnet in vNetSettings.subnets:{
      name: subnet.name
          properties: {
            addressPrefix: subnet.addressPrefix
            networkSecurityGroup:{
              id: subnet.nsgId
            }
            routeTable:{
              id: subnet.routeTableId
            }
            delegations: []
            privateEndpointNetworkPolicies: 'Enabled'
            privateLinkServiceNetworkPolicies: 'Enabled'
          }
    }] 
  }
}
output name string = vnet.name
output id string = vnet.id
olljanat commented 2 years ago

We are aware of the issue and are trying to fix it, but it will likely take some time.

@johndowns @stephaniezyen any progress with this one? I can see that at least label have been changed.

Before this one is fixed it is very hard to implement pull request validation for deployment which uses ALZ-Bicep

Also I think that #179 is related.

alex-frankel commented 2 years ago

No updates unfortunately. Networking RP most likely needs some API changes to fix this behavior.

Related to https://github.com/Azure/azure-quickstart-templates/issues/2786