Skip to content

Commit d5fa170

Browse files
committed
adsfasdfafds
1 parent 3c71180 commit d5fa170

File tree

7 files changed

+126
-39
lines changed

7 files changed

+126
-39
lines changed

database/transaction_test.go

Lines changed: 43 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -287,6 +287,7 @@ func TestMutateOp(t *testing.T) {
287287
"baz": "quux",
288288
"waldo": "fred",
289289
},
290+
FloodVLANs: []int{1, 2, 3},
290291
}
291292
bridgeInfo, err := dbModel.NewModelInfo(&bridge)
292293
require.NoError(t, err)
@@ -383,6 +384,21 @@ func TestMutateOp(t *testing.T) {
383384
assert.Equal(t, diffExternalIds, gotModify["external_ids"])
384385
assert.Equal(t, oldExternalIds, gotOld["external_ids"])
385386
assert.Equal(t, newExternalIds, gotNew["external_ids"])
387+
388+
// Test that attempting to mutate a set to exceed its allowed size results in an error
389+
floodVLANsSchema := bridgeInfo.Metadata.TableSchema.Column("flood_vlans")
390+
keyInsert, err := ovsdb.NewOvsSet(floodVLANsSchema.TypeObj.Key.Type, []int{33})
391+
assert.Nil(t, err)
392+
gotResult, gotUpdate, err = transaction.Mutate(
393+
"Bridge",
394+
[]ovsdb.Condition{
395+
ovsdb.NewCondition("_uuid", ovsdb.ConditionEqual, ovsdb.UUID{GoUUID: bridgeUUID}),
396+
},
397+
[]ovsdb.Mutation{
398+
*ovsdb.NewMutation("flood_vlans", ovsdb.MutateOperationInsert, keyInsert),
399+
},
400+
)
401+
assert.Error(t, err)
386402
}
387403

388404
func TestOvsdbServerInsert(t *testing.T) {
@@ -481,10 +497,12 @@ func TestOvsdbServerUpdate(t *testing.T) {
481497

482498
halloween := testhelpers.MakeOvsSet(t, ovsdb.TypeString, []string{"halloween"})
483499
emptySet := testhelpers.MakeOvsSet(t, ovsdb.TypeString, []string{})
500+
floodVlanSet := testhelpers.MakeOvsSet(t, ovsdb.TypeInteger, []int{1, 2, 3, 4, 5, 6, 7})
484501
tests := []struct {
485-
name string
486-
row ovsdb.Row
487-
expected *ovsdb.RowUpdate2
502+
name string
503+
row ovsdb.Row
504+
expected *ovsdb.RowUpdate2
505+
expectErr bool
488506
}{
489507
{
490508
"update single field",
@@ -494,6 +512,13 @@ func TestOvsdbServerUpdate(t *testing.T) {
494512
"datapath_type": "waldo",
495513
},
496514
},
515+
false,
516+
},
517+
{
518+
"update single field with too-large array",
519+
ovsdb.Row{"flood_vlans": floodVlanSet},
520+
nil,
521+
true,
497522
},
498523
{
499524
"update single optional field, with direct value",
@@ -503,6 +528,7 @@ func TestOvsdbServerUpdate(t *testing.T) {
503528
"datapath_id": halloween,
504529
},
505530
},
531+
false,
506532
},
507533
{
508534
"update single optional field, with set",
@@ -512,6 +538,7 @@ func TestOvsdbServerUpdate(t *testing.T) {
512538
"datapath_id": halloween,
513539
},
514540
},
541+
false,
515542
},
516543
{
517544
"unset single optional field",
@@ -521,6 +548,7 @@ func TestOvsdbServerUpdate(t *testing.T) {
521548
"datapath_id": emptySet,
522549
},
523550
},
551+
false,
524552
},
525553
}
526554
for _, tt := range tests {
@@ -535,14 +563,18 @@ func TestOvsdbServerUpdate(t *testing.T) {
535563
}
536564
res, updates := transaction.Update(&op)
537565
errs, err := ovsdb.CheckOperationResults([]ovsdb.OperationResult{res}, []ovsdb.Operation{{Op: "update"}})
538-
require.NoErrorf(t, err, "%+v", errs)
539-
540-
bridge.UUID = bridgeUUID
541-
row, err := db.Get("Open_vSwitch", "Bridge", bridgeUUID)
542-
assert.NoError(t, err)
543-
br := row.(*BridgeType)
544-
assert.NotEqual(t, br, bridgeRow)
545-
assert.Equal(t, tt.expected.Modify, getTableUpdates(*updates)["Bridge"][bridgeUUID].Modify)
566+
if tt.expectErr {
567+
require.Error(t, err)
568+
} else {
569+
require.NoErrorf(t, err, "%+v", errs)
570+
571+
bridge.UUID = bridgeUUID
572+
row, err := db.Get("Open_vSwitch", "Bridge", bridgeUUID)
573+
assert.NoError(t, err)
574+
br := row.(*BridgeType)
575+
assert.NotEqual(t, br, bridgeRow)
576+
assert.Equal(t, tt.expected.Modify, getTableUpdates(*updates)["Bridge"][bridgeUUID].Modify)
577+
}
546578
})
547579
}
548580
}

mapper/info.go

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -71,20 +71,11 @@ func (i *Info) SetField(column string, value interface{}) error {
7171
if colSchema == nil {
7272
return fmt.Errorf("SetField: column %s schema not found", column)
7373
}
74-
75-
// Validate set length requirements
76-
newVal := reflect.ValueOf(value)
77-
if colSchema.Type == ovsdb.TypeSet || colSchema.Type == ovsdb.TypeEnum {
78-
maxVal := colSchema.TypeObj.Max()
79-
minVal := colSchema.TypeObj.Min()
80-
if maxVal > 1 && newVal.Len() > maxVal {
81-
return fmt.Errorf("SetField: column %s overflow: %d new elements but max is %d", column, newVal.Len(), maxVal)
82-
} else if minVal > 0 && newVal.Len() < minVal {
83-
return fmt.Errorf("SetField: column %s underflow: %d new elements but min is %d", column, newVal.Len(), minVal)
84-
}
74+
if err := ovsdb.ValidateColumnConstraints(colSchema, value); err != nil {
75+
return fmt.Errorf("SetField: column %s failed validation: %v", column, err)
8576
}
8677

87-
fieldValue.Set(newVal)
78+
fieldValue.Set(reflect.ValueOf(value))
8879
return nil
8980
}
9081

mapper/mapper.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,9 @@ func (m Mapper) NewRow(data *Info, fields ...interface{}) (ovsdb.Row, error) {
118118
if len(fields) == 0 && ovsdb.IsDefaultValue(column, nativeElem) {
119119
continue
120120
}
121+
if err := ovsdb.ValidateColumnConstraints(column, nativeElem); err != nil {
122+
return nil, fmt.Errorf("column %s assignment failed: %w", column, err)
123+
}
121124
ovsElem, err := ovsdb.NativeToOvs(column, nativeElem)
122125
if err != nil {
123126
return nil, fmt.Errorf("table %s, column %s: failed to generate ovs element. %s", data.Metadata.TableName, name, err.Error())

mapper/mapper_test.go

Lines changed: 46 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@ var (
3939
42.0,
4040
}
4141

42+
aFloatSetTooBig = []float64{1.0, 2.0, 3.0, 4.0, 5.0, 6.0}
43+
4244
aMap = map[string]string{
4345
"key1": "value1",
4446
"key2": "value2",
@@ -114,7 +116,7 @@ var testSchema = []byte(`{
114116
"type": "real"
115117
},
116118
"min": 0,
117-
"max": 10
119+
"max": 5
118120
}
119121
},
120122
"aEmptySet": {
@@ -215,28 +217,49 @@ func TestMapperGetData(t *testing.T) {
215217
NonTagged: "something",
216218
}
217219

218-
ovsRow := getOvsTestRow(t)
219220
/* Code under test */
220221
var schema ovsdb.DatabaseSchema
221222
if err := json.Unmarshal(testSchema, &schema); err != nil {
222223
t.Error(err)
223224
}
224225

225-
mapper := NewMapper(schema)
226-
test := ormTestType{
227-
NonTagged: "something",
228-
}
229-
testInfo, err := NewInfo("TestTable", schema.Table("TestTable"), &test)
230-
assert.NoError(t, err)
231-
232-
err = mapper.GetRowData(&ovsRow, testInfo)
233-
assert.NoError(t, err)
234-
/*End code under test*/
226+
tests := []struct {
227+
name string
228+
setup func() ovsdb.Row
229+
expectErr bool
230+
}{{
231+
name: "basic",
232+
setup: func() ovsdb.Row {
233+
return getOvsTestRow(t)
234+
},
235+
}, {
236+
name: "too big array",
237+
setup: func() ovsdb.Row {
238+
testRow := getOvsTestRow(t)
239+
testRow["aFloatSet"] = test.MakeOvsSet(t, ovsdb.TypeReal, aFloatSetTooBig)
240+
return testRow
241+
},
242+
expectErr: true,
243+
}}
244+
for _, test := range tests {
245+
t.Run(fmt.Sprintf("GetData: %s", test.name), func(t *testing.T) {
246+
mapper := NewMapper(schema)
247+
tt := ormTestType{
248+
NonTagged: "something",
249+
}
250+
testInfo, err := NewInfo("TestTable", schema.Table("TestTable"), &tt)
251+
assert.NoError(t, err)
235252

236-
if err != nil {
237-
t.Error(err)
253+
ovsRow := test.setup()
254+
err = mapper.GetRowData(&ovsRow, testInfo)
255+
if test.expectErr {
256+
assert.Error(t, err)
257+
} else {
258+
assert.NoError(t, err)
259+
assert.Equal(t, expected, tt)
260+
}
261+
})
238262
}
239-
assert.Equal(t, expected, test)
240263
}
241264

242265
func TestMapperNewRow(t *testing.T) {
@@ -314,6 +337,14 @@ func TestMapperNewRow(t *testing.T) {
314337
MyFloatSet: aFloatSet,
315338
},
316339
expectedRow: ovsdb.Row(map[string]interface{}{"aFloatSet": testhelpers.MakeOvsSet(t, ovsdb.TypeReal, aFloatSet)}),
340+
}, {
341+
name: "aFloatSet too big",
342+
objInput: &struct {
343+
MyFloatSet []float64 `ovsdb:"aFloatSet"`
344+
}{
345+
MyFloatSet: aFloatSetTooBig,
346+
},
347+
shoulderr: true,
317348
}, {
318349
name: "Enum",
319350
objInput: &struct {

ovsdb/bindings.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -400,3 +400,21 @@ func isDefaultBaseValue(elem interface{}, etype ExtendedType) bool {
400400
return false
401401
}
402402
}
403+
404+
// ValidateColumnConstraints validates the native value against any constraints
405+
// of a given column.
406+
func ValidateColumnConstraints(column *ColumnSchema, nativeValue interface{}) error {
407+
switch column.Type {
408+
case TypeSet, TypeEnum:
409+
// Validate set length requirements
410+
newVal := reflect.ValueOf(nativeValue)
411+
maxVal := column.TypeObj.Max()
412+
minVal := column.TypeObj.Min()
413+
if maxVal > 1 && newVal.Len() > maxVal {
414+
return fmt.Errorf("slice would overflow (%d elements but %d allowed)", newVal.Len(), maxVal)
415+
} else if minVal > 0 && newVal.Len() < minVal {
416+
return fmt.Errorf("slice would underflow (%d elements but %d required)", newVal.Len(), minVal)
417+
}
418+
}
419+
return nil
420+
}

ovsdb/error.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ func NewConstraintViolation(details string) *ConstraintViolation {
153153
}
154154

155155
// Error implements the error interface
156-
func (e *ConstraintViolation) Error() string {
156+
func (e ConstraintViolation) Error() string {
157157
msg := constraintViolation
158158
if e.details != "" {
159159
msg += ": " + e.details

test/test_data.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,17 @@ const schema = `
8080
"min": 0,
8181
"max": "unlimited"
8282
}
83+
},
84+
"flood_vlans": {
85+
"type": {
86+
"key": {
87+
"type": "integer",
88+
"minInteger": 0,
89+
"maxInteger": 4095
90+
},
91+
"min": 0,
92+
"max": 3
93+
}
8394
}
8495
},
8596
"indexes": [
@@ -142,6 +153,7 @@ type BridgeType struct {
142153
ExternalIds map[string]string `ovsdb:"external_ids"`
143154
Ports []string `ovsdb:"ports"`
144155
Status map[string]string `ovsdb:"status"`
156+
FloodVLANs []int `ovsdb:"flood_vlans"`
145157
}
146158

147159
// OvsType is the simplified ORM model of the Bridge table

0 commit comments

Comments
 (0)