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

Run conftest on tfmigrate-plan #511

Closed
kyontan opened this issue Sep 12, 2022 · 10 comments · Fixed by #516
Closed

Run conftest on tfmigrate-plan #511

kyontan opened this issue Sep 12, 2022 · 10 comments · Fixed by #516
Labels
enhancement New feature or request
Milestone

Comments

@kyontan
Copy link
Sponsor Contributor

kyontan commented Sep 12, 2022

Hi, this is a some kind of bug report, and feature request.

I noticed that the conftest validation doesn't run during tfmigrate-plan action.
We noticed that the resource definition violating our conftest policy in terraform-plan after tfmigrate'd.

I think this part in terraform-plan should also be in tfmigrate-plan.

if [ -d "$ROOT_DIR/policy" ]; then
github-comment exec -- terraform show -json tfplan.binary >tfplan.json
conftest -v # Install conftest in advance to exclude aqua lazy install log from github-comment's comment
github-comment exec \
--config "${GITHUB_ACTION_PATH}/github-comment.yaml" \
-var "tfaction_target:$TFACTION_TARGET" \
-k conftest -- \
conftest test --no-color -p "$ROOT_DIR/policy" tfplan.json
fi

Furthermore, there's a edge case related to this: skip: prefix used in tfmgirate multi-state migration.
If a working directory that was added resources skipped tfmigrate-plan by skip:xxx label, this would cause an issue.
To fix the case, I propose splitting build matrix from [terraform, tfmigrate] to [terraform, tfmigrate, conftest (or validation)]. What do you think?

@suzuki-shunsuke suzuki-shunsuke added bug Something isn't working enhancement New feature or request and removed bug Something isn't working labels Sep 12, 2022
@suzuki-shunsuke
Copy link
Owner

Thank you for your proposal.

I understood your problem.

I propose splitting build matrix from [terraform, tfmigrate] to [terraform, tfmigrate, conftest (or validation)]. What do you think?

conftest is run against terraform's plan file.
tfmigrate doesn't create a plan file, so we have to run terraform plan and conftest apart from tfmigrate.
But if we run terraform plan, the result is different from terraform plan tfmigrate runs internally, because the result of state migration isn't reflected.
So the result of conftest may be unexpected.

@kyontan
Copy link
Sponsor Contributor Author

kyontan commented Sep 12, 2022

But if we run terraform plan, the result is different from terraform plan tfmigrate runs internally, because the result of state migration isn't reflected.
So the result of conftest may be unexpected.

Ah, I forget about that, that's right.
Since it's difficult (or no way) to get plan.json before terraform plan, I understood its difficulty.

Can we change conftest target from plan.json to hcl directly?
It seems conftest supports it.
https://www.conftest.dev/examples/

@suzuki-shunsuke
Copy link
Owner

suzuki-shunsuke commented Sep 12, 2022

Can we change conftest target from plan.json to hcl directly?

I guess conftest can't evaluate Terraform expression (local value, input variables, functions, resource and module attributes, etc).

tfmigrate doesn't create a plan file

I'm sorry but this may be wrong.
I verified the following ways to create a plan file with tfmigrate.

  1. tfmigrate plan's --out option
  2. Terraform's environment variable TF_CLI_ARGS_plan

1. tfmigrate plan's --out option

I've confirmed it works well, but this option is deprecated.

minamijoyo/tfmigrate@dba98a6

Actually, the plan file can't be used for apply, but can be used for conftest.
But this option would be removed and we should avoid to use deprecated option.

2. Terraform's environment variable TF_CLI_ARGS_plan

I expected we can pass terraform plan option to tfmigrate plan with TF_CLI_ARGS_plan.

$ env TF_CLI_ARGS_plan="-out=tfplan.out $TF_CLI_ARGS_plan" tfmigrate plan

But unfortunately plan file isn't created. I don't know why... 🤔

This approach is recommended by the author of tfmigrate, so it should work.
minamijoyo/tfmigrate#27 (comment)

@suzuki-shunsuke
Copy link
Owner

@suzuki-shunsuke
Copy link
Owner

I understood why TF_CLI_ARGS_plan="-out=tfplan.out doesn't work.
minamijoyo/tfmigrate#106 (comment)

@suzuki-shunsuke
Copy link
Owner

tfmigrate has reverted the deprecation of the tfmigrate plan --out=tfplan option.

So we can fix tfmigrate-plan to run conftest.

@suzuki-shunsuke
Copy link
Owner

@suzuki-shunsuke
Copy link
Owner

Published the prerelease version v0.5.18-0. https://github.com/suzuki-shunsuke/tfaction/releases/tag/v0.5.18-0
We'll release v0.5.18 in few days.
We'll really appreciate if you try the prerelease version before releasing v0.5.18.
Thanks.

@kyontan
Copy link
Sponsor Contributor Author

kyontan commented Sep 14, 2022

Thanks for your great work!
I'll try the new version.

@suzuki-shunsuke
Copy link
Owner

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

Successfully merging a pull request may close this issue.

2 participants