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.

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

🐛 Cannot deploy bicep for Azure SQL #585

Closed
2 tasks
matt-goldman opened this issue Jan 16, 2024 · 1 comment · Fixed by #586
Closed
2 tasks

🐛 Cannot deploy bicep for Azure SQL #585

matt-goldman opened this issue Jan 16, 2024 · 1 comment · Fixed by #586
Assignees
Labels

Comments

@matt-goldman
Copy link
Contributor

Cc: @william-liebenberg @tkapa @zacharykeeping @wicksipedia @Anton-Polkanov

Describe the Bug

We are getting failed deployments at the moment due to an issue with the Azure SQL bicep. The issue is with this code:

resource sqlServer 'Microsoft.Sql/servers@2021-02-01-preview' = {
  name: sqlserverName
  location: location
  properties: {
    administratorLogin: sqlAdministratorLogin
    administratorLoginPassword: sqlAdministratorLoginPassword
    administrators: {
      administratorType: 'ActiveDirectory'
      login: sqlAdministratorsLoginName
      sid: sqlAdministratorsObjectId
      tenantId: tenant().tenantId
      principalType: 'Group'
      azureADOnlyAuthentication: false
    }
    version: '12.0'
  }
}

In particular, the expected values under administrators for login and sid are the display name and object ID of an AAD group (or user, although the principal type here is group) that you want to set as an administrator for the SQL server. Ostensibly this deployment should be able to replace whatever the current administrator is with whatever you specify here, however, due to a seemingly well-known issue, the deployment will fail unless you specify the values here of the current user or group.

The current user or group is not known and the secrets are in the archived repo and unreadable.

To Reproduce

Steps to reproduce the behavior:

  1. Go to https://github.com/SSWConsulting/SSW.Rewards.Mobile/actions/workflows/api-main.yml
  2. Click on Run workflow
  3. Wait for workflow to fail
  4. See error

Expected Behavior

The workflow should successfully deploy the resources to Azure

Tasks

  • Investigate
  • Fix

More Information

I don't know how to fix this, but here are some potential workarounds (all of which are temporary):

  • Remove this section from the bicep file
  • Manually add the newly created group that we are trying to use in this deployment as administrator of these resources
  • Run a workflow in the old repo to spit out the secrets
  • Backup the database and nuke the resource, then redeploy

Some relevant links:
https://techcommunity.microsoft.com/t5/azure-database-support-blog/arm-template-conflict-azure-sql-database-deployment-fails-with/ba-p/3321629

https://learn.microsoft.com/en-us/answers/questions/1360255/not-able-to-change-aad-administrator-for-sql-serve

https://stackoverflow.com/questions/77084543/bicep-microsoft-sql-servers2022-05-01-preview-exception-invalid-value-given-f

https://learn.microsoft.com/en-us/answers/questions/723847/azure-sql-database-deployment-fails-with-bicep-set

Possible fix

According to this discussion: Azure/bicep-types-az#2320

It looks as though it may be possible to set the administrators resource separately then assign the value, rather than trying to set the administrators directly on the SQL Server resource. Also explained that what we are trying to do is not supported here: MicrosoftDocs/azure-docs#111440

See also: https://learn.microsoft.com/en-us/azure/templates/microsoft.sql/servers/administrators?pivots=deployment-language-arm-template

Thanks!

@matt-goldman
Copy link
Contributor Author

Comment to add that it appears the app service is not using a managed identity and is using a sql username and password in the connection string, stored in key vault. So changing this should theoretically not impact the resource. But switching to a managed identity in the appropriate group is presumably the intended outcome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant