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

feat: Allow the shell to be configured for workflow command steps #5024

Merged
merged 5 commits into from
Nov 4, 2024

Conversation

anryko
Copy link
Contributor

@anryko anryko commented Oct 22, 2024

what

Added an option to define the shell for the custom workflow steps with the command filed, like run, env, and multienv.

why

  • This change simplifies writing more elaborate shell logic.
  • The same functionality is achievable by running command: bash -c "my inline shell script", but dealing with double quote escaping within the shell script is quite cumbersome.
  • The default busybox shell is limited and having "native" access to bash will improve user experience.

tests

  • I run the unit tests.
  • I verified manually tested this functionality on a local setup.
  • log example:
    {"level":"debug","ts":"2024-11-03T20:45:25.618Z","caller":"models/shell_command_runner.go:114","msg":"starting 'sh -c \"echo 002\"' in '/home/atlantis/.atlantis/repos/test/infra-test/355/default/terraform/gcp/gcp-demo-1111/random'","json":{"repo":"test/infra-test","pull":"355"}}
    {"level":"info","ts":"2024-11-03T20:45:25.626Z","caller":"models/shell_command_runner.go:181","msg":"successfully ran 'sh -c \"echo 002\"' in '/home/atlantis/.atlantis/repos/test/infra-test/355/default/terraform/gcp/gcp-demo-1111/random'","json":{"repo":"test/infra-test","pull":"355","duration":0.009693026}}
    {"level":"debug","ts":"2024-11-03T20:45:25.630Z","caller":"models/shell_command_runner.go:114","msg":"starting 'bash -c \"echo ${DIR%$REPO_REL_DIR}\"' in '/home/atlantis/.atlantis/repos/test/infra-test/355/default/terraform/gcp/gcp-demo-1111/random'","json":{"repo":"test/infra-test","pull":"355"}}
    {"level":"info","ts":"2024-11-03T20:45:25.662Z","caller":"models/shell_command_runner.go:181","msg":"successfully ran 'bash -c \"echo ${DIR%$REPO_REL_DIR}\"' in '/home/atlantis/.atlantis/repos/test/infra-test/355/default/terraform/gcp/gcp-demo-1111/random'","json":{"repo":"test/infra-test","pull":"355","duration":0.032133774}}
    

references

@anryko anryko requested review from a team as code owners October 22, 2024 16:54
@anryko anryko requested review from chenrui333, nitrocode and X-Guardian and removed request for a team October 22, 2024 16:54
@github-actions github-actions bot added docs Documentation go Pull requests that update Go code labels Oct 22, 2024
@dosubot dosubot bot added the feature New functionality/enhancement label Oct 22, 2024
@X-Guardian
Copy link
Contributor

Hi @anryko, thanks for this. Any chance you can follow the same pattern as the pre/post workflow hooks to have the ability to specify the shell and shellargs. See https://www.runatlantis.io/docs/pre-workflow-hooks.html#reference.

I also see no reason to limit the shell to bash and sh.

@anryko
Copy link
Contributor Author

anryko commented Oct 23, 2024

@X-Guardian sure, I'll do that. I would suggest to make shellArgs to be a list. This would make sense because it's a list of arguments and will be consistent with extra_args property (like plan.extra_args []string).

@anryko anryko force-pushed the feat_1417_1 branch 3 times, most recently from e365fca to c7cbbe1 Compare October 30, 2024 20:57
@anryko
Copy link
Contributor Author

anryko commented Oct 30, 2024

@X-Guardian I added support for shellArgs and removed the limit for the shell option as it was suggested.

Copy link
Contributor

@X-Guardian X-Guardian left a comment

Choose a reason for hiding this comment

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

Nearly there @anryko. Once you have added the debug log lines, can you add some log snippets to the PR description as evidence that this functionality is working.

server/core/config/raw/step.go Show resolved Hide resolved
server/core/config/raw/step.go Show resolved Hide resolved
server/core/runtime/models/shell_command_runner.go Outdated Show resolved Hide resolved
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Nov 4, 2024
@X-Guardian X-Guardian enabled auto-merge (squash) November 4, 2024 18:10
@X-Guardian X-Guardian changed the title feat: set the shell for workflow command steps feat: Allow the shell to be configured for workflow command steps Nov 4, 2024
@X-Guardian X-Guardian merged commit 8285a0f into runatlantis:main Nov 4, 2024
35 checks passed
@X-Guardian
Copy link
Contributor

Thanks for your work on this @anryko. You can test it by using one of the following container images: dev-debian-8285a0f or dev-alpine-8285a0f

@anryko anryko deleted the feat_1417_1 branch November 4, 2024 20:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation feature New functionality/enhancement go Pull requests that update Go code lgtm This PR has been approved by a maintainer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow Specifying Custom Script Runner
3 participants