[release/9.1] Don't set properties on existing Azure SQL server resources (#7707)

* Don't set properties on existing Azure SQL server resources

* Set admin for existing resources correctly

* Add workaround reference for Azure SDK issue

* PR feedback.

Add a link to Azure.Provisioning issue.

* Remove `principalType` parameter from test assertion

---------

Co-authored-by: Safia Abdalla <[email protected]>
Co-authored-by: Safia Abdalla <[email protected]>
Co-authored-by: Eric Erhardt <[email protected]>

Author: 3 people

src/Aspire.Hosting.Azure.Sql/AzureSqlExtensions.cs

@@ -5,6 +5,7 @@
 using Aspire.Hosting.Azure;
 using Azure.Provisioning;
 using Azure.Provisioning.Expressions;
+using Azure.Provisioning.Primitives;
 using Azure.Provisioning.Sql;
 
 namespace Aspire.Hosting;
@@ -205,14 +206,6 @@ private static void CreateSqlServer(
         {
             var resource = SqlServer.FromExisting(identifier);
             resource.Name = name;
-            resource.Administrators = new ServerExternalAdministrator()
-            {
-                AdministratorType = SqlAdministratorType.ActiveDirectory,
-                IsAzureADOnlyAuthenticationEnabled = true,
-                Sid = principalIdParameter,
-                Login = principalNameParameter,
-                TenantId = BicepFunction.GetSubscription().TenantId
-            };
             return resource;
         },
         (infrastructure) =>
@@ -234,6 +227,20 @@ private static void CreateSqlServer(
             };
         });
 
+        // If the resource is an existing resource, we model the administrator access
+        // for the managed identity as an "edge" between the parent SqlServer resource
+        // and a custom SqlServerAzureADAdministrator resource.
+        if (sqlServer.IsExistingResource)
+        {
+            var admin = new SqlServerAzureADAdministratorWorkaround($"{sqlServer.BicepIdentifier}_admin")
+            {
+                ParentOverride = sqlServer,
+                LoginOverride = principalNameParameter,
+                SidOverride = principalIdParameter
+            };
+            infrastructure.Add(admin);
+        }
+
         infrastructure.Add(new SqlFirewallRule("sqlFirewallRule_AllowAllAzureIps")
         {
             Parent = sqlServer,
@@ -244,11 +251,15 @@ private static void CreateSqlServer(
 
         if (distributedApplicationBuilder.ExecutionContext.IsRunMode)
         {
-            // When in run mode we inject the users identity and we need to specify
-            // the principalType.
-            var principalTypeParameter = new ProvisioningParameter(AzureBicepResource.KnownParameters.PrincipalType, typeof(string));
-            infrastructure.Add(principalTypeParameter);
-            sqlServer.Administrators.PrincipalType = principalTypeParameter;
+            // Avoid mutating properties on existing resources.
+            if (!sqlServer.IsExistingResource)
+            {
+                // When in run mode we inject the users identity and we need to specify
+                // the principalType.
+                var principalTypeParameter = new ProvisioningParameter(AzureBicepResource.KnownParameters.PrincipalType, typeof(string));
+                infrastructure.Add(principalTypeParameter);
+                sqlServer.Administrators.PrincipalType = principalTypeParameter;
+            }
 
             infrastructure.Add(new SqlFirewallRule("sqlFirewallRule_AllowAllIps")
             {
@@ -273,4 +284,80 @@ private static void CreateSqlServer(
 
         infrastructure.Add(new ProvisioningOutput("sqlServerFqdn", typeof(string)) { Value = sqlServer.FullyQualifiedDomainName });
     }
+
+    /// <remarks>
+    /// Workaround for issue using SqlServerAzureADAdministrator.
+    /// See https://github.com/Azure/azure-sdk-for-net/issues/48364 for more information.
+    /// </remarks>
+    private sealed class SqlServerAzureADAdministratorWorkaround(string bicepIdentifier) : SqlServerAzureADAdministrator(bicepIdentifier)
+    {
+        private BicepValue<string>? _name;
+        private BicepValue<string>? _login;
+        private BicepValue<Guid>? _sid;
+        private ResourceReference<SqlServer>? _parent;
+
+        /// <summary>
+        /// Login name of the server administrator.
+        /// </summary>
+        public BicepValue<string> LoginOverride
+        {
+            get
+            {
+                Initialize();
+                return _login!;
+            }
+            set
+            {
+                Initialize();
+                _login!.Assign(value);
+            }
+        }
+
+        /// <summary>
+        /// SID (object ID) of the server administrator.
+        /// </summary>
+        public BicepValue<Guid> SidOverride
+        {
+            get
+            {
+                Initialize();
+                return _sid!;
+            }
+            set
+            {
+                Initialize();
+                _sid!.Assign(value);
+            }
+        }
+
+        /// <summary>
+        /// Parent resource of the server administrator.
+        /// </summary>
+        public SqlServer? ParentOverride
+        {
+            get
+            {
+                Initialize();
+                return _parent!.Value;
+            }
+            set
+            {
+                Initialize();
+                _parent!.Value = value;
+            }
+        }
+
+        private static BicepValue<string> GetNameDefaultValue()
+        {
+            return new StringLiteralExpression("ActiveDirectory");
+        }
+
+        protected override void DefineProvisionableProperties()
+        {
+            _name = DefineProperty("Name", ["name"], defaultValue: GetNameDefaultValue());
+            _login = DefineProperty<string>("Login", ["properties", "login"]);
+            _sid = DefineProperty<Guid>("Sid", ["properties", "sid"]);
+            _parent = DefineResource<SqlServer>("Parent", ["parent"], isOutput: false, isRequired: true);
+        }
+    }
 }

tests/Aspire.Hosting.Azure.Tests/ExistingAzureResourceTests.cs

@@ -1050,15 +1050,15 @@ param existingResourceName string
 
             resource sqlServer 'Microsoft.Sql/servers@2021-11-01' existing = {
               name: existingResourceName
+            }
+
+            resource sqlServer_admin 'Microsoft.Sql/servers/administrators@2021-11-01' = {
+              name: 'ActiveDirectory'
               properties: {
-                administrators: {
-                  administratorType: 'ActiveDirectory'
-                  login: principalName
-                  sid: principalId
-                  tenantId: subscription().tenantId
-                  azureADOnlyAuthentication: true
-                }
+                login: principalName
+                sid: principalId
               }
+              parent: sqlServer
             }
 
             resource sqlFirewallRule_AllowAllAzureIps 'Microsoft.Sql/servers/firewallRules@2021-11-01' = {
@@ -1096,8 +1096,7 @@ public async Task SupportsExistingAzureSqlServerInRunMode()
               "params": {
                 "existingResourceName": "{existingResourceName.value}",
                 "principalId": "",
-                "principalName": "",
-                "principalType": ""
+                "principalName": ""
               }
             }
             """;
@@ -1114,20 +1113,17 @@ param principalName string
 
             param existingResourceName string
 
-            param principalType string
-
             resource sqlServer 'Microsoft.Sql/servers@2021-11-01' existing = {
               name: existingResourceName
+            }
+
+            resource sqlServer_admin 'Microsoft.Sql/servers/administrators@2021-11-01' = {
+              name: 'ActiveDirectory'
               properties: {
-                administrators: {
-                  administratorType: 'ActiveDirectory'
-                  principalType: principalType
-                  login: principalName
-                  sid: principalId
-                  tenantId: subscription().tenantId
-                  azureADOnlyAuthentication: true
-                }
+                login: principalName
+                sid: principalId
               }
+              parent: sqlServer
             }
 
             resource sqlFirewallRule_AllowAllAzureIps 'Microsoft.Sql/servers/firewallRules@2021-11-01' = {
@@ -1300,13 +1296,13 @@ public async Task SupportsExistingAzureApplicationInsightsWithResourceGroup()
         var expectedBicep = """
             @description('The location for the resource(s) to be deployed.')
             param location string = resourceGroup().location
-    
+
             param existingResourceName string
-    
+
             resource appInsights 'Microsoft.Insights/components@2020-02-02' existing = {
               name: existingResourceName
             }
-    
+
             output appInsightsConnectionString string = appInsights.properties.ConnectionString
             """;
 
@@ -1347,17 +1343,17 @@ public async Task SupportsExistingAzureOpenAIWithResourceGroup()
         var expectedBicep = """
             @description('The location for the resource(s) to be deployed.')
             param location string = resourceGroup().location
-    
+
             param existingResourceName string
-    
+
             param principalType string
-    
+
             param principalId string
-    
+
             resource openAI 'Microsoft.CognitiveServices/accounts@2024-10-01' existing = {
               name: existingResourceName
             }
-    
+
             resource openAI_CognitiveServicesOpenAIContributor 'Microsoft.Authorization/roleAssignments@2022-04-01' = {
               name: guid(openAI.id, principalId, subscriptionResourceId('Microsoft.Authorization/roleDefinitions', 'a001fd3d-188f-4b5d-821b-7da978bf7442'))
               properties: {
@@ -1367,7 +1363,7 @@ param principalId string
               }
               scope: openAI
             }
-    
+
             resource mymodel 'Microsoft.CognitiveServices/accounts/deployments@2024-10-01' = {
               name: 'mymodel'
               properties: {