Azure / bicep-registry-modules

Bicep registry modules
MIT License
460 stars 305 forks source link

[Bug Report]: Template deployment in portal fails on diagnostic settings #2501

Open clarked-msft opened 1 year ago

clarked-msft commented 1 year ago

Describe the bug

Templates define diagnostic settings params as arrays but define their allowed values as strings. Validation fails when deploying in the portal.

image

To reproduce

Attempt to deploy ApiManagement via 'Template Deployment' in the portal.

Code snippet

No response

Relevant log output

No response

rahalan commented 1 year ago

@clarked-msft the modules themselves are deploying without error. Maybe the portal experience has some issues. According to our investigations, arrays are sometimes not translated correctly. This appears to be a platform level issue, it would be need to be addressed by MS support. Can you please raise a support ticket?

clarked-msft commented 1 year ago

It appears when deploying the modules outside the portal the allowed values annotation on parameters is not enforced. In the portal it is enforced by restricting param value selection to a dropdown of allowed values.

From ApiManagement there is the following param.

@allowed([
  'allLogs'
  'GatewayLogs'
])
param diagnosticLogCategoriesToEnable array = [
  'allLogs'
]

I am able to pass an array when deploying outside the portal, say ['allLogs'], and this works great.

When I deploy via the portal, the portal restricts value selection via a dropdown that only includes the allowed values. These allowed values are strings, not arrays. This does not work.

I created a fork and updated the allowed values to be arrays, see below, and was able to deploy without any issue. This does not seem to be an issue with the portal to me.

@allowed([
  ['allLogs']
  ['GatewayLogs']
])
param diagnosticLogCategoriesToEnable array = [
  'allLogs'
]
lsnoddy commented 1 year ago

Needs further investigation.

AlexanderSehr commented 3 months ago

This is seemingly a bug of the Azure Portal that I also experienced in the past. I need to re-test it with the AVM modules. If it's still happening, this should be filed as a bug... I guess in the Azure/bicep repository. Not ideal, but hopefully it would lead in the right direction.

segraef commented 1 month ago

Hey @AlexanderSehr, how did the testing go? Just adding in addtion, tags and lock parameter get defined as Required although declared Optional. This happens to any AVM mdoule.

image

Since this is a portal experience an issue should be raised with https://github.com/Azure/bicep and/or referred to CSS.

AlexanderSehr commented 1 month ago

Hey @segraef, using KVLT as an example, I'd say not great - not great at all: image

It's not the original diagnostics setting problem anymore, but it was replaced by a 'nullable in UDT not detected' issue. 🤔

Used: Deploy to Azure

AlexanderSehr commented 1 month ago

And here's the matching issue in the Azure Bicep repository: https://github.com/Azure/bicep/issues/13838

aserfass-msft commented 1 month ago

Hey @AlexanderSehr, how did the testing go? Just adding in addtion, tags and lock parameter get defined as Required although declared Optional. This happens to any AVM mdoule.

image

Since this is a portal experience an issue should be raised with https://github.com/Azure/bicep and/or referred to CSS.

@segraef and I talk about this a little more and it seems like an issue for inputs that are key:value pairs. I added a specific default value with both keys as null and that rendered correctly as not "required" parameters. image From the Disk Encryption Set template.

image

AlexanderSehr commented 1 month ago

Hey @aserfass-msft, that looks about right. You may recall, originally, only parameters with a default value where considered 'optional' - which is why a lot of parmaeters in e.g., CARML had dummy defaults 😄 Since nullable paramters were introduced, this issue went away - but - as I understand it, the Azure Portal was never updated to handle nullable types. Adding a default essentially falls back to the 'vanilla' implementation :)

Did somebody happen to have created an issue for this?

aserfass-msft commented 3 weeks ago

I don't know of an issue already created. Where would that be?

AlexanderSehr commented 3 weeks ago

I don't know of an issue already created. Where would that be?

My best guess would be the www.github.com/Azure/Bicep repository as I assume the same PG owns the Portal Experience for IaC. @segraef, would you happen to know? If it's not the correct place, I'm sure they'll direct us accordingly 😄

segraef commented 2 weeks ago

Either @clarked-msft or @aserfass-msft please refer to CSS or raise an issue via https://github.com/Azure/bicep, thanks.