feat(docker, firewall): Add stateful network connection check and optimize NAT rules
continuous-integration/drone/push Build is passing
continuous-integration/drone/push Build is passing
This adds an IsConnected method to verify if a container is already connected to a network with the expected IP, preventing redundant operations. In reconcileIPs, it skips reconnections if the state is correct. In applyNATRule, MASQUERADE is now applied in the same namespace as DNAT (container or host) for consistent and accurate rule application.
This commit is contained in:
@@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
// 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
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user