Skip to content

fix: Fix dangling references in provisioner/terraform/testdata #3193

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

Merged
merged 1 commit into from
Jul 26, 2022

Conversation

dwahler
Copy link
Contributor

@dwahler dwahler commented Jul 25, 2022

A recent PR (#3150) updated several of the Terraform test data files in provisioner/terraform/testdata to rename agent resources from dev to main. However, some references to those resources were still using the old names, causing the Terraform modules to fail to compile. This wasn't caught in CI because the test suite is still using plan/state files that were generated before the change.

This PR fixes those references, regenerates the Terraform plan/state output files so that they match the .tf files they're derived from (using provisioner/terraform/testdata/generate.sh), and updates the Go test cases to expect the new names.

The changes to the generated .tfplan/.tfstate files are a bit verbose, but they consist entirely of:

  • Replacing dev to main
  • Bumping the terraform-provider-coder version in the state from 1.2.2 to 1.2.5
  • Random values (e.g. UUIDs) that were regenerated

@dwahler dwahler requested a review from a team July 25, 2022 23:36
Copy link
Member

@johnstcn johnstcn left a comment

Choose a reason for hiding this comment

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

❤️

@johnstcn
Copy link
Member

I'm guessing that we can't run provisioner/terraform/testdata/generate.sh in every CI instance, because the UUIDs aren't deterministic and would just change every time?

Copy link
Member

@mafredri mafredri left a comment

Choose a reason for hiding this comment

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

Nice find!

@dwahler
Copy link
Contributor Author

dwahler commented Jul 26, 2022

I'm guessing that we can't run provisioner/terraform/testdata/generate.sh in every CI instance, because the UUIDs aren't deterministic and would just change every time?

That would be nice, and I can think of a couple of different ways we could approach it:

  1. Only check in the .tf files, and regenerate the plan/state files on every CI run. This is straightforward but has the disadvantage that you would no longer be able to just check out the repository and run go test.
  2. Keep the plan/state files under source control, re-run generate.sh on every CI run, and enforce that the checked-in version matches the generated version except for IDs. This is more involved, but I think still reasonably doable using jq, which we're already using for pretty-printing.

One of those (or something similar) is probably worth doing, but not super urgent IMO, since these tests don't change very frequently.

There's also the concern of slowing down our CI runs. In my dev environment, generate.sh takes about 20 seconds to run. Not huge, but little things like that add up over time.

@dwahler dwahler merged commit fbd1a27 into main Jul 26, 2022
@dwahler dwahler deleted the dwahler/fix-terraform-testdata branch July 26, 2022 17:04
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 this pull request may close these issues.

3 participants