Skip to content

docker_service: build ferm rules with a filter plugin - #2766

Open
cogito-mens wants to merge 9 commits into
debops:masterfrom
cogito-mens:docker-service-ferm-filter
Open

cogito-mens wants to merge 9 commits into
debops:masterfrom
cogito-mens:docker-service-ferm-filter

Conversation

@cogito-mens

Copy link
Copy Markdown
Contributor

The docker_service role built its ferm dependent rules with a template lookup that rendered a Jinja2 template and re-parsed the result as YAML, cleaning up rules for containers that no longer exist by way of a separate ferm_rule_names fact that the role had to publish. This replaces the lookup with a controller-side filter plugin that constructs the rule structures directly, and discovers stale rules from the rule files already present in /etc/ferm/rules.d/, which removes the need for the extra fact. Because the generation is now ordinary Python, it can also fail loudly: duplicate rule names and unrecognised default-deny action values raise an error instead of quietly emitting a broken rule.

Rendering the published port rules in Jinja reimplemented name
composition and dict merging in template syntax. A filter plugin runs
controller-side and returns a list directly, so the from_yaml round
trip goes away too.

Setting reject_with on the default-deny rule was an oversight: ferm
fell back to icmp-admin-prohibited for every protocol, including TCP.
A rejected TCP connection now gets a tcp-reset, which clients can act
on, while UDP keeps icmp-port-unreachable.

Stale rules are detected by globbing the role's files under
/etc/ferm/rules.d rather than reading back the names written by the
previous run. The on-disk state is authoritative and does not depend
on a fact cache, which may be absent or stale.

Generated-By: LLM
Open and close a devnull file descriptor by hand where the standard
library already provides one. Verified the script still collects
containers and still swallows daemon errors into
containers_query_error.

Generated-By: LLM
The scope playbook imported docker_service without deriving the ferm rules
the role computes, so published_ports allow lists had nothing behind them
when the scope playbook was used.

The ferm role stays out of scope here: applying it would reconcile the
whole host firewall, which belongs to the service playbook. Deriving the
variables keeps them available without widening what a scope run touches.

Generated-By: LLM
The variable is rebuilt by build_ferm_vars on every run, so assigning
it in inventory is silently discarded. Saying so avoids the guesswork.

Generated-By: LLM
The task looped over the service definitions a second time and then
rebuilt a list of all non-directory paths for every single volume, so
the cost grew with the square of the number of volumes. It also
repeated the state, create_volume_dirs and absolute path conditions
that the stat task had already applied.

Looping over the stat results and reading item.stat directly removes
the quadratic list rebuild and the duplicated conditions, while
keeping the config_files, data_dirs and existing non-directory
exclusions unchanged.

Generated-By: LLM
The docs claimed reject sends a TCP reset or an ICMP admin-prohibited
reply, which read as a per-protocol choice. The reply type is chosen
from the protocol, so state the mapping instead and explain why TCP
should not be answered with ICMP.

Generated-By: LLM
The action mapping treated any value other than 'reject' as 'drop', so
a typo such as 'rejectt' closed the published port silently and the
symptom looked like a firewall problem rather than a bad inventory
value. Map the supported actions explicitly and report the rest.

Also correct the action_default documentation, which repeated the
stale claim that reject sends a TCP reset or an ICMP reply; the reply
type follows the protocol.

Generated-By: LLM
The docs described only the service playbook, so there was no record that a
scope playbook exists. Add the entry point section used by the other roles
and list the scope playbook in the man page synopsis.

Generated-By: LLM
@drybjed

drybjed commented Sep 30, 2026

Copy link
Copy Markdown
Member

@scibi Please check and confirm that the PR works as expected before I merge it.

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