fix: improve role detachment logic in DetachRolesFromSP method by DariuszPorowski · Pull Request #179 · Azure/mpf

DariuszPorowski · 2025-12-13T07:03:00Z

No description provided.

Signed-off-by: Dariusz Porowski <3431813+DariuszPorowski@users.noreply.github.com>

github-actions · 2025-12-13T07:03:11Z

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

Copilot

Pull request overview

This PR improves the role detachment logic in the DetachRolesFromSP method by refactoring the conditional checks and adding support for detaching all role assignments when no specific role is provided. The changes restructure the code to be more defensive against nil values and clarify the control flow.

Key Changes

Added nil safety check for roleAssignment.ID before processing
Restructured the conditional logic to support detaching all role assignments when RoleDefinitionResourceID is empty
Improved nil handling for roleAssignment.Properties and roleAssignment.Properties.RoleDefinitionID

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

…adjust .gitignore Signed-off-by: Dariusz Porowski <3431813+DariuszPorowski@users.noreply.github.com>

Signed-off-by: Dariusz Porowski <3431813+DariuszPorowski@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 9 out of 10 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Signed-off-by: Dariusz Porowski <3431813+DariuszPorowski@users.noreply.github.com>

…kflows Signed-off-by: Dariusz Porowski <3431813+DariuszPorowski@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 14 out of 15 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Signed-off-by: Dariusz Porowski <3431813+DariuszPorowski@users.noreply.github.com>

…container.json Signed-off-by: Dariusz Porowski <3431813+DariuszPorowski@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 15 out of 17 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Signed-off-by: Dariusz Porowski <3431813+DariuszPorowski@users.noreply.github.com>

test

Signed-off-by: Dariusz Porowski <3431813+DariuszPorowski@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 15 out of 17 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

fix: improve role detachment logic in DetachRolesFromSP method

caf2550

Signed-off-by: Dariusz Porowski <3431813+DariuszPorowski@users.noreply.github.com>

DariuszPorowski requested a review from a team as a code owner December 13, 2025 07:03

Copilot AI review requested due to automatic review settings December 13, 2025 07:03

Copilot started reviewing on behalf of DariuszPorowski December 13, 2025 07:03 View session

Copilot AI reviewed Dec 13, 2025

View reviewed changes

Comment thread pkg/infrastructure/spRoleAssignmentManager/defaultSPRoleAssignmentManager.go

Comment thread pkg/infrastructure/spRoleAssignmentManager/defaultSPRoleAssignmentManager.go

DariuszPorowski added this pull request to the merge queue Dec 13, 2025

github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Dec 13, 2025

DariuszPorowski added this pull request to the merge queue Dec 13, 2025

github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Dec 13, 2025

fix: update role detachment logic to remove all role assignments and …

1eee244

…adjust .gitignore Signed-off-by: Dariusz Porowski <3431813+DariuszPorowski@users.noreply.github.com>

DariuszPorowski added this pull request to the merge queue Dec 13, 2025

github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Dec 13, 2025

fix: update location to eastus2 in multiple configuration files

cf9d212

Signed-off-by: Dariusz Porowski <3431813+DariuszPorowski@users.noreply.github.com>

Copilot AI review requested due to automatic review settings December 13, 2025 07:53

Copilot started reviewing on behalf of DariuszPorowski December 13, 2025 07:54 View session

DariuszPorowski added this pull request to the merge queue Dec 13, 2025

Copilot AI reviewed Dec 13, 2025

View reviewed changes

Comment thread pkg/usecase/mpfService.go

github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Dec 13, 2025

fix: update expected permissions count for new region and VM SKU

6c2445f

Signed-off-by: Dariusz Porowski <3431813+DariuszPorowski@users.noreply.github.com>

DariuszPorowski force-pushed the fix-v2 branch from d039e86 to 6c2445f Compare December 13, 2025 08:27

DariuszPorowski added this pull request to the merge queue Dec 13, 2025

github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Dec 13, 2025

fix: increase timeout for e2e tests to 60 minutes across multiple wor…

b84bebc

…kflows Signed-off-by: Dariusz Porowski <3431813+DariuszPorowski@users.noreply.github.com>

Copilot AI review requested due to automatic review settings December 13, 2025 16:11

Copilot started reviewing on behalf of DariuszPorowski December 13, 2025 16:11 View session

Copilot AI reviewed Dec 13, 2025

View reviewed changes

Comment thread ...nfrastructure/authorizationCheckers/ARMTemplateDeployment/armTemplateAuthorizationChecker.go

Comment thread pkg/infrastructure/spRoleAssignmentManager/defaultSPRoleAssignmentManager.go

Comment thread e2eTests/e2eArm_test.go

DariuszPorowski added 2 commits December 17, 2025 05:08

fix: add newline at end of launch.json file

578c7aa

Signed-off-by: Dariusz Porowski <3431813+DariuszPorowski@users.noreply.github.com>

fix: update versions for azure-cli, terraform, and go features in dev…

73b3fc3

…container.json Signed-off-by: Dariusz Porowski <3431813+DariuszPorowski@users.noreply.github.com>

DariuszPorowski added this pull request to the merge queue Dec 17, 2025

Copilot AI review requested due to automatic review settings December 17, 2025 08:32

DariuszPorowski enabled auto-merge December 17, 2025 08:32

Copilot started reviewing on behalf of DariuszPorowski December 17, 2025 08:32 View session

DariuszPorowski added this pull request to the merge queue Dec 17, 2025

Copilot AI reviewed Dec 17, 2025

View reviewed changes

Comment thread e2eTests/e2eArm_test.go

Comment thread pkg/infrastructure/spRoleAssignmentManager/defaultSPRoleAssignmentManager.go

Comment thread Taskfile.yml

github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Dec 17, 2025

fix: downgrade terraform version to 1.13.5 in workflows and devcontainer

eb44e2a

Signed-off-by: Dariusz Porowski <3431813+DariuszPorowski@users.noreply.github.com>

DariuszPorowski added this pull request to the merge queue Dec 17, 2025

github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Dec 17, 2025

maniSbindra previously requested changes Dec 17, 2025

View reviewed changes

Comment thread e2eTests/e2eArm_test.go

DariuszPorowski enabled auto-merge December 17, 2025 17:07

DariuszPorowski disabled auto-merge December 17, 2025 17:33

DariuszPorowski requested a review from maniSbindra December 17, 2025 17:33

DariuszPorowski enabled auto-merge December 17, 2025 17:33

DariuszPorowski added this pull request to the merge queue Dec 17, 2025

github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Dec 17, 2025

DariuszPorowski added this pull request to the merge queue Dec 17, 2025

github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Dec 17, 2025

DariuszPorowski added this pull request to the merge queue Dec 17, 2025

github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Dec 17, 2025

fix: downgrade terraform version to 1.10.4 in devcontainer and workflows

5602765

Signed-off-by: Dariusz Porowski <3431813+DariuszPorowski@users.noreply.github.com>

Copilot AI review requested due to automatic review settings December 17, 2025 18:32

Copilot started reviewing on behalf of DariuszPorowski December 17, 2025 18:32 View session

Copilot AI reviewed Dec 17, 2025

View reviewed changes

Comment thread ...nfrastructure/authorizationCheckers/ARMTemplateDeployment/armTemplateAuthorizationChecker.go

Comment thread e2eTests/e2eArm_test.go

DariuszPorowski added this pull request to the merge queue Dec 17, 2025

Merged via the queue into main with commit 299d0d5 Dec 17, 2025
20 checks passed

DariuszPorowski deleted the fix-v2 branch December 17, 2025 19:24

maniSbindra mentioned this pull request Jan 9, 2026

Validate minimum required permissions by content instead of count in TestARMTemplatMultiResourceTemplateFullDeployment** #199

Closed

Conversation

DariuszPorowski commented Dec 13, 2025

Uh oh!

github-actions Bot commented Dec 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Dependency Review

Scanned Files

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Key Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions Bot commented Dec 13, 2025 •

edited

Loading