nACL optimization - fix cmd

cmd/subcmds/optimize.go

@@ -5,17 +5,39 @@ 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{
 		Use:   "optimize",
-		Short: "Optimize is not supported yet",
-		Long:  `Optimize is not supported yet`,
+		Short: "optimization of existing SG and nACLs",
+		Long:  `optimization of existing SG and nACLs`,
 	}
 
-	cmd.AddCommand(NewOptimizeACLCommand(args))
+	// sub cmds
 	cmd.AddCommand(NewOptimizeSGCommand(args))
+	cmd.AddCommand(NewOptimizeACLCommand(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/optimizeACL.go

@@ -5,18 +5,27 @@ SPDX-License-Identifier: Apache-2.0
 
 package subcmds
 
-import "github.com/spf13/cobra"
+import (
+	"github.com/spf13/cobra"
+
+	acloptimizer "github.com/np-guard/vpc-network-config-synthesis/pkg/optimize/acl"
+)
+
+const aclNameFlag = "acl-name"
 
 func NewOptimizeACLCommand(args *inArgs) *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "acl",
-		Short: "OptimizeACL is not supported yet",
-		Long:  `OptimizeACL is not supported yet`,
+		Short: "OptimizeACL attempts to reduce the number of nACL rules in an nACL without changing the semantic.",
+		Long:  `OptimizeACL attempts to reduce the number of nACL rules in an nACL without changing the semantic.`,
 		Args:  cobra.NoArgs,
 		RunE: func(cmd *cobra.Command, _ []string) error {
-			return nil
+			return optimization(cmd, args, acloptimizer.NewACLOptimizer, false)
 		},
 	}
 
+	// flags
+	cmd.PersistentFlags().StringVarP(&args.firewallName, aclNameFlag, "n", "", "which nacl to optimize")
+
 	return cmd
 }

cmd/subcmds/optimizeSG.go

@@ -5,18 +5,27 @@ SPDX-License-Identifier: Apache-2.0
 
 package subcmds
 
-import "github.com/spf13/cobra"
+import (
+	"github.com/spf13/cobra"
+
+	sgoptimizer "github.com/np-guard/vpc-network-config-synthesis/pkg/optimize/sg"
+)
+
+const sgNameFlag = "sg-name"
 
 func NewOptimizeSGCommand(args *inArgs) *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "sg",
-		Short: "OptimizeSG is not supported yet",
-		Long:  `OptimizeSG is not supported yet`,
+		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 nil
+			return optimization(cmd, args, sgoptimizer.NewSGOptimizer, true)
 		},
 	}
 
+	// flags
+	cmd.PersistentFlags().StringVarP(&args.firewallName, sgNameFlag, "n", "", "which security group to optimize")
+
 	return cmd
 }

cmd/subcmds/output.go

@@ -12,37 +12,30 @@ import (
 	"os"
 	"path/filepath"
 
+	"github.com/np-guard/vpc-network-config-synthesis/pkg/io"
 	"github.com/np-guard/vpc-network-config-synthesis/pkg/io/confio"
-	"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 +48,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,53 +58,50 @@ 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)
 	if err != nil {
 		return nil, err
 	}
-	if err := collection.Write(writer, vpc); err != nil {
+	if err := collection.Write(writer, vpc, isSynth); err != nil {
 		return nil, err
 	}
 	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) {
 	w := bufio.NewWriter(data)
 	switch args.outputFmt {
 	case tfOutputFormat:
 		return tfio.NewWriter(w), nil
 	case csvOutputFormat:
-		return csvio.NewWriter(w), nil
+		return io.NewCSVWriter(w), nil
 	case mdOutputFormat:
-		return mdio.NewWriter(w), nil
-	case apiOutputFormat:
+		return io.NewMDWriter(w), nil
+	case jsonOutputFormat:
 		return confio.NewWriter(w, args.configFile)
-	default:
-		return nil, fmt.Errorf("bad output format: %q", args.outputFmt)
 	}
+	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,7 +14,6 @@ import (
 
 const (
 	configFlag     = "config"
-	specFlag       = "spec"
 	outputFmtFlag  = "format"
 	outputFileFlag = "output-file"
 	outputDirFlag  = "output-dir"
@@ -24,44 +23,53 @@ const (
 )
 
 type inArgs struct {
-	configFile string
-	specFile   string
-	outputFmt  string
-	outputFile string
-	outputDir  string
-	prefix     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))
-	// Todo: add optimize command
+	rootCmd.AddCommand(NewOptimizeCommand(args))
+
+	// prevent Cobra from creating a default 'completion' command
+	rootCmd.CompletionOptions.DisableDefaultCmd = true
 
-	rootCmd.CompletionOptions.HiddenDefaultCmd = true
-	rootCmd.SetHelpCommand(&cobra.Command{Hidden: true}) // disable help command. should use --help flag instead
+	// 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)
+}