Skip to content

CreateUpdateCustomRole returns nil after exhausting its retry budget, reporting success when the role was never updated #313

Description

What happened

CreateUpdateCustomRole in pkg/infrastructure/spRoleAssignmentManager/defaultSPRoleAssignmentManager.go retries a fixed number of times when Azure rejects an action with InvalidActionOrNotAction:

func (r *SPRoleAssignmentManager) CreateUpdateCustomRole(subscription string, role domain.Role, permissions []string) (error, []string) {
	retryCount := 5
	permissionsToAdd := permissions
	var invalidActions []string
	for i := range retryCount {
		err := r.createUpdateCustomRole(subscription, role, permissionsToAdd)
		if err != nil && strings.Contains(err.Error(), "InvalidActionOrNotAction") {
			// parse the invalid action, record it, drop it, retry
			continue
		}
		if err != nil {
			return err, []string{}
		}
		log.Infof("Role definition created/updated successfully")
		break
	}
	return nil, invalidActions // reached even when every attempt failed
}

Azure reports invalid actions one at a time, so each rejected action costs one retry. If a single call submits more rejected actions than the retry budget, the loop runs out of attempts and falls through to return nil, invalidActions.

The caller cannot distinguish this from success: the role was never updated, but the returned error is nil.

Why it matters

The discovery loop keeps iterating against a role that no longer reflects the permissions MPF believes it granted. The run either takes far longer than it should or reports a permission set that was never actually validated against Azure, with nothing in the output indicating anything went wrong. The only visible symptom is a warning log line for the removed actions.

Evidence

Observed while testing #62 against a live subscription with a multi provider Terraform sample. MPF appends a RESOURCE_TYPE/operationStatuses/read candidate per discovered write permission, and most providers do not expose that action, so five candidates were rejected in a single update:

  • 28 discovery iterations (a comparable run normally takes 6-10)
  • 127 InvalidActionOrNotAction error occurred events
  • only 9 Role definition created/updated successfully events
  • the removal batch size was 5 — the full retry budget — on 21 separate occasions

The run had to be killed; it was making no progress. Every one of those 21 calls returned nil.

To be clear about attribution: #62 made this easy to hit, and the resubmission of already rejected candidates was fixed there. This issue is about the underlying behaviour, which is independent of #62 — any caller that submits more than retryCount invalid actions in one update hits it.

Suggested fix

  1. Return an explicit error when the retry budget is exhausted rather than falling through to nil, so callers can fail loudly instead of continuing against a stale role.
  2. Consider sizing the budget against the number of submitted permissions instead of a fixed 5, since the number of possible rejections scales with the request.

Item 1 is the important one. A silent nil on a failed role update is the part that makes this hard to diagnose.

Version

main as of 2026-07-27.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions