Azure / bicep-registry-modules

Bicep registry modules
MIT License
412 stars 275 forks source link

[Bug Report]: Databricks Workspace test hardcoded value for AzureDatabricks Enterprise app object ID #2407

Open tyconsulting opened 4 months ago

tyconsulting commented 4 months ago

Describe the bug

The Object ID for the AzureDatabricks Enterprise app is hardcoded HERE. This value is different in each tenant. Therefore it should be tokenized in the settings.yml file

To reproduce

When the repo is forked, the databricks pipeline will fail out of the box because this Object Id is incorrect. image

Code snippet

No response

Relevant log output

No response

microsoft-github-policy-service[bot] commented 2 weeks ago

[!IMPORTANT] The "Needs: Triage :mag:" label must be removed once the triage process is complete!

[!TIP] For additional guidance on how to triage this issue/PR, see the BRM Issue Triage documentation.

AlexanderSehr commented 2 weeks ago

Just migrated this issue from CARML and it's still very much relevant in AVM.

However - the implementation changed slightly in that we moved the value into the main.test.bicep file to make it more obvious and easier to maintain (as opposed to be hidden away in some nested dependencies file). Ideally we'd be able to fetch the value, but MSIs don't have the permissions to fetch Entra ID information.

To this end, I don't think we can actually solve it - unless we'd add it as a secret to the environment. But as it's very test specific, I'm not sure that's a good idea either.

One last thought: I guess what we could think about is introducing a general task to the deployment pipeline that fetches IDs we know we need and makes them available as tokens. This would take care of that issue, even though it would require new code for every other ID we need (- like the 2 or 3 we know of today). It should be implemented in a way that the value is only fetched if a specific resource type (e.g. in this case for databricks) is targeted to not waste runtime. The caveat: The service principal would need to be able to fetch this kind of information. However, I do believe this is possible by default.

@eriqua / @ChrisSidebotham what are your thoughts?