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

Inconsistent final plans for IP addresses without CIDR notation #2728

Closed
2 tasks done
uhthomas opened this issue Aug 31, 2023 · 5 comments
Closed
2 tasks done

Inconsistent final plans for IP addresses without CIDR notation #2728

uhthomas opened this issue Aug 31, 2023 · 5 comments
Labels
working-as-intended Indicates an issue is working as designed.

Comments

@uhthomas
Copy link
Member

uhthomas commented Aug 31, 2023

Confirmation

  • My issue isn't already found on the issue tracker.
  • I have replicated my issue using the latest version of the provider and it is still present.

Terraform and Cloudflare provider version

cloudflare/cloudflare: 4.13.0

Affected resource(s)

cloudflare_tunnel_route
cloudflare_teams_list

Terraform configuration files

resource "cloudflare_tunnel_route" "test_ipv4" {
  account_id = var.account_id
  tunnel_id  = cloudflare_tunnel.test.id
  network    = "127.0.0.1"
  comment    = "Test IPv4"
}

resource "cloudflare_tunnel_route" "test_ipv6" {
  account_id = var.account_id
  tunnel_id  = cloudflare_tunnel.test.id
  network    = "100::"
  comment    = "Test IPv6"
}

resource "cloudflare_teams_list" "test_ip_list" {
  account_id = var.account_id
  name       = "Test"
  type       = "IP"
  items = [
    cloudflare_tunnel_route.test_ipv4.network,
    cloudflare_tunnel_route.test_ipv6.network
  ]
}

Link to debug output

N/A

Panic output

No response

Expected output

Should plan successfully, or not at all.

Actual output

╷
│ Error: Provider produced inconsistent final plan
│ 
│ When expanding the plan for
│ cloudflare_teams_list.test_ip_list to include new values
│ learned so far during apply, provider
│ "registry.terraform.io/cloudflare/cloudflare" produced an invalid new value
│ for .items: planned set element cty.StringVal("127.0.0.1") does not
│ correlate with any element in actual.
│ 
│ This is a bug in the provider, which should be reported in the provider's
│ own issue tracker.
╵
╷
│ Error: Provider produced inconsistent final plan
│ 
│ When expanding the plan for
│ cloudflare_teams_list.test_ip_list to include new values
│ learned so far during apply, provider
│ "registry.terraform.io/cloudflare/cloudflare" produced an invalid new value
│ for .items: planned set element cty.StringVal("100::") does
│ not correlate with any element in actual.
│ 
│ This is a bug in the provider, which should be reported in the provider's
│ own issue tracker.
╵

Steps to reproduce

Apply the given resources.

Additional factoids

I noticed this output:

  # cloudflare_tunnel_route.test_ipv4 will be updated in-place
~ resource "cloudflare_tunnel_route" "test_ipv4" {
        id         = "127.0.0.1/32"
      ~ network    = "127.0.0.1/32" -> "127.0.0.1"
        # (3 unchanged attributes hidden)
    }

So, I modified the resources to use the CIDR notation and the apply was successful.

References

No response

@uhthomas uhthomas added kind/bug Categorizes issue or PR as related to a bug. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Aug 31, 2023
@github-actions
Copy link
Contributor

Thank you for reporting this issue! For maintainers to dig into issues it is required that all issues include the entirety of TF_LOG=DEBUG output to be provided. The only parts that should be redacted are your user credentials in the X-Auth-Key, X-Auth-Email and Authorization HTTP headers. Details such as zone or account identifiers are not considered sensitive but can be redacted if you are very cautious. This log file provides additional context from Terraform, the provider and the Cloudflare API that helps in debugging issues. Without it, maintainers are very limited in what they can do and may hamper diagnosis efforts.

This issue has been marked with triage/needs-information and is unlikely to receive maintainer attention until the log file is provided making this a complete bug report.

@github-actions github-actions bot added triage/needs-information Indicates an issue needs more information in order to work on it. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Aug 31, 2023
@github-actions
Copy link
Contributor

Community Note

Voting for Prioritization

  • Please vote on this issue by adding a 👍 reaction to the original post to help the community and maintainers prioritize this request.
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request.

Volunteering to Work on This Issue

  • If you are interested in working on this issue, please leave a comment.
  • If this would be your first contribution, please review the contribution guide.

@jacobbednarz
Copy link
Member

the issue here is that the service wants the IP represented in one form however, it is provided in another. we don't duplicate the service logic here (yet) so this is working as expected until such a time we can unify the validation and bring it into the provider.

@jacobbednarz jacobbednarz closed this as not planned Won't fix, can't repro, duplicate, stale Sep 1, 2023
@jacobbednarz jacobbednarz added working-as-intended Indicates an issue is working as designed. and removed kind/bug Categorizes issue or PR as related to a bug. triage/needs-information Indicates an issue needs more information in order to work on it. labels Sep 1, 2023
@uhthomas
Copy link
Member Author

uhthomas commented Sep 1, 2023

I'm not sure it's fair to label this as 'working as intended' given Terraform cannot apply the resource and gives zero indication as to why.

There are a whole class of bugs which are caused by the API mutating data, surely there just be a better way to handle this?

#2608

@jacobbednarz
Copy link
Member

jacobbednarz commented Sep 1, 2023

the working as intended part is the drift that comes from one part of the service wanting the IP in one particular notion whereas it is provided it in another. you're right that there are improvements here but they reside with the service itself to either not remove the CIDR notation or fail the API response. here, terraform is comparing the two values and identifying their is a discrepancy.

we do diff suppression in some cases (like correcting case) but here, we should really have the service decide how it wants the value and validate against it on the payload since this will also be an issue for anyone calling the API directly.

additionally, I suspect you have two issues here as the error you are seeing about a the values not being present in the list is different to the drift error you are seeing. if you can provide the sanitised debug output logs, I can take a look at that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
working-as-intended Indicates an issue is working as designed.
Projects
None yet
Development

No branches or pull requests

2 participants