fix: validate overlap with BCM reserved VLANs

[edipascale]

starting from 4.6.0 any overlap with the reserved range returns a hard error, so let's make sure we stay clear of it.

This will fail CI until githedgehog/fabricator#1862 gets merged

related to https://github.com/githedgehog/internal/issues/435

[coderabbitai]

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 48dbd7c9-10ec-4d0f-b33c-83d306daf532

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

_{Comment @coderabbitai help to get the list of available commands.}

[Copilot]

Pull request overview

Adds validation to prevent user-allocated VLAN ranges from overlapping Broadcom (BCM) reserved VLAN space, aligning with stricter behavior introduced in fabricator 4.6.0+.

Changes:

• Introduces a Broadcom/BCM reserved VLAN range constant and validates FabricConfig VLAN ranges against it during FabricConfig.Init().
• Extends VLANNamespace.Validate() to reject namespace ranges that overlap the Broadcom/BCM reserved VLAN range.
• Updates VLANNamespace validation tests to include a Broadcom/BCM reserved overlap scenario.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File	Description
api/wiring/v1beta1/vlannamespace_types.go	Adds overlap validation against Broadcom/BCM reserved VLAN ranges during VLANNamespace admission validation.
api/wiring/v1beta1/vlannamespace_types_test.go	Updates/extends validation tests for collisions, including a new Broadcom/BCM reserved collision case.
api/meta/types.go	Defines Broadcom/BCM reserved VLAN range(s) and enforces non-overlap in FabricConfig.Init() for reserved VLAN allocations.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[github-actions]

🚀 Temp artifacts published: v0-e3f64b14f 🚀

[github-actions]

🚀 Temp artifacts published: v0-6667ec11b 🚀

[github-actions]

🚀 Temp artifacts published: v0-e46f42731 🚀

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

api/wiring/v1beta1/vlannamespace_types.go:133

• CheckVLANRangesOverlap(append(BCMReservedVLANRanges, ns.Spec.Ranges...)) can return an error for overlaps within ns.Spec.Ranges (or later fabricCfg overlap checks) and then wrap it as "overlap with Broadcom reserved VLANs", which is misleading and can reject namespaces that don't touch the Broadcom reserved range. Consider validating namespace ranges don't overlap among themselves first (on a cloned slice), and then do the reserved-range overlap checks using the already-validated ranges.

	if err := meta.CheckVLANRangesOverlap(append(slices.Clone(meta.BCMReservedVLANRanges), ns.Spec.Ranges...)); err != nil {
		return nil, errors.Wrapf(err, "ranges overlap with Broadcom reserved VLANs")
	}

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (3)

api/wiring/v1beta1/vlannamespace_types.go:133

• The error message here uses "Broadcom reserved VLANs" while FabricConfig validation uses "Broadcom reserved VLAN range". Aligning the wording makes errors easier to search and keeps messages consistent.

	if err := meta.CheckVLANRangesOverlap(append(slices.Clone(meta.BCMReservedVLANRanges), ns.Spec.Ranges...)); err != nil {
		return nil, errors.Wrapf(err, "ranges overlap with Broadcom reserved VLANs")
	}

api/meta/types.go:77

• BCMReservedVLANRanges is an exported slice, so any external package can mutate it (e.g., meta.BCMReservedVLANRanges[0].From = ...), which would make validation behavior non-deterministic. Since this is a new API surface, consider keeping the backing data unexported and exposing it via a function that returns a cloned slice (or otherwise ensuring callers cannot mutate shared state).

var BCMReservedVLANRanges = []VLANRange{{From: 3967, To: 4094}}

api/meta/types.go:267

• New validation was added to FabricConfig.Init() to reject overlaps with Broadcom reserved VLANs, but there are no unit tests exercising these new failure modes (e.g., VPC IRB / peering / TH5 ranges overlapping 3967-4094). Adding tests would help prevent regressions and clarify the expected boundaries.

		if err := CheckVLANRangesOverlap(append(slices.Clone(r), BCMReservedVLANRanges...)); err != nil {
			return nil, errors.Wrapf(err, "config: vpcIRBVLANRange overlaps with Broadcom reserved VLAN range")
		}

[github-actions]

🚀 Temp artifacts published: v0-e297569e9 🚀