Skip to content

New provider and new registrar: hosting.de#1041

Merged
tlimoncelli merged 23 commits into
DNSControl:masterfrom
juliusrickert:http.net
Mar 9, 2021
Merged

New provider and new registrar: hosting.de#1041
tlimoncelli merged 23 commits into
DNSControl:masterfrom
juliusrickert:http.net

Conversation

@juliusrickert
Copy link
Copy Markdown
Member

No description provided.

Copy link
Copy Markdown
Contributor

@tlimoncelli tlimoncelli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great so far! Some minor feedback.

Comment thread providers/httpnet/api.go Outdated
Comment thread providers/httpnet/api.go Outdated
Comment thread providers/httpnet/httpnetProvider.go
@juliusrickert
Copy link
Copy Markdown
Member Author

http.net is connected deeply to hosting.de ("their" consumer-facing business) and they share the same API.

I could add a provider for hosting.de very easily. How should I go about doing this without copying the code?

@tlimoncelli
Copy link
Copy Markdown
Contributor

hosting.de + http.net: I don't have a preference for 1 or 2 providers. Is it likely that they would diverge? If not, one provider is better.

@juliusrickert
Copy link
Copy Markdown
Member Author

I think it is unlikely for them to diverge regarding the DNS and domain APIs.

I am thinking about letting the user specify the API endpoint for maximum flexibility (e.g. ability to use it with their demo systems) and maybe optionally set a different set of default nameservers (honestly not that important as they are the same servers with the same IP addresses).

@tlimoncelli
Copy link
Copy Markdown
Contributor

Allowing the user to specify the API endpoint sounds like the best solution. Many providers permit one to specify the endpoint to change to a testing environment, etc. (Namecheap, NameDotCom, PowerDNS are examples.)

@juliusrickert
Copy link
Copy Markdown
Member Author

Done. Extended the docs as well.

@membero
Copy link
Copy Markdown
Contributor

membero commented Mar 1, 2021

Hello @tlimoncelli,

I am from hosting.de / http.net. We gladly support this plugin. I have talked to @juliusrickert and he will rename/relabel it to hosting.de. It would be great if you could merge this plugin after renaming. We will take over support and further development after that.

membero and others added 2 commits March 1, 2021 17:00
- Fix EnsureDomainExists
- GetNameservers read from NS Records
@tlimoncelli tlimoncelli changed the title New provider and new registrar: http.net New provider and new registrar: hosting.de Mar 1, 2021
Copy link
Copy Markdown
Contributor

@tlimoncelli tlimoncelli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi there!

I'm super excited to hear you'll be supporting this!

I made a few small suggestions about the code. HTH

Comment thread OWNERS Outdated
providers/hedns @rblenkinsopp
providers/hetzner @das7pad
providers/hexonet @papakai
providers/httpnet @juliusrickert
Copy link
Copy Markdown
Contributor

@tlimoncelli tlimoncelli Mar 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change to providers/hostingde ?

Comment thread README.md Outdated
- Hetzner
- HEXONET
- Hurricane Electric DNS
- http.net
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change to - hosting.de / http.net ?

Comment thread docs/provider-list.md Outdated
* `HEDNS` @rblenkinsopp
* `HETZNER` @das7pad
* `HEXONET` @papakai
* `HTTPNET` @juliusrickert
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/HTTPNET/HOSTINGDE/

Contributor Support from hosting.de
@membero
Copy link
Copy Markdown
Contributor

membero commented Mar 1, 2021

Thanks! I have updated these lines.


**If you want to use this provider with http.net or a demo system you need to provide a custom `baseURL`.**

* http.net: `https://partner.http.net`
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: Add a bullet:

  • Hosting.de: default

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

jsId: hostingde
---
# hosting.de Provider

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: Add a paragraph saying that this provider suports the API used by Hosting.de and http.net, which both use the same API. (or whatever)

Copy link
Copy Markdown
Contributor

@membero membero Mar 4, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not think that this is neccessary. http.net only provides registration services for bigger companies. Also http.net is mentioned later in the documentation.

Comment thread providers/hostingde/api.go Outdated
func (hp *hostingdeProvider) getDomainConfig(domain string) (*domainConfig, error) {
zc, err := hp.getZoneConfig(domain)
if err != nil {
return nil, fmt.Errorf("error getting zone config: %v", err)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use %w instead of %v

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. I have replaced all of them.

Comment thread providers/hostingde/api.go Outdated

resp, err := hp.get("domain", "domainsFind", params)
if err != nil {
return nil, fmt.Errorf("error getting domain info: %v", err)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use %w instead of %v

Comment thread providers/hostingde/api.go Outdated

domainConf := []*domainConfig{}
if err := json.Unmarshal(resp.Data, &domainConf); err != nil {
return nil, fmt.Errorf("error parsing response: %v", err)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use %w instead of %v

Comment thread providers/hostingde/api.go Outdated

zc := []*zoneConfig{}
if err := json.Unmarshal(resp.Data, &zc); err != nil {
return nil, fmt.Errorf("could not parse response: %v", err)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use %w instead of %v

Comment thread providers/hostingde/types.go Outdated
// Nothing special.
case "MX":
record.Priority = rc.MxPreference
record.Content = rc.Target
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than rc.Target, please use rc.GetTargetFrield()

For TXT records, please use .TxtStrings (which may require special handling, since it is a []string)

TXT handling will change in a few days. It might break this code. See #1063

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Once you merge the tlim_target_and_txt branch I will create an auditrecords.go.

membero and others added 3 commits March 4, 2021 13:46
replaced %v with %w for errors
special handling for txt records using .TxtStrings
@tlimoncelli
Copy link
Copy Markdown
Contributor

Please rebase (i.e. get merge master) and go generate

@tlimoncelli tlimoncelli merged commit c883c1a into DNSControl:master Mar 9, 2021
@tlimoncelli
Copy link
Copy Markdown
Contributor

Yay! Another new provider! Thank you for contributing this!

@membero membero deleted the http.net branch March 9, 2021 08:50
tlimoncelli added a commit that referenced this pull request Mar 12, 2021
* Add http.net provider

* Rename httpnetProvider

* Add SSHFP capability

* Add paging for records

* Sort documentation notes alphabetically

* Add custom base URL

* Extend documentation for custom base URL

* - renamed to hosting.de
- Fix EnsureDomainExists
- GetNameservers read from NS Records

* Replaced http.net with hosting.de
Contributor Support from hosting.de

* baseURL for hosting.de in documentation
replaced %v with %w for errors
special handling for txt records using .TxtStrings

* removed last references to rc.Target
fixed Trim of last dot

* Re-engineer TXT records for simplicity and better compliance (#1063)

Co-authored-by: Tom Limoncelli <tlimoncelli@stackoverflow.com>
Co-authored-by: Oliver Dick <o.dick@hosting.de>
Co-authored-by: Oliver Dick <31733320+membero@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants