Refactor e2e tests to use acktest.tags helper (#106)

Issue #, if available: aws-controllers-k8s/community#1235

Description of changes: 
* Use `acktest.tags` assertions in e2e tests to accurately test for ACK tags and user-defined tags
* Enhance existing tag tests by checking both server-side values as well as `Spec.Tags` values
* Add hook code for `Instance` and `RouteTable` so ACK tags are not populated in `Spec`

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Author: brycahta

apis/v1alpha1/ack-generate-metadata.yaml

@@ -1,13 +1,13 @@
 ack_generate_info:
-  build_date: "2022-09-21T13:36:34Z"
+  build_date: "2022-09-27T22:29:46Z"
   build_hash: 6e2ffbc3b16a30ac59be6719918c601c2c864064
   go_version: go1.18.1
   version: v0.20.1-3-g6e2ffbc
 api_directory_checksum: 127aa0f51fbcdde596b8f42f93bcdf3b6dee8488
 api_version: v1alpha1
 aws_sdk_go_version: v1.44.93
 generator_config_info:
-  file_checksum: 8573a89b8220023263610579bfb84b762d99f5fa
+  file_checksum: a31177552ce6ea5ce3b624520711382f113cf05b
   original_file_name: generator.yaml
 last_modification:
   reason: API generation

apis/v1alpha1/generator.yaml

@@ -205,6 +205,10 @@ resources:
     hooks:
       sdk_create_post_build_request:
         template_path: hooks/instance/sdk_create_post_build_request.go.tpl
+      sdk_create_post_set_output:
+        template_path: hooks/instance/sdk_create_post_set_output.go.tpl
+      sdk_read_many_post_set_output:
+        template_path: hooks/instance/sdk_read_many_post_set_output.go.tpl
       sdk_delete_post_build_request:
         template_path: hooks/instance/sdk_delete_post_build_request.go.tpl
       sdk_file_end:

generator.yaml

@@ -205,6 +205,10 @@ resources:
     hooks:
       sdk_create_post_build_request:
         template_path: hooks/instance/sdk_create_post_build_request.go.tpl
+      sdk_create_post_set_output:
+        template_path: hooks/instance/sdk_create_post_set_output.go.tpl
+      sdk_read_many_post_set_output:
+        template_path: hooks/instance/sdk_read_many_post_set_output.go.tpl
       sdk_delete_post_build_request:
         template_path: hooks/instance/sdk_delete_post_build_request.go.tpl
       sdk_file_end:

pkg/resource/instance/sdk.go

@@ -0,0 +1,6 @@
+	toAdd, toDelete := computeTagsDelta(desired.ko.Spec.Tags, ko.Spec.Tags)
+	if len(toAdd) == 0 && len(toDelete) == 0 {
+		// if desired tags and response tags are equal,
+		// then assign desired tags to maintain tag order
+		ko.Spec.Tags = desired.ko.Spec.Tags
+	}

pkg/resource/route_table/sdk.go

@@ -0,0 +1,8 @@
+	
+	toAdd, toDelete := computeTagsDelta(r.ko.Spec.Tags, ko.Spec.Tags)
+	if len(toAdd) == 0 && len(toDelete) == 0 {
+		// if resource's initial tags and response tags are equal,
+		// then assign resource's tags to maintain tag order
+		ko.Spec.Tags = r.ko.Spec.Tags
+	}
+    

templates/hooks/instance/sdk_create_post_set_output.go.tpl

@@ -10,4 +10,11 @@
 		if err := rm.createRoutes(ctx, &resource{ko}); err != nil {
 			return nil, err
 		}
+	}
+
+	toAdd, toDelete := computeTagsDelta(desired.ko.Spec.Tags, ko.Spec.Tags)
+	if len(toAdd) == 0 && len(toDelete) == 0 {
+		// if desired tags and response tags are equal,
+		// then assign desired tags to maintain tag order
+		ko.Spec.Tags = desired.ko.Spec.Tags
 	}

templates/hooks/instance/sdk_read_many_post_set_output.go.tpl

@@ -2,4 +2,10 @@
     if found {
         rm.addRoutesToStatus(ko, resp.RouteTables[0])
     }
+	toAdd, toDelete := computeTagsDelta(r.ko.Spec.Tags, ko.Spec.Tags)
+	if len(toAdd) == 0 && len(toDelete) == 0 {
+		// if resource's initial tags and response tags are equal,
+		// then assign resource's tags to maintain tag order
+		ko.Spec.Tags = r.ko.Spec.Tags
+	}
 

templates/hooks/route_table/sdk_create_post_set_output.go.tpl

@@ -1 +1 @@
-acktest @ git+https://github.com/aws-controllers-k8s/test-infra.git@5220331d003e3a23de4470e68d02c99b16c81989
+acktest @ git+https://github.com/aws-controllers-k8s/test-infra.git@73eb0d5af8ac19d17fad8787d6462d56f4e3e37d

templates/hooks/route_table/sdk_read_many_post_set_output.go.tpl

@@ -21,6 +21,15 @@ class EC2Validator:
     def __init__(self, ec2_client):
         self.ec2_client = ec2_client
 
+    def get_dhcp_options(self, dhcp_options_id: str):
+        try:
+            aws_res = self.ec2_client.describe_dhcp_options(DhcpOptionsIds=[dhcp_options_id])
+            if len(aws_res["DhcpOptions"]) > 0:
+                return aws_res["DhcpOptions"][0]
+            return None
+        except self.ec2_client.exceptions.ClientError:
+            return None
+
     def assert_dhcp_options(self, dhcp_options_id: str, exists=True):
         res_found = False
         try:
@@ -42,6 +51,15 @@ def get_internet_gateway(self, igw_id: str):
     def assert_internet_gateway(self, igw_id: str, exists=True):
         assert (self.get_internet_gateway(igw_id) is not None) == exists
 
+    def get_nat_gateway(self, ngw_id: str):
+        try:
+            aws_res = self.ec2_client.describe_nat_gateways(NatGatewayIds=[ngw_id])
+            if len(aws_res["NatGateways"]) > 0:
+                return aws_res["NatGateways"][0]
+            return None
+        except self.ec2_client.exceptions.ClientError:
+            return None
+
     def assert_nat_gateway(self, ngw_id: str, exists=True):
         res_found = False
         try:
@@ -116,6 +134,15 @@ def get_subnet(self, subnet_id: str) -> Union[None, Dict]:
     def assert_subnet(self, subnet_id: str, exists=True):
         assert (self.get_subnet(subnet_id) is not None) == exists
 
+    def get_transit_gateway(self, tgw_id: str) -> Union[None, Dict]:
+        try:
+            aws_res = self.ec2_client.describe_transit_gateways(TransitGatewayIds=[tgw_id])
+            if len(aws_res["TransitGateways"]) > 0:
+                return aws_res["TransitGateways"][0]
+            return None
+        except self.ec2_client.exceptions.ClientError:
+            return None
+
     def assert_transit_gateway(self, tgw_id: str, exists=True):
         res_found = False
         try:
@@ -146,6 +173,15 @@ def assert_vpc(self, vpc_id: str, exists=True):
             pass
         assert res_found is exists
 
+    def get_vpc_endpoint(self, vpc_endpoint_id: str) -> Union[None, Dict]:
+        try:
+            aws_res = self.ec2_client.describe_vpc_endpoints(VpcEndpointIds=[vpc_endpoint_id])
+            if len(aws_res["VpcEndpoints"]) > 0:
+                return aws_res["VpcEndpoints"][0]
+            return None
+        except self.ec2_client.exceptions.ClientError:
+            return None
+
     def assert_vpc_endpoint(self, vpc_endpoint_id: str, exists=True):
         res_found = False
         try:

test/e2e/requirements.txt

@@ -18,6 +18,7 @@
 import time
 import logging
 
+from acktest import tags
 from acktest.resources import random_suffix_name
 from acktest.k8s import resource as k8s
 from e2e import service_marker, CRD_GROUP, CRD_VERSION, load_ec2_resource
@@ -131,23 +132,36 @@ def test_crud_tags(self, ec2_client, simple_dhcp_options):
         # Check dhcpOptions exists in AWS
         ec2_validator = EC2Validator(ec2_client)
         ec2_validator.assert_dhcp_options(resource_id)
+
+        # Check system and user tags exist for dhcpOptions resource
+        dhcp_options = ec2_validator.get_dhcp_options(resource_id)
+        user_tags = {
+            "initialtagkey": "initialtagvalue"
+        }
+        tags.assert_ack_system_tags(
+            tags=dhcp_options["Tags"],
+        )
+        tags.assert_equal_without_ack_tags(
+            expected=user_tags,
+            actual=dhcp_options["Tags"],
+        )
 
-        # Check tags exist for dhcpOptions resource
+        # Only user tags should be present in Spec
+        assert len(resource["spec"]["tags"]) == 1
         assert resource["spec"]["tags"][0]["key"] == "initialtagkey"
         assert resource["spec"]["tags"][0]["value"] == "initialtagvalue"
 
-        # New pair of tags
-        new_tags = [
+        # Update tags
+        update_tags = [
                 {
                     "key": "updatedtagkey",
                     "value": "updatedtagvalue",
                 }
-               
             ]
 
         # Patch the dhcpOptions, updating the tags with new pair
         updates = {
-            "spec": {"tags": new_tags},
+            "spec": {"tags": update_tags},
         }
 
         k8s.patch_custom_resource(ref, updates)
@@ -156,8 +170,22 @@ def test_crud_tags(self, ec2_client, simple_dhcp_options):
         # Check resource synced successfully
         assert k8s.wait_on_condition(ref, "ACK.ResourceSynced", "True", wait_periods=5)
 
-        # Assert tags are updated for dhcpOptions resource
+        # Check for updated user tags; system tags should persist
+        dhcp_options = ec2_validator.get_dhcp_options(resource_id)
+        updated_tags = {
+            "updatedtagkey": "updatedtagvalue"
+        }
+        tags.assert_ack_system_tags(
+            tags=dhcp_options["Tags"],
+        )
+        tags.assert_equal_without_ack_tags(
+            expected=updated_tags,
+            actual=dhcp_options["Tags"],
+        )
+               
+        # Only user tags should be present in Spec
         resource = k8s.get_resource(ref)
+        assert len(resource["spec"]["tags"]) == 1
         assert resource["spec"]["tags"][0]["key"] == "updatedtagkey"
         assert resource["spec"]["tags"][0]["value"] == "updatedtagvalue"
 
@@ -172,9 +200,19 @@ def test_crud_tags(self, ec2_client, simple_dhcp_options):
         # Check resource synced successfully
         assert k8s.wait_on_condition(ref, "ACK.ResourceSynced", "True", wait_periods=5)
 
-        # Assert tags are deleted
+        # Check for removed user tags; system tags should persist
+        dhcp_options = ec2_validator.get_dhcp_options(resource_id)
+        tags.assert_ack_system_tags(
+            tags=dhcp_options["Tags"],
+        )
+        tags.assert_equal_without_ack_tags(
+            expected=[],
+            actual=dhcp_options["Tags"],
+        )
+        
+        # Check user tags are removed from Spec
         resource = k8s.get_resource(ref)
-        assert len(resource['spec']['tags']) == 0
+        assert len(resource["spec"]["tags"]) == 0
 
         # Delete k8s resource
         _, deleted = k8s.delete_custom_resource(ref)

test/e2e/tests/helper.py

@@ -18,6 +18,7 @@
 import time
 import logging
 
+from acktest import tags
 from acktest.resources import random_suffix_name
 from acktest.k8s import resource as k8s
 from e2e import service_marker, CRD_GROUP, CRD_VERSION, load_ec2_resource
@@ -153,13 +154,27 @@ def test_crud_tags(self, ec2_client, simple_elastic_ip_address):
         # Check Address exists
         exists = address_exists(ec2_client, resource_id)
         assert exists
-        
-        # Check tags exist for elasticipaddress resource
+
+         # Check system and user tags exist for elasticIP resource
+        elastic_ip = get_address(ec2_client, resource_id)
+        user_tags = {
+            "initialtagkey": "initialtagvalue"
+        }
+        tags.assert_ack_system_tags(
+            tags=elastic_ip["Tags"],
+        )
+        tags.assert_equal_without_ack_tags(
+            expected=user_tags,
+            actual=elastic_ip["Tags"],
+        )
+
+        # Only user tags should be present in Spec
+        assert len(resource["spec"]["tags"]) == 1
         assert resource["spec"]["tags"][0]["key"] == "initialtagkey"
         assert resource["spec"]["tags"][0]["value"] == "initialtagvalue"
 
-        # New pair of tags
-        new_tags = [
+        # Update tags
+        update_tags = [
                 {
                     "key": "updatedtagkey",
                     "value": "updatedtagvalue",
@@ -169,17 +184,31 @@ def test_crud_tags(self, ec2_client, simple_elastic_ip_address):
 
         # Patch the elasticipaddress, updating the tags with new pair
         updates = {
-            "spec": {"tags": new_tags},
+            "spec": {"tags": update_tags},
         }
 
         k8s.patch_custom_resource(ref, updates)
         time.sleep(MODIFY_WAIT_AFTER_SECONDS)
 
         # Check resource synced successfully
         assert k8s.wait_on_condition(ref, "ACK.ResourceSynced", "True", wait_periods=5)
-        
-        # Assert tags are updated for elasticipaddress resource
+
+        # Check for updated user tags; system tags should persist
+        elastic_ip = get_address(ec2_client, resource_id)
+        updated_tags = {
+            "updatedtagkey": "updatedtagvalue"
+        }
+        tags.assert_ack_system_tags(
+            tags=elastic_ip["Tags"],
+        )
+        tags.assert_equal_without_ack_tags(
+            expected=updated_tags,
+            actual=elastic_ip["Tags"],
+        )
+               
+        # Only user tags should be present in Spec
         resource = k8s.get_resource(ref)
+        assert len(resource["spec"]["tags"]) == 1
         assert resource["spec"]["tags"][0]["key"] == "updatedtagkey"
         assert resource["spec"]["tags"][0]["value"] == "updatedtagvalue"
 
@@ -194,9 +223,19 @@ def test_crud_tags(self, ec2_client, simple_elastic_ip_address):
         # Check resource synced successfully
         assert k8s.wait_on_condition(ref, "ACK.ResourceSynced", "True", wait_periods=5)
 
-        # Assert tags are deleted
+        # Check for removed user tags; system tags should persist
+        elastic_ip = get_address(ec2_client, resource_id)
+        tags.assert_ack_system_tags(
+            tags=elastic_ip["Tags"],
+        )
+        tags.assert_equal_without_ack_tags(
+            expected=[],
+            actual=elastic_ip["Tags"],
+        )
+        
+        # Check user tags are removed from Spec
         resource = k8s.get_resource(ref)
-        assert len(resource['spec']['tags']) == 0
+        assert len(resource["spec"]["tags"]) == 0
 
         # Delete k8s resource
         _, deleted = k8s.delete_custom_resource(ref)