FCN-254: role arns api call#6498
Open
davidaznaur wants to merge 10 commits into
Open
Conversation
Signed-off-by: David Aznaurov <daznauro@redhat.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe ROSA Wizard adds authenticated STS role endpoints, frontend API helpers and response types, query-key support, proxy paths, and a React Query hook that normalizes role ARNs and exposes combined role data and status. ChangesROSA Wizard STS integration
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ROSAWizardClient
participant useFetchRoleARNs
participant rosa-hcp-api
participant rosaWizardApi
participant OCM
ROSAWizardClient->>useFetchRoleARNs: fetch(accountId)
useFetchRoleARNs->>rosa-hcp-api: Request three role resources
rosa-hcp-api->>rosaWizardApi: POST STS role endpoints
rosaWizardApi->>OCM: Authenticate and query role APIs
OCM-->>rosaWizardApi: Return role responses
rosaWizardApi-->>rosa-hcp-api: Return JSON data
rosa-hcp-api-->>useFetchRoleARNs: Provide query results
useFetchRoleARNs-->>ROSAWizardClient: Return normalized roles and status
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
Signed-off-by: David Aznaurov <daznauro@redhat.com>
Contributor
Author
|
/cc @kelvah |
Contributor
Author
|
/cc @KevinFCormier |
kelvah
reviewed
Jul 21, 2026
Signed-off-by: David Aznaurov <daznauro@redhat.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
backend/src/routes/rosaWizardApi.ts (1)
105-183: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate handler logic between
getRoleARNsandgetOCMRoleARN.Both functions share the same auth/body-parsing/token/POST/response scaffolding, differing only in
accountPath. Consider extracting a shared helper (e.g.postStsInquiry(req, res, path)) to reduce duplication for this and any future STS inquiry endpoint.🤖 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 `@backend/src/routes/rosaWizardApi.ts` around lines 105 - 183, The handlers getRoleARNs and getOCMRoleARN duplicate the same request-processing flow. Extract their shared authentication, body parsing, service-token retrieval, POST, error handling, and response logic into a helper such as postStsInquiry(req, res, accountPath), then have each handler call it with only its distinct sts_account_roles or sts_ocm_role endpoint path.frontend/src/lib/rosa-hcp-api.ts (1)
3-12: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winType-only imports should use
import typeacross the new STS role files. Same root cause in all three: named imports of pure TypeScript interfaces/types used only as annotations, never as runtime values.
frontend/src/lib/rosa-hcp-api.ts#L3-L12: changeimport { ... } from '~/resources'toimport type { ... } from '~/resources'.frontend/src/routes/Infrastructure/Clusters/ManagedClusters/components/rosahcp/queries/useFetchRolesARNs.ts#L5-L6: change both~/resourcesand../constants/typesimports toimport type.frontend/src/routes/Infrastructure/Clusters/ManagedClusters/components/rosahcp/queries/useFetchRolesARNs.test.tsx#L5-L5: changeimport { SelectedSecret } from '../constants/types'toimport type { SelectedSecret } from '../constants/types'.🤖 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/src/lib/rosa-hcp-api.ts` around lines 3 - 12, Convert the type-only imports to `import type` in frontend/src/lib/rosa-hcp-api.ts lines 3-12, both `~/resources` and `../constants/types` imports in frontend/src/routes/Infrastructure/Clusters/ManagedClusters/components/rosahcp/queries/useFetchRolesARNs.ts lines 5-6, and the `SelectedSecret` import in frontend/src/routes/Infrastructure/Clusters/ManagedClusters/components/rosahcp/queries/useFetchRolesARNs.test.tsx line 5; no other changes are needed.Source: Coding guidelines
🤖 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 `@backend/src/routes/rosaWizardApi.ts`:
- Around line 127-132: Update the account lookup error handling in both
getRoleARNs and getOCMRoleARN to match getUserRole: when jsonPost rejects, log
the error, call respondInternalServerError, and return immediately. Do not
continue to set headers or serialize the undefined result; preserve the existing
successful response flow.
In `@frontend/src/lib/rosa-hcp-api.ts`:
- Around line 62-76: Update getWizardRoleARNs and getWizardOCMRoleARN to unwrap
the wizard API response before returning it: return res.data.body when present,
falling back to res.data otherwise. Preserve the existing generic types and
request parameters while ensuring consumers receive the payload with items or
role fields at the top level.
In
`@frontend/src/routes/Infrastructure/Clusters/ManagedClusters/components/rosahcp/queries/useFetchRolesARNs.ts`:
- Line 140: Update the isLoading value in the roles ARN query result to also
include userRoleQuery.isLoading, alongside rolesQuery.isLoading and
ocmRoleQuery.isLoading, so it remains true while any of the three queries is
fetching.
---
Nitpick comments:
In `@backend/src/routes/rosaWizardApi.ts`:
- Around line 105-183: The handlers getRoleARNs and getOCMRoleARN duplicate the
same request-processing flow. Extract their shared authentication, body parsing,
service-token retrieval, POST, error handling, and response logic into a helper
such as postStsInquiry(req, res, accountPath), then have each handler call it
with only its distinct sts_account_roles or sts_ocm_role endpoint path.
In `@frontend/src/lib/rosa-hcp-api.ts`:
- Around line 3-12: Convert the type-only imports to `import type` in
frontend/src/lib/rosa-hcp-api.ts lines 3-12, both `~/resources` and
`../constants/types` imports in
frontend/src/routes/Infrastructure/Clusters/ManagedClusters/components/rosahcp/queries/useFetchRolesARNs.ts
lines 5-6, and the `SelectedSecret` import in
frontend/src/routes/Infrastructure/Clusters/ManagedClusters/components/rosahcp/queries/useFetchRolesARNs.test.tsx
line 5; no other changes are needed.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: f6149555-903c-42cb-bead-dd73ef1d8408
📒 Files selected for processing (12)
backend/src/app.tsbackend/src/routes/rosaWizardApi.tsbackend/test/routes/rosaWizardApi.test.tsfrontend/src/lib/rosa-hcp-api.test.tsfrontend/src/lib/rosa-hcp-api.tsfrontend/src/resources/rosa-hcp-wizard.tsfrontend/src/routes/Infrastructure/Clusters/ManagedClusters/components/rosahcp/constants/types.tsfrontend/src/routes/Infrastructure/Clusters/ManagedClusters/components/rosahcp/queries/queryKeyFactory.test.tsfrontend/src/routes/Infrastructure/Clusters/ManagedClusters/components/rosahcp/queries/queryKeyFactory.tsfrontend/src/routes/Infrastructure/Clusters/ManagedClusters/components/rosahcp/queries/useFetchRolesARNs.test.tsxfrontend/src/routes/Infrastructure/Clusters/ManagedClusters/components/rosahcp/queries/useFetchRolesARNs.tsfrontend/webpack.config.ts
Signed-off-by: David Aznaurov <daznauro@redhat.com>
Signed-off-by: David Aznaurov <daznauro@redhat.com>
Signed-off-by: David Aznaurov <daznauro@redhat.com>
kelvah
reviewed
Jul 22, 2026
Signed-off-by: David Aznaurov <daznauro@redhat.com>
KevinFCormier
requested changes
Jul 22, 2026
Signed-off-by: David Aznaurov <daznauro@redhat.com>
KevinFCormier
requested changes
Jul 22, 2026
KevinFCormier
left a comment
KevinFCormier
left a comment
Contributor
There was a problem hiding this comment.
AI-code review generated a few more things that deserve consideration. Please take a look. Thanks.
19 tasks
Signed-off-by: David Aznaurov <daznauro@redhat.com>
|
19 tasks
kelvah
approved these changes
Jul 22, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: davidaznaur, kelvah 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 |
Signed-off-by: David Aznaurov <daznauro@redhat.com>
|
New changes are detected. LGTM label has been removed. |
Contributor
Author
|
/retest |
|
@davidaznaur: The following test failed, say
Full PR test history. Your PR dashboard. 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. I understand the commands that are listed here. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📝 Summary
Ticket Summary (Title):
Account, OCM and User role ARNs api calls
Ticket Link:
Type of Change:
✅ Checklist
General
ACM-12340 Fix bug with...)If Feature
If Bugfix
🗒️ Notes for Reviewers
Summary by CodeRabbit
/sts-role-arns,/sts-ocm-role, and/sts-user-role.