From 9c4a6541057dca9784311639e7bf85585ef0b3f6 Mon Sep 17 00:00:00 2001 From: Roman Dodin Date: Wed, 15 Jan 2025 16:03:54 +0100 Subject: [PATCH 01/10] added v6 rule for mgmt bridge with nftables --- docs/manual/network.md | 15 ++++++++++----- runtime/docker/firewall.go | 2 +- runtime/docker/firewall/nftables/client.go | 18 +++++++++++++++--- 3 files changed, 26 insertions(+), 9 deletions(-) diff --git a/docs/manual/network.md b/docs/manual/network.md index 57915b038..ca94520de 100644 --- a/docs/manual/network.md +++ b/docs/manual/network.md @@ -247,17 +247,22 @@ With this approach, users can prevent IP address overlap with nodes deployed on #### external access -Containerlab will attempt to enable external access to the nodes by default. This means that external systems/hosts will be able to communicate with the nodes of your topology without requiring any manual iptables/nftables rules to be installed. +Containerlab will attempt to enable external management access to the nodes by default. This means that external systems/hosts will be able to communicate with the nodes of your topology without requiring any manual iptables/nftables rules to be installed. -To allow external communications containerlab installs a rule in the `DOCKER-USER` chain, allowing all packets targeting containerlab's management network. The rule looks like follows: +To allow external communications containerlab installs a rule in the `DOCKER-USER` chain for v4 and v6, allowing all packets targeting containerlab's management network. The rule looks like follows: ```shell -❯ sudo iptables -vnL DOCKER-USER +sudo iptables -vnL DOCKER-USER +``` + +
+```{.no-copy .no-select} Chain DOCKER-USER (1 references) - pkts bytes target prot opt in out source destination + pkts bytes target prot opt in out source destination 0 0 ACCEPT all -- * br-a8b9fc8b33a2 0.0.0.0/0 0.0.0.0/0 /* set by containerlab */ -12719 79M RETURN all -- * * 0.0.0.0/0 0.0.0.0/0 +12719 79M RETURN all -- * * 0.0.0.0/0 0.0.0.0/0 ``` +
1. The `br-a8b9fc8b33a2` bridge interface is the interface that backs up the containerlab's management network (`clab` docker network). diff --git a/runtime/docker/firewall.go b/runtime/docker/firewall.go index e5a60a676..0905ea1ea 100644 --- a/runtime/docker/firewall.go +++ b/runtime/docker/firewall.go @@ -20,7 +20,7 @@ func (d *DockerRuntime) deleteFwdRule() (err error) { } // installFwdRule installs the `allow` rule for traffic destined to the nodes -// on the clab management network. +// on the clab management network for v4 and v6. // This rule is required for external access to the nodes. func (d *DockerRuntime) installFwdRule() (err error) { if !*d.mgmt.ExternalAccess { diff --git a/runtime/docker/firewall/nftables/client.go b/runtime/docker/firewall/nftables/client.go index eb61d54ff..40fc61ab1 100644 --- a/runtime/docker/firewall/nftables/client.go +++ b/runtime/docker/firewall/nftables/client.go @@ -90,11 +90,23 @@ func (c *NftablesClient) DeleteForwardingRules() error { return nil } -// InstallForwardingRules installs the forwarding rules. +// InstallForwardingRules installs the forwarding rules for v4 and v6 address families. func (c *NftablesClient) InstallForwardingRules() error { defer c.close() - rules, err := c.getRules(definitions.DockerFWUserChain, definitions.DockerFWTable, nftables.TableFamilyIPv4) + err := c.InstallForwardingRulesForAF(nftables.TableFamilyIPv4) + if err != nil { + return err + } + + return c.InstallForwardingRulesForAF(nftables.TableFamilyIPv6) + +} + +// InstallForwardingRulesForAF installs the forwarding rules for the specified address family. +func (c *NftablesClient) InstallForwardingRulesForAF(af nftables.TableFamily) error { + + rules, err := c.getRules(definitions.DockerFWUserChain, definitions.DockerFWTable, af) if err != nil { return fmt.Errorf("%w. See http://containerlab.dev/manual/network/#external-access", err) } @@ -107,7 +119,7 @@ func (c *NftablesClient) InstallForwardingRules() error { log.Debugf("Installing iptables rules for bridge %q", c.bridgeName) // create a new rule - rule, err := c.newClabNftablesRule(definitions.DockerFWUserChain, definitions.DockerFWTable, nftables.TableFamilyIPv4, 0) + rule, err := c.newClabNftablesRule(definitions.DockerFWUserChain, definitions.DockerFWTable, af, 0) if err != nil { return err } From a2bbf00146dfdd0ddd06a15c0061cecb2ef947ca Mon Sep 17 00:00:00 2001 From: Roman Dodin Date: Thu, 16 Jan 2025 12:38:46 +0100 Subject: [PATCH 02/10] delete nftables v6 rules --- runtime/docker/firewall/nftables/client.go | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/runtime/docker/firewall/nftables/client.go b/runtime/docker/firewall/nftables/client.go index 40fc61ab1..09c94a12e 100644 --- a/runtime/docker/firewall/nftables/client.go +++ b/runtime/docker/firewall/nftables/client.go @@ -58,15 +58,21 @@ func (c *NftablesClient) DeleteForwardingRules() error { return nil } - // first check if a rule already exists to not create duplicates defer c.close() - rules, err := c.getRules(definitions.DockerFWUserChain, definitions.DockerFWTable, nftables.TableFamilyIPv4) + v4rules, err := c.getRules(definitions.DockerFWUserChain, definitions.DockerFWTable, nftables.TableFamilyIPv4) if err != nil { return fmt.Errorf("%w. See http://containerlab.dev/manual/network/#external-access", err) } - mgmtBrRules := c.getRulesForMgmtBr(c.bridgeName, rules) + v6rules, err := c.getRules(definitions.DockerFWUserChain, definitions.DockerFWTable, nftables.TableFamilyIPv6) + if err != nil { + return fmt.Errorf("%w. See http://containerlab.dev/manual/network/#external-access", err) + } + + v4v6rules := append(v4rules, v6rules...) + + mgmtBrRules := c.getRulesForMgmtBr(c.bridgeName, v4v6rules) if len(mgmtBrRules) == 0 { log.Debug("external access iptables rule doesn't exist. Skipping deletion") return nil From 99e6ce803c15c8201f39382c3e5a9490a05d0882 Mon Sep 17 00:00:00 2001 From: Roman Dodin Date: Thu, 16 Jan 2025 12:51:58 +0100 Subject: [PATCH 03/10] added iptables based add/delete for v6 --- runtime/docker/firewall/iptables/client.go | 108 ++++++++++++++++----- tests/01-smoke/01-basic-flow.robot | 31 ++++++ 2 files changed, 117 insertions(+), 22 deletions(-) diff --git a/runtime/docker/firewall/iptables/client.go b/runtime/docker/firewall/iptables/client.go index 8af5d29e0..977017525 100644 --- a/runtime/docker/firewall/iptables/client.go +++ b/runtime/docker/firewall/iptables/client.go @@ -13,25 +13,33 @@ import ( ) const ( - iptCheckCmd = "-vL DOCKER-USER" - iptAllowCmd = "-I DOCKER-USER -o %s -j ACCEPT -m comment --comment \"" + definitions.IPTablesRuleComment + "\"" - iptDelCmd = "-D DOCKER-USER -o %s -j ACCEPT -m comment --comment \"" + definitions.IPTablesRuleComment + "\"" - ipTables = "ip_tables" + iptCheckArgs = "-vL DOCKER-USER" + iptAllowArgs = "-I DOCKER-USER -o %s -j ACCEPT -m comment --comment \"" + definitions.IPTablesRuleComment + "\"" + iptDelArgs = "-D DOCKER-USER -o %s -j ACCEPT -m comment --comment \"" + definitions.IPTablesRuleComment + "\"" + ipTables = "ip_tables" + + v4AF = "v4" + ip4tablesCmd = "iptables" + v6AF = "v6" + ip6tablesCmd = "ip6tables" ) // IpTablesClient is a client for iptables. type IpTablesClient struct { bridgeName string + ip6_tables bool } // NewIpTablesClient returns a new IpTablesClient. func NewIpTablesClient(bridgeName string) (*IpTablesClient, error) { - loaded, err := utils.IsKernelModuleLoaded("ip_tables") + v4ModLoaded, err := utils.IsKernelModuleLoaded("ip_tables") if err != nil { return nil, err } - if !loaded { + v6ModLoaded, _ := utils.IsKernelModuleLoaded("ip6_tables") + + if !v4ModLoaded { log.Debug("ip_tables kernel module not available") // module is not loaded return nil, definitions.ErrNotAvailable @@ -39,6 +47,7 @@ func NewIpTablesClient(bridgeName string) (*IpTablesClient, error) { return &IpTablesClient{ bridgeName: bridgeName, + ip6_tables: v6ModLoaded, }, nil } @@ -47,28 +56,40 @@ func (*IpTablesClient) Name() string { return ipTables } -// InstallForwardingRules installs the forwarding rules. +// InstallForwardingRules installs the forwarding rules for v4 and v6 address families. func (c *IpTablesClient) InstallForwardingRules() error { - // first check if a rule already exists to not create duplicates - res, err := exec.Command("iptables", strings.Split(iptCheckCmd, " ")...).Output() - if bytes.Contains(res, []byte(c.bridgeName)) { - log.Debugf("found iptables forwarding rule targeting the bridge %q. Skipping creation of the forwarding rule.", c.bridgeName) + err := c.InstallForwardingRulesForAF(v4AF) + if err != nil { return err } - if err != nil { - // non nil error typically means that DOCKER-USER chain doesn't exist - // this happens with old docker installations (centos7 hello) from default repos - return fmt.Errorf("missing DOCKER-USER iptables chain. See http://containerlab.dev/manual/network/#external-access") + + if c.ip6_tables { + err = c.InstallForwardingRulesForAF(v6AF) } - cmd, err := shlex.Split(fmt.Sprintf(iptAllowCmd, c.bridgeName)) + return err +} + +// InstallForwardingRulesForAF installs the forwarding rules for the specified address family. +func (c *IpTablesClient) InstallForwardingRulesForAF(af string) error { + iptCmd := ip4tablesCmd + if af == v6AF { + iptCmd = ip6tablesCmd + } + + // first check if a rule already exists to not create duplicates + if c.allowRuleForMgmtBrExists(af) { + return nil + } + + cmd, err := shlex.Split(fmt.Sprintf(iptAllowArgs, c.bridgeName)) if err != nil { return err } - log.Debugf("Installing iptables rules for bridge %q", c.bridgeName) + log.Debugf("Installing iptables (%s) rules for bridge %q", af, c.bridgeName) - stdOutErr, err := exec.Command("iptables", cmd...).CombinedOutput() + stdOutErr, err := exec.Command(iptCmd, cmd...).CombinedOutput() if err != nil { log.Warnf("Iptables install stdout/stderr result is: %s", stdOutErr) return fmt.Errorf("unable to install iptables rule using '%s' command: %w", cmd, err) @@ -77,10 +98,29 @@ func (c *IpTablesClient) InstallForwardingRules() error { return nil } -// DeleteForwardingRules deletes the forwarding rules. +// DeleteForwardingRules deletes the forwarding rules for v4 and v6 address families. func (c *IpTablesClient) DeleteForwardingRules() error { + err := c.DeleteForwardingRulesForAF(v4AF) + if err != nil { + return err + } + + if c.ip6_tables { + err = c.InstallForwardingRulesForAF(v6AF) + } + + return err +} + +// DeleteForwardingRulesForAF deletes the forwarding rules for a specified AF. +func (c *IpTablesClient) DeleteForwardingRulesForAF(af string) error { + iptCmd := ip4tablesCmd + if af == v6AF { + iptCmd = ip6tablesCmd + } + // first check if a rule exists before trying to delete it - res, err := exec.Command("iptables", strings.Split(iptCheckCmd, " ")...).Output() + res, err := exec.Command(iptCmd, strings.Split(iptCheckArgs, " ")...).Output() if err != nil { // non nil error typically means that DOCKER-USER chain doesn't exist // this happens with old docker installations (centos7 hello) from default repos @@ -101,7 +141,7 @@ func (c *IpTablesClient) DeleteForwardingRules() error { return nil } - cmd, err := shlex.Split(fmt.Sprintf(iptDelCmd, c.bridgeName)) + cmd, err := shlex.Split(fmt.Sprintf(iptDelArgs, c.bridgeName)) if err != nil { return err } @@ -109,7 +149,7 @@ func (c *IpTablesClient) DeleteForwardingRules() error { log.Debugf("removing clab iptables rules for bridge %q", c.bridgeName) log.Debugf("trying to delete the forwarding rule with cmd: iptables %s", cmd) - stdOutErr, err := exec.Command("iptables", cmd...).CombinedOutput() + stdOutErr, err := exec.Command(iptCmd, cmd...).CombinedOutput() if err != nil { log.Warnf("Iptables delete stdout/stderr result is: %s", stdOutErr) return fmt.Errorf("unable to delete iptables rules: %w", err) @@ -117,3 +157,27 @@ func (c *IpTablesClient) DeleteForwardingRules() error { return nil } + +// allowRuleForMgmtBrExists checks if an allow rule for the provided bridge name exists. +// The actual check doesn't verify that `allow` is set, it just checks if the rule +// has the provided bridge name in the output interface. +func (c *IpTablesClient) allowRuleForMgmtBrExists(af string) bool { + iptCmd := ip4tablesCmd + if af == v6AF { + iptCmd = ip6tablesCmd + } + + res, err := exec.Command(iptCmd, strings.Split(iptCheckArgs, " ")...).CombinedOutput() + if err != nil { + log.Warnf("iptables check error: %s. Output: %s", err, string(res)) + // if we errored on check we don't want to try setting up the rule + return true + } + if bytes.Contains(res, []byte(c.bridgeName)) { + log.Debugf("found iptables forwarding rule targeting the bridge %q. Skipping creation of the forwarding rule.", c.bridgeName) + + return true + } + + return false +} diff --git a/tests/01-smoke/01-basic-flow.robot b/tests/01-smoke/01-basic-flow.robot index 61ac8c263..257655872 100644 --- a/tests/01-smoke/01-basic-flow.robot +++ b/tests/01-smoke/01-basic-flow.robot @@ -317,6 +317,25 @@ Verify iptables allow rule is set ... ignore_case=True ... collapse_spaces=True +Verify ip6tables allow rule is set + [Documentation] Checking if ip6tables allow rule is set so that external traffic can reach containerlab management network + Skip If '${runtime}' != 'docker' + + # Add check for ip6tables availability + ${rc} ${output} = Run And Return Rc And Output which ip6tables + Skip If ${rc} != 0 ip6tables command not found + + + ${ipt} = Run + ... sudo ip6tables -vnL DOCKER-USER + Log ${ipt} + # debian 12 uses `0` for protocol, while previous versions use `all` + Should Contain Any ${ipt} + ... ACCEPT all -- * ${MgmtBr} + ... ACCEPT 0 -- * ${MgmtBr} + ... ignore_case=True + ... collapse_spaces=True + Verify DNS-Server Config [Documentation] Check if the DNS config did take effect Skip If '${runtime}' != 'docker' @@ -403,6 +422,18 @@ Verify iptables allow rule are gone Log ${ipt} Should Not Contain ${ipt} ${MgmtBr} +Verify ip6tables allow rule are gone + [Documentation] Checking if ip6tables allow rule is removed once the lab is destroyed + Skip If '${runtime}' != 'docker' + + # Add check for ip6tables availability + ${rc} ${output} = Run And Return Rc And Output which ip6tables + Skip If ${rc} != 0 ip6tables command not found + + ${ipt} = Run + ... sudo ip6tables -vnL DOCKER-USER + Log ${ipt} + Should Not Contain ${ipt} ${MgmtBr} *** Keywords *** Match IPv6 Address From 511333ef88c2bc1d00f7c90cc8a9338de93d0244 Mon Sep 17 00:00:00 2001 From: Roman Dodin Date: Thu, 16 Jan 2025 13:18:30 +0100 Subject: [PATCH 04/10] use nft cli instead of ip6tables --- tests/01-smoke/01-basic-flow.robot | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/tests/01-smoke/01-basic-flow.robot b/tests/01-smoke/01-basic-flow.robot index 257655872..f8472317e 100644 --- a/tests/01-smoke/01-basic-flow.robot +++ b/tests/01-smoke/01-basic-flow.robot @@ -322,19 +322,15 @@ Verify ip6tables allow rule is set Skip If '${runtime}' != 'docker' # Add check for ip6tables availability - ${rc} ${output} = Run And Return Rc And Output which ip6tables - Skip If ${rc} != 0 ip6tables command not found + ${rc} ${output} = Run And Return Rc And Output which nft + Skip If ${rc} != 0 nft command not found ${ipt} = Run - ... sudo ip6tables -vnL DOCKER-USER + ... sudo nft list chain ip6 filter DOCKER-USER Log ${ipt} - # debian 12 uses `0` for protocol, while previous versions use `all` - Should Contain Any ${ipt} - ... ACCEPT all -- * ${MgmtBr} - ... ACCEPT 0 -- * ${MgmtBr} - ... ignore_case=True - ... collapse_spaces=True + Should Match Regexp ${ipt} oifname.*${MgmtBr}.*accept + Verify DNS-Server Config [Documentation] Check if the DNS config did take effect @@ -427,11 +423,11 @@ Verify ip6tables allow rule are gone Skip If '${runtime}' != 'docker' # Add check for ip6tables availability - ${rc} ${output} = Run And Return Rc And Output which ip6tables - Skip If ${rc} != 0 ip6tables command not found + ${rc} ${output} = Run And Return Rc And Output which nft + Skip If ${rc} != 0 nft command not found ${ipt} = Run - ... sudo ip6tables -vnL DOCKER-USER + ... sudo nft list chain ip6 filter DOCKER-USER Log ${ipt} Should Not Contain ${ipt} ${MgmtBr} From fa99a817fd26b0c3d73cdb87ac39837a0dc9c9b2 Mon Sep 17 00:00:00 2001 From: Roman Dodin Date: Thu, 16 Jan 2025 13:31:34 +0100 Subject: [PATCH 05/10] time to dive deeper --- .github/workflows/smoke-tests.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/smoke-tests.yml b/.github/workflows/smoke-tests.yml index 628749b90..bb0b9f654 100644 --- a/.github/workflows/smoke-tests.yml +++ b/.github/workflows/smoke-tests.yml @@ -70,6 +70,9 @@ jobs: - name: Sanitize test-suite name run: echo "TEST_SUITE=$(echo ${{ matrix.test-suite }} | tr -d '*')" >> $GITHUB_ENV + - name: setup tmate session + uses: mxschmitt/action-tmate@v3 + - name: Run smoke tests run: | bash ./tests/rf-run.sh ${{ matrix.runtime }} ./tests/01-smoke/${{ matrix.test-suite }} From 06beb6c23d038467bdd5c00baae299022022eeee Mon Sep 17 00:00:00 2001 From: Roman Dodin Date: Thu, 16 Jan 2025 13:41:33 +0100 Subject: [PATCH 06/10] try ubuntu 24.04 --- .github/workflows/smoke-tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/smoke-tests.yml b/.github/workflows/smoke-tests.yml index bb0b9f654..361e05860 100644 --- a/.github/workflows/smoke-tests.yml +++ b/.github/workflows/smoke-tests.yml @@ -9,7 +9,7 @@ name: smoke-tests jobs: smoke-tests: - runs-on: ubuntu-22.04 + runs-on: ubuntu-24.04 timeout-minutes: 5 strategy: fail-fast: false From 610814ba42f7c0716c44bce0776ce17851f593c8 Mon Sep 17 00:00:00 2001 From: Roman Dodin Date: Thu, 16 Jan 2025 14:21:11 +0100 Subject: [PATCH 07/10] install v6 tables with nftables only if v6 tables are available --- runtime/docker/firewall/nftables/client.go | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/runtime/docker/firewall/nftables/client.go b/runtime/docker/firewall/nftables/client.go index 09c94a12e..0fb7a74c1 100644 --- a/runtime/docker/firewall/nftables/client.go +++ b/runtime/docker/firewall/nftables/client.go @@ -19,6 +19,8 @@ const nfTables = "nf_tables" type NftablesClient struct { nftConn *nftables.Conn bridgeName string + // is ip6_tables supported + ip6_tables bool } // NewNftablesClient returns a new NftablesClient. @@ -43,6 +45,12 @@ func NewNftablesClient(bridgeName string) (*NftablesClient, error) { return nil, definitions.ErrNotAvailable } + // check if ip6_tables is available + v6Tables, err := nftC.nftConn.ListTablesOfFamily(nftables.TableFamilyIPv6) + if err != nil || len(v6Tables) == 0 { + nftC.ip6_tables = false + } + return nftC, nil } @@ -105,8 +113,11 @@ func (c *NftablesClient) InstallForwardingRules() error { return err } - return c.InstallForwardingRulesForAF(nftables.TableFamilyIPv6) + if c.ip6_tables { + err = c.InstallForwardingRulesForAF(nftables.TableFamilyIPv6) + } + return err } // InstallForwardingRulesForAF installs the forwarding rules for the specified address family. From 9cc1c38a46f085226e2e4ed59c506bbc4bf4fa86 Mon Sep 17 00:00:00 2001 From: Roman Dodin Date: Thu, 16 Jan 2025 14:29:45 +0100 Subject: [PATCH 08/10] skip checks if ip6 tables not found --- .github/workflows/smoke-tests.yml | 4 ++-- tests/01-smoke/01-basic-flow.robot | 5 +++++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/.github/workflows/smoke-tests.yml b/.github/workflows/smoke-tests.yml index 361e05860..6687103be 100644 --- a/.github/workflows/smoke-tests.yml +++ b/.github/workflows/smoke-tests.yml @@ -70,8 +70,8 @@ jobs: - name: Sanitize test-suite name run: echo "TEST_SUITE=$(echo ${{ matrix.test-suite }} | tr -d '*')" >> $GITHUB_ENV - - name: setup tmate session - uses: mxschmitt/action-tmate@v3 + # - name: setup tmate session + # uses: mxschmitt/action-tmate@v3 - name: Run smoke tests run: | diff --git a/tests/01-smoke/01-basic-flow.robot b/tests/01-smoke/01-basic-flow.robot index f8472317e..484c36b4a 100644 --- a/tests/01-smoke/01-basic-flow.robot +++ b/tests/01-smoke/01-basic-flow.robot @@ -325,6 +325,8 @@ Verify ip6tables allow rule is set ${rc} ${output} = Run And Return Rc And Output which nft Skip If ${rc} != 0 nft command not found + ${rc} ${output} = Run And Return Rc And Output sudo nft list tables + Skip If 'ip6 filter' not in '''${output}''' ip6 filter chain not found ${ipt} = Run ... sudo nft list chain ip6 filter DOCKER-USER @@ -426,6 +428,9 @@ Verify ip6tables allow rule are gone ${rc} ${output} = Run And Return Rc And Output which nft Skip If ${rc} != 0 nft command not found + ${rc} ${output} = Run And Return Rc And Output sudo nft list tables + Skip If 'ip6 filter' not in '''${output}''' ip6 filter chain not found + ${ipt} = Run ... sudo nft list chain ip6 filter DOCKER-USER Log ${ipt} From 0bab43c352cf02f82e6a182756e7ff36c13a563c Mon Sep 17 00:00:00 2001 From: Roman Dodin Date: Thu, 16 Jan 2025 14:41:46 +0100 Subject: [PATCH 09/10] bring back tmate --- .github/workflows/smoke-tests.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/smoke-tests.yml b/.github/workflows/smoke-tests.yml index 6687103be..361e05860 100644 --- a/.github/workflows/smoke-tests.yml +++ b/.github/workflows/smoke-tests.yml @@ -70,8 +70,8 @@ jobs: - name: Sanitize test-suite name run: echo "TEST_SUITE=$(echo ${{ matrix.test-suite }} | tr -d '*')" >> $GITHUB_ENV - # - name: setup tmate session - # uses: mxschmitt/action-tmate@v3 + - name: setup tmate session + uses: mxschmitt/action-tmate@v3 - name: Run smoke tests run: | From f880e3845d20a39f04337baf9b4b5d6fb32a233a Mon Sep 17 00:00:00 2001 From: Roman Dodin Date: Thu, 16 Jan 2025 14:53:26 +0100 Subject: [PATCH 10/10] delete v6 rules only if supported --- .github/workflows/smoke-tests.yml | 4 ++-- runtime/docker/firewall/nftables/client.go | 15 +++++++++------ 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/.github/workflows/smoke-tests.yml b/.github/workflows/smoke-tests.yml index 361e05860..6687103be 100644 --- a/.github/workflows/smoke-tests.yml +++ b/.github/workflows/smoke-tests.yml @@ -70,8 +70,8 @@ jobs: - name: Sanitize test-suite name run: echo "TEST_SUITE=$(echo ${{ matrix.test-suite }} | tr -d '*')" >> $GITHUB_ENV - - name: setup tmate session - uses: mxschmitt/action-tmate@v3 + # - name: setup tmate session + # uses: mxschmitt/action-tmate@v3 - name: Run smoke tests run: | diff --git a/runtime/docker/firewall/nftables/client.go b/runtime/docker/firewall/nftables/client.go index 0fb7a74c1..6e91de159 100644 --- a/runtime/docker/firewall/nftables/client.go +++ b/runtime/docker/firewall/nftables/client.go @@ -68,19 +68,22 @@ func (c *NftablesClient) DeleteForwardingRules() error { defer c.close() - v4rules, err := c.getRules(definitions.DockerFWUserChain, definitions.DockerFWTable, nftables.TableFamilyIPv4) + allRules, err := c.getRules(definitions.DockerFWUserChain, definitions.DockerFWTable, nftables.TableFamilyIPv4) if err != nil { return fmt.Errorf("%w. See http://containerlab.dev/manual/network/#external-access", err) } - v6rules, err := c.getRules(definitions.DockerFWUserChain, definitions.DockerFWTable, nftables.TableFamilyIPv6) - if err != nil { - return fmt.Errorf("%w. See http://containerlab.dev/manual/network/#external-access", err) + var v6rules []*nftables.Rule + if c.ip6_tables { + v6rules, err = c.getRules(definitions.DockerFWUserChain, definitions.DockerFWTable, nftables.TableFamilyIPv6) + if err != nil { + return fmt.Errorf("%w. See http://containerlab.dev/manual/network/#external-access", err) + } } - v4v6rules := append(v4rules, v6rules...) + allRules = append(allRules, v6rules...) - mgmtBrRules := c.getRulesForMgmtBr(c.bridgeName, v4v6rules) + mgmtBrRules := c.getRulesForMgmtBr(c.bridgeName, allRules) if len(mgmtBrRules) == 0 { log.Debug("external access iptables rule doesn't exist. Skipping deletion") return nil