Skip to content

Docker CMD compatibility#7861

Merged
Alkarex merged 7 commits intoFreshRSS:edgefrom
Alkarex:docker-cmd-sh
Aug 27, 2025
Merged

Docker CMD compatibility#7861
Alkarex merged 7 commits intoFreshRSS:edgefrom
Alkarex:docker-cmd-sh

Conversation

@Alkarex
Copy link
Member

@Alkarex Alkarex commented Aug 26, 2025

Some caller systems do not seem to obey the SHELL instruction (or may be wrongly escaping quotes)
fix #7859 (comment)
fix #5611
fix #7267

CMD ([ -z "$CRON_MIN" ] || cron) && \
. /etc/apache2/envvars && \
exec apache2 -D FOREGROUND $([ -n "$OIDC_ENABLED" ] && [ "$OIDC_ENABLED" -ne 0 ] && echo '-D OIDC_ENABLED')
CMD sh -c '[ -z "$CRON_MIN" ] || cron; . /etc/apache2/envvars; if [ "${OIDC_ENABLED:-0}" -ne 0 ]; then exec apache2 -D FOREGROUND -D OIDC_ENABLED; else exec apache2 -D FOREGROUND; fi'
Copy link
Member

Choose a reason for hiding this comment

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

Nothing in that command is Bash syntax so I'd rather not rewrite the command without the && to fail early for example.

Copy link
Member

Choose a reason for hiding this comment

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

Or put another way, you could just change the SHELL command to /bin/sh.

Copy link
Member Author

Choose a reason for hiding this comment

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

Edits welcome

Copy link
Member

@Frenzie Frenzie Aug 27, 2025

Choose a reason for hiding this comment

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

I wouldn't be inclined to edit anything at all.

You might phrase that as:

Suggested change
CMD sh -c '[ -z "$CRON_MIN" ] || cron; . /etc/apache2/envvars; if [ "${OIDC_ENABLED:-0}" -ne 0 ]; then exec apache2 -D FOREGROUND -D OIDC_ENABLED; else exec apache2 -D FOREGROUND; fi'
CMD sh -c '([ -z "$CRON_MIN" ] || cron) && . /etc/apache2/envvars && exec apache2 -D FOREGROUND $([ -n "$OIDC_ENABLED" ] && [ "$OIDC_ENABLED" -ne 0 ] && echo "-D OIDC_ENABLED")'

But in doing so I just realized something. Naturally I had to change echo '-D OIDC_ENABLED' to make that work, which might indicate they're not ignoring the SHELL instruction but are astoundingly bad at escaping, because it'd need to become either \' or " to work as part of -c (or vice versa if you use -c "").

Which suggests that maybe you just need to change the quotes:

Suggested change
CMD sh -c '[ -z "$CRON_MIN" ] || cron; . /etc/apache2/envvars; if [ "${OIDC_ENABLED:-0}" -ne 0 ]; then exec apache2 -D FOREGROUND -D OIDC_ENABLED; else exec apache2 -D FOREGROUND; fi'
CMD ([ -z "$CRON_MIN" ] || cron) && \
. /etc/apache2/envvars && \
exec apache2 -D FOREGROUND $([ -n "$OIDC_ENABLED" ] && [ "$OIDC_ENABLED" -ne 0 ] && echo "-D OIDC_ENABLED")

Because if you don't do that, the error you'll see is of course:

Syntax error: end of file unexpected (expecting ")")

Which I believe is the exact error in the referenced issue.

Copy link
Member

@Frenzie Frenzie Aug 27, 2025

Choose a reason for hiding this comment

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

In fact on closer inspection, the specific form of the error from #7859 (comment)

OIDC_ENABLED): -c: line 2: unexpected EOF while looking for matching `)'

is how Bash presents that syntax error! The one I quoted above was from Dash.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, could be good to test with one of the failing systems

Copy link
Member Author

Choose a reason for hiding this comment

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

What about now?
c8f3ec8

Copy link
Member

Choose a reason for hiding this comment

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

lgtm in any case

@Alkarex Alkarex changed the title Docker use only sh for CMD Docker CMD compatibility Aug 27, 2025
@Alkarex
Copy link
Member Author

Alkarex commented Aug 27, 2025

Ok, now it only boils down to changing the quotes, which should be safe enough to merge

@Alkarex Alkarex merged commit 6c64e7b into FreshRSS:edge Aug 27, 2025
1 check passed
@Alkarex Alkarex deleted the docker-cmd-sh branch August 27, 2025 12:34
@Alkarex Alkarex modified the milestones: 1.28.0, 1.27.1 Sep 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Docker 🐋 Everything related to Docker

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Latest docker image results in start loop, extensions folder permissions issue

2 participants