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

ovh_ip_reverse fails to unmarshal the response because it receives an array of ip addresses #208

Closed
berbiche opened this issue Jul 4, 2021 · 2 comments · Fixed by #209
Closed

Comments

@berbiche
Copy link

berbiche commented Jul 4, 2021

Terraform Version

Terraform v0.14.11
+ provider registry.terraform.io/backblaze/b2 v0.2.1
+ provider registry.terraform.io/carlpett/sops v0.6.3
+ provider registry.terraform.io/cloudflare/cloudflare v2.23.0
+ provider registry.terraform.io/hashicorp/local v2.1.0
+ provider registry.terraform.io/hashicorp/null v3.1.0
+ provider registry.terraform.io/ovh/ovh v0.14.0

Affected Resource(s)

  • ovh_ip_reverse

Terraform Configuration Files

data "ovh_vps" "something" {
  service_name = "<elided>"
}

locals {
  ipv4_addresses = toset([for s in data.ovh_vps.something.ips : s if !can(cidrnetmask("${s}/128"))])
  ipv6_addresses = toset([for s in data.ovh_vps.something.ips : s if can(cidrnetmask("${s}/128"))])
}

resource "ovh_ip_reverse" "something_ipv4" {
  for_each = local.ipv4_addresses
  ip = "${each.value}/32"
  reverse = "example.com."
}

resource "ovh_ip_reverse" "something_ipv6" {
  for_each = local.ipv6_addresses
  ip = "${each.value}/128"
  reverse = "example.com."
}

Debug Output

Sorry I did not respect the guidelines here but the debug output is short and the bug has been precisely defined in the section "Actual behavior".

2021-07-04T01:01:31.892-0400 [INFO]  plugin.terraform-provider-ovh_v0.14.0: 2021/07/04 01:01:31 [DEBUG] OVH API Request Details:
---[ REQUEST ]---------------------------------------
GET /1.0/ip/<elided>%2F32/reverse/ HTTP/1.1
Host: ca.api.ovh.com
User-Agent: Go-http-client/1.1
<elided>

2021-07-04T01:01:32.063-0400 [INFO]  plugin.terraform-provider-ovh_v0.14.0: 2021/07/04 01:01:32 [DEBUG] OVH API Response Details:
---[ RESPONSE ]--------------------------------------
HTTP/1.1 200 OK
<elided>

[
 "XX.XXX.XX.XXX"
]

Panic Output

This used to crash Terraform in release 0.11 but after upgrading to 0.14 there's a useful error message:

Error: calling /ip/<elided>%2F32/reverse/:
	 json: cannot unmarshal array into Go value of type ovh.OvhIpReverse

Expected Behavior

Per the title, when looking up the state of ovh_ip_reverse, it should not fail to validate.

Actual Behavior

The code expects the API to return a flat object but it returns an array.

func resourceOvhIpReverseRead(d *schema.ResourceData, meta interface{}) error {
provider := meta.(*Config)
reverse := OvhIpReverse{}
endpoint := fmt.Sprintf(
"/ip/%s/reverse/%s",
strings.Replace(d.Get("ip").(string), "/", "%2F", 1),
d.Get("ipreverse").(string),
)
if err := provider.OVHClient.Get(endpoint, &reverse); err != nil {
return helpers.CheckDeleted(d, err, endpoint)
}
d.Set("ipreverse", reverse.IpReverse)
d.Set("reverse", reverse.Reverse)
return nil
}

As you can see from my configuration file, I did not specify the ipreverse field, the code then uses the API endpoint ip/<ip>/reverse and according to the official documentation this endpoint returns a list of ips.

The problem is "resolved" if I set ipreverse to the value of ip without the mask.
Though I need to delete the resource from my state because the state does not have an ipreverse.

Steps to Reproduce

  1. terraform apply

Important Factoids

Nothing

References

Nothing

@yanndegat
Copy link
Collaborator

hi @berbiche

thank you for this very clean report. it greatly helps the debugging.

While trying to reproduce & fix the issue, i noticed several flaws in the resource implementation, so i rewrote it
following the same patterns we use for other resources.

the most notable (and breaking) changes are that now

  • ipreverse is renamed ip_reverse to follow the naming convention
  • ip_reverse is mandatory.

in a global manner, we'd like the provider to mimic the API behaviour & design, without embedding custom logic.
this would eventually allow us to generate the provider from the api definition.

i'ill try to release a new version which this new version included soon

@berbiche
Copy link
Author

berbiche commented Aug 6, 2021

Thanks the fix in #209 indeed work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants