From fc7059c1aebee235ab5f9791a3cb302c1fa469a2 Mon Sep 17 00:00:00 2001
From: Erik Sipsma <sipsma@amazon.com>
Date: Mon, 23 Sep 2019 20:00:23 +0000
Subject: [PATCH 1/2] firewall: don't return error in DEL if prevResult is not
 found.

The CNI spec states that for DEL implementations, "when CNI_NETNS and/or
prevResult are not provided, the plugin should clean up as many resources as
possible (e.g. releasing IPAM allocations) and return a successful response".
This change results in the firewall plugin conforming to the spec by not
returning an error whenever the del method is not provided a prevResult.

Signed-off-by: Erik Sipsma <sipsma@amazon.com>
---
 plugins/meta/firewall/firewall.go | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/plugins/meta/firewall/firewall.go b/plugins/meta/firewall/firewall.go
index c2eef93a..c348d6fa 100644
--- a/plugins/meta/firewall/firewall.go
+++ b/plugins/meta/firewall/firewall.go
@@ -68,9 +68,15 @@ func parseConf(data []byte) (*FirewallNetConf, *current.Result, error) {
 		return nil, nil, fmt.Errorf("failed to load netconf: %v", err)
 	}
 
+	// Default the firewalld zone to trusted
+	if conf.FirewalldZone == "" {
+		conf.FirewalldZone = "trusted"
+	}
+
 	// Parse previous result.
 	if conf.RawPrevResult == nil {
-		return nil, nil, fmt.Errorf("missing prevResult from earlier plugin")
+		// return early if there was no previous result, which is allowed for DEL calls
+		return &conf, &current.Result{}, nil
 	}
 
 	// Parse previous result.
@@ -85,11 +91,6 @@ func parseConf(data []byte) (*FirewallNetConf, *current.Result, error) {
 		return nil, 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, result, nil
 }
 
@@ -116,6 +117,10 @@ func cmdAdd(args *skel.CmdArgs) error {
 		return err
 	}
 
+	if conf.PrevResult == nil {
+		return fmt.Errorf("missing prevResult from earlier plugin")
+	}
+
 	backend, err := getBackend(conf)
 	if err != nil {
 		return err
@@ -167,8 +172,8 @@ func cmdCheck(args *skel.CmdArgs) error {
 	}
 
 	// Ensure we have previous result.
-	if result == nil {
-		return fmt.Errorf("Required prevResult missing")
+	if conf.PrevResult == nil {
+		return fmt.Errorf("missing prevResult from earlier plugin")
 	}
 
 	backend, err := getBackend(conf)

From 0a1421a08cc909aa67360f749e90bab60206a01f Mon Sep 17 00:00:00 2001
From: Erik Sipsma <sipsma@amazon.com>
Date: Wed, 25 Sep 2019 20:38:02 +0000
Subject: [PATCH 2/2] firewall: remove unused netns check from DEL method

Signed-off-by: Erik Sipsma <sipsma@amazon.com>
---
 plugins/meta/firewall/firewall.go | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/plugins/meta/firewall/firewall.go b/plugins/meta/firewall/firewall.go
index c348d6fa..875943be 100644
--- a/plugins/meta/firewall/firewall.go
+++ b/plugins/meta/firewall/firewall.go
@@ -27,7 +27,6 @@ import (
 	"github.com/containernetworking/cni/pkg/types/current"
 	"github.com/containernetworking/cni/pkg/version"
 
-	"github.com/containernetworking/plugins/pkg/ns"
 	bv "github.com/containernetworking/plugins/pkg/utils/buildversion"
 )
 
@@ -147,12 +146,6 @@ func cmdDel(args *skel.CmdArgs) error {
 		return err
 	}
 
-	// Tolerate errors if the container namespace has been torn down already
-	containerNS, err := ns.GetNS(args.Netns)
-	if err == nil {
-		defer containerNS.Close()
-	}
-
 	// Runtime errors are ignored
 	if err := backend.Del(conf, result); err != nil {
 		return err