Azure / azure-rest-api-specs

The source for REST API specifications for Microsoft Azure.
MIT License
2.68k stars 5.1k forks source link

Azure SQL Server: External AAD Administrator has incorrect mutability #20156

Open xenon8 opened 2 years ago

xenon8 commented 2 years ago

The spec for Microsoft.Sql/servers lists read and create under x-ms-mutability.

API does support updates through this endpoint, since assigning an External Active Directory user is possible through Azure PowerShell and the Azure Portal without re-creating the resource.

Related Issue

Pulumi issue also related.

This issue is also probably the cause of this issue

ghost commented 2 years ago

Thanks for the feedback! We are routing this to the appropriate team for follow-up. cc @azureSQLGitHub.

Issue Details
The [spec for Microsoft.Sql/servers](https://github.com/Azure/azure-rest-api-specs/blob/main/specification/sql/resource-manager/Microsoft.Sql/preview/2022-02-01-preview/Servers.json#L971-L978) lists read and create under x-ms-mutability. API does support updates through this endpoint, since assigning an External Active Directory user is possible through [Azure PowerShell](https://docs.microsoft.com/en-us/powershell/module/az.sql/set-azsqlinstanceactivedirectoryadministrator?view=azps-8.2.0) and the Azure Portal without re-creating the resource. Related [Issue](https://github.com/Azure/azure-rest-api-specs/issues/16803) Pulumi [issue](https://github.com/pulumi/pulumi-azure-native/issues/1738) also related. This issue is also probably the cause of this [issue](https://github.com/Azure/azure-rest-api-specs/issues/16423)
Author: xenon8
Assignees: -
Labels: `question`, `SQL`, `Service Attention`, `customer-reported`, `CXP Attention`
Milestone: -
navba-MSFT commented 2 years ago

Removing the CXP attention label since the Service team has been involved already.

@azureSQLGitHub Could you please look into this and provide an update ?

darena-patrick commented 2 years ago

Can we please get an update on this one?

nofield commented 1 year ago

Thanks for your feedback. The mutability statement is accurate for the Server API- updates to the Azure AD Admin are not currently supported there. However, we support updates to the Azure AD Admin through the ServerAzureADAdministrators API: azure-rest-api-specs/ServerAzureADAdministrators.json.

We've made updates to the administrators property description in recent API releases to clarify this behavior:

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.

We don't support updates to the Azure AD Admin through the Server API due to a difference in RBAC permission requirements. The permission model is SQL DB contributor for create server, and SQL Security Manager for updating the Azure AD Only property, and we decided to maintain consistency in API behavior between the two Azure AD properties.

We are revisiting this design to determine if we should expose the Azure AD Admin property through the Server API.

xenon8 commented 1 year ago

The problem with this stance is that it makes it a lot harder to utilise Infrastructure as Code.

If another user updates the property, our next run of the IaC should revert it, to keep things as designed.

By making it impossible to update, the IaC does a replace, deleting the database, which is less than ideal.

In short, please can you do this: "expose the Azure AD Admin property through the Server API"

thecodetinker commented 1 year ago

@nofield thanks for this - so I think the answer here is to ignore the AAD Administrator field on the SQL Server, and instead utilise the discrete API which Pulumi does have a separate resource for:

https://www.pulumi.com/registry/packages/azure-native/api-docs/sql/serverazureadadministrator/

I think this will solve the issue.

nofield commented 1 year ago

@thecodetinker Happy to help! If you have any follow up questions about the discrete APIs we expose, please do share.

@xenon8 We really appreciate this feedback! It's a great help for us in prioritizing our work and ensuring we provide the best experience for our developers. I'll provide an update when I have more to share on this specific ask.