firewall: consolidate firewalld code into firewall plugin

pkg/firewalld/firewalld.go

@@ -1,72 +0,0 @@
-// Copyright 2018 CNI authors
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//     http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package firewalld
-
-import (
-	"log"
-	"net"
-	"strings"
-
-	"github.com/godbus/dbus"
-)
-
-const (
-	dbusName               = "org.freedesktop.DBus"
-	dbusPath               = "/org/freedesktop/DBus"
-	dbusGetNameOwnerMethod = ".GetNameOwner"
-
-	FirewalldName            = "org.fedoraproject.FirewallD1"
-	FirewalldPath            = "/org/fedoraproject/FirewallD1"
-	FirewalldZoneInterface   = "org.fedoraproject.FirewallD1.zone"
-	FirewalldAddSourceMethod = ".addSource"
-	FirewalldRemoveSourceMethod = ".removeSource"
-
-	ErrZoneAlreadySet = "ZONE_ALREADY_SET"
-)
-
-// IsRunning checks whether firewalld is running.
-func IsRunning(conn *dbus.Conn) bool {
-	dbusObj := conn.Object(dbusName, dbusPath)
-	var res string
-	if err := dbusObj.Call(dbusName+dbusGetNameOwnerMethod, 0, FirewalldName).Store(&res); err != nil {
-		return false
-	}
-
-	return true
-}
-
-// AddSourceToZone adds a firewalld rule which assigns the given source IP
-// to the given zone.
-func AddSourceToZone(conn *dbus.Conn, source net.IP, zone string) error {
-	firewalldObj := conn.Object(FirewalldName, FirewalldPath)
-	var res string
-	if err := firewalldObj.Call(FirewalldZoneInterface+FirewalldAddSourceMethod, 0, zone, source.String()).Store(&res); err != nil {
-		if strings.Contains(err.Error(), ErrZoneAlreadySet) {
-			log.Printf("ip %v already bound to %q zone, it can mean that this address was assigned before to the another container without cleanup\n", source, zone)
-		} else {
-			return err
-		}
-	}
-
-	return nil
-}
-
-// RemoveSourceFromZone removes firewalld rules which assigned the given source IP
-// to the given zone.
-func RemoveSourceFromZone(conn *dbus.Conn, source net.IP, zone string) error {
-	firewalldObj := conn.Object(FirewalldName, FirewalldPath)
-	var res string
-	return firewalldObj.Call(FirewalldZoneInterface+FirewalldRemoveSourceMethod, 0, zone, source.String()).Store(&res)
-}

plugins/meta/firewall/firewall_firewalld_test.go

@@ -24,7 +24,6 @@ import (
 
 	"github.com/containernetworking/cni/pkg/invoke"
 	"github.com/containernetworking/cni/pkg/skel"
-	"github.com/containernetworking/plugins/pkg/firewalld"
 	"github.com/containernetworking/plugins/pkg/ns"
 	"github.com/containernetworking/plugins/pkg/testutils"
 
@@ -141,7 +140,7 @@ var _ = Describe("firewalld test", func() {
 		Expect(err).NotTo(HaveOccurred())
 
 		// Start our fake firewalld
-		reply, err := conn.RequestName(firewalld.FirewalldName, dbus.NameFlagDoNotQueue)
+		reply, err := conn.RequestName(firewalldName, dbus.NameFlagDoNotQueue)
 		Expect(err).NotTo(HaveOccurred())
 		Expect(reply).To(Equal(dbus.RequestNameReplyPrimaryOwner))
 
@@ -150,17 +149,17 @@ var _ = Describe("firewalld test", func() {
 		// because in Go lower-case methods are private, we need to remap
 		// Go public methods to the D-Bus name
 		methods := map[string]string{
-			"AddSource":    "addSource",
+			"AddSource":    firewalldAddSourceMethod,
-			"RemoveSource": "removeSource",
+			"RemoveSource": firewalldRemoveSourceMethod,
 		}
-		conn.ExportWithMap(fwd, methods, firewalld.FirewalldPath, firewalld.FirewalldZoneInterface)
+		conn.ExportWithMap(fwd, methods, firewalldPath, firewalldZoneInterface)
 
 		// Make sure the plugin uses our private session bus
 		testConn = conn
 	})
 
 	AfterEach(func() {
-		_, err := conn.ReleaseName(firewalld.FirewalldName)
+		_, err := conn.ReleaseName(firewalldName)
 		Expect(err).NotTo(HaveOccurred())
 
 		err = cmd.Process.Signal(syscall.SIGTERM)
@@ -170,7 +169,7 @@ var _ = Describe("firewalld test", func() {
 	})
 
 	It("works with a 0.3.1 config", func() {
-		Expect(firewalld.IsRunning(conn)).To(BeTrue())
+		Expect(isFirewalldRunning()).To(BeTrue())
 
 		conf := fmt.Sprintf(confTmpl, ifname, targetNs.Path())
 		args := &skel.CmdArgs{

plugins/meta/firewall/firewalld.go

@@ -16,12 +16,25 @@ package main
 
 import (
 	"fmt"
-
+	"strings"
-	"github.com/containernetworking/plugins/pkg/firewalld"
 
 	"github.com/godbus/dbus"
 )
 
+const (
+	dbusName               = "org.freedesktop.DBus"
+	dbusPath               = "/org/freedesktop/DBus"
+	dbusGetNameOwnerMethod = "GetNameOwner"
+
+	firewalldName               = "org.fedoraproject.FirewallD1"
+	firewalldPath               = "/org/fedoraproject/FirewallD1"
+	firewalldZoneInterface      = "org.fedoraproject.FirewallD1.zone"
+	firewalldAddSourceMethod    = "addSource"
+	firewalldRemoveSourceMethod = "removeSource"
+
+	errZoneAlreadySet = "ZONE_ALREADY_SET"
+)
+
 // Only used for testcases to override the D-Bus connection
 var testConn *dbus.Conn
 
@@ -39,12 +52,20 @@ func getConn() (*dbus.Conn, error) {
 	return dbus.SystemBus()
 }
 
+// isFirewalldRunning checks whether firewalld is running.
 func isFirewalldRunning() bool {
 	conn, err := getConn()
 	if err != nil {
 		return false
 	}
-	return firewalld.IsRunning(conn)
+
+	dbusObj := conn.Object(dbusName, dbusPath)
+	var res string
+	if err := dbusObj.Call(dbusName+"."+dbusGetNameOwnerMethod, 0, firewalldName).Store(&res); err != nil {
+		return false
+	}
+
+	return true
 }
 
 func newFirewalldBackend(conf *FirewallNetConf) (FirewallBackend, error) {
@@ -61,8 +82,14 @@ func newFirewalldBackend(conf *FirewallNetConf) (FirewallBackend, error) {
 
 func (fb *fwdBackend) Add(conf *FirewallNetConf) error {
 	for _, ip := range conf.PrevResult.IPs {
-		if err := firewalld.AddSourceToZone(fb.conn, ip.Address.IP, conf.FirewalldZone); err != nil {
+		ipStr := ipString(ip.Address)
-			return fmt.Errorf("failed to add the address %v to %v zone: %v", ip.Address.IP, conf.FirewalldZone, err)
+		// Add a firewalld rule which assigns the given source IP to the given zone
+		firewalldObj := fb.conn.Object(firewalldName, firewalldPath)
+		var res string
+		if err := firewalldObj.Call(firewalldZoneInterface+"."+firewalldAddSourceMethod, 0, conf.FirewalldZone, ipStr).Store(&res); err != nil {
+			if !strings.Contains(err.Error(), errZoneAlreadySet) {
+				return fmt.Errorf("failed to add the address %v to %v zone: %v", ipStr, conf.FirewalldZone, err)
+			}
 		}
 	}
 	return nil
@@ -70,7 +97,11 @@ func (fb *fwdBackend) Add(conf *FirewallNetConf) error {
 
 func (fb *fwdBackend) Del(conf *FirewallNetConf) error {
 	for _, ip := range conf.PrevResult.IPs {
-		firewalld.RemoveSourceFromZone(fb.conn, ip.Address.IP, conf.FirewalldZone)
+		ipStr := ipString(ip.Address)
+		// Remove firewalld rules which assigned the given source IP to the given zone
+		firewalldObj := fb.conn.Object(firewalldName, firewalldPath)
+		var res string
+		firewalldObj.Call(firewalldZoneInterface+"."+firewalldRemoveSourceMethod, 0, conf.FirewalldZone, ipStr).Store(&res)
 	}
 	return nil
 }