Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 5 commits into from
Feb 20, 2025
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b20ee71
Don't set properties on existing Azure SQL server resources
captainsafia Feb 20, 2025
884f94c
Set admin for existing resources correctly
captainsafia Feb 20, 2025
ba06ed7
Add workaround reference for Azure SDK issue
captainsafia Feb 20, 2025
f4d23c7
PR feedback.
eerhardt Feb 20, 2025
783d95e
Remove `principalType` parameter from test assertion
captainsafia Feb 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
113 changes: 100 additions & 13 deletions src/Aspire.Hosting.Azure.Sql/AzureSqlExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) =>
Expand All @@ -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,
Expand All @@ -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")
{
Expand All @@ -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);
}
}
}
52 changes: 24 additions & 28 deletions tests/Aspire.Hosting.Azure.Tests/ExistingAzureResourceTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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' = {
Expand Down Expand Up @@ -1096,8 +1096,7 @@ public async Task SupportsExistingAzureSqlServerInRunMode()
"params": {
"existingResourceName": "{existingResourceName.value}",
"principalId": "",
"principalName": "",
"principalType": ""
"principalName": ""
}
}
""";
Expand All @@ -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' = {
Expand Down Expand Up @@ -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
""";

Expand Down Expand Up @@ -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: {
Expand All @@ -1367,7 +1363,7 @@ param principalId string
}
scope: openAI
}

resource mymodel 'Microsoft.CognitiveServices/accounts/deployments@2024-10-01' = {
name: 'mymodel'
properties: {
Expand Down