MGMT-21625: Add Dual-Stack with Primary IPv6 Support#3190
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@linoyaslan: This pull request references MGMT-21625 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 task to target the "4.21.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. |
openshift-ci
bot
added
the
size/L
WalkthroughPropagates optional Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant NC as NetworkConfiguration
participant ASC as AvailableSubnetsControl
participant SD as SubnetsDropdown
participant VS as ValidationSchemas
participant DU as DualStackUtils
participant VU as VersionUtils
User->>NC: open network config (cluster with openshiftVersion)
NC->>ASC: render(props including openshiftVersion)
ASC->>VU: isMajorMinorVersionEqualOrGreater(openshiftVersion,"4.12")
VU-->>ASC: supportsIPv6Primary (true/false)
ASC->>SD: request dropdown items (version-aware)
SD-->>ASC: grouped ipv4/ipv6 items or flat list
ASC-->>User: render primary (and optional secondary) subnet dropdowns
User->>ASC: select subnet(s)
ASC-->>NC: update form values
NC->>VS: validate via dualStackValidationSchema(field, openshiftVersion)
VS->>DU: isDualStack({ ...networks, openshiftVersion })
DU-->>VS: boolean (dual-stack)
VS-->>NC: validation result
NC-->>User: show validation state / labels
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 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 |
|
@linoyaslan: This pull request references MGMT-21625 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 task to target the "4.21.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. |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AvailableSubnetsControl.tsx (1)
63-126: Do not expose IPv6 subnets in single-stack flowsWhen
stackType !== DUAL_STACK, we still buildallSubnetsand feed it both to the auto-select and the single-stack dropdown (Line 125). For OCP ≥ 4.12 this lets single-stack clusters auto-select an IPv6 CIDR, which immediately violates the IPv4-only validation and forces the user to undo the preselection. Worse, it surfaces unsupported IPv6 options in a flow that should remain IPv4-only. Please keep IPv6-primary logic behind anisDualStackcheck so single-stack clusters continue to see (and auto-select) IPv4 subnets only.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
libs/ui-lib/lib/common/components/clusterConfiguration/utils.ts(2 hunks)libs/ui-lib/lib/common/components/ui/formik/validationSchemas.ts(2 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AvailableSubnetsControl.tsx(3 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkConfiguration.tsx(1 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkConfigurationForm.tsx(1 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts(3 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/review/ReviewNetworkingTable.tsx(4 hunks)libs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.tsx(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (7)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AvailableSubnetsControl.tsx (3)
libs/ui-lib/lib/common/types/clusters.ts (1)
NetworkConfigurationValues(34-53)libs/ui-lib/lib/ocm/store/slices/current-cluster/selectors.ts (1)
selectCurrentClusterPermissionsState(9-17)libs/ui-lib/lib/common/utils.ts (1)
isMajorMinorVersionEqualOrGreater(138-143)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkConfigurationForm.tsx (1)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts (1)
getNetworkConfigurationValidationSchema(66-110)
libs/ui-lib/lib/common/components/clusterConfiguration/utils.ts (2)
libs/ui-lib/lib/common/utils.ts (1)
isMajorMinorVersionEqualOrGreater(138-143)libs/types/assisted-installer-service.d.ts (1)
Cluster(110-407)
libs/ui-lib/lib/ocm/components/clusterConfiguration/review/ReviewNetworkingTable.tsx (1)
libs/ui-lib/lib/common/components/clusterConfiguration/utils.ts (1)
isDualStack(204-212)
libs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.tsx (1)
libs/ui-lib/lib/common/components/clusterConfiguration/utils.ts (1)
isDualStack(204-212)
libs/ui-lib/lib/common/components/ui/formik/validationSchemas.ts (2)
libs/types/assisted-installer-service.d.ts (1)
MachineNetwork(2182-2191)libs/ui-lib/lib/common/utils.ts (1)
isMajorMinorVersionEqualOrGreater(138-143)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts (3)
libs/ui-lib/lib/common/components/clusterConfiguration/utils.ts (1)
isDualStack(204-212)libs/ui-lib/lib/common/types/clusters.ts (1)
NetworkConfigurationValues(34-53)libs/ui-lib/lib/common/components/ui/formik/validationSchemas.ts (1)
dualStackValidationSchema(707-737)
🪛 Biome (2.1.2)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts
[error] 85-85: Do not add then to an object.
(lint/suspicious/noThenProperty)
[error] 95-95: Do not add then to an object.
(lint/suspicious/noThenProperty)
[error] 103-103: Do not add then to an object.
(lint/suspicious/noThenProperty)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: tests
- GitHub Check: circular-deps
- GitHub Check: lint
- GitHub Check: translation-files
- GitHub Check: format
- GitHub Check: unit-tests
libs/ui-lib/lib/common/components/ui/formik/validationSchemas.ts
Outdated
Show resolved
Hide resolved
linoyaslan
force-pushed
the
allow_dualstack_primary_ipv6
branch
from
821e267 to
8624d5a
Compare
openshift-ci
bot
added
size/XL
|
@linoyaslan: This pull request references MGMT-21625 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 task to target the "4.21.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. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
libs/ui-lib/lib/ocm/components/clusterConfiguration/review/ReviewNetworkingTable.tsx (1)
61-61: Simplify the isDualStack calls by passing cluster directly.The pattern
{ ...cluster, openshiftVersion: cluster.openshiftVersion }is redundant. Sincecluster.openshiftVersionis already included in the spread, explicitly setting it again is unnecessary.Apply this diff to simplify:
- isDualStack({ ...cluster, openshiftVersion: cluster.openshiftVersion }) ? { title: 'Primary' } : { title: '' }, + isDualStack(cluster) ? { title: 'Primary' } : { title: '' },- isDualStack({ ...cluster, openshiftVersion: cluster.openshiftVersion }) && { title: 'Primary' }, + isDualStack(cluster) && { title: 'Primary' },Apply the same pattern to lines 125 and 141.
Also applies to: 109-109, 125-125, 141-141
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
libs/ui-lib/lib/common/components/clusterConfiguration/utils.test.ts(1 hunks)libs/ui-lib/lib/common/components/clusterConfiguration/utils.ts(2 hunks)libs/ui-lib/lib/common/components/ui/formik/_validationSchemas.test.ts(2 hunks)libs/ui-lib/lib/common/components/ui/formik/validationSchemas.ts(3 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AvailableSubnetsControl.tsx(3 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkConfiguration.tsx(1 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkConfigurationForm.tsx(1 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts(3 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/review/ReviewNetworkingTable.tsx(4 hunks)libs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.test.ts(1 hunks)libs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- libs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.tsx
- libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkConfiguration.tsx
🧰 Additional context used
🧬 Code graph analysis (9)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkConfigurationForm.tsx (1)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts (1)
getNetworkConfigurationValidationSchema(66-110)
libs/ui-lib/lib/common/components/ui/formik/_validationSchemas.test.ts (1)
libs/ui-lib/lib/common/components/ui/formik/validationSchemas.ts (1)
dualStackValidationSchema(707-737)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts (3)
libs/ui-lib/lib/common/components/clusterConfiguration/utils.ts (1)
isDualStack(204-212)libs/ui-lib/lib/common/config/constants.ts (1)
IPV4_STACK(305-305)libs/ui-lib/lib/common/components/ui/formik/validationSchemas.ts (1)
dualStackValidationSchema(707-737)
libs/ui-lib/lib/common/components/clusterConfiguration/utils.ts (1)
libs/ui-lib/lib/common/utils.ts (1)
isMajorMinorVersionEqualOrGreater(138-143)
libs/ui-lib/lib/common/components/clusterConfiguration/utils.test.ts (2)
libs/types/assisted-installer-service.d.ts (3)
MachineNetwork(2182-2191)ClusterNetwork(620-633)ServiceNetwork(2577-2586)libs/ui-lib/lib/common/components/clusterConfiguration/utils.ts (1)
isDualStack(204-212)
libs/ui-lib/lib/common/components/ui/formik/validationSchemas.ts (2)
libs/types/assisted-installer-service.d.ts (1)
MachineNetwork(2182-2191)libs/ui-lib/lib/common/utils.ts (1)
isMajorMinorVersionEqualOrGreater(138-143)
libs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.test.ts (2)
libs/types/assisted-installer-service.d.ts (1)
Cluster(110-407)libs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.tsx (1)
getStackTypeLabel(43-44)
libs/ui-lib/lib/ocm/components/clusterConfiguration/review/ReviewNetworkingTable.tsx (1)
libs/ui-lib/lib/common/components/clusterConfiguration/utils.ts (1)
isDualStack(204-212)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AvailableSubnetsControl.tsx (4)
libs/ui-lib/lib/common/types/clusters.ts (1)
NetworkConfigurationValues(34-53)libs/ui-lib/lib/common/config/constants.ts (2)
DUAL_STACK(306-306)NO_SUBNET_SET(300-300)libs/ui-lib/lib/ocm/store/slices/current-cluster/selectors.ts (1)
selectCurrentClusterPermissionsState(9-17)libs/ui-lib/lib/common/utils.ts (1)
isMajorMinorVersionEqualOrGreater(138-143)
🪛 Biome (2.1.2)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts
[error] 85-85: Do not add then to an object.
(lint/suspicious/noThenProperty)
[error] 95-95: Do not add then to an object.
(lint/suspicious/noThenProperty)
[error] 103-103: Do not add then to an object.
(lint/suspicious/noThenProperty)
🔇 Additional comments (4)
libs/ui-lib/lib/common/components/ui/formik/_validationSchemas.test.ts (1)
352-494: LGTM! Comprehensive test coverage for version-aware dual-stack validation.The test suite thoroughly covers:
- Legacy behavior for OpenShift < 4.12
- New behavior for OpenShift >= 4.12 allowing IPv6-primary dual-stack
- Default fallback to legacy behavior when no version is specified
- Edge cases including network limits, empty CIDRs, and invalid configurations
The tests correctly validate the expected behavior and error messages.
libs/ui-lib/lib/common/components/clusterConfiguration/utils.test.ts (1)
1-276: LGTM! Excellent test coverage for version-aware dual-stack logic.The test suite is comprehensive and well-structured:
- Covers all OpenShift version scenarios (< 4.12, >= 4.12, no version)
- Tests both positive and negative cases for dual-stack configurations
- Includes edge cases for undefined networks, empty CIDRs, and invalid CIDRs
- Helper factories make tests readable and maintainable
The tests align perfectly with the updated
isDualStackimplementation.libs/ui-lib/lib/common/components/clusterConfiguration/utils.ts (2)
13-13: LGTM! Appropriate import for version comparison.The
isMajorMinorVersionEqualOrGreaterutility is correctly imported to support version-aware dual-stack logic.
181-212: LGTM! Version-aware dual-stack logic correctly implemented.The implementation properly handles version-based dual-stack requirements:
- OpenShift >= 4.12: Allows either IPv4-primary with IPv6-secondary OR IPv6-primary with IPv4-secondary
- Older versions: Requires IPv4-primary with IPv6-secondary only
- Edge cases: Properly handles undefined/empty networks with early returns
The
isDualStackfunction correctly propagates theopenshiftVersionto all three network type checks (machine, cluster, service), ensuring consistent version-aware validation across the board.
libs/ui-lib/lib/common/components/ui/formik/validationSchemas.ts
Outdated
Show resolved
Hide resolved
linoyaslan
force-pushed
the
allow_dualstack_primary_ipv6
branch
from
8624d5a to
870c827
Compare
|
@linoyaslan: This pull request references MGMT-21625 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 task to target the "4.21.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. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
libs/ui-lib/lib/common/components/ui/formik/validationSchemas.ts (1)
707-739: Fix IPv6-primary dual-stack validation messagingLine 712 still says “First network has to be IPv4 subnet.” and Line 728 still says “Second network has to be IPv6 subnet.” Even on OpenShift ≥4.12, when the form legitimately allows an IPv6 primary, these errors fire with the old copy, telling the user to pick the opposite family. In practice this blocks IPv6-primary clusters because the UI insists on IPv6 while the validator demands IPv4. Please switch to version-aware messages that reflect the actual rule. Example patch:
export const dualStackValidationSchema = (field: string, openshiftVersion?: string) => Yup.array() .max(2, `Maximum number of ${field} subnets in dual stack is 2.`) - .test( - 'dual-stack-ipv4', - 'First network has to be IPv4 subnet.', - (values?: { cidr: MachineNetwork['cidr'] }[]): boolean => { - // For OCP versions > 4.11, allow IPv6 as primary network - if (openshiftVersion && isMajorMinorVersionEqualOrGreater(openshiftVersion, '4.12')) { - return ( - !!values?.[0].cidr && - (Address4.isValid(values[0].cidr) || Address6.isValid(values[0].cidr)) - ); - } - // For older versions, require IPv4 as primary network - return !!values?.[0].cidr && Address4.isValid(values[0].cidr); - }, - ) - .test( - 'dual-stack-ipv6', - 'Second network has to be IPv6 subnet.', - (values?: { cidr: MachineNetwork['cidr'] }[]): boolean => { - // For OCP versions > 4.11, allow IPv4 as secondary network if primary is IPv6 - if (openshiftVersion && isMajorMinorVersionEqualOrGreater(openshiftVersion, '4.12')) { - const isFirstIPv6 = values?.[0].cidr && Address6.isValid(values[0].cidr); - if (isFirstIPv6) { - // If first is IPv6, second must be IPv4 - return !!values?.[1].cidr && Address4.isValid(values[1].cidr); - } - } - // Default behavior: second network must be IPv6 - return !!values?.[1].cidr && Address6.isValid(values[1].cidr); - }, - ); + .test('dual-stack-ipv4', function ( + this: Yup.TestContext, + values?: { cidr: MachineNetwork['cidr'] }[], + ) { + const primaryCidr = values?.[0]?.cidr; + const allowIPv6Primary = + openshiftVersion && isMajorMinorVersionEqualOrGreater(openshiftVersion, '4.12'); + + if (!primaryCidr) { + return this.createError({ message: 'Primary subnet is required.' }); + } + + if (allowIPv6Primary) { + if (!Address4.isValid(primaryCidr) && !Address6.isValid(primaryCidr)) { + return this.createError({ + message: 'Primary subnet must be a valid IPv4 or IPv6 CIDR.', + }); + } + return true; + } + + if (!Address4.isValid(primaryCidr)) { + return this.createError({ message: 'Primary subnet must be an IPv4 CIDR.' }); + } + + return true; + }) + .test('dual-stack-ipv6', function ( + this: Yup.TestContext, + values?: { cidr: MachineNetwork['cidr'] }[], + ) { + const primaryCidr = values?.[0]?.cidr; + const secondaryCidr = values?.[1]?.cidr; + const allowIPv6Primary = + openshiftVersion && isMajorMinorVersionEqualOrGreater(openshiftVersion, '4.12'); + const primaryIsIPv6 = !!primaryCidr && Address6.isValid(primaryCidr); + + if (!secondaryCidr) { + const message = + allowIPv6Primary && primaryIsIPv6 + ? 'Secondary subnet must be an IPv4 CIDR when the primary subnet is IPv6 on OpenShift 4.12+.' + : 'Secondary subnet must be an IPv6 CIDR.'; + return this.createError({ message }); + } + + if (allowIPv6Primary && primaryIsIPv6) { + if (!Address4.isValid(secondaryCidr)) { + return this.createError({ + message: + 'Secondary subnet must be an IPv4 CIDR when the primary subnet is IPv6 on OpenShift 4.12+.', + }); + } + return true; + } + + if (!Address6.isValid(secondaryCidr)) { + return this.createError({ message: 'Secondary subnet must be an IPv6 CIDR.' }); + } + + return true; + });
🧹 Nitpick comments (1)
libs/ui-lib/lib/common/components/clusterConfiguration/utils.ts (1)
181-202: Clarify dual-stack network count handling
Explicitly enforcenetworks.length === 2(returnfalseotherwise) or document that any entries beyond the first two are ignored when determining dual-stack.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
libs/ui-lib/lib/common/components/clusterConfiguration/utils.test.ts(1 hunks)libs/ui-lib/lib/common/components/clusterConfiguration/utils.ts(2 hunks)libs/ui-lib/lib/common/components/ui/formik/_validationSchemas.test.ts(2 hunks)libs/ui-lib/lib/common/components/ui/formik/validationSchemas.ts(2 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AvailableSubnetsControl.tsx(3 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkConfiguration.tsx(1 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkConfigurationForm.tsx(1 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts(4 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/review/ReviewNetworkingTable.tsx(4 hunks)libs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.test.ts(1 hunks)libs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- libs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.tsx
🧰 Additional context used
🧬 Code graph analysis (9)
libs/ui-lib/lib/common/components/ui/formik/_validationSchemas.test.ts (1)
libs/ui-lib/lib/common/components/ui/formik/validationSchemas.ts (1)
dualStackValidationSchema(707-740)
libs/ui-lib/lib/common/components/clusterConfiguration/utils.ts (2)
libs/ui-lib/lib/common/utils.ts (1)
isMajorMinorVersionEqualOrGreater(138-143)libs/types/assisted-installer-service.d.ts (1)
Cluster(110-407)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkConfigurationForm.tsx (1)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts (1)
getNetworkConfigurationValidationSchema(66-114)
libs/ui-lib/lib/ocm/components/clusterConfiguration/review/ReviewNetworkingTable.tsx (1)
libs/ui-lib/lib/common/components/clusterConfiguration/utils.ts (1)
isDualStack(204-214)
libs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.test.ts (2)
libs/types/assisted-installer-service.d.ts (1)
Cluster(110-407)libs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.tsx (1)
getStackTypeLabel(43-44)
libs/ui-lib/lib/common/components/ui/formik/validationSchemas.ts (2)
libs/types/assisted-installer-service.d.ts (1)
MachineNetwork(2182-2191)libs/ui-lib/lib/common/utils.ts (1)
isMajorMinorVersionEqualOrGreater(138-143)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AvailableSubnetsControl.tsx (4)
libs/ui-lib/lib/common/types/clusters.ts (1)
NetworkConfigurationValues(34-53)libs/ui-lib/lib/common/config/constants.ts (2)
DUAL_STACK(306-306)NO_SUBNET_SET(300-300)libs/ui-lib/lib/ocm/store/slices/current-cluster/selectors.ts (1)
selectCurrentClusterPermissionsState(9-17)libs/ui-lib/lib/common/utils.ts (1)
isMajorMinorVersionEqualOrGreater(138-143)
libs/ui-lib/lib/common/components/clusterConfiguration/utils.test.ts (2)
libs/types/assisted-installer-service.d.ts (3)
MachineNetwork(2182-2191)ClusterNetwork(620-633)ServiceNetwork(2577-2586)libs/ui-lib/lib/common/components/clusterConfiguration/utils.ts (1)
isDualStack(204-214)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts (4)
libs/ui-lib/lib/common/components/clusterConfiguration/utils.ts (1)
isDualStack(204-214)libs/ui-lib/lib/common/components/ui/formik/validationSchemas.ts (1)
dualStackValidationSchema(707-740)libs/ui-lib/lib/common/types/clusters.ts (1)
NetworkConfigurationValues(34-53)libs/ui-lib/lib/common/config/constants.ts (1)
IPV4_STACK(305-305)
🪛 Biome (2.1.2)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts
[error] 105-105: Do not add then to an object.
(lint/suspicious/noThenProperty)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: translation-files
- GitHub Check: tests
- GitHub Check: circular-deps
- GitHub Check: unit-tests
- GitHub Check: lint
- GitHub Check: format
🔇 Additional comments (10)
libs/ui-lib/lib/common/components/clusterConfiguration/utils.ts (2)
13-13: LGTM!The import of
isMajorMinorVersionEqualOrGreateris correct and necessary for the version-aware dual-stack logic.
204-214: LGTM!The function correctly propagates
openshiftVersionto all threeareNetworksDualStackcalls, maintaining the requirement that machine, cluster, and service networks must all be dual-stack for the cluster to be considered dual-stack.libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AvailableSubnetsControl.tsx (8)
12-12: LGTM!The import is necessary for version-aware IPv6 primary support.
39-39: LGTM!The
openshiftVersionprop addition is correctly typed as optional and properly destructured, allowing backward compatibility with existing code.Also applies to: 47-47
53-55: LGTM!The version check correctly determines IPv6 primary support for OpenShift 4.12+, with proper short-circuit evaluation to handle undefined
openshiftVersion.
64-67: LGTM!The
allSubnetslogic correctly combines IPv4 and IPv6 subnets for OpenShift 4.12+ while maintaining backward compatibility with the IPv4-only behavior for older versions.
69-72: Note the auto-selection behavior change.For OpenShift 4.12+, the first subnet from
allSubnets(which includes both IPv4 and IPv6) will be auto-selected. Depending on the sort order, this could default to an IPv6 subnet. This appears intentional for IPv6-primary support, but verify that this default selection aligns with user expectations.
77-77: LGTM!The label correctly indicates IPv6 primary capability for OpenShift 4.12+ dual-stack configurations.
85-121: LGTM with minor observation.The dual-stack subnet filtering logic correctly implements IPv6-primary support:
- First network: shows all available subnets (IPv4 or IPv6)
- Second network: intelligently filters to show only the opposite IP family
- Fallback: shows all subnets when the first network is not yet selected
For OpenShift versions older than 4.12, it preserves the original behavior (first network IPv4, second network IPv6).
Minor observation: If
machineNetworkscontains more than 2 networks (unusual for dual-stack), networks beyond the second will fall back to the older version logic. This is acceptable since dual-stack configurations typically have exactly 2 networks.
123-129: LGTM!The single-stack subnet selection correctly enables IPv6-only clusters for OpenShift 4.12+ by allowing selection from all available subnets, while maintaining IPv4-only behavior for older versions.
linoyaslan
force-pushed
the
allow_dualstack_primary_ipv6
branch
from
870c827 to
414a9bb
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
libs/ui-lib/lib/common/components/ui/formik/_validationSchemas.test.ts (1)
357-361: Prefer structural equality for Yup results
schema.validatemay return a cloned array, soresolves.toBe(value)can start failing despite equivalent content. Switching toresolves.toEqual(value)keeps the assertion stable without relying on reference identity.- await expect(schema.validate(value)).resolves.toBe(value); + await expect(schema.validate(value)).resolves.toEqual(value);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
libs/ui-lib/lib/common/components/clusterConfiguration/utils.test.ts(1 hunks)libs/ui-lib/lib/common/components/clusterConfiguration/utils.ts(2 hunks)libs/ui-lib/lib/common/components/ui/formik/_validationSchemas.test.ts(2 hunks)libs/ui-lib/lib/common/components/ui/formik/validationSchemas.ts(3 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AvailableSubnetsControl.tsx(3 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkConfiguration.tsx(1 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkConfigurationForm.tsx(1 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts(4 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/review/ReviewNetworkingTable.tsx(4 hunks)libs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.test.ts(1 hunks)libs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- libs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.tsx
- libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkConfiguration.tsx
- libs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.test.ts
- libs/ui-lib/lib/ocm/components/clusterConfiguration/review/ReviewNetworkingTable.tsx
- libs/ui-lib/lib/common/components/clusterConfiguration/utils.test.ts
🧰 Additional context used
🧬 Code graph analysis (6)
libs/ui-lib/lib/common/components/clusterConfiguration/utils.ts (2)
libs/ui-lib/lib/common/utils.ts (1)
isMajorMinorVersionEqualOrGreater(138-143)libs/types/assisted-installer-service.d.ts (1)
Cluster(110-407)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AvailableSubnetsControl.tsx (4)
libs/ui-lib/lib/common/types/clusters.ts (1)
NetworkConfigurationValues(34-53)libs/ui-lib/lib/common/config/constants.ts (2)
DUAL_STACK(306-306)NO_SUBNET_SET(300-300)libs/ui-lib/lib/ocm/store/slices/current-cluster/selectors.ts (1)
selectCurrentClusterPermissionsState(9-17)libs/ui-lib/lib/common/utils.ts (1)
isMajorMinorVersionEqualOrGreater(138-143)
libs/ui-lib/lib/common/components/ui/formik/_validationSchemas.test.ts (1)
libs/ui-lib/lib/common/components/ui/formik/validationSchemas.ts (1)
dualStackValidationSchema(707-744)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkConfigurationForm.tsx (1)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts (1)
getNetworkConfigurationValidationSchema(66-114)
libs/ui-lib/lib/common/components/ui/formik/validationSchemas.ts (2)
libs/ui-lib/lib/common/utils.ts (1)
isMajorMinorVersionEqualOrGreater(138-143)libs/types/assisted-installer-service.d.ts (1)
MachineNetwork(2182-2191)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts (4)
libs/ui-lib/lib/common/components/clusterConfiguration/utils.ts (1)
isDualStack(204-214)libs/ui-lib/lib/common/components/ui/formik/validationSchemas.ts (4)
dualStackValidationSchema(707-744)clusterNetworksValidationSchema(686-696)serviceNetworkValidationSchema(698-705)IPv4ValidationSchema(746-750)libs/ui-lib/lib/common/types/clusters.ts (1)
NetworkConfigurationValues(34-53)libs/ui-lib/lib/common/config/constants.ts (1)
IPV4_STACK(305-305)
🪛 Biome (2.1.2)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts
[error] 105-105: Do not add then to an object.
(lint/suspicious/noThenProperty)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: circular-deps
- GitHub Check: tests
- GitHub Check: translation-files
- GitHub Check: unit-tests
- GitHub Check: lint
- GitHub Check: format
🔇 Additional comments (6)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AvailableSubnetsControl.tsx (6)
12-12: LGTM! Clean integration of version awareness.The addition of
openshiftVersionas an optional prop and the import of the version comparison utility are well-structured. The optional nature maintains backwards compatibility while enabling the new IPv6-primary functionality.Also applies to: 39-39, 47-47
53-67: Version-aware subnet handling looks solid.The check for OCP >= 4.12 correctly enables IPv6-primary support, and the conditional merging of subnet arrays is well-implemented. The fallback to
IPv4Subnetsfor older versions maintains backwards compatibility.
77-77: LGTM! Clear user-facing label.The dynamic label correctly indicates the enhanced flexibility of IPv4 or IPv6 selection for OpenShift 4.12+, while maintaining the simpler label for older versions.
126-126: LGTM! Enables IP family flexibility for single-stack.For OpenShift 4.12+, single-stack clusters can now use either IPv4 or IPv6 subnets, providing users with the flexibility to choose their preferred IP family. This aligns well with the IPv6-primary support objective.
88-106: Verify form validation for opposite‐family subnet availability
The filtering logic for the second network shows only the opposite IP family, but if no subnets exist for that family, the dropdown will be empty. Ensure form validation or UI feedback prevents or explains this scenario.
69-69: Verify and document IPv6-first auto-selection
No tests or documentation cover selecting the first subnet from an alphabetically sorted IPv4/IPv6 list. Confirm this IPv6-first behavior is intentional and add tests or docs to capture it.
linoyaslan
force-pushed
the
allow_dualstack_primary_ipv6
branch
from
414a9bb to
8383e98
Compare
|
@linoyaslan: This pull request references MGMT-21625 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 task to target the "4.21.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. |
linoyaslan
force-pushed
the
allow_dualstack_primary_ipv6
branch
2 times, most recently
from
698d986 to
b3bbac2
Compare
|
@linoyaslan: This pull request references MGMT-21625 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 task to target the "4.21.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. |
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)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AdvancedNetworkFields.tsx (1)
44-57: Index-based IPv6 detection breaks with IPv6‑primary; use CIDR family instead.When IPv6 is primary (index 0),
isSubnetIPv6(index)returns false, yielding wrong popover text andmax(IPv4 limits) for an IPv6 CIDR. Compute by CIDR, not index.@@ - const isSubnetIPv6 = (index: number) => (isDualStack ? !!index : false); + // Determine family from the actual CIDR + const isSubnetIPv6 = (index: number) => + Address6.isValid(values.clusterNetworks?.[index]?.cidr || ''); @@ - max={ - isSubnetIPv6(index) - ? PREFIX_MAX_RESTRICTION.IPv6 - : PREFIX_MAX_RESTRICTION.IPv4 - } + max={ + isSubnetIPv6(index) + ? PREFIX_MAX_RESTRICTION.IPv6 + : PREFIX_MAX_RESTRICTION.IPv4 + }Add the missing import at the top:
import { Alert, AlertVariant, FormGroup, GridItem, TextInputTypes, Grid, } from '@patternfly/react-core'; +import { Address6 } from 'ip-address';Also applies to: 100-103
♻️ Duplicate comments (1)
libs/ui-lib/lib/common/components/ui/formik/validationSchemas.ts (1)
707-726: Dual‑stack schema does not require a second subnet or enforce opposite family.Currently: only “first subnet is IPv4 or IPv6 (≥4.12) / IPv4 (<4.12)” and
max(2). Missing:min(2)and a rule that the second subnet exists and is the opposite family (IPv4↔IPv6). This allows invalid single‑entry or same‑family arrays.export const dualStackValidationSchema = (field: string, openshiftVersion?: string) => - Yup.array() - .max(2, `Maximum number of ${field} subnets in dual stack is 2.`) - .test( - 'dual-stack-ipv4', - openshiftVersion && isMajorMinorVersionEqualOrGreater(openshiftVersion, '4.12') - ? 'First network has to be a valid IPv4 or IPv6 subnet.' - : 'First network has to be IPv4 subnet.', - (values?: { cidr: MachineNetwork['cidr'] }[]): boolean => { - // For OCP versions > 4.11, allow IPv6 as primary network - if (openshiftVersion && isMajorMinorVersionEqualOrGreater(openshiftVersion, '4.12')) { - return ( - !!values?.[0].cidr && - (Address4.isValid(values[0].cidr) || Address6.isValid(values[0].cidr)) - ); - } - // For older versions, require IPv4 as primary network - return !!values?.[0].cidr && Address4.isValid(values[0].cidr); - }, - ); + Yup.array() + .min(2, `Dual-stack requires exactly 2 ${field} subnets (primary and secondary).`) + .max(2, `Maximum number of ${field} subnets in dual stack is 2.`) + .test( + 'dual-stack-primary', + openshiftVersion && isMajorMinorVersionEqualOrGreater(openshiftVersion, '4.12') + ? 'Primary subnet must be a valid IPv4 or IPv6 CIDR.' + : 'Primary subnet must be a valid IPv4 CIDR.', + (values?: { cidr?: MachineNetwork['cidr'] }[]): boolean => { + const first = values?.[0]?.cidr || ''; + if (openshiftVersion && isMajorMinorVersionEqualOrGreater(openshiftVersion, '4.12')) { + return Address4.isValid(first) || Address6.isValid(first); + } + return Address4.isValid(first); + }, + ) + .test( + 'dual-stack-secondary-opposite', + openshiftVersion && isMajorMinorVersionEqualOrGreater(openshiftVersion, '4.12') + ? 'Secondary subnet must be the opposite IP family of the primary.' + : 'Secondary subnet must be a valid IPv6 CIDR.', + (values?: { cidr?: MachineNetwork['cidr'] }[]): boolean => { + const first = values?.[0]?.cidr || ''; + const second = values?.[1]?.cidr || ''; + if (!second) return false; + if (openshiftVersion && isMajorMinorVersionEqualOrGreater(openshiftVersion, '4.12')) { + return ( + (Address4.isValid(first) && Address6.isValid(second)) || + (Address6.isValid(first) && Address4.isValid(second)) + ); + } + return Address4.isValid(first) && Address6.isValid(second); + }, + );
🧹 Nitpick comments (3)
libs/ui-lib/lib/ocm/components/clusterConfiguration/review/ReviewNetworkingTable.tsx (1)
61-70: Consider simplifying the isDualStack call pattern.The pattern
isDualStack({ ...cluster, openshiftVersion: cluster.openshiftVersion })spreads cluster and then explicitly sets openshiftVersion. Ifcluster.openshiftVersionalready exists on the cluster object, the explicit property assignment is redundant.Consider simplifying to just
isDualStack(cluster)if the Cluster type includes openshiftVersion, or if explicit passing is required for type safety, document why.Apply this pattern if Cluster type includes openshiftVersion:
-isDualStack({ ...cluster, openshiftVersion: cluster.openshiftVersion }) +isDualStack(cluster)Also applies to: 118-125, 141-148, 164-171
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts (1)
37-37: Consider simplifying the isDualStack call pattern.Similar to the pattern in ReviewNetworkingTable.tsx,
isDualStack({ ...cluster, openshiftVersion: cluster.openshiftVersion })is redundant if the Cluster type already includes openshiftVersion.If the Cluster type includes openshiftVersion, simplify to:
-const isDualStackType = isDualStack({ ...cluster, openshiftVersion: cluster.openshiftVersion }); +const isDualStackType = isDualStack(cluster);libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AdvancedNetworkFields.tsx (1)
63-63: Avoid duplicate “Primary/Secondary” labels.Both the FormGroup and each field render
labelInfoas “Primary/Secondary”, causing redundancy. Prefer per‑fieldlabelInfoand drop it from the FormGroup for clarity.- <FormGroup fieldId="clusterNetworks" labelInfo={isDualStack && 'Primary'}> + <FormGroup fieldId="clusterNetworks"> @@ - <FormGroup fieldId="serviceNetworks" labelInfo={isDualStack && 'Primary'}> + <FormGroup fieldId="serviceNetworks">Also applies to: 121-121, 80-82, 136-138
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
libs/ui-lib/lib/common/components/clusterConfiguration/utils.test.ts(1 hunks)libs/ui-lib/lib/common/components/clusterConfiguration/utils.ts(2 hunks)libs/ui-lib/lib/common/components/ui/formik/_validationSchemas.test.ts(2 hunks)libs/ui-lib/lib/common/components/ui/formik/validationSchemas.ts(3 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AdvancedNetworkFields.tsx(2 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AvailableSubnetsControl.tsx(2 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkConfiguration.tsx(1 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkConfigurationForm.tsx(1 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/SubnetsDropdown.tsx(3 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/VirtualIPControlGroup.tsx(4 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts(4 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/review/ReviewNetworkingTable.tsx(4 hunks)libs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.test.ts(1 hunks)libs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.tsx(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- libs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- libs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.tsx
🧰 Additional context used
🧬 Code graph analysis (9)
libs/ui-lib/lib/common/components/ui/formik/_validationSchemas.test.ts (1)
libs/ui-lib/lib/common/components/ui/formik/validationSchemas.ts (1)
dualStackValidationSchema(707-726)
libs/ui-lib/lib/ocm/components/clusterConfiguration/review/ReviewNetworkingTable.tsx (1)
libs/ui-lib/lib/common/components/clusterConfiguration/utils.ts (1)
isDualStack(204-214)
libs/ui-lib/lib/common/components/clusterConfiguration/utils.ts (2)
libs/ui-lib/lib/common/utils.ts (1)
isMajorMinorVersionEqualOrGreater(138-143)libs/types/assisted-installer-service.d.ts (1)
Cluster(110-407)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AvailableSubnetsControl.tsx (3)
libs/ui-lib/lib/common/types/clusters.ts (1)
NetworkConfigurationValues(34-53)libs/ui-lib/lib/common/utils.ts (1)
isMajorMinorVersionEqualOrGreater(138-143)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/SubnetsDropdown.tsx (1)
SubnetsDropdown(43-168)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts (4)
libs/ui-lib/lib/common/components/clusterConfiguration/utils.ts (1)
isDualStack(204-214)libs/ui-lib/lib/common/components/ui/formik/validationSchemas.ts (4)
dualStackValidationSchema(707-726)clusterNetworksValidationSchema(686-696)serviceNetworkValidationSchema(698-705)IPv4ValidationSchema(728-732)libs/ui-lib/lib/common/types/clusters.ts (1)
NetworkConfigurationValues(34-53)libs/ui-lib/lib/common/config/constants.ts (1)
IPV4_STACK(299-299)
libs/ui-lib/lib/common/components/clusterConfiguration/utils.test.ts (2)
libs/types/assisted-installer-service.d.ts (3)
MachineNetwork(2182-2191)ClusterNetwork(620-633)ServiceNetwork(2577-2586)libs/ui-lib/lib/common/components/clusterConfiguration/utils.ts (1)
isDualStack(204-214)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AdvancedNetworkFields.tsx (1)
libs/ui-lib/lib/common/components/clusterConfiguration/utils.ts (1)
isDualStack(204-214)
libs/ui-lib/lib/common/components/ui/formik/validationSchemas.ts (2)
libs/ui-lib/lib/common/utils.ts (1)
isMajorMinorVersionEqualOrGreater(138-143)libs/types/assisted-installer-service.d.ts (1)
MachineNetwork(2182-2191)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkConfigurationForm.tsx (1)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts (1)
getNetworkConfigurationValidationSchema(66-114)
🪛 Biome (2.1.2)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts
[error] 105-105: Do not add then to an object.
(lint/suspicious/noThenProperty)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: tests
🔇 Additional comments (9)
libs/ui-lib/lib/common/components/clusterConfiguration/utils.test.ts (1)
1-275: LGTM! Comprehensive test coverage for version-aware dual-stack logic.The test suite thoroughly covers:
- Legacy behavior (OCP < 4.12): IPv4 primary only
- New behavior (OCP >= 4.12): Both IPv4 and IPv6 can be primary
- Default behavior when version is unspecified
- Edge cases: undefined networks, empty CIDRs, invalid CIDRs
The factory helpers and test organization make the suite maintainable and clear.
libs/ui-lib/lib/common/components/ui/formik/_validationSchemas.test.ts (1)
352-431: LGTM! Test coverage aligns with version-aware dual-stack validation.The test suite properly exercises:
- Legacy behavior (OCP < 4.12): rejects IPv6 as primary with "First network has to be IPv4 subnet"
- New behavior (OCP >= 4.12): accepts both IPv4 and IPv6 as primary
- Default behavior: falls back to legacy behavior
- Edge case: validates maximum 2 networks constraint
The error message expectations match the implementation shown in the relevant code snippets.
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkConfiguration.tsx (1)
295-295: LGTM! Version propagation enables IPv6-primary support in subnet control.The openshiftVersion prop allows AvailableSubnetsControl to determine IPv6 primary support based on the OpenShift version, aligning with the broader PR objective.
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkConfigurationForm.tsx (1)
203-205: LGTM! Validation schema now receives version for dual-stack rules.The openshiftVersion is correctly passed to the validation schema and included in the memoization dependencies, enabling version-aware dual-stack validation.
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts (1)
69-69: LGTM! Version-aware validation properly propagated.The openshiftVersion parameter is correctly added to the schema function signature and consistently passed to all three dual-stack validation calls (machine networks, cluster networks, service networks).
Note: The Biome linter warning at line 105 about "Do not add then to an object" is a false positive—this is Yup's fluent API
.when().then()pattern, not a custom property.Also applies to: 89-89, 99-100, 109-110
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/SubnetsDropdown.tsx (2)
53-77: LGTM! IPv4/IPv6 subnet separation is clean and well-memoized.The logic correctly separates subnets by IP version using the
ip-addresslibrary and maintains the original itemsSubnets for display value lookup.
79-134: Well-structured dropdown with IPv4/IPv6 grouping.The conditional rendering logic properly handles three cases:
- No subnets: shows unavailable message
- Both IPv4 and IPv6: groups them with labels
- Single IP version: flat list without grouping
The implementation enhances UX for dual-stack configurations where users need to distinguish between IPv4 and IPv6 subnets.
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/VirtualIPControlGroup.tsx (1)
146-146: Verify that static "API IP" and "Ingress IP" labels appropriately represent the actual IP version(s) in dual-stack configurations.The concern is valid. Backend Cluster type explicitly supports dual-stack with "up to two for dual-stack clusters (at most one IP address per IP stack used)," but the UI displays only the primary VIP (first array element) without indicating whether it's IPv4 or IPv6. While
Address4andAddress6validation utilities are available in the codebase, labels inVirtualIPControlGroup.tsx(lines 146, 165, 192, 209) don't leverage them to provide version context. Other UI components (e.g.,SubnetsDropdown.tsx) use explicit "IPv4"/"IPv6" labels, establishing a precedent.In a dual-stack cluster with IPv6 as primary, displaying "Ingress IP" without indicating the version could create confusion. Either dynamically detect the IP version and update labels, or explicitly document that labels represent the primary VIP regardless of version.
libs/ui-lib/lib/common/components/clusterConfiguration/utils.ts (1)
181-202: LGTM: version‑aware dual‑stack detection is correct.Opposite‑family pairing for ≥4.12 and IPv4→IPv6 for older versions is implemented clearly and consistently with
isDualStack.Please ensure all callers pass
openshiftVersionwhen available so UI indicators remain consistent.Also applies to: 204-214
...lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AvailableSubnetsControl.tsx
Show resolved
Hide resolved
linoyaslan
force-pushed
the
allow_dualstack_primary_ipv6
branch
from
b3bbac2 to
e586faa
Compare
|
@linoyaslan: This pull request references MGMT-21625 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 task to target the "4.21.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. |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AdvancedNetworkFields.tsx (1)
44-56: Bug confirmed: IPv6 detection assumes index order; breaks on IPv6-primaryFor OCP ≥ 4.12, index 0 can be IPv6. The current implementation uses index position instead of CIDR validation, causing wrong popover text and incorrect hostPrefix bounds.
Apply fix at lines 44 and used at lines 50 and 99-101:
+import { Address6 } from 'ip-address'; - const isSubnetIPv6 = (index: number) => (isDualStack ? !!index : false); + const isSubnetIPv6 = (index: number) => { + const cidr = values.clusterNetworks?.[index]?.cidr || ''; + return Address6.isValid(cidr); + };This detects the actual IP family from CIDR instead of assuming index order, while preserving the existing Primary/Secondary labels.
libs/ui-lib/lib/common/components/clusterConfiguration/utils.ts (1)
71-79: Address4/Address6.isValid() methods are not available in ip-address v7.1.0 (installed version)—code is broken.The workspace has ip-address v7.1.0 installed, which removed the
isValid()static method. The code at lines 72, 74 (and 33 instances across the codebase) will fail at runtime. The codebase already demonstrates the correct pattern incommonValidationSchemas.tsx: use try/catch with the constructor instead.Refactor
getSubnetand related functions inutils.ts(lines 71–79, 91–97, and other locations usingAddress4.isValid()orAddress6.isValid()) to use the try/catch pattern:const isValidIPv4 = (cidr: string) => { try { new Address4(cidr); return true; } catch { return false; } };
♻️ Duplicate comments (4)
libs/ui-lib/lib/common/components/ui/formik/validationSchemas.ts (1)
707-726: Confirm second‑family enforcement remains intactFirst‑subnet rule is now version‑aware. Please confirm the schema also enforces that the second subnet is the opposite family (IPv4↔IPv6), including the IPv6‑primary flip for OCP ≥ 4.12. If already addressed in recent commits, no action needed.
Run to verify:
#!/bin/bash # Search for the complementary dual-stack test enforcing second-family rules rg -n -C2 -P "dual-stack-(ipv6|ipv4).*Second network|Second.*(IPv4|IPv6).*dual" libs/ui-lib/lib/common/components/ui/formik/validationSchemas.ts # Also ensure the schema is used for machine/cluster/service networks rg -n "dualStackValidationSchema\\(" libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfigurationlibs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AvailableSubnetsControl.tsx (3)
64-70: Prefer IPv4 as default primary; fallback to IPv6.Current default uses
allSubnets[0]which depends on string sorting. Pick first IPv4 when present for predictable UX.- const allSubnets = supportsIPv6Primary - ? [...IPv4Subnets, ...IPv6Subnets].sort(subnetSort) - : IPv4Subnets; - - const cidr = allSubnets.length >= 1 ? allSubnets[0].subnet : NO_SUBNET_SET; + const allSubnets = supportsIPv6Primary + ? [...IPv4Subnets, ...IPv6Subnets].sort(subnetSort) + : IPv4Subnets; + const primaryDefault = IPv4Subnets[0] || IPv6Subnets[0]; + const cidr = primaryDefault ? primaryDefault.subnet : NO_SUBNET_SET;
89-99: Fix misleading test IDs; use primary/secondary semantics.IDs shouldn’t assume family or rely on index. Align with “primary/secondary”.
- <SubnetsDropdown + <SubnetsDropdown name={`machineNetworks.${index}.cidr`} machineSubnets={machineSubnets} isDisabled={isDisabled} - data-testid={`subnets-dropdown-toggle-${index ? 'ipv6' : 'ipv4'}`} + data-testid={'subnets-dropdown-toggle-primary'} /> ... - <SubnetsDropdown + <SubnetsDropdown name={`machineNetworks.${index}.cidr`} machineSubnets={machineSubnets} isDisabled={isDisabled} - data-testid={`subnets-dropdown-toggle-${index ? 'ipv6' : 'ipv4'}`} + data-testid={'subnets-dropdown-toggle-secondary'} />Also applies to: 151-157
103-110: Single‑stack dropdown must not offer IPv6 (validation forbids it).UI currently shows IPv6 in single‑stack when ≥4.12, but validation enforces IPv4‑only. Restrict single‑stack to IPv4 to avoid UX dead‑ends.
- <SubnetsDropdown - name={`machineNetworks.0.cidr`} - machineSubnets={supportsIPv6Primary ? allSubnets : IPv4Subnets} - isDisabled={isDisabled} - data-testid={'subnets-dropdown-toggle-ipv4'} - /> + <SubnetsDropdown + name={`machineNetworks.0.cidr`} + machineSubnets={IPv4Subnets} + isDisabled={isDisabled} + data-testid={'subnets-dropdown-toggle-primary'} + />If you intend to allow single‑stack IPv6 on ≥4.12, update the validation to drop IPv4ValidationSchema accordingly.
🧹 Nitpick comments (5)
libs/ui-lib/lib/common/components/clusterConfiguration/utils.test.ts (1)
1-275: Robust coverage across versions — niceScenarios look comprehensive for legacy vs 4.12+. Consider adding a case where more than two subnets are present to ensure utils stays strict.
libs/ui-lib/lib/ocm/components/clusterConfiguration/review/ReviewNetworkingTable.tsx (1)
61-71: Dual-stack Primary/Secondary labeling — LGTMConditional labels for machine/cluster/service rows align with version-aware dual‑stack. Consider wrapping “Primary/Secondary” in i18n later.
Also applies to: 118-126, 141-149, 164-172
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts (1)
83-93: Silence Biome’s noThenProperty on Yup.when blocks.Biome flags “then” in object literals; this is idiomatic with Yup.when. Add targeted ignore comments.
- : machineNetworksValidationSchema.when('stackType', { + : machineNetworksValidationSchema.when('stackType', { + /* biome-ignore lint/suspicious/noThenProperty: Yup.when requires then/otherwise keys */ is: (stackType: NetworkConfigurationValues['stackType']) => stackType === IPV4_STACK, then: () => machineNetworksValidationSchema.concat(IPv4ValidationSchema), otherwise: () => values.machineNetworks && values.machineNetworks?.length >= 2 ? machineNetworksValidationSchema.concat( dualStackValidationSchema('machine networks', openshiftVersion), ) : Yup.array(), }), - clusterNetworks: clusterNetworksValidationSchema.when('stackType', { + clusterNetworks: clusterNetworksValidationSchema.when('stackType', { + /* biome-ignore lint/suspicious/noThenProperty: Yup.when requires then/otherwise keys */ is: (stackType: NetworkConfigurationValues['stackType']) => stackType === IPV4_STACK, then: () => clusterNetworksValidationSchema.concat(IPv4ValidationSchema), otherwise: () => values.clusterNetworks && values.clusterNetworks?.length >= 2 ? clusterNetworksValidationSchema.concat( dualStackValidationSchema('cluster network', openshiftVersion), ) : Yup.array(), }), - serviceNetworks: serviceNetworkValidationSchema.when('stackType', { + serviceNetworks: serviceNetworkValidationSchema.when('stackType', { + /* biome-ignore lint/suspicious/noThenProperty: Yup.when requires then/otherwise keys */ is: (stackType: NetworkConfigurationValues['stackType']) => stackType === IPV4_STACK, then: () => serviceNetworkValidationSchema.concat(IPv4ValidationSchema), otherwise: () => values.serviceNetworks && values.serviceNetworks?.length >= 2 ? serviceNetworkValidationSchema.concat( dualStackValidationSchema('service network', openshiftVersion), ) : Yup.array(), }),Also applies to: 93-101, 104-111
libs/ui-lib/lib/common/components/clusterConfiguration/utils.ts (1)
181-202: Optional: tolerate >2 entries by checking first valid pair.Current check inspects only [0] and [1]. If arrays accidentally contain >2 entries, a valid dual‑stack pair later won’t be considered. Consider scanning first two valid, distinct‑family entries.
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AvailableSubnetsControl.tsx (1)
53-56: Coerce supportsIPv6Primary to boolean.Avoid string|boolean type from
openshiftVersion && ....- const supportsIPv6Primary = - openshiftVersion && isMajorMinorVersionEqualOrGreater(openshiftVersion, '4.12'); + const supportsIPv6Primary = + !!openshiftVersion && isMajorMinorVersionEqualOrGreater(openshiftVersion, '4.12');
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
libs/ui-lib/lib/common/components/clusterConfiguration/utils.test.ts(1 hunks)libs/ui-lib/lib/common/components/clusterConfiguration/utils.ts(2 hunks)libs/ui-lib/lib/common/components/ui/formik/_validationSchemas.test.ts(2 hunks)libs/ui-lib/lib/common/components/ui/formik/validationSchemas.ts(3 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AdvancedNetworkFields.tsx(2 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AvailableSubnetsControl.tsx(2 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkConfiguration.tsx(2 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkConfigurationForm.tsx(1 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/SubnetsDropdown.tsx(3 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/VirtualIPControlGroup.tsx(4 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts(4 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/review/ReviewNetworkingTable.tsx(4 hunks)libs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.test.ts(1 hunks)libs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- libs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.tsx
- libs/ui-lib/lib/common/components/ui/formik/_validationSchemas.test.ts
🧰 Additional context used
🧬 Code graph analysis (10)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AdvancedNetworkFields.tsx (1)
libs/ui-lib/lib/common/components/clusterConfiguration/utils.ts (1)
isDualStack(204-214)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkConfiguration.tsx (1)
libs/ui-lib/lib/common/components/clusterConfiguration/utils.ts (1)
isDualStack(204-214)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AvailableSubnetsControl.tsx (5)
libs/ui-lib/lib/common/types/clusters.ts (1)
NetworkConfigurationValues(34-53)libs/ui-lib/lib/common/config/constants.ts (2)
DUAL_STACK(300-300)NO_SUBNET_SET(294-294)libs/ui-lib/lib/ocm/store/slices/current-cluster/selectors.ts (1)
selectCurrentClusterPermissionsState(9-17)libs/ui-lib/lib/common/utils.ts (1)
isMajorMinorVersionEqualOrGreater(138-143)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/SubnetsDropdown.tsx (1)
SubnetsDropdown(43-168)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkConfigurationForm.tsx (1)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts (1)
getNetworkConfigurationValidationSchema(66-114)
libs/ui-lib/lib/common/components/ui/formik/validationSchemas.ts (2)
libs/ui-lib/lib/common/utils.ts (1)
isMajorMinorVersionEqualOrGreater(138-143)libs/types/assisted-installer-service.d.ts (1)
MachineNetwork(2182-2191)
libs/ui-lib/lib/ocm/components/clusterConfiguration/review/ReviewNetworkingTable.tsx (1)
libs/ui-lib/lib/common/components/clusterConfiguration/utils.ts (1)
isDualStack(204-214)
libs/ui-lib/lib/common/components/clusterConfiguration/utils.test.ts (2)
libs/types/assisted-installer-service.d.ts (3)
MachineNetwork(2182-2191)ClusterNetwork(620-633)ServiceNetwork(2577-2586)libs/ui-lib/lib/common/components/clusterConfiguration/utils.ts (1)
isDualStack(204-214)
libs/ui-lib/lib/common/components/clusterConfiguration/utils.ts (1)
libs/ui-lib/lib/common/utils.ts (1)
isMajorMinorVersionEqualOrGreater(138-143)
libs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.test.ts (2)
libs/types/assisted-installer-service.d.ts (1)
Cluster(110-407)libs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.tsx (1)
getStackTypeLabel(43-44)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts (4)
libs/ui-lib/lib/common/components/clusterConfiguration/utils.ts (1)
isDualStack(204-214)libs/ui-lib/lib/common/components/ui/formik/validationSchemas.ts (4)
dualStackValidationSchema(707-726)clusterNetworksValidationSchema(686-696)serviceNetworkValidationSchema(698-705)IPv4ValidationSchema(728-732)libs/ui-lib/lib/common/types/clusters.ts (1)
NetworkConfigurationValues(34-53)libs/ui-lib/lib/common/config/constants.ts (1)
IPV4_STACK(299-299)
🪛 Biome (2.1.2)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts
[error] 105-105: Do not add then to an object.
(lint/suspicious/noThenProperty)
🔇 Additional comments (10)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/VirtualIPControlGroup.tsx (1)
146-147: Labels updated for IPv6‑primary — looks goodDropping the “(IPv4)” suffix from API/Ingress labels aligns with IPv6‑primary support. Please ensure any tests/selectors that matched old label text are updated.
Also applies to: 165-167, 192-195, 209-211
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AdvancedNetworkFields.tsx (1)
72-83: Static labels/labelInfo changes LGTMThe new “Cluster network CIDR/host prefix/Service network CIDR” labels and Primary/Secondary labelInfo read clearly for dual‑stack.
Also applies to: 90-96, 128-133, 136-138
libs/ui-lib/lib/common/components/ui/formik/validationSchemas.ts (1)
321-325: VIP array schema tweak LGTMValidating ip without suffix per entry under clusterManaged is correct and consistent with other VIP checks.
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkConfiguration.tsx (1)
226-231: Version-aware wiring and advanced toggle — LGTMAuto‑enabling advanced for dual‑stack and passing openshiftVersion to AvailableSubnetsControl are consistent with IPv6‑primary support.
Also applies to: 295-296
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkConfigurationForm.tsx (1)
203-206: Validation schema now version-aware — LGTMIncluding cluster.openshiftVersion in the memoized schema ensures correct revalidation on version changes.
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts (1)
37-37: Good: version-aware isDualStack initialization.Passing openshiftVersion into isDualStack keeps initial stackType consistent with OCP rules.
libs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.test.ts (1)
28-80: Solid coverage of pre/post‑4.12 and edge cases.Tests exercise ordering and undefined cases appropriately, aligning with new isDualStack behavior.
Also applies to: 82-170, 172-215
libs/ui-lib/lib/common/components/clusterConfiguration/utils.ts (2)
181-202: Version-aware dual‑stack logic looks correct.Allows IPv6‑primary on ≥4.12; enforces IPv4‑primary on older or unknown versions. Early length check avoids OOB.
204-214: All upstream callers properly updated with openshiftVersion parameter.Verification confirms all 7 production call sites across ClusterProperties, ReviewNetworkingTable, and networkConfigurationValidation have been updated to pass
openshiftVersion: cluster.openshiftVersion. Tests correctly exercise both optional and explicit parameter scenarios.libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AvailableSubnetsControl.tsx (1)
133-149: Secondary subnet filtering logic is good.Filters opposite family on ≥4.12 and IPv6-only for older versions; sensible UX.
...b/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts
Show resolved
Hide resolved
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/SubnetsDropdown.tsx
Show resolved
Hide resolved
linoyaslan
force-pushed
the
allow_dualstack_primary_ipv6
branch
from
e586faa to
cbfe9c3
Compare
|
@linoyaslan: This pull request references MGMT-21625 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 task to target the "4.21.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. |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AdvancedNetworkFields.tsx (1)
44-56: Primary IPv6 breaks hostPrefix limits and popover examples — detect family from CIDR, not index.
isSubnetIPv6(index) => !!indexassumes IPv6 is always second. With IPv6‑primary (OCP ≥ 4.12), index 0 can be IPv6, so max prefix and popover text are wrong for the first row.Apply this diff:
- const isSubnetIPv6 = (index: number) => (isDualStack ? !!index : false); + const isSubnetIPv6 = (index: number) => { + const cidr = values.clusterNetworks?.[index]?.cidr ?? ''; + // cheap, dependency-free check; alternative: use Address6.isValid(cidr) + return cidr.includes(':'); + };This ensures the IPv6/IPv4 max (
PREFIX_MAX_RESTRICTION) and the popover examples match the actual subnet family regardless of order.Also applies to: 98-104
🧹 Nitpick comments (11)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AdvancedNetworkFields.tsx (2)
63-65: Duplicate “Primary” label at FormGroup and field level.
labelInfo={isDualStack && 'Primary'}on the FormGroup plus per‑field Primary/Secondary causes redundant badges.Apply this diff:
- <FormGroup fieldId="clusterNetworks" labelInfo={isDualStack && 'Primary'}> + <FormGroup fieldId="clusterNetworks"> ... - <FormGroup fieldId="serviceNetworks" labelInfo={isDualStack && 'Primary'}> + <FormGroup fieldId="serviceNetworks">Also applies to: 121-123
31-33: Service network helper text conflicts with dual‑stack UI.Text says “Enter only 1 IP address pool.” but dual‑stack renders two. Make it conditional.
Apply this diff:
-const serviceCidrHelperText = - 'Enter only 1 IP address pool. If you need to access the services from an external network, configure load balancers and routers to manage the traffic.'; +const serviceCidrHelperTextSingle = + 'Enter only 1 IP address pool. If you need to access the services from an external network, configure load balancers and routers to manage the traffic.'; +const serviceCidrHelperTextDual = + 'Enter one IP address pool per IP family (IPv4 and IPv6). If you need external access, configure load balancers and routers.';- helperText={serviceCidrHelperText} + helperText={isDualStack ? serviceCidrHelperTextDual : serviceCidrHelperTextSingle}Also applies to: 134-136
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkConfiguration.tsx (1)
289-295: Remove redundant|| falsefromisDisabled.It’s a no‑op.
- hostSubnets.length === 0 || - false + hostSubnets.length === 0libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts (2)
37-39: Narrow theisDualStackcall for type clarity.Passing the entire
clusterspreads unrelated props. Prefer minimal shape.-const isDualStackType = isDualStack({ ...cluster, openshiftVersion: cluster.openshiftVersion }); +const isDualStackType = isDualStack({ + machineNetworks: cluster.machineNetworks, + clusterNetworks: cluster.clusterNetworks, + serviceNetworks: cluster.serviceNetworks, + openshiftVersion: cluster.openshiftVersion, +});
84-92: Linter false positive on Yup.when({ then, otherwise }).Biome’s
noThenPropertycan misfire here. Add a local ignore to prevent noise.- : machineNetworksValidationSchema.when('stackType', { + : machineNetworksValidationSchema.when('stackType', { + /* biome-ignore lint/suspicious/noThenProperty -- Yup WhenOptions uses "then"/"otherwise" */ is: (stackType: NetworkConfigurationValues['stackType']) => stackType === IPV4_STACK, then: () => machineNetworksValidationSchema.concat(IPv4ValidationSchema), otherwise: () =>Repeat the inline comment for the other
.whenobjects if Biome flags them too.Also applies to: 96-101, 104-112
libs/ui-lib/lib/common/components/ui/formik/validationSchemas.ts (1)
314-326: VIP schema shape change looks good; ensure non‑clusterManaged path remains intentional.Cluster‑managed now validates objects with {clusterId, ip} — OK. The else branch accepts any array without per‑item schema; confirm this is intended for user‑managed mode.
If user‑managed still needs structure, mirror the object shape in the else branch.
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/SubnetsDropdown.tsx (1)
53-60: Dropdown grouping works; consider small cleanup and i18n.
- Compute and reuse the “no subnet selected” option once to avoid repeated calls (lines 110–117).
- Labels "IPv4"/"IPv6" should be localized like the rest of UI strings.
- Minor: using CIDR as DOM id works, but unusual characters (':' '/') can complicate selectors. If feasible, use stable generated ids and keep CIDR in value.
Example minimal change:
- return [ - <DropdownItem - key={makeNoSubnetSelectedOption(machineSubnets.length).value} - id={makeNoSubnetSelectedOption(machineSubnets.length).value} - isDisabled={makeNoSubnetSelectedOption(machineSubnets.length).isDisabled} - value={makeNoSubnetSelectedOption(machineSubnets.length).value} - > - {makeNoSubnetSelectedOption(machineSubnets.length).label} - </DropdownItem>, + const placeholder = makeNoSubnetSelectedOption(machineSubnets.length); + return [ + <DropdownItem key={placeholder.value} id={placeholder.value} isDisabled={placeholder.isDisabled} value={placeholder.value}> + {placeholder.label} + </DropdownItem>,Also applies to: 61-64, 79-91, 96-126, 128-134
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AvailableSubnetsControl.tsx (4)
53-56: CoercesupportsIPv6Primaryto a strict boolean.Avoid
string | falseby explicit boolean coercion.- const supportsIPv6Primary = - openshiftVersion && isMajorMinorVersionEqualOrGreater(openshiftVersion, '4.12'); + const supportsIPv6Primary = + !!openshiftVersion && isMajorMinorVersionEqualOrGreater(openshiftVersion, '4.12');
69-73: Default primary selection may unexpectedly choose IPv6. Prefer IPv4 when available.Sorting by humanized can put IPv6 first. Prefer first IPv4, else fall back to IPv6.
- const cidr = allSubnets.length >= 1 ? allSubnets[0].subnet : NO_SUBNET_SET; + const primaryDefault = IPv4Subnets[0] || IPv6Subnets[0]; + const cidr = primaryDefault ? primaryDefault.subnet : NO_SUBNET_SET;
85-101: Test IDs should not assume family by index; use primary/secondary.Index‑based
'ipv4'/'ipv6'is wrong when IPv6 can be primary. Rename to primary/secondary to match semantics.- data-testid={`subnets-dropdown-toggle-${index ? 'ipv6' : 'ipv4'}`} + data-testid={'subnets-dropdown-toggle-primary'} @@ - data-testid={`subnets-dropdown-toggle-${index ? 'ipv6' : 'ipv4'}`} + data-testid={'subnets-dropdown-toggle-secondary'}Also applies to: 151-159
133-149: Secondary filtering logic is correct. Minor: show only opposite family when primary selected.Good enforcement of opposite family for ≥4.12. Consider not showing “all subnets” when first is unset to subtly guide users (optional).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
libs/ui-lib/lib/common/components/clusterConfiguration/utils.test.ts(1 hunks)libs/ui-lib/lib/common/components/clusterConfiguration/utils.ts(2 hunks)libs/ui-lib/lib/common/components/ui/formik/_validationSchemas.test.ts(2 hunks)libs/ui-lib/lib/common/components/ui/formik/validationSchemas.ts(3 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AdvancedNetworkFields.tsx(2 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AvailableSubnetsControl.tsx(2 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkConfiguration.tsx(2 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkConfigurationForm.tsx(1 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/SubnetsDropdown.tsx(4 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/VirtualIPControlGroup.tsx(4 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts(4 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/review/ReviewNetworkingTable.tsx(4 hunks)libs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.test.ts(1 hunks)libs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- libs/ui-lib/lib/ocm/components/clusterConfiguration/review/ReviewNetworkingTable.tsx
- libs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.tsx
- libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/VirtualIPControlGroup.tsx
- libs/ui-lib/lib/common/components/clusterConfiguration/utils.test.ts
- libs/ui-lib/lib/common/components/ui/formik/_validationSchemas.test.ts
🧰 Additional context used
🧬 Code graph analysis (8)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AdvancedNetworkFields.tsx (1)
libs/ui-lib/lib/common/components/clusterConfiguration/utils.ts (1)
isDualStack(204-214)
libs/ui-lib/lib/common/components/clusterConfiguration/utils.ts (2)
libs/ui-lib/lib/common/utils.ts (1)
isMajorMinorVersionEqualOrGreater(138-143)libs/types/assisted-installer-service.d.ts (1)
Cluster(110-407)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AvailableSubnetsControl.tsx (4)
libs/ui-lib/lib/common/types/clusters.ts (1)
NetworkConfigurationValues(34-53)libs/ui-lib/lib/ocm/store/slices/current-cluster/selectors.ts (1)
selectCurrentClusterPermissionsState(9-17)libs/ui-lib/lib/common/utils.ts (1)
isMajorMinorVersionEqualOrGreater(138-143)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/SubnetsDropdown.tsx (1)
SubnetsDropdown(43-168)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkConfiguration.tsx (1)
libs/ui-lib/lib/common/components/clusterConfiguration/utils.ts (1)
isDualStack(204-214)
libs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.test.ts (2)
libs/types/assisted-installer-service.d.ts (1)
Cluster(110-407)libs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.tsx (1)
getStackTypeLabel(43-44)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkConfigurationForm.tsx (1)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts (1)
getNetworkConfigurationValidationSchema(66-114)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts (3)
libs/ui-lib/lib/common/components/clusterConfiguration/utils.ts (1)
isDualStack(204-214)libs/ui-lib/lib/common/components/ui/formik/validationSchemas.ts (2)
dualStackValidationSchema(707-726)IPv4ValidationSchema(728-732)libs/ui-lib/lib/common/types/clusters.ts (1)
NetworkConfigurationValues(34-53)
libs/ui-lib/lib/common/components/ui/formik/validationSchemas.ts (2)
libs/ui-lib/lib/common/utils.ts (1)
isMajorMinorVersionEqualOrGreater(138-143)libs/types/assisted-installer-service.d.ts (1)
MachineNetwork(2182-2191)
🪛 Biome (2.1.2)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts
[error] 105-105: Do not add then to an object.
(lint/suspicious/noThenProperty)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: tests
- GitHub Check: translation-files
- GitHub Check: circular-deps
- GitHub Check: unit-tests
- GitHub Check: lint
- GitHub Check: format
🔇 Additional comments (8)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkConfiguration.tsx (2)
226-231: Auto‑enable Advanced networking for dual‑stack — confirm UX.Effect now toggles Advanced on for any dual‑stack (viewer‑mode excluded), regardless of UMN/CMN. If intended, LGTM; otherwise, re‑add the UMN guard.
295-296: Good: version‑aware subnets logic wired.Passing
openshiftVersion={cluster.openshiftVersion}toAvailableSubnetsControlaligns it with IPv6‑primary rules.libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts (1)
66-71: Version propagation through validation — looks correct.
openshiftVersionis threaded to all dual‑stack validations (machine/cluster/service). This aligns with IPv6‑primary rules.Also applies to: 87-91, 98-101, 106-111
libs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.test.ts (2)
82-121: Solid coverage for IPv6‑primary and legacy behavior.Tests for OCP ≥ 4.12 with IPv6‑primary and for < 4.12 constraints look good.
172-191: Confirm default whenopenshiftVersionis undefined.Expectation is “Dual‑stack” for mixed families if version is missing. Validate this is the intended fallback across the app.
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkConfigurationForm.tsx (1)
203-206: The original review comment is based on a misunderstanding of the codebase.There are two different functions named
getNetworkConfigurationValidationSchemain different scopes:
- Exported version (networkConfigurationValidation.ts:66):
(initialValues, hostSubnets, openshiftVersion?: string)— 3rd parameter is optional- Local version (use-networking-formik.ts:44):
(initialValues, hostSubnets)— only 2 parametersEach call site correctly invokes its respective function:
use-networking-formik.ts:96calls the local version with 2 args ✓NetworkConfigurationForm.tsx:204calls the imported exported version with 3 args ✓The review comment's directive to "ensure all
getNetworkConfigurationValidationSchema(...)calls pass the 3rd arg" conflates two separate functions and misses that the 3rd parameter is optional in the exported version anyway.Likely an incorrect or invalid review comment.
libs/ui-lib/lib/common/components/clusterConfiguration/utils.ts (2)
181-202: Correct, version‑aware opposite‑family enforcement.Logic enforces v4↔v6 pairing and flips primary for ≥4.12. Looks good.
Note: If callers can provide >2 entries, only the first two are considered. Ensure upstream validation caps length at 2 (see dualStackValidationSchema).
204-215: isDualStack propagation is consistent.Passing openshiftVersion through all three network sets aligns behavior across machine/cluster/service networks.
...lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AvailableSubnetsControl.tsx
Show resolved
Hide resolved
linoyaslan
force-pushed
the
allow_dualstack_primary_ipv6
branch
from
415a2b1 to
2afc70e
Compare
|
@linoyaslan: This pull request references MGMT-21625 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 task to target the "4.21.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. |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AdvancedNetworkFields.tsx (1)
48-64: Don’t early‑return from the effect; it skips service‑network reordering.Use local guards so serviceNetworks still reorder even if first cluster CIDR is empty.
- // Check Cluster Networks - if (values.clusterNetworks && values.clusterNetworks.length >= 2) { - const firstClusterCidr = values.clusterNetworks[0].cidr; - if (!firstClusterCidr) return; - - const isFirstClusterIPv6 = Address6.isValid(firstClusterCidr); - // If primary machine is IPv6 but first cluster is IPv4, swap them - if (isPrimaryIPv6 && !isFirstClusterIPv6) { - const reordered = [values.clusterNetworks[1], values.clusterNetworks[0]]; - setFieldValue('clusterNetworks', reordered, false); - } - // If primary machine is IPv4 but first cluster is IPv6, swap them - else if (!isPrimaryIPv6 && isFirstClusterIPv6) { - const reordered = [values.clusterNetworks[1], values.clusterNetworks[0]]; - setFieldValue('clusterNetworks', reordered, false); - } - } + // Check Cluster Networks + if (values.clusterNetworks?.length >= 2) { + const firstClusterCidr = values.clusterNetworks[0].cidr; + if (firstClusterCidr) { + const isFirstClusterIPv6 = Address6.isValid(firstClusterCidr); + if (isPrimaryIPv6 && !isFirstClusterIPv6) { + const reordered = [values.clusterNetworks[1], values.clusterNetworks[0]]; + setFieldValue('clusterNetworks', reordered, false); + } else if (!isPrimaryIPv6 && isFirstClusterIPv6) { + const reordered = [values.clusterNetworks[1], values.clusterNetworks[0]]; + setFieldValue('clusterNetworks', reordered, false); + } + } + } @@ - // Check Service Networks - if (values.serviceNetworks && values.serviceNetworks.length >= 2) { - const firstServiceCidr = values.serviceNetworks[0].cidr; - if (!firstServiceCidr) return; - - const isFirstServiceIPv6 = Address6.isValid(firstServiceCidr); - // If primary machine is IPv6 but first service is IPv4, swap them - if (isPrimaryIPv6 && !isFirstServiceIPv6) { - const reordered = [values.serviceNetworks[1], values.serviceNetworks[0]]; - setFieldValue('serviceNetworks', reordered, false); - } - // If primary machine is IPv4 but first service is IPv6, swap them - else if (!isPrimaryIPv6 && isFirstServiceIPv6) { - const reordered = [values.serviceNetworks[1], values.serviceNetworks[0]]; - setFieldValue('serviceNetworks', reordered, false); - } - } + // Check Service Networks + if (values.serviceNetworks?.length >= 2) { + const firstServiceCidr = values.serviceNetworks[0].cidr; + if (firstServiceCidr) { + const isFirstServiceIPv6 = Address6.isValid(firstServiceCidr); + if (isPrimaryIPv6 && !isFirstServiceIPv6) { + const reordered = [values.serviceNetworks[1], values.serviceNetworks[0]]; + setFieldValue('serviceNetworks', reordered, false); + } else if (!isPrimaryIPv6 && isFirstServiceIPv6) { + const reordered = [values.serviceNetworks[1], values.serviceNetworks[0]]; + setFieldValue('serviceNetworks', reordered, false); + } + } + }Also applies to: 66-83
🧹 Nitpick comments (5)
libs/ui-lib/lib/common/components/ui/formik/_validationSchemas.test.ts (1)
375-393: Add a failing OCP ≥ 4.12 case to lock error messaging.You cover success paths; add a negative test asserting the first-subnet error text for 4.12+ to exercise that branch.
Example to append under “OCP >= 4.12 (new behavior)”:
+ test('rejects when first entry is missing/invalid (4.12+)', async () => { + const schema = dualStackValidationSchema('machine networks', '4.12'); + const invalidValues = [[{ cidr: '' }, { cidr: '192.168.1.0/24' }]]; + await expect(schema.validate(invalidValues[0])).rejects.toThrow( + 'First network has to be a valid IPv4 or IPv6 subnet.', + ); + });libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkConfiguration.tsx (1)
226-231: Avoid redundant state sets when dual‑stack stays true.Guard the effect so it only toggles once.
- React.useEffect(() => { - if (!isViewerMode && isDualStack) { - toggleAdvConfiguration(true); - } - }, [isDualStack, isViewerMode, toggleAdvConfiguration]); + React.useEffect(() => { + if (!isViewerMode && isDualStack && !isAdvanced) { + toggleAdvConfiguration(true); + } + }, [isDualStack, isViewerMode, isAdvanced, toggleAdvConfiguration]);libs/ui-lib/lib/ocm/components/clusterConfiguration/review/ReviewNetworkingTable.tsx (1)
61-71: Avoid empty third cell in single‑stack; mirror advanced rows.Use conditional cell + filter(Boolean) like below to prevent a blank column.
- cells: [ + cells: [ { title: 'Machine networks CIDR' }, { title: cluster.machineNetworks?.map((network) => ( <span key={network.cidr}> {network.cidr} <br /> </span> )), props: { 'data-testid': 'machine-networks' }, }, - isDualStack({ ...cluster, openshiftVersion: cluster.openshiftVersion }) - ? { - title: cluster.machineNetworks?.map((network, index) => ( - <span key={network.cidr}> - {index === 0 ? 'Primary' : 'Secondary'} - <br /> - </span> - )), - } - : { title: '' }, - ], + isDualStack({ ...cluster, openshiftVersion: cluster.openshiftVersion }) && { + title: cluster.machineNetworks?.map((network, index) => ( + <span key={network.cidr}> + {index === 0 ? 'Primary' : 'Secondary'} + <br /> + </span> + )), + }, + ].filter(Boolean),libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AvailableSubnetsControl.tsx (2)
53-55: CoercesupportsIPv6Primaryto explicit boolean.The expression can evaluate to
undefinedwhenopenshiftVersionis not provided, resulting inundefined | booleantype. Explicitly coerce to boolean for type safety.- const supportsIPv6Primary = - openshiftVersion && isMajorMinorVersionEqualOrGreater(openshiftVersion, '4.12'); + const supportsIPv6Primary = + !!openshiftVersion && isMajorMinorVersionEqualOrGreater(openshiftVersion, '4.12');
98-98: Use semantic test IDs instead of index-based family inference.Both test IDs use index-based ternary logic that doesn't reflect the actual network family:
- Line 98: The ternary
index ? 'ipv6' : 'ipv4'always evaluates to'ipv4'because of the early return at line 88 whenindex > 0.- Line 157: The hardcoded
index = 1(line 131) always yields'ipv6', but withsupportsIPv6Primary, the secondary network can be IPv4 when the primary is IPv6 (lines 137-139).Use semantic labels like
'primary'and'secondary'for clarity.<StackItem key={index}> <SubnetsDropdown name={`machineNetworks.${index}.cidr`} machineSubnets={machineSubnets} isDisabled={isDisabled} - data-testid={`subnets-dropdown-toggle-${index ? 'ipv6' : 'ipv4'}`} + data-testid="subnets-dropdown-toggle-primary" /> </StackItem>return ( <SubnetsDropdown name={`machineNetworks.${index}.cidr`} machineSubnets={machineSubnets} isDisabled={isDisabled} - data-testid={`subnets-dropdown-toggle-${index ? 'ipv6' : 'ipv4'}`} + data-testid="subnets-dropdown-toggle-secondary" /> );Also applies to: 157-157
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
libs/ui-lib/lib/common/components/clusterConfiguration/utils.test.ts(1 hunks)libs/ui-lib/lib/common/components/clusterConfiguration/utils.ts(2 hunks)libs/ui-lib/lib/common/components/ui/formik/_validationSchemas.test.ts(2 hunks)libs/ui-lib/lib/common/components/ui/formik/validationSchemas.ts(2 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AdvancedNetworkFields.tsx(4 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AvailableSubnetsControl.tsx(2 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkConfiguration.tsx(2 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkConfigurationForm.tsx(1 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/StackTypeControl.tsx(3 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/SubnetsDropdown.tsx(4 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/VirtualIPControlGroup.tsx(4 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts(4 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/review/ReviewNetworkingTable.tsx(4 hunks)libs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.test.ts(1 hunks)libs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/VirtualIPControlGroup.tsx
- libs/ui-lib/lib/common/components/ui/formik/validationSchemas.ts
- libs/ui-lib/lib/common/components/clusterConfiguration/utils.test.ts
- libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/SubnetsDropdown.tsx
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-10-20T08:57:04.935Z
Learnt from: linoyaslan
PR: openshift-assisted/assisted-installer-ui#3190
File: libs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.tsx:43-45
Timestamp: 2025-10-20T08:57:04.935Z
Learning: In the file `libs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.tsx`, the `getStackTypeLabel` function returns 'Dual-stack' or 'IPv4' because the system currently only supports IPv4 single-stack clusters. IPv6-only single-stack clusters are not supported.
Applied to files:
libs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.test.tslibs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AdvancedNetworkFields.tsxlibs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/StackTypeControl.tsxlibs/ui-lib/lib/ocm/components/clusterConfiguration/review/ReviewNetworkingTable.tsxlibs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.tsx
📚 Learning: 2025-10-21T04:53:07.339Z
Learnt from: linoyaslan
PR: openshift-assisted/assisted-installer-ui#3190
File: libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/StackTypeControl.tsx:71-91
Timestamp: 2025-10-21T04:53:07.339Z
Learning: In libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/StackTypeControl.tsx, dual-stack mode requires both IPv4 and IPv6 networks (machineNetworks must have at least 2 entries: one IPv4 and one IPv6). Dual-stack with only IPv6 is not supported—dual-stack must have both IP families present.
Applied to files:
libs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.test.tslibs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AdvancedNetworkFields.tsxlibs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/StackTypeControl.tsxlibs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.tslibs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AvailableSubnetsControl.tsxlibs/ui-lib/lib/ocm/components/clusterConfiguration/review/ReviewNetworkingTable.tsxlibs/ui-lib/lib/common/components/clusterConfiguration/utils.tslibs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.tsx
📚 Learning: 2025-10-21T04:40:36.274Z
Learnt from: linoyaslan
PR: openshift-assisted/assisted-installer-ui#3190
File: libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AdvancedNetworkFields.tsx:55-63
Timestamp: 2025-10-21T04:40:36.274Z
Learning: In libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AdvancedNetworkFields.tsx, the network reordering logic in the useEffect (swapping clusterNetworks and serviceNetworks based on the primary machine network's IP family) is for UI consistency only. Validation of empty or invalid CIDRs is handled separately by validation schemas, not by the reordering logic.
Applied to files:
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AdvancedNetworkFields.tsxlibs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/StackTypeControl.tsxlibs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkConfiguration.tsxlibs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.tslibs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AvailableSubnetsControl.tsxlibs/ui-lib/lib/ocm/components/clusterConfiguration/review/ReviewNetworkingTable.tsxlibs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkConfigurationForm.tsx
📚 Learning: 2025-10-19T17:22:52.502Z
Learnt from: linoyaslan
PR: openshift-assisted/assisted-installer-ui#3190
File: libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts:69-69
Timestamp: 2025-10-19T17:22:52.502Z
Learning: CIM UI changes in the repository openshift-assisted/assisted-installer-ui (e.g., files under libs/ui-lib/lib/cim/) are handled separately by the CIM team and should be tracked via separate issues rather than being included in PRs for other UI components.
Applied to files:
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts
🧬 Code graph analysis (11)
libs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.test.ts (2)
libs/types/assisted-installer-service.d.ts (1)
Cluster(110-407)libs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.tsx (1)
getStackTypeLabel(43-44)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AdvancedNetworkFields.tsx (3)
libs/ui-lib/lib/common/types/clusters.ts (1)
NetworkConfigurationValues(34-53)libs/ui-lib/lib/common/components/clusterConfiguration/utils.ts (1)
isDualStack(204-214)libs/ui-lib/lib/common/config/constants.ts (1)
DUAL_STACK(300-300)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/StackTypeControl.tsx (1)
libs/ui-lib/lib/common/config/constants.ts (1)
NO_SUBNET_SET(294-294)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkConfiguration.tsx (1)
libs/ui-lib/lib/common/components/clusterConfiguration/utils.ts (1)
isDualStack(204-214)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts (4)
libs/ui-lib/lib/common/components/clusterConfiguration/utils.ts (1)
isDualStack(204-214)libs/ui-lib/lib/common/types/clusters.ts (2)
NetworkConfigurationValues(34-53)HostSubnets(33-33)libs/ui-lib/lib/common/components/ui/formik/validationSchemas.ts (4)
dualStackValidationSchema(707-726)clusterNetworksValidationSchema(686-696)serviceNetworkValidationSchema(698-705)IPv4ValidationSchema(728-732)libs/ui-lib/lib/common/config/constants.ts (1)
IPV4_STACK(299-299)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AvailableSubnetsControl.tsx (4)
libs/ui-lib/lib/common/types/clusters.ts (1)
NetworkConfigurationValues(34-53)libs/ui-lib/lib/common/config/constants.ts (2)
DUAL_STACK(300-300)NO_SUBNET_SET(294-294)libs/ui-lib/lib/common/utils.ts (1)
isMajorMinorVersionEqualOrGreater(138-143)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/SubnetsDropdown.tsx (1)
SubnetsDropdown(43-168)
libs/ui-lib/lib/ocm/components/clusterConfiguration/review/ReviewNetworkingTable.tsx (1)
libs/ui-lib/lib/common/components/clusterConfiguration/utils.ts (1)
isDualStack(204-214)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkConfigurationForm.tsx (1)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts (1)
getNetworkConfigurationValidationSchema(72-120)
libs/ui-lib/lib/common/components/clusterConfiguration/utils.ts (2)
libs/ui-lib/lib/common/utils.ts (1)
isMajorMinorVersionEqualOrGreater(138-143)libs/types/assisted-installer-service.d.ts (1)
Cluster(110-407)
libs/ui-lib/lib/common/components/ui/formik/_validationSchemas.test.ts (1)
libs/ui-lib/lib/common/components/ui/formik/validationSchemas.ts (1)
dualStackValidationSchema(707-726)
libs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.tsx (1)
libs/ui-lib/lib/common/components/clusterConfiguration/utils.ts (1)
isDualStack(204-214)
🪛 Biome (2.1.2)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts
[error] 111-111: Do not add then to an object.
(lint/suspicious/noThenProperty)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: circular-deps
- GitHub Check: unit-tests
- GitHub Check: tests
- GitHub Check: format
- GitHub Check: lint
- GitHub Check: translation-files
🔇 Additional comments (12)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/StackTypeControl.tsx (5)
5-5: LGTM! Necessary import for IPv4 validation.The
Address4import is correctly added and used throughout the file for IPv4 CIDR validation when handling single-stack and dual-stack transitions.
71-91: LGTM! IPv4 selection logic is correct.The logic properly ensures IPv4-only networks when switching from dual-stack to single-stack. The condition
values.machineNetworks?.length >= 2is correct given that dual-stack requires both IPv4 and IPv6 networks.Based on learnings: This was discussed in previous reviews and confirmed correct.
92-124: LGTM! IPv4 default reset logic is correct.The code properly resets
clusterNetworksandserviceNetworksto IPv4 defaults when switching to single-stack. TheclusterIdpreservation logic and fallbacks for missing defaults are handled correctly.
127-136: LGTM! Dependencies correctly updated.All values used within the
setSingleStackcallback are properly included in the dependency array, ensuring the callback updates correctly when these values change.
146-171: LGTM! Default IPv4-primary logic is correct.The code correctly sets IPv4 as the default primary network when switching to dual-stack mode, providing a consistent initial state. Users can subsequently change to IPv6-primary through the UI controls (for OpenShift >= 4.12), as version-aware validation is handled elsewhere.
Based on learnings: This was extensively discussed in previous reviews and confirmed correct.
libs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.tsx (1)
43-45: Version-aware isDualStack propagation: LGTM.Passing cluster.openshiftVersion into isDualStack aligns with the new signature and UI logic.
Also applies to: 70-71
libs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.test.ts (1)
28-170: Solid coverage for stack labeling across versions.Scenarios look comprehensive and match the supported modes. LGTM.
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkConfiguration.tsx (1)
295-296: Propagating openshiftVersion to AvailableSubnetsControl: LGTM.Keeps subnet UI behavior version‑aware and consistent.
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkConfigurationForm.tsx (1)
202-206: Version‑aware validation schema wiring: LGTM.Passing cluster.openshiftVersion and adding it to deps ensures auto‑revalidation on version changes.
libs/ui-lib/lib/common/components/clusterConfiguration/utils.ts (1)
181-202: Version‑aware dual‑stack detection: LGTM.Checks opposite families with relaxed primary for OCP ≥ 4.12 and strict IPv4‑first for older/unspecified versions. Matches the schema.
Confirm max‑2 enforcement is consistently applied for machine, cluster, and service networks so areNetworksDualStack’s “first two” assumption can’t be violated by a third entry.
Also applies to: 204-214
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts (1)
57-68: LGTM: clusterId mapping for single-stack defaults.The addition of
clusterIdto default IPv4 network configurations ensures proper initialization for single-stack clusters. The mapping is correctly scoped to single-stack only (whenisDualStackTypeis false), preserving existing dual-stack behavior.libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AvailableSubnetsControl.tsx (1)
103-111: LGTM: Single-stack correctly restricted to IPv4.The single-stack branch correctly uses
IPv4Subnetsonly (line 107), ensuring the UI aligns with theIPv4ValidationSchemaenforcement. This addresses the concern from previous reviews.
linoyaslan
force-pushed
the
allow_dualstack_primary_ipv6
branch
from
2afc70e to
d72f84c
Compare
|
@linoyaslan: This pull request references MGMT-21625 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 task to target the "4.21.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. |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts (1)
53-68: Apply clusterId mapping consistently to both IPv4 and dual-stack defaults.Verification confirms the asymmetry: IPv4 defaults for
clusterNetworksandserviceNetworksare mapped to addclusterId: cluster.id(lines 57–60, 65–68), but dual-stack defaults (clusterNetworksDualstack,serviceNetworksDualstackat lines 56, 64) are used unchanged. Both types defineClusterNetworkandServiceNetworkwith optionalclusterId?: stringfields, flow through identical form initialization and submission pipelines, and have no documented reason for differential treatment.Apply the same
.map()transformation to dual-stack defaults to align with IPv4 handling:clusterNetworks: cluster.clusterNetworks || (isDualStackType ? defaultNetworkValues.clusterNetworksDualstack?.map((network) => ({ ...network, clusterId: cluster.id, })) : defaultNetworkValues.clusterNetworksIpv4?.map((network) => ({ ...network, clusterId: cluster.id, }))), serviceNetworks: cluster.serviceNetworks || (isDualStackType ? defaultNetworkValues.serviceNetworksDualstack?.map((network) => ({ ...network, clusterId: cluster.id, })) : defaultNetworkValues.serviceNetworksIpv4?.map((network) => ({ ...network, clusterId: cluster.id, }))),
♻️ Duplicate comments (1)
libs/ui-lib/lib/common/components/ui/formik/validationSchemas.ts (1)
707-726: Missing validation for secondary network's IP family.The
dualStackValidationSchemaonly validates the first network's CIDR but does not enforce that the second network is the opposite IP family. This allows invalid dual-stack configurations like IPv4+IPv4 or IPv6+IPv6 to pass validation.Add a second test to validate the secondary network:
export const dualStackValidationSchema = (field: string, openshiftVersion?: string) => Yup.array() .max(2, `Maximum number of ${field} subnets in dual stack is 2.`) .test( 'dual-stack-ipv4', openshiftVersion && isMajorMinorVersionEqualOrGreater(openshiftVersion, '4.12') ? 'First network has to be a valid IPv4 or IPv6 subnet.' : 'First network has to be IPv4 subnet.', (values?: { cidr: MachineNetwork['cidr'] }[]): boolean => { // For OCP versions > 4.11, allow IPv6 as primary network if (openshiftVersion && isMajorMinorVersionEqualOrGreater(openshiftVersion, '4.12')) { return ( !!values?.[0].cidr && (Address4.isValid(values[0].cidr) || Address6.isValid(values[0].cidr)) ); } // For older versions, require IPv4 as primary network return !!values?.[0].cidr && Address4.isValid(values[0].cidr); }, - ); + ) + .test( + 'dual-stack-secondary-family', + openshiftVersion && isMajorMinorVersionEqualOrGreater(openshiftVersion, '4.12') + ? 'Second network must be the opposite IP family of the first.' + : 'Second network has to be IPv6 subnet.', + (values?: { cidr: MachineNetwork['cidr'] }[]): boolean => { + if (!values || values.length < 2) return true; // Skip if not dual-stack + const firstCidr = values[0]?.cidr || ''; + const secondCidr = values[1]?.cidr || ''; + if (!firstCidr || !secondCidr) return false; + + if (openshiftVersion && isMajorMinorVersionEqualOrGreater(openshiftVersion, '4.12')) { + // For 4.12+, require opposite families (IPv4+IPv6 or IPv6+IPv4) + return ( + (Address4.isValid(firstCidr) && Address6.isValid(secondCidr)) || + (Address6.isValid(firstCidr) && Address4.isValid(secondCidr)) + ); + } + // For older versions, require IPv6 as secondary + return Address6.isValid(secondCidr); + }, + );
🧹 Nitpick comments (2)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AvailableSubnetsControl.tsx (2)
53-56: Explicitly coercesupportsIPv6Primaryto boolean for type safety.While the current code works,
supportsIPv6Primarycan technically beundefined | string | boolean. Explicitly coerce to boolean for clarity:const supportsIPv6Primary = - openshiftVersion && isMajorMinorVersionEqualOrGreater(openshiftVersion, '4.12'); + !!openshiftVersion && isMajorMinorVersionEqualOrGreater(openshiftVersion, '4.12');
98-98: Test IDs use misleading index-based family detection.The test IDs at lines 98 and 157 use a ternary based on index (
${index ? 'ipv6' : 'ipv4'}), but:
- Line 98:
indexis always 0 (due to early return at line 88), so always renders'ipv4'even when the selected subnet might be IPv6 on OpenShift >= 4.12- Line 157:
indexis always 1, so always renders'ipv6'even when the subnet might be IPv4 (when primary is IPv6)Use semantic test IDs that reflect primary/secondary roles:
- data-testid={`subnets-dropdown-toggle-${index ? 'ipv6' : 'ipv4'}`} + data-testid="subnets-dropdown-toggle-primary"And for the secondary dropdown:
- data-testid={`subnets-dropdown-toggle-${index ? 'ipv6' : 'ipv4'}`} + data-testid="subnets-dropdown-toggle-secondary"Also applies to: 157-157
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
libs/ui-lib/lib/common/components/clusterConfiguration/utils.test.ts(1 hunks)libs/ui-lib/lib/common/components/clusterConfiguration/utils.ts(2 hunks)libs/ui-lib/lib/common/components/ui/formik/_validationSchemas.test.ts(2 hunks)libs/ui-lib/lib/common/components/ui/formik/validationSchemas.ts(2 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AdvancedNetworkFields.tsx(4 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AvailableSubnetsControl.tsx(2 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkConfiguration.tsx(2 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkConfigurationForm.tsx(1 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/StackTypeControl.tsx(3 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/SubnetsDropdown.tsx(4 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/VirtualIPControlGroup.tsx(4 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts(4 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/review/ReviewNetworkingTable.tsx(4 hunks)libs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.test.ts(1 hunks)libs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- libs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.test.ts
- libs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.tsx
- libs/ui-lib/lib/ocm/components/clusterConfiguration/review/ReviewNetworkingTable.tsx
- libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkConfigurationForm.tsx
- libs/ui-lib/lib/common/components/clusterConfiguration/utils.test.ts
- libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/SubnetsDropdown.tsx
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-10-21T04:40:36.274Z
Learnt from: linoyaslan
PR: openshift-assisted/assisted-installer-ui#3190
File: libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AdvancedNetworkFields.tsx:55-63
Timestamp: 2025-10-21T04:40:36.274Z
Learning: In libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AdvancedNetworkFields.tsx, the network reordering logic in the useEffect (swapping clusterNetworks and serviceNetworks based on the primary machine network's IP family) is for UI consistency only. Validation of empty or invalid CIDRs is handled separately by validation schemas, not by the reordering logic.
Applied to files:
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkConfiguration.tsxlibs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AdvancedNetworkFields.tsxlibs/ui-lib/lib/common/components/clusterConfiguration/utils.tslibs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/StackTypeControl.tsxlibs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/VirtualIPControlGroup.tsxlibs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts
📚 Learning: 2025-10-21T04:53:07.339Z
Learnt from: linoyaslan
PR: openshift-assisted/assisted-installer-ui#3190
File: libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/StackTypeControl.tsx:71-91
Timestamp: 2025-10-21T04:53:07.339Z
Learning: In libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/StackTypeControl.tsx, dual-stack mode requires both IPv4 and IPv6 networks (machineNetworks must have at least 2 entries: one IPv4 and one IPv6). Dual-stack with only IPv6 is not supported—dual-stack must have both IP families present.
Applied to files:
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AdvancedNetworkFields.tsxlibs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AvailableSubnetsControl.tsxlibs/ui-lib/lib/common/components/ui/formik/validationSchemas.tslibs/ui-lib/lib/common/components/clusterConfiguration/utils.tslibs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/StackTypeControl.tsxlibs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/VirtualIPControlGroup.tsxlibs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts
📚 Learning: 2025-10-20T08:57:04.935Z
Learnt from: linoyaslan
PR: openshift-assisted/assisted-installer-ui#3190
File: libs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.tsx:43-45
Timestamp: 2025-10-20T08:57:04.935Z
Learning: In the file `libs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.tsx`, the `getStackTypeLabel` function returns 'Dual-stack' or 'IPv4' because the system currently only supports IPv4 single-stack clusters. IPv6-only single-stack clusters are not supported.
Applied to files:
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AdvancedNetworkFields.tsxlibs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/StackTypeControl.tsxlibs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/VirtualIPControlGroup.tsx
📚 Learning: 2025-10-19T17:22:52.502Z
Learnt from: linoyaslan
PR: openshift-assisted/assisted-installer-ui#3190
File: libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts:69-69
Timestamp: 2025-10-19T17:22:52.502Z
Learning: CIM UI changes in the repository openshift-assisted/assisted-installer-ui (e.g., files under libs/ui-lib/lib/cim/) are handled separately by the CIM team and should be tracked via separate issues rather than being included in PRs for other UI components.
Applied to files:
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts
🧬 Code graph analysis (8)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkConfiguration.tsx (1)
libs/ui-lib/lib/common/components/clusterConfiguration/utils.ts (1)
isDualStack(204-214)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AdvancedNetworkFields.tsx (3)
libs/ui-lib/lib/common/types/clusters.ts (1)
NetworkConfigurationValues(34-53)libs/ui-lib/lib/common/components/clusterConfiguration/utils.ts (1)
isDualStack(204-214)libs/ui-lib/lib/common/config/constants.ts (1)
DUAL_STACK(300-300)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AvailableSubnetsControl.tsx (4)
libs/ui-lib/lib/common/types/clusters.ts (1)
NetworkConfigurationValues(34-53)libs/ui-lib/lib/common/config/constants.ts (2)
DUAL_STACK(300-300)NO_SUBNET_SET(294-294)libs/ui-lib/lib/common/utils.ts (1)
isMajorMinorVersionEqualOrGreater(138-143)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/SubnetsDropdown.tsx (1)
SubnetsDropdown(43-168)
libs/ui-lib/lib/common/components/ui/formik/validationSchemas.ts (2)
libs/ui-lib/lib/common/utils.ts (1)
isMajorMinorVersionEqualOrGreater(138-143)libs/types/assisted-installer-service.d.ts (1)
MachineNetwork(2182-2191)
libs/ui-lib/lib/common/components/ui/formik/_validationSchemas.test.ts (1)
libs/ui-lib/lib/common/components/ui/formik/validationSchemas.ts (1)
dualStackValidationSchema(707-726)
libs/ui-lib/lib/common/components/clusterConfiguration/utils.ts (2)
libs/ui-lib/lib/common/utils.ts (1)
isMajorMinorVersionEqualOrGreater(138-143)libs/types/assisted-installer-service.d.ts (1)
Cluster(110-407)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/StackTypeControl.tsx (1)
libs/ui-lib/lib/common/config/constants.ts (1)
NO_SUBNET_SET(294-294)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts (4)
libs/ui-lib/lib/common/components/clusterConfiguration/utils.ts (1)
isDualStack(204-214)libs/ui-lib/lib/common/types/clusters.ts (2)
NetworkConfigurationValues(34-53)HostSubnets(33-33)libs/ui-lib/lib/common/components/ui/formik/validationSchemas.ts (4)
dualStackValidationSchema(707-726)clusterNetworksValidationSchema(686-696)serviceNetworkValidationSchema(698-705)IPv4ValidationSchema(728-732)libs/ui-lib/lib/common/config/constants.ts (1)
IPV4_STACK(299-299)
🪛 Biome (2.1.2)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts
[error] 111-111: Do not add then to an object.
(lint/suspicious/noThenProperty)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: tests
- GitHub Check: translation-files
- GitHub Check: unit-tests
- GitHub Check: circular-deps
- GitHub Check: format
- GitHub Check: lint
🔇 Additional comments (15)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/VirtualIPControlGroup.tsx (1)
146-146: LGTM—VIP label simplification is consistent.The removal of "(IPv4)" suffixes from API IP and Ingress IP labels aligns with the PR's broader goal of centralizing dual-stack indicators. The changes are applied consistently across all four label locations (static fields and input fields for both VIPs).
Also applies to: 165-165, 191-191, 207-207
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AdvancedNetworkFields.tsx (2)
40-90: Network reordering logic is correct for UI consistency.The useEffect correctly reorders cluster and service networks to match the primary machine network's IP family. The early returns at lines 50 and 69 are appropriate since validation schemas handle empty/invalid CIDRs separately (as confirmed in learnings).
Based on learnings
125-126: Label updates correctly remove per-index suffixes.The changes replace per-index suffix logic with static labels and add Primary/Secondary
labelInfobased on dual-stack state and index. This aligns with the centralized dual-stack labeling strategy across the PR.Also applies to: 133-133, 141-141, 179-179, 187-187
libs/ui-lib/lib/common/components/clusterConfiguration/utils.ts (1)
181-214: Version-aware dual-stack detection implemented correctly.The updated
areNetworksDualStackandisDualStackfunctions properly handle the version-aware logic:
- For OpenShift >= 4.12: allows either IPv4-primary/IPv6-secondary OR IPv6-primary/IPv4-secondary
- For older versions or no version: requires IPv4-primary/IPv6-secondary only
The early returns, CIDR extraction, and version checks are all correct.
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/StackTypeControl.tsx (2)
71-124: Single-stack conversion correctly enforces IPv4.The
setSingleStacklogic appropriately:
- Selects an IPv4 machine network (or clears if none available)
- Replaces cluster/service networks with IPv4 defaults from configuration
This aligns with the system's IPv4-only single-stack requirement.
146-170: Dual-stack initialization provides sensible IPv4-primary default.The
setDualStacklogic correctly defaults to IPv4-primary + IPv6-secondary when toggling to dual-stack mode. As discussed in past reviews, users can subsequently change the primary network via subnet dropdowns, and the version-aware validation will allow IPv6-primary for OpenShift >= 4.12.libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkConfiguration.tsx (2)
227-230: Automatic advanced config toggle for dual-stack is appropriate.The updated useEffect correctly auto-enables advanced networking configuration when dual-stack is active and the user is not in viewer mode. The dependency array correctly includes the relevant values.
295-295: OpenShift version correctly propagated to subnet control.Passing
cluster.openshiftVersiontoAvailableSubnetsControlenables version-aware dual-stack subnet selection and validation as implemented throughout this PR.libs/ui-lib/lib/common/components/ui/formik/_validationSchemas.test.ts (1)
352-431: Comprehensive test coverage for version-aware dual-stack validation.The test suite thoroughly covers:
- Legacy behavior (OCP < 4.12): IPv4-primary only
- New behavior (OCP >= 4.12): IPv4-primary or IPv6-primary
- Default behavior (no version): legacy mode
- Edge case: maximum 2 networks
The test structure is clear and validates both positive and negative cases for each scenario.
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AvailableSubnetsControl.tsx (2)
103-111: Single-stack correctly restricted to IPv4 subnets.The single-stack path properly uses
IPv4Subnetsonly, aligning with the system's IPv4-only single-stack requirement and theIPv4ValidationSchemavalidation.
134-150: Smart secondary subnet filtering logic is well-designed.The conditional logic for the secondary dropdown correctly filters available subnets based on the primary network's IP family:
- If primary is IPv6 → show IPv4 subnets
- If primary is IPv4 → show IPv6 subnets
- If primary is not selected → show all subnets
This provides good UX while enforcing dual-stack requirements.
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts (4)
37-37: LGTM! Version-aware dual-stack detection correctly implemented.Passing
openshiftVersiontoisDualStackenables version-aware stack type determination, which is central to the IPv6-primary dual-stack feature.
75-75: API signature extended to support version-aware validation.The addition of the optional
openshiftVersionparameter enables version-aware dual-stack validation. As discussed in previous reviews, the CIM UI call site will be updated separately by the CIM team.
86-98: LGTM! Version-aware validation correctly propagated for machine networks.The
openshiftVersionparameter is now passed todualStackValidationSchema, enabling version-specific IPv4/IPv6 primary network rules for OpenShift >= 4.12.
99-118: LGTM! Version-aware validation correctly propagated for cluster and service networks.The
openshiftVersionparameter is now threaded throughdualStackValidationSchemacalls for bothclusterNetworksandserviceNetworks, enabling consistent version-aware validation across all network types.Note: The static analysis warning on line 111 ("Do not add then to an object") is a false positive—
thenis a valid Yup schema method, not a Promise-like property.
|
/unhold |
openshift-ci
bot
removed
the
do-not-merge/hold
This PR adds support for IPv6-primary dual-stack clusters, enabling users to create dual-stack clusters where IPv6 is the primary IP stack (version-aware for OpenShift 4.12+)
linoyaslan
force-pushed
the
allow_dualstack_primary_ipv6
branch
from
d72f84c to
3b7936a
Compare
|
@linoyaslan: This pull request references MGMT-21625 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 task to target the "4.21.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. |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AdvancedNetworkFields.tsx (1)
97-104: Index-based IPv6 detection is wrong for IPv6‑primary; use CIDR‑based check.When IPv6 is primary (index 0), current logic treats it as IPv4, breaking hostPrefix limits and helper text.
-const isSubnetIPv6 = (index: number) => (isDualStack ? !!index : false); +const isSubnetIPv6 = (index: number) => + Address6.isValid(values.clusterNetworks?.[index]?.cidr || '');Also applies to: 149-154
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/StackTypeControl.tsx (1)
34-46: Out-of-bounds[1]access can throw at runtime.
values.*[1]is read without ensuring length ≥ 2; same forclusterNetworks[1]andserviceNetworks[1]params.-const hasDualStackConfigurationChanged = ( - clusterNetworks: ClusterNetwork[], - serviceNetworks: ServiceNetwork[], - cidrIPv6: MachineNetwork['cidr'], - values: NetworkConfigurationValues, -) => - clusterNetworks && - serviceNetworks && - ((values.machineNetworks && values.machineNetworks[1].cidr !== cidrIPv6) || - (values.clusterNetworks && values.clusterNetworks[1].cidr !== clusterNetworks[1].cidr) || - (values.clusterNetworks && - values.clusterNetworks[1].hostPrefix !== clusterNetworks[1].hostPrefix) || - (values.serviceNetworks && values.serviceNetworks[1].cidr !== serviceNetworks[1].cidr)); +const hasDualStackConfigurationChanged = ( + clusterNetworks: ClusterNetwork[] = [], + serviceNetworks: ServiceNetwork[] = [], + cidrIPv6: MachineNetwork['cidr'], + values: NetworkConfigurationValues, +) => { + const hasSecondMachine = values.machineNetworks?.length >= 2; + const hasSecondCluster = values.clusterNetworks?.length >= 2 && clusterNetworks.length >= 2; + const hasSecondService = values.serviceNetworks?.length >= 2 && serviceNetworks.length >= 2; + return ( + (hasSecondMachine && values.machineNetworks![1].cidr !== cidrIPv6) || + (hasSecondCluster && + (values.clusterNetworks![1].cidr !== clusterNetworks[1].cidr || + values.clusterNetworks![1].hostPrefix !== clusterNetworks[1].hostPrefix)) || + (hasSecondService && values.serviceNetworks![1].cidr !== serviceNetworks[1].cidr) + ); +};
♻️ Duplicate comments (4)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AdvancedNetworkFields.tsx (1)
48-64: Don’t return early from the effect; it skips service-network reordering.Replace the early
returnguards with local checks so the service block always executes.- // Check Cluster Networks - if (values.clusterNetworks && values.clusterNetworks.length >= 2) { - const firstClusterCidr = values.clusterNetworks[0].cidr; - if (!firstClusterCidr) return; - - const isFirstClusterIPv6 = Address6.isValid(firstClusterCidr); - // If primary machine is IPv6 but first cluster is IPv4, swap them - if (isPrimaryIPv6 && !isFirstClusterIPv6) { - const reordered = [values.clusterNetworks[1], values.clusterNetworks[0]]; - setFieldValue('clusterNetworks', reordered, false); - } - // If primary machine is IPv4 but first cluster is IPv6, swap them - else if (!isPrimaryIPv6 && isFirstClusterIPv6) { - const reordered = [values.clusterNetworks[1], values.clusterNetworks[0]]; - setFieldValue('clusterNetworks', reordered, false); - } - } + // Check Cluster Networks + if (values.clusterNetworks?.length >= 2) { + const firstClusterCidr = values.clusterNetworks[0].cidr; + if (firstClusterCidr) { + const isFirstClusterIPv6 = Address6.isValid(firstClusterCidr); + if (isPrimaryIPv6 && !isFirstClusterIPv6) { + const reordered = [values.clusterNetworks[1], values.clusterNetworks[0]]; + setFieldValue('clusterNetworks', reordered, false); + } else if (!isPrimaryIPv6 && isFirstClusterIPv6) { + const reordered = [values.clusterNetworks[1], values.clusterNetworks[0]]; + setFieldValue('clusterNetworks', reordered, false); + } + } + } @@ - // Check Service Networks - if (values.serviceNetworks && values.serviceNetworks.length >= 2) { - const firstServiceCidr = values.serviceNetworks[0].cidr; - if (!firstServiceCidr) return; - - const isFirstServiceIPv6 = Address6.isValid(firstServiceCidr); - // If primary machine is IPv6 but first service is IPv4, swap them - if (isPrimaryIPv6 && !isFirstServiceIPv6) { - const reordered = [values.serviceNetworks[1], values.serviceNetworks[0]]; - setFieldValue('serviceNetworks', reordered, false); - } - // If primary machine is IPv4 but first service is IPv6, swap them - else if (!isPrimaryIPv6 && isFirstServiceIPv6) { - const reordered = [values.serviceNetworks[1], values.serviceNetworks[0]]; - setFieldValue('serviceNetworks', reordered, false); - } - } + // Check Service Networks + if (values.serviceNetworks?.length >= 2) { + const firstServiceCidr = values.serviceNetworks[0].cidr; + if (firstServiceCidr) { + const isFirstServiceIPv6 = Address6.isValid(firstServiceCidr); + if (isPrimaryIPv6 && !isFirstServiceIPv6) { + const reordered = [values.serviceNetworks[1], values.serviceNetworks[0]]; + setFieldValue('serviceNetworks', reordered, false); + } else if (!isPrimaryIPv6 && isFirstServiceIPv6) { + const reordered = [values.serviceNetworks[1], values.serviceNetworks[0]]; + setFieldValue('serviceNetworks', reordered, false); + } + } + }Also applies to: 66-83
libs/ui-lib/lib/common/components/ui/formik/validationSchemas.ts (1)
707-726: Dual‑stack must require exactly two subnets and enforce opposite IP families.Currently only the primary is validated; IPv6+IPv6 can pass on ≥4.12. Add length and second‑family checks.
export const dualStackValidationSchema = (field: string, openshiftVersion?: string) => Yup.array() - .max(2, `Maximum number of ${field} subnets in dual stack is 2.`) + .max(2, `Maximum number of ${field} subnets in dual stack is 2.`) + .min(2, `Exactly 2 ${field} subnets are required for dual stack.`) .test( 'dual-stack-ipv4', openshiftVersion && isMajorMinorVersionEqualOrGreater(openshiftVersion, '4.12') ? 'First network has to be a valid IPv4 or IPv6 subnet.' : 'First network has to be IPv4 subnet.', (values?: { cidr: MachineNetwork['cidr'] }[]): boolean => { // For OCP versions > 4.11, allow IPv6 as primary network if (openshiftVersion && isMajorMinorVersionEqualOrGreater(openshiftVersion, '4.12')) { return ( !!values?.[0].cidr && (Address4.isValid(values[0].cidr) || Address6.isValid(values[0].cidr)) ); } // For older versions, require IPv4 as primary network return !!values?.[0].cidr && Address4.isValid(values[0].cidr); }, - ); + ) + .test( + 'dual-stack-second-family', + openshiftVersion && isMajorMinorVersionEqualOrGreater(openshiftVersion, '4.12') + ? 'Second network must be the opposite IP family of the first.' + : 'Second network has to be IPv6 subnet.', + (values?: { cidr: MachineNetwork['cidr'] }[]): boolean => { + const first = values?.[0]?.cidr || ''; + const second = values?.[1]?.cidr || ''; + if (!first || !second) return false; + if (openshiftVersion && isMajorMinorVersionEqualOrGreater(openshiftVersion, '4.12')) { + return ( + (Address4.isValid(first) && Address6.isValid(second)) || + (Address6.isValid(first) && Address4.isValid(second)) + ); + } + return Address6.isValid(second); + }, + );libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AvailableSubnetsControl.tsx (1)
53-56: Coerce version-gate to boolean to avoid string|boolean type.Use boolean coercion for supportsIPv6Primary.
- const supportsIPv6Primary = - openshiftVersion && isMajorMinorVersionEqualOrGreater(openshiftVersion, '4.12'); + const supportsIPv6Primary = + !!openshiftVersion && isMajorMinorVersionEqualOrGreater(openshiftVersion, '4.12');libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts (1)
92-98: Note: length-gated dual-stack validation kept as-is.Prior discussion marked this behavior intentional and preexisting; not re-litigating here.
Also applies to: 102-108, 112-118
🧹 Nitpick comments (5)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/StackTypeControl.tsx (1)
146-170: Handle empty machineNetworks when switching to dual‑stack (defensive default).If no first network exists, initialize both families (IPv4 primary + IPv6 secondary) to satisfy dual‑stack invariant.
- if (values.machineNetworks && values.machineNetworks?.length < 2) { - // Ensure IPv4 is primary when switching to dual-stack - const firstNetwork = values.machineNetworks[0]; + if (values.machineNetworks && values.machineNetworks?.length < 2) { + // Ensure IPv4 is primary when switching to dual-stack + const firstNetwork = values.machineNetworks[0]; + if (!firstNetwork?.cidr) { + const IPv4Subnets = hostSubnets.filter((subnet) => Address4.isValid(subnet.subnet)); + const cidrIPv4 = IPv4Subnets.length >= 1 ? IPv4Subnets[0].subnet : NO_SUBNET_SET; + setFieldValue( + 'machineNetworks', + [ + { cidr: cidrIPv4, clusterId: clusterId }, + { cidr: cidrIPv6, clusterId: clusterId }, + ], + false, + ); + return; + } const isFirstIPv6 = firstNetwork?.cidr && Address6.isValid(firstNetwork.cidr);libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AvailableSubnetsControl.tsx (2)
98-100: Use stable test IDs: prefer “primary/secondary” over family-by-index.Index-based family is wrong when IPv6 is primary; switch to semantic IDs.
- data-testid={`subnets-dropdown-toggle-${index ? 'ipv6' : 'ipv4'}`} + data-testid="subnets-dropdown-toggle-primary"- data-testid={`subnets-dropdown-toggle-${index ? 'ipv6' : 'ipv4'}`} + data-testid="subnets-dropdown-toggle-secondary"Also applies to: 157-158
64-67: Minor: drop redundant resort of combined subnets.Lists are already sorted per family; SubnetsDropdown groups them anyway. Remove extra sort to preserve per‑group order and avoid extra work.
- const allSubnets = supportsIPv6Primary - ? [...IPv4Subnets, ...IPv6Subnets].sort(subnetSort) - : IPv4Subnets; + const allSubnets = supportsIPv6Primary + ? [...IPv4Subnets, ...IPv6Subnets] + : IPv4Subnets;libs/ui-lib/lib/common/components/clusterConfiguration/utils.ts (1)
181-202: Make version check explicitly boolean for clarity and types.Avoid “string | boolean” by introducing a boolean guard.
[Based on learnings]- // For OCP >= 4.12, allow either IPv4 or IPv6 as primary, with opposite as secondary - if (openshiftVersion && isMajorMinorVersionEqualOrGreater(openshiftVersion, '4.12')) { + // For OCP >= 4.12, allow either IPv4 or IPv6 as primary, with opposite as secondary + const supportsIPv6Primary = + !!openshiftVersion && isMajorMinorVersionEqualOrGreater(openshiftVersion, '4.12'); + if (supportsIPv6Primary) { return ( (Address4.isValid(firstCidr) && Address6.isValid(secondCidr)) || (Address6.isValid(firstCidr) && Address4.isValid(secondCidr)) ); }libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts (1)
90-96: Unblock Biome: suppress false-positive “noThenProperty” on Yup.when.Biome flags “then” keys on objects; this is standard Yup API. Add a targeted ignore comment.
is: (stackType: NetworkConfigurationValues['stackType']) => stackType === IPV4_STACK, - then: () => machineNetworksValidationSchema.concat(IPv4ValidationSchema), + /* biome-ignore lint/suspicious/noThenProperty: Yup.when uses 'then' key */ + then: () => machineNetworksValidationSchema.concat(IPv4ValidationSchema), otherwise: () => values.machineNetworks && values.machineNetworks?.length >= 2 ? machineNetworksValidationSchema.concat( dualStackValidationSchema('machine networks', openshiftVersion), ) : Yup.array(),is: (stackType: NetworkConfigurationValues['stackType']) => stackType === IPV4_STACK, - then: () => clusterNetworksValidationSchema.concat(IPv4ValidationSchema), + /* biome-ignore lint/suspicious/noThenProperty: Yup.when uses 'then' key */ + then: () => clusterNetworksValidationSchema.concat(IPv4ValidationSchema), otherwise: () => values.clusterNetworks && values.clusterNetworks?.length >= 2 ? clusterNetworksValidationSchema.concat( dualStackValidationSchema('cluster network', openshiftVersion), ) : Yup.array(),is: (stackType: NetworkConfigurationValues['stackType']) => stackType === IPV4_STACK, - then: () => serviceNetworkValidationSchema.concat(IPv4ValidationSchema), + /* biome-ignore lint/suspicious/noThenProperty: Yup.when uses 'then' key */ + then: () => serviceNetworkValidationSchema.concat(IPv4ValidationSchema), otherwise: () => values.serviceNetworks && values.serviceNetworks?.length >= 2 ? serviceNetworkValidationSchema.concat( dualStackValidationSchema('service network', openshiftVersion), ) : Yup.array(),Also applies to: 100-106, 110-116
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
libs/ui-lib/lib/common/components/clusterConfiguration/utils.test.ts(1 hunks)libs/ui-lib/lib/common/components/clusterConfiguration/utils.ts(2 hunks)libs/ui-lib/lib/common/components/ui/formik/_validationSchemas.test.ts(2 hunks)libs/ui-lib/lib/common/components/ui/formik/validationSchemas.ts(2 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AdvancedNetworkFields.tsx(4 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AvailableSubnetsControl.tsx(2 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkConfiguration.tsx(2 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkConfigurationForm.tsx(1 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/StackTypeControl.tsx(3 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/SubnetsDropdown.tsx(4 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/VirtualIPControlGroup.tsx(4 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts(4 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/review/ReviewNetworkingTable.tsx(4 hunks)libs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.test.ts(1 hunks)libs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- libs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.test.ts
- libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/SubnetsDropdown.tsx
- libs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.tsx
- libs/ui-lib/lib/common/components/ui/formik/_validationSchemas.test.ts
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-10-21T04:53:07.339Z
Learnt from: linoyaslan
PR: openshift-assisted/assisted-installer-ui#3190
File: libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/StackTypeControl.tsx:71-91
Timestamp: 2025-10-21T04:53:07.339Z
Learning: In libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/StackTypeControl.tsx, dual-stack mode requires both IPv4 and IPv6 networks (machineNetworks must have at least 2 entries: one IPv4 and one IPv6). Dual-stack with only IPv6 is not supported—dual-stack must have both IP families present.
Applied to files:
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AvailableSubnetsControl.tsxlibs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/VirtualIPControlGroup.tsxlibs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AdvancedNetworkFields.tsxlibs/ui-lib/lib/ocm/components/clusterConfiguration/review/ReviewNetworkingTable.tsxlibs/ui-lib/lib/common/components/ui/formik/validationSchemas.tslibs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/StackTypeControl.tsxlibs/ui-lib/lib/common/components/clusterConfiguration/utils.test.tslibs/ui-lib/lib/common/components/clusterConfiguration/utils.tslibs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts
📚 Learning: 2025-10-21T04:40:36.274Z
Learnt from: linoyaslan
PR: openshift-assisted/assisted-installer-ui#3190
File: libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AdvancedNetworkFields.tsx:55-63
Timestamp: 2025-10-21T04:40:36.274Z
Learning: In libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AdvancedNetworkFields.tsx, the network reordering logic in the useEffect (swapping clusterNetworks and serviceNetworks based on the primary machine network's IP family) is for UI consistency only. Validation of empty or invalid CIDRs is handled separately by validation schemas, not by the reordering logic.
Applied to files:
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/VirtualIPControlGroup.tsxlibs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AdvancedNetworkFields.tsxlibs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkConfiguration.tsxlibs/ui-lib/lib/ocm/components/clusterConfiguration/review/ReviewNetworkingTable.tsxlibs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkConfigurationForm.tsxlibs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/StackTypeControl.tsxlibs/ui-lib/lib/common/components/clusterConfiguration/utils.tslibs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts
📚 Learning: 2025-10-20T08:57:04.935Z
Learnt from: linoyaslan
PR: openshift-assisted/assisted-installer-ui#3190
File: libs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.tsx:43-45
Timestamp: 2025-10-20T08:57:04.935Z
Learning: In the file `libs/ui-lib/lib/ocm/components/clusterDetail/ClusterProperties.tsx`, the `getStackTypeLabel` function returns 'Dual-stack' or 'IPv4' because the system currently only supports IPv4 single-stack clusters. IPv6-only single-stack clusters are not supported.
Applied to files:
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/VirtualIPControlGroup.tsxlibs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AdvancedNetworkFields.tsxlibs/ui-lib/lib/ocm/components/clusterConfiguration/review/ReviewNetworkingTable.tsxlibs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/StackTypeControl.tsx
📚 Learning: 2025-10-19T17:22:52.502Z
Learnt from: linoyaslan
PR: openshift-assisted/assisted-installer-ui#3190
File: libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts:69-69
Timestamp: 2025-10-19T17:22:52.502Z
Learning: CIM UI changes in the repository openshift-assisted/assisted-installer-ui (e.g., files under libs/ui-lib/lib/cim/) are handled separately by the CIM team and should be tracked via separate issues rather than being included in PRs for other UI components.
Applied to files:
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts
🧬 Code graph analysis (10)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AvailableSubnetsControl.tsx (5)
libs/ui-lib/lib/common/types/clusters.ts (1)
NetworkConfigurationValues(34-53)libs/ui-lib/lib/common/config/constants.ts (2)
DUAL_STACK(300-300)NO_SUBNET_SET(294-294)libs/ui-lib/lib/ocm/store/slices/current-cluster/selectors.ts (1)
selectCurrentClusterPermissionsState(9-17)libs/ui-lib/lib/common/utils.ts (1)
isMajorMinorVersionEqualOrGreater(138-143)libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/SubnetsDropdown.tsx (1)
SubnetsDropdown(43-168)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/AdvancedNetworkFields.tsx (2)
libs/ui-lib/lib/common/types/clusters.ts (1)
NetworkConfigurationValues(34-53)libs/ui-lib/lib/common/config/constants.ts (1)
DUAL_STACK(300-300)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkConfiguration.tsx (1)
libs/ui-lib/lib/common/components/clusterConfiguration/utils.ts (1)
isDualStack(204-214)
libs/ui-lib/lib/ocm/components/clusterConfiguration/review/ReviewNetworkingTable.tsx (1)
libs/ui-lib/lib/common/components/clusterConfiguration/utils.ts (1)
isDualStack(204-214)
libs/ui-lib/lib/common/components/ui/formik/validationSchemas.ts (2)
libs/ui-lib/lib/common/utils.ts (1)
isMajorMinorVersionEqualOrGreater(138-143)libs/types/assisted-installer-service.d.ts (1)
MachineNetwork(2182-2191)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkConfigurationForm.tsx (1)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts (1)
getNetworkConfigurationValidationSchema(72-120)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/StackTypeControl.tsx (1)
libs/ui-lib/lib/common/config/constants.ts (1)
NO_SUBNET_SET(294-294)
libs/ui-lib/lib/common/components/clusterConfiguration/utils.test.ts (2)
libs/types/assisted-installer-service.d.ts (3)
MachineNetwork(2182-2191)ClusterNetwork(620-633)ServiceNetwork(2577-2586)libs/ui-lib/lib/common/components/clusterConfiguration/utils.ts (1)
isDualStack(204-214)
libs/ui-lib/lib/common/components/clusterConfiguration/utils.ts (2)
libs/ui-lib/lib/common/utils.ts (1)
isMajorMinorVersionEqualOrGreater(138-143)libs/types/assisted-installer-service.d.ts (1)
Cluster(110-407)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts (3)
libs/ui-lib/lib/common/components/clusterConfiguration/utils.ts (1)
isDualStack(204-214)libs/ui-lib/lib/common/types/clusters.ts (1)
NetworkConfigurationValues(34-53)libs/ui-lib/lib/common/components/ui/formik/validationSchemas.ts (3)
dualStackValidationSchema(707-726)clusterNetworksValidationSchema(686-696)serviceNetworkValidationSchema(698-705)
🪛 Biome (2.1.2)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts
[error] 111-111: Do not add then to an object.
(lint/suspicious/noThenProperty)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: format
- GitHub Check: translation-files
- GitHub Check: tests
- GitHub Check: lint
🔇 Additional comments (6)
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/VirtualIPControlGroup.tsx (1)
146-146: Verify whether VIP labels need IP family indicators in dual-stack mode.The labels have been simplified to "API IP" and "Ingress IP" (removing the explicit "(IPv4)" suffix), but the component provides no context about which IP family these VIPs belong to when in dual-stack configurations. In dual-stack mode with IPv6 as the primary family (OpenShift ≥ 4.12), users cannot determine whether these VIPs are IPv4 or IPv6 addresses.
Additionally, the type definitions support up to two VIPs per type in dual-stack mode (one per IP stack), but the component currently displays only the first VIP. Clarify one of the following:
- Are VIPs intentionally single-family (always primary)? If so, consider adding a comment or documentation noting this.
- Should secondary VIPs have separate form fields and labels (e.g., "API IP (Primary)" / "API IP (Secondary)")? If so, the current labels are incomplete.
Also applies to: 165-165, 191-191, 207-207
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkConfiguration.tsx (1)
226-231: LGTM: version-aware wiring and UX.Auto-enabling advanced for dual‑stack and passing
openshiftVersionto subnets control looks correct.Also applies to: 295-296
libs/ui-lib/lib/ocm/components/clusterConfiguration/review/ReviewNetworkingTable.tsx (1)
61-70: LGTM: version-aware dual‑stack rendering.Consistently passing
openshiftVersiontoisDualStackacross rows is correct.Also applies to: 118-125, 141-148, 164-171
libs/ui-lib/lib/common/components/clusterConfiguration/utils.test.ts (1)
1-275: Solid coverage.Version-conditional paths and edge cases are well covered.
libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/NetworkConfigurationForm.tsx (1)
203-206: LGTM: schema now version‑aware.Passing
cluster.openshiftVersionwith matching memo deps is correct.libs/ui-lib/lib/ocm/components/clusterConfiguration/networkConfiguration/networkConfigurationValidation.ts (1)
72-76: CIM call site missing openshiftVersion parameter.Verification found that
NetworkConfigurationForm.tsx:204correctly passescluster.openshiftVersion, butuse-networking-formik.ts:96is missing this parameter. If the CIM module is in-scope for this PR, update line 96 to:return getNetworkConfigurationValidationSchema(initialValues, hostSubnets, cluster.openshiftVersion);Otherwise, open a follow-up issue for the CIM owners.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ammont82, linoyaslan 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-merge-bot
bot
merged commit 895b5c8
into
openshift-assisted:master
* Remove preview badge from bundles card (#3169) * Migration to PF6 (#3168) * pf6 deps and codemods updates Signed-off-by: gitdallas <5322142+gitdallas@users.noreply.github.com> get emptystate pf6 issues building Signed-off-by: gitdallas <5322142+gitdallas@users.noreply.github.com> fix chip/label changes Signed-off-by: gitdallas <5322142+gitdallas@users.noreply.github.com> fix icon type Signed-off-by: gitdallas <5322142+gitdallas@users.noreply.github.com> fix a prop type that codemods missed Signed-off-by: gitdallas <5322142+gitdallas@users.noreply.github.com> update pf5 classnames Signed-off-by: gitdallas <5322142+gitdallas@users.noreply.github.com> fix tokens, some cleanup Signed-off-by: gitdallas <5322142+gitdallas@users.noreply.github.com> fix tests and remaining v5->v6 Signed-off-by: gitdallas <5322142+gitdallas@users.noreply.github.com> yarn lock Signed-off-by: gitdallas <5322142+gitdallas@users.noreply.github.com> work on some cy tests Signed-off-by: gitdallas <5322142+gitdallas@users.noreply.github.com> fix tests for pf6 changes Signed-off-by: gitdallas <5322142+gitdallas@users.noreply.github.com> fix some tests Signed-off-by: gitdallas <5322142+gitdallas@users.noreply.github.com> fix build error Signed-off-by: gitdallas <5322142+gitdallas@users.noreply.github.com> * Change version of package.json to 2.0.0 for testing * MGMT-20974: Rremove unnecessary hyphens in helper text operators * MGMT-20968: align 'learn more about openshift releases' link in the first page of the cluster creation wizard * MGMT-20965: Put the same min-width in the dropdowns of the first page of the cluster creation wizard * MGMT-20966: improving the readibility of text on Troubleshooter Modal * MGMT-20972: Cluster summary section is not collapsable * MGMT-20964: Detached Warning Message for TMPv2 selection * Changes in PrismCode to use the correct color * MGMT-20969: hostname column header is truncated and unredeable * MGMT-20967: errors beneath text boxes dissapears but icon remains in place * fix (20975): make bundle selected operators show in count Signed-off-by: gitdallas <5322142+gitdallas@users.noreply.github.com> * MGMT-21103: change size of Provisioning type droodown in Add hosts modal * MGMT-21105: Networking page-machine network dropdown truncate values * add layout with gutter around cluster progress/buttons Signed-off-by: gitdallas <5322142+gitdallas@users.noreply.github.com> * MGMT-21147: R&C page - missing vertical spacing between kubeconfig warning and buttons * MGMT-21151: inconsistend icon and text alignment in preflight checks in R&C page * Add data-test-id to MenuToggle components * Add data-testid to HelperTextItem components * Add data-testid to BreadCrumb component * Add data-testid to Spinner component * Add data-testid to Wizard components * Add data-testid to Host's network configuration group * Add data-testid to Use bond option * Add data-testid to Add hosts modal * Add more data-testids for tests * Solving problem with operators count * Remove duplicated scrollbars * Solving errors in Networking page * Add data-testid to close button in Events modal * Migration of TextContent component * Solving problems with some old components * Solving lint issues * Solving problems with unit tests * Solving format issues * Remove unnecessary test * Solving problems with tests --------- Signed-off-by: gitdallas <5322142+gitdallas@users.noreply.github.com> Co-authored-by: gitdallas <5322142+gitdallas@users.noreply.github.com> * Use masthead (#3173) * Button text (#3174) * OCPBUGS-61953: Update dependency sourcing to remote (#3155) * Bump axios from 1.6.8 to 1.12.2 (#3180) Bumps [axios](https://github.com/axios/axios) from 1.6.8 to 1.12.2. - [Release notes](https://github.com/axios/axios/releases) - [Changelog](https://github.com/axios/axios/blob/v1.x/CHANGELOG.md) - [Commits](axios/axios@v1.6.8...v1.12.2) --- updated-dependencies: - dependency-name: axios dependency-version: 1.12.2 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Updating assisted-installer-ui-container image to be consistent with ART for 4.21 (#3179) Reconciling with https://github.com/openshift/ocp-build-data/tree/4fbe3fab45239dc4be6f5d9d98a0bf36e0274ec9/images/agent-installer-ui.yml Co-authored-by: AOS Automation Release Team <noreply@redhat.com> * OCPBUGS-61787: Change NMstate operator docs link (#3182) * MGMT-21862: Add message about vSphere limitations (#3181) * OCPBUGS-62680: Include assisted disconnected UI image in release payload (#3188) * Fixing errors when creating cluster from Assisted Migration (#3191) * MGMT-21025: installing Two Node OpenShift with Arbiter (TNA) (#3170) * Clean up ClusterDetailsFormFields * Allow TNA arbiter in CIM * Remove unused component (#3193) * Add unique data-testid to dualstack subnet dropdowns (#3199) * Tweak spacing between advanced network fields (#3198) * Tweak host status spacing (#3200) * Improve alert spacing (#3201) * MGMT-21825: Textarea field should show both error and helper text (#3202) * Improve alert spacing * Show both error and helper text under textarea fields * Add border to table headers (#3197) * Remove border from rich input field (#3208) * Bump happy-dom from 15.10.2 to 20.0.0 (#3211) Bumps [happy-dom](https://github.com/capricorn86/happy-dom) from 15.10.2 to 20.0.0. - [Release notes](https://github.com/capricorn86/happy-dom/releases) - [Commits](capricorn86/happy-dom@v15.10.2...v20.0.0) --- updated-dependencies: - dependency-name: happy-dom dependency-version: 20.0.0 dependency-type: direct:development ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Remove nested 'Content' components (#3196) * Upgrade the PF modal component in /common and /ocm (#3213) * Bump happy-dom from 20.0.0 to 20.0.2 (#3218) Bumps [happy-dom](https://github.com/capricorn86/happy-dom) from 20.0.0 to 20.0.2. - [Release notes](https://github.com/capricorn86/happy-dom/releases) - [Commits](capricorn86/happy-dom@v20.0.0...v20.0.2) --- updated-dependencies: - dependency-name: happy-dom dependency-version: 20.0.2 dependency-type: direct:development ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Consistent ISO Download behavior (#3221) * Add 'noopener noreferrer' to ISO download button (#3222) * Fix preflight check collapsed styling (#3215) * Remove unnecessary custom manifest field (#3217) * Bump vite from 5.4.20 to 5.4.21 (#3223) Bumps [vite](https://github.com/vitejs/vite/tree/HEAD/packages/vite) from 5.4.20 to 5.4.21. - [Release notes](https://github.com/vitejs/vite/releases) - [Changelog](https://github.com/vitejs/vite/blob/v5.4.21/packages/vite/CHANGELOG.md) - [Commits](https://github.com/vitejs/vite/commits/v5.4.21/packages/vite) --- updated-dependencies: - dependency-name: vite dependency-version: 5.4.21 dependency-type: direct:development ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * MGMT-21625: Add Dual-Stack with Primary IPv6 Support (#3190) This PR adds support for IPv6-primary dual-stack clusters, enabling users to create dual-stack clusters where IPv6 is the primary IP stack (version-aware for OpenShift 4.12+) * sort OCP versions in create infra (#3171) Signed-off-by: Lior Soffer <liorsoffer1@gmail.com> * Update types/parsing for latest lightspeed API (#3226) * Update release job to use trusted-publishers flow (#3228) * Update setup-node config (#3232) * - MGMT-22057:Primary IPv6 should be Tech Preview (#3231) - MGMT-22045: Change style of Dualstack - IPv4 / IPv6 section headers in subnet dropdown - When we change the primary machine network IP to ipv6 we have to select the seconday machine network IP to ipv4 * MGMT-22080: Allow users to install Openshift AI as standalone operator in SNO clusters (#3234) * Release job: Configure Yarn for npm registry (#3235) * configure yarn before publish (#3239) * Pass OIDC token to yarn config (#3241) * MGMT-22047: Add apiVIP and ingressVIP for dual-stack ipv4 and ipv6 (#3246) * Add apiVIP and ingressVIP for dual-stack ipv4 and ipv6 * Add validations to ensure that users add primary and secondary apiVips and ingressVips when using dual-stack * Updating tests to dual-stack changes * Prevent InfraEnv creation on ABI to support OVE Late-Binding workflow (#3244) Signed-off-by: Elay Aharoni <elayaha@gmail.com> * MGMT-22116: Add info about apiVIPs and ingressVIPs for dual-stack in Review and Create page (#3250) * Add info about apiVIPs and ingressVIPs for dual-stack in Review and Create page * Fixing review comments * MGMT-22119: Solving errors with SNO dual-stack (#3249) * Solving errors with SNO dual-stack * Solving errors when change between dual-stack and single-stack * MGMT-22165: Remove what's new link until the info appear in the chatbot (#3253) * Update logic to extract the tool response (#3255) * MGMT-17220: dual-stack with second machine network not populates (#3256) * MGMT-17220: dual-stack with second machine network not populates * Improving the code * Make the parsing compatible with new & old API (#3260) * MGMT-22047: solving errors with dual-stack (#3263) * MGMT-22047: solving errors with dual-stack 1. Changes in AvailableSubnetsControl.tsx so our corrective setFieldValue calls don’t validate immediately. This avoids heavy, repeated validations the moment you pick the first IPv6 option and should stop the freeze when secondary VIPs are IPv6 and empty. Specifically, set the third arg to false for: - Auto-select initialization of machineNetworks - Duplicate-primary fix that updates machineNetworks.1.cidr 2. Changes in AdvancesNetworkFields.tsx: -I found the infinite loop in AdvancedNetworkFields.tsx: when the primary machine network flips to IPv6, the effect was swapping clusterNetworks/serviceNetworks even when both entries had the same IP family, causing continuous reorders and a freeze. -I added guards so we only swap when both entries exist and have opposite families, and the first one mismatches the primary machine family. Lint is clean. * Solving issues in code * MGMT-21837: in YAML view in Static Network Configuration we add the radio buttons to change the form to another view (#3268) * late binding hosts to cluster in ABI (#3259) Signed-off-by: Elay Aharoni <elayaha@gmail.com> * AGENT-1373: Rename feature gate to NoRegistryClusterInstall (#3267) Update the feature gate name from NoRegistryClusterOperations to NoRegistryClusterInstall. * AGENT-1352: Handle cluster reset with late binding (#3270) * late binding hosts to cluster in ABI Signed-off-by: Elay Aharoni <elayaha@gmail.com> * Handle cluster reset on ABI Signed-off-by: Elay Aharoni <elayaha@gmail.com> --------- Signed-off-by: Elay Aharoni <elayaha@gmail.com> * MGMT-22281: Dual stack seconday vips fields not mandatory (#3275) * Show TechPreview badge only in Primary Machine network when user chooses ipV6 IP (#3276) * add new fields to above sea level ABI (#3274) Signed-off-by: Elay Aharoni <elayaha@gmail.com> * Revert "Handle cluster reset on ABI" (#3281) This reverts commit d35b70a. * add loki and logging operators (#3285) Signed-off-by: Elay Aharoni <elayaha@gmail.com> * OCPBUGS-65657: Display OpenShift AI GPU validation message from API (#3279) Fix UI to show the friendly label and detailed message from the API instead of displaying the technical validation ID when OpenShift AI operator is selected without GPU support. * Bug fix: OVE Agent Installer UI: Red Hat OCP logo not displaying correctly (#3289) Signed-off-by: Elay Aharoni <elayaha@gmail.com> * Adding TechPreview Budge for Assisted installer and agent (#3293) Signed-off-by: Elay Aharoni <elayaha@gmail.com> * MGMT-20138 : Show 'Add hosts' tab for all cases (#3297) * Edit OWNERS file (#3172) * Bump js-yaml from 4.1.0 to 4.1.1 (#3264) Bumps [js-yaml](https://github.com/nodeca/js-yaml) from 4.1.0 to 4.1.1. - [Changelog](https://github.com/nodeca/js-yaml/blob/master/CHANGELOG.md) - [Commits](nodeca/js-yaml@4.1.0...4.1.1) --- updated-dependencies: - dependency-name: js-yaml dependency-version: 4.1.1 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump zx from 7.2.3 to 8.8.5 (#3273) Bumps [zx](https://github.com/google/zx) from 7.2.3 to 8.8.5. - [Release notes](https://github.com/google/zx/releases) - [Commits](google/zx@7.2.3...8.8.5) --- updated-dependencies: - dependency-name: zx dependency-version: 8.8.5 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump mdast-util-to-hast from 13.2.0 to 13.2.1 (#3287) Bumps [mdast-util-to-hast](https://github.com/syntax-tree/mdast-util-to-hast) from 13.2.0 to 13.2.1. - [Release notes](https://github.com/syntax-tree/mdast-util-to-hast/releases) - [Commits](syntax-tree/mdast-util-to-hast@13.2.0...13.2.1) --- updated-dependencies: - dependency-name: mdast-util-to-hast dependency-version: 13.2.1 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * provides parameters for the GET /v2/operators/bundles route (#3306) Signed-off-by: Elay Aharoni <elayaha@gmail.com> * Bump js-yaml from 4.1.0 to 4.1.1 (#3310) Bumps [js-yaml](https://github.com/nodeca/js-yaml) from 4.1.0 to 4.1.1. - [Changelog](https://github.com/nodeca/js-yaml/blob/master/CHANGELOG.md) - [Commits](nodeca/js-yaml@4.1.0...4.1.1) --- updated-dependencies: - dependency-name: js-yaml dependency-version: 4.1.1 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Fix Cluster summary styling (#3216) * remove external platforms field from below sea level UI (#3316) Signed-off-by: Elay Aharoni <elayaha@gmail.com> * Openshift AI Bundle on SNO enables ODF and LVM which are uncompatible (#3320) Signed-off-by: Elay Aharoni <elayaha@gmail.com> * change ABI above sea level iso size (#3322) Signed-off-by: Elay Aharoni <elayaha@gmail.com> * MGMT-22465: Merge the contents of releases/v2.17-cim into master (#3319) * Fix patches when there were 0 nodes in hypershift nodepool (#3177) * Make OVN the default network type if the version is invalid (#3183) * MGMT-20076: Support external platform in Assisted-installer Kube API (#3151) * Add External platforms field * Restructure files related to CIM cluster deployment wizard * Create SelectFieldWithSearch component * Update '@openshift-console/dynamic-plugin-sdk' * CIM custom manifest step * CIM custom manifests review * Make 'Baremetal' the default external platform * Translations for 2.16-cim (#3225) * MGMT-21025: installing Two Node OpenShift with Arbiter (TNA) (#3224) * Clean up ClusterDetailsFormFields * Allow TNA arbiter in CIM * Set 'userManagedNetworking' as true with 'external' platform (#3245) * Tweak TNA-related strings in CIM (#3248) * Restrict platform options for SNO clusters (#3262) Signed-off-by: Lior Soffer <liorsoffer1@gmail.com> * Fix: Undefined OpenShift version producing an incorrect documentation link (#3271) Signed-off-by: Lior Soffer <liorsoffer1@gmail.com> * Fix bug - the option to remove host from the cluster disabled while the host is installing (#3219) Signed-off-by: Elay Aharoni <elayaha@gmail.com> * Implement user interface for the multiple SSH keys (#3292) Signed-off-by: Lior Soffer <liorsoffer1@gmail.com> * Edit OWNERS file (#3303) Co-authored-by: jgyselov <jgyselov@redhat.com> * Fix arbiter translation (#3304) Co-authored-by: jgyselov <jgyselov@redhat.com> * [releases/v2.17-cim] MGMT-22264: Add 'Labels' and 'GPU' columns to infra env host table (#3307) * Add GPUs column to infra agent table * Add labels column and filtering to infra agent table --------- Co-authored-by: jgyselov <jgyselov@redhat.com> * Prevent mass approve crash when hosts are still discovering (#3305) Co-authored-by: jgyselov <jgyselov@redhat.com> * Fix 'Required' translations (#3308) Co-authored-by: jgyselov <jgyselov@redhat.com> * Do not exclude hosts with SpecSyncError status from host selection during binding (#3309) Co-authored-by: jgyselov <jgyselov@redhat.com> * MGMT-22438: Handle empty labels in infra env host table (#3314) * Handle empty labels in infra env host table Signed-off-by: Lior Soffer <liorsoffer1@gmail.com> * format fix Signed-off-by: Lior Soffer <liorsoffer1@gmail.com> --------- Signed-off-by: Lior Soffer <liorsoffer1@gmail.com> * MGMT-19743: Icon from agent status available is confusing (#3220) * Fix bug - the option to remove host from the cluster disabled while the host is installing Signed-off-by: Elay Aharoni <elayaha@gmail.com> * icon from agent status Available is confusing Signed-off-by: Elay Aharoni <elayaha@gmail.com> --------- Signed-off-by: Elay Aharoni <elayaha@gmail.com> Co-authored-by: Julie Gyselova <jgyselov@redhat.com> --------- Signed-off-by: Lior Soffer <liorsoffer1@gmail.com> Signed-off-by: Elay Aharoni <elayaha@gmail.com> Co-authored-by: Lior Soffer <liorsoffer1@gmail.com> Co-authored-by: Elay Aharoni <elayaha@gmail.com> Co-authored-by: OpenShift Cherrypick Robot <openshift-cherrypick-robot@redhat.com> --------- Signed-off-by: gitdallas <5322142+gitdallas@users.noreply.github.com> Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: Lior Soffer <liorsoffer1@gmail.com> Signed-off-by: Elay Aharoni <elayaha@gmail.com> Co-authored-by: Montse Ortega Gallart <ammont82@users.noreply.github.com> Co-authored-by: gitdallas <5322142+gitdallas@users.noreply.github.com> Co-authored-by: Pawan Pinjarkar <ppinjark@redhat.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: OpenShift Bot <openshift-bot@redhat.com> Co-authored-by: AOS Automation Release Team <noreply@redhat.com> Co-authored-by: Linoy Hadad <Linoyaslan@gmail.com> Co-authored-by: Lior Soffer <liorsoffer1@gmail.com> Co-authored-by: Rastislav Wagner <rawagner@redhat.com> Co-authored-by: Elay Aharoni <elayaha@gmail.com> Co-authored-by: Richard Su <rwsu@redhat.com> Co-authored-by: Yoav Schwammenthal <33420608+yoavsc0302@users.noreply.github.com> Co-authored-by: OpenShift Cherrypick Robot <openshift-cherrypick-robot@redhat.com>
3 participants
This PR adds support for IPv6-primary dual-stack clusters, enabling users to create dual-stack clusters where IPv6 is the primary IP stack (version-aware for OpenShift 4.12+).
assisted-by: cursor
Screenshots:
Summary by CodeRabbit
New Features
Validation
UI
Tests