Optimize output and flags

cmd/subcmds/optimize.go

@@ -5,7 +5,13 @@ SPDX-License-Identifier: Apache-2.0
 
 package subcmds
 
-import "github.com/spf13/cobra"
+import (
+	"fmt"
+
+	"github.com/spf13/cobra"
+
+	"github.com/np-guard/vpc-network-config-synthesis/pkg/optimize"
+)
 
 func NewOptimizeCommand(args *inArgs) *cobra.Command {
 	cmd := &cobra.Command{
@@ -14,7 +20,23 @@ func NewOptimizeCommand(args *inArgs) *cobra.Command {
 		Long:  `optimization of existing SG (nACLS are not supported yet)`,
 	}
 
+	// flags
+	cmd.PersistentFlags().StringVarP(&args.firewallName, firewallNameFlag, "n", "", "which vpcFirewall to optimize")
+
+	// sub cmds
 	cmd.AddCommand(NewOptimizeSGCommand(args))
 
 	return cmd
 }
+
+func optimization(cmd *cobra.Command, args *inArgs, newOptimizer optimize.Optimizer) error {
+	cmd.SilenceUsage = true // if we got this far, flags are syntactically correct, so no need to print usage
+	if err := newOptimizer.ParseCollection(args.configFile); err != nil {
+		return fmt.Errorf("could not parse config file %v: %w", args.configFile, err)
+	}
+	collection, err := newOptimizer.Optimize()
+	if err != nil {
+		return err
+	}
+	return writeOutput(args, collection, newOptimizer.VpcNames(), false)
+}

cmd/subcmds/optimizeSG.go

@@ -6,11 +6,8 @@ SPDX-License-Identifier: Apache-2.0
 package subcmds
 
 import (
-	"fmt"
-
 	"github.com/spf13/cobra"
 
-	"github.com/np-guard/vpc-network-config-synthesis/pkg/io/confio"
 	"github.com/np-guard/vpc-network-config-synthesis/pkg/optimize"
 )
 
@@ -21,24 +18,11 @@ func NewOptimizeSGCommand(args *inArgs) *cobra.Command {
 		Long:  `OptimizeSG attempts to reduce the number of security group rules in a SG without changing the semantic.`,
 		Args:  cobra.NoArgs,
 		RunE: func(cmd *cobra.Command, _ []string) error {
-			return optimization(cmd, args)
+			if err := validateFlags(args); err != nil {
+				return err
+			}
+			return optimization(cmd, args, optimize.NewSGOptimizer(args.firewallName))
 		},
 	}
-
-	cmd.Flags().StringVarP(&args.sgName, sgNameFlag, "s", "", "which SG to optimize")
-	_ = cmd.MarkFlagRequired(sgNameFlag) // Todo: delete this line. if sgName flag is not supplied - optimize all SGs
-
 	return cmd
 }
-
-func optimization(cmd *cobra.Command, args *inArgs) error {
-	cmd.SilenceUsage = true // if we got this far, flags are syntactically correct, so no need to print usage
-	sgs, err := confio.ReadSGs(args.configFile)
-	if err != nil {
-		return fmt.Errorf("could not parse config file %v: %w", args.configFile, err)
-	}
-	if optimize.ReduceSGRules(sgs, args.sgName) != nil {
-		return err
-	}
-	return writeOptimizeOutput(args, sgs)
-}

cmd/subcmds/output.go

@@ -16,33 +16,27 @@ import (
 	"github.com/np-guard/vpc-network-config-synthesis/pkg/io/csvio"
 	"github.com/np-guard/vpc-network-config-synthesis/pkg/io/mdio"
 	"github.com/np-guard/vpc-network-config-synthesis/pkg/io/tfio"
-
 	"github.com/np-guard/vpc-network-config-synthesis/pkg/ir"
 )
 
 const defaultFilePermission = 0o644
 const defaultDirectoryPermission = 0o755
 
-func writeOutput(args *inArgs, collection ir.Collection, vpcNames []ir.ID) error {
-	if err := updateOutputFormat(args); err != nil {
-		return err
-	}
-	if args.outputDir != "" && args.outputFmt == apiOutputFormat {
-		return fmt.Errorf("-d cannot be used with format json")
-	}
+func writeOutput(args *inArgs, collection ir.Collection, vpcNames []string, isSynth bool) error {
 	if args.outputDir != "" { // create the directory if needed
 		if err := os.MkdirAll(args.outputDir, defaultDirectoryPermission); err != nil {
 			return err
 		}
 	}
-	if err := writeLocals(args, collection, vpcNames); err != nil {
+	_, isACLCollection := collection.(*ir.ACLCollection)
+	if err := writeLocals(args, vpcNames, isACLCollection); err != nil {
 		return err
 	}
 
 	var data *bytes.Buffer
 	var err error
 	if args.outputDir == "" {
-		if data, err = writeCollection(args, collection, ""); err != nil {
+		if data, err = writeCollection(args, collection, "", isSynth); err != nil {
 			return err
 		}
 		return writeToFile(args.outputFile, data)
@@ -55,7 +49,7 @@ func writeOutput(args *inArgs, collection ir.Collection, vpcNames []ir.ID) error
 		if args.prefix != "" {
 			args.outputFile = args.outputDir + "/" + args.prefix + "_" + suffix
 		}
-		if data, err = writeCollection(args, collection, vpc); err != nil {
+		if data, err = writeCollection(args, collection, vpc, isSynth); err != nil {
 			return err
 		}
 		if err := writeToFile(args.outputFile, data); err != nil {
@@ -65,9 +59,9 @@ func writeOutput(args *inArgs, collection ir.Collection, vpcNames []ir.ID) error
 	return nil
 }
 
-func writeCollection(args *inArgs, collection ir.Collection, vpc string) (*bytes.Buffer, error) {
+func writeCollection(args *inArgs, collection ir.Collection, vpc string, isSynth bool) (*bytes.Buffer, error) {
 	var data bytes.Buffer
-	writer, err := pickWriter(args, &data)
+	writer, err := pickWriter(args, &data, isSynth)
 	if err != nil {
 		return nil, err
 	}
@@ -77,15 +71,7 @@ func writeCollection(args *inArgs, collection ir.Collection, vpc string) (*bytes
 	return &data, nil
 }
 
-func writeToFile(outputFile string, data *bytes.Buffer) error {
-	if outputFile == "" {
-		fmt.Println(data.String())
-		return nil
-	}
-	return os.WriteFile(outputFile, data.Bytes(), defaultFilePermission)
-}
-
-func pickWriter(args *inArgs, data *bytes.Buffer) (ir.Writer, error) {
+func pickWriter(args *inArgs, data *bytes.Buffer, isSynth bool) (ir.Writer, error) {
 	w := bufio.NewWriter(data)
 	switch args.outputFmt {
 	case tfOutputFormat:
@@ -94,25 +80,30 @@ func pickWriter(args *inArgs, data *bytes.Buffer) (ir.Writer, error) {
 		return csvio.NewWriter(w), nil
 	case mdOutputFormat:
 		return mdio.NewWriter(w), nil
-	case apiOutputFormat:
-		return confio.NewWriter(w, args.configFile)
-	default:
-		return nil, fmt.Errorf("bad output format: %q", args.outputFmt)
+	case jsonOutputFormat:
+		if isSynth {
+			return confio.NewWriter(w, args.configFile)
+		}
 	}
+	return nil, fmt.Errorf("bad output format: %q", args.outputFmt)
 }
 
-func writeLocals(args *inArgs, collection ir.Collection, vpcNames []ir.ID) error {
-	if !args.locals {
+func writeToFile(outputFile string, data *bytes.Buffer) error {
+	if outputFile == "" {
+		fmt.Println(data.String())
 		return nil
 	}
-	if args.outputFmt != tfOutputFormat {
-		return fmt.Errorf("--locals flag requires setting the output format to tf")
+	return os.WriteFile(outputFile, data.Bytes(), defaultFilePermission)
+}
+
+func writeLocals(args *inArgs, vpcNames []ir.ID, isACL bool) error {
+	if !args.locals {
+		return nil
 	}
 
-	_, isACLCollection := collection.(*ir.ACLCollection)
 	var data *bytes.Buffer
 	var err error
-	if data, err = tfio.WriteLocals(vpcNames, isACLCollection); err != nil {
+	if data, err = tfio.WriteLocals(vpcNames, isACL); err != nil {
 		return err
 	}
 

cmd/subcmds/outputFormat.go

@@ -14,11 +14,11 @@ const (
 	tfOutputFormat      = "tf"
 	csvOutputFormat     = "csv"
 	mdOutputFormat      = "md"
-	apiOutputFormat     = "json"
+	jsonOutputFormat    = "json"
 	defaultOutputFormat = csvOutputFormat
 )
 
-var outputFormats = []string{tfOutputFormat, csvOutputFormat, mdOutputFormat, apiOutputFormat}
+var outputFormats = []string{tfOutputFormat, csvOutputFormat, mdOutputFormat, jsonOutputFormat}
 
 func updateOutputFormat(args *inArgs) error {
 	var err error
@@ -40,7 +40,7 @@ func inferFormatUsingFilename(filename string) (string, error) {
 	case strings.HasSuffix(filename, ".md"):
 		return mdOutputFormat, nil
 	case strings.HasSuffix(filename, ".json"):
-		return apiOutputFormat, nil
+		return jsonOutputFormat, nil
 	default:
 		return "", fmt.Errorf("bad output format")
 	}

cmd/subcmds/root.go

@@ -13,27 +13,27 @@ import (
 )
 
 const (
-	configFlag     = "config"
-	specFlag       = "spec"
-	outputFmtFlag  = "format"
-	outputFileFlag = "output-file"
-	outputDirFlag  = "output-dir"
-	prefixFlag     = "prefix"
-	sgNameFlag     = "sg-name"
-	singleACLFlag  = "single"
-	localsFlag     = "locals"
+	configFlag       = "config"
+	specFlag         = "spec"
+	outputFmtFlag    = "format"
+	outputFileFlag   = "output-file"
+	outputDirFlag    = "output-dir"
+	prefixFlag       = "prefix"
+	firewallNameFlag = "firewall-name"
+	singleACLFlag    = "single"
+	localsFlag       = "locals"
 )
 
 type inArgs struct {
-	configFile string
-	specFile   string
-	outputFmt  string
-	outputFile string
-	outputDir  string
-	prefix     string
-	sgName     string
-	singleacl  bool
-	locals     bool
+	configFile   string
+	specFile     string
+	outputFmt    string
+	outputFile   string
+	outputDir    string
+	prefix       string
+	firewallName string
+	singleacl    bool
+	locals       bool
 }
 
 func NewRootCommand() *cobra.Command {
@@ -45,25 +45,25 @@ func NewRootCommand() *cobra.Command {
 		Long:  `Tool for automatic synthesis of VPC network configurations, namely Network ACLs and Security Groups.`,
 	}
 
+	// flags
 	rootCmd.PersistentFlags().StringVarP(&args.configFile, configFlag, "c", "",
 		"JSON file containing a configuration object of existing resources")
 	rootCmd.PersistentFlags().StringVarP(&args.outputFmt, outputFmtFlag, "f", "", "Output format; "+mustBeOneOf(outputFormats))
 	rootCmd.PersistentFlags().StringVarP(&args.outputFile, outputFileFlag, "o", "", "Write all generated resources to the specified file.")
-	rootCmd.PersistentFlags().StringVarP(&args.outputDir, outputDirFlag, "d", "",
-		"Write generated resources to files in the specified directory, one file per VPC.")
-	rootCmd.PersistentFlags().StringVarP(&args.prefix, prefixFlag, "p", "", "The prefix of the files that will be created.")
 	rootCmd.PersistentFlags().BoolVarP(&args.locals, localsFlag, "l", false,
 		"whether to generate a locals.tf file (only possible when the output format is tf)")
-	rootCmd.PersistentFlags().SortFlags = false
 
+	// flags set for all commands
+	rootCmd.PersistentFlags().SortFlags = false
 	_ = rootCmd.MarkPersistentFlagRequired(configFlag)
-	rootCmd.MarkFlagsMutuallyExclusive(outputFileFlag, outputDirFlag)
 
+	// sub cmds
 	rootCmd.AddCommand(NewSynthCommand(args))
 	rootCmd.AddCommand(NewOptimizeCommand(args))
 
+	// disable help command. should use --help flag instead
 	rootCmd.CompletionOptions.HiddenDefaultCmd = true
-	rootCmd.SetHelpCommand(&cobra.Command{Hidden: true}) // disable help command. should use --help flag instead
+	rootCmd.SetHelpCommand(&cobra.Command{Hidden: true})
 
 	return rootCmd
 }

cmd/subcmds/synth.go

@@ -21,9 +21,16 @@ func NewSynthCommand(args *inArgs) *cobra.Command {
 		--config and --spec parameters must be supplied.`,
 	}
 
+	// flags
 	cmd.PersistentFlags().StringVarP(&args.specFile, specFlag, "s", "", "JSON file containing spec file")
+	cmd.PersistentFlags().StringVarP(&args.outputDir, outputDirFlag, "d", "",
+		"Write generated resources to files in the specified directory, one file per VPC.")
+	cmd.PersistentFlags().StringVarP(&args.prefix, prefixFlag, "p", "", "The prefix of the files that will be created.")
+
+	// flags settings
 	_ = cmd.MarkPersistentFlagRequired(specFlag)
 
+	// sub cmds
 	cmd.AddCommand(NewSynthACLCommand(args))
 	cmd.AddCommand(NewSynthSGCommand(args))
 
@@ -41,5 +48,5 @@ func synthesis(cmd *cobra.Command, args *inArgs, newSynthesizer func(*ir.Spec, b
 	if err != nil {
 		return err
 	}
-	return writeOutput(args, collection, utils.MapKeys(spec.Defs.ConfigDefs.VPCs))
+	return writeOutput(args, collection, utils.MapKeys(spec.Defs.ConfigDefs.VPCs), true)
 }

cmd/subcmds/synthACL.go

@@ -19,6 +19,9 @@ func NewSynthACLCommand(args *inArgs) *cobra.Command {
 		Endpoints in the required-connectivity specification may be subnets, subnet segments, CIDR segments and externals.`,
 		Args: cobra.NoArgs,
 		RunE: func(cmd *cobra.Command, _ []string) error {
+			if err := validateFlags(args); err != nil {
+				return err
+			}
 			return synthesis(cmd, args, synth.NewACLSynthesizer, args.singleacl)
 		},
 	}

cmd/subcmds/synthSG.go

@@ -19,6 +19,9 @@ func NewSynthSGCommand(args *inArgs) *cobra.Command {
 		Endpoints in the required-connectivity specification may be Instances (VSIs), Network Interfaces, VPEs and externals.`,
 		Args: cobra.NoArgs,
 		RunE: func(cmd *cobra.Command, _ []string) error {
+			if err := validateFlags(args); err != nil {
+				return err
+			}
 			return synthesis(cmd, args, synth.NewSGSynthesizer, false)
 		},
 	}

cmd/subcmds/verifyFlags.go

@@ -0,0 +1,24 @@
+/*
+Copyright 2023- IBM Inc. All Rights Reserved.
+SPDX-License-Identifier: Apache-2.0
+*/
+
+package subcmds
+
+import "fmt"
+
+func validateFlags(args *inArgs) error {
+	if args.outputDir != "" && args.outputFile != "" {
+		return fmt.Errorf("only one of -d and -o can be supplied")
+	}
+	if err := updateOutputFormat(args); err != nil {
+		return err
+	}
+	if args.outputDir != "" && args.outputFmt == jsonOutputFormat {
+		return fmt.Errorf("-d cannot be used with format json")
+	}
+	if args.locals && args.outputFmt != tfOutputFormat {
+		return fmt.Errorf("--locals flag requires setting the output format to tf")
+	}
+	return nil
+}

pkg/io/confio/parse_sgs.go

@@ -39,7 +39,7 @@ func ReadSGs(filename string) (*ir.SGCollection, error) {
 		if result.SGs[vpcName] == nil {
 			result.SGs[vpcName] = make(map[ir.SGName]*ir.SG)
 		}
-		result.SGs[vpcName][ir.SGName(*sg.Name)] = &ir.SG{InboundRules: inbound, OutboundRules: outbound}
+		result.SGs[vpcName][ir.SGName(*sg.Name)] = &ir.SG{VpcName: vpcName, InboundRules: inbound, OutboundRules: outbound}
 	}
 	return result, nil
 }