Skip to content

[AZINTS-4454] Don't log expected az role assignment list --assignee error#155

Merged
mshvartsberg merged 4 commits into
mainfrom
meggan.shvartsberg/azints-4454/role-exists-error
Mar 30, 2026
Merged

[AZINTS-4454] Don't log expected az role assignment list --assignee error#155
mshvartsberg merged 4 commits into
mainfrom
meggan.shvartsberg/azints-4454/role-exists-error

Conversation

@mshvartsberg
Copy link
Copy Markdown
Contributor

Motivation

az role assignment list --assignee often fails right after a principal is created because Microsoft Graph has not resolved the assignee yet. That shows up as "Cannot find user or service principal in graph database for '<principal_id>'. The installer already treats a failed existence check as “no assignment” and continues with role assignment create, so this case is expected and non-blocking.
We still logged it at ERROR in two places: execute() always logged failed CLI stderr before raising, and role_exists logged again on catch. This would show an error to the user in the cloud shell / terminal for a non-blocking, benign error. We want to stop this error log.

Changes

  • execute_cmd.py: If stderr contains the Graph assignee-resolution message, log the failure at DEBUG instead of ERROR, still raise RuntimeError (callers unchanged).
  • role_setup.py: In role_exists, skip log.error when the caught exception matches the expected Graph related message and includes the same principal_id; always return False from the handler so assignment creation still runs.

Testing

  • Added unit test test_execute_role_list_graph_assignee_logs_debug_not_error in test_execute_cmd.py
  • E2E tested with the add log forwarder flow and confirmed the previous error logs for this Graph message no longer appear.
  • E2E tested with the modify log forwarder flow and no error messages appeared (although I think they were only showing up for add flow beforehand anyway).

@mshvartsberg mshvartsberg requested a review from a team as a code owner March 24, 2026 20:58
@mshvartsberg mshvartsberg self-assigned this Mar 24, 2026
@mshvartsberg mshvartsberg force-pushed the meggan.shvartsberg/azints-4406/removing-lfo-scopes branch from 33b3348 to 195f9c1 Compare March 26, 2026 18:39
@parsons90 parsons90 removed their request for review March 27, 2026 14:32
@mshvartsberg mshvartsberg force-pushed the meggan.shvartsberg/azints-4454/role-exists-error branch from f4c786f to 9807b12 Compare March 27, 2026 15:28
Base automatically changed from meggan.shvartsberg/azints-4406/removing-lfo-scopes to main March 27, 2026 18:46
Copy link
Copy Markdown
Collaborator

@benjjs benjjs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise looks good

log.error(f"Failed to check if role assignment exists: {e}")
# Graph lag on list --assignee; omit error log (create path still runs).
if not (
"Cannot find user or service principal in graph database" in str(e) and principal_id in str(e)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should import this string from az_shared, mostly to make sure there's a single source of truth in case we need to change it in future.

@mshvartsberg mshvartsberg merged commit b0615c4 into main Mar 30, 2026
1 check passed
@mshvartsberg mshvartsberg deleted the meggan.shvartsberg/azints-4454/role-exists-error branch March 30, 2026 16:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants