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.

Sign up for GitHub

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

SONARIAC-1856 Modify S7019: add EXEC alternatives and exceptions #4597

Merged
merged 9 commits into from
Jan 8, 2025
Merged

SONARIAC-1856 Modify S7019: add EXEC alternatives and exceptions #4597

Changes from 6 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
e2b4061
SONARIAC-1856 Update S7019 content
rudy-regazzoni-sonarsource Jan 7, 2025
39abaf7
Merge branch 'master' into SONARIAC-1856
rudy-regazzoni-sonarsource Jan 7, 2025
f90fd2f
Remove script example
rudy-regazzoni-sonarsource Jan 7, 2025
3300b75
Merge branch 'SONARIAC-1856' of https://github.com/SonarSource/rspec …
rudy-regazzoni-sonarsource Jan 7, 2025
0719796
Fix id
rudy-regazzoni-sonarsource Jan 7, 2025
8f00905
Merge branch 'master' into SONARIAC-1856
rudy-regazzoni-sonarsource Jan 7, 2025
dd2372c
Update rules/S7019/docker/rule.adoc
rudy-regazzoni-sonarsource Jan 8, 2025
2b73aba
Address review comment
rudy-regazzoni-sonarsource Jan 8, 2025
c4b533b
Merge branch 'SONARIAC-1856' of https://github.com/SonarSource/rspec …
rudy-regazzoni-sonarsource Jan 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions rules/S7019/docker/rule.adoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,15 @@ This can cause problems when trying to gracefully stop containers because the ma
Moreover, the exec form provides more control and predictability over the execution of the command.
It does not invoke a command shell, which means it does not have the potential side effects of shell processing.

Although, the exec form does not allow shell features such as variable expansion, piping (`|`) and command chaining (`&&`, `||`, `;`).
In case you need to use these features, there is few alternatives:
rudy-regazzoni-sonarsource marked this conversation as resolved.
Show resolved Hide resolved
- create a wrapper script
rudy-regazzoni-sonarsource marked this conversation as resolved.
Show resolved Hide resolved
- explicitly specify the shell with the `SHELL` instruction before the `CMD` or `ENTRYPOINT` instruction

=== Exceptions

As mentioned above, this rule will not raise an issue if the `SHELL` instruction is used before the `CMD` or `ENTRYPOINT` instruction, as we consider this is a conscious decision.

rudy-regazzoni-sonarsource marked this conversation as resolved.
Show resolved Hide resolved
== How to fix it

=== Code examples
Expand All @@ -22,6 +31,22 @@ FROM scratch
ENTRYPOINT echo "Welcome!"
----

[source,docker,diff-id=2,diff-type=noncompliant]
----
FROM scratch
ENTRYPOINT echo "Long script with chaining commands" \
&& echo "Welcome!" \
&& echo "Goodbye"
----

[source,docker,diff-id=3,diff-type=noncompliant]
----
FROM scratch
ENTRYPOINT echo "Long script with chaining commands" \
&& echo "Welcome!" \
&& echo "Goodbye"
----

Copy link
Contributor

Choose a reason for hiding this comment

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

I would remove the diff-id=3 noncompliant example.
Keep only the compliant one with the wrapper script, but add the relation to a non compliant one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah very good point! I didn't know we could link multiple compliant examples to a single non-compliant one. Way better!

Copy link
Contributor

Choose a reason for hiding this comment

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

That's not what I meant, sorry for the confusion.
I meant that diff-id and diff-type will be removed from the wrapper file solution.
Unfortunately we can only use one diff-id per unique example, so I we need to adress this in a follow up PR 😅

==== Compliant solution

[source,docker,diff-id=1,diff-type=compliant]
Expand All @@ -30,6 +55,21 @@ FROM scratch
ENTRYPOINT ["echo", "Welcome!"]
----

[source,docker,diff-id=2,diff-type=compliant]
----
FROM scratch
SHELL ["/bin/bash", "-c"]
ENTRYPOINT echo "Long script with chaining commands" \
&& echo "Welcome!" \
&& echo "Goodbye"
----

[source,docker,diff-id=3,diff-type=compliant]
----
FROM scratch
ENTRYPOINT ["/entrypoint.sh"]
----

== Resources
=== Documentation

Expand Down
Loading