fix(nat): Fix validation for one-direction NAT #664
Merged
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
This is similar to commit 7344ee0 ("fix(nat): Use ips if as_range are empty for stateless src NAT 1st lookup"): when we only NAT on one direction, then one of the two VPCs uses its "ips" prefix list as public IPs (no NAT), and these are the prefixes we should compare public addresses from as_range in other expose objects with. In particular, we need to do this comparison when validating VpcManifest objects: when checking for prefix overlap, we may have to check the as_range blocks of an expose object with the ips blocks of another, if the latter expose has no as_range. Note that even in that case, we still check for overlap between the ips lists of both expose objects, as we can't afford to have overlap for them (ips lists of exposes in a same manifest). Signed-off-by: Quentin Monnet <[email protected]>
mvachhar
requested changes
Jul 2, 2025
A given VpcExpose may or may not required NAT, depending on whether the as_range list contains any prefix or not. This has consequences in some portions of the NAT processing logics: at times, we need to consider the publicly-exposed addresses for an expose object, and these may not be in as_range, but in ips, if NAT is not in use for this expose. Add some helper methods to handle public IPs in a more convenient way. This makes the logics easier to follow when we need to refer to these public IPs whether they are in as_range or ips. Note that this commit does not account for the case where we have an empty as_range list but non-empty not_as, given that this case is currently rejected at validation time. Suggested-by: Manish Vachharajani <[email protected]> Signed-off-by: Quentin Monnet <[email protected]>
|
@mvachhar I think there is, please take a look at the last commit. |
mvachhar
approved these changes
Jul 2, 2025
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.
This is similar to commit 7344ee0: when we only NAT on one direction, then one of the two VPCs uses its
ipsprefix list as publicips(no NAT), and these are the prefixes we should compare public addresses fromas_rangein other expose objects with.In particular, we need to do this comparison when validating VpcManifest objects: when checking for prefix overlap, we may have to check the
as_rangeblocks of an expose object with theipsblocks of another, if the latter expose has noas_range.Note that even in that case, we still check for overlap between the
ipslists of both expose objects, as we can't afford to have overlap for them (ipslists of exposes in a same manifest).