diff --git a/pkg/ibmvpc/examples/acl_testing5subnetsDiff.txt b/pkg/ibmvpc/examples/acl_testing5subnetsDiff.txt index 5a3d7510e..3119e1da7 100644 --- a/pkg/ibmvpc/examples/acl_testing5subnetsDiff.txt +++ b/pkg/ibmvpc/examples/acl_testing5subnetsDiff.txt @@ -1,9 +1,10 @@ Analysis for diff between VPC test-vpc-ky1 and VPC test-vpc-ky2 --- sub1-2-ky => sub1-1-ky : missing connection --- sub1-3-ky => sub1-1-ky : missing connection - -++ sub2-1-ky => Public Internet [8.8.8.0/29] : missing connection -++ sub2-1-ky => Public Internet [8.8.8.10/31] : missing connection -++ sub2-1-ky => Public Internet [8.8.8.12/30] : missing connection -++ sub2-1-ky => Public Internet [8.8.8.8/32] : changed connection protocol: UDP dst-ports: 43 -++ sub2-1-ky => Public Internet [8.8.8.9/32] : missing connection +diff-type: changed, source: sub1-1-ky, destination: sub1-2-ky, config1: protocol: TCP, config2: protocol: TCP *, subnets-diff-info: +diff-type: changed, source: sub1-1-ky, destination: sub1-3-ky, config1: protocol: TCP, config2: protocol: TCP *, subnets-diff-info: +diff-type: changed, source: sub2-1-ky, destination: Public Internet [8.8.8.8/32], config1: protocol: UDP dst-ports: 53, config2: protocol: UDP dst-ports: 43,53, subnets-diff-info: +diff-type: removed, source: sub1-2-ky, destination: sub1-1-ky, config1: protocol: TCP, config2: No connection, subnets-diff-info: +diff-type: removed, source: sub1-3-ky, destination: sub1-1-ky, config1: protocol: TCP, config2: No connection, subnets-diff-info: +diff-type: added, source: sub2-1-ky, destination: Public Internet [8.8.8.0/29], config1: No connection, config2: protocol: UDP dst-ports: 53 *, subnets-diff-info: +diff-type: added, source: sub2-1-ky, destination: Public Internet [8.8.8.10/31], config1: No connection, config2: protocol: UDP dst-ports: 53 *, subnets-diff-info: +diff-type: added, source: sub2-1-ky, destination: Public Internet [8.8.8.12/30], config1: No connection, config2: protocol: UDP dst-ports: 53 *, subnets-diff-info: +diff-type: added, source: sub2-1-ky, destination: Public Internet [8.8.8.9/32], config1: No connection, config2: protocol: UDP dst-ports: 53 *, subnets-diff-info: diff --git a/pkg/vpcmodel/diffSubnets_test.go b/pkg/vpcmodel/diffSubnets_test.go index 3d6ac12af..3b7abd7cd 100644 --- a/pkg/vpcmodel/diffSubnets_test.go +++ b/pkg/vpcmodel/diffSubnets_test.go @@ -22,15 +22,15 @@ import ( // subnet3 -> subnet2 // subnet3 -> subnet4 -// expected diff cfg1 subtract cfg2: -// cfg1 subtract cfg2 +// expected diff cfg1 connMissingOrChanged cfg2: +// cfg1 connMissingOrChanged 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: +// cfg2 connMissingOrChanged cfg1 +// subnet1 connMissingOrChanged subnet2: // subnet3 -> subnet4 different connection func configSimpleSubnetSubtract() (subnetConfigConn1, subnetConfigConn2 *SubnetConfigConnectivity) { @@ -50,10 +50,12 @@ func configSimpleSubnetSubtract() (subnetConfigConn1, subnetConfigConn2 *SubnetC 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"}) + &mockNetIntf{cidr: "10.9.20.7/32", name: "vsi2-3"}, + &mockNetIntf{cidr: "11.4.20.6/32", name: "vsi2-4"}) 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]}}) + &mockSubnet{"10.4.20.0/22", "subnet4", []Node{cfg2.Nodes[2]}}, + &mockSubnet{"11.4.20.0/22", "subnet5", []Node{cfg2.Nodes[3]}}) connectionTCP := common.NewConnectionSet(false) connectionTCP.AddTCPorUDPConn(common.ProtocolTCP, 10, 100, 443, 443) @@ -68,6 +70,7 @@ func configSimpleSubnetSubtract() (subnetConfigConn1, subnetConfigConn2 *SubnetC 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)) + subnetConnMap2.AllowedConnsCombined.updateAllowedSubnetConnsMap(cfg2.NodeSets[2], cfg2.NodeSets[3], common.NewConnectionSet(true)) subnetConfigConn1 = &SubnetConfigConnectivity{cfg1, subnetConnMap1.AllowedConnsCombined} subnetConfigConn2 = &SubnetConfigConnectivity{cfg2, subnetConnMap2.AllowedConnsCombined} @@ -77,31 +80,35 @@ func configSimpleSubnetSubtract() (subnetConfigConn1, subnetConfigConn2 *SubnetC func TestSimpleSubnetSubtract(t *testing.T) { subnetConfigConn1, subnetConfigConn2 := configSimpleSubnetSubtract() - subnet1Subtract2, err := subnetConfigConn1.subtract(subnetConfigConn2) + subnet1Subtract2, err := subnetConfigConn1.connMissingOrChanged(subnetConfigConn2, true) if err != nil { fmt.Println("error:", err.Error()) } - require.Equal(t, err, nil) subnet1Subtract2Str := subnet1Subtract2.EnhancedString(true) fmt.Printf("subnet1Subtract2:\n%v\n", subnet1Subtract2Str) + require.Equal(t, err, nil) 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") - - cfg2Subtract1, err := subnetConfigConn2.subtract(subnetConfigConn1) + require.Equal(t, 5, newLines) + require.Contains(t, subnet1Subtract2Str, "diff-type: removed, source: subnet0, destination: subnet1, "+ + "config1: All Connections, config2: No connection, subnets-diff-info: subnet0 and subnet1 removed") + require.Contains(t, subnet1Subtract2Str, "diff-type: removed, source: subnet1, destination: subnet2, "+ + "config1: All Connections, config2: No connection, subnets-diff-info: subnet1 removed") + require.Contains(t, subnet1Subtract2Str, "diff-type: removed, source: subnet2, destination: subnet3, "+ + "config1: All Connections, config2: No connection, subnets-diff-info:") + require.Contains(t, subnet1Subtract2Str, "diff-type: removed, source: subnet3, destination: subnet1, "+ + "config1: All Connections, config2: No connection, subnets-diff-info: subnet1 removed") + require.Contains(t, subnet1Subtract2Str, "diff-type: changed, source: subnet3, destination: subnet4, "+ + "config1: protocol: TCP src-ports: 10-100 dst-ports: 443, config2: All Connections, subnets-diff-info:") + + cfg2Subtract1, err := subnetConfigConn2.connMissingOrChanged(subnetConfigConn1, false) if err != nil { fmt.Println("error:", err.Error()) } require.Equal(t, err, nil) subnet2Subtract1Str := cfg2Subtract1.EnhancedString(false) - fmt.Printf("cfg2Subtract1:\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) + fmt.Printf("cfg2Subtract1:\n%v", subnet2Subtract1Str) + require.Equal(t, subnet2Subtract1Str, "diff-type: added, source: subnet4, destination: subnet5, config1: "+ + "No connection, config2: All Connections, subnets-diff-info: subnet5 added\n") } func configSimpleIPAndSubnetSubtract() (subnetConfigConn1, subnetConfigConn2 *SubnetConfigConnectivity) { @@ -158,7 +165,7 @@ func TestSimpleIPAndSubnetSubtract(t *testing.T) { } // verified bit by bit :-) - cfg1SubCfg2, err := alignedCfgConn1.subtract(alignedCfgConn2) + cfg1SubCfg2, err := alignedCfgConn1.connMissingOrChanged(alignedCfgConn2, true) if err != nil { fmt.Println("error:", err.Error()) } @@ -166,14 +173,19 @@ func TestSimpleIPAndSubnetSubtract(t *testing.T) { cfg1SubtractCfg2Str := cfg1SubCfg2.EnhancedString(true) fmt.Printf("cfg1SubCfg2:\n%v\n", cfg1SubtractCfg2Str) newLines := strings.Count(cfg1SubtractCfg2Str, "\n") - // there should be 6 lines in subnet1Subtract2Str require.Equal(t, 7, newLines) - require.Contains(t, cfg1SubtractCfg2Str, "-- Public Internet [250.2.4.4/30] => subnet2 : missing connection") - require.Contains(t, cfg1SubtractCfg2Str, "-- Public Internet [250.2.4.4/30] => subnet2 : missing connection") - require.Contains(t, cfg1SubtractCfg2Str, "-- Public Internet [250.2.4.64/26] => subnet2 : missing connection") - require.Contains(t, cfg1SubtractCfg2Str, "-- Public Internet [250.2.4.128/25] => subnet2 : missing connection") - require.Contains(t, cfg1SubtractCfg2Str, "-- Public Internet [250.2.4.8/29] => subnet2 : missing connection") - require.Contains(t, cfg1SubtractCfg2Str, "-- Public Internet [250.2.4.32/27] => subnet2 : missing connection") - require.Contains(t, cfg1SubtractCfg2Str, "-- subnet2 => Public Internet [200.2.4.0/24] : changed connection "+ - "protocol: TCP src-ports: 1-1000 dst-ports: 444-65535; protocol: TCP src-ports: 1001-65535; protocol: UDP,ICMP") + require.Contains(t, cfg1SubtractCfg2Str, "diff-type: removed, source: Public Internet [250.2.4.128/25], destination: subnet2, "+ + "config1: All Connections, config2: No connection, subnets-diff-info:") + require.Contains(t, cfg1SubtractCfg2Str, "diff-type: removed, source: Public Internet [250.2.4.16/28], destination: subnet2, "+ + "config1: All Connections, config2: No connection, subnets-diff-info:") + require.Contains(t, cfg1SubtractCfg2Str, "diff-type: removed, source: Public Internet [250.2.4.32/27], destination: subnet2, "+ + "config1: All Connections, config2: No connection, subnets-diff-info:") + require.Contains(t, cfg1SubtractCfg2Str, "diff-type: removed, source: Public Internet [250.2.4.4/30], destination: subnet2, "+ + "config1: All Connections, config2: No connection, subnets-diff-info:") + require.Contains(t, cfg1SubtractCfg2Str, "diff-type: removed, source: Public Internet [250.2.4.64/26], destination: subnet2, "+ + "config1: All Connections, config2: No connection, subnets-diff-info:") + require.Contains(t, cfg1SubtractCfg2Str, "diff-type: removed, source: Public Internet [250.2.4.8/29], destination: subnet2, "+ + "config1: All Connections, config2: No connection, subnets-diff-info:") + require.Contains(t, cfg1SubtractCfg2Str, "diff-type: changed, source: subnet2, destination: Public Internet [200.2.4.0/24], "+ + "config1: All Connections, config2: protocol: TCP src-ports: 0-1000 dst-ports: 0-443, subnets-diff-info:") } diff --git a/pkg/vpcmodel/semanticDiffSubnets.go b/pkg/vpcmodel/semanticDiffSubnets.go index 296634f0a..c45349a7c 100644 --- a/pkg/vpcmodel/semanticDiffSubnets.go +++ b/pkg/vpcmodel/semanticDiffSubnets.go @@ -11,21 +11,22 @@ import ( type DiffType = int const ( - NoDiff DiffType = iota - MissingSrcEP - MissingDstEP - MissingSrcDstEP - MissingConnection - ChangedConnection + noDiff DiffType = iota + missingSrcEP + missingDstEP + missingSrcDstEP + missingConnection + changedConnection ) const ( - CastingNodeErr = "%s should be external node but casting to Node failed" + castingNodeErr = "%s should be external node but casting to Node failed" ) type connectionDiff struct { - *common.ConnectionSet - diff DiffType + conn1 *common.ConnectionSet + conn2 *common.ConnectionSet + diff DiffType } type SubnetsDiff map[VPCResourceIntf]map[VPCResourceIntf]*connectionDiff @@ -66,11 +67,11 @@ func (configs ConfigsForDiff) GetSubnetsDiff(grouping bool) (*DiffBetweenSubnets if err != nil { return nil, err } - subnet1Subtract2, err1 := alignedConfigConnectivity1.subtract(alignedConfigConnectivity2) + subnet1Subtract2, err1 := alignedConfigConnectivity1.connMissingOrChanged(alignedConfigConnectivity2, true) if err1 != nil { return nil, err1 } - subnet2Subtract1, err2 := alignedConfigConnectivity2.subtract(alignedConfigConnectivity1) + subnet2Subtract1, err2 := alignedConfigConnectivity2.connMissingOrChanged(alignedConfigConnectivity1, false) if err2 != nil { return nil, err2 } @@ -93,7 +94,7 @@ func (c *VPCConfig) getVPCResourceInfInOtherConfig(other *VPCConfig, ep VPCResou nodeSameCidr := findNodeWithCidr(other.Nodes, ep.(Node).Cidr()) return nodeSameCidr, nil } - return nil, fmt.Errorf(CastingNodeErr, node.Name()) + return nil, fmt.Errorf(castingNodeErr, node.Name()) } for _, nodeSet := range other.NodeSets { if nodeSet.Name() == ep.Name() { @@ -104,9 +105,12 @@ func (c *VPCConfig) getVPCResourceInfInOtherConfig(other *VPCConfig, ep VPCResou return nil, nil } -// subtract Subtract one SubnetConnectivityMap from the other +// connMissingOrChanged of subnetConfConnectivity w.r.t. the other: +// connections may be identical, non-existing in other or existing in other but changed; +// the latter are included only if includeChanged, to avoid duplication in the final presentation +// // assumption: any connection from connectivity and "other" have src (dst) which are either disjoint or equal -func (subnetConfConnectivity *SubnetConfigConnectivity) subtract(other *SubnetConfigConnectivity) ( +func (subnetConfConnectivity *SubnetConfigConnectivity) connMissingOrChanged(other *SubnetConfigConnectivity, includeChanged bool) ( connectivitySubtract SubnetsDiff, err error) { connectivitySubtract = map[VPCResourceIntf]map[VPCResourceIntf]*connectionDiff{} for src, endpointConns := range subnetConfConnectivity.subnetConnectivity { @@ -117,7 +121,6 @@ func (subnetConfConnectivity *SubnetConfigConnectivity) subtract(other *SubnetCo if _, ok := connectivitySubtract[src]; !ok { connectivitySubtract[src] = map[VPCResourceIntf]*connectionDiff{} } - diffConnectionWithType := &connectionDiff{nil, NoDiff} srcInOther, err1 := subnetConfConnectivity.config.getVPCResourceInfInOtherConfig(other.config, src) if err1 != nil { return nil, err1 @@ -126,25 +129,24 @@ func (subnetConfConnectivity *SubnetConfigConnectivity) subtract(other *SubnetCo if err2 != nil { return nil, err2 } + connDiff := &connectionDiff{conns, nil, missingConnection} if srcInOther != nil && dstInOther != nil { if otherSrc, ok := other.subnetConnectivity[srcInOther]; ok { - if otherSrcDst, ok := otherSrc[dstInOther]; ok { - // ToDo: https://github.com/np-guard/vpc-network-config-analyzer/issues/199 - subtractConn := conns.Subtract(otherSrcDst) - if subtractConn.IsEmpty() { - continue // no diff + if otherConn, ok := otherSrc[dstInOther]; ok { + equalConnections := conns.Equal(otherConn) && + // ToDo: https://github.com/np-guard/vpc-network-config-analyzer/issues/199 + conns.IsStateful == otherConn.IsStateful + if !includeChanged || equalConnections { + continue } - diffConnectionWithType.ConnectionSet = subtractConn - diffConnectionWithType.diff = ChangedConnection + connDiff.conn2 = otherConn + connDiff.diff = changedConnection } } - if diffConnectionWithType.diff != ChangedConnection { - diffConnectionWithType.diff = MissingConnection - } } else { // srcInOther == nil || dstInOther == nil - diffConnectionWithType.diff = getDiffType(src, srcInOther, dst, dstInOther) + connDiff.diff = getDiffType(src, srcInOther, dst, dstInOther) } - connectivitySubtract[src][dst] = diffConnectionWithType + connectivitySubtract[src][dst] = connDiff } } return connectivitySubtract, nil @@ -160,41 +162,40 @@ func getDiffType(src, srcInOther, dst, dstInOther VPCResourceIntf) DiffType { missingDst := dstInOther == nil && dstIsSubnet switch { case missingSrc && missingDst: - return MissingSrcDstEP + return missingSrcDstEP case missingSrc: - return MissingSrcEP + return missingSrcEP case missingDst: - return MissingDstEP + return missingDstEP case srcInOther == nil || dstInOther == nil: - return MissingConnection + return missingConnection } - return NoDiff + 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 (diff *DiffBetweenSubnets) String() string { - return diff.subnet1Subtract2.EnhancedString(true) + "\n" + + return diff.subnet1Subtract2.EnhancedString(true) + diff.subnet2Subtract1.EnhancedString(false) } func (subnetDiff *SubnetsDiff) EnhancedString(thisMinusOther bool) string { - var diffDirection string strList := []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() + conn1Str, conn2Str := "", "" + if thisMinusOther { + conn1Str = connStr(connDiff.conn1) + conn2Str = connStr(connDiff.conn2) + } else { + conn1Str = connStr(connDiff.conn2) + conn2Str = connStr(connDiff.conn1) } - printDiff := fmt.Sprintf("%s %s => %s : %s %s\n", diffDirection, src.Name(), dst.Name(), - diffDescription(connDiff.diff), connectionSetDiff) + diffType, endpointsDiff := diffAndEndpointsDisc(connDiff.diff, src, dst, thisMinusOther) + printDiff := fmt.Sprintf("diff-type: %s, source: %s, destination: %s, config1: %s, config2: %s, subnets-diff-info: %s\n", + diffType, src.Name(), dst.Name(), conn1Str, conn2Str, endpointsDiff) strList = append(strList, printDiff) } } @@ -203,20 +204,38 @@ func (subnetDiff *SubnetsDiff) EnhancedString(thisMinusOther bool) string { return res } -func diffDescription(diff DiffType) string { +// prints connection for func (subnetDiff *SubnetsDiff) EnhancedString(..) where the connection could be empty +func connStr(conn *common.ConnectionSet) string { + if conn == nil { + return "No connection" + } + return conn.EnhancedString() +} + +func diffAndEndpointsDisc(diff DiffType, src, dst VPCResourceIntf, thisMinusOther bool) (diffDisc, workLoad string) { + const ( + doubleString = "%s %s" + ) + addOrRemoved := "" + if thisMinusOther { + addOrRemoved = "removed" + } else { + addOrRemoved = "added" + } 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" + case missingSrcEP: + return addOrRemoved, fmt.Sprintf(doubleString, src.Name(), addOrRemoved) + case missingDstEP: + return addOrRemoved, fmt.Sprintf(doubleString, dst.Name(), addOrRemoved) + case missingSrcDstEP: + return addOrRemoved, fmt.Sprintf("%s and %s %s", + src.Name(), dst.Name(), addOrRemoved) + case missingConnection: + return addOrRemoved, "" + case changedConnection: + return "changed", "" } - return "" + return "", "" } // getConnectivesWithSameIPBlocks generates from subnet1Connectivity.AllowedConnsCombined and subnet2Connectivity.AllowedConnsCombined @@ -342,13 +361,13 @@ func (subnetConnectivity *SubnetConnectivityMap) actualAlignSrcOrDstGivenIPBlist if node, ok := src.(Node); ok { origIPBlock, err = externalNodeToIPBlock(node) } else { - return nil, fmt.Errorf(CastingNodeErr, node.Name()) + return nil, fmt.Errorf(castingNodeErr, node.Name()) } } else { if node, ok := dst.(Node); ok { origIPBlock, err = externalNodeToIPBlock(node) } else { - return nil, fmt.Errorf(CastingNodeErr, node.Name()) + return nil, fmt.Errorf(castingNodeErr, node.Name()) } } if err != nil { @@ -417,7 +436,7 @@ func (subnetConnectivity SubnetConnectivityMap) getIPBlocksList() (ipbList []*co } ipbList = append(ipbList, ipBlock) } else { - return nil, fmt.Errorf(CastingNodeErr, src.Name()) + return nil, fmt.Errorf(castingNodeErr, src.Name()) } } if dst.IsExternal() { @@ -428,7 +447,7 @@ func (subnetConnectivity SubnetConnectivityMap) getIPBlocksList() (ipbList []*co } ipbList = append(ipbList, ipBlock) } else { - return nil, fmt.Errorf(CastingNodeErr, dst.Name()) + return nil, fmt.Errorf(castingNodeErr, dst.Name()) } } }