Skip to content

feat(provider): add support for Vultr - #829

Merged
qdm12 merged 40 commits into
qdm12:masterfrom
amroessam:master
Oct 20, 2024
Merged

feat(provider): add support for Vultr#829
qdm12 merged 40 commits into
qdm12:masterfrom
amroessam:master

Conversation

@amroessam

@amroessam amroessam commented Oct 2, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

First time writing go, this was easy thanks to great docs and example 🙏🏽
Go easy on the roast 😛

Vultr DNS API docs for ref

Excited for the merge, so I can use this instead of my janky script https://github.com/amroessam/vultr-ddns

@amroessam

amroessam commented Oct 2, 2024

Copy link
Copy Markdown
Contributor Author

Preemptively answering these questions

  1. Yes, the default behavior is to create the record if it doesn't exist
  2. Yes, I've tested it
  3. Wildcards are supported but only for 1 record type, if it errors out it should reflect the error

@amroessam

amroessam commented Oct 4, 2024

Copy link
Copy Markdown
Contributor Author

@qdm12 any luck getting to this? 🙏🏽

qdm12
qdm12 requested changes Oct 8, 2024
View reviewed changes

@qdm12 qdm12 left a comment

Copy link
Copy Markdown
Owner

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 and congrats on your first few Go steps! 💯
Looking pretty good, with a few minor changes to make 😉

Comment thread .gitignore Outdated
Comment thread docs/vultr.md Outdated
Comment thread docs/vultr.md Outdated
Comment thread docs/vultr.md Outdated
Comment thread docs/vultr.md Outdated
Comment thread internal/provider/providers/vultr/provider.go
Comment thread internal/provider/providers/vultr/provider.go Outdated
Comment thread internal/provider/providers/vultr/provider.go Outdated
Comment thread internal/provider/providers/vultr/provider.go Outdated
Comment thread internal/provider/providers/vultr/provider.go Outdated
qdm12
qdm12 reviewed Oct 9, 2024
View reviewed changes
Comment thread internal/provider/providers/vultr/provider.go Outdated
decoder := json.NewDecoder(response.Body)
var parsedJSON struct {
Error string
Status uint32

@qdm12 qdm12 Oct 9, 2024

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Also what is that Status used for? 🤔

@amroessam amroessam Oct 9, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's the HTTP status code

@qdm12 qdm12 Oct 9, 2024

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Ok that's weird 😄 Should we... remove it from that struct?? Since it's... well, in the HTTP response already...

@amroessam amroessam Oct 17, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

For the sake of implementing the API, I've added it, can remove it if you insist 🙏🏽

qdm12
qdm12 reviewed Oct 9, 2024
View reviewed changes
Comment thread internal/provider/providers/vultr/provider.go Outdated
Comment thread docs/vultr.md Outdated
Comment thread docs/vultr.md Outdated
@amroessam

amroessam commented Oct 17, 2024

Copy link
Copy Markdown
Contributor Author

Sorry for the delay, I was away. Can you recheck?

@qdm12
qdm12 force-pushed the master branch 2 times, most recently from 126315b to 691ed32 Compare October 20, 2024 13:15
qdm12 and others added 21 commits October 20, 2024 16:21
@qdm12
chore(deps): tidy go modules
dff1ba7
Amr Essam @qdm12
Add dev files to gitignore
694319a
Amr Essam @qdm12
Add vultr provider
c7117ee
Amr Essam @qdm12
Add vultr docs
faac595
Amr Essam @qdm12
Add vultr to provider choice
ad3116a
Amr Essam @qdm12
Remove debug message
bf3680c
@amroessam @qdm12
Update docs/vultr.md
e01f8bc 
Co-authored-by: Quentin McGaw <quentin.mcgaw@gmail.com>
@amroessam @qdm12
Update docs/vultr.md
425200b 
Co-authored-by: Quentin McGaw <quentin.mcgaw@gmail.com>
@amroessam @qdm12
Update internal/provider/providers/vultr/provider.go
b31be3c 
Co-authored-by: Quentin McGaw <quentin.mcgaw@gmail.com>
@amroessam @qdm12
Update internal/provider/providers/vultr/provider.go
e7f0710 
Co-authored-by: Quentin McGaw <quentin.mcgaw@gmail.com>
@amroessam @qdm12
Update internal/provider/providers/vultr/provider.go
80942df 
Co-authored-by: Quentin McGaw <quentin.mcgaw@gmail.com>
Amr Essam @qdm12
Update error for token not set
2694d6f
Amr Essam @qdm12
Remove checking for ttl
0cc3a71
Amr Essam @qdm12
Revert gitignore
c43db65
Amr Essam @qdm12
Added info about wildcard
947193f
Amr Essam @qdm12
Switch to apiKey instead of Token
a68afad
@amroessam @qdm12
Update internal/provider/providers/vultr/provider.go
ce6ef3f 
Co-authored-by: Quentin McGaw <quentin.mcgaw@gmail.com>
@amroessam @qdm12
Update docs/vultr.md
f2dd4bc 
Co-authored-by: Quentin McGaw <quentin.mcgaw@gmail.com>
@amroessam @qdm12
Update docs/vultr.md
77ada26 
Co-authored-by: Quentin McGaw <quentin.mcgaw@gmail.com>
Amr Essam @qdm12
Add update record and define record type
af8aedf
Amr Essam @qdm12
Add switch case for create record response
8d6d608
Amr Essam and others added 17 commits October 20, 2024 16:26
Amr Essam @qdm12
Add pagination comment
0364369
@qdm12
vultr.md: small rewording here and there
85eb99b
@qdm12
formatting: use gofumpt
4339602
@qdm12
chore: add API documentation url for each function
e2626b7
@qdm12
chore: use Data struct field name for "data" json fields
c6222bd
@qdm12
fix: getrecord to match a single record type
52e59a7
@qdm12
fix: return an error if record is not found
1c34925
@qdm12
chore: updateRecord does not return a newIP
74c2127
@qdm12
chore: get rid of record struct in favor of narrower fields
9219ef0
@qdm12
chore: remove the use of undocumented field Status
9d46d56
@qdm12
fix: associate error fields with error json key
eb5001f
@qdm12
feat: parseJSONError returns the whole body if decoding fails or pars…
f263792 
…ed error is empty
@qdm12
fix: check error field even on successful status codes
33fd1f7
@qdm12
chore: fix last lint error
4c3e064
@qdm12
chore: createRecord does not check response ip against sent ip anymore
e46309e 
- we don't do it for updateRecord
- not really sure why we do it in the first place honestly
@qdm12
chore: minor changes and formatting
459e397
@qdm12
fix: getRecord handles response bodies after decoding it once
a7e9b0a
@qdm12

qdm12 commented Oct 20, 2024

Copy link
Copy Markdown
Owner

Hey there, thanks for the last few commits!
In the end I decided to push directly on your branch, since there were quite a few things to change (ranging from not important at all to somewhat important - see commits with fix: prefix). Feel free to browse commit by commit, since I tried my best to separate changes one by one for knowledge sharing purposes!
I also rebased your branch on the master branch here, so feel free to get changes using git checkout master; git fetch; git reset --hard origin/master 😉
Just waiting for the CI to pass and I'll merge this. Thanks!

@qdm12
qdm12 merged commit 949dcd9 into qdm12:master Oct 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@qdm12 qdm12 qdm12 requested changes
+1 more reviewer
@ehab-qadah-lean ehab-qadah-lean ehab-qadah-lean approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@amroessam @qdm12 @ehab-qadah-lean