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

azurerm_virtual_machine - VM Size should be case insensitive #487

Closed
clangaxon opened this issue Nov 2, 2017 · 17 comments
Closed

azurerm_virtual_machine - VM Size should be case insensitive #487

clangaxon opened this issue Nov 2, 2017 · 17 comments
Assignees
Labels
bug good first issue microsoft/1 service/virtual-machine upstream/terraform This issue is blocked on an upstream issue within Terraform (Terraform Core/CLI, The Plugin SDK etc)
Milestone

Comments

@clangaxon
Copy link

Example:

~ azurerm_storage_account.mystorageaccount
resource_group_name: "myresourcegroup" => "MyResourceGroup"

apply does nothing, so it always shows as an outstanding change in 0.3.2.

@clangaxon
Copy link
Author

I'm also seeing this with vm_size:

~ module.test.azurerm_virtual_machine.vm[0]
vm_size: "Standard_DS13_v2" => "Standard_DS13_V2"

@clangaxon
Copy link
Author

and on VM network interface attachments:

network_interface_ids.#: "1" => "1"
network_interface_ids.0: "/subscriptions/xxx/resourceGroups/yyy/providers/Microsoft.Network/networkInterfaces/zzznetworkInterface" => "/subscriptions/xxx/resourceGroups/yyy/providers/Microsoft.network/networkInterfaces/zzznetworkInterface"

"Microsoft.Network" v. "Microsoft.network".

@clangaxon clangaxon changed the title azurerm_storage_account - difference in capitalization of resource_group_name shows endless changes Difference in capitalization of various resources show endless changes Nov 17, 2017
@internetstaff
Copy link

This is still a problem in 1.0.1.

@tombuildsstuff tombuildsstuff added this to the 1.1.2 milestone Feb 8, 2018
@tombuildsstuff
Copy link
Contributor

hey @clangaxon

Thanks for opening this issue

In general Providers in Terraform are case sensitive (for instance in the AWS and GCP Providers are mostly case sensitive) - for the Azure Provider we have a little bit of leeway since folks are coming from ARM Templates which mostly is case insensitive. Unfortunately this doesn't apply to all fields, since certain Azure API's are case sensitive - and Terraform requires ID's to be case sensitive:

So to reply to each issue in-line:

~ azurerm_storage_account.mystorageaccount
resource_group_name: "myresourcegroup" => "MyResourceGroup"

Resource Group names are inconsistent through Azure - in some API's they're case sensitive, whereas in others they're case insensitive. As this is such a common field (it's on every resource) - I think it's better to be consistent here and not make this field case insensitive, than be inconsistent throughout resources.

As such, we'd recommend updating the Resource Group Name in your Terraform Configuration to match the casing used in Azure.

~ module.test.azurerm_virtual_machine.vm[0]
vm_size: "Standard_DS13_v2" => "Standard_DS13_V2"

Users commonly write the VM Size in a different case (either all upper or all lower case) than Azure returns. Since the API isn't case sensitive here - @echuvyrov is going to send a PR to make this field case insensitive.

and on VM network interface attachments:

network_interface_ids.#: "1" => "1"
network_interface_ids.0: "/subscriptions/xxx/resourceGroups/yyy/providers/Microsoft.Network/networkInterfaces/zzznetworkInterface" => "/subscriptions/xxx/resourceGroups/yyy/providers/Microsoft.network/networkInterfaces/zzznetworkInterface"

"Microsoft.Network" v. "Microsoft.network".

Given this is an identifier Terraform requires that this is case sensitive (since we split the URI into a Map/Dictionary and access the values by their case sensitive names) - as such we can't update this field to be case insensitive.

Technically what's happening here is that the Microsoft.network value is being sent to Azure - but Azure's returning with Microsoft.Network - which is conflicting. Whilst the Azure API can handle submitting values in different casings - Terraform is case specific here; in general we'd recommend using a Data Source to access the ID for a resource, rather than hard-coding it - but I'm aware there's no Data Source for Network Interface at this time. @dcaro has started work on that this week - and I'd expect that to ship in a release in the near future.

In the mean time - to resolve this issue you should be able to update the input data from Microsoft.network to Microsoft.Network.

Given the outstanding issue is related to the VM Size - I'm going to update the title of this issue to mention this; and @echuvyrov should be adding a fix for this in the near future.

Thanks!

@tombuildsstuff tombuildsstuff changed the title Difference in capitalization of various resources show endless changes azurerm_virtual_machine - VM Size should be case insensitive Feb 8, 2018
@echuvyrov
Copy link
Contributor

Hi @clangaxon, I tried to repro the issue with vm_size casing, but cannot seem to duplicate it. Is it possible to provide a small repro script/scenario? I tried running the block of code below with both "Standard_DS13_v2" and "Standard_DS13_V2" and successive applies do not detect the changes after initial deploy.

...
# Create virtual machine
resource "azurerm_virtual_machine" "myterraformvm" {
    name                  = "myVM"
    location              = "East US"
    resource_group_name   = "${azurerm_resource_group.myterraformgroup.name}"
    network_interface_ids = ["${azurerm_network_interface.myterraformnic.id}"]
    vm_size               = "Standard_DS1_v2"
...

@tombuildsstuff tombuildsstuff modified the milestones: 1.1.2, 1.1.3 Feb 15, 2018
@clangaxon
Copy link
Author

@tombuildsstuff

  1. Resource Group naming:

I disagree with not fixing this because the case change doesn't apply. If it can't be applied, it makes more sense for Terraform be aware that Azure doesn't consider it a change and not to try to apply it repeatedly.

Since it behaves this way, it seems that this API is not case sensitive either and the same conclusion would be reached as for vm sizes?

  1. NIC names:

We aren't hard coding a resource id: all of these were either created by Terraform or imported.

In this case the NIC is "Microsoft.network" and the VM is "Microsoft.Network".

Removing the state of both "corrects" the NIC to "Microsoft.Network", but ironically, now they disagree on the actual NIC name - the NIC id imports as "hostnameNetworkInterface" and the VM imports as "hostnamenetworkInterface".

This may be the result of some old state transition, variances across Terraform versions, or internal changes in Azure, but the underlying problem here is, as in both other cases, that Azure appears case-insensitive since the case change continually reapplies but changes nothing.

This may be challenging to fix due to the URI splitting, but so far I don't see a work-around.

@dcaro
Copy link
Contributor

dcaro commented Feb 19, 2018

@clangaxon I have submitted the following PR for a data source on network interfaces. #854

Please share any feedback that you may have.

@clangaxon
Copy link
Author

Another case sensitivity issue:

security_rule.0.protocol: "TCP" => "tcp" security_rule.1.protocol: "TCP" => "tcp" security_rule.10.protocol: "TCP" => "tcp" security_rule.11.protocol: "UDP" => "udp" security_rule.12.protocol: "UDP" => "udp" security_rule.13.protocol: "UDP" => "udp" security_rule.14.protocol: "TCP" => "tcp" security_rule.15.protocol: "UDP" => "udp" security_rule.16.protocol: "TCP" => "tcp" security_rule.2.protocol: "TCP" => "tcp" security_rule.3.protocol: "TCP" => "tcp" security_rule.4.protocol: "TCP" => "tcp" security_rule.5.protocol: "TCP" => "tcp" security_rule.6.protocol: "UDP" => "udp" security_rule.7.name: "X" => "Y" security_rule.7.protocol: "TCP" => "tcp" security_rule.8.protocol: "TCP" => "tcp" security_rule.9.protocol: "TCP" => "tcp"

The difference in protocol case alone won't trigger a change, but if another change triggers an NSG change, it lists all the differences in protocol capitalization.

@clangaxon
Copy link
Author

@dcaro That seems like a nice addition, but I'm not sure how that addresses our issue of the states getting out of sync between Azure and multiple tfstate references to a NIC? We build the NIC and attach it to a VM ...

@achandmsft achandmsft added the M1 label Mar 8, 2018
@achandmsft achandmsft modified the milestones: 1.3.0, 1.4.0 Mar 10, 2018
@achandmsft achandmsft added M1 and removed M1 labels Mar 10, 2018
@metacpp
Copy link
Contributor

metacpp commented Mar 21, 2018

@echuvyrov is the VM size issue fixed ?

@metacpp
Copy link
Contributor

metacpp commented Mar 21, 2018

Windows 10 environment only:

resource "azurerm_resource_group" "test" {
  name     = "new-resource-group"
  location = "West Europe"
}
  1. terraform init
  2. terraform plan
  3. terraform apply
  4. terraform plan

Even you don't have any code change, the terraform plan will always report it needs to create a new resource group:

An execution plan has been generated and is shown below.
Resource actions are indicated with the following symbols:
  + create

Terraform will perform the following actions:

  + azurerm_resource_group.test
      id:       <computed>
      location: "westeurope"
      name:     "new-resource-group"
      tags.%:   <computed>


Plan: 1 to add, 0 to change, 0 to destroy.

@jeffreyCline we need to fix this issue for arm_resource_group, although API tolerate this duplicated call.

@WodansSon
Copy link
Collaborator

I am still not able to repro this issue on Linux, this maybe a Windows only bug. I will attempt to repro the issue on a Windows OS.

Refreshing Terraform state in-memory prior to plan...
The refreshed state will be used to calculate this plan, but will not be
persisted to local or remote state storage.

azurerm_resource_group.test: Refreshing state... (ID: /subscriptions/.../resourceGroups/New-resource-group)

------------------------------------------------------------------------

No changes. Infrastructure is up-to-date.

This means that Terraform did not detect any differences between your
configuration and real physical resources that exist. As a result, no
actions need to be performed.

@tombuildsstuff
Copy link
Contributor

@metacpp @jeffreyCline heads up that’s a regression with Windows that’s been fixed in Terraform 0.11.5, released earlier today

@metacpp
Copy link
Contributor

metacpp commented Mar 23, 2018

@jeffreyCline please double confirm if all the casing issues discussed in this thread are fixed by 0.11.5.

@metacpp metacpp added microsoft/1 upstream/terraform This issue is blocked on an upstream issue within Terraform (Terraform Core/CLI, The Plugin SDK etc) and removed microsoft/3 labels Mar 23, 2018
@metacpp
Copy link
Contributor

metacpp commented Mar 26, 2018

Issue List

Linux

  • Resource Group Naming.
  • VM Size Naming.
  • NIC Naming.
  • Security Rule protocol Naming.
  • LoadBalanceRule Protocol and FE IP CONFIG Naming.

Windows

  • Resource Group Naming.
    Action Item: Update to v0.11.5.
  • VM Size Naming.
  • NIC Naming.
  • Security Rule protocol Naming.
  • LoadBalanceRule Protocol and FE IP CONFIG Naming.

Configuration

Terraform v0.11.5
+ provider.azurerm v1.3.1

@jeffreyCline please use this list to update the validation result and the action items.
@metacpp I have validated all of the above scenarios on both Linux and Windows, with the new Terraform v0.11.5 both behave the same. I am now closing this issue as fixed.

@tombuildsstuff
Copy link
Contributor

@metacpp @jeffreyCline sorry, to clarify in this comment I was referring to this comment:

Even you don't have any code change, the terraform plan will always report it needs to create a new resource group:

which was broken in v0.11.4 of Terraform Core and fixed in v0.11.5 (I can't comment to the rest of this issue, however)

@tombuildsstuff tombuildsstuff modified the milestones: 1.4.0, 1.3.3 Apr 17, 2018
@ghost
Copy link

ghost commented Mar 31, 2020

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 feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks!

@ghost ghost locked and limited conversation to collaborators Mar 31, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug good first issue microsoft/1 service/virtual-machine upstream/terraform This issue is blocked on an upstream issue within Terraform (Terraform Core/CLI, The Plugin SDK etc)
Projects
None yet
Development

No branches or pull requests

9 participants