From c5c806ebfdc1abf2705959233c08dd46ee8779a8 Mon Sep 17 00:00:00 2001 From: michaelhtm <98621731+michaelhtm@users.noreply.github.com> Date: Tue, 17 Sep 2024 14:31:47 -0700 Subject: [PATCH 1/2] Add tests to ensure resource reference is not replaced by controller These tests ensure that after an update, the controller won't replace the resource reference with the ID itself. --- test/e2e/resources/internet_gateway_ref.yaml | 8 ++ test/e2e/resources/route_table_ref.yaml | 16 +++ test/e2e/tests/test_references.py | 118 ++++++++++++++++++- 3 files changed, 141 insertions(+), 1 deletion(-) create mode 100644 test/e2e/resources/internet_gateway_ref.yaml create mode 100644 test/e2e/resources/route_table_ref.yaml diff --git a/test/e2e/resources/internet_gateway_ref.yaml b/test/e2e/resources/internet_gateway_ref.yaml new file mode 100644 index 00000000..b3ead8b0 --- /dev/null +++ b/test/e2e/resources/internet_gateway_ref.yaml @@ -0,0 +1,8 @@ +apiVersion: ec2.services.k8s.aws/v1alpha1 +kind: InternetGateway +metadata: + name: $INTERNET_GATEWAY_NAME +spec: + vpcRef: + from: + name: $VPC_NAME diff --git a/test/e2e/resources/route_table_ref.yaml b/test/e2e/resources/route_table_ref.yaml new file mode 100644 index 00000000..516248e5 --- /dev/null +++ b/test/e2e/resources/route_table_ref.yaml @@ -0,0 +1,16 @@ +apiVersion: ec2.services.k8s.aws/v1alpha1 +kind: RouteTable +metadata: + name: $ROUTE_TABLE_NAME +spec: + routes: + - destinationCIDRBlock: $DEST_CIDR_BLOCK + gatewayRef: + from: + name: $INTERNET_GATEWAY_NAME + vpcRef: + from: + name: $VPC_NAME + tags: + - key: $TAG_KEY + value: $TAG_VALUE \ No newline at end of file diff --git a/test/e2e/tests/test_references.py b/test/e2e/tests/test_references.py index a5056a13..1a32361a 100644 --- a/test/e2e/tests/test_references.py +++ b/test/e2e/tests/test_references.py @@ -27,6 +27,7 @@ from e2e.tests.helper import EC2Validator CREATE_WAIT_AFTER_SECONDS = 20 +MODIFY_WAIT_AFTER_SECONDS = 30 DELETE_WAIT_AFTER_SECONDS = 10 DELETE_TIMEOUT_SECONDS = 300 @@ -167,4 +168,119 @@ def test_references(self, ec2_client): ec2_validator.assert_vpc_endpoint(vpc_endpoint_id, exists=False) ec2_validator.assert_subnet(subnet_id, exists=False) ec2_validator.assert_security_group(sg_id, exists=False) - ec2_validator.assert_vpc(vpc_id, exists=False) \ No newline at end of file + ec2_validator.assert_vpc(vpc_id, exists=False) + + def test_array_references(self, ec2_client): + route_table_name = random_suffix_name("route-table-test", 24) + vpc_name = random_suffix_name("vpc-ref-test", 24) + gateway_name = random_suffix_name("gateway-ref-test", 24) + + test_values = REPLACEMENT_VALUES.copy() + test_values["ROUTE_TABLE_NAME"] = route_table_name + test_values["DEST_CIDR_BLOCK"] = "0.0.0.0/0" + test_values["INTERNET_GATEWAY_NAME"] = gateway_name + test_values["VPC_NAME"] = vpc_name + test_values["CIDR_BLOCK"] = "10.0.0.0/16" + test_values["ENABLE_DNS_SUPPORT"] = "False" + test_values["ENABLE_DNS_HOSTNAMES"] = "False" + test_values["DISALLOW_DEFAULT_SECURITY_GROUP_RULE"] = "False" + + # Load CRs + route_table_resource_data = load_ec2_resource( + "route_table_ref", + additional_replacements=test_values + ) + vpc_resource_data = load_ec2_resource( + "vpc", + additional_replacements=test_values + ) + gateway_resource_data = load_ec2_resource( + "internet_gateway_ref", + additional_replacements=test_values + ) + + # This test creates resources in order, + + # Create VPC + vpc_ref = k8s.CustomResourceReference( + CRD_GROUP, CRD_VERSION, 'vpcs', + vpc_name, namespace="default", + ) + k8s.create_custom_resource(vpc_ref, vpc_resource_data) + + # Create Internet Gateway + gateway_ref = k8s.CustomResourceReference( + CRD_GROUP, CRD_VERSION, 'internetgateways', + gateway_name, namespace="default", + ) + k8s.create_custom_resource(gateway_ref, gateway_resource_data) + + # Create route table + route_table_ref = k8s.CustomResourceReference( + CRD_GROUP, CRD_VERSION, 'routetables', + route_table_name, namespace="default", + ) + k8s.create_custom_resource(route_table_ref, route_table_resource_data) + + # Wait a few seconds so resources are synced + time.sleep(CREATE_WAIT_AFTER_SECONDS) + assert k8s.wait_on_condition(vpc_ref, "ACK.ResourceSynced", "True", wait_periods=5) + assert k8s.wait_on_condition(gateway_ref, "ACK.ResourceSynced", "True", wait_periods=5) + assert k8s.wait_on_condition(route_table_ref, "ACK.ResourceSynced", "True", wait_periods=10) + + assert k8s.wait_on_condition(gateway_ref, "ACK.ReferencesResolved", "True", wait_periods=5) + assert k8s.wait_on_condition(route_table_ref, "ACK.ReferencesResolved", "True", wait_periods=10) + + # Acquire Internet Gateway ID + gateway_cr = k8s.get_resource(gateway_ref) + assert 'status' in gateway_cr + gateway_id = gateway_cr["status"]["internetGatewayID"] + + # Ensure routetable contains reference in spec + route_table_cr = k8s.get_resource(route_table_ref) + assert 'spec' in route_table_cr + assert 'vpcRef' in route_table_cr['spec'] + assert route_table_cr['spec']['vpcRef']['from']['name'] == vpc_name + assert 'routes' in route_table_cr['spec'] + assert len(route_table_cr['spec']['routes']) == 1 + assert 'gatewayID' not in route_table_cr['spec']['routes'][0] + assert 'gatewayRef' in route_table_cr['spec']['routes'][0] + assert route_table_cr['spec']['routes'][0]['gatewayRef']['from']['name'] == gateway_name + assert 'status' in route_table_cr + assert 'routeStatuses' in route_table_cr['status'] + found_gateway_id = False + for rs in route_table_cr['status']['routeStatuses']: + if 'gatewayID' in rs and rs['gatewayID'] == gateway_id: + found_gateway_id = True + assert found_gateway_id + + user_tag = { + "tag": "my_tag", + "value": "my_val" + } + route_table_update = { + 'spec': { + 'tags': [user_tag] + } + } + k8s.patch_custom_resource(route_table_ref, route_table_update) + time.sleep(MODIFY_WAIT_AFTER_SECONDS) + assert k8s.wait_on_condition(route_table_ref, "ACK.ResourceSynced", "True", wait_periods=5) + assert k8s.wait_on_condition(route_table_ref, "ACK.ReferencesResolved", "True", wait_periods=5) + + # Ensure that the reference has not changed + route_table_cr = k8s.get_resource(route_table_ref) + assert 'spec' in route_table_cr + assert 'routes' in route_table_cr['spec'] + assert len(route_table_cr['spec']['routes']) == 1 + assert 'gatewayID' not in route_table_cr['spec']['routes'][0] + assert 'gatewayRef' in route_table_cr['spec']['routes'][0] + assert route_table_cr['spec']['routes'][0]['gatewayRef']['from']['name'] == gateway_name + + # Delete All + _, deleted = k8s.delete_custom_resource(route_table_ref) + assert deleted + _, deleted = k8s.delete_custom_resource(gateway_ref) + assert deleted + _, deleted = k8s.delete_custom_resource(vpc_ref) + assert deleted \ No newline at end of file From 52b81c84204cd62490ecb9fd2c650f4c7786be50 Mon Sep 17 00:00:00 2001 From: michaelhtm <98621731+michaelhtm@users.noreply.github.com> Date: Wed, 18 Sep 2024 10:19:36 -0700 Subject: [PATCH 2/2] Preserve routeTable spec in update operation This commit addresses a customer issue, where the update operation was overwriting the routeTable spec, getting rid of the reference names placed by the user, and replacing them with the values retrieved from the AWS API. In the future we want to make changes in how controllers are generated to ensure the sdkfind has places in store to support not overriding user defined resource references --- pkg/resource/route_table/hooks.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/resource/route_table/hooks.go b/pkg/resource/route_table/hooks.go index 6f1ca1ef..a0812b31 100644 --- a/pkg/resource/route_table/hooks.go +++ b/pkg/resource/route_table/hooks.go @@ -236,7 +236,9 @@ func (rm *resourceManager) customUpdateRouteTable( } } - return updated, nil + newDesired := rm.concreteResource(desired.DeepCopy()) + newDesired.ko.Status = updated.ko.Status + return newDesired, nil } func (rm *resourceManager) requiredFieldsMissingForCreateRoute(