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

dns_hostingde.sh zoneConfig missing templateValues object #2207

Merged

Conversation

Kimmax
Copy link
Contributor

@Kimmax Kimmax commented Apr 2, 2019

These changes are not yet reflected in the official API documentation, but I could verify that this field is required by testing and via mail exchange with an employee.

@Neilpang
Copy link
Member

Neilpang commented Apr 3, 2019

Does this change break the current official api ?

@Kimmax
Copy link
Contributor Author

Kimmax commented Apr 3, 2019

It is already broken, this change should fix it again. I tested it on both a domain which is tied to a template and with one that is not, both worked fine. Ran it against 32 domains after that, did also work without problems
I can of course only speak for me on the http.net endpoint, maybe someone else can test this too?

@Neilpang Neilpang merged commit 28694e8 into acmesh-official:dev Apr 26, 2019
@membero
Copy link
Contributor

membero commented May 7, 2019

Wonderful. I had not in mind that zones might be tied to templates. I can confirm that this patch fixes the problem also for the hosting.de endpoint as both endpoints point to the same system.

@membero
Copy link
Contributor

membero commented May 8, 2019

Hello @Kimmax,

there is a bug report with your PR: #2058

I tested a little bit and if I replace _hostingde_parse_no_strip_whitespace with _hostingde_parse, everything runs fine (with and without a template). Could you explain why you implemented _hostingde_parse_no_strip_whitespace?

Could you please test the attached test?

hostingde-template-patch.txt

@Kimmax
Copy link
Contributor Author

Kimmax commented Jun 3, 2019

@membero Hey, sorry for the late response, I have been busy all month. I'll look into this as soon as I can!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants