fix review comments

CompositionalIT · Jul 11, 2023 · b0dc1d6 · b0dc1d6
1 parent 04756cc
commit b0dc1d6
Show file tree

Hide file tree

Showing 3 changed files with 75 additions and 53 deletions.
diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md
@@ -1,11 +1,9 @@
 Release Notes
 =============
-## 1.7.26
-* SQL Azure: Adds support for AD admin
-
 ## 1.7.25
 * Network Security Groups: Fix bug where a SecurityRule without a source throws a meaningful exception
 * Network Security Groups: Add rule to existing security group
+* SQL Azure: Adds support for AD admin
 
 ## 1.7.24
 * Network Interface: Adds support for network interface creation.

diff --git a/src/Farmer/Arm/Sql.fs b/src/Farmer/Arm/Sql.fs
@@ -37,6 +37,23 @@ type ActiveDirectoryAdminSettings =
         AdOnlyAuth: bool
     }
 
+type SqlServerADAdminJsonProperties =
+    {
+        administratorType: string
+        principalType: string
+        login: string
+        sid: string
+        azureADOnlyAuthentication: bool
+    }
+type SqlServerJsonProperties =
+    {
+        version: string
+        minimalTlsVersion: string
+        administratorLogin: string
+        administratorLoginPassword: string
+        administrators: SqlServerADAdminJsonProperties
+    }
+
 type Server =
     {
         ServerName: SqlAccountName
@@ -48,6 +65,57 @@ type Server =
         Tags: Map<string, string>
     }
 
+    member private this.BuildSqlSeverPropertiesBase(): SqlServerJsonProperties =
+        {
+            version = "12.0"
+            minimalTlsVersion =
+                match this.MinTlsVersion with
+                | Some Tls10 -> "1.0"
+                | Some Tls11 -> "1.1"
+                | Some Tls12 -> "1.2"
+                | None -> null
+            administratorLogin = null
+            administratorLoginPassword = null
+            administrators = Unchecked.defaultof<SqlServerADAdminJsonProperties>
+        }
+
+    member private this.BuildSqlServerADOnlyAdmin(x: ActiveDirectoryAdminSettings): SqlServerADAdminJsonProperties =
+        {
+            administratorType = "ActiveDirectory"
+            principalType =
+                match x.PrincipalType with
+                | Group -> "Group"
+                | User -> "User"
+            login = x.Login
+            sid = x.Sid
+            azureADOnlyAuthentication = true
+        }
+
+    member private this.BuildSqlServerPropertiesWithMixedModeAdministrator(x: ActiveDirectoryAdminSettings): SqlServerJsonProperties =
+        {
+            this.BuildSqlSeverPropertiesBase() with
+                administratorLogin = this.Credentials.Username
+                administratorLoginPassword = this.Credentials.Password.ArmExpression.Eval()
+                administrators =
+                {
+                    this.BuildSqlServerADOnlyAdmin(x) with
+                        azureADOnlyAuthentication = false 
+                }
+        }
+
+    member private this.BuildSqlServerPropertiesWithADOnlyAdministrator(x: ActiveDirectoryAdminSettings): SqlServerJsonProperties =
+        {
+            this.BuildSqlSeverPropertiesBase() with
+                administrators = this.BuildSqlServerADOnlyAdmin(x)
+        }
+
+    member private this.BuildSqlServerPropertiesWithSqlOnlyAdministrator(): SqlServerJsonProperties =
+        {
+            this.BuildSqlSeverPropertiesBase() with
+                administratorLogin = this.Credentials.Username
+                administratorLoginPassword = this.Credentials.Password.ArmExpression.Eval()
+        }   
+
     interface IParameters with
         member this.SecureParameters =
             match this.ActiveDirectoryAdmin with
@@ -56,62 +124,18 @@ type Server =
 
     interface IArmResource with
         member this.ResourceId = servers.resourceId this.ServerName.ResourceName
-
+        
         member this.JsonModel =
             {| servers.Create(
                    this.ServerName.ResourceName,
                    this.Location,
                    tags = (this.Tags |> Map.add "displayName" this.ServerName.ResourceName.Value)
                ) with
                 properties =
-                    let props =
-                        {|
-                            version = "12.0"
-                            minimalTlsVersion =
-                                match this.MinTlsVersion with
-                                | Some Tls10 -> "1.0"
-                                | Some Tls11 -> "1.1"
-                                | Some Tls12 -> "1.2"
-                                | None -> null
-                            administratorLogin = null
-                            administratorLoginPassword = null
-                        |}
-
                     match this.ActiveDirectoryAdmin with
-                    | Some x ->
-                        let propsWithAdAdmin =
-                            {| props with
-                                administrators =
-                                    {|
-                                        administratorType = "ActiveDirectory"
-                                        principalType =
-                                            match x.PrincipalType with
-                                            | Group -> "Group"
-                                            | User -> "User"
-                                        login = x.Login
-                                        sid = x.Sid
-                                        azureADOnlyAuthentication = false
-                                    |}
-                            |}
-
-                        if x.AdOnlyAuth then
-                            {| propsWithAdAdmin with
-                                administrators =
-                                    {| propsWithAdAdmin.administrators with
-                                        azureADOnlyAuthentication = true
-                                    |}
-                            |}
-                        else
-                            {| propsWithAdAdmin with
-                                administratorLogin = this.Credentials.Username
-                                administratorLoginPassword = this.Credentials.Password.ArmExpression.Eval()
-                            |}
-                    | _ ->
-                        {| props with
-                            administratorLogin = this.Credentials.Username
-                            administratorLoginPassword = this.Credentials.Password.ArmExpression.Eval()
-                            administrators = Unchecked.defaultof<_>
-                        |}
+                    | Some x when not x.AdOnlyAuth -> this.BuildSqlServerPropertiesWithMixedModeAdministrator(x)
+                    | Some x when x.AdOnlyAuth -> this.BuildSqlServerPropertiesWithADOnlyAdministrator(x)
+                    | _ -> this.BuildSqlServerPropertiesWithSqlOnlyAdministrator()
             |}
 
 module Servers =

diff --git a/src/Tests/test-data/lots-of-resources.json b/src/Tests/test-data/lots-of-resources.json
@@ -16,9 +16,9 @@
       "location": "northeurope",
       "name": "farmersql1979",
       "properties": {
+        "version": "12.0",
         "administratorLogin": "farmersqladmin",
-        "administratorLoginPassword": "[parameters('password-for-farmersql1979')]",
-        "version": "12.0"
+        "administratorLoginPassword": "[parameters('password-for-farmersql1979')]"
       },
       "tags": {
         "displayName": "farmersql1979"