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

Add comments for all the nftable::rules entries #13

Merged
merged 2 commits into from
Nov 19, 2020

Conversation

traylenator
Copy link
Collaborator

For each nftable::rule this adds an extra concat fragment to
add a comment containing the name and order number for the rule.

The motivation here is to make the mapping from resulting rules back
to puppet code more obvious. When adding a new rule it should be more
obvious to understand what order to choose.

An example resulting table ends up reading as:

# Start of fragment order:00 default_in header
chain default_in {
#   Start of fragment order:50 rulename:default_in-dhcpv6_client
  ip6 saddr fe80::/10 ip6 daddr fe80::/10 udp sport 547 udp dport 546 accept
#   Start of fragment order:50 rulename:default_in-ssh
  tcp dport {22} accept
#   Start of fragment order:90 rulename:default_in-drop_broadcasts
  meta pkttype broadcast counter drop
# Start of fragment order:99 default_in footer
}

In addition there is a new test nftables::rule. This includes
a pending test since I would assume setting source and content
on a rule should be an error however this currently not the case.

@traylenator
Copy link
Collaborator Author

traylenator commented Nov 19, 2020

To add I did consider using the comment 'rule name order' within the rules but it is less obvious what to do with multline contents or source => specifying rules.

Happy to switch to prefixing rules with comments rather than extra fragments but the input for content might have to be stricter.

content => "\nruleA\nruleB\n"

should probably be blocked as should

content => "comment 'my comment" myrule"

since the double comment causes problems.

manifests/rule.pp Outdated Show resolved Hide resolved
For each nftable::rule this adds an extra concat fragment to
add a comment containing the name and order number for the rule.

The motivation here is to make the mapping from resulting rules back
to puppet code more obvious. When adding a new rule it should be more
obvious to understand what order to choose.

An example resulting table ends up reading as:

```
HASH Start of fragment order:00 default_in header
chain default_in {
HASH   Start of fragment order:50 rulename:default_in-dhcpv6_client
  ip6 saddr fe80::/10 ip6 daddr fe80::/10 udp sport 547 udp dport 546 accept
HASH   Start of fragment order:50 rulename:default_in-ssh
  tcp dport {22} accept
HASH   Start of fragment order:90 rulename:default_in-drop_broadcasts
  meta pkttype broadcast counter drop
HASH Start of fragment order:99 default_in footer
}

```

In addition there is a new test `nftables::rule`. This includes
a pending test since I would assume setting source and content
on a rule should be an error however this currently not the case.
@nbarrientos
Copy link
Collaborator

Happy to switch to prefixing rules with comments rather than extra fragments but the input for content might have to be stricter.

An advantage of using built-in comments is that they show up when executing nft list ruleset which might be handy.

@duritong duritong merged commit a5f5fb1 into voxpupuli:master Nov 19, 2020
traylenator referenced this pull request in traylenator/puppet-nftables Nov 20, 2020
@traylenator traylenator mentioned this pull request Nov 20, 2020
@traylenator traylenator added the enhancement New feature or request label Dec 10, 2020
@traylenator traylenator deleted the comment branch December 15, 2020 10:28
figless pushed a commit to figless/puppet-nftables that referenced this pull request Aug 25, 2021
a5f5fb1 Merge pull request voxpupuli#13 from traylenator/comment
21d0496 Merge pull request voxpupuli#14 from cernops/ct_away
7b14f6d Merge pull request voxpupuli#6 from traylenator/afs
ea96d5d Move ct rules from global to INPUT and OUTPUT
61f03b4 Switch $order$fragmenta/b to $order-$fragment-a/b
e53053c Add comments for all the nftable::rules entries
9785cd5 lint fix
215aee1 Add kerberos out and openafs_client out
f3f2870 Add rules for afs3_callback

git-subtree-dir: code
git-subtree-split: a5f5fb1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants