Skip to content

fix: Disable interpolation of HEREDOC strings containing runner hook scripts #4333

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

Merged
merged 2 commits into from
Jan 9, 2025

Conversation

Peter-Sh
Copy link
Contributor

@Peter-Sh Peter-Sh commented Jan 2, 2025

Interpolation was applied within the user-data.sh context, causing unexpected empty values for variables and other unintended effects.

While this behavior might be useful in certain cases, it requires consistently escaping everything in hook scripts. This approach feels error-prone and unintuitive.

For example, the following configuration will echo an empty string and the date of string interpolation, but not the date of the job's start:

  runner_hook_job_started: |
    echo $GITHUB_WORKSPACE
    echo $(date)

This PR disables interpolation of the hook script contents when they are being written to the file.

Also the link in user-data.sh to GitHub start/completed docs has been updated.

@Peter-Sh Peter-Sh changed the title Disable interpolation of HEREDOC strings containing runner hook scripts fix: Disable interpolation of HEREDOC strings containing runner hook scripts Jan 2, 2025
Interpolation was applied within the user-data.sh context, causing
unexpected empty values for variables and other unintentend effects.

While this behavior might be useful in certain cases, it requires
consistently escaping everything in hook scripts. This approach feels
error-prone and unintuitive.

For example, the following configuration will echo an empty string and
the date of string interpolation, but not the date of the job's start:
```
  runner_hook_job_started: |
    echo $GITHUB_WORKSPACE
    echo $(date)
```
Copy link
Member

@npalm npalm left a comment

Choose a reason for hiding this comment

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

thx, tested and all good

@npalm npalm merged commit 2622589 into github-aws-runners:main Jan 9, 2025
2 checks passed
npalm pushed a commit that referenced this pull request Jan 9, 2025
🤖 I have created a release *beep* *boop*
---


##
[6.1.0](philips-labs/terraform-aws-github-runner@v6.0.1...v6.1.0)
(2025-01-09)


### Features

* **packer:** Allow passing in an instance_profile to packer
([#4278](https://github.com/philips-labs/terraform-aws-github-runner/issues/4278))
([bb7346b](philips-labs/terraform-aws-github-runner@bb7346b))


### Bug Fixes

* Disable interpolation of HEREDOC strings containing runner hook
scripts
([#4333](https://github.com/philips-labs/terraform-aws-github-runner/issues/4333))
([2622589](philips-labs/terraform-aws-github-runner@2622589))
* **lambda:** bump the aws group in /lambdas with 6 updates
([#4336](https://github.com/philips-labs/terraform-aws-github-runner/issues/4336))
([1282e11](philips-labs/terraform-aws-github-runner@1282e11))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: forest-releaser[bot] <80285352+forest-releaser[bot]@users.noreply.github.com>
@winwinashwin
Copy link
Contributor

Thanks for this! Learned something new today :P
Just a note that the examples still escape $ which is not required after this change I believe.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants