diff --git a/acl.go b/acl.go index 969b66e..d1a65d9 100644 --- a/acl.go +++ b/acl.go @@ -18,6 +18,7 @@ package goovn import ( "github.com/ebay/libovsdb" + "reflect" ) // ACL ovnnb item @@ -78,8 +79,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 !reflect.DeepEqual(cacheACL.Fields["external_ids"].(libovsdb.OvsMap).GoMap, value.(*libovsdb.OvsMap).GoMap) { + goto unmatched + } } } } @@ -119,8 +126,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 !reflect.DeepEqual(cacheACL.Fields["external_ids"].(libovsdb.OvsMap).GoMap, value.(*libovsdb.OvsMap).GoMap) { + goto out + } } } } @@ -144,14 +157,6 @@ func (odbi *ovndb) aclAddImp(lsw, direct, match, action string, priority int, ex row["match"] = match row["priority"] = priority - if external_ids != nil { - oMap, err := libovsdb.NewOvsMap(external_ids) - if err != nil { - return nil, err - } - row["external_ids"] = oMap - } - _, err = odbi.getACLUUIDByRow(lsw, TableACL, row) switch err { case ErrorNotFound: @@ -162,6 +167,14 @@ func (odbi *ovndb) aclAddImp(lsw, direct, match, action string, priority int, ex return nil, err } + if external_ids != nil { + oMap, err := libovsdb.NewOvsMap(external_ids) + if err != nil { + return nil, err + } + row["external_ids"] = oMap + } + row["action"] = action row["log"] = logflag if logflag { @@ -220,13 +233,6 @@ func (odbi *ovndb) aclDelImp(lsw, direct, match string, priority int, external_i row["priority"] = priority } - if external_ids != nil { - oMap, err := libovsdb.NewOvsMap(external_ids) - if err != nil { - return nil, err - } - row["external_ids"] = oMap - } aclUUID, err := odbi.getACLUUIDByRow(lsw, TableACL, row) if err != nil { diff --git a/acl_test.go b/acl_test.go index 824f337..9984686 100644 --- a/acl_test.go +++ b/acl_test.go @@ -122,6 +122,9 @@ func TestACLs(t *testing.T) { cmd, err = ovndbapi.ACLAdd(LSW, "to-lport", MATCH, "drop", 1001, nil, true, "", "") // ACLAdd must return error assert.Equal(t, true, nil != err, "test[%s]", "add same acl twice, should only one added.") + if cmd != nil { + t.Fatal(err) + } // cmd is nil, so this is noop err = ovndbapi.Execute(cmd) assert.Equal(t, true, nil == err, "test[%s]", "add same acl twice, should only one added.") @@ -141,7 +144,35 @@ func TestACLs(t *testing.T) { } assert.Equal(t, true, len(acls) == 2, "test[%s]", "add second acl") + // The following should fail because it is considered a duplicate of an existing ACL cmd, err = ovndbapi.ACLAdd(LSW, "to-lport", MATCH_SECOND, "drop", 1001, map[string]string{"A": "b", "B": "b"}, false, "", "") + if err == nil { + t.Fatal(err) + } + if cmd != nil { + t.Fatal(err) + } + // cmd is nil, so this is noop + err = ovndbapi.Execute(cmd) + if err != nil { + t.Fatal(err) + } + + acls, err = ovndbapi.ACLList(LSW) + if err != nil { + t.Fatal(err) + } + assert.Equal(t, true, len(acls) == 2, "test[%s]", "add second acl") + + //***************** Add more tests here + // The following should fail because it is considered a duplicate of an existing ACL + cmd, err = ovndbapi.ACLAdd(LSW, "to-lport", MATCH_SECOND, "drop", 1001, nil, false, "", "") + if err == nil { + t.Fatal(err) + } + + // Different priority is a different ACL, so this should succeed + cmd, err = ovndbapi.ACLAdd(LSW, "to-lport", MATCH_SECOND, "drop", 1002, map[string]string{"A": "a", "B": "b"}, false, "", "") if err != nil { t.Fatal(err) } @@ -156,7 +187,8 @@ func TestACLs(t *testing.T) { } assert.Equal(t, true, len(acls) == 3, "test[%s]", "add second acl") - cmd, err = ovndbapi.ACLDel(LSW, "to-lport", MATCH, 1001, map[string]string{}) + // Different direction is a different ACL, so this should succeed + cmd, err = ovndbapi.ACLAdd(LSW, "from-lport", MATCH_SECOND, "drop", 1001, map[string]string{"A": "a", "B": "b"}, false, "", "") if err != nil { t.Fatal(err) } @@ -169,9 +201,10 @@ func TestACLs(t *testing.T) { if err != nil { t.Fatal(err) } - assert.Equal(t, true, len(acls) == 2, "test[%s]", "acl remove") + assert.Equal(t, true, len(acls) == 4, "test[%s]", "add second acl") - cmd, err = ovndbapi.ACLDel(LSW, "to-lport", MATCH_SECOND, 1001, map[string]string{"A": "a"}) + + cmd, err = ovndbapi.ACLDel(LSW, "to-lport", MATCH_SECOND, 1002, nil) if err != nil { t.Fatal(err) } @@ -184,10 +217,40 @@ func TestACLs(t *testing.T) { if err != nil { t.Fatal(err) } + assert.Equal(t, true, len(acls) == 3, "test[%s]", "add second acl") + cmd, err = ovndbapi.ACLDel(LSW, "from-lport", MATCH_SECOND, 1001, nil) + if err != nil { + t.Fatal(err) + } + err = ovndbapi.Execute(cmd) + if err != nil { + t.Fatal(err) + } + + acls, err = ovndbapi.ACLList(LSW) + if err != nil { + t.Fatal(err) + } + assert.Equal(t, true, len(acls) == 2, "test[%s]", "acl remove") + + cmd, err = ovndbapi.ACLDel(LSW, "to-lport", MATCH, 1001, map[string]string{}) + if err != nil { + t.Fatal(err) + } + err = ovndbapi.Execute(cmd) + if err != nil { + t.Fatal(err) + } + + acls, err = ovndbapi.ACLList(LSW) + if err != nil { + t.Fatal(err) + } assert.Equal(t, true, len(acls) == 1, "test[%s]", "acl remove") - cmd, err = ovndbapi.ACLDel(LSW, "to-lport", MATCH_SECOND, 1001, map[string]string{"A": "b"}) + // The following ACLDel should succeed. The external_ids are different, but they are ignored during the delete. + cmd, err = ovndbapi.ACLDel(LSW, "to-lport", MATCH_SECOND, 1001, map[string]string{"A": "a"}) if err != nil { t.Fatal(err) } @@ -202,6 +265,12 @@ func TestACLs(t *testing.T) { } assert.Equal(t, true, len(acls) == 0, "test[%s]", "acl remove") + // The following ACLDel should fail because all the ACLs have been deleted. + cmd, err = ovndbapi.ACLDel(LSW, "to-lport", MATCH_SECOND, 1001, map[string]string{"A": "b"}) + if err == nil { + t.Fatal(err) + } + cmd, err = ovndbapi.LSPDel(LSP) if err != nil { t.Fatal(err)