[release-4.20 ]OCPBUGS-84041: Fix template placeholder {{name}} not resolved and runtime error on Group details page#16313
Conversation
…time error on Group details page Fix impersonate action to pass group name as a named i18n parameter instead of the whole metadata object. Add optional chaining to guard against undefined metadata. Validate user input in AddUsersModal before submitting.
openshift-ci-robot
added
jira/severity-moderate
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository: openshift/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThe changes add defensive null-safety checks across group management components using optional chaining ( Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 12✅ Passed checks (12 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@Leo6Leo: This pull request references Jira Issue OCPBUGS-84041, which is valid. The bug has been moved to the POST state. 7 validation(s) were run on this bug
No GitHub users were found matching the public email listed for the QA contact in Jira (xiyuzhao@redhat.com), skipping review request. 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: Leo6Leo 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 |
openshift-ci
Bot
added
the
kind/i18n
Leo6Leo
changed the title
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@Leo6Leo: This pull request references Jira Issue OCPBUGS-84041, which is valid. The bug has been moved to the POST state. 7 validation(s) were run on this bug
No GitHub users were found matching the public email listed for the QA contact in Jira (xiyuzhao@redhat.com), skipping review request. 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. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/public/components/group.tsx`:
- Around line 60-71: The callback passes group?.metadata?.name (possibly
undefined) into startImpersonate and accessReview.name which can cause runtime
errors; add a guard that checks for a non-empty group.metadata.name before
calling startImpersonate or constructing the accessReview, e.g., disable or
no-op the callback when name is falsy, and ensure accessReview.name and the i18n
label use a safe fallback only when a real name exists; update the anonymous
callback in the menu entry (the function that calls startImpersonate and
navigate) and the accessReview object to only run/contain values when
group?.metadata?.name is defined.
🪄 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: Pro Plus
Run ID: 814d2222-5502-4460-baa6-00bbf07b2ee4
📒 Files selected for processing (3)
frontend/public/components/group.tsxfrontend/public/components/modals/add-users-modal.tsxfrontend/public/locales/en/public.json
| label: i18next.t('public~Impersonate Group {{name}}', { name: group?.metadata?.name }), | ||
| callback: () => { | ||
| startImpersonate('Group', group.metadata.name); | ||
| startImpersonate('Group', group?.metadata?.name); | ||
| navigate(window.SERVER_FLAGS.basePath); | ||
| }, | ||
| // Must use API group authorization.k8s.io, NOT user.openshift.io | ||
| // See https://kubernetes.io/docs/reference/access-authn-authz/authentication/#user-impersonation | ||
| accessReview: { | ||
| group: 'authorization.k8s.io', | ||
| resource: 'groups', | ||
| name: group.metadata.name, | ||
| name: group?.metadata?.name, | ||
| verb: 'impersonate', |
There was a problem hiding this comment.
Guard impersonation when group name is missing.
At Line 62, startImpersonate can still receive undefined via group?.metadata?.name, which can trigger a runtime failure in the impersonation path. Line 70 propagates the same risk into accessReview.name.
Suggested fix
-const getImpersonateAction = (
- startImpersonate: StartImpersonate,
- navigate: NavigateFunction,
-): KebabAction => (kind: K8sKind, group: GroupKind) => ({
- label: i18next.t('public~Impersonate Group {{name}}', { name: group?.metadata?.name }),
- callback: () => {
- startImpersonate('Group', group?.metadata?.name);
- navigate(window.SERVER_FLAGS.basePath);
- },
- accessReview: {
- group: 'authorization.k8s.io',
- resource: 'groups',
- name: group?.metadata?.name,
- verb: 'impersonate',
- },
-});
+const getImpersonateAction = (
+ startImpersonate: StartImpersonate,
+ navigate: NavigateFunction,
+): KebabAction => (kind: K8sKind, group: GroupKind) => {
+ const groupName = group?.metadata?.name;
+ return {
+ label: i18next.t('public~Impersonate Group {{name}}', { name: groupName ?? '' }),
+ callback: () => {
+ if (!groupName) {
+ return;
+ }
+ startImpersonate('Group', groupName);
+ navigate(window.SERVER_FLAGS.basePath);
+ },
+ accessReview: {
+ group: 'authorization.k8s.io',
+ resource: 'groups',
+ name: groupName,
+ verb: 'impersonate',
+ },
+ };
+};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/public/components/group.tsx` around lines 60 - 71, The callback
passes group?.metadata?.name (possibly undefined) into startImpersonate and
accessReview.name which can cause runtime errors; add a guard that checks for a
non-empty group.metadata.name before calling startImpersonate or constructing
the accessReview, e.g., disable or no-op the callback when name is falsy, and
ensure accessReview.name and the i18n label use a safe fallback only when a real
name exists; update the anonymous callback in the menu entry (the function that
calls startImpersonate and navigate) and the accessReview object to only
run/contain values when group?.metadata?.name is defined.
|
/retest-required |
|
@Leo6Leo: all tests passed! 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. |
2 participants
Summary
Cherry-pick of #15780 to release-4.20 (manually adapted due to code structure differences between main and release-4.20).
group.tsxto pass group name as a named i18n parameter ({ name: group?.metadata?.name }) instead of the whole metadata object, preventing raw{{name}}placeholder displayAddUsersModal— guard against undefined group and filter empty user entries before submittingBug
https://redhat.atlassian.net/browse/OCPBUGS-84041
Test plan
Summary by CodeRabbit
Bug Fixes
Improvements