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.

Sign up for GitHub

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

Jump to bottom

dynu: fix subdomain support #1842

Merged
merged 3 commits into from
Feb 19, 2023
Merged

dynu: fix subdomain support #1842

merged 3 commits into from
Feb 19, 2023

Conversation

SMHRambo
Copy link
Contributor

Hi,

Fixes #1672

It should fix the subdomain issue.
Hope it helps.

@ldez ldez changed the title fix: dynu subdomain support (#1672) dynu: fix subdomain support Feb 19, 2023
SMHRambo
SMHRambo commented Feb 19, 2023
View reviewed changes
Copy link
Contributor Author

@SMHRambo SMHRambo left a comment

Choose a reason for hiding this comment

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

I tested your code change but I get the error "dynu: could not find root domain".
I also tested it with a CNAME recorde, with the same result.
The old code accepted CNAME records without problems.
That being said, DYNU's API or the GetRootDomain function don't seem to like the dot at the end of the fqdn.
I'm not very good at reading go code, it's my second day with go.

@ldez
Copy link
Member

ldez commented Feb 19, 2023 •
edited
Loading

Thank you for the feedback 👍

The old code wasn't working with CNAME, I'm 100% sure.
I think I have to trim the _acme-challenge prefix from the FQDN or just unFQDN the data.

@ldez
Copy link
Member

ldez commented Feb 19, 2023

I changed the code, can you try it?

@SMHRambo
Copy link
Contributor Author

The problem is the dot at the end.
I tested the code and added the string "_acme-challenge" before the domain without a problem.

@ldez
Copy link
Member

ldez commented Feb 19, 2023

So my latest change fixes the problem.

SMHRambo
SMHRambo commented Feb 19, 2023
View reviewed changes
Copy link
Contributor Author

@SMHRambo SMHRambo left a comment

Choose a reason for hiding this comment

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

This is working!

@ldez ldez self-requested a review February 19, 2023 23:54
@ldez ldez added this to the v4.10 milestone Feb 19, 2023
ldez
ldez approved these changes Feb 19, 2023
View reviewed changes
Copy link
Member

@ldez ldez left a comment

Choose a reason for hiding this comment

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

Thank you 👍

@ldez ldez enabled auto-merge (squash) February 19, 2023 23:55
@ldez ldez merged commit 1e2793f into go-acme:master Feb 19, 2023
orangepizza pushed a commit to orangepizza/lego that referenced this pull request Feb 22, 2023
@SMHRambo @ldez @orangepizza
dynu: fix subdomain support (go-acme#1842)
94362c6 
Co-authored-by: Fernandez Ludovic <ldez@users.noreply.github.com>
orangepizza pushed a commit to orangepizza/lego that referenced this pull request Feb 22, 2023
@SMHRambo @orangepizza
dynu: fix subdomain support (go-acme#1842)
0fab042 
Co-authored-by: Fernandez Ludovic <ldez@users.noreply.github.com>

send RST to ACME server so it doens't retry

use not deprecated version of func
orangepizza pushed a commit to orangepizza/lego that referenced this pull request Feb 22, 2023
@SMHRambo @orangepizza
dynu: fix subdomain support (go-acme#1842)
758045d 
Co-authored-by: Fernandez Ludovic <ldez@users.noreply.github.com>

send RST to ACME server so it doens't retry

use not deprecated version of func
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ldez ldez ldez approved these changes

Assignees
No one assigned
Labels
area/dnsprovider bug
Milestone
v4.10
Development

Successfully merging this pull request may close these issues.

Dynu dns-challenge fails creating a certificates for subdomains
2 participants
@SMHRambo @ldez