feat: Apply firewall config for multiple bridges and IPv6#28
Conversation
|
Nice thanks! |
|
@bschimke95 needs to be signed off please |
bschimke95
force-pushed
the
bschimke95/make-bridges-configurable
branch
from
65e8821 to
f8bb0b8
Compare
Add a new `bridges` input parameter to allow specifying a comma-separated list of LXD bridges for iptables configuration. This makes the action more flexible for setups with multiple bridge interfaces. Also, extends the firewall configuration for IPv6 Signed-off-by: Benjamin Schimke <benjamin.schimke@canonical.com>
bschimke95
force-pushed
the
bschimke95/make-bridges-configurable
branch
from
f8bb0b8 to
3e62261
Compare
|
Thanks, that was a quick review :) Can we do a release with that change or should I use the commit? |
|
Test run using the multiple bridges input: https://github.com/canonical/k8s-snap/actions/runs/14901878737/job/41856443865?pr=1422#step:8:108 |
|
I'm happy to make a release, but will wait for @simondeziel review first. |
simondeziel
left a comment
simondeziel
left a comment
There was a problem hiding this comment.
Thanks, LGTM but I have a tiny question.
| IFS=',' read -ra bridges <<< "${{ inputs.bridges }}" | ||
| for i in "${bridges[@]}"; do | ||
| bridge=$(echo "$i" | xargs) # Trim whitespace | ||
| set +e |
There was a problem hiding this comment.
Is that because the IPv6 chain might be missing? If so, how about having another if block to check for its existence and move the bridges array out of the conditional blocks?
There was a problem hiding this comment.
Yes, this essentially applies the rules on a best-effort basis, and it's not a problem if some steps fail — they can fail, for example, if:
- the
DOCKER-USERchain doesn’t exist, or - iptables or ip6tables aren’t available on the system.
I feel that adding conditional checks would make this section unnecessarily complex, but I’m happy to include them if you think it’s worthwhile.
There was a problem hiding this comment.
The if sudo iptables -L DOCKER-USER; then condition validates that both the tool and the chain are there so I've opened up #29 to do so.
|
Hey @simondeziel |
3 participants
Add a new
bridgesinput parameter to allow specifying a comma-separated list of LXD bridges for iptables configuration. This makes the action more flexible for setups with multiple bridge interfaces.Also, extends the firewall configuration for IPv6