OCPBUGS-81749: Gate Install button and operand Create button on RBAC#16614
OCPBUGS-81749: Gate Install button and operand Create button on RBAC#16614sampadasawant-36 wants to merge 1 commit into
Conversation
Follow-up to openshift#16554 covering two gaps from the original bug description: - Disable the Install button on the Operator Hub subscribe form when the user lacks 'create' on subscriptions.operators.coreos.com, using the same useAccessReview pattern already in the file for canPatchConsoleOperatorConfig. - Pass createAccessReview to ListPageCreateLink in DefaultProvidedAPIPage (operand list page) so RequireCreatePermission hides the Create button when the user lacks 'create' on the operand resource kind. Adds unit tests for both behaviors: - operator-hub-subscribe.spec.tsx: Install button disabled when canCreateSubscription is false - operand/index.spec.tsx: Create button hidden when useAccessReviewAllowed returns false Relates-to: https://redhat.atlassian.net/browse/OCPBUGS-81749 Co-authored-by: Cursor <cursoragent@cursor.com>
openshift-ci-robot
added
jira/valid-reference
|
@sampadasawant-36: This pull request references Jira Issue OCPBUGS-81749, which is valid. 3 validation(s) were run on this bug
The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: sampadasawant-36 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
WalkthroughAdds RBAC permission checks to operator creation flows. ChangesRBAC Permission Controls for Creation Flows
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
openshift-ci
Bot
added
the
needs-ok-to-test
|
Hi @sampadasawant-36. Thanks for your PR. I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@frontend/packages/operator-lifecycle-manager/src/components/operator-hub/__tests__/operator-hub-subscribe.spec.tsx`:
- Around line 147-150: Mocks for the curried APIs used by
OperatorHubSubscribePage are returning non-curried values; update
providedAPIsForChannel and referenceForProvidedAPI to return functions (e.g.,
change providedAPIsForChannel: jest.fn(() => []) to providedAPIsForChannel:
jest.fn(() => () => []) and referenceForProvidedAPI: jest.fn(() =>
'test~v1~Kind') to referenceForProvidedAPI: jest.fn(() => () =>
'test~v1~Kind')), and apply the same curry-fix to any other similar mocks around
the 153-154 area so their shapes match the component’s curried calls.
In
`@frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-hub-subscribe.tsx`:
- Around line 237-241: The RBAC check using useAccessReview for
canCreateSubscription currently omits the namespace, causing false negatives for
users with namespace-scoped create permission; update the useAccessReview call
(the canCreateSubscription check) to include the target namespace (the same
namespace used for subscription.metadata.namespace) so the access review is
performed against that namespace (and apply the same fix where useAccessReview
is used again for SubscriptionModel elsewhere in this file).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 3a99315f-f66a-42b3-a57f-fc307c8ec3f2
📒 Files selected for processing (4)
frontend/packages/operator-lifecycle-manager/src/components/operand/__tests__/index.spec.tsxfrontend/packages/operator-lifecycle-manager/src/components/operand/index.tsxfrontend/packages/operator-lifecycle-manager/src/components/operator-hub/__tests__/operator-hub-subscribe.spec.tsxfrontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-hub-subscribe.tsx
| providedAPIsForChannel: jest.fn(() => []), | ||
| referenceForProvidedAPI: jest.fn(() => 'test~v1~Kind'), | ||
| supportedInstallModesFor: jest.fn(() => () => true), | ||
| })); |
There was a problem hiding this comment.
Fix mocked function shapes to match the component’s curried API contracts.
At Lines 147-150 and 153-154, several mocks return non-curried values, but OperatorHubSubscribePage calls these APIs as chained/curried functions. This can cause runtime TypeError during render and make the RBAC assertions non-representative.
Suggested fix
+import { InstallModeType } from '../../../types';
@@
jest.mock('../../index', () => ({
defaultChannelNameFor: jest.fn((pkg) => pkg?.status?.defaultChannel || null),
getManualSubscriptionsInNamespace: jest.fn(() => []),
iconFor: jest.fn(() => null),
NamespaceIncludesManualApproval: jest.fn(() => null),
- providedAPIsForChannel: jest.fn(() => []),
+ providedAPIsForChannel: jest.fn(() => () => []),
referenceForProvidedAPI: jest.fn(() => 'test~v1~Kind'),
- supportedInstallModesFor: jest.fn(() => () => true),
+ supportedInstallModesFor: jest.fn(() => () => [
+ { type: InstallModeType.InstallModeTypeOwnNamespace, supported: true },
+ { type: InstallModeType.InstallModeTypeAllNamespaces, supported: true },
+ ]),
}));
@@
jest.mock('../../operator-group', () => ({
- installedFor: jest.fn(() => () => false),
+ installedFor: jest.fn(() => () => () => () => false),
supports: jest.fn(() => () => true),
providedAPIsForOperatorGroup: jest.fn(() => []),
isGlobal: jest.fn(() => false),
}));Also applies to: 153-154
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@frontend/packages/operator-lifecycle-manager/src/components/operator-hub/__tests__/operator-hub-subscribe.spec.tsx`
around lines 147 - 150, Mocks for the curried APIs used by
OperatorHubSubscribePage are returning non-curried values; update
providedAPIsForChannel and referenceForProvidedAPI to return functions (e.g.,
change providedAPIsForChannel: jest.fn(() => []) to providedAPIsForChannel:
jest.fn(() => () => []) and referenceForProvidedAPI: jest.fn(() =>
'test~v1~Kind') to referenceForProvidedAPI: jest.fn(() => () =>
'test~v1~Kind')), and apply the same curry-fix to any other similar mocks around
the 153-154 area so their shapes match the component’s curried calls.
| const canCreateSubscription = useAccessReview({ | ||
| group: SubscriptionModel.apiGroup, | ||
| resource: SubscriptionModel.plural, | ||
| verb: 'create', | ||
| }); |
There was a problem hiding this comment.
Scope the Subscription access review to the selected namespace.
At Line 237, the RBAC check omits namespace, but the actual create operation is namespace-scoped (subscription.metadata.namespace at Line 480). This can disable Install for users who have create on subscriptions only in specific namespaces.
Suggested fix
- const canCreateSubscription = useAccessReview({
- group: SubscriptionModel.apiGroup,
- resource: SubscriptionModel.plural,
- verb: 'create',
- });
+ // Place this after `selectedTargetNamespace` is derived
+ const canCreateSubscription = useAccessReview({
+ group: SubscriptionModel.apiGroup,
+ resource: SubscriptionModel.plural,
+ verb: 'create',
+ namespace: selectedTargetNamespace,
+ });Also applies to: 1196-1196
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-hub-subscribe.tsx`
around lines 237 - 241, The RBAC check using useAccessReview for
canCreateSubscription currently omits the namespace, causing false negatives for
users with namespace-scoped create permission; update the useAccessReview call
(the canCreateSubscription check) to include the target namespace (the same
namespace used for subscription.metadata.namespace) so the access review is
performed against that namespace (and apply the same fix where useAccessReview
is used again for SubscriptionModel elsewhere in this file).
|
PR needs rebase. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
2 participants
Summary
Follow-up to #16554 which covered most of OCPBUGS-81749. That PR fixed Edit Subscription, channel/approval pencil buttons, descriptor edit buttons, and the catalog source Enable button, but the original Jira description also explicitly lists Install Operator and Create custom resources as actions that should be gated on RBAC. This PR fills those two remaining gaps.
Changes
frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-hub-subscribe.tsxAdd a
useAccessReviewcheck forcreateonSubscriptionModeland disable the Install button when the user lacks that permission:frontend/packages/operator-lifecycle-manager/src/components/operand/index.tsxPass
createAccessReviewtoListPageCreateLinkinDefaultProvidedAPIPageso the existingRequireCreatePermissionmechanism hides the Create <Kind> button when the user cannot create that operand type — consistent with the CRD card pattern already in place:Before / After
createonsubscriptionscreateon that CR typeTests
operator-hub/__tests__/operator-hub-subscribe.spec.tsx(new) — verifies Install button is disabled whenuseAccessReviewreturnsfalsefor Subscription createoperand/__tests__/index.spec.tsx— verifies Create button is hidden whenuseAccessReviewAllowedreturnsfalseTest plan
create subscriptionspermission, the Install button on the Operator Hub subscribe form is disabledcreateon a CR kind, the Create <Kind> button is hidden on the operand list pageMade with Cursor
Summary by CodeRabbit
New Features
Tests