From 36a239e8be4970f7dcce2439a015d6297ff892a6 Mon Sep 17 00:00:00 2001 From: Ilya Dmitrichenko Date: Thu, 25 Oct 2018 14:32:26 +0100 Subject: [PATCH] Add `--vpc-cidr` flag - automate splitting of subnets - create private subnets as well as public --- Makefile | 6 +- pkg/cfn/builder/api_test.go | 72 +++++++++++++++---- pkg/cfn/builder/cluster.go | 17 ++++- pkg/cfn/builder/nodegroup.go | 2 +- pkg/cfn/builder/vpc.go | 9 +-- pkg/ctl/create/cluster.go | 8 ++- pkg/eks/api/api.go | 96 ++----------------------- pkg/eks/api/vpc.go | 75 +++++++++++++++++++ pkg/eks/vpc.go | 47 ++++++++++++ pkg/kops/kops.go | 4 +- pkg/utils/kubeconfig/kubeconfig_test.go | 2 +- 11 files changed, 221 insertions(+), 117 deletions(-) create mode 100644 pkg/eks/api/vpc.go create mode 100644 pkg/eks/vpc.go diff --git a/Makefile b/Makefile index 3fa9d48a054..1638889f106 100644 --- a/Makefile +++ b/Makefile @@ -24,9 +24,13 @@ build: ## Build eksctl test: generate ## Run unit tests @git diff --exit-code pkg/nodebootstrap/assets.go > /dev/null || (git --no-pager diff; exit 1) @git diff --exit-code ./pkg/eks/mocks > /dev/null || (git --no-pager diff; exit 1) - @CGO_ENABLED=0 go test -v -covermode=count -coverprofile=coverage.out ./pkg/... ./cmd/... + @$(MAKE) unit-test @test -z $(COVERALLS_TOKEN) || $(GOPATH)/bin/goveralls -coverprofile=coverage.out -service=circle-ci +.PHONY: unit-test +unit-test: + @CGO_ENABLED=0 go test -v -covermode=count -coverprofile=coverage.out ./pkg/... ./cmd/... + LINTER ?= gometalinter ./... .PHONY: lint lint: ## Run linter over the codebase diff --git a/pkg/cfn/builder/api_test.go b/pkg/cfn/builder/api_test.go index f43957e2377..d270efd0f4d 100644 --- a/pkg/cfn/builder/api_test.go +++ b/pkg/cfn/builder/api_test.go @@ -13,6 +13,7 @@ import ( . "github.com/onsi/gomega" . "github.com/weaveworks/eksctl/pkg/cfn/builder" "github.com/weaveworks/eksctl/pkg/cloudconfig" + "github.com/weaveworks/eksctl/pkg/eks" "github.com/weaveworks/eksctl/pkg/eks/api" "github.com/weaveworks/eksctl/pkg/nodebootstrap" ) @@ -100,6 +101,8 @@ var _ = Describe("CloudFormation template builder API", func() { cfg.AvailabilityZones = testAZs ng.InstanceType = "t2.medium" + *cfg.VPC.CIDR = api.DefaultCIDR() + return cfg } @@ -115,7 +118,7 @@ var _ = Describe("CloudFormation template builder API", func() { ARN: arn, AvailabilityZones: testAZs, - VPC: api.ClusterVPC{ + VPC: &api.ClusterVPC{ Network: api.Network{ ID: "vpc-0e265ad953062b94b", CIDR: &net.IPNet{ @@ -129,22 +132,45 @@ var _ = Describe("CloudFormation template builder API", func() { "us-west-2b": { ID: "subnet-0f98135715dfcf55f", CIDR: &net.IPNet{ - IP: []byte{192, 168, 64, 0}, - Mask: []byte{255, 255, 192, 0}, + IP: []byte{192, 168, 0, 0}, + Mask: []byte{255, 255, 224, 0}, }, }, "us-west-2a": { ID: "subnet-0ade11bad78dced9e", CIDR: &net.IPNet{ - IP: []byte{192, 168, 128, 0}, - Mask: []byte{255, 255, 192, 0}, + IP: []byte{192, 168, 32, 0}, + Mask: []byte{255, 255, 224, 0}, }, }, "us-west-2c": { ID: "subnet-0e2e63ff1712bf6ef", CIDR: &net.IPNet{ - IP: []byte{192, 168, 192, 0}, - Mask: []byte{255, 255, 192, 0}, + IP: []byte{192, 168, 64, 0}, + Mask: []byte{255, 255, 224, 0}, + }, + }, + }, + "Private": map[string]api.Network{ + "us-west-2b": { + ID: "subnet-0f98135715dfcf55a", + CIDR: &net.IPNet{ + IP: []byte{192, 168, 96, 0}, + Mask: []byte{255, 255, 224, 0}, + }, + }, + "us-west-2a": { + ID: "subnet-0ade11bad78dced9f", + CIDR: &net.IPNet{ + IP: []byte{192, 168, 128, 0}, + Mask: []byte{255, 255, 224, 0}, + }, + }, + "us-west-2c": { + ID: "subnet-0e2e63ff1712bf6ea", + CIDR: &net.IPNet{ + IP: []byte{192, 168, 160, 0}, + Mask: []byte{255, 255, 224, 0}, }, }, }, @@ -159,22 +185,40 @@ var _ = Describe("CloudFormation template builder API", func() { }, } - initial := newClusterConfig() + cfg := newClusterConfig() + ctl := eks.New(cfg) - initial.SetSubnets() + It("should not error when calling SetSubnets", func() { + err := ctl.SetSubnets() + Expect(err).ShouldNot(HaveOccurred()) + }) - rs := NewClusterResourceSet(initial) - rs.AddAllResources() + It("should have public and private subnets", func() { + Expect(len(cfg.VPC.Subnets)).To(Equal(2)) + for _, k := range []api.SubnetTopology{"Public", "Private"} { + Expect(cfg.VPC.Subnets).To(HaveKey(k)) + Expect(len(cfg.VPC.Subnets[k])).To(Equal(3)) + } + }) + + rs := NewClusterResourceSet(cfg) + It("should add all resources without error", func() { + err := rs.AddAllResources() + Expect(err).ShouldNot(HaveOccurred()) + }) - sampleStack := newStackWithOutputs(map[string]string{ + sampleOutputs := map[string]string{ "SecurityGroup": "sg-0b44c48bcba5b7362", "SubnetsPublic": "subnet-0f98135715dfcf55f,subnet-0ade11bad78dced9e,subnet-0e2e63ff1712bf6ef", + "SubnetsPrivate": "subnet-0f98135715dfcf55a,subnet-0ade11bad78dced9f,subnet-0e2e63ff1712bf6ea", "VPC": "vpc-0e265ad953062b94b", "Endpoint": endpoint, "CertificateAuthorityData": caCert, "ARN": arn, "ClusterStackName": "", - }) + } + + sampleStack := newStackWithOutputs(sampleOutputs) It("should not error", func() { err := rs.GetAllOutputs(sampleStack) @@ -182,7 +226,7 @@ var _ = Describe("CloudFormation template builder API", func() { }) It("should be equal", func() { - Expect(*initial).To(Equal(*expected)) + Expect(*cfg).To(Equal(*expected)) }) }) diff --git a/pkg/cfn/builder/cluster.go b/pkg/cfn/builder/cluster.go index 25f019f76b2..dc1555fc756 100644 --- a/pkg/cfn/builder/cluster.go +++ b/pkg/cfn/builder/cluster.go @@ -1,6 +1,8 @@ package builder import ( + "fmt" + cfn "github.com/aws/aws-sdk-go/service/cloudformation" gfn "github.com/awslabs/goformation/cloudformation" @@ -31,10 +33,19 @@ func (c *ClusterResourceSet) AddAllResources() error { templateDescriptionFeatures := clusterTemplateDescriptionDefaultFeatures - if c.spec.VPC.ID != "" && c.spec.VPC.HasSufficientPublicSubnets() { + if c.spec.VPC.ID != "" && c.spec.HasSufficientPublicSubnets() { c.importResourcesForVPC() templateDescriptionFeatures = " (with shared VPC and dedicated IAM role) " } else { + topologies := len(c.spec.VPC.Subnets) + switch { + case topologies < 1: + return fmt.Errorf("too few subnet topologies: %v", c.spec.VPC.Subnets) + case topologies == 1 && !c.spec.HasSufficientPublicSubnets(): + return fmt.Errorf("too few public subnets: %v", c.spec.VPC.Subnets["Public"]) + case topologies == 2 && !c.spec.HasSufficientPrivateSubnets(): + return fmt.Errorf("too few private subnets: %v", c.spec.VPC.Subnets["Private"]) + } c.addResourcesForVPC() } c.addOutputsForVPC() @@ -93,11 +104,11 @@ func (c *ClusterResourceSet) GetAllOutputs(stack cfn.Stack) error { // TODO: shouldn't assume the order is the same, can probably do an API lookup for i, subnet := range c.outputs.SubnetsPrivate { - c.spec.VPC.ImportSubnet(api.SubnetTopologyPrivate, c.spec.AvailabilityZones[i], subnet) + c.spec.ImportSubnet(api.SubnetTopologyPrivate, c.spec.AvailabilityZones[i], subnet) } for i, subnet := range c.outputs.SubnetsPublic { - c.spec.VPC.ImportSubnet(api.SubnetTopologyPublic, c.spec.AvailabilityZones[i], subnet) + c.spec.ImportSubnet(api.SubnetTopologyPublic, c.spec.AvailabilityZones[i], subnet) } c.spec.ClusterStackName = c.outputs.ClusterStackName diff --git a/pkg/cfn/builder/nodegroup.go b/pkg/cfn/builder/nodegroup.go index bfb261221aa..b1fc38552bd 100644 --- a/pkg/cfn/builder/nodegroup.go +++ b/pkg/cfn/builder/nodegroup.go @@ -111,7 +111,7 @@ func (n *NodeGroupResourceSet) addResourcesForNodeGroup() { // and tags don't have `PropagateAtLaunch` field, so we have a custom method here until this gets resolved var vpcZoneIdentifier interface{} if len(n.spec.AvailabilityZones) > 0 { - vpcZoneIdentifier = n.clusterSpec.VPC.SubnetIDs(api.SubnetTopologyPublic) + vpcZoneIdentifier = n.clusterSpec.SubnetIDs(api.SubnetTopologyPublic) } else { vpcZoneIdentifier = map[string][]interface{}{ gfn.FnSplit: []interface{}{ diff --git a/pkg/cfn/builder/vpc.go b/pkg/cfn/builder/vpc.go index 7c33e337648..d8f02c0381f 100644 --- a/pkg/cfn/builder/vpc.go +++ b/pkg/cfn/builder/vpc.go @@ -8,7 +8,6 @@ import ( ) func (c *ClusterResourceSet) addSubnets(refRT *gfn.Value, topology api.SubnetTopology) { - c.subnets = make(map[api.SubnetTopology][]*gfn.Value) for az, subnet := range c.spec.VPC.Subnets[topology] { alias := strings.ToUpper(strings.Join(strings.Split(az, "-"), "")) refSubnet := c.newResource("Subnet"+string(topology)+alias, &gfn.AWSEC2Subnet{ @@ -32,6 +31,8 @@ func (c *ClusterResourceSet) addResourcesForVPC() { EnableDnsHostnames: gfn.True(), }) + c.subnets = make(map[api.SubnetTopology][]*gfn.Value) + refIG := c.newResource("InternetGateway", &gfn.AWSEC2InternetGateway{}) c.newResource("VPCGatewayAttachment", &gfn.AWSEC2VPCGatewayAttachment{ InternetGatewayId: refIG, @@ -59,8 +60,8 @@ func (c *ClusterResourceSet) addResourcesForVPC() { func (c *ClusterResourceSet) importResourcesForVPC() { c.vpc = gfn.NewString(c.spec.VPC.ID) - for _, topology := range c.spec.VPC.SubnetTopologies() { - for _, subnet := range c.spec.VPC.SubnetIDs(topology) { + for topology := range c.spec.VPC.Subnets { + for _, subnet := range c.spec.SubnetIDs(topology) { c.subnets[topology] = append(c.subnets[topology], gfn.NewString(subnet)) } } @@ -68,7 +69,7 @@ func (c *ClusterResourceSet) importResourcesForVPC() { func (c *ClusterResourceSet) addOutputsForVPC() { c.rs.newOutput(cfnOutputClusterVPC, c.vpc, true) - for _, topology := range c.spec.VPC.SubnetTopologies() { + for topology := range c.spec.VPC.Subnets { c.rs.newJoinedOutput(cfnOutputClusterSubnets+string(topology), c.subnets[topology], true) } } diff --git a/pkg/ctl/create/cluster.go b/pkg/ctl/create/cluster.go index ce8833ebc64..b0a929c2532 100644 --- a/pkg/ctl/create/cluster.go +++ b/pkg/ctl/create/cluster.go @@ -90,6 +90,8 @@ func createClusterCmd() *cobra.Command { fs.StringVar(&kopsClusterNameForVPC, "vpc-from-kops-cluster", "", "re-use VPC from a given kops cluster") + fs.IPNetVar(cfg.VPC.CIDR, "vpc-cidr", api.DefaultCIDR(), "global CIDR to use for VPC") + return cmd } @@ -132,13 +134,15 @@ func doCreateCluster(cfg *api.ClusterConfig, ng *api.NodeGroup, name string) err if err := kw.UseVPC(cfg); err != nil { return err } - logger.Success("using VPC (%s) and subnets (%v) from kops cluster %q", cfg.VPC.ID, cfg.VPC.SubnetIDs(api.SubnetTopologyPublic), kopsClusterNameForVPC) + logger.Success("using VPC (%s) and subnets (%v) from kops cluster %q", cfg.VPC.ID, cfg.SubnetIDs(api.SubnetTopologyPublic), kopsClusterNameForVPC) } else { // kw.UseVPC() sets AZs based on subenets used if err := ctl.SetAvailabilityZones(availabilityZones); err != nil { return err } - cfg.SetSubnets() + if err := ctl.SetSubnets(); err != nil { + return err + } } if err := ctl.EnsureAMI(ng); err != nil { diff --git a/pkg/eks/api/api.go b/pkg/eks/api/api.go index 484f5e0c628..f51278cde76 100644 --- a/pkg/eks/api/api.go +++ b/pkg/eks/api/api.go @@ -38,7 +38,7 @@ var SupportedRegions = []string{ var DefaultWaitTimeout = 20 * time.Minute // DefaultNodeCount defines the default number of nodes to be created -var DefaultNodeCount = 2 +const DefaultNodeCount = 2 // ClusterProvider provides an interface with the needed AWS APIs type ClusterProvider interface { @@ -57,7 +57,7 @@ type ClusterConfig struct { WaitTimeout time.Duration - VPC ClusterVPC + VPC *ClusterVPC NodeGroups []*NodeGroup @@ -76,29 +76,13 @@ type ClusterConfig struct { // it doesn't include initial nodegroup, so user must // call NewNodeGroup to create one func NewClusterConfig() *ClusterConfig { - return &ClusterConfig{} -} - -// SetSubnets defines CIDRs for each of the subnets, -// it must be called after SetAvailabilityZones -func (c *ClusterConfig) SetSubnets() { - _, c.VPC.CIDR, _ = net.ParseCIDR("192.168.0.0/16") - - c.VPC.Subnets = map[SubnetTopology]map[string]Network{ - SubnetTopologyPublic: map[string]Network{}, + cfg := &ClusterConfig{ + VPC: &ClusterVPC{}, } - zoneCIDRs := []string{ - "192.168.64.0/18", - "192.168.128.0/18", - "192.168.192.0/18", - } - for i, zone := range c.AvailabilityZones { - _, zoneCIDR, _ := net.ParseCIDR(zoneCIDRs[i]) - c.VPC.Subnets[SubnetTopologyPublic][zone] = Network{ - CIDR: zoneCIDR, - } - } + cfg.VPC.CIDR = &net.IPNet{} + + return cfg } // NewNodeGroup crears new nodegroup inside cluster config, @@ -142,72 +126,6 @@ type NodeGroup struct { SSHPublicKeyName string } -type ( - // ClusterVPC holds global subnet and all child public/private subnet - ClusterVPC struct { - Network // global CIRD and VPC ID - SecurityGroup string // cluster SG - // subnets are either public or private for use with separate nodegroups - // these are keyed by AZ for conveninece - Subnets map[SubnetTopology]map[string]Network - // for additional CIRD associations, e.g. to use with separate CIDR for - // private subnets or any ad-hoc subnets - ExtraCIDRs []*net.IPNet - } - // SubnetTopology can be SubnetTopologyPrivate or SubnetTopologyPublic - SubnetTopology string - // Network holds ID and CIDR - Network struct { - ID string - CIDR *net.IPNet - } -) - -const ( - // SubnetTopologyPrivate repesents privately-routed subnets - SubnetTopologyPrivate SubnetTopology = "Private" - // SubnetTopologyPublic repesents publicly-routed subnets - SubnetTopologyPublic SubnetTopology = "Public" -) - -// SubnetIDs returns list of subnets -func (c ClusterVPC) SubnetIDs(topology SubnetTopology) []string { - subnets := []string{} - for _, s := range c.Subnets[topology] { - subnets = append(subnets, s.ID) - } - return subnets -} - -// SubnetTopologies returns list of topologies supported -// by a given cluster config -func (c ClusterVPC) SubnetTopologies() []SubnetTopology { - topologies := []SubnetTopology{} - for topology := range c.Subnets { - topologies = append(topologies, topology) - } - return topologies -} - -// ImportSubnet loads a given subnet into cluster config -func (c ClusterVPC) ImportSubnet(topology SubnetTopology, az, subnetID string) { - if _, ok := c.Subnets[topology]; !ok { - c.Subnets[topology] = map[string]Network{} - } - if network, ok := c.Subnets[topology][az]; !ok { - c.Subnets[topology][az] = Network{ID: subnetID} - } else { - network.ID = subnetID - c.Subnets[topology][az] = network - } -} - -// HasSufficientPublicSubnets validates if there is a suffiecent -// number of subnets available to create a cluster -func (c ClusterVPC) HasSufficientPublicSubnets() bool { - return len(c.SubnetIDs(SubnetTopologyPublic)) >= 3 -} - type ( // ClusterAddons provides addons for the created EKS cluster ClusterAddons struct { diff --git a/pkg/eks/api/vpc.go b/pkg/eks/api/vpc.go new file mode 100644 index 00000000000..21ce8dccc98 --- /dev/null +++ b/pkg/eks/api/vpc.go @@ -0,0 +1,75 @@ +package api + +import ( + "net" +) + +type ( + // ClusterVPC holds global subnet and all child public/private subnet + ClusterVPC struct { + Network // global CIRD and VPC ID + SecurityGroup string // cluster SG + // subnets are either public or private for use with separate nodegroups + // these are keyed by AZ for conveninece + Subnets map[SubnetTopology]map[string]Network + // for additional CIRD associations, e.g. to use with separate CIDR for + // private subnets or any ad-hoc subnets + ExtraCIDRs []*net.IPNet + } + // SubnetTopology can be SubnetTopologyPrivate or SubnetTopologyPublic + SubnetTopology string + // Network holds ID and CIDR + Network struct { + ID string + CIDR *net.IPNet + } +) + +const ( + // SubnetTopologyPrivate repesents privately-routed subnets + SubnetTopologyPrivate SubnetTopology = "Private" + // SubnetTopologyPublic repesents publicly-routed subnets + SubnetTopologyPublic SubnetTopology = "Public" +) + +// DefaultCIDR returns default global CIDR for VPC +func DefaultCIDR() net.IPNet { + return net.IPNet{ + IP: []byte{192, 168, 0, 0}, + Mask: []byte{255, 255, 0, 0}, + } +} + +// SubnetIDs returns list of subnets +func (c *ClusterConfig) SubnetIDs(topology SubnetTopology) []string { + subnets := []string{} + for _, s := range c.VPC.Subnets[topology] { + subnets = append(subnets, s.ID) + } + return subnets +} + +// ImportSubnet loads a given subnet into cluster config +func (c *ClusterConfig) ImportSubnet(topology SubnetTopology, az, subnetID string) { + if _, ok := c.VPC.Subnets[topology]; !ok { + c.VPC.Subnets[topology] = map[string]Network{} + } + if network, ok := c.VPC.Subnets[topology][az]; !ok { + c.VPC.Subnets[topology][az] = Network{ID: subnetID} + } else { + network.ID = subnetID + c.VPC.Subnets[topology][az] = network + } +} + +// HasSufficientPublicSubnets validates if there is a suffiecent +// number of subnets available to create a cluster +func (c *ClusterConfig) HasSufficientPublicSubnets() bool { + return len(c.SubnetIDs(SubnetTopologyPublic)) >= 3 +} + +// HasSufficientPrivateSubnets validates if there is a suffiecent +// number of subnets available to create a cluster +func (c *ClusterConfig) HasSufficientPrivateSubnets() bool { + return len(c.SubnetIDs(SubnetTopologyPrivate)) >= 3 +} diff --git a/pkg/eks/vpc.go b/pkg/eks/vpc.go new file mode 100644 index 00000000000..ee88ab4d69c --- /dev/null +++ b/pkg/eks/vpc.go @@ -0,0 +1,47 @@ +package eks + +import ( + "fmt" + + "github.com/kubicorn/kubicorn/pkg/logger" + "github.com/weaveworks/eksctl/pkg/eks/api" + "k8s.io/kops/pkg/util/subnet" +) + +// SetSubnets defines CIDRs for each of the subnets, +// it must be called after SetAvailabilityZones +func (c *ClusterProvider) SetSubnets() error { + var err error + + vpc := c.Spec.VPC + vpc.Subnets = map[api.SubnetTopology]map[string]api.Network{ + api.SubnetTopologyPublic: map[string]api.Network{}, + api.SubnetTopologyPrivate: map[string]api.Network{}, + } + + zoneCIDRs, err := subnet.SplitInto8(c.Spec.VPC.CIDR) + if err != nil { + return err + } + + logger.Debug("VPC CIDR (%s) was divided into 8 subnets %v", vpc.CIDR.String(), zoneCIDRs) + + zonesTotal := len(c.Spec.AvailabilityZones) + if 2*zonesTotal > len(zoneCIDRs) { + return fmt.Errorf("insuffience number of subnets (have %d, but need %d) for %d availability zones", len(zoneCIDRs), 2*zonesTotal, zonesTotal) + } + + for i, zone := range c.Spec.AvailabilityZones { + public := zoneCIDRs[i] + private := zoneCIDRs[i+zonesTotal] + vpc.Subnets[api.SubnetTopologyPublic][zone] = api.Network{ + CIDR: public, + } + vpc.Subnets[api.SubnetTopologyPrivate][zone] = api.Network{ + CIDR: private, + } + logger.Info("subnets for %s - public:%s private:%s", zone, public.String(), private.String()) + } + + return nil +} diff --git a/pkg/kops/kops.go b/pkg/kops/kops.go index 247103d0707..002f5d35568 100644 --- a/pkg/kops/kops.go +++ b/pkg/kops/kops.go @@ -61,13 +61,13 @@ func (k *Wrapper) UseVPC(spec *api.ClusterConfig) error { subnet := subnet.Obj.(*ec2.Subnet) for _, tag := range subnet.Tags { if k.isOwned(tag) && *subnet.VpcId == vpcs[0] { - spec.VPC.ImportSubnet(api.SubnetTopologyPublic, *subnet.AvailabilityZone, *subnet.SubnetId) + spec.ImportSubnet(api.SubnetTopologyPublic, *subnet.AvailabilityZone, *subnet.SubnetId) spec.AvailabilityZones = append(spec.AvailabilityZones, *subnet.AvailabilityZone) } } } logger.Debug("subnets = %#v", spec.VPC.Subnets) - if !spec.VPC.HasSufficientPublicSubnets() { + if !spec.HasSufficientPublicSubnets() { return fmt.Errorf("cannot use VPC from kops cluster with less then 3 subnets") } diff --git a/pkg/utils/kubeconfig/kubeconfig_test.go b/pkg/utils/kubeconfig/kubeconfig_test.go index d77f981e530..bce1281a5e6 100644 --- a/pkg/utils/kubeconfig/kubeconfig_test.go +++ b/pkg/utils/kubeconfig/kubeconfig_test.go @@ -153,7 +153,7 @@ var _ = Describe("Kubeconfig", func() { InstanceRoleARN: "", }, }, - VPC: eksctlapi.ClusterVPC{ + VPC: &eksctlapi.ClusterVPC{ Network: eksctlapi.Network{ ID: "", CIDR: nil,