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.

Sign up for GitHub

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

Jump to bottom

contrib: add firewall reload services #20249

Closed
wants to merge 2 commits into from
Closed

contrib: add firewall reload services #20249

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fa60cec
contrib: add firewall reload services
danishprakash Oct 4, 2023
1390929
Update Makefile
danishprakash Oct 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -887,7 +887,9 @@ PODMAN_UNIT_FILES = contrib/systemd/auto-update/podman-auto-update.service \
contrib/systemd/system/podman.service \
contrib/systemd/system/podman-restart.service \
contrib/systemd/system/podman-kube@.service \
contrib/systemd/system/podman-clean-transient.service
contrib/systemd/system/podman-clean-transient.service \
contrib/systemd/system/podman-firewalld-reload.service \
contrib/systemd/system/podman-firewalld-restart.service

%.service: %.service.in
sed -e 's;@@PODMAN@@;$(BINDIR)/podman;g' $< >$@.tmp.$$ \
Expand All @@ -902,6 +904,8 @@ install.systemd: $(PODMAN_UNIT_FILES)
install ${SELINUXOPT} -m 644 contrib/systemd/system/podman.service ${DESTDIR}${USERSYSTEMDDIR}/podman.service
install ${SELINUXOPT} -m 644 contrib/systemd/system/podman-restart.service ${DESTDIR}${USERSYSTEMDDIR}/podman-restart.service
install ${SELINUXOPT} -m 644 contrib/systemd/system/podman-kube@.service ${DESTDIR}${USERSYSTEMDDIR}/podman-kube@.service
install ${SELINUXOPT} -m 644 contrib/systemd/system/podman-firewalld-reload.service ${DESTDIR}${USERSYSTEMDDIR}/podman-firewalld-reload.service
install ${SELINUXOPT} -m 644 contrib/systemd/system/podman-firewalld-restart.service ${DESTDIR}${USERSYSTEMDDIR}/podman-firewalld-restart.service
# System services
install ${SELINUXOPT} -m 644 contrib/systemd/auto-update/podman-auto-update.service ${DESTDIR}${SYSTEMDDIR}/podman-auto-update.service
install ${SELINUXOPT} -m 644 contrib/systemd/auto-update/podman-auto-update.timer ${DESTDIR}${SYSTEMDDIR}/podman-auto-update.timer
Expand All @@ -910,6 +914,8 @@ install.systemd: $(PODMAN_UNIT_FILES)
install ${SELINUXOPT} -m 644 contrib/systemd/system/podman-restart.service ${DESTDIR}${SYSTEMDDIR}/podman-restart.service
install ${SELINUXOPT} -m 644 contrib/systemd/system/podman-kube@.service ${DESTDIR}${SYSTEMDDIR}/podman-kube@.service
install ${SELINUXOPT} -m 644 contrib/systemd/system/podman-clean-transient.service ${DESTDIR}${SYSTEMDDIR}/podman-clean-transient.service
install ${SELINUXOPT} -m 644 contrib/systemd/system/podman-firewalld-reload.service ${DESTDIR}${SYSTEMDDIR}/podman-firewalld-reload.service
install ${SELINUXOPT} -m 644 contrib/systemd/system/podman-firewalld-restart.service ${DESTDIR}${SYSTEMDDIR}/podman-firewalld-restart.service
rm -f $(PODMAN_UNIT_FILES)
else
install.systemd:
Expand Down
11 changes: 11 additions & 0 deletions contrib/systemd/system/podman-firewalld-reload.service.in
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
[Unit]
Description=firewalld reload hook - run a hook script on firewalld reload
Wants=dbus.service
After=dbus.service

[Service]
Type=simple
ExecStart=/usr/bin/bash -c '/usr/bin/busctl monitor --system --match "interface=org.fedoraproject.FirewallD1,member=Reloaded" --match "interface=org.fedoraproject.FirewallD1,member=PropertiesChanged" | while read -r line ; do @@PODMAN@@ network reload --all ; done'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the purpose of the read -r line, if $line is never used?


[Install]
WantedBy=multi-user.target
12 changes: 12 additions & 0 deletions contrib/systemd/system/podman-firewalld-restart.service.in
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
[Unit]
Description=Redo podman NAT rules after firewalld starts or reloads
Wants=dbus.service
After=dbus.service

[Service]
Type=simple
ExecStart=/usr/bin/bash -c '/usr/bin/dbus-monitor --profile --system "type=signal,sender=org.freedesktop.DBus,path=/org/freedesktop/DBus,interface=org.freedesktop.DBus,member=NameAcquired,arg0=org.fedoraproject.FirewallD1" "type=signal,path=/org/fedoraproject/FirewallD1,interface=org.fedoraproject.FirewallD1,member=Reloaded" | sed -u "/^#/d" | while read -r type timestamp serial sender destination path interface member _junk; do if [[ $type = "#"* ]]; then continue; elif [[ $interface = org.freedesktop.DBus && $member = NameAcquired ]]; then echo "firewalld started"; @@PODMAN@@ network reload --all; elif [[ $interface = org.fedoraproject.FirewallD1 && $member = Reloaded ]]; then echo "firewalld reloaded"; @@PODMAN@@ network reload --all; fi; done'
Restart=Always

[Install]
WantedBy=default.target
37 changes: 37 additions & 0 deletions test/system/250-systemd.bats
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -479,4 +479,41 @@ $name stderr" "logs work with passthrough"
run_podman generate --help
is "$output" ".*\[DEPRECATED\] Generate systemd units"
}

@test "podman network reload on firewall-cmd --reload" {
setup_firewalld_services
Comment on lines +483 to +484
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test must be skipped as rootless and when firewalld is not installed or not active.

But in general I am not sure we really want to test this in systems test, reloading the firewalld could have negative consequences for other applications as well outside of the tests. Given the high amount of different environments these tests are run I don't think it is a good idea to do it.


systemctl daemon-reload

reload_service="podman-firewalld-reload.service"
systemctl start $reload_service
systemctl is-active $reload_service

restart_service="podman-firewalld-restart.service"
systemctl start $restart_service
systemctl is-active $restart_service

cname="testctr"
run_podman run -d --rm --name $cname fedora:latest sleep 10d
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't use fedora:latest like that. Use $IMAGE if possible, or $SYSTEMD_IMAGE if for some reason you actually need fedora


# reload firewalld
firewall-cmd --reload

# ensure the rules are present
fout=$(run firewall-cmd --zone=trusted --list-all | grep "sources")
assert "$fout" != " sources: " # non-empty

# restart firewalld service
systemctl restart firewalld.service
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems unlikely to work rootless...?


# ensure the rules are still present
fout=$(run firewall-cmd --zone=trusted --list-all | grep "sources")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

run is not intended to be used this way. First, it produces no output. Second, it hides exit status.

assert "$fout" != " sources: " # non-empty
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...therefore this assertion is a NOP. $fout will always be an empty string.

Comment on lines +500 to +511
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just asking for flakes, there is no guarantee that by the time you check the network reload command was already run.
Also it would make much more sense to actually test the rules are restored by ding connectivity tests like done in the networking tests.


run_podman kill $cname
run_podman rm $cname

systemctl stop $reload_service
systemctl stop $restart_service
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not seeing a cleanup of the unit files...

}
# vim: filetype=sh
16 changes: 16 additions & 0 deletions test/system/helpers.systemd.bash
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,3 +63,19 @@ quadlet_to_service_name() {

echo "$filename$suffix.service"
}

setup_firewalld_services() {
unit_names=("podman-firewalld-reload.service" "podman-firewalld-restart.service")

for unit_name in "${unit_names[@]}"; do
unit_file="contrib/systemd/system/${unit_name}"

if [[ -e $unit_file.in ]]; then
echo "# [Building & using $unit_name from source]" >&3
# Force regenerating unit file (existing one may have /usr/bin path)
rm -f "$unit_file"
BINDIR=$(dirname "$PODMAN") make "$unit_file"
cp "$unit_file" "$UNIT_DIR/$unit_name"
fi
done
}