feat(KFLUXUI-175): give up SpaceBindingRequest and enjoy RoleBinding #110
Conversation
testcara
force-pushed
the
KFLUXUI-175
branch
from
7865c99 to
f290535
Compare
| return ( | ||
| <TableData className={column.className}> | ||
| {roleMapLoading && column.className === rbTableColumnClasses.role ? ( | ||
| <Skeleton /> |
There was a problem hiding this comment.
We give up spinner and enjoy the skeleton here.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #110 +/- ##
==========================================
+ Coverage 80.10% 80.14% +0.04%
==========================================
Files 577 546 -31
Lines 21495 21203 -292
Branches 5064 5320 +256
==========================================
- Hits 17219 16994 -225
+ Misses 4252 4184 -68
- Partials 24 25 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 36 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
|
|
||
| return ( | ||
| <> | ||
| <FormSection title="Assign role"> | ||
| <DropdownField | ||
| className="role-section" | ||
| name="role" | ||
| placeholder="Select role" | ||
| placeholder={roleMapLoading ? 'Loading...' : 'Select role'} |
There was a problem hiding this comment.
As suggested, we remove the spinner disable the drop-down with 'Loading' before the roleMap is ready.
src/hooks/useKonfluxPublicInfo.ts
Outdated
| import React from 'react'; | ||
| import { useK8sWatchResource } from '../k8s'; | ||
| import { ConfigMapGroupVersionKind, ConfigMapModel } from '../models'; | ||
| import { KonfluxPublicInfo, KonfluxPublicInfoConfigMap } from '../types/konfluxPublicInfo'; |
There was a problem hiding this comment.
As suggested, the separate hook for konflux public info and related types has been created.
| <Alert variant={AlertVariant.info} title="Username not validated" isInline> | ||
| Konflux is no longer validating the username except for its format. Please make sure that | ||
| the usernames are correct before proceeding. | ||
| </Alert> |
There was a problem hiding this comment.
As suggested by ux team, they consider Info Alert without close button is better.
testcara
force-pushed
the
KFLUXUI-175
branch
2 times, most recently
from
4894b58 to
55a1203
Compare
|
@sahil143 , hi, Sahil, I refreshed the PR with your latest suggestions and added comments in the target review place. |
testcara
force-pushed
the
KFLUXUI-175
branch
from
55a1203 to
dd0ef58
Compare
|
The ux related parts of the PR follows the page https://docs.google.com/document/d/1Iu5s0pkUZnLSHElpI2-iATo2RctUsEkq4_PBj3yuD_M/edit?tab=t.0. |
testcara
force-pushed
the
KFLUXUI-175
branch
2 times, most recently
from
3683ee6 to
070ab1e
Compare
|
FYI, The recording is the latest one. I also explained test points for it. Hope it is useful to review. Thank you so much. |
Key Points: 1. We watch the konflux-public-info configmap to get the roles. 2. For editing role, we create one one then delete obsoleted one.
testcara
force-pushed
the
KFLUXUI-175
branch
from
070ab1e to
80cbdb0
Compare
Fixes
KFLUXUI-175
Description
Konflux is migrating off workspace api and also the related SpaceBindingRequest.
For Konflux UI, we need to enjoy the shared Konflux cluster role and rolebinding for the namespace access and permission checks.
For the details:
We use k8swatch the konflux-public-info configmap to get the roles. then create/delete the rolebindings to the shared konflux clusterRole to support the UI actions.
More, the status of the user access list are removed.
Type of change
Screen shots / Gifs for design review
In the recording, i did the following actions for testing:
Screen.Recording.2025-02-12.at.21.38.46.mov
How to test or reproduce?
Just enjoy the use access page as previous.
You can visit access list, grant access, edit access, revoke access like before.
Browser conformance: