From 7e70c95d8dfa34965b0bc820097faf1736d01cb5 Mon Sep 17 00:00:00 2001 From: Yair Slobodin Date: Mon, 26 Aug 2024 12:40:46 +0300 Subject: [PATCH 01/24] fixed --- pkg/io/confio/parse_sgs.go | 161 ++++++++++++++++++++++++++++++++++++- pkg/ir/sg.go | 2 + pkg/utils/utils.go | 16 ++++ 3 files changed, 177 insertions(+), 2 deletions(-) diff --git a/pkg/io/confio/parse_sgs.go b/pkg/io/confio/parse_sgs.go index fa6f7685..96d5038d 100644 --- a/pkg/io/confio/parse_sgs.go +++ b/pkg/io/confio/parse_sgs.go @@ -5,8 +5,165 @@ SPDX-License-Identifier: Apache-2.0 package confio -import "github.com/np-guard/vpc-network-config-synthesis/pkg/ir" +import ( + "fmt" + "reflect" + vpc1 "github.com/IBM/vpc-go-sdk/vpcv1" + + "github.com/np-guard/models/pkg/netp" + "github.com/np-guard/models/pkg/netset" + + "github.com/np-guard/vpc-network-config-synthesis/pkg/ir" + "github.com/np-guard/vpc-network-config-synthesis/pkg/utils" +) + +// ReadSG translates SGs from a config_object file to map[ir.SGName]*SG func ReadSGs(filename string) (map[ir.SGName]*ir.SG, error) { - return map[ir.SGName]*ir.SG{}, nil + config, err := readModel(filename) + if err != nil { + return nil, err + } + result := make(map[ir.SGName]*ir.SG, len(config.SecurityGroupList)) + + for _, sg := range config.SecurityGroupList { + inbound, outbound, err := translateSGRules(&sg.SecurityGroup) + if err != nil { + return nil, err + } + if sg.Name != nil { + result[ir.SGName(*sg.Name)] = &ir.SG{InboundRules: inbound, OutboundRules: outbound} + } + } + return result, nil +} + +// parse security rules, splitted into ingress and egress rules +func translateSGRules(sg *vpc1.SecurityGroup) (ingressRules, egressRules []ir.SGRule, err error) { + ingressRules = []ir.SGRule{} + egressRules = []ir.SGRule{} + for index := range sg.Rules { + rule, err := translateSGRule(sg, index) + if err != nil { + return nil, nil, err + } + if rule.Direction == ir.Inbound { + ingressRules = append(ingressRules, rule) + } else { + egressRules = append(egressRules, rule) + } + } + return ingressRules, egressRules, nil +} + +// translateSGRule translates a security group rule to ir.SGRule +func translateSGRule(sg *vpc1.SecurityGroup, index int) (sgRule ir.SGRule, err error) { + fmt.Printf("Type of sgRule[%d]: %s \n", index, reflect.TypeOf(sg.Rules[index])) + switch r := sg.Rules[index].(type) { + case *vpc1.SecurityGroupRuleSecurityGroupRuleProtocolAll: + return translateSGRuleProtocolAll(r) + case *vpc1.SecurityGroupRuleSecurityGroupRuleProtocolTcpudp: + return translateSGRuleProtocolTCPUDP(r) + case *vpc1.SecurityGroupRuleSecurityGroupRuleProtocolIcmp: + return translateSGRuleProtocolIcmp(r) + } + return ir.SGRule{}, fmt.Errorf("error parsing rule number %d in %s sg", index, *sg.Name) +} + +func translateSGRuleProtocolAll(rule *vpc1.SecurityGroupRuleSecurityGroupRuleProtocolAll) (sgRule ir.SGRule, err error) { + direction, err := translateDirection(*rule.Direction) + if err != nil { + return ir.SGRule{}, err + } + remote, err := translateRemote(rule.Remote) + if err != nil { + return ir.SGRule{}, err + } + local, err := translateLocal(rule.Local) + if err != nil { + return ir.SGRule{}, err + } + return ir.SGRule{Direction: direction, Remote: remote, Protocol: netp.AnyProtocol{}, Local: local}, nil +} + +func translateSGRuleProtocolTCPUDP(rule *vpc1.SecurityGroupRuleSecurityGroupRuleProtocolTcpudp) (sgRule ir.SGRule, err error) { + direction, err := translateDirection(*rule.Direction) + if err != nil { + return ir.SGRule{}, err + } + remote, err := translateRemote(rule.Remote) + if err != nil { + return ir.SGRule{}, err + } + local, err := translateLocal(rule.Local) + if err != nil { + return ir.SGRule{}, err + } + protocol, err := translateProtocolTCPUDP(rule) + if err != nil { + return ir.SGRule{}, err + } + return ir.SGRule{Direction: direction, Remote: remote, Protocol: protocol, Local: local}, nil +} + +func translateSGRuleProtocolIcmp(rule *vpc1.SecurityGroupRuleSecurityGroupRuleProtocolIcmp) (sgRule ir.SGRule, err error) { + direction, err := translateDirection(*rule.Direction) + if err != nil { + return ir.SGRule{}, err + } + remote, err := translateRemote(rule.Remote) + if err != nil { + return ir.SGRule{}, err + } + local, err := translateLocal(rule.Local) + if err != nil { + return ir.SGRule{}, err + } + protocol, err := netp.ICMPFromTypeAndCode64(rule.Type, rule.Code) + if err != nil { + return ir.SGRule{}, err + } + return ir.SGRule{Direction: direction, Remote: remote, Protocol: protocol, Local: local}, nil +} + +func translateDirection(direction string) (ir.Direction, error) { + if direction == "inbound" { + return ir.Inbound, nil + } else if direction == "outbound" { + return ir.Outbound, nil + } + return ir.Inbound, fmt.Errorf("a SG rule direction must be either inbound or outbound") +} + +func translateRemote(remote vpc1.SecurityGroupRuleRemoteIntf) (ir.RemoteType, error) { + if r, ok := remote.(*vpc1.SecurityGroupRuleRemote); ok { + switch { + case r.CIDRBlock != nil: + return netset.IPBlockFromCidr(*r.CIDRBlock) + case r.Address != nil: + return netset.IPBlockFromIPAddress(*r.Address) + case r.Name != nil: + return ir.SGName(*r.Name), nil + } + } + return nil, fmt.Errorf("unexpected SG rule remote") +} + +func translateLocal(local vpc1.SecurityGroupRuleLocalIntf) (*netset.IPBlock, error) { + if l, ok := local.(*vpc1.SecurityGroupRuleLocal); ok { + if l.CIDRBlock != nil { + return netset.IPBlockFromCidr(*l.CIDRBlock) + } + if l.Address != nil { + return netset.IPBlockFromIPAddress(*l.CIDRBlock) + } + } + return nil, fmt.Errorf("error parsing Local field") +} + +func translateProtocolTCPUDP(rule *vpc1.SecurityGroupRuleSecurityGroupRuleProtocolTcpudp) (netp.Protocol, error) { + isTCP := *rule.Protocol == string(netp.ProtocolStringTCP) + minDstPort := utils.GetProperty(rule.PortMin, netp.MinPort) + maxDstPort := utils.GetProperty(rule.PortMax, netp.MaxPort) + return netp.NewTCPUDP(isTCP, netp.MinPort, netp.MaxPort, int(minDstPort), int(maxDstPort)) } diff --git a/pkg/ir/sg.go b/pkg/ir/sg.go index 7c42fe01..5cf895ec 100644 --- a/pkg/ir/sg.go +++ b/pkg/ir/sg.go @@ -10,6 +10,7 @@ import ( "reflect" "github.com/np-guard/models/pkg/netp" + "github.com/np-guard/models/pkg/netset" "github.com/np-guard/vpc-network-config-synthesis/pkg/utils" ) @@ -40,6 +41,7 @@ type SGRule struct { Direction Direction Remote RemoteType Protocol netp.Protocol + Local *netset.IPBlock Explanation string } diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index acf9e3d2..ede1bb67 100644 --- a/pkg/utils/utils.go +++ b/pkg/utils/utils.go @@ -31,3 +31,19 @@ func SortedAllInnerMapsKeys[T, K cmp.Ordered, V any](m map[K]map[T]V) []T { slices.Sort(keys) return keys } + +// GetProperty returns pointer p if it is valid, else it returns the provided default value +// used to get min/max port or icmp type +func GetProperty(p *int64, defaultP int64) int64 { + if p == nil { + return defaultP + } + return *p +} + +func Int64PointerToIntPointer(v *int64) *int { + if v == nil { + return nil + } + return Ptr(int(*v)) +} From 19123f98c2a850278ee832c61f548c5aa81d54c3 Mon Sep 17 00:00:00 2001 From: Yair Slobodin Date: Mon, 26 Aug 2024 12:44:49 +0300 Subject: [PATCH 02/24] minor change --- pkg/io/confio/parse_sgs.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/io/confio/parse_sgs.go b/pkg/io/confio/parse_sgs.go index 96d5038d..daf044cc 100644 --- a/pkg/io/confio/parse_sgs.go +++ b/pkg/io/confio/parse_sgs.go @@ -24,8 +24,8 @@ func ReadSGs(filename string) (map[ir.SGName]*ir.SG, error) { if err != nil { return nil, err } - result := make(map[ir.SGName]*ir.SG, len(config.SecurityGroupList)) + result := make(map[ir.SGName]*ir.SG, len(config.SecurityGroupList)) for _, sg := range config.SecurityGroupList { inbound, outbound, err := translateSGRules(&sg.SecurityGroup) if err != nil { From 2b7f244db852b6c29894752d91b4cd3e1cf5fc4c Mon Sep 17 00:00:00 2001 From: Yair Slobodin Date: Mon, 26 Aug 2024 14:29:32 +0300 Subject: [PATCH 03/24] added synth prefix --- cmd/subcmds/{output.go => synthOutput.go} | 2 +- pkg/io/confio/acl.go | 2 +- pkg/io/confio/sg.go | 2 +- pkg/io/csvio/acl.go | 2 +- pkg/io/csvio/common.go | 2 +- pkg/io/csvio/sg.go | 2 +- pkg/io/mdio/acl.go | 2 +- pkg/io/mdio/common.go | 2 +- pkg/io/mdio/sg.go | 2 +- pkg/io/tfio/acl.go | 2 +- pkg/io/tfio/common.go | 2 +- pkg/io/tfio/sg.go | 2 +- pkg/ir/sg.go | 56 ++++++++++++----------- 13 files changed, 41 insertions(+), 39 deletions(-) rename cmd/subcmds/{output.go => synthOutput.go} (97%) diff --git a/cmd/subcmds/output.go b/cmd/subcmds/synthOutput.go similarity index 97% rename from cmd/subcmds/output.go rename to cmd/subcmds/synthOutput.go index aa817b7e..ff694c47 100644 --- a/cmd/subcmds/output.go +++ b/cmd/subcmds/synthOutput.go @@ -85,7 +85,7 @@ func writeToFile(outputFile string, data *bytes.Buffer) error { return os.WriteFile(outputFile, data.Bytes(), defaultFilePermission) } -func pickWriter(args *inArgs, data *bytes.Buffer) (ir.Writer, error) { +func pickWriter(args *inArgs, data *bytes.Buffer) (ir.SynthWriter, error) { w := bufio.NewWriter(data) switch args.outputFmt { case tfOutputFormat: diff --git a/pkg/io/confio/acl.go b/pkg/io/confio/acl.go index 20f06d1a..09231c65 100644 --- a/pkg/io/confio/acl.go +++ b/pkg/io/confio/acl.go @@ -169,7 +169,7 @@ func subnetRef(subnet *configModel.Subnet) *vpcv1.SubnetReference { } } -func (w *Writer) WriteACL(collection *ir.ACLCollection, _ string) error { +func (w *Writer) WriteSynthACL(collection *ir.ACLCollection, _ string) error { updateACL(w.model, collection) return w.writeModel() } diff --git a/pkg/io/confio/sg.go b/pkg/io/confio/sg.go index ee551214..cadf8232 100644 --- a/pkg/io/confio/sg.go +++ b/pkg/io/confio/sg.go @@ -245,7 +245,7 @@ func updateSG(model *configModel.ResourcesContainerModel, collection *ir.SGColle globalIndex = 0 // making test results more predictable } -func (w *Writer) WriteSG(collection *ir.SGCollection, _ string) error { +func (w *Writer) WriteSynthSG(collection *ir.SGCollection, _ string) error { updateSG(w.model, collection) return w.writeModel() } diff --git a/pkg/io/csvio/acl.go b/pkg/io/csvio/acl.go index a5e9c00e..47e9b5ea 100644 --- a/pkg/io/csvio/acl.go +++ b/pkg/io/csvio/acl.go @@ -36,7 +36,7 @@ func ACLPort(p interval.Interval) string { } // Write prints an entire collection of acls as a single CSV table. -func (w *Writer) WriteACL(collection *ir.ACLCollection, vpc string) error { +func (w *Writer) WriteSynthACL(collection *ir.ACLCollection, vpc string) error { if err := w.w.WriteAll(aclHeader()); err != nil { return err } diff --git a/pkg/io/csvio/common.go b/pkg/io/csvio/common.go index 41cd664d..c61e180e 100644 --- a/pkg/io/csvio/common.go +++ b/pkg/io/csvio/common.go @@ -18,7 +18,7 @@ import ( "github.com/np-guard/vpc-network-config-synthesis/pkg/ir" ) -// Writer implements ir.Writer +// Writer implements ir.SynthWriter type Writer struct { w *csv.Writer } diff --git a/pkg/io/csvio/sg.go b/pkg/io/csvio/sg.go index a10dc8db..78fa772c 100644 --- a/pkg/io/csvio/sg.go +++ b/pkg/io/csvio/sg.go @@ -16,7 +16,7 @@ import ( "github.com/np-guard/vpc-network-config-synthesis/pkg/ir" ) -func (w *Writer) WriteSG(collection *ir.SGCollection, vpc string) error { +func (w *Writer) WriteSynthSG(collection *ir.SGCollection, vpc string) error { if err := w.w.WriteAll(sgHeader()); err != nil { return err } diff --git a/pkg/io/mdio/acl.go b/pkg/io/mdio/acl.go index e593de0e..34fd55c7 100644 --- a/pkg/io/mdio/acl.go +++ b/pkg/io/mdio/acl.go @@ -36,7 +36,7 @@ func ACLPort(p interval.Interval) string { } // Write prints an entire collection of acls as a single MD table. -func (w *Writer) WriteACL(collection *ir.ACLCollection, vpc string) error { +func (w *Writer) WriteSynthACL(collection *ir.ACLCollection, vpc string) error { if err := w.writeAll(aclHeader()); err != nil { return err } diff --git a/pkg/io/mdio/common.go b/pkg/io/mdio/common.go index abfb24a8..fad3d774 100644 --- a/pkg/io/mdio/common.go +++ b/pkg/io/mdio/common.go @@ -20,7 +20,7 @@ import ( const leftAlign = " :--- " -// Writer implements ir.Writer +// Writer implements ir.SynthWriter type Writer struct { w *bufio.Writer } diff --git a/pkg/io/mdio/sg.go b/pkg/io/mdio/sg.go index 044ddbca..8535fb19 100644 --- a/pkg/io/mdio/sg.go +++ b/pkg/io/mdio/sg.go @@ -16,7 +16,7 @@ import ( "github.com/np-guard/vpc-network-config-synthesis/pkg/ir" ) -func (w *Writer) WriteSG(collection *ir.SGCollection, vpc string) error { +func (w *Writer) WriteSynthSG(collection *ir.SGCollection, vpc string) error { if err := w.writeAll(sgHeader()); err != nil { return err } diff --git a/pkg/io/tfio/acl.go b/pkg/io/tfio/acl.go index e6d36637..b09e3a11 100644 --- a/pkg/io/tfio/acl.go +++ b/pkg/io/tfio/acl.go @@ -18,7 +18,7 @@ import ( ) // WriteACL prints an entire collection of acls as a sequence of terraform resources. -func (w *Writer) WriteACL(c *ir.ACLCollection, vpc string) error { +func (w *Writer) WriteSynthACL(c *ir.ACLCollection, vpc string) error { output := aclCollection(c, vpc).Print() _, err := w.w.WriteString(output) if err != nil { diff --git a/pkg/io/tfio/common.go b/pkg/io/tfio/common.go index 00b9d2b7..60b9f1da 100644 --- a/pkg/io/tfio/common.go +++ b/pkg/io/tfio/common.go @@ -21,7 +21,7 @@ import ( "github.com/np-guard/vpc-network-config-synthesis/pkg/ir" ) -// Writer implements ir.Writer +// Writer implements ir.SynthWriter type Writer struct { w *bufio.Writer } diff --git a/pkg/io/tfio/sg.go b/pkg/io/tfio/sg.go index 1958ec24..6cd5447d 100644 --- a/pkg/io/tfio/sg.go +++ b/pkg/io/tfio/sg.go @@ -19,7 +19,7 @@ import ( ) // WriteSG prints an entire collection of Security Groups as a sequence of terraform resources. -func (w *Writer) WriteSG(c *ir.SGCollection, vpc string) error { +func (w *Writer) WriteSynthSG(c *ir.SGCollection, vpc string) error { output := sgCollection(c, vpc).Print() _, err := w.w.WriteString(output) if err != nil { diff --git a/pkg/ir/sg.go b/pkg/ir/sg.go index 5cf895ec..b9783071 100644 --- a/pkg/ir/sg.go +++ b/pkg/ir/sg.go @@ -26,37 +26,39 @@ const ( SGResourceFileShareMountTarget SGResource = "fsmt" ) -type SGName string +type ( + SGName string -func (s SGName) String() string { - return string(s) -} + RemoteType interface { + fmt.Stringer + // *netset.IPBlock | SGName + } -type RemoteType interface { - fmt.Stringer - // *netset.IPBlock | SGName -} + SGRule struct { + Direction Direction + Remote RemoteType + Protocol netp.Protocol + Local *netset.IPBlock + Explanation string + } -type SGRule struct { - Direction Direction - Remote RemoteType - Protocol netp.Protocol - Local *netset.IPBlock - Explanation string -} + SG struct { + InboundRules []SGRule + OutboundRules []SGRule + Attached []ID + } -type SG struct { - InboundRules []SGRule - OutboundRules []SGRule - Attached []ID -} + SGCollection struct { + SGs map[ID]map[SGName]*SG + } -type SGCollection struct { - SGs map[ID]map[SGName]*SG -} + SynthSGWriter interface { + WriteSynthSG(sgColl *SGCollection, vpc string) error + } +) -type SGWriter interface { - WriteSG(sgColl *SGCollection, vpc string) error +func (s SGName) String() string { + return string(s) } func (r *SGRule) isRedundant(rules []SGRule) bool { @@ -110,8 +112,8 @@ func (a *SG) AllRules() []SGRule { return append(a.InboundRules, a.OutboundRules...) } -func (c *SGCollection) Write(w Writer, vpc string) error { - return w.WriteSG(c, vpc) +func (c *SGCollection) Write(w SynthWriter, vpc string) error { + return w.WriteSynthSG(c, vpc) } func (c *SGCollection) SortedSGNames(vpc ID) []SGName { From 8203f7884c890c1c243b51024674a2d54e0b4275 Mon Sep 17 00:00:00 2001 From: Yair Slobodin Date: Mon, 26 Aug 2024 14:45:58 +0300 Subject: [PATCH 04/24] unexported two functions --- pkg/io/csvio/acl.go | 32 ++++++++++++++++---------------- pkg/io/csvio/sg.go | 8 ++++---- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/pkg/io/csvio/acl.go b/pkg/io/csvio/acl.go index 47e9b5ea..308910a1 100644 --- a/pkg/io/csvio/acl.go +++ b/pkg/io/csvio/acl.go @@ -17,6 +17,20 @@ import ( "github.com/np-guard/vpc-network-config-synthesis/pkg/ir" ) +// Write prints an entire collection of acls as a single CSV table. +func (w *Writer) WriteSynthACL(collection *ir.ACLCollection, vpc string) error { + if err := w.w.WriteAll(aclHeader()); err != nil { + return err + } + for _, subnet := range collection.SortedACLSubnets(vpc) { + vpcName := ir.VpcFromScopedResource(subnet) + if err := w.w.WriteAll(makeACLTable(collection.ACLs[vpcName][subnet], subnet)); err != nil { + return err + } + } + return nil +} + func makeACLTable(t *ir.ACL, subnet string) [][]string { rules := t.Rules() rows := make([][]string, len(rules)) @@ -26,7 +40,7 @@ func makeACLTable(t *ir.ACL, subnet string) [][]string { return rows } -func ACLPort(p interval.Interval) string { +func aclPort(p interval.Interval) string { switch { case p.Start() == netp.MinPort && p.End() == netp.MaxPort: return "any port" //nolint:goconst // independent decision for SG and ACL @@ -35,20 +49,6 @@ func ACLPort(p interval.Interval) string { } } -// Write prints an entire collection of acls as a single CSV table. -func (w *Writer) WriteSynthACL(collection *ir.ACLCollection, vpc string) error { - if err := w.w.WriteAll(aclHeader()); err != nil { - return err - } - for _, subnet := range collection.SortedACLSubnets(vpc) { - vpcName := ir.VpcFromScopedResource(subnet) - if err := w.w.WriteAll(makeACLTable(collection.ACLs[vpcName][subnet], subnet)); err != nil { - return err - } - } - return nil -} - func action(a ir.Action) string { if a == ir.Deny { return "Deny" @@ -101,7 +101,7 @@ func printIP(ip *netset.IPBlock, protocol netp.Protocol, isSource bool) string { } else { r = p.DstPorts() } - return fmt.Sprintf("%v, %v", ipString, ACLPort(r)) + return fmt.Sprintf("%v, %v", ipString, aclPort(r)) case netp.AnyProtocol: return ipString default: diff --git a/pkg/io/csvio/sg.go b/pkg/io/csvio/sg.go index 78fa772c..aa320afe 100644 --- a/pkg/io/csvio/sg.go +++ b/pkg/io/csvio/sg.go @@ -45,7 +45,7 @@ func makeSGRow(rule *ir.SGRule, sgName ir.SGName) []string { return []string{ string(sgName), direction(rule.Direction), - sGRemoteType(rule.Remote), + sgRemoteType(rule.Remote), sgRemote(rule.Remote), printProtocolName(rule.Protocol), printProtocolParams(rule.Protocol, rule.Direction == ir.Inbound), @@ -62,7 +62,7 @@ func makeSGTable(t *ir.SG, sgName ir.SGName) [][]string { return rows } -func sGPort(p interval.Interval) string { +func sgPort(p interval.Interval) string { switch { case p.Start() == netp.MinPort && p.End() == netp.MaxPort: return "any port" @@ -71,7 +71,7 @@ func sGPort(p interval.Interval) string { } } -func sGRemoteType(t ir.RemoteType) string { +func sgRemoteType(t ir.RemoteType) string { switch t := t.(type) { case *netset.IPBlock: if t.Size() == 1 { @@ -112,7 +112,7 @@ func printProtocolParams(protocol netp.Protocol, isSource bool) string { } else { r = p.DstPorts() } - return sGPort(r) + return sgPort(r) case netp.AnyProtocol: return "" default: From 5fbeefc1455268fb3a99ed2bc33a2e2154f51c93 Mon Sep 17 00:00:00 2001 From: Yair Slobodin Date: Mon, 26 Aug 2024 14:47:59 +0300 Subject: [PATCH 05/24] wip --- pkg/io/confio/common.go | 2 +- pkg/ir/acl.go | 54 +++++++++++++++++++++-------------------- pkg/ir/common.go | 16 ++++++------ test/end_to_end_test.go | 2 +- 4 files changed, 39 insertions(+), 35 deletions(-) diff --git a/pkg/io/confio/common.go b/pkg/io/confio/common.go index e3e86f61..97aa4274 100644 --- a/pkg/io/confio/common.go +++ b/pkg/io/confio/common.go @@ -12,7 +12,7 @@ import ( configModel "github.com/np-guard/cloud-resource-collector/pkg/ibm/datamodel" ) -// Writer implements ir.Writer +// Writer implements ir.SynthWriter type Writer struct { w *bufio.Writer model *configModel.ResourcesContainerModel diff --git a/pkg/ir/acl.go b/pkg/ir/acl.go index 41248c82..993e5047 100644 --- a/pkg/ir/acl.go +++ b/pkg/ir/acl.go @@ -15,36 +15,38 @@ import ( "github.com/np-guard/vpc-network-config-synthesis/pkg/utils" ) -type Action string +type ( + Action string + + ACLRule struct { + Action Action + Direction Direction + Source *netset.IPBlock + Destination *netset.IPBlock + Protocol netp.Protocol + Explanation string + } + + ACL struct { + Subnet string + Internal []ACLRule + External []ACLRule + } + + ACLCollection struct { + ACLs map[ID]map[string]*ACL + } + + SynthACLWriter interface { + WriteSynthACL(aclColl *ACLCollection, vpc string) error + } +) const ( Allow Action = "allow" Deny Action = "deny" ) -type ACLRule struct { - Action Action - Direction Direction - Source *netset.IPBlock - Destination *netset.IPBlock - Protocol netp.Protocol - Explanation string -} - -type ACL struct { - Subnet string - Internal []ACLRule - External []ACLRule -} - -type ACLCollection struct { - ACLs map[ID]map[string]*ACL -} - -type ACLWriter interface { - WriteACL(aclColl *ACLCollection, vpc string) error -} - func (r *ACLRule) isRedundant(rules []ACLRule) bool { for i := range rules { if rules[i].mustSupersede(r) { @@ -123,8 +125,8 @@ func (c *ACLCollection) LookupOrCreate(name string) *ACL { return newACL } -func (c *ACLCollection) Write(w Writer, vpc string) error { - return w.WriteACL(c, vpc) +func (c *ACLCollection) Write(w SynthWriter, vpc string) error { + return w.WriteSynthACL(c, vpc) } func (c *ACLCollection) SortedACLSubnets(vpc string) []string { diff --git a/pkg/ir/common.go b/pkg/ir/common.go index e10b1265..17b2a69d 100644 --- a/pkg/ir/common.go +++ b/pkg/ir/common.go @@ -5,18 +5,20 @@ SPDX-License-Identifier: Apache-2.0 package ir -type Direction string +type ( + Direction string + + SynthWriter interface { + SynthACLWriter + SynthSGWriter + } +) const ( Outbound Direction = "outbound" Inbound Direction = "inbound" ) -type Writer interface { - ACLWriter - SGWriter -} - type Collection interface { - Write(writer Writer, vpc string) error + Write(writer SynthWriter, vpc string) error } diff --git a/test/end_to_end_test.go b/test/end_to_end_test.go index 695a98d5..484ec4c0 100644 --- a/test/end_to_end_test.go +++ b/test/end_to_end_test.go @@ -146,7 +146,7 @@ func shrinkWhitespace(s string) string { func write(collection ir.Collection, outputFormat, conn, vpc string) (text string, err error) { buf := new(bytes.Buffer) - var writer ir.Writer + var writer ir.SynthWriter switch outputFormat { case "csv": writer = csvio.NewWriter(buf) From 02030aca2af8c3896f5a4a7ccd68547eaa609f6a Mon Sep 17 00:00:00 2001 From: Yair Slobodin Date: Mon, 26 Aug 2024 15:32:22 +0300 Subject: [PATCH 06/24] updated --- pkg/io/confio/parse_sgs.go | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/pkg/io/confio/parse_sgs.go b/pkg/io/confio/parse_sgs.go index daf044cc..a079f9c0 100644 --- a/pkg/io/confio/parse_sgs.go +++ b/pkg/io/confio/parse_sgs.go @@ -19,20 +19,25 @@ import ( ) // ReadSG translates SGs from a config_object file to map[ir.SGName]*SG -func ReadSGs(filename string) (map[ir.SGName]*ir.SG, error) { +func ReadSGs(filename string) (*ir.SGCollection, error) { config, err := readModel(filename) if err != nil { return nil, err } - result := make(map[ir.SGName]*ir.SG, len(config.SecurityGroupList)) + result := ir.NewSGCollection() + for _, sg := range config.SecurityGroupList { inbound, outbound, err := translateSGRules(&sg.SecurityGroup) if err != nil { return nil, err } if sg.Name != nil { - result[ir.SGName(*sg.Name)] = &ir.SG{InboundRules: inbound, OutboundRules: outbound} + vpcName := *sg.VPC.Name + 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} } } return result, nil From 0c9cfe29f5f7591ca8660ae1ef5b97d1c17e9281 Mon Sep 17 00:00:00 2001 From: Yair Slobodin Date: Mon, 26 Aug 2024 17:27:34 +0300 Subject: [PATCH 07/24] wip --- cmd/subcmds/optimizeOutput.go | 24 ++++++++++++++- cmd/subcmds/optimizeSG.go | 3 +- cmd/subcmds/outputCommon.go | 58 +++++++++++++++++++++++++++++++++++ cmd/subcmds/synthOutput.go | 56 ++++++--------------------------- pkg/io/tfio/acl.go | 4 +++ pkg/io/tfio/sg.go | 4 +++ pkg/ir/acl.go | 10 +++++- pkg/ir/common.go | 17 +++++++--- pkg/ir/sg.go | 10 +++++- pkg/synth/acl.go | 2 +- pkg/synth/common.go | 2 +- pkg/synth/sg.go | 2 +- test/end_to_end_test.go | 8 ++--- 13 files changed, 139 insertions(+), 61 deletions(-) create mode 100644 cmd/subcmds/outputCommon.go diff --git a/cmd/subcmds/optimizeOutput.go b/cmd/subcmds/optimizeOutput.go index af92e337..1d20542c 100644 --- a/cmd/subcmds/optimizeOutput.go +++ b/cmd/subcmds/optimizeOutput.go @@ -6,9 +6,31 @@ SPDX-License-Identifier: Apache-2.0 package subcmds import ( + "bufio" + "bytes" + "fmt" + + "github.com/np-guard/vpc-network-config-synthesis/pkg/io/tfio" "github.com/np-guard/vpc-network-config-synthesis/pkg/ir" ) -func writeOptimizeOutput(_ *inArgs, _ ir.Collection) error { +func writeOptimizeOutput(args *inArgs, collection ir.OptimizeCollection, vpcNames []string) error { + if err := checkOutputFlags(args); err != nil { + return err + } + _, isACLCollection := collection.(*ir.ACLCollection) + if err := writeLocals(args, vpcNames, isACLCollection); err != nil { + return err + } return nil } + +func pickOptimizeWriter(args *inArgs, data *bytes.Buffer) (ir.SynthWriter, error) { + w := bufio.NewWriter(data) + switch args.outputFmt { + case tfOutputFormat: + return tfio.NewWriter(w), nil + default: + return nil, fmt.Errorf("bad output format: %q", args.outputFmt) + } +} diff --git a/cmd/subcmds/optimizeSG.go b/cmd/subcmds/optimizeSG.go index 57ee6c32..53f83477 100644 --- a/cmd/subcmds/optimizeSG.go +++ b/cmd/subcmds/optimizeSG.go @@ -12,6 +12,7 @@ import ( "github.com/np-guard/vpc-network-config-synthesis/pkg/io/confio" "github.com/np-guard/vpc-network-config-synthesis/pkg/optimize" + "github.com/np-guard/vpc-network-config-synthesis/pkg/utils" ) func NewOptimizeSGCommand(args *inArgs) *cobra.Command { @@ -40,5 +41,5 @@ func optimization(cmd *cobra.Command, args *inArgs) error { if optimize.ReduceSGRules(sgs, args.sgName) != nil { return err } - return writeOptimizeOutput(args, sgs) + return writeOptimizeOutput(args, sgs, utils.MapKeys(sgs.SGs)) } diff --git a/cmd/subcmds/outputCommon.go b/cmd/subcmds/outputCommon.go new file mode 100644 index 00000000..d663670c --- /dev/null +++ b/cmd/subcmds/outputCommon.go @@ -0,0 +1,58 @@ +/* +Copyright 2023- IBM Inc. All Rights Reserved. +SPDX-License-Identifier: Apache-2.0 +*/ + +package subcmds + +import ( + "bytes" + "fmt" + "os" + "path/filepath" + + "github.com/np-guard/vpc-network-config-synthesis/pkg/io/tfio" + "github.com/np-guard/vpc-network-config-synthesis/pkg/ir" +) + +func checkOutputFlags(args *inArgs) 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") + } + return 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 writeLocals(args *inArgs, vpcNames []ir.ID, isACL bool) error { + if !args.locals { + return nil + } + if args.outputFmt != tfOutputFormat { + return fmt.Errorf("--locals flag requires setting the output format to tf") + } + + var data *bytes.Buffer + var err error + if data, err = tfio.WriteLocals(vpcNames, isACL); err != nil { + return err + } + + outputFile := "" + suffix := "/locals.tf" + if args.outputDir != "" { + outputFile = args.outputDir + suffix + } else if args.outputFile != "" { + outputFile = filepath.Dir(args.outputFile) + suffix + } + return writeToFile(outputFile, data) +} diff --git a/cmd/subcmds/synthOutput.go b/cmd/subcmds/synthOutput.go index ff694c47..c1ba8a0d 100644 --- a/cmd/subcmds/synthOutput.go +++ b/cmd/subcmds/synthOutput.go @@ -10,7 +10,6 @@ import ( "bytes" "fmt" "os" - "path/filepath" "github.com/np-guard/vpc-network-config-synthesis/pkg/io/confio" "github.com/np-guard/vpc-network-config-synthesis/pkg/io/csvio" @@ -23,26 +22,24 @@ import ( const defaultFilePermission = 0o644 const defaultDirectoryPermission = 0o755 -func writeOutput(args *inArgs, collection ir.Collection, vpcNames []ir.ID) error { - if err := updateOutputFormat(args); err != nil { +func writeOutput(args *inArgs, collection ir.SynthCollection, vpcNames []ir.ID) error { + if err := checkOutputFlags(args); err != nil { return err } - if args.outputDir != "" && args.outputFmt == apiOutputFormat { - return fmt.Errorf("-d cannot be used with format json") - } 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 = writeSynthCollection(args, collection, ""); err != nil { return err } return writeToFile(args.outputFile, data) @@ -55,7 +52,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 = writeSynthCollection(args, collection, vpc); err != nil { return err } if err := writeToFile(args.outputFile, data); err != nil { @@ -65,27 +62,19 @@ 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 writeSynthCollection(args *inArgs, collection ir.SynthCollection, vpc string) (*bytes.Buffer, error) { var data bytes.Buffer - writer, err := pickWriter(args, &data) + writer, err := pickSynthWriter(args, &data) if err != nil { return nil, err } - if err := collection.Write(writer, vpc); err != nil { + if err := collection.WriteSynth(writer, vpc); 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.SynthWriter, error) { +func pickSynthWriter(args *inArgs, data *bytes.Buffer) (ir.SynthWriter, error) { w := bufio.NewWriter(data) switch args.outputFmt { case tfOutputFormat: @@ -100,28 +89,3 @@ func pickWriter(args *inArgs, data *bytes.Buffer) (ir.SynthWriter, error) { return nil, fmt.Errorf("bad output format: %q", args.outputFmt) } } - -func writeLocals(args *inArgs, collection ir.Collection, vpcNames []ir.ID) error { - if !args.locals { - return nil - } - if args.outputFmt != tfOutputFormat { - return fmt.Errorf("--locals flag requires setting the output format to tf") - } - - _, isACLCollection := collection.(*ir.ACLCollection) - var data *bytes.Buffer - var err error - if data, err = tfio.WriteLocals(vpcNames, isACLCollection); err != nil { - return err - } - - outputFile := "" - suffix := "/locals.tf" - if args.outputDir != "" { - outputFile = args.outputDir + suffix - } else if args.outputFile != "" { - outputFile = filepath.Dir(args.outputFile) + suffix - } - return writeToFile(outputFile, data) -} diff --git a/pkg/io/tfio/acl.go b/pkg/io/tfio/acl.go index b09e3a11..1c0d7cda 100644 --- a/pkg/io/tfio/acl.go +++ b/pkg/io/tfio/acl.go @@ -28,6 +28,10 @@ func (w *Writer) WriteSynthACL(c *ir.ACLCollection, vpc string) error { return err } +func (w *Writer) WriteOptimizeACL(c *ir.ACLCollection) error { + return nil +} + func aclProtocol(t netp.Protocol) []tf.Block { switch p := t.(type) { case netp.TCPUDP: diff --git a/pkg/io/tfio/sg.go b/pkg/io/tfio/sg.go index 6cd5447d..ddb66ac7 100644 --- a/pkg/io/tfio/sg.go +++ b/pkg/io/tfio/sg.go @@ -29,6 +29,10 @@ func (w *Writer) WriteSynthSG(c *ir.SGCollection, vpc string) error { return err } +func (w *Writer) WriteOptimizeSG(c *ir.SGCollection, vpc string) error { + return nil +} + func value(x interface{}) string { switch v := x.(type) { case *netset.IPBlock: diff --git a/pkg/ir/acl.go b/pkg/ir/acl.go index 993e5047..9075a28e 100644 --- a/pkg/ir/acl.go +++ b/pkg/ir/acl.go @@ -40,6 +40,10 @@ type ( SynthACLWriter interface { WriteSynthACL(aclColl *ACLCollection, vpc string) error } + + OptimizeACLWriter interface { + WriteOptimizeACL(aclColl *ACLCollection) error + } ) const ( @@ -125,10 +129,14 @@ func (c *ACLCollection) LookupOrCreate(name string) *ACL { return newACL } -func (c *ACLCollection) Write(w SynthWriter, vpc string) error { +func (c *ACLCollection) WriteSynth(w SynthWriter, vpc string) error { return w.WriteSynthACL(c, vpc) } +func (c *ACLCollection) WriteOptimize(w OptimizeWriter) error { + return w.WriteOptimizeACL(c) +} + func (c *ACLCollection) SortedACLSubnets(vpc string) []string { if vpc == "" { return utils.SortedAllInnerMapsKeys(c.ACLs) diff --git a/pkg/ir/common.go b/pkg/ir/common.go index 17b2a69d..5f9fddd8 100644 --- a/pkg/ir/common.go +++ b/pkg/ir/common.go @@ -8,17 +8,26 @@ package ir type ( Direction string + SynthCollection interface { + WriteSynth(writer SynthWriter, vpc string) error + } + + OptimizeCollection interface { + WriteOptimize(writer OptimizeWriter) error + } + SynthWriter interface { SynthACLWriter SynthSGWriter } + + OptimizeWriter interface { + OptimizeACLWriter + OptimizeSGWriter + } ) const ( Outbound Direction = "outbound" Inbound Direction = "inbound" ) - -type Collection interface { - Write(writer SynthWriter, vpc string) error -} diff --git a/pkg/ir/sg.go b/pkg/ir/sg.go index b9783071..917125e3 100644 --- a/pkg/ir/sg.go +++ b/pkg/ir/sg.go @@ -55,6 +55,10 @@ type ( SynthSGWriter interface { WriteSynthSG(sgColl *SGCollection, vpc string) error } + + OptimizeSGWriter interface { + WriteOptimizeSG(sgColl *SGCollection) error + } ) func (s SGName) String() string { @@ -112,10 +116,14 @@ func (a *SG) AllRules() []SGRule { return append(a.InboundRules, a.OutboundRules...) } -func (c *SGCollection) Write(w SynthWriter, vpc string) error { +func (c *SGCollection) WriteSynth(w SynthWriter, vpc string) error { return w.WriteSynthSG(c, vpc) } +func (c *SGCollection) WriteOptimize(w OptimizeWriter) error { + return w.WriteOptimizeSG(c) +} + func (c *SGCollection) SortedSGNames(vpc ID) []SGName { if vpc == "" { return utils.SortedAllInnerMapsKeys(c.SGs) diff --git a/pkg/synth/acl.go b/pkg/synth/acl.go index ab16eb35..50e999fd 100644 --- a/pkg/synth/acl.go +++ b/pkg/synth/acl.go @@ -28,7 +28,7 @@ func NewACLSynthesizer(s *ir.Spec, single bool) Synthesizer { return &ACLSynthesizer{spec: s, singleACL: single, result: ir.NewACLCollection()} } -func (a *ACLSynthesizer) Synth() ir.Collection { +func (a *ACLSynthesizer) Synth() ir.SynthCollection { return a.makeACL() } diff --git a/pkg/synth/common.go b/pkg/synth/common.go index 9833c121..5ce7b220 100644 --- a/pkg/synth/common.go +++ b/pkg/synth/common.go @@ -15,7 +15,7 @@ import ( type ( Synthesizer interface { - Synth() ir.Collection + Synth() ir.SynthCollection } namedAddrs struct { diff --git a/pkg/synth/sg.go b/pkg/synth/sg.go index 054171cf..13938223 100644 --- a/pkg/synth/sg.go +++ b/pkg/synth/sg.go @@ -23,7 +23,7 @@ func NewSGSynthesizer(s *ir.Spec, _ bool) Synthesizer { return &SGSynthesizer{spec: s, result: ir.NewSGCollection()} } -func (s *SGSynthesizer) Synth() ir.Collection { +func (s *SGSynthesizer) Synth() ir.SynthCollection { return s.makeSG() } diff --git a/test/end_to_end_test.go b/test/end_to_end_test.go index 484ec4c0..a3cceeb4 100644 --- a/test/end_to_end_test.go +++ b/test/end_to_end_test.go @@ -144,7 +144,7 @@ func shrinkWhitespace(s string) string { return regexp.MustCompile(`[ \t]+`).ReplaceAllString(s, " ") } -func write(collection ir.Collection, outputFormat, conn, vpc string) (text string, err error) { +func write(collection ir.SynthCollection, outputFormat, conn, vpc string) (text string, err error) { buf := new(bytes.Buffer) var writer ir.SynthWriter switch outputFormat { @@ -160,7 +160,7 @@ func write(collection ir.Collection, outputFormat, conn, vpc string) (text strin if err != nil { return "", err } - err = collection.Write(writer, vpc) + err = collection.WriteSynth(writer, vpc) if err != nil { return "", err } @@ -175,7 +175,7 @@ func readExpectedFile(filename string) string { return shrinkWhitespace(string(buf)) } -func writeSingleFile(testCase TestCase, t *testing.T, collection ir.Collection) { +func writeSingleFile(testCase TestCase, t *testing.T, collection ir.SynthCollection) { actual, err := write(collection, testCase.outputFormat, fmt.Sprintf("../test/%s", testCase.resolve(defaultConfigName)), "") if err != nil { t.Fatal(err) @@ -188,7 +188,7 @@ func writeSingleFile(testCase TestCase, t *testing.T, collection ir.Collection) } } -func writeMultipleFiles(testCase TestCase, t *testing.T, collection ir.Collection, s *ir.Spec) { +func writeMultipleFiles(testCase TestCase, t *testing.T, collection ir.SynthCollection, s *ir.Spec) { for vpcName := range s.Defs.VPCs { actual, err := write(collection, testCase.outputFormat, fmt.Sprintf("../test/%s", testCase.resolve(defaultConfigName)), vpcName) if err != nil { From eebd628a1ff4be48a286ca963910e8ec6bbb2f21 Mon Sep 17 00:00:00 2001 From: Yair Slobodin Date: Tue, 27 Aug 2024 14:56:06 +0300 Subject: [PATCH 08/24] template --- cmd/subcmds/optimize.go | 23 ++++++++++++++++- cmd/subcmds/optimizeOutput.go | 20 +++++++++++++-- cmd/subcmds/optimizeSG.go | 22 +--------------- cmd/subcmds/outputCommon.go | 3 +++ cmd/subcmds/root.go | 48 +++++++++++++++++------------------ cmd/subcmds/synth.go | 7 +++++ pkg/io/tfio/common.go | 2 +- pkg/io/tfio/sg.go | 2 +- pkg/ir/acl.go | 4 +++ pkg/ir/sg.go | 4 +++ pkg/optimize/common.go | 19 ++++++++++++++ pkg/optimize/sg.go | 26 ++++++++++++++++++- 12 files changed, 129 insertions(+), 51 deletions(-) create mode 100644 pkg/optimize/common.go diff --git a/cmd/subcmds/optimize.go b/cmd/subcmds/optimize.go index 877662a9..4b215fe1 100644 --- a/cmd/subcmds/optimize.go +++ b/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,22 @@ func NewOptimizeCommand(args *inArgs) *cobra.Command { Long: `optimization of existing SG (nACLS are not supported yet)`, } + // flags + cmd.Flags().StringVarP(&args.firewallName, firewallNameFlag, "s", "", "which vpcFirewall to optimize") + + // flags settings + _ = cmd.MarkPersistentFlagRequired(firewallNameFlag) // temporary + + // 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) + } + return writeOptimizeOutput(args, newOptimizer.Optimize(), newOptimizer.VpcNames()) +} diff --git a/cmd/subcmds/optimizeOutput.go b/cmd/subcmds/optimizeOutput.go index 1d20542c..2b602f1c 100644 --- a/cmd/subcmds/optimizeOutput.go +++ b/cmd/subcmds/optimizeOutput.go @@ -22,10 +22,26 @@ func writeOptimizeOutput(args *inArgs, collection ir.OptimizeCollection, vpcName if err := writeLocals(args, vpcNames, isACLCollection); err != nil { return err } - return nil + data, err := writeOptimizeCollection(args, collection) + if err != nil { + return err + } + return writeToFile(args.outputFile, data) +} + +func writeOptimizeCollection(args *inArgs, collection ir.OptimizeCollection) (*bytes.Buffer, error) { + var data bytes.Buffer + writer, err := pickOptimizeWriter(args, &data) + if err != nil { + return nil, err + } + if err := collection.WriteOptimize(writer); err != nil { + return nil, err + } + return &data, nil } -func pickOptimizeWriter(args *inArgs, data *bytes.Buffer) (ir.SynthWriter, error) { +func pickOptimizeWriter(args *inArgs, data *bytes.Buffer) (ir.OptimizeWriter, error) { w := bufio.NewWriter(data) switch args.outputFmt { case tfOutputFormat: diff --git a/cmd/subcmds/optimizeSG.go b/cmd/subcmds/optimizeSG.go index 53f83477..6a07c367 100644 --- a/cmd/subcmds/optimizeSG.go +++ b/cmd/subcmds/optimizeSG.go @@ -6,13 +6,9 @@ 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" - "github.com/np-guard/vpc-network-config-synthesis/pkg/utils" ) func NewOptimizeSGCommand(args *inArgs) *cobra.Command { @@ -22,24 +18,8 @@ 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) + 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, utils.MapKeys(sgs.SGs)) -} diff --git a/cmd/subcmds/outputCommon.go b/cmd/subcmds/outputCommon.go index d663670c..55872136 100644 --- a/cmd/subcmds/outputCommon.go +++ b/cmd/subcmds/outputCommon.go @@ -16,6 +16,9 @@ import ( ) func checkOutputFlags(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 } diff --git a/cmd/subcmds/root.go b/cmd/subcmds/root.go index 1f47e591..a6e13486 100644 --- a/cmd/subcmds/root.go +++ b/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 } diff --git a/cmd/subcmds/synth.go b/cmd/subcmds/synth.go index 04e4cc1c..b0b2210b 100644 --- a/cmd/subcmds/synth.go +++ b/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)) diff --git a/pkg/io/tfio/common.go b/pkg/io/tfio/common.go index 60b9f1da..7726b591 100644 --- a/pkg/io/tfio/common.go +++ b/pkg/io/tfio/common.go @@ -21,7 +21,7 @@ import ( "github.com/np-guard/vpc-network-config-synthesis/pkg/ir" ) -// Writer implements ir.SynthWriter +// Writer implements ir.SynthWriter and ir.OptimizeWriter type Writer struct { w *bufio.Writer } diff --git a/pkg/io/tfio/sg.go b/pkg/io/tfio/sg.go index ddb66ac7..981f44cb 100644 --- a/pkg/io/tfio/sg.go +++ b/pkg/io/tfio/sg.go @@ -29,7 +29,7 @@ func (w *Writer) WriteSynthSG(c *ir.SGCollection, vpc string) error { return err } -func (w *Writer) WriteOptimizeSG(c *ir.SGCollection, vpc string) error { +func (w *Writer) WriteOptimizeSG(c *ir.SGCollection) error { return nil } diff --git a/pkg/ir/acl.go b/pkg/ir/acl.go index 9075a28e..094ee726 100644 --- a/pkg/ir/acl.go +++ b/pkg/ir/acl.go @@ -129,6 +129,10 @@ func (c *ACLCollection) LookupOrCreate(name string) *ACL { return newACL } +func (c *ACLCollection) VpcNames() []string { + return utils.MapKeys(c.ACLs) +} + func (c *ACLCollection) WriteSynth(w SynthWriter, vpc string) error { return w.WriteSynthACL(c, vpc) } diff --git a/pkg/ir/sg.go b/pkg/ir/sg.go index 917125e3..f92d94e8 100644 --- a/pkg/ir/sg.go +++ b/pkg/ir/sg.go @@ -116,6 +116,10 @@ func (a *SG) AllRules() []SGRule { return append(a.InboundRules, a.OutboundRules...) } +func (c *SGCollection) VpcNames() []string { + return utils.MapKeys(c.SGs) +} + func (c *SGCollection) WriteSynth(w SynthWriter, vpc string) error { return w.WriteSynthSG(c, vpc) } diff --git a/pkg/optimize/common.go b/pkg/optimize/common.go new file mode 100644 index 00000000..d0aa9e52 --- /dev/null +++ b/pkg/optimize/common.go @@ -0,0 +1,19 @@ +/* +Copyright 2023- IBM Inc. All Rights Reserved. +SPDX-License-Identifier: Apache-2.0 +*/ + +package optimize + +import "github.com/np-guard/vpc-network-config-synthesis/pkg/ir" + +type Optimizer interface { + // read the collection from the config object file + ParseCollection(filename string) error + + // optimize number of SG/nACL rules + Optimize() ir.OptimizeCollection + + // returns a slice of all vpc names. used to generate locals file + VpcNames() []string +} diff --git a/pkg/optimize/sg.go b/pkg/optimize/sg.go index f3301962..4c74a514 100644 --- a/pkg/optimize/sg.go +++ b/pkg/optimize/sg.go @@ -6,9 +6,33 @@ SPDX-License-Identifier: Apache-2.0 package optimize import ( + "github.com/np-guard/vpc-network-config-synthesis/pkg/io/confio" "github.com/np-guard/vpc-network-config-synthesis/pkg/ir" + "github.com/np-guard/vpc-network-config-synthesis/pkg/utils" ) -func ReduceSGRules(_ *ir.SGCollection, _ string) error { +type SGOptimizer struct { + sgCollection *ir.SGCollection + sgName string +} + +func NewSGOptimizer(sgName string) Optimizer { + return &SGOptimizer{sgCollection: nil, sgName: sgName} +} + +func (s *SGOptimizer) ParseCollection(filename string) error { + c, err := confio.ReadSGs(filename) + if err != nil { + return err + } + s.sgCollection = c return nil } + +func (s *SGOptimizer) Optimize() ir.OptimizeCollection { + return s.sgCollection +} + +func (s *SGOptimizer) VpcNames() []string { + return utils.MapKeys(s.sgCollection.SGs) +} From 26473b326f930a9b34d054add9a954084888f15e Mon Sep 17 00:00:00 2001 From: Yair Slobodin Date: Wed, 28 Aug 2024 16:32:19 +0300 Subject: [PATCH 09/24] done --- cmd/subcmds/synth.go | 2 +- pkg/io/confio/parse_sgs.go | 4 ++- pkg/io/tfio/acl.go | 2 +- pkg/io/tfio/sg.go | 57 ++++++++++++++++++++++++-------------- pkg/ir/acl.go | 18 ++++++------ pkg/ir/sg.go | 10 ++++--- 6 files changed, 56 insertions(+), 37 deletions(-) diff --git a/cmd/subcmds/synth.go b/cmd/subcmds/synth.go index b0b2210b..71d94e01 100644 --- a/cmd/subcmds/synth.go +++ b/cmd/subcmds/synth.go @@ -44,5 +44,5 @@ func synthesis(cmd *cobra.Command, args *inArgs, newSynthesizer func(*ir.Spec, b return err } synthesizer := newSynthesizer(spec, single) - return writeOutput(args, synthesizer.Synth(), utils.MapKeys(spec.Defs.ConfigDefs.VPCs)) + return writeOutput(args, synthesizer.Synth(), utils.SortedMapKeys(spec.Defs.ConfigDefs.VPCs)) } diff --git a/pkg/io/confio/parse_sgs.go b/pkg/io/confio/parse_sgs.go index ce685abd..8d8eac44 100644 --- a/pkg/io/confio/parse_sgs.go +++ b/pkg/io/confio/parse_sgs.go @@ -35,7 +35,9 @@ 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} + sgName := ir.SGName(*sg.Name) + result.SGs[vpcName][sgName] = &ir.SG{SGName: sgName, VpcName: vpcName, + InboundRules: inbound, OutboundRules: outbound} } } return result, nil diff --git a/pkg/io/tfio/acl.go b/pkg/io/tfio/acl.go index 1c0d7cda..9a91f680 100644 --- a/pkg/io/tfio/acl.go +++ b/pkg/io/tfio/acl.go @@ -29,7 +29,7 @@ func (w *Writer) WriteSynthACL(c *ir.ACLCollection, vpc string) error { } func (w *Writer) WriteOptimizeACL(c *ir.ACLCollection) error { - return nil + return fmt.Errorf("OptimizeACL is not supported yet") } func aclProtocol(t netp.Protocol) []tf.Block { diff --git a/pkg/io/tfio/sg.go b/pkg/io/tfio/sg.go index 981f44cb..8f5a6fd7 100644 --- a/pkg/io/tfio/sg.go +++ b/pkg/io/tfio/sg.go @@ -18,9 +18,17 @@ import ( "github.com/np-guard/vpc-network-config-synthesis/pkg/ir" ) -// WriteSG prints an entire collection of Security Groups as a sequence of terraform resources. func (w *Writer) WriteSynthSG(c *ir.SGCollection, vpc string) error { - output := sgCollection(c, vpc).Print() + return w.writeSGCollection(c, vpc, true) +} + +func (w *Writer) WriteOptimizeSG(c *ir.SGCollection) error { + return w.writeSGCollection(c, "", false) +} + +// writeSGCollection prints an entire collection of Security Groups as a sequence of terraform resources. +func (w *Writer) writeSGCollection(c *ir.SGCollection, vpc string, writeComments bool) error { + output := sgCollection(c, vpc, writeComments).Print() _, err := w.w.WriteString(output) if err != nil { return err @@ -29,10 +37,6 @@ func (w *Writer) WriteSynthSG(c *ir.SGCollection, vpc string) error { return err } -func (w *Writer) WriteOptimizeSG(c *ir.SGCollection) error { - return nil -} - func value(x interface{}) string { switch v := x.(type) { case *netset.IPBlock: @@ -69,13 +73,17 @@ func sgProtocol(t netp.Protocol, d ir.Direction) []tf.Block { return nil } -func sgRule(rule *ir.SGRule, sgName ir.SGName, i int) tf.Block { +func sgRule(rule *ir.SGRule, sgName ir.SGName, i int, writeComment bool) tf.Block { ruleName := fmt.Sprintf("%v-%v", sgName, i) verifyName(ruleName) + comment := "" + if writeComment { + comment = fmt.Sprintf("# %v", rule.Explanation) + } return tf.Block{ Name: "resource", Labels: []string{quote("ibm_is_security_group_rule"), ir.ChangeScoping(quote(ruleName))}, - Comment: fmt.Sprintf("# %v", rule.Explanation), + Comment: comment, Arguments: []tf.Argument{ {Name: "group", Value: value(sgName)}, {Name: "direction", Value: quote(direction(rule.Direction))}, @@ -85,7 +93,8 @@ func sgRule(rule *ir.SGRule, sgName ir.SGName, i int) tf.Block { } } -func sg(sgName, comment string) tf.Block { +func sgBlock(sg *ir.SG, comment string) tf.Block { + sgName := sg.SGName.String() verifyName(sgName) return tf.Block{ Name: "resource", //nolint:revive // obvious false positive @@ -94,22 +103,28 @@ func sg(sgName, comment string) tf.Block { Arguments: []tf.Argument{ {Name: "name", Value: ir.ChangeScoping(quote("sg-" + sgName))}, {Name: "resource_group", Value: "local.sg_synth_resource_group_id"}, - {Name: "vpc", Value: fmt.Sprintf("local.sg_synth_%s_id", ir.VpcFromScopedResource(sgName))}, + {Name: "vpc", Value: fmt.Sprintf("local.sg_synth_%s_id", sg.VpcName)}, }, } } -func sgCollection(t *ir.SGCollection, vpc string) *tf.ConfigFile { - var resources []tf.Block //nolint:prealloc // nontrivial to calculate, and an unlikely performance bottleneck - for _, sgName := range t.SortedSGNames(vpc) { - comment := "" - vpcName := ir.VpcFromScopedResource(string(sgName)) - rules := t.SGs[vpcName][sgName].AllRules() - comment = fmt.Sprintf("\n### SG attached to %v", sgName) - resources = append(resources, sg(sgName.String(), comment)) - for i := range rules { - rule := sgRule(&rules[i], sgName, i) - resources = append(resources, rule) +func sgCollection(t *ir.SGCollection, vpc string, writeComments bool) *tf.ConfigFile { + var resources []tf.Block + for _, vpcName := range t.VpcNames() { + if vpc != vpcName && vpc != "" { + continue + } + for _, sgName := range t.SortedSGNames(vpcName) { + sg := t.SGs[vpcName][sgName] + comment := "\n" + if writeComments { + comment = fmt.Sprintf("\n### SG attached to %v", sgName) + } + resources = append(resources, sgBlock(sg, comment)) + rules := sg.AllRules() + for i := range rules { + resources = append(resources, sgRule(&rules[i], sgName, i, writeComments)) + } } } return &tf.ConfigFile{ diff --git a/pkg/ir/acl.go b/pkg/ir/acl.go index 094ee726..81c621be 100644 --- a/pkg/ir/acl.go +++ b/pkg/ir/acl.go @@ -29,6 +29,7 @@ type ( ACL struct { Subnet string + VpcName string Internal []ACLRule External []ACLRule } @@ -111,26 +112,25 @@ func NewACLCollection() *ACLCollection { return &ACLCollection{ACLs: map[ID]map[string]*ACL{}} } -func NewACL() *ACL { - return &ACL{Internal: []ACLRule{}, External: []ACLRule{}} +func NewACL(subnet, vpcName string) *ACL { + return &ACL{Subnet: subnet, VpcName: vpcName, Internal: []ACLRule{}, External: []ACLRule{}} } -func (c *ACLCollection) LookupOrCreate(name string) *ACL { - vpcName := VpcFromScopedResource(name) - if acl, ok := c.ACLs[vpcName][name]; ok { +func (c *ACLCollection) LookupOrCreate(subnet string) *ACL { + vpcName := VpcFromScopedResource(subnet) + if acl, ok := c.ACLs[vpcName][subnet]; ok { return acl } - newACL := NewACL() - newACL.Subnet = name + newACL := NewACL(subnet, vpcName) if c.ACLs[vpcName] == nil { c.ACLs[vpcName] = make(map[string]*ACL) } - c.ACLs[vpcName][name] = newACL + c.ACLs[vpcName][subnet] = newACL return newACL } func (c *ACLCollection) VpcNames() []string { - return utils.MapKeys(c.ACLs) + return utils.SortedMapKeys(c.ACLs) } func (c *ACLCollection) WriteSynth(w SynthWriter, vpc string) error { diff --git a/pkg/ir/sg.go b/pkg/ir/sg.go index f92d94e8..e1d2f0a3 100644 --- a/pkg/ir/sg.go +++ b/pkg/ir/sg.go @@ -43,6 +43,8 @@ type ( } SG struct { + SGName SGName + VpcName string InboundRules []SGRule OutboundRules []SGRule Attached []ID @@ -82,8 +84,8 @@ func (r *SGRule) mustSupersede(other *SGRule) bool { return res } -func NewSG() *SG { - return &SG{InboundRules: []SGRule{}, OutboundRules: []SGRule{}, Attached: []ID{}} +func NewSG(vpcName string, sgName SGName) *SG { + return &SG{SGName: sgName, VpcName: vpcName, InboundRules: []SGRule{}, OutboundRules: []SGRule{}, Attached: []ID{}} } func NewSGCollection() *SGCollection { @@ -95,7 +97,7 @@ func (c *SGCollection) LookupOrCreate(name SGName) *SG { if sg, ok := c.SGs[vpcName][name]; ok { return sg } - newSG := NewSG() + newSG := NewSG(vpcName, name) if c.SGs[vpcName] == nil { c.SGs[vpcName] = make(map[SGName]*SG) } @@ -117,7 +119,7 @@ func (a *SG) AllRules() []SGRule { } func (c *SGCollection) VpcNames() []string { - return utils.MapKeys(c.SGs) + return utils.SortedMapKeys(c.SGs) } func (c *SGCollection) WriteSynth(w SynthWriter, vpc string) error { From a7f68f57921dc5d6e4d1f3de8917a6776afc832f Mon Sep 17 00:00:00 2001 From: Yair Slobodin Date: Wed, 11 Sep 2024 11:51:36 +0300 Subject: [PATCH 10/24] lint --- pkg/synth/sg.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/synth/sg.go b/pkg/synth/sg.go index 9e4d65c4..285b0185 100644 --- a/pkg/synth/sg.go +++ b/pkg/synth/sg.go @@ -114,7 +114,6 @@ func (s *SGSynthesizer) allowConnectionToDst(conn *ir.Connection, trackedProtoco } sgDst.Add(rule) } - } // generate SGs for blocked endpoints (endpoints that do not appear in Spec) From 380c62e9cb3c812555e35c563e4a8860b3160b31 Mon Sep 17 00:00:00 2001 From: Yair Slobodin Date: Thu, 12 Sep 2024 15:42:33 +0300 Subject: [PATCH 11/24] fixed --- pkg/io/confio/parse_sgs.go | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/pkg/io/confio/parse_sgs.go b/pkg/io/confio/parse_sgs.go index ce685abd..f97cdb6d 100644 --- a/pkg/io/confio/parse_sgs.go +++ b/pkg/io/confio/parse_sgs.go @@ -152,19 +152,33 @@ func translateRemote(remote vpcv1.SecurityGroupRuleRemoteIntf) (ir.RemoteType, e } func translateLocal(local vpcv1.SecurityGroupRuleLocalIntf) (*netset.IPBlock, error) { + var err error + var ipAddrs *netset.IPBlock if l, ok := local.(*vpcv1.SecurityGroupRuleLocal); ok { if l.CIDRBlock != nil { - return netset.IPBlockFromCidr(*l.CIDRBlock) + ipAddrs, err = netset.IPBlockFromCidr(*l.CIDRBlock) } if l.Address != nil { - return netset.IPBlockFromIPAddress(*l.CIDRBlock) + ipAddrs, err = netset.IPBlockFromIPAddress(*l.CIDRBlock) } + if err != nil { + return nil, err + } + return verifyLocalValue(ipAddrs) } return nil, fmt.Errorf("error parsing Local field") } +// temporary - first version of optimization requires that local value will be 0.0.0.0/32 +func verifyLocalValue(ipAddrs *netset.IPBlock) (*netset.IPBlock, error) { + if !ipAddrs.Equal(netset.GetCidrAll()) { + return nil, fmt.Errorf("only 0.0.0.0/32 CIDR block is supported for local values") + } + return ipAddrs, nil +} + func translateProtocolTCPUDP(rule *vpcv1.SecurityGroupRuleSecurityGroupRuleProtocolTcpudp) (netp.Protocol, error) { - isTCP := *rule.Protocol == string(netp.ProtocolStringTCP) + isTCP := *rule.Protocol == "tcp" minDstPort := utils.GetProperty(rule.PortMin, netp.MinPort) maxDstPort := utils.GetProperty(rule.PortMax, netp.MaxPort) return netp.NewTCPUDP(isTCP, netp.MinPort, netp.MaxPort, int(minDstPort), int(maxDstPort)) From 197ed317f695b0c621b5c58766bc4c06f8348217 Mon Sep 17 00:00:00 2001 From: Yair Slobodin Date: Thu, 12 Sep 2024 15:47:21 +0300 Subject: [PATCH 12/24] fixed --- cmd/subcmds/optimize.go | 2 +- cmd/subcmds/outputCommon.go | 2 ++ cmd/subcmds/synthOutput.go | 1 - 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/cmd/subcmds/optimize.go b/cmd/subcmds/optimize.go index 4b215fe1..fe137a15 100644 --- a/cmd/subcmds/optimize.go +++ b/cmd/subcmds/optimize.go @@ -21,7 +21,7 @@ func NewOptimizeCommand(args *inArgs) *cobra.Command { } // flags - cmd.Flags().StringVarP(&args.firewallName, firewallNameFlag, "s", "", "which vpcFirewall to optimize") + cmd.PersistentFlags().StringVarP(&args.firewallName, firewallNameFlag, "n", "", "which vpcFirewall to optimize") // flags settings _ = cmd.MarkPersistentFlagRequired(firewallNameFlag) // temporary diff --git a/cmd/subcmds/outputCommon.go b/cmd/subcmds/outputCommon.go index 55872136..aa399315 100644 --- a/cmd/subcmds/outputCommon.go +++ b/cmd/subcmds/outputCommon.go @@ -15,6 +15,8 @@ import ( "github.com/np-guard/vpc-network-config-synthesis/pkg/ir" ) +const defaultFilePermission = 0o644 + func checkOutputFlags(args *inArgs) error { if args.outputDir != "" && args.outputFile != "" { return fmt.Errorf("only one of -d and -o can be supplied") diff --git a/cmd/subcmds/synthOutput.go b/cmd/subcmds/synthOutput.go index c1ba8a0d..12e0670b 100644 --- a/cmd/subcmds/synthOutput.go +++ b/cmd/subcmds/synthOutput.go @@ -19,7 +19,6 @@ import ( "github.com/np-guard/vpc-network-config-synthesis/pkg/ir" ) -const defaultFilePermission = 0o644 const defaultDirectoryPermission = 0o755 func writeOutput(args *inArgs, collection ir.SynthCollection, vpcNames []ir.ID) error { From 6be5ee02f0971e871b014df71ca4cad819c29a12 Mon Sep 17 00:00:00 2001 From: Yair Slobodin Date: Wed, 25 Sep 2024 13:41:58 +0300 Subject: [PATCH 13/24] fixed --- pkg/io/confio/parse_sgs.go | 59 +++++++++++++++++++------------------- 1 file changed, 30 insertions(+), 29 deletions(-) diff --git a/pkg/io/confio/parse_sgs.go b/pkg/io/confio/parse_sgs.go index f97cdb6d..250a7e6e 100644 --- a/pkg/io/confio/parse_sgs.go +++ b/pkg/io/confio/parse_sgs.go @@ -30,21 +30,22 @@ func ReadSGs(filename string) (*ir.SGCollection, error) { if err != nil { return nil, err } - if sg.Name != nil { - vpcName := *sg.VPC.Name - 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} + if sg.Name == nil || sg.VPC == nil || sg.VPC.Name == nil { + continue } + vpcName := *sg.VPC.Name + 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} } return result, nil } // parse security rules, splitted into ingress and egress rules -func translateSGRules(sg *vpcv1.SecurityGroup) (ingressRules, egressRules []ir.SGRule, err error) { - ingressRules = []ir.SGRule{} - egressRules = []ir.SGRule{} +func translateSGRules(sg *vpcv1.SecurityGroup) (ingressRules, egressRules []*ir.SGRule, err error) { + ingressRules = []*ir.SGRule{} + egressRules = []*ir.SGRule{} for index := range sg.Rules { rule, err := translateSGRule(sg, index) if err != nil { @@ -60,7 +61,7 @@ func translateSGRules(sg *vpcv1.SecurityGroup) (ingressRules, egressRules []ir.S } // translateSGRule translates a security group rule to ir.SGRule -func translateSGRule(sg *vpcv1.SecurityGroup, index int) (sgRule ir.SGRule, err error) { +func translateSGRule(sg *vpcv1.SecurityGroup, index int) (sgRule *ir.SGRule, err error) { switch r := sg.Rules[index].(type) { case *vpcv1.SecurityGroupRuleSecurityGroupRuleProtocolAll: return translateSGRuleProtocolAll(r) @@ -69,63 +70,63 @@ func translateSGRule(sg *vpcv1.SecurityGroup, index int) (sgRule ir.SGRule, err case *vpcv1.SecurityGroupRuleSecurityGroupRuleProtocolIcmp: return translateSGRuleProtocolIcmp(r) } - return ir.SGRule{}, fmt.Errorf("error parsing rule number %d in %s sg", index, *sg.Name) + return nil, fmt.Errorf("error parsing rule number %d in %s sg", index, *sg.Name) } -func translateSGRuleProtocolAll(rule *vpcv1.SecurityGroupRuleSecurityGroupRuleProtocolAll) (sgRule ir.SGRule, err error) { +func translateSGRuleProtocolAll(rule *vpcv1.SecurityGroupRuleSecurityGroupRuleProtocolAll) (sgRule *ir.SGRule, err error) { direction, err := translateDirection(*rule.Direction) if err != nil { - return ir.SGRule{}, err + return nil, err } remote, err := translateRemote(rule.Remote) if err != nil { - return ir.SGRule{}, err + return nil, err } local, err := translateLocal(rule.Local) if err != nil { - return ir.SGRule{}, err + return nil, err } - return ir.SGRule{Direction: direction, Remote: remote, Protocol: netp.AnyProtocol{}, Local: local}, nil + return &ir.SGRule{Direction: direction, Remote: remote, Protocol: netp.AnyProtocol{}, Local: local}, nil } -func translateSGRuleProtocolTCPUDP(rule *vpcv1.SecurityGroupRuleSecurityGroupRuleProtocolTcpudp) (sgRule ir.SGRule, err error) { +func translateSGRuleProtocolTCPUDP(rule *vpcv1.SecurityGroupRuleSecurityGroupRuleProtocolTcpudp) (sgRule *ir.SGRule, err error) { direction, err := translateDirection(*rule.Direction) if err != nil { - return ir.SGRule{}, err + return nil, err } remote, err := translateRemote(rule.Remote) if err != nil { - return ir.SGRule{}, err + return nil, err } local, err := translateLocal(rule.Local) if err != nil { - return ir.SGRule{}, err + return nil, err } protocol, err := translateProtocolTCPUDP(rule) if err != nil { - return ir.SGRule{}, err + return nil, err } - return ir.SGRule{Direction: direction, Remote: remote, Protocol: protocol, Local: local}, nil + return &ir.SGRule{Direction: direction, Remote: remote, Protocol: protocol, Local: local}, nil } -func translateSGRuleProtocolIcmp(rule *vpcv1.SecurityGroupRuleSecurityGroupRuleProtocolIcmp) (sgRule ir.SGRule, err error) { +func translateSGRuleProtocolIcmp(rule *vpcv1.SecurityGroupRuleSecurityGroupRuleProtocolIcmp) (sgRule *ir.SGRule, err error) { direction, err := translateDirection(*rule.Direction) if err != nil { - return ir.SGRule{}, err + return nil, err } remote, err := translateRemote(rule.Remote) if err != nil { - return ir.SGRule{}, err + return nil, err } local, err := translateLocal(rule.Local) if err != nil { - return ir.SGRule{}, err + return nil, err } protocol, err := netp.ICMPFromTypeAndCode64(rule.Type, rule.Code) if err != nil { - return ir.SGRule{}, err + return nil, err } - return ir.SGRule{Direction: direction, Remote: remote, Protocol: protocol, Local: local}, nil + return &ir.SGRule{Direction: direction, Remote: remote, Protocol: protocol, Local: local}, nil } func translateDirection(direction string) (ir.Direction, error) { @@ -169,7 +170,7 @@ func translateLocal(local vpcv1.SecurityGroupRuleLocalIntf) (*netset.IPBlock, er return nil, fmt.Errorf("error parsing Local field") } -// temporary - first version of optimization requires that local value will be 0.0.0.0/32 +// temporary - first version of optimization requires local = 0.0.0.0/32 func verifyLocalValue(ipAddrs *netset.IPBlock) (*netset.IPBlock, error) { if !ipAddrs.Equal(netset.GetCidrAll()) { return nil, fmt.Errorf("only 0.0.0.0/32 CIDR block is supported for local values") From bf9438fd772d6468b9deda511414c38d1717a19b Mon Sep 17 00:00:00 2001 From: Yair Slobodin Date: Wed, 25 Sep 2024 15:40:31 +0300 Subject: [PATCH 14/24] wip --- cmd/subcmds/optimizeOutput.go | 8 ++-- cmd/subcmds/synthOutput.go | 8 ++-- pkg/io/confio/acl.go | 2 +- pkg/io/confio/common.go | 2 +- pkg/io/confio/sg.go | 2 +- pkg/io/csvio/acl.go | 2 +- pkg/io/csvio/common.go | 2 +- pkg/io/csvio/sg.go | 2 +- pkg/io/mdio/acl.go | 2 +- pkg/io/mdio/common.go | 2 +- pkg/io/mdio/sg.go | 2 +- pkg/io/tfio/acl.go | 2 +- pkg/io/tfio/common.go | 2 +- pkg/io/tfio/sg.go | 70 +++++++++++++++-------------------- pkg/ir/acl.go | 16 ++------ pkg/ir/common.go | 19 +++------- pkg/ir/sg.go | 16 ++------ pkg/optimize/common.go | 2 +- pkg/optimize/sg.go | 2 +- pkg/synth/acl.go | 2 +- pkg/synth/common.go | 2 +- pkg/synth/sg.go | 2 +- 22 files changed, 67 insertions(+), 102 deletions(-) diff --git a/cmd/subcmds/optimizeOutput.go b/cmd/subcmds/optimizeOutput.go index 2b602f1c..eedf753e 100644 --- a/cmd/subcmds/optimizeOutput.go +++ b/cmd/subcmds/optimizeOutput.go @@ -14,7 +14,7 @@ import ( "github.com/np-guard/vpc-network-config-synthesis/pkg/ir" ) -func writeOptimizeOutput(args *inArgs, collection ir.OptimizeCollection, vpcNames []string) error { +func writeOptimizeOutput(args *inArgs, collection ir.Collection, vpcNames []string) error { if err := checkOutputFlags(args); err != nil { return err } @@ -29,19 +29,19 @@ func writeOptimizeOutput(args *inArgs, collection ir.OptimizeCollection, vpcName return writeToFile(args.outputFile, data) } -func writeOptimizeCollection(args *inArgs, collection ir.OptimizeCollection) (*bytes.Buffer, error) { +func writeOptimizeCollection(args *inArgs, collection ir.Collection) (*bytes.Buffer, error) { var data bytes.Buffer writer, err := pickOptimizeWriter(args, &data) if err != nil { return nil, err } - if err := collection.WriteOptimize(writer); err != nil { + if err := collection.Write(writer, ""); err != nil { return nil, err } return &data, nil } -func pickOptimizeWriter(args *inArgs, data *bytes.Buffer) (ir.OptimizeWriter, error) { +func pickOptimizeWriter(args *inArgs, data *bytes.Buffer) (ir.Writer, error) { w := bufio.NewWriter(data) switch args.outputFmt { case tfOutputFormat: diff --git a/cmd/subcmds/synthOutput.go b/cmd/subcmds/synthOutput.go index 12e0670b..ad70e1a8 100644 --- a/cmd/subcmds/synthOutput.go +++ b/cmd/subcmds/synthOutput.go @@ -21,7 +21,7 @@ import ( const defaultDirectoryPermission = 0o755 -func writeOutput(args *inArgs, collection ir.SynthCollection, vpcNames []ir.ID) error { +func writeOutput(args *inArgs, collection ir.Collection, vpcNames []ir.ID) error { if err := checkOutputFlags(args); err != nil { return err } @@ -61,19 +61,19 @@ func writeOutput(args *inArgs, collection ir.SynthCollection, vpcNames []ir.ID) return nil } -func writeSynthCollection(args *inArgs, collection ir.SynthCollection, vpc string) (*bytes.Buffer, error) { +func writeSynthCollection(args *inArgs, collection ir.Collection, vpc string) (*bytes.Buffer, error) { var data bytes.Buffer writer, err := pickSynthWriter(args, &data) if err != nil { return nil, err } - if err := collection.WriteSynth(writer, vpc); err != nil { + if err := collection.Write(writer, vpc); err != nil { return nil, err } return &data, nil } -func pickSynthWriter(args *inArgs, data *bytes.Buffer) (ir.SynthWriter, error) { +func pickSynthWriter(args *inArgs, data *bytes.Buffer) (ir.Writer, error) { w := bufio.NewWriter(data) switch args.outputFmt { case tfOutputFormat: diff --git a/pkg/io/confio/acl.go b/pkg/io/confio/acl.go index b2aa75d8..74ad494c 100644 --- a/pkg/io/confio/acl.go +++ b/pkg/io/confio/acl.go @@ -209,7 +209,7 @@ func subnetRef(subnet *configModel.Subnet) *vpcv1.SubnetReference { } } -func (w *Writer) WriteSynthACL(collection *ir.ACLCollection, _ string) error { +func (w *Writer) WriteACL(collection *ir.ACLCollection, _ string) error { if err := updateACLList(w.model, collection); err != nil { return err } diff --git a/pkg/io/confio/common.go b/pkg/io/confio/common.go index 97aa4274..e3e86f61 100644 --- a/pkg/io/confio/common.go +++ b/pkg/io/confio/common.go @@ -12,7 +12,7 @@ import ( configModel "github.com/np-guard/cloud-resource-collector/pkg/ibm/datamodel" ) -// Writer implements ir.SynthWriter +// Writer implements ir.Writer type Writer struct { w *bufio.Writer model *configModel.ResourcesContainerModel diff --git a/pkg/io/confio/sg.go b/pkg/io/confio/sg.go index 02a30ed7..e4b74be7 100644 --- a/pkg/io/confio/sg.go +++ b/pkg/io/confio/sg.go @@ -258,7 +258,7 @@ func updateSG(model *configModel.ResourcesContainerModel, collection *ir.SGColle return errors.Join(err1, err2) } -func (w *Writer) WriteSynthSG(collection *ir.SGCollection, _ string) error { +func (w *Writer) WriteSG(collection *ir.SGCollection, _ string) error { if err := updateSG(w.model, collection); err != nil { return err } diff --git a/pkg/io/csvio/acl.go b/pkg/io/csvio/acl.go index 0da377ef..0c1f62f4 100644 --- a/pkg/io/csvio/acl.go +++ b/pkg/io/csvio/acl.go @@ -18,7 +18,7 @@ import ( ) // Write prints an entire collection of acls as a single CSV table. -func (w *Writer) WriteSynthACL(collection *ir.ACLCollection, vpc string) error { +func (w *Writer) WriteACL(collection *ir.ACLCollection, vpc string) error { if err := w.w.WriteAll(aclHeader()); err != nil { return err } diff --git a/pkg/io/csvio/common.go b/pkg/io/csvio/common.go index c61e180e..41cd664d 100644 --- a/pkg/io/csvio/common.go +++ b/pkg/io/csvio/common.go @@ -18,7 +18,7 @@ import ( "github.com/np-guard/vpc-network-config-synthesis/pkg/ir" ) -// Writer implements ir.SynthWriter +// Writer implements ir.Writer type Writer struct { w *csv.Writer } diff --git a/pkg/io/csvio/sg.go b/pkg/io/csvio/sg.go index 67128c51..ed91a1ef 100644 --- a/pkg/io/csvio/sg.go +++ b/pkg/io/csvio/sg.go @@ -16,7 +16,7 @@ import ( "github.com/np-guard/vpc-network-config-synthesis/pkg/ir" ) -func (w *Writer) WriteSynthSG(collection *ir.SGCollection, vpc string) error { +func (w *Writer) WriteSG(collection *ir.SGCollection, vpc string) error { if err := w.w.WriteAll(sgHeader()); err != nil { return err } diff --git a/pkg/io/mdio/acl.go b/pkg/io/mdio/acl.go index 0f895366..c75a54a1 100644 --- a/pkg/io/mdio/acl.go +++ b/pkg/io/mdio/acl.go @@ -18,7 +18,7 @@ import ( ) // Write prints an entire collection of acls as a single MD table. -func (w *Writer) WriteSynthACL(collection *ir.ACLCollection, vpc string) error { +func (w *Writer) WriteACL(collection *ir.ACLCollection, vpc string) error { if err := w.writeAll(aclHeader()); err != nil { return err } diff --git a/pkg/io/mdio/common.go b/pkg/io/mdio/common.go index fad3d774..abfb24a8 100644 --- a/pkg/io/mdio/common.go +++ b/pkg/io/mdio/common.go @@ -20,7 +20,7 @@ import ( const leftAlign = " :--- " -// Writer implements ir.SynthWriter +// Writer implements ir.Writer type Writer struct { w *bufio.Writer } diff --git a/pkg/io/mdio/sg.go b/pkg/io/mdio/sg.go index 5cf4308d..3277b3fa 100644 --- a/pkg/io/mdio/sg.go +++ b/pkg/io/mdio/sg.go @@ -16,7 +16,7 @@ import ( "github.com/np-guard/vpc-network-config-synthesis/pkg/ir" ) -func (w *Writer) WriteSynthSG(collection *ir.SGCollection, vpc string) error { +func (w *Writer) WriteSG(collection *ir.SGCollection, vpc string) error { if err := w.writeAll(sgHeader()); err != nil { return err } diff --git a/pkg/io/tfio/acl.go b/pkg/io/tfio/acl.go index 95d77473..3575629a 100644 --- a/pkg/io/tfio/acl.go +++ b/pkg/io/tfio/acl.go @@ -18,7 +18,7 @@ import ( ) // WriteACL prints an entire collection of acls as a sequence of terraform resources. -func (w *Writer) WriteSynthACL(c *ir.ACLCollection, vpc string) error { +func (w *Writer) WriteACL(c *ir.ACLCollection, vpc string) error { collection, err := aclCollection(c, vpc) if err != nil { return err diff --git a/pkg/io/tfio/common.go b/pkg/io/tfio/common.go index 6fd5c8f3..96611c65 100644 --- a/pkg/io/tfio/common.go +++ b/pkg/io/tfio/common.go @@ -20,7 +20,7 @@ import ( "github.com/np-guard/vpc-network-config-synthesis/pkg/ir" ) -// Writer implements ir.SynthWriter and ir.OptimizeWriter +// Writer implements ir.Writer type Writer struct { w *bufio.Writer } diff --git a/pkg/io/tfio/sg.go b/pkg/io/tfio/sg.go index 05f8e017..747be97d 100644 --- a/pkg/io/tfio/sg.go +++ b/pkg/io/tfio/sg.go @@ -17,17 +17,9 @@ import ( "github.com/np-guard/vpc-network-config-synthesis/pkg/ir" ) -func (w *Writer) WriteSynthSG(c *ir.SGCollection, vpc string) error { - return w.writeSGCollection(c, vpc, true) -} - -func (w *Writer) WriteOptimizeSG(c *ir.SGCollection) error { - return w.writeSGCollection(c, "", false) -} - -// writeSGCollection prints an entire collection of Security Groups as a sequence of terraform resources. -func (w *Writer) writeSGCollection(c *ir.SGCollection, vpc string, writeComments bool) error { - collection, err := sgCollection(c, vpc, writeComments) +// WriteSG prints an entire collection of Security Groups as a sequence of terraform resources. +func (w *Writer) WriteSG(c *ir.SGCollection, vpc string) error { + collection, err := sgCollection(c, vpc) if err != nil { return err } @@ -36,7 +28,8 @@ func (w *Writer) writeSGCollection(c *ir.SGCollection, vpc string, writeComments if err != nil { return err } - return w.w.Flush() + err = w.w.Flush() + return err } func value(x interface{}) (string, error) { @@ -67,24 +60,22 @@ func sgProtocol(t netp.Protocol) []tf.Block { return nil } -func sgRule(rule *ir.SGRule, sgName ir.SGName, i int, writeComment bool) (tf.Block, error) { +func sgRule(rule *ir.SGRule, sgName ir.SGName, i int) (tf.Block, error) { ruleName := fmt.Sprintf("%s-%v", ir.ChangeScoping(sgName.String()), i) if err := verifyName(ruleName); err != nil { return tf.Block{}, err } - comment := "" - if writeComment { - comment = fmt.Sprintf("# %v", rule.Explanation) - } + group, err1 := value(sgName) remote, err2 := value(rule.Remote) if err := errors.Join(err1, err2); err != nil { return tf.Block{}, err } + return tf.Block{ Name: "resource", - Labels: []string{quote("ibm_is_security_group_rule"), quote(ruleName)}, - Comment: comment, + Labels: []string{quote("ibm_is_security_group_rule"), ir.ChangeScoping(quote(ruleName))}, + Comment: fmt.Sprintf("# %v", rule.Explanation), Arguments: []tf.Argument{ {Name: "group", Value: group}, {Name: "direction", Value: quote(direction(rule.Direction))}, @@ -112,27 +103,26 @@ func sg(sgName, comment string) (tf.Block, error) { }, nil } -func sgCollection(t *ir.SGCollection, vpc string, writeComments bool) (*tf.ConfigFile, error) { - resources := make([]tf.Block, 0) - // for _, sgName := range t.SortedSGNames(vpc) { - // comment := "\n" - // if writeComments { - // comment = fmt.Sprintf("\n### SG attached to %v", sgName) - // } - // sg, err := sg(sgName.String(), comment) - // if err != nil { - // return nil, err - // } - // resources = append(resources, sg) - // rules := sg.AllRules() - // for i, rule := range rules { - // rule, err := sgRule(rule, sgName, i) - // if err != nil { - // return nil, err - // } - // resources = append(resources, rule) - // } - // } +func sgCollection(t *ir.SGCollection, vpc string) (*tf.ConfigFile, error) { + var resources []tf.Block //nolint:prealloc // nontrivial to calculate, and an unlikely performance bottleneck + for _, sgName := range t.SortedSGNames(vpc) { + comment := "" + vpcName := ir.VpcFromScopedResource(string(sgName)) + rules := t.SGs[vpcName][sgName].AllRules() + comment = fmt.Sprintf("\n### SG attached to %v", sgName) + sg, err := sg(sgName.String(), comment) + if err != nil { + return nil, err + } + resources = append(resources, sg) + for i, rule := range rules { + rule, err := sgRule(rule, sgName, i) + if err != nil { + return nil, err + } + resources = append(resources, rule) + } + } return &tf.ConfigFile{ Resources: resources, }, nil diff --git a/pkg/ir/acl.go b/pkg/ir/acl.go index 81c621be..b8e23d6e 100644 --- a/pkg/ir/acl.go +++ b/pkg/ir/acl.go @@ -38,12 +38,8 @@ type ( ACLs map[ID]map[string]*ACL } - SynthACLWriter interface { - WriteSynthACL(aclColl *ACLCollection, vpc string) error - } - - OptimizeACLWriter interface { - WriteOptimizeACL(aclColl *ACLCollection) error + ACLWriter interface { + WriteACL(aclColl *ACLCollection, vpc string) error } ) @@ -133,12 +129,8 @@ func (c *ACLCollection) VpcNames() []string { return utils.SortedMapKeys(c.ACLs) } -func (c *ACLCollection) WriteSynth(w SynthWriter, vpc string) error { - return w.WriteSynthACL(c, vpc) -} - -func (c *ACLCollection) WriteOptimize(w OptimizeWriter) error { - return w.WriteOptimizeACL(c) +func (c *ACLCollection) Write(w Writer, vpc string) error { + return w.WriteACL(c, vpc) } func (c *ACLCollection) SortedACLSubnets(vpc string) []string { diff --git a/pkg/ir/common.go b/pkg/ir/common.go index 5f9fddd8..b747840f 100644 --- a/pkg/ir/common.go +++ b/pkg/ir/common.go @@ -8,22 +8,13 @@ package ir type ( Direction string - SynthCollection interface { - WriteSynth(writer SynthWriter, vpc string) error + Collection interface { + Write(writer Writer, vpc string) error } - OptimizeCollection interface { - WriteOptimize(writer OptimizeWriter) error - } - - SynthWriter interface { - SynthACLWriter - SynthSGWriter - } - - OptimizeWriter interface { - OptimizeACLWriter - OptimizeSGWriter + Writer interface { + ACLWriter + SGWriter } ) diff --git a/pkg/ir/sg.go b/pkg/ir/sg.go index 4ad83116..d394e36c 100644 --- a/pkg/ir/sg.go +++ b/pkg/ir/sg.go @@ -54,12 +54,8 @@ type ( SGs map[ID]map[SGName]*SG } - SynthSGWriter interface { - WriteSynthSG(sgColl *SGCollection, vpc string) error - } - - OptimizeSGWriter interface { - WriteOptimizeSG(sgColl *SGCollection) error + SGWriter interface { + WriteSG(sgColl *SGCollection, vpc string) error } ) @@ -122,12 +118,8 @@ func (c *SGCollection) VpcNames() []string { return utils.SortedMapKeys(c.SGs) } -func (c *SGCollection) WriteSynth(w SynthWriter, vpc string) error { - return w.WriteSynthSG(c, vpc) -} - -func (c *SGCollection) WriteOptimize(w OptimizeWriter) error { - return w.WriteOptimizeSG(c) +func (c *SGCollection) Write(w Writer, vpc string) error { + return w.WriteSG(c, vpc) } func (c *SGCollection) SortedSGNames(vpc ID) []SGName { diff --git a/pkg/optimize/common.go b/pkg/optimize/common.go index 96ed3b9d..b9ee7306 100644 --- a/pkg/optimize/common.go +++ b/pkg/optimize/common.go @@ -12,7 +12,7 @@ type Optimizer interface { ParseCollection(filename string) error // optimize number of SG/nACL rules - Optimize() (ir.OptimizeCollection, error) + Optimize() (ir.Collection, error) // returns a slice of all vpc names. used to generate locals file VpcNames() []string diff --git a/pkg/optimize/sg.go b/pkg/optimize/sg.go index 16828db5..d039544d 100644 --- a/pkg/optimize/sg.go +++ b/pkg/optimize/sg.go @@ -29,7 +29,7 @@ func (s *SGOptimizer) ParseCollection(filename string) error { return nil } -func (s *SGOptimizer) Optimize() (ir.OptimizeCollection, error) { +func (s *SGOptimizer) Optimize() (ir.Collection, error) { return s.sgCollection, nil } diff --git a/pkg/synth/acl.go b/pkg/synth/acl.go index 8807a865..2b3dff12 100644 --- a/pkg/synth/acl.go +++ b/pkg/synth/acl.go @@ -27,7 +27,7 @@ func NewACLSynthesizer(s *ir.Spec, single bool) Synthesizer { return &ACLSynthesizer{spec: s, singleACL: single, result: ir.NewACLCollection()} } -func (a *ACLSynthesizer) Synth() (ir.SynthCollection, error) { +func (a *ACLSynthesizer) Synth() (ir.Collection, error) { return a.makeACL() } diff --git a/pkg/synth/common.go b/pkg/synth/common.go index d29100ca..e6e516f3 100644 --- a/pkg/synth/common.go +++ b/pkg/synth/common.go @@ -15,7 +15,7 @@ import ( type ( Synthesizer interface { - Synth() (ir.SynthCollection, error) + Synth() (ir.Collection, error) } namedAddrs struct { diff --git a/pkg/synth/sg.go b/pkg/synth/sg.go index e9947d23..70c2433a 100644 --- a/pkg/synth/sg.go +++ b/pkg/synth/sg.go @@ -25,7 +25,7 @@ func NewSGSynthesizer(s *ir.Spec, _ bool) Synthesizer { return &SGSynthesizer{spec: s, result: ir.NewSGCollection()} } -func (s *SGSynthesizer) Synth() (ir.SynthCollection, error) { +func (s *SGSynthesizer) Synth() (ir.Collection, error) { return s.makeSG() } From e3d4668fa06a388f953dbe692ce23d5fdcb4ba12 Mon Sep 17 00:00:00 2001 From: Yair Slobodin Date: Wed, 25 Sep 2024 15:58:35 +0300 Subject: [PATCH 15/24] wip --- pkg/io/tfio/acl.go | 4 ---- pkg/ir/acl.go | 7 +++---- pkg/ir/sg.go | 7 +++---- 3 files changed, 6 insertions(+), 12 deletions(-) diff --git a/pkg/io/tfio/acl.go b/pkg/io/tfio/acl.go index 3575629a..b7f3df42 100644 --- a/pkg/io/tfio/acl.go +++ b/pkg/io/tfio/acl.go @@ -32,10 +32,6 @@ func (w *Writer) WriteACL(c *ir.ACLCollection, vpc string) error { return err } -func (w *Writer) WriteOptimizeACL(c *ir.ACLCollection) error { - return fmt.Errorf("OptimizeACL is not supported yet") -} - func aclProtocol(t netp.Protocol) []tf.Block { switch p := t.(type) { case netp.TCPUDP: diff --git a/pkg/ir/acl.go b/pkg/ir/acl.go index b8e23d6e..e2f60cf6 100644 --- a/pkg/ir/acl.go +++ b/pkg/ir/acl.go @@ -29,7 +29,6 @@ type ( ACL struct { Subnet string - VpcName string Internal []ACLRule External []ACLRule } @@ -108,8 +107,8 @@ func NewACLCollection() *ACLCollection { return &ACLCollection{ACLs: map[ID]map[string]*ACL{}} } -func NewACL(subnet, vpcName string) *ACL { - return &ACL{Subnet: subnet, VpcName: vpcName, Internal: []ACLRule{}, External: []ACLRule{}} +func NewACL(subnet string) *ACL { + return &ACL{Subnet: subnet, Internal: []ACLRule{}, External: []ACLRule{}} } func (c *ACLCollection) LookupOrCreate(subnet string) *ACL { @@ -117,7 +116,7 @@ func (c *ACLCollection) LookupOrCreate(subnet string) *ACL { if acl, ok := c.ACLs[vpcName][subnet]; ok { return acl } - newACL := NewACL(subnet, vpcName) + newACL := NewACL(subnet) if c.ACLs[vpcName] == nil { c.ACLs[vpcName] = make(map[string]*ACL) } diff --git a/pkg/ir/sg.go b/pkg/ir/sg.go index d394e36c..2317a4ec 100644 --- a/pkg/ir/sg.go +++ b/pkg/ir/sg.go @@ -44,7 +44,6 @@ type ( SG struct { SGName SGName - VpcName string InboundRules []*SGRule OutboundRules []*SGRule Attached []ID @@ -80,8 +79,8 @@ func (r *SGRule) mustSupersede(other *SGRule) bool { return res } -func NewSG(vpcName string, sgName SGName) *SG { - return &SG{SGName: sgName, VpcName: vpcName, InboundRules: []*SGRule{}, OutboundRules: []*SGRule{}, Attached: []ID{}} +func NewSG(sgName SGName) *SG { + return &SG{SGName: sgName, InboundRules: []*SGRule{}, OutboundRules: []*SGRule{}, Attached: []ID{}} } func NewSGCollection() *SGCollection { @@ -93,7 +92,7 @@ func (c *SGCollection) LookupOrCreate(name SGName) *SG { if sg, ok := c.SGs[vpcName][name]; ok { return sg } - newSG := NewSG(vpcName, name) + newSG := NewSG(name) if c.SGs[vpcName] == nil { c.SGs[vpcName] = make(map[SGName]*SG) } From ebb4b3b48278eda39866976f0d3166177ed663c8 Mon Sep 17 00:00:00 2001 From: Yair Slobodin Date: Wed, 25 Sep 2024 16:22:05 +0300 Subject: [PATCH 16/24] wip --- pkg/io/confio/parse_sgs.go | 2 +- pkg/io/tfio/acl.go | 8 +++++++- pkg/io/tfio/sg.go | 21 ++++++++++++++++++++- pkg/ir/acl.go | 7 ++++--- pkg/ir/sg.go | 7 ++++--- 5 files changed, 36 insertions(+), 9 deletions(-) diff --git a/pkg/io/confio/parse_sgs.go b/pkg/io/confio/parse_sgs.go index 250a7e6e..1fff790b 100644 --- a/pkg/io/confio/parse_sgs.go +++ b/pkg/io/confio/parse_sgs.go @@ -37,7 +37,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 } diff --git a/pkg/io/tfio/acl.go b/pkg/io/tfio/acl.go index b7f3df42..401aef06 100644 --- a/pkg/io/tfio/acl.go +++ b/pkg/io/tfio/acl.go @@ -64,8 +64,14 @@ func aclRule(rule *ir.ACLRule, name string) (tf.Block, error) { {Name: "source", Value: quote(rule.Source.String())}, {Name: "destination", Value: quote(rule.Destination.String())}, } + + comment := "" + if rule.Explanation != "" { + comment = fmt.Sprintf("# %v", rule.Explanation) + } + return tf.Block{Name: "rules", - Comment: fmt.Sprintf("# %v", rule.Explanation), + Comment: comment, Arguments: arguments, Blocks: aclProtocol(rule.Protocol), }, nil diff --git a/pkg/io/tfio/sg.go b/pkg/io/tfio/sg.go index 747be97d..c4dadcb7 100644 --- a/pkg/io/tfio/sg.go +++ b/pkg/io/tfio/sg.go @@ -72,10 +72,15 @@ func sgRule(rule *ir.SGRule, sgName ir.SGName, i int) (tf.Block, error) { return tf.Block{}, err } + comment := "" + if rule.Explanation != "" { + comment = fmt.Sprintf("# %v", rule.Explanation) + } + return tf.Block{ Name: "resource", Labels: []string{quote("ibm_is_security_group_rule"), ir.ChangeScoping(quote(ruleName))}, - Comment: fmt.Sprintf("# %v", rule.Explanation), + Comment: comment, Arguments: []tf.Argument{ {Name: "group", Value: group}, {Name: "direction", Value: quote(direction(rule.Direction))}, @@ -105,6 +110,20 @@ func sg(sgName, comment string) (tf.Block, error) { func sgCollection(t *ir.SGCollection, vpc string) (*tf.ConfigFile, error) { var resources []tf.Block //nolint:prealloc // nontrivial to calculate, and an unlikely performance bottleneck + + for _, vpcName := range t.VpcNames() { + if vpc != vpcName && vpc != "" { + continue + } + for _, sgName := range t.SortedSGNames(vpcName) { + rules := t.SGs[vpcName][sgName].AllRules() + comment := fmt.Sprintf("\n### SG attached to %s", sgName) + } + if vpc == "" { + break + } + } + for _, sgName := range t.SortedSGNames(vpc) { comment := "" vpcName := ir.VpcFromScopedResource(string(sgName)) diff --git a/pkg/ir/acl.go b/pkg/ir/acl.go index e2f60cf6..b8e23d6e 100644 --- a/pkg/ir/acl.go +++ b/pkg/ir/acl.go @@ -29,6 +29,7 @@ type ( ACL struct { Subnet string + VpcName string Internal []ACLRule External []ACLRule } @@ -107,8 +108,8 @@ func NewACLCollection() *ACLCollection { return &ACLCollection{ACLs: map[ID]map[string]*ACL{}} } -func NewACL(subnet string) *ACL { - return &ACL{Subnet: subnet, Internal: []ACLRule{}, External: []ACLRule{}} +func NewACL(subnet, vpcName string) *ACL { + return &ACL{Subnet: subnet, VpcName: vpcName, Internal: []ACLRule{}, External: []ACLRule{}} } func (c *ACLCollection) LookupOrCreate(subnet string) *ACL { @@ -116,7 +117,7 @@ func (c *ACLCollection) LookupOrCreate(subnet string) *ACL { if acl, ok := c.ACLs[vpcName][subnet]; ok { return acl } - newACL := NewACL(subnet) + newACL := NewACL(subnet, vpcName) if c.ACLs[vpcName] == nil { c.ACLs[vpcName] = make(map[string]*ACL) } diff --git a/pkg/ir/sg.go b/pkg/ir/sg.go index 2317a4ec..1245fcae 100644 --- a/pkg/ir/sg.go +++ b/pkg/ir/sg.go @@ -44,6 +44,7 @@ type ( SG struct { SGName SGName + VpcName string InboundRules []*SGRule OutboundRules []*SGRule Attached []ID @@ -79,8 +80,8 @@ func (r *SGRule) mustSupersede(other *SGRule) bool { return res } -func NewSG(sgName SGName) *SG { - return &SG{SGName: sgName, InboundRules: []*SGRule{}, OutboundRules: []*SGRule{}, Attached: []ID{}} +func NewSG(sgName SGName, vpcName string) *SG { + return &SG{SGName: sgName, VpcName: vpcName, InboundRules: []*SGRule{}, OutboundRules: []*SGRule{}, Attached: []ID{}} } func NewSGCollection() *SGCollection { @@ -92,7 +93,7 @@ func (c *SGCollection) LookupOrCreate(name SGName) *SG { if sg, ok := c.SGs[vpcName][name]; ok { return sg } - newSG := NewSG(name) + newSG := NewSG(name, vpcName) if c.SGs[vpcName] == nil { c.SGs[vpcName] = make(map[SGName]*SG) } From f34b9b7fd411ccdad95349ff39bc5039be9e5af1 Mon Sep 17 00:00:00 2001 From: Yair Slobodin Date: Wed, 25 Sep 2024 17:06:48 +0300 Subject: [PATCH 17/24] fixed --- pkg/io/tfio/sg.go | 46 +++++++++++++++++++--------------------------- 1 file changed, 19 insertions(+), 27 deletions(-) diff --git a/pkg/io/tfio/sg.go b/pkg/io/tfio/sg.go index c4dadcb7..88389dc0 100644 --- a/pkg/io/tfio/sg.go +++ b/pkg/io/tfio/sg.go @@ -90,18 +90,21 @@ func sgRule(rule *ir.SGRule, sgName ir.SGName, i int) (tf.Block, error) { }, nil } -func sg(sgName, comment string) (tf.Block, error) { - vpcName := ir.VpcFromScopedResource(sgName) - sgName = ir.ChangeScoping(sgName) - if err := verifyName(sgName); err != nil { +func sg(sgName, vpcName string) (tf.Block, error) { + tfSGName := ir.ChangeScoping(sgName) + comment := fmt.Sprintf("\n### SG attached to %s", sgName) + if sgName == tfSGName { // its optimization + comment = "" + } + if err := verifyName(tfSGName); err != nil { return tf.Block{}, err } return tf.Block{ Name: "resource", //nolint:revive // obvious false positive - Labels: []string{quote("ibm_is_security_group"), ir.ChangeScoping(quote(sgName))}, + Labels: []string{quote("ibm_is_security_group"), quote(tfSGName)}, Comment: comment, Arguments: []tf.Argument{ - {Name: "name", Value: quote("sg-" + sgName)}, + {Name: "name", Value: quote("sg-" + tfSGName)}, {Name: "resource_group", Value: "local.sg_synth_resource_group_id"}, {Name: "vpc", Value: fmt.Sprintf("local.sg_synth_%s_id", vpcName)}, }, @@ -109,7 +112,7 @@ func sg(sgName, comment string) (tf.Block, error) { } func sgCollection(t *ir.SGCollection, vpc string) (*tf.ConfigFile, error) { - var resources []tf.Block //nolint:prealloc // nontrivial to calculate, and an unlikely performance bottleneck + var resources []tf.Block for _, vpcName := range t.VpcNames() { if vpc != vpcName && vpc != "" { @@ -117,29 +120,18 @@ func sgCollection(t *ir.SGCollection, vpc string) (*tf.ConfigFile, error) { } for _, sgName := range t.SortedSGNames(vpcName) { rules := t.SGs[vpcName][sgName].AllRules() - comment := fmt.Sprintf("\n### SG attached to %s", sgName) - } - if vpc == "" { - break - } - } - - for _, sgName := range t.SortedSGNames(vpc) { - comment := "" - vpcName := ir.VpcFromScopedResource(string(sgName)) - rules := t.SGs[vpcName][sgName].AllRules() - comment = fmt.Sprintf("\n### SG attached to %v", sgName) - sg, err := sg(sgName.String(), comment) - if err != nil { - return nil, err - } - resources = append(resources, sg) - for i, rule := range rules { - rule, err := sgRule(rule, sgName, i) + sg, err := sg(sgName.String(), vpcName) if err != nil { return nil, err } - resources = append(resources, rule) + resources = append(resources, sg) + for i, rule := range rules { + rule, err := sgRule(rule, sgName, i) + if err != nil { + return nil, err + } + resources = append(resources, rule) + } } } return &tf.ConfigFile{ From ca64cee72f46f0ddf99d6d265265b9e729fb43ab Mon Sep 17 00:00:00 2001 From: Yair Slobodin Date: Mon, 30 Sep 2024 11:01:02 +0300 Subject: [PATCH 18/24] avoid code dup --- cmd/subcmds/optimize.go | 2 +- cmd/subcmds/optimizeOutput.go | 52 ------------------- cmd/subcmds/optimizeSG.go | 3 ++ cmd/subcmds/{synthOutput.go => output.go} | 56 +++++++++++++++----- cmd/subcmds/outputCommon.go | 63 ----------------------- cmd/subcmds/outputFormat.go | 6 +-- cmd/subcmds/synth.go | 2 +- cmd/subcmds/synthACL.go | 3 ++ cmd/subcmds/synthSG.go | 3 ++ cmd/subcmds/verifyFlags.go | 24 +++++++++ 10 files changed, 80 insertions(+), 134 deletions(-) delete mode 100644 cmd/subcmds/optimizeOutput.go rename cmd/subcmds/{synthOutput.go => output.go} (57%) delete mode 100644 cmd/subcmds/outputCommon.go create mode 100644 cmd/subcmds/verifyFlags.go diff --git a/cmd/subcmds/optimize.go b/cmd/subcmds/optimize.go index c3ea273c..483d3705 100644 --- a/cmd/subcmds/optimize.go +++ b/cmd/subcmds/optimize.go @@ -41,5 +41,5 @@ func optimization(cmd *cobra.Command, args *inArgs, newOptimizer optimize.Optimi if err != nil { return err } - return writeOptimizeOutput(args, collection, newOptimizer.VpcNames()) + return writeOutput(args, collection, newOptimizer.VpcNames(), false) } diff --git a/cmd/subcmds/optimizeOutput.go b/cmd/subcmds/optimizeOutput.go deleted file mode 100644 index eedf753e..00000000 --- a/cmd/subcmds/optimizeOutput.go +++ /dev/null @@ -1,52 +0,0 @@ -/* -Copyright 2023- IBM Inc. All Rights Reserved. -SPDX-License-Identifier: Apache-2.0 -*/ - -package subcmds - -import ( - "bufio" - "bytes" - "fmt" - - "github.com/np-guard/vpc-network-config-synthesis/pkg/io/tfio" - "github.com/np-guard/vpc-network-config-synthesis/pkg/ir" -) - -func writeOptimizeOutput(args *inArgs, collection ir.Collection, vpcNames []string) error { - if err := checkOutputFlags(args); err != nil { - return err - } - _, isACLCollection := collection.(*ir.ACLCollection) - if err := writeLocals(args, vpcNames, isACLCollection); err != nil { - return err - } - data, err := writeOptimizeCollection(args, collection) - if err != nil { - return err - } - return writeToFile(args.outputFile, data) -} - -func writeOptimizeCollection(args *inArgs, collection ir.Collection) (*bytes.Buffer, error) { - var data bytes.Buffer - writer, err := pickOptimizeWriter(args, &data) - if err != nil { - return nil, err - } - if err := collection.Write(writer, ""); err != nil { - return nil, err - } - return &data, nil -} - -func pickOptimizeWriter(args *inArgs, data *bytes.Buffer) (ir.Writer, error) { - w := bufio.NewWriter(data) - switch args.outputFmt { - case tfOutputFormat: - return tfio.NewWriter(w), nil - default: - return nil, fmt.Errorf("bad output format: %q", args.outputFmt) - } -} diff --git a/cmd/subcmds/optimizeSG.go b/cmd/subcmds/optimizeSG.go index 6a07c367..7311dd55 100644 --- a/cmd/subcmds/optimizeSG.go +++ b/cmd/subcmds/optimizeSG.go @@ -18,6 +18,9 @@ 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 { + if err := validateFlags(args); err != nil { + return err + } return optimization(cmd, args, optimize.NewSGOptimizer(args.firewallName)) }, } diff --git a/cmd/subcmds/synthOutput.go b/cmd/subcmds/output.go similarity index 57% rename from cmd/subcmds/synthOutput.go rename to cmd/subcmds/output.go index ad70e1a8..9def6e87 100644 --- a/cmd/subcmds/synthOutput.go +++ b/cmd/subcmds/output.go @@ -10,21 +10,19 @@ import ( "bytes" "fmt" "os" + "path/filepath" "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 := checkOutputFlags(args); err != nil { - return err - } +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 @@ -38,7 +36,7 @@ func writeOutput(args *inArgs, collection ir.Collection, vpcNames []ir.ID) error var data *bytes.Buffer var err error if args.outputDir == "" { - if data, err = writeSynthCollection(args, collection, ""); err != nil { + if data, err = writeCollection(args, collection, "", isSynth); err != nil { return err } return writeToFile(args.outputFile, data) @@ -51,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 = writeSynthCollection(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 { @@ -61,9 +59,9 @@ func writeOutput(args *inArgs, collection ir.Collection, vpcNames []ir.ID) error return nil } -func writeSynthCollection(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 := pickSynthWriter(args, &data) + writer, err := pickWriter(args, &data, isSynth) if err != nil { return nil, err } @@ -73,7 +71,7 @@ func writeSynthCollection(args *inArgs, collection ir.Collection, vpc string) (* return &data, nil } -func pickSynthWriter(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: @@ -82,9 +80,39 @@ func pickSynthWriter(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 writeToFile(outputFile string, data *bytes.Buffer) error { + if outputFile == "" { + fmt.Println(data.String()) + return nil + } + return os.WriteFile(outputFile, data.Bytes(), defaultFilePermission) +} + +func writeLocals(args *inArgs, vpcNames []ir.ID, isACL bool) error { + if !args.locals { + return nil + } + + var data *bytes.Buffer + var err error + if data, err = tfio.WriteLocals(vpcNames, isACL); err != nil { + return err + } + + outputFile := "" + suffix := "/locals.tf" + if args.outputDir != "" { + outputFile = args.outputDir + suffix + } else if args.outputFile != "" { + outputFile = filepath.Dir(args.outputFile) + suffix } + return writeToFile(outputFile, data) } diff --git a/cmd/subcmds/outputCommon.go b/cmd/subcmds/outputCommon.go deleted file mode 100644 index aa399315..00000000 --- a/cmd/subcmds/outputCommon.go +++ /dev/null @@ -1,63 +0,0 @@ -/* -Copyright 2023- IBM Inc. All Rights Reserved. -SPDX-License-Identifier: Apache-2.0 -*/ - -package subcmds - -import ( - "bytes" - "fmt" - "os" - "path/filepath" - - "github.com/np-guard/vpc-network-config-synthesis/pkg/io/tfio" - "github.com/np-guard/vpc-network-config-synthesis/pkg/ir" -) - -const defaultFilePermission = 0o644 - -func checkOutputFlags(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 == apiOutputFormat { - return fmt.Errorf("-d cannot be used with format json") - } - return 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 writeLocals(args *inArgs, vpcNames []ir.ID, isACL bool) error { - if !args.locals { - return nil - } - if args.outputFmt != tfOutputFormat { - return fmt.Errorf("--locals flag requires setting the output format to tf") - } - - var data *bytes.Buffer - var err error - if data, err = tfio.WriteLocals(vpcNames, isACL); err != nil { - return err - } - - outputFile := "" - suffix := "/locals.tf" - if args.outputDir != "" { - outputFile = args.outputDir + suffix - } else if args.outputFile != "" { - outputFile = filepath.Dir(args.outputFile) + suffix - } - return writeToFile(outputFile, data) -} diff --git a/cmd/subcmds/outputFormat.go b/cmd/subcmds/outputFormat.go index 51a36c60..f435c43a 100644 --- a/cmd/subcmds/outputFormat.go +++ b/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") } diff --git a/cmd/subcmds/synth.go b/cmd/subcmds/synth.go index fab5ee4b..91b8250b 100644 --- a/cmd/subcmds/synth.go +++ b/cmd/subcmds/synth.go @@ -48,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) } diff --git a/cmd/subcmds/synthACL.go b/cmd/subcmds/synthACL.go index 4aee4040..96c6aea3 100644 --- a/cmd/subcmds/synthACL.go +++ b/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) }, } diff --git a/cmd/subcmds/synthSG.go b/cmd/subcmds/synthSG.go index 88bb4a56..215324ca 100644 --- a/cmd/subcmds/synthSG.go +++ b/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) }, } diff --git a/cmd/subcmds/verifyFlags.go b/cmd/subcmds/verifyFlags.go new file mode 100644 index 00000000..b794732a --- /dev/null +++ b/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 +} From 49f4a474856d97a26d8e292c90b1ab56a0aa483c Mon Sep 17 00:00:00 2001 From: Yair Slobodin Date: Mon, 30 Sep 2024 11:05:10 +0300 Subject: [PATCH 19/24] fixed --- cmd/subcmds/optimize.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/cmd/subcmds/optimize.go b/cmd/subcmds/optimize.go index 483d3705..c346e15b 100644 --- a/cmd/subcmds/optimize.go +++ b/cmd/subcmds/optimize.go @@ -23,9 +23,6 @@ func NewOptimizeCommand(args *inArgs) *cobra.Command { // flags cmd.PersistentFlags().StringVarP(&args.firewallName, firewallNameFlag, "n", "", "which vpcFirewall to optimize") - // flags settings - _ = cmd.MarkPersistentFlagRequired(firewallNameFlag) // temporary - // sub cmds cmd.AddCommand(NewOptimizeSGCommand(args)) From 766f737583eb1974e907e66c9401e9a91666fb77 Mon Sep 17 00:00:00 2001 From: Yair Slobodin Date: Mon, 30 Sep 2024 11:14:52 +0300 Subject: [PATCH 20/24] fix output fmts --- pkg/io/csvio/sg.go | 18 +++++++++++------- pkg/io/mdio/sg.go | 18 +++++++++++------- pkg/io/tfio/sg.go | 8 ++++---- 3 files changed, 26 insertions(+), 18 deletions(-) diff --git a/pkg/io/csvio/sg.go b/pkg/io/csvio/sg.go index ed91a1ef..35c74f8b 100644 --- a/pkg/io/csvio/sg.go +++ b/pkg/io/csvio/sg.go @@ -20,14 +20,18 @@ func (w *Writer) WriteSG(collection *ir.SGCollection, vpc string) error { if err := w.w.WriteAll(sgHeader()); err != nil { return err } - for _, sgName := range collection.SortedSGNames(vpc) { - vpcName := ir.VpcFromScopedResource(string(sgName)) - sgTable, err := makeSGTable(collection.SGs[vpcName][sgName], sgName) - if err != nil { - return err + for _, vpcName := range collection.VpcNames() { + if vpc != vpcName && vpc != "" { + continue } - if err := w.w.WriteAll(sgTable); err != nil { - return err + for _, sgName := range collection.SortedSGNames(vpcName) { + sgTable, err := makeSGTable(collection.SGs[vpcName][sgName], sgName) + if err != nil { + return err + } + if err := w.w.WriteAll(sgTable); err != nil { + return err + } } } return nil diff --git a/pkg/io/mdio/sg.go b/pkg/io/mdio/sg.go index 3277b3fa..0fcbf734 100644 --- a/pkg/io/mdio/sg.go +++ b/pkg/io/mdio/sg.go @@ -20,14 +20,18 @@ func (w *Writer) WriteSG(collection *ir.SGCollection, vpc string) error { if err := w.writeAll(sgHeader()); err != nil { return err } - for _, sgName := range collection.SortedSGNames(vpc) { - vpcName := ir.VpcFromScopedResource(string(sgName)) - sgTable, err := makeSGTable(collection.SGs[vpcName][sgName], sgName) - if err != nil { - return err + for _, vpcName := range collection.VpcNames() { + if vpc != vpcName && vpc != "" { + continue } - if err := w.writeAll(sgTable); err != nil { - return err + for _, sgName := range collection.SortedSGNames(vpcName) { + sgTable, err := makeSGTable(collection.SGs[vpcName][sgName], sgName) + if err != nil { + return err + } + if err := w.writeAll(sgTable); err != nil { + return err + } } } return nil diff --git a/pkg/io/tfio/sg.go b/pkg/io/tfio/sg.go index 88389dc0..793c5c7a 100644 --- a/pkg/io/tfio/sg.go +++ b/pkg/io/tfio/sg.go @@ -111,15 +111,15 @@ func sg(sgName, vpcName string) (tf.Block, error) { }, nil } -func sgCollection(t *ir.SGCollection, vpc string) (*tf.ConfigFile, error) { +func sgCollection(collection *ir.SGCollection, vpc string) (*tf.ConfigFile, error) { var resources []tf.Block - for _, vpcName := range t.VpcNames() { + for _, vpcName := range collection.VpcNames() { if vpc != vpcName && vpc != "" { continue } - for _, sgName := range t.SortedSGNames(vpcName) { - rules := t.SGs[vpcName][sgName].AllRules() + for _, sgName := range collection.SortedSGNames(vpcName) { + rules := collection.SGs[vpcName][sgName].AllRules() sg, err := sg(sgName.String(), vpcName) if err != nil { return nil, err From cc374695a8cfafc3062a32eab957a845114b836a Mon Sep 17 00:00:00 2001 From: Yair Slobodin Date: Tue, 1 Oct 2024 19:48:05 +0300 Subject: [PATCH 21/24] comments wip --- cmd/subcmds/optimizeSG.go | 3 --- cmd/subcmds/root.go | 6 ++++++ cmd/subcmds/synthACL.go | 3 --- cmd/subcmds/synthSG.go | 3 --- cmd/subcmds/{verifyFlags.go => validateFlags.go} | 2 +- pkg/ir/acl.go | 4 ++-- pkg/optimize/sg.go | 3 +-- 7 files changed, 10 insertions(+), 14 deletions(-) rename cmd/subcmds/{verifyFlags.go => validateFlags.go} (89%) diff --git a/cmd/subcmds/optimizeSG.go b/cmd/subcmds/optimizeSG.go index 7311dd55..6a07c367 100644 --- a/cmd/subcmds/optimizeSG.go +++ b/cmd/subcmds/optimizeSG.go @@ -18,9 +18,6 @@ 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 { - if err := validateFlags(args); err != nil { - return err - } return optimization(cmd, args, optimize.NewSGOptimizer(args.firewallName)) }, } diff --git a/cmd/subcmds/root.go b/cmd/subcmds/root.go index a6e13486..0356d39d 100644 --- a/cmd/subcmds/root.go +++ b/cmd/subcmds/root.go @@ -39,10 +39,16 @@ type inArgs struct { 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.`, + PersistentPreRunE: func(cmd *cobra.Command, _ []string) error { + return validateFlags(args) + }, } // flags diff --git a/cmd/subcmds/synthACL.go b/cmd/subcmds/synthACL.go index 96c6aea3..4aee4040 100644 --- a/cmd/subcmds/synthACL.go +++ b/cmd/subcmds/synthACL.go @@ -19,9 +19,6 @@ 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) }, } diff --git a/cmd/subcmds/synthSG.go b/cmd/subcmds/synthSG.go index 215324ca..88bb4a56 100644 --- a/cmd/subcmds/synthSG.go +++ b/cmd/subcmds/synthSG.go @@ -19,9 +19,6 @@ 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) }, } diff --git a/cmd/subcmds/verifyFlags.go b/cmd/subcmds/validateFlags.go similarity index 89% rename from cmd/subcmds/verifyFlags.go rename to cmd/subcmds/validateFlags.go index b794732a..be56f6fd 100644 --- a/cmd/subcmds/verifyFlags.go +++ b/cmd/subcmds/validateFlags.go @@ -9,7 +9,7 @@ import "fmt" func validateFlags(args *inArgs) error { if args.outputDir != "" && args.outputFile != "" { - return fmt.Errorf("only one of -d and -o can be supplied") + return fmt.Errorf("specifying both -d and -o is not allowed") } if err := updateOutputFormat(args); err != nil { return err diff --git a/pkg/ir/acl.go b/pkg/ir/acl.go index fa85a1b4..7cc47f84 100644 --- a/pkg/ir/acl.go +++ b/pkg/ir/acl.go @@ -49,8 +49,8 @@ const ( ) func (r *ACLRule) isRedundant(rules []*ACLRule) bool { - for i := range rules { - if rules[i].mustSupersede(r) { + for _, rule := range rules { + if rule.mustSupersede(r) { return true } } diff --git a/pkg/optimize/sg.go b/pkg/optimize/sg.go index d039544d..78f4dc50 100644 --- a/pkg/optimize/sg.go +++ b/pkg/optimize/sg.go @@ -8,7 +8,6 @@ package optimize import ( "github.com/np-guard/vpc-network-config-synthesis/pkg/io/confio" "github.com/np-guard/vpc-network-config-synthesis/pkg/ir" - "github.com/np-guard/vpc-network-config-synthesis/pkg/utils" ) type SGOptimizer struct { @@ -34,5 +33,5 @@ func (s *SGOptimizer) Optimize() (ir.Collection, error) { } func (s *SGOptimizer) VpcNames() []string { - return utils.MapKeys(s.sgCollection.SGs) + return s.sgCollection.VpcNames() } From 7226233f95adc87b4bfac0c45535658057294b5d Mon Sep 17 00:00:00 2001 From: Yair Slobodin Date: Tue, 1 Oct 2024 21:22:54 +0300 Subject: [PATCH 22/24] fix review comments --- cmd/subcmds/optimize.go | 14 +++++++------- cmd/subcmds/optimizeSG.go | 8 +++++++- cmd/subcmds/root.go | 16 +++++++--------- cmd/subcmds/synth.go | 2 ++ cmd/subcmds/unmarshal.go | 7 +++++++ pkg/io/confio/parse_acls.go | 12 ++++++++++++ pkg/ir/common.go | 1 + pkg/optimize/common.go | 8 +------- pkg/optimize/sg.go | 20 +++----------------- pkg/synth/common.go | 1 + 10 files changed, 48 insertions(+), 41 deletions(-) create mode 100644 pkg/io/confio/parse_acls.go diff --git a/cmd/subcmds/optimize.go b/cmd/subcmds/optimize.go index c346e15b..353a86eb 100644 --- a/cmd/subcmds/optimize.go +++ b/cmd/subcmds/optimize.go @@ -10,6 +10,7 @@ import ( "github.com/spf13/cobra" + "github.com/np-guard/vpc-network-config-synthesis/pkg/ir" "github.com/np-guard/vpc-network-config-synthesis/pkg/optimize" ) @@ -20,23 +21,22 @@ 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 { +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 - if err := newOptimizer.ParseCollection(args.configFile); err != nil { + collection, err := parseCollection(args, isSG) + if err != nil { return fmt.Errorf("could not parse config file %v: %w", args.configFile, err) } - collection, err := newOptimizer.Optimize() + optimizer := newOptimizer(collection, args.firewallName) + optimizedCollection, err := optimizer.Optimize() if err != nil { return err } - return writeOutput(args, collection, newOptimizer.VpcNames(), false) + return writeOutput(args, optimizedCollection, collection.VpcNames(), false) } diff --git a/cmd/subcmds/optimizeSG.go b/cmd/subcmds/optimizeSG.go index 6a07c367..7b5fe908 100644 --- a/cmd/subcmds/optimizeSG.go +++ b/cmd/subcmds/optimizeSG.go @@ -11,6 +11,8 @@ import ( "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", @@ -18,8 +20,12 @@ 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, optimize.NewSGOptimizer(args.firewallName)) + return optimization(cmd, args, optimize.NewSGOptimizer, true) }, } + + // flags + cmd.PersistentFlags().StringVarP(&args.firewallName, sgNameFlag, "n", "", "which security group to optimize") + return cmd } diff --git a/cmd/subcmds/root.go b/cmd/subcmds/root.go index 0356d39d..7a55704d 100644 --- a/cmd/subcmds/root.go +++ b/cmd/subcmds/root.go @@ -13,15 +13,13 @@ import ( ) const ( - configFlag = "config" - specFlag = "spec" - outputFmtFlag = "format" - outputFileFlag = "output-file" - outputDirFlag = "output-dir" - prefixFlag = "prefix" - firewallNameFlag = "firewall-name" - singleACLFlag = "single" - localsFlag = "locals" + configFlag = "config" + outputFmtFlag = "format" + outputFileFlag = "output-file" + outputDirFlag = "output-dir" + prefixFlag = "prefix" + singleACLFlag = "single" + localsFlag = "locals" ) type inArgs struct { diff --git a/cmd/subcmds/synth.go b/cmd/subcmds/synth.go index 91b8250b..83eb348a 100644 --- a/cmd/subcmds/synth.go +++ b/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", diff --git a/cmd/subcmds/unmarshal.go b/cmd/subcmds/unmarshal.go index 5fa7c720..fcb1f76e 100644 --- a/cmd/subcmds/unmarshal.go +++ b/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) +} diff --git a/pkg/io/confio/parse_acls.go b/pkg/io/confio/parse_acls.go new file mode 100644 index 00000000..13481978 --- /dev/null +++ b/pkg/io/confio/parse_acls.go @@ -0,0 +1,12 @@ +/* +Copyright 2023- IBM Inc. All Rights Reserved. +SPDX-License-Identifier: Apache-2.0 +*/ + +package confio + +import "github.com/np-guard/vpc-network-config-synthesis/pkg/ir" + +func ReadACLs(_ string) (*ir.ACLCollection, error) { + return nil, nil +} diff --git a/pkg/ir/common.go b/pkg/ir/common.go index b747840f..a89aeae8 100644 --- a/pkg/ir/common.go +++ b/pkg/ir/common.go @@ -10,6 +10,7 @@ type ( Collection interface { Write(writer Writer, vpc string) error + VpcNames() []string } Writer interface { diff --git a/pkg/optimize/common.go b/pkg/optimize/common.go index b9ee7306..010c2239 100644 --- a/pkg/optimize/common.go +++ b/pkg/optimize/common.go @@ -8,12 +8,6 @@ package optimize import "github.com/np-guard/vpc-network-config-synthesis/pkg/ir" type Optimizer interface { - // read the collection from the config object file - ParseCollection(filename string) error - - // optimize number of SG/nACL rules + // attempts to reduce number of SG/nACL rules Optimize() (ir.Collection, error) - - // returns a slice of all vpc names. used to generate locals file - VpcNames() []string } diff --git a/pkg/optimize/sg.go b/pkg/optimize/sg.go index 78f4dc50..10aa1e42 100644 --- a/pkg/optimize/sg.go +++ b/pkg/optimize/sg.go @@ -6,32 +6,18 @@ SPDX-License-Identifier: Apache-2.0 package optimize import ( - "github.com/np-guard/vpc-network-config-synthesis/pkg/io/confio" "github.com/np-guard/vpc-network-config-synthesis/pkg/ir" ) type SGOptimizer struct { sgCollection *ir.SGCollection - sgName string + sgName ir.SGName } -func NewSGOptimizer(sgName string) Optimizer { - return &SGOptimizer{sgCollection: nil, sgName: sgName} -} - -func (s *SGOptimizer) ParseCollection(filename string) error { - c, err := confio.ReadSGs(filename) - if err != nil { - return err - } - s.sgCollection = c - return nil +func NewSGOptimizer(collection ir.Collection, sgName string) Optimizer { + return &SGOptimizer{sgCollection: collection.(*ir.SGCollection), sgName: ir.SGName(sgName)} } func (s *SGOptimizer) Optimize() (ir.Collection, error) { return s.sgCollection, nil } - -func (s *SGOptimizer) VpcNames() []string { - return s.sgCollection.VpcNames() -} diff --git a/pkg/synth/common.go b/pkg/synth/common.go index e6e516f3..a9b7b51f 100644 --- a/pkg/synth/common.go +++ b/pkg/synth/common.go @@ -15,6 +15,7 @@ import ( type ( Synthesizer interface { + // generates SGs/nACLs Synth() (ir.Collection, error) } From 7cb3792b251aae1336c0f3a31d18ff28e59ec198 Mon Sep 17 00:00:00 2001 From: Yair Slobodin Date: Wed, 2 Oct 2024 10:49:57 +0300 Subject: [PATCH 23/24] fix review comments --- cmd/subcmds/output.go | 12 +++++++----- cmd/subcmds/root.go | 8 +++++--- pkg/optimize/sg.go | 7 ++++++- 3 files changed, 18 insertions(+), 9 deletions(-) diff --git a/cmd/subcmds/output.go b/cmd/subcmds/output.go index 9def6e87..b3f71013 100644 --- a/cmd/subcmds/output.go +++ b/cmd/subcmds/output.go @@ -28,8 +28,8 @@ func writeOutput(args *inArgs, collection ir.Collection, vpcNames []string, isSy return err } } - _, isACLCollection := collection.(*ir.ACLCollection) - if err := writeLocals(args, vpcNames, isACLCollection); err != nil { + + if err := writeLocals(args, vpcNames, collection); err != nil { return err } @@ -96,14 +96,16 @@ func writeToFile(outputFile string, data *bytes.Buffer) error { return os.WriteFile(outputFile, data.Bytes(), defaultFilePermission) } -func writeLocals(args *inArgs, vpcNames []ir.ID, isACL bool) error { +func writeLocals(args *inArgs, vpcNames []ir.ID, collection ir.Collection) error { if !args.locals { return nil } - var data *bytes.Buffer var err error - if data, err = tfio.WriteLocals(vpcNames, isACL); err != nil { + + _, isACLCollection := collection.(*ir.ACLCollection) + + if data, err = tfio.WriteLocals(vpcNames, isACLCollection); err != nil { return err } diff --git a/cmd/subcmds/root.go b/cmd/subcmds/root.go index 7a55704d..98ff7973 100644 --- a/cmd/subcmds/root.go +++ b/cmd/subcmds/root.go @@ -42,8 +42,8 @@ func NewRootCommand() *cobra.Command { 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) }, @@ -65,8 +65,10 @@ func NewRootCommand() *cobra.Command { rootCmd.AddCommand(NewSynthCommand(args)) rootCmd.AddCommand(NewOptimizeCommand(args)) + // prevent Cobra from creating a default 'completion' command + rootCmd.CompletionOptions.DisableDefaultCmd = true + // disable help command. should use --help flag instead - rootCmd.CompletionOptions.HiddenDefaultCmd = true rootCmd.SetHelpCommand(&cobra.Command{Hidden: true}) return rootCmd diff --git a/pkg/optimize/sg.go b/pkg/optimize/sg.go index 10aa1e42..3288d2a7 100644 --- a/pkg/optimize/sg.go +++ b/pkg/optimize/sg.go @@ -12,10 +12,15 @@ import ( type SGOptimizer struct { sgCollection *ir.SGCollection sgName ir.SGName + sgVPC *string } func NewSGOptimizer(collection ir.Collection, sgName string) Optimizer { - return &SGOptimizer{sgCollection: collection.(*ir.SGCollection), sgName: ir.SGName(sgName)} + components := ir.ScopingComponents(sgName) + if len(components) == 1 { + return &SGOptimizer{sgCollection: collection.(*ir.SGCollection), sgName: ir.SGName(sgName), sgVPC: nil} + } + return &SGOptimizer{sgCollection: collection.(*ir.SGCollection), sgName: ir.SGName(components[1]), sgVPC: &components[0]} } func (s *SGOptimizer) Optimize() (ir.Collection, error) { From be1ee94fe3818f5987cfad6cb5346dd9132fcbda Mon Sep 17 00:00:00 2001 From: Yair Slobodin Date: Wed, 2 Oct 2024 11:17:00 +0300 Subject: [PATCH 24/24] revie comments --- pkg/io/confio/parse_sgs.go | 2 +- pkg/ir/acl.go | 7 +++---- pkg/ir/sg.go | 7 +++---- 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/pkg/io/confio/parse_sgs.go b/pkg/io/confio/parse_sgs.go index ad6e698a..91853903 100644 --- a/pkg/io/confio/parse_sgs.go +++ b/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{VpcName: vpcName, InboundRules: inbound, OutboundRules: outbound} + result.SGs[vpcName][ir.SGName(*sg.Name)] = &ir.SG{InboundRules: inbound, OutboundRules: outbound} } return result, nil } diff --git a/pkg/ir/acl.go b/pkg/ir/acl.go index 7cc47f84..e13b91bb 100644 --- a/pkg/ir/acl.go +++ b/pkg/ir/acl.go @@ -29,7 +29,6 @@ type ( ACL struct { Subnet string - VpcName string Internal []*ACLRule External []*ACLRule } @@ -108,8 +107,8 @@ func NewACLCollection() *ACLCollection { return &ACLCollection{ACLs: map[ID]map[string]*ACL{}} } -func NewACL(subnet, vpcName string) *ACL { - return &ACL{Subnet: subnet, VpcName: vpcName, Internal: []*ACLRule{}, External: []*ACLRule{}} +func NewACL(subnet string) *ACL { + return &ACL{Subnet: subnet, Internal: []*ACLRule{}, External: []*ACLRule{}} } func (c *ACLCollection) LookupOrCreate(subnet string) *ACL { @@ -117,7 +116,7 @@ func (c *ACLCollection) LookupOrCreate(subnet string) *ACL { if acl, ok := c.ACLs[vpcName][subnet]; ok { return acl } - newACL := NewACL(subnet, vpcName) + newACL := NewACL(subnet) if c.ACLs[vpcName] == nil { c.ACLs[vpcName] = make(map[string]*ACL) } diff --git a/pkg/ir/sg.go b/pkg/ir/sg.go index 1245fcae..2317a4ec 100644 --- a/pkg/ir/sg.go +++ b/pkg/ir/sg.go @@ -44,7 +44,6 @@ type ( SG struct { SGName SGName - VpcName string InboundRules []*SGRule OutboundRules []*SGRule Attached []ID @@ -80,8 +79,8 @@ func (r *SGRule) mustSupersede(other *SGRule) bool { return res } -func NewSG(sgName SGName, vpcName string) *SG { - return &SG{SGName: sgName, VpcName: vpcName, InboundRules: []*SGRule{}, OutboundRules: []*SGRule{}, Attached: []ID{}} +func NewSG(sgName SGName) *SG { + return &SG{SGName: sgName, InboundRules: []*SGRule{}, OutboundRules: []*SGRule{}, Attached: []ID{}} } func NewSGCollection() *SGCollection { @@ -93,7 +92,7 @@ func (c *SGCollection) LookupOrCreate(name SGName) *SG { if sg, ok := c.SGs[vpcName][name]; ok { return sg } - newSG := NewSG(name, vpcName) + newSG := NewSG(name) if c.SGs[vpcName] == nil { c.SGs[vpcName] = make(map[SGName]*SG) }