Skip to content

Conversation

@mvachhar
Copy link
Contributor

@mvachhar mvachhar commented Dec 7, 2025

Next up is the Device section and then code to actually communicate with k8s.

This PR just adds the conversion for the Overlay external config entity.

@mvachhar mvachhar added this to the GW R2 milestone Dec 7, 2025
@mvachhar mvachhar self-assigned this Dec 7, 2025
@mvachhar mvachhar requested a review from a team as a code owner December 7, 2025 22:29
@mvachhar mvachhar requested review from sergeymatov and removed request for a team December 7, 2025 22:29
@mvachhar mvachhar added ci:-upgrade Disable VLAB upgrade tests ci:-vlab Disable VLAB tests labels Dec 7, 2025
@mvachhar mvachhar force-pushed the pr/mvachhar/k8s-direct branch from 11b877a to 5b2fe37 Compare December 7, 2025 22:31
@mvachhar mvachhar requested review from Fredi-raspall, Copilot, daniel-noland and qmonnet and removed request for sergeymatov December 7, 2025 22:31
Copy link
Contributor

Copilot AI left a comment

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 adds conversion logic to transform Kubernetes Custom Resource Definitions (CRDs) into internal overlay configuration entities. It introduces conversion implementations for VPCs, VPC peerings, and expose configurations, along with supporting test infrastructure using the bolero fuzzing library.

Key Changes:

  • Implemented conversion logic from GatewayAgentSpec (K8s CRD) to internal Overlay configuration
  • Added FromStr implementation for Prefix type to support string parsing
  • Created comprehensive fuzzing test generators for VPCs, peerings, and expose configurations

Reviewed changes

Copilot reviewed 16 out of 18 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
lpm/src/prefix/mod.rs Added FromStr implementation for Prefix to enable string parsing
k8s-intf/src/bolero/*.rs Added fuzzing test generators for VPC, peering, expose, and spec configurations
k8s-intf/build.rs Extended type fixups for CRD code generation (VNI, idle_timeout)
k8s-intf/Cargo.toml Added dependencies for lpm and serde-duration-ext
config/src/converters/k8s/*.rs Implemented K8s CRD to internal config conversions for overlay entities
Cargo.toml Added serde-duration-ext workspace dependency

@mvachhar mvachhar force-pushed the pr/mvachhar/k8s-direct branch from 5b2fe37 to b9c8285 Compare December 7, 2025 22:36
@mvachhar
Copy link
Contributor Author

mvachhar commented Dec 7, 2025

This is failing due to the pem file issue that will be resolved by #1117.

@mvachhar mvachhar force-pushed the pr/mvachhar/k8s-direct branch from b9c8285 to 4a8a0be Compare December 7, 2025 23:51
@mvachhar
Copy link
Contributor Author

mvachhar commented Dec 7, 2025

This is failing due to the pem file issue that will be resolved by #1117.

Ok, this is now rebased onto main that contains #1117

Move from using if c.is_empty() { None } else { Some(c) }
which falls across multiple lines and results in uglier
construction of objects.

This commit uses Some(c).filter(|c| !c.is_empty()) instead
as this is a one liner and formats nicely, even though it
may be a bit slower because Some(c) is constructed.

In the case of this commit, Some(c.normalize()) is called
which is even more inefficient, but for the purposes of the
modified test code, the better formatting is preferable to
the slightly enhanced performance.

Signed-off-by: Manish Vachharajani <[email protected]>
As with prior commits, openapi v3 does not have notation
for unsigned types, just a format hint with a generic
integer.  As a result kopium converts go uint32 via CRD
to i32.  This commit fixes up the type for vni.

Signed-off-by: Manish Vachharajani <[email protected]>
Use std::time::Duration for idle_timeout since the
default kopium type of String isn't very helpful
semantically.

Signed-off-by: Manish Vachharajani <[email protected]>
This allows things like "123.1.0.0/16".parse::<Prefix>() to work
as expected.

Signed-off-by: Manish Vachharajani <[email protected]>
Adds ValueGenerators for legal values of the peering types.

Also adds SubnetMap to clean up typing.

Signed-off-by: Manish Vachharajani <[email protected]>
Also adds SubnetMap type for cleanup.

Signed-off-by: Manish Vachharajani <[email protected]>
Copy link
Collaborator

@daniel-noland daniel-noland left a comment

Choose a reason for hiding this comment

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

I don't have any meaningful issue with this PR.

I doodled up some thoughts on trivial improvements while I was reviewing it in #1123 but you can feel free to take or ditch any and all of that. It makes the Some(whatever).filter pattern a little cleaner and trims a few trivial memory allocations, but nothing worth a delay.

@mvachhar mvachhar added this pull request to the merge queue Dec 9, 2025
@mvachhar
Copy link
Contributor Author

mvachhar commented Dec 9, 2025

I don't have any meaningful issue with this PR.

I doodled up some thoughts on trivial improvements while I was reviewing it in #1123 but you can feel free to take or ditch any and all of that. It makes the Some(whatever).filter pattern a little cleaner and trims a few trivial memory allocations, but nothing worth a delay.

I think I am going to leave things as is, but having a NoneIsEmpty trait that is auto implemented does make the Some(...).filter(|v| !v.is_empty()) go away, but then again it is yet another trait you have to know about. I'm torn on it.

Merged via the queue into main with commit c27e708 Dec 9, 2025
21 checks passed
@mvachhar mvachhar deleted the pr/mvachhar/k8s-direct branch December 9, 2025 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci:-upgrade Disable VLAB upgrade tests ci:-vlab Disable VLAB tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants