diff --git a/api/types/network_types.go b/api/types/network_types.go index 7378b829..d9e63bd0 100644 --- a/api/types/network_types.go +++ b/api/types/network_types.go @@ -156,8 +156,6 @@ type NetworkCreateRequest struct { IPAM IPAM `json:"IPAM"` // EnableIPv6 specifies to enable IPv6 on the network. - // - // IPv6 networks are not currently supported. EnableIPv6 bool `json:"EnableIPv6"` // Options specifies network specific options to be used by the drivers. diff --git a/go.mod b/go.mod index b8844b08..1a131ca1 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,8 @@ module github.com/runfinch/finch-daemon -go 1.22 +go 1.22.0 + +toolchain go1.22.7 require ( github.com/containerd/cgroups/v3 v3.0.3 @@ -13,6 +15,7 @@ require ( github.com/containerd/platforms v0.2.1 github.com/containerd/typeurl/v2 v2.2.0 github.com/containernetworking/cni v1.2.2 + github.com/coreos/go-iptables v0.7.0 github.com/coreos/go-systemd/v22 v22.5.0 github.com/distribution/reference v0.6.0 github.com/docker/cli v26.0.0+incompatible @@ -65,14 +68,14 @@ require ( github.com/containerd/typeurl v1.0.3-0.20220422153119-7f6e6d160d67 // indirect github.com/containernetworking/plugins v1.4.1 // indirect github.com/containers/ocicrypt v1.1.10 // indirect - github.com/coreos/go-iptables v0.7.0 // indirect - github.com/cyphar/filepath-securejoin v0.2.4 // indirect - github.com/davecgh/go-spew v1.1.1 // indirect + github.com/cyphar/filepath-securejoin v0.3.1 // indirect + github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect github.com/djherbis/times v1.6.0 // indirect github.com/docker/docker-credential-helpers v0.8.1 // indirect github.com/docker/go-events v0.0.0-20190806004212-e31b211e4f1c // indirect github.com/docker/go-units v0.5.0 // indirect github.com/fahedouch/go-logrotate v0.2.0 // indirect + github.com/fatih/color v1.17.0 // indirect github.com/felixge/httpsnoop v1.0.4 // indirect github.com/fluent/fluent-logger-golang v1.9.0 // indirect github.com/getlantern/mockconn v0.0.0-20200818071412-cb30d065a848 // indirect diff --git a/go.sum b/go.sum index fc27189a..6217b410 100644 --- a/go.sum +++ b/go.sum @@ -78,11 +78,12 @@ github.com/coreos/go-systemd/v22 v22.5.0/go.mod h1:Y58oyj3AT4RCenI/lSvhwexgC+NSV github.com/cpuguy83/go-md2man/v2 v2.0.4/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o= github.com/creack/pty v1.1.18 h1:n56/Zwd5o6whRC5PMGretI4IdRLlmBXYNjScPaBgsbY= github.com/creack/pty v1.1.18/go.mod h1:MOBLtS5ELjhRRrroQr9kyvTxUAFNvYEK993ew/Vr4O4= -github.com/cyphar/filepath-securejoin v0.2.4 h1:Ugdm7cg7i6ZK6x3xDF1oEu1nfkyfH53EtKeQYTC3kyg= -github.com/cyphar/filepath-securejoin v0.2.4/go.mod h1:aPGpWjXOXUn2NCNjFvBE6aRxGGx79pTxQpKOJNYHHl4= +github.com/cyphar/filepath-securejoin v0.3.1 h1:1V7cHiaW+C+39wEfpH6XlLBQo3j/PciWFrgfCLS8XrE= +github.com/cyphar/filepath-securejoin v0.3.1/go.mod h1:F7i41x/9cBF7lzCrVsYs9fuzwRZm4NQsGTBdpp6mETc= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= -github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1VwoXQT9A3Wy9MM3WgvqSxFWenqJduM= +github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/distribution/reference v0.6.0 h1:0IXCQ5g4/QMHHkarYzh5l+u8T3t73zM5QvfrDyIgxBk= github.com/distribution/reference v0.6.0/go.mod h1:BbU0aIcezP1/5jX/8MP0YiH4SdvB5Y4f/wlDRiLyi3E= github.com/djherbis/times v1.6.0 h1:w2ctJ92J8fBvWPxugmXIv7Nz7Q3iDMKNx9v5ocVH20c= @@ -105,8 +106,8 @@ github.com/envoyproxy/go-control-plane v0.9.4/go.mod h1:6rpuAdCZL397s3pYoYcLgu1m github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7+kN2VEUnK/pcBlmesArF7c= github.com/fahedouch/go-logrotate v0.2.0 h1:UR9Fv8MDVfWwnkirmFHck+tRSWzqOwRjVRLMpQgSxaI= github.com/fahedouch/go-logrotate v0.2.0/go.mod h1:1RL/yr7LntS4zadAC6FT6yB/C1CQt3V6eHAZzymfwzE= -github.com/fatih/color v1.16.0 h1:zmkK9Ngbjj+K0yRhTVONQh1p/HknKYSlNT+vZCzyokM= -github.com/fatih/color v1.16.0/go.mod h1:fL2Sau1YI5c0pdGEVCbKQbLXB6edEj1ZgiY4NijnWvE= +github.com/fatih/color v1.17.0 h1:GlRw1BRJxkpqUCBKzKOw098ed57fEsKeNjpTe3cSjK4= +github.com/fatih/color v1.17.0/go.mod h1:YZ7TlrGPkiz6ku9fK3TLD/pl3CpsiFyu8N92HLgmosI= github.com/felixge/httpsnoop v1.0.4 h1:NFTV2Zj1bL4mc9sqWACXbQFVBBg2W3GPvqp8/ESS2Wg= github.com/felixge/httpsnoop v1.0.4/go.mod h1:m8KPJKqk1gH5J9DgRY2ASl2lWCfGKXixSwevea8zH2U= github.com/fluent/fluent-logger-golang v1.9.0 h1:zUdY44CHX2oIUc7VTNZc+4m+ORuO/mldQDA7czhWXEg= diff --git a/internal/service/network/create.go b/internal/service/network/create.go index 031718d3..0c7f1493 100644 --- a/internal/service/network/create.go +++ b/internal/service/network/create.go @@ -5,65 +5,42 @@ package network import ( "context" - "encoding/json" "fmt" - "os" - "path/filepath" "strings" - "github.com/containerd/nerdctl/pkg/lockutil" "github.com/containerd/nerdctl/pkg/netutil" "github.com/runfinch/finch-daemon/api/types" + "github.com/runfinch/finch-daemon/internal/service/network/driver" "github.com/runfinch/finch-daemon/pkg/errdefs" "github.com/runfinch/finch-daemon/pkg/utility/maputility" ) // Create implements the logic to turn a network create request to the back-end nerdctl create network calls. func (s *service) Create(ctx context.Context, request types.NetworkCreateRequest) (types.NetworkCreateResponse, error) { - // enable_ip_masquerade, host_binding_ipv4, and bridge name network options are not supported by nerdctl. - // So we must filter out any unsupported options which would prevent the network from being created and accept the defaults. - bridge := "" - filterUnsupportedOptions := func(original map[string]string) map[string]string { - options := map[string]string{} - for k, v := range original { - switch k { - case "com.docker.network.bridge.enable_ip_masquerade": - // must be true - if v != "true" { - s.logger.Warnf("network option com.docker.network.bridge.enable_ip_masquerade is set to %s, but it must be true", v) - } - case "com.docker.network.bridge.host_binding_ipv4": - // must be 0.0.0.0 - if v != "0.0.0.0" { - s.logger.Warnf("network option com.docker.network.bridge.host_binding_ipv4 is set to %s, but it must be 0.0.0.0", v) - } - case "com.docker.network.bridge.enable_icc": - s.logger.Warnf("network option com.docker.network.bridge.enable_icc is not currently supported in nerdctl", v) - case "com.docker.network.bridge.name": - bridge = v - default: - options[k] = v - } + var bridgeDriver driver.DriverHandler + var err error + + createOptionsFrom := func(request types.NetworkCreateRequest) (netutil.CreateOptions, error) { + // Default to "bridge" driver if request does not specify a driver + networkDriver := request.Driver + if networkDriver == "" { + networkDriver = "bridge" } - return options - } - createOptionsFrom := func(r types.NetworkCreateRequest) netutil.CreateOptions { options := netutil.CreateOptions{ - Name: r.Name, - Driver: "bridge", + Name: request.Name, + Driver: networkDriver, IPAMDriver: "default", - IPAMOptions: r.IPAM.Options, - Options: filterUnsupportedOptions(r.Options), - Labels: maputility.Flatten(r.Labels, maputility.KeyEqualsValueFormat), - } - if r.Driver != "" { - options.Driver = r.Driver + IPAMOptions: request.IPAM.Options, + Labels: maputility.Flatten(request.Labels, maputility.KeyEqualsValueFormat), + IPv6: request.EnableIPv6, } - if r.IPAM.Driver != "" { - options.IPAMDriver = r.IPAM.Driver + + if request.IPAM.Driver != "" { + options.IPAMDriver = request.IPAM.Driver } + if len(request.IPAM.Config) != 0 { options.Subnets = []string{} if subnet, ok := request.IPAM.Config[0]["Subnet"]; ok { @@ -76,7 +53,21 @@ func (s *service) Create(ctx context.Context, request types.NetworkCreateRequest options.Gateway = gateway } } - return options + + // Handle driver-specific options + switch networkDriver { + case "bridge": + bridgeDriver, err = driver.NewBridgeDriver(s.netClient, s.logger, options.IPv6) + if err != nil { + return options, err + } + options, err = bridgeDriver.HandleCreateOptions(request, options) + return options, err + default: + options.Options = request.Options + } + + return options, nil } if config, err := s.getNetwork(request.Name); err == nil { @@ -92,8 +83,21 @@ func (s *service) Create(ctx context.Context, request types.NetworkCreateRequest return response, nil } - net, err := s.netClient.CreateNetwork(createOptionsFrom(request)) - warning := "" + options, err := createOptionsFrom(request) + if err != nil { + return types.NetworkCreateResponse{}, err + } + + // Ensure thread-safety for network operations using a per-network mutex. + // Operations on different network IDs can proceed concurrently. + netMu := s.ensureLock(request.Name) + + netMu.Lock() + defer netMu.Unlock() + defer s.releaseLock(request.Name) + + // Create network + net, err := s.netClient.CreateNetwork(options) if err != nil && strings.Contains(err.Error(), "unsupported cni driver") { return types.NetworkCreateResponse{}, errdefs.NewNotFound(errPluginNotFound) } else if err != nil { @@ -104,103 +108,33 @@ func (s *service) Create(ctx context.Context, request types.NetworkCreateRequest return types.NetworkCreateResponse{}, errNetworkIDNotFound } - // Since nerdctl currently does not support custom bridge names, - // we explicitly override bridge name in the conflist file for the network that was just created - if bridge != "" { - if err = s.setBridgeName(net, bridge); err != nil { - warning = fmt.Sprintf("Failed to set network bridge name %s: %s", bridge, err) + // Add cleanup func to remove the network if an error is encountered during post processing + cleanup := func(ctx context.Context, name string) { + if cleanupErr := s.Remove(ctx, name); cleanupErr != nil { + // ignore if the network does not exist + if !errdefs.IsNotFound(cleanupErr) { + s.logger.Errorf("cleanup failed in defer %s: %v", name, cleanupErr) + } } } - return types.NetworkCreateResponse{ - ID: *net.NerdctlID, - Warning: warning, - }, nil -} - -// setBridgeName will override the bridge name in an existing CNI config file for a network. -func (s *service) setBridgeName(net *netutil.NetworkConfig, bridge string) error { - return lockutil.WithDirLock(s.netClient.NetconfPath(), func() error { - // first, make sure that the bridge name is not used by any of the existing bridge networks - bridgeNet, err := s.getNetworkByBridgeName(bridge) + defer func() { if err != nil { - return err - } - if bridgeNet != nil { - return fmt.Errorf("bridge name %s already in use by network %s", bridge, bridgeNet.Name) + cleanup(ctx, request.Name) } + }() - // load the CNI config file and set bridge name - configFilename := s.getConfigPathForNetworkName(net.Name) - configFile, err := os.Open(configFilename) + // Handle post network create actions + warning := "" + if bridgeDriver != nil { + warning, err = bridgeDriver.HandlePostCreate(net) if err != nil { - return err - } - defer configFile.Close() - var netJSON interface{} - if err = json.NewDecoder(configFile).Decode(&netJSON); err != nil { - return err - } - netMap, ok := netJSON.(map[string]interface{}) - if !ok { - return fmt.Errorf("network config file %s is not a valid map", configFilename) - } - plugins, ok := netMap["plugins"] - if !ok { - return fmt.Errorf("could not find plugins in network config file %s", configFilename) - } - pluginsMap, ok := plugins.([]interface{}) - if !ok { - return fmt.Errorf("could not parse plugins in network config file %s", configFilename) - } - for _, plugin := range pluginsMap { - pluginMap, ok := plugin.(map[string]interface{}) - if !ok { - continue - } - if pluginMap["type"] == "bridge" { - pluginMap["bridge"] = bridge - data, err := json.MarshalIndent(netJSON, "", " ") - if err != nil { - return err - } - return os.WriteFile(configFilename, data, 0o644) - } - } - return fmt.Errorf("bridge plugin not found in network config file %s", configFilename) - }) -} - -// From https://github.com/containerd/nerdctl/blob/v1.5.0/pkg/netutil/netutil.go#L186-L188 -func (s *service) getConfigPathForNetworkName(netName string) string { - return filepath.Join(s.netClient.NetconfPath(), "nerdctl-"+netName+".conflist") -} - -type bridgePlugin struct { - Type string `json:"type"` - Bridge string `json:"bridge"` -} - -func (s *service) getNetworkByBridgeName(bridge string) (*netutil.NetworkConfig, error) { - networks, err := s.netClient.FilterNetworks(func(*netutil.NetworkConfig) bool { - return true - }) - if err != nil { - return nil, err - } - for _, network := range networks { - for _, plugin := range network.Plugins { - if plugin.Network.Type != "bridge" { - continue - } - var bridgeJSON bridgePlugin - if err = json.Unmarshal(plugin.Bytes, &bridgeJSON); err != nil { - continue - } - if bridgeJSON.Bridge == bridge { - return network, nil - } + return types.NetworkCreateResponse{}, err } } - return nil, nil + + return types.NetworkCreateResponse{ + ID: *net.NerdctlID, + Warning: warning, + }, nil } diff --git a/internal/service/network/create_test.go b/internal/service/network/create_test.go index edcd302e..f93fe505 100644 --- a/internal/service/network/create_test.go +++ b/internal/service/network/create_test.go @@ -6,6 +6,7 @@ package network import ( "context" "errors" + "fmt" "github.com/containerd/nerdctl/pkg/netutil" "github.com/golang/mock/gomock" @@ -14,8 +15,12 @@ import ( "github.com/runfinch/finch-daemon/api/handlers/network" "github.com/runfinch/finch-daemon/api/types" + "github.com/runfinch/finch-daemon/internal/backend" + "github.com/runfinch/finch-daemon/internal/service/network/driver" "github.com/runfinch/finch-daemon/mocks/mocks_backend" "github.com/runfinch/finch-daemon/mocks/mocks_logger" + "github.com/runfinch/finch-daemon/mocks/mocks_network" + "github.com/runfinch/finch-daemon/pkg/flog" ) var _ = Describe("Network Service Create Network Implementation", func() { @@ -25,12 +30,13 @@ var _ = Describe("Network Service Create Network Implementation", func() { ) var ( - ctx context.Context - mockController *gomock.Controller - cdClient *mocks_backend.MockContainerdClient - ncNetClient *mocks_backend.MockNerdctlNetworkSvc - logger *mocks_logger.Logger - service network.Service + ctx context.Context + mockController *gomock.Controller + cdClient *mocks_backend.MockContainerdClient + ncNetClient *mocks_backend.MockNerdctlNetworkSvc + logger *mocks_logger.Logger + service network.Service + mockBridgeDriver *mocks_network.DriverHandler ) BeforeEach(func() { @@ -40,6 +46,7 @@ var _ = Describe("Network Service Create Network Implementation", func() { ncNetClient = mocks_backend.NewMockNerdctlNetworkSvc(mockController) logger = mocks_logger.NewLogger(mockController) service = NewService(cdClient, ncNetClient, logger) + mockBridgeDriver = mocks_network.NewDriverHandler(mockController) }) When("a create network call is successful", func() { @@ -336,4 +343,125 @@ var _ = Describe("Network Service Create Network Implementation", func() { }) }) }) + + Context("ICC configuration", func() { + When("com.docker.network.bridge.enable_icc is set to true in the request", func() { + It("should not change default behavior", func() { + request := types.NewCreateNetworkRequest( + networkName, + types.WithOptions(map[string]string{ + driver.BridgeICCOption: "true", + }), + ) + + ncNetClient.EXPECT().FilterNetworks(gomock.Any()).Return([]*netutil.NetworkConfig{}, nil) + logger.EXPECT().Debugf(gomock.Any(), gomock.Any()) + + nid := networkID + ncNetClient.EXPECT().CreateNetwork(gomock.Any()).DoAndReturn(func(actual netutil.CreateOptions) (*netutil.NetworkConfig, error) { + // Check if the label exists + checkLabel := driver.BridgeICCOption + "=true" + labelExists := false + for _, label := range actual.Labels { + if label == checkLabel { + labelExists = true + break + } + } + + Expect(labelExists).To(BeFalse(), fmt.Sprintf("Label '%s' should not exist in Labels", checkLabel)) + + return &netutil.NetworkConfig{NerdctlID: &nid}, nil + }) + + response, err := service.Create(ctx, *request) + Expect(err).ShouldNot(HaveOccurred()) + Expect(response.ID).Should(Equal(networkID)) + }) + }) + + When("ICC is not specified in the request", func() { + It("should use the default setting in the create options and not set any icc labels", func() { + request := types.NewCreateNetworkRequest(networkName) + + ncNetClient.EXPECT().FilterNetworks(gomock.Any()).Return([]*netutil.NetworkConfig{}, nil) + logger.EXPECT().Debugf(gomock.Any(), gomock.Any()) + + nid := networkID + ncNetClient.EXPECT().CreateNetwork(gomock.Any()).DoAndReturn(func(actual netutil.CreateOptions) (*netutil.NetworkConfig, error) { + // Check if the label exists + expectedLabel := driver.BridgeICCOption + "=true" + labelExists := false + for _, label := range actual.Labels { + if label == expectedLabel { + labelExists = true + break + } + } + + Expect(labelExists).To(BeFalse(), fmt.Sprintf("Label '%s' should not exist in Labels", expectedLabel)) + + return &netutil.NetworkConfig{NerdctlID: &nid}, nil + }) + + response, err := service.Create(ctx, *request) + Expect(err).ShouldNot(HaveOccurred()) + Expect(response.ID).Should(Equal(networkID)) + }) + }) + + When("com.docker.network.bridge.enable_icc is set to false in the request", func() { + It("should set ICC to false in the create options", func() { + request := types.NewCreateNetworkRequest( + networkName, + types.WithOptions(map[string]string{ + driver.BridgeICCOption: "false", + }), + ) + + ncNetClient.EXPECT().FilterNetworks(gomock.Any()).Return([]*netutil.NetworkConfig{}, nil) + logger.EXPECT().Debugf(gomock.Any(), gomock.Any()) + + nid := networkID + ncNetClient.EXPECT().CreateNetwork(gomock.Any()).DoAndReturn(func(actual netutil.CreateOptions) (*netutil.NetworkConfig, error) { + // Check if the label exists + expectedLabel := driver.FinchICCLabelIPv4 + "=false" + labelExists := false + for _, label := range actual.Labels { + if label == expectedLabel { + labelExists = true + break + } + } + + Expect(labelExists).To(BeTrue(), fmt.Sprintf("Label '%s' should exist in Labels", expectedLabel)) + + return &netutil.NetworkConfig{NerdctlID: &nid}, nil + }) + + originalDriverFunc := driver.NewBridgeDriver + driver.NewBridgeDriver = func(netClient backend.NerdctlNetworkSvc, logger flog.Logger, isIPv6 bool) (driver.DriverHandler, error) { + return mockBridgeDriver, nil + } + defer func() { driver.NewBridgeDriver = originalDriverFunc }() + + // Set up expectations for mockBridgeDriver + mockBridgeDriver.EXPECT().HandleCreateOptions(gomock.Any(), gomock.Any()).DoAndReturn( + func(request types.NetworkCreateRequest, options netutil.CreateOptions) (netutil.CreateOptions, error) { + // Mock the behavior for BridgeICCOption set to false + // Remove the option from the options map + + delete(options.Options, driver.BridgeICCOption) + options.Labels = append(options.Labels, driver.FinchICCLabelIPv4+"=false") + return options, nil + }).AnyTimes() + + mockBridgeDriver.EXPECT().HandlePostCreate(gomock.Any()).Return("", nil).AnyTimes() + + response, err := service.Create(ctx, *request) + Expect(err).ShouldNot(HaveOccurred()) + Expect(response.ID).Should(Equal(networkID)) + }) + }) + }) }) diff --git a/internal/service/network/driver/bridge.go b/internal/service/network/driver/bridge.go new file mode 100644 index 00000000..eb592440 --- /dev/null +++ b/internal/service/network/driver/bridge.go @@ -0,0 +1,307 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package driver + +import ( + "encoding/json" + "fmt" + "os" + "path/filepath" + "strconv" + + "github.com/containerd/nerdctl/pkg/lockutil" + "github.com/containerd/nerdctl/pkg/netutil" + "github.com/runfinch/finch-daemon/api/types" + "github.com/runfinch/finch-daemon/internal/backend" + "github.com/runfinch/finch-daemon/pkg/flog" +) + +const ( + FinchICCLabelIPv4 = "finch.network.bridge.enable_icc.ipv4" + FinchICCLabelIPv6 = "finch.network.bridge.enable_icc.ipv6" + BridgeICCOption = "com.docker.network.bridge.enable_icc" + BridgeHostBindingIpv4Option = "com.docker.network.bridge.host_binding_ipv4" + BridgeNameOption = "com.docker.network.bridge.name" +) + +type bridgeDriver struct { + bridgeName string + disableICC bool + netClient backend.NerdctlNetworkSvc + logger flog.Logger + IPv6 bool +} + +var _ DriverHandler = (*bridgeDriver)(nil) + +var NewBridgeDriver = func(netClient backend.NerdctlNetworkSvc, logger flog.Logger, IPv6 bool) (DriverHandler, error) { + return &bridgeDriver{ + netClient: netClient, + logger: logger, + IPv6: IPv6, + }, nil +} + +// HandleCreateOptions processes finch specific options for the bridge driver. +func (bd *bridgeDriver) HandleCreateOptions(request types.NetworkCreateRequest, options netutil.CreateOptions) (netutil.CreateOptions, error) { + // enable_icc, host_binding_ipv4, and bridge name network options are not supported by nerdctl. + // So we process these options here and filter them out from the network create request to nerdctl. + processUnsupportedOptions := func(original map[string]string) map[string]string { + opts := map[string]string{} + for k, v := range original { + switch k { + case BridgeHostBindingIpv4Option: + if v != "0.0.0.0" { + bd.logger.Warnf("network option com.docker.network.bridge.host_binding_ipv4 is set to %s, but it must be 0.0.0.0", v) + } + case BridgeICCOption: + iccOption, err := strconv.ParseBool(v) + if err != nil { + bd.logger.Warnf("invalid value for com.docker.network.bridge.enable_icc") + continue + } + bd.disableICC = !iccOption + case BridgeNameOption: + bd.bridgeName = v + default: + opts[k] = v + } + } + return opts + } + + options.Options = processUnsupportedOptions(request.Options) + + if bd.disableICC { + finchICCLabel := FinchICCLabelIPv4 + if bd.IPv6 { + finchICCLabel = FinchICCLabelIPv6 + } + options.Labels = append(options.Labels, finchICCLabel+"=false") + } + return options, nil +} + +func (bd *bridgeDriver) HandlePostCreate(net *netutil.NetworkConfig) (string, error) { + // Handle bridge driver post create actions + var warning string + var err error + bridgeName := bd.bridgeName + if bridgeName != "" { + // Since nerdctl currently does not support custom bridge names, + // we explicitly override bridge name in the conflist file for the network that was just created + if err = bd.setBridgeName(net, bridgeName); err != nil { + warning = fmt.Sprintf("Failed to set network bridge name %s: %s", bridgeName, err) + } + } + + if bd.disableICC { + // Handle "enable_icc=false" option if set (bd.disableICC is true) + // By default, CNI allows connectivity between containers attached to the same bridge. + // If "com.docker.network.bridge.enable_icc" option is explicitly set to false, + // we disable inter-container connectivity by applying iptable rules + // If "com.docker.network.bridge.enable_icc=true" is set, it is considered a noop + if bridgeName == "" { + bridgeName, err = bd.getBridgeName(net) + if err != nil { + return "", fmt.Errorf("failed to get bridge name to enable inter-container connectivity: %w ", err) + } + } + + err = bd.addICCDropRule(bridgeName) + if err != nil { + return "", fmt.Errorf("failed to disable inter-container connectivity: %w", err) + } + } + + return warning, nil +} + +func (bd *bridgeDriver) HandleRemove(net *netutil.NetworkConfig) error { + bridgeName, err := bd.getBridgeName(net) + if err != nil { + return fmt.Errorf("failed to get bridge name to remove inter-container connectivity: %w ", err) + } + err = bd.removeICCDropRule(bridgeName) + if err != nil { + return fmt.Errorf("failed to remove iptables DROP rule : %w", err) + } + return nil +} + +// setBridgeName will override the bridge name in an existing CNI config file for a network. +func (bd *bridgeDriver) setBridgeName(net *netutil.NetworkConfig, bridgeName string) error { + return lockutil.WithDirLock(bd.netClient.NetconfPath(), func() error { + // first, make sure that the bridge name is not used by any of the existing bridge networks + bridgeNet, err := bd.getNetworkByBridgeName(bridgeName) + if err != nil { + return err + } + if bridgeNet != nil { + return fmt.Errorf("bridge name %s already in use by network %s", bridgeName, bridgeNet.Name) + } + + configFilename := bd.getConfigPathForNetworkName(net.Name) + netMap, bridgePlugin, err := bd.parseBridgeConfig(configFilename) + if err != nil { + return err + } + + // Update the bridge name in the bridge plugin + bridgePlugin["bridge"] = bridgeName + + // Update the plugins in the full config + plugins := netMap["plugins"].([]interface{}) + for i, plugin := range plugins { + if p, ok := plugin.(map[string]interface{}); ok && p["type"] == "bridge" { + plugins[i] = bridgePlugin + break + } + } + + data, err := json.MarshalIndent(netMap, "", " ") + if err != nil { + return err + } + + // Write the updated config back to the file with the original permissions + fileInfo, err := os.Stat(configFilename) + if err != nil { + return err + } + + return os.WriteFile(configFilename, data, fileInfo.Mode().Perm()) + }) +} + +func (bd *bridgeDriver) getBridgeName(net *netutil.NetworkConfig) (string, error) { + var bridgeName string + err := lockutil.WithDirLock(bd.netClient.NetconfPath(), func() error { + configFilename := bd.getConfigPathForNetworkName(net.Name) + _, bridgePlugin, err := bd.parseBridgeConfig(configFilename) + if err != nil { + return err + } + + bridge, ok := bridgePlugin["bridge"].(string) + if !ok { + return fmt.Errorf("bridge name in config file %s is not a string", configFilename) + } + bridgeName = bridge + return nil + }) + + if err != nil { + return "", err + } + + return bridgeName, nil +} + +func (bd *bridgeDriver) parseBridgeConfig(configFilename string) (map[string]interface{}, map[string]interface{}, error) { + configFile, err := os.Open(configFilename) + if err != nil { + return nil, nil, err + } + defer configFile.Close() + + var netJSON interface{} + if err = json.NewDecoder(configFile).Decode(&netJSON); err != nil { + return nil, nil, err + } + + netMap, ok := netJSON.(map[string]interface{}) + if !ok { + return nil, nil, fmt.Errorf("network config file %s is not a valid map", configFilename) + } + + plugins, ok := netMap["plugins"] + if !ok { + return nil, nil, fmt.Errorf("could not find plugins in network config file %s", configFilename) + } + + pluginsMap, ok := plugins.([]interface{}) + if !ok { + return nil, nil, fmt.Errorf("could not parse plugins in network config file %s", configFilename) + } + + for _, plugin := range pluginsMap { + pluginMap, ok := plugin.(map[string]interface{}) + if !ok { + continue + } + if pluginMap["type"] == "bridge" { + return netMap, pluginMap, nil + } + } + + return nil, nil, fmt.Errorf("bridge plugin not found in network config file %s", configFilename) +} + +func (bd *bridgeDriver) getNetworkByBridgeName(bridgeName string) (*netutil.NetworkConfig, error) { + networks, err := bd.netClient.FilterNetworks(func(*netutil.NetworkConfig) bool { + return true + }) + if err != nil { + return nil, err + } + + var bridgePlugin struct { + Type string `json:"type"` + Bridge string `json:"bridge"` + } + + for _, network := range networks { + for _, plugin := range network.Plugins { + if plugin.Network.Type != "bridge" { + continue + } + + if err = json.Unmarshal(plugin.Bytes, &bridgePlugin); err != nil { + continue + } + if bridgePlugin.Bridge == bridgeName { + return network, nil + } + } + } + return nil, nil +} + +func (bd *bridgeDriver) addICCDropRule(bridgeIface string) error { + bd.logger.Debugf("adding ICC drop rule for bridge: %s", bridgeIface) + iccDropRule := []string{"-i", bridgeIface, "-o", bridgeIface, "-j", "DROP"} + ipc, err := newIptablesCommand(bd.IPv6) + if err != nil { + return err + } + + err = ipc.AddRule(iccDropRule...) + if err != nil { + return fmt.Errorf("failed to add iptables rule to drop ICC: %v", err) + } + + return nil +} + +func (bd *bridgeDriver) removeICCDropRule(bridgeIface string) error { + bd.logger.Debugf("removing ICC drop rule for bridge: %s", bridgeIface) + iccDropRule := []string{"-i", bridgeIface, "-o", bridgeIface, "-j", "DROP"} + ipc, err := newIptablesCommand(bd.IPv6) + if err != nil { + return err + } + + err = ipc.DelRule(iccDropRule...) + if err != nil { + return fmt.Errorf("failed to remove iptables rules to drop ICC: %v", err) + } + + return nil +} + +// From https://github.com/containerd/nerdctl/blob/v1.5.0/pkg/netutil/netutil.go#L186-L188 +func (bd *bridgeDriver) getConfigPathForNetworkName(netName string) string { + return filepath.Join(bd.netClient.NetconfPath(), "nerdctl-"+netName+".conflist") +} diff --git a/internal/service/network/driver/bridge_test.go b/internal/service/network/driver/bridge_test.go new file mode 100644 index 00000000..dfc4270b --- /dev/null +++ b/internal/service/network/driver/bridge_test.go @@ -0,0 +1,271 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +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 + logger *mocks_logger.Logger + mockIpt *mocks_network.MockIPTablesWrapper + driver *bridgeDriver + bridgeIface string + ) + + BeforeEach(func() { + mockController = gomock.NewController(GinkgoT()) + logger = mocks_logger.NewLogger(mockController) + mockIpt = mocks_network.NewMockIPTablesWrapper(mockController) + driver = &bridgeDriver{logger: logger} + bridgeIface = "br-mock" + newIptablesCommand = func(_ bool) (*iptablesCommand, error) { + iptCommand := &iptablesCommand{ + protos: make(map[iptables.Protocol]IPTablesWrapper), + } + iptCommand.protos[iptables.ProtocolIPv4] = mockIpt + return iptCommand, nil + } + logger.EXPECT().Debugf(gomock.Any(), gomock.Any()).AnyTimes() + }) + + Context("chain does not exist", func() { + It("should create and set up the FINCH-ISOLATE-CHAIN", func() { + mockIpt.EXPECT().ChainExists("filter", "FINCH-ISOLATE-CHAIN").Return(false, nil).Times(1) + mockIpt.EXPECT().NewChain("filter", "FINCH-ISOLATE-CHAIN").Return(nil) + mockIpt.EXPECT().InsertUnique("filter", "FORWARD", 1, "-j", "FINCH-ISOLATE-CHAIN").Return(nil) + mockIpt.EXPECT().AppendUnique("filter", "FINCH-ISOLATE-CHAIN", "-j", "RETURN").Return(nil) + + // Expect the second Insert call to FINCH-ISOLATE-CHAIN for the DROP rule + mockIpt.EXPECT().InsertUnique("filter", "FINCH-ISOLATE-CHAIN", 1, "-i", bridgeIface, "-o", bridgeIface, "-j", "DROP").Return(nil) + err := driver.addICCDropRule(bridgeIface) + Expect(err).ShouldNot(HaveOccurred()) + }) + }) + + Context("chain setup fails", func() { + When("we fail to check if chain exists", func() { + It("should return an error and cleanup", func() { + mockIpt.EXPECT().ChainExists("filter", "FINCH-ISOLATE-CHAIN").Return(false, fmt.Errorf("iptables failed")) + + ////expect cleanup to be called + mockIpt.EXPECT().DeleteChain("filter", "FINCH-ISOLATE-CHAIN").Return(nil) + + err := driver.addICCDropRule(bridgeIface) + Expect(err).Should(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("failed to check if iptables FINCH-ISOLATE-CHAIN exists")) + }) + }) + When("a new chain creation fails", func() { + It("should return an error and cleanup", func() { + mockIpt.EXPECT().ChainExists("filter", "FINCH-ISOLATE-CHAIN").Return(false, nil) + mockIpt.EXPECT().NewChain("filter", "FINCH-ISOLATE-CHAIN").Return(fmt.Errorf("iptables failed")) + + //expect cleanup to be called + mockIpt.EXPECT().DeleteChain("filter", "FINCH-ISOLATE-CHAIN").Return(nil) + + err := driver.addICCDropRule(bridgeIface) + Expect(err).Should(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("failed to create FINCH-ISOLATE-CHAIN chain")) + }) + }) + }) + + Context("chain already exists", func() { + BeforeEach(func() { + mockIpt.EXPECT().ChainExists("filter", "FINCH-ISOLATE-CHAIN").Return(true, nil).AnyTimes() + mockIpt.EXPECT().InsertUnique("filter", "FORWARD", 1, "-j", "FINCH-ISOLATE-CHAIN").Return(nil).AnyTimes() + mockIpt.EXPECT().AppendUnique("filter", "FINCH-ISOLATE-CHAIN", "-j", "RETURN").Return(nil).AnyTimes() + }) + When("addICCDropRule is successful", func() { + It("should add the DROP rule to the FINCH-ISOLATE-CHAIN", func() { + // Expect the DROP rule to be inserted for packets from and to the same bridge + mockIpt.EXPECT().InsertUnique("filter", "FINCH-ISOLATE-CHAIN", 1, "-i", bridgeIface, "-o", bridgeIface, "-j", "DROP").Return(nil) + err := driver.addICCDropRule(bridgeIface) + Expect(err).ShouldNot(HaveOccurred()) + }) + }) + + When("removeICCDropRule is successful", func() { + It("should remove the DROP rule from the FINCH-ISOLATE-CHAIN", func() { + mockIpt.EXPECT().DeleteIfExists("filter", "FINCH-ISOLATE-CHAIN", "-i", bridgeIface, "-o", bridgeIface, "-j", "DROP").Return(nil) + err := driver.removeICCDropRule(bridgeIface) + Expect(err).ShouldNot(HaveOccurred()) + }) + }) + When("addICCDropRule fails", func() { + It("should return an error", func() { + mockIpt.EXPECT().InsertUnique("filter", "FINCH-ISOLATE-CHAIN", 1, "-i", bridgeIface, "-o", bridgeIface, "-j", "DROP").Return(fmt.Errorf("iptables error")) + + // //expect cleanup to be called + mockIpt.EXPECT().DeleteIfExists("filter", "FINCH-ISOLATE-CHAIN", "-i", bridgeIface, "-o", bridgeIface, "-j", "DROP").Return(nil) + + err := driver.addICCDropRule(bridgeIface) + Expect(err).Should(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("failed to add iptables rule to drop ICC")) + }) + }) + + When("removeICCDropRule fails", func() { + It("should return an error ", func() { + mockIpt.EXPECT().DeleteIfExists("filter", "FINCH-ISOLATE-CHAIN", "-i", bridgeIface, "-o", bridgeIface, "-j", "DROP").Return(fmt.Errorf("iptables error")) + + err := driver.removeICCDropRule(bridgeIface) + Expect(err).Should(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("failed to remove iptables rules to drop ICC")) + }) + }) + }) +}) diff --git a/internal/service/network/driver/driver.go b/internal/service/network/driver/driver.go new file mode 100644 index 00000000..d55f22b9 --- /dev/null +++ b/internal/service/network/driver/driver.go @@ -0,0 +1,16 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package driver + +import ( + "github.com/containerd/nerdctl/pkg/netutil" + "github.com/runfinch/finch-daemon/api/types" +) + +//go:generate mockgen --destination=../../../../mocks/mocks_network/driver.go -package=mocks_network -mock_names DriverHandler=DriverHandler . DriverHandler +type DriverHandler interface { + HandleCreateOptions(request types.NetworkCreateRequest, options netutil.CreateOptions) (netutil.CreateOptions, error) + HandlePostCreate(net *netutil.NetworkConfig) (string, error) + HandleRemove(net *netutil.NetworkConfig) error +} diff --git a/internal/service/network/driver/driver_test.go b/internal/service/network/driver/driver_test.go new file mode 100644 index 00000000..8c9ac13c --- /dev/null +++ b/internal/service/network/driver/driver_test.go @@ -0,0 +1,17 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package driver + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +// TestNetworkService is the entry point of the network service package's unit tests using ginkgo. +func TestNetworkService(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "UnitTests - Network Driver Operations") +} diff --git a/internal/service/network/driver/iptables.go b/internal/service/network/driver/iptables.go new file mode 100644 index 00000000..2987d8fb --- /dev/null +++ b/internal/service/network/driver/iptables.go @@ -0,0 +1,146 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package driver + +import ( + "errors" + "fmt" + + "github.com/coreos/go-iptables/iptables" +) + +const ( + statusChainExists = 1 + + FilterTableName = "filter" + ForwardChainName = "FORWARD" + FinchIsolateChain = "FINCH-ISOLATE-CHAIN" +) + +// IPTablesWrapper is an interface that wraps the methods of iptables.IPTables +// to help with mock +// +//go:generate mockgen --destination=../../../../mocks/mocks_network/iptables.go -package=mocks_network . IPTablesWrapper +type IPTablesWrapper interface { + ChainExists(table, chain string) (bool, error) + NewChain(table, chain string) error + InsertUnique(table, chain string, pos int, rulespec ...string) error + AppendUnique(table, chain string, rulespec ...string) error + DeleteIfExists(table, chain string, rulespec ...string) error + DeleteChain(table, chain string) error +} + +type iptablesCommand struct { + protos map[iptables.Protocol]IPTablesWrapper +} + +var newIptablesCommand = func(ipv6 bool) (*iptablesCommand, error) { + iptCommand := &iptablesCommand{ + protos: make(map[iptables.Protocol]IPTablesWrapper), + } + + protocols := []iptables.Protocol{iptables.ProtocolIPv4} + if ipv6 { + protocols = append(protocols, iptables.ProtocolIPv6) + } + + for _, proto := range protocols { + ipt, err := iptables.NewWithProtocol(proto) + if err != nil { + return nil, fmt.Errorf("could not initialize iptables protocol %v: %w", proto, err) + } + iptCommand.protos[proto] = ipt + } + + return iptCommand, nil +} + +// EnsureChain creates a new iptables chain if one doesn't exist already. +func (ipc *iptablesCommand) ensureChain(ipt IPTablesWrapper, table, chain string) error { + if ipt == nil { + return errors.New("failed to ensure iptable chain: IPTables was nil") + } + exists, err := ipt.ChainExists(table, chain) + if err != nil { + return fmt.Errorf("failed to check if iptables %s exists: %v", chain, err) + } + if !exists { + err = ipt.NewChain(table, chain) + if err != nil { + eerr, eok := err.(*iptables.Error) + if !eok { + // type assertion failed, return the original error + return fmt.Errorf("failed to create %s chain: %w", chain, err) + } + + // ignore if the chain was created in the meantime + if eerr.ExitStatus() != statusChainExists { + return fmt.Errorf("failed to create %s chain: %w", chain, err) + } + } + } + return nil +} + +func (ipc *iptablesCommand) cleanupChain(ipt IPTablesWrapper, tableName string, chainName string) { + // attempt to delete the chain if it exists and is empty + ipt.DeleteChain(tableName, chainName) +} + +func (ipc *iptablesCommand) setupChains(ipt IPTablesWrapper) error { + if err := ipc.ensureChain(ipt, FilterTableName, FinchIsolateChain); err != nil { + ipc.cleanupChain(ipt, FilterTableName, FinchIsolateChain) + return err + } + // Add a rule to the FORWARD chain that jumps to the FINCH-ISOLATE-CHAIN for all packets + jumpRule := []string{"-j", FinchIsolateChain} + if err := ipt.InsertUnique(FilterTableName, ForwardChainName, 1, jumpRule...); err != nil { + return fmt.Errorf("failed to add jump rule to FINCH-ISOLATE-CHAIN chain: %v", err) + } + // In the FINCH-ISOLATE-CHAIN, add a rule to return to the FORWARD chain when no match + returnRule := []string{"-j", "RETURN"} + if err := ipt.AppendUnique(FilterTableName, FinchIsolateChain, returnRule...); err != nil { + return fmt.Errorf("failed to add RETURN rule in FINCH-ISOLATE-CHAIN chain: %v", err) + } + return nil +} + +func (ipc *iptablesCommand) cleanupRules(ipt IPTablesWrapper, tableName string, chainName string, rulespec ...string) { + ipt.DeleteIfExists(tableName, chainName, rulespec...) +} + +func (ipc *iptablesCommand) addRule(ipt IPTablesWrapper, rulespec ...string) error { + if err := ipc.setupChains(ipt); err != nil { + return err + } + + if err := ipt.InsertUnique(FilterTableName, FinchIsolateChain, 1, rulespec...); err != nil { + ipc.cleanupRules(ipt, FilterTableName, FinchIsolateChain, rulespec...) + return fmt.Errorf("failed to add iptables rule: %w", err) + } + + return nil +} + +func (ipc *iptablesCommand) AddRule(rulespec ...string) error { + for _, ipt := range ipc.protos { + if err := ipc.addRule(ipt, rulespec...); err != nil { + return err + } + } + return nil +} + +func (ipc *iptablesCommand) delRule(ipt IPTablesWrapper, rulespec ...string) error { + return ipt.DeleteIfExists(FilterTableName, FinchIsolateChain, rulespec...) +} + +func (ipc *iptablesCommand) DelRule(rulespec ...string) error { + for _, ipt := range ipc.protos { + if err := ipc.delRule(ipt, rulespec...); err != nil { + return err + } + } + return nil +} diff --git a/internal/service/network/network.go b/internal/service/network/network.go index 8f29f54a..c4c0fd68 100644 --- a/internal/service/network/network.go +++ b/internal/service/network/network.go @@ -8,6 +8,7 @@ import ( "errors" "fmt" "regexp" + "sync" "github.com/containerd/containerd" "github.com/containerd/nerdctl/pkg/netutil" @@ -24,17 +25,25 @@ var ( errNetworkIDNotFound = errors.New("network ID not found") ) +type mutexCounter struct { + mutex *sync.Mutex + activeReqs int // activeReqs will track concurrent active requests for the same network +} + type service struct { - client backend.ContainerdClient - netClient backend.NerdctlNetworkSvc - logger flog.Logger + client backend.ContainerdClient + netClient backend.NerdctlNetworkSvc + logger flog.Logger + mu sync.Mutex + networkMutexes map[string]*mutexCounter } func NewService(client backend.ContainerdClient, netClient backend.NerdctlNetworkSvc, logger flog.Logger) network.Service { return &service{ - client: client, - netClient: netClient, - logger: logger, + client: client, + netClient: netClient, + logger: logger, + networkMutexes: make(map[string]*mutexCounter), } } @@ -97,3 +106,28 @@ func (s *service) getContainer(ctx context.Context, containerId string) (contain return searchResult[0], nil } + +func (s *service) ensureLock(networkName string) *sync.Mutex { + s.mu.Lock() + defer s.mu.Unlock() + if mwc, exists := s.networkMutexes[networkName]; exists { + mwc.activeReqs++ + return mwc.mutex + } + + netMu := &sync.Mutex{} + s.networkMutexes[networkName] = &mutexCounter{mutex: netMu, activeReqs: 1} + return netMu +} + +func (s *service) releaseLock(networkName string) { + s.mu.Lock() + defer s.mu.Unlock() + + if mwc, exists := s.networkMutexes[networkName]; exists { + mwc.activeReqs-- + if mwc.activeReqs < 1 { + delete(s.networkMutexes, networkName) + } + } +} diff --git a/internal/service/network/remove.go b/internal/service/network/remove.go index 674bb49e..a2a6692a 100644 --- a/internal/service/network/remove.go +++ b/internal/service/network/remove.go @@ -7,6 +7,8 @@ import ( "context" "fmt" + "github.com/containerd/nerdctl/pkg/netutil" + "github.com/runfinch/finch-daemon/internal/service/network/driver" "github.com/runfinch/finch-daemon/pkg/errdefs" ) @@ -25,8 +27,69 @@ func (s *service) Remove(ctx context.Context, networkId string) error { if value, ok := usedNetworkInfo[net.Name]; ok { return errdefs.NewForbidden(fmt.Errorf("network %q is in use by container %q", networkId, value)) } + if net.File == "" { return errdefs.NewForbidden(fmt.Errorf("%s is a pre-defined network and cannot be removed", networkId)) } - return s.netClient.RemoveNetwork(net) + + // Ensure thread-safety for network operations using a per-network mutex. + // RemoveNetwork and CreateNetwork operations on the same network ID are mutually exclusive. + // Operations on different network IDs can proceed concurrently. + netMu := s.ensureLock(net.Name) + + netMu.Lock() + defer netMu.Unlock() + defer s.releaseLock(net.Name) + + // Perform additional workflow based on the assigned network labels + if err := s.handleNetworkLabels(net); err != nil { + return fmt.Errorf("failed to handle nerdctl label: %w", err) + } + + err = s.netClient.RemoveNetwork(net) + if err != nil { + return fmt.Errorf("failed to remove network: %w", err) + } + + return nil +} + +func (s *service) handleNetworkLabels(net *netutil.NetworkConfig) error { + if net.NerdctlLabels == nil { + return nil + } + + for key, value := range *net.NerdctlLabels { + switch key { + case driver.FinchICCLabelIPv4: + if err := s.handleICCLabel(net, value, false); err != nil { + return fmt.Errorf("error handling IPv4 ICC label: %w", err) + } + case driver.FinchICCLabelIPv6: + if err := s.handleICCLabel(net, value, true); err != nil { + return fmt.Errorf("error handling IPv6 ICC label: %w", err) + } + } + } + return nil +} + +func (s *service) handleICCLabel(net *netutil.NetworkConfig, value string, isIPv6 bool) error { + if value != "false" { + // for some reason the label value got modified. + // we will still try to remove the iptable rules. + // iptable.DeleteIfExists is used to ignore non-existent errors + s.logger.Warnf("unexpected value for ICC label: %s", value) + } + + bridgeDriver, err := driver.NewBridgeDriver(s.netClient, s.logger, isIPv6) + if err != nil { + return fmt.Errorf("unable to create bridge driver: %w", err) + } + + if err := bridgeDriver.HandleRemove(net); err != nil { + return fmt.Errorf("error handling ICC label: %w", err) + } + + return nil } diff --git a/mocks/mocks_backend/nerdctlimagesvc.go b/mocks/mocks_backend/nerdctlimagesvc.go index 018db5da..e4121ae2 100644 --- a/mocks/mocks_backend/nerdctlimagesvc.go +++ b/mocks/mocks_backend/nerdctlimagesvc.go @@ -10,12 +10,12 @@ import ( reflect "reflect" images "github.com/containerd/containerd/images" - platforms "github.com/containerd/platforms" remotes "github.com/containerd/containerd/remotes" docker "github.com/containerd/containerd/remotes/docker" imgutil "github.com/containerd/nerdctl/pkg/imgutil" dockerconfigresolver "github.com/containerd/nerdctl/pkg/imgutil/dockerconfigresolver" dockercompat "github.com/containerd/nerdctl/pkg/inspecttypes/dockercompat" + platforms "github.com/containerd/platforms" gomock "github.com/golang/mock/gomock" v1 "github.com/opencontainers/image-spec/specs-go/v1" ) diff --git a/mocks/mocks_container/containersvc.go b/mocks/mocks_container/containersvc.go index be39abd1..7b80ef56 100644 --- a/mocks/mocks_container/containersvc.go +++ b/mocks/mocks_container/containersvc.go @@ -191,7 +191,7 @@ func (m *MockService) Restart(arg0 context.Context, arg1 string, arg2 time.Durat ret0, _ := ret[0].(error) return ret0 } - + // Restart indicates an expected call of Restart. func (mr *MockServiceMockRecorder) Restart(arg0, arg1, arg2 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() diff --git a/mocks/mocks_network/driver.go b/mocks/mocks_network/driver.go new file mode 100644 index 00000000..21d6846f --- /dev/null +++ b/mocks/mocks_network/driver.go @@ -0,0 +1,80 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: github.com/runfinch/finch-daemon/internal/service/network/driver (interfaces: DriverHandler) + +// Package mocks_network is a generated GoMock package. +package mocks_network + +import ( + reflect "reflect" + + netutil "github.com/containerd/nerdctl/pkg/netutil" + gomock "github.com/golang/mock/gomock" + types "github.com/runfinch/finch-daemon/api/types" +) + +// DriverHandler is a mock of DriverHandler interface. +type DriverHandler struct { + ctrl *gomock.Controller + recorder *DriverHandlerMockRecorder +} + +// DriverHandlerMockRecorder is the mock recorder for DriverHandler. +type DriverHandlerMockRecorder struct { + mock *DriverHandler +} + +// NewDriverHandler creates a new mock instance. +func NewDriverHandler(ctrl *gomock.Controller) *DriverHandler { + mock := &DriverHandler{ctrl: ctrl} + mock.recorder = &DriverHandlerMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *DriverHandler) EXPECT() *DriverHandlerMockRecorder { + return m.recorder +} + +// HandleCreateOptions mocks base method. +func (m *DriverHandler) HandleCreateOptions(arg0 types.NetworkCreateRequest, arg1 netutil.CreateOptions) (netutil.CreateOptions, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "HandleCreateOptions", arg0, arg1) + ret0, _ := ret[0].(netutil.CreateOptions) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// HandleCreateOptions indicates an expected call of HandleCreateOptions. +func (mr *DriverHandlerMockRecorder) HandleCreateOptions(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "HandleCreateOptions", reflect.TypeOf((*DriverHandler)(nil).HandleCreateOptions), arg0, arg1) +} + +// HandlePostCreate mocks base method. +func (m *DriverHandler) HandlePostCreate(arg0 *netutil.NetworkConfig) (string, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "HandlePostCreate", arg0) + ret0, _ := ret[0].(string) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// HandlePostCreate indicates an expected call of HandlePostCreate. +func (mr *DriverHandlerMockRecorder) HandlePostCreate(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "HandlePostCreate", reflect.TypeOf((*DriverHandler)(nil).HandlePostCreate), arg0) +} + +// HandleRemove mocks base method. +func (m *DriverHandler) HandleRemove(arg0 *netutil.NetworkConfig) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "HandleRemove", arg0) + ret0, _ := ret[0].(error) + return ret0 +} + +// HandleRemove indicates an expected call of HandleRemove. +func (mr *DriverHandlerMockRecorder) HandleRemove(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "HandleRemove", reflect.TypeOf((*DriverHandler)(nil).HandleRemove), arg0) +} diff --git a/mocks/mocks_network/iptables.go b/mocks/mocks_network/iptables.go new file mode 100644 index 00000000..94dc213b --- /dev/null +++ b/mocks/mocks_network/iptables.go @@ -0,0 +1,134 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: github.com/runfinch/finch-daemon/internal/service/network/driver (interfaces: IPTablesWrapper) + +// Package mocks_network is a generated GoMock package. +package mocks_network + +import ( + reflect "reflect" + + gomock "github.com/golang/mock/gomock" +) + +// MockIPTablesWrapper is a mock of IPTablesWrapper interface. +type MockIPTablesWrapper struct { + ctrl *gomock.Controller + recorder *MockIPTablesWrapperMockRecorder +} + +// MockIPTablesWrapperMockRecorder is the mock recorder for MockIPTablesWrapper. +type MockIPTablesWrapperMockRecorder struct { + mock *MockIPTablesWrapper +} + +// NewMockIPTablesWrapper creates a new mock instance. +func NewMockIPTablesWrapper(ctrl *gomock.Controller) *MockIPTablesWrapper { + mock := &MockIPTablesWrapper{ctrl: ctrl} + mock.recorder = &MockIPTablesWrapperMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockIPTablesWrapper) EXPECT() *MockIPTablesWrapperMockRecorder { + return m.recorder +} + +// AppendUnique mocks base method. +func (m *MockIPTablesWrapper) AppendUnique(arg0, arg1 string, arg2 ...string) error { + m.ctrl.T.Helper() + varargs := []interface{}{arg0, arg1} + for _, a := range arg2 { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "AppendUnique", varargs...) + ret0, _ := ret[0].(error) + return ret0 +} + +// AppendUnique indicates an expected call of AppendUnique. +func (mr *MockIPTablesWrapperMockRecorder) AppendUnique(arg0, arg1 interface{}, arg2 ...interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + varargs := append([]interface{}{arg0, arg1}, arg2...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AppendUnique", reflect.TypeOf((*MockIPTablesWrapper)(nil).AppendUnique), varargs...) +} + +// ChainExists mocks base method. +func (m *MockIPTablesWrapper) ChainExists(arg0, arg1 string) (bool, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ChainExists", arg0, arg1) + ret0, _ := ret[0].(bool) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// ChainExists indicates an expected call of ChainExists. +func (mr *MockIPTablesWrapperMockRecorder) ChainExists(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ChainExists", reflect.TypeOf((*MockIPTablesWrapper)(nil).ChainExists), arg0, arg1) +} + +// DeleteChain mocks base method. +func (m *MockIPTablesWrapper) DeleteChain(arg0, arg1 string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "DeleteChain", arg0, arg1) + ret0, _ := ret[0].(error) + return ret0 +} + +// DeleteChain indicates an expected call of DeleteChain. +func (mr *MockIPTablesWrapperMockRecorder) DeleteChain(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteChain", reflect.TypeOf((*MockIPTablesWrapper)(nil).DeleteChain), arg0, arg1) +} + +// DeleteIfExists mocks base method. +func (m *MockIPTablesWrapper) DeleteIfExists(arg0, arg1 string, arg2 ...string) error { + m.ctrl.T.Helper() + varargs := []interface{}{arg0, arg1} + for _, a := range arg2 { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "DeleteIfExists", varargs...) + ret0, _ := ret[0].(error) + return ret0 +} + +// DeleteIfExists indicates an expected call of DeleteIfExists. +func (mr *MockIPTablesWrapperMockRecorder) DeleteIfExists(arg0, arg1 interface{}, arg2 ...interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + varargs := append([]interface{}{arg0, arg1}, arg2...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteIfExists", reflect.TypeOf((*MockIPTablesWrapper)(nil).DeleteIfExists), varargs...) +} + +// InsertUnique mocks base method. +func (m *MockIPTablesWrapper) InsertUnique(arg0, arg1 string, arg2 int, arg3 ...string) error { + m.ctrl.T.Helper() + varargs := []interface{}{arg0, arg1, arg2} + for _, a := range arg3 { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "InsertUnique", varargs...) + ret0, _ := ret[0].(error) + return ret0 +} + +// InsertUnique indicates an expected call of InsertUnique. +func (mr *MockIPTablesWrapperMockRecorder) InsertUnique(arg0, arg1, arg2 interface{}, arg3 ...interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + varargs := append([]interface{}{arg0, arg1, arg2}, arg3...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "InsertUnique", reflect.TypeOf((*MockIPTablesWrapper)(nil).InsertUnique), varargs...) +} + +// NewChain mocks base method. +func (m *MockIPTablesWrapper) NewChain(arg0, arg1 string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "NewChain", arg0, arg1) + ret0, _ := ret[0].(error) + return ret0 +} + +// NewChain indicates an expected call of NewChain. +func (mr *MockIPTablesWrapperMockRecorder) NewChain(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "NewChain", reflect.TypeOf((*MockIPTablesWrapper)(nil).NewChain), arg0, arg1) +}