Feature: Pass in a pre-configured route53 boto3 client to Route53Dns provider

[tkalus]

Which version of python are you using?

Python 3.8.3

What operating system and version of operating system are you using?

macOS, Ubuntu 20.04, Amazon Linux 2

What version of sewer are you using?

0.8.2

What is it that you would like to propose to add/remove/change?

Ability to pass in an pre-configured Route53 boto3 client to the Route53Dns provider.

Why do you want to add/remove/change that?

There are limitations in the current implementation that it would solve:

• use of STS credentials (can't currently pass the session token).
• where a AssumeRole is necessary (E.G. Hosted Zone in a different AWS Account).

How do you want to go about adding/removing/changing that?

• Add a client=None kwarg to Route53Dns's __init__() function.
• Check for exclusivity between passed AK/SK and client (Exception if both are passed).
• Check/set class instance's self.r53 to be the passed client, if appropriate.
• Add'l unit tests covering new case(s).

[mmaney]

@tkalus Would you like to contribute a code sketch of how this would be used? I'm trying to work up better documentation in general, so I'd prefer to have some in hand for new additions like this. Maybe expand the Why section a little, or is that going to be obvious to someone who uses route53?

@lowid @hobosteaux You-all have worked on the route53 driver, any comments on this change?

Thanks!

[tkalus]

@mmaney I'd be happy to provide a code sketch, but it would be little more than instantiating a boto3.client() object and passing it into the Route53Dns class. Where other dns providers might have a single way of authn'ing into their APIs, AWS has a handful of ways and the proposed PR #198 opens up a few more use-cases.

While I can't say that the "Whys" will be remarkably obvious to every AWS user, it will be greatly appreciated by those of us who have somewhat advanced use-cases and/or restrictions around how we use credentials with AWS.

Sample code-sketch for Cross Account AssumeRole where Route53 is in a different AWS Account:

import boto3
import sewer.client
import sewer.dns_providers.route53

# ARN of the Role that has permissions to mutate the Route53 Hosted Zone.
role_arn = "arn:aws:iam::123456789012:role/route53_mutate"

# let boto3 find the initial credentials.
sts_client = boto3.client("sts")

# AssumeRole and get credentials that allow us to mutate Route53.
credentials = sts_client.assume_role(
    RoleArn=role_arn,
    RoleSessionName="sewer_le_client"
).get("Credentials")

# Make a route53 boto3 client we can pass into the `Route53Dns` provider.
route53_client = boto3.client(
    "route53",
    aws_access_key_id=credentials["AccessKeyId"],
    aws_secret_access_key=credentials["SecretAccessKey"],
    aws_session_token=credentials["SessionToken"],
)

dns_class = sewer.dns_providers.route53.Route53Dns(client=route53_client)

# 1. to create a new certificate:
client = sewer.client.Client(
    domain_name='example.com',
    dns_class=dns_class
)

...
[snip -- identical to README.md]

[tkalus]

Hey @mmaney would any additional detail on this use-case be helpful? I'm looking to understand if this can be unblocked or if I'm going to have to take a different strategy to cover my use-case.

[mmaney]

@tkalus no, sorry, this has just had the bad luck to arrive when I'm trying to clean up the loose ends... and it turns out that one of the loose ends is that the route53 driver was never wired into cli.py. So I've been poking at the docs and looking at the code, trying to work out how to handle it in the new context (which you can see in #200, though it will change again before it's ready to merge). I think your PR will just merge cleanly on top of that.

[mmaney]

Picking up the threads I dropped until after 0.8.3 got released. Can I ask you for some further work to cleanup this driver?

If I understand this correctly, the driver [now] supports three ways of authenticating API access: your new client parameter, the older access_key_id / secret_key_id pair of parameters (1), or some sort of boto/AWS magic. :-) I'd like to see that one-of-three logic more clearly, with the consistency checks integrated. It might be reasonable to just log a warning about ignoring id/password when you're using the client parameter (I think if client: should come first), but an exception if only one of id/password was passed (the elif step), and the AWS magic only when none of the three were passed.

(1) and BTW, are those names from the AWS documentation? Looks like (semantically) id and password to me. :-)

That does seem to leave self.aws_config as the unloved stepchild, but apparently that's only used in these setup steps? If so, it should just be a local variable, not a class namespace clutterer. I'm sorry, I know you didn't start this, but I really hate making changes that move things around so much to a driver that I can't test myself.

The last thing that comes to mind is that in 0.8.3 the nonsense that made that "import inside try and report error later" dance necessary is gone, ripped up like the ugly kluge that it was. So you could put that back to just being an import and let it fail loudly up front.

Thanks for as much of this as you feel like helping with, and for helping me understand more about AWS. Maybe more than I'd wanted to know, but that seems to be what's needed here. .

[tkalus]

Hey @mmaney, I'd be happy to work toward cleaning up this driver.

Your assessment is spot-on on all counts and I think you're right on the ordering, logging, and exception paths; my add was in the vein of "minimal disruption", but I can work my PR into something a bit more meaningful and solid. I'm using a derivation of this in a production scenario and I'd love to contribute my changes back upstream, if they make sense.

As far a pushing work back upstream, I'm thinking I'll rebase #198 master and use that to add boto3.client support and do the tidying you've mentioned. I'll open a second PR with the improvements I'm using locally (E.G. "wait for Route 53 to converge globally before pinging back into LE to check on auth state", which'll be a fun unit test!).

Thoughts?

[mmaney]

/me jumps up and down! Yes, all of that! And I want to ask about that waiting - is this the first driver that has a way to support the unpropagated method in ProviderBase?

It looks like route53 is one of those that does some additional querying of the service in order to match the domain up with a zone. If you're interested in amortizing at least some of that work, the new-model interface changes the setup and teardown by passing all the names on the certificate in one batch. Apparently that's the only carrot I have to dangle to encourage migration of legacy DNS drivers, since aliasing and the propagation check can be added to the old drivers after all.