fix: Add enableAad parameter to conditionally configure AAD profile in Kubernetes
#3828
Conversation
avm-team-linter
bot
added
the
Needs: Module Owner 📣
microsoft-github-policy-service
bot
added
the
Needs: Triage 🔍
|
Important The "Needs: Triage 🔍" 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. |
microsoft-github-policy-service
bot
added
the
Type: AVM 🅰️ ✌️ Ⓜ️
|
Important If this is a module-related PR, being submitted by the sole owner of the module, the AVM core team must review and approve it (as module owners can't approve their own PRs). To indicate this PR needs the core team''s attention, apply the "Needs: Core Team 🧞" label! The core team will only review and approve PRs that have this label applied! |
update main.json update main.json update main.json update main.json update main.json
Menghua1
force-pushed
the
fix/add-aad-profile-conditional
branch
from
113393b
to
71933fd
Compare
|
@AlexanderSehr Could you please review this PR? |
Hey @Menghua1, Also, there is a very blocking PR here. Without that being resolved, we cannot move forward as this PR. If merged regardless, it will fail the deployment tests in AVM and hence never make it to the publish stage. |
| @@ -739,15 +742,15 @@ resource managedCluster 'Microsoft.ContainerService/managedClusters@2024-03-02-p | |||
| } | |||
| } | |||
| publicNetworkAccess: publicNetworkAccess | |||
| aadProfile: { | |||
| aadProfile: enableAad ? { | |||
There was a problem hiding this comment.
If there are any required parameters, would it not maybe be simpler to change this condition do something like aadProfile: !empty(aadProfileClientAppID) ? { ?
Mind you, I have no clue if any of those parameter is required, so this is just an example.
An alternative, by the way, would be to add a user defined type for the aadProfile that is used by a corresponding parameter: param aadProfile aadProfileType? and would contain all the below properties. If implemented, the above suggested check could be changed to something like aadProfile: !empty(aadProfile) ? {
Just some ideas 😏
Description
This PR introduces a conditional check for the
aadProfileconfiguration in Kubernetes cluster setup. WhenenableAadis set tofalse, the AAD (Azure Active Directory) integration will be disabled by settingaadProfileto null. Ensures AAD integration is skipped entirely when not needed, optimizing resource usage and configuration complexity.Requested by the AZD team: Azure/Azure-Verified-Modules#261, to ensure consistency with the functionality implemented in the aks-managed-cluster.bicep file located in infra/core.
Pipeline Reference
Type of Change
version.json:version.json.version.json.Checklist
Set-AVMModulelocally to generate the supporting module files.@rajeshkamal5050 for notification.