Fix #26857: separate stdout from stderr in AzureCliCredential (#26953) (#27076)

* Fix #26857: separate stdout from stderr in AzureCliCredential

* Fix unit tests for AzureCliCredential

* Fix #26857 for aio.AzureCliCredential

* azure_cli.py: Fix types in _run_command

* Fix test_cli_credential_async.py

* Update sdk/identity/azure-identity/CHANGELOG.md

Co-authored-by: Paul Van Eck <[email protected]>

Co-authored-by: Tingmao Wang <[email protected]>
Co-authored-by: Paul Van Eck <[email protected]>
Co-authored-by: Xiang Yan <[email protected]>

Co-authored-by: maowtm <[email protected]>
Co-authored-by: Tingmao Wang <[email protected]>
Co-authored-by: Paul Van Eck <[email protected]>

Author: 3 people

sdk/identity/azure-identity/CHANGELOG.md

@@ -4,6 +4,7 @@
 
 ### Bugs Fixed
 
+- `AzureCliCredential` now works even when `az` prints warnings to stderr. ([#26857](https://github.com/Azure/azure-sdk-for-python/issues/26857))
 - Fixed issue where user-supplied `TokenCachePersistenceOptions` weren't propagated when using `SharedTokenCacheCredential` ([#26982](https://github.com/Azure/azure-sdk-for-python/issues/26982))
 
 ### Breaking Changes

sdk/identity/azure-identity/azure/identity/_credentials/azure_cli.py

@@ -143,7 +143,7 @@ def _run_command(command):
         working_directory = get_safe_working_dir()
 
         kwargs = {
-            "stderr": subprocess.STDOUT,
+            "stderr": subprocess.PIPE,
             "cwd": working_directory,
             "universal_newlines": True,
             "env": dict(os.environ, AZURE_CORE_NO_COLOR="true"),
@@ -154,14 +154,14 @@ def _run_command(command):
         return subprocess.check_output(args, **kwargs)
     except subprocess.CalledProcessError as ex:
         # non-zero return from shell
-        if ex.returncode == 127 or ex.output.startswith("'az' is not recognized"):
+        if ex.returncode == 127 or ex.stderr.startswith("'az' is not recognized"):
             raise CredentialUnavailableError(message=CLI_NOT_FOUND)
-        if "az login" in ex.output or "az account set" in ex.output:
+        if "az login" in ex.stderr or "az account set" in ex.stderr:
             raise CredentialUnavailableError(message=NOT_LOGGED_IN)
 
         # return code is from the CLI -> propagate its output
-        if ex.output:
-            message = sanitize_output(ex.output)
+        if ex.stderr:
+            message = sanitize_output(ex.stderr)
         else:
             message = "Failed to invoke Azure CLI"
         raise ClientAuthenticationError(message=message)

sdk/identity/azure-identity/azure/identity/aio/_credentials/azure_cli.py

@@ -100,12 +100,13 @@ async def _run_command(command: str) -> str:
         proc = await asyncio.create_subprocess_exec(
             *args,
             stdout=asyncio.subprocess.PIPE,
-            stderr=asyncio.subprocess.STDOUT,
+            stderr=asyncio.subprocess.PIPE,
             cwd=working_directory,
             env=dict(os.environ, AZURE_CORE_NO_COLOR="true")
         )
-        stdout, _ = await asyncio.wait_for(proc.communicate(), 10)
-        output = stdout.decode()
+        stdout_b, stderr_b = await asyncio.wait_for(proc.communicate(), 10)
+        output = stdout_b.decode()
+        stderr = stderr_b.decode()
     except OSError as ex:
         # failed to execute 'cmd' or '/bin/sh'; CLI may or may not be installed
         error = CredentialUnavailableError(message="Failed to execute '{}'".format(args[0]))
@@ -117,11 +118,11 @@ async def _run_command(command: str) -> str:
     if proc.returncode == 0:
         return output
 
-    if proc.returncode == 127 or output.startswith("'az' is not recognized"):
+    if proc.returncode == 127 or stderr.startswith("'az' is not recognized"):
         raise CredentialUnavailableError(CLI_NOT_FOUND)
 
-    if "az login" in output or "az account set" in output:
+    if "az login" in stderr or "az account set" in stderr:
         raise CredentialUnavailableError(message=NOT_LOGGED_IN)
 
-    message = sanitize_output(output) if output else "Failed to invoke Azure CLI"
+    message = sanitize_output(stderr) if stderr else "Failed to invoke Azure CLI"
     raise ClientAuthenticationError(message=message)

sdk/identity/azure-identity/tests/test_cli_credential.py

@@ -31,8 +31,8 @@
 )
 
 
-def raise_called_process_error(return_code, output, cmd="..."):
-    error = subprocess.CalledProcessError(return_code, cmd=cmd, output=output)
+def raise_called_process_error(return_code, output="", cmd="...", stderr=""):
+    error = subprocess.CalledProcessError(return_code, cmd=cmd, output=output, stderr=stderr)
     return mock.Mock(side_effect=error)
 
 
@@ -76,17 +76,17 @@ def test_get_token():
 def test_cli_not_installed_linux():
     """The credential should raise CredentialUnavailableError when the CLI isn't installed"""
 
-    output = "/bin/sh: 1: az: not found"
-    with mock.patch(CHECK_OUTPUT, raise_called_process_error(127, output)):
+    stderr = "/bin/sh: 1: az: not found"
+    with mock.patch(CHECK_OUTPUT, raise_called_process_error(127, stderr=stderr)):
         with pytest.raises(CredentialUnavailableError, match=CLI_NOT_FOUND):
             AzureCliCredential().get_token("scope")
 
 
 def test_cli_not_installed_windows():
     """The credential should raise CredentialUnavailableError when the CLI isn't installed"""
 
-    output = "'az' is not recognized as an internal or external command, operable program or batch file."
-    with mock.patch(CHECK_OUTPUT, raise_called_process_error(1, output)):
+    stderr = "'az' is not recognized as an internal or external command, operable program or batch file."
+    with mock.patch(CHECK_OUTPUT, raise_called_process_error(1, stderr=stderr)):
         with pytest.raises(CredentialUnavailableError, match=CLI_NOT_FOUND):
             AzureCliCredential().get_token("scope")
 
@@ -102,18 +102,18 @@ def test_cannot_execute_shell():
 def test_not_logged_in():
     """When the CLI isn't logged in, the credential should raise CredentialUnavailableError"""
 
-    output = "ERROR: Please run 'az login' to setup account."
-    with mock.patch(CHECK_OUTPUT, raise_called_process_error(1, output)):
+    stderr = "ERROR: Please run 'az login' to setup account."
+    with mock.patch(CHECK_OUTPUT, raise_called_process_error(1, stderr=stderr)):
         with pytest.raises(CredentialUnavailableError, match=NOT_LOGGED_IN):
             AzureCliCredential().get_token("scope")
 
 
 def test_unexpected_error():
     """When the CLI returns an unexpected error, the credential should raise an error containing the CLI's output"""
 
-    output = "something went wrong"
-    with mock.patch(CHECK_OUTPUT, raise_called_process_error(42, output)):
-        with pytest.raises(ClientAuthenticationError, match=output):
+    stderr = "something went wrong"
+    with mock.patch(CHECK_OUTPUT, raise_called_process_error(42, stderr=stderr)):
+        with pytest.raises(ClientAuthenticationError, match=stderr):
             AzureCliCredential().get_token("scope")
 
 
@@ -181,7 +181,7 @@ def fake_check_output(command_line, **_):
 
         token = AzureCliCredential(tenant_id=second_tenant).get_token("scope")
         assert token.token == second_token
-        
+
 def test_multitenant_authentication():
     default_tenant = "first-tenant"
     first_token = "***"

sdk/identity/azure-identity/tests/test_cli_credential_async.py

@@ -101,8 +101,8 @@ async def test_get_token():
 async def test_cli_not_installed_linux():
     """The credential should raise CredentialUnavailableError when the CLI isn't installed"""
 
-    output = "/bin/sh: 1: az: not found"
-    with mock.patch(SUBPROCESS_EXEC, mock_exec(output, return_code=127)):
+    stderr = "/bin/sh: 1: az: not found"
+    with mock.patch(SUBPROCESS_EXEC, mock_exec("", stderr, return_code=127)):
         with pytest.raises(CredentialUnavailableError, match=CLI_NOT_FOUND):
             credential = AzureCliCredential()
             await credential.get_token("scope")
@@ -111,8 +111,8 @@ async def test_cli_not_installed_linux():
 async def test_cli_not_installed_windows():
     """The credential should raise CredentialUnavailableError when the CLI isn't installed"""
 
-    output = "'az' is not recognized as an internal or external command, operable program or batch file."
-    with mock.patch(SUBPROCESS_EXEC, mock_exec(output, return_code=1)):
+    stderr = "'az' is not recognized as an internal or external command, operable program or batch file."
+    with mock.patch(SUBPROCESS_EXEC, mock_exec("", stderr, return_code=1)):
         with pytest.raises(CredentialUnavailableError, match=CLI_NOT_FOUND):
             credential = AzureCliCredential()
             await credential.get_token("scope")
@@ -130,8 +130,8 @@ async def test_cannot_execute_shell():
 async def test_not_logged_in():
     """When the CLI isn't logged in, the credential should raise CredentialUnavailableError"""
 
-    output = "ERROR: Please run 'az login' to setup account."
-    with mock.patch(SUBPROCESS_EXEC, mock_exec(output, return_code=1)):
+    stderr = "ERROR: Please run 'az login' to setup account."
+    with mock.patch(SUBPROCESS_EXEC, mock_exec("", stderr, return_code=1)):
         with pytest.raises(CredentialUnavailableError, match=NOT_LOGGED_IN):
             credential = AzureCliCredential()
             await credential.get_token("scope")
@@ -140,9 +140,9 @@ async def test_not_logged_in():
 async def test_unexpected_error():
     """When the CLI returns an unexpected error, the credential should raise an error containing the CLI's output"""
 
-    output = "something went wrong"
-    with mock.patch(SUBPROCESS_EXEC, mock_exec(output, return_code=42)):
-        with pytest.raises(ClientAuthenticationError, match=output):
+    stderr = "something went wrong"
+    with mock.patch(SUBPROCESS_EXEC, mock_exec("", stderr, return_code=42)):
+        with pytest.raises(ClientAuthenticationError, match=stderr):
             credential = AzureCliCredential()
             await credential.get_token("scope")