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_windows_web_app - cors in site_config is removed #19323

Closed
1 task done
peterprumbach-telekom opened this issue Nov 16, 2022 · 22 comments · Fixed by #20987
Closed
1 task done

azurerm_windows_web_app - cors in site_config is removed #19323

peterprumbach-telekom opened this issue Nov 16, 2022 · 22 comments · Fixed by #20987
Labels

Comments

@peterprumbach-telekom
Copy link

Is there an existing issue for this?

  • I have searched the existing issues

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

Terraform Version

1.3.4

AzureRM Provider Version

3.31.0

Affected Resource(s)/Data Source(s)

azurerm_windows_web_app

Terraform Configuration Files

resource "azurerm_windows_web_app" "example" {
  name                = "example"
  resource_group_name = azurerm_resource_group.example.name
  location            = azurerm_service_plan.example.location
  service_plan_id     = azurerm_service_plan.example.id

  site_config {}
}

Debug Output/Panic Output

15:35:54        ~ site_config {
15:35:54              # (22 unchanged attributes hidden)
15:35:54  
15:35:54            - cors {
15:35:54                - allowed_origins     = [] -> null
15:35:54                - support_credentials = false -> null
15:35:54              }
15:35:54          }
15:35:54  
15:35:54          # (3 unchanged blocks hidden)
15:35:54      }

Expected Behaviour

The block "cors" should not be displayed as a change, as no changes were made here

Actual Behaviour

Since the block "cors" has not been defined, there should be no change here. The "cors" block is listed as optional in the documentation

Steps to Reproduce

No response

Important Factoids

Azure GermanyWestCentral

References

No response

@drdamour
Copy link
Contributor

drdamour commented Nov 17, 2022

this was def introduced with #19078 / #19141 we have to pin to 3.30 for now to not see thousands of changes proposed every plan/apply :( cc @xiaxyi

@xiaxyi
Copy link
Contributor

xiaxyi commented Nov 29, 2022

@peterprumbach-telekom , Thanks for raising this issue, the fix was merged, can you confirm if it's working from your side?

@drdamour
Copy link
Contributor

@xiaxyi what pr merged a fix?

@xiaxyi
Copy link
Contributor

xiaxyi commented Nov 29, 2022

@drdamour Sorry I misunderstood your last comments. Are you suggesting that you are not able to upgrade the provider to the version that have the fix deployed?

@drdamour
Copy link
Contributor

@xiaxyi i'm saying #19141 introduced the bug being raised here, so fixed one bug and introduced a worse one.

@peterprumbach-telekom
Copy link
Author

@peterprumbach-telekom , Thanks for raising this issue, the fix was merged, can you confirm if it's working from your side?

In which version the fix was deployed?

@xiaxyi
Copy link
Contributor

xiaxyi commented Nov 29, 2022

@drdamour Can you be more specific about the issue you are facing? would be better if you can share the detailed diff here.

@peterprumbach-telekom 3.31.0 provider.

@drdamour
Copy link
Contributor

drdamour commented Nov 29, 2022

@xiaxyi the diff was provided in the original post by @peterprumbach-telekom did you read through it its quite clear the problem.

if you omit the cors block every plan shows setting the values to null. Apply does not change this subsequent plan shows same change again and again

3.31 introduced this problem. Im pinning to 3.30 to avoid it

@xiaxyi
Copy link
Contributor

xiaxyi commented Nov 30, 2022

@drdamour , I see, this may due to the default value sets by the api.... let me check how to deal with it.

@xiaxyi
Copy link
Contributor

xiaxyi commented Nov 30, 2022

@peterprumbach-telekom , if user doesn't specify cors setting in TF config, the api will return "cors": null,, in this case, TF won't set cors neither.

The diff you are getting somehow suggests that there was a cors setting either made from TF or from anywhere else outside TF (maybe the portal... ), I need your help to confirm how did you get the cors set.

Thanks!

@drdamour
Copy link
Contributor

i don't think so, we don't even have access to change things in the portal, everything goes through terraform. i don't know what version of azurerm would have created it.

it seems like it shouldn't matter though, if cors isn't set in tf it should either leave it be or set it to the defaults so subsequent applies don't show it changing each time.

in resource explorer i'm seeing cors: null at the website level (providers/Microsoft.Web/sites//) but NOT NULL at providers/Microsoft.Web/sites//config so double check what api your testing with.

@peterprumbach-telekom
Copy link
Author

peterprumbach-telekom commented Nov 30, 2022

@peterprumbach-telekom , if user doesn't specify cors setting in TF config, the api will return "cors": null,, in this case, TF won't set cors neither.

The diff you are getting somehow suggests that there was a cors setting either made from TF or from anywhere else outside TF (maybe the portal... ), I need your help to confirm how did you get the cors set.

Thanks!

We have not added cors, neither via Terraform nor via the Azure Portal. Since version 3.31.0, this change appears in every windows_web_app and linux_web_app.

i don't think so, we don't even have access to change things in the portal, everything goes through terraform. i don't know what version of azurerm would have created it.

it seems like it shouldn't matter though, if cors isn't set in tf it should either leave it be or set it to the defaults so subsequent applies don't show it changing each time.

in resource explorer i'm seeing cors: null at the website level (providers/Microsoft.Web/sites//) but NOT NULL at providers/Microsoft.Web/sites//config so double check what api your testing with.

The same is true for us. The changes are always displayed in Terraform - even with subsequent applies

@xiaxyi
Copy link
Contributor

xiaxyi commented Nov 30, 2022

Thanks @peterprumbach-telekom and @drdamour for the discussion. I'm using 2021-02-01 for the testing. I'm not sure about any changes from the api side, but when I tested the feature today by using TF confg:

resource "azurerm_windows_web_app" "testCors" {
  name                = "xiaxintest-cors"
  resource_group_name = azurerm_resource_group.test.name
  location            = azurerm_service_plan.test.location
  service_plan_id     = azurerm_service_plan.test.id
  
  site_config {
  }
  
}

I got the cors sets to null from the api...

let me have more tests, meanwhile, is it possible to help me to confirm this by creating a new web app?

@derSchtefan
Copy link

derSchtefan commented Nov 30, 2022

As requested in #19473 , I think I know how this can happen:

  1. There are 20+ other web apps that do not have this issue
  2. Like most of the others, this was a resource that was imported with terraform import
  3. At the time of import, there was a CORS setting on that web app
  4. CORS was removed by Terraform (to bring it in line with what is required).
  5. In the portal, there is no cors setting anymore.
  6. Calling the REST API directly, i see a return of cors: null. Clicking "Show JSON view" in the portal also shows cors:null
  7. Yet terraform detects a change, applies it, and still detects a change afterwards
    - cors {
              - allowed_origins     = [] -> null
              - support_credentials = false -> null
            }

Suspected reason:
The only difference between this resource and the other resources is, that the terraform state file itself contains this:

"cors": [
                  {
                    "allowed_origins": [],
                    "support_credentials": false
                  }

instead of

"cors": []

as it is on the other web apps. That state will also not be set to [] but will always stay like this after a terraform apply.

The state is stored in an azure storage account. I tried manually changing the tfstate file to say "cors": [] for that resource and then did a terraform state pull . But It did not help. It keeps detecting this change, and then it updates the state in the cloud to the explicitly stated empty values. The REST api keeps returning "null".

@xiaxyi
Copy link
Contributor

xiaxyi commented Dec 1, 2022

Thanks @derSchtefan for the detailed information.

I did more test and I suspect what's happening here is the setting: cors = null won't override the setting

cors {
 allowed_origins     = []
 support_credentials = false 
}

which means, once you have the cors set, no matter what the value is, you won't be able to get the status: cors: null from the api side again. from api perspective, the null value is possibly have the same effect to as below setting

 allowed_origins     = []
 support_credentials = false 

@derSchtefan
Copy link

which means, once you have the cors set, no matter what the value is, you won't be able to get the status: cors: null from the api side again. from api perspective.

This is where my experience differs, since I do get "cors": null back from the API if I call it manually. For this and all other web apps.

Using https://management.azure.com/subscriptions/xxx/resourceGroups/ggg/providers/Microsoft.Web/sites/yyyyyyyy?api-version=2022-03-01

The only place that has allowed_origins = [] etc. is the tfstate file in the Azure BLOB, for that specific service. As I said, if i modify the state to also be null, it returns to allowed_origins = [], even though the api returns null.

@drdamour
Copy link
Contributor

drdamour commented Dec 2, 2022

@derSchtefan azure api for

/siteName

has retuned null for most props for a long time and it sucks. U have to use /siteName/config to see the real values

@drdamour
Copy link
Contributor

drdamour commented Dec 2, 2022

@derSchtefan and i bet this calls /config..most the other things in azurerm do. Either way tf is bugged here and this needs the bug label badly

@drdamour
Copy link
Contributor

drdamour commented Dec 2, 2022

@xiaxyi i suggest stop trying to minimize this problem to some sub case. Refresh is finding cors props being set it’s irrelevant how they are set. Azurerm should see those and leave them or change them..what it cant do is the current behaviour of saying its gonna change them And then NOT change them on apply. So for your test just set the value and experience the problem.

In your fix to #19078 u moved the logic outside the if block which is why this is happening now. That was probably there to avoid this situation, but lead to the problems #19078 was trying to fix.

im not sure what azurerm should do..have defaults matching azure defaults or avoid making a change when cors is omitted. Both have drawbacks but the current behavior sucks.

@xiaxyi
Copy link
Contributor

xiaxyi commented Dec 2, 2022

@drdamour , yes, I will add the condition set.

@derSchtefan
Copy link

azure api for

/siteName

has retuned null for most props for a long time and it sucks. U have to use /siteName/config to see the real values

@drdamour I can confirm this is the case indeed.

@github-actions
Copy link

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 have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.