Skip to content

Commit 443ff80

Browse files
authored
Merge pull request #1481 from marquiz/devel/api-creation-funcs
apis/nfd: drop creation helper functions
2 parents 3ce5a1b + 0bc1b6c commit 443ff80

File tree

7 files changed

+42
-76
lines changed

7 files changed

+42
-76
lines changed

deployment/base/nfd-crds/nfd-api-crds.yaml

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -193,16 +193,11 @@ spec:
193193
type: string
194194
matchExpressions:
195195
additionalProperties:
196-
description: "MatchExpression specifies an expression
196+
description: MatchExpression specifies an expression
197197
to evaluate against a set of input values. It
198198
contains an operator that is applied when matching
199199
the input and an array of values that the operator
200-
evaluates the input against. \n NB: CreateMatchExpression
201-
or MustCreateMatchExpression() should be used
202-
for creating new instances. \n NB: Validate()
203-
must be called if Op or Value fields are modified
204-
or if a new instance is created from scratch
205-
without using the helper functions."
200+
evaluates the input against.
206201
properties:
207202
op:
208203
description: Op is the operator to be applied.
@@ -259,15 +254,11 @@ spec:
259254
type: string
260255
matchExpressions:
261256
additionalProperties:
262-
description: "MatchExpression specifies an expression
257+
description: MatchExpression specifies an expression
263258
to evaluate against a set of input values. It contains
264259
an operator that is applied when matching the input
265260
and an array of values that the operator evaluates
266-
the input against. \n NB: CreateMatchExpression or
267-
MustCreateMatchExpression() should be used for creating
268-
new instances. \n NB: Validate() must be called if
269-
Op or Value fields are modified or if a new instance
270-
is created from scratch without using the helper functions."
261+
the input against.
271262
properties:
272263
op:
273264
description: Op is the operator to be applied.

deployment/helm/node-feature-discovery/crds/nfd-api-crds.yaml

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -193,16 +193,11 @@ spec:
193193
type: string
194194
matchExpressions:
195195
additionalProperties:
196-
description: "MatchExpression specifies an expression
196+
description: MatchExpression specifies an expression
197197
to evaluate against a set of input values. It
198198
contains an operator that is applied when matching
199199
the input and an array of values that the operator
200-
evaluates the input against. \n NB: CreateMatchExpression
201-
or MustCreateMatchExpression() should be used
202-
for creating new instances. \n NB: Validate()
203-
must be called if Op or Value fields are modified
204-
or if a new instance is created from scratch
205-
without using the helper functions."
200+
evaluates the input against.
206201
properties:
207202
op:
208203
description: Op is the operator to be applied.
@@ -259,15 +254,11 @@ spec:
259254
type: string
260255
matchExpressions:
261256
additionalProperties:
262-
description: "MatchExpression specifies an expression
257+
description: MatchExpression specifies an expression
263258
to evaluate against a set of input values. It contains
264259
an operator that is applied when matching the input
265260
and an array of values that the operator evaluates
266-
the input against. \n NB: CreateMatchExpression or
267-
MustCreateMatchExpression() should be used for creating
268-
new instances. \n NB: Validate() must be called if
269-
Op or Value fields are modified or if a new instance
270-
is created from scratch without using the helper functions."
261+
the input against.
271262
properties:
272263
op:
273264
description: Op is the operator to be applied.

pkg/apis/nfd/v1alpha1/expression.go

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -42,23 +42,6 @@ var matchOps = map[MatchOp]struct{}{
4242
MatchIsFalse: {},
4343
}
4444

45-
// CreateMatchExpression creates a new MatchExpression instance. Returns an
46-
// error if validation fails.
47-
func CreateMatchExpression(op MatchOp, values ...string) (*MatchExpression, error) {
48-
m := newMatchExpression(op, values...)
49-
return m, m.Validate()
50-
}
51-
52-
// MustCreateMatchExpression creates a new MatchExpression instance. Panics if
53-
// validation fails.
54-
func MustCreateMatchExpression(op MatchOp, values ...string) *MatchExpression {
55-
m, err := CreateMatchExpression(op, values...)
56-
if err != nil {
57-
panic(err)
58-
}
59-
return m
60-
}
61-
6245
// newMatchExpression returns a new MatchExpression instance.
6346
func newMatchExpression(op MatchOp, values ...string) *MatchExpression {
6447
return &MatchExpression{

pkg/apis/nfd/v1alpha1/expression_test.go

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ type BoolAssertionFunc func(assert.TestingT, bool, ...interface{}) bool
2929

3030
type ValueAssertionFunc func(assert.TestingT, interface{}, ...interface{}) bool
3131

32-
func TestCreateMatchExpression(t *testing.T) {
32+
func TestMatchExpressionValidate(t *testing.T) {
3333
type V = api.MatchValue
3434
type TC struct {
3535
name string
@@ -89,7 +89,8 @@ func TestCreateMatchExpression(t *testing.T) {
8989

9090
for _, tc := range tcs {
9191
t.Run(tc.name, func(t *testing.T) {
92-
_, err := api.CreateMatchExpression(tc.op, tc.values...)
92+
me := api.MatchExpression{Op: tc.op, Value: tc.values}
93+
err := me.Validate()
9394
tc.err(t, err)
9495
})
9596
}
@@ -157,7 +158,7 @@ func TestMatch(t *testing.T) {
157158

158159
for _, tc := range tcs {
159160
t.Run(tc.name, func(t *testing.T) {
160-
me := api.MustCreateMatchExpression(tc.op, tc.values...)
161+
me := api.MatchExpression{Op: tc.op, Value: tc.values}
161162
res, err := me.Match(tc.valid, tc.input)
162163
tc.result(t, res)
163164
assert.Nil(t, err)
@@ -251,7 +252,7 @@ func TestMatchKeys(t *testing.T) {
251252

252253
for _, tc := range tcs {
253254
t.Run(tc.name, func(t *testing.T) {
254-
me := api.MustCreateMatchExpression(tc.op, tc.values...)
255+
me := api.MatchExpression{Op: tc.op, Value: tc.values}
255256
res, err := me.MatchKeys(tc.key, tc.input)
256257
tc.result(t, res)
257258
tc.err(t, err)
@@ -320,7 +321,7 @@ func TestMatchValues(t *testing.T) {
320321

321322
for _, tc := range tcs {
322323
t.Run(tc.name, func(t *testing.T) {
323-
me := api.MustCreateMatchExpression(tc.op, tc.values...)
324+
me := api.MatchExpression{Op: tc.op, Value: tc.values}
324325
res, err := me.MatchValues(tc.key, tc.input)
325326
tc.result(t, res)
326327
tc.err(t, err)

pkg/apis/nfd/v1alpha1/rule_test.go

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ func TestRule(t *testing.T) {
3232
FeatureMatcherTerm{
3333
Feature: "domain-1.kf-1",
3434
MatchExpressions: MatchExpressionSet{
35-
"key-1": MustCreateMatchExpression(MatchExists),
35+
"key-1": newMatchExpression(MatchExists),
3636
},
3737
},
3838
},
@@ -109,7 +109,7 @@ func TestRule(t *testing.T) {
109109
FeatureMatcherTerm{
110110
Feature: "domain-1.vf-1",
111111
MatchExpressions: MatchExpressionSet{
112-
"key-1": MustCreateMatchExpression(MatchIn, "val-1"),
112+
"key-1": newMatchExpression(MatchIn, "val-1"),
113113
},
114114
},
115115
},
@@ -130,7 +130,7 @@ func TestRule(t *testing.T) {
130130
FeatureMatcherTerm{
131131
Feature: "domain-1.if-1",
132132
MatchExpressions: MatchExpressionSet{
133-
"attr-1": MustCreateMatchExpression(MatchIn, "val-1"),
133+
"attr-1": newMatchExpression(MatchIn, "val-1"),
134134
},
135135
},
136136
},
@@ -151,13 +151,13 @@ func TestRule(t *testing.T) {
151151
FeatureMatcherTerm{
152152
Feature: "domain-1.vf-1",
153153
MatchExpressions: MatchExpressionSet{
154-
"key-1": MustCreateMatchExpression(MatchIn, "val-x"),
154+
"key-1": newMatchExpression(MatchIn, "val-x"),
155155
},
156156
},
157157
FeatureMatcherTerm{
158158
Feature: "domain-1.if-1",
159159
MatchExpressions: MatchExpressionSet{
160-
"attr-1": MustCreateMatchExpression(MatchIn, "val-1"),
160+
"attr-1": newMatchExpression(MatchIn, "val-1"),
161161
},
162162
},
163163
},
@@ -166,7 +166,7 @@ func TestRule(t *testing.T) {
166166
assert.Nilf(t, err, "unexpected error: %v", err)
167167
assert.Nil(t, m.Labels, "instances should not have matched")
168168

169-
r5.MatchFeatures[0].MatchExpressions["key-1"] = MustCreateMatchExpression(MatchIn, "val-1")
169+
r5.MatchFeatures[0].MatchExpressions["key-1"] = newMatchExpression(MatchIn, "val-1")
170170
m, err = r5.Execute(f)
171171
assert.Nilf(t, err, "unexpected error: %v", err)
172172
assert.Equal(t, r5.Labels, m.Labels, "instances should have matched")
@@ -178,7 +178,7 @@ func TestRule(t *testing.T) {
178178
FeatureMatcherTerm{
179179
Feature: "domain-1.kf-1",
180180
MatchExpressions: MatchExpressionSet{
181-
"key-na": MustCreateMatchExpression(MatchExists),
181+
"key-na": newMatchExpression(MatchExists),
182182
},
183183
},
184184
},
@@ -194,12 +194,12 @@ func TestRule(t *testing.T) {
194194
FeatureMatcherTerm{
195195
Feature: "domain-1.kf-1",
196196
MatchExpressions: MatchExpressionSet{
197-
"key-1": MustCreateMatchExpression(MatchExists),
197+
"key-1": newMatchExpression(MatchExists),
198198
},
199199
},
200200
},
201201
})
202-
r5.MatchFeatures[0].MatchExpressions["key-1"] = MustCreateMatchExpression(MatchIn, "val-1")
202+
r5.MatchFeatures[0].MatchExpressions["key-1"] = newMatchExpression(MatchIn, "val-1")
203203
m, err = r5.Execute(f)
204204
assert.Nilf(t, err, "unexpected error: %v", err)
205205
assert.Equal(t, r5.Labels, m.Labels, "instances should have matched")
@@ -279,30 +279,30 @@ var-2=
279279
FeatureMatcherTerm{
280280
Feature: "domain_1.kf_1",
281281
MatchExpressions: MatchExpressionSet{
282-
"key-a": MustCreateMatchExpression(MatchExists),
283-
"key-c": MustCreateMatchExpression(MatchExists),
284-
"foo": MustCreateMatchExpression(MatchDoesNotExist),
282+
"key-a": newMatchExpression(MatchExists),
283+
"key-c": newMatchExpression(MatchExists),
284+
"foo": newMatchExpression(MatchDoesNotExist),
285285
},
286286
},
287287
FeatureMatcherTerm{
288288
Feature: "domain_1.vf_1",
289289
MatchExpressions: MatchExpressionSet{
290-
"key-1": MustCreateMatchExpression(MatchIn, "val-1", "val-2"),
291-
"bar": MustCreateMatchExpression(MatchDoesNotExist),
290+
"key-1": newMatchExpression(MatchIn, "val-1", "val-2"),
291+
"bar": newMatchExpression(MatchDoesNotExist),
292292
},
293293
},
294294
FeatureMatcherTerm{
295295
Feature: "domain_1.if_1",
296296
MatchExpressions: MatchExpressionSet{
297-
"attr-1": MustCreateMatchExpression(MatchLt, "100"),
297+
"attr-1": newMatchExpression(MatchLt, "100"),
298298
},
299299
},
300300
FeatureMatcherTerm{
301301
Feature: "domain_1.if_1",
302302
MatchExpressions: MatchExpressionSet{
303-
"attr-1": MustCreateMatchExpression(MatchExists),
304-
"attr-2": MustCreateMatchExpression(MatchExists),
305-
"attr-3": MustCreateMatchExpression(MatchExists),
303+
"attr-1": newMatchExpression(MatchExists),
304+
"attr-2": newMatchExpression(MatchExists),
305+
"attr-3": newMatchExpression(MatchExists),
306306
},
307307
},
308308
},
@@ -357,7 +357,7 @@ var-2=
357357
FeatureMatcherTerm{
358358
Feature: "domain_1.kf_1",
359359
MatchExpressions: MatchExpressionSet{
360-
"key-a": MustCreateMatchExpression(MatchExists),
360+
"key-a": newMatchExpression(MatchExists),
361361
},
362362
},
363363
},

pkg/apis/nfd/v1alpha1/types.go

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -209,12 +209,6 @@ type MatchExpressionSet map[string]*MatchExpression
209209
// MatchExpression specifies an expression to evaluate against a set of input
210210
// values. It contains an operator that is applied when matching the input and
211211
// an array of values that the operator evaluates the input against.
212-
//
213-
// NB: CreateMatchExpression or MustCreateMatchExpression() should be used for
214-
// creating new instances.
215-
//
216-
// NB: Validate() must be called if Op or Value fields are modified or if a new
217-
// instance is created from scratch without using the helper functions.
218212
type MatchExpression struct {
219213
// Op is the operator to be applied.
220214
Op MatchOp `json:"op"`

source/custom/static_features.go

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,9 @@ func getStaticFeatureConfig() []CustomRule {
3232
nfdv1alpha1.FeatureMatcherTerm{
3333
Feature: "pci.device",
3434
MatchExpressions: nfdv1alpha1.MatchExpressionSet{
35-
"vendor": nfdv1alpha1.MustCreateMatchExpression(nfdv1alpha1.MatchIn, "15b3"),
35+
"vendor": &nfdv1alpha1.MatchExpression{
36+
Op: nfdv1alpha1.MatchIn,
37+
Value: nfdv1alpha1.MatchValue{"15b3"}},
3638
},
3739
},
3840
},
@@ -46,8 +48,12 @@ func getStaticFeatureConfig() []CustomRule {
4648
nfdv1alpha1.FeatureMatcherTerm{
4749
Feature: "kernel.loadedmodule",
4850
MatchExpressions: nfdv1alpha1.MatchExpressionSet{
49-
"ib_uverbs": nfdv1alpha1.MustCreateMatchExpression(nfdv1alpha1.MatchExists),
50-
"rdma_ucm": nfdv1alpha1.MustCreateMatchExpression(nfdv1alpha1.MatchExists),
51+
"ib_uverbs": &nfdv1alpha1.MatchExpression{
52+
Op: nfdv1alpha1.MatchExists,
53+
},
54+
"rdma_ucm": &nfdv1alpha1.MatchExpression{
55+
Op: nfdv1alpha1.MatchExists,
56+
},
5157
},
5258
},
5359
},

0 commit comments

Comments
 (0)