Create tests for pkg/iptables

Docker-DCO-1.1-Signed-off-by: Ian Bishop <ianbishop@pace7.com> (github: porjo)

daemon/networkdriver/bridge/driver.go

@@ -5,8 +5,6 @@ import (
 	"io/ioutil"
 	"net"
 	"os"
-	"strconv"
-	"strings"
 	"sync"
 
 	log "github.com/Sirupsen/logrus"
@@ -513,10 +511,6 @@ func LinkContainers(job *engine.Job) engine.Status {
 		ports        = job.GetenvList("Ports")
 		chain        = iptables.Chain{}
 	)
-	split := func(p string) (string, string) {
-		parts := strings.Split(p, "/")
-		return parts[0], parts[1]
-	}
 
 	switch action {
 	case "-A":
@@ -541,13 +535,11 @@ func LinkContainers(job *engine.Job) engine.Status {
 	chain.Name = "DOCKER"
 	chain.Bridge = bridgeIface
 	for _, p := range ports {
-		portStr, proto := split(p)
-		port, err := strconv.Atoi(portStr)
-		if !ignoreErrors && err != nil {
-			return job.Errorf("port '%s' is invalid", portStr)
-		}
-		if err := chain.Link(nfAction, ip1, ip2, port, proto); !ignoreErrors && err != nil {
+		port := nat.Port(p)
+		if err := chain.Link(nfAction, ip1, ip2, port.Int(), port.Proto()); !ignoreErrors && err != nil {
+			fmt.Print(err)
 			return job.Error(err)
+
 		}
 	}
 	return engine.StatusOK

daemon/networkdriver/bridge/driver_test.go

@@ -7,6 +7,7 @@ import (
 
 	"github.com/docker/docker/daemon/networkdriver/portmapper"
 	"github.com/docker/docker/engine"
+	"github.com/docker/docker/pkg/iptables"
 )
 
 func init() {
@@ -118,3 +119,43 @@ func TestMacAddrGeneration(t *testing.T) {
 		t.Fatal("Non-unique MAC address")
 	}
 }
+
+func TestLinkContainers(t *testing.T) {
+	eng := engine.New()
+	eng.Logging = false
+
+	// Init driver
+	job := eng.Job("initdriver")
+	if res := InitDriver(job); res != engine.StatusOK {
+		t.Fatal("Failed to initialize network driver")
+	}
+
+	// Allocate interface
+	job = eng.Job("allocate_interface", "container_id")
+	if res := Allocate(job); res != engine.StatusOK {
+		t.Fatal("Failed to allocate network interface")
+	}
+
+	job.Args[0] = "-I"
+
+	job.Setenv("ChildIP", "172.17.0.2")
+	job.Setenv("ParentIP", "172.17.0.1")
+	job.SetenvBool("IgnoreErrors", false)
+	job.SetenvList("Ports", []string{"1234"})
+
+	bridgeIface = "lo"
+	_, err := iptables.NewChain("DOCKER", bridgeIface, iptables.Filter)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if res := LinkContainers(job); res != engine.StatusOK {
+		t.Fatalf("LinkContainers failed")
+	}
+
+	// flush rules
+	if _, err = iptables.Raw([]string{"-F", "DOCKER"}...); err != nil {
+		t.Fatal(err)
+	}
+
+}

integration-cli/docker_cli_links_test.go

@@ -83,8 +83,8 @@ func TestLinksIpTablesRulesWhenLinkAndUnlink(t *testing.T) {
 	childIP := findContainerIP(t, "child")
 	parentIP := findContainerIP(t, "parent")
 
-	sourceRule := []string{"FORWARD", "-i", "docker0", "-o", "docker0", "-p", "tcp", "-s", childIP, "--sport", "80", "-d", parentIP, "-j", "ACCEPT"}
-	destinationRule := []string{"FORWARD", "-i", "docker0", "-o", "docker0", "-p", "tcp", "-s", parentIP, "--dport", "80", "-d", childIP, "-j", "ACCEPT"}
+	sourceRule := []string{"DOCKER", "-i", "docker0", "-o", "docker0", "-p", "tcp", "-s", childIP, "--sport", "80", "-d", parentIP, "-j", "ACCEPT"}
+	destinationRule := []string{"DOCKER", "-i", "docker0", "-o", "docker0", "-p", "tcp", "-s", parentIP, "--dport", "80", "-d", childIP, "-j", "ACCEPT"}
 	if !iptables.Exists(sourceRule...) || !iptables.Exists(destinationRule...) {
 		t.Fatal("Iptables rules not found")
 	}

integration-cli/docker_cli_run_test.go

@@ -2070,7 +2070,7 @@ func TestRunDeallocatePortOnMissingIptablesRule(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	iptCmd := exec.Command("iptables", "-D", "FORWARD", "-d", fmt.Sprintf("%s/32", ip),
+	iptCmd := exec.Command("iptables", "-D", "DOCKER", "-d", fmt.Sprintf("%s/32", ip),
 		"!", "-i", "docker0", "-o", "docker0", "-p", "tcp", "-m", "tcp", "--dport", "23", "-j", "ACCEPT")
 	out, _, err = runCommandWithOutput(iptCmd)
 	if err != nil {

pkg/iptables/iptables.go

@@ -114,7 +114,7 @@ func RemoveExistingChain(name string, table Table) error {
 }
 
 // Add forwarding rule to 'filter' table and corresponding nat rule to 'nat' table
-func (c *Chain) Forward(action Action, ip net.IP, port int, proto, dest_addr string, dest_port int) error {
+func (c *Chain) Forward(action Action, ip net.IP, port int, proto, destAddr string, destPort int) error {
 	daddr := ip.String()
 	if ip.IsUnspecified() {
 		// iptables interprets "0.0.0.0" as "0.0.0.0/32", whereas we
@@ -128,7 +128,7 @@ func (c *Chain) Forward(action Action, ip net.IP, port int, proto, dest_addr str
 		"--dport", strconv.Itoa(port),
 		"!", "-i", c.Bridge,
 		"-j", "DNAT",
-		"--to-destination", net.JoinHostPort(dest_addr, strconv.Itoa(dest_port))); err != nil {
+		"--to-destination", net.JoinHostPort(destAddr, strconv.Itoa(destPort))); err != nil {
 		return err
 	} else if len(output) != 0 {
 		return &ChainError{Chain: "FORWARD", Output: output}
@@ -138,14 +138,25 @@ func (c *Chain) Forward(action Action, ip net.IP, port int, proto, dest_addr str
 		"!", "-i", c.Bridge,
 		"-o", c.Bridge,
 		"-p", proto,
-		"-d", dest_addr,
-		"--dport", strconv.Itoa(dest_port),
+		"-d", destAddr,
+		"--dport", strconv.Itoa(destPort),
 		"-j", "ACCEPT"); err != nil {
 		return err
 	} else if len(output) != 0 {
 		return &ChainError{Chain: "FORWARD", Output: output}
 	}
 
+	if output, err := Raw("-t", string(Nat), string(action), "POSTROUTING",
+		"-p", proto,
+		"-s", destAddr,
+		"-d", destAddr,
+		"--dport", strconv.Itoa(destPort),
+		"-j", "MASQUERADE"); err != nil {
+		return err
+	} else if len(output) != 0 {
+		return &ChainError{Chain: "FORWARD", Output: output}
+	}
+
 	return nil
 }
 
@@ -156,8 +167,8 @@ func (c *Chain) Link(action Action, ip1, ip2 net.IP, port int, proto string) err
 		"-i", c.Bridge, "-o", c.Bridge,
 		"-p", proto,
 		"-s", ip1.String(),
-		"--dport", strconv.Itoa(port),
 		"-d", ip2.String(),
+		"--dport", strconv.Itoa(port),
 		"-j", "ACCEPT"); err != nil {
 		return err
 	} else if len(output) != 0 {
@@ -167,8 +178,8 @@ func (c *Chain) Link(action Action, ip1, ip2 net.IP, port int, proto string) err
 		"-i", c.Bridge, "-o", c.Bridge,
 		"-p", proto,
 		"-s", ip2.String(),
-		"--dport", strconv.Itoa(port),
 		"-d", ip1.String(),
+		"--sport", strconv.Itoa(port),
 		"-j", "ACCEPT"); err != nil {
 		return err
 	} else if len(output) != 0 {
@@ -206,18 +217,17 @@ func (c *Chain) Output(action Action, args ...string) error {
 }
 
 func (c *Chain) Remove() error {
+	// Ignore errors - This could mean the chains were never set up
 	if c.Table == Nat {
-		// Ignore errors - This could mean the chains were never set up
 		c.Prerouting(Delete, "-m", "addrtype", "--dst-type", "LOCAL")
 		c.Output(Delete, "-m", "addrtype", "--dst-type", "LOCAL", "!", "--dst", "127.0.0.0/8")
 		c.Output(Delete, "-m", "addrtype", "--dst-type", "LOCAL") // Created in versions <= 0.1.6
 
 		c.Prerouting(Delete)
 		c.Output(Delete)
-
-		Raw("-t", string(Nat), "-F", c.Name)
-		Raw("-t", string(Nat), "-X", c.Name)
 	}
+	Raw("-t", string(c.Table), "-F", c.Name)
+	Raw("-t", string(c.Table), "-X", c.Name)
 	return nil
 }
 

pkg/iptables/iptables_test.go

@@ -0,0 +1,204 @@
+package iptables
+
+import (
+	"net"
+	"os/exec"
+	"strconv"
+	"strings"
+	"testing"
+)
+
+const chainName = "DOCKERTEST"
+
+var natChain *Chain
+var filterChain *Chain
+
+func TestNewChain(t *testing.T) {
+	var err error
+
+	natChain, err = NewChain(chainName, "lo", Nat)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	filterChain, err = NewChain(chainName, "lo", Filter)
+	if err != nil {
+		t.Fatal(err)
+	}
+}
+
+func TestForward(t *testing.T) {
+	ip := net.ParseIP("192.168.1.1")
+	port := 1234
+	dstAddr := "172.17.0.1"
+	dstPort := 4321
+	proto := "tcp"
+
+	err := natChain.Forward(Insert, ip, port, proto, dstAddr, dstPort)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	dnatRule := []string{natChain.Name,
+		"-t", string(natChain.Table),
+		"!", "-i", filterChain.Bridge,
+		"-d", ip.String(),
+		"-p", proto,
+		"--dport", strconv.Itoa(port),
+		"-j", "DNAT",
+		"--to-destination", dstAddr + ":" + strconv.Itoa(dstPort),
+	}
+
+	if !Exists(dnatRule...) {
+		t.Fatalf("DNAT rule does not exist")
+	}
+
+	filterRule := []string{filterChain.Name,
+		"-t", string(filterChain.Table),
+		"!", "-i", filterChain.Bridge,
+		"-o", filterChain.Bridge,
+		"-d", dstAddr,
+		"-p", proto,
+		"--dport", strconv.Itoa(dstPort),
+		"-j", "ACCEPT",
+	}
+
+	if !Exists(filterRule...) {
+		t.Fatalf("filter rule does not exist")
+	}
+
+	masqRule := []string{"POSTROUTING",
+		"-t", string(natChain.Table),
+		"-d", dstAddr,
+		"-s", dstAddr,
+		"-p", proto,
+		"--dport", strconv.Itoa(dstPort),
+		"-j", "MASQUERADE",
+	}
+
+	if !Exists(masqRule...) {
+		t.Fatalf("MASQUERADE rule does not exist")
+	}
+}
+
+func TestLink(t *testing.T) {
+	var err error
+
+	ip1 := net.ParseIP("192.168.1.1")
+	ip2 := net.ParseIP("192.168.1.2")
+	port := 1234
+	proto := "tcp"
+
+	err = filterChain.Link(Append, ip1, ip2, port, proto)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	rule1 := []string{filterChain.Name,
+		"-t", string(filterChain.Table),
+		"-i", filterChain.Bridge,
+		"-o", filterChain.Bridge,
+		"-p", proto,
+		"-s", ip1.String(),
+		"-d", ip2.String(),
+		"--dport", strconv.Itoa(port),
+		"-j", "ACCEPT"}
+
+	if !Exists(rule1...) {
+		t.Fatalf("rule1 does not exist")
+	}
+
+	rule2 := []string{filterChain.Name,
+		"-t", string(filterChain.Table),
+		"-i", filterChain.Bridge,
+		"-o", filterChain.Bridge,
+		"-p", proto,
+		"-s", ip2.String(),
+		"-d", ip1.String(),
+		"--sport", strconv.Itoa(port),
+		"-j", "ACCEPT"}
+
+	if !Exists(rule2...) {
+		t.Fatalf("rule2 does not exist")
+	}
+}
+
+func TestPrerouting(t *testing.T) {
+	args := []string{
+		"-i", "lo",
+		"-d", "192.168.1.1"}
+
+	err := natChain.Prerouting(Insert, args...)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	rule := []string{"PREROUTING",
+		"-t", string(Nat),
+		"-j", natChain.Name}
+
+	rule = append(rule, args...)
+
+	if !Exists(rule...) {
+		t.Fatalf("rule does not exist")
+	}
+
+	delRule := append([]string{"-D"}, rule...)
+	if _, err = Raw(delRule...); err != nil {
+		t.Fatal(err)
+	}
+}
+
+func TestOutput(t *testing.T) {
+	args := []string{
+		"-o", "lo",
+		"-d", "192.168.1.1"}
+
+	err := natChain.Output(Insert, args...)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	rule := []string{"OUTPUT",
+		"-t", string(natChain.Table),
+		"-j", natChain.Name}
+
+	rule = append(rule, args...)
+
+	if !Exists(rule...) {
+		t.Fatalf("rule does not exist")
+	}
+
+	delRule := append([]string{"-D"}, rule...)
+	if _, err = Raw(delRule...); err != nil {
+		t.Fatal(err)
+	}
+}
+
+func TestCleanup(t *testing.T) {
+	var err error
+	var rules []byte
+
+	// Cleanup filter/FORWARD first otherwise output of iptables-save is dirty
+	link := []string{"-t", string(filterChain.Table),
+		string(Delete), "FORWARD",
+		"-o", filterChain.Bridge,
+		"-j", filterChain.Name}
+	if _, err = Raw(link...); err != nil {
+		t.Fatal(err)
+	}
+	filterChain.Remove()
+
+	err = RemoveExistingChain(chainName, Nat)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	rules, err = exec.Command("iptables-save").Output()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if strings.Contains(string(rules), chainName) {
+		t.Fatalf("Removing chain failed. %s found in iptables-save", chainName)
+	}
+}