diff --git a/network-go/iptables/iptables.go b/network-go/iptables/iptables.go index c096dbb..db01aab 100644 --- a/network-go/iptables/iptables.go +++ b/network-go/iptables/iptables.go @@ -253,13 +253,18 @@ func (m *Manager) deleteMatchingLinesInContainer(pid int, table, chain string, g // InsertPreroutingRule inserts a DNAT PREROUTING rule on the host func (m *Manager) InsertPreroutingRule(sourceIP, proto, sourcePort, targetIP, targetPort, comment string) error { - logger.Info("IPTABLES: inserting PREROUTING DNAT rule: src=%s proto=%s sport=%s -> dst=%s dport=%s comment=%q", + logger.Info("IPTABLES: checking PREROUTING DNAT rule: src=%s proto=%s sport=%s -> dst=%s dport=%s comment=%q", sourceIP, proto, sourcePort, targetIP, targetPort, comment) - patterns := []string{"DNAT", sourcePort, targetIP, targetPort, comment} - if err := m.deleteMatchingLines("PREROUTING", "nat", patterns...); err != nil { - return fmt.Errorf("failed to delete old PREROUTING rules: %w", err) + + // Idempotent: check if rule already exists + existing := m.getLineNumbers("PREROUTING", "nat", comment, "DNAT", targetIP, targetPort) + if len(existing) > 0 { + logger.Debug("IPTABLES: PREROUTING DNAT rule already exists (lines=%v), skipping", existing) + return nil } + logger.Info("IPTABLES: inserting PREROUTING DNAT rule: src=%s proto=%s sport=%s -> dst=%s dport=%s comment=%q", + sourceIP, proto, sourcePort, targetIP, targetPort, comment) args := []string{ "-w", "-t", "nat", "-I", "PREROUTING", "-d", sourceIP, @@ -323,13 +328,18 @@ func (m *Manager) InsertPostroutingMasquerade(sourceCIDR, proto, sourcePort, com // InsertPostroutingMasqueradeForTarget inserts a MASQUERADE POSTROUTING rule for a target func (m *Manager) InsertPostroutingMasqueradeForTarget(targetCIDR, proto, targetPort, comment string) error { - logger.Info("IPTABLES: inserting POSTROUTING MASQUERADE rule for target: dst=%s proto=%s dport=%s comment=%q", + logger.Info("IPTABLES: checking POSTROUTING MASQUERADE rule for target: dst=%s proto=%s dport=%s comment=%q", targetCIDR, proto, targetPort, comment) - patterns := []string{"MASQUERADE", comment, targetCIDR, targetPort} - if err := m.deleteMatchingLines("POSTROUTING", "nat", patterns...); err != nil { - return fmt.Errorf("failed to delete old POSTROUTING rules: %w", err) + + // Idempotent: check if rule already exists + existing := m.getLineNumbers("POSTROUTING", "nat", comment, "MASQUERADE", targetCIDR) + if len(existing) > 0 { + logger.Debug("IPTABLES: POSTROUTING MASQUERADE for target already exists (lines=%v), skipping", existing) + return nil } + logger.Info("IPTABLES: inserting POSTROUTING MASQUERADE rule for target: dst=%s proto=%s dport=%s comment=%q", + targetCIDR, proto, targetPort, comment) args := []string{ "-w", "-t", "nat", "-I", "POSTROUTING", "-d", targetCIDR, @@ -343,10 +353,12 @@ func (m *Manager) InsertPostroutingMasqueradeForTarget(targetCIDR, proto, target // InsertForwardAccept inserts a FORWARD ACCEPT rule on the host func (m *Manager) InsertForwardAccept(chain, sourceIP, targetIP, proto, sourcePort, targetPort, comment string) error { - logger.Info("IPTABLES: inserting FORWARD ACCEPT rule: chain=%s src=%s dst=%s proto=%s sport=%s dport=%s comment=%q", + logger.Info("IPTABLES: checking FORWARD ACCEPT rule: chain=%s src=%s dst=%s proto=%s sport=%s dport=%s comment=%q", chain, sourceIP, targetIP, proto, sourcePort, targetPort, comment) + + // Idempotent: check if rule already exists var grepPatterns []string - grepPatterns = append(grepPatterns, proto) + grepPatterns = append(grepPatterns, comment, proto) if sourceIP != "" { grepPatterns = append(grepPatterns, sourceIP) } @@ -359,11 +371,14 @@ func (m *Manager) InsertForwardAccept(chain, sourceIP, targetIP, proto, sourcePo if targetPort != "" { grepPatterns = append(grepPatterns, targetPort) } - - if err := m.deleteMatchingLines(chain, "", grepPatterns...); err != nil { - return fmt.Errorf("failed to delete old FORWARD rules: %w", err) + existing := m.getLineNumbers(chain, "", grepPatterns...) + if len(existing) > 0 { + logger.Debug("IPTABLES: FORWARD ACCEPT rule already exists in %s (lines=%v), skipping", chain, existing) + return nil } + logger.Info("IPTABLES: inserting FORWARD ACCEPT rule: chain=%s src=%s dst=%s proto=%s sport=%s dport=%s comment=%q", + chain, sourceIP, targetIP, proto, sourcePort, targetPort, comment) args := []string{"-w", "-I", chain, "-p", proto} if sourceIP != "" { args = append(args, "-s", sourceIP) @@ -397,31 +412,17 @@ func (m *Manager) DeleteForwardAccept(chain, comment string) error { return nil } -// lineExistsInContainer checks if a line matching the patterns exists inside a container namespace -func (m *Manager) lineExistsInContainer(pid int, table, chain string, patterns ...string) bool { +// checkContainerChainExists lists rules inside a container namespace and returns +// the output, or an error if nsenter/iptables fails inside the container. +func (m *Manager) checkContainerChainExists(pid int, table, chain string) (string, error) { iptPath := m.binary - nsenterArgs := []string{"-t", fmt.Sprintf("%d", pid), "-n", "--", iptPath, "-w", "-n", "-t", table, "-L", chain} cmd := exec.Command("nsenter", nsenterArgs...) - output, err := cmd.Output() + output, err := cmd.CombinedOutput() if err != nil { - logger.Debug("IPTABLES: lineExistsInContainer list failed for PID %d chain %s: %v", pid, chain, err) - return false + return "", fmt.Errorf("nsenter iptables list failed for PID %d chain %s/%s: %w (output: %s)", pid, table, chain, err, strings.TrimSpace(string(output))) } - - for _, line := range strings.Split(string(output), "\n") { - matchesAll := true - for _, pattern := range patterns { - if !strings.Contains(line, pattern) { - matchesAll = false - break - } - } - if matchesAll { - return true - } - } - return false + return string(output), nil } // InsertPreroutingRuleInContainer inserts a DNAT PREROUTING rule inside a container namespace @@ -429,16 +430,34 @@ func (m *Manager) InsertPreroutingRuleInContainer(pid int, sourceIP, proto, sour logger.Info("IPTABLES: inserting PREROUTING DNAT rule in container PID %d: src=%s proto=%s dport=%s -> %s:%s comment=%q", pid, sourceIP, proto, sourcePort, targetIP, targetPort, comment) - // Idempotent check: if the rule already exists, skip - if m.lineExistsInContainer(pid, "nat", "PREROUTING", "DNAT", sourcePort, targetIP, comment) { - logger.Debug("IPTABLES: PREROUTING DNAT rule already exists in container PID %d (pattern=%s -> %s), skipping", pid, sourcePort, targetIP) + // First, try to list the chain inside the container to check state + output, err := m.checkContainerChainExists(pid, "nat", "PREROUTING") + if err != nil { + // Cannot inspect container iptables — return error so caller falls back to host + logger.Warn("IPTABLES: cannot check container PREROUTING (PID %d): %v", pid, err) + return fmt.Errorf("cannot check container iptables: %w", err) + } + + // Idempotent check: scan existing rules for matching patterns + ruleExists := false + for _, line := range strings.Split(output, "\n") { + if strings.Contains(line, "DNAT") && + strings.Contains(line, sourcePort) && + strings.Contains(line, targetIP) && + strings.Contains(line, comment) { + ruleExists = true + break + } + } + if ruleExists { + logger.Info("IPTABLES: PREROUTING DNAT rule already exists in container PID %d (dport=%s -> %s), skipping", pid, sourcePort, targetIP) return nil } - // Clean up any stale/duplicate rules first + // Rule doesn't exist — clean up stale/duplicate rules then insert patterns := []string{"DNAT", sourcePort, targetIP, targetPort, comment} - if err := m.deleteMatchingLinesInContainer(pid, "nat", "PREROUTING", patterns...); err != nil { - logger.Warn("IPTABLES: failed to delete old container PREROUTING rules: %v (continuing)", err) + if delErr := m.deleteMatchingLinesInContainer(pid, "nat", "PREROUTING", patterns...); delErr != nil { + logger.Debug("IPTABLES: stale PREROUTING cleanup in container PID %d: %v", pid, delErr) } args := []string{ @@ -457,16 +476,32 @@ func (m *Manager) InsertPostroutingMasqueradeInContainer(pid int, sourceCIDR, pr logger.Info("IPTABLES: inserting POSTROUTING MASQUERADE rule in container PID %d: src=%s proto=%s sport=%s comment=%q", pid, sourceCIDR, proto, sourcePort, comment) - // Idempotent check: if the rule already exists, skip - if m.lineExistsInContainer(pid, "nat", "POSTROUTING", "MASQUERADE", comment, sourceCIDR) { - logger.Debug("IPTABLES: POSTROUTING MASQUERADE rule already exists in container PID %d (src=%s comment=%s), skipping", pid, sourceCIDR, comment) + // First, try to list the chain inside the container to check state + output, err := m.checkContainerChainExists(pid, "nat", "POSTROUTING") + if err != nil { + logger.Warn("IPTABLES: cannot check container POSTROUTING (PID %d): %v", pid, err) + return fmt.Errorf("cannot check container iptables: %w", err) + } + + // Idempotent check: scan existing rules for matching patterns + ruleExists := false + for _, line := range strings.Split(output, "\n") { + if strings.Contains(line, "MASQUERADE") && + strings.Contains(line, comment) && + strings.Contains(line, sourceCIDR) { + ruleExists = true + break + } + } + if ruleExists { + logger.Info("IPTABLES: POSTROUTING MASQUERADE rule already exists in container PID %d (src=%s), skipping", pid, sourceCIDR) return nil } - // Clean up any stale/duplicate rules first + // Rule doesn't exist — clean up stale/duplicate rules then insert patterns := []string{"MASQUERADE", comment, sourceCIDR, sourcePort} - if err := m.deleteMatchingLinesInContainer(pid, "nat", "POSTROUTING", patterns...); err != nil { - logger.Warn("IPTABLES: failed to delete old container POSTROUTING rules: %v (continuing)", err) + if delErr := m.deleteMatchingLinesInContainer(pid, "nat", "POSTROUTING", patterns...); delErr != nil { + logger.Debug("IPTABLES: stale POSTROUTING cleanup in container PID %d: %v", pid, delErr) } args := []string{