Fix issue (eBay#124):

acl add/delete code isn't handling "nil" for external_ids correctly Also, fix typo in name of oMapContians() to oMapContains() Signed-off-by: Andre Fredette <[email protected]>
anfredette · Jan 5, 2021 · c82fd27 · c82fd27
1 parent 746e766
commit c82fd27
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 5 deletions.
diff --git a/acl.go b/acl.go
@@ -78,8 +78,14 @@ func (odbi *ovndb) getACLUUIDByRow(lsw, table string, row OVNRow) (string, error
 											goto unmatched
 										}
 									case "external_ids":
-										if value != nil && !odbi.oMapContians(cacheACL.Fields["external_ids"].(libovsdb.OvsMap).GoMap, value.(*libovsdb.OvsMap).GoMap) {
-											goto unmatched
+										if value == nil {
+											if len(cacheACL.Fields["external_ids"].(libovsdb.OvsMap).GoMap) != 0 {
+												goto unmatched
+											}
+										} else {
+											if !odbi.oMapContains(cacheACL.Fields["external_ids"].(libovsdb.OvsMap).GoMap, value.(*libovsdb.OvsMap).GoMap) {
+												goto unmatched
+											}
 										}
 									}
 								}
@@ -119,8 +125,14 @@ func (odbi *ovndb) getACLUUIDByRow(lsw, table string, row OVNRow) (string, error
 									goto out
 								}
 							case "external_ids":
-								if value != nil && !odbi.oMapContians(cacheACL.Fields["external_ids"].(libovsdb.OvsMap).GoMap, value.(*libovsdb.OvsMap).GoMap) {
-									goto out
+								if value == nil {
+									if len(cacheACL.Fields["external_ids"].(libovsdb.OvsMap).GoMap) != 0 {
+										goto out
+									}
+								} else {
+									if !odbi.oMapContains(cacheACL.Fields["external_ids"].(libovsdb.OvsMap).GoMap, value.(*libovsdb.OvsMap).GoMap) {
+										goto out
+									}
 								}
 							}
 						}
@@ -150,6 +162,9 @@ func (odbi *ovndb) aclAddImp(lsw, direct, match, action string, priority int, ex
 			return nil, err
 		}
 		row["external_ids"] = oMap
+	} else {
+		// Add nil so we only match existing ACLs without external_ids in getACLUUIDByRow
+		row["external_ids"] = nil
 	}
 
 	_, err = odbi.getACLUUIDByRow(lsw, TableACL, row)
@@ -162,6 +177,11 @@ func (odbi *ovndb) aclAddImp(lsw, direct, match, action string, priority int, ex
 		return nil, err
 	}
 
+	// Remove row["external_ids"] = nil because it causes an error during execution
+	if external_ids == nil {
+		delete(row, "external_ids")
+	}
+
 	row["action"] = action
 	row["log"] = logflag
 	if logflag {
@@ -226,13 +246,21 @@ func (odbi *ovndb) aclDelImp(lsw, direct, match string, priority int, external_i
 			return nil, err
 		}
 		row["external_ids"] = oMap
+	} else {
+		// Add nil so we only match existing ACLs without external_ids in getACLUUIDByRow
+		row["external_ids"] = nil
 	}
 
 	aclUUID, err := odbi.getACLUUIDByRow(lsw, TableACL, row)
 	if err != nil {
 		return nil, err
 	}
 
+	// Remove row["external_ids"] = nil because it causes an error during execution
+	if external_ids == nil {
+		delete(row, "external_ids")
+	}
+
 	uuidcondition := libovsdb.NewCondition("_uuid", "==", stringToGoUUID(aclUUID))
 	wherecondition = append(wherecondition, uuidcondition)
 	deleteOp := libovsdb.Operation{

diff --git a/ovnimp.go b/ovnimp.go
@@ -94,7 +94,7 @@ func (odbi *ovndb) getRowUUID(table string, row OVNRow) string {
 
 //test if map s contains t
 //This function is not both s and t are nil at same time
-func (odbi *ovndb) oMapContians(s, t map[interface{}]interface{}) bool {
+func (odbi *ovndb) oMapContains(s, t map[interface{}]interface{}) bool {
 	if s == nil || t == nil {
 		return false
 	}