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

Set shell options during docker builds #310

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

julienp
Copy link
Contributor

@julienp julienp commented Oct 30, 2024

Set stricter options for shell commands, particulary pipefail so that commands like curl ... | sh fail the build when the curl download fails. Without this option, the shell will only consider the exit code of the last command in the pipeline, which might erroneously succeed.

Our tests typically catch builds of bad images like this, but it can be confusing to debug the test failures. Let's make sure we fail as early as possible.

Base automatically changed from julienp/more-versions to main October 30, 2024 12:55
@julienp julienp added the impact/no-changelog-required This issue doesn't require a CHANGELOG update label Oct 30, 2024
Set stricter options for shell commands, particulary `pipefail` so that commands like `curl … | bash` fail the build when the curl download fails. Without this option, the shell will only consider the exit code of the last command in the pipeline, which might erroneously succeed.
@julienp julienp marked this pull request as ready for review October 30, 2024 13:50
@julienp julienp requested a review from a team as a code owner October 30, 2024 13:50
@tgummerer
Copy link
Contributor

My docker setup locally is broken right now, so I can't test it, but wouldn't this affect the default shell for users as well, and they would get different behaviour of the shell than they would expect normally?

@julienp
Copy link
Contributor Author

julienp commented Oct 30, 2024

My docker setup locally is broken right now, so I can't test it, but wouldn't this affect the default shell for users as well, and they would get different behaviour of the shell than they would expect normally?

Indeed it does. Maybe instead we should just set these options for specific steps, for example

RUN set -exo pipefail && curl ... | sh

@tgummerer
Copy link
Contributor

One option would be to reset the shell to just be bash -c before the end of the dockerfile. If we just want to set the options on specific steps I would probably just rewrite them as

RUN curl ... >install.sh && sh install.sh

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
impact/no-changelog-required This issue doesn't require a CHANGELOG update
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants