Azure / azure-quickstart-templates

Azure Quickstart Templates
https://aka.ms/azqst
MIT License
13.99k stars 16.09k forks source link

Create VNET without destroying all subnets #2786

Closed gregjhogan closed 3 months ago

gregjhogan commented 7 years ago

I want to create subnets dynamically (separately from my VNET). I see there is a way to create a subnet inside an existing VNET, indicating that a subnet is a child resource of a VNET.

https://github.com/Azure/azure-quickstart-templates/blob/master/101-subnet-add-vnet-existing/azuredeploy.json

However, I can't find a way to re-deploy the VNET template without blowing away all subnets, because if I leave off the subnets property it deletes all the VNETs. Is there a way to deploy a VNET without including (and destroying) the subnets?

Here is an example template deploying a VNET (which destroys all subnets)

{
    "$schema": "https://schema.management.azure.com/schemas/2015-01-01/deploymentTemplate.json#",
    "contentVersion": "1.0.0.0",
    "parameters": {
        "vnetAddressPrefixes": {
            "type": "array"
        }
    },
    "variables": {
        "apiVersion": "2015-06-15",
        "namePrefix": "[concat(replace(subscription().displayName, ' ', '_'), '_')]",
        "vnetName": "[concat(variables('namePrefix'), 'VNET')]"
    },
    "resources": [
        {
            "apiVersion": "[variables('apiVersion')]",
            "type": "Microsoft.Network/virtualNetworks",
            "name": "[variables('vnetName')]",
            "location": "[resourceGroup().location]",
            "dependsOn": [],
            "properties": {
                "addressSpace": {
                    "addressPrefixes": "[parameters('vnetAddressPrefixes')]"
                }
            }
        }
    ]
}
scottgorlick commented 4 years ago

now that we have management group and subscription, and even multi subscription deployments, can we please make this work?

I have a scenario where we deploy many resource groups with VNET Peering... one option in our architecture is to deploy Azure Bastion... once we deploy bastion (which adds a subnet to our original VNET), our system becomes immutable by our deployment due to this failure... our fix was to detect the bastion subnets, delete them, and then do the deployment.

MCKLMT commented 4 years ago

@NarayanAnnamalai @anavinahar Can you help on this issue?

C0DINGpanda commented 4 years ago

This happens only with that deployment of V-net not with the subnet's. I added a condition(only in virtual network resource) in creation of VNET so that only in first time deployment Virtual network gets created and subnets can be update\create based on parameter(i am dealing subnet as separate resource not as a subnet property.) this way i can ADD a subnet :)

"resources": [ { "type": "Microsoft.Network/virtualNetworks", "apiVersion": "2019-09-01", "condition": "[parameters('NewDeployment')]", "name": "[parameters('VirtualNetworkName')]", "location": "[parameters('Location')]", "properties": {} }, { "type": "Microsoft.Network/virtualNetworks/subnets", "apiVersion": "2019-09-01", "dependsOn": [ "[resourceId('Microsoft.Network/virtualNetworks', parameters('VirtualNetworkName') )]" ], "name": "[concat(variables('TVirtualNetworkName'),parameters('ApplicationGatewaySubnetName'))]", "properties": { "addressPrefix": "[parameters('ApplicationGatewaySubnetPrefix')]" } },

scottgorlick commented 4 years ago

would be nice to have analogous behavior to ARM's deployment mode settings, "Complete" or "Incremental", but this should suffice with a limited prerequisite check... in my bash az cli deployment (that wraps around the declarative deployments, i'm doing this now, per your workaround". my condition statement will be:

"condition": "[parameters('deployVnet')]"

local deployVnet="false"
if [[ -z $(az network list --resource-group "${MY_GROUP_NAME}" --name "${MY_VNET_NAME}" --query "[].name" --output tsv) ]]; then
  deployVnet="true"
fi
# other params, but simplified for this example:
local params=local params="{ 
  \"deployVnet\": { \"value\": $deployVnet}
}"  
 az deployment sub create \
        --location $DEPLOYMENT_LOCATION \
        --name "$DEPLOYMENT_NAME" \
        --template-file "$DEPLOYMENT_TEMPLATE_FILE_PATH" \
        --parameters "$params" \
        --query 'properties.outputs' | jq "map_values(. | .value)"
nirvanatao commented 4 years ago

This solution worked for me https://pkm-technology.com/azure-vnet-json/

scottgorlick commented 4 years ago

@esahanhunter , with the setup you describe above (snipped) - i think the issue is resourceId(...) call in dependsOn property:

"type": "Microsoft.Network/virtualNetworks",
"apiVersion": "2019-09-01",
"condition": "[parameters('NewDeployment')]",
"name": "[parameters('VirtualNetworkName')]",
"location": "[parameters('Location')]",
"properties": {}
},
{
"type": "Microsoft.Network/virtualNetworks/subnets",
"apiVersion": "2019-09-01",
"dependsOn": [
"[resourceId('Microsoft.Network/virtualNetworks', parameters('VirtualNetworkName') )]"
],

I keep getting our favorite error: InvalidTemplate - Deployment template validation failed: 'The resource 'Microsoft.Network/virtualNetworks/' is not defined in the template. Please see https://aka.ms/arm-template for usage details

I'm using AZ CLI to deploy from an ubuntu host... pretty sure the error is not happening on ARM server side. I don't see any deployment errors in Azure logs when i get this error.

C0DINGpanda commented 4 years ago

@scottgorlick u might be passing wrong name in parameter i just copied a working snippet here :)

scottleyg commented 4 years ago

Your scenario is very simple. I assure you, my deployments, that work 100% the first time and fail 100% on redeploy with this error are properly crafted, but just don't work with this broken semantic.

I can make your simple template work... and if i apply the pattern to my deployment (which is a subscription level deployment to a variable amount of resource groups), it will succeed when I deploy one of the linked templates by itself. When i deploy the whole apparatus, it fails.

Overly simplified workarounds are not valid always. Try your fix in a nested template and a linked template and tell me how it works for you, it doesn't work for me.

VOVELEE commented 4 years ago

Any update on this one? This is behavior is quite limiting any scenarios for Blueprint management where you leave subnet management to resources and Blueprint only manage the configuration of DNS, address space and VNET peerings.

audhage commented 4 years ago

Any update on this one? This is behavior is quite limiting any scenarios for Blueprint management where you leave subnet management to resources and Blueprint only manage the configuration of DNS, address space and VNET peerings.

It's a major pain for sure. Working on a similar scenario and what I have done is to separate the create and update operations in separate templates. I use the same strategy for keyvaults and their access policies.

I am using a simple boolean blueprint parameter to enforce create or update behavior, but you can use Microsoft.Resources/deploymentScript to do this as well.

Create:

{
  "type": "Microsoft.Blueprint/blueprints/artifacts",
  "name": "vnet-1-create",
  "kind": "template",
  "properties": {
    "displayName": "vnet-1-create",
    "resourceGroup": "rg-vnet",
    "parameters": {
      "deployed": {
        "value": "[parameters('vnetDeployed')]"
      },
      "addressPrefix": {
        "value": "[parameters('vnetAddressPrefix')]"
      }
    },
    "template": {
      "$schema": "https://schema.management.azure.com/schemas/2019-04-01/deploymentTemplate.json#",
      "contentVersion": "1.0.0.0",
      "parameters": {
        "deployed": {
          "type": "bool"
        },
        "addressPrefix": {
          "type": "string"
        }
      },
      "variables": {
        "name": "[concat(resourceGroup().name, '-vnet')]",
        "id": "[resourceId('Microsoft.Network/virtualNetworks', variables('name'))]"
      },
      "resources": [
        {
          "apiVersion": "2020-04-01",
          "type": "Microsoft.Network/virtualNetworks",
          "name": "[variables('name')]",
          "location": "[resourceGroup().location]",
          "condition": "[not(parameters('deployed'))]",
          "properties": {
            "addressSpace": {
              "addressPrefixes": [
                "[parameters('addressPrefix')]"
              ]
            },
            "subnets": []
          }
        }
      ],
      "outputs": {
        "object": {
          "type": "object",
          "value": "[reference(variables('id'), '2020-04-01', 'Full')]"
        }
      }
    }
  }
}

Update:

{
  "type": "Microsoft.Blueprint/blueprints/artifacts",
  "name": "vnet-2-update",
  "kind": "template",
  "properties": {
    "displayName": "vnet-2-update",
    "resourceGroup": "rg-vnet",
    "dependsOn": [
      "vnet-1-create"
    ],
    "parameters": {
      "object": {
        "value": "[artifacts('vnet-1-create').outputs.object]"
      }
    },
    "template": {
      "$schema": "https://schema.management.azure.com/schemas/2015-01-01/deploymentTemplate.json#",
      "contentVersion": "1.0.0.0",
      "parameters": {
        "object": {
          "type": "object"
        }
      },
      "variables": {
        "id": "[parameters('object').resourceId]",
        "name": "[last(split(variables('id'), '/'))]"
      },
      "resources": [
        {
          "type": "Microsoft.Network/virtualNetworks",
          "apiVersion": "2020-04-01",
          "name": "[variables('name')]",
          "location": "[parameters('object').location]",
          "properties": {
            "addressSpace": "[parameters('object').properties.addressSpace]",
            "subnets": "[parameters('object').properties.subnets]"
          }
        }
      ]
    }
  }
}
drewswiredin commented 3 years ago

This is one of the many limitations of ARM. In order to achieve fully automated infrastructure, ARM templates alone just don't cut it. ARM automation must be bolstered with scripting and creative usage of parameters and outputs. Another example like this one is keyvault access policies. They get destroyed upon redeployment of the keyvault during even 'Incremental' deployments. The trick here is to include a script task prior to the ARM deployment task. The script will query and take inventory of the resouces/properties that you wish to survive the ARM deployment. You would write the script in a way that outputs an array of objects containing the information needed to recreate the existing subnets etc. Then output that data into the pipeline to be picked up by the subsequent ARM deployment task. The ARM template would need to accept the array as an input and the reconstruction of the resources is possible by writing an intelligent ARM template using loops/conditions etc. I've done it with the keyvault issue and It's looking as if though I'll be doing it to solve this issue. I'll try to post my solution and link it here in the near future.

frankzo commented 3 years ago

The script will query and take inventory of the resouces/properties that you wish to survive the ARM deployment. You would write the script in a way that outputs an array of objects containing the information needed to recreate the existing subnets etc.

This could become a problem from a desired state perspective. I would like everything to stay in code. If you first query what is already there (if that is the case) and then "redeploy" it you might end up with subnets manually created being part of the deployment.

With Key Vault Access Policies or Web App settings you run into the same challenge. You could try and incorporated the "union" function in ARM to combine 2 arrays and deploy them as one.

"variables": {
        "defaultProperties": [
            {
                "name": "Setting1",
                "value": "[parameters('Setting1')]"
            },
            {
                "name": "Setting2",
                "value": "Value for Setting2"
            }
        ],
        "appProperties": "[union(variables('defaultProperties'), parameters('appProperties'))]"
{
    "$schema": "https://schema.management.azure.com/schemas/2015-01-01/deploymentParameters.json#",
    "contentVersion": "1.0.0.1",
    "parameters": {
        "appProperties": {
            "value": [
                {
                    "name": "Setting3",
                    "value": "Value for setting3"
                },
                {
                    "name": "Setting4",
                    "value": "Value for setting4"
                }
            ]
        }
    }
}
drewswiredin commented 3 years ago

I would like everything to stay in code. If you first query what is already there (if that is the case) and then "redeploy" it you might end up with subnets manually created being part of the deployment.

That's exactly what we want in our case.

The issue being addressed here is how to persist configurations/resources that were 'deployed manually'. And by 'manually' we really mean any resources/properties that are not included in the initial ARM deployment of the parent resource. This may seem like an anti-pattern but there are some instances where we want to manage the creation of certain resources after the parent. In our case we are defining some subnets in the initial VNet ARM template. However, since we don't know the exact number/sizes of subnets we will need in the future and we don't want to have to run 2 pipelines to deploy a subnet dependent resource. We want to take a dynamic approach by facilitating subnet creation/delegation in the same release pipelines as certain resources like App Services etc.

Another problem to tackle is cleaning up/repurposing orphaned subnets or address blocks no longer in use. Parameterizing the size desired for subnet creation, checking to see if there are enough contiguous addresses for the size desired etc. This is again all going to be solved by the dynamic creation/delegation script. It may not be the cleanest solution but it fits our requirements.

And yes the 'Union' function is exactly what you would want to do to merge the array of existing 'manually deployed' resources into the parent ARM deployment alongside the 'automated' resources. This is exactly what we are currently doing with KeyVault policies. In our case we do this to ensure that DevOps variable libraries maintain their connection to KeyVaults using the service principals and access policies that were created by the DevOps team after the initial KV deployments.

takekazuomi commented 3 years ago

The condition operator has been a great help to me. However, this limitation is a major impediment to ARM Template modularization. I want to modify and deploy the same template to maintain the environment. It is the worst to be destroyed by redeployment.

miqm commented 3 years ago

@alex-frankel could you give explanation why when deplyoing vnet without subnet property speficied it tries to remove it? when property is not specified, it should not touch it. if it's necessary to specify subnets in properties - why having separate resource?

alex-frankel commented 3 years ago

It's a reflection of an old api design pattern. In an ideal world, subnets are only child resources because they could (and often do) have different lifecycles.

The context is, at one point, in order to support more efficient resource creates, it was decided that we should support being able to create both VNET + Subnet in one PUT call. As a result, the subnet ended up as an array property on the vnet resource. According to the ARM RPC, vnets are actually doing the right thing because a PUT call should represent the current state of the resource. Therefore, since the subnet is just a property, it should be removed if it's not explicitly declared in subsequent VNET PUTs.

Of course, in practice, this tends to get in the way more than it is helpful. In theory, this would allow you to delete subnets efficiently, but I'm not sure how often that is done in reality.

The challenge is that changing the behavior would be a significant breaking change, so it has been difficult work to prioritize for the networking team. I'm in full support of the breaking change, but sadly this one is out of my hands..

miqm commented 3 years ago

perhaps a middle point would be to keep the property but if not provided - do nothing. if provided with empty array - clear subnets. that's how it works in some other api's i. e. in web/sites.

with so many properties it can be an overkill to need to specify them all, especially in incremental deployments.

gregjhogan commented 3 years ago

@alex-frankel why does this have to be a breaking change? add a new (optional) property that defaults to current behavior but when set to true allows new (improved) behavior. New behavior just means current subnets property on vnet is deprecated and instead you have to use child resources (which already exist) for subnets

alex-frankel commented 3 years ago

perhaps a middle point would be to keep the property but if not provided - do nothing

Even this is considered a breaking change believe it or not. In theory, there is someone out there relying on the behavior of not declaring subnets in order to delete them all. It certainly feels like an edge case, but I don't have the data to back that statement up.

alex-frankel commented 3 years ago

add a new (optional) property that defaults to current behavior but when set to true allows new (improved) behavior.

This could work - @anavinahar have you all looked into this as an option?

Anavi is no longer on the team, but I ping'd a longstanding internal thread with the suggestion @gregjhogan. Will update if anything positive develops from it.

miqm commented 3 years ago

perhaps a middle point would be to keep the property but if not provided - do nothing

Even this is considered a breaking change believe it or not. In theory, there is someone out there relying on the behavior of not declaring subnets in order to delete them all. It certainly feels like an edge case, but I don't have the data to back that statement up.

this is exactly why we have api versioning, right? want to have old behaviour? use old api...

audhage commented 3 years ago

perhaps a middle point would be to keep the property but if not provided - do nothing

Even this is considered a breaking change believe it or not. In theory, there is someone out there relying on the behavior of not declaring subnets in order to delete them all. It certainly feels like an edge case, but I don't have the data to back that statement up.

this is exactly why we have api versioning, right? want to have old behaviour? use old api...

Spot on. It's already versioned. There's plenty of breaking changes all over the ARM space. Fail to see how this is any different.

alex-frankel commented 3 years ago

It's possible with tools like PS or CLI there could be unforeseen consequences since you don't pick individual api versions on resources when using those tools. Either way, I agree that this is a valid breaking change to make and this is why we have api versions.

jamesbjackson commented 3 years ago

Can this also be applied to DNS records and zones as they suffer the same problem? On 29 Jan 2021, 19:19 +0000, Alex Frankel notifications@github.com, wrote:

It's possible with tools like PS or CLI there could be unforeseen consequences since you don't pick individual api versions on resources when using those tools. Either way, I agree that this is a valid breaking change to make and this is why we have api versions. — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.

alanbaldwin commented 3 years ago

Route tables would be useful as well, as routes are sub resources. There's a few other things I can think of - load balancer backend pools - preferably anything that works this way.

kiranbaddi commented 3 years ago

I thinking we may just have to give up on ARM Templates at this rate and look at Terraform. Why can't ARM Templates work like CloudFormation Stacks and only update the properties defined.

Absolutely. So many limitations with ARM templates. Terraform is the way to go.

abatishchev commented 3 years ago

Any news on this? 5 (five) years and several api versions later - the issue is still here. Deployment of a vnet attempt to delete all its subnets and understandably fails. Key Vault access policies and Traffic Manager endpoints "work" (they don't) same way.

Here's what I attempt to do:

{
  "type": "Microsoft.Network/virtualNetworks",
  "apiVersion": "[variables('networkApiVersion')]",
  "name": "[variables('vnetName')]",
  "location": "[parameters('location')]",
  "properties": {
    "addressSpace": {
      "addressPrefixes": [
        "10.0.0.0/16"
      ]
    }
  }
},
{
  "type": "Microsoft.Network/virtualNetworks/subnets",
  "apiVersion": "[variables('networkApiVersion')]",
  "name": "[concat(variables('vnetName'), '/', variables('vngSubnetName'))]",
  "location": "[parameters('location')]",
  "properties": {
    "addressPrefix": "10.0.252.0/24"
  },
  "dependsOn": [
    "[resourceId('Microsoft.Network/virtualNetworks/', variables('vnetName'))]"
  ]
},
{
  "type": "Microsoft.Network/virtualNetworks/subnets",
  "apiVersion": "[variables('networkApiVersion')]",
  "name": "[concat(parameters('vnetName'), '/', 'subnet-', parameters('subnets')[copyIndex('subnetLoop')])]",
  "location": "[parameters('location')]",
  "properties": {
    "addressPrefix": "[concat('10.0.', parameters('subnets')[copyIndex('subnetLoop')], '.0/24')]",
    "networkSecurityGroup": {
      "id": "[resourceId('Microsoft.Network/networkSecurityGroups', variables('nsgName')]"
    }
  },
  "copy": {
    "name": "subnetLoop",
    "count": "[length(parameters('subnets'))]",
    "mode": "Parallel"
  },
  "dependsOn": [
    "[resourceId('Microsoft.Network/virtualNetworks/', variables('vnetName'))]"
  ]
}

It works once, the very first time. A subsequent deployment fail as it attempt to deploy a vnet with "subnets": null. Why on earth?

nirvanatao commented 3 years ago

this solution works:

{
      "apiVersion": "2018-10-01",
      "type": "Microsoft.Network/virtualNetworks",
      "name": "[variables('vnetName')]",
      "location": "[resourceGroup().location]",
      "dependsOn": [
      ],
      "properties": {
        "addressSpace": {
          "addressPrefixes": [
            "172.40.0.0/16"
          ]
        },
        "subnets": [
          {
            "name": "[variables('nameOfDefaultSubnet')]",
            "properties": {
              "addressPrefix": "172.40.0.0/24"
            }
          },
          {
            "name": "[variables('nameOfSubnet1')]",
            "properties": {
              "addressPrefix": "172.40.1.0/24"
            }
          },
          {
            "name": "[variables('nameOfSubnet2')]",
            "properties": {
              "addressPrefix": "172.40.240.0/20"
            }
          }
        ]
      }
    },
abatishchev commented 3 years ago

@nirvanatao it works because you create a vnet with the subnets. That always works.

CtrlDot commented 3 years ago

As an original commentator on this issue, I think it's time to unsubscribe. Seems like this will never be fixed.

ChristopherGLewis commented 3 years ago

I've actually got a partial work-around for this. First some comments on the issue and why it's so hard.

tl;dr https://github.com/ChristopherGLewis/vNet-Bicep

1) This is best solved by completely defining the subnets in the vNet block. This solves all the issues since you have a full definition of the vNet/subnets.

2) That being stated, my issue with this approach is that there are different requirements for subnets (most subnets can have RouteTables and NSGs. Gateways can have RT's but no NSGs. BastionSubnet can have an NSG but no RT. FirewallSubnet can have neither). In defining your subnet within the vnet resource, using conditional logic or nulls on the NSG/RT ids is NOT allowed, or involves complex variable evaluations to build up the appropriate Json blocks.

3) None of this complexity would be required if the subnet object in ARM would allow for Nulls for the RouteTable and NSGTable ids:

routeTable: {
      id: ''       <-- Causes errors
}

-or-

routeTable:  null() <-- Causes errors

My solution is here: https://github.com/ChristopherGLewis/vNet-Bicep

It uses a complex object to describe the vNet and Subnets that are being built in this resource group. The vNet loop has a minimalistic subnet block that prevents the subnets from being dropped and re-added.

Note that the NSG/RT's do disappear and get re-added, so this is still somewhat disruptive. However, it does not break if you have NIC's assigned to the subnets since the subnets don't get dropped. It does allow vNet re-addressing if all the subnets work in the new address space, and does allow adding.resizing/removing subnets as long as there's nothing attached to them.

Please let me know if this helps.

bmoore-msft commented 3 years ago

Yes it's been a while... but ok. Are any of you up for a short conversation on this topic? We have some ideas on how to solve but definitely want to get some feedback on them before deciding... email me if you're interested (just truncate the -msft on my gh handle)

kumarz commented 3 years ago

@bmoore-msft I am up for the conversation as I am facing this issue for our deployment automation. Please feel free to include me in the discussion.

I have the same requirement to add subnets in the future, however, it is trying to delete the existing subnet (redeploy), I would prefer it to be an incremental mode for the existing Vnet. I do understand that it's an array of subresource for Vnet. However, from a customer perspective, it is an incremental deployment to an existing resource.

It is common for us to get evolved to add more subnets and re-use the same V-net for our application. So please let us know if there is a solution for this. I would really appreciate it.

bmoore-msft commented 3 years ago

Sure - just shoot me an email, lmk what timezone you're in and I'll schedule some some time.

JustinGrote commented 3 years ago

@bmoore-msft I'm good to discuss as well, the API should simply be changed to remove the subnet definition from being a property of the vnet and instead just allow them to be defined as child resources. Breaking change yes, but all ARM templates today have the API version hardcoded so they have to specify the new version, and if you see something with the old pattern and the new API, you can throw a specific error informing the user they need to change their behavior or use API version xxx or below. Alternatively if there's a way to make it backwards compatible so that 'subnets' property can be additive with child resources and gets treated more as an 'incremental' deployment rather than a 'complete' deployment that's good too.

Ultimately I want to be able to do this:

resource vnet 'Microsoft.Network/virtualNetworks@2021-02-01' = {
  name: BaseName
  location: Location
  properties: {
    addressSpace: {
      addressPrefixes: AddressPrefixes
    }
  }
}

resource subnet 'Microsoft.Network/virtualNetworks/subnets@2020-11-01' = {
  name: 'default'
  parent: vnet
  properties: {
    addressPrefix: DefaultSubnetAddressPrefix
  }
}

And then in another file be able to do:

resource existingVirtualNetwork 'Microsoft.Network/virtualNetworks@2020-05-01' existing = if (vnetNewOrExisting == 'existing') {
  name: vnetName
}
resource subnet 'Microsoft.Network/virtualNetworks/subnets@2020-05-01' = if (vnetNewOrExisting == 'existing') {
  parent: existingVirtualNetwork
  name: bastionSubnetName
  properties: {
    addressPrefix: bastionSubnetIpPrefix
  }

And have it "just work" rather than trying to delete all my subnets that aren't in the same file.

bmoore-msft commented 3 years ago

@JustinGrote - just shoot me an email (so I have your email) and I'll grab some time for us...

abatishchev commented 3 years ago

Frankly I don't understand too why ARM / Resource Providers are so hesitant making breaking changes even in cases when they would clearly benefit customer and/or resolve long-standing issues, such as this one. ARM dictates a strict (anti) breaking changes policy that requires a new api version on any significant change. Why not to use the policy in the intended way and actually make breaking changes with the next API version? Proper validation and clear error messages already available. Why one group of customers (who suffer from well-known, long-standing issues for years) is sacrificed over another (who wants a no-brainer updates between api version)?

blueelvis commented 3 years ago

@JustinGrote @bmoore-msft - Were you able to discuss this issue? I am also running into this issue while I am trying to deploy several environments where if I want to add another subnet, I can only do it manually. ARM Templates refuse to work in this case.

JustinGrote commented 3 years ago

@JustinGrote @bmoore-msft - Were you able to discuss this issue? I am also running into this issue while I am trying to deploy several environments where if I want to add another subnet, I can only do it manually. ARM Templates refuse to work in this case.

I was contacted for a meeting but that meeting never actually got scheduled.

For now you have to put all subnets you want as properties of the vnet in the same module to get it to work and not try to recreate, or disable your vnet definition and reference it instead for the subnets after it is defined. Even child resources in the same doc barf for some reason.

bmoore-msft commented 3 years ago

We've paused conversations for a bit... @blueelvis - shoot me an email if you want me to add you to the list.

4c74356b41 commented 3 years ago

@ChristopherGLewis you can actually use nulls for those properties:

{
  "name": "xxx",
  "properties": {
    "addressPrefix": "yyy",
    "routeTable": "[if(expression(), 'id', json('null'))]",
    "networkSecurityGroup": "[if(expression(), 'id', json('null'))]"
  }
},

example from my arm template (with most of the noise ommited)

4c74356b41 commented 3 years ago

@bmoore-msft not sure why don't you want to just omit subnets when subnets are not sent and preserve existing values in the api (like it does* for pretty much everything else)

bmoore-msft commented 3 years ago

Currently the vnet resource treats the missing property the same as an empty property (and removes subnets)... "fixing" that is a breaking change. It is an option, just not without consequence.

JustinGrote commented 3 years ago

@bmoore-msft haven't we been over this a bunch of times? Isn't that what API versioning is for? Every ARM template in existence has their API version locked by definition to the API date (unless they are doing something unsupported like adjusting it dynamically which is on them for taking that risk), it won't break any existing templates, only someone who is using a new version and is not aware of the new behavior (which there should be extensive notice and documentation about).

bmoore-msft commented 3 years ago

For templates sure, but not all clients allow the users to set or control the apiVersion as templates do. It's a bit of a philosophical debate that I couldn't change if I wanted to... So I'll work within the confines.

JustinGrote commented 3 years ago

@bmoore-msft then why can't it just be a new compatibiltiy flag? A property on a vnet that says subnetMode: incremental or something like that to enable the new behavior? I really don't get why this is such a head-scratcher.

bmoore-msft commented 3 years ago

It can - but there are an awful lot of properties in ARM with a similar problem (potentially any array property). I'd rather see if these can be modelled correctly and add an extra property for each array.

miqm commented 3 years ago

It can - but there are an awful lot of properties in ARM with a similar problem (potentially any array property). I'd rather see if these can be modelled correctly and add an extra property for each array.

But there aren't many properties that are resources. The subnet property should be removed - you should be creating subnets using child resources not a property.

Additional problem with subnets as property is that it allows to bypass policies (e.g. naming policy) on subnets - they are not checked on deployment time and that may cause a lot of headache later.

JustinGrote commented 3 years ago

@miqm that was my though, if subnetmode: incremental is specified then it basically either ignores the property, or treats the items in the property like they are child items, but then also accepts child resources. If a subnet is removed from the subnets property it would be treated the way other removed resources do today (just be ignored)

This is the quirky aspect of vnets, most other array resources don't have this problem because as miqm stated, they're normally done as child resources (e.g. azure file shares, etc.), this would be aligning the behavior to that.

rouke-broersma commented 3 years ago

It can - but there are an awful lot of properties in ARM with a similar problem (potentially any array property). I'd rather see if these can be modelled correctly and add an extra property for each array.

Is this something that can be accomplished near-future? I guess you can't give any time frames but this has been a problem for so many years and the work arounds (for my scenario) are disgusting. It would be nice to have something to look forward to.