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

Checks: Introduce check blocks into the terraform node and transform graph #32735

Merged
merged 12 commits into from Mar 23, 2023

Conversation

liamcervante
Copy link
Member

@liamcervante liamcervante commented Feb 22, 2023

This PR adds Transform and Node structures into the Terraform processing graph, ensuring that Check blocks are processed with the rest of the Terraform configuration.

It outputs the Check results into diagnostics and the plan file, using the same mechanism through which existing CheckRule blocks are exposed. CheckRules from within Check blocks do, however, only ever expose warning diagnostics instead of error diagnostics.

CheckRules within Check blocks are also not exported to the state files, as they execute on every execution of a plan or apply anyway.


This PR is part of chain of PRs introducing the new Checks feature into Terraform for v1.5. The chain of PRs is as follows:

Branch Description Pull Request
liamcervante/checks/scoped Add support for scoped resources. #32732
liamcervante/checks/addr Add Terraform addresses for new Check blocks. #32733
liamcervante/checks/configs Add config parsing new Check blocks. #32734
liamcervante/checks/graph Add nodes and transforms for processing new Check blocks. #32735

I have created the chain to make reviewing the smaller contained part of the process easier. IF you want to view all the changes together in a single PR, navigate to the last PR in the chain and compare it to the main branch and you will see all changes from all prior PRs in a single place.

@jbardin
Copy link
Member

jbardin commented Mar 22, 2023

Do checks need to run during destroy? This isn't going to work currently, because modules are not expanded during destroy, so nested check blocks will panic with no expansion has been registered for module.name. They also are not linked up in the destroy graph to anything since the normal resource nodes don't exist, and may fail to read any data they are validating.

I think what we want here is just to skip the check blocks entirely for any full destroy operation.

Base automatically changed from liamcervante/checks/configs to main March 23, 2023 08:12
@liamcervante
Copy link
Member Author

I think what we want here is just to skip the check blocks entirely for any full destroy operation.

Done!

return &nodeCheckAssert{
addr: addr,
config: cfg,
executeChecks: t.ExecuteChecks(),
Copy link
Member

Choose a reason for hiding this comment

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

Because having nodes in the graph at all can change how the destroy graph is constructed (i.e. these are currently still going to run into problems when placed in an expanding module) , I think it's better to exclude the nodes entirely whenever possible.

Copy link
Member Author

Choose a reason for hiding this comment

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

Okay, done much more strongly now!

@liamcervante liamcervante merged commit 978263e into main Mar 23, 2023
4 checks passed
@liamcervante liamcervante deleted the liamcervante/checks/graph branch March 23, 2023 15:07
@github-actions
Copy link

Reminder for the merging maintainer: if this is a user-visible change, please update the changelog on the appropriate release branch.

@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants