CreateUpdateCustomRole returns nil after exhausting its retry budget, reporting success when the role was never updated
- Dominant language
- Go
- Stars
- 66
- Forks
- 11
- Avg merge
- 2d 4h
- Merged PRs (30d)
- 12
Description
### What happened
`CreateUpdateCustomRole` in [`pkg/infrastructure/spRoleAssignmentManager/defaultSPRoleAssignmentManager.go`](https://github.com/Azure/mpf/blob/main/pkg/infrastructure/spRoleAssignmentManager/defaultSPRoleAssignmentManager.go) retries a fixed number of times when Azure rejects an action with `InvalidActionOrNotAction`:
```go
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.
Contributor guide
No contributing guide indexed for this repository
Research direction
Start in pkg/infrastructure/spRoleAssignmentManager/defaultSPRoleAssignmentManager.go and trace CreateUpdateCustomRole into createUpdateCustomRole. Inspect the retry-budget path when every attempt returns InvalidActionOrNotAction, then check how the caller handles the returned error. Done means exhausting retries cannot report nil success, and the failure is visible to the caller.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- azure, go
- Domain
- cloud, security
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 76/100