-
-
Notifications
You must be signed in to change notification settings - Fork 173
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
Add NameSilo provider #866
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi there! Thanks for the PR! Nice code and thank you for the great documentation!
I have a few comments here and there, but overall it's pretty much already good code 😉 💯 !
### Compulsory parameters | ||
|
||
- `"domain"` is the domain to update. It can be `example.com` (root domain), `sub.example.com` (subdomain of `example.com`) or `*.example.com` for the wildcard. | ||
- `"key"` is the NameSilo API Key obtained using domain setup instructions below. Example: `71dZaE8c2Aa926Ca2E8c1`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- `"key"` is the NameSilo API Key obtained using domain setup instructions below. Example: `71dZaE8c2Aa926Ca2E8c1`. | |
- `"key"` is the NameSilo API Key obtained using the domain setup instructions below. For example: `71dZaE8c2Aa926Ca2E8c1`. |
## Domain setup | ||
|
||
1. Login to the [Namesilo API Manager](https://www.namesilo.com/account/api-manager) with your account credentials. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit no need for an empty line
## Testing | ||
|
||
1. Go to the [NameSilo Domain Manager](https://www.namesilo.com/account_domains.php). | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit no need for an empty line
ttl *uint32 | ||
} | ||
|
||
type APIResponse struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do not export this, rename to apiResponse
case ttl != nil && *ttl < minTTL: | ||
return fmt.Errorf("%w: %d must be at least %d", errors.ErrTTLTooLow, *ttl, minTTL) | ||
case ttl != nil && *ttl > maxTTL: | ||
return fmt.Errorf("%w: %d must be at least %d", errors.ErrTTLTooHigh, *ttl, maxTTL) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return fmt.Errorf("%w: %d must be at least %d", errors.ErrTTLTooHigh, *ttl, maxTTL) | |
return fmt.Errorf("%w: %d must be at most %d", errors.ErrTTLTooHigh, *ttl, maxTTL) |
return nil, fmt.Errorf("%w: %d: %s", | ||
errors.ErrHTTPStatusNotValid, response.StatusCode, utils.ToSingleLine(string(data))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do they return any structured error body i.e. json when the status code isn't ok?
Also, when the record is created, do they return status ok or status created (201)?
|
||
err = p.validateResponseCode(parsedResponse.Reply.Code, parsedResponse.Reply.Detail) | ||
if err != nil { | ||
return nil, err |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return nil, err | |
return nil, fmt.Errorf("validating reply code: %w", err) |
// The API inconsistently swaps between number and string typing for the code field, | ||
// but the value should always be an integer. | ||
parsedCode, err := code.Int64() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💯 !
return fmt.Errorf("%w: %d: %s", err, parsedCode, detail) | ||
} | ||
|
||
// Unknown response code |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit unneeded comment
// Unknown response code |
[![Before NameSilo API Key](../readme/namesilo1.png)](https://www.namesilo.com/account/api-manager) | ||
[![After NameSilo API Key](../readme/namesilo2.png)](https://www.namesilo.com/account/api-manager) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you so much for taking the extra time to do screenshots! However, being mindful of the size of repo, would you mind compressing them in jpeg format so save a few kilobytes (since png is lossless)? Thanks!!
Adds support for NameSilo. Resolves #313
I have tested locally and it seems to work for my domains. Wildcards and IPv6 are supported. The provider will create a new record if one does not already exist.
With credit to @Zeustopher for writing most of the readme in this comment.
The API documentation is here for reference: NameSilo API Reference.