Merge pull request #9996 from rifelpet/additional-network-cidr

Fix support for multiple additionalNetworkCIDR blocks

pkg/model/network.go

@@ -73,6 +73,7 @@ func (b *NetworkModelBuilder) Build(c *fi.ModelBuilderContext) error {
 			// but seems safer to stick with existing behaviour
 
 			t.EnableDNSHostnames = fi.Bool(true)
+			t.AssociateExtraCIDRBlocks = b.Cluster.Spec.AdditionalNetworkCIDRs
 		}
 
 		if b.Cluster.Spec.NetworkID != "" {
@@ -93,7 +94,7 @@ func (b *NetworkModelBuilder) Build(c *fi.ModelBuilderContext) error {
 				Lifecycle: b.Lifecycle,
 				VPC:       b.LinkToVPC(),
 				Shared:    fi.Bool(sharedVPC),
-				CIDRBlock: &cidr,
+				CIDRBlock: s(cidr),
 			})
 		}
 	}

tests/integration/update_cluster/complex/cloudformation.json

@@ -934,6 +934,15 @@
         "CidrBlock": "10.1.0.0/16"
       }
     },
+    "AWSEC2VPCCidrBlock1020016": {
+      "Type": "AWS::EC2::VPCCidrBlock",
+      "Properties": {
+        "VpcId": {
+          "Ref": "AWSEC2VPCcomplexexamplecom"
+        },
+        "CidrBlock": "10.2.0.0/16"
+      }
+    },
     "AWSEC2VPCDHCPOptionsAssociationcomplexexamplecom": {
       "Type": "AWS::EC2::VPCDHCPOptionsAssociation",
       "Properties": {

tests/integration/update_cluster/complex/in-legacy-v1alpha2.yaml

@@ -42,6 +42,7 @@ spec:
   networkCIDR: 172.20.0.0/16
   additionalNetworkCIDRs:
   - 10.1.0.0/16
+  - 10.2.0.0/16
   networking:
     kubenet: {}
   nodePortAccess:

tests/integration/update_cluster/complex/in-v1alpha2.yaml

@@ -42,6 +42,7 @@ spec:
   networkCIDR: 172.20.0.0/16
   additionalNetworkCIDRs:
   - 10.1.0.0/16
+  - 10.2.0.0/16
   networking:
     kubenet: {}
   nodePortAccess:

tests/integration/update_cluster/complex/kubernetes.tf

@@ -736,6 +736,11 @@ resource "aws_vpc_ipv4_cidr_block_association" "cidr-10-1-0-0--16" {
   vpc_id     = aws_vpc.complex-example-com.id
 }
 
+resource "aws_vpc_ipv4_cidr_block_association" "cidr-10-2-0-0--16" {
+  cidr_block = "10.2.0.0/16"
+  vpc_id     = aws_vpc.complex-example-com.id
+}
+
 resource "aws_vpc" "complex-example-com" {
   cidr_block           = "172.20.0.0/16"
   enable_dns_hostnames = true

upup/pkg/fi/cloudup/awstasks/vpc.go

@@ -44,9 +44,15 @@ type VPC struct {
 	Shared *bool
 
 	Tags map[string]string
+
+	// AssociateExtraCIDRBlocks contains a list of cidr blocks that should be
+	// associated with the VPC; any other CIDR blocks should be disassociated.
+	// The associations themselves are created through the VPCCIDRBlock awstask.
+	AssociateExtraCIDRBlocks []string
 }
 
 var _ fi.CompareWithID = &VPC{}
+var _ fi.ProducesDeletions = &VPC{}
 
 func (e *VPC) CompareWithID() *string {
 	return e.ID
@@ -109,6 +115,7 @@ func (e *VPC) Find(c *fi.Context) (*VPC, error) {
 	}
 	actual.Lifecycle = e.Lifecycle
 	actual.Name = e.Name // Name is part of Tags
+	actual.AssociateExtraCIDRBlocks = e.AssociateExtraCIDRBlocks
 
 	return actual, nil
 }
@@ -194,6 +201,53 @@ func (_ *VPC) RenderAWS(t *awsup.AWSAPITarget, a, e, changes *VPC) error {
 	return t.AddAWSTags(*e.ID, e.Tags)
 }
 
+func (e *VPC) FindDeletions(c *fi.Context) ([]fi.Deletion, error) {
+	if fi.StringValue(e.ID) == "" {
+		return nil, nil
+	}
+
+	var removals []fi.Deletion
+	request := &ec2.DescribeVpcsInput{
+		VpcIds: []*string{e.ID},
+	}
+	cloud := c.Cloud.(awsup.AWSCloud)
+	response, err := cloud.EC2().DescribeVpcs(request)
+	if err != nil {
+		return nil, err
+	}
+	if response == nil || len(response.Vpcs) == 0 {
+		return nil, nil
+	}
+
+	if len(response.Vpcs) != 1 {
+		return nil, fmt.Errorf("found multiple VPCs matching tags")
+	}
+	vpc := response.Vpcs[0]
+	for _, association := range vpc.CidrBlockAssociationSet {
+		// We'll only delete CIDR associations that are not the primary association
+		// and that have a state of "associated"
+		if fi.StringValue(association.CidrBlock) == fi.StringValue(vpc.CidrBlock) ||
+			association.CidrBlockState != nil && fi.StringValue(association.CidrBlockState.State) != ec2.VpcCidrBlockStateCodeAssociated {
+			continue
+		}
+		match := false
+		for _, cidr := range e.AssociateExtraCIDRBlocks {
+			if fi.StringValue(association.CidrBlock) == cidr {
+				match = true
+				break
+			}
+		}
+		if !match {
+			removals = append(removals, &deleteVPCCIDRBlock{
+				vpcID:         vpc.VpcId,
+				cidrBlock:     association.CidrBlock,
+				associationID: association.AssociationId,
+			})
+		}
+	}
+	return removals, nil
+}
+
 type terraformVPC struct {
 	CIDR               *string           `json:"cidr_block,omitempty" cty:"cidr_block"`
 	EnableDNSHostnames *bool             `json:"enable_dns_hostnames,omitempty" cty:"enable_dns_hostnames"`
@@ -280,3 +334,32 @@ func (e *VPC) CloudformationLink() *cloudformation.Literal {
 
 	return cloudformation.Ref("AWS::EC2::VPC", *e.Name)
 }
+
+type deleteVPCCIDRBlock struct {
+	vpcID         *string
+	cidrBlock     *string
+	associationID *string
+}
+
+var _ fi.Deletion = &deleteVPCCIDRBlock{}
+
+func (d *deleteVPCCIDRBlock) Delete(t fi.Target) error {
+
+	awsTarget, ok := t.(*awsup.AWSAPITarget)
+	if !ok {
+		return fmt.Errorf("unexpected target type for deletion: %T", t)
+	}
+	request := &ec2.DisassociateVpcCidrBlockInput{
+		AssociationId: d.associationID,
+	}
+	_, err := awsTarget.Cloud.EC2().DisassociateVpcCidrBlock(request)
+	return err
+}
+
+func (d *deleteVPCCIDRBlock) TaskName() string {
+	return "VPCCIDRBlock"
+}
+
+func (d *deleteVPCCIDRBlock) Item() string {
+	return fmt.Sprintf("%v: cidr=%v", *d.vpcID, *d.cidrBlock)
+}

upup/pkg/fi/cloudup/awstasks/vpccidrblock.go

@@ -48,10 +48,22 @@ func (e *VPCCIDRBlock) Find(c *fi.Context) (*VPCCIDRBlock, error) {
 		return nil, err
 	}
 
+	found := false
+	for _, cba := range vpc.CidrBlockAssociationSet {
+		if fi.StringValue(cba.CidrBlock) == fi.StringValue(e.CIDRBlock) &&
+			cba.CidrBlockState != nil && fi.StringValue(cba.CidrBlockState.State) == ec2.VpcCidrBlockStateCodeAssociated {
+			found = true
+			break
+		}
+	}
+	if !found {
+		return nil, nil
+	}
+
 	actual := &VPCCIDRBlock{
 		CIDRBlock: e.CIDRBlock,
+		VPC:       &VPC{ID: vpc.VpcId},
 	}
-	actual.VPC = &VPC{ID: vpc.VpcId}
 
 	// Prevent spurious changes
 	actual.Shared = e.Shared

upup/pkg/fi/cloudup/awsup/aws_cloud.go

@@ -1470,7 +1470,7 @@ func ValidateZones(zones []string, cloud AWSCloud) error {
 			klog.Warningf("Zone %q has message: %q", zone, aws.StringValue(message.Message))
 		}
 
-		if aws.StringValue(z.State) != "available" {
+		if aws.StringValue(z.State) != ec2.AvailabilityZoneStateAvailable {
 			klog.Warningf("Zone %q has state %q", zone, aws.StringValue(z.State))
 		}
 	}
@@ -1608,9 +1608,9 @@ func (c *awsCloudImplementation) zonesWithInstanceType(instanceType string) (set
 	request := &ec2.DescribeReservedInstancesOfferingsInput{}
 	request.InstanceTenancy = aws.String("default")
 	request.IncludeMarketplace = aws.Bool(false)
-	request.OfferingClass = aws.String("standard")
-	request.OfferingType = aws.String("No Upfront")
-	request.ProductDescription = aws.String("Linux/UNIX (Amazon VPC)")
+	request.OfferingClass = aws.String(ec2.OfferingClassTypeStandard)
+	request.OfferingType = aws.String(ec2.OfferingTypeValuesNoUpfront)
+	request.ProductDescription = aws.String(ec2.RIProductDescriptionLinuxUnixamazonVpc)
 	request.InstanceType = aws.String(instanceType)
 
 	zones := sets.NewString()