chore: support infinite plans #35
Conversation
hvoigt
requested review from
Bonko and
doslindos
and removed request for
a team
hvoigt
force-pushed
the
invest-infinite-plans
branch
3 times, most recently
from
e73845d to
658f5de
Compare
hvoigt
force-pushed
the
invest-infinite-plans
branch
from
658f5de to
0e578e8
Compare
| TF_WORKSPACE: stage | ||
| with: | ||
| commenter_type: fmt/init/plan/validate # Choose one | ||
| commenter_input: ${{ format('{0}{1}', steps.step_id.outputs.stdout, steps.step_id.outputs.stderr) }} |
There was a problem hiding this comment.
I would suggest to keep commenter_input in the example and add commenter_input_file as option
There was a problem hiding this comment.
I would argue that since this is the more stable option to suggest commenter_input_file as default. commenter_input is only there to stay backwards compatible for people using this action as suggested
with Jimdo/terraform-pr-commenter@main. 🤔 we could introduce versioning and make it a breaking change.
There was a problem hiding this comment.
It turns out versioning is complicated as
- our runners do like to run on this repository
- we are not able to change the default branch
so for now we will go ahead with the current PR and maybe later cleanup by introducing versioning and breaking changes.
| [...] | ||
| - name: Terraform Plan | ||
| id: plan | ||
| run: terraform plan -out workspace.plan 2>&1 | tee {{ github.workspace }}/terraform-plan.out |
There was a problem hiding this comment.
should error messages really be in the plan output?
why is the outputfile written 2 times?
There was a problem hiding this comment.
should error messages really be in the plan output?
This is how we pass the output currently stderr and stdout combined. Maybe this is wrong but I would change that in a separate commit/PR.
why is the outputfile written 2 times?
What do you mean? The -out option does not write the output meant for human consumption. It is meant for apply.
-out=path Write a plan file to the given path. This can be
used as input to the "apply" command.
action.yml
Outdated
| required: false | ||
| default: '' | ||
| commenter_input_file: | ||
| description: 'the comment to post from a previous step output passed as file name (located in )' |
There was a problem hiding this comment.
Yes I had {{ github.workspace }} in there but it leads to an error when executing the action as it seems to be replaced.
| comment="$4" | ||
| file="$5" | ||
|
|
||
| if [ "$file" ]; then |
There was a problem hiding this comment.
| if [ "$file" ]; then | |
| if [ -s "$file" ]; then |
test if its a non-empty file
There was a problem hiding this comment.
That is the default for test:
man test
[...]
string True if string is not the null string.
Passing a file that does not exist should result in an error and not silently lead to input_file being used if it was passed. We are not strict enough for that case at the moment.
| fi | ||
|
|
||
| comment="$4" | ||
| file="$5" |
There was a problem hiding this comment.
does this work if input is empty?
Then $2 should be empty and everything is shifted left, so file would be $4
There was a problem hiding this comment.
The parameters are passed unconditionally here:
terraform-pr-commenter/action.yml
Lines 26 to 34 in 0e578e8
I would expect github action to not do any magic behind the scenes. So there is no shifting being done it will just be an empty string passed.
Sometimes terraform plans become so large that they do not fit into the
command line as arguments. As the path ${{ github.workspace }} is
mounted automatically as /github/workspace in the container we can use
that to pass the input as file.
To stay backwards compatible we add support to pass the input using a
file.
hvoigt
force-pushed
the
invest-infinite-plans
branch
from
0e578e8 to
c3bd1ae
Compare
Sometimes plans become so large that they do not fit into the command line as args. To stay backwards compatible lets us add support to pass the input using a file.
Tested in a repository with a large plan output.
References: