Skip to content

INWX: Null MX is permitted#2687

Closed
tlimoncelli wants to merge 3 commits intomasterfrom
tlim_b2643_inwx_nullmx
Closed

INWX: Null MX is permitted#2687
tlimoncelli wants to merge 3 commits intomasterfrom
tlim_b2643_inwx_nullmx

Conversation

@tlimoncelli
Copy link
Copy Markdown
Contributor

@tlimoncelli tlimoncelli commented Dec 5, 2023 •
edited
Loading

Fixes #2643

@tlimoncelli
Copy link
Copy Markdown
Contributor Author

tlimoncelli commented Dec 5, 2023

gvangool and @patschi: Please take a look.

@tlimoncelli tlimoncelli mentioned this pull request Dec 5, 2023
Closed
@gvangool
Copy link
Copy Markdown
Contributor

gvangool commented Dec 7, 2023 •
edited
Loading

gvangool and @patschi: Please take a look.

I've created a test account (on https://www.ote.inwx.de/) and tried this out.
It results in a failure:

=== RUN   TestDNSProviders/phyx-ote.dev/15:MX:Null_MX
    integration_test.go:247:
        + CREATE MX phyx-ote.dev 0 . ttl=300
    integration_test.go:252: (2005) Parameter value syntax error. Reason: (The given parameter value for 'content' does not match the expected parameter type 'text') The given parameter value for 'content'
 does not match the expected parameter type 'text'

Full log at https://gist.github.com/gvangool/73b6e1ebbf827894660fcedf14824f72

@tlimoncelli
Copy link
Copy Markdown
Contributor Author

tlimoncelli commented Dec 7, 2023

That's an odd error message! The type is wrong? Maybe no field is being sent (noempty) therefore the type is not "string".

Take a look at providers/inwx/inwxProvider.go and add a debug printf something like...

  case "MX":
    req.Priority = int(rec.MxPreference)
    req.Content = content[:len(content)-1]
    printer.Printf("DEBUG: inwx mx=%q\n", req.Content)

That might shed some light on the situation.

@gvangool
Copy link
Copy Markdown
Contributor

gvangool commented Dec 8, 2023

That gives the following (partial) log:

=== RUN   TestDNSProviders/phyx-ote.dev/15:MX:Null_MX
    integration_test.go:247:
        + CREATE MX phyx-ote.dev 0 . ttl=300
DEBUG: inwx mx=""
    integration_test.go:252: (2005) Parameter value syntax error. Reason: (The given parameter value for 'content' does not match the expected parameter type 'text') The given parameter value for 'content'
 does not match the expected parameter type 'text'

Looking at the way the trailing dot is handled for INWX, I ended up with this patch to fix it

diff --git i/providers/inwx/inwxProvider.go w/providers/inwx/inwxProvider.go
index c511d85b..23d3a233 100644
--- i/providers/inwx/inwxProvider.go
+++ w/providers/inwx/inwxProvider.go
@@ -180,7 +180,11 @@ func makeNameserverRecordRequest(domain string, rec *models.RecordConfig) *goinw
                req.Content = content[:len(content)-1]
        case "MX":
                req.Priority = int(rec.MxPreference)
-               req.Content = content[:len(content)-1]
+               if content == "." {
+                       req.Content = content
+               } else {
+                       req.Content = content[:len(content)-1]
+               }
        case "SRV":
                req.Priority = int(rec.SrvPriority)
                req.Content = fmt.Sprintf("%d %d %v", rec.SrvWeight, rec.SrvPort, content[:len(content)-1])
@@ -305,7 +309,11 @@ func (api *inwxAPI) GetZoneRecords(domain string, meta map[string]string) (model
                        "PTR":   true,
                }
                if rtypeAddDot[record.Type] {
-                       record.Content = record.Content + "."
+                       if record.Type  == "MX" && record.Content == "." {
+                               // null records don't need to be modified
+                       } else {
+                               record.Content = record.Content + "."
+                       }
                }

                rc := &models.RecordConfig{

@tlimoncelli
Copy link
Copy Markdown
Contributor Author

tlimoncelli commented Dec 8, 2023

Excellent detective work!

Would you please submit a PR?

@gvangool gvangool mentioned this pull request Dec 8, 2023
Merged
@tlimoncelli tlimoncelli closed this Dec 8, 2023
@tlimoncelli tlimoncelli deleted the tlim_b2643_inwx_nullmx branch December 8, 2023 17:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

provider-INWX

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

INWX: support null MX records

3 participants

@tlimoncelli @gvangool @cafferata