Skip to content

Commit 6812dbc

Browse files
dcbwdcbw
committed
adsfasdfafds
1 parent cd5a90d commit 6812dbc

File tree

8 files changed

+140
-44
lines changed

8 files changed

+140
-44
lines changed

‎database/transaction_test.go‎

Lines changed: 45 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,7 @@ func TestMutateOp(t *testing.T) {
296296
"baz": "quux",
297297
"waldo": "fred",
298298
},
299+
FloodVLANs: []int{1, 2, 3},
299300
}
300301
bridgeInfo, err := dbModel.NewModelInfo(&bridge)
301302
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 TestDiff(t *testing.T) {
@@ -627,10 +643,12 @@ func TestOvsdbServerUpdate(t *testing.T) {
627643

628644
halloween := MakeOvsSet(t, ovsdb.TypeString, []string{"halloween"})
629645
emptySet := MakeOvsSet(t, ovsdb.TypeString, []string{})
646+
floodVlanSet := MakeOvsSet(t, ovsdb.TypeInteger, []int{1, 2, 3, 4, 5, 6, 7})
630647
tests := []struct {
631-
name string
632-
row ovsdb.Row
633-
expected *ovsdb.RowUpdate2
648+
name string
649+
row ovsdb.Row
650+
expected *ovsdb.RowUpdate2
651+
expectErr bool
634652
}{
635653
{
636654
"update single field",
@@ -640,6 +658,13 @@ func TestOvsdbServerUpdate(t *testing.T) {
640658
"datapath_type": "waldo",
641659
},
642660
},
661+
false,
662+
},
663+
{
664+
"update single field with too-large array",
665+
ovsdb.Row{"flood_vlans": floodVlanSet},
666+
nil,
667+
true,
643668
},
644669
{
645670
"update single optional field, with direct value",
@@ -649,6 +674,7 @@ func TestOvsdbServerUpdate(t *testing.T) {
649674
"datapath_id": halloween,
650675
},
651676
},
677+
false,
652678
},
653679
{
654680
"update single optional field, with set",
@@ -658,6 +684,7 @@ func TestOvsdbServerUpdate(t *testing.T) {
658684
"datapath_id": halloween,
659685
},
660686
},
687+
false,
661688
},
662689
{
663690
"unset single optional field",
@@ -667,6 +694,7 @@ func TestOvsdbServerUpdate(t *testing.T) {
667694
"datapath_id": emptySet,
668695
},
669696
},
697+
false,
670698
},
671699
}
672700
for _, tt := range tests {
@@ -676,16 +704,20 @@ func TestOvsdbServerUpdate(t *testing.T) {
676704
[]ovsdb.Condition{{
677705
Column: "_uuid", Function: ovsdb.ConditionEqual, Value: ovsdb.UUID{GoUUID: bridgeUUID},
678706
}}, tt.row)
679-
assert.NoError(t, err)
680-
errs, err := ovsdb.CheckOperationResults([]ovsdb.OperationResult{res}, []ovsdb.Operation{{Op: "update"}})
681-
require.NoErrorf(t, err, "%+v", errs)
682-
683-
bridge.UUID = bridgeUUID
684-
row, err := db.Get("Open_vSwitch", "Bridge", bridgeUUID)
685-
assert.NoError(t, err)
686-
br := row.(*BridgeType)
687-
assert.NotEqual(t, br, bridgeRow)
688-
assert.Equal(t, tt.expected.Modify, updates["Bridge"][bridgeUUID].Modify)
707+
if tt.expectErr {
708+
require.Error(t, err)
709+
} else {
710+
assert.NoError(t, err)
711+
errs, err := ovsdb.CheckOperationResults([]ovsdb.OperationResult{res}, []ovsdb.Operation{{Op: "update"}})
712+
require.NoErrorf(t, err, "%+v", errs)
713+
714+
bridge.UUID = bridgeUUID
715+
row, err := db.Get("Open_vSwitch", "Bridge", bridgeUUID)
716+
assert.NoError(t, err)
717+
br := row.(*BridgeType)
718+
assert.NotEqual(t, br, bridgeRow)
719+
assert.Equal(t, tt.expected.Modify, updates["Bridge"][bridgeUUID].Modify)
720+
}
689721
})
690722
}
691723
}

‎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
@@ -40,6 +40,8 @@ var (
4040
42.0,
4141
}
4242

43+
aFloatSetTooBig = []float64{1.0, 2.0, 3.0, 4.0, 5.0, 6.0}
44+
4345
aMap = map[string]string{
4446
"key1": "value1",
4547
"key2": "value2",
@@ -115,7 +117,7 @@ var testSchema = []byte(`{
115117
"type": "real"
116118
},
117119
"min": 0,
118-
"max": 10
120+
"max": 5
119121
}
120122
},
121123
"aEmptySet": {
@@ -216,28 +218,49 @@ func TestMapperGetData(t *testing.T) {
216218
NonTagged: "something",
217219
}
218220

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

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

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

243266
func TestMapperNewRow(t *testing.T) {
@@ -315,6 +338,14 @@ func TestMapperNewRow(t *testing.T) {
315338
MyFloatSet: aFloatSet,
316339
},
317340
expectedRow: ovsdb.Row(map[string]interface{}{"aFloatSet": test.MakeOvsSet(t, ovsdb.TypeReal, aFloatSet)}),
341+
}, {
342+
name: "aFloatSet too big",
343+
objInput: &struct {
344+
MyFloatSet []float64 `ovsdb:"aFloatSet"`
345+
}{
346+
MyFloatSet: aFloatSetTooBig,
347+
},
348+
shoulderr: true,
318349
}, {
319350
name: "Enum",
320351
objInput: &struct {

‎ovsdb/bindings.go‎

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

‎ovsdb/error.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ type ConstraintViolation struct {
117117
}
118118

119119
// Error implements the error interface
120-
func (e *ConstraintViolation) Error() string {
120+
func (e ConstraintViolation) Error() string {
121121
msg := constraintViolation
122122
if e.details != "" {
123123
msg += ": " + e.details

‎server/server.go‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -298,7 +298,10 @@ func (o *OvsdbServer) Monitor(client *rpc2.Client, args []json.RawMessage, reply
298298

299299
tableUpdates := make(ovsdb.TableUpdates)
300300
for t, request := range request {
301-
rows := transaction.Select(t, nil, request.Columns)
301+
rows, err := transaction.Select(t, nil, request.Columns)
302+
if err != nil {
303+
return err
304+
}
302305
for i := range rows.Rows {
303306
tu := make(ovsdb.TableUpdate)
304307
uuid := rows.Rows[i]["_uuid"].(ovsdb.UUID).GoUUID
@@ -345,7 +348,10 @@ func (o *OvsdbServer) MonitorCond(client *rpc2.Client, args []json.RawMessage, r
345348

346349
tableUpdates := make(ovsdb.TableUpdates2)
347350
for t, request := range request {
348-
rows := transaction.Select(t, nil, request.Columns)
351+
rows, err := transaction.Select(t, nil, request.Columns)
352+
if err != nil {
353+
return err
354+
}
349355
for i := range rows.Rows {
350356
tu := make(ovsdb.TableUpdate2)
351357
uuid := rows.Rows[i]["_uuid"].(ovsdb.UUID).GoUUID
@@ -392,7 +398,10 @@ func (o *OvsdbServer) MonitorCondSince(client *rpc2.Client, args []json.RawMessa
392398

393399
tableUpdates := make(ovsdb.TableUpdates2)
394400
for t, request := range request {
395-
rows := transaction.Select(t, nil, request.Columns)
401+
rows, err := transaction.Select(t, nil, request.Columns)
402+
if err != nil {
403+
return err
404+
}
396405
for i := range rows.Rows {
397406
tu := make(ovsdb.TableUpdate2)
398407
uuid := rows.Rows[i]["_uuid"].(ovsdb.UUID).GoUUID

‎test/test_data.go‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,17 @@ const schema = `
7878
"min": 0,
7979
"max": "unlimited"
8080
}
81+
},
82+
"flood_vlans": {
83+
"type": {
84+
"key": {
85+
"type": "integer",
86+
"minInteger": 0,
87+
"maxInteger": 4095
88+
},
89+
"min": 0,
90+
"max": 3
91+
}
8192
}
8293
},
8394
"indexes": [
@@ -140,6 +151,7 @@ type BridgeType struct {
140151
ExternalIds map[string]string `ovsdb:"external_ids"`
141152
Ports []string `ovsdb:"ports"`
142153
Status map[string]string `ovsdb:"status"`
154+
FloodVLANs []int `ovsdb:"flood_vlans"`
143155
}
144156

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

0 commit comments

Comments
 (0)