Optimize output and flags (#193)

cmd/subcmds/optimize.go

@@ -5,7 +5,14 @@ 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/ir"
+	"github.com/np-guard/vpc-network-config-synthesis/pkg/optimize"
+)
 
 func NewOptimizeCommand(args *inArgs) *cobra.Command {
 	cmd := &cobra.Command{
@@ -14,7 +21,22 @@ func NewOptimizeCommand(args *inArgs) *cobra.Command {
 		Long:  `optimization of existing SG (nACLS are not supported yet)`,
 	}
 
+	// sub cmds
 	cmd.AddCommand(NewOptimizeSGCommand(args))
 
 	return cmd
 }
+
+func optimization(cmd *cobra.Command, args *inArgs, newOptimizer func(ir.Collection, string) optimize.Optimizer, isSG bool) error {
+	cmd.SilenceUsage = true // if we got this far, flags are syntactically correct, so no need to print usage
+	collection, err := parseCollection(args, isSG)
+	if err != nil {
+		return fmt.Errorf("could not parse config file %v: %w", args.configFile, err)
+	}
+	optimizer := newOptimizer(collection, args.firewallName)
+	optimizedCollection, err := optimizer.Optimize()
+	if err != nil {
+		return err
+	}
+	return writeOutput(args, optimizedCollection, collection.VpcNames(), false)
+}

cmd/subcmds/optimizeSG.go

@@ -6,39 +6,26 @@ 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"
 )
 
+const sgNameFlag = "sg-name"
+
 func NewOptimizeSGCommand(args *inArgs) *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "sg",
 		Short: "OptimizeSG attempts to reduce the number of security group rules in a SG without changing the semantic.",
 		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)
+			return optimization(cmd, args, optimize.NewSGOptimizer, true)
 		},
 	}
 
-	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
+	// flags
+	cmd.PersistentFlags().StringVarP(&args.firewallName, sgNameFlag, "n", "", "which security group to optimize")
 
 	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 {
+
+	if err := writeLocals(args, vpcNames, collection); 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,24 +80,31 @@ 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)
+}
 
-	_, isACLCollection := collection.(*ir.ACLCollection)
+func writeLocals(args *inArgs, vpcNames []ir.ID, collection ir.Collection) error {
+	if !args.locals {
+		return nil
+	}
 	var data *bytes.Buffer
 	var err error
+
+	_, isACLCollection := collection.(*ir.ACLCollection)
+
 	if data, err = tfio.WriteLocals(vpcNames, isACLCollection); 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

@@ -14,56 +14,62 @@ import (
 
 const (
 	configFlag     = "config"
-	specFlag       = "spec"
 	outputFmtFlag  = "format"
 	outputFileFlag = "output-file"
 	outputDirFlag  = "output-dir"
 	prefixFlag     = "prefix"
-	sgNameFlag     = "sg-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 {
 	args := &inArgs{}
 
+	// allow PersistentPreRunE
+	cobra.EnableTraverseRunHooks = true
+
 	rootCmd := &cobra.Command{
 		Use:   "vpcgen",
-		Short: "Tool for automatic synthesis of VPC network configurations",
-		Long:  `Tool for automatic synthesis of VPC network configurations, namely Network ACLs and Security Groups.`,
+		Short: "A tool for synthesizing and optimizing VPC network configurations",
+		Long:  `A tool for synthesizing and optimizing VPC network configurations, namely Network ACLs and Security Groups.`,
+		PersistentPreRunE: func(cmd *cobra.Command, _ []string) error {
+			return validateFlags(args)
+		},
 	}
 
+	// 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))
 
-	rootCmd.CompletionOptions.HiddenDefaultCmd = true
-	rootCmd.SetHelpCommand(&cobra.Command{Hidden: true}) // disable help command. should use --help flag instead
+	// prevent Cobra from creating a default 'completion' command
+	rootCmd.CompletionOptions.DisableDefaultCmd = true
+
+	// disable help command. should use --help flag instead
+	rootCmd.SetHelpCommand(&cobra.Command{Hidden: true})
 
 	return rootCmd
 }

cmd/subcmds/synth.go

@@ -13,6 +13,8 @@ import (
 	"github.com/np-guard/vpc-network-config-synthesis/pkg/utils"
 )
 
+const specFlag = "spec"
+
 func NewSynthCommand(args *inArgs) *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "synth",
@@ -21,9 +23,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 +50,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/unmarshal.go

@@ -26,3 +26,10 @@ func unmarshal(args *inArgs) (*ir.Spec, error) {
 
 	return model, nil
 }
+
+func parseCollection(args *inArgs, isSG bool) (ir.Collection, error) {
+	if isSG {
+		return confio.ReadSGs(args.configFile)
+	}
+	return confio.ReadACLs(args.configFile)
+}

cmd/subcmds/validateFlags.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("specifying both -d and -o is not allowed")
+	}
+	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
+}