Skip to content

Commit 0bc1b6c

Browse files
marquizmarquiz
committed
apis/nfd: drop creation helper functions
Drop the creation helper functions as one step in an effort to tidy up the api package. These functions were not much used outside unit tests anyway, the static rules of the nfd-worker custom feature source being the only exception (and if those happened to be invalid we'd catch that e.g. in the e2e-tests).
1 parent 3ce5a1b commit 0bc1b6c

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)