Skip to content
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

Fix import of ovh_ip_reverse resources with IPv6 #346

Merged
merged 2 commits into from
Jan 4, 2023

Conversation

smortex
Copy link
Contributor

@smortex smortex commented Dec 14, 2022

When importing an ovh_ip_reverse resources, the user is expected to
provide the IP block and IP address separated with a colon, e.g.
198.51.100.0/24:198.51.100.42:

  • 198.51.100.0/24 is extracted as the IP block;
  • 198.51.100.42 is extracted as the address.

Unfortunately, the : separator does not work well with IPv6 addresses,
e.g. 2001:0DB8::/32:2001:0DB8::42 is interpreted as:

  • 2001 is extracted as the IP block;
  • 0DB8::/32:2001:0DB8::42 is extracted as the address.

This is obviously wrong and raise an API error:

│ Error: calling /ip/%222001/reverse/0DB8::%2F32:2001:0DB8::42:
│ 	 HTTP Error 400: "[ipReverse] Given data (0DB8::/32:2001:0DB8::42) is not valid for type ip"

Replace the : separator with a | separator to avoid this issue.

When importing an `ovh_ip_reverse` resources, the user is expected to
provide the IP block and IP address separated with a colon, e.g.
`198.51.100.0/24:198.51.100.42`:

  * `198.51.100.0/24` is extracted as the IP block;
  * `198.51.100.42` is extracted as the address.

Unfortunately, the `:` separator does not work well with IPv6 addresses,
e.g. `2001:0DB8::/32:2001:0DB8::42` is interpreted as:

  * `2001` is extracted as the IP block;
  * `0DB8::/32:2001:0DB8::42` is extracted as the address.

This is obviously wrong and raise an API error:

```
│ Error: calling /ip/%222001/reverse/0DB8::%2F32:2001:0DB8::42:
│ 	 HTTP Error 400: "[ipReverse] Given data (0DB8::/32:2001:0DB8::42) is not valid for type ip"
```

Replace the `:` separator with a `|` separator to avoid this issue.
@scraly
Copy link
Collaborator

scraly commented Dec 15, 2022

Thanks for your fix.

Can you copy-paste me the terraform import command you tested with this fix please?

Thanks :)

@smortex
Copy link
Contributor Author

smortex commented Dec 15, 2022

Hey @scraly ! I have some terraform definition like this that ensure each dedicated server reverse DNS for both IPv4 and IPv6 is set to <host>.example.com (barely readable unfortunately but I am not sure about how to improve this regarding readability of terraform code):

data "ovh_dedicated_servers" "all_dedicated_servers" {}

data "ovh_dedicated_server" "dedicated_server" {
  for_each     = toset(data.ovh_dedicated_servers.all_dedicated_servers.result)
  service_name = each.key
}

resource "ovh_ip_reverse" "dedicated_server" {
  for_each = toset(data.ovh_dedicated_servers.all_dedicated_servers.result)

  ip         = "${data.ovh_dedicated_server.dedicated_server[each.key].ip}/32"
  ip_reverse = data.ovh_dedicated_server.dedicated_server[each.key].ip
  reverse    = "${split(".", data.ovh_dedicated_server.dedicated_server[each.key].name)[0]}.example.com."
}

resource "ovh_ip_reverse" "dedicated_server6" {
  for_each = toset(data.ovh_dedicated_servers.all_dedicated_servers.result)

  ip         = element([for ip in data.ovh_dedicated_server.dedicated_server[each.key].ips : ip if length(regexall("^[:[:xdigit:]]+/\\d+$", ip)) > 0], 0)
  ip_reverse = regex("^([:[:xdigit:]]+)/\\d+$", element([for ip in data.ovh_dedicated_server.dedicated_server[each.key].ips : ip if length(regexall("^[:[:xdigit:]]+/\\d+$", ip)) > 0], 0))[0]
  reverse    = "${split(".", data.ovh_dedicated_server.dedicated_server[each.key].name)[0]}.example.com."
}

For each server I ran an import command for IPv4 and another one for IPv6, for example:

terraform import 'ovh_ip_reverse.dedicated_server["ns573621.ip-51-161-116.net"]' '51.161.116.19/32|51.161.116.19'
terraform import 'ovh_ip_reverse.dedicated_server6["ns573621.ip-51-161-116.net"]' '2607:5300:203:6e13::/64|2607:5300:203:6e13::'

Then terraform plan shown me misconfigured reverse DNS entries and terraform apply fixed them. 🎉

Copy link
Collaborator

@scraly scraly left a comment

Choose a reason for hiding this comment

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

Thanks,
moreover can you add the import section in the documentation page (with the format you recommend)? Like this:
https://registry.terraform.io/providers/ovh/ovh/latest/docs/resources/cloud_project_kube#import

Thank you :)

Use the 2001:0DB8::/32 IPv6 prefix reserved for documentation purpose.
@smortex
Copy link
Contributor Author

smortex commented Dec 27, 2022

Example for importing an IPv6 address added. Do you also want an example with IPv4 or is it fine this way since the syntax is basically the same?

@scraly
Copy link
Collaborator

scraly commented Dec 28, 2022

Example for importing an IPv6 address added. Do you also want an example with IPv4 or is it fine this way since the syntax is basically the same?

I think only one example should be OK :)

@scraly
Copy link
Collaborator

scraly commented Jan 4, 2023

Any news about the example to add? :-)
I plan to create a new release in the coming days.

@smortex
Copy link
Contributor Author

smortex commented Jan 4, 2023

Hey @scraly

Isn't 9b8f2d2 I added to this PR a few days ago what you are looking for? I think this is ready for merging unless some other issue is raised :-)

Thanks!

@scraly
Copy link
Collaborator

scraly commented Jan 4, 2023

The status was and is still in "changes request" so I guess you didn't see my last message :-)

Copy link
Collaborator

@scraly scraly left a comment

Choose a reason for hiding this comment

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

top, thank you :)

@scraly scraly added the 0.26.0 label Jan 4, 2023
@scraly scraly merged commit 7b82639 into ovh:master Jan 4, 2023
@smortex smortex deleted the ovh_ip_reverse-ipv6 branch January 4, 2023 15:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants