Skip to content

fix(crd): make vlanInterfaces item validation actually take effect - #7108

Draft
oilbeater wants to merge 1 commit into
masterfrom
worktree-fix-vlan-interfaces-crd-marker
Draft

fix(crd): make vlanInterfaces item validation actually take effect#7108
oilbeater wants to merge 1 commit into
masterfrom
worktree-fix-vlan-interfaces-crd-marker

Conversation

@oilbeater

Copy link
Copy Markdown
Member

What

ProviderNetwork.Spec.VlanInterfaces carried this marker:

// +kubebuilder:validation:Items={Type=string,Pattern=`^[a-zA-Z0-9_-]+\\.[0-9]{1,4}$`}

Items={...} is not a marker controller-gen recognises, so it was silently
dropped. The generated CRD ships a bare items: {type: string} — the field
accepts any string today.

The pattern was wrong too: inside a Go raw string literal \\. is a literal
backslash followed by any character, not an escaped dot, so the expression
rejects every legal value such as eth0.10. Fixing only the marker name
would have turned a silent no-op into a hard regression
— worth knowing for
anyone reviewing similar markers.

Change

Use the items: marker prefix controller-gen supports, escape the dot once,
and cap the length at IFNAMSIZ-1 to match defaultInterface and
customInterfaces[].interface:

// +kubebuilder:validation:items:Pattern=`^[a-zA-Z0-9_-]+\.[0-9]{1,4}$`
// +kubebuilder:validation:items:MaxLength=15

Regenerated CRDs via make gen-crd (charts v1/v2 + install.sh).

Why this shape

The pattern encodes exactly what the daemon accepts. createVlanSubinterfaces
splits the name on the first dot and requires the parent to match the provider
network's default interface, and ExtractVlanIDFromInterface derives the VLAN
ID from the suffix. A name the daemon cannot parse is only reported as a
klog.Warningf and then skipped, so today a typo in this field is invisible to
the user — the resource is accepted and the interface is silently never
programmed.

Validation

Checked against the regenerated schema:

value before after
eth0.10, bond0.4094, enp1s0f0.100, eth-0.1, eth_0.4094 accepted (no check) accepted
eth0, vlan100, eth0.10.20, eth0.99999, .10, eth0. accepted (no check) rejected

make lint (includes verify-crd) passes with 0 issues; go build ./... and
go test ./pkg/util/... pass.

Compatibility

Existing ProviderNetwork resources that already hold a value the daemon
accepts stay valid — the pattern is strictly narrower than "any string" but
strictly wider than what the daemon has ever been able to program. A resource
holding an unparseable name (which never worked) will be rejected on its next
update.

🤖 Generated with Claude Code

The `+kubebuilder:validation:Items={Type=string,Pattern=...}` marker on
ProviderNetwork.Spec.VlanInterfaces is not a marker controller-gen knows,
so it was silently dropped: the generated CRD carried a bare
`items: {type: string}` with no constraint at all. Any string could be
stored in the field, and daemon just logs a warning and skips entries it
cannot parse, so a typo was invisible to the user.

The pattern itself was also wrong. Inside a Go raw string literal `\\.`
is a literal backslash followed by any character, not an escaped dot, so
the expression rejected every legal value such as `eth0.10` — fixing only
the marker name would have turned a no-op into a regression.

Use the `items:` marker prefix that controller-gen supports, with the dot
escaped once, and cap the length at IFNAMSIZ-1 like the other interface
name fields. The pattern matches exactly what the daemon accepts: it
splits the name on the first dot and derives the VLAN ID from the
suffix (pkg/daemon/controller.go, pkg/daemon/ovs_linux.go).

Verified against the regenerated CRD: `eth0.10`, `bond0.4094`,
`enp1s0f0.100`, `eth-0.1` and `eth_0.4094` are accepted; `eth0`,
`vlan100`, `eth0.10.20`, `eth0.99999`, `.10` and `eth0.` are rejected.

Signed-off-by: Mengxin Liu <liumengxinfly@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant