Adds a 'vultr_reverse_name' resource #78

cimnine · 2019-06-12T00:53:39Z

This PR implements a new resource for setting reverse dns entries.

An extra resource has been chosen, because in order to set the reverse name, the IP(s) of the instance have to be known. And that's only the case after the instance has been created.

The syntax is like this:

// Define reverse name for main IPv4
resource "vultr_reverse_name" "example_v4" {
  instance_id = "${vultr_instance.example.id}"
  ip          = "${vultr_instance.example.ipv4_address}"
  name        = "example.com"
}

// Define reverse name for main IPv6
resource "vultr_reverse_name" "example_v6" {
  instance_id = "${vultr_instance.example.id}"
  ip          = "${vultr_instance.example.ipv6_addresses[0]}"
  name        = "example.com"
}

As you see, it works for IPv4 and IPv6 likewise. (Although internally they are handled by two different API endpoints. But the behavior as observed by the is very similar. That's why I've decided to combine the IPv4 and IPv6 case into one resource type.)

When destroying an IPv4 reverse name resource, nothing actually happens. That's because there is no API endpoint by Vultr to "restore the default name".

I think this resolves #34 and #56.

It works for IPv4 and IPv6 likewise. An extra resource has been chosen, because in order to set the reverse name, the IP(s) of the instance have to be known. And that's only after they have been created. When an IPv4 reverse name is deleted, nothing really happens. This is because there is not API of Vultr to 'reset' the reverse name.

squat · 2019-06-12T07:10:42Z

@cimnine thank you for making the PR! I made a few notes for small changes and I have one question about the name attribute. Otherwise this looks great!

vultr/resource_reverse_name.go

cimnine · 2019-06-12T08:45:46Z

Thank you for your feedback!

I've implemented all your suggestions and added some additional validation.

Because adding additional validation required two external dependencies, and glide updates all dependencies when adding new ones, this PR suddenly got huge :-/

Maybe it's time to move on and go mod init github.com/squat/terraform-provider-vultr 😉

squat · 2019-06-12T09:08:05Z

Hmm I don’t believe that adding the validation dependency really needs 9,000,000 new lines of code, not diff’d lines but added lines. Can you please try make vendor to remove the unneeded files?

Command used: ```bash glide get \ --quick \ -v \ "golang.org/x/net/idna" \ "github.com/hashicorp/terraform/helper/validation" ```

Even if it's technically speaking not required, this resource doesn't make sense if isnt'.

For querying existing reverse names without declaring them through terraform.

cimnine · 2019-06-12T11:58:11Z

Unfortunately I've found it impossible to pass the build and have less code changes in vendor. (Or I just don't get how.) Still, by excluding nested vendor directories, I've managed to remove about 2 million loc. But there are still plenty left.

glide get -v "golang.org/x/net/idna" "github.com/hashicorp/terraform/helper/validation"

# tried, but doesn't work on travis:
glide get --quick -v "golang.org/x/net/idna" "github.com/hashicorp/terraform/helper/validation"

I've used the command above. Maybe you could give it a try?

We could argue if it's really necessary to add these dependencies. The IP checker isn't, that's trivial to implement. But the conversion between IDN and ASCII domain name representations is not easy to implement. And I believe that conversion is necessary in order to reliably compare whether domain names have changed.

squat · 2019-06-12T12:03:17Z

I’ll take a look at the vendoring commit

cimnine · 2019-06-24T18:46:19Z

As you might have seen I've went ahead (maybe too far?) and presented a way that creates less changes to the code base by migrating over to the go mod ecosystem.

Feel free to close these PRs anytime should you not be comfortable with the way I was approaching things.

~Chris

cimnine added 2 commits June 12, 2019 02:49

Fix typo in README.md

536e8c1

cimnine changed the title ~~Implements 'vultr_reverse_name' resource~~ Adds a 'vultr_reverse_name' resource Jun 12, 2019

squat reviewed Jun 12, 2019

View reviewed changes

cimnine added 6 commits June 12, 2019 12:05

implement validation

96d11d0

Fix style

a5e2357

add dependencies to glide

6d93145

Command used: ```bash glide get \ --quick \ -v \ "golang.org/x/net/idna" \ "github.com/hashicorp/terraform/helper/validation" ```

Change the 'name' to required

11f79e9

Even if it's technically speaking not required, this resource doesn't make sense if isnt'.

Implemented a 'vultr_reverse_name' data source

94701a1

For querying existing reverse names without declaring them through terraform.

Add all vendor

01e51fb

This was referenced Jun 12, 2019

Sensible Defaults #48

Open

[go mod] Adds a 'vultr_reverse_name' resource #80

Closed

cimnine closed this Oct 29, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds a 'vultr_reverse_name' resource #78

Adds a 'vultr_reverse_name' resource #78

cimnine commented Jun 12, 2019 •

edited

Loading

squat commented Jun 12, 2019

cimnine commented Jun 12, 2019

squat commented Jun 12, 2019

cimnine commented Jun 12, 2019 •

edited

Loading

squat commented Jun 12, 2019

cimnine commented Jun 24, 2019

Adds a 'vultr_reverse_name' resource #78

Adds a 'vultr_reverse_name' resource #78

Conversation

cimnine commented Jun 12, 2019 • edited Loading

squat commented Jun 12, 2019

cimnine commented Jun 12, 2019

squat commented Jun 12, 2019

cimnine commented Jun 12, 2019 • edited Loading

squat commented Jun 12, 2019

cimnine commented Jun 24, 2019

cimnine commented Jun 12, 2019 •

edited

Loading

cimnine commented Jun 12, 2019 •

edited

Loading