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

ip_version casing #2295

Closed
rohrerb opened this issue Nov 12, 2018 · 5 comments
Closed

ip_version casing #2295

rohrerb opened this issue Nov 12, 2018 · 5 comments
Assignees
Milestone

Comments

@rohrerb
Copy link
Contributor

rohrerb commented Nov 12, 2018

Terraform (and AzureRM Provider) Version

Terraform v0.11.10
terraform-provider-azurerm_v1.18

Affected Resource(s)

  • azurerm_public_ip

Terraform Configuration Files

resource "azurerm_public_ip" "vm_pip" {
  count                        = "${var.public_ip ? var.instance_count : 0}"
  name                         = "${format("%s%03d", local.base_hostname, count.index + 1)}"
  location                     = "${var.azure_location}"
  resource_group_name          = "${var.resource_group_name}"
  public_ip_address_allocation = "static"
  domain_name_label            = "${var.instance_type == "xyz" ? format("%s%s-%d", var.deployment_code, var.location_code, count.index + 1) : format("%s%03d", local.base_hostname, count.index + 1)}"
  
  lifecycle {
    prevent_destroy = true
  }
}

Expected Behavior

ip_version casing shouldn't cause a change in the plan.

Can we please get the case of this field ignored?

Actual Behavior

  ~ module.lnet.azurerm_public_ip.vm_pip[0]
      ip_version: "ipv4" => "IPv4"

Important Factoids

  • Removing that resource and re-adding via import doesn't resolve state.
  • Applying the change doesn't resolve the item
  • This didn't occur until the upgrade to v1.18

@tombuildsstuff this seems to be the change which is causing the issue > 5862ac5#diff-ed59d26a8bebf9bf3b188b200b02dd27

@tombuildsstuff
Copy link
Contributor

hey @rohrerb

Thanks for opening this issue :)

Taking a quick look into this this appears to be a bad merge conflict - I'll open a PR to fix this (and update the test to ensure this casing is matched) - sorry about this!

On this topic - in the next major version of the AzureRM Provider (2.0) we're planning on making the majority of fields which are currently case insensitive (e.g. SKU's) case sensitive; this is to work around some bugs in API's and to better match the behaviour of other Terraform Providers - so this'll be becoming more consistent in the not-too-distant future anyway :)

Thanks!

@amcguign
Copy link

@tombuildsstuff is it easy to provide a list of fields that you are making case-sensitive so we can scrub our Terraform files?

@rohrerb
Copy link
Contributor Author

rohrerb commented Nov 12, 2018

Thank you Tom. In the past we ran into a several casing issues (you are aware) which makes me a bit concerned. Why would we make all fields case sensitive when ARM doesn't require it? Won't this cause a bunch of superficial changes in everyone's plan?

@tombuildsstuff
Copy link
Contributor

@rohrerb

In the past we ran into a several casing issues (you are aware) which makes me a bit concerned. Why would we make all fields case sensitive when ARM doesn't require it?

Unfortunately more of the ARM API's are case sensitive than it first appears, but in unexpected ways (e.g. SKU's/Kind's) that it ends up causing a bunch of subtle issues (for example, provisioning an Application Insights resource with a kind that isn't cased correctly provisions a completely different kind of Application Insights resource, which provisions successfully but fails in unexpected ways).

Won't this cause a bunch of superficial changes in everyone's plan?

It shouldn't do - we can look into state migrations where they're necessary - but in either case the Terraform Configurations will need updating and the intention is that the validation catches these changes so it should be pretty easy to see what needs fixing. Whilst we appreciate that's a breaking change - it'll be a part of 2.0 so 1.x should be able to work where possible.

Unfortunately some of these are bugs in the API's which need to be fixed - but we can't get them fixed if we don't know what's broken :)


@amcguign

is it easy to provide a list of fields that you are making case-sensitive so we can scrub our Terraform files?

This should include any kind of Enum/Constant (e.g. SKU's, Kind's etc) - but it's hard to confirm exactly which fields at this point (until we go through each resource) however they'll be listed in the changelog for that release.

We'll be putting out some more information about 2.0 in the near future, we're still trying to work out al the details :)

@ghost
Copy link

ghost commented Mar 6, 2019

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 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants