azure-cli icon indicating copy to clipboard operation
azure-cli copied to clipboard

Release of MSI version 2022-01-31-preview

Open isolenov opened this issue 3 years ago • 13 comments

Related command

az identity

Resource Provider

Microsoft.ManagedIdentity

Description of Feature or Work Requested

MSI has added a new resource Federated Identity Credentials (FIC) as a sub-resource of existing User Assigned Identities "userAssignedIdentities/federatedIdentityCredentials"

CLI is expected to provide CRUD operation for the given sub-resource. Take a look Request example.

high-level documentation: https://aka.ms/ami/wif/docs

FIC object looks like this:

{
        "issuer": "https://oidc.prod-aks.azure.com/IssuerGUID",
        "subject": "system:serviceaccount:ns:svcaccount",
        "audiences": ["api://AzureADTokenExchange"],
}

In AAD context Audiences array is supposed to have exactly one element in most of the cases. Is it possible to make "audiences": ["api://AzureADTokenExchange"] a default value with the ability to override?

Minimum API Version Required

2022-01-31-preview

Swagger PR link

https://github.com/Azure/azure-rest-api-specs/pull/19548

Request Example

Existing command to create managed identity

az identity create --name $uaId --resource-group $rg --location $location --subscription $subscription

then this managed identity can be used for FIC CRUD operations below. Those 4 operations are expected to become available in CLI.

create/update FIC

az rest --method put `
--url "/subscriptions/$subscription/resourceGroups/$rg/providers/Microsoft.ManagedIdentity/userAssignedIdentities/$uaId/federatedIdentityCredentials/$ficId?api-version=2022-01-31-PREVIEW" `
--headers "Content-Type=application/json" `
--body "{'properties': { 'issuer': 'https://kubernetes-oauth.azure.com/', 'subject': 'system:serviceaccount:ns:svcaccount', 'audiences': ['api://AzureADTokenExchange'] }}"

read FIC

az rest --method get --url "/subscriptions/$subscription/resourceGroups/$rg/providers/Microsoft.ManagedIdentity/userAssignedIdentities/$uaId/federatedIdentityCredentials/$ficId?api-version=2022-01-31-PREVIEW"

read all FICs associated with the user-assigned identity

supports paging

az rest --method get --url "/subscriptions/$subscription/resourceGroups/$rg/providers/Microsoft.ManagedIdentity/userAssignedIdentities/$uaId/federatedIdentityCredentials?api-version=2022-01-31-PREVIEW"

delete fic

az rest --method delete --url "/subscriptions/$subscription/resourceGroups/$rg/providers/Microsoft.ManagedIdentity/userAssignedIdentities/$uaId/federatedIdentityCredentials/$ficId?api-version=2022-01-31-PREVIEW"

Target Date

July

Additional context

pypi link: https://pypi.org/project/azure-mgmt-msi/6.1.0/ Given API version "2022-01-31-PREVIEW" is available in public cloud, mooncake, fairfax, but not air-gapped clouds

isolenov avatar Jul 08 '22 11:07 isolenov

@zhoxing for awareness

yonzhan avatar Jul 08 '22 12:07 yonzhan

Hey, @yonzhan please advise what is the timeline for this request?

isolenov avatar Jul 11 '22 19:07 isolenov

@isolenov Since the work of this sprint is fully scheduled, so we may not have enough time to develop this requirement in this sprint. We plan to release this feature in the next sprint (09-06). May I ask does this time meet your expectations?

zhoxing-ms avatar Jul 12 '22 01:07 zhoxing-ms

@zhoxing-ms thanks for update. it would be great to have some kind of internal preview version before this date to make sure it looks like expected

isolenov avatar Jul 12 '22 09:07 isolenov

@isolenov OK, we will generate a private package for you to experience and test in advance through edge build before official release

zhoxing-ms avatar Jul 12 '22 10:07 zhoxing-ms

Hello @isolenov, could you please help to confirm below design for this cli command? To be consistent with the naming convention of other commands in the cli, we refer to the naming convention of az ad app federated-credential.

create FIC: az identity federated-credential create --federated-credential-name myFicName --name myIdentityName --resource-group myResourceGroup --issuer myIssuer --subject mySubject --audiences myAudiences

update FIC: az identity federated-credential update --federated-credential-name myFicName --name myIdentityName --resource-group myResourceGroup --issuer myIssuer --subject mySubject --audiences myAudiences

read FIC: az identity federated-credential show --federated-credential-name myFicName --name myIdentityName --resource-group myResourceGroup

read all FICs associated with the user-assigned identity: az identity federated-credential list --name myIdentityName --resource-group myResourceGroup

delete FIC: az identity federated-credential delete --federated-credential-name myFicName --name myIdentityName --resource-group myResourceGroup

yanzhudd avatar Aug 02 '22 08:08 yanzhudd

Hey @yanzhudd thank you for providing CLI commands.

Have one proposal from the team. In our impression '--name' is quite confusing in this context. User is creating federated-credential object and name should refer to this object name.

Would it be possible to change commands this way?

az identity federated-credential --name myFicName  --identity-name myIdentityName 

isolenov avatar Aug 02 '22 09:08 isolenov

Hey @yanzhudd thank you for providing CLI commands.

Have one proposal from the team. In our impression '--name' is quite confusing in this context. User is creating federated-credential object and name should refer to this object name.

Would it be possible to change commands this way?

az identity federated-credential --name myFicName  --identity-name myIdentityName 

okay, looks great!

yanzhudd avatar Aug 02 '22 11:08 yanzhudd

awesome. thanks.

isolenov avatar Aug 02 '22 13:08 isolenov

Hello @isolenov, when we develop this feature, an error 'Request contains additional data that the service does not understand' is reported. The details are attached below. Could you please help to take a look?

Request URL: 'https://management.azure.com/subscriptions/0b1f6471-1bf0-4dda-aec3-cb9272f09590/resourceGroups/zytest/providers/Microsoft.ManagedIdentity/userAssignedIdentities/ide/federatedIdentityCredentials/fic?api-version=2022-01-31-preview' Request body: { "properties": { "audiences": ["api://AzureADTokenExchange"] } } x-ms-request-id: '37218ee2-4b4a-49b1-9f9e-8ec0941b9324' x-ms-correlation-request-id: '37218ee2-4b4a-49b1-9f9e-8ec0941b9324' x-ms-routing-request-id: 'SOUTHEASTASIA:20220803T101847Z:37218ee2-4b4a-49b1-9f9e-8ec0941b9324' image

yanzhudd avatar Aug 03 '22 10:08 yanzhudd

this basically means that body cant be deserialized. it should not be "properties": {} object inside

body should look like this

{ "issuer": "https://oidc.prod-aks.azure.com/IssuerGUID", "subject": "system:serviceaccount:ns:svcaccount", "audiences": ["api://AzureADTokenExchange"], }

isolenov avatar Aug 09 '22 10:08 isolenov

this basically means that body cant be deserialized. it should not be "properties": {} object inside

body should look like this

{ "issuer": "https://oidc.prod-aks.azure.com/IssuerGUID", "subject": "system:serviceaccount:ns:svcaccount", "audiences": ["api://AzureADTokenExchange"], }

Okay, thanks. We will try it.

yanzhudd avatar Aug 09 '22 10:08 yanzhudd

this basically means that body cant be deserialized. it should not be "properties": {} object inside

body should look like this

{ "issuer": "https://oidc.prod-aks.azure.com/IssuerGUID", "subject": "system:serviceaccount:ns:svcaccount", "audiences": ["api://AzureADTokenExchange"], }

Hi @isolenov, when our request body contains all three parameters (issuer, subject, audiences) and not remove the 'peoperties' object, the error is not reported (The request and response are attached below). It seems that only when we pass one or two parameters, the error is reported. So we thought it may not be deserialization causing the problem, could you please help to check it again? image image

yanzhudd avatar Aug 10 '22 08:08 yanzhudd

this basically means that body cant be deserialized. it should not be "properties": {} object inside body should look like this { "issuer": "https://oidc.prod-aks.azure.com/IssuerGUID", "subject": "system:serviceaccount:ns:svcaccount", "audiences": ["api://AzureADTokenExchange"], }

Hi @isolenov, when our request body contains all three parameters (issuer, subject, audiences) and not remove the 'peoperties' object, the error is not reported (The request and response are attached below). It seems that only when we pass one or two parameters, the error is reported. So we thought it may not be deserialization causing the problem, could you please help to check it again? image image

Hello @isolenov, since this has blocked our development progress, please check the reason causing this problem and reply to me before Aug 22th. Otherwise, this sprint will not have enough time for us to develop and test, so we will have to postpone it to the next sprint for Ignite Event (10-12). Thanks for understanding.

yanzhudd avatar Aug 15 '22 02:08 yanzhudd

@yanzhudd, sorry for the late response, just returned back from vacation.

Results look good now. "properties" object is expected in body as it is a part if ARM resource definition, please, have a look on samples in preview docs

All 3 parameters (issuer, subject, audiences) are mandatory that's why creation is failing when anything is missing. So, the CLI looks as expected.

This item is for Ignite as well. Kubernetes team is planning to announce FIC feature public preview on the event ant this can't happen without the CLI. Please let me know if this can be released.

isolenov avatar Aug 25 '22 15:08 isolenov

@yanzhudd, sorry for the late response, just returned back from vacation.

Results look good now. "properties" object is expected in body as it is a part if ARM resource definition, please, have a look on samples in preview docs

All 3 parameters (issuer, subject, audiences) are mandatory that's why creation is failing when anything is missing. So, the CLI looks as expected.

This item is for Ignite as well. Kubernetes team is planning to announce FIC feature public preview on the event ant this can't happen without the CLI. Please let me know if this can be released.

Hello @isolenov, Since the error message Request contains additional data that the service does not understand may confuse user why the creation is failing, we propose to add a check logic in our side, i.e. if any parameters are missing we will report RequiredArgumentMissingError error. Could you please help to confirm this?

yanzhudd avatar Aug 26 '22 02:08 yanzhudd

@yanzhudd "Request contains additional data that the service does not understand" appears when "issuer" or "subject" is missing. This is a bug on server side which is hiding more meaningful error message. This will be fixed on our side eventually. Since all 3 properties are mandatory, please add a check on az cli side: "issuer" - mandatory, not empty string "subject" - mandatory, not empty string "audiences" - mandatory, not empty array (array with at least one element)

isolenov avatar Aug 26 '22 08:08 isolenov

@yanzhudd "Request contains additional data that the service does not understand" appears when "issuer" or "subject" is missing. This is a bug on server side which is hiding more meaningful error message. This will be fixed on our side eventually. Since all 3 properties are mandatory, please add a check on az cli side: "issuer" - mandatory, not empty string "subject" - mandatory, not empty string "audiences" - mandatory, not empty array (array with at least one element)

Since a default value of "audiences" is specified above (https://github.com/Azure/azure-cli/issues/23152#issue-1298911431):

Is it possible to make "audiences": ["api://AzureADTokenExchange"] a default value with the ability to override?

may I ask if the parameter "audiences" also need to be checked?

yanzhudd avatar Aug 26 '22 08:08 yanzhudd

@yanzhudd for the sake of consistency "audiences" should be checked as well. "audiences" should have a default value of ["api://AzureADTokenExchange"] when overridden by the user let's check that it is not empty array (array with at least one element)

isolenov avatar Aug 26 '22 09:08 isolenov

@yanzhudd for the sake of consistency "audiences" should be checked as well. "audiences" should have a default value of ["api://AzureADTokenExchange"] when overridden by the user let's check that it is not empty array (array with at least one element)

Is that the rule for parameter "audiences" check is as follows?

  • If the user does not pass this parameter, no error is reported. The default value ["api://AzureADTokenExchange"] is used.
  • if the user passes this parameter, we check the size of the array.

yanzhudd avatar Aug 26 '22 09:08 yanzhudd

@yanzhudd this is correct. thank you

isolenov avatar Aug 26 '22 09:08 isolenov