validate entry in ipset

pkg/proxy/ipvs/ipset.go

@@ -89,6 +89,10 @@ func NewIPSet(handle utilipset.Interface, name string, setType utilipset.Type, i
 	return set
 }
 
+func (set *IPSet) validateEntry(entry *utilipset.Entry) (bool, error) {
+	return entry.Validate(&set.IPSet)
+}
+
 func (set *IPSet) isEmpty() bool {
 	return len(set.activeEntries.UnsortedList()) == 0
 }

pkg/util/ipset/ipset.go

@@ -19,6 +19,7 @@ package ipset
 import (
 	"bytes"
 	"fmt"
+	"net"
 	"regexp"
 	"strconv"
 	"strings"
@@ -86,6 +87,7 @@ type IPSet struct {
 	MaxElem int
 	// PortRange specifies the port range of bitmap:port type ipset.
 	PortRange string
+	// TODO: add comment message for ipset
 }
 
 // Validate checks if a given ipset is valid or not.
@@ -133,10 +135,96 @@ type Entry struct {
 	Net string
 	// IP2 is the entry's second IP.  IP2 may not be empty for `hash:ip,port,ip` type ip set.
 	IP2 string
-	// SetType specifies the type of ip set where the entry exists.
+	// SetType is the type of ipset where the entry exists.
 	SetType Type
 }
 
+// Validate checks if a given ipset entry is valid or not.  The set parameter is the ipset that entry belongs to.
+func (e *Entry) Validate(set *IPSet) (bool, error) {
+	switch e.SetType {
+	case HashIPPort:
+		// set default protocol to tcp if empty
+		if len(e.Protocol) == 0 {
+			e.Protocol = ProtocolTCP
+		}
+
+		if net.ParseIP(e.IP) == nil {
+			return false, fmt.Errorf("error parsing entry's ip address %v for %s type set", e.IP, HashIPPort)
+		}
+
+		if valid := validateProtocol(e.Protocol); !valid {
+			return false, fmt.Errorf("invalid entry's protocol: %s, supported protocols are [%s, %s]", e.Protocol, ProtocolTCP, ProtocolUDP)
+		}
+
+		if e.Port < 0 {
+			return false, fmt.Errorf("port number %d should be >=0 ", e.Port)
+		}
+	case HashIPPortIP:
+		// set default protocol to tcp if empty
+		if len(e.Protocol) == 0 {
+			e.Protocol = ProtocolTCP
+		}
+
+		if net.ParseIP(e.IP) == nil {
+			return false, fmt.Errorf("error parsing entry's ip address %v for %s type set", e.IP, HashIPPortIP)
+		}
+
+		if valid := validateProtocol(e.Protocol); !valid {
+			return false, fmt.Errorf("invalid entry's protocol, supported protocols are [%s, %s]", ProtocolTCP, ProtocolUDP)
+		}
+
+		if e.Port < 0 {
+			return false, fmt.Errorf("port number %d should be >=0 ", e.Port)
+		}
+
+		// IP2 can not be empty for `hash:ip,port,ip` type ip set
+		if net.ParseIP(e.IP2) == nil {
+			return false, fmt.Errorf("error parsing entry's second ip address %v for %s type set", e.IP2, HashIPPortIP)
+		}
+	case HashIPPortNet:
+		// set default protocol to tcp if empty
+		if len(e.Protocol) == 0 {
+			e.Protocol = ProtocolTCP
+		}
+
+		if net.ParseIP(e.IP) == nil {
+			return false, fmt.Errorf("error parsing entry's ip address %v for %s type set", e.IP, HashIPPortIP)
+		}
+
+		if valid := validateProtocol(e.Protocol); !valid {
+			return false, fmt.Errorf("invalid entry's protocol, supported protocols are [%s, %s]", ProtocolTCP, ProtocolUDP)
+		}
+
+		if e.Port < 0 {
+			return false, fmt.Errorf("port number %d should be >=0 ", e.Port)
+		}
+
+		// Net can not be empty for `hash:ip,port,net` type ip set
+		if _, ipNet, _ := net.ParseCIDR(e.Net); ipNet == nil {
+			return false, fmt.Errorf("error parsing entry's ip net %v for %s type set", e.Net, HashIPPortNet)
+		}
+	case BitmapPort:
+		if e.Port < 0 {
+			return false, fmt.Errorf("port number %d should be >=0 ", e.Port)
+		}
+
+		// check if port number satisfies its ipset's requirement of port range
+		if set == nil {
+			return false, fmt.Errorf("can not reference ip set where the entry exists")
+		}
+		begin, end, err := parsePortRange(set.PortRange)
+		if err != nil {
+			return false, err
+		}
+		if e.Port < begin || e.Port > end {
+			return false, fmt.Errorf("entry's port number %d is not in the port range %s of its ipset", e.Port, set.PortRange)
+		}
+	}
+
+	return true, nil
+}
+
+// String returns the string format for ipset entry.
 func (e *Entry) String() string {
 	switch e.SetType {
 	case HashIPPort:
@@ -172,28 +260,30 @@ func New(exec utilexec.Interface) Interface {
 
 // CreateSet creates a new set,  it will ignore error when the set already exists if ignoreExistErr=true.
 func (runner *runner) CreateSet(set *IPSet, ignoreExistErr bool) error {
-	// Using default values.
+	// Setting default values if not present
 	if set.HashSize == 0 {
 		set.HashSize = 1024
 	}
 	if set.MaxElem == 0 {
 		set.MaxElem = 65536
 	}
+	// Default protocol is IPv4
 	if set.HashFamily == "" {
 		set.HashFamily = ProtocolFamilyIPV4
 	}
-	if len(set.HashFamily) != 0 && set.HashFamily != ProtocolFamilyIPV4 && set.HashFamily != ProtocolFamilyIPV6 {
-		return fmt.Errorf("Currently supported protocol families are: %s and %s, %s is not supported", ProtocolFamilyIPV4, ProtocolFamilyIPV6, set.HashFamily)
-	}
 	// Default ipset type is "hash:ip,port"
 	if len(set.SetType) == 0 {
 		set.SetType = HashIPPort
 	}
-	// Check if setType is supported
+	if len(set.PortRange) == 0 {
-	if !IsValidIPSetType(set.SetType) {
+		set.PortRange = DefaultPortRange
-		return fmt.Errorf("Currently supported ipset types are: %v, %s is not supported", ValidIPSetTypes, set.SetType)
 	}
 
+	// Validate ipset before creating
+	valid, err := set.Validate()
+	if err != nil || !valid {
+		return fmt.Errorf("ipset is invalid because of %v", err)
+	}
 	return runner.createSet(set, ignoreExistErr)
 }
 
@@ -209,12 +299,6 @@ func (runner *runner) createSet(set *IPSet, ignoreExistErr bool) error {
 		)
 	}
 	if set.SetType == BitmapPort {
-		if len(set.PortRange) == 0 {
-			set.PortRange = DefaultPortRange
-		}
-		if !validatePortRange(set.PortRange) {
-			return fmt.Errorf("invalid port range for %s type ip set: %s, expect: a-b", BitmapPort, set.PortRange)
-		}
 		args = append(args, "range", set.PortRange)
 	}
 	if ignoreExistErr {
@@ -396,4 +480,48 @@ func IsNotFoundError(err error) bool {
 	return false
 }
 
+// checks if given hash family is supported in ipset
+func validateProtocol(protocol string) bool {
+	if protocol == ProtocolTCP || protocol == ProtocolUDP {
+		return true
+	}
+	return false
+}
+
+// parsePortRange parse the begin and end port from a raw string(format: a-b).  beginPort <= endPort
+// in the return value.
+func parsePortRange(portRange string) (beginPort int, endPort int, err error) {
+	if len(portRange) == 0 {
+		portRange = DefaultPortRange
+	}
+
+	strs := strings.Split(portRange, "-")
+	if len(strs) != 2 {
+		// port number -1 indicates invalid
+		return -1, -1, fmt.Errorf("port range should be in the format of `a-b`")
+	}
+	for i := range strs {
+		num, err := strconv.Atoi(strs[i])
+		if err != nil {
+			// port number -1 indicates invalid
+			return -1, -1, err
+		}
+		if num < 0 {
+			// port number -1 indicates invalid
+			return -1, -1, fmt.Errorf("port number %d should be >=0", num)
+		}
+		if i == 0 {
+			beginPort = num
+			continue
+		}
+		endPort = num
+		// switch when first port number > second port number
+		if beginPort > endPort {
+			endPort = beginPort
+			beginPort = num
+		}
+	}
+	return beginPort, endPort, nil
+}
+
 var _ = Interface(&runner{})

pkg/util/ipset/ipset_test.go

@@ -312,6 +312,7 @@ func TestDelEntry(t *testing.T) {
 		},
 	}
 	runner := New(&fexec)
+
 	err := runner.DelEntry(testEntry.String(), "FOOBAR")
 	if err != nil {
 		t.Errorf("expected success, got %v", err)
@@ -654,3 +655,648 @@ func Test_validateFamily(t *testing.T) {
 		}
 	}
 }
+
+func Test_validateProtocol(t *testing.T) {
+	testCases := []struct {
+		protocol string
+		valid    bool
+		desc     string
+	}{
+		{ // case[0]
+			protocol: "foo",
+			valid:    false,
+		},
+		{ // case[1]
+			protocol: ProtocolTCP,
+			valid:    true,
+		},
+		{ // case[2]
+			protocol: ProtocolUDP,
+			valid:    true,
+		},
+		{ // case[3]
+			protocol: "ipv4",
+			valid:    false,
+		},
+		{ // case[4]
+			protocol: "ipv6",
+			valid:    false,
+		},
+		{ // case[5]
+			protocol: "TCP",
+			valid:    false,
+			desc:     "should be low case",
+		},
+		{ // case[6]
+			protocol: "UDP",
+			valid:    false,
+			desc:     "should be low case",
+		},
+		{ // case[7]
+			protocol: "",
+			valid:    false,
+		},
+	}
+	for i := range testCases {
+		valid := validateProtocol(testCases[i].protocol)
+		if valid != testCases[i].valid {
+			t.Errorf("case [%d]: unexpected mismatch, expect valid[%v], got valid[%v], desc: %s", i, testCases[i].valid, valid, testCases[i].desc)
+		}
+	}
+}
+
+func TestValidateIPSet(t *testing.T) {
+	testCases := []struct {
+		ipset *IPSet
+		valid bool
+		desc  string
+	}{
+		{ // case[0]
+			ipset: &IPSet{
+				Name:       "test",
+				SetType:    HashIPPort,
+				HashFamily: ProtocolFamilyIPV4,
+				HashSize:   1024,
+				MaxElem:    1024,
+			},
+			valid: true,
+		},
+		{ // case[1]
+			ipset: &IPSet{
+				Name:       "SET",
+				SetType:    BitmapPort,
+				HashFamily: ProtocolFamilyIPV6,
+				HashSize:   65535,
+				MaxElem:    2048,
+				PortRange:  DefaultPortRange,
+			},
+			valid: true,
+		},
+		{ // case[2]
+			ipset: &IPSet{
+				Name:       "foo",
+				SetType:    BitmapPort,
+				HashFamily: ProtocolFamilyIPV6,
+				HashSize:   65535,
+				MaxElem:    2048,
+			},
+			valid: false,
+			desc:  "should specify right port range for bitmap type set",
+		},
+		{ // case[3]
+			ipset: &IPSet{
+				Name:       "bar",
+				SetType:    BitmapPort,
+				HashFamily: ProtocolFamilyIPV6,
+				HashSize:   0,
+				MaxElem:    2048,
+			},
+			valid: false,
+			desc:  "wrong hash size number",
+		},
+		{ // case[4]
+			ipset: &IPSet{
+				Name:       "baz",
+				SetType:    BitmapPort,
+				HashFamily: ProtocolFamilyIPV6,
+				HashSize:   1024,
+				MaxElem:    -1,
+			},
+			valid: false,
+			desc:  "wrong hash max elem number",
+		},
+		{ // case[5]
+			ipset: &IPSet{
+				Name:       "baz",
+				SetType:    HashIPPortNet,
+				HashFamily: "ip",
+				HashSize:   1024,
+				MaxElem:    1024,
+			},
+			valid: false,
+			desc:  "wrong protocol",
+		},
+		{ // case[6]
+			ipset: &IPSet{
+				Name:       "foo-bar",
+				SetType:    "xxx",
+				HashFamily: ProtocolFamilyIPV4,
+				HashSize:   1024,
+				MaxElem:    1024,
+			},
+			valid: false,
+			desc:  "wrong set type",
+		},
+	}
+	for i := range testCases {
+		valid, _ := testCases[i].ipset.Validate()
+		if valid != testCases[i].valid {
+			t.Errorf("case [%d]: unexpected mismatch, expect valid[%v], got valid[%v], desc: %s", i, testCases[i].valid, valid, testCases[i].desc)
+		}
+	}
+}
+
+func Test_parsePortRange(t *testing.T) {
+	testCases := []struct {
+		portRange string
+		expectErr bool
+		beginPort int
+		endPort   int
+		desc      string
+	}{
+		{ // case[0]
+			portRange: "1-100",
+			expectErr: false,
+			beginPort: 1,
+			endPort:   100,
+		},
+		{ // case[1]
+			portRange: "0-0",
+			expectErr: false,
+			beginPort: 0,
+			endPort:   0,
+		},
+		{ // case[2]
+			portRange: "100-10",
+			expectErr: false,
+			beginPort: 10,
+			endPort:   100,
+		},
+		{ // case[3]
+			portRange: "1024",
+			expectErr: true,
+			desc:      "single port number is not allowed",
+		},
+		{ // case[4]
+			portRange: DefaultPortRange,
+			expectErr: false,
+			beginPort: 0,
+			endPort:   65535,
+		},
+		{ // case[5]
+			portRange: "1-",
+			expectErr: true,
+			desc:      "should specify end port",
+		},
+		{ // case[6]
+			portRange: "-100",
+			expectErr: true,
+			desc:      "should specify begin port",
+		},
+		{ // case[7]
+			portRange: "1:100",
+			expectErr: true,
+			desc:      "delimiter should be -",
+		},
+		{ // case[8]
+			portRange: "1~100",
+			expectErr: true,
+			desc:      "delimiter should be -",
+		},
+		{ // case[9]
+			portRange: "1,100",
+			expectErr: true,
+			desc:      "delimiter should be -",
+		},
+		{ // case[10]
+			portRange: "100-100",
+			expectErr: false,
+			desc:      "begin port number can be equal to end port number",
+			beginPort: 100,
+			endPort:   100,
+		},
+		{ // case[11]
+			portRange: "",
+			expectErr: false,
+			desc:      "empty string indicates default port range",
+			beginPort: 0,
+			endPort:   65535,
+		},
+		{ // case[12]
+			portRange: "-1-12",
+			expectErr: true,
+			desc:      "port number can not be negative value",
+		},
+		{ // case[13]
+			portRange: "-1--8",
+			expectErr: true,
+			desc:      "port number can not be negative value",
+		},
+	}
+	for i := range testCases {
+		begin, end, err := parsePortRange(testCases[i].portRange)
+		if err != nil {
+			if !testCases[i].expectErr {
+				t.Errorf("case [%d]: unexpected err: %v, desc: %s", i, err, testCases[i].desc)
+			}
+			continue
+		}
+		if begin != testCases[i].beginPort || end != testCases[i].endPort {
+			t.Errorf("case [%d]: unexpected mismatch [beginPort, endPort] pair, expect [%d, %d], got [%d, %d], desc: %s", i, testCases[i].beginPort, testCases[i].endPort, begin, end, testCases[i].desc)
+		}
+	}
+}
+
+// This is a coarse test, but it offers some modicum of confidence as the code is evolved.
+func TestValidateEntry(t *testing.T) {
+	testCases := []struct {
+		entry *Entry
+		set   *IPSet
+		valid bool
+		desc  string
+	}{
+		{ // case[0]
+			entry: &Entry{
+				SetType: BitmapPort,
+			},
+			set: &IPSet{
+				PortRange: DefaultPortRange,
+			},
+			valid: true,
+			desc:  "port number can be empty, default is 0. And port number is in the range of its ipset's port range",
+		},
+		{ // case[1]
+			entry: &Entry{
+				SetType: BitmapPort,
+				Port:    0,
+			},
+			set: &IPSet{
+				PortRange: DefaultPortRange,
+			},
+			valid: true,
+			desc:  "port number can be 0. And port number is in the range of its ipset's port range",
+		},
+		{ // case[2]
+			entry: &Entry{
+				SetType: BitmapPort,
+				Port:    -1,
+			},
+			valid: false,
+			desc:  "port number can not be negative value",
+		},
+		{ // case[3]
+			entry: &Entry{
+				SetType: BitmapPort,
+				Port:    1080,
+			},
+			set: &IPSet{
+				Name:      "baz",
+				PortRange: DefaultPortRange,
+			},
+			desc:  "port number is in the range of its ipset's port range",
+			valid: true,
+		},
+		{ // case[4]
+			entry: &Entry{
+				SetType: BitmapPort,
+				Port:    1080,
+			},
+			set: &IPSet{
+				Name:      "foo",
+				PortRange: "0-1079",
+			},
+			desc:  "port number is NOT in the range of its ipset's port range",
+			valid: false,
+		},
+		{ // case[5]
+			entry: &Entry{
+				SetType:  HashIPPort,
+				IP:       "1.2.3.4",
+				Protocol: ProtocolTCP,
+				Port:     8080,
+			},
+			set: &IPSet{
+				Name: "bar",
+			},
+			valid: true,
+		},
+		{ // case[6]
+			entry: &Entry{
+				SetType:  HashIPPort,
+				IP:       "1.2.3.4",
+				Protocol: ProtocolUDP,
+				Port:     0,
+			},
+			set: &IPSet{
+				Name: "bar",
+			},
+			valid: true,
+		},
+		{ // case[7]
+			entry: &Entry{
+				SetType:  HashIPPort,
+				IP:       "FE80:0000:0000:0000:0202:B3FF:FE1E:8329",
+				Protocol: ProtocolTCP,
+				Port:     1111,
+			},
+			set: &IPSet{
+				Name: "ipv6",
+			},
+			valid: true,
+		},
+		{ // case[8]
+			entry: &Entry{
+				SetType:  HashIPPort,
+				IP:       "",
+				Protocol: ProtocolTCP,
+				Port:     1234,
+			},
+			set: &IPSet{
+				Name: "empty-ip",
+			},
+			valid: false,
+		},
+		{ // case[9]
+			entry: &Entry{
+				SetType:  HashIPPort,
+				IP:       "1-2-3-4",
+				Protocol: ProtocolTCP,
+				Port:     8900,
+			},
+			set: &IPSet{
+				Name: "bad-ip",
+			},
+			valid: false,
+		},
+		{ // case[10]
+			entry: &Entry{
+				SetType:  HashIPPort,
+				IP:       "10.20.30.40",
+				Protocol: "",
+				Port:     8090,
+			},
+			set: &IPSet{
+				Name: "empty-protocol",
+			},
+			valid: true,
+		},
+		{ // case[11]
+			entry: &Entry{
+				SetType:  HashIPPort,
+				IP:       "10.20.30.40",
+				Protocol: "ICMP",
+				Port:     8090,
+			},
+			set: &IPSet{
+				Name: "unsupported-protocol",
+			},
+			valid: false,
+		},
+		{ // case[11]
+			entry: &Entry{
+				SetType:  HashIPPort,
+				IP:       "10.20.30.40",
+				Protocol: "ICMP",
+				Port:     -1,
+			},
+			set: &IPSet{
+				// TODO: set name string with white space?
+				Name: "negative-port-number",
+			},
+			valid: false,
+		},
+		{ // case[12]
+			entry: &Entry{
+				SetType:  HashIPPortIP,
+				IP:       "10.20.30.40",
+				Protocol: ProtocolUDP,
+				Port:     53,
+				IP2:      "10.20.30.40",
+			},
+			set: &IPSet{
+				Name: "LOOP-BACK",
+			},
+			valid: true,
+		},
+		{ // case[13]
+			entry: &Entry{
+				SetType:  HashIPPortIP,
+				IP:       "10.20.30.40",
+				Protocol: ProtocolUDP,
+				Port:     53,
+				IP2:      "",
+			},
+			set: &IPSet{
+				Name: "empty IP2",
+			},
+			valid: false,
+		},
+		{ // case[14]
+			entry: &Entry{
+				SetType:  HashIPPortIP,
+				IP:       "10.20.30.40",
+				Protocol: ProtocolUDP,
+				Port:     53,
+				IP2:      "foo",
+			},
+			set: &IPSet{
+				Name: "invalid IP2",
+			},
+			valid: false,
+		},
+		{ // case[15]
+			entry: &Entry{
+				SetType:  HashIPPortIP,
+				IP:       "10.20.30.40",
+				Protocol: ProtocolTCP,
+				Port:     0,
+				IP2:      "1.2.3.4",
+			},
+			set: &IPSet{
+				Name: "zero port",
+			},
+			valid: true,
+		},
+		{ // case[16]
+			entry: &Entry{
+				SetType:  HashIPPortIP,
+				IP:       "10::40",
+				Protocol: ProtocolTCP,
+				Port:     10000,
+				IP2:      "1::4",
+			},
+			set: &IPSet{
+				Name: "IPV6",
+				// TODO: check set's hash family
+			},
+			valid: true,
+		},
+		{ // case[17]
+			entry: &Entry{
+				SetType:  HashIPPortIP,
+				IP:       "",
+				Protocol: ProtocolTCP,
+				Port:     1234,
+				IP2:      "1.2.3.4",
+			},
+			set: &IPSet{
+				Name: "empty-ip",
+			},
+			valid: false,
+		},
+		{ // case[18]
+			entry: &Entry{
+				SetType:  HashIPPortIP,
+				IP:       "1-2-3-4",
+				Protocol: ProtocolTCP,
+				Port:     8900,
+				IP2:      "10.20.30.41",
+			},
+			set: &IPSet{
+				Name: "bad-ip",
+			},
+			valid: false,
+		},
+		{ // case[19]
+			entry: &Entry{
+				SetType: HashIPPortIP,
+				IP:      "10.20.30.40",
+				Protocol: "SCTP	",
+				Port: 8090,
+				IP2:  "10.20.30.41",
+			},
+			set: &IPSet{
+				Name: "unsupported-protocol",
+			},
+			valid: false,
+		},
+		{ // case[20]
+			entry: &Entry{
+				SetType:  HashIPPortIP,
+				IP:       "10.20.30.40",
+				Protocol: "ICMP",
+				Port:     -1,
+				IP2:      "100.200.30.41",
+			},
+			set: &IPSet{
+				Name: "negative-port-number",
+			},
+			valid: false,
+		},
+		{ // case[21]
+			entry: &Entry{
+				SetType:  HashIPPortNet,
+				IP:       "10.20.30.40",
+				Protocol: ProtocolTCP,
+				Port:     53,
+				// TODO: CIDR /32 may not be valid
+				Net: "10.20.30.0/24",
+			},
+			set: &IPSet{
+				Name: "abc",
+			},
+			valid: true,
+		},
+		{ // case[22]
+			entry: &Entry{
+				SetType:  HashIPPortNet,
+				IP:       "11.21.31.41",
+				Protocol: ProtocolUDP,
+				Port:     1122,
+				Net:      "",
+			},
+			set: &IPSet{
+				Name: "empty Net",
+			},
+			valid: false,
+		},
+		{ // case[23]
+			entry: &Entry{
+				SetType:  HashIPPortNet,
+				IP:       "10.20.30.40",
+				Protocol: ProtocolUDP,
+				Port:     8080,
+				Net:      "x-y-z-w",
+			},
+			set: &IPSet{
+				Name: "invalid Net",
+			},
+			valid: false,
+		},
+		{ // case[24]
+			entry: &Entry{
+				SetType:  HashIPPortNet,
+				IP:       "10.20.30.40",
+				Protocol: ProtocolTCP,
+				Port:     0,
+				Net:      "10.1.0.0/16",
+			},
+			set: &IPSet{
+				Name: "zero port",
+			},
+			valid: true,
+		},
+		{ // case[25]
+			entry: &Entry{
+				SetType:  HashIPPortNet,
+				IP:       "10::40",
+				Protocol: ProtocolTCP,
+				Port:     80,
+				Net:      "2001:db8::/32",
+			},
+			set: &IPSet{
+				Name: "IPV6",
+				// TODO: check set's hash family
+			},
+			valid: true,
+		},
+		{ // case[26]
+			entry: &Entry{
+				SetType:  HashIPPortNet,
+				IP:       "",
+				Protocol: ProtocolTCP,
+				Port:     1234,
+				Net:      "1.2.3.4/22",
+			},
+			set: &IPSet{
+				Name: "empty-ip",
+			},
+			valid: false,
+		},
+		{ // case[27]
+			entry: &Entry{
+				SetType:  HashIPPortNet,
+				IP:       "1-2-3-4",
+				Protocol: ProtocolTCP,
+				Port:     8900,
+				Net:      "10.20.30.41/31",
+			},
+			set: &IPSet{
+				Name: "bad-ip",
+			},
+			valid: false,
+		},
+		{ // case[28]
+			entry: &Entry{
+				SetType:  HashIPPortIP,
+				IP:       "10.20.30.40",
+				Protocol: "FOO",
+				Port:     8090,
+				IP2:      "10.20.30.0/10",
+			},
+			set: &IPSet{
+				Name: "unsupported-protocol",
+			},
+			valid: false,
+		},
+		{ // case[29]
+			entry: &Entry{
+				SetType:  HashIPPortIP,
+				IP:       "10.20.30.40",
+				Protocol: ProtocolUDP,
+				Port:     -1,
+				IP2:      "100.200.30.0/12",
+			},
+			set: &IPSet{
+				Name: "negative-port-number",
+			},
+			valid: false,
+		},
+	}
+	for i := range testCases {
+		valid, _ := testCases[i].entry.Validate(testCases[i].set)
+		if valid != testCases[i].valid {
+			t.Errorf("case [%d]: unexpected mismatch, expect valid[%v], got valid[%v], desc: %s", i, testCases[i].valid, valid, testCases[i].entry)
+		}
+	}
+}

pkg/util/ipset/types.go

@@ -58,13 +58,3 @@ var ValidIPSetTypes = []Type{
 	BitmapPort,
 	HashIPPortNet,
 }
-
-// IsValidIPSetType checks if the given ipset type is valid.
-func IsValidIPSetType(set Type) bool {
-	for _, valid := range ValidIPSetTypes {
-		if set == valid {
-			return true
-		}
-	}
-	return false
-}