Add ability to configure NAT type and parameters from K8s #1197
+70
−8
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mvachhar
requested review from
Fredi-raspall and
Copilot
and removed request for
a team
mvachhar
force-pushed
the
pr/mvachhar/k8s-nat-config
branch
from
d27619f to
3e1da5c
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds support for configuring NAT type and parameters directly from Kubernetes manifests, addressing a gap that existed after the migration from gRPC to K8s-based configuration.
Changes:
- Adds K8s NAT configuration parsing with validation for stateful/stateless modes
- Updates stateful NAT idle timeout default from Duration::default() to 2 minutes
- Adds comprehensive unit tests for NAT configuration validation
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| config/src/external/overlay/vpcpeering.rs | Updates idle timeout default value and simplifies NAT configuration assignment |
| config/src/converters/k8s/config/expose.rs | Adds K8s NAT configuration parsing logic and comprehensive test coverage |
mvachhar
force-pushed
the
pr/mvachhar/k8s-nat-config
branch
from
3e1da5c to
127334f
Compare
Fredi-raspall
requested changes
Jan 15, 2026
Contributor
Fredi-raspall
left a comment
Fredi-raspall
left a comment
There was a problem hiding this comment.
Thanks for addressing this. The changes look good to me, except for the couple of things mentioned.
qmonnet
requested changes
Jan 15, 2026
Previously idle_timeout would be set to 0 if not explicitly set when calling VpcExpose::make_stateful_nat. This commit restructures the make_stateful_nat code to use the default implementation of the stateful NAT options which has the correct default duration. Signed-off-by: Manish Vachharajani <[email protected]>
mvachhar
force-pushed
the
pr/mvachhar/k8s-nat-config
branch
from
127334f to
f78522d
Compare
The k8s conversion code was not properly setting the NAT config even when configured in k8s making it impossible to set stateful NAT. This adds the conversion and some unit tests. Signed-off-by: Manish Vachharajani <[email protected]>
mvachhar
force-pushed
the
pr/mvachhar/k8s-nat-config
branch
from
f78522d to
6e58612
Compare
qmonnet
approved these changes
Jan 15, 2026
Member
qmonnet
left a comment
qmonnet
left a comment
There was a problem hiding this comment.
Looks good to me now, thank you!
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
Member
|
Merge queue: failure in pre-populating the hhfab-cache 
Let's give it another try |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In the move from gRPC to K8s direct configuration, the NAT configuration was not properly converted (see #1194). This fixes that problem and adds unit tests for that configuration case.
Fixes #1194