Skip to content

Commit

Permalink
318 improve unittests (#933)
Browse files Browse the repository at this point in the history 
* removing function AnalyzeNACLRules and fix tests

* unit-tests improvements
  • Loading branch information
@olasaadi99
olasaadi99 authored Dec 2, 2024
1 parent 2ec517c commit 55b39e4
Show file tree
Hide file tree
Showing 3 changed files with 327 additions and 178 deletions.
49 changes: 0 additions & 49 deletions pkg/commonvpc/nacl_analysis.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -252,55 +252,6 @@ func initConnectivityResult(connectivityMap map[string]*ConnectivityResult, indx
}
}

func getConnStr(src, dst, conn string) string {
return fmt.Sprintf("%s => %s : %s\n", src, dst, conn)
}

// AnalyzeNACLRules todo: this is used only in testing. Did not expand for deny.
func (na *NACLAnalyzer) AnalyzeNACLRules(rules []*NACLRule, subnet *netset.IPBlock,
isIngress bool, subnetDisjointTarget *netset.IPBlock,
) (string, *ConnectivityResult) {
res := []string{}
connResult := &ConnectivityResult{IsIngress: isIngress}
connResult.AllowedConns = map[*netset.IPBlock]*netset.TransportSet{}
connResult.DeniedConns = map[*netset.IPBlock]*netset.TransportSet{}
if subnetDisjointTarget == nil {
connResult = nil
}
if isIngress {
disjointSrcPeers, disjointDstPeers := getDisjointPeersForIngressAnalysis(rules, subnet)
// ingress
for _, src := range disjointSrcPeers {
allowedIngressConns, _, allowRules, _ := GetAllowedXgressConnections(rules, src, subnet, disjointDstPeers, true)
for dst, conn := range allowedIngressConns {
res = append(res, getConnStr(src.ToIPRanges(), dst, common.LongString(conn)))
dstIP, err := netset.IPBlockFromIPRangeStr(dst)
if err == nil && subnetDisjointTarget != nil && subnetDisjointTarget.IsSubset(dstIP) {
connResult.AllowedConns[src] = conn
// the indexing of allowedIngressConns and allowRules are identical
connResult.AllowRules[src] = allowRules[dst]
}
}
}
return strings.Join(res, ""), connResult
}
// egress
disjointSrcPeers, disjointDstPeers := getDisjointPeersForEgressAnalysis(rules, subnet)
for _, dst := range disjointDstPeers {
allowedEgressConns, _, allowRules, _ := GetAllowedXgressConnections(rules, dst, subnet, disjointSrcPeers, false)
for src, conn := range allowedEgressConns {
res = append(res, getConnStr(src, dst.ToIPRanges(), common.LongString(conn)))
srcIP, err := netset.IPBlockFromIPRangeStr(src)
if err == nil && subnetDisjointTarget != nil && subnetDisjointTarget.IsSubset(srcIP) {
connResult.AllowedConns[dst] = conn
// the indexing of allowedEgressConns and allowRules are identical
connResult.AllowRules[dst] = allowRules[src]
}
}
}
return strings.Join(res, ""), connResult
}

// TODO: return a map from each possible subnetDisjointTarget to its ConnectivityResult, instead of a specific ConnectivityResult
// get allowed and denied connections (ingress and egress) for a certain subnet to which this nacl is applied
func (na *NACLAnalyzer) AnalyzeNACL(subnet *netset.IPBlock) (
Expand Down
90 changes: 56 additions & 34 deletions pkg/ibmvpc/connectivityAnalysis_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -156,52 +156,74 @@ var nc6 = &naclConfig{

// tests below

var expectedConnStrTest1 = `vsi-0-subnet-1[10.240.10.4] => vsi-0-subnet-2[10.240.20.4] : All Connections
vsi-0-subnet-2[10.240.20.4] => vsi-0-subnet-1[10.240.10.4] : All Connections
`

func TestAnalyzeConnectivity1(t *testing.T) {
runConnectivityTest(t, tc1, []*naclConfig{nc1}, expectedConnStrTest1)
}

var expectedConnStrTest2 = `vsi-0-subnet-1[10.240.10.4] => vsi-0-subnet-2[10.240.20.4] : TCP * ; ICMP,UDP
`

func TestAnalyzeConnectivity2(t *testing.T) {
runConnectivityTest(t, tc1, []*naclConfig{nc2, nc3}, expectedConnStrTest2)
type ConnectivityAnalysisTest struct {
name string
tc *testNodesConfig
ncList []*naclConfig
expectedStrResult string
}

var expectedConnStrTest2a = `vsi-0-subnet-1[10.240.10.4] => vsi-0-subnet-2[10.240.20.4] : ICMP
` // ICMP is actually enabled only unidirectional in this case, but responsive analysis does not apply to ICMP

func TestAnalyzeConnectivity2a(t *testing.T) {
runConnectivityTest(t, tc1, []*naclConfig{nc2a, nc3a}, expectedConnStrTest2a)
}

var expectedConnStrTest3 = `vsi-0-subnet-1[10.240.10.4] => vsi-0-subnet-2[10.240.20.4] : All Connections
var connectivityAnalysisTests = []*ConnectivityAnalysisTest{
{
name: "testAnalyzeConnectivity1",
tc: tc1,
ncList: []*naclConfig{nc1},
expectedStrResult: `vsi-0-subnet-1[10.240.10.4] => vsi-0-subnet-2[10.240.20.4] : All Connections
vsi-0-subnet-2[10.240.20.4] => vsi-0-subnet-1[10.240.10.4] : All Connections
`,
},
{
name: "testAnalyzeConnectivity2",
tc: tc1,
ncList: []*naclConfig{nc2, nc3},
expectedStrResult: `vsi-0-subnet-1[10.240.10.4] => vsi-0-subnet-2[10.240.20.4] : TCP * ; ICMP,UDP
`,
},
{
name: "testAnalyzeConnectivity2a",
tc: tc1,
ncList: []*naclConfig{nc2a, nc3a},
expectedStrResult: `vsi-0-subnet-1[10.240.10.4] => vsi-0-subnet-2[10.240.20.4] : ICMP
`, // ICMP is actually enabled only unidirectional in this case, but responsive analysis does not apply to ICMP
},
{
name: "testAnalyzeConnectivity3",
tc: tc1,
ncList: []*naclConfig{nc2, nc4},
expectedStrResult: `vsi-0-subnet-1[10.240.10.4] => vsi-0-subnet-2[10.240.20.4] : All Connections
vsi-0-subnet-2[10.240.20.4] => vsi-0-subnet-1[10.240.10.4] : TCP
`

func TestAnalyzeConnectivity3(t *testing.T) {
runConnectivityTest(t, tc1, []*naclConfig{nc2, nc4}, expectedConnStrTest3)
}

var expectedConnStrTest4 = `vsi-0-subnet-1[10.240.10.4] => vsi-0-subnet-2[10.240.20.4] : TCP src-ports: 10-100 dst-ports: 443
`,
},
{
name: "testAnalyzeConnectivity4",
tc: tc1,
ncList: []*naclConfig{nc5, nc6},
expectedStrResult: `vsi-0-subnet-1[10.240.10.4] => vsi-0-subnet-2[10.240.20.4] : TCP src-ports: 10-100 dst-ports: 443
vsi-0-subnet-2[10.240.20.4] => vsi-0-subnet-1[10.240.10.4] : TCP src-ports: 443 dst-ports: 10-100
`
`,
},
}

func TestAnalyzeConnectivity4(t *testing.T) {
runConnectivityTest(t, tc1, []*naclConfig{nc5, nc6}, expectedConnStrTest4)
func TestConnectivityAnalysis(t *testing.T) {
// connectivityAnalysisTests is the list of tests to run
for testIdx := range connectivityAnalysisTests {
tt := connectivityAnalysisTests[testIdx]
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
tt.runConnectivityTest(t)
})
}
fmt.Println("done")
}

func runConnectivityTest(t *testing.T, tc *testNodesConfig, ncList []*naclConfig, expectedStrResult string) {
c := createConfigFromTestConfig(tc, ncList)
func (tt *ConnectivityAnalysisTest) runConnectivityTest(t *testing.T) {
c := createConfigFromTestConfig(tt.tc, tt.ncList)
connectivity, err := c.GetVPCNetworkConnectivity(false, vpcmodel.NoGroupingNoConsistencyEdges)
require.Nil(t, err)
connectivityStr := connectivity.String()
fmt.Println(connectivityStr)
fmt.Println("done")
require.Equal(t, expectedStrResult, connectivityStr)
require.Equal(t, tt.expectedStrResult, connectivityStr)
}

////////////////////////////////////////////////////////////////////////////////////////////////
Expand Down
Loading

0 comments on commit 55b39e4

Please sign in to comment.