Azure/azure-sdk-for-python

Azure CLI will *always* cause AzureCliCredential to fail on unupgraded Python/MacOS/Homebrew builds using secret-client.

Closed this issue · 4 comments

  • Package Names:
    azure-common==1.1.28
    azure-core==1.28.0
    azure-identity==1.13.0
    azure-keyvault==4.2.0
    azure-keyvault-certificates==4.7.0
    azure-keyvault-keys==4.8.0
    azure-keyvault-secrets==4.7.0
  • Operating System: MacOS
  • Python Version: 3.10 (I know this isn't supported, but it makes no difference here)

Describe the bug
DefaultAzureCredential runs through a bunch of options, including AzureCliCredential.

When it reaches this line within AzureCliCredential, it will always fail the timeout with an outdated software version, as the cli prompt returned by az account get-access-token --output json --resource <whatever> embedded within that page will always return the following which requires a user response:

New Azure CLI version available. Running 'az upgrade' to update automatically.
This command is in preview and under development. Reference and support levels: https://aka.ms/CLI_refstatus
Your current Azure CLI version is 2.50.0. Latest version available is 2.51.0.
Please check the release notes first: https://docs.microsoft.com/cli/azure/release-notes-azure-cli
Do you want to continue? (Y/n):

To Reproduce
Steps to reproduce the behavior:

  1. Replace key_vault_url and secret_name within x/function_app.py with some legitimate values.
  2. Use func start to start
  3. Optionally, attach a debugger of your choice to the process and watch the call to return subprocess.check_output(args, **kwargs) venv/lib/python3.10/site-packages/azure/identity/_credentials/azure_cli.py - github link to specific line
  4. Run curl http://localhost:7071/api/hello to trigger the http function
  5. Read your logs for the following string: AzureCliCredential: Failed to invoke the Azure CLI

Expected behavior
Standard auth flow to work in the usual way.

Screenshots
Attached debugger showing the point where the timeout occurs:
image

Additional context
This may well be intended behaviour, so I apologise if so. However, I am currently unable to upgrade Azure CLI locally due to Homebrew/homebrew-core#138158 & Azure/azure-cli#27047 so I did a bit of digging.
I presume this probably hits in other areas than just my specific area of focus so it may be worth passing this on to other devs.

mccoyp commented

Hi @furnivall, thank you for opening an issue! I'll tag some folks who can take a look and get back to you as soon as possible.

jiasli commented

See Azure/azure-cli#27111 (comment) for more details.

Automatic Update in Azure CLI is indeed guarded by a verify_is_a_tty check:

https://github.com/Azure/azure-cli/blob/2c3f5e099ccf7074dd21ec7b20620f9b5b933495/src/azure-cli/azure/cli/__main__.py#L97-L104

It's better for Python SDK to make sure stdin is not connected to az when subprocessing az:

kwargs: Dict[str, Any] = {
"stderr": subprocess.PIPE,
"cwd": working_directory,
"universal_newlines": True,
"timeout": timeout,
"env": dict(os.environ, AZURE_CORE_NO_COLOR="true"),
}
return subprocess.check_output(args, **kwargs)

https://docs.python.org/3/library/subprocess.html#frequently-used-arguments

With the default settings of None, no redirection will occur; the child’s file handles will be inherited from the parent.

Thanks all for the feedback and investigation, @furnivall, @jiasli. I agree that stdin should be ignored for all credentials that spawn subprocesses in order to avoid potential hangs/timeouts. I'll have a PR for this shortly.

Thanks all for the feedback and investigation, @furnivall, @jiasli. I agree that stdin should be ignored for all credentials that spawn subprocesses in order to avoid potential hangs/timeouts. I'll have a PR for this shortly.

No problem at all, was fun digging into the issue - learned a lot in the process.

Also @jiasli seriously well done for spotting that subprocess architecture quirk! Would never have imagined a core python module like subprocess would have such different behaviour across architectures.