Repository navigation
docker_service: build ferm rules with a filter plugin - #2766
Open
cogito-mens wants to merge 9 commits into
Open
cogito-mens wants to merge 9 commits into
cogito-mens wants to merge 9 commits into
Conversation
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
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
Member
|
@scibi Please check and confirm that the PR works as expected before I merge it. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The
docker_servicerole built itsfermdependent rules with atemplatelookup 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 separateferm_rule_namesfact 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.