Skip to content

Commit

Permalink
libovsdb: Use libovsdb for ACLs and port groups
Browse files Browse the repository at this point in the history 
Moves almost all ACL and port group operations over to libovsdb. Worth
mentioning:

* Node logical switch ACLs still rely on go-ovn due to dependency with
  logical switch being moved over to libovsdb test harness in unit tests.
  Will be handled in an upcoming PR when this happens.
* libovsdb low level operations have been aggregated in ovn/libovsdbops
  package to avoid cluttering main bussiness logic.
* Each network policy is now handled in a single batched transaction
  with an additional transaction if it is the first policy in the
  namespace to setup default deny ACLs.

Signed-off-by: Jaime Caamaño Ruiz <jcaamano@redhat.com>
  • Loading branch information
@jcaamano
jcaamano committed Sep 22, 2021
1 parent 8a46c32 commit f68d302
Show file tree
Hide file tree
Showing 23 changed files with 1,646 additions and 1,697 deletions.
125 changes: 15 additions & 110 deletions go-controller/pkg/ovn/acl/acl.go
View file
@@ -1,59 +1,19 @@
package acl

import (
"encoding/json"
"fmt"
"strings"

libovsdbclient "github.com/ovn-org/libovsdb/client"
"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/ovn/libovsdbops"
"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/types"
"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/util"

"github.com/pkg/errors"

v1 "k8s.io/api/core/v1"
"k8s.io/klog/v2"
utilnet "k8s.io/utils/net"
)

// GetRejectACLs returns a map with the ACLs with a reject action
// the map uses the name of the ACL as key and the uuid as value
func GetRejectACLs() (map[string]string, error) {
// Get OVN's current reject ACLs. Note, currently only services use reject ACLs.
result := make(map[string]string)
type ovnACLData struct {
Data [][]interface{}
}
data, stderr, err := util.RunOVNNbctl("--columns=name,_uuid", "--format=json", "find", "acl", "action=reject")
if err != nil {
return result, errors.Wrapf(err, "Error while querying ACLs with reject action: %s", stderr)
}
// Process the output
x := ovnACLData{}
if err := json.Unmarshal([]byte(data), &x); err != nil {
return result, errors.Wrapf(err, "Unable to get current OVN reject ACLs. Unable to sync reject ACLs")
}
for _, entry := range x.Data {
// ACL entry format is a slice: [<aclName>, ["_uuid", <uuid>]]
if len(entry) != 2 {
continue
}
name, ok := entry[0].(string)
if !ok {
continue
}
uuidData, ok := entry[1].([]interface{})
if !ok || len(uuidData) != 2 {
continue
}
uuid, ok := uuidData[1].(string)
if !ok {
continue
}
result[name] = uuid
}
return result, nil
}

// RemoveACLFromNodeSwitches removes the ACL uuid entry from Logical Switch acl's list.
func RemoveACLFromNodeSwitches(switches []string, aclUUID string) error {
if len(switches) == 0 {
Expand All @@ -71,83 +31,28 @@ func RemoveACLFromNodeSwitches(switches []string, aclUUID string) error {
return nil
}

// RemoveACLFromPortGroup removes the ACL from the port-group
func RemoveACLFromPortGroup(aclUUID, clusterPortGroupUUID string) error {
_, stderr, err := util.RunOVNNbctl("--", "--if-exists", "remove", "port_group", clusterPortGroupUUID, "acls", aclUUID)
if err != nil {
return errors.Wrapf(err, "Failed to remove reject ACL %s from port group %s: stderr: %q", aclUUID, clusterPortGroupUUID, stderr)
}
klog.Infof("ACL: %s, removed from the port group : %s", aclUUID, clusterPortGroupUUID)
return nil
}

// AddRejectACLToPortGroup adds a reject ACL to a PortGroup
func AddRejectACLToPortGroup(clusterPortGroupUUID, aclName, sourceIP string, sourcePort int, proto v1.Protocol) (string, error) {
l3Prefix := "ip4"
if utilnet.IsIPv6String(sourceIP) {
l3Prefix = "ip6"
}

aclMatch := fmt.Sprintf("match=\"%s.dst==%s && %s && %s.dst==%d\"", l3Prefix, sourceIP,
strings.ToLower(string(proto)), strings.ToLower(string(proto)), sourcePort)
cmd := []string{"--id=@reject-acl", "create", "acl", "direction=" + types.DirectionFromLPort, "priority=" + types.DefaultDenyPriority, aclMatch, "action=reject",
fmt.Sprintf("name=%s", aclName), "--", "add", "port_group", clusterPortGroupUUID, "acls", "@reject-acl"}
aclUUID, stderr, err := util.RunOVNNbctl(cmd...)
if err != nil {
return "", errors.Wrapf(err, "Failed to add ACL: %s, %q, to cluster port group %s, stderr: %q", aclUUID, aclName, clusterPortGroupUUID, stderr)
}
return aclUUID, nil
}

// AddRejectACLToLogicalSwitch adds a reject ACL to a logical switch
func AddRejectACLToLogicalSwitch(logicalSwitch, aclName, sourceIP string, sourcePort int, proto v1.Protocol) (string, error) {
l3Prefix := "ip4"
if utilnet.IsIPv6String(sourceIP) {
l3Prefix = "ip6"
}

aclMatch := fmt.Sprintf("match=\"%s.dst==%s && %s && %s.dst==%d\"", l3Prefix, sourceIP,
strings.ToLower(string(proto)), strings.ToLower(string(proto)), sourcePort)
cmd := []string{"--id=@reject-acl", "create", "acl", "direction=" + types.DirectionFromLPort, "priority=" + types.DefaultDenyPriority, aclMatch, "action=reject",
fmt.Sprintf("name=%s", aclName), "--", "add", "logical_switch", logicalSwitch, "acls", "@reject-acl"}

aclUUID, stderr, err := util.RunOVNNbctl(cmd...)
if err != nil {
return "", errors.Wrapf(err, "Failed to add ACL: %s, %q, to cluster switch %s, stderr: %q", aclUUID, aclName, logicalSwitch, stderr)
}
return aclUUID, nil
}

func PurgeRejectRules(clusterPortGroupUUID string) error {
data, stderr, err := util.RunOVNNbctl("--columns=_uuid", "--format=csv", "--data=bare", "--no-headings", "find", "acl", "action=reject")
func PurgeRejectRules(nbClient libovsdbclient.Client) error {
acls, err := libovsdbops.FindRejectACLs(nbClient)
if err != nil {
return errors.Wrapf(err, "Error while querying ACLs with reject action: %s", stderr)
}
if strings.TrimSpace(data) == "" {
klog.Info("No reject ACLs to remove")
return nil
return errors.Wrap(err, "Error while finding rejct ACLs")
}

for _, uuid := range strings.Split(data, "\n") {
err = RemoveACLFromPortGroup(uuid, clusterPortGroupUUID)
for _, acl := range acls {
data, stderr, err := util.RunOVNNbctl("--format=csv", "--data=bare", "--no-headings", "--columns=_uuid", "find", "logical_switch", fmt.Sprintf("acls{>=}%s", acl.UUID))
if err != nil {
klog.Errorf("Error trying to remove ACL for clusterPortGroupUUID %s/%s: %v", uuid, clusterPortGroupUUID, err)
}

data, stderr, err := util.RunOVNNbctl("--format=csv", "--data=bare", "--no-headings", "--columns=_uuid", "find", "logical_switch", fmt.Sprintf("acls{>=}%s", uuid))
if err != nil {
return errors.Wrapf(err, "Error while querying ACLs uuid:%s with reject action: %s", uuid, stderr)
return errors.Wrapf(err, "Error while querying ACLs uuid:%s with reject action: %s", acl.UUID, stderr)
}
ls := strings.Split(data, "\n")
err = RemoveACLFromNodeSwitches(ls, uuid)
err = RemoveACLFromNodeSwitches(ls, acl.UUID)
if err != nil {
return errors.Wrapf(err, "Failed to remove reject acl from logical switches")
}
_, stderr, err = util.RunOVNNbctl("--if-exists", "destroy", "acl", uuid)
if err != nil {
klog.Errorf("Failed to destroy ACL %s, stderr: %q, (%v)",
uuid, stderr, err)
}
}

err = libovsdbops.DeleteACLsFromPortGroup(nbClient, types.ClusterPortGroupName, acls...)
if err != nil {
klog.Errorf("Error trying to remove ACLs %+v from port group %s: %v", acls, types.ClusterPortGroupName, err)
}

return nil
}
205 changes: 0 additions & 205 deletions go-controller/pkg/ovn/acl/acl_test.go
View file
Expand Up @@ -2,13 +2,10 @@ package acl

import (
"fmt"
"reflect"
"testing"

ovntest "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/testing"
"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/types"
"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/util"
v1 "k8s.io/api/core/v1"
)

func TestRemoveACLFromNodeSwitches(t *testing.T) {
Expand Down Expand Up @@ -65,205 +62,3 @@ func TestRemoveACLFromNodeSwitches(t *testing.T) {
})
}
}

func TestRemoveACLFromPortGroup(t *testing.T) {
tests := []struct {
name string
clusterPortGroupUUID string
aclUUID string
ovnCmd ovntest.ExpectedCmd
wantErr bool
}{
{
name: "remove acl from portgroup",
clusterPortGroupUUID: "63c3e636-387d-11eb-ac78-a8a1590cda29",
aclUUID: "a08ea426-2288-11eb-a30b-a8a1590cda29",
ovnCmd: ovntest.ExpectedCmd{
Cmd: "ovn-nbctl --timeout=15 -- --if-exists remove port_group 63c3e636-387d-11eb-ac78-a8a1590cda29 acls a08ea426-2288-11eb-a30b-a8a1590cda29",
Output: "",
},
wantErr: false,
},

// TODO: Add test cases for errors
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
fexec := ovntest.NewLooseCompareFakeExec()
fexec.AddFakeCmd(&tt.ovnCmd)
err := util.SetExec(fexec)
if err != nil {
t.Errorf("fexec error: %v", err)
}

err = RemoveACLFromPortGroup(tt.aclUUID, tt.clusterPortGroupUUID)
if (err != nil) != tt.wantErr {
t.Errorf("RemoveACLFromPortGroup() error = %v, wantErr %v", err, tt.wantErr)
return
}
})
}
}

func TestGetRejectACLs(t *testing.T) {
tests := []struct {
name string
ovnCmd ovntest.ExpectedCmd
want map[string]string
wantErr bool
}{
{
name: "no reject ACLs in OVN",
ovnCmd: ovntest.ExpectedCmd{
Cmd: "ovn-nbctl --timeout=15 --columns=name,_uuid --format=json find acl action=reject",
Output: `{"data":[],"headings":["name","_uuid"]}`,
},
want: make(map[string]string),
wantErr: false,
},

// TODO: Add test cases with some data
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
fexec := ovntest.NewLooseCompareFakeExec()
fexec.AddFakeCmd(&tt.ovnCmd)
err := util.SetExec(fexec)
if err != nil {
t.Errorf("fexec error: %v", err)
}
got, err := GetRejectACLs()
if (err != nil) != tt.wantErr {
t.Errorf("GetRejectACLs() error = %v, wantErr %v", err, tt.wantErr)
return
}
if !reflect.DeepEqual(got, tt.want) {
t.Errorf("GetRejectACLs() = %v, want %v", got, tt.want)
}
})
}
}

func TestAddRejectACLToLogicalSwitch(t *testing.T) {
tests := []struct {
name string
logicalSwitch string
aclName string
sourceIP string
sourcePort int
proto v1.Protocol
ovnCmd ovntest.ExpectedCmd
want string
wantErr bool
}{
{
name: "add ipv4 acl",
logicalSwitch: "545dc436-387e-11eb-9f38-a8a1590cda29",
aclName: "myacl",
sourceIP: "192.168.2.2",
sourcePort: 80,
proto: v1.ProtocolTCP,
ovnCmd: ovntest.ExpectedCmd{
Cmd: `ovn-nbctl --timeout=15 --id=@reject-acl create acl direction=` + types.DirectionFromLPort + ` priority=` + types.DefaultDenyPriority + ` match="ip4.dst==192.168.2.2 && tcp && tcp.dst==80" action=reject name=myacl -- add logical_switch 545dc436-387e-11eb-9f38-a8a1590cda29 acls @reject-acl`,
Output: "97347886-387e-11eb-9fdf-a8a1590cda29",
},
want: "97347886-387e-11eb-9fdf-a8a1590cda29",
wantErr: false,
},
{
name: "add ipv6 acl",
logicalSwitch: "545dc436-387e-11eb-9f38-a8a1590cda29",
aclName: "myacl",
sourceIP: "2001:db2:1:2::23",
sourcePort: 80,
proto: v1.ProtocolTCP,
ovnCmd: ovntest.ExpectedCmd{
Cmd: `ovn-nbctl --timeout=15 --id=@reject-acl create acl direction=` + types.DirectionFromLPort + ` priority=` + types.DefaultDenyPriority + ` match="ip6.dst==2001:db2:1:2::23 && tcp && tcp.dst==80" action=reject name=myacl -- add logical_switch 545dc436-387e-11eb-9f38-a8a1590cda29 acls @reject-acl`,
Output: "97347886-387e-11eb-9fdf-a8a1590cda29",
},
want: "97347886-387e-11eb-9fdf-a8a1590cda29",
wantErr: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
fexec := ovntest.NewLooseCompareFakeExec()
fexec.AddFakeCmd(&tt.ovnCmd)
err := util.SetExec(fexec)
if err != nil {
t.Errorf("fexec error: %v", err)
}

got, err := AddRejectACLToLogicalSwitch(tt.logicalSwitch, tt.aclName, tt.sourceIP, tt.sourcePort, tt.proto)
if (err != nil) != tt.wantErr {
t.Errorf("AddRejectACLToLogicalSwitch() error = %v, wantErr %v", err, tt.wantErr)
return
}
if got != tt.want {
t.Errorf("AddRejectACLToLogicalSwitch() = %v, want %v", got, tt.want)
}
})
}
}

func TestAddRejectACLToPortGroup(t *testing.T) {
tests := []struct {
name string
portGroup string
aclName string
sourceIP string
sourcePort int
proto v1.Protocol
ovnCmd ovntest.ExpectedCmd
want string
wantErr bool
}{
{
name: "add ipv4 acl",
portGroup: "545dc436-387e-11eb-9f38-a8a1590cda29",
aclName: "myacl",
sourceIP: "192.168.2.2",
sourcePort: 80,
proto: v1.ProtocolTCP,
ovnCmd: ovntest.ExpectedCmd{
Cmd: `ovn-nbctl --timeout=15 --id=@reject-acl create acl direction=` + types.DirectionFromLPort + ` priority=` + types.DefaultDenyPriority + ` match="ip4.dst==192.168.2.2 && tcp && tcp.dst==80" action=reject name=myacl -- add port_group 545dc436-387e-11eb-9f38-a8a1590cda29 acls @reject-acl`,
Output: "97347886-387e-11eb-9fdf-a8a1590cda29",
},
want: "97347886-387e-11eb-9fdf-a8a1590cda29",
wantErr: false,
},
{
name: "add ipv6 acl",
portGroup: "545dc436-387e-11eb-9f38-a8a1590cda29",
aclName: "myacl",
sourceIP: "2001:db2:1:2::23",
sourcePort: 80,
proto: v1.ProtocolTCP,
ovnCmd: ovntest.ExpectedCmd{
Cmd: `ovn-nbctl --timeout=15 --id=@reject-acl create acl direction=` + types.DirectionFromLPort + ` priority=` + types.DefaultDenyPriority + ` match="ip6.dst==2001:db2:1:2::23 && tcp && tcp.dst==80" action=reject name=myacl -- add port_group 545dc436-387e-11eb-9f38-a8a1590cda29 acls @reject-acl`,
Output: "97347886-387e-11eb-9fdf-a8a1590cda29",
},
want: "97347886-387e-11eb-9fdf-a8a1590cda29",
wantErr: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
fexec := ovntest.NewLooseCompareFakeExec()
fexec.AddFakeCmd(&tt.ovnCmd)
err := util.SetExec(fexec)
if err != nil {
t.Errorf("fexec error: %v", err)
}

got, err := AddRejectACLToPortGroup(tt.portGroup, tt.aclName, tt.sourceIP, tt.sourcePort, tt.proto)
if (err != nil) != tt.wantErr {
t.Errorf("AddRejectACLToPortGroup() error = %v, wantErr %v", err, tt.wantErr)
return
}
if got != tt.want {
t.Errorf("AddRejectACLToPortGroup() = %v, want %v", got, tt.want)
}
})
}
}

0 comments on commit f68d302

Please sign in to comment.