Add Update functionality for VPC.CIDRBlocks (#85)

brycahta · web-flow · commit 9b10dc2874a1 · 2022-09-06T13:39:15.000-07:00
Issue #, if available: aws-controllers-k8s/community#1198 Description of changes: Enables associating multiple IPv4 CIDR Blocks to a single VPC By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
diff --git a/apis/v1alpha1/ack-generate-metadata.yaml b/apis/v1alpha1/ack-generate-metadata.yaml
@@ -1,13 +1,13 @@
 ack_generate_info:
-  build_date: "2022-09-06T18:39:23Z"
+  build_date: "2022-09-06T20:17:17Z"
   build_hash: 87477ae8ca8ac6ddb8c565bbd910cc7e30f55ed0
   go_version: go1.18.1
   version: v0.19.3
-api_directory_checksum: 6b7708a8cacb471891466f5851f248e0e895f2c2
+api_directory_checksum: 3960da50b98c36b05635d3abbfd129ae7bb48e32
 api_version: v1alpha1
 aws_sdk_go_version: v1.42.0
 generator_config_info:
-  file_checksum: 9586b277b014daec6188d09741c607c96b1201fb
+  file_checksum: b17fda4c7e3ae9446dd568f3be1f08d70ce15588
   original_file_name: generator.yaml
 last_modification:
   reason: API generation
diff --git a/apis/v1alpha1/generator.yaml b/apis/v1alpha1/generator.yaml
@@ -429,6 +429,7 @@ resources:
       CIDRBlocks:
         custom_field:
           list_of: String
+        is_required: true
       EnableDNSSupport:
         from:
           operation: ModifyVpcAttribute
diff --git a/apis/v1alpha1/vpc.go b/apis/v1alpha1/vpc.go
diff --git a/config/crd/bases/ec2.services.k8s.aws_vpcs.yaml b/config/crd/bases/ec2.services.k8s.aws_vpcs.yaml
@@ -90,6 +90,8 @@ spec:
                       type: string
                   type: object
                 type: array
+            required:
+            - cidrBlocks
             type: object
           status:
             description: VPCStatus defines the observed state of VPC
diff --git a/generator.yaml b/generator.yaml
@@ -429,6 +429,7 @@ resources:
       CIDRBlocks:
         custom_field:
           list_of: String
+        is_required: true
       EnableDNSSupport:
         from:
           operation: ModifyVpcAttribute
diff --git a/helm/crds/ec2.services.k8s.aws_vpcs.yaml b/helm/crds/ec2.services.k8s.aws_vpcs.yaml
@@ -90,6 +90,8 @@ spec:
                       type: string
                   type: object
                 type: array
+            required:
+            - cidrBlocks
             type: object
           status:
             description: VPCStatus defines the observed state of VPC
diff --git a/pkg/resource/vpc/hook.go b/pkg/resource/vpc/hook.go
@@ -16,6 +16,7 @@ package vpc
 import (
 	"context"
 
+	svcapitypes "github.com/aws-controllers-k8s/ec2-controller/apis/v1alpha1"
 	ackcompare "github.com/aws-controllers-k8s/runtime/pkg/compare"
 	ackrtlog "github.com/aws-controllers-k8s/runtime/pkg/runtime/log"
 	svcsdk "github.com/aws/aws-sdk-go/service/ec2"
@@ -128,6 +129,93 @@ func (rm *resourceManager) syncDNSHostnamesAttribute(
 	return nil
 }
 
+// computeStringPDifference uses the underlying string value
+// to discern which elements are in slice `a` that aren't in slice `b`
+// and vice-versa
+func computeStringPDifference(a, b []*string) (aNotB, bNotA []*string) {
+	mapOfB := map[string]struct{}{}
+	for _, elemB := range b {
+		mapOfB[*elemB] = struct{}{}
+	}
+	mapOfA := map[string]struct{}{}
+	for _, elemA := range a {
+		mapOfA[*elemA] = struct{}{}
+	}
+
+	for _, elemA := range a {
+		if _, found := mapOfB[*elemA]; !found {
+			aNotB = append(aNotB, elemA)
+		}
+	}
+	for _, elemB := range b {
+		if _, found := mapOfB[*elemB]; !found {
+			bNotA = append(bNotA, elemB)
+		}
+	}
+
+	return aNotB, bNotA
+}
+
+// syncCIDRBlocks analyzes desired and latest
+// IPv4 CIDRBlocks and executes API calls to
+// Associate/Disassociate CIDRs as needed
+func (rm *resourceManager) syncCIDRBlocks(
+	ctx context.Context,
+	desired *resource,
+	latest *resource,
+) (err error) {
+	rlog := ackrtlog.FromContext(ctx)
+	exit := rlog.Trace("rm.syncCIDRBlocks")
+	defer exit(err)
+
+	desiredCIDRs := desired.ko.Spec.CIDRBlocks
+	latestCIDRs := latest.ko.Spec.CIDRBlocks
+	latestCIDRStates := latest.ko.Status.CIDRBlockAssociationSet
+	toAddCIDRs, toDeleteCIDRs := computeStringPDifference(desiredCIDRs, latestCIDRs)
+
+	// extract associationID for the DisassociateVpcCidr request
+	for _, cidr := range toDeleteCIDRs {
+		input := &svcsdk.DisassociateVpcCidrBlockInput{}
+		for _, cidrAssociation := range latestCIDRStates {
+			if *cidr == *cidrAssociation.CIDRBlock {
+				input.AssociationId = cidrAssociation.AssociationID
+				_, err = rm.sdkapi.DisassociateVpcCidrBlockWithContext(ctx, input)
+				rm.metrics.RecordAPICall("UPDATE", "DisassociateVpcCidrBlock", err)
+				if err != nil {
+					return err
+				}
+			}
+		}
+	}
+
+	for _, cidr := range toAddCIDRs {
+		input := &svcsdk.AssociateVpcCidrBlockInput{
+			VpcId:     latest.ko.Status.VPCID,
+			CidrBlock: cidr,
+		}
+		_, err = rm.sdkapi.AssociateVpcCidrBlockWithContext(ctx, input)
+		rm.metrics.RecordAPICall("UPDATE", "AssociateVpcCidrBlock", err)
+		if err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
+// setSpecCIDRs sets Spec.CIDRBlocks using the CIDRs in
+// Status.CIDRBlockAssociationSet, which is set via sdkCreate/sdkFind
+func (rm *resourceManager) setSpecCIDRs(
+	ko *svcapitypes.VPC,
+) {
+	ko.Spec.CIDRBlocks = nil
+	if ko.Status.CIDRBlockAssociationSet != nil {
+		for _, cidrAssoc := range ko.Status.CIDRBlockAssociationSet {
+			ko.Spec.CIDRBlocks = append(ko.Spec.CIDRBlocks, cidrAssoc.CIDRBlock)
+		}
+	}
+}
+
 func (rm *resourceManager) createAttributes(
 	ctx context.Context,
 	r *resource,
@@ -161,6 +249,12 @@ func (rm *resourceManager) customUpdate(
 	// the original Kubernetes object we passed to the function
 	ko := desired.ko.DeepCopy()
 
+	if delta.DifferentAt("Spec.CIDRBlocks") {
+		if err := rm.syncCIDRBlocks(ctx, desired, latest); err != nil {
+			return nil, err
+		}
+	}
+
 	if delta.DifferentAt("Spec.EnableDNSSupport") {
 		if err := rm.syncDNSSupportAttribute(ctx, desired); err != nil {
 			return nil, err
@@ -182,7 +276,7 @@ func (rm *resourceManager) customUpdate(
 // CIDR block defined in the resource's Spec
 func applyPrimaryCIDRBlockInCreateRequest(r *resource,
 	input *svcsdk.CreateVpcInput) {
-	if r.ko.Spec.CIDRBlocks != nil && len(r.ko.Spec.CIDRBlocks) > 0 {
+	if len(r.ko.Spec.CIDRBlocks) > 0 {
 		input.CidrBlock = r.ko.Spec.CIDRBlocks[0]
 	}
 }
diff --git a/pkg/resource/vpc/sdk.go b/pkg/resource/vpc/sdk.go
diff --git a/templates/hooks/vpc/sdk_create_post_set_output.go.tpl b/templates/hooks/vpc/sdk_create_post_set_output.go.tpl
@@ -1,3 +1,4 @@
+    rm.setSpecCIDRs(ko)
     err = rm.createAttributes(ctx, &resource{ko})
     if err != nil {
         return nil, err
diff --git a/templates/hooks/vpc/sdk_read_many_post_set_output.go.tpl b/templates/hooks/vpc/sdk_read_many_post_set_output.go.tpl
@@ -1,3 +1,4 @@
+	rm.setSpecCIDRs(ko)
 	if dnsAttrs, err := rm.getDNSAttributes(ctx, *ko.Status.VPCID); err != nil {
 		return nil, err
 	} else {
diff --git a/test/e2e/tests/helper.py b/test/e2e/tests/helper.py
@@ -128,6 +128,15 @@ def assert_transit_gateway(self, tgw_id: str, exists=True):
             pass
         assert res_found is exists
 
+    def get_vpc(self, vpc_id: str) -> Union[None, Dict]:
+        try:
+            aws_res = self.ec2_client.describe_vpcs(VpcIds=[vpc_id])
+            if len(aws_res["Vpcs"]) > 0:
+                return aws_res["Vpcs"][0]
+            return None
+        except self.ec2_client.exceptions.ClientError:
+            return None
+            
     def assert_vpc(self, vpc_id: str, exists=True):
         res_found = False
         try:
diff --git a/test/e2e/tests/test_vpc.py b/test/e2e/tests/test_vpc.py
@@ -26,6 +26,7 @@
 from e2e.tests.helper import EC2Validator
 
 RESOURCE_PLURAL = "vpcs"
+PRIMARY_CIDR_DEFAULT = "10.0.0.0/16"
 
 CREATE_WAIT_AFTER_SECONDS = 10
 DELETE_WAIT_AFTER_SECONDS = 10
@@ -45,7 +46,7 @@ def get_dns_hostnames(ec2_client, vpc_id: str) -> bool:
 @service_marker
 @pytest.mark.canary
 class TestVpc:
-    def test_create_delete(self, ec2_client, simple_vpc):
+    def test_crud(self, ec2_client, simple_vpc):
         (ref, cr) = simple_vpc
 
         resource_id = cr["status"]["vpcID"]
@@ -56,6 +57,24 @@ def test_create_delete(self, ec2_client, simple_vpc):
         ec2_validator = EC2Validator(ec2_client)
         ec2_validator.assert_vpc(resource_id)
 
+        # Validate CIDR Block
+        vpc = ec2_validator.get_vpc(resource_id)
+        assert len(vpc['CidrBlockAssociationSet']) == 1
+        assert vpc['CidrBlockAssociationSet'][0]['CidrBlock'] == PRIMARY_CIDR_DEFAULT
+
+        # Associate secondary CIDR
+        secondary_cidr = "10.2.0.0/16"
+        updates = {
+            "spec": {"cidrBlocks": [PRIMARY_CIDR_DEFAULT, secondary_cidr]}
+        }
+        k8s.patch_custom_resource(ref, updates)
+        time.sleep(MODIFY_WAIT_AFTER_SECONDS)
+
+        vpc = ec2_validator.get_vpc(resource_id)
+        assert len(vpc['CidrBlockAssociationSet']) == 2
+        assert vpc['CidrBlockAssociationSet'][0]['CidrBlock'] == PRIMARY_CIDR_DEFAULT
+        assert vpc['CidrBlockAssociationSet'][1]['CidrBlock'] == secondary_cidr
+
         # Delete k8s resource
         _, deleted = k8s.delete_custom_resource(ref, 2, 5)
         assert deleted is True

Original file line number	Diff line number	Diff line change
`@@ -1,3 +1,4 @@`
	`1`	`+ rm.setSpecCIDRs(ko)`
`1`	`2`	`err = rm.createAttributes(ctx, &resource{ko})`
`2`	`3`	`if err != nil {`
`3`	`4`	`return nil, err`
Original file line number	Diff line number	Diff line change
`@@ -1,3 +1,4 @@`
	`1`	`+ rm.setSpecCIDRs(ko)`
`1`	`2`	`if dnsAttrs, err := rm.getDNSAttributes(ctx, *ko.Status.VPCID); err != nil {`
`2`	`3`	`return nil, err`
`3`	`4`	`} else {`