Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updating tags with az ad sp update runs into trace #23027

Closed
dmoessne opened this issue Jun 26, 2022 · 9 comments
Closed

Updating tags with az ad sp update runs into trace #23027

dmoessne opened this issue Jun 26, 2022 · 9 comments
Assignees
Labels
Account az login/account Auto-Assign Auto assign by bot customer-reported Issues that are reported by GitHub users external to the Azure organization. Graph az ad
Milestone

Comments

@dmoessne
Copy link

Describe the bug

Running az ad sp update --id $app_id --add tags WindowsAzureActiveDirectoryIntegratedApp runs into trace with cli version 2.37.0
(tested via rpm and container)

Command Name
az ad sp update --id $app_id --add tags WindowsAzureActiveDirectoryIntegratedApp

Errors:

The command failed with an unexpected error. Here is the traceback:
'GraphClient' object has no attribute 'service_principals'
Traceback (most recent call last):
  File "/usr/local/lib/python3.10/site-packages/knack/cli.py", line 231, in invoke
    cmd_result = self.invocation.execute(args)
  File "/usr/local/lib/python3.10/site-packages/azure/cli/core/commands/__init__.py", line 663, in execute
    raise ex
  File "/usr/local/lib/python3.10/site-packages/azure/cli/core/commands/__init__.py", line 726, in _run_jobs_serially
    results.append(self._run_job(expanded_arg, cmd_copy))
  File "/usr/local/lib/python3.10/site-packages/azure/cli/core/commands/__init__.py", line 718, in _run_job
    return cmd_copy.exception_handler(ex)
  File "/usr/local/lib/python3.10/site-packages/azure/cli/command_modules/role/commands.py", line 54, in graph_err_handler
    raise ex
  File "/usr/local/lib/python3.10/site-packages/azure/cli/core/commands/__init__.py", line 697, in _run_job
    result = cmd_copy(params)
  File "/usr/local/lib/python3.10/site-packages/azure/cli/core/commands/__init__.py", line 333, in __call__
    return self.handler(*args, **kwargs)
  File "/usr/local/lib/python3.10/site-packages/azure/cli/core/commands/command_operation.py", line 240, in handler
    result = cached_put(self.cmd, setter, **setterargs)
  File "/usr/local/lib/python3.10/site-packages/azure/cli/core/commands/__init__.py", line 452, in cached_put
    return _put_operation()
  File "/usr/local/lib/python3.10/site-packages/azure/cli/core/commands/__init__.py", line 446, in _put_operation
    result = operation(**kwargs)
  File "/usr/local/lib/python3.10/site-packages/azure/cli/command_modules/role/custom.py", line 988, in patch_service_principal
    object_id = _resolve_service_principal(graph_client.service_principals, identifier)
AttributeError: 'GraphClient' object has no attribute 'service_principals'

To Reproduce:

Steps to reproduce the behavior. Note that argument values have been redacted, as they may contain sensitive information.

  • Put any pre-requisite steps here...
  • DISPLAYNAME=my_SP_test01
  • app_id=$(az ad app list --display-name $DISPLAYNAME --query [].appId -o tsv)

Expected Behavior

Tags can either be added with the cli >= 2.37.0 and/or https://docs.microsoft.com/en-us/cli/azure/microsoft-graph-migration outlines alternative steps

Environment Summary

Linux-5.18.6-200.fc36.x86_64-x86_64-with, Alpine Linux v3.15
Python 3.10.4
Installer: DOCKER

azure-cli 2.37.0

Additional Context

This works still with az cli 2.35.0

podman run -it mcr.microsoft.com/azure-cli:2.35.0
bash-5.1# az version
{
  "azure-cli": "2.35.0",
  "azure-cli-core": "2.35.0",
  "azure-cli-telemetry": "1.0.6",
  "extensions": {}
}
bash-5.1# 
bash-5.1# az login
bash-5.1# DISPLAYNAME=my_SP_test01
bash-5.1# app_id=$(az ad app list --display-name $DISPLAYNAME --query [].appId -o tsv)
WARNING: The underlying Active Directory Graph API will be replaced by Microsoft Graph API in Azure CLI 2.37.0. Please carefully review all breaking changes introduced during this migration: https://docs.microsoft.com/cli/azure/microsoft-graph-migration
bash-5.1#  az ad sp update --id $app_id --add tags WindowsAzureActiveDirectoryIntegratedApp
The underlying Active Directory Graph API will be replaced by Microsoft Graph API in Azure CLI 2.37.0. Please carefully review all breaking changes introduced during this migration: https://docs.microsoft.com/cli/azure/microsoft-graph-migration
bash-5.1# 
bash-5.1# az ad sp show --id $app_id --query tags
The underlying Active Directory Graph API will be replaced by Microsoft Graph API in Azure CLI 2.37.0. Please carefully review all breaking changes introduced during this migration: https://docs.microsoft.com/cli/azure/microsoft-graph-migration
[
  "WindowsAzureActiveDirectoryIntegratedApp"
]
bash-5.1# 
@ghost ghost added customer-reported Issues that are reported by GitHub users external to the Azure organization. Auto-Assign Auto assign by bot Account az login/account labels Jun 26, 2022
@ghost ghost assigned jiasli Jun 26, 2022
@ghost ghost added this to the Backlog milestone Jun 26, 2022
@ghost ghost added the Graph az ad label Jun 26, 2022
@yonzhan
Copy link
Collaborator

yonzhan commented Jun 26, 2022

@jiasli for awareness

@jiasli
Copy link
Member

jiasli commented Jun 27, 2022

This is a known issue that generic update argument --add doesn't work with Microsoft Graph yet. In the next version 2.38.0, you may use --set to set tags property (#22798).

@dmoessne
Copy link
Author

This is a known issue that generic update argument --add doesn't work with Microsoft Graph yet. In the next version 2.38.0, you may use --set to set tags property (#22798).

Thanks for looking into it! Any ETA of 2.38.0 ?

@yonzhan
Copy link
Collaborator

yonzhan commented Jun 27, 2022

Official Release: 07/05/2022

@dmoessne
Copy link
Author

Official Release: 07/05/2022

Thanks, If you don't mind, I'd like to keep this open till 2.38.0 is released so I can test with the new version.

@SjoerdV
Copy link

SjoerdV commented Jul 11, 2022

with 2.38.0 in combination with PowerShell (pwsh) I only got it to work by feeding the command a minimized json file, with the Tags in a JSON array construct

[object]$TagConfig = @{tags=@("MyTag1","MyTag2")}
$TagConfigJson = (ConvertTo-Json $TagConfig.tags -Depth 10 -Compress) > ".temp-body-tags.json"
$mycommand = az ad sp update --id $spn.id --set tags="@.temp-body-tags.json" | ConvertFrom-Cli

The .temp-body-tags.json file looked like this:

["MyTag1","MyTag2"]

It did NOT work when:

  1. typing out the full JSON string manually in the final command (even when escaping all double quotes) (maybe just an issue with powershell, or some escaping I did not catch?)
  2. feeding the full JSON with the tags key in the final command $mycommand = az ad sp update --id $spn.id --set "@.temp-body-tags.json" | ConvertFrom-Cli. The .temp-body-tags.json file would have looked like this:
{"tags":["MyTag1","MyTag2"]}

This is probably why the release notes state:

Support generic update --set on root level -> so no full 'objects' yet

All in all a bit awkward, but it works. Tags key is filled now:

az ad sp show --id $app.appId

@jiasli
Copy link
Member

jiasli commented Jul 12, 2022

1. typing out the full JSON string manually in the final command (even when escaping all double quotes) (maybe just an issue with powershell, or some escaping I did not catch?)

For PowerShell quoting issue, you may refer to https://github.com/Azure/azure-cli/blob/dev/doc/quoting-issues-with-powershell.md. It described how to pass JSON strings.

2. feeding the full JSON with the tags key in the final command $mycommand = az ad sp update --id $spn.id --set "@.temp-body-tags.json" | ConvertFrom-Cli. The .temp-body-tags.json file would have looked like this:

As described in #22798, the supported syntax for --set is

az ad sp update --id 233dd73b-72e3-424a-9367-7588d957267e --set 'tags=["mytag"]'

If you want to directly use {"tags":["MyTag1","MyTag2"]} to PATCH the service principal, az rest will be the best choice. Please see #22580 (comment).

@dmoessne
Copy link
Author

So I can confirm this works with 2.38 for me as follows:

APPID=$(az ad app list --display-name $DISPLAYNAME --query [].appId -o tsv)
az ad sp update --id $APPID --set 'tags=["WindowsAzureActiveDirectoryIntegratedApp"]'

for 2.37 I used the following as a workaround

APP_ID=$(az ad app list --display-name $DISPLAYNAME --query [].id -o tsv)
az rest --method PATCH --url https://graph.microsoft.com/v1.0/applications/$APP_ID --body '{"tags":["WindowsAzureActiveDirectoryIntegratedApp"]}'

I am going ahead and close this issue as it seems to be resolved by 2.38

@dmoessne
Copy link
Author

I am going ahead and close this issue as it seems to be resolved by 2.38

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Account az login/account Auto-Assign Auto assign by bot customer-reported Issues that are reported by GitHub users external to the Azure organization. Graph az ad
Projects
None yet
Development

No branches or pull requests

4 participants