Azure / azure-powershell

Microsoft Azure PowerShell
Other
4.21k stars 3.81k forks source link

Connect-AzContainerRegistry ExposeToken parameter undocumented #25482

Open hilari0n opened 2 months ago

hilari0n commented 2 months ago

Type of issue

Missing information

Feedback

The documentation provides only a placeholder text for the parameter description: {{ Fill ExposeToken Description }} There's also no examples of use, no mention of a different output type of the command, when the parameter/switch is used.

As for the functionality itself (aside from the documentation issue aspect): It seems to be intended when we need to call the ACR REST API directly. Unfortunately Connect-AzContainerRegistry is very slow, so having to call it each time we need a fresh access token for a call, is suicide. There also seems to be no other support for doing it (e.g. something like Invoke-AzContainerRegistryApi), which would handle the authentication, the base endpoint address, etc. So if there's an ACR REST API, which is not covered by the Az.ContainerRegistry module, (or poorly covered, e.g. Get-AzContainerRegistryTag not allowing lazy paging), one has to implement everything again, even if it's implemented in Az.ContainerRegistry, (but not exposed). ... Or maybe there is something to support such scenario, but not documented?

Page URL

https://learn.microsoft.com/en-us/powershell/module/az.containerregistry/connect-azcontainerregistry?view=azps-12.0.0

Content source URL

https://github.com/Azure/azure-powershell/blob/main/src/ContainerRegistry/ContainerRegistry/help/Connect-AzContainerRegistry.md

Author

@mikefrobbins

Document Id

5976e2d9-6dec-dfb6-f759-db056dd77f0e

Nickcandy commented 1 month ago

@hilari0n Thank you for your suggestions and feedback.

Regarding the slow performance of Connect-AzContainerRegistry, we have tested it on our end and could not replicate the issue. Could you please provide us with the debug log information so that we can better understand and pinpoint the problem?

We acknowledge the documentation issues and will address them in our next release. The ExposeToken parameter is designed to expose an access token instead of logging in through the Docker CLI.

For the concerns about cmdlets covering the API poorly, we will discuss with the service team to see if we can enrich our cmdlets to make the functionality more comprehensive. We will also consider the possibility of creating a cmdlet similar to Invoke-AzContainerRegistryApi.

Thank you for your valuable suggestions.

hilari0n commented 1 month ago

You wrote, that:

The ExposeToken parameter is designed to expose an access token instead of logging in through the Docker CLI.

From looking at the code, it's not exposing the access token instead of logging in, but rather is exposing the access token after logging in. (Line 70 still executes the docker login script.) Which may be one of the reasons - if not the most important one - why it's so slow. (And also makes this call reliant on Docker CLI being installed, while I don't necessarily need it for doing the registry API calls.)

Also it seems to have an error of filling the returned data structure loginServer property/attribute with a concatenation of the registry name with a constant ".azurecr.io", which is correct for working with the global Azure Cloud, but not necessarily correct for e.g. one of the government or local variants of Azure. (E.g. for US government flavour of Azure, it should rather be ".azurecr.us", I think.) It should rather use the appropriate connection context's environment information (available e.g., from (Get-AzContext).Environment.ContainerRegistryEndpointSuffix). Edit: It's probably also available and retrieved a couple of lines above, by calling RegistryDataPlaneClient.GetEndPoint().

For the debug log: Connect-AzContainerRegistry_twice_log.txt

What I did there: I already have active connection (Connext-AzAccount). I actually have 3 contexts (Get-AzContext -ListAvailable returns 3 entries) and the current context (returned by Get-AzContext) is the only one appropriate for the Azure Container Registry (ACR) I'm working with. And that's my usual setup. (Which probably is not part of the issue, but I'm providing the info just in case.) Then - what is shown in the attached log - I'm calling this:

$DebugPreference = 'Continue'; $token = Connect-AzContainerRegistry -Name [redacted] -ExposeToken -Verbose; $DebugPreference = 'Ignore';

then I wait for a couple of seconds (about 12, to be more precise) and I'm calling the above once again. As you can see in the log (my local time zone is UTC+02:00), the first execution takes around 8 seconds and does a couple of HTTP calls underneath. The second call should be able to pull all necessary tokens from caches, as nothing should be expired yet, so should be much faster. And although the log for the second execution shows, that the amount of HTTP calls is lower, there still are some, and the process takes .... about the same amount of time, as the first one (about 8.5 seconds this time). Interestingly, most of that time seems to be spent after the last HTTP call, and I can confirm, that I get a long pause after this warning: WARNING: You can perform manual login using the provided access token, for example: 'docker login <loginServer> -u 00000000-0000-0000-0000-000000000000 -p <accessToken>' Which could prove, what I wrote above, that the problem is, that the command is not exposing the token instead of logging it, but does call the docker login and this is what is taking most of this time.

If I call commands like e.g. Get-AzContainerRegistryRepository -RegistryName [redacted], the debug log shows no calls for tokens - just calls to the ACR API - so evidently the module is capable of caching the tokens, just not with Connect-AzContainerRegistry.

So, if I would rely on Connect-AzContainerRegistry to get the tokens I need, to call the ACR API-s, my PowerShell code would spend most of the time waiting for this command, instead on actually doing its job. The issue would be less severe, if the returned data structure also contained the token's expiration date, as it would allow me to cache the token on my side for as long, as it's reusable, before calling Connect-AzContainerRegistry again, for a new one. Unfortunately the returned undocumented structure has useless status (which apparently is the output of the docker login script call) and basically also useless loginServer (which also may be wrong, as mentioned above), but no expiration date. Yes, as the token returned is a JWT token, I could work around this, by parsing the token and extracting the expiration date from it, but this is ridiculous, as the logs clearly show, that - as expected - the command has access to the expiration date.

Edit: Also the command passes a refresh token to the docker login script (where how this token is requested from the RegistryDataPlaneClient is probably why token cache is bypassed), while what it returns and suggests to use with docker login is an access token (which is taken from the token cache). Why the inconsistency?