Update cloudflare record

[angaz]

Thank you for contributing to sewer.
Every contribution to sewer is important to us.
You may not know it, but you have just contributed to making the world a more safer and secure place.

Contributor offers to license certain software (a “Contribution” or multiple
“Contributions”) to sewer, and sewer agrees to accept said Contributions,
under the terms of the MIT License.
Contributor understands and agrees that sewer shall have the irrevocable and perpetual right to make
and distribute copies of any Contribution, as well as to create and distribute collective works and
derivative works of any Contribution, under the MIT License.

Now,

What(What have you changed?)

If you get an error from Cloudflare saying that the record already exists, try to update the record before erroring.

Why(Why did you change it?)

If you have, for example, example.com and *.example.com as records for your certificate, then you will get an error that the record already exists when it tries to make the record for *.example.com.

[mmaney]

@SN9NV sorry, but I'm just stepping in to address some probably similar issues. I have a few questions about cloudflare's API and this patch.

1. I take it that it actually does add the second value when you receive this error? This sounds like it might be similar to an older bug, Batching DNS changes #91 for example.

2. Would it be possible to deal with this "duplicate record" more cleanly for cloudflare if it got all the auth records at one time so that it could find these "duplicates"? Or at least it could know when it was adding a duplicate and expect the non-fatal error.

3. Does this affect deleting records as well as adding them?

Thanks!

[angaz]

Hi,

I'ts been a while since I had this problem, and I'm just using my fork in production.

I think one of the problems was if you wanted to make a cert for example.com and *.example.com, then sewer would try to add 2 records, one for example.com, and one for *.example.com. That makes sense, there's 2 things to validate, but Cloudflare saw this as a conflict and stopped you from being able to add the second record for *.exmaple.com. Then an exception was thrown.

And, I think, a second problem was if something went wrong and the script exists mid-way validating, and a TXT record is left over, then if you try to run the script again, instead of exiting because of an error, it would make more sense to try to update the previous record.

To answer your 3 questions directly:

1. Yes, it does try to add 2 records.
2. If sewer could find duplicates and eliminate them before trying to add the records, I think that would be a good solution. For checking if the record exists, that is what is happening here: https://github.com/komuw/sewer/pull/123/files#diff-299a0a85079042d7998ded262fcc539fR159 If I go to Cloudflare's docs, I can't quickly find where the error messages are, I might have found that error code just by observing what was being returned during testing.
3. I can't remember having an issue with deleting the records. You would expect to have a problem because there's only 1 record, but you're trying to delete 2. Like I said above, I am running my fork in production, and I haven't had an issue with sewer trying to delete records that don't exist. Maybe it's accidentally failing silently.

[mmaney]

@SN9NV thanks much!

On Q2, I don't believe sewer will actually request duplicates... it's just that if the certificate requests both domain.tld and *.domain.tld, there will be two different challenges that need to associate with _acme-challenge.domain.tld. And it would be very odd if cloudflare didn't permit more than one TXT record per name, though the devil, as usual, is in the details. It even makes sense if they internally collect all TXT records for one name into a single value. And that might explain the lack of error when deleting - the first deletion removes the matching value but not the existence of other TXT values.

Okay, this is a mess, but it's much less opaque than before. Thanks again, and I'll pester you when I get back to this. I want to get some things cleaned up and the ACME protocol updated to what LE's staging is using since last December so we can all run real integration tests.

81057, nope, all I saw were a bunch of 1xxx codes...

[komuw]

@mmaney @SN9NV Is the issue been addressed in this PR covered by this other PR that was merged: #163 ?

[mmaney]

@komuw Partly, but not really the hard part. #163 gets rid of Client's habit of sticking "*." back onto the domain after the ACME server has thrown it away. The trickier, and service-specific part, is how to deal with an API like route53's, where some part of the backend keys on just the domain_name and record type. "Real" DNS servers, by which I mean ones that work like good old bind's zone record files consider TXT records distinct based on the domain, type, and the text value.

NB: http-01 doesn't have an issue like this, since the filenames as well as the content are unique per challenge.