feat: Add Support for Customising the Shell for Pre and Post Workflow Hooks

[X-Guardian]

what

Update the pre and post workflow hooks with additional shell and shellArgs properties to allow customisation of the shell
used to run the hooks.

Grammar fixes have also been actioned on the contents of the runatlantis.io/docs/pre-workflow-hooks.md file.

why

• Closes feat: Add Support for Customising the Shell for Pre and Post Workflow Hooks #3447
• Closes Can we use bash for custom workflow's run step? #2177

tests

Default Shell

repos:
- id: /.*/
  pre_workflow_hooks:
    -  description: pre workflow shell test
       run: echo "pre workflow $0"
  post_workflow_hooks:
    -  description: post workflow shell test
       run: echo "post workflow $0"

Pre Workflow Output

Post Workflow Output

Custom Shell

repos.yaml

repos:
- id: /.*/
  pre_workflow_hooks:
    -  description: pre workflow shell test
       shell: bash
       run: echo "pre workflow $0"
  post_workflow_hooks:
    -  description: post workflow shell test
       shell: bash
       run: echo "post workflow $0"

Pre Workflow Output

Post Workflow Output

Custom ShellArgs

repos.yaml

repos:
- id: /.*/
  pre_workflow_hooks:
    -  description: pre workflow shell test
       shellArgs: -cx
       run: echo "pre workflow $0"
  post_workflow_hooks:
    -  description: post workflow shell test
       shellArgs: -cx
       run: echo "post workflow $0"

Pre Workflow Output

Post Workflow Output

Custom Shell and ShellArgs

repos.yaml

repos:
- id: /.*/
  pre_workflow_hooks:
    -  description: pre workflow shell test
       shell: bash
       shellArgs: -cv
       run: echo "pre workflow $0"
  post_workflow_hooks:
    -  description: post workflow shell test
       shell: bash
       shellArgs: -cv
       run: echo "post workflow $0"

Pre Workflow Output

Post Workflow Output

[nitrocode]

Thank you for the contribution but can we also add this for run steps, not just for pre and post hooks?

[X-Guardian]

I'm sure that is possible, but is outside the scope of this issue/feature request, so not within this PR.

[nitrocode]

To me, it doesn't make sense to do it in one section and not in another. It's confusing and some people will reach out and ask why it's implemented in workflow hooks which is a recent feature but not in "steps" which is the one of the oldest features.

Please reconsider.

[X-Guardian]

I hate scope creep in PRs. The code is completely separate for the two features, so it can be done in a different PR.

[jamengual]

I will prefer in different PRs, pre and post workflow hooks are very specific steps compared with run and some people do not use pre/post workflows but I will say 99% people use the run command. Separating the PR make it easier to revert in case affects any of the two.

[nitrocode]

I appreciate your work and am thankful for this contribution. I'm just concerned that this pr may merge and a follow up for overriding the shell for run steps isn't contributed leaving the project with a half finished feature. I'm ok with 2 PRs as well.

[nitrocode]

One thing I'd like to call out too. It's odd that this logic is in multiple places which then requires separate prs. We'd want the pre/post/regular run steps to all use the same logic so if the logic was updated, it would impact all 3. It would seem that all 3 are currently using a copy and pasted interface instead of using a shared interface.

It would be a great contribution if the follow up PR could help refactor and make the code more DRY. But i know i may be asking for too much here. Thank you again for your effort.

[X-Guardian]

@nitrocode, can you re-review this PR?

[nitrocode]

Hi @X-Guardian , i appreciate all your work but my original comment hasn't been addressed.

I'd prefer to merge a pr to change the interpretter for the individual run steps prior to merging this pr for pre and post workflow hooks.

[X-Guardian]

OK @nitrocode. I have to say I am disappointed with your response to this. Just so you know, I won't be implementing this request as I have no need for it, and the original scope of the issue and PR was always only for the pre/post workflows.

[nitrocode]

I'm also disappointed as I think if and when this pr is merged, the implementation will be half baked because it will be implemented on workflow hooks but not run steps. I also understand that you do not want to finish that implementation. Please remember that this is volunteer based on both contributors and reviewers.

cc @jamengual has volunteered to take over this. Thanks for being patient @X-Guardian

[jamengual]

LGTM

[jamengual]

@GenPage any thoughs?

[GenPage]

I think we can resolve this easily without disappointments. It would be an easy quick win to add this feature to the run_step_runner based on the changes I see here.

However, it does not seem prudent to gate this PR on the basis of requiring the contributor to perform a refactor or expanding PR scope if they've already refused. This project is very much heavily driven by contributors in terms of adding features and code changes.

Given the situation, if someone wants this added to the run step, they will have a solid template for that work in this PR.

[GenPage]

LGTM

[X-Guardian]

@GenPage, @jamengual this PR needs another review as I needed to resolve some conflicts.

[GenPage]

@X-Guardian Looks like the other PR conflicts the tests with this one.