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.

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

290 explainability public ip grouping #301

Merged
merged 13 commits into from
Jan 3, 2024
77 changes: 64 additions & 13 deletions pkg/ibmvpc/explainability_test.go
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
89 changes: 67 additions & 22 deletions pkg/vpcmodel/grouping.go
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 @@ -85,13 +88,26 @@ func newGroupConnLinesDiff(d *diffBetweenCfgs) (res *GroupConnLines, err error)
return res, err
}

func newGroupConnExplainability(c *VPCConfig, e *explainStruct) (res *GroupConnLines, err error) {
res = &GroupConnLines{
c: c,
e: e,
adisos marked this conversation as resolved.
Show resolved Hide resolved
srcToDst: newGroupingConnections(),
dstToSrc: newGroupingConnections(),
groupedEndpointsElemsMap: make(map[string]*groupedEndpointsElems),
groupedExternalNodesMap: make(map[string]*groupedExternalNodes)}
err = res.groupExternalAddressesForExplainability()
ShiriMoran marked this conversation as resolved.
Show resolved Hide resolved
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
e *explainStruct
srcToDst *groupingConnections
dstToSrc *groupingConnections
// a map to groupedEndpointsElems used by GroupedConnLine from a unified key of such elements
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 @@ -256,10 +273,12 @@ func (g *GroupConnLines) groupExternalAddresses(vsi bool) error {
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 @@ -282,23 +301,40 @@ func (g *GroupConnLines) groupExternalAddressesForDiff(thisMinusOther bool) erro
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.e {
connStr := ""
if rulesSrcDst.conn != nil {
adisos marked this conversation as resolved.
Show resolved Hide resolved
connStr = rulesSrcDst.conn.String() + semicolon
}
groupingStrKey := connStr + rulesSrcDst.rules.rulesEncode(g.c)
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 +390,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 @@ -495,11 +531,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
}
4 changes: 2 additions & 2 deletions pkg/vpcmodel/groupingSelfLoop.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ func (g *GroupConnLines) findMergeCandidates(groupingSrcOrDst map[string][]*grou
bucketToKeys := make(map[string]map[string]struct{})
for _, key := range relevantKeys {
lines := groupingSrcOrDst[key]
bucket := lines[0].commonProperties.connStrKey
bucket := lines[0].commonProperties.groupingStrKey
subnetIfVsi := g.getSubnetIfVsi(lines[0].src)
if subnetIfVsi != "" {
bucket += ";" + subnetIfVsi
Expand Down Expand Up @@ -332,7 +332,7 @@ func listOfUniqueEndpoints(oldGroupingSrcOrDst map[string][]*groupedConnLine, sr
for _, line := range oldGroupingSrcOrDst[oldKeyToMerge] {
endPointInKey := line.getSrcOrDst(!srcGrouping)
if conn == "" {
conn = line.commonProperties.connStrKey // connection is the same for all lines to be merged
conn = line.commonProperties.groupingStrKey // connection is the same for all lines to be merged
connProps = line.commonProperties
}
if _, isSliceEndpoints := endPointInKey.(*groupedEndpointsElems); isSliceEndpoints {
Expand Down
2 changes: 1 addition & 1 deletion pkg/vpcmodel/mdOutput.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,5 +99,5 @@ func connectivityLineMD(src, dst, conn string) string {
}

func getGroupedMDLine(line *groupedConnLine) string {
return connectivityLineMD(line.src.Name(), line.dst.Name(), line.commonProperties.connStrKey)
return connectivityLineMD(line.src.Name(), line.dst.Name(), line.commonProperties.groupingStrKey)
}
Loading
Loading