Skip to content

Revisit validation#1630

Open
Fredi-raspall wants to merge 12 commits into
mainfrom
pr/fredi/validation_revisited
Open

Revisit validation#1630
Fredi-raspall wants to merge 12 commits into
mainfrom
pr/fredi/validation_revisited

Conversation

@Fredi-raspall

Copy link
Copy Markdown
Contributor

Notes

@Fredi-raspall Fredi-raspall added the ci:+vlab Enable VLAB tests label Jul 10, 2026
@Fredi-raspall
Fredi-raspall requested a review from a team as a code owner July 10, 2026 19:38
@Fredi-raspall
Fredi-raspall requested review from daniel-noland and removed request for a team July 10, 2026 19:38
@coderabbitai

coderabbitai Bot commented Jul 10, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

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: de748286-5033-4649-bc7f-275a6d9e5933

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 AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors the dataplane configuration “validated” model by removing separate Validated* wrapper types and switching validation to in-place enrichment (&mut self -> ConfigResult). It then updates router/mgmt/NAT/flow-filter call sites and tests to work with the new mutation-based validation flow.

Changes:

  • Replace ValidatedGwConfig/ValidatedOverlay/ValidatedVpcTable/ValidatedManifest/ValidatedExpose with runtime types (GwConfig, Overlay, VpcTable, VpcManifest, VpcExpose) that are validated/mutated in place.
  • Update mgmt/router/NAT/flow-filter modules and tests to call validate() via mut values and to use GwConfig::from_validated(ExternalConfig).
  • Adjust config validation/enrichment logic (route table building, expose collapsing, underlay VTEP population) to happen during validate().

Reviewed changes

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

Show a summary per file
File Description
validator/src/main.rs Update validator to handle in-place validation API.
routing/src/router/rio.rs Switch router runtime config storage to GwConfig.
routing/src/router/ctl.rs Update router control channel messages to carry GwConfig.
routing/src/cli/handler.rs Update CLI “show config” to use GwConfig.
nat/src/test.rs Update NAT integration tests to validate Overlay in place.
nat/src/static_nat/test.rs Update static NAT tests to work with GwConfig and in-place peering validation.
nat/src/static_nat/setup/mod.rs Update NAT table builders to accept VpcTable/Peering/VpcExpose (no validated wrappers).
nat/src/portfw/portfwtable/setup.rs Update port-forwarding ruleset builder to use non-validated overlay types.
nat/src/portfw/portfwtable/access.rs Update port-forwarding table writer to accept VpcTable.
nat/src/masquerade/test.rs Update masquerade tests to validate configs/overlays in place and use new accessors.
nat/src/masquerade/apalloc/test_alloc.rs Update allocator tests to use validated-in-place VpcTable.
nat/src/masquerade/apalloc/setup.rs Update allocator setup to use VpcManifest/VpcExpose and non-validated peerings.
nat/src/masquerade/allocator_writer.rs Update masquerade config to accept VpcTable and non-validated peerings/exposes.
mgmt/src/tests/mgmt.rs Update mgmt tests to build GwConfig via GwConfig::from_validated.
mgmt/src/processor/proc.rs Switch mgmt processor pipeline to GwConfig and non-validated overlay types.
mgmt/src/processor/mgmt_client.rs Update mgmt client API to return Arc<GwConfig>.
mgmt/src/processor/k8s_client.rs Minor async call style change.
mgmt/src/processor/gwconfigdb.rs Store applied config/history using GwConfig.
mgmt/src/processor/confbuild/router.rs Update router config generation to accept Arc<GwConfig>.
mgmt/src/processor/confbuild/namegen.rs Update name generation helpers to use Vpc.
mgmt/src/processor/confbuild/internal.rs Update internal config builder to use GwConfig and non-validated overlay types.
flow-filter/src/tests.rs Update flow-filter tests for in-place overlay validation.
flow-filter/src/setup.rs Change flow-filter build API to accept &Overlay (no validated wrapper).
config/src/utils/overlap.rs Update overlap helpers to operate on VpcExpose.
config/src/lib.rs Export GwConfig instead of ValidatedGwConfig.
config/src/gwconfig.rs Introduce GwConfig (wrapping ExternalConfig) and from_validated constructor.
config/src/external/underlay/mod.rs Convert underlay validation to in-place enrichment (populate vtep).
config/src/external/overlay/vpcrouting.rs Update VPC route table logic to use non-validated peerings/exposes and in-place validate.
config/src/external/overlay/vpcpeering.rs Convert expose/manifest validation to in-place collapsing; remove validated expose/manifest types.
config/src/external/overlay/vpc.rs Convert VPC/peering/table validation to in-place enrichment and build route tables during validate.
config/src/external/overlay/validation_tests.rs Update overlay validation tests for new validate semantics.
config/src/external/overlay/tests.rs Update overlay tests for new validate semantics.
config/src/external/overlay/mod.rs Convert overlay validation to in-place (collect peerings + validate vpc table).
config/src/external/mod.rs Convert ExternalConfig::validate to in-place enrichment and adjust helpers.
config/src/display.rs Remove display impls for removed validated types; update overlay/VPC displays accordingly.
Comments suppressed due to low confidence (1)

flow-filter/src/setup.rs:26

confidence: 7
tags: [logic]

`FlowFilterTable::build_from_overlay` relies on peerings having already been collected into each VPC (via `Overlay::validate()`), but the signature no longer enforces that. If a caller passes an unvalidated overlay with a non-empty `peering_table`, this will silently skip peerings and build an incomplete table.

A cheap runtime guard (plus clearer docs) avoids both silent misconfiguration and the overhead of cloning/validating here.
/// Build a [`FlowFilterTable`] from an overlay
pub fn build_from_overlay(overlay: &Overlay) -> Result<Self, ConfigError> {
    let mut table = FlowFilterTable::new();

    for vpc in overlay.vpc_table().values() {
        for peering in vpc.peerings() {
            table.add_peering(overlay, vpc, peering)?;
        }
</details>

Comment on lines +438 to 449
/// FOR TESTS ONLY. Produces an enriched-but-not-collapsed expose (exclusion prefixes are
/// dropped without being applied), bypassing real validation.
#[cfg(feature = "testing")]
#[must_use]
#[allow(unsafe_code)]
unsafe fn fake_validated_expose(&self) -> ValidatedExpose {
ValidatedExpose {
unsafe fn fake_validated_expose(&self) -> VpcExpose {
VpcExpose {
default: self.default,
ips: self.ips.clone(),
nots: PrefixPortsSet::new(),
nat: self.nat.clone(),
}
Comment on lines +103 to 108
/// Validate and enrich the external configuration in place (validating the underlay and
/// overlay and collecting peerings into each VPC).
///
/// To obtain the runtime [`GwConfig`], validate then wrap: `cfg.validate()?; GwConfig::new(cfg)`.
///
/// # Errors
Comment thread config/src/gwconfig.rs Outdated
Comment on lines +80 to +91
/// Validate an [`ExternalConfig`] and wrap it into a runtime [`GwConfig`].
///
/// This is the only way to obtain a non-blank [`GwConfig`], which guarantees its
/// [`ExternalConfig`] has been validated.
///
/// # Errors
///
/// Returns a [`ConfigError`] if validation fails.
pub fn from_validated(mut external: ExternalConfig) -> Result<Self, ConfigError> {
external.validate()?;
Ok(Self::new(external))
}
@qmonnet
qmonnet self-requested a review July 10, 2026 19:55
@qmonnet qmonnet added the dont-merge Do not merge this Pull Request label Jul 10, 2026
@qmonnet

qmonnet commented Jul 10, 2026

Copy link
Copy Markdown
Member

(I tagged to block the merge because I expect conflicts with #1618.)

The vpcid map is a concept specific to a vpc table.
Make it private, and construct it from a vpc table instead of an
overlay.

Signed-off-by: Fredi Raspall <fredi@githedgehog.com>
Extend the VpcId map with Vnis and let peerings include the Vni of
the remote VPC.

Signed-off-by: Fredi Raspall <fredi@githedgehog.com>
The builders will be used to represent default peering exposes.

Signed-off-by: Fredi Raspall <fredi@githedgehog.com>
The vpc routing module defines a vpc routing table which is an
an alternate representation of the peerings of a given VPC. This
representation is easier to validate and closer to what some NFs
require to build their internal state.

The validation of the vpc routing table includes now the constraint
that remote destinations of a VPC that are allowed to overlap must
be mapped to the same gateway group for failover to work correctly.
This restriction is not one imposed by a single gateway but a
fabric-wide one.

Signed-off-by: Fredi Raspall <fredi@githedgehog.com>
Build a vpc routing table from the set of validated peerings of a
VPC and validate it, replacing the previous validation function.

Signed-off-by: Fredi Raspall <fredi@githedgehog.com>
Signed-off-by: Fredi Raspall <fredi@githedgehog.com>
Signed-off-by: Fredi Raspall <fredi@githedgehog.com>
Remove unnecessary derived impls as these impose unnecessary
requirements on embedded types.

Signed-off-by: Fredi Raspall <fredi@githedgehog.com>
Store the vpc routing table in a ValidatedVpc for future use.

Signed-off-by: Fredi Raspall <fredi@githedgehog.com>
Signed-off-by: Fredi Raspall <fredi@githedgehog.com>
Signed-off-by: Fredi Raspall <fredi@githedgehog.com>
@Fredi-raspall
Fredi-raspall force-pushed the pr/fredi/config_constraints branch from 6009f0f to 5818ca6 Compare July 11, 2026 10:47
@Fredi-raspall
Fredi-raspall force-pushed the pr/fredi/validation_revisited branch from d3bb30b to 0af2025 Compare July 11, 2026 12:59
Revert back to non-validated types

Signed-off-by: Fredi Raspall <fredi@githedgehog.com>
@Fredi-raspall
Fredi-raspall force-pushed the pr/fredi/validation_revisited branch from 0af2025 to 5c59bf3 Compare July 11, 2026 13:08
@Fredi-raspall
Fredi-raspall force-pushed the pr/fredi/config_constraints branch from 5818ca6 to e8460cc Compare July 14, 2026 17:33
Base automatically changed from pr/fredi/config_constraints to main July 15, 2026 10:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci:+vlab Enable VLAB tests dont-merge Do not merge this Pull Request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants