Skip to content

Commit

Permalink
290 explainability public ip grouping (#301)
Browse files Browse the repository at this point in the history 
* added external IP grouping of explainability results with printing
  • Loading branch information
@ShiriMoran @olasaadi99
ShiriMoran authored and olasaadi99 committed Jan 4, 2024
1 parent 59ef0e7 commit 46858a6
Show file tree
Hide file tree
Showing 6 changed files with 223 additions and 101 deletions.
77 changes: 64 additions & 13 deletions pkg/ibmvpc/explainability_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ func TestVsiToVsi(t *testing.T) {
"\n\tindex: 5, direction: outbound, protocol: all, cidr: 10.240.30.0/24"+
"\n\tindex: 6, direction: outbound, conns: protocol: tcp, dstPorts: 1-65535, cidr: 10.240.20.4/32,10.240.30.4/32"+
"\nIngress Rules:\n~~~~~~~~~~~~~~\nSecurityGroupLayer Rules\n------------------------\nenabling rules from sg2-ky:"+
"\n\tindex: 7, direction: inbound, conns: protocol: tcp, dstPorts: 1-65535, cidr: 10.240.20.4/32,10.240.30.4/32\n", explanbilityStr1)
"\n\tindex: 7, direction: inbound, conns: protocol: tcp, dstPorts: 1-65535, cidr: 10.240.20.4/32,10.240.30.4/32\n\n",
explanbilityStr1)
explanbilityStr2, err2 := vpcConfig.ExplainConnectivity("vsi2-ky[10.240.20.4]", "vsi1-ky[10.240.10.4]")
if err2 != nil {
require.Fail(t, err2.Error())
Expand All @@ -40,7 +41,7 @@ func TestVsiToVsi(t *testing.T) {
"SecurityGroupLayer Rules\n------------------------\nenabling rules from sg2-ky:"+
"\n\tindex: 1, direction: outbound, protocol: all, cidr: 10.240.10.0/24\nIngress Rules:"+
"\n~~~~~~~~~~~~~~\nSecurityGroupLayer Rules\n------------------------\nenabling rules from sg1-ky:\n\t"+
"index: 3, direction: inbound, protocol: all, cidr: 10.240.20.4/32,10.240.30.4/32\n", explanbilityStr2)
"index: 3, direction: inbound, protocol: all, cidr: 10.240.20.4/32,10.240.30.4/32\n\n", explanbilityStr2)
explanbilityStr3, err3 := vpcConfig.ExplainConnectivity("vsi3a-ky[10.240.30.5]", "vsi1-ky[10.240.10.4]")
if err3 != nil {
require.Fail(t, err3.Error())
Expand All @@ -51,23 +52,23 @@ func TestVsiToVsi(t *testing.T) {
"Egress Rules:\n~~~~~~~~~~~~~\nSecurityGroupLayer Rules\n------------------------\nenabling rules from sg3-ky:\n"+
"\tindex: 0, direction: outbound, protocol: all, cidr: 0.0.0.0/0\nIngress Rules:\n~~~~~~~~~~~~~~\nSecurityGroupLayer Rules"+
"\n------------------------\nenabling rules from sg1-ky:\n"+
"\tindex: 4, direction: inbound, protocol: all, cidr: 10.240.30.5/32,10.240.30.6/32\n", explanbilityStr3)
"\tindex: 4, direction: inbound, protocol: all, cidr: 10.240.30.5/32,10.240.30.6/32\n\n", explanbilityStr3)
explanbilityStr4, err4 := vpcConfig.ExplainConnectivity("vsi1-ky[10.240.10.4]", "vsi2-ky[10.240.20.4]")
if err4 != nil {
require.Fail(t, err4.Error())
}
fmt.Println(explanbilityStr4)
require.Equal(t, "No connection between vsi1-ky[10.240.10.4] and vsi2-ky[10.240.20.4]; "+
"connection blocked by egress\nIngress Rules:\n~~~~~~~~~~~~~~\nSecurityGroupLayer Rules\n------------------------\n"+
"enabling rules from sg2-ky:\n\tindex: 4, direction: inbound, protocol: all, cidr: 10.240.10.4/32\n", explanbilityStr4)
"enabling rules from sg2-ky:\n\tindex: 4, direction: inbound, protocol: all, cidr: 10.240.10.4/32\n\n", explanbilityStr4)
explanbilityStr5, err5 := vpcConfig.ExplainConnectivity("vsi3a-ky[10.240.30.5]", "vsi2-ky[10.240.20.4]")
if err5 != nil {
require.Fail(t, err5.Error())
}
fmt.Println(explanbilityStr5)
require.Equal(t, "No connection between vsi3a-ky[10.240.30.5] and vsi2-ky[10.240.20.4]; connection blocked by ingress\n"+
"Egress Rules:\n~~~~~~~~~~~~~\nSecurityGroupLayer Rules\n------------------------\nenabling rules from sg3-ky:"+
"\n\tindex: 0, direction: outbound, protocol: all, cidr: 0.0.0.0/0\n", explanbilityStr5)
"\n\tindex: 0, direction: outbound, protocol: all, cidr: 0.0.0.0/0\n\n", explanbilityStr5)
fmt.Println("done")
}

Expand All @@ -90,7 +91,7 @@ func TestSGDefaultRules(t *testing.T) {
require.Equal(t, "No connection between vsi1-ky[10.240.10.4] and vsi3a-ky[10.240.30.5]; "+
"connection blocked by ingress\nEgress Rules:\n~~~~~~~~~~~~~\nSecurityGroupLayer Rules\n"+
"------------------------\nrules in sg1-ky are the default, namely this is the enabling egress rule:\n"+
"\tindex: 0, direction: outbound, protocol: all, cidr: 0.0.0.0/0\n", explanbilityStr1)
"\tindex: 0, direction: outbound, protocol: all, cidr: 0.0.0.0/0\n\n", explanbilityStr1)
// connection, egress (sg3-ky) is default
explanbilityStr2, err2 := vpcConfig.ExplainConnectivity("vsi3a-ky[10.240.30.5]", "vsi2-ky[10.240.20.4]")
if err2 != nil {
Expand All @@ -102,7 +103,7 @@ func TestSGDefaultRules(t *testing.T) {
"rules in sg3-ky are the default, namely this is the enabling egress rule:\n"+
"\tindex: 0, direction: outbound, protocol: all, cidr: 0.0.0.0/0\n"+
"Ingress Rules:\n~~~~~~~~~~~~~~\nSecurityGroupLayer Rules\n------------------------\n"+
"enabling rules from sg2-ky:\n\tindex: 1, direction: inbound, protocol: all, cidr: 0.0.0.0/0\n", explanbilityStr2)
"enabling rules from sg2-ky:\n\tindex: 1, direction: inbound, protocol: all, cidr: 0.0.0.0/0\n\n", explanbilityStr2)
fmt.Println("done")
}

Expand Down Expand Up @@ -138,27 +139,77 @@ func TestSimpleExternal(t *testing.T) {
if err1 != nil {
require.Fail(t, err1.Error())
}
require.Equal(t, "The following connection exists between vsi1-ky[10.240.10.4] and Public Internet [161.26.0.0/16]: "+
require.Equal(t, "The following connection exists between vsi1-ky[10.240.10.4] and Public Internet 161.26.0.0/16: "+
"protocol: UDP; its enabled by\n"+
"Egress Rules:\n~~~~~~~~~~~~~\nSecurityGroupLayer Rules\n------------------------\nenabling rules from sg1-ky:\n\t"+
"index: 2, direction: outbound, conns: protocol: udp, dstPorts: 1-65535, cidr: 161.26.0.0/16\n", explanbilityStr1)
"index: 2, direction: outbound, conns: protocol: udp, dstPorts: 1-65535, cidr: 161.26.0.0/16\n\n", explanbilityStr1)
fmt.Println(explanbilityStr1)
fmt.Println("---------------------------------------------------------------------------------------------------------------------------")
explanbilityStr2, err2 := vpcConfig.ExplainConnectivity(cidr1, vsi1)
if err2 != nil {
require.Fail(t, err2.Error())
}
fmt.Println(explanbilityStr2)
require.Equal(t, "No connection between Public Internet [161.26.0.0/16] and vsi1-ky[10.240.10.4]; "+
"connection blocked by ingress\n", explanbilityStr2)
fmt.Println("---------------------------------------------------------------------------------------------------------------------------")
require.Equal(t, "No connection between Public Internet 161.26.0.0/16 and vsi1-ky[10.240.10.4]; "+
"connection blocked by ingress\n\n", explanbilityStr2)
explanbilityStr3, err3 := vpcConfig.ExplainConnectivity(vsi1, cidr2)
if err3 != nil {
require.Fail(t, err3.Error())
}
require.Equal(t, "The following connection exists between vsi1-ky[10.240.10.4] and Public Internet [161.26.0.0/32]: "+
require.Equal(t, "The following connection exists between vsi1-ky[10.240.10.4] and Public Internet 161.26.0.0/32: "+
"protocol: UDP; its enabled by\n"+
"Egress Rules:\n~~~~~~~~~~~~~\nSecurityGroupLayer Rules\n------------------------\nenabling rules from sg1-ky:\n\t"+
"index: 2, direction: outbound, conns: protocol: udp, dstPorts: 1-65535, cidr: 161.26.0.0/16\n", explanbilityStr3)
"index: 2, direction: outbound, conns: protocol: udp, dstPorts: 1-65535, cidr: 161.26.0.0/16\n\n", explanbilityStr3)
fmt.Println(explanbilityStr3)
fmt.Println("---------------------------------------------------------------------------------------------------------------------------")
}

func TestGroupingExternal(t *testing.T) {
vpcConfig := getConfig(t, "input_sg_testing1_new.json")
if vpcConfig == nil {
require.Fail(t, "vpcConfig equals nil")
}
vsi1 := "vsi1-ky[10.240.10.4]"
cidr1 := "161.26.0.0/8"
explanbilityStr1, err1 := vpcConfig.ExplainConnectivity(vsi1, cidr1)
if err1 != nil {
require.Fail(t, err1.Error())
}
require.Equal(t, "No connection between vsi1-ky[10.240.10.4] and Public Internet 161.0.0.0-161.25.255.255,161.27.0.0-161.255.255.255; "+
"connection blocked by egress\n\n"+
"The following connection exists between vsi1-ky[10.240.10.4] and Public Internet 161.26.0.0/16: protocol: UDP; its enabled by\n"+
"Egress Rules:\n~~~~~~~~~~~~~\nSecurityGroupLayer Rules\n------------------------\nenabling rules from sg1-ky:\n"+
"\tindex: 2, direction: outbound, conns: protocol: udp, dstPorts: 1-65535, cidr: 161.26.0.0/16\n\n",
explanbilityStr1)
fmt.Println(explanbilityStr1)
fmt.Println("---------------------------------------------------------------------------------------------------------------------------")
vsi2 := "vsi2-ky[10.240.20.4]"
cidrAll := "0.0.0.0/0"
explanbilityStr2, err2 := vpcConfig.ExplainConnectivity(vsi2, cidrAll)
if err2 != nil {
require.Fail(t, err2.Error())
}
fmt.Println(explanbilityStr2)
fmt.Println("---------------------------------------------------------------------------------------------------------------------------")
require.Equal(t, "No connection between vsi2-ky[10.240.20.4] and Public Internet 0.0.0.0-141.255.255.255,143.0.0.0-255.255.255.255; "+
"connection blocked by egress\n\n"+
"The following connection exists between vsi2-ky[10.240.20.4] and Public Internet 142.0.0.0/8: protocol: ICMP; its enabled by\n"+
"Egress Rules:\n~~~~~~~~~~~~~\nSecurityGroupLayer Rules\n------------------------\nenabling rules from sg2-ky:\n"+
"\tindex: 3, direction: outbound, conns: protocol: icmp, icmpType: protocol: ICMP, cidr: 142.0.0.0/8\n\n",
explanbilityStr2)
explanbilityStr3, err3 := vpcConfig.ExplainConnectivity(cidrAll, vsi2)
if err3 != nil {
require.Fail(t, err3.Error())
}
fmt.Println(explanbilityStr3)
require.Equal(t, "No connection between vsi2-ky[10.240.20.4] and Public Internet 0.0.0.0-141.255.255.255,143.0.0.0-255.255.255.255; "+
"connection blocked by egress\n\nThe following connection exists between vsi2-ky[10.240.20.4] "+
"and Public Internet 142.0.0.0/8: protocol: ICMP; its enabled by\nEgress Rules:\n"+
"~~~~~~~~~~~~~\nSecurityGroupLayer Rules\n------------------------\nenabling rules from sg2-ky:\n\t"+
"index: 3, direction: outbound, conns: protocol: icmp, icmpType: protocol: ICMP, cidr: 142.0.0.0/8\n\n",
explanbilityStr2)
fmt.Println("---------------------------------------------------------------------------------------------------------------------------")
}

// getConfigs returns map[string]*vpcmodel.VPCConfig obj for the input test (config json file)
Expand Down
118 changes: 80 additions & 38 deletions pkg/vpcmodel/grouping.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,11 @@ type groupedNodesInfo struct {
type groupedCommonProperties struct {
conn *common.ConnectionSet
connDiff *connectionDiff
// connStrKey is the string of conn per grouping of conn lines, and string of connDiff per grouping of diff lines
connStrKey string // the key used for grouping per connectivity lines or diff lines
rules *rulesConnection
// groupingStrKey is the key by which the grouping is done:
// the string of conn per grouping of conn lines, string of connDiff per grouping of diff lines
// and string of conn and rules for explainblity
groupingStrKey string // the key used for grouping per connectivity lines or diff lines
}

func (g *groupedNodesInfo) appendNode(n Node) {
Expand Down Expand Up @@ -55,18 +58,18 @@ func newGroupingConnections() *groupingConnections {

func newGroupConnLines(c *VPCConfig, v *VPCConnectivity,
grouping bool) (res *GroupConnLines, err error) {
res = &GroupConnLines{c: c, v: v,
res = &GroupConnLines{config: c, nodesConn: v,
srcToDst: newGroupingConnections(),
dstToSrc: newGroupingConnections(),
groupedEndpointsElemsMap: make(map[string]*groupedEndpointsElems),
groupedExternalNodesMap: make(map[string]*groupedExternalNodes)}
groupedEndpointsElemsMap: map[string]*groupedEndpointsElems{},
groupedExternalNodesMap: map[string]*groupedExternalNodes{}}
err = res.computeGrouping(true, grouping)
return res, err
}

func newGroupConnLinesSubnetConnectivity(c *VPCConfig, s *VPCsubnetConnectivity,
grouping bool) (res *GroupConnLines, err error) {
res = &GroupConnLines{c: c, s: s,
res = &GroupConnLines{config: c, subnetsConn: s,
srcToDst: newGroupingConnections(),
dstToSrc: newGroupingConnections(),
groupedEndpointsElemsMap: make(map[string]*groupedEndpointsElems),
Expand All @@ -76,7 +79,7 @@ func newGroupConnLinesSubnetConnectivity(c *VPCConfig, s *VPCsubnetConnectivity,
}

func newGroupConnLinesDiff(d *diffBetweenCfgs) (res *GroupConnLines, err error) {
res = &GroupConnLines{d: d,
res = &GroupConnLines{diff: d,
srcToDst: newGroupingConnections(),
dstToSrc: newGroupingConnections(),
groupedEndpointsElemsMap: make(map[string]*groupedEndpointsElems),
Expand All @@ -85,15 +88,28 @@ func newGroupConnLinesDiff(d *diffBetweenCfgs) (res *GroupConnLines, err error)
return res, err
}

func newGroupConnExplainability(c *VPCConfig, e *explainStruct) (res *GroupConnLines, err error) {
res = &GroupConnLines{
config: c,
explain: e,
srcToDst: newGroupingConnections(),
dstToSrc: newGroupingConnections(),
groupedEndpointsElemsMap: make(map[string]*groupedEndpointsElems),
groupedExternalNodesMap: make(map[string]*groupedExternalNodes)}
err = res.groupExternalAddressesForExplainability()
return res, err
}

// GroupConnLines used both for VPCConnectivity and for VPCsubnetConnectivity, one at a time. The other must be nil
// todo: define abstraction above both?
type GroupConnLines struct {
c *VPCConfig
v *VPCConnectivity
s *VPCsubnetConnectivity
d *diffBetweenCfgs
srcToDst *groupingConnections
dstToSrc *groupingConnections
config *VPCConfig
nodesConn *VPCConnectivity
subnetsConn *VPCsubnetConnectivity
diff *diffBetweenCfgs
explain *explainStruct
srcToDst *groupingConnections
dstToSrc *groupingConnections
// a map to groupedEndpointsElems used by GroupedConnLine from a unified key of such elements
// representing grouped vsis or grouped subnets
// this is to avoid duplication of identical groupedEndpointsElems
Expand All @@ -106,6 +122,7 @@ type GroupConnLines struct {
// EndpointElem can be Node(networkInterface) / groupedExternalNodes / groupedNetworkInterfaces / NodeSet(subnet)
type EndpointElem interface {
Name() string
IsExternal() bool
DrawioResourceIntf
}

Expand All @@ -116,14 +133,14 @@ type groupedConnLine struct {
}

func (g *groupedConnLine) String() string {
return g.src.Name() + " => " + g.dst.Name() + " : " + g.commonProperties.connStrKey
return g.src.Name() + " => " + g.dst.Name() + " : " + g.commonProperties.groupingStrKey
}

func (g *groupedConnLine) ConnLabel() string {
if g.commonProperties.conn.AllowAll {
return ""
}
return g.commonProperties.connStrKey
return g.commonProperties.groupingStrKey
}

func (g *groupedConnLine) getSrcOrDst(isSrc bool) EndpointElem {
Expand Down Expand Up @@ -183,7 +200,7 @@ func (g *GroupConnLines) getGroupedExternalNodes(grouped groupedExternalNodes) *
}

func (g *groupingConnections) addPublicConnectivity(ep EndpointElem, commonProps *groupedCommonProperties, targetNode Node) {
connKey := commonProps.connStrKey
connKey := commonProps.groupingStrKey
if _, ok := (*g)[ep]; !ok {
(*g)[ep] = map[string]*groupedNodesInfo{}
}
Expand Down Expand Up @@ -249,17 +266,19 @@ func subnetGrouping(groupedConnLines *GroupConnLines,
func (g *GroupConnLines) groupExternalAddresses(vsi bool) error {
var allowedConnsCombined GeneralConnectivityMap
if vsi {
allowedConnsCombined = g.v.AllowedConnsCombined
allowedConnsCombined = g.nodesConn.AllowedConnsCombined
} else {
allowedConnsCombined = g.s.AllowedConnsCombined
allowedConnsCombined = g.subnetsConn.AllowedConnsCombined
}
res := []*groupedConnLine{}
for src, nodeConns := range allowedConnsCombined {
for dst, conns := range nodeConns {
err := g.addLineToExternalGrouping(&res, conns.IsEmpty(), src, dst,
&groupedCommonProperties{conn: conns, connStrKey: conns.EnhancedString()})
if err != nil {
return err
if !conns.IsEmpty() {
err := g.addLineToExternalGrouping(&res, src, dst,
&groupedCommonProperties{conn: conns, groupingStrKey: conns.EnhancedString()})
if err != nil {
return err
}
}
}
}
Expand All @@ -275,30 +294,44 @@ func (g *GroupConnLines) groupExternalAddressesForDiff(thisMinusOther bool) erro
var res []*groupedConnLine
var connRemovedChanged connectivityDiff
if thisMinusOther {
connRemovedChanged = g.d.cfg1ConnRemovedFrom2
connRemovedChanged = g.diff.cfg1ConnRemovedFrom2
} else {
connRemovedChanged = g.d.cfg2ConnRemovedFrom1
connRemovedChanged = g.diff.cfg2ConnRemovedFrom1
}
for src, endpointConnDiff := range connRemovedChanged {
for dst, connDiff := range endpointConnDiff {
connDiffString := connDiffEncode(src, dst, connDiff)
connsEmpty := connDiff.conn1.IsEmpty() && connDiff.conn2.IsEmpty()
err := g.addLineToExternalGrouping(&res, connsEmpty, src, dst,
&groupedCommonProperties{connDiff: connDiff, connStrKey: connDiffString})
if err != nil {
return err
if !(connDiff.conn1.IsEmpty() && connDiff.conn2.IsEmpty()) {
err := g.addLineToExternalGrouping(&res, src, dst,
&groupedCommonProperties{connDiff: connDiff, groupingStrKey: connDiffString})
if err != nil {
return err
}
}
}
}
g.appendGrouped(res)
return nil
}

func (g *GroupConnLines) addLineToExternalGrouping(res *[]*groupedConnLine, emptyConn bool,
src, dst VPCResourceIntf, commonProps *groupedCommonProperties) error {
if emptyConn {
return nil
// group public internet ranges for explainability lines
func (g *GroupConnLines) groupExternalAddressesForExplainability() error {
var res []*groupedConnLine
for _, rulesSrcDst := range *g.explain {
connStr := rulesSrcDst.conn.String() + semicolon
groupingStrKey := connStr + rulesSrcDst.rules.rulesEncode(g.config)
err := g.addLineToExternalGrouping(&res, rulesSrcDst.src, rulesSrcDst.dst,
&groupedCommonProperties{conn: rulesSrcDst.conn, rules: rulesSrcDst.rules, groupingStrKey: groupingStrKey})
if err != nil {
return err
}
}
g.appendGrouped(res)
return nil
}

func (g *GroupConnLines) addLineToExternalGrouping(res *[]*groupedConnLine,
src, dst VPCResourceIntf, commonProps *groupedCommonProperties) error {
srcNode, srcIsNode := src.(Node)
dstNode, dstIsNode := dst.(Node)
if dst.IsExternal() && !dstIsNode ||
Expand Down Expand Up @@ -354,7 +387,7 @@ func (g *GroupConnLines) groupLinesByKey(srcGrouping, groupVsi bool) (res []*gro
res = append(res, line)
continue
}
key := getKeyOfGroupConnLines(dstOrSrc, line.commonProperties.connStrKey)
key := getKeyOfGroupConnLines(dstOrSrc, line.commonProperties.groupingStrKey)
if _, ok := groupingSrcOrDst[key]; !ok {
groupingSrcOrDst[key] = []*groupedConnLine{}
}
Expand Down Expand Up @@ -382,7 +415,7 @@ func (g *GroupConnLines) groupInternalSrcOrDst(srcGrouping, groupVsi bool) {
}
var groupedSrcOrDst []EndpointElem
if groupVsi {
groupedSrcOrDst = vsiGroupingBySubnets(g, srcOrDstGroup, g.c)
groupedSrcOrDst = vsiGroupingBySubnets(g, srcOrDstGroup, g.config)
} else {
groupedSrcOrDst = subnetGrouping(g, srcOrDstGroup)
}
Expand Down Expand Up @@ -504,11 +537,20 @@ func (g *groupedExternalNodes) String() string {
// 2. connection of config1
// 3. connection of config2
// 4. info regarding missing endpoints: e.g. vsi0 removed
//
// this encoding prevents the need to change all the grouping datastuctures
// along the pipe: srcToDst and dstToSrc in addition to GroupedConnLine
func connDiffEncode(src, dst VPCResourceIntf, connDiff *connectionDiff) string {
conn1Str, conn2Str := conn1And2Str(connDiff)
diffType, endpointsDiff := diffAndEndpointsDescription(connDiff.diff, src, dst, connDiff.thisMinusOther)
return strings.Join([]string{diffType, conn1Str, conn2Str, endpointsDiff}, semicolon)
}

// encodes rulesConnection for grouping
func (rules *rulesConnection) rulesEncode(c *VPCConfig) string {
egressStr, ingressStr := "", ""
if len(rules.egressRules) > 0 {
egressStr = "egress:" + rules.egressRules.string(c) + semicolon
}
if len(rules.ingressRules) > 0 {
egressStr = "ingress:" + rules.ingressRules.string(c) + semicolon
}
return egressStr + ingressStr
}
Loading

0 comments on commit 46858a6

Please sign in to comment.