OCPNETUI-21, OCPNETUI-25: made form for creating services - #453
Merged
openshift-merge-bot[bot] merged 10 commits intoJul 31, 2026
Conversation
Contributor
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci-robot
added
the
jira/valid-reference
|
@Parthivk100: This pull request references OCPNETUI-21 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set. This pull request references OCPNETUI-25 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set. 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. |
|
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:
WalkthroughAdds Service creation and editing forms with validation, routing, YAML fallback, selector previews, port parsing, and localized networking-console messages. Shared selector preview logic now scopes resource queries by namespace and supports parameterized resource names. ChangesService form workflow
Estimated code review effort: 4 (Complex) | ~45 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 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 |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Parthivk100 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (1)
src/views/services/form/generateDefaultService.ts (1)
12-16: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAvoid suppressing the
targetPorttype with a double cast.
9376 as unknown as stringdoes not convert the runtime number; it only hides the generated model contract. Use the actual model type and keepportsToText/textToPortsconsistent with it.🤖 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 `@src/views/services/form/generateDefaultService.ts` around lines 12 - 16, Update the default service port definition in generateDefaultService to use the generated model’s actual targetPort type instead of the double cast on 9376. Ensure portsToText and textToPorts use the same targetPort representation consistently, preserving the intended numeric port value at runtime.
🤖 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 `@locales/es/plugin__networking-console-plugin.json`:
- Line 70: Translate the newly added entries in the Spanish networking console
locale, including the keys at the referenced locations, instead of retaining
their English source text. Preserve each existing key and JSON structure while
providing accurate, natural Spanish translations for the validation, help, and
form strings.
In `@locales/fr/plugin__networking-console-plugin.json`:
- Line 70: Translate all newly added Service-form validation and help-text keys
from English into the appropriate language in
locales/fr/plugin__networking-console-plugin.json lines 70-70,
locales/ja/plugin__networking-console-plugin.json lines 68-68,
locales/ko/plugin__networking-console-plugin.json lines 68-68, and
locales/zh/plugin__networking-console-plugin.json lines 68-68; preserve the
existing keys and JSON structure.
In `@src/views/services/form/constants.ts`:
- Line 7: Remove ExternalName from SERVICE_TYPES in the constants definition
unless the form is being fully extended to collect externalName and omit
selector and ports for that type. Ensure ServiceForm validation and submission
cannot produce an invalid ExternalName Service.
In `@src/views/services/form/ServiceForm.tsx`:
- Around line 14-15: Remove the unused Content and ContentVariants imports from
the import list in ServiceForm.tsx, preserving the remaining imports and their
valid ordering to resolve the ESLint errors.
- Around line 88-128: Update the onSubmit handler to return the k8sPromise
chain, including the existing navigation and error handling, so handleSubmit can
keep isSubmitting active until the create or update request completes. Preserve
the current k8sCreate/k8sUpdate selection and setError behavior.
In `@src/views/services/form/ServiceFormPage.tsx`:
- Line 55: Reformat the long translation call in the service form view by
wrapping the t(...) invocation according to the repository’s Prettier rules,
without changing its text or behavior.
In `@src/views/services/form/ServiceTypeSelect.tsx`:
- Around line 23-57: Update the Controller render in ServiceTypeSelect to
destructure fieldState.error and pass its presence to the surrounding FormGroup
as the validation error state, including a localized required-field message.
Keep rules={{ required: true }} so clearing spec.type remains invalid and
visibly reported.
In `@src/views/services/form/utils.ts`:
- Around line 95-112: Update validatePortsText to validate the optional parsed
protocol against the Service-supported allow-list TCP, UDP, and SCTP before
accepting the entry; reject any other protocol while preserving the existing
port and target-port validation behavior and the textToPorts output contract.
- Around line 23-50: The portsToText/textToPorts conversion currently loses
Service port names, making multi-port submissions invalid. Extend the text
format handled by portsToText and textToPorts to serialize and parse each port’s
name, preserving existing names and emitting valid dns-label names for
multi-port Services; retain current protocol and targetPort behavior.
---
Nitpick comments:
In `@src/views/services/form/generateDefaultService.ts`:
- Around line 12-16: Update the default service port definition in
generateDefaultService to use the generated model’s actual targetPort type
instead of the double cast on 9376. Ensure portsToText and textToPorts use the
same targetPort representation consistently, preserving the intended numeric
port value at runtime.
🪄 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: 1e4a18e2-762d-4ddf-86d3-902e036cac27
📒 Files selected for processing (20)
locales/en/plugin__networking-console-plugin.jsonlocales/es/plugin__networking-console-plugin.jsonlocales/fr/plugin__networking-console-plugin.jsonlocales/ja/plugin__networking-console-plugin.jsonlocales/ko/plugin__networking-console-plugin.jsonlocales/zh/plugin__networking-console-plugin.jsonsrc/utils/components/FormGroupHelperText/FormGroupHelperText.tsxsrc/utils/components/StatusBox/StatusBox.tsxsrc/views/services/actions/hooks/useServiceActions.tsxsrc/views/services/form/EditService.tsxsrc/views/services/form/ServiceForm.tsxsrc/views/services/form/ServiceFormActions.tsxsrc/views/services/form/ServiceFormPage.tsxsrc/views/services/form/ServiceTypeSelect.tsxsrc/views/services/form/constants.tssrc/views/services/form/generateDefaultService.tssrc/views/services/form/useIsCreationForm.tsxsrc/views/services/form/utils.tssrc/views/services/list/ServiceList.tsxsrc/views/services/manifest.ts
pcbailey
reviewed
Jul 28, 2026
pcbailey
left a comment
pcbailey
left a comment
Contributor
There was a problem hiding this comment.
Please also go through the CodeRabbit comments.
…eType and edit service action
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/views/services/form/ServiceFormPage.tsx (1)
45-58: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDescription text always says "Create," even when editing.
The
Titlecorrectly branches between "Edit"/"Create" (Lines 49-52), but the paragraph below is static and always reads "Create a Service using the form..." even whenisEditingis true.✏️ Proposed fix
<Content component={ContentVariants.p}> - {t( - 'Create a Service using the form. Switch to YAML view for full control, including fields not shown in the form.', - )} + {isEditing + ? t( + 'Edit this Service using the form. Switch to YAML view for full control, including fields not shown in the form.', + ) + : t( + 'Create a Service using the form. Switch to YAML view for full control, including fields not shown in the form.', + )} </Content>🤖 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 `@src/views/services/form/ServiceFormPage.tsx` around lines 45 - 58, Update the paragraph translation in the ServiceFormPage render to branch on isEditing, using edit wording when editing and preserving the existing create wording for new services. Keep the Title behavior and YAML-view guidance unchanged.src/views/services/form/ServiceTypeSelect.tsx (1)
22-73: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winType field shows a "danger" toggle but no visible error message.
erroris now wired intoaria-invalid/statuson theMenuToggle, but unlike every other field in this form (Name, Namespace, Selector, Ports, ExternalName), there's noFormGroupHelperTextrenderingerror.message. Users get a red toggle with no explanation of what's wrong.✏️ Proposed fix
</Dropdown> + <FormGroupHelperText validated={error ? ValidatedOptions.error : ValidatedOptions.default}> + {error?.message} + </FormGroupHelperText> </FormGroup>🤖 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 `@src/views/services/form/ServiceTypeSelect.tsx` around lines 22 - 73, Add the type validation message to the FormGroup containing the `ServiceTypeSelect` dropdown by rendering `FormGroupHelperText` with `error.message`, matching the error-display pattern used by the other form fields while preserving the existing toggle error styling.
🤖 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 `@src/views/services/form/utils/utils.ts`:
- Around line 55-62: The PORT_LINE_REGEX validation currently accepts invalid
symbolic targetPort values; update the port-line parsing or validation using
PORT_LINE_REGEX so nonnumeric targetPort values must satisfy the Kubernetes
service port-name constraint (DNS_LABEL-compatible), while preserving numeric
targetPort handling and valid optional protocols.
---
Outside diff comments:
In `@src/views/services/form/ServiceFormPage.tsx`:
- Around line 45-58: Update the paragraph translation in the ServiceFormPage
render to branch on isEditing, using edit wording when editing and preserving
the existing create wording for new services. Keep the Title behavior and
YAML-view guidance unchanged.
In `@src/views/services/form/ServiceTypeSelect.tsx`:
- Around line 22-73: Add the type validation message to the FormGroup containing
the `ServiceTypeSelect` dropdown by rendering `FormGroupHelperText` with
`error.message`, matching the error-display pattern used by the other form
fields while preserving the existing toggle error styling.
🪄 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: bc83ddd5-8531-4da9-a444-ce1a89df8c9c
📒 Files selected for processing (17)
locales/en/plugin__networking-console-plugin.jsonlocales/es/plugin__networking-console-plugin.jsonlocales/fr/plugin__networking-console-plugin.jsonlocales/ja/plugin__networking-console-plugin.jsonlocales/ko/plugin__networking-console-plugin.jsonlocales/zh/plugin__networking-console-plugin.jsonsrc/views/services/actions/hooks/useServiceActions.tsxsrc/views/services/details/components/ServiceDetailsPageTitle.tsxsrc/views/services/form/EditService.tsxsrc/views/services/form/ExternalNameField.tsxsrc/views/services/form/ServiceForm.tsxsrc/views/services/form/ServiceFormPage.tsxsrc/views/services/form/ServiceTypeSelect.tsxsrc/views/services/form/hooks/useIsCreationForm.tsxsrc/views/services/form/utils/constants.tssrc/views/services/form/utils/utils.tssrc/views/services/form/utils/validationUtils.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/views/services/form/EditService.tsx
- src/views/services/actions/hooks/useServiceActions.tsx
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 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 `@locales/es/plugin__networking-console-plugin.json`:
- Line 225: Update the Spanish translation for “List of pods matching” to refer
to matching pods rather than groups, using the established “pods” terminology
such as “Lista de pods coincidentes.”
In `@src/utils/components/SelectorPreview/hooks/useSelectorPreviewData.ts`:
- Around line 40-55: The useSelectorPreviewData watch currently broadens to all
resources when either selector is invalid. Update the flow around
safeNsSelector, offendingNsSelector, safeLabelSelector, and
offendingLabelSelector to disable or return a no-match state before
useK8sWatchResource runs whenever either selector is invalid; preserve normal
watching and selector error behavior for valid selectors.
In `@src/utils/components/SelectorPreview/SelectorPreviewContent.tsx`:
- Around line 116-124: Update the “show more” link in SelectorPreviewContent so
its destination preserves the active namespaceSelector filtering used for the
preview. Encode the selector in the URL using the existing supported query
mechanism, or suppress the link when the selector cannot be represented; retain
the current namespace and label filtering behavior.
In `@src/utils/components/SelectorPreview/utils/const.ts`:
- Line 10: Update the allowedSelector regular expression to remove the backslash
from its permitted character class, while retaining the documented allow-list of
alphanumerics, hyphens, underscores, slashes, and periods and preserving the
existing boundary rules.
In `@src/views/services/form/ServiceSelectorField.tsx`:
- Around line 32-44: The onPairsChange handler must prevent stale spec.selector
values from being submitted when validateSelectorPairs reports invalid input,
including an entirely cleared selector. Update the surrounding
ServiceSelectorField/ServiceForm form state so invalid raw pairs register a
blocking validation error (or otherwise prevent submission), and clear that
error only when the pairs become valid while preserving the existing valid
setValue behavior.
- Line 26: Update ServiceSelectorField’s rendered selector preview trigger near
the SelectorPreview usage to add an accessible button element and attach
podsPreviewPopoverRef via its ref prop. Keep passing the ref to SelectorPreview
through triggerRef, and ensure the button provides the trigger interaction
needed to open the popover.
In `@src/views/services/form/utils/validationUtils.ts`:
- Around line 53-54: Update validateSelectorPairs so it ignores only rows where
both selector fields are empty, while retaining partially filled rows such as an
empty key with a non-empty value for validation to reject. Ensure
labelPairsToRecord cannot silently omit any row containing a value without a
key.
🪄 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: c92407d3-2593-498c-b7fe-a742c5a8b21f
📒 Files selected for processing (17)
locales/en/plugin__networking-console-plugin.jsonlocales/es/plugin__networking-console-plugin.jsonlocales/fr/plugin__networking-console-plugin.jsonlocales/ja/plugin__networking-console-plugin.jsonlocales/ko/plugin__networking-console-plugin.jsonlocales/zh/plugin__networking-console-plugin.jsonsrc/utils/components/SelectorPreview/SelectorPreview.tsxsrc/utils/components/SelectorPreview/SelectorPreviewContent.tsxsrc/utils/components/SelectorPreview/hooks/useSelectorPreviewData.tssrc/utils/components/SelectorPreview/index.tssrc/utils/components/SelectorPreview/utils/const.tssrc/utils/components/SelectorPreview/utils/types.tssrc/utils/components/SelectorPreview/utils/utils.tssrc/views/services/form/ServiceForm.tsxsrc/views/services/form/ServiceSelectorField.tsxsrc/views/services/form/utils/utils.tssrc/views/services/form/utils/validationUtils.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- locales/ja/plugin__networking-console-plugin.json
- locales/fr/plugin__networking-console-plugin.json
- locales/ko/plugin__networking-console-plugin.json
pcbailey
reviewed
Jul 30, 2026
pcbailey
left a comment
pcbailey
left a comment
Contributor
There was a problem hiding this comment.
Please address any new CodeRabbit comments.
pcbailey
reviewed
Jul 30, 2026
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
locales/ja/plugin__networking-console-plugin.json (1)
68-73: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winTranslate the new strings in the Japanese catalog.
All changed values remain English, so Japanese users will see untranslated service-form guidance, validation errors, and selector-preview messages. Translate the right-hand values while preserving placeholders such as
{{resourceName}}and<2>...</2>.Also applies to: 105-105, 162-167, 221-222, 245-247, 266-266, 290-290, 310-328, 374-377, 391-391, 405-405, 440-440
🤖 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 `@locales/ja/plugin__networking-console-plugin.json` around lines 68 - 73, Translate every newly added English value in the Japanese locale catalog to natural Japanese, including the listed ranges and the shown entries. Preserve each translation key, interpolation such as {{resourceName}}, and markup placeholders such as <2>...</2> exactly while changing only the right-hand values.
🤖 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 `@src/views/services/form/utils/constants.ts`:
- Around line 13-14: Update EXTERNAL_NAME_HOSTNAME_REGEX to match Kubernetes
ExternalName validation: use a case-sensitive RFC-1123 hostname pattern, allow
single-label hostnames such as “host,” reject uppercase characters, and preserve
the API’s hostname length limits if enforced by this validation.
---
Outside diff comments:
In `@locales/ja/plugin__networking-console-plugin.json`:
- Around line 68-73: Translate every newly added English value in the Japanese
locale catalog to natural Japanese, including the listed ranges and the shown
entries. Preserve each translation key, interpolation such as {{resourceName}},
and markup placeholders such as <2>...</2> exactly while changing only the
right-hand values.
🪄 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: 614ffb80-2f5a-4052-a2f1-6e0f9f467e6a
📒 Files selected for processing (24)
locales/en/plugin__networking-console-plugin.jsonlocales/es/plugin__networking-console-plugin.jsonlocales/fr/plugin__networking-console-plugin.jsonlocales/ja/plugin__networking-console-plugin.jsonlocales/ko/plugin__networking-console-plugin.jsonlocales/zh/plugin__networking-console-plugin.jsonsrc/utils/components/SelectorPreview/SelectorPreview.tsxsrc/utils/components/SelectorPreview/SelectorPreviewContent.tsxsrc/utils/components/SelectorPreview/hooks/useSelectorPreviewData.tssrc/utils/components/SelectorPreview/utils/utils.tssrc/utils/resources/shared.tssrc/utils/utils/selector.tssrc/views/networkpolicies/new/components/NetworkPolicySelectorPreview/components/NetworkPolicyPodsPreview.tsxsrc/views/networkpolicies/new/components/NetworkPolicySelectorPreview/components/hooks/useNetworkPolicyPodPreviewData.tssrc/views/networkpolicies/new/components/NetworkPolicySelectorPreview/utils/const.tssrc/views/networkpolicies/new/components/NetworkPolicySelectorPreview/utils/types.tssrc/views/services/form/ExternalNameField.tsxsrc/views/services/form/ServiceForm.tsxsrc/views/services/form/ServiceSelectorField.tsxsrc/views/services/form/ServiceTypeFields.tsxsrc/views/services/form/utils/constants.tssrc/views/services/form/utils/types.tssrc/views/services/form/utils/utils.tssrc/views/services/form/utils/validationUtils.ts
💤 Files with no reviewable changes (2)
- src/views/networkpolicies/new/components/NetworkPolicySelectorPreview/utils/types.ts
- src/views/networkpolicies/new/components/NetworkPolicySelectorPreview/utils/const.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- locales/fr/plugin__networking-console-plugin.json
- locales/es/plugin__networking-console-plugin.json
- locales/ko/plugin__networking-console-plugin.json
- locales/en/plugin__networking-console-plugin.json
- locales/zh/plugin__networking-console-plugin.json
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/views/services/form/utils/constants.ts`:
- Around line 12-14: The EXTERNAL_NAME_HOSTNAME_REGEX used by
validateExternalName must enforce Kubernetes limits in addition to format:
restrict each hostname label to at most 63 characters and the full hostname to
at most 253 characters. Update the regex or validation logic accordingly, and
add boundary tests covering 63/64-character labels and 253/254-character
hostnames.
🪄 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: 00e2309f-0165-43e3-bc3c-08a6b889dc4e
📒 Files selected for processing (1)
src/views/services/form/utils/constants.ts
Contributor
|
/lgtm |
Contributor
|
Scheduling required tests: |
|
@Parthivk100: 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. |
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.
created form for making services utilizing SyncedEditor.
Design Document
Screenshare.-.2026-07-30.4_49_25.PM.mp4
Summary by CodeRabbit
Summary by CodeRabbit