CompositionalIT / farmer

Repeatable Azure deployments with ARM templates - made easy!
https://compositionalit.github.io/farmer
MIT License
517 stars 156 forks source link

SQL Azure - add support for AD admin #1047

Closed gursharan001 closed 1 year ago

gursharan001 commented 1 year ago

This PR closes #1036

The changes in this PR are as follows:

I have read the contributing guidelines and have completed the following:

Below is a minimal example configuration that includes the new features, which can be used to deploy to Azure:

let activeDirectoryAdmin: ActiveDirectoryAdminSettings =
  {
    Login = "admingroup"
    Sid = "F9D49C34-01BA-4897-B7E2-3694BF3DE2CF"
    PrincipalType = ActiveDirectoryPrincipalType.Group
    AdOnlyAuth = false
  }

sqlServer {
     name "adtestserver"
     active_directory_admin (Some(activeDirectoryAdmin))
     admin_username "sqladminuser"
}

Issues

I have found that ARM templates need modification to switch SQL server configuration among following -

  1. sql admin with no AD admin
  2. sql admin with AD admin
  3. AD admin only

Please refer this blog post - https://www.codez.one/azure-sql-with-managed-identities-part-2/

One possible way forward is to make AdOnlyAuth member optional and let the user write correct farmer template by detecting existence of SQL server out of band (using AZ cli or powershell etc)

gursharan001 commented 1 year ago

@ninjarobot I have made further changes to hopefully simplify the code. Please do let me know if it can be made better. Thanks again.

martin-bmc commented 10 months ago

Maybe the wrong place to add this but I just tested this feature and found an issue when changing from an existing Azure AD admin to a new one. It seems like this is not supported in ARM/Bicep: https://github.com/Azure/bicep/issues/4988

ninjarobot commented 10 months ago

Does it work in json, just not in bicep? Or is this impacted by the bug regardless?

martin-bmc commented 10 months ago

As far as I understand the comments on the issue, the Microsoft SQL team allows the value to be set but not changed via the ARM template that is generated in this PR:

The initial (deployment) template has to have AAD admin set as a property of "Microsoft.Sql/servers" resource. The "update" template would have to have "Microsoft.Sql/servers/administrators" resource.

https://github.com/Azure/bicep/issues/4988#issuecomment-1291611393

It might be to complex for Farmer to handle this case but perhaps it should be mentioned in the documentation?