Azure / bicep

Bicep is a declarative language for describing and deploying Azure resources
MIT License
3.19k stars 736 forks source link

Microsoft.Sql/servers not supporting changing AAD admin with templating #4988

Open jikuja opened 2 years ago

jikuja commented 2 years ago

Bicep version Bicep CLI version 0.4.1008 (223b8d227a)

Describe the bug Re-deployment Microsoft.Sql/servers [2020-11-01-preview and 2021-05-01-preview and probably others] does not change AD admin property(properties.administrators).

To Reproduce Deploy template once:

targetScope = 'resourceGroup'

param location string

param environmentName string
param project string
param tags object

param sqlAdministratorLogin string
@secure()
param sqlAdministratorLoginPassword string
param administrators object

resource SqlServer 'Microsoft.Sql/servers@2021-05-01-preview' = {
  name: '${project}-${environmentName}-sqlserver'
  location: location
  tags: tags
  properties: {
    administratorLogin: sqlAdministratorLogin
    administratorLoginPassword: sqlAdministratorLoginPassword
    version: '12.0'
    administrators: administrators
  }
}

params:

{
    "$schema": "https://schema.management.azure.com/schemas/2019-04-01/deploymentParameters.json#",
    "contentVersion": "1.0.0.0",
    "parameters": {
      "administrators": {
        "value": {
          "administratorType": "ActiveDirectory",
          "azureADOnlyAuthentication": false,
          "principalType": "Group",
          "tenantId": "0a8edbb9-xxxx-xxxx-xxxx-2cd241f8236f",
          "sid": "5bc4b275-xxxx-xxxx-xxxx-2fb5b5ff5fe1",
          "login": "SQL Admin2"
        }
      }
    }
  }

Change sid and login to other AAD group and run What-If analysis. Results:

The deployment will update the following scope:

Scope: /subscriptions/8a4fb900-xxxx-xxxx-xxxx-7faf38f07f51/resourceGroups/sqltest-rg

  = Microsoft.Sql/servers/xyz-test-sqlserver [2021-05-01-preview]
    x properties.version: "12.0"

  * Microsoft.Sql/servers/xyz-test-sqlserver/databases/master
  * Microsoft.Sql/servers/xyz-test-sqlserver/databases/my-sql

Resource changes: 1 no change, 2 to ignore.

Additional context

Running API request changes AAD admin value:

Invoke-AzRestMethod -Path "/subscriptions/8a4fb900-xxxx-xxxx-xxxx-7faf38f07f51/resourceGroups/sqltest-rg/providers/Microsoft.Sql/servers/xyz-test-sqlserver/administrators/activeDirectory?api-version=2021-05-01-preview" -Method PUT -Payload $payload

Headers    : {[Cache-Control, System.String[]], [Pragma, System.String[]], [Location, System.String[]], [Retry-After, System.String[]]…}
Version    : 1.1
StatusCode : 202
Method     : PUT
Content    : {"operation":"UpdateActiveDirectoryAdministrator","startTime":"2021-10-26T08:22:02.33Z"}
anthony-c-martin commented 2 years ago

The PUT request you invoke with Invoke-AzRestMethod is interacting with the administrators child resource. Modifying your example, to achieve something similar in Bicep, you would write:

targetScope = 'resourceGroup'

param location string

param environmentName string
param project string
param tags object

param sqlAdministratorLogin string
@secure()
param sqlAdministratorLoginPassword string
param administrators object

resource SqlServer 'Microsoft.Sql/servers@2021-05-01-preview' = {
  name: '${project}-${environmentName}-sqlserver'
  location: location
  tags: tags
  properties: {
    administratorLogin: sqlAdministratorLogin
    administratorLoginPassword: sqlAdministratorLoginPassword
    version: '12.0'
  }
}

resource sqlAdmins 'Microsoft.Sql/servers/administrators@2021-05-01-preview' = {
  parent: SqlServer
  name: 'ActiveDirectory'
  properties: administrators
}

Does that help?

jikuja commented 2 years ago

That works nicely. Good catch.

Anyway, looks like you can set the setting once with bicep + Microsoft.Sql/servers, and then changes are just ignored.

That should be somehow reflected on VS code tooltips and https://docs.microsoft.com/en-us/azure/templates/microsoft.sql/servers?tabs=bicep

martinakarolina commented 2 years ago

Is there any news on this one? It is still only possible to set the AAD admin once. When trying to change it via bicep, changes are ignored.

jikuja commented 2 years ago

Is there any news on this one? It is still only possible to set the AAD admin once. When trying to change it via bicep, changes are ignored.

Using Microsoft.Sql/servers/administrators child resource type instead of Microsoft.Sql/servers property fixed problem for me. Documentation still does not warn about property usage.

martinakarolina commented 2 years ago

Is there any news on this one? It is still only possible to set the AAD admin once. When trying to change it via bicep, changes are ignored.

Using Microsoft.Sql/servers/administrators child resource type instead of Microsoft.Sql/servers property fixed problem for me. Documentation still does not warn about property usage.

Like this it working. Thanks. The reason why we put the administators directly inside Microsoft.Sql/servers is because we were facing the issue described here: https://docs.microsoft.com/en-us/answers/questions/723847/azure-sql-database-deployment-fails-with-bicep-set.html. Hopefully this has been resolved.

martinakarolina commented 2 years ago

And unfortunately this is still the case: image

alex-frankel commented 2 years ago

Any InternalServerErrors can only be debugged/fixed by the Resource Provider team. Please open a support case to get this routed to the SQL RP team to further traction on the issue.

amolagar5 commented 2 years ago

Hello,

I apologize for late reply, just came to know about this thread.

Let me explain a little bit behavior wise.

  1. When a new server is created, AAD Admin can be added using servers API.
  2. Now if AAD admin needs to be updated, administrators API need to be used. This is for security reasons and to provide separation of duty.

I recently made a change to fail this update workflow when servers call is made with administrators in it.

I will take a look at this again to make sure behavior is documented and errors are visible to the caller.

amolagar5 commented 2 years ago

@martinakarolina @jikuja @anthony-c-martin

I validated on my prod server that if on update server, changes are made to the AAD Admin , it will throw an error now. In no changes are made, it will just be a no-op and no error will be thrown. This is to prevent unnecessary failures in scripts.

"status": "Failed",
"error": {
    "code": "InvalidParameterValue",
    "message": "Invalid value given for parameter ExternalAdministratorLoginName. Specify a valid parameter value."
}

This is what my template is. { "$schema": "https://schema.management.azure.com/schemas/2019-04-01/deploymentTemplate.json#", "contentVersion": "1.0.0.0", "parameters": { "vulnerabilityAssessments_Default_storageContainerPath": { "type": "SecureString" }, "servers_name": { "defaultValue": "", "type": "String" } }, "variables": {}, "resources": [ { "type": "Microsoft.Sql/servers", "apiVersion": "2021-11-01-preview", "name": "[parameters('servers_name')]", "location": "westeurope", "kind": "v12.0", "properties": { "administratorLogin": "Cl0785616", "version": "12.0", "minimalTlsVersion": "1.2", "publicNetworkAccess": "Enabled", "administrators": { "administratorType": "ActiveDirectory", "principalType": "Group", "login": "tesrme@microsoft.com", "sid": "5x89xfae-e9x7-4xd6-8x2d-1dx39x8xfx92", "tenantId": "71fx8xbf-86x1-41xf-91xb-2d7xd01xdbx7", "azureADOnlyAuthentication": true }, "restrictOutboundNetworkAccess": "Disabled" } } ] }

leifdude commented 1 year ago

And if AD Admin is set already, how do we remove/unset it via template/pipeline?

leifdude commented 1 year ago

Also, please please change the error message to something that does not make me think something is wrong with parameter passing. Maybe like "You can only set AD Admin on the servers api on initial deployment. Please set on the administrators api otherwise" And at the very least put this in the documentation so when others on my team run into this they can figure out whats wrong. I can not put comments in the template because you guys decided "its not allowed / within spec".

alex-frankel commented 1 year ago

Hey folks -- this is not something we can fix on the bicep/ARM template side. @leifdude / @jikuja / @amolagar5 -- can any of you open a support ticket so this can be routed to the SQL team? I will also pass this on to the contacts we have on the SQL team.

cmpl-giedriusk commented 1 year ago

Hi.

For anyone, who is stuck on this: I've got an answer from MSFT support, that "upsert is not a supported operation for the administrators property in the SQL Server API" and that you would have to use two separate templates. One for deployment and the other one for changing / updating the AAD admin. Unfortunately, not so declarative :(

alex-frankel commented 1 year ago

That is not an acceptable answer. Per the ARM Resource Provider Contract, they must support idempotent PUT operations if the payloads are identical to the current state. It's ok to not allow updates to specific values, but if the values are the same then it must be allowed.

Can you ask the support contact at microsoft to reach out to us at alfran@microsoft.com?

cmpl-giedriusk commented 1 year ago

That is not an acceptable answer. Per the ARM Resource Provider Contract, they must support idempotent PUT operations if the payloads are identical to the current state. It's ok to not allow updates to specific values, but if the values are the same then it must be allowed.

Can you ask the support contact at microsoft to reach out to us at alfran@microsoft.com?

Hi Alex.

Maybe I explained it incorrectly. They do allow using the same value, which results in no-op. The "update" operation is the issue.

alex-frankel commented 1 year ago

Ah sorry for misreading.

One for deployment and the other one for changing / updating the AAD admin.

Did they share what a template would look like that could update this property? From what you are saying, I am assuming it is not possible to update this resource via Bicep/ARM Templates at all.

juho-turunen-tampuuri commented 1 year ago

I think I'm hitting this same issue. I'm not able to update the adminstrator, but my deployment also breaks if I change the AD administror to different value and then back to the original via PowerShell or Azure Portal.

So if I touch the ad admin in any way my next bicep deploy will fail even though I'm trying to set properties to same values the server already has.

 {
  "status": "Failed",
  "error": {
    "code": "ResourceDeploymentFailure",
    "message": "The resource operation completed with terminal provisioning state 'Failed'.",
    "details": [
      {
        "code": "InvalidParameterValue",
        "message": "Invalid value given for parameter ExternalAdministratorLoginSid. Specify a valid parameter value."
      }
    ]
  }
} (Code:Conflict)

I've also noticed that if I've originally set the AD admin principal type to Application and then change the AD via PowerShell or Portal the principal type changes to Group. Even though admin is not a group. Using Group as admin type in bicep instead of Application doesn't help with the issue.

Next I'll try to work around the issue by checking if the server already exists via script and then passing that information to my bicep template. If someone doesn't have a better idea.

cmpl-giedriusk commented 1 year ago

Ah sorry for misreading.

One for deployment and the other one for changing / updating the AAD admin.

Did they share what a template would look like that could update this property? From what you are saying, I am assuming it is not possible to update this resource via Bicep/ARM Templates at all.

They did not share the template, but a link to SQL server administrators API (https://learn.microsoft.com/en-us/azure/templates/microsoft.sql/servers/administrators?pivots=deployment-language-bicep). Update works, you only have to use different template for that. The initial (deployment) template has to have AAD admin set as a property of "Microsoft.Sql/servers" resource. The "update" template would have to have "Microsoft.Sql/servers/administrators" resource.

@juho-turunen-tampuuri I've just tested deploying bicep template with User A as AAD admin, changing to user B via portal, changing back to User A via portal and then deploying the bicep template again. I've got no error messages. Maybe it's the API version? I'm using 'Microsoft.Sql/servers@2022-05-01-preview'

juho-turunen-tampuuri commented 1 year ago

@cmpl-giedriusk

I've just tested deploying bicep template with User A as AAD admin, changing to user B via portal, changing back to User A via portal and then deploying the bicep template again. I've got no error messages. Maybe it's the API version? I'm using 'Microsoft.Sql/servers@2022-05-01-preview'

Thank you for testing, but I can't get it to work. I'm changing the administrator via PowerShell command Set-AzSqlServerActiveDirectoryAdministrator. The original admin is managed identity with principal type Application and the temporary admin is a human user.

I thought I tried it with portal, too, but I'm not sure anymore. Might be some other problem with my setup but I'm pretty sure I've narrowed it down to this.

The reason for using a managed identity as admin is to give application access to the database without needing a human to create a user for it. But if a need arises to check out the database by a person you would need to change the administrator at least for the duration of creating an additional user for the database.

At the moment I don't have possibility to let Service principal create additional users and I'm trying to stick with AAD only authentication.

Here's my template for reference. I've tried principalType Group. I don't know if the principalType has any meaning. PowerShell always changes it to Group.

resource sqlserver 'Microsoft.Sql/servers@2022-05-01-preview' =  {
  name: 'somename'
  properties: {
    publicNetworkAccess: 'Enabled'
    administrators: {
      azureADOnlyAuthentication: true
      administratorType: 'ActiveDirectory'
      principalType: 'Application'
      sid: adminSid
      login: adminName
    }
    restrictOutboundNetworkAccess: 'Disabled'
    minimalTlsVersion: '1.2'
  }
  identity: {
    type: 'SystemAssigned'
  }
  location: location
}

EDIT: It seems it works sometimes. I can't really pinpoint what's happening. This might be related to time somehow. Perhaps chnaging the administrator takes some time and if deploying too soon after a change it fails?

mennolaan commented 1 year ago

Ah sorry for misreading.

One for deployment and the other one for changing / updating the AAD admin.

Did they share what a template would look like that could update this property? From what you are saying, I am assuming it is not possible to update this resource via Bicep/ARM Templates at all.

They did not share the template, but a link to SQL server administrators API (https://learn.microsoft.com/en-us/azure/templates/microsoft.sql/servers/administrators?pivots=deployment-language-bicep). Update works, you only have to use different template for that. The initial (deployment) template has to have AAD admin set as a property of "Microsoft.Sql/servers" resource. The "update" template would have to have "Microsoft.Sql/servers/administrators" resource.

@juho-turunen-tampuuri I've just tested deploying bicep template with User A as AAD admin, changing to user B via portal, changing back to User A via portal and then deploying the bicep template again. I've got no error messages. Maybe it's the API version? I'm using 'Microsoft.Sql/servers@2022-05-01-preview'

This would not be a solution. Scenario:

You do a check if sqlserver exist, Btw this can be done within bicep with existing and checking if the resource is null. i dont see why we need to use that whole powershell command to check if a resource exist.

You deploy initial sql server. Great

Week later you figure you need to change one parameter and redeploy. Won't work because the script detect sql exist and therefor it will not do an incremental update of the sql server.

jikuja commented 1 year ago

You do a check if sqlserver exist, Btw this can be done within bicep with existing and checking if the resource is null. i dont see why we need to use that whole powershell command to check if a resource exist.

Wiat what? Is that really a sipported feature?

mennolaan commented 1 year ago

You do a check if sqlserver exist, Btw this can be done within bicep with existing and checking if the resource is null. i dont see why we need to use that whole powershell command to check if a resource exist.

Wiat what? Is that really a sipported feature?

Yeah, resource sql_server 'Microsoft.Sql/servers@2022-05-01-preview' existing = { name: sql_server_name scope: reg_name }

module deploy_db './modules/sqldb.bicep' = if (sql_server ) {

{

see https://learn.microsoft.com/en-us/azure/azure-resource-manager/bicep/existing-resource

jikuja commented 1 year ago

https://learn.microsoft.com/en-us/azure/azure-resource-manager/bicep/existing-resource#troubleshooting

If you attempt to reference a resource that doesn't exist, you get the NotFound error and your deployment fails. Check the name and scope of the resource you're trying to reference.

--

Janne Kujanpää

From: Menno Laan @.> Reply to: Azure/bicep @.> Date: Tuesday 14. March 2023 at 11.09 To: Azure/bicep @.> Cc: Janne Kujanpää @.>, Mention @.***> Subject: Re: [Azure/bicep] Microsoft.Sql/servers not supporting changing AAD admin with templating (Issue #4988)

You do a check if sqlserver exist, Btw this can be done within bicep with existing and checking if the resource is null. i dont see why we need to use that whole powershell command to check if a resource exist.

Wiat what? Is that really a sipported feature?

Yeah, resource sql_server @.***' existing = { name: sql_server_name scope: reg_name }

module deploy_db './modules/sqldb.bicep' = if (sql_server ) {

{

see https://learn.microsoft.com/en-us/azure/azure-resource-manager/bicep/existing-resource

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: @.***>

mennolaan commented 1 year ago

https://learn.microsoft.com/en-us/azure/azure-resource-manager/bicep/existing-resource#troubleshooting

If you attempt to reference a resource that doesn't exist, you get the NotFound error and your deployment fails. Check the name and scope of the resource you're trying to reference. -- Janne Kujanpää From: Menno Laan @.> Reply to: Azure/bicep @.> Date: Tuesday 14. March 2023 at 11.09 To: Azure/bicep @.> Cc: Janne Kujanpää @.>, Mention @.> Subject: Re: [Azure/bicep] Microsoft.Sql/servers not supporting changing AAD admin with templating (Issue #4988) You do a check if sqlserver exist, Btw this can be done within bicep with existing and checking if the resource is null. i dont see why we need to use that whole powershell command to check if a resource exist. Wiat what? Is that really a sipported feature? Yeah, resource sql_server @.' existing = { name: sql_server_name scope: reg_name } module deploy_db './modules/sqldb.bicep' = if (sql_server ) { { see https://learn.microsoft.com/en-us/azure/azure-resource-manager/bicep/existing-resource — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: @.***>

Yes and no! let me explain.

If you do try to reference a resource that doesn't exist like they do with accessing a sub property, yes it fails! How ever if(sqlServe) is a bool return on the check. So yes and no, this wont fail.

But they created a solution for this! Please check this video, nullable types which makes it even easier to do so, if you re familiar with C# you would understand it quicker: sqlSever.?property_whatever. Hence the question mark, it is inline null check :) https://youtu.be/q23DDshzzvg?t=873

So in this case you can do an inline check to see if a value from slqExisting is null, if not you can access the value, otherwise use a different value

gnfsdybvik commented 1 year ago

Yeah, resource sql_server 'Microsoft.Sql/servers@2022-05-01-preview' existing = { name: sql_server_name scope: reg_name }

module deploy_db './modules/sqldb.bicep' = if (sql_server ) {

{

see https://learn.microsoft.com/en-us/azure/azure-resource-manager/bicep/existing-resource

Are you sure this works? It would have made my life easier.

But in the provided link it says "If you attempt to reference a resource that doesn't exist, you get the NotFound error and your deployment fails." And that is what I experience.

jikuja commented 1 year ago

FYI, some existing work:

mennolaan commented 1 year ago

Yeah, resource sql_server 'Microsoft.Sql/servers@2022-05-01-preview' existing = { name: sql_server_name scope: reg_name } module deploy_db './modules/sqldb.bicep' = if (sql_server ) { { see https://learn.microsoft.com/en-us/azure/azure-resource-manager/bicep/existing-resource

Are you sure this works? It would have made my life easier.

But in the provided link it says "If you attempt to reference a resource that doesn't exist, you get the NotFound error and your deployment fails." And that is what I experience.

That refers to the output. So yes if the resource doesn’t exist it will fail as the output can’t handle null reference

jr01 commented 8 months ago

This issue seems documented now. From https://learn.microsoft.com/en-us/azure/templates/microsoft.sql/2022-05-01-preview/servers?pivots=deployment-language-bicep

administrators: The Azure Active Directory administrator of the server. This can only be used at server create time. If used for server update, it will be ignored or it will result in an error. For updates individual APIs will need to be used.

This is the bicep we are using (might help someone):

// https://learn.microsoft.com/en-us/azure/templates/microsoft.sql/2022-05-01-preview/servers?pivots=deployment-language-bicep
resource sqlServerResource 'Microsoft.Sql/servers@2022-05-01-preview' = {
  location: location
  name: serverName
  properties: {
    minimalTlsVersion: '1.2'
    administratorLogin: 'sqladmin'
    // administratorLoginPassword: required for server creation - just set to dummy value, will be disabled by deploying '/azureADOnlyAuthentications' later.
    administratorLoginPassword: guid(serverName)
    // administrators: The Azure Active Directory administrator of the server. This can only be used at server create time. If used for server update, it will be ignored or it will result in an error. For updates individual APIs will need to be used. 
    administrators: {}
    version: '12.0'
  }
}

// https://learn.microsoft.com/en-us/azure/templates/microsoft.sql/servers/administrators?pivots=deployment-language-bicep
resource sqlAdminsResource 'Microsoft.Sql/servers/administrators@2022-05-01-preview' = {
  parent: sqlServerResource
  name: 'ActiveDirectory'
  properties: {
        administratorType: 'ActiveDirectory'
        login: 'managedIdentity.name' 
        sid: 'managedIdentity.properties.principalId'
        tenantId: 'managedIdentity.properties.tenantId'
    }
}

// https://learn.microsoft.com/en-us/azure/templates/microsoft.sql/servers/azureadonlyauthentications?pivots=deployment-language-bicep
resource sqlAzureAdOnly 'Microsoft.Sql/servers/azureADOnlyAuthentications@2022-05-01-preview' = {
  name: 'Default'
  parent: sqlServerResource
  properties: {
    azureADOnlyAuthentication: true
  }
  dependsOn:[sqlAdminsResource]
}
jikuja commented 8 months ago

This is the bicep we are using (might help someone):

Thanks.

Which use-cases work with that template:

All or only some?

juho-turunen-tampuuri commented 8 months ago

This is the bicep we are using (might help someone):

Thanks.

Which use-cases work with that template:

* Initial deployment

* re-deployment without changes

* re-deployment with changed `'Microsoft.Sql/servers/administrators` properties?

All or only some?

I'm not the author, but I would think that works for every situation.

But if the api changes in the future versions in a way that administrators-property is also respected in updates, this might cause an issue where administrator is removed and then re-added in the next step. This might go unnoticed for a long while, so you should check this every time you change to a newer api-version.

Seems unlikely that this behaviour would change, though.

At the moment we're checking if the server is allready deployed and have and condition in the bicep file for that. But that's a bit of a hassle and cant' be done in bicep alone. So maby we'll switch to jikuja's solution.

jr01 commented 8 months ago

This is the bicep we are using (might help someone):

Thanks.

Which use-cases work with that template:

  • Initial deployment
  • re-deployment without changes
  • re-deployment with changed 'Microsoft.Sql/servers/administrators properties?

All or only some?

Things I tested :

All had expected outcome.

jvandertil commented 6 months ago

This is the bicep we are using (might help someone):

Thanks. Which use-cases work with that template:

  • Initial deployment
  • re-deployment without changes
  • re-deployment with changed 'Microsoft.Sql/servers/administrators properties?

All or only some?

Things I tested :

  • initial deployment
  • redeployment w/o changes
  • disable azureADOnlyAuthentication in Azure Portal + redeploy bicep
  • remove Entra admin from Azure Portal + redeploy bicep
  • change Entra admin in bicep and redeploy

All had expected outcome.

Sad to report that this is not the case (anymore?), on initial deployment everything is fine. But on redeployment the deployment now fails with:

Azure Active Directory Only Authentication is enabled. Please contact your system administrator. (Code: AadOnlyAuthenticationIsEnabled)

Subsequent deploys of the template work when you set the administratorLogin and administratorLoginPassword properties to null, but then the initial deployment no longer works. Which loops back to having to detect whether this is a first deployment or not outside of the bicep file 😢

trisdrobinson commented 6 months ago

We've also just hit the same, that re-deployment w/o changes breaks the pipeline all of a sudden.