Fix Azure SQL Server private endpoints#14664
Open
eerhardt wants to merge 13 commits intodotnet:release/13.2from
Open
Fix Azure SQL Server private endpoints#14664eerhardt wants to merge 13 commits intodotnet:release/13.2from
eerhardt wants to merge 13 commits intodotnet:release/13.2from
Conversation
Added support for Azure SQL Server deployment scripts in private endpoint scenarios, enabling secure, automated database role assignments for managed identities. Deployment scripts need to be part of the vnet and need Azure Storage accounts to mount file shares. Introduced resource annotations and preparer classes to detect and configure ACI subnet and storage account infrastructure, with both automatic and explicit configuration options. Implemented subnet address allocation logic, updated Bicep modules and manifest files, and ensured scripts run in delegated subnets with proper outbound rules and Azure Files access. Added tests to verify all configuration combinations. Updated project references and dependencies. Fix dotnet#14421
Contributor
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 14664Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 14664" |
Member
Author
|
I think this is ready for review. One issue is that Azure.Provisioning.Network is still unstable and we need to reference it from our Aspire.Hosting.Azure.Sql (stable) package. We will need to get that package stable and make our Aspire.Hosting.Azure.Network package stable (but will add [Experimental] to the whole assembly). |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds VNet-integrated Azure deployment script support for Azure SQL Server private endpoint scenarios, enabling database role assignment scripts to run inside delegated subnets and use Azure Storage for script/log/file share mounting.
Changes:
- Introduces private-endpoint notification plumbing to trigger SQL deployment-script infrastructure (ACI subnet + storage + NSG + storage files private endpoint).
- Adds subnet address allocation logic and new configuration APIs to explicitly choose deployment-script subnet/storage.
- Adds snapshot-based tests and updates playground modules/manifests to cover private endpoint scenarios.
Reviewed changes
Copilot reviewed 35 out of 35 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Aspire.Hosting.Azure.Tests/Snapshots/AzureSqlDeploymentScriptTests.SqlWithPrivateEndpoint_SubnetBeforePrivateEndpoint.verified.bicep | New snapshot covering subnet-first configuration ordering. |
| tests/Aspire.Hosting.Azure.Tests/Snapshots/AzureSqlDeploymentScriptTests.SqlWithPrivateEndpoint_StorageBeforePrivateEndpoint.verified.bicep | New snapshot covering storage-first configuration ordering. |
| tests/Aspire.Hosting.Azure.Tests/Snapshots/AzureSqlDeploymentScriptTests.SqlWithPrivateEndpoint_ExplicitSubnet_AutoCreatesStorage.verified.bicep | Snapshot for explicit subnet + auto storage behavior. |
| tests/Aspire.Hosting.Azure.Tests/Snapshots/AzureSqlDeploymentScriptTests.SqlWithPrivateEndpoint_ExplicitStorage_AutoCreatesSubnet.verified.bicep | Snapshot for explicit storage + auto subnet behavior. |
| tests/Aspire.Hosting.Azure.Tests/Snapshots/AzureSqlDeploymentScriptTests.SqlWithPrivateEndpoint_BothExplicitSubnetAndStorage.verified.bicep | Snapshot for explicit subnet + explicit storage behavior. |
| tests/Aspire.Hosting.Azure.Tests/Snapshots/AzureSqlDeploymentScriptTests.SqlWithPrivateEndpoint_AutoCreatesBothSubnetAndStorage.verified.bicep | Snapshot for full auto behavior. |
| tests/Aspire.Hosting.Azure.Tests/AzureSqlDeploymentScriptTests.cs | Adds tests for multiple ordering/configuration combinations. |
| src/Aspire.Hosting.Azure/ExistingAzureResourceAnnotation.cs | Expands “existing resource name” annotation to support BicepOutputReference. |
| src/Aspire.Hosting.Azure/AzureProvisioningResource.cs | Implements BicepOutputReference handling when creating existing resources. |
| src/Aspire.Hosting.Azure.Sql/SubnetAddressAllocator.cs | Adds CIDR parsing + /29 allocation logic for auto-created ACI subnet. |
| src/Aspire.Hosting.Azure.Sql/AzureSqlServerResource.cs | Adds private-endpoint notification handling, deployment script subnet/storage wiring, and model preparation logic. |
| src/Aspire.Hosting.Azure.Sql/AzureSqlExtensions.cs | Adds WithAdminDeploymentScriptSubnet / WithAdminDeploymentScriptStorage APIs. |
| src/Aspire.Hosting.Azure.Sql/Aspire.Hosting.Azure.Sql.csproj | Adds project references to Azure Network and Azure Storage packages. |
| src/Aspire.Hosting.Azure.Sql/AdminDeploymentScriptSubnetAnnotation.cs | Introduces an annotation to carry the deployment-script subnet reference. |
| src/Aspire.Hosting.Azure.Network/AzureVirtualNetworkResource.cs | Makes Subnets publicly accessible. |
| src/Aspire.Hosting.Azure.Network/AzureSubnetServiceDelegationAnnotation.cs | Makes subnet delegation annotation public. |
| src/Aspire.Hosting.Azure.Network/AzurePrivateEndpointExtensions.cs | Adds notification hook interface and calls it during private endpoint creation. |
| playground/AzureVirtualNetworkEndToEnd/AzureVirtualNetworkEndToEnd.AppHost/vnet.module.bicep | Adds SQL ACI subnet outputs/params to the playground VNet module. |
| playground/AzureVirtualNetworkEndToEnd/AzureVirtualNetworkEndToEnd.AppHost/sql.module.bicep | Adds SQL module for the playground scenario. |
| playground/AzureVirtualNetworkEndToEnd/AzureVirtualNetworkEndToEnd.AppHost/sql-store.module.bicep | Adds storage module used by deployment scripts in playground. |
| playground/AzureVirtualNetworkEndToEnd/AzureVirtualNetworkEndToEnd.AppHost/sql-nsg.module.bicep | Adds NSG module for deployment script outbound rules in playground. |
| playground/AzureVirtualNetworkEndToEnd/AzureVirtualNetworkEndToEnd.AppHost/sql-admin-identity.module.bicep | Adds module to reference the SQL admin identity output. |
| playground/AzureVirtualNetworkEndToEnd/AzureVirtualNetworkEndToEnd.AppHost/sql-admin-identity-roles-sql-store.module.bicep | Adds role assignment module for admin identity -> storage files access. |
| playground/AzureVirtualNetworkEndToEnd/AzureVirtualNetworkEndToEnd.AppHost/privatelink-file-core-windows-net.module.bicep | Adds private DNS zone module for storage files. |
| playground/AzureVirtualNetworkEndToEnd/AzureVirtualNetworkEndToEnd.AppHost/privatelink-database-windows-net.module.bicep | Adds private DNS zone module for SQL. |
| playground/AzureVirtualNetworkEndToEnd/AzureVirtualNetworkEndToEnd.AppHost/private-endpoints-sql-pe.module.bicep | Adds playground SQL private endpoint module. |
| playground/AzureVirtualNetworkEndToEnd/AzureVirtualNetworkEndToEnd.AppHost/private-endpoints-files-pe.module.bicep | Adds playground storage files private endpoint module. |
| playground/AzureVirtualNetworkEndToEnd/AzureVirtualNetworkEndToEnd.AppHost/aspire-manifest.json | Wires new modules/params for SQL + deployment-script infra. |
| playground/AzureVirtualNetworkEndToEnd/AzureVirtualNetworkEndToEnd.AppHost/api-roles-sql.module.bicep | Adds deployment script role assignment module for SQL DB access. |
| playground/AzureVirtualNetworkEndToEnd/AzureVirtualNetworkEndToEnd.AppHost/api-containerapp.module.bicep | Passes SQL connection env vars/connection strings into the playground container app. |
| playground/AzureVirtualNetworkEndToEnd/AzureVirtualNetworkEndToEnd.AppHost/Program.cs | Adds SQL + private endpoint + DB reference to the playground AppHost. |
| playground/AzureVirtualNetworkEndToEnd/AzureVirtualNetworkEndToEnd.AppHost/AzureVirtualNetworkEndToEnd.AppHost.csproj | Adds Azure SQL hosting package reference for the playground. |
| playground/AzureVirtualNetworkEndToEnd/AzureVirtualNetworkEndToEnd.ApiService/Program.cs | Adds a simple /sql endpoint using SqlConnection. |
| playground/AzureVirtualNetworkEndToEnd/AzureVirtualNetworkEndToEnd.ApiService/AzureVirtualNetworkEndToEnd.ApiService.csproj | Adds SqlClient package reference for the playground API. |
src/Aspire.Hosting.Azure.Network/AzurePrivateEndpointExtensions.cs
Outdated
Show resolved
Hide resolved
Member
Author
|
/deployment-test |
Contributor
|
🚀 Deployment tests starting on PR #14664... This will deploy to real Azure infrastructure. Results will be posted here when complete. |
Member
Author
|
/deployment-test |
Contributor
|
🚀 Deployment tests starting on PR #14664... This will deploy to real Azure infrastructure. Results will be posted here when complete. |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 37 out of 37 changed files in this pull request and generated 5 comments.
Comments suppressed due to low confidence (1)
src/Aspire.Hosting.Azure.Network/AzureSubnetServiceDelegationAnnotation.cs:24
AzureSubnetServiceDelegationAnnotationis nowpublic, but its XML docs are still minimal (no details on expected values, behavior, or usage). Since this is a newly public API insrc/, it should include more complete documentation (e.g., describe how it affects subnet provisioning, whatNamevsServiceNameshould be set to, and include an example of applying the annotation to a subnet).
/// <summary>
/// Annotation to specify a service delegation for an Azure Subnet.
/// </summary>
/// <param name="name">The name of the service delegation.</param>
/// <param name="serviceName">The service name for the delegation (e.g., "Microsoft.App/environments").</param>
public sealed class AzureSubnetServiceDelegationAnnotation(string name, string serviceName) : IResourceAnnotation
{
/// <summary>
/// Gets or sets the name associated with the service delegation.
/// </summary>
public string Name { get; set; } = name;
/// <summary>
/// Gets or sets the name of the service associated with the service delegation.
/// </summary>
public string ServiceName { get; set; } = serviceName;
}
Comment on lines
+445
to
+490
| // Create a deployment script storage account (publish mode only). | ||
| // The BeforeStartEvent handler will remove the default storage if it's no longer | ||
| // needed if the user swapped it via WithAdminDeploymentScriptStorage. | ||
| AzureStorageResource? createdStorage = null; | ||
| if (DeploymentScriptStorage is null) | ||
| { | ||
| DeploymentScriptStorage = CreateDeploymentScriptStorage(builder, builder.CreateResourceBuilder(this)).Resource; | ||
| createdStorage = DeploymentScriptStorage; | ||
| } | ||
|
|
||
| var admin = builder.AddAzureUserAssignedIdentity($"{Name}-admin-identity") | ||
| .WithAnnotation(new ExistingAzureResourceAnnotation(AdminName)); | ||
| AdminIdentity = admin.Resource; | ||
|
|
||
| DeploymentScriptNetworkSecurityGroup = builder.AddNetworkSecurityGroup($"{Name}-nsg") | ||
| .WithSecurityRule(new AzureSecurityRule() | ||
| { | ||
| Name = "allow-outbound-443-AzureActiveDirectory", | ||
| Priority = 100, | ||
| Direction = SecurityRuleDirection.Outbound, | ||
| Access = SecurityRuleAccess.Allow, | ||
| Protocol = SecurityRuleProtocol.Tcp, | ||
| SourceAddressPrefix = "*", | ||
| SourcePortRange = "*", | ||
| DestinationAddressPrefix = AzureServiceTags.AzureActiveDirectory, | ||
| DestinationPortRange = "443", | ||
| }) | ||
| .WithSecurityRule(new AzureSecurityRule() | ||
| { | ||
| Name = "allow-outbound-443-Sql", | ||
| Priority = 200, | ||
| Direction = SecurityRuleDirection.Outbound, | ||
| Access = SecurityRuleAccess.Allow, | ||
| Protocol = SecurityRuleProtocol.Tcp, | ||
| SourceAddressPrefix = "*", | ||
| SourcePortRange = "*", | ||
| DestinationAddressPrefix = AzureServiceTags.Sql, | ||
| DestinationPortRange = "443", | ||
| }).Resource; | ||
|
|
||
| builder.Eventing.Subscribe<BeforeStartEvent>((data, token) => | ||
| { | ||
| PrepareDeploymentScriptInfrastructure(data.Model, this, createdStorage); | ||
|
|
||
| return Task.CompletedTask; | ||
| }); |
There was a problem hiding this comment.
OnPrivateEndpointCreated is not idempotent: if multiple private endpoints are added for the same SQL server (or the extension is called more than once), this method will create additional admin identity/NSG resources and register multiple BeforeStartEvent subscriptions. Consider guarding so the setup runs only once per SQL server (e.g., check AdminIdentity/DeploymentScriptNetworkSecurityGroup/a dedicated flag before adding resources and subscribing).
Suggested change
| // Create a deployment script storage account (publish mode only). | |
| // The BeforeStartEvent handler will remove the default storage if it's no longer | |
| // needed if the user swapped it via WithAdminDeploymentScriptStorage. | |
| AzureStorageResource? createdStorage = null; | |
| if (DeploymentScriptStorage is null) | |
| { | |
| DeploymentScriptStorage = CreateDeploymentScriptStorage(builder, builder.CreateResourceBuilder(this)).Resource; | |
| createdStorage = DeploymentScriptStorage; | |
| } | |
| var admin = builder.AddAzureUserAssignedIdentity($"{Name}-admin-identity") | |
| .WithAnnotation(new ExistingAzureResourceAnnotation(AdminName)); | |
| AdminIdentity = admin.Resource; | |
| DeploymentScriptNetworkSecurityGroup = builder.AddNetworkSecurityGroup($"{Name}-nsg") | |
| .WithSecurityRule(new AzureSecurityRule() | |
| { | |
| Name = "allow-outbound-443-AzureActiveDirectory", | |
| Priority = 100, | |
| Direction = SecurityRuleDirection.Outbound, | |
| Access = SecurityRuleAccess.Allow, | |
| Protocol = SecurityRuleProtocol.Tcp, | |
| SourceAddressPrefix = "*", | |
| SourcePortRange = "*", | |
| DestinationAddressPrefix = AzureServiceTags.AzureActiveDirectory, | |
| DestinationPortRange = "443", | |
| }) | |
| .WithSecurityRule(new AzureSecurityRule() | |
| { | |
| Name = "allow-outbound-443-Sql", | |
| Priority = 200, | |
| Direction = SecurityRuleDirection.Outbound, | |
| Access = SecurityRuleAccess.Allow, | |
| Protocol = SecurityRuleProtocol.Tcp, | |
| SourceAddressPrefix = "*", | |
| SourcePortRange = "*", | |
| DestinationAddressPrefix = AzureServiceTags.Sql, | |
| DestinationPortRange = "443", | |
| }).Resource; | |
| builder.Eventing.Subscribe<BeforeStartEvent>((data, token) => | |
| { | |
| PrepareDeploymentScriptInfrastructure(data.Model, this, createdStorage); | |
| return Task.CompletedTask; | |
| }); | |
| // Create deployment script infrastructure (publish mode only) once per SQL server. | |
| // The BeforeStartEvent handler will remove the default storage if it's no longer | |
| // needed if the user swapped it via WithAdminDeploymentScriptStorage. | |
| if (AdminIdentity is null && DeploymentScriptNetworkSecurityGroup is null) | |
| { | |
| AzureStorageResource? createdStorage = null; | |
| if (DeploymentScriptStorage is null) | |
| { | |
| DeploymentScriptStorage = CreateDeploymentScriptStorage(builder, builder.CreateResourceBuilder(this)).Resource; | |
| createdStorage = DeploymentScriptStorage; | |
| } | |
| var admin = builder.AddAzureUserAssignedIdentity($"{Name}-admin-identity") | |
| .WithAnnotation(new ExistingAzureResourceAnnotation(AdminName)); | |
| AdminIdentity = admin.Resource; | |
| DeploymentScriptNetworkSecurityGroup = builder.AddNetworkSecurityGroup($"{Name}-nsg") | |
| .WithSecurityRule(new AzureSecurityRule() | |
| { | |
| Name = "allow-outbound-443-AzureActiveDirectory", | |
| Priority = 100, | |
| Direction = SecurityRuleDirection.Outbound, | |
| Access = SecurityRuleAccess.Allow, | |
| Protocol = SecurityRuleProtocol.Tcp, | |
| SourceAddressPrefix = "*", | |
| SourcePortRange = "*", | |
| DestinationAddressPrefix = AzureServiceTags.AzureActiveDirectory, | |
| DestinationPortRange = "443", | |
| }) | |
| .WithSecurityRule(new AzureSecurityRule() | |
| { | |
| Name = "allow-outbound-443-Sql", | |
| Priority = 200, | |
| Direction = SecurityRuleDirection.Outbound, | |
| Access = SecurityRuleAccess.Allow, | |
| Protocol = SecurityRuleProtocol.Tcp, | |
| SourceAddressPrefix = "*", | |
| SourcePortRange = "*", | |
| DestinationAddressPrefix = AzureServiceTags.Sql, | |
| DestinationPortRange = "443", | |
| }).Resource; | |
| builder.Eventing.Subscribe<BeforeStartEvent>((data, token) => | |
| { | |
| PrepareDeploymentScriptInfrastructure(data.Model, this, createdStorage); | |
| return Task.CompletedTask; | |
| }); | |
| } |
Comment on lines
+9
to
+17
| /// An optional interface that can be implemented by resources that are targets for | ||
| /// Azure private endpoints, to receive a notification when a private endpoint is created for them. | ||
| /// </summary> | ||
| public interface IAzurePrivateEndpointTargetNotification : IAzurePrivateEndpointTarget | ||
| { | ||
| /// <summary> | ||
| /// Handles the event that occurs when a new Azure private endpoint resource is created. | ||
| /// </summary> | ||
| /// <param name="privateEndpoint">The Azure private endpoint resource that was created. Cannot be null.</param> |
There was a problem hiding this comment.
IAzurePrivateEndpointTargetNotification introduces a new public extensibility surface, but the XML docs don’t explain when OnPrivateEndpointCreated is called (run vs publish mode), what implementations are allowed/expected to do inside it, or any ordering/side-effect constraints. Please expand the docs (remarks + example) to make this contract clear to consumers implementing it.
Suggested change
| /// An optional interface that can be implemented by resources that are targets for | |
| /// Azure private endpoints, to receive a notification when a private endpoint is created for them. | |
| /// </summary> | |
| public interface IAzurePrivateEndpointTargetNotification : IAzurePrivateEndpointTarget | |
| { | |
| /// <summary> | |
| /// Handles the event that occurs when a new Azure private endpoint resource is created. | |
| /// </summary> | |
| /// <param name="privateEndpoint">The Azure private endpoint resource that was created. Cannot be null.</param> | |
| /// Defines a contract for resources that are targets for Azure private endpoints and want to | |
| /// receive notifications when private endpoint resources are created for them. | |
| /// </summary> | |
| /// <remarks> | |
| /// Implement this interface on a resource type that also implements <see cref="IAzurePrivateEndpointTarget"/> | |
| /// when you need to customize the Aspire application model each time an <see cref="AzurePrivateEndpointResource"/> | |
| /// is created targeting that resource. | |
| /// </remarks> | |
| public interface IAzurePrivateEndpointTargetNotification : IAzurePrivateEndpointTarget | |
| { | |
| /// <summary> | |
| /// Called by the hosting infrastructure when a new Azure private endpoint resource is created for this target. | |
| /// </summary> | |
| /// <param name="privateEndpoint"> | |
| /// The builder for the Azure private endpoint resource that was created. This value is never <c>null</c>. | |
| /// </param> | |
| /// <remarks> | |
| /// <para> | |
| /// This callback is invoked as part of Aspire application model construction, during both "run" and "publish" | |
| /// workflows, whenever the hosting infrastructure creates an <see cref="AzurePrivateEndpointResource"/> that | |
| /// targets the implementing resource. | |
| /// Implementations should treat this method as a configuration hook for the <paramref name="privateEndpoint"/> | |
| /// and must not assume anything about the order in which different resources or callbacks are processed. | |
| /// </para> | |
| /// <para> | |
| /// Implementations are expected to: | |
| /// <list type="bullet"> | |
| /// <item> | |
| /// <description> | |
| /// Restrict their work to updating the supplied <paramref name="privateEndpoint"/>, for example by configuring | |
| /// DNS zone groups, tags, or other application model metadata on the private endpoint resource. | |
| /// </description> | |
| /// </item> | |
| /// <item> | |
| /// <description> | |
| /// Avoid performing long-running or blocking operations, network calls, or other side effects external to the | |
| /// Aspire application model. This method should be fast and side-effect free with respect to external systems. | |
| /// </description> | |
| /// </item> | |
| /// <item> | |
| /// <description> | |
| /// Be idempotent. Callers may invoke this method more than once for the same logical endpoint during model | |
| /// construction, so implementations should tolerate duplicate calls without changing the resulting configuration. | |
| /// </description> | |
| /// </item> | |
| /// </list> | |
| /// </para> | |
| /// </remarks> | |
| /// <example> | |
| /// This example shows a resource that configures each created private endpoint by adding a custom parameter: | |
| /// <code> | |
| /// public sealed class MyDatabaseResource : IAzurePrivateEndpointTargetNotification | |
| /// { | |
| /// public void OnPrivateEndpointCreated(IResourceBuilder<AzurePrivateEndpointResource> privateEndpoint) | |
| /// { | |
| /// // Configure the private endpoint resource in the Aspire application model. | |
| /// privateEndpoint.WithParameter("dnsZoneGroupName", "my-database-dns-zone-group"); | |
| /// } | |
| /// } | |
| /// </code> | |
| /// </example> |
Comment on lines
+355
to
+439
| /// <summary> | ||
| /// Configures the Azure SQL Server to use the specified subnet for deployment script execution. | ||
| /// </summary> | ||
| /// <param name="builder">The Azure SQL Server resource builder.</param> | ||
| /// <param name="subnet">The subnet to delegate for Azure Container Instances used by deployment scripts.</param> | ||
| /// <returns>A reference to the <see cref="IResourceBuilder{AzureSqlServerResource}"/> for chaining.</returns> | ||
| /// <remarks> | ||
| /// <para> | ||
| /// When an Azure SQL Server has a private endpoint, deployment scripts that add database role assignments | ||
| /// run inside Azure Container Instances (ACI). This method allows you to provide an explicit subnet for those | ||
| /// containers instead of having one auto-created. | ||
| /// </para> | ||
| /// <para> | ||
| /// The specified subnet will be automatically delegated to <c>Microsoft.ContainerInstance/containerGroups</c>. | ||
| /// Ensure the subnet has outbound network security rules allowing access to Azure Active Directory (port 443) | ||
| /// and SQL (port 443) service tags. | ||
| /// </para> | ||
| /// </remarks> | ||
| [Experimental("ASPIREAZURE003", UrlFormat = "https://aka.ms/dotnet/aspire/diagnostics#{0}")] | ||
| public static IResourceBuilder<AzureSqlServerResource> WithAdminDeploymentScriptSubnet( | ||
| this IResourceBuilder<AzureSqlServerResource> builder, | ||
| IResourceBuilder<AzureSubnetResource> subnet) | ||
| { | ||
| ArgumentNullException.ThrowIfNull(builder); | ||
| ArgumentNullException.ThrowIfNull(subnet); | ||
|
|
||
| if (builder.ApplicationBuilder.ExecutionContext.IsRunMode) | ||
| { | ||
| return builder; | ||
| } | ||
|
|
||
| builder.Resource.Annotations.Add(new AdminDeploymentScriptSubnetAnnotation(subnet.Resource)); | ||
|
|
||
| return builder; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Configures the Azure SQL Server to use the specified storage account for deployment script execution. | ||
| /// </summary> | ||
| /// <param name="builder">The Azure SQL Server resource builder.</param> | ||
| /// <param name="storage">The storage account to use for deployment scripts.</param> | ||
| /// <returns>A reference to the <see cref="IResourceBuilder{AzureSqlServerResource}"/> for chaining.</returns> | ||
| /// <remarks> | ||
| /// <para> | ||
| /// When an Azure SQL Server has a private endpoint, deployment scripts require a storage account to upload | ||
| /// scripts and write logs. This method allows you to provide an explicit storage account instead of having | ||
| /// one auto-created. | ||
| /// </para> | ||
| /// <para> | ||
| /// The storage account must have <c>AllowSharedKeyAccess</c> enabled, as deployment scripts need to mount | ||
| /// file shares. If the storage is not an existing resource, this method will automatically configure | ||
| /// <c>AllowSharedKeyAccess = true</c>. | ||
| /// </para> | ||
| /// </remarks> | ||
| [Experimental("ASPIREAZURE003", UrlFormat = "https://aka.ms/dotnet/aspire/diagnostics#{0}")] | ||
| public static IResourceBuilder<AzureSqlServerResource> WithAdminDeploymentScriptStorage( | ||
| this IResourceBuilder<AzureSqlServerResource> builder, | ||
| IResourceBuilder<AzureStorageResource> storage) | ||
| { | ||
| ArgumentNullException.ThrowIfNull(builder); | ||
| ArgumentNullException.ThrowIfNull(storage); | ||
|
|
||
| if (!builder.ApplicationBuilder.ExecutionContext.IsPublishMode) | ||
| { | ||
| return builder; | ||
| } | ||
|
|
||
| // Set the user's storage. The BeforeStartEvent handler will remove the | ||
| // original default storage since it no longer matches. | ||
| builder.Resource.DeploymentScriptStorage = storage.Resource; | ||
|
|
||
| // If the storage is not an existing resource, ensure AllowSharedKeyAccess is enabled | ||
| if (!storage.Resource.IsExisting()) | ||
| { | ||
| storage.ConfigureInfrastructure(infra => | ||
| { | ||
| var sa = infra.GetProvisionableResources().OfType<StorageAccount>().SingleOrDefault() | ||
| ?? throw new InvalidOperationException("Could not find a StorageAccount resource in the infrastructure. Ensure that the provided storage builder creates a StorageAccount resource."); | ||
|
|
||
| sa.AllowSharedKeyAccess = true; | ||
| }); | ||
| } | ||
|
|
||
| return builder; | ||
| } |
There was a problem hiding this comment.
The new public extension methods WithAdminDeploymentScriptSubnet / WithAdminDeploymentScriptStorage have <remarks> but no <example> / <code> section, while other public Azure SQL extension methods in this file (e.g., RunAsContainer) include a usage example. Consider adding an example showing a private-endpoint SQL server using these methods so consumers can apply them correctly.
Comment on lines
+344
to
360
| //var process = new System.Diagnostics.Process | ||
| //{ | ||
| // StartInfo = new System.Diagnostics.ProcessStartInfo | ||
| // { | ||
| // FileName = "az", | ||
| // Arguments = $"group delete --name {resourceGroupName} --yes --no-wait", | ||
| // RedirectStandardOutput = true, | ||
| // RedirectStandardError = true, | ||
| // UseShellExecute = false, | ||
| // CreateNoWindow = true | ||
| // } | ||
| //}; | ||
|
|
||
| try | ||
| { | ||
| process.Start(); | ||
| //process.Start(); | ||
| output.WriteLine($"Cleanup triggered for resource group: {resourceGroupName}"); |
There was a problem hiding this comment.
TriggerCleanupResourceGroup no longer starts the az group delete process (the process creation and Start() call are commented out). This will leave resource groups behind after the test run and can cause leaks/costs and quota issues in CI. Please re-enable the deletion or replace it with an equivalent cleanup mechanism (and ensure the cleanup path is still best-effort / fire-and-forget if that’s the intent).
Member
Author
There was a problem hiding this comment.
@copilot - revert this change so th resource group gets cleaned up.
Comment on lines
+24
to
+66
| public static string AllocateDeploymentScriptSubnet(AzureVirtualNetworkResource vnet, IEnumerable<AzureSubnetResource> existingSubnets) | ||
| { | ||
| const int prefixLength = 29; | ||
| const uint blockSize = 8; // 2^(32-29) = 8 | ||
|
|
||
| var vnetAddressPrefix = vnet.AddressPrefix | ||
| ?? throw new InvalidOperationException( | ||
| $"Cannot automatically allocate a deployment script subnet for virtual network '{vnet.Name}' because it uses a parameterized address prefix. " + | ||
| $"Use 'WithAdminDeploymentScriptSubnet' to provide an explicit subnet."); | ||
|
|
||
| var (vnetStart, vnetEnd) = ParseCidr(vnetAddressPrefix); | ||
|
|
||
| // Collect all existing subnet ranges | ||
| var existingRanges = new List<(uint Start, uint End)>(); | ||
| foreach (var subnet in existingSubnets) | ||
| { | ||
| if (subnet.AddressPrefix is { } subnetCidr) | ||
| { | ||
| var range = ParseCidr(subnetCidr); | ||
| existingRanges.Add(range); | ||
| } | ||
| // Skip subnets with parameterized addresses — can't check overlap | ||
| } | ||
|
|
||
| // Start from the highest /29-aligned address and work downward | ||
| var candidate = (vnetEnd - blockSize + 1) & ~(blockSize - 1); | ||
|
|
||
| while (candidate >= vnetStart) | ||
| { | ||
| var candidateEnd = candidate + blockSize - 1; | ||
|
|
||
| if (candidateEnd <= vnetEnd && !OverlapsAny(candidate, candidateEnd, existingRanges)) | ||
| { | ||
| return $"{UintToIp(candidate)}/{prefixLength}"; | ||
| } | ||
|
|
||
| if (candidate < blockSize) | ||
| { | ||
| break; // Prevent underflow | ||
| } | ||
|
|
||
| candidate -= blockSize; | ||
| } |
There was a problem hiding this comment.
AllocateDeploymentScriptSubnet can underflow and effectively loop for a very long time if the VNet CIDR range is smaller than a /29 (e.g., /30 or /31): candidate is computed with unsigned arithmetic and can wrap to a huge value, making the while (candidate >= vnetStart) loop iterate billions of times. Add an early guard that validates the VNet prefix provides at least 8 addresses (or directly checks vnetEnd - vnetStart + 1 >= blockSize) and fail fast with a clear exception when it doesn't.
Contributor
|
❌ Deployment E2E Tests failed Summary: 23 passed, 1 failed, 0 cancelled Passed Tests
Failed Tests
🎬 Terminal Recordings
|
This was referenced Feb 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Added support for Azure SQL Server deployment scripts in private endpoint scenarios, enabling secure, automated database role assignments for managed identities.
Deployment scripts need to be part of the vnet and need Azure Storage accounts to mount file shares.
Introduced resource annotations and preparer classes to detect and configure ACI subnet and storage account infrastructure, with both automatic and explicit configuration options. Implemented subnet address allocation logic, updated Bicep modules and manifest files, and ensured scripts run in delegated subnets with proper outbound rules and Azure Files access. Added tests to verify all configuration combinations. Updated project references and dependencies.
Fix #14421
Checklist
<remarks />and<code />elements on your triple slash comments?aspire.devissue: