Implement RDNS setting via UpdateIPAddress

leslitagordita commented 5 years ago

Terraform Version

Terraform v0.11.9

provider.linode v1.2.0

Feature Request

Please consider adding support for setting RDNS on an IP Address.
See equivalent APIv4 request

References

https://github.com/linode/linodego/issues/62

displague commented 5 years ago

As mentioned, the LinodeGo wrapper will need support for UpdateIPAddress.

I'm thinking the resource would be linode_rdns, and it would take a name and a list of IP addresses.

// hypothetical resource - it may not be possible, or efficient, to discover all of the addresses associated with a RDNS name.
// If this approach was taken, address would need to be a TypeSet to avoid the ordering problems of TypeList
resource "linode_rdns" "www" {
  rdns = "www.example.com"
  addresses = [ "${linode_instance.www.ip_address}", "${linode_instance.www.ipv6}" ]
}

or perhaps

// hypothetical resource - closely aligns to the RDNS API endpoint fields. One address per resource.
resource "linode_rdns" "www" {
  rdns = "www.example.com"
  address = "${linode_instance.www.ip_address}"
}

resource "linode_rdns" "www-v6" {
  rdns = "www.example.com"
  address = "${linode_instance.www.ipv6}"
}

or perhaps

// hypothetical resource - Two fields to represent the two addresses a Linode typically has.  This is weak because there may still be a need for additional addresses to be use the same RDNS name
resource "linode_rdns" "www" {
  rdns = "www.example.com"
  address = "${linode_instance.www.ip_address}" // or call this address_ipv4
  address_ipv6 = "${linode_instance.www.ipv6}"
}

I'm leaning towards the first or second approach.

This wouldn't be a direct 1-to-1 with the API, but I think it would be problematic to try to shoe horn an attribute like rdns in to linode_instance since:

changing the RDNS doesn't really affect the instance
you may want to apply the name to something that is not an instance IP
you can choose to apply the RDNS name to either or both of the IPv4 and IPv6 addresses

displague commented 5 years ago

RDNS support was added to LinodeGo in https://github.com/linode/linodego/pull/66

This can move forward.

hielee commented 5 years ago

Is there a timeline for when the Reverse DNS support will be shipped?

displague commented 5 years ago

@hielee The short answer is that I don't have a timeline for this.

Feedback like this is a key motivator that there is interest in a feature. @rmcintosh happened to ask me if this was a feature of the Terraform provider today too.

This now holds a much more central position on my personal radar.

PRs always welcome. I think I can find time in the next 30 days to work on this. This thread will hold me accountable. 🤞

displague commented 5 years ago

I was thinking about an alternate approach that aligns with the /v4/networking/ips space, which includes RDNS, IPv4 sharing, IPv4 assignment, and address details (such as routing address).

This would be a linode_address resource, and would combine the features of:

GET /networking/ips/{address} (address details)
POST /networking/ips (assign additional IP to Linode)
PUT /networking/ips/{address} (set RDNS)
POST /networking/ipv4/assign (assign/move IP address)
POST /networking/ipv4/share (floating IP support, multiple Linodes can bring up a specific set of IP addresses (but in practice, only one instance at a time can use any particular address))

It could look like this:

`linode_address`

address optional/computed
- When provided this resource will represent an existing IP address. It may be used to update the RDNS value, or establish sharing the address with a set of Linodes or reassign the address to a specific Linode (this means both a primary linode_id and a shareable list of linode_ids would be needed as fields)
- When not provided, this is a request to create a add a new IP address to the Linode specified by linode_id (sharing could be established using the linode_ids field (should this prefixed? sharing_ ?))
rnds optional/computed
- When set with a value, the RDNS name to assign to this address
- When set and empty, clears the RDNS name of this address
public optional/computed (bool) Public or Private, needed when creating a new address, but also returned/computed when getting the details of an existing address
type optional/computed 'ipv4' or 'ipv6', this is a required field when adding an IP address, but it is also computed when retrieving details of an address

The other fields that would be provided would align with those provided by GET /networking/ips/{address}

Taking advantage of the API design and aligning these properties and functions with a related resource feels idiomatic for Terraform.

But, I don't know if this is a sound design for a Terraform resource. The data resource aspect of this seems like a code smell. By data resource aspect, I mean the ability to provide an existing address and have a resource that doesn't create something, but instead is populated with the values of something that already exists.

Another concern is that this resource would encompass nearly all IPv4 and some IPv6 API methods. Trying to shoehorn the remaining IPv6 features into this one resource is tempting, but I'm not convinced that they can be made to fit. Especially because these remaining endpoints deal with ranges/pools of IPv6 addresses.

displague commented 5 years ago

@dave-lew99 would this approach suite your needs?

displague commented 5 years ago

Swapping IP addresses may interfere with parallel provisioning steps and might require rebooting nodes to apply the desired changes. Floating IPs (IP Failover, IP Sharing) don't require a swap so it should be safe in a Terraform setting.

The IP donor node may need to be accessed/provisioned over IPv6.

dave-lew99 commented 5 years ago

I'm not too famlier with the Linode platform and it's particular behaviour, but yes that sounds like it would work for us.

On other platforms we create a floating IP allocation in terraform, then have create_before_destroy set on the instance - causing the new instance to be launched and have the IP assigned to the new instance before the old one is destroyed.

I suppose we could also use a load balancer - this would allow us to keep the same IP/DNS config and zero downtime, but it's additional cost and complexity.

linode / terraform-provider-linode