1
0
Fork 0
mirror of https://github.com/kubernetes-sigs/node-feature-discovery.git synced 2024-12-14 11:57:51 +00:00

apis/nfd: validate input when matching expression

Don't assume that the fields are correct.
This commit is contained in:
Markus Lehtonen 2023-12-01 09:09:59 +02:00
parent ea1264ae68
commit b988139094
2 changed files with 121 additions and 48 deletions

View file

@ -123,12 +123,25 @@ func (m *MatchExpression) Validate() error {
// Match evaluates the MatchExpression against a single input value.
func (m *MatchExpression) Match(valid bool, value interface{}) (bool, error) {
if _, ok := matchOps[m.Op]; !ok {
return false, fmt.Errorf("invalid Op %q", m.Op)
}
switch m.Op {
case MatchAny:
if len(m.Value) != 0 {
return false, fmt.Errorf("invalid expression, 'value' field must be empty for Op %q (have %v)", m.Op, m.Value)
}
return true, nil
case MatchExists:
if len(m.Value) != 0 {
return false, fmt.Errorf("invalid expression, 'value' field must be empty for Op %q (have %v)", m.Op, m.Value)
}
return valid, nil
case MatchDoesNotExist:
if len(m.Value) != 0 {
return false, fmt.Errorf("invalid expression, 'value' field must be empty for Op %q (have %v)", m.Op, m.Value)
}
return !valid, nil
}
@ -136,12 +149,18 @@ func (m *MatchExpression) Match(valid bool, value interface{}) (bool, error) {
value := fmt.Sprintf("%v", value)
switch m.Op {
case MatchIn:
if len(m.Value) == 0 {
return false, fmt.Errorf("invalid expression, 'value' field must be non-empty for Op %q", m.Op)
}
for _, v := range m.Value {
if value == v {
return true, nil
}
}
case MatchNotIn:
if len(m.Value) == 0 {
return false, fmt.Errorf("invalid expression, 'value' field must be non-empty for Op %q", m.Op)
}
for _, v := range m.Value {
if value == v {
return false, nil
@ -149,8 +168,19 @@ func (m *MatchExpression) Match(valid bool, value interface{}) (bool, error) {
}
return true, nil
case MatchInRegexp:
if len(m.Value) == 0 {
return false, fmt.Errorf("invalid expression, 'value' field must be non-empty for Op %q", m.Op)
}
if m.valueRe == nil {
return false, fmt.Errorf("BUG: MatchExpression has not been initialized properly, regexps missing")
m.valueRe = make([]*regexp.Regexp, len(m.Value))
for i, v := range m.Value {
re, err := regexp.Compile(v)
if err != nil {
m.valueRe = nil
return false, fmt.Errorf("invalid expressiom, 'value' field must only contain valid regexps for Op %q (have %v)", m.Op, m.Value)
}
m.valueRe[i] = re
}
}
for _, re := range m.valueRe {
if re.MatchString(value) {
@ -158,6 +188,10 @@ func (m *MatchExpression) Match(valid bool, value interface{}) (bool, error) {
}
}
case MatchGt, MatchLt:
if len(m.Value) != 1 {
return false, fmt.Errorf("invalid expression, 'value' field must contain exactly one element for Op %q (have %v)", m.Op, m.Value)
}
l, err := strconv.Atoi(value)
if err != nil {
return false, fmt.Errorf("not a number %q", value)
@ -171,6 +205,9 @@ func (m *MatchExpression) Match(valid bool, value interface{}) (bool, error) {
return true, nil
}
case MatchGtLt:
if len(m.Value) != 2 {
return false, fmt.Errorf("invalid expression, value' field must contain exactly two elements for Op %q (have %v)", m.Op, m.Value)
}
v, err := strconv.Atoi(value)
if err != nil {
return false, fmt.Errorf("not a number %q", value)
@ -182,10 +219,19 @@ func (m *MatchExpression) Match(valid bool, value interface{}) (bool, error) {
return false, fmt.Errorf("not a number %q in %v", m.Value[i], m)
}
}
if lr[0] >= lr[1] {
return false, fmt.Errorf("invalid expression, value[0] must be less than Value[1] for Op %q (have %v)", m.Op, m.Value)
}
return v > lr[0] && v < lr[1], nil
case MatchIsTrue:
if len(m.Value) != 0 {
return false, fmt.Errorf("invalid expression, 'value' field must be empty for Op %q (have %v)", m.Op, m.Value)
}
return value == "true", nil
case MatchIsFalse:
if len(m.Value) != 0 {
return false, fmt.Errorf("invalid expression, 'value' field must be empty for Op %q (have %v)", m.Op, m.Value)
}
return value == "false", nil
default:
return false, fmt.Errorf("unsupported Op %q", m.Op)

View file

@ -104,59 +104,55 @@ func TestMatch(t *testing.T) {
input interface{}
valid bool
result BoolAssertionFunc
err ValueAssertionFunc
}
tcs := []TC{
{name: "1", op: api.MatchAny, result: assert.True, err: assert.Nil},
{name: "2", op: api.MatchAny, input: "2", valid: false, result: assert.True, err: assert.Nil},
{name: "MatchAny-1", op: api.MatchAny, result: assert.True},
{name: "MatchAny-2", op: api.MatchAny, input: "2", valid: false, result: assert.True},
{name: "3", op: api.MatchIn, values: V{"1"}, input: "2", valid: false, result: assert.False, err: assert.Nil},
{name: "4", op: api.MatchIn, values: V{"1"}, input: "2", valid: true, result: assert.False, err: assert.Nil},
{name: "5", op: api.MatchIn, values: V{"1", "2", "3"}, input: "2", valid: false, result: assert.False, err: assert.Nil},
{name: "6", op: api.MatchIn, values: V{"1", "2", "3"}, input: "2", valid: true, result: assert.True, err: assert.Nil},
{name: "MatchIn-1", op: api.MatchIn, values: V{"1"}, input: "2", valid: false, result: assert.False},
{name: "MatchIn-2", op: api.MatchIn, values: V{"1"}, input: "2", valid: true, result: assert.False},
{name: "MatchIn-3", op: api.MatchIn, values: V{"1", "2", "3"}, input: "2", valid: false, result: assert.False},
{name: "MatchIn-4", op: api.MatchIn, values: V{"1", "2", "3"}, input: "2", valid: true, result: assert.True},
{name: "7", op: api.MatchNotIn, values: V{"2"}, input: 2, valid: false, result: assert.False, err: assert.Nil},
{name: "8", op: api.MatchNotIn, values: V{"1"}, input: 2, valid: true, result: assert.True, err: assert.Nil},
{name: "9", op: api.MatchNotIn, values: V{"1", "2", "3"}, input: "2", valid: false, result: assert.False, err: assert.Nil},
{name: "10", op: api.MatchNotIn, values: V{"1", "2", "3"}, input: "2", valid: true, result: assert.False, err: assert.Nil},
{name: "MatchNotIn-1", op: api.MatchNotIn, values: V{"2"}, input: 2, valid: false, result: assert.False},
{name: "MatchNotIn-2", op: api.MatchNotIn, values: V{"1"}, input: 2, valid: true, result: assert.True},
{name: "MatchNotIn-3", op: api.MatchNotIn, values: V{"1", "2", "3"}, input: "2", valid: false, result: assert.False},
{name: "MatchNotIn-4", op: api.MatchNotIn, values: V{"1", "2", "3"}, input: "2", valid: true, result: assert.False},
{name: "11", op: api.MatchInRegexp, values: V{"val-[0-9]$"}, input: "val-1", valid: false, result: assert.False, err: assert.Nil},
{name: "12", op: api.MatchInRegexp, values: V{"val-[0-9]$"}, input: "val-1", valid: true, result: assert.True, err: assert.Nil},
{name: "13", op: api.MatchInRegexp, values: V{"val-[0-9]$"}, input: "val-12", valid: true, result: assert.False, err: assert.Nil},
{name: "14", op: api.MatchInRegexp, values: V{"val-[0-9]$", "al-[1-9]"}, input: "val-12", valid: true, result: assert.True, err: assert.Nil},
{name: "MatchInRegexp-1", op: api.MatchInRegexp, values: V{"val-[0-9]$"}, input: "val-1", valid: false, result: assert.False},
{name: "MatchInRegexp-2", op: api.MatchInRegexp, values: V{"val-[0-9]$"}, input: "val-1", valid: true, result: assert.True},
{name: "MatchInRegexp-3", op: api.MatchInRegexp, values: V{"val-[0-9]$"}, input: "val-12", valid: true, result: assert.False},
{name: "MatchInRegexp-4", op: api.MatchInRegexp, values: V{"val-[0-9]$", "al-[1-9]"}, input: "val-12", valid: true, result: assert.True},
{name: "15", op: api.MatchExists, input: nil, valid: false, result: assert.False, err: assert.Nil},
{name: "16", op: api.MatchExists, input: nil, valid: true, result: assert.True, err: assert.Nil},
{name: "MatchExists-1", op: api.MatchExists, input: nil, valid: false, result: assert.False},
{name: "MatchExists-2", op: api.MatchExists, input: nil, valid: true, result: assert.True},
{name: "17", op: api.MatchDoesNotExist, input: false, valid: false, result: assert.True, err: assert.Nil},
{name: "18", op: api.MatchDoesNotExist, input: false, valid: true, result: assert.False, err: assert.Nil},
{name: "MatchDoesNotExist-1", op: api.MatchDoesNotExist, input: false, valid: false, result: assert.True},
{name: "MatchDoesNotExist-2", op: api.MatchDoesNotExist, input: false, valid: true, result: assert.False},
{name: "19", op: api.MatchGt, values: V{"2"}, input: 3, valid: false, result: assert.False, err: assert.Nil},
{name: "20", op: api.MatchGt, values: V{"2"}, input: 2, valid: true, result: assert.False, err: assert.Nil},
{name: "21", op: api.MatchGt, values: V{"2"}, input: 3, valid: true, result: assert.True, err: assert.Nil},
{name: "22", op: api.MatchGt, values: V{"-10"}, input: -3, valid: true, result: assert.True, err: assert.Nil},
{name: "23", op: api.MatchGt, values: V{"2"}, input: "3a", valid: true, result: assert.False, err: assert.NotNil},
{name: "MatchGt-1", op: api.MatchGt, values: V{"2"}, input: 3, valid: false, result: assert.False},
{name: "MatchGt-2", op: api.MatchGt, values: V{"2"}, input: 2, valid: true, result: assert.False},
{name: "MatchGt-3", op: api.MatchGt, values: V{"2"}, input: 3, valid: true, result: assert.True},
{name: "MatchGt-4", op: api.MatchGt, values: V{"-10"}, input: -3, valid: true, result: assert.True},
{name: "24", op: api.MatchLt, values: V{"2"}, input: "1", valid: false, result: assert.False, err: assert.Nil},
{name: "25", op: api.MatchLt, values: V{"2"}, input: "2", valid: true, result: assert.False, err: assert.Nil},
{name: "26", op: api.MatchLt, values: V{"-10"}, input: -3, valid: true, result: assert.False, err: assert.Nil},
{name: "27", op: api.MatchLt, values: V{"2"}, input: "1", valid: true, result: assert.True, err: assert.Nil},
{name: "28", op: api.MatchLt, values: V{"2"}, input: "1.0", valid: true, result: assert.False, err: assert.NotNil},
{name: "MatchLt-1", op: api.MatchLt, values: V{"2"}, input: "1", valid: false, result: assert.False},
{name: "MatchLt-2", op: api.MatchLt, values: V{"2"}, input: "2", valid: true, result: assert.False},
{name: "MatchLt-3", op: api.MatchLt, values: V{"-10"}, input: -3, valid: true, result: assert.False},
{name: "MatchLt-4", op: api.MatchLt, values: V{"2"}, input: "1", valid: true, result: assert.True},
{name: "29", op: api.MatchGtLt, values: V{"1", "10"}, input: "1", valid: false, result: assert.False, err: assert.Nil},
{name: "30", op: api.MatchGtLt, values: V{"1", "10"}, input: "1", valid: true, result: assert.False, err: assert.Nil},
{name: "31", op: api.MatchGtLt, values: V{"1", "10"}, input: "10", valid: true, result: assert.False, err: assert.Nil},
{name: "32", op: api.MatchGtLt, values: V{"1", "10"}, input: "2", valid: true, result: assert.True, err: assert.Nil},
{name: "33", op: api.MatchGtLt, values: V{"1", "10"}, input: "1.0", valid: true, result: assert.False, err: assert.NotNil},
{name: "MatchGtLt-1", op: api.MatchGtLt, values: V{"1", "10"}, input: "1", valid: false, result: assert.False},
{name: "MatchGtLt-2", op: api.MatchGtLt, values: V{"1", "10"}, input: "1", valid: true, result: assert.False},
{name: "MatchGtLt-3", op: api.MatchGtLt, values: V{"1", "10"}, input: "10", valid: true, result: assert.False},
{name: "MatchGtLt-4", op: api.MatchGtLt, values: V{"1", "10"}, input: "2", valid: true, result: assert.True},
{name: "34", op: api.MatchIsTrue, input: true, valid: false, result: assert.False, err: assert.Nil},
{name: "35", op: api.MatchIsTrue, input: true, valid: true, result: assert.True, err: assert.Nil},
{name: "36", op: api.MatchIsTrue, input: false, valid: true, result: assert.False, err: assert.Nil},
{name: "MatchIsTrue-1", op: api.MatchIsTrue, input: true, valid: false, result: assert.False},
{name: "MatchIsTrue-2", op: api.MatchIsTrue, input: true, valid: true, result: assert.True},
{name: "MatchIsTrue-3", op: api.MatchIsTrue, input: false, valid: true, result: assert.False},
{name: "37", op: api.MatchIsFalse, input: "false", valid: false, result: assert.False, err: assert.Nil},
{name: "38", op: api.MatchIsFalse, input: "false", valid: true, result: assert.True, err: assert.Nil},
{name: "39", op: api.MatchIsFalse, input: "true", valid: true, result: assert.False, err: assert.Nil},
{name: "MatchIsFalse-1", op: api.MatchIsFalse, input: "false", valid: false, result: assert.False},
{name: "MatchIsFalse-2", op: api.MatchIsFalse, input: "false", valid: true, result: assert.True},
{name: "MatchIsFalse-3", op: api.MatchIsFalse, input: "true", valid: true, result: assert.False},
}
for _, tc := range tcs {
@ -164,22 +160,53 @@ func TestMatch(t *testing.T) {
me := api.MustCreateMatchExpression(tc.op, tc.values...)
res, err := me.Match(tc.valid, tc.input)
tc.result(t, res)
tc.err(t, err)
assert.Nil(t, err)
})
}
// Check some special error cases separately because MustCreateMatch panics
// Error cases
tcs = []TC{
{name: "err-1", op: api.MatchGt, values: V{"3.0"}, input: 1, valid: true},
{name: "err-2", op: api.MatchLt, values: V{"0x2"}, input: 1, valid: true},
{name: "err-3", op: api.MatchGtLt, values: V{"1", "str"}, input: 1, valid: true},
{name: "err-4", op: "non-existent-op", values: V{"1"}, input: 1, valid: true},
{name: "MatchAny-err-1", op: api.MatchAny, values: V{"1"}, input: "val"},
{name: "MatchIn-err-1", op: api.MatchIn, input: "val"},
{name: "MatchNotIn-err-1", op: api.MatchNotIn, input: "val"},
{name: "MatchInRegexp-err-1", op: api.MatchInRegexp, input: "val"},
{name: "MatchInRegexp-err-2", op: api.MatchInRegexp, values: V{"("}, input: "val"},
{name: "MatchExists-err-1", op: api.MatchExists, values: V{"1"}},
{name: "MatchDoesNotExist-err-1", op: api.MatchDoesNotExist, values: V{"1"}},
{name: "MatchGt-err-1", op: api.MatchGt, input: "1"},
{name: "MatchGt-err-2", op: api.MatchGt, values: V{"1", "2"}, input: "1"},
{name: "MatchGt-err-3", op: api.MatchGt, values: V{""}, input: "1"},
{name: "MatchGt-err-4", op: api.MatchGt, values: V{"2"}, input: "3a"},
{name: "MatchLt-err-1", op: api.MatchLt, input: "1"},
{name: "MatchLt-err-2", op: api.MatchLt, values: V{"1", "2", "3"}, input: "1"},
{name: "MatchLt-err-3", op: api.MatchLt, values: V{"a"}, input: "1"},
{name: "MatchLt-err-4", op: api.MatchLt, values: V{"2"}, input: "1.0"},
{name: "MatchGtLt-err-1", op: api.MatchGtLt, input: "1"},
{name: "MatchGtLt-err-2", op: api.MatchGtLt, values: V{"1"}, input: "1"},
{name: "MatchGtLt-err-3", op: api.MatchGtLt, values: V{"2", "1"}, input: "1"},
{name: "MatchGtLt-err-4", op: api.MatchGtLt, values: V{"1", "2", "3"}, input: "1"},
{name: "MatchGtLt-err-5", op: api.MatchGtLt, values: V{"a", "2"}, input: "1"},
{name: "MatchGtLt-err-6", op: api.MatchGtLt, values: V{"1", "10"}, input: "1.0"},
{name: "MatchIsTrue-err-1", op: api.MatchIsTrue, values: V{"1"}, input: "true"},
{name: "MatchIsFalse-err-1", op: api.MatchIsFalse, values: V{"1", "2"}, input: "false"},
{name: "invalid-op-err", op: "non-existent-op", values: V{"1"}, input: 1},
}
for _, tc := range tcs {
t.Run(tc.name, func(t *testing.T) {
me := api.MatchExpression{Op: tc.op, Value: tc.values}
res, err := me.Match(tc.valid, tc.input)
res, err := me.Match(true, tc.input)
assert.False(t, res)
assert.NotNil(t, err)
})