Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: migrate to aws-sdk-go-v2 (lightsail, route53) #1973

Merged
merged 4 commits into from
Jul 27, 2023

Conversation

ldez
Copy link
Member

@ldez ldez commented Jul 25, 2023

The aws-sdk-for-go v2 is available since 2021/01/19

As the SDK has been split into a constellation of modules, this will potentially reduce the size of the dependencies.
(Edit: after a comparison this increase the size of the binary...)

It's a pure technical migration, I haven't tested the providers in a real context.

Fixes #1743

dmke

This comment was marked as outdated.

@ldez ldez added the state/need-user-tests Need users to test functionality label Jul 25, 2023
@dmke
Copy link
Member

dmke commented Jul 26, 2023

I don't think Github issues notifications when editing a comment, only when creating it. Let me try that again:

Pinging some people who have contributed in the past to these providers:

Has anyone of you a bit free of time on their hands and still have access to Route53 or Lightsail and could check if all still work?

@kingcdavid
Copy link
Contributor

Not sure if it helps but a build with the new AWS SDK V2 worked for me

lego git:(feat/migrate-aws-sdk-v2) ./dist/lego --domains <redacted> --dns route53 -m <redacted> run
2023/07/26 20:39:49 [INFO] [<redacted>] acme: Obtaining bundled SAN certificate
2023/07/26 20:39:50 [INFO] [<redacted>] AuthURL: https://acme-v02.api.letsencrypt.org/acme/authz-v3/<redacted>
2023/07/26 20:39:50 [INFO] [<redacted>] acme: Could not find solver for: tls-alpn-01
2023/07/26 20:39:50 [INFO] [<redacted>] acme: Could not find solver for: http-01
2023/07/26 20:39:50 [INFO] [<redacted>] acme: use dns-01 solver
2023/07/26 20:39:50 [INFO] [<redacted>] acme: Preparing to solve DNS-01
2023/07/26 20:39:51 [INFO] Wait for route53 [timeout: 2m0s, interval: 4s]
2023/07/26 20:40:26 [INFO] [<redacted>] acme: Trying to solve DNS-01
2023/07/26 20:40:26 [INFO] [<redacted>] acme: Checking DNS record propagation using [127.0.0.53:53]
2023/07/26 20:40:30 [INFO] Wait for propagation [timeout: 2m0s, interval: 4s]
2023/07/26 20:40:30 [INFO] [<redacted>] acme: Waiting for DNS record propagation.
2023/07/26 20:40:34 [INFO] [<redacted>] acme: Waiting for DNS record propagation.
2023/07/26 20:40:38 [INFO] [<redacted>] acme: Waiting for DNS record propagation.
2023/07/26 20:40:48 [INFO] [<redacted>] The server validated our request
2023/07/26 20:40:48 [INFO] [<redacted>] acme: Cleaning DNS-01 challenge
2023/07/26 20:40:49 [INFO] Wait for route53 [timeout: 2m0s, interval: 4s]
2023/07/26 20:41:15 [INFO] [<redacted>] acme: Validations succeeded; requesting certificates
2023/07/26 20:41:16 [INFO] [<redacted>] Server responded with a certificate.

though i couldn't get the integration tests to pass, but that's probably more a config error

@dmke
Copy link
Member

dmke commented Jul 26, 2023

Thank you very much, @kingcdavid!

As for the integration test, I believe you need to call something like:

AWS_ACCESS_KEY_ID="..." \
AWS_SECRET_ACCESS_KEY="..." \
AWS_REGION="..." \
R53_DOMAIN="..." \
  go test -v -run TestLiveTTL github.com/go-acme/lego/v4/providers/dns/route53

where the AWS_* credentials are per usual, and R53_DOMAIN is the domain you have access to.

You can omit the -run TestLiveTTL to run all Route53 tests, not just the integration test.

@kingcdavid
Copy link
Contributor

kingcdavid commented Jul 26, 2023

I tried that, but it still skipped the test, possibly need to add envDomain into the NewEnvTest?
If i do this, i can get the test to pass

diff --git a/providers/dns/route53/route53_test.go b/providers/dns/route53/route53_test.go
index 1c8e5f5f..c6028692 100644
--- a/providers/dns/route53/route53_test.go
+++ b/providers/dns/route53/route53_test.go
@@ -25,7 +25,8 @@ var envTest = tester.NewEnvTest(
        EnvMaxRetries,
        EnvTTL,
        EnvPropagationTimeout,
-       EnvPollingInterval).
+       EnvPollingInterval,
+       envDomain).
        WithDomain(envDomain).
        WithLiveTestRequirements(EnvAccessKeyID, EnvSecretAccessKey, EnvRegion, envDomain)

Then the tests pass

lego git:(feat/migrate-aws-sdk-v2) ✗ go test -v -run TestLiveTTL github.com/go-acme/lego/v4/providers/dns/route53
=== RUN   TestLiveTTL
2023/07/26 21:26:14 [INFO] Wait for route53 [timeout: 2m0s, interval: 4s]
2023/07/26 21:26:47 [INFO] Wait for route53 [timeout: 2m0s, interval: 4s]
--- PASS: TestLiveTTL (56.58s)
PASS
ok  	github.com/go-acme/lego/v4/providers/dns/route53	56.588s

also the route53.toml may need updating referencing the new SDK?

@ldez
Copy link
Member Author

ldez commented Jul 27, 2023

I updated the links.

enDomain should not be inside the constructor because it's handled by WithDomain, the problem is not here.

only EnvAccessKeyID, EnvSecretAccessKey, EnvRegion, envDomain are required to run the test.

Edit: I found and fix the problem.

@ldez
Copy link
Member Author

ldez commented Jul 27, 2023

Thank you a lot @kingcdavid ❤️

@dmke I think we can merge now: route53 was the more impacted by the migration and for lightsail it's mainly just modification of package names.

Copy link
Member

@dmke dmke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, let's merge this :-)

@ldez ldez removed the state/need-user-tests Need users to test functionality label Jul 27, 2023
@ldez ldez merged commit fc47c35 into go-acme:master Jul 27, 2023
10 checks passed
@ldez ldez deleted the feat/migrate-aws-sdk-v2 branch July 27, 2023 10:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

Update aws-sdk-go to v2
3 participants