Webhook stops when there are no records for a domain at netcup

[webwurst]

Thanks for providing this webhook :)

I think I found an issue:

level=debug ts=2024-01-22T22:17:21.637774483Z caller=netcup.go:274 msg="performing login to Netcup DNS API"
level=debug ts=2024-01-22T22:17:21.898232064Z caller=netcup.go:280 msg="successfully logged in to Netcup DNS API"
time="2024-01-22T22:17:22Z" level=error msg="Failed to get Records: unable to get DNS records for domain 'replaced-example.com': InfoDnsRecords failed: (5029) 'error' 'Getting DNS records failed' 'Can not get DNS records for zone.  The zone does not webwur.st contain any DNS records.'"

In case all records are deleted for a domain the Netcup DNS API fails instead of responding with an empty result when trying to list the records.

And it seems this webhook then just stops. It should be fine to ignore this error and just go on with creating the records collected from the Kubernetes sources.

[mrueg]

Good catch, I never tried it with a completely empty domain. It should be easy to reproduce, so hopefully can provide a quick fix in the next days.

[mrueg]

@aellwein do you know if there's a way to detect that specific error in the api client? ? Otherwise I guess I'll remove the error handling around InfoDnsRecords() to make this work.

[aellwein]

@mrueg unfortunately, the Netcup DNS API response is very unspecific and sometimes misleading; fmpov this particular one should be a "warning" type of response, not an error.

In the API client, however, the response is assumed to be a fatal one, if it's not a "success". An error is passed in any non-"success" type of response.

A suggestion: i can extend the API client to pass through the NetcupBaseResponseMessage as an additional return type from any API call. The API's user can then inspect the error code and status on his own.

This would be a breaking change for the client and require a new release with a major version increase.
As a "dirty" workaround, the error string could be parsed for the return code/status -- i would not suggest it, since the error message itself is not part of the client's API.

So i would rather proceed with the API client's extension.
WDYT?

[aellwein]

@mrueg on a second thought: you could check the LastResponse attribute of the NetcupSession, it contains exactly the as-is response from Netcup DNS API.

Hint: probably you'll need to add a treatment of (response.Status == string(netcup.StatusError) && response.StatusCode == 5029).

Maybe we don't need the extension of API client, but to document how the potential error could be extracted.

[mrueg]

@mrueg on a second thought: you could check the LastResponse attribute of the NetcupSession, it contains exactly the as-is response from Netcup DNS API.

Hint: probably you'll need to add a treatment of (response.Status == string(netcup.StatusError) && response.StatusCode == 5029).

Maybe we don't need the extension of API client, but to document how the potential error could be extracted.

I'll take a look and see if that works. Otherwise your other suggestion sounds good to me as well.

[mrueg]

Please let me know if prerelease 0.0.7 resolves the issue for you.

[webwurst]

With the container image of 0.0.7 I get this in a row somehow:

level=debug ts=2024-02-04T02:46:19.547109402Z caller=netcup.go:282 msg="performing login to Netcup DNS API"
level=debug ts=2024-02-04T02:46:19.758539913Z caller=netcup.go:288 msg="successfully logged in to Netcup DNS API"
level=debug ts=2024-02-04T02:46:20.18907635Z caller=netcup.go:94 msg="no records exist" domain=webwur.st error="InfoDnsRecords failed: (5029) 'error' 'Getting DNS records failed' 'Can not get DNS records for zone.  The zone does not webwur.st contain any DNS records.'"
level=info ts=2024-02-04T02:46:20.189129089Z caller=netcup.go:99 msg="got DNS records for domain" domain=webwur.st
2024/02/04 02:46:20 http: panic serving [::1]:59910: runtime error: invalid memory address or nil pointer dereference
goroutine 529 [running]:
net/http.(*conn).serve.func1()
        /usr/local/go/src/net/http/server.go:1868 +0xb9
panic({0xbe75c0?, 0x15f7800?})
        /usr/local/go/src/runtime/panic.go:920 +0x270
github.com/mrueg/external-dns-netcup-webhook/provider.(*NetcupProvider).Records(0xc0000ac700, {0x8?, 0xcb16a4?})
        /app/provider/netcup.go:100 +0x60a
sigs.k8s.io/external-dns/provider/webhook/api.(*WebhookServer).RecordsHandler(0x10?, {0x1072e20, 0xc000234000}, 0x1611740?)
        /go/pkg/mod/sigs.k8s.io/[email protected]/provider/webhook/api/httpapi.go:45 +0x71
net/http.HandlerFunc.ServeHTTP(0x445280?, {0x1072e20?, 0xc000234000?}, 0x6baeda?)
        /usr/local/go/src/net/http/server.go:2136 +0x29
net/http.(*ServeMux).ServeHTTP(0x1641240?, {0x1072e20, 0xc000234000}, 0xc0008ae000)
        /usr/local/go/src/net/http/server.go:2514 +0x142
net/http.serverHandler.ServeHTTP({0xc0000f0090?}, {0x1072e20?, 0xc000234000?}, 0x6?)
        /usr/local/go/src/net/http/server.go:2938 +0x8e
net/http.(*conn).serve(0xc0000a6120, {0x1074920, 0xc000508d20})
        /usr/local/go/src/net/http/server.go:2009 +0x5f4
created by net/http.(*Server).Serve in goroutine 12
        /usr/local/go/src/net/http/server.go:3086 +0x5cb

After adding a dummy entry via netcup web-ui it works fine.

[aellwein]

@mrueg i think the API client does not return an empty *[]DnsRecord set in any case, sometimes it's just nil: https://github.com/aellwein/netcup-dns-api/blob/219cb99d019322b027001984eaf9c0aea54d6e76/pkg/v1/api.go#L318

We could add an additional check for the pointer before iterating, or maybe adapt the API client to return a pointer to an an empty slice in any case to prevent such errors. WDYT?

Update: i've released the client 1.0.4 with mentioned improvements, any feedback is welcome, thanks.

[mrueg]

Thanks @aellwein for the fix in the library!

@webwurst can you test with the following release please?
https://github.com/mrueg/external-dns-netcup-webhook/releases/tag/0.0.8

[mrueg]

Closing this as I haven't received a response.