Multi vpc output (#306)

* init

* go 20

* go 20

* not 1.2

* ShowOnSubnetMode for tgw

* use go-version-file

* CR from Ziv

* removing subnetMode flag

* does not work

* another try

* change subnetmode Location

* test

* lint

* code review

* documenting

* using maps from golang 21

* first implementation

* code review

* not use pointer to map

* handle pointers to string

* single file for json

* drawio

* update testing

* adding tests to main

* support via main

* renaming

* remove code

* lint

* lint

* lint

* lint

* aMapEntry

* lint

* TextualOutputFormatter

* merge from main

* fix for json

* moving a method to common

* comments

* removing writeoutputgeneric()

* lint

* comments

* remove redundent code from main

* comment

* code review

* CR

cmd/analyzer/main.go

@@ -52,46 +52,35 @@ func analysisTypeToUseCase(inArgs *InArgs) vpcmodel.OutputUseCase {
 	return vpcmodel.AllEndpoints
 }
 
-func analysisPerVPCConfig(c *vpcmodel.VPCConfig, inArgs *InArgs, outFile string) (*vpcmodel.SingleAnalysisOutput, error) {
-	og, err := vpcmodel.NewOutputGenerator(c, nil,
+func analysisVPCConfigs(c1, c2 map[string]*vpcmodel.VPCConfig, inArgs *InArgs, outFile string) (string, error) {
+	og, err := vpcmodel.NewOutputGenerator(c1, c2,
 		*inArgs.Grouping,
 		analysisTypeToUseCase(inArgs),
-		*inArgs.OutputFormat == ARCHDRAWIOFormat)
+		false)
 	if err != nil {
-		return nil, err
+		return "", err
 	}
 
-	var genOutFile string
-	// currently for drawio output only one vpc level is supported, and not as aggregated output of multiple vpcs
-	if *inArgs.OutputFormat == ARCHDRAWIOFormat || *inArgs.OutputFormat == DRAWIOFormat {
-		genOutFile = outFile
-	}
 	outFormat := getOutputFormat(inArgs)
-	output, err := og.Generate(outFormat, genOutFile)
+	analysisOut, err := og.Generate(outFormat, outFile)
 	if err != nil {
-		return nil, fmt.Errorf(ErrorFormat, OutGenerationErr, err)
+		return "", fmt.Errorf(ErrorFormat, OutGenerationErr, err)
 	}
 
-	return output, nil
+	return analysisOut, nil
 }
 
-func analysisDiffVPCConfig(c1, c2 *vpcmodel.VPCConfig, inArgs *InArgs, outFile string) (*vpcmodel.SingleAnalysisOutput, error) {
-	og, err := vpcmodel.NewOutputGenerator(c1, c2,
-		*inArgs.Grouping,
-		analysisTypeToUseCase(inArgs),
-		false)
-	if err != nil {
-		return nil, err
+func vpcConfigsFromFile(fileName string, inArgs *InArgs) (map[string]*vpcmodel.VPCConfig, error) {
+	rc, err1 := ibmvpc.ParseResourcesFromFile(fileName)
+	if err1 != nil {
+		return nil, fmt.Errorf("error parsing input vpc resources file: %w", err1)
 	}
 
-	var analysisOut *vpcmodel.SingleAnalysisOutput
-	outFormat := getOutputFormat(inArgs)
-	analysisOut, err = og.Generate(outFormat, outFile)
-	if err != nil {
-		return nil, fmt.Errorf(ErrorFormat, OutGenerationErr, err)
+	vpcConfigs, err2 := ibmvpc.VPCConfigsFromResources(rc, *inArgs.VPC, *inArgs.Debug)
+	if err2 != nil {
+		return nil, fmt.Errorf(ErrorFormat, InGenerationErr, err2)
 	}
-
-	return analysisOut, nil
+	return vpcConfigs, nil
 }
 
 // The actual main function
@@ -109,86 +98,34 @@ func _main(cmdlineArgs []string) error {
 		fmt.Printf("vpc-network-config-analyzer v%s\n", version.VersionCore)
 		return nil
 	}
-
-	rc, err1 := ibmvpc.ParseResourcesFromFile(*inArgs.InputConfigFile)
-	if err1 != nil {
-		return fmt.Errorf("error parsing input vpc resources file: %w", err1)
-	}
-
-	vpcConfigs, err2 := ibmvpc.VPCConfigsFromResources(rc, *inArgs.VPC, *inArgs.Debug)
+	vpcConfigs1, err2 := vpcConfigsFromFile(*inArgs.InputConfigFile, inArgs)
 	if err2 != nil {
-		return fmt.Errorf(ErrorFormat, InGenerationErr, err2)
+		return err2
 	}
-
-	outFile := ""
-	if inArgs.OutputFile != nil {
-		outFile = *inArgs.OutputFile
-	}
-
-	diffAnalysis := *inArgs.AnalysisType == allEndpointsDiff || *inArgs.AnalysisType == allSubnetsDiff
-	if !diffAnalysis {
-		outputPerVPC := make([]*vpcmodel.SingleAnalysisOutput, len(vpcConfigs))
-		i := 0
-		for _, vpcConfig := range vpcConfigs {
-			vpcAnalysisOutput, err2 := analysisPerVPCConfig(vpcConfig, inArgs, outFile)
-			if err2 != nil {
-				return err2
-			}
-			outputPerVPC[i] = vpcAnalysisOutput
-			i++
+	var vpcConfigs2 map[string]*vpcmodel.VPCConfig
+	if inArgs.InputSecondConfigFile != nil && *inArgs.InputSecondConfigFile != "" {
+		vpcConfigs2, err2 = vpcConfigsFromFile(*inArgs.InputSecondConfigFile, inArgs)
+		if err2 != nil {
+			return err2
 		}
-
-		var out string
-		out, err = vpcmodel.AggregateVPCsOutput(outputPerVPC, getOutputFormat(inArgs), analysisTypeToUseCase(inArgs), outFile)
-		if err != nil {
-			return err
+		// we are in diff mode, checking we have only one config per file:
+		if len(vpcConfigs1) != 1 || len(vpcConfigs2) != 1 {
+			return fmt.Errorf("for diff mode %v a single configuration should be provided "+
+				"for both -vpc-config and -vpc-config-second", *inArgs.AnalysisType)
 		}
-		fmt.Println(out)
-	} else {
-		return diffAnalysisMain(inArgs, vpcConfigs, outFile)
-	}
-	return nil
-}
-
-func diffAnalysisMain(inArgs *InArgs, vpcConfigs map[string]*vpcmodel.VPCConfig, outFile string) error {
-	// ToDo SM: for diff analysis assume 2 configs only, the 2nd given through vpc-config-second
-	rc2ndForDiff, err1 := ibmvpc.ParseResourcesFromFile(*inArgs.InputSecondConfigFile)
-	if err1 != nil {
-		return fmt.Errorf(ErrorFormat, ParsingErr, err1)
 	}
-	vpc2ndConfigs, err4 := ibmvpc.VPCConfigsFromResources(rc2ndForDiff, *inArgs.VPC, *inArgs.Debug)
-	if err4 != nil {
-		return fmt.Errorf(ErrorFormat, InGenerationErr, err4)
-	}
-	// For diff analysis each vpcConfigs have a single element
-	c1, single1 := getSingleCfg(vpcConfigs)
-	c2, single2 := getSingleCfg(vpc2ndConfigs)
-	if !single1 || !single2 {
-		return fmt.Errorf("for diff mode %v a single configuration should be provided "+
-			"for both -vpc-config and -vpc-config-second", *inArgs.AnalysisType)
-	}
-	analysisOutput, err4 := analysisDiffVPCConfig(c1, c2, inArgs, outFile)
-	if err4 != nil {
-		return err4
+	outFile := ""
+	if inArgs.OutputFile != nil {
+		outFile = *inArgs.OutputFile
 	}
-	out, err5 := vpcmodel.WriteDiffOutput(analysisOutput, getOutputFormat(inArgs), outFile)
-	if err5 != nil {
-		return err5
+	vpcAnalysisOutput, err2 := analysisVPCConfigs(vpcConfigs1, vpcConfigs2, inArgs, outFile)
+	if err2 != nil {
+		return err2
 	}
-	fmt.Println(out)
+	fmt.Println(vpcAnalysisOutput)
 	return nil
 }
 
-func getSingleCfg(vpcConfigs map[string]*vpcmodel.VPCConfig) (*vpcmodel.VPCConfig, bool) {
-	if len(vpcConfigs) > 1 {
-		return nil, false
-	}
-	for _, vpcConfig := range vpcConfigs {
-		return vpcConfig, true
-	}
-	return nil, false
-}
-
 func main() {
 	err := _main(os.Args[1:])
 	if err != nil {

cmd/analyzer/main_test.go

@@ -1,17 +1,25 @@
+//nolint:lll // styles are too long and can not be split
 package main
 
 import (
 	"strings"
 	"testing"
 )
 
+// //////////////////////////////////
+// this file need to be rewritten, no need to code review it
+// ///////////////////////////////////////
 func Test_main(t *testing.T) {
 	tests := []struct {
 		name string
 		args string
 	}{
-		{"drawio_multi_vpc", "-output-file aaa.drawio -vpc-config ../../pkg/ibmvpc/examples/input_multiple_vpcs.json -format drawio"},
-		{"txt_multi_vpc", "-output-file aaa.txt -vpc-config ../../pkg/ibmvpc/examples/input_multiple_vpcs.json -format txt"},
+		{"drawio_multi_vpc_all_subnets", "-output-file multi_vpc.drawio -vpc-config ../../pkg/ibmvpc/examples/input_multiple_vpcs.json -format drawio -analysis-type all_subnets"},
+		{"drawio_multi_vpc_all_subnets_grouped", "-output-file multi_vpc_grouped.drawio -vpc-config ../../pkg/ibmvpc/examples/input_multiple_vpcs.json -format drawio -analysis-type all_subnets -grouping"},
+		{"txt_multi_vpc", "-output-file multi_vpc.txt -vpc-config ../../pkg/ibmvpc/examples/input_multiple_vpcs.json -format txt -analysis-type all_subnets"},
+		// {"json_diff_acl_testing5", "-output-file acl_testing5_diff.json -vpc-config ../../pkg/ibmvpc/examples/input_acl_testing5.json -vpc-config-second ../../pkg/ibmvpc/examples/input_acl_testing5_2nd.json -format json -analysis-type diff_all_subnets"},
+		{"txt_diff_acl_testing5", "-output-file acl_testing5_diff.txt -vpc-config ../../pkg/ibmvpc/examples/input_acl_testing5.json -vpc-config-second ../../pkg/ibmvpc/examples/input_acl_testing5_2nd.json -format txt -analysis-type diff_all_subnets"},
+		{"txt_diff_acl_testing3", "-output-file acl_testing3_diff.txt -vpc-config ../../pkg/ibmvpc/examples/input_acl_testing3.json -vpc-config-second ../../pkg/ibmvpc/examples/input_acl_testing3_2nd.json -format txt -analysis-type diff_all_endpoints"},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

cmd/analyzer/parse_args.go

@@ -3,6 +3,7 @@ package main
 import (
 	"flag"
 	"fmt"
+	"slices"
 	"strings"
 )
 
@@ -75,7 +76,7 @@ var supportedOutputFormatsMap = map[string]bool{
 // supportedAnalysisTypesMap is a map from analysis type to its list of supported output formats
 var supportedAnalysisTypesMap = map[string][]string{
 	allEndpoints:     {TEXTFormat, MDFormat, JSONFormat, DRAWIOFormat, ARCHDRAWIOFormat, DEBUGFormat},
-	allSubnets:       {TEXTFormat, JSONFormat},
+	allSubnets:       {TEXTFormat, JSONFormat, DRAWIOFormat, ARCHDRAWIOFormat},
 	singleSubnet:     {TEXTFormat},
 	allEndpointsDiff: {TEXTFormat, MDFormat},
 	allSubnetsDiff:   {TEXTFormat, MDFormat},
@@ -189,14 +190,18 @@ func errorInErgs(args *InArgs, flagset *flag.FlagSet) error {
 	}
 	if _, ok := supportedAnalysisTypesMap[*args.AnalysisType]; !ok {
 		flagset.PrintDefaults()
-		return fmt.Errorf("wrong analysis type %s; must be one of: %s", *args.AnalysisType, strings.Join(supportedAnalysisTypesList, separator))
+		return fmt.Errorf("wrong analysis type '%s'; must be one of: '%s'",
+			*args.AnalysisType, strings.Join(supportedAnalysisTypesList, separator))
 	}
 	if !supportedOutputFormatsMap[*args.OutputFormat] {
 		flagset.PrintDefaults()
-		return fmt.Errorf("wrong output format %s; must be one of: %s", *args.OutputFormat, strings.Join(supportedOutputFormatsList, separator))
+		return fmt.Errorf("wrong output format '%s'; must be one of: '%s'",
+			*args.OutputFormat, strings.Join(supportedOutputFormatsList, separator))
 	}
-	if *args.OutputFormat == DEBUGFormat && *args.AnalysisType != allEndpoints {
-		return fmt.Errorf("output format %s supported on for %s", DEBUGFormat, allEndpoints)
+	if !slices.Contains(supportedAnalysisTypesMap[*args.AnalysisType], *args.OutputFormat) {
+		flagset.PrintDefaults()
+		return fmt.Errorf("wrong output format '%s' for analysis type '%s'; must be one of: %s",
+			*args.OutputFormat, *args.AnalysisType, strings.Join(supportedAnalysisTypesMap[*args.AnalysisType], separator))
 	}
 	diffAnalysis := *args.AnalysisType == allEndpointsDiff || *args.AnalysisType == allSubnetsDiff
 	fileForDiffSpecified := args.InputSecondConfigFile != nil && *args.InputSecondConfigFile != ""
@@ -212,13 +217,6 @@ func errorInErgs(args *InArgs, flagset *flag.FlagSet) error {
 
 func notSupportedYetArgs(args *InArgs) error {
 	diffAnalysis := *args.AnalysisType == allEndpointsDiff || *args.AnalysisType == allSubnetsDiff
-	if !diffAnalysis && *args.AnalysisType != allEndpoints && *args.OutputFormat != TEXTFormat &&
-		*args.OutputFormat != JSONFormat {
-		return fmt.Errorf("currently only txt/json output format supported with %s analysis type", *args.AnalysisType)
-	}
-	if diffAnalysis && *args.OutputFormat != TEXTFormat && *args.OutputFormat != MDFormat {
-		return fmt.Errorf("currently only txt/md output format supported with %s analysis type", *args.AnalysisType)
-	}
 	if (*args.AnalysisType == singleSubnet || diffAnalysis) && *args.Grouping {
 		return fmt.Errorf("currently %s analysis type does not support grouping", *args.AnalysisType)
 	}

pkg/common/genericSet.go

@@ -51,3 +51,18 @@ func (s GenericSet[T]) IsIntersect(s2 GenericSet[T]) bool {
 	}
 	return false
 }
+
+// /////////////////////////////////////////////////////////////////
+// AnyMapEntry() return an arbitrary (not random) entry of a map.
+// Needed for cases we do not care which entry.
+// it just returns the first entry returned by range
+// this func is not related to genericSet,
+// todo: consider moving to another file
+// todo: create AnyMapEntry() for GenericSet, and use it
+// /////////////////////////////////////////////////////////
+func AnyMapEntry[K comparable, V any](m map[K]V) (k K, v V) {
+	for k, v = range m {
+		break
+	}
+	return k, v
+}

pkg/ibmvpc/analysis_output_test.go

@@ -6,7 +6,6 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
-	"sort"
 	"strings"
 	"testing"
 
@@ -84,9 +83,6 @@ func getTestFileName(testName string,
 	switch uc {
 	case vpcmodel.AllEndpoints:
 		res = baseName
-		if grouping {
-			res += suffixOutFileWithGrouping
-		}
 	case vpcmodel.SingleSubnet:
 		res = baseName + suffixOutFileDebugSubnet
 	case vpcmodel.AllSubnets:
@@ -98,6 +94,9 @@ func getTestFileName(testName string,
 	case vpcmodel.EndpointsDiff:
 		res = baseName + suffixOutFileDiffEndpoints
 	}
+	if grouping {
+		res += suffixOutFileWithGrouping
+	}
 	suffix, suffixErr := getTestFileSuffix(format)
 	if suffixErr != nil {
 		return "", "", suffixErr
@@ -391,6 +390,29 @@ var tests = []*vpcGeneralTest{
 		useCases: []vpcmodel.OutputUseCase{vpcmodel.AllEndpoints, vpcmodel.AllSubnets},
 		format:   vpcmodel.Text,
 	},
+	// multivpc drawio:
+	{
+		name:     "multiple_vpcs",
+		useCases: []vpcmodel.OutputUseCase{vpcmodel.AllSubnets},
+		format:   vpcmodel.DRAWIO,
+	},
+	{
+		name:     "multiple_vpcs",
+		useCases: []vpcmodel.OutputUseCase{vpcmodel.AllSubnets},
+		grouping: true,
+		format:   vpcmodel.DRAWIO,
+	},
+	{
+		name:     "experiments_env",
+		useCases: []vpcmodel.OutputUseCase{vpcmodel.AllEndpoints},
+		format:   vpcmodel.ARCHDRAWIO,
+	},
+	{
+		name:     "experiments_env",
+		useCases: []vpcmodel.OutputUseCase{vpcmodel.AllEndpoints},
+		grouping: true,
+		format:   vpcmodel.DRAWIO,
+	},
 }
 
 var formatsAvoidComparison = map[vpcmodel.OutFormat]bool{vpcmodel.ARCHDRAWIO: true, vpcmodel.DRAWIO: true}
@@ -547,51 +569,20 @@ func runTestPerUseCase(t *testing.T,
 	c1, c2 map[string]*vpcmodel.VPCConfig,
 	uc vpcmodel.OutputUseCase,
 	mode testMode) error {
-	numConfigs := len(c1)
-	allVPCsOutput := make([]*vpcmodel.SingleAnalysisOutput, numConfigs)
-	i := 0
-	var vpcConfig2nd *vpcmodel.VPCConfig
-	// note that for diff analysis mode a single vpcConfig in c1 is provided; c2 is assumed to have a single cfg
-	for _, vpcConfig := range c2 {
-		vpcConfig2nd = vpcConfig
-	}
-	for _, vpcConfig := range c1 {
-		if err := initTestFileNames(tt, uc, vpcConfig.VPC.Name(), false); err != nil {
-			return err
-		}
-
-		og, err := vpcmodel.NewOutputGenerator(vpcConfig, vpcConfig2nd, tt.grouping, uc, tt.format == vpcmodel.ARCHDRAWIO)
-		if err != nil {
-			return err
-		}
-		vpcOutput, err := og.Generate(tt.format, tt.actualOutput[uc])
-		if err != nil {
-			return err
-		}
-		allVPCsOutput[i] = vpcOutput
-		i++
-		if err := compareOrRegenerateOutputPerTest(t, mode, vpcOutput.Output, tt, uc); err != nil {
-			return err
-		}
-	}
-	// compare also the aggregated output
 	if err := initTestFileNames(tt, uc, "", true); err != nil {
 		return err
 	}
-
-	// sort allVPCsOutput by vpc name
-	sort.Slice(allVPCsOutput, func(i, j int) bool {
-		return allVPCsOutput[i].VPC1Name < allVPCsOutput[j].VPC1Name
-	})
-
-	actualOutput, err := vpcmodel.AggregateVPCsOutput(allVPCsOutput, tt.format, uc, tt.actualOutput[uc])
+	og, err := vpcmodel.NewOutputGenerator(c1, c2, tt.grouping, uc, tt.format == vpcmodel.ARCHDRAWIO)
+	if err != nil {
+		return err
+	}
+	actualOutput, err := og.Generate(tt.format, tt.actualOutput[uc])
 	if err != nil {
 		return err
 	}
 	if err := compareOrRegenerateOutputPerTest(t, mode, actualOutput, tt, uc); err != nil {
 		return err
 	}
-
 	return nil
 }