Add additional unit-test for HandleCreateOptions

Signed-off-by: Swagat Bora <[email protected]>
runfinch · Nov 12, 2024 · ba979e7 · ba979e7
1 parent b9da503
commit ba979e7
Show file tree

Hide file tree

Showing 3 changed files with 146 additions and 6 deletions.
diff --git a/internal/service/network/driver/bridge.go b/internal/service/network/driver/bridge.go
@@ -58,12 +58,10 @@ func (bd *bridgeDriver) HandleCreateOptions(request types.NetworkCreateRequest,
 			case BridgeICCOption:
 				iccOption, err := strconv.ParseBool(v)
 				if err != nil {
-					bd.logger.Warnf("invalid value for com.docker.network.bridge.enable_icc: %s", v)
+					bd.logger.Warnf("invalid value for com.docker.network.bridge.enable_icc")
+					continue
 				}
-				if !iccOption {
-					bd.disableICC = true
-				}
-
+				bd.disableICC = !iccOption
 			case BridgeNameOption:
 				bd.bridgeName = v
 			default:

diff --git a/internal/service/network/driver/bridge_test.go b/internal/service/network/driver/bridge_test.go
@@ -6,14 +6,156 @@ package driver
 import (
 	"fmt"
 
+	"github.com/containerd/nerdctl/pkg/netutil"
 	"github.com/coreos/go-iptables/iptables"
 	"github.com/golang/mock/gomock"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
+	"github.com/runfinch/finch-daemon/api/types"
 	"github.com/runfinch/finch-daemon/mocks/mocks_logger"
 	"github.com/runfinch/finch-daemon/mocks/mocks_network"
 )
 
+var _ = Describe("bridgeDriver HandleCreateOptions", func() {
+	var (
+		mockController *gomock.Controller
+		logger         *mocks_logger.Logger
+		driver         *bridgeDriver
+		request        types.NetworkCreateRequest
+		options        netutil.CreateOptions
+	)
+
+	BeforeEach(func() {
+		mockController = gomock.NewController(GinkgoT())
+		logger = mocks_logger.NewLogger(mockController)
+		driver = &bridgeDriver{logger: logger}
+		request = types.NetworkCreateRequest{
+			Options: make(map[string]string),
+		}
+		options = netutil.CreateOptions{
+			Options: make(map[string]string),
+			Labels:  []string{},
+		}
+	})
+
+	Context("when processing bridge options", func() {
+		It("should handle empty options", func() {
+			result, err := driver.HandleCreateOptions(request, options)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(result.Options).To(BeEmpty())
+			Expect(result.Labels).To(BeEmpty())
+		})
+
+		It("should process valid host binding IPv4", func() {
+			request.Options = map[string]string{
+				BridgeHostBindingIpv4Option: "0.0.0.0",
+			}
+			logger.EXPECT().Warnf(gomock.Any(), gomock.Any()).Times(0)
+			result, err := driver.HandleCreateOptions(request, options)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(result.Options).To(BeEmpty())
+		})
+
+		It("should warn on invalid host binding IPv4", func() {
+			request.Options = map[string]string{
+				BridgeHostBindingIpv4Option: "192.168.1.1",
+			}
+			logger.EXPECT().Warnf("network option com.docker.network.bridge.host_binding_ipv4 is set to %s, but it must be 0.0.0.0", "192.168.1.1")
+			result, err := driver.HandleCreateOptions(request, options)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(result.Options).To(BeEmpty())
+		})
+
+		It("should handle valid ICC option true", func() {
+			trueValues := []string{"1", "t", "T", "TRUE", "true", "True"}
+			for _, v := range trueValues {
+				request.Options = map[string]string{
+					BridgeICCOption: v,
+				}
+				result, err := driver.HandleCreateOptions(request, options)
+				Expect(err).NotTo(HaveOccurred())
+				Expect(result.Options).To(BeEmpty())
+				Expect(driver.disableICC).To(BeFalse(), "Failed for value: "+v)
+				Expect(result.Labels).To(BeEmpty(), "Failed for value: "+v)
+			}
+		})
+
+		It("should handle valid ICC option false", func() {
+			falseValues := []string{"0", "f", "F", "FALSE", "false", "False"}
+			for _, v := range falseValues {
+				request.Options = map[string]string{
+					BridgeICCOption: v,
+				}
+				result, err := driver.HandleCreateOptions(request, options)
+				Expect(err).NotTo(HaveOccurred())
+				Expect(result.Options).To(BeEmpty())
+				Expect(driver.disableICC).To(BeTrue(), "Failed for value: "+v)
+				Expect(result.Labels).To(ContainElement(FinchICCLabelIPv4+"=false"), "Failed for value: "+v)
+			}
+		})
+
+		It("should handle valid ICC option false with IPv6", func() {
+			request.Options = map[string]string{
+				BridgeICCOption: "false",
+			}
+			driver.IPv6 = true
+			result, err := driver.HandleCreateOptions(request, options)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(result.Options).To(BeEmpty())
+			Expect(driver.disableICC).To(BeTrue())
+			Expect(result.Labels).To(ContainElement(FinchICCLabelIPv6 + "=false"))
+		})
+
+		It("should warn on invalid ICC option", func() {
+			request.Options = map[string]string{
+				BridgeICCOption: "invalid",
+			}
+			logger.EXPECT().Warnf("invalid value for com.docker.network.bridge.enable_icc")
+			result, err := driver.HandleCreateOptions(request, options)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(result.Options).To(BeEmpty())
+			Expect(driver.disableICC).To(BeFalse())
+			Expect(result.Labels).To(BeEmpty())
+		})
+
+		It("should set bridge name", func() {
+			request.Options = map[string]string{
+				BridgeNameOption: "testbridge",
+			}
+			result, err := driver.HandleCreateOptions(request, options)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(result.Options).To(BeEmpty())
+			Expect(driver.bridgeName).To(Equal("testbridge"))
+		})
+
+		It("should pass through unknown options", func() {
+			request.Options = map[string]string{
+				"unknown.option": "value",
+			}
+			result, err := driver.HandleCreateOptions(request, options)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(result.Options).To(HaveKey("unknown.option"))
+			Expect(result.Options["unknown.option"]).To(Equal("value"))
+		})
+
+		It("should handle multiple options together", func() {
+			request.Options = map[string]string{
+				BridgeHostBindingIpv4Option: "0.0.0.0",
+				BridgeICCOption:             "false",
+				BridgeNameOption:            "testbridge",
+				"unknown.option":            "value",
+			}
+			logger.EXPECT().Warnf(gomock.Any(), gomock.Any()).Times(0)
+			result, err := driver.HandleCreateOptions(request, options)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(result.Options).To(HaveKey("unknown.option"))
+			Expect(driver.disableICC).To(BeTrue())
+			Expect(driver.bridgeName).To(Equal("testbridge"))
+			Expect(result.Labels).To(ContainElement(FinchICCLabelIPv4 + "=false"))
+		})
+	})
+})
+
 var _ = Describe("bridgeDriver Disable ICC", func() {
 	var (
 		mockController *gomock.Controller

diff --git a/internal/service/network/network.go b/internal/service/network/network.go
@@ -126,7 +126,7 @@ func (s *service) releaseLock(networkName string) {
 
 	if mwc, exists := s.networkMutexes[networkName]; exists {
 		mwc.activeReqs--
-		if mwc.activeReqs == 0 {
+		if mwc.activeReqs < 1 {
 			delete(s.networkMutexes, networkName)
 		}
 	}