Generate the communication matrix in nftables format #14
Conversation
|
I think for a user, generating the matrix in nftables format and applying them should be two option in the Makefile and not done by default |
|
LGTM from a nftables syntax point of view. |
aabughosh
changed the title
types/types.go
Outdated
|
|
||
| chain INPUT { | ||
| type filter hook input priority 0; policy accept; | ||
| jump FIREWALL |
There was a problem hiding this comment.
Why not just insert all the rules in the OPENSHIFT chain here and skip the chain jump?
There was a problem hiding this comment.
because the input chain might already exist and have other stuff in it
or at least that was my initial thought when I saw this - that we have everything openshift need in it's own chain, and that can be used anywhere the user wants..
but then we might need a different way to modify the input chain?
There was a problem hiding this comment.
I know. The thing is another entity manages the whole table so we should not touch it. There is even a warning comment in it. That is another advantage of nftables compared to iptables: you can have multiple tables. Also important is if two tables have chains on the same Netfilter hook, it is paramount to have them with different priorities or otherwise there is no guarantee of evaluation order:
It's possible to give two base chains the same priority, but there is no guaranteed evaluation order of base chains with identical priority that are attached to the same hook location
https://wiki.nftables.org/wiki-nftables/index.php/Configuring_chains
In the latest PR change, Amal addressed my comment about the priority and table separation where the communication matrix rules have lower priority than other ones. If it had higher or same but nftables would evaluate those rules first, traffic could be dropped (there is a drop-all rule with log at the end of the proposed chain).
There was a problem hiding this comment.
Yuval, think of firewalld. It creates its own table so that it doesn't conflict with anything else on the system.
|
Does this tool also support IPv6? |
Yes but we actually wanted to split the communication matrix into two files, one for ipv4 ports and another for ipv6 ports. We found out there are no IPv6 endpointslices all are IPv4 due to the following bug https://issues.redhat.com/browse/OCPBUGS-35025 |
|
I had a chat with @aabughosh offline. She has now consolidated the nftables output in a single chain and in its own table. Additionally, she also added support for IPv6 by changing the table family to inet and adding icmpv6. I haven't tested with these latest changes but I like the way it is now. |
|
|
||
| err := writeMatrixToFile(matrix, fileName, format, printFn, destDir) | ||
| assert.NoError(t, err) | ||
| assert.FileExists(t, filepath.Join(destDir, "test-matrix.json")) |
There was a problem hiding this comment.
this doesn't do much, wdyt about expanding the test to a table (with all formats) and actually checking the content of the files? you can probably use what we did on metallb with the .golden files.
remove unnecessary newline arrange the main function and add option on make file for apply firewall update folders name add nft table format for the output and generate it by defualt
Update generate_matrix.go Specify static entries namespaces This commit updates the static-custom-entries.go. fill the namespaces of some static custom entries, and renames the variable name of cloudStaticEntries to be cloud generic and not specific for AWS. Signed-off-by: Lior Noy <[email protected]> Fix static entries typos Fix the pod name for comDetails Modifies the function for creating comDetails, so the pod name will be more accurate. We fetch the name from the OwnerReference, and modify the name if needed. Added unit tests for the function. Signed-off-by: Lior Noy <[email protected]> Fix extractPodName func Update README.md delete the rile from functions and remove the firewallapply Delete apply_firewall.go remove applyflag move generate file path change names and remove unused function update GenerateCommatrix ans mpve the sperate function to generate file update the name of the function with sma,ll litter Update generate.go Update types.go
sperate generate and wrie functions Update main.go Update main.go Update generate.go Update types.go resolved comments from sabina Update commatrix_test.go change matrix variable name arragment to the code-add generate ss raws and ss matrix then write every matrix to a file fix linter another update and remove for the unused function-resolve sabinas comments remove the test function change functions and names update nft table file
remove createSSOutputFiles function and add new lines
in this PR there is few things
main thing is 1.adding option of FORMAT=nft to create the communication matrix in output file as .nft file
user can do nft apply -f to that specific file
2.few arrangements to the code separate into functions and files