diff --git a/network-go/docker/docker.go b/network-go/docker/docker.go index 7824ce2..94d698c 100644 --- a/network-go/docker/docker.go +++ b/network-go/docker/docker.go @@ -31,6 +31,7 @@ type DockerAPI interface { GetContainerPID(ctx context.Context, containerName string) (int, error) AddRouteInContainer(ctx context.Context, containerName, network, gateway string) error FindContainerName(ctx context.Context, name, selector string) (string, error) + IsConnected(ctx context.Context, containerName, networkName, expectedIP string) bool } // Client wraps the Docker SDK client @@ -308,4 +309,42 @@ func (c *Client) FindContainerName(ctx context.Context, name, selector string) ( } return "", fmt.Errorf("no running container found matching name=%q selector=%q", name, selector) -} \ No newline at end of file +} + +// IsConnected checks if a container is already connected to a network with the expected IP. +// Returns true if the connection already exists with the correct IP (stateful check). +func (c *Client) IsConnected(ctx context.Context, containerName, networkName, expectedIP string) bool { + logger.Debug("DOCKER: checking if container %q is connected to %q with IP %s", containerName, networkName, expectedIP) + + containers, err := c.cli.ContainerList(ctx, container.ListOptions{ + Filters: filters.NewArgs( + filters.Arg("name", "^/?"+regexp.QuoteMeta(containerName)+"$"), + filters.Arg("status", "running"), + ), + }) + if err != nil || len(containers) == 0 { + logger.Debug("DOCKER: container %q not found or error: %v", containerName, err) + return false + } + + // Check the network settings for the expected IP + inspect, err := c.cli.ContainerInspect(ctx, containerName) + if err != nil { + logger.Debug("DOCKER: inspect failed for %q: %v", containerName, err) + return false + } + + if inspect.NetworkSettings != nil && inspect.NetworkSettings.Networks != nil { + if netSettings, ok := inspect.NetworkSettings.Networks[networkName]; ok { + currentIP := netSettings.IPAddress + logger.Debug("DOCKER: container %q on network %q has IP=%s (expected=%s)", containerName, networkName, currentIP, expectedIP) + if currentIP == expectedIP { + logger.Debug("DOCKER: container %q already correctly connected to %q with IP %s", containerName, networkName, expectedIP) + return true + } + } + } + + logger.Debug("DOCKER: container %q not yet correctly connected to %q", containerName, networkName) + return false +} diff --git a/network-go/firewall/firewall.go b/network-go/firewall/firewall.go index 7df73bc..dddda85 100644 --- a/network-go/firewall/firewall.go +++ b/network-go/firewall/firewall.go @@ -98,6 +98,13 @@ func (o *Orchestrator) reconcileIPs(ctx context.Context, cfg *config.NetworksCon ipCfg.ContainerName, containerName) } + // Stateful check: verify container already has the correct IP on this network + if o.dockerClient.IsConnected(ctx, containerName, networkName, ipStr) { + logger.Debug("FIREWALL: container %s already connected to %s with IP %s, skipping", + containerName, networkName, ipStr) + continue + } + logger.Info("FIREWALL: connecting container %s to network %s with IP %s", containerName, networkName, ipStr) @@ -228,6 +235,7 @@ func (o *Orchestrator) applyNATRule(ctx context.Context, cfg *config.NetworksCon // Try to insert rules inside the container namespace via nsenter usedContainer := false + var containerPID int if selector != "" { pid, err := o.dockerClient.GetContainerPID(ctx, selector) if err == nil { @@ -238,6 +246,7 @@ func (o *Orchestrator) applyNATRule(ctx context.Context, cfg *config.NetworksCon logger.Info("FIREWALL: DNAT rule inserted in container %s: target=%s proto=%s port=%s", selector, targetIP, proto, port) usedContainer = true + containerPID = pid } } else { logger.Warn("FIREWALL: cannot get PID for container %s: %v, trying host rules", selector, err) @@ -257,23 +266,34 @@ func (o *Orchestrator) applyNATRule(ctx context.Context, cfg *config.NetworksCon // Always add MASQUERADE on POSTROUTING so return traffic from the // DNAT target can route back through the same interface. - // This mirrors the old shell script behavior where POSTROUTING - // was always set alongside PREROUTING DNAT rules. - // Required regardless of whether DNAT was in container namespace or host. + // If DNAT was in a container namespace, apply POSTROUTING in the same namespace. + // If DNAT was on the host, apply POSTROUTING on the host. if targetIP != "" { masqComment := comment + "-masq" targetSubnet := "" - // Use the target's /24 subnet as the source CIDR for masquerade if strings.Contains(targetIP, ".") { targetSubnet = targetIP[:strings.LastIndex(targetIP, ".")] + ".0/24" } if targetSubnet != "" { logger.Info("FIREWALL: inserting POSTROUTING MASQUERADE for %s", targetSubnet) - if err := o.iptablesMgr.InsertPostroutingMasquerade(targetSubnet, proto, port, masqComment); err != nil { - logger.Error("FIREWALL: failed to insert POSTROUTING MASQUERADE: %v", err) + if usedContainer && containerPID > 0 { + // Apply in container namespace alongside the DNAT rule + logger.Info("FIREWALL: POSTROUTING MASQUERADE in container PID %d", containerPID) + if err := o.iptablesMgr.InsertPostroutingMasqueradeInContainer(containerPID, targetSubnet, proto, port, masqComment); err != nil { + logger.Error("FIREWALL: failed to insert container POSTROUTING MASQUERADE: %v", err) + } else { + logger.Info("FIREWALL: container POSTROUTING MASQUERADE inserted: subnet=%s proto=%s port=%s", + targetSubnet, proto, port) + } } else { - logger.Info("FIREWALL: POSTROUTING MASQUERADE inserted: subnet=%s proto=%s port=%s", - targetSubnet, proto, port) + // Apply on host POSTROUTING + logger.Info("FIREWALL: POSTROUTING MASQUERADE on host") + if err := o.iptablesMgr.InsertPostroutingMasquerade(targetSubnet, proto, port, masqComment); err != nil { + logger.Error("FIREWALL: failed to insert host POSTROUTING MASQUERADE: %v", err) + } else { + logger.Info("FIREWALL: host POSTROUTING MASQUERADE inserted: subnet=%s proto=%s port=%s", + targetSubnet, proto, port) + } } } } diff --git a/network-go/iptables/iptables.go b/network-go/iptables/iptables.go index 7866efb..86f9896 100644 --- a/network-go/iptables/iptables.go +++ b/network-go/iptables/iptables.go @@ -273,6 +273,16 @@ func (m *Manager) InsertPreroutingRule(sourceIP, proto, sourcePort, targetIP, ta // InsertPreroutingRuleOnInterface inserts a DNAT PREROUTING rule on a specific interface func (m *Manager) InsertPreroutingRuleOnInterface(iface, proto, sourcePort, targetIP, targetPort, comment string) error { + logger.Info("IPTABLES: checking PREROUTING DNAT rule on interface %s: proto=%s dport=%s -> %s:%s comment=%q", + iface, proto, sourcePort, targetIP, targetPort, comment) + + // Check if rule already exists (idempotent: don't re-apply) + existing := m.getLineNumbers("PREROUTING", "nat", comment, "DNAT", targetIP) + if len(existing) > 0 { + logger.Debug("IPTABLES: PREROUTING DNAT rule already exists on %s (lines=%v), skipping", iface, existing) + return nil + } + logger.Info("IPTABLES: inserting PREROUTING DNAT rule on interface %s: proto=%s dport=%s -> %s:%s comment=%q", iface, proto, sourcePort, targetIP, targetPort, comment) args := []string{ @@ -288,13 +298,18 @@ func (m *Manager) InsertPreroutingRuleOnInterface(iface, proto, sourcePort, targ // InsertPostroutingMasquerade inserts a MASQUERADE POSTROUTING rule on the host func (m *Manager) InsertPostroutingMasquerade(sourceCIDR, proto, sourcePort, comment string) error { - logger.Info("IPTABLES: inserting POSTROUTING MASQUERADE rule: src=%s proto=%s sport=%s comment=%q", + logger.Info("IPTABLES: checking POSTROUTING MASQUERADE rule: src=%s proto=%s sport=%s comment=%q", sourceCIDR, proto, sourcePort, comment) - patterns := []string{"MASQUERADE", comment, sourceCIDR, sourcePort} - if err := m.deleteMatchingLines("POSTROUTING", "nat", patterns...); err != nil { - return fmt.Errorf("failed to delete old POSTROUTING rules: %w", err) + + // Check if rule already exists (idempotent: don't re-apply) + existing := m.getLineNumbers("POSTROUTING", "nat", comment, "MASQUERADE", sourceCIDR) + if len(existing) > 0 { + logger.Debug("IPTABLES: POSTROUTING MASQUERADE rule already exists (lines=%v), skipping", existing) + return nil } + logger.Info("IPTABLES: inserting POSTROUTING MASQUERADE rule: src=%s proto=%s sport=%s comment=%q", + sourceCIDR, proto, sourcePort, comment) args := []string{ "-w", "-t", "nat", "-I", "POSTROUTING", "-s", sourceCIDR, diff --git a/network-go/mock/mock.go b/network-go/mock/mock.go index ea05bae..ac3b414 100644 --- a/network-go/mock/mock.go +++ b/network-go/mock/mock.go @@ -48,6 +48,9 @@ type MockDockerClient struct { InspectContainerErr error RemoveNetworkErr error DisconnectContainerErr error + + IsConnectedCalled bool + IsConnectedResult bool } func (m *MockDockerClient) Close() error { return nil } @@ -106,6 +109,11 @@ func (m *MockDockerClient) FindContainerName(ctx context.Context, name, selector return name, m.FindContainerNameErr } +func (m *MockDockerClient) IsConnected(ctx context.Context, containerName, networkName, expectedIP string) bool { + m.IsConnectedCalled = true + return m.IsConnectedResult +} + // MockIPTablesManager implements iptables.IPTablesAPI for testing type MockIPTablesManager struct { BinaryResult string