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

v5.0.0 Grit migration not working #4966

Closed
3 tasks done
MattJeanes opened this issue Jan 29, 2025 · 18 comments · Fixed by #5030
Closed
3 tasks done

v5.0.0 Grit migration not working #4966

MattJeanes opened this issue Jan 29, 2025 · 18 comments · Fixed by #5030
Labels
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.

Comments

@MattJeanes
Copy link

Confirmation

  • This is a bug with an existing resource and is not a feature request or enhancement. Feature requests should be submitted with Cloudflare Support or your account team.
  • I have searched the issue tracker and my issue isn't already found.
  • I have replicated my issue using the latest version of the provider and it is still present.

Terraform and Cloudflare provider version

Terraform v1.10.3
on windows_amd64

  • provider registry.terraform.io/cloudflare/cloudflare v5.0.0
  • provider registry.terraform.io/hashicorp/azurerm v4.16.0

Affected resource(s)

  • cloudflare_dns_record

But affects everything from the migration

Terraform configuration files

locals {
  dns_records = [
    "hello-world",
    "@"
  ]
  proxied_records = {
    "hello-world" = true,
    "@"           = true
  }
}

resource "cloudflare_record" "dns" {
  for_each = toset(local.dns_records)

  zone_id = var.cloudflare_zone_id
  name    = each.key
  type    = "CNAME"
  content = azurerm_public_ip.abyss_public.fqdn
  ttl     = 1
  proxied = lookup(local.proxied_records, each.key, false)
}

Link to debug output

N/A

Panic output

N/A

Expected output

The Grit migration will run successfully

Actual output

Thie Grit migration is not found

Steps to reproduce

https://registry.terraform.io/providers/cloudflare/cloudflare/latest/docs/guides/version-5-upgrade#automatic-migration

  1. Install grit
  2. CD to terraform directory
  3. grit apply cloudflare_terraform_v5

ERROR (code: 200) - pattern definition not found: cloudflare_terraform_v5. Try running grit init.

(NOTE: grit init did not help)

Additional factoids

I also tried adding this repo to the Grit config file and it nows shows other migrations but not the V5 upgrade one:

version: 0.0.1
patterns:
  - name: github.com/getgrit/stdlib#*
  - name: github.com/cloudflare/terraform-provider-cloudflare#*
C:\_git\Abyss-Infrastructure\terraform [renovate/cloudflare-5.x ≡ +1 ~0 -0 !]>  grit list --language hcl

STANDARD LIBRARY PATTERNS


HCL patterns
  ✔ cloudflare_record_deprecate_value_for_content (source: cloudflare/terraform-provider-cloudflare)
  ✔ cloudflare_terraform_v4_40_0_zero_trust_namespace_renames_configuration (source: cloudflare/terraform-provider-cloudflare)
  ✔ cloudflare_zone_settings_override_deprecate_minify (source: cloudflare/terraform-provider-cloudflare)
  ✔ edit_module (source: getgrit/stdlib)
  ✔ kv_pair (source: getgrit/stdlib)

I also even tried taking the file from https://github.com/cloudflare/terraform-provider-cloudflare/tree/gritql-v5/.grit/patterns and putting that in my own .grit/patterns which it then recognised and ran with grit apply terraform_cloudflare_v5 but didn't actually change anything:

C:\_git\Abyss-Infrastructure\terraform [renovate/cloudflare-5.x ≡ +1 ~0 -0 !]> grit apply terraform_cloudflare_v5
Your working tree currently has untracked changes and Grit will rewrite files in place. Do you want to proceed? yes
Processed 21 files and found 0 matches

References

No response

@MattJeanes MattJeanes 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 Jan 29, 2025
Copy link

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.

Copy link

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 Jan 29, 2025
@MattJeanes
Copy link
Author

Terraform logs are not relevant to this issue

@larivierec
Copy link

larivierec commented Jan 29, 2025

Can confirm the same issue is present on my end.

Image

I have even less patterns than what @MattJeanes has shown.
I'm running version v0.1.1 though.

EDIT: I also tried this from the branch containing the update to cloudflare 5.x to make sure, this provided the same result.

@MattJeanes
Copy link
Author

I don't know if adding this to your local repos .grit/grit.yaml is a step you're supposed to take or not - but adding this plus grit init made the other Cloudflare ones show up for me - but the v5 migration documentation doesn't say anything about this currently:

version: 0.0.1
patterns:
  - name: github.com/getgrit/stdlib#*
+ - name: github.com/cloudflare/terraform-provider-cloudflare#*

@jacobbednarz
Copy link
Member

there are a few things still in flight here (default branch cleanup, stdlib updates) so for now you can use grit apply github.com/cloudflare/terraform-provider-cloudflare#terraform_cloudflare_v5 to invoke it directly.

@MattJeanes
Copy link
Author

No dice unfortunately - I looked up Grit's documentation couldn't seem to find any way to load stuff from another branch which that migration seems to sit on so it's trying to load it from main where it doesn't exist when I do that (I think)

C:\_git\Abyss-Infrastructure\terraform [renovate/cloudflare-5.x ≡ +1 ~0 -0 !]> grit apply github.com/cloudflare/terraform-provider-cloudflare#terraform_cloudflare_v5
ERROR (code: 200) - pattern definition not found: terraform_cloudflare_v5. Try running grit init.

@jacobbednarz
Copy link
Member

@MattJeanes that may be a grit problem then. it's working for me locally however, i have had issues in the past with it not finding patterns outside of a grit init --global.

@MattJeanes
Copy link
Author

Huh - that does actually seem to do the trick! Weird!

Although still didn't actually change anything in my environment like the example above in my original issue I would have thought it would change to the new name, unless that's unsupported for some reason? I guess if it's still being worked on we'll have to wait and see.

In my test scenario all of my Cloudflare provider resources are cloudflare_record and all of them use for_each.

C:\_git\Abyss-Infrastructure\terraform [renovate/cloudflare-5.x ≡ +1 ~0 -0 !]> grit apply github.com/cloudflare/terraform-provider-cloudflare#terraform_cloudflare_v5
Processed 21 files and found 0 matches

@jacobbednarz
Copy link
Member

i don't understand why grit has that issue (i've tried to debug in the past with no heading) but that occasionally fixes it for me.

we mention this in the migration docs but we don't automatically migrate resource renames as it's not safe for all resources to be performed in place. you can check out the options and steps at https://registry.terraform.io/providers/cloudflare/cloudflare/latest/docs/guides/version-5-upgrade#resource-renames. the patterns for v5 are:

  • cloudflare_terraform_v5_resource_renames_configuration
  • cloudflare_terraform_v5_resource_renames_state

(these make more sense after you read the doc and choose your path)

@tchwpkgorg
Copy link

tchwpkgorg commented Jan 31, 2025

The instruction on https://registry.terraform.io/providers/cloudflare/cloudflare/latest/docs/guides/version-5-upgrade is wrong:

$ grit -V
grit 0.1.1

$ grit apply cloudflare_terraform_v5
ERROR (code: 200) - pattern definition not found: cloudflare_terraform_v5. Try running grit init.

$ grit init

$ grit init --global

$ grit apply cloudflare_terraform_v5
ERROR (code: 200) - pattern definition not found: cloudflare_terraform_v5. Try running grit init.

$ grit apply github.com/cloudflare/terraform-provider-cloudflare#terraform_cloudflare_v5
ERROR (code: 200) - pattern definition not found: terraform_cloudflare_v5. Try running grit init.

@jacobbednarz
Copy link
Member

@tchwpkgorg please read the previous comments; this is known and has already been fixed awaiting a release. you can call the patterns directly in the meantime to achieve the same thing.

@strangeman
Copy link

#4966 (comment) that solved the problem, thanks @MattJeanes!

@MattJeanes
Copy link
Author

It seems like we should update the documentation around both the grit init --global and also updating the local .grit/grit.yaml, because it doesn't say anything about this last I checked and seems to be causing some confusion. Maybe also worth noting better about the resource name/state file migration as I was under the assumption that the V5 migration would do all the migrations for V5

I would understand this more for a pre-release version but now 5.0.0 is in full release and basically everyone who uses it is going to have to do some migrations manually or otherwise the docs should probably be a bit clearer

@jacobbednarz
Copy link
Member

you shouldn't need grit init --global for patterns to work. that seems to be something broken in grit land that we're yet to get to the bottom of.

re: the grit URL to use, that was fixed shortly after 5.0.0 was released. the documentation is dependent on a new version ring release for it to appear in the registry. in the meantime, you can use the git branches or release PR for the most up to date guidance.

@Dragotic
Copy link

Still no luck...

grit apply github.com/cloudflare/terraform-provider-cloudflare#terraform_cloudflare_v5
ERROR (code: 200) - pattern definition not found: terraform_cloudflare_v5. Try running grit init.

grit init --global

grit apply github.com/cloudflare/terraform-provider-cloudflare#terraform_cloudflare_v5
ERROR (code: 200) - pattern definition not found: terraform_cloudflare_v5. Try running grit init.

@jacobbednarz
Copy link
Member

closing this one as the grit migrations as documented in next are working. the lingering issue of grit init not working is something worth raising with the grit maintainers as i have no insight into how to best debug that beyond what i've already discovered.

@morgante
Copy link
Contributor

morgante commented Feb 3, 2025

Please make sure you use the right name. It is cloudflare_terraform_v5 not terraform_cloudflare_v5.

grit apply github.com/cloudflare/terraform-provider-cloudflare#cloudflare_terraform_v5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants