Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Preparing for workload attributes support #277

Merged
merged 11 commits into from
Jan 29, 2024
Merged

Preparing for workload attributes support #277

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
d58b65e
Delete unused consts
zivnevo Jan 22, 2024
1ea718c
Remove unused code/structs/consts
zivnevo Jan 22, 2024
fc8a023
Move Wildcard const to loadBalancer.go
zivnevo Jan 22, 2024
b320c2d
Simplify ExportPeer method
zivnevo Jan 22, 2024
dd24e22
Moving conn req and resp from event to policyengine
zivnevo Jan 22, 2024
5654f7e
lint
zivnevo Jan 22, 2024
4fbdb67
Disable policy checks on export for now
zivnevo Jan 22, 2024
a2dd5ac
export comments
zivnevo Jan 22, 2024
6f2181e
No need for src/dst peer in conn req. Using attrs instead
zivnevo Jan 22, 2024
4a7115e
Shorten names of enumerated values
zivnevo Jan 22, 2024
049d069
Merge branch 'main' into workload_attributes
zivnevo Jan 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 11 additions & 10 deletions pkg/controlplane/authz.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ import (
"github.com/lestrrat-go/jwx/jwt"

"github.com/clusterlink-net/clusterlink/pkg/controlplane/api"
"github.com/clusterlink-net/clusterlink/pkg/controlplane/eventmanager"
"github.com/clusterlink-net/clusterlink/pkg/policyengine"
"github.com/clusterlink-net/clusterlink/pkg/policyengine/policytypes"
)

const (
Expand Down Expand Up @@ -80,23 +81,23 @@ func (cp *Instance) AuthorizeEgress(req *EgressAuthorizationRequest) (*EgressAut
return nil, fmt.Errorf("no bindings found for import '%s'", req.Import)
}

connReq := eventmanager.ConnectionRequestAttr{DstService: req.Import, Direction: eventmanager.Outgoing}
connReq := policytypes.ConnectionRequest{DstSvcName: req.Import, Direction: policytypes.Outgoing}
srcLabels := cp.platform.GetLabelsFromIP(req.IP)
if src, ok := srcLabels["app"]; ok { // TODO: Add support for labels other than just the "app" key.
cp.logger.Infof("Received egress authorization srcLabels[app]: %v.", srcLabels["app"])
connReq.SrcService = src
connReq.SrcWorkloadAttrs = policytypes.WorkloadAttrs{policyengine.ServiceNameLabel: src}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

q: does it make sense to have member structs for ConnectionRequest.Source and ConnectionRequest.Destination instead of prefixing value names (e.g., ConnectionRequest.SrcWorkloadAttrs vs ConnectionRequest.Source.Attributes)?
We can encapsulate some logic if it is a proper type?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SrcWorkloadAttrs is now the only information on the source, as SrcPeer was removed. If needed, we can encapsulate more logic into policytypes.WorkloadAttrs

}

authResp, err := cp.policyDecider.AuthorizeAndRouteConnection(&connReq)
if err != nil {
return nil, err
}

if authResp.Action != eventmanager.Allow {
if authResp.Action != policytypes.ActionAllow {
return &EgressAuthorizationResponse{Allowed: false}, nil
}

target := authResp.TargetPeer
target := authResp.DstPeer
peer := cp.GetPeer(target)
if peer == nil {
return nil, fmt.Errorf("peer '%s' does not exist", target)
Expand Down Expand Up @@ -141,16 +142,16 @@ func (cp *Instance) AuthorizeIngress(req *IngressAuthorizationRequest, peer stri

resp.ServiceExists = true

connReq := eventmanager.ConnectionRequestAttr{
DstService: req.Service,
Direction: eventmanager.Incoming,
OtherPeer: peer,
connReq := policytypes.ConnectionRequest{
DstSvcName: req.Service,
Direction: policytypes.Incoming,
SrcWorkloadAttrs: policytypes.WorkloadAttrs{policyengine.GatewayNameLabel: peer},
}
authResp, err := cp.policyDecider.AuthorizeAndRouteConnection(&connReq)
if err != nil {
return nil, err
}
if authResp.Action != eventmanager.Allow {
if authResp.Action != policytypes.ActionAllow {
resp.Allowed = false
return resp, nil
}
Expand Down
92 changes: 1 addition & 91 deletions pkg/controlplane/eventmanager/events.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,19 +22,6 @@ const (
Outgoing
)

func (d Direction) String() string {
return [...]string{"Incoming", "Outgoing"}[d]
}

type Action int

const (
Allow Action = iota
Deny
AllowAll
AllowPartial
)

type ConnectionState int

const (
Expand All @@ -44,35 +31,10 @@ const (
PeerDenied
)

func (a Action) String() string {
return [...]string{"Allow", "Deny", "AllowAll", "AllowPartial"}[a]
}

const Wildcard = "*"

const (
NewConnectionRequest = "NewConnectionRequest"
ConnectionStatus = "ConnectionStatus"
AddPeerRequest = "AddPeerRequest"
NewRemoteService = "NewRemoteService"
ExposeRequest = "ExposeRequest"
RemovePeerRequest = "RemovePeerRequest"
RemoveRemoteService = "RemoveRemoteService"
ConnectionStatus = "ConnectionStatus"
)

type ConnectionRequestAttr struct {
SrcService string
DstService string
Direction Direction
OtherPeer string // Optional: Would not be set if its an outgoing connection
}

type ConnectionRequestResp struct {
Action Action
TargetPeer string
BitRate int // Mbps
}

type ConnectionStatusAttr struct {
ConnectionID string // Unique ID to track a connection from start to end within the gateway
SrcService string // Source application/service initiating the connection
Expand All @@ -85,55 +47,3 @@ type ConnectionStatusAttr struct {
Direction Direction // Incoming/Outgoing
State ConnectionState
}

type NewRemoteServiceAttr struct {
Service string
Peer string
}

type RemoveRemoteServiceAttr struct {
Service string
Peer string
}

type NewRemoteServiceResp struct {
Action Action
}

type ExposeRequestAttr struct {
Service string
}

type ExposeRequestResp struct {
Action Action
TargetPeers []string
}

type AddPeerAttr struct {
Peer string
}

type AddPeerResp struct {
Action Action
}

type RemovePeerAttr struct {
Peer string
}

type ServiceListRequestAttr struct {
SrcPeer string
}

type ServiceListRequestResp struct {
Action Action
Services []string
}

type ServiceRequestAttr struct {
SrcPeer string
}

type ServiceRequestResp struct {
Action Action
}
20 changes: 7 additions & 13 deletions pkg/controlplane/instance.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,10 @@ import (
"github.com/sirupsen/logrus"

"github.com/clusterlink-net/clusterlink/pkg/api"
event "github.com/clusterlink-net/clusterlink/pkg/controlplane/eventmanager"
cpstore "github.com/clusterlink-net/clusterlink/pkg/controlplane/store"
"github.com/clusterlink-net/clusterlink/pkg/platform"
"github.com/clusterlink-net/clusterlink/pkg/policyengine"
"github.com/clusterlink-net/clusterlink/pkg/policyengine/policytypes"
"github.com/clusterlink-net/clusterlink/pkg/store"
"github.com/clusterlink-net/clusterlink/pkg/util"
"github.com/clusterlink-net/clusterlink/pkg/utils/netutils"
Expand Down Expand Up @@ -175,14 +175,11 @@ func (cp *Instance) CreateExport(export *cpstore.Export) error {
if eSpec.ExternalService != "" && !netutils.IsIP(eSpec.ExternalService) && !netutils.IsDNS(eSpec.ExternalService) {
return fmt.Errorf("the external service %s is not a hostname or an IP address", eSpec.ExternalService)
}
resp, err := cp.policyDecider.AddExport(&api.Export{Name: export.Name, Spec: export.ExportSpec})
// TODO: check policyDecider's answer
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: can you assign the TODO to yourself in the comment (if that's the right assignment)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have to decide if we want separate export policies or rather derive export behavior from access policies

_, err := cp.policyDecider.AddExport(&api.Export{Name: export.Name, Spec: export.ExportSpec})
if err != nil {
return err
}
if resp.Action != event.AllowAll {
cp.logger.Warnf("Access policies deny creating export '%s'.", export.Name)
return nil
}

if cp.initialized {
if err := cp.exports.Create(export); err != nil {
Expand Down Expand Up @@ -210,14 +207,11 @@ func (cp *Instance) UpdateExport(export *cpstore.Export) error {
return fmt.Errorf("the external service %s is not a hostname or an IP address", eSpec.ExternalService)
}

resp, err := cp.policyDecider.AddExport(&api.Export{Name: export.Name, Spec: export.ExportSpec})
// TODO: check policyDecider's answer
_, err := cp.policyDecider.AddExport(&api.Export{Name: export.Name, Spec: export.ExportSpec})
if err != nil {
return err
}
if resp.Action != event.AllowAll {
cp.logger.Warnf("Access policies deny creating export '%s'.", export.Name)
return nil
}

err = cp.exports.Update(export.Name, func(old *cpstore.Export) *cpstore.Export {
return export
Expand Down Expand Up @@ -377,7 +371,7 @@ func (cp *Instance) CreateBinding(binding *cpstore.Binding) error {
if err != nil {
return err
}
if action != event.Allow {
if action != policytypes.ActionAllow {
cp.logger.Warnf("Access policies deny creating binding '%s'->'%s' .", binding.Import, binding.Peer)
return nil
}
Expand All @@ -399,7 +393,7 @@ func (cp *Instance) UpdateBinding(binding *cpstore.Binding) error {
if err != nil {
return err
}
if action != event.Allow {
if action != policytypes.ActionAllow {
cp.logger.Warnf("Access policies deny creating binding '%s'->'%s' .", binding.Import, binding.Peer)
return nil
}
Expand Down
Loading
Loading