Improve managed identity for sql server #9041
Conversation
github-actions
bot
added
the
area-integrations
sebastienros
changed the title
# Conflicts: # src/Aspire.Hosting.Azure/AzurePublisher.cs
src/Aspire.Hosting.Azure.Sql/SqlServerScriptProvisioningResource.cs
Outdated
Show resolved
Hide resolved
playground/SqlServerScript/AppHost1/api1-roles-mysqlserver.module.bicep
Outdated
Show resolved
Hide resolved
src/Aspire.Hosting.Azure.Sql/SqlServerScriptProvisioningResource.cs
Outdated
Show resolved
Hide resolved
|
This code can now be deleted. aspire/src/Aspire.Hosting.Azure.Sql/AzureSqlExtensions.cs Lines 249 to 257 in 88ff939 |
| } | ||
| kind: 'AzurePowerShell' | ||
| properties: { | ||
| scriptContent: '\$sqlServerFqdn = "\$env:DBSERVER"\r\n\$sqlDatabaseName = "\$env:DBNAME"\r\n\$principalName = "\$env:PRINCIPALNAME"\r\n\$id = "\$env:ID"\r\n\r\n# Install SqlServer module\r\nInstall-Module -Name SqlServer -Force -AllowClobber -Scope CurrentUser\r\nImport-Module SqlServer\r\n\r\n\$sqlCmd = @"\r\nDECLARE @name SYSNAME = \'\$principalName\';\r\nDECLARE @id UNIQUEIDENTIFIER = \'\$id\';\r\n\r\n-- Convert the guid to the right type\r\nDECLARE @castId NVARCHAR(MAX) = CONVERT(VARCHAR(MAX), CONVERT (VARBINARY(16), @id), 1);\r\n\r\n-- Construct command: CREATE USER [@name] WITH SID = @castId, TYPE = E;\r\nDECLARE @cmd NVARCHAR(MAX) = N\'CREATE USER [\' + @name + \'] WITH SID = \' + @castId + \', TYPE = E;\'\r\nEXEC (@cmd);\r\n\r\n-- Assign roles to the new user\r\nDECLARE @role1 NVARCHAR(MAX) = N\'ALTER ROLE db_owner ADD MEMBER [\' + @name + \']\';\r\nEXEC (@role1);\r\n\r\n"@\r\n# Note: the string terminator must not have whitespace before it, therefore it is not indented.\r\n\r\nWrite-Host \$sqlCmd\r\n\r\n\$connectionString = "Server=tcp:\${sqlServerFqdn},1433;Initial Catalog=\${sqlDatabaseName};Authentication=Active Directory Default;"\r\n\r\nInvoke-Sqlcmd -ConnectionString \$connectionString -Query \$sqlCmd' |
There was a problem hiding this comment.
Is there a way to make this scriptContent readable? e.g. have newlines in it. I assume not, but needed to ask.
There was a problem hiding this comment.
I think it's a CDK limitation, we can set a bicep string value to the property, which will be encoded in bicep, but I don't know if we can set the literal that has the multi-line string (line in c#) fragment. @tg-msft
|
@eerhardt things for closing:
|
| @@ -1,12 +1,18 @@ | |||
| <?xml version="1.0" encoding="utf-8"?> | |||
| <!-- https://learn.microsoft.com/dotnet/fundamentals/package-validation/diagnostic-ids --> | |||
| <Suppressions xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:xsd="http://www.w3.org/2001/XMLSchema"> | |||
| <!-- This API shipped as experimental in 9.2 so it is okay to remove. --> | |||
There was a problem hiding this comment.
I think this comment should stay
There was a problem hiding this comment.
Also added a comment for the new suppression
| @@ -1,4 +1,4 @@ | |||
| <Project Sdk="Microsoft.NET.Sdk"> | |||
| <Project Sdk="Microsoft.NET.Sdk"> | |||
There was a problem hiding this comment.
Can you revert this? git checkout <sha-before-your-changes> -- src/Aspire.Hosting.Azure.Sql/Aspire.Hosting.Azure.Sql.csproj
| _sid = DefineProperty<Guid>("Sid", ["properties", "sid"]); | ||
| _parent = DefineResource<SqlServer>("Parent", ["parent"], isOutput: false, isRequired: true); | ||
| } | ||
| infrastructure.Add(new ProvisioningOutput("sqlServerAdminName", typeof(string)) { Value = sqlServer.Administrators.Login }); |
There was a problem hiding this comment.
What does this output for an existing SQL Server that has a username/password administrator? The username?
There was a problem hiding this comment.
Correct, the username, but I believe this case is fine since in that case we don't try to update the resource.
| { | ||
| Name = BicepFunction.Take(BicepFunction.Interpolate($"script-{BicepFunction.GetUniqueString(this.GetBicepIdentifier(), roleAssignmentContext.PrincipalName, new StringLiteralExpression(database), BicepFunction.GetResourceGroup().Id)}"), 24), | ||
| Kind = "AzurePowerShell", | ||
| AZPowerShellVersion = "7.4" |
There was a problem hiding this comment.
How do we keep this up-to-date?
There was a problem hiding this comment.
I updated it to the latest right now and added a comment with the official list. I assume we'll be able to remove it once the resource is supported in the CDK.
# Conflicts: # src/Aspire.Hosting.Azure.Sql/AzureSqlExtensions.cs # tests/Aspire.Hosting.Azure.Tests/AzureSqlExtensionsTests.cs
davidfowl
changed the title
Description
Please include a summary of the changes and the related issue. Please also include relevant motivation and context. List any dependencies that are required for this change.
Fixes #8389
Remaining tasks:
Checklist
<remarks />and<code />elements on your triple slash comments?