Add NATGateway CRD

[RedbackThomson]

Issue #, if available:

Description of changes:
Creates the NATGateway CRD, with references to ElasticIPAddress and Subnet for their respective fields

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

[jaypipes]

Left a comment inline to address in a future PR. All the generated code and tests in here look A-OK, thank you @RedbackThomson :)

[jaypipes]

I actually don't think either of these is necessary... we should be figuring out the wrapper field automagically when the resource name matches the wrapper field name... (not needed to change in this patch, just sayin')

[jaypipes]

/cc @brycahta

Not something to change in this PR, but a future PR...

I know this EC2Validator class was recently added with this style of assertions, but I would recommend reworking the code so that assert statements are kept out of the helper class methods and instead remain only in the test methods themselves.

In other words, have helper classes or methods that get some state and then have the test method assert that state matches some expectation.

For example, consider reworking this class to be like this:

class EC2Helper:
    def __init__(self, ec2_client):
        self.ec2_client = ec2_client
...
    def nat_gateway(self, ngw_id):
        """Returns a dict containing the NAT gateway info or None
        if no such gateway exists
        """
        try:
            aws_res = self.ec2_client.describe_nat_gateways(NatGatewayIds=[ngw_id])
            return aws_res["NatGateways"][0]
        except self.ec2_client.exceptions.ClientError:
            return None

    def nat_gateway_exists(self, ngw_id):
        """Returns whether a NAT gateway exists (deleting counts as not existing)"""
        # NATGateway may take awhile to be removed server-side, so 
        # treat 'deleting' and 'deleted' states as resource no longer existing
        ngw = self.nate_gateway(ngw_id)
        return ngw is not None and ngw['State'] != "deleting" and ngw['State'] != "deleted"

and then in your test methods, use assert to validate an expectation:

        # Check NAT Gateway exists in AWS
        ec2_helper = EC2Helper(ec2_client)
        assert ec2_helper.nat_gateway_exists(resource_id)

        # Delete k8s resource
        _, deleted = k8s.delete_custom_resource(ref)
        assert deleted is True

        time.sleep(DELETE_WAIT_AFTER_SECONDS)

        # Check NAT Gateway no longer exists in AWS
        assert not ec2_helper.nat_gateway_exists(resource_id)

The fundamental principle at issue here is ensuring that functions/methods have as close to a single purpose as possible. Instead of having a single function that retrieves some information, checks for existence and asserts some expected condition, just have a method that does one thing and leave the assert statements for test methods, where they are obvious and explicit...

[brycahta]

aws-controllers-k8s/community#1235

[brycahta]

Looks good

[brycahta]

what's the purpose of this file?

[RedbackThomson]

So we can do relative imports from inside tests. I needed it for from .test_elastic_ip_address import RESOURCE_PLURAL as ELASTIC_IP_PLURAL.

[brycahta]

aws-controllers-k8s/community#1235

[a-hilaly]

Neat!

[brycahta]

/lgtm

[ack-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: A-Hilaly, brycahta, jaypipes, RedbackThomson

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [A-Hilaly,RedbackThomson,brycahta,jaypipes]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment