feat: suggest Azure built-in roles for required permissions (#36) - #300
feat: suggest Azure built-in roles for required permissions (#36)#300Brian Gordon Davis (bgdnext64) wants to merge 3 commits into
Conversation
Add a --suggestRoles flag that, after MPF computes the minimum permissions, queries Azure built-in role definitions and suggests role(s) covering them. - domain: SuggestBuiltInRoles with wildcard-aware matching, NotActions handling, least-privilege breadth-score ranking, and greedy minimal set cover - infrastructure: RoleDefinitionManager fetches built-in roles via RoleDefinitionsClient - presentation: text and JSON formatting of suggestions - wired into arm, bicep, and terraform commands - unit tests for matching/ranking and formatter; opt-in read-only Azure integration test
There was a problem hiding this comment.
Pull request overview
Adds an opt-in “built-in role suggestion” feature to MPF that, after discovering minimum required permissions, fetches Azure built-in role definitions and suggests least-privilege role(s) (single-role matches, greedy combinations, and uncovered permissions) in text or JSON form.
Changes:
- Introduces domain logic to match required permissions to built-in roles (wildcard + NotActions support) and rank suggestions by specificity, with unit tests.
- Adds infrastructure for enumerating Azure built-in role definitions and wires role suggestion into ARM/Bicep/Terraform commands behind
--suggestRoles. - Adds presentation formatting (text + JSON) and documents the new CLI flag/environment variable.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/usecase/roleSuggester.go | Defines BuiltInRoleProvider interface used for role enumeration. |
| pkg/domain/roleSuggestion.go | Implements role matching, greedy set cover, and specificity scoring. |
| pkg/domain/roleSuggestion_test.go | Unit tests for wildcard matching, ordering, combinations, and edge cases. |
| pkg/infrastructure/azureAPI/azureApiClient.go | Adds RoleDefinitionsClient initialization for role enumeration. |
| pkg/infrastructure/roleDefinitionManager/roleDefinitionManager.go | Implements built-in role listing and conversion into domain model. |
| pkg/infrastructure/roleDefinitionManager/roleDefinitionManager_integration_test.go | Read-only integration test for listing built-in roles and running suggestion. |
| pkg/presentation/roleSuggestionFormatter.go | Adds text/JSON formatting for role suggestions. |
| pkg/presentation/roleSuggestionFormatter_test.go | Tests for text output cases, JSON output, and single-match capping. |
| cmd/rootCmd.go | Adds --suggestRoles persistent flag. |
| cmd/roleSuggestion.go | Implements end-to-end “fetch → suggest → display” flow for role suggestions. |
| cmd/armCmd.go | Calls role suggestion after displaying ARM MPF results. |
| cmd/bicepCmd.go | Calls role suggestion after displaying Bicep MPF results. |
| cmd/terraformCmd.go | Calls role suggestion after displaying Terraform MPF results. |
| docs/commandline-flags-and-env-variables.md | Documents --suggestRoles / MPF_SUGGESTROLES and behavior. |
| suggestion := domain.SuggestBuiltInRoles(requiredPermissions, builtInRoles) | ||
|
|
||
| if err := presentation.DisplayRoleSuggestion(os.Stdout, suggestion, flgJSONOutput); err != nil { | ||
| log.Errorf("Error displaying role suggestion: %v", err) | ||
| } |
There was a problem hiding this comment.
Good catch, this was a real bug. Stdout is now a single JSON object shaped as requiredPermissions plus roleSuggestion, rather than two documents written one after the other. I took the approach you suggested: the suggestion function now returns the result instead of writing to stdout itself, and a single display function decides whether to emit the combined JSON document or the existing text output. When suggestRoles is not set the JSON output is unchanged, so existing consumers are unaffected. I verified this against a live subscription with jsonOutput and suggestRoles together, and the output parses as exactly one JSON document with nothing trailing. There is also a unit test that decodes the document and then asserts the decoder is at end of input.
| func actionMatchesPattern(pattern string, action string) bool { | ||
| if pattern == "" { | ||
| return false | ||
| } | ||
| if !strings.Contains(pattern, "*") { | ||
| return strings.EqualFold(pattern, action) | ||
| } | ||
|
|
||
| var sb strings.Builder | ||
| sb.WriteString("(?i)^") | ||
| for _, segment := range strings.Split(pattern, "*") { | ||
| sb.WriteString(regexp.QuoteMeta(segment)) | ||
| sb.WriteString(".*") | ||
| } | ||
| // Remove the trailing ".*" added after the final segment and anchor the end. | ||
| regexStr := strings.TrimSuffix(sb.String(), ".*") + "$" | ||
|
|
||
| re, err := regexp.Compile(regexStr) | ||
| if err != nil { | ||
| return false | ||
| } | ||
| return re.MatchString(action) | ||
| } |
There was a problem hiding this comment.
Agreed, thank you. Wildcard matching now short circuits the global wildcard, uses a case insensitive string comparison for the common trailing wildcard form such as Microsoft.Storage slash star, and memoises the compiled expression for any remaining pattern so a given pattern is compiled at most once. I added test cases covering case insensitivity, an action shorter than the prefix, a near miss where Microsoft.StorageSync must not match Microsoft.Storage, and patterns containing more than one wildcard. This path was also exercised against the full set of Azure built in roles during a live run.
Address PR review feedback: combine required permissions and role suggestion into one JSON object when --suggestRoles is used with --jsonOutput, and memoise compiled wildcard patterns with fast paths for prefix matches.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
cmd/roleSuggestion.go:57
- If printing the role suggestion fails (e.g., broken pipe / write error), this currently only logs and continues. Since this is stdout output, failing to write should exit non-zero so callers can detect it.
if err := presentation.DisplayRoleSuggestion(os.Stdout, suggestion, displayOptions.JSONOutput); err != nil {
log.Errorf("Error displaying role suggestion: %v", err)
}
| if ok && displayOptions.JSONOutput { | ||
| if err := presentation.DisplayCombinedJSON(os.Stdout, mpfResult, suggestion); err != nil { | ||
| log.Errorf("Error displaying result: %v", err) | ||
| } | ||
| return | ||
| } |
Summary
Implements #36 — suggest Azure built-in role(s) that cover the minimum permissions discovered by MPF.
After MPF determines the minimum required permissions for a deployment, this feature optionally matches those permissions against the subscription's Azure built-in role definitions and prints:
The feature is opt-in via a new global flag
--suggestRoles(envMPF_SUGGESTROLES) and works across thearm,bicep, andterraformsubcommands. Output honors the existing--jsonOutputflag.Behavior
*, e.g. Owner/Contributor) are deprioritized and rank last.Microsoft.Storage/*) are matched case-insensitively against required permissions, andNotActionsexclusions are respected.Changes
pkg/domain/roleSuggestion.go— pure role-matching and ranking logic (wildcard matching, greedy set cover, specificity scoring) with unit tests.pkg/usecase/roleSuggester.go—BuiltInRoleProviderinterface.pkg/infrastructure/roleDefinitionManager/— fetches built-in role definitions viaRoleDefinitionsClient, with an opt-in read-only integration test.pkg/infrastructure/azureAPI/azureApiClient.go— adds and initializesRoleDefinitionsClient.pkg/presentation/roleSuggestionFormatter.go— text and JSON output, with tests.cmd/—--suggestRolesflag and wiring into the arm/bicep/terraform commands.docs/commandline-flags-and-env-variables.md— documents the new flag.Testing
go build ./...,go vet ./..., and the full unit suite pass.Closes #36