UltraDNS Provider #2716

jmasseo · 2015-07-13T22:14:19Z

I've implemented a simple UltraDNS provider that can manage DNS records with UltraDNS' REST API.
I've included documentation and acceptance tests.

It is 'inspired' by the DNSimple provider.

josephholsten · 2015-07-13T22:22:19Z

👍 it's even got docs!

phinze · 2015-07-14T00:16:11Z

On first glance this looks great! Will do a full review sometime this week.

jtpace · 2015-07-18T00:45:50Z

What's left for getting this added to the official project?

Pryz · 2015-08-17T13:26:50Z

Any update of this PR ?

phinze · 2015-09-04T23:26:24Z

builtin/bins/provider-ultradns/main_test.go

@@ -0,0 +1 @@
+package main


No need to include the empty test file here.

phinze · 2015-09-04T23:42:12Z

Alright @jmasseo - finally got around to doing a proper review on this. Made a bunch of comments in line for you, but on the whole this is coming along nicely!

josephholsten · 2015-10-06T05:54:41Z

This patch is failing due to go vet failing for other files.

josephholsten · 2015-10-06T05:54:55Z

@phinze this is ready for your 👀

radeksimko · 2015-10-06T14:36:03Z

@josephholsten

This patch is failing due to go vet failing for other files.

The failure coming from tests is related to this PR and needs fixing before someone starts reviewing it:

# github.com/hashicorp/terraform/builtin/providers/ultradns
builtin/providers/ultradns/resource_ultradns_record.go:91: undefined: ErrorResponse
builtin/providers/ultradns/resource_ultradns_record.go:99: no new variables on left side of :=

josephholsten · 2015-10-27T06:29:47Z

@radeksimko indeed it was.

@phinze all fixed now, and with much better error handling.

josephholsten · 2015-11-02T18:26:56Z

rebased again. @phinze, @radeksimko ready for your 👓

phinze · 2015-11-02T18:31:15Z

builtin/providers/ultradns/provider_test.go

+
+	if v := os.Getenv("ULTRADNS_DOMAIN"); v == "" {
+		t.Fatal("ULTRADNS_DOMAIN must be set for acceptance tests. The domain is used to create and destroy record against.")
+	}


Variable says DOMAIN here but it's BASEURL in the provider config. Is there a difference?

the DOMAIN is specified for the test. This is the domain it will perform the test operation with in resource_ultradns_record_test.go. BASEURL specifies if it should use production or the development sandbox.

phinze · 2015-11-02T18:37:39Z

Okay one more review pass w/ a few questions, nearly there!

josephholsten · 2016-01-15T19:11:50Z

ok, async work has been done. @phinze, @radeksimko ready for your 👓

we've got some more advanced features in the pipe, would be nice to get these basics merged before we start a ticket for the new shiny.

phinze · 2016-01-20T00:51:19Z

@josephholsten Still not seeing any resource.Retry polling on this PR - possible some commits didn't make it to the right spot?

Also, can somebody could point me in the direction of the account that we'd need to sign up for to to properly set up Travis to run nightly tests? I want to make sure we pick up the correct account type.

josephholsten · 2016-01-21T18:17:58Z

@phinze oh, @jmasseo put the async handling into the sdk. Does the async stuff need to be visible to tf to play nice? Or is it cool that it's abstracted away?

phinze · 2016-01-21T18:25:18Z

Ah okay, didn't catch that part. That's nicer! Will give it one more pass through and pull it in. 👍 🚢

phinze · 2016-01-21T18:26:11Z

(Also @josephholsten did you have any knowledge you could share on the account type necessary to acceptance test this provider?)

jen20 · 2016-01-25T17:25:23Z

@phinze I've signed up for an UltraDNS account to get acceptance tests running at our end for this; looks like it will take a few hours, so I'll come back to it later. Code LGTM (following reasonably extensive review I guess that shouldn't be a surprise!) though.

jen20 · 2016-01-25T23:50:00Z

Unfortunately it seems we either don't have the correct type of account, or have a credentials issue:

=== RUN   TestAccUltraDNSRecord_Updated
ResCode: 401 Body: {"errorCode":60001,"errorMessage":"invalid_grant:Invalid username & password combination.","error":"invalid_grant","error_description":"60001: invalid_grant:Invalid username & password combination."}
--- FAIL: TestAccUltraDNSRecord_Updated (0.19s)
    testing.go:148: Step 0 error: Error refreshing: 1 error(s) occurred:

        * Error setting up client: POST https://test-restapi.ultradns.com/v1/authorization/token: 401 60001 invalid_grant:Invalid username & password combination.

I have contacted UltraDNS support in the hope they can help us establish which account type or credentials we need to use. @josephholsten or @jmasseo, might either of you be able to shed any light on this?

jmasseo · 2016-01-26T13:06:07Z

The sandbox does not update from their production system very frequently. They just recently updated it for the first time in several months. We may have to bother them to force a sync to the testing environment for your credentials to work. I had to do a lot of my development testing against production credentials because the sandbox is so wonky. I have made a lot of changes to the SDK to provide robustness against UltraDNS weirdness as well, and will be pushing that changeset over to the SDK when I'm finished this week. Then I have a few more changes to make to this branch to add support for the fancy geo features that UltraDNS supports.

jen20 · 2016-01-27T18:06:09Z

Hi @jmasseo - thanks for the update. It turns out API access is only provided with enterprise accounts so we don't really have any way to test this. We're hoping to start a discussion with UltraDNS about whether this is a possibility. One other possibility, though less nice, is to split this out into it's own repository as a third party provider. We can aim to make some kind of decision on this soon - sorry for the delay in reviewing!

jmasseo · 2016-02-01T14:40:11Z

I've reached out to UltraDNS and heard back and they are trying to work on some API accounts for us.

jmasseo · 2016-02-11T23:15:08Z

Who from the terraform team would end up being the custodian of the acceptance tests account for terraform? @phinze @jen20

phinze · 2016-02-11T23:18:19Z

@jmasseo - if you shoot an email to terraform at hashicorp we can chat about it!

phinze · 2016-03-16T23:55:20Z

Just noting out here that I bumped a thread w/ @jmasseo today - hoping to get this landed soon!

josephholsten · 2016-03-18T16:51:58Z

I was rebasing this branch up to master and somehow it's marked as merged. re-opening...

lwander · 2016-03-18T16:53:58Z

Seems to be a bug, it was somehow merged after I merged this one: #5715

josephholsten · 2016-03-18T16:54:49Z

ok, the actual patch now lives at #5716

phinze · 2016-03-18T16:55:06Z

@josephholsten yeah we're trying to figure out what happened here heh. Just confirmed that the UDNS code did not in fact land on master.

josephholsten · 2016-03-18T17:07:31Z

And this is why you should never rewrite history while wearing emperor's attire in public.

ghost · 2020-04-27T02:11:41Z

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

radeksimko added the enhancement label Jul 14, 2015

phinze reviewed Sep 4, 2015
View reviewed changes

builtin/bins/provider-ultradns/main_test.go

@@ -0,0 +1 @@

package main

Copy link

Contributor

phinze Sep 4, 2015

Choose a reason for hiding this comment

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

No need to include the empty test file here.

radeksimko added the wip label Sep 19, 2015

josephholsten force-pushed the master branch from 4a9f690 to 511b430 Compare September 22, 2015 04:02

josephholsten force-pushed the master branch from 12051a7 to 374070d Compare October 6, 2015 04:50

josephholsten force-pushed the master branch from 0a7a2f9 to e559738 Compare October 27, 2015 06:25

josephholsten force-pushed the master branch from e559738 to 13ebbca Compare November 2, 2015 18:26

phinze reviewed Nov 2, 2015
View reviewed changes

phinze added the waiting-response An issue/pull request is waiting for a response from the community label Jan 20, 2016

jen20 removed waiting-response An issue/pull request is waiting for a response from the community wip labels Jan 25, 2016

josephholsten force-pushed the master branch from 3005de8 to 753e7a2 Compare March 18, 2016 16:47

lwander merged commit 753e7a2 into hashicorp:master Mar 18, 2016

josephholsten mentioned this pull request Mar 18, 2016

UltraDNS provider #5716

Closed

ghost locked and limited conversation to collaborators Apr 27, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

UltraDNS Provider #2716

UltraDNS Provider #2716

jmasseo commented Jul 13, 2015

josephholsten commented Jul 13, 2015

phinze commented Jul 14, 2015

jtpace commented Jul 18, 2015

Pryz commented Aug 17, 2015

phinze Sep 4, 2015

phinze commented Sep 4, 2015

josephholsten commented Oct 6, 2015

josephholsten commented Oct 6, 2015

radeksimko commented Oct 6, 2015

josephholsten commented Oct 27, 2015

josephholsten commented Nov 2, 2015

phinze Nov 2, 2015

jmasseo Nov 17, 2015

phinze commented Nov 2, 2015

josephholsten commented Jan 15, 2016

phinze commented Jan 20, 2016

josephholsten commented Jan 21, 2016

phinze commented Jan 21, 2016

phinze commented Jan 21, 2016

jen20 commented Jan 25, 2016

jen20 commented Jan 25, 2016

jmasseo commented Jan 26, 2016

jen20 commented Jan 27, 2016

jmasseo commented Feb 1, 2016

jmasseo commented Feb 11, 2016

phinze commented Feb 11, 2016

phinze commented Mar 16, 2016

josephholsten commented Mar 18, 2016

lwander commented Mar 18, 2016

josephholsten commented Mar 18, 2016

phinze commented Mar 18, 2016

josephholsten commented Mar 18, 2016

ghost commented Apr 27, 2020

UltraDNS Provider #2716

UltraDNS Provider #2716

Conversation

jmasseo commented Jul 13, 2015

josephholsten commented Jul 13, 2015

phinze commented Jul 14, 2015

jtpace commented Jul 18, 2015

Pryz commented Aug 17, 2015

phinze Sep 4, 2015

Choose a reason for hiding this comment

phinze commented Sep 4, 2015

josephholsten commented Oct 6, 2015

josephholsten commented Oct 6, 2015

radeksimko commented Oct 6, 2015

josephholsten commented Oct 27, 2015

josephholsten commented Nov 2, 2015

phinze Nov 2, 2015

Choose a reason for hiding this comment

jmasseo Nov 17, 2015

Choose a reason for hiding this comment

phinze commented Nov 2, 2015

josephholsten commented Jan 15, 2016

phinze commented Jan 20, 2016

josephholsten commented Jan 21, 2016

phinze commented Jan 21, 2016

phinze commented Jan 21, 2016

jen20 commented Jan 25, 2016

jen20 commented Jan 25, 2016

jmasseo commented Jan 26, 2016

jen20 commented Jan 27, 2016

jmasseo commented Feb 1, 2016

jmasseo commented Feb 11, 2016

phinze commented Feb 11, 2016

phinze commented Mar 16, 2016

josephholsten commented Mar 18, 2016

lwander commented Mar 18, 2016

josephholsten commented Mar 18, 2016

phinze commented Mar 18, 2016

josephholsten commented Mar 18, 2016

ghost commented Apr 27, 2020