azure-sdk-for-net icon indicating copy to clipboard operation
azure-sdk-for-net copied to clipboard

[BUG] ContainerGroupImageRegistryCredential requires username

Open mburumaxwell opened this issue 2 years ago • 1 comments
trafficstars

Library name and version

Azure.ResourceManager.ContainerInstance 1.0.0

Describe the bug

Adding to ImageRegistryCredentials when creating a ContainerGroupResource, the type required ContainerGroupImageRegistryCredential requires a username to be supplied even when working with managed identity. Supplying null does not work because of the guard clause.

https://github.com/Azure/azure-sdk-for-net/blob/7cf0c4fd21d3d537aa05b83fa62c0b4431b08b92/sdk/containerinstance/Azure.ResourceManager.ContainerInstance/src/Generated/Models/ContainerGroupImageRegistryCredential.cs#L20-L28

Expected behavior

When working with managed identity the only server and managedIdentityId properties should be required hence the username argument in the constructor should be removed.

This should work:

new ContainerGroupImageRegistryCredential("contoso.azurecr.io")
{
  Identity = "/subscriptions/00000000-0000-1111-0001-000000000000/resourceGroups/CONTOSO/providers/Microsoft.ManagedIdentity/userAssignedIdentities/contoso",
}

Actual behavior

Cannot create a ContainerGroupResource with ImageRegistryCredentials using a managed identity; an exception is thrown.

Reproduction Steps

The following code does not work:

new ContainerGroupImageRegistryCredential("contoso.azurecr.io", null)
{
  Identity = "/subscriptions/00000000-0000-1111-0001-000000000000/resourceGroups/CONTOSO/providers/Microsoft.ManagedIdentity/userAssignedIdentities/contoso",
}

Environment

No response

mburumaxwell avatar Jan 10 '23 15:01 mburumaxwell

Thank you for your feedback. Tagging and routing to the team member best able to assist.

jsquire avatar Jan 10 '23 15:01 jsquire

Yes, it is confirmed that this is a swagger spec issue. As this has been fixed in swagger, I opened https://github.com/Azure/azure-sdk-for-net/pull/33410 to fix this in SDK side, will release a new version once this PR is merged.

Yao725 avatar Jan 11 '23 07:01 Yao725

Thanks @Yao725 Looking forward to the fixed release

mburumaxwell avatar Jan 11 '23 07:01 mburumaxwell

Hi @mburumaxwell, we have released a new version for Azure.ResourceManager.ContainerInstance to fix this issue, please have a try.

By the way, if you have any other feedback to our Azure SDK, feel free to let us know in this survey

Yao725 avatar Jan 12 '23 02:01 Yao725

@Yao725 I can confirm that it is fixed. Thanks

mburumaxwell avatar Jan 12 '23 16:01 mburumaxwell