Skip to content

OCPBUGS-84303: fix(api): add IPv6 OVN join subnet config to prevent dual-stack routing collision #8421

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
openshift-merge-bot merged 1 commit into from
May 28, 2026
Merged

OCPBUGS-84303: fix(api): add IPv6 OVN join subnet config to prevent dual-stack routing collision #8421

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 74 additions & 0 deletions api/hypershift/v1beta1/operator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,20 @@ import (
operatorv1 "github.com/openshift/api/operator/v1"
)

const (
// KubevirtDefaultV6InternalJoinSubnet is the default IPv6 OVN join subnet
// for KubeVirt hosted clusters. The upstream OVN-Kubernetes default is fd98::/64,
// but KubeVirt guests use fd99::/64 to avoid collisions with the management
// cluster's join subnet when both run OVN-Kubernetes.
KubevirtDefaultV6InternalJoinSubnet = "fd99::/64"

// KubevirtDefaultV4InternalSubnet is the default IPv4 OVN internal subnet
// for KubeVirt hosted clusters. The upstream OVN-Kubernetes default gateway
// router LRP CIDR is 100.64.0.0/16 and the default UDNs is 100.65.0.0/16.
// KubeVirt guests use 100.66.0.0/16 to avoid collisions with the management cluster.
KubevirtDefaultV4InternalSubnet = "100.66.0.0/16"
)

// +kubebuilder:validation:Enum="";Normal;Debug;Trace;TraceAll
type LogLevel string

Expand Down Expand Up @@ -38,6 +52,7 @@ type ClusterVersionOperatorSpec struct {
OperatorLogLevel LogLevel `json:"operatorLogLevel,omitempty"`
}

// +kubebuilder:validation:XValidation:rule="!has(oldSelf.ovnKubernetesConfig) || has(self.ovnKubernetesConfig)", message="ovnKubernetesConfig is immutable once set and cannot be removed"
type ClusterNetworkOperatorSpec struct {
// disableMultiNetwork when set to true disables the Multus CNI plugin and related components
// in the hosted cluster. This prevents the installation of multus daemon sets in the
Expand All @@ -62,7 +77,11 @@ type ClusterNetworkOperatorSpec struct {
// OVNKubernetesConfig contains OVN-Kubernetes specific configuration options.
// https://github.com/openshift/api/blob/6d3c4e25a8d3aeb57ad61649d80c38cbd27d1cc8/operator/v1/types_network.go#L400-L471
// +kubebuilder:validation:XValidation:rule="!has(self.ipv4) || !has(self.ipv4.internalJoinSubnet) || !has(self.ipv4.internalTransitSwitchSubnet) || self.ipv4.internalJoinSubnet != self.ipv4.internalTransitSwitchSubnet", message="internalJoinSubnet and internalTransitSwitchSubnet must not be the same"
// +kubebuilder:validation:XValidation:rule="!has(self.ipv6) || !has(self.ipv6.internalJoinSubnet) || !has(self.ipv6.internalTransitSwitchSubnet) || self.ipv6.internalJoinSubnet != self.ipv6.internalTransitSwitchSubnet", message="ipv6 internalJoinSubnet and internalTransitSwitchSubnet must not be the same"
// +kubebuilder:validation:XValidation:rule="!has(oldSelf.mtu) || has(self.mtu)",message="mtu is immutable once set and cannot be removed"
// +kubebuilder:validation:XValidation:rule="!has(oldSelf.ipv6) || has(self.ipv6)", message="ipv6 is immutable once set and cannot be removed"
// +kubebuilder:validation:XValidation:rule="!has(oldSelf.ipv6) || !has(oldSelf.ipv6.internalJoinSubnet) || (has(self.ipv6) && has(self.ipv6.internalJoinSubnet))", message="ipv6.internalJoinSubnet cannot be removed once set"
// +kubebuilder:validation:XValidation:rule="!has(oldSelf.ipv6) || !has(oldSelf.ipv6.internalTransitSwitchSubnet) || (has(self.ipv6) && has(self.ipv6.internalTransitSwitchSubnet))", message="ipv6.internalTransitSwitchSubnet cannot be removed once set"
Comment on lines +80 to +84
Copy link
Copy Markdown
Contributor

@everettraven everettraven May 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For these immutability rules, what happens if the parent ovnKubernetesConfig field is set, removed, and re-applied with new values?

Copy link
Copy Markdown
Contributor Author

@orenc1 orenc1 May 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you're right, good catch. if we remove the entire ovnKubernetesConfig field, then re-apply with different values, the immutability rules could be bypassed.
to mitigate that, i'm adding a validation rule of "cannot be removed once set" on ClusterNetworkOperatorSpec for ovnKubernetesConfig.

Copy link
Copy Markdown
Contributor

@everettraven everettraven May 18, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It still looks like you could just remove the ClusterNetworkOperatorSpec field based on:

hypershift/api/hypershift/v1beta1/hostedcluster_types.go

Lines 2372 to 2375 in f76be88

// clusterNetworkOperator specifies the configuration for the Cluster Network Operator in the hosted cluster.
//
// +optional
ClusterNetworkOperator *ClusterNetworkOperatorSpec `json:"clusterNetworkOperator,omitempty"`

If you want true immutability you need to set the rule all the way up to the first required field, which looks like it means on the root HostedCluster type because even spec is optional:

hypershift/api/hypershift/v1beta1/hostedcluster_types.go

Lines 2411 to 2413 in f76be88

// spec is the desired behavior of the HostedCluster.
// +optional
Spec HostedClusterSpec `json:"spec,omitempty"`

I'd make sure you pay particular attention to how immutability applies as you go up the chain of fields to make sure you don't make something that is OK to mutate immutable.

Copy link
Copy Markdown
Contributor Author

@orenc1 orenc1 May 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. how do you suggest to proceed? should I modify the validation rules of the higher fields in the chain, up to the root? I feel it's beyond the scope of this PR.

Copy link
Copy Markdown
Contributor

@everettraven everettraven May 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have a strong preference. If this significantly increases the scope of this work to achieve this, I'm OK with best effort immutability for now with a more concrete immutability story for the near-ish future.

// +kubebuilder:validation:MinProperties=1
type OVNKubernetesConfig struct {
// ipv4 allows users to configure IP settings for IPv4 connections. When omitted,
Expand All @@ -71,6 +90,15 @@ type OVNKubernetesConfig struct {
// +optional
IPv4 *OVNIPv4Config `json:"ipv4,omitempty"`

// ipv6 allows users to configure IP settings for IPv6 connections. When omitted,
// this means no opinions and the default configuration is used. Check individual
// fields within ipv6 for details of default values.
// For KubeVirt hosted clusters using dual-stack networking, it is recommended to
// set ipv6.internalJoinSubnet to a value different from the management cluster's
// join subnet (default fd98::/64) to avoid IPv6 routing conflicts.
// +optional
IPv6 OVNIPv6Config `json:"ipv6,omitzero,omitempty"`

// mtu is the MTU to use for the tunnel interface on hosted cluster nodes.
// This must be 100 bytes smaller than the uplink MTU.
// When unset, the cluster-network-operator will determine the MTU automatically
Expand Down Expand Up @@ -126,6 +154,52 @@ type OVNIPv4Config struct {
InternalJoinSubnet string `json:"internalJoinSubnet,omitempty"`
}

// OVNIPv6Config contains IPv6-specific configuration options for OVN-Kubernetes.
// https://github.com/openshift/api/blob/6d3c4e25a8d3aeb57ad61649d80c38cbd27d1cc8/operator/v1/types_network.go#L541-L570
// +kubebuilder:validation:MinProperties=1
type OVNIPv6Config struct {
Copy link
Copy Markdown
Contributor

@jparrill jparrill May 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per our api/CLAUDE.md conventions, new API types should have a round-trip serialization test to make sure N-1 code can handle data from the new version and vice versa. I know omitzero should make this safe in practice, but having the explicit test keeps us consistent with the guidelines and protects against regressions.

Copy link
Copy Markdown
Contributor Author

@orenc1 orenc1 May 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are no existing round-trip serialization test patterns in this repo's api/ module. The omitzero tag on the non-pointer OVNIPv6Config field handles the N-1 case correctly (zero value is omitted from serialization).

Copy link
Copy Markdown
Contributor

@everettraven everettraven May 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New optional fields that are correctly omitted (as this one is) will round trip successfully.

// internalTransitSwitchSubnet is a v6 subnet in IPv6 CIDR format used internally
// by OVN-Kubernetes for the distributed transit switch in the OVN Interconnect
// architecture that connects the cluster routers on each node together to enable
// east west traffic. The subnet chosen should not overlap with other networks
// specified for OVN-Kubernetes as well as other networks used on the host.
// When omitted, this means no opinion and the platform is left to choose a reasonable
// default which is subject to change over time.
// The current default subnet is fd97::/64.
// The subnet must be large enough to accommodate one IP per node in your cluster.
// The value must be a valid IPv6 CIDR (e.g. fd97::/64). IPv4 addresses,
// IPv4-mapped IPv6 addresses, and dual-stack addresses are not permitted.
// The prefix length must be in the range /0 to /125 inclusive.
// This field is immutable once set.
// +kubebuilder:validation:MaxLength=48
// +kubebuilder:validation:MinLength=3
// +kubebuilder:validation:XValidation:rule="isCIDR(self) && cidr(self).ip().family() == 6", message="Subnet must be in valid IPv6 CIDR format (e.g., fd97::/64)"
// +kubebuilder:validation:XValidation:rule="isCIDR(self) && cidr(self).prefixLength() <= 125", message="subnet must be in the range /0 to /125 inclusive"
Comment thread
everettraven marked this conversation as resolved.
Comment on lines +176 to +177
Copy link
Copy Markdown
Contributor

@everettraven everettraven May 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

General note for any other reviewers - the CIDR CEL library was introduced in Kubernetes 1.30. From what I recall being told, we are look for N-4 compatibility from the kube version skew, so with 4.22 being based on 1.35 that means the oldest kube version we would end up running on would be 1.31. This should be OK.

// +kubebuilder:validation:XValidation:rule="self == oldSelf", message="internalTransitSwitchSubnet is immutable"
// +optional
InternalTransitSwitchSubnet string `json:"internalTransitSwitchSubnet,omitempty"`
// internalJoinSubnet is a v6 subnet used internally by ovn-kubernetes in case the
// default one is being already used by something else. It must not overlap with
// any other subnet being used by OpenShift or by the node network. The size of the
// subnet must be larger than the number of nodes.
// The current default value is fd98::/64.
// For KubeVirt hosted clusters, if this field is not set, HyperShift will
// automatically use fd99::/64 to avoid collisions with the management cluster's
// default join subnet (fd98::/64).
// The subnet must be large enough to accommodate one IP per node in your cluster.
// The value must be a valid IPv6 CIDR (e.g. fd98::/64). IPv4 addresses,
// IPv4-mapped IPv6 addresses, and dual-stack addresses are not permitted.
// The prefix length must be in the range /0 to /125 inclusive.
// This field is immutable once set.
// +kubebuilder:validation:MaxLength=48
// +kubebuilder:validation:MinLength=3
// +kubebuilder:validation:XValidation:rule="isCIDR(self) && cidr(self).ip().family() == 6", message="Subnet must be in valid IPv6 CIDR format (e.g., fd98::/64)"
// +kubebuilder:validation:XValidation:rule="isCIDR(self) && cidr(self).prefixLength() <= 125", message="subnet must be in the range /0 to /125 inclusive"
Comment thread
everettraven marked this conversation as resolved.
// +kubebuilder:validation:XValidation:rule="self == oldSelf", message="internalJoinSubnet is immutable"
// +optional
InternalJoinSubnet string `json:"internalJoinSubnet,omitempty"`
Copy link
Copy Markdown
Contributor

@jparrill jparrill May 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not for this PR, but changing the OVN join or transit subnet on a running cluster would break networking pretty badly. Might be worth a follow-up to add self == oldSelf CEL immutability rules here. The IPv4 fields don't have them either (pre-existing gap), so we could add immutability to both families at once.

Copy link
Copy Markdown
Contributor

@everettraven everettraven May 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we know ahead of time that making a change to the networking configuration on a running cluster would be bad, let's go ahead and prevent it where we can now.

Just a note that immutability rules must be done on the closest parent field that is required, otherwise the field isn't actually immutable because it can be removed by removing an optional parent (or the field itself) in the chain and re-applying with a different value.

Copy link
Copy Markdown
Contributor Author

@orenc1 orenc1 May 14, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, added three layers of protection:

self == oldSelf on each field (prevents value changes)
!has(oldSelf.ipv6) || has(self.ipv6) on OVNKubernetesConfig (prevents removing the ipv6 parent to bypass field immutability)
!has(oldSelf.ipv6) || !has(oldSelf.ipv6.internalJoinSubnet) || (has(self.ipv6) && has(self.ipv6.internalJoinSubnet)) (and same for transit subnet) to prevent removing individual fields through the optional parent chain

}

// IngressOperatorSpec is the specification of the desired behavior of the Ingress Operator.
type IngressOperatorSpec struct {
// endpointPublishingStrategy is used to publish the default ingress controller endpoints.
Expand Down
16 changes: 16 additions & 0 deletions api/hypershift/v1beta1/zz_generated.deepcopy.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

88 changes: 88 additions & 0 deletions ...erated.featuregated-crd-manifests/hostedclusters.hypershift.openshift.io/AAA_ungated.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3193,6 +3193,75 @@ spec:
rule: self.matches('^[0-9]{1,3}\\..*') && int(self.split('/')[0].split('.')[0])
> 0
type: object
ipv6:
description: |-
ipv6 allows users to configure IP settings for IPv6 connections. When omitted,
this means no opinions and the default configuration is used. Check individual
fields within ipv6 for details of default values.
For KubeVirt hosted clusters using dual-stack networking, it is recommended to
set ipv6.internalJoinSubnet to a value different from the management cluster's
join subnet (default fd98::/64) to avoid IPv6 routing conflicts.
minProperties: 1
properties:
internalJoinSubnet:
description: |-
internalJoinSubnet is a v6 subnet used internally by ovn-kubernetes in case the
default one is being already used by something else. It must not overlap with
any other subnet being used by OpenShift or by the node network. The size of the
subnet must be larger than the number of nodes.
The current default value is fd98::/64.
For KubeVirt hosted clusters, if this field is not set, HyperShift will
automatically use fd99::/64 to avoid collisions with the management cluster's
default join subnet (fd98::/64).
The subnet must be large enough to accommodate one IP per node in your cluster.
The value must be a valid IPv6 CIDR (e.g. fd98::/64). IPv4 addresses,
IPv4-mapped IPv6 addresses, and dual-stack addresses are not permitted.
The prefix length must be in the range /0 to /125 inclusive.
This field is immutable once set.
maxLength: 48
minLength: 3
type: string
x-kubernetes-validations:
- message: Subnet must be in valid IPv6 CIDR format
(e.g., fd98::/64)
rule: isCIDR(self) && cidr(self).ip().family() ==
6
- message: subnet must be in the range /0 to /125
inclusive
rule: isCIDR(self) && cidr(self).prefixLength()
<= 125
- message: internalJoinSubnet is immutable
rule: self == oldSelf
internalTransitSwitchSubnet:
description: |-
internalTransitSwitchSubnet is a v6 subnet in IPv6 CIDR format used internally
by OVN-Kubernetes for the distributed transit switch in the OVN Interconnect
architecture that connects the cluster routers on each node together to enable
east west traffic. The subnet chosen should not overlap with other networks
specified for OVN-Kubernetes as well as other networks used on the host.
When omitted, this means no opinion and the platform is left to choose a reasonable
default which is subject to change over time.
The current default subnet is fd97::/64.
The subnet must be large enough to accommodate one IP per node in your cluster.
The value must be a valid IPv6 CIDR (e.g. fd97::/64). IPv4 addresses,
IPv4-mapped IPv6 addresses, and dual-stack addresses are not permitted.
The prefix length must be in the range /0 to /125 inclusive.
This field is immutable once set.
maxLength: 48
minLength: 3
type: string
x-kubernetes-validations:
- message: Subnet must be in valid IPv6 CIDR format
(e.g., fd97::/64)
rule: isCIDR(self) && cidr(self).ip().family() ==
6
- message: subnet must be in the range /0 to /125
inclusive
rule: isCIDR(self) && cidr(self).prefixLength()
<= 125
- message: internalTransitSwitchSubnet is immutable
rule: self == oldSelf
type: object
mtu:
description: |-
mtu is the MTU to use for the tunnel interface on hosted cluster nodes.
Expand Down Expand Up @@ -3220,9 +3289,28 @@ spec:
rule: '!has(self.ipv4) || !has(self.ipv4.internalJoinSubnet)
|| !has(self.ipv4.internalTransitSwitchSubnet) || self.ipv4.internalJoinSubnet
!= self.ipv4.internalTransitSwitchSubnet'
- message: ipv6 internalJoinSubnet and internalTransitSwitchSubnet
must not be the same
rule: '!has(self.ipv6) || !has(self.ipv6.internalJoinSubnet)
|| !has(self.ipv6.internalTransitSwitchSubnet) || self.ipv6.internalJoinSubnet
!= self.ipv6.internalTransitSwitchSubnet'
- message: mtu is immutable once set and cannot be removed
rule: '!has(oldSelf.mtu) || has(self.mtu)'
- message: ipv6 is immutable once set and cannot be removed
rule: '!has(oldSelf.ipv6) || has(self.ipv6)'
- message: ipv6.internalJoinSubnet cannot be removed once
set
rule: '!has(oldSelf.ipv6) || !has(oldSelf.ipv6.internalJoinSubnet)
|| (has(self.ipv6) && has(self.ipv6.internalJoinSubnet))'
- message: ipv6.internalTransitSwitchSubnet cannot be removed
once set
rule: '!has(oldSelf.ipv6) || !has(oldSelf.ipv6.internalTransitSwitchSubnet)
|| (has(self.ipv6) && has(self.ipv6.internalTransitSwitchSubnet))'
type: object
x-kubernetes-validations:
- message: ovnKubernetesConfig is immutable once set and cannot
be removed
rule: '!has(oldSelf.ovnKubernetesConfig) || has(self.ovnKubernetesConfig)'
ingressOperator:
description: |-
ingressOperator specifies the configuration for the Ingress Operator in the hosted cluster.
Expand Down
Loading