Make Location case-insensitive #1809
Comments
thecodetinker
added
kind/bug
|
I can further see in the state file that the Resource Group's Location case differs between the Input and Output "inputs": {
"location": "WestEurope",
"resourceGroupName": "IXS-WESTEUROPE-JUP001-PRIMARY"
}
"outputs": {
"id": "/subscriptions/21863b6c-XXXX/resourceGroups/IXS-WESTEUROPE-JUP001-PRIMARY",
"location": "westeurope",
}So I can see that a simple refactoring to use |
|
Aha thought I, I'll just update the stack. But still Pulumi is wanting to replace (MI this time) even though there's no change.
NB I changed the file directly in the Blob storage, rather than import/export. Would that make any difference? |
|
Hi @thecodetinker thanks for the detailed writeup. Yes, the provider doesn't currently have knowledge of which fields are case-insensitive, but it would be a great addition to enhance the usibility of the provider – along with the other issue you referenced around URN case sensitivity. The core reason behind a lot of this behaviour is that the value of the output is based on whatever the Azure API returns back to us. A similar issue would happen during a refresh too - where the value being read might vary from your original input. I think the reason that your last example highlights the property as a replacement is because during preview it only knows that the value is going to change; it makes a guess as to the value it's likely to change to, but until it actually does the call to the service for the first resource it doesn't know for certain what value will come back for the output – and therefore the preview flags it as a possible change and therefore a possible replace. Doing these changes in separate steps might be helpful to see the change one at a time. Another approach which might be helpful it to refactor the code for all resources to reference location from the same simple variable which reduces the complexities which might arise from outputs having the possibility of changing: var location = "NorthEurope";
var resourceGroup = new ResourceGroup("ResourceGroup", new ResourceGroupArgs
{
Location = location,
ResourceGroupName = "RGName"
});
var storageAccount = new StorageAccount("StorageAccount", new StorageAccountArgs
{
AccountName = accountName,
Location = location,
ResourceGroupName = resourceGroup.Name
}); |
danielrbradley
added
impact/usability
|
Hi @danielrbradley thanks for the quick response. I understand it's hard to make any general assumptions about whether or not a change of case is really a change - for example it would be in most text fields. However I wonder if it would be possible to put in place one or two exceptions for cases like this? I can see now that I'm actually in this scenario precisely because of a code refactoring along the lines you suggest. We are migrating from TF to Pulumi so importing resources. The locations are all imported as lower case, so any that were defined in Pulumi as coming from the Config (which was PascalCase) did not import. Resources whose location was defined as coming from resourceGroup.Location output field were fine. So I just changed everything to use the latter and was then able to successfully import pre-existing environments. All was good until we've come to update environments that were built in Pulumi from the start, and now it's wanting to delete half the resources and recreate them due to what seemed to be a fairly innocuous code change. One final thing - the preview was indeed telling the truth, approving the change does result in the resources getting deleted, even after updating the state file to use lower case. |
|
Yup, I agree this would be worth special case handling for this field given it applies to all resources. We're currently deep into a few other issues right now, but we'll keep this one in the backlog to look at further. |
danielrbradley
changed the title
|
Thanks, I'd appreciate that. One final thing: I thought I'd figured out it was because of the dependency change so I updated the propertyDependencies section in the state file too (based off a before/after run). Still no luck, sadly. |
|
Ok one final thing, I think I've worked around this by setting |
|
I created some resources using azure-native provider, I configured the location for the provider, now I changed the way of creating the resources so I set the location directly on the resource (it is not taken from the provider configuration). Pulumi now wants to replace the resource because of the location changed from |
) All `location` properties in Azure are case- and space- insensitive, so both "West Europe" and "westeurope" are valid values representing the same region. This PR adjusts our diff calculation logic with this special rule. We get a diff calculated by vanilla platform diffing, then we check if there is an Update to the property called `location`, and if so, it normalizes both values and compares those. Any non-significant changes are ignored. Fix #1809 Fix #1005
What happened?
I am trying to update one of our environments, and for some reason the case of the Location has changed from
NorthEuropetonortheurope. This is anInput<string>based off theResourceGroupwhose Location is actually specified asNorthEurope. It's not clear why the case has changed in Pulumi's eyes (edit: I've put a possible reason in the comment, and updated the code sample).Nonetheless, now Pulumi wants to Replace the storage account which is... inconvenient.
Steps to reproduce
Unclear, but along the lines of
Then change to
Expected Behavior
As the Azure Region is the same, I expect that Pulumi would not make a change here.
Actual Behavior
Pulumi replaces the storage account.
Versions used
This is using the Pulumi Automation API so not sure how to get the 'about' information. Nuget packages are:
Pulumi: 3.25.1
AzureNative: 1.60
Additional context
Case sensitivity feels like a disaster waiting to happen. I've had a lot of issues with importing existing environments due to Azure's own inconsistency in URNs (
resourcegroupvsresourceGroupbeing quite a prevalent one). See #854Now this is a problem too. It would be really helpful if Pulumi could be more lenient when it comes to differences in case only.
Contributing
Vote on this issue by adding a 👍 reaction.
To contribute a fix for this issue, leave a comment (and link to your pull request, if you've opened one already).
The text was updated successfully, but these errors were encountered: