Skip to content

feat!: migrate firewall to custom resources - #324

Open
damacus wants to merge 6 commits into
mainfrom
feat/migrate-firewall-custom-resources
Open

feat!: migrate firewall to custom resources#324
damacus wants to merge 6 commits into
mainfrom
feat/migrate-firewall-custom-resources

Conversation

@damacus

@damacus damacus commented May 20, 2026

Copy link
Copy Markdown
Member

Summary

  • Selected cookbook: firewall
  • Scope: Full Migration to modern Chef custom resources
  • Adds firewall and unified firewall_rule custom resources for UFW, iptables, firewalld, and Windows backends
  • Moves legacy recipe and attribute behaviour into resource properties and test cookbook examples
  • Removes the root legacy recipes/ and attributes/ public API
  • Moves kitchen test cookbooks under test/cookbooks, adds InSpec profile metadata, and updates the platform matrix to currently supportable platforms

Breaking changes

  • Root recipes have been removed. Consumers should declare firewall 'default' directly.
  • Root attributes have been removed. Consumers should configure the firewall resource properties directly.
  • Legacy library providers/resources and split firewalld wrapper resources have been removed in favour of custom resources in resources/.

Platform and support evidence

  • Preserved non-EOL Linux platform coverage where support remains practical.
  • Dropped EOL platforms from active kitchen/CI coverage: Debian 11, Ubuntu 20.04, Amazon Linux 2, and openSUSE Leap 15.
  • Added current platform coverage where relevant: AlmaLinux 10, CentOS Stream 10, Debian 13, Rocky Linux 10, and Oracle Linux naming aligned to oraclelinux-*.
  • Windows remains documented/supported through the resource backend and InSpec profile, but the prior CI workflow already kept Windows kitchen suites disabled.

@damacus
damacus requested a review from a team as a code owner May 20, 2026 07:16
@damacus
damacus force-pushed the feat/migrate-firewall-custom-resources branch from c2f3788 to b24e527 Compare May 20, 2026 07:55
@a7b81a9086

Copy link
Copy Markdown
Contributor

I just skimmed over it and three questions popped up:

  1. Why is ufw selected as default firewall for debian? Shouldn't that be nftables, as is stated here?
  2. Why is nftables not selectable as solution from the firewall-resource? The interface should be the same as for the other firewalls.
  3. Maybe deprecate iptables? Debian only offers iptables as frontend for nftables, I did not check what the other distributions do.

@damacus
damacus force-pushed the feat/migrate-firewall-custom-resources branch from b24e527 to 6dd613c Compare May 20, 2026 13:24
@damacus

damacus commented May 20, 2026

Copy link
Copy Markdown
Member Author

I just skimmed over it and three questions popped up:

  1. Why is ufw selected as default firewall for debian? Shouldn't that be nftables, as is stated here?
  2. Why is nftables not selectable as solution from the firewall-resource? The interface should be the same as for the other firewalls.
  3. Maybe deprecate iptables? Debian only offers iptables as frontend for nftables, I did not check what the other distributions do.

thanks for the feedback.
Much appreceated!

@damacus
damacus force-pushed the feat/migrate-firewall-custom-resources branch 5 times, most recently from 97cf023 to 0d5df99 Compare May 20, 2026 14:10
@damacus
damacus force-pushed the feat/migrate-firewall-custom-resources branch from 0d5df99 to ee94bf9 Compare May 20, 2026 14:19
@damacus

damacus commented May 20, 2026

Copy link
Copy Markdown
Member Author

@a7b81a9086 does that make more sense now?

@damacus damacus self-assigned this May 20, 2026
@a7b81a9086

Copy link
Copy Markdown
Contributor

@damacus I have been thinking about and I think we need a sort of design decision first. I remember that when I was starting to work on the nftables_rule resource, I was told that a firewall_rule that is able to configure rules for every firewall might not be the best idea, and I can still see why:
nftables and iptables are mostly compatible, just the syntax is different. But ufw and firewalld provide other abstraction layers, and mapping all of them to the same firewall_rule resource will probably turn out to be really confusing. However, if we keep the nftables_rule-, iptables_rule-, ufw_rule-, .., -resources, we do not need all the chosing-logic anymore. I also do not see the advantage of unifying everything in one resource for users of the cookbook. I doubt that anyone will truly benefit from changing firewalls on the fly.

Onother note: I assume that the MR was generated with the help of AI. May I suggest, that you split the MR into smaller chunks? That would make it way easier to follow what is going on. One commit for making each of the firewalls resource based, one for updating the test suites, one for the updating all the metadata of the repo. If this is done, please try to avoid force pushing over your previous commits, because it is really hard for me to see what has changed.

Last remark: I am currently using a fork of just the nftables-part of this cookbook, so I do not actually use this cookbook (but I would like to in the future). So I am not sure how much I weight my opinion should have here, maybe someone else wants to chime in?

@damacus

damacus commented Jun 4, 2026

Copy link
Copy Markdown
Member Author

Thanks for the feedback there @a7b81a9086

I think I'm going to merge as is.
But leave that with me.
Let me brainstorm better UX with this

I've decided to move to a router model, where we pick the correct backend based on platform. The user can still pick their backend or just use the backend directly.

On another note: I assume that the MR was generated with the help of AI.

Yes indeed, it was, it's literally the only way I can keep up with my OSS work!

damacus added 2 commits June 4, 2026 11:40
Rename the firewall selector from solution to backend and make firewall_rule delegate to backend-specific rule resources.

Keep nftables as the Debian default while preserving explicit iptables, ufw, firewalld, and windows backends.
Signed-off-by: Dan Webb <dan.webb@damacus.io>
@damacus
damacus force-pushed the feat/migrate-firewall-custom-resources branch from 500791f to 907059a Compare June 4, 2026 11:21
@github-actions

github-actions Bot commented Jun 4, 2026

Copy link
Copy Markdown

Slowest examples

Top 10 slowest examples (0.78 seconds, 46.9% of total time)
Example Description Time in seconds
spec/unit/resources/firewalld_rule_spec.rb:40 firewalld_rule with a redirect rule is expected to add firewalld_rich_rule "redirect" 0.12749
spec/unit/resources/firewall_rule_spec.rb:72 firewall_rule with nftables-specific rule options through the firewall_rule facade is expected to create nftables_rule "logged ssh" 0.12219
spec/unit/resources/firewall_rule_spec.rb:21 firewall_rule with a UFW backend is expected to create ufw_rule "ssh" 0.09763
spec/unit/resources/firewalld_rule_spec.rb:28 firewalld_rule with an array port rule is expected to add firewalld_rich_rule "ssh-alt [2200/tcp]" 0.07436
spec/unit/resources/firewall_spec.rb:66 firewall on Ubuntu with iptables is expected to install package "iptables-persistent" 0.06597
spec/unit/resources/firewall_spec.rb:21 firewall on Ubuntu with UFW is expected to enable service "ufw" 0.06179
spec/unit/resources/firewall_spec.rb:79 firewall on AlmaLinux with firewalld is expected to install firewalld "default" 0.05992
spec/unit/resources/firewalld_rule_spec.rb:16 firewalld_rule with a simple port rule is expected to create firewalld_rule "ssh" 0.0594
spec/unit/resources/firewall_spec.rb:19 firewall on Ubuntu with UFW is expected to install package "ufw" 0.05873
spec/unit/resources/firewalld_rule_spec.rb:27 firewalld_rule with an array port rule is expected to add firewalld_rich_rule "ssh-alt [2222/tcp]" 0.05709

@damacus

damacus commented Jun 4, 2026

Copy link
Copy Markdown
Member Author

@a7b81a9086 what do you think about this shape?

@a7b81a9086

Copy link
Copy Markdown
Contributor

Hi Dan,
looks good to me. Did you consider using using apply_common_rule_properties when calling create_nftables_rule in firewall_rule.rb? Seems like a lot of duplication is going on there.

Same (but different) for the firewall-Resource, which has allow_mosh etc. as configurable default-rules, the nftables-Resource supports it, but it is not propagated from the firewall-resource to the nftables-resource.

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