Skip to content

NAT: Move tests back to "nat" crate #645

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jun 27, 2025
Merged

Conversation

qmonnet
Copy link
Member

@qmonnet qmonnet commented Jun 26, 2025

  • feat(nat): Implement PartialEq, Eq for NatTables (and related types)
  • test(nat): Add stateless NAT test in "stateless" module
  • test(mgmt): Remove redundant tests for stateless NAT

Please take a look at individual commit descriptions for details.

qmonnet added 3 commits June 26, 2025 23:29
Implement PartialEq and Eq traits for struct NatTables, so we can
compare different objects in tests in a future commit.

We need to dreive the traits for structs PerVniTable,
NatPrefixRuleTable, NatPeerRuleTable, and TrieValue. We also need the
trait to be implemented for PrefixTrie, but we cannot derive it (the
RTrieMap objects do not implement the trait), so we implement it
manually by comparing all entries.

Signed-off-by: Quentin Monnet <[email protected]>
We used to have a similar test in the module, but we recently moved it
to the "mgmt" crate because it depended on some objects from "mgmt", but
the "nat" crate can no longer use "mgmt" as a dependency.

Restore the test in the stateless NAT submodule, but instead of building
the NAT tables from the configuration code from the "mgmt" crate, just
build the table manually.

In a future commit, we'll remove the copy of the test from the "mgmt"
crate and will instead check that the generated config object matches
with the manually-initialised NatTables object from this commit.

Signed-off-by: Quentin Monnet <[email protected]>
In a recent commit we copied the tests for stateless NAT from the "mgmt"
crate back to the "nat" crate. We no longer need the version from the
"mgmt" crate. Instead, ensure that the NatTables object generated from
the config matches the reference object that we manually instantiated in
the tests of the "nat" crate. In order to share and compare this object,
gate it behind a "testing" feature for the "nat" crate.

Signed-off-by: Quentin Monnet <[email protected]>
@qmonnet qmonnet added this to the GW R1 milestone Jun 26, 2025
@qmonnet qmonnet requested a review from Fredi-raspall June 26, 2025 22:50
@qmonnet qmonnet self-assigned this Jun 26, 2025
@qmonnet qmonnet added the area/nat Related to Network Address Translation (NAT) label Jun 26, 2025
@Fredi-raspall Fredi-raspall marked this pull request as ready for review June 27, 2025 07:30
@Fredi-raspall Fredi-raspall requested a review from a team as a code owner June 27, 2025 07:30
Copy link
Contributor

@Fredi-raspall Fredi-raspall left a comment

Choose a reason for hiding this comment

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

Looks good to me @qmonnet , thanks!

@Fredi-raspall Fredi-raspall added this pull request to the merge queue Jun 27, 2025
@Fredi-raspall Fredi-raspall removed this pull request from the merge queue due to a manual request Jun 27, 2025
Copy link
Contributor

@Fredi-raspall Fredi-raspall left a comment

Choose a reason for hiding this comment

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

Can we remove the PartialEq?

@Fredi-raspall Fredi-raspall self-requested a review June 27, 2025 07:52
@Fredi-raspall Fredi-raspall added this pull request to the merge queue Jun 27, 2025
Merged via the queue into main with commit 05bca3d Jun 27, 2025
17 checks passed
@Fredi-raspall Fredi-raspall deleted the pr/qmonnet/mv-nat-test branch June 27, 2025 07:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/nat Related to Network Address Translation (NAT)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants