Merge pull request #53618 from thaJeztah/fix_iptables_rm

iptables: handle missing rules on delete

daemon/libnetwork/iptables/iptables.go

@@ -440,7 +440,11 @@ func (iptable IPTable) DeleteJumpRule(table Table, fromChain, toChain string, ru
 	rule = append(rule, "-j", toChain)
 	if iptable.Exists(table, fromChain, rule...) {
 		if err := iptable.RawCombinedOutput(append([]string{"-t", string(table), "-D", fromChain}, rule...)...); err != nil {
-			return fmt.Errorf("unable to remove jump to %s rule in %s chain: %v", toChain, fromChain, err)
+			// Another process may have removed the rule between Exists and Delete.
+			if isRuleNotFoundError(err) {
+				return nil
+			}
+			return fmt.Errorf("unable to remove jump to %s rule in %s chain: %w", toChain, fromChain, err)
 		}
 	}
 	return nil
@@ -500,7 +504,14 @@ func (r Rule) Delete() error {
 	if !r.Exists() {
 		return nil
 	}
-	return r.exec(Delete)
+	if err := r.exec(Delete); err != nil {
+		// Another process may have removed the rule between Exists and Delete.
+		if isRuleNotFoundError(err) {
+			return nil
+		}
+		return err
+	}
+	return nil
 }
 
 func (r Rule) String() string {
@@ -510,3 +521,18 @@ func (r Rule) String() string {
 	}
 	return strings.Join(cmd, " ")
 }
+
+// isRuleNotFoundError reports whether err indicates that an iptables
+// delete could not find a matching rule.
+//
+// The iptables-nft backend maps ENOENT from nft_rule_delete to this
+// diagnostic:
+//
+//	iptables: Bad rule (does a matching rule exist in that chain?).
+//
+// see: https://git.netfilter.org/iptables/tree/iptables/nft.c?id=384958620abab397062b67fb2763e813b63f74f0#n2745
+// ref: https://git.netfilter.org/iptables/commit/?id=384958620abab397062b67fb2763e813b63f74f0
+func isRuleNotFoundError(err error) bool {
+	return err != nil &&
+		strings.Contains(err.Error(), "Bad rule (does a matching rule exist in that chain?)")
+}

daemon/libnetwork/iptables/iptables_test.go

@@ -336,3 +336,33 @@ func TestFlushChain(t *testing.T) {
 	// Cleanup
 	_ = iptable.RemoveExistingChain(chain, table)
 }
+
+// TestDeleteNonExistingRule verifies the diagnostic returned by iptables when
+// deleting a rule that does not exist. This is used to handle the race where a
+// rule is removed by another process between checking for it and deleting it.
+//
+// Reproducing that race directly would be timing-dependent, so exercise the
+// delete error deterministically with a rule in a freshly-created chain.
+func TestDeleteNonExistingRule(t *testing.T) {
+	if UsingFirewalld() {
+		t.Skip("firewalld in host netns cannot create rules in the test's netns")
+	}
+	defer netnsutils.SetupTestOSContext(t)()
+	iptable := GetIptable(IPv4)
+
+	const chain = "TEST-DELETEMISSING"
+	_, err := iptable.NewChain(chain, Filter)
+	assert.NilError(t, err)
+	defer func() { _ = iptable.RemoveExistingChain(chain, Filter) }()
+
+	// Use TEST-NET-1 to make the rule clearly test-only. The freshly-created
+	// chain guarantees that the rule does not already exist.
+	err = iptable.RawCombinedOutput(
+		"-t", string(Filter),
+		"-D", chain,
+		"-s", "192.0.2.1",
+		"-j", "ACCEPT",
+	)
+	assert.Assert(t, err != nil)
+	assert.Assert(t, isRuleNotFoundError(err), err)
+}