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

163 semantic diff subnets a #203

Merged
merged 20 commits into from
Oct 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
78401ee
structs and main functionality of https://github.com/np-guard/vpc-net…
ShiriMoran Oct 12, 2023
9a4f947
Highlevel code and structs
ShiriMoran Oct 15, 2023
15970f4
subnetConnectivitySubtract code; still needs to fill inside functiona…
ShiriMoran Oct 16, 2023
9400063
Redefined made the connection set diff and added todos
ShiriMoran Oct 17, 2023
bf18d01
Minor reorgs
ShiriMoran Oct 17, 2023
625fa41
Export SubnetConnectivityMap and enable external creation for unit test
ShiriMoran Oct 17, 2023
d2fbe4e
Added grouping subnet unittesting as preliminery stage to writing uni…
ShiriMoran Oct 18, 2023
823503e
exporting functionality for unit test; SubnetConnectivitySubtract sho…
ShiriMoran Oct 18, 2023
769eb7a
semantic diff simple unit test
ShiriMoran Oct 18, 2023
5a652cc
improved semantic diff computation
ShiriMoran Oct 18, 2023
f735908
fixed a bug/typo, added ad-hoc printing functionality
ShiriMoran Oct 18, 2023
20b4e98
unit test written for current functionality
ShiriMoran Oct 18, 2023
ce31dda
lint comments
ShiriMoran Oct 19, 2023
e45eac5
Update pkg/common/connectionset.go
ShiriMoran Oct 22, 2023
947f08f
Update pkg/vpcmodel/semanticDiffSubnets.go
ShiriMoran Oct 22, 2023
1e869e8
PR comments
ShiriMoran Oct 22, 2023
c1547ec
CR: lack of certain IP range is not considered as a missing connection
ShiriMoran Oct 22, 2023
300c664
added changed connection test
ShiriMoran Oct 23, 2023
6ca9e48
added comment emphasizing diff between connections that include exter…
ShiriMoran Oct 23, 2023
988dc41
Merge branch 'main' into 163_semantic_diff_subnets_a
ShiriMoran Oct 23, 2023
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
6 changes: 6 additions & 0 deletions pkg/common/connectionset.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,12 @@ func (conn *ConnectionSet) isAllConnectionsWithoutAllowAll() bool {
return conn.connectionProperties.Equals(getAllPropertiesObject())
}

// Subtract
// ToDo: Subtract seems to ignore IsStateful (see issue #199):
// 1. is the delta connection stateful
// 2. connectionProperties is identical but conn stateful while other is not
// the 2nd item can be computed here, with enhancement to relevant structure
// the 1st can not since we do not know where exactly the statefullness came from
func (conn *ConnectionSet) Subtract(other *ConnectionSet) *ConnectionSet {
if conn.IsEmpty() || other.IsEmpty() {
return conn
Expand Down
97 changes: 97 additions & 0 deletions pkg/vpcmodel/diffSubnets_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
package vpcmodel

import (
"fmt"
"strings"
"testing"

"github.com/stretchr/testify/require"

"github.com/np-guard/vpc-network-config-analyzer/pkg/common"
)

// simple diff:
// cfg1 has subnet0, subnet1, subnet2, subnet3, subnet4
// subnet0 -> subnet1
// subnet1 -> subnet2
// subnet3 -> subnet1
// subnet2 -> subnet3
// subnet3 -> subnet2
// subnet3 -> subnet4 not all connections
// cfg2 has subnet2, subnet3, subnet4
// subnet3 -> subnet2
// subnet3 -> subnet4

// expected diff cfg1 subtract cfg2:
// cfg1 subtract cfg2
// subnet0 -> subnet1 missing src and dst
// subnet1 -> subnet2 missing src
// subnet3 -> subnet1 missing dst
// subnet2 -> subnet3 missing connection
//
// cfg2 subtract cfg1
// subnet1 subtract subnet2:
// subnet3 -> subnet4 different connection

func configSimpleSubnetSubtract() (subnetConfigConn1, subnetConfigConn2 *SubnetConfigConnectivity) {
cfg1 := &CloudConfig{Nodes: []Node{}, NodeSets: []NodeSet{}}
cfg1.Nodes = append(cfg1.Nodes,
&mockNetIntf{cidr: "10.0.20.5/32", name: "vsi1-1"},
&mockNetIntf{cidr: "10.3.20.6/32", name: "vsi1-2"},
&mockNetIntf{cidr: "10.7.20.7/32", name: "vsi1-3"})

cfg1.NodeSets = append(cfg1.NodeSets, &mockSubnet{"10.0.20.0/22", "subnet0", []Node{cfg1.Nodes[0]}},
&mockSubnet{"10.1.20.0/22", "subnet1", []Node{cfg1.Nodes[0]}},
&mockSubnet{"10.2.20.0/22", "subnet2", []Node{cfg1.Nodes[1]}})
cfg1.NodeSets = append(cfg1.NodeSets, &mockSubnet{"10.3.20.0/22", "subnet3", []Node{cfg1.Nodes[2]}},
&mockSubnet{"10.4.20.0/22", "subnet4", []Node{cfg1.Nodes[2]}})

cfg2 := &CloudConfig{Nodes: []Node{}, NodeSets: []NodeSet{}}
cfg2.Nodes = append(cfg2.Nodes,
&mockNetIntf{cidr: "10.3.20.5/32", name: "vsi2-1"},
&mockNetIntf{cidr: "10.7.20.6/32", name: "vsi2-2"},
&mockNetIntf{cidr: "10.9.20.7/32", name: "vsi2-3"})
cfg2.NodeSets = append(cfg2.NodeSets, &mockSubnet{"10.2.20.0/22", "subnet2", []Node{cfg2.Nodes[0]}},
&mockSubnet{"10.3.20.0/22", "subnet3", []Node{cfg2.Nodes[1]}},
&mockSubnet{"10.4.20.0/22", "subnet4", []Node{cfg2.Nodes[2]}})

connectionTCP := common.NewConnectionSet(false)
connectionTCP.AddTCPorUDPConn(common.ProtocolTCP, 10, 100, 443, 443)
subnetConnMap1 := &VPCsubnetConnectivity{AllowedConnsCombined: NewSubnetConnectivityMap()}
subnetConnMap1.AllowedConnsCombined.updateAllowedSubnetConnsMap(cfg1.NodeSets[0], cfg1.NodeSets[1], common.NewConnectionSet(true))
subnetConnMap1.AllowedConnsCombined.updateAllowedSubnetConnsMap(cfg1.NodeSets[1], cfg1.NodeSets[2], common.NewConnectionSet(true))
subnetConnMap1.AllowedConnsCombined.updateAllowedSubnetConnsMap(cfg1.NodeSets[3], cfg1.NodeSets[1], common.NewConnectionSet(true))
subnetConnMap1.AllowedConnsCombined.updateAllowedSubnetConnsMap(cfg1.NodeSets[2], cfg1.NodeSets[3], common.NewConnectionSet(true))
subnetConnMap1.AllowedConnsCombined.updateAllowedSubnetConnsMap(cfg1.NodeSets[3], cfg1.NodeSets[2], common.NewConnectionSet(true))
subnetConnMap1.AllowedConnsCombined.updateAllowedSubnetConnsMap(cfg1.NodeSets[3], cfg1.NodeSets[4], connectionTCP)

subnetConnMap2 := &VPCsubnetConnectivity{AllowedConnsCombined: NewSubnetConnectivityMap()}
subnetConnMap2.AllowedConnsCombined.updateAllowedSubnetConnsMap(cfg2.NodeSets[1], cfg2.NodeSets[0], common.NewConnectionSet(true))
subnetConnMap2.AllowedConnsCombined.updateAllowedSubnetConnsMap(cfg2.NodeSets[1], cfg2.NodeSets[2], common.NewConnectionSet(true))

subnetConfigConn1 = &SubnetConfigConnectivity{cfg1, subnetConnMap1.AllowedConnsCombined}
subnetConfigConn2 = &SubnetConfigConnectivity{cfg2, subnetConnMap2.AllowedConnsCombined}

return subnetConfigConn1, subnetConfigConn2
}

func TestSimpleSubnetSubtract(t *testing.T) {
subnetConfigConn1, subnetConfigConn2 := configSimpleSubnetSubtract()
subnet1Subtract2 := subnetConfigConn1.SubnetConnectivitySubtract(subnetConfigConn2)
subnet1Subtract2Str := subnet1Subtract2.EnhancedString(true)
fmt.Printf("subnet1Subtract2:\n%v\n", subnet1Subtract2Str)
newLines := strings.Count(subnet1Subtract2Str, "\n")
// there should be 4 lines in subnet1Subtract2Str
require.Equal(t, 4, newLines)
require.Contains(t, subnet1Subtract2Str, "-- subnet3 => subnet1 : missing destination")
require.Contains(t, subnet1Subtract2Str, "-- subnet2 => subnet3 : missing connection")
require.Contains(t, subnet1Subtract2Str, "-- subnet0 => subnet1 : missing source and destination")
require.Contains(t, subnet1Subtract2Str, "-- subnet1 => subnet2 : missing source")

subnet2Subtract1 := subnetConfigConn2.SubnetConnectivitySubtract(subnetConfigConn1)
subnet2Subtract1Str := subnet2Subtract1.EnhancedString(false)
fmt.Printf("subnet2Subtract1:\n%v\n", subnet2Subtract1Str)
require.Equal(t, "++ subnet3 => subnet4 : changed connection "+
"protocol: TCP src-ports: 1-9,101-65535; protocol: TCP src-ports: "+
"10-100 dst-ports: 1-442,444-65535; protocol: UDP,ICMP\n", subnet2Subtract1Str)
}
37 changes: 37 additions & 0 deletions pkg/vpcmodel/grouping_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -371,3 +371,40 @@ func TestConfigSelfLoopCliqueLace(t *testing.T) {
fmt.Println(groupingStr)
fmt.Println("done")
}
func configSubnetSelfLoop() (*CloudConfig, *VPCsubnetConnectivity) {
res := &CloudConfig{Nodes: []Node{}}
res.Nodes = append(res.Nodes,
&mockNetIntf{cidr: "10.0.20.5/32", name: "vsi1"},
&mockNetIntf{cidr: "10.3.20.6/32", name: "vsi2"},
&mockNetIntf{cidr: "10.7.20.7/32", name: "vsi3"})

res.NodeSets = append(res.NodeSets, &mockSubnet{"10.0.20.0/22", "subnet1", []Node{res.Nodes[0]}},
&mockSubnet{"10.3.20.0/22", "subnet2", []Node{res.Nodes[1]}},
&mockSubnet{"10.7.20.0/22", "subnet3", []Node{res.Nodes[2]}})

res1 := &VPCsubnetConnectivity{AllowedConnsCombined: NewSubnetConnectivityMap()}
res1.AllowedConnsCombined.updateAllowedSubnetConnsMap(res.NodeSets[0], res.NodeSets[1], common.NewConnectionSet(true))
res1.AllowedConnsCombined.updateAllowedSubnetConnsMap(res.NodeSets[0], res.NodeSets[2], common.NewConnectionSet(true))
res1.AllowedConnsCombined.updateAllowedSubnetConnsMap(res.NodeSets[1], res.NodeSets[0], common.NewConnectionSet(true))
res1.AllowedConnsCombined.updateAllowedSubnetConnsMap(res.NodeSets[1], res.NodeSets[2], common.NewConnectionSet(true))
res1.AllowedConnsCombined.updateAllowedSubnetConnsMap(res.NodeSets[2], res.NodeSets[0], common.NewConnectionSet(true))
res1.AllowedConnsCombined.updateAllowedSubnetConnsMap(res.NodeSets[2], res.NodeSets[1], common.NewConnectionSet(true))

return res, res1
}

func TestSubnetSelfLoop(t *testing.T) {
c, s := configSubnetSelfLoop()
res := &GroupConnLines{c: c, s: s,
srcToDst: newGroupingConnections(), dstToSrc: newGroupingConnections(),
groupedEndpointsElemsMap: make(map[string]*groupedEndpointsElems),
groupedExternalNodesMap: make(map[string]*groupedExternalNodes)}
res.groupExternalAddressesForSubnets()
res.groupInternalSrcOrDst(false, false)
res.groupInternalSrcOrDst(true, false)
groupingStr := res.String()
require.Equal(t, "subnet1,subnet2,subnet3 => subnet1,subnet2,subnet3 : All Connections\n\n"+
"connections are stateful unless marked with *\n", groupingStr)
fmt.Println(groupingStr)
fmt.Println("done")
}
1 change: 1 addition & 0 deletions pkg/vpcmodel/output.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ func NewOutputGenerator(c *CloudConfig, grouping bool, uc OutputUseCase, archOnl
res.subnetsConn = subnetsConn
}
}
// todo: add for diff
return res, nil
}

Expand Down
222 changes: 222 additions & 0 deletions pkg/vpcmodel/semanticDiffSubnets.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,222 @@
package vpcmodel

import (
"fmt"

"github.com/np-guard/vpc-network-config-analyzer/pkg/common"
)

// ToDo: getConnectivesWithSameIPBlocks not yet implemented - namely, diff between connections that include external addresses
// is not yet supported

type DiffType = int

const (
NoDiff DiffType = iota
MissingSrcEP
MissingDstEP
MissingSrcDstEP
MissingConnection
ChangedConnection
)

type connectionDiff struct {
*common.ConnectionSet
diff DiffType
}

type SubnetsDiff map[EndpointElem]map[EndpointElem]*connectionDiff

type ConfigsForDiff struct {
config1 *CloudConfig
config2 *CloudConfig
}

type SubnetConfigConnectivity struct {
config *CloudConfig
subnetConnectivity SubnetConnectivityMap
}

type diffBetweenSubnets struct {
subnet1Subtract2 SubnetsDiff
subnet2Subtract1 SubnetsDiff

GroupedSubnet1Minus2 *GroupConnLines
GroupedSubnet1Minus1 *GroupConnLines
}

func (configs ConfigsForDiff) GetSubnetsDiff(grouping bool) (*diffBetweenSubnets, error) {
// 1. compute connectivity for each of the subnets
subnetsConn1, err := configs.config1.GetSubnetsConnectivity(true, false)
if err != nil {
return nil, nil
}
subnetsConn2, err := configs.config2.GetSubnetsConnectivity(true, false)
if err != nil {
return nil, nil
}

// 2. Computes delta in both directions
subnet1Aligned, subnet2Aligned := subnetsConn1.AllowedConnsCombined.getConnectivesWithSameIPBlocks(subnetsConn2.AllowedConnsCombined)
subnetConfigConnectivity1 := SubnetConfigConnectivity{configs.config1, subnet1Aligned}
subnetConfigConnectivity2 := SubnetConfigConnectivity{configs.config2, subnet2Aligned}
subnet1Subtract2 := subnetConfigConnectivity1.SubnetConnectivitySubtract(&subnetConfigConnectivity2)
subnet2Subtract1 := subnetConfigConnectivity2.SubnetConnectivitySubtract(&subnetConfigConnectivity1)

// 3. ToDo: grouping, see comment at the end of this file

res := &diffBetweenSubnets{
subnet1Subtract2: subnet1Subtract2,
subnet2Subtract1: subnet2Subtract1}
return res, nil
}

// for a given EndpointElem (representing a subnet or an external ip) in config return the EndpointElem representing the
// subnet/external address in otherConfig or nil if the subnet does not exist in the other config.
// ToDo: this is done based on names only at the moment. Perhaps take into account other factors such as cidr?
// ToDo: instead of performing this search each time, use a map created once
func (c *CloudConfig) getEndpointElemInOtherConfig(other *CloudConfig, ep EndpointElem) EndpointElem {
if ep.IsExternal() {
for _, node := range other.Nodes {
if node.Name() == ep.Name() {
res := EndpointElem(node)
return res
}
}
adisos marked this conversation as resolved.
Show resolved Hide resolved
} else {
for _, nodeSet := range other.NodeSets {
if nodeSet.Name() == ep.Name() {
res := EndpointElem(nodeSet)
return res
}
}
}
return nil
}

// generates from subnet1Connectivity.AllowedConnsCombined and subnet2Connectivity.AllowedConnsCombined
// Two equivalent SubnetConnectivityMap objects s.t. any (src1, dst1) of subnet1Connectivity and
// (src2, dst2) of subnet2Connectivity are either:
// 1. src1 disjoint src2 or dst1 disjoint dst2
// 2. src1 = src2 and dst1 = dst2
// What is done here is repartitioning the ipBlocks so that the above will hold
//
// todo: verify that the returns objects indeed have exactly the same ipBlocks
func (connectivity SubnetConnectivityMap) getConnectivesWithSameIPBlocks(other SubnetConnectivityMap) (
alignedConnectivity SubnetConnectivityMap, alignedOther SubnetConnectivityMap) {
// todo: use DisjointIPBlocks(set1, set2 []*IPBlock) []*IPBlock of ipBlock.go
alignedConnectivity = connectivity
alignedOther = other
return
}

// SubnetConnectivitySubtract Subtract one SubnetConnectivityMap from the other
// assumption: any connection from connectivity and "other" have src (dst) which are either disjoint or equal
func (subnetConfConnectivity *SubnetConfigConnectivity) SubnetConnectivitySubtract(other *SubnetConfigConnectivity) SubnetsDiff {
connectivitySubtract := map[EndpointElem]map[EndpointElem]*connectionDiff{}
for src, endpointConns := range subnetConfConnectivity.subnetConnectivity {
for dst, conns := range endpointConns {
if conns.IsEmpty() {
continue
}
if _, ok := connectivitySubtract[src]; !ok {
connectivitySubtract[src] = map[EndpointElem]*connectionDiff{}
}
diffConnectionWithType := &connectionDiff{nil, NoDiff}
srcInOther := subnetConfConnectivity.config.getEndpointElemInOtherConfig(other.config, src)
dstInOther := subnetConfConnectivity.config.getEndpointElemInOtherConfig(other.config, dst)
adisos marked this conversation as resolved.
Show resolved Hide resolved
if srcInOther != nil && dstInOther != nil {
if otherSrc, ok := other.subnetConnectivity[srcInOther]; ok {
if otherSrcDst, ok := otherSrc[dstInOther]; ok {
// ToDo: current missing stateful:
// todo 1. is the delta connection stateful
// todo 2. connectionProperties is identical but conn stateful while other is not
// the 2nd item can be computed by conns.Subtract, with enhancement to relevant structure
// the 1st can not since we do not know where exactly the statefullness came from
// we might need to repeat the statefullness computation for the delta connection
subtractConn := conns.Subtract(otherSrcDst)
if subtractConn.IsEmpty() {
continue // no diff
}
diffConnectionWithType.ConnectionSet = subtractConn
diffConnectionWithType.diff = ChangedConnection
}
}
if diffConnectionWithType.diff != ChangedConnection {
diffConnectionWithType.diff = MissingConnection
}
} else { // srcInOther == nil || dstInOther == nil
diffConnectionWithType.diff = getDiffType(src, srcInOther, dst, dstInOther)
}
connectivitySubtract[src][dst] = diffConnectionWithType
}
}
return connectivitySubtract
}

// lack of a subnet is marked as a missing endpoint
// a lack of identical external endpoint is considered as a missing connection
// and not as a missing endpoint
func getDiffType(src, srcInOther, dst, dstInOther EndpointElem) DiffType {
_, srcIsSubnet := src.(NodeSet)
_, dstIsSubnet := dst.(NodeSet)
missingSrc := srcInOther == nil && srcIsSubnet
missingDst := dstInOther == nil && dstIsSubnet
switch {
case missingSrc && missingDst:
return MissingSrcDstEP
case missingSrc:
return MissingSrcEP
case missingDst:
return MissingDstEP
case srcInOther == nil || dstInOther == nil:
return MissingConnection
}
return NoDiff
}

// EnhancedString ToDo: likely the current printing functionality will no longer be needed once the grouping is added
// anyways the diff print will be worked on before the final merge
func (subnetDiff *SubnetsDiff) EnhancedString(thisMinusOther bool) string {
var diffDirection, printDiff string
if thisMinusOther {
diffDirection = "--"
} else {
diffDirection = "++"
}
for src, endpointConnDiff := range *subnetDiff {
for dst, connDiff := range endpointConnDiff {
var connectionSetDiff string
if connDiff.ConnectionSet != nil {
connectionSetDiff = connDiff.ConnectionSet.EnhancedString()
}
printDiff += fmt.Sprintf("%s %s => %s : %s %s\n", diffDirection, src.Name(), dst.Name(),
diffDescription(connDiff.diff), connectionSetDiff)
}
}
return printDiff
}

func diffDescription(diff DiffType) string {
switch diff {
case MissingSrcEP:
return "missing source"
case MissingDstEP:
return "missing destination"
case MissingSrcDstEP:
return "missing source and destination"
case MissingConnection:
return "missing connection"
case ChangedConnection:
return "changed connection"
}
return ""
}

// todo: instead of adding functionality to grouping, I plan to have more generic connectivity items that will be grouped
// encode the SubnetsDiff into this generic item as well as the other entities we are grouping
// and then decode in the printing
// the idea is to use instead of *common.ConnectionSet in the grouped entity a string which will encode the connection
// and also the diff where relevant
// this will requires some rewriting in the existing grouping functionality and the way it provides
// service to subnetsConnectivity and nodesConnectivity
Loading
Loading