Skip to content

Test TCPMSS Configuration#1998

Merged
aauren merged 2 commits intomasterfrom
test_TCPMSS_configuration
Feb 1, 2026
Merged

Test TCPMSS Configuration#1998
aauren merged 2 commits intomasterfrom
test_TCPMSS_configuration

Conversation

@aauren
Copy link
Copy Markdown
Collaborator

@aauren aauren commented Jan 25, 2026

This adds testing for TCPMSS configurations and ensures backwards compatibility and that old rules are removed when upgrading kube-router versions.

This also fixes a minor bug introduced in #1996 when @rkojedzinszky fixed up IPv6 MSS values for us, by cleaning up any old incorrect IPv6 mangle values that might still be around if someone did an in-place upgrade of kube-router.

@aauren aauren requested a review from catherinetcai January 25, 2026 22:44
@aauren
Copy link
Copy Markdown
Collaborator Author

aauren commented Jan 25, 2026

This PR stacks with #1997

}

// Helper function to find all MSS values in Exists calls
func findMSSInExistsCalls(calls []struct {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Are findMSSInExistsCalls and findMSSInAppendCalls meant to be identical? If yes, should they be consolidated into one helper func instead?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Good catch! I'll update to a single call!


err := nsc.setupMangleTableRule("10.0.0.1", "tcp", "8080", "1234")

assert.Error(t, err, "Expected error when FWMARK OUTPUT AppendUnique fails")
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nit (feel free to ignore) - I think https://pkg.go.dev/github.com/stretchr/testify@v1.11.1/assert#ErrorContains does both of these assertions in 1

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

No, this is also good feedback, I'll make the change! 🙂

// setupMangleTableRule Error Handling Tests
// =============================================================================

func TestSetupMangleTableRule_ErrorHandling_FWMarkPreRoutingFails(t *testing.T) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think all of the setupMangleTableRule tests all have a very similar structure that loans itself well to table driven tests. It might help with making these easier to read if they're grouped that way.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Absolutely agree. I'll fix em! Thanks for the recommendation!

Base automatically changed from fix/convert_ginkgo_tests to master January 31, 2026 18:15
@aauren aauren force-pushed the test_TCPMSS_configuration branch from ac2f7f3 to 667b7c3 Compare January 31, 2026 18:52
@aauren
Copy link
Copy Markdown
Collaborator Author

aauren commented Jan 31, 2026

@catherinetcai this should be ready for you to take another look now.

@aauren aauren merged commit 0486807 into master Feb 1, 2026
7 checks passed
@aauren aauren deleted the test_TCPMSS_configuration branch February 1, 2026 16:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants