Add Check support to firewall meta plugin, test cases

README.md

@@ -28,6 +28,7 @@ Read [CONTRIBUTING](CONTRIBUTING.md) for build and test instructions.
 * `portmap`: An iptables-based portmapping plugin. Maps ports from the host's address space to the container.
 * `bandwidth`: Allows bandwidth-limiting through use of traffic control tbf (ingress/egress).
 * `sbr`: A plugin that configures source based routing for an interface (from which it is chained).
+* `firewall`: A firewall plugin which uses iptables or firewalld to add rules to allow traffic to/from the container.
 
 ### Sample
 The sample plugin provides an example for building your own plugin.

plugins/meta/firewall/README.md

@@ -4,7 +4,7 @@
 
 This plugin creates firewall rules to allow traffic to/from container IP address via the host network .
 It does not create any network interfaces and therefore does not set up connectivity by itself.
-It is only useful when used in addition to other plugins.
+It is intended to be used as a chained plugins.
 
 ## Operation
 The following network configuration file
@@ -45,7 +45,91 @@ Available backends include `iptables` and `firewalld` and may be selected with t
 If no `backend` key is given, the plugin will use firewalld if the service exists on the D-Bus system bus.
 If no firewalld service is found, it will fall back to iptables.
 
+## firewalld backend rule structure
+When the `firewalld` backend is used, this example will place the IPAM allocated address for the container (e.g. 10.88.0.2) into firewalld's `trusted` zone, allowing it to send/receive traffic.
+
+
+A sample standalone config list (with the file extension .conflist) using firewalld backend might
+look like:
+
+```json
+{
+    "cniVersion": "0.3.1",
+    "name": "bridge-firewalld",
+    "plugins": [
+      {
+        "type": "bridge",
+        "bridge": "cni0",
+        "isGateway": true,
+        "ipMasq": true,
+        "ipam": {
+            "type": "host-local",
+            "subnet": "10.88.0.0/16",
+            "routes": [
+                { "dst": "0.0.0.0/0" }
+            ]
+        }
+      },
+      {
+        "type": "firewall",
+	"backend": "firewalld"
+      }
+    ]
+}
+```
+
+
+`FORWARD_IN_ZONES_SOURCE` chain:
+- `-d 10.88.0.2 -j FWDI_trusted`
+
+`CNI_FORWARD_OUT_ZONES_SOURCE` chain:
+- `-s 10.88.0.2 -j FWDO_trusted`
+
+
+## iptables backend rule structure
+
+A sample standalone config list (with the file extension .conflist) using iptables backend might
+look like:
+
+```json
+{
+    "cniVersion": "0.3.1",
+    "name": "bridge-firewalld",
+    "plugins": [
+      {
+        "type": "bridge",
+        "bridge": "cni0",
+        "isGateway": true,
+        "ipMasq": true,
+        "ipam": {
+            "type": "host-local",
+            "subnet": "10.88.0.0/16",
+            "routes": [
+                { "dst": "0.0.0.0/0" }
+            ]
+        }
+      },
+      {
+        "type": "firewall",
+	"backend": "iptables"
+      }
+    ]
+}
+```
+
 When the `iptables` backend is used, the above example will create two new iptables chains in the `filter` table and add rules that allow the given interface to send/receive traffic.
-When the `firewalld` backend is used, the above example will place the `cni0` interface into firewalld's `trusted` zone, allowing it to send/receive traffic.
 
+### FORWARD
+A new chain, CNI-FORWARD is added to the FORWARD chain.  CNI-FORWARD is the chain where rules will be added
+when containers are created and from where rules will be removed when containers terminate.
+
+`FORWARD` chain:
+- `-j CNI-FORWARD`
+
+CNI-FORWARD will have a pair of rules added, one for each direction, using the IPAM assigned IP address
+of the container as shown:
+
+`CNI_FORWARD` chain:
+- `-s 10.88.0.2 -m conntrack --ctstate RELATED,ESTABLISHED -j CNI-FORWARD`
+- `-d 10.88.0.2 -j CNI-FORWARD`
 

plugins/meta/firewall/firewall.go

@@ -45,14 +45,12 @@ type FirewallNetConf struct {
 	// the firewalld backend is used but the zone is not given, it defaults
 	// to 'trusted'
 	FirewalldZone string `json:"firewalldZone,omitempty"`
-
-	RawPrevResult map[string]interface{} `json:"prevResult,omitempty"`
-	PrevResult    *current.Result        `json:"-"`
 }
 
 type FirewallBackend interface {
-	Add(*FirewallNetConf) error
-	Del(*FirewallNetConf) error
+	Add(*FirewallNetConf, *current.Result) error
+	Del(*FirewallNetConf, *current.Result) error
+	Check(*FirewallNetConf, *current.Result) error
 }
 
 func ipString(ip net.IPNet) string {
@@ -62,37 +60,35 @@ func ipString(ip net.IPNet) string {
 	return ip.IP.String() + "/32"
 }
 
-func parseConf(data []byte) (*FirewallNetConf, error) {
+func parseConf(data []byte) (*FirewallNetConf, *current.Result, error) {
 	conf := FirewallNetConf{}
 	if err := json.Unmarshal(data, &conf); err != nil {
-		return nil, fmt.Errorf("failed to load netconf: %v", err)
-	}
-
-	// Default the firewalld zone to trusted
-	if conf.FirewalldZone == "" {
-		conf.FirewalldZone = "trusted"
+		return nil, nil, fmt.Errorf("failed to load netconf: %v", err)
 	}
 
 	// Parse previous result.
 	if conf.RawPrevResult == nil {
-		return nil, fmt.Errorf("missing prevResult from earlier plugin")
+		return nil, nil, fmt.Errorf("missing prevResult from earlier plugin")
 	}
 
-	resultBytes, err := json.Marshal(conf.RawPrevResult)
-	if err != nil {
-		return nil, fmt.Errorf("could not serialize prevResult: %v", err)
+	// Parse previous result.
+	var result *current.Result
+	var err error
+	if err = version.ParsePrevResult(&conf.NetConf); err != nil {
+		return nil, nil, fmt.Errorf("could not parse prevResult: %v", err)
 	}
-	res, err := version.NewResult(conf.CNIVersion, resultBytes)
+
+	result, err = current.NewResultFromResult(conf.PrevResult)
 	if err != nil {
-		return nil, fmt.Errorf("could not parse prevResult: %v", err)
+		return nil, nil, fmt.Errorf("could not convert result to current version: %v", err)
 	}
-	conf.RawPrevResult = nil
-	conf.PrevResult, err = current.NewResultFromResult(res)
-	if err != nil {
-		return nil, fmt.Errorf("could not convert result to current version: %v", err)
+
+	// Default the firewalld zone to trusted
+	if conf.FirewalldZone == "" {
+		conf.FirewalldZone = "trusted"
 	}
 
-	return &conf, nil
+	return &conf, result, nil
 }
 
 func getBackend(conf *FirewallNetConf) (FirewallBackend, error) {
@@ -113,7 +109,7 @@ func getBackend(conf *FirewallNetConf) (FirewallBackend, error) {
 }
 
 func cmdAdd(args *skel.CmdArgs) error {
-	conf, err := parseConf(args.StdinData)
+	conf, result, err := parseConf(args.StdinData)
 	if err != nil {
 		return err
 	}
@@ -123,19 +119,18 @@ func cmdAdd(args *skel.CmdArgs) error {
 		return err
 	}
 
-	if err := backend.Add(conf); err != nil {
+	if err := backend.Add(conf, result); err != nil {
 		return err
 	}
 
-	result := conf.PrevResult
 	if result == nil {
 		result = &current.Result{}
 	}
 	return types.PrintResult(result, conf.CNIVersion)
 }
 
 func cmdDel(args *skel.CmdArgs) error {
-	conf, err := parseConf(args.StdinData)
+	conf, result, err := parseConf(args.StdinData)
 	if err != nil {
 		return err
 	}
@@ -152,13 +147,36 @@ func cmdDel(args *skel.CmdArgs) error {
 	}
 
 	// Runtime errors are ignored
-	if err := backend.Del(conf); err != nil {
+	if err := backend.Del(conf, result); err != nil {
 		return err
 	}
 
 	return nil
 }
 
 func main() {
-	skel.PluginMain(cmdAdd, cmdDel, version.All)
+	skel.PluginMain(cmdAdd, cmdCheck, cmdDel, version.PluginSupports("0.4.0"), "TODO")
+}
+
+func cmdCheck(args *skel.CmdArgs) error {
+	conf, result, err := parseConf(args.StdinData)
+	if err != nil {
+		return err
+	}
+
+	// Ensure we have previous result.
+	if result == nil {
+		return fmt.Errorf("Required prevResult missing")
+	}
+
+	backend, err := getBackend(conf)
+	if err != nil {
+		return err
+	}
+
+	if err := backend.Check(conf, result); err != nil {
+		return err
+	}
+
+	return nil
 }

plugins/meta/firewall/firewall_firewalld_test.go

@@ -81,6 +81,16 @@ func (f *fakeFirewalld) RemoveSource(zone, source string) (string, *dbus.Error)
 	return "", nil
 }
 
+func (f *fakeFirewalld) QuerySource(zone, source string) (bool, *dbus.Error) {
+	if f.zone != zone {
+		return false, nil
+	}
+	if f.source != source {
+		return false, nil
+	}
+	return true, nil
+}
+
 func spawnSessionDbus(wg *sync.WaitGroup) (string, *exec.Cmd) {
 	// Start a private D-Bus session bus
 	path, err := invoke.FindInPath("dbus-daemon", []string{
@@ -150,6 +160,7 @@ var _ = Describe("firewalld test", func() {
 		// Go public methods to the D-Bus name
 		methods := map[string]string{
 			"AddSource":    firewalldAddSourceMethod,
+			"QuerySource":  firewalldQuerySourceMethod,
 			"RemoveSource": firewalldRemoveSourceMethod,
 		}
 		conn.ExportWithMap(fwd, methods, firewalldPath, firewalldZoneInterface)
@@ -178,15 +189,15 @@ var _ = Describe("firewalld test", func() {
 			IfName:      ifname,
 			StdinData:   []byte(conf),
 		}
-		_, _, err := testutils.CmdAddWithResult(targetNs.Path(), ifname, []byte(conf), func() error {
+		_, _, err := testutils.CmdAdd(targetNs.Path(), args.ContainerID, ifname, []byte(conf), func() error {
 			return cmdAdd(args)
 		})
 		Expect(err).NotTo(HaveOccurred())
 		Expect(fwd.zone).To(Equal("trusted"))
 		Expect(fwd.source).To(Equal("10.0.0.2/32"))
 		fwd.clear()
 
-		err = testutils.CmdDelWithResult(targetNs.Path(), ifname, func() error {
+		err = testutils.CmdDel(targetNs.Path(), args.ContainerID, ifname, func() error {
 			return cmdDel(args)
 		})
 		Expect(err).NotTo(HaveOccurred())
@@ -224,7 +235,7 @@ var _ = Describe("firewalld test", func() {
 			IfName:      ifname,
 			StdinData:   []byte(conf),
 		}
-		_, _, err := testutils.CmdAddWithResult(targetNs.Path(), ifname, []byte(conf), func() error {
+		_, _, err := testutils.CmdAdd(targetNs.Path(), args.ContainerID, ifname, []byte(conf), func() error {
 			return cmdAdd(args)
 		})
 		Expect(err).NotTo(HaveOccurred())
@@ -262,7 +273,7 @@ var _ = Describe("firewalld test", func() {
 			IfName:      ifname,
 			StdinData:   []byte(conf),
 		}
-		r, _, err := testutils.CmdAddWithResult(targetNs.Path(), ifname, []byte(conf), func() error {
+		r, _, err := testutils.CmdAdd(targetNs.Path(), args.ContainerID, ifname, []byte(conf), func() error {
 			return cmdAdd(args)
 		})
 		Expect(err).NotTo(HaveOccurred())
@@ -275,4 +286,58 @@ var _ = Describe("firewalld test", func() {
 		Expect(len(result.IPs)).To(Equal(1))
 		Expect(result.IPs[0].Address.String()).To(Equal("10.0.0.2/24"))
 	})
+
+	It("works with a 0.4.0 config, including Check", func() {
+		Expect(isFirewalldRunning()).To(BeTrue())
+
+		conf := `{
+			  "cniVersion": "0.4.0",
+			  "name": "firewalld-test",
+			  "type": "firewall",
+			  "backend": "firewalld",
+			  "zone": "trusted",
+			  "prevResult": {
+			    "cniVersion": "0.4.0",
+			    "interfaces": [
+			      {"name": "eth0", "sandbox": "/foobar"}
+			    ],
+			    "ips": [
+			      {
+				"version": "4",
+				"address": "10.0.0.2/24",
+				"gateway": "10.0.0.1",
+				"interface": 0
+			      }
+			    ]
+			  }
+			}`
+
+		args := &skel.CmdArgs{
+			ContainerID: "dummy",
+			Netns:       targetNs.Path(),
+			IfName:      ifname,
+			StdinData:   []byte(conf),
+		}
+		r, _, err := testutils.CmdAddWithArgs(args, func() error {
+			return cmdAdd(args)
+		})
+		Expect(err).NotTo(HaveOccurred())
+		Expect(fwd.zone).To(Equal("trusted"))
+		Expect(fwd.source).To(Equal("10.0.0.2/32"))
+
+		_, err = current.GetResult(r)
+		Expect(err).NotTo(HaveOccurred())
+
+		err = testutils.CmdCheckWithArgs(args, func() error {
+			return cmdCheck(args)
+		})
+		Expect(err).NotTo(HaveOccurred())
+
+		err = testutils.CmdDelWithArgs(args, func() error {
+			return cmdDel(args)
+		})
+		Expect(err).NotTo(HaveOccurred())
+		Expect(fwd.zone).To(Equal("trusted"))
+		Expect(fwd.source).To(Equal("10.0.0.2/32"))
+	})
 })