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
INWX: support changing from null MX to non-null MX #2800
Comments
CC @blackshadev (author of dnsgraph, which decides the ordering of updates) |
I am interested to know if any other provider has the same issue. I do not (yet) have any exceptions in the ordering flow for any provider. And it seems weird to me to make an exception since technically you can depent on empty records and the diff shown below should work in most systems. I don't know of the INWX maintainer (@patschi) agrees, but it seems INWX specific and thus is should be best handled as an exception in the provider. My suggestion would be to detect the case in the provider : modify-ing an empty MX record. And treat is as a Delete + Create. The TransIP also have some weird edgecases were I ended up deleting and recreating the record instead. |
On one hand....it does seem prudent that an NullMX should have a different weight than regular MX. On the other hand... how often will this happen? Is there a sufficient workaround? (The workaround would be to delete all MX records in one |
I'd like to see if its possible to create an integration test that detects this situation. I created this: #2801 Could someone with an INWX account try running the integration tests from that branch? Tom P.S. The providers I do have creds for all passed the test: https://github.com/StackExchange/dnscontrol/actions/runs/7559961364/job/20584943571?pr=2801 |
Seems to work (the test itself at least):
TRANSIP seems to fail the test as well: https://github.com/StackExchange/dnscontrol/actions/runs/7559961364/job/20585007084?pr=2801 If we prefer having some sort of workaround in INWX provider itself, I can try to work something out. @tlimoncelli Is the CI running basically the integrationTest just like when running them locally? I could then create a new account with INWX with one test domain and add it to the CI pipeline. So that we have this covered as well. |
@patschi Yes, the CI is running the same integrationTest. If you'd like to send me creds, please follow these instructions: https://docs.dnscontrol.org/developer-info/byo-secrets (this is a new doc; if you see something that needs correcting please let me know!) |
Since TRANSIP also fails the test (link) I lean towards fixing this in dnsgraph. If 2 have this issue, that's a trend. I'm concerned (but not certain!) that fixing it at the dnsgraph level might not help INWX. INWX uses |
Looks like almost all providers seem to use the same? Shouldn't all providers then be adjusted to the new diff2 code to be consistent? (Was looking for some example code how I can convert it) |
@blackshadev hmm... this is interesting! The BIND provider outputs this message: https://github.com/StackExchange/dnscontrol/actions/runs/7559961364/job/20585004068?pr=2801
I'm seeing the same message in NAMEDOTCOM, HEXONET, HEDNS, and then I stopped looking. |
The diff.NewCompat() call is a bandaid. I wrote that so that I could get rid of the old
I think INWX would use |
This is a different "issue" all together, one I cannot fix: TransIP Will not allow an empty MX record. So the whole test setup doesn't even work for TransIP. I suspect most providers will not accept an empty MX record and thus fail the test.
Hmm very weird, I hope I can take a crack at this issue upcoming friday. It is weird, there shouldn't be a difference in the behavior of providers, but for some reason there is? |
About TransIP: Ooops! My bad. Yes, TransIP is a non-issue as NullMX is not processed. |
@tlimoncelli I am unable to reproduce your issue with the integration test on the BIND provider:
I also ran it without the start parameter. But everything runs fine without warning. What did you do differently? I am on the main branch commit d926e45 |
Welp, now I can't reproduce it either. I even re-ran it in GHA: https://github.com/StackExchange/dnscontrol/actions/runs/7586343357/job/20664596836 :confused emoji: Ok, let's say that was a fluke. That said, I'm still interested in seeing if the original bug can be solved at the dnsgraph level. It is similar to the fact that when converting an A record to a CNAME yet a challenge because it is different. |
Wel given that it works as expected on BIND. How do we expect it to work any other way on INWX any differently? If @patschi uses the diff2 algorithm DNSGraph should sort things out right? If not, we need a way for me to debug and run some test suite. For now: My understanding is without using diff2 , it will not use DNSGraph . So lets do that first and check for the result afterward. Or am I missing the point completely? |
I think the next step should be to convert INWX to use diff2. If that fixes the NullMX problem, we've killed two birds with one stone. If @patschi is interested in doing the conversion, that'd be great. If you want to add a test account (https://docs.dnscontrol.org/developer-info/byo-secrets) it would enable me to help out. (Don't let the long doc fool you... the actual work to set this up is rather small.) If converting to diff2 doesn't work... then we'll loop back to @blackshadev with a repro case. :) |
@patschi To securely send the credentials for use with the project, use this link: https://transfer.secretoverflow.com/u/tlimoncelli (I should be able to get to it in the next week) |
Describe the bug
With INWX, when changing from null MX to non-null MX records, the ordering matters because INWX enforces that you cannot add further MX records while you have a null MX record
dnscontrol does not seem to handle this, leading to try to add non-null MX records before the null MX record is modified
To Reproduce
Steps to reproduce the behavior:
#1: + CREATE MX example.org 10 alt3.aspmx.l.google.com. ttl=3600
FAILURE! (2308) Data management policy violation
#2: + CREATE MX example.org 10 alt4.aspmx.l.google.com. ttl=3600
FAILURE! (2308) Data management policy violation
#3: + CREATE MX example.org 5 alt1.aspmx.l.google.com. ttl=3600
FAILURE! (2308) Data management policy violation
#4: + CREATE MX example.org 5 alt2.aspmx.l.google.com. ttl=3600
FAILURE! (2308) Data management policy violation
...
#6: ± MODIFY MX example.org: (0 . ttl=3600) -> (1 aspmx.l.google.com. ttl=3600)
SUCCESS!
Expected behavior
The existing null MX record should be modified first so that the subsequent additions do not fail
DNS Provider
INWX
The text was updated successfully, but these errors were encountered: