From 33a4cfbf25d45a015246f46041a50051844a6ba6 Mon Sep 17 00:00:00 2001 From: mdutka-dell <115641913+mdutka-dell@users.noreply.github.com> Date: Wed, 17 Apr 2024 12:55:26 +0200 Subject: [PATCH] fix: add configurable export IP (#201) * add allowedNetworks * test: add unit tests * fix address types * Add panic after no matching network * Add comment to GetAddresses * add new testcases, change vaules parsing Signed-off-by: Novikov * return error level above, gofumpt test file --------- Signed-off-by: Novikov Co-authored-by: Bartosz Ciesielczyk Co-authored-by: adamginna-dell <115641695+adamginna-dell@users.noreply.github.com> Co-authored-by: Novikov --- service/envvars.go | 3 ++ service/node.go | 21 +++++++-- service/service.go | 7 +++ service/utils/emcutils.go | 48 ++++++++++++++++++++ service/utils/emcutils_test.go | 80 ++++++++++++++++++++++++++++++++++ 5 files changed, 156 insertions(+), 3 deletions(-) create mode 100644 service/utils/emcutils_test.go diff --git a/service/envvars.go b/service/envvars.go index 1f7e290..d34451b 100644 --- a/service/envvars.go +++ b/service/envvars.go @@ -56,4 +56,7 @@ const ( // EnvIsVolumeHealthMonitorEnabled - Environment variable to enable/disable health monitor of CSI volumes EnvIsVolumeHealthMonitorEnabled = "X_CSI_HEALTH_MONITOR_ENABLED" + + // EnvAllowedNetworks indicates list of networks on which NFS traffic is allowed + EnvAllowedNetworks = "X_CSI_ALLOWED_NETWORKS" ) diff --git a/service/node.go b/service/node.go index 45d67c7..c5ff148 100644 --- a/service/node.go +++ b/service/node.go @@ -1522,9 +1522,24 @@ func (s *service) addNodeInformationIntoArray(ctx context.Context, array *Storag log.Infof("Node %s does not have FC or iSCSI initiators and can only be used for NFS exports", s.opts.NodeName) } - nodeIps, err := utils.GetHostIP() - if err != nil { - return status.Error(codes.Unknown, utils.GetMessageWithRunID(rid, "Unable to get node IP. Error: %v", err)) + // logic if else + // if allowedNetowrks is set + // else utils.GetHostIP() with hostname -I/i method + var nodeIps []string + var err error + if len(s.opts.allowedNetworks) > 0 { + log.Debugf("Fetching IP address of custom network for NFS I/O traffic") + nodeIps, err = utils.GetNFSClientIP(s.opts.allowedNetworks) + if err != nil { + log.Fatalf("Failed to find IP address corresponding to the allowed network with error %s", err.Error()) + return err + } + } else { + // existing mechanism with hostname -I + nodeIps, err = utils.GetHostIP() + if err != nil { + return status.Error(codes.Unknown, utils.GetMessageWithRunID(rid, "Unable to get node IP. Error: %v", err)) + } } fqdnHost := false diff --git a/service/service.go b/service/service.go index cfde671..4f21738 100644 --- a/service/service.go +++ b/service/service.go @@ -130,6 +130,7 @@ type Opts struct { LogLevel string TenantName string IsVolumeHealthMonitorEnabled bool + allowedNetworks []string } type service struct { @@ -259,6 +260,12 @@ func (s *service) BeforeServe( opts.EnvEphemeralStagingTargetPath = ephemeralStagePath } + var networksList []string + if allNetworks, ok := csictx.LookupEnv(ctx, EnvAllowedNetworks); ok { + networksList = strings.Split(allNetworks, " ") + } + opts.allowedNetworks = networksList + // setup the iscsi client iscsiOpts := make(map[string]string, 0) if chroot, ok := csictx.LookupEnv(ctx, EnvISCSIChroot); ok { diff --git a/service/utils/emcutils.go b/service/utils/emcutils.go index 5fdadc4..1effc64 100644 --- a/service/utils/emcutils.go +++ b/service/utils/emcutils.go @@ -180,6 +180,54 @@ func GetHostIP() ([]string, error) { return lookupIps, nil } +// GetNFSClientIP is used to fetch IP address from networks on which NFS traffic is allowed +func GetNFSClientIP(allowedNetworks []string) ([]string, error) { + addrs, err := net.InterfaceAddrs() + if err != nil { + return nil, err + } + nodeIPs, err := GetAddresses(allowedNetworks, addrs) + if err != nil { + return nil, err + } + return nodeIPs, nil +} + +// GetAddresses is used to get validated IPs with allowed networks +func GetAddresses(allowedNetworks []string, addrs []net.Addr) ([]string, error) { + var nodeIPs []string + networks := make(map[string]bool) + for _, cnet := range allowedNetworks { + _, cnet, err := net.ParseCIDR(cnet) + if err != nil { + return nil, err + } + networks[cnet.String()] = false + } + + for _, a := range addrs { + switch v := a.(type) { + case *net.IPNet: + if v.IP.To4() != nil { + ip, cnet, err := net.ParseCIDR(v.String()) + if err != nil { + continue + } + + if _, ok := networks[cnet.String()]; ok { + nodeIPs = append(nodeIPs, ip.String()) + } + } + } + } + + // If a valid IP address matching allowedNetworks is not found return error + if len(nodeIPs) == 0 { + return nil, fmt.Errorf("no valid IP address found matching against allowedNetworks %v", allowedNetworks) + } + return nodeIPs, nil +} + // GetSnapshotResponseFromSnapshot - Utility method to convert Unity XT Rest type Snapshot to CSI standard Snapshot Response func GetSnapshotResponseFromSnapshot(snap *types.Snapshot, protocol, arrayID string) *csi.CreateSnapshotResponse { content := snap.SnapshotContent diff --git a/service/utils/emcutils_test.go b/service/utils/emcutils_test.go new file mode 100644 index 0000000..84a3e16 --- /dev/null +++ b/service/utils/emcutils_test.go @@ -0,0 +1,80 @@ +/* + Copyright © 2019 Dell Inc. or its subsidiaries. All Rights Reserved. + + 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 utils + +import ( + "fmt" + "net" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestGetAddresses(t *testing.T) { + type errorTestCases struct { + description string + addrs []net.Addr + networkAddresses []string + expectedAddresses []string + expectedError string + } + + for _, scenario := range []errorTestCases{ + { + description: "invalid", + addrs: []net.Addr{&net.IPNet{IP: net.ParseIP("192.168.2.1"), Mask: net.CIDRMask(24, 32)}}, + networkAddresses: []string{"192.168.1.1/24"}, + expectedAddresses: []string{}, + expectedError: fmt.Sprintf("no valid IP address found matching against allowedNetworks %v", []string{"192.168.1.1/24"}), + }, + { + description: "successful", + addrs: []net.Addr{&net.IPNet{IP: net.ParseIP("192.168.1.1"), Mask: net.CIDRMask(24, 32)}}, + networkAddresses: []string{"192.168.1.0/24"}, + expectedAddresses: []string{"192.168.1.1"}, + expectedError: "", + }, + { + description: "multiple networks, multiple addresses-successful", + addrs: []net.Addr{ + &net.IPNet{IP: net.ParseIP("192.168.1.1"), Mask: net.CIDRMask(24, 32)}, + &net.IPNet{IP: net.ParseIP("192.168.2.1"), Mask: net.CIDRMask(24, 32)}, + }, + networkAddresses: []string{"192.168.1.0/24", "192.168.2.0/24"}, + expectedAddresses: []string{"192.168.1.1", "192.168.2.1"}, + expectedError: "", + }, + { + description: "multiple networks, one erroneous address", + addrs: []net.Addr{ + &net.IPNet{IP: net.ParseIP("192.168.1.1"), Mask: net.CIDRMask(24, 32)}, + &net.IPNet{IP: net.ParseIP("192.168.3.1"), Mask: net.CIDRMask(24, 32)}, + }, + networkAddresses: []string{"192.168.1.0/24", "192.168.2.0/24"}, + expectedAddresses: []string{"192.168.1.1"}, + expectedError: "", + }, + } { + t.Run(scenario.description, func(t *testing.T) { + addresses, err := GetAddresses(scenario.networkAddresses, scenario.addrs) + if err != nil { + assert.EqualError(t, err, scenario.expectedError) + } else { + assert.NoError(t, err) + assert.Equal(t, scenario.expectedAddresses, addresses) + } + }) + } +}