Add firewalld reload service #840
Merged
Conversation
This file contains 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
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Luap99 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Ephemeral COPR build failed. @containers/packit-build please check. |
Luap99
force-pushed
the
firewalld-reload
branch
from
d5ef8cd
to
3e62f6a
Compare
This command listens on dbus for the firewalld reload event. Also include a systemd unit that starts this process with firewalld (only when unit was enabled) and stops it when firewalld is stopped. With this the process only runs when actually needed and does not consume resources on systems without firewalld. Signed-off-by: Paul Holzinger <pholzing@redhat.com>
This makes the interface simpler. And because I plan to serialize the data later I need it anyway in the struct. Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Because I plan to serialze this struct so we should only include information that is strictly necessary. Thus only add the bridge name and ipnet's (subnet) for the network. Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
This will be used for the firewalld reload service. On firewall setup we write the configs to disk (tmpfs) and then each time the firewalld reload can read the configs at a later point when it needs to readd rules. On teardown we need to remove the config again. This commit does not yet integrate any of this it just adds the code to do the writing, reading or removing of the config. Also in order to deserialize the configs we need a owned struct, thus we cannot use the PortForwardConfig with references. Becuase I like to avoid the unnecessary clones on the setup path I created two structs: PortForwardConfig and PortForwardConfigOwned which should help with that. Signed-off-by: Paul Holzinger <pholzing@redhat.com>
- include useful error context (the normal rust error do not include the file path for example) - ignore ENOENT on remove as it should not cause cleanup failure Signed-off-by: Paul Holzinger <pholzing@redhat.com>
First since the service is started after firewalld we assume it cleared the rules and thus we need to relaod rules right away. Then listen for the firewalld reloaded signal to reload later. Only store the rules as root as rootless has no need for the service and we do not need to pay the overhead of the writes. The --config options is now always required for setup, teardown and update. We always set it in podman so it is not a problem. Signed-off-by: Paul Holzinger <pholzing@redhat.com>
When we read the firewall configs we should assume ENOENT is no error because the reload service might be starte before any container was set up. Instead Return a Option<> so the caller can just skip the fw setup in this case. To avoid boilerplate I added some macros to deal with it better. Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Add a integration test for the firewalld-reload command. This makes sure we are actually adding the rule back and are listing for the correct event. Unfortunately this is racy as the services run in the background so I have to add sleep 1 to wait for the rules to be added back. Hopefully this is enough to avoid flakes in CI. Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Make sure we install the new unit file. Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Luap99
force-pushed
the
firewalld-reload
branch
from
e11fdec
to
708bd43
Compare
|
LGTM |
|
/lgtm |
Luap99
added a commit
to Luap99/netavark
that referenced
this pull request
Nov 16, 2023
A new change in firewalld 2.0 no longer flushes all rules. This means the test cannot check for it. Instead we must check for the trusted sources that are added in firewalld. This is causing CI failures on main right now because this test was merged with CI running on f38 while the update to f39 was just merged before that. containers#826 containers#840 Signed-off-by: Paul Holzinger <pholzing@redhat.com>
This was referenced Nov 27, 2023
jakecorrenti
pushed a commit
to jakecorrenti/netavark
that referenced
this pull request
Jan 3, 2024
A new change in firewalld 2.0 no longer flushes all rules. This means the test cannot check for it. Instead we must check for the trusted sources that are added in firewalld. This is causing CI failures on main right now because this test was merged with CI running on f38 while the update to f39 was just merged before that. containers#826 containers#840 Signed-off-by: Paul Holzinger <pholzing@redhat.com>
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.
Add a new service command which listens on for the firewalld reload dbus event and then re-adds all firewall rules back.
The unit file is written so that it is only started and stopped with firewalld so it never runs unless needed. The new unit is called netavark-firewalld-reload.service.
Tests have been added to ensure it is working as expected.
This required more changes that I would have liked but it is important to only write the minimal firewall config to disk to not waste memory as the config dir is on tmpfs.
Fixes https://issues.redhat.com/browse/RHEL-3079