CompositionalIT / farmer

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

Azure SQL Server - Active Directory Admin #1036

Closed gursharan001 closed 1 year ago

gursharan001 commented 1 year ago

Hello and Thanks for the excellent product.

I am setting my Azure SQL server instance with a Active Directory Group as admin and also setting it to AD only auth.

I currently do this as az cli commands. There is a pre and post set of commands to make farmer deploy repeatable (turn AD only off before applying template and subsequently turn on AD only).

I thought it will be useful to add this capability into Farmer.

The specific change would be to enable

  1. creating a Azure SQL instance with Activity Directory admin
  2. turn on AD only auth

I tried this with an Arm expression first and it is supported.

I have made a few changes to see if I could figure out the code changes required and everything still seems to hang together. It took me a few hours to make this change as I don't writing F# everyday.

So now I am here to check if this can be added to Farmer. Shall I submit a PR for this change?

isaacabraham commented 1 year ago

Hey @gursharan001 . Sorry for the slow response. Yes, I would definitely support adding this functionality. Happy to help in terms of API design etc.. I'm sure @ninjarobot would be happy as well to lend some advice.

gursharan001 commented 1 year ago

Thanks @isaacabraham @ninjarobot

I thought API could be something like below

type ActiveDirectoryPrincipalType = User | Group

type ActiveDirectoryAdminSettings =
    {
         /// Active Directory name of user or group
         Login: string
         /// Active Directory object id of user or group
         Sid: string
         PrincipalType: ActiveDirectoryPrincipalType
         AdOnlyAuth: bool
    }

and then Farmers.Builders.SqlAzure.SqlAzureConfig updated as

type SqlAzureConfig =
    {
        Name: SqlAccountName
        AdministratorCredentials: {| UserName: string
                                     Password: SecureParameter |}
        /// new setting
        ActiveDirectoryAdmin: ActiveDirectoryAdminSettings option

        MinTlsVersion: TlsVersion option
        FirewallRules: {| Name: ResourceName
                          Start: IPAddress
                          End: IPAddress |} list
        ElasticPoolSettings: {| Name: ResourceName option
                                Sku: PoolSku
                                PerDbLimits: {| Min: int<DTU>; Max: int<DTU> |} option
                                Capacity: int<Mb> option |}
        Databases: SqlAzureDbConfig list
        GeoReplicaServer: GeoReplicationSettings option
        Tags: Map<string, string>
    }

for scenario when AdOnlyAuth is true, Farmers.Builders.SqlAzure.SqlServerBuilder.Run might raise an error or silently ignore the password if provided. I prefer raising an error.

What are your thoughts?

ninjarobot commented 1 year ago

@gursharan001 thanks for the proposal. The config structure looks good - do you know if both the SID and the login name are required or can either work?

As far as the password, it's a SecureParameter and Farmer can control whether to emit a parameter for it, so I suggest that when AdOnlyAuth is set to true, just don't emit one.

gursharan001 commented 1 year ago

@ninjarobot yes both SID and login are required. I will submit a PR soon. Thanks!

gursharan001 commented 1 year ago

@isaacabraham / @ninjarobot , please review the PR when you get a chance