Skip to content
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

Prevent masquerading pod -> NodeIP traffic #174

Merged
merged 6 commits into from
Oct 7, 2017
Merged

Conversation

bzub
Copy link
Collaborator

@bzub bzub commented Oct 4, 2017

Fixes #172
Fixes #173

@bzub
Copy link
Collaborator Author

bzub commented Oct 5, 2017

Appears to work well, and automatically deletes the problematic masquerading rule upon kube-router upgrade.

Running e2e tests now.

@ryarnyah
Copy link
Contributor

ryarnyah commented Oct 5, 2017

@bzub Why add go-ipset when utils/ipset do the same thing?

@bzub
Copy link
Collaborator Author

bzub commented Oct 5, 2017

@ryarnyah I hit some bugs and I had some trouble understanding the workflow with IPSet/Set/Entry. I created #176 to get your help.

Remove redundant ipset lookups

utils.NewIPSet() does this for us.
Previously with Pod egress enabled, this would get masqueraded.
This change also adds cleanup for said ipset.
- Delete old/bad pod egress iptables rule(s) from old versions
- When pod egress or overlay are disabled, cleanup as needed
- Avoid providing method that would delete all ipset sets on a system
- New method DestroyAllWithin() destroys sets tracked by an IPSet
- Create() now handles cases where Sets/System state are not in sync
- Refresh() now handles leftover -temp set gracefully
- Swap() now uses ipset swap
- Delete() improved sync of Sets and system state
- Get() now validates if map element exists before trying
- etc
@bzub bzub merged commit 342ea5a into master Oct 7, 2017
@bzub bzub deleted the 173-pod-to-node-nonat branch October 7, 2017 09:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants