diff --git a/npm/pkg/dataplane/ipsets/ipset.go b/npm/pkg/dataplane/ipsets/ipset.go index 9b860bf225..a17c3f7902 100644 --- a/npm/pkg/dataplane/ipsets/ipset.go +++ b/npm/pkg/dataplane/ipsets/ipset.go @@ -3,12 +3,9 @@ package ipsets import ( "errors" "fmt" - "net" - "strings" "github.com/Azure/azure-container-networking/log" "github.com/Azure/azure-container-networking/npm/util" - npmerrors "github.com/Azure/azure-container-networking/npm/util/errors" ) type IPSetMetadata struct { @@ -397,18 +394,6 @@ func (set *IPSet) canSetBeSelectorIPSet() bool { set.Type == NestedLabelOfPod) } -// TODO: This is an adhoc approach for linux, but need to refactor data structure for better management. -func ValidateIPBlock(ipblock string) error { - // TODO: This is fragile code with strong dependency with " "(space). - // onlyCidr has only cidr without "space" and "nomatch" in case except ipblock to validate cidr format. - onlyCidr := strings.Split(ipblock, " ")[0] - _, _, err := net.ParseCIDR(onlyCidr) - if err != nil { - return npmerrors.SimpleErrorWrapper("failed to parse CIDR", err) - } - return nil -} - func GetMembersOfTranslatedSets(members []string) []*IPSetMetadata { memberList := make([]*IPSetMetadata, len(members)) i := 0 diff --git a/npm/pkg/dataplane/ipsets/ipsetmanager.go b/npm/pkg/dataplane/ipsets/ipsetmanager.go index a6a3f473ba..6ebfaf0178 100644 --- a/npm/pkg/dataplane/ipsets/ipsetmanager.go +++ b/npm/pkg/dataplane/ipsets/ipsetmanager.go @@ -554,10 +554,7 @@ func validateIPSetMemberIP(ip string) bool { // 192.168.0.0/24 nomatch // always guaranteed to have ip, not guaranteed to have port + protocol ipDetails := strings.Split(ip, ",") - if util.IsIPV4(ipDetails[0]) { - return true - } + ipField := strings.Split(ipDetails[0], " ") - err := ValidateIPBlock(ip) - return err == nil + return util.IsIPV4(ipField[0]) } diff --git a/npm/pkg/dataplane/ipsets/ipsetmanager_test.go b/npm/pkg/dataplane/ipsets/ipsetmanager_test.go index 51d35a86fc..921c077d0e 100644 --- a/npm/pkg/dataplane/ipsets/ipsetmanager_test.go +++ b/npm/pkg/dataplane/ipsets/ipsetmanager_test.go @@ -1357,48 +1357,84 @@ func TestMain(m *testing.M) { os.Exit(exitCode) } -func TestValidateIPBlock(t *testing.T) { +func TestValidateIPSetMemberIP(t *testing.T) { tests := []struct { name string ipblock string - wantErr bool + want bool }{ { name: "cidr", ipblock: "172.17.0.0/16", - wantErr: false, + want: true, }, { name: "except ipblock", ipblock: "172.17.1.0/24 nomatch", - wantErr: false, + want: true, }, { name: "incorrect ip format", ipblock: "1234", - wantErr: true, + want: false, }, { name: "incorrect ip range", ipblock: "256.1.2.3", - wantErr: true, + want: false, }, { name: "empty cidr", ipblock: "", - wantErr: true, + want: false, + }, + { + name: "ipv6", + ipblock: "2345:0425:2CA1:0000:0000:0567:5673:23b5/24", + want: false, + }, + { + name: "tcp", + ipblock: "192.168.0.0/24,tcp:25227", + want: true, + }, + { + name: "valid ip no cidr", + ipblock: "10.0.0.0", + want: true, + }, + { + name: "invalid cidr", + ipblock: "10.0.0.1/33", + want: false, + }, + { + name: "valid ip nomatch", + ipblock: "192.168.0.1 nomatch", + want: true, + }, + { + name: "valid ip tcp", + ipblock: "192.168.0.1,tcp:25227", + want: true, + }, + { + name: "ipv6 tcp", + ipblock: "2345:0425:2CA1:0000:0000:0567:5673:23b5/24,tcp:25227", + want: false, + }, + { + name: "ipv6 nomatch", + ipblock: "2345:0425:2CA1:0000:0000:0567:5673:23b5 nomatch", + want: false, }, } for _, tt := range tests { tt := tt t.Run(tt.name, func(t *testing.T) { - err := ValidateIPBlock(tt.ipblock) - if tt.wantErr { - require.Error(t, err) - } else { - require.NoError(t, err) - } + got := validateIPSetMemberIP(tt.ipblock) + require.Equal(t, tt.want, got) }) } } diff --git a/npm/util/util.go b/npm/util/util.go index ce95c31acc..0df8a649aa 100644 --- a/npm/util/util.go +++ b/npm/util/util.go @@ -6,6 +6,7 @@ import ( "fmt" "hash/fnv" "net" + "net/netip" "os" "regexp" "runtime" @@ -355,10 +356,17 @@ func SliceToString(list []string) string { } func IsIPV4(ip string) bool { - if net.ParseIP(ip).To4() != nil { - return true + isIPBlock := strings.Contains(ip, "/") + ipOnly := strings.Split(ip, "/") + address, err := netip.ParseAddr(ipOnly[0]) + if err != nil { + return false + } + + if address.Is4() && isIPBlock { + _, _, err := net.ParseCIDR(ip) + return err == nil } - _, _, err := net.ParseCIDR(ip) - return err == nil + return address.Is4() }