Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Dynamic validation for workload identity permissions and requirements #3619

Merged
merged 14 commits into from
Sep 10, 2024
Merged

Dynamic validation for workload identity permissions and requirements #3619

merged 14 commits into from
Sep 10, 2024

Conversation

rajdeepc2792
Copy link
Collaborator

@rajdeepc2792 rajdeepc2792 commented Jun 6, 2024 •
edited
Loading

Which issue this PR addresses:

JIRA: ARO-4376

What this PR does / why we need it:

For the MIWI Cluster feature, this PR performs dynamic validation for the Platform Workload Identities and Cluster MSI.
Uses CheckAccessV2 to validate the actions for all the Platform Workload Identities and Cluster MSI.

During Cluster Creation:-

  • Fetch PlatformWorkloadIdentityRoleSet for the requested OCP version.
  • Match the count and OperatorNames associated with customer provided Platform Workload Identities and fetched PlatformWorkloadIdentityRoleSet.
  • Check permissions on the network resources for all the Platform Workload Identities using CheckAccessV2
  • Check the count of Cluster MSI(User Assigned Identity), it should be one.
  • Check the permission/actions(RoleAzureRedHatOpenShiftFederatedCredentialRole) on Platform Workload Identity for Cluster MSI.

Test plan for issue:

  • Unit Tests
  • CI
  • CI e2e
  • Local cluster installation

Is there any documentation that needs to be updated for this PR?

No

How do you know this will function as expected in production?

None of the environment should have an impact due to this change as the additional validation will only happen for MIWI clusters.

@rajdeepc2792 rajdeepc2792 marked this pull request as draft June 6, 2024 19:29
@rajdeepc2792 rajdeepc2792 self-assigned this Jun 6, 2024
@rajdeepc2792 rajdeepc2792 added work-in-progress chainsaw Pull requests or issues owned by Team Chainsaw labels Jun 6, 2024
@rajdeepc2792
Copy link
Collaborator Author

/azp run ci,e2e

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@github-actions github-actions bot added the needs-rebase branch needs a rebase label Jun 7, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented Jun 7, 2024

Please rebase pull request.

@github-actions github-actions bot removed the needs-rebase branch needs a rebase label Jun 12, 2024
@rajdeepc2792 rajdeepc2792 force-pushed the rajdeepc2792/ARO-4376 branch 2 times, most recently from 072ad43 to 71a3159 Compare June 13, 2024 21:56
@github-actions GitHub Actions
Copy link

Please rebase pull request.

SudoBrendan
SudoBrendan requested changes Sep 5, 2024
View reviewed changes
Copy link
Collaborator

@SudoBrendan SudoBrendan left a comment

Choose a reason for hiding this comment

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

.bingo has merged - please rebase this PR on master and fix the generate cases.

@rajdeepc2792
ARO-4376 Track2 authorization api addition for roledefinitions
09c1b76
@rajdeepc2792
ARO-4376 add a stringutil funcs
8c54456
@rajdeepc2792
ARO-4376 use dbPlatformWorkloadIdentityRoleSets to get platform ident…
9a0aa7c 
…ity roles for cluster version
@rajdeepc2792
ARO-4376 add dynamic validation for platformworkloadidentityprofile
e49d5ba
@rajdeepc2792
ARO-4376 resolve initial comments
71a1abc
@rajdeepc2792
ARO-4376 refactor error messages and checkaccess action crosscheck
126069c
@rajdeepc2792
ARO-4376 Add unit tests and comments resolution
afc766b
@rajdeepc2792
ARO-4376 add validation for upgradeableTo
5186f2d
@rajdeepc2792
ARO-4376 Comment resoultion and additional unit tests
ba157ea
@rajdeepc2792
ARO-4376 minor version comparison handling
1caba5e
@rajdeepc2792
ARO-4376 update permission error messaging handling for MIWI
735b54b
@rajdeepc2792
ARO-4376 update constructors to return non-interface type
c1a70c7
@rajdeepc2792
ARO-4376 add unit tests for GroupsIntersect
7e3c02f
@rajdeepc2792
ARO-4376 update generate files to support bingo
8086154
@rajdeepc2792
Copy link
Collaborator Author

/azp run ci,e2e

1 similar comment
@tsatam
Copy link
Collaborator

tsatam commented Sep 5, 2024

/azp run ci,e2e

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

1 similar comment
@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

SudoBrendan
SudoBrendan approved these changes Sep 9, 2024
View reviewed changes
Copy link
Collaborator

@SudoBrendan SudoBrendan left a comment

Choose a reason for hiding this comment

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

I think all my comments have been addressed :)

@tsatam tsatam merged commit 66f073f into master Sep 10, 2024
24 checks passed
@rajdeepc2792 rajdeepc2792 mentioned this pull request Sep 11, 2024
Merged
edisonLcardenas pushed a commit that referenced this pull request Sep 16, 2024
@rajdeepc2792 @edisonLcardenas
Dynamic validation for workload identity permissions and requirements (…
e664b34 
…#3619)

* ARO-4376 Track2 authorization api addition for roledefinitions

* ARO-4376 add a stringutil funcs

* ARO-4376 use dbPlatformWorkloadIdentityRoleSets to get platform identity roles for cluster version

* ARO-4376 add dynamic validation for platformworkloadidentityprofile

* ARO-4376 resolve initial comments

* ARO-4376 refactor error messages and checkaccess action crosscheck

* ARO-4376 Add unit tests and comments resolution

* ARO-4376 add validation for upgradeableTo

* ARO-4376 Comment resoultion and additional unit tests

* ARO-4376 minor version comparison handling

* ARO-4376 update permission error messaging handling for MIWI

* ARO-4376 update constructors to return non-interface type

* ARO-4376 add unit tests for GroupsIntersect

* ARO-4376 update generate files to support bingo
edisonLcardenas pushed a commit that referenced this pull request Sep 17, 2024
@rajdeepc2792 @edisonLcardenas
Dynamic validation for workload identity permissions and requirements (…
6a73b0e 
…#3619)

* ARO-4376 Track2 authorization api addition for roledefinitions

* ARO-4376 add a stringutil funcs

* ARO-4376 use dbPlatformWorkloadIdentityRoleSets to get platform identity roles for cluster version

* ARO-4376 add dynamic validation for platformworkloadidentityprofile

* ARO-4376 resolve initial comments

* ARO-4376 refactor error messages and checkaccess action crosscheck

* ARO-4376 Add unit tests and comments resolution

* ARO-4376 add validation for upgradeableTo

* ARO-4376 Comment resoultion and additional unit tests

* ARO-4376 minor version comparison handling

* ARO-4376 update permission error messaging handling for MIWI

* ARO-4376 update constructors to return non-interface type

* ARO-4376 add unit tests for GroupsIntersect

* ARO-4376 update generate files to support bingo
@rajdeepc2792 rajdeepc2792 mentioned this pull request Sep 25, 2024
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hlipsig hlipsig hlipsig left review comments

@niontive niontive niontive left review comments

@SudoBrendan SudoBrendan SudoBrendan approved these changes

@cadenmarchese cadenmarchese cadenmarchese approved these changes

@kimorris27 kimorris27 kimorris27 approved these changes

@jewzaam jewzaam Awaiting requested review from jewzaam jewzaam is a code owner

@hawkowl hawkowl Awaiting requested review from hawkowl hawkowl is a code owner

@rogbas rogbas Awaiting requested review from rogbas rogbas is a code owner

@petrkotas petrkotas Awaiting requested review from petrkotas petrkotas is a code owner

@jharrington22 jharrington22 Awaiting requested review from jharrington22 jharrington22 is a code owner

@cblecker cblecker Awaiting requested review from cblecker cblecker is a code owner

@UlrichSchlueter UlrichSchlueter Awaiting requested review from UlrichSchlueter UlrichSchlueter is a code owner

@Shivkumar13 Shivkumar13 Awaiting requested review from Shivkumar13

@yjst2012 yjst2012 Awaiting requested review from yjst2012 yjst2012 is a code owner

@jaitaiwan jaitaiwan Awaiting requested review from jaitaiwan jaitaiwan is a code owner

@anshulvermapatel anshulvermapatel Awaiting requested review from anshulvermapatel anshulvermapatel is a code owner

@tiguelu tiguelu Awaiting requested review from tiguelu tiguelu is a code owner

@SrinivasAtmakuri SrinivasAtmakuri Awaiting requested review from SrinivasAtmakuri SrinivasAtmakuri is a code owner

@mociarain mociarain Awaiting requested review from mociarain mociarain is a code owner

@AldoFusterTurpin AldoFusterTurpin Awaiting requested review from AldoFusterTurpin

@tsatam tsatam Awaiting requested review from tsatam tsatam is a code owner

@bitoku bitoku Awaiting requested review from bitoku bitoku is a code owner

@bennerv bennerv Awaiting requested review from bennerv bennerv is a code owner

Assignees

@rajdeepc2792 rajdeepc2792

Labels
chainsaw Pull requests or issues owned by Team Chainsaw ready-for-review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@rajdeepc2792 @tsatam @hlipsig @bennerv @SudoBrendan @niontive @cadenmarchese @kimorris27