Datasource logic cleanup #12608
Datasource logic cleanup #12608
Conversation
|
Note: marking as draft for now since I realise the change in the datasource evaluation scheme may increase the likelihood of panics appearing in client code. We definitely need to figure out a more reliable way to detect dependencies before we can proceed with merging this PR. |
lbajolet-hashicorp
force-pushed
the
datasource_logic_cleanup
branch
from
70983d1 to
1617134
Compare
|
Update: after discussing and investigating some more, this code should work on our templates, both in JSON and HCL. I have not seen in the code or the docs other types being possible after parsing, without some extra wrapping, so I would think handling both cases makes it reasonably reliable to get the dependencies of a datasource. In the meantime, I suggest we review and merge this eventually, as it should make dependency detection between datasources more reliable, and reduce the likelihood of evaluation in a wrong order. |
|
So for clarity, I mention #12607 as it is part of the series of fixes/improvements that I stumbled upon while working on refactoring the execution logic. |
There was a problem hiding this comment.
This looks good to me. Nicely done on distilling the datasource parsing and execution logic. The utils for getting variable attributes from an HCL body looks good.
I left one suggestion to help with readability but this is otherwise good. Feel free to ping once you've rolled in the suggested changes.
Not sure why this was defined and returned, but the value was set, but never used, as such this is not useful to keep in the code, so let's simplify this now.
Since datasources are recursively evaluated depending on their dependencies, we don't need to pre-execute those that depend on nothing, as the recursive traversal of the datasources will take care of that for us.
When a datasource fails to be evaluated because a cycle has been detected, we point out one of the links of the chain now so that users have a better idea of what to look at.
Datasources use their attribute's expressions to determine whether or not they depend on another datasource, in order to get the list of dependencies and execute them before executing a datasource. This code may be useful later on for figuring out the dependencies for any block, so we move this code to the utils.go file, and use this for datasources.
The previous implementation of the GetVarsByType function worked only on top-level attributes, ignoring the nested blocks in the structure. This implies that if a datasource depends on another through an expression within a nested block, we may not execute it first, and then executing this datasource before its dependent is possible, resulting in an error in the end. This commit is an attempt at making this more reliable for HCL configs, but only works on configs lifted from HCL files for now. We need to make this more reliable for later iterations.
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
lbajolet-hashicorp
force-pushed
the
datasource_logic_cleanup
branch
from
1617134 to
febaada
Compare
lbajolet-hashicorp
added
bug
backport/1.9.x
|
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 issues. |
This PR is linked to #12607 as it is part of the ongoing refactoring efforts regarding the HCL parsing/execution logic.
This focuses on datasources, and simplifies the code a bit by not invoking the datasources that have no dependency prior to invoking the recursive execution function.
The code to extract the variables from the expressions in the block is moved away from the datasource evaluation as it will be reusable for other block types, but needs to be improved as it only works on the top-level block here, which makes dependency evaluation for datasources dependent on their presence as an attribute in a non-nested block.
Cycle detection now also returns the location of the conflict, this way it is easier for users to troubleshoot their configs if such a situation occurs.