Skip to content

Commit 6be769d

Browse files
GabinL21sonartech
GabinL21
authored and
sonartech
committed
SONARIAC-1840 Should only raise on instructions in the final image
1 parent ac90d86 commit 6be769d

File tree

9 files changed

+103
-87
lines changed

9 files changed

+103
-87
lines changed

‎iac-extensions/docker/src/main/java/org/sonar/iac/docker/checks/AbstractFinalImageCheck.java‎

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -16,31 +16,47 @@
1616
*/
1717
package org.sonar.iac.docker.checks;
1818

19+
import java.util.function.BiConsumer;
1920
import org.sonar.iac.common.api.checks.CheckContext;
2021
import org.sonar.iac.common.api.checks.IacCheck;
2122
import org.sonar.iac.common.api.checks.InitContext;
23+
import org.sonar.iac.common.extension.visitors.TreeContext;
24+
import org.sonar.iac.common.extension.visitors.TreeVisitor;
2225
import org.sonar.iac.docker.checks.utils.MultiStageBuildInspector;
2326
import org.sonar.iac.docker.tree.api.Body;
2427
import org.sonar.iac.docker.tree.api.DockerTree;
25-
import org.sonar.iac.docker.tree.api.RunInstruction;
2628

2729
public abstract class AbstractFinalImageCheck implements IacCheck {
2830

31+
private final TreeVisitor<FinalImageContext> visitor = new TreeVisitor<>();
32+
2933
@Override
3034
public void initialize(InitContext init) {
31-
init.register(Body.class, this::checkBody);
35+
init.register(Body.class, this::processBody);
36+
initializeOnFinalImage();
37+
}
38+
39+
protected abstract void initializeOnFinalImage();
40+
41+
protected <T extends DockerTree> void register(Class<T> cls, BiConsumer<CheckContext, T> consumer) {
42+
visitor.register(cls, (ctx, node) -> consumer.accept(ctx.checkContext, node));
3243
}
3344

34-
private void checkBody(CheckContext ctx, Body body) {
45+
private void processBody(CheckContext checkContext, Body body) {
3546
var multiStageBuildInspector = MultiStageBuildInspector.of(body);
47+
var stages = body.dockerImages();
48+
var finalImageContext = new FinalImageContext(checkContext);
3649

37-
body.dockerImages().stream()
38-
.flatMap(image -> image.instructions().stream())
39-
.filter(instruction -> instruction.is(DockerTree.Kind.RUN))
40-
.map(RunInstruction.class::cast)
41-
.filter(multiStageBuildInspector::isInFinalImage)
42-
.forEach(runInstruction -> checkRunInstructionFromFinalImage(ctx, runInstruction));
50+
stages.stream()
51+
.filter(multiStageBuildInspector::isStageInFinalImage)
52+
.forEach(stage -> visitor.scan(finalImageContext, stage));
4353
}
4454

45-
protected abstract void checkRunInstructionFromFinalImage(CheckContext ctx, RunInstruction runInstruction);
55+
static class FinalImageContext extends TreeContext {
56+
public final CheckContext checkContext;
57+
58+
public FinalImageContext(CheckContext checkContext) {
59+
this.checkContext = checkContext;
60+
}
61+
}
4662
}

‎iac-extensions/docker/src/main/java/org/sonar/iac/docker/checks/ConsecutiveRunInstructionCheck.java‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,6 @@
2222
import java.util.Set;
2323
import org.sonar.check.Rule;
2424
import org.sonar.iac.common.api.checks.CheckContext;
25-
import org.sonar.iac.common.api.checks.IacCheck;
26-
import org.sonar.iac.common.api.checks.InitContext;
2725
import org.sonar.iac.common.api.checks.SecondaryLocation;
2826
import org.sonar.iac.docker.tree.api.DockerImage;
2927
import org.sonar.iac.docker.tree.api.DockerTree;
@@ -32,17 +30,17 @@
3230
import org.sonar.iac.docker.tree.api.RunInstruction;
3331

3432
@Rule(key = "S7031")
35-
public class ConsecutiveRunInstructionCheck implements IacCheck {
33+
public class ConsecutiveRunInstructionCheck extends AbstractFinalImageCheck {
3634
private static final String PRIMARY_MESSAGE = "Merge this RUN instruction with the consecutive ones.";
3735
private static final String SECONDARY_MESSAGE = "consecutive RUN instruction";
3836

3937
@Override
40-
public void initialize(InitContext init) {
41-
init.register(DockerImage.class, ConsecutiveRunInstructionCheck::checkFromRunsInstruction);
38+
protected void initializeOnFinalImage() {
39+
register(DockerImage.class, this::checkStageFromFinalImage);
4240
}
4341

44-
private static void checkFromRunsInstruction(CheckContext ctx, DockerImage image) {
45-
var listOfConsecutiveRunInstructions = extractConsecutiveRunInstructions(image);
42+
protected void checkStageFromFinalImage(CheckContext ctx, DockerImage stage) {
43+
var listOfConsecutiveRunInstructions = extractConsecutiveRunInstructions(stage);
4644
for (List<RunInstruction> consecutiveRunInstructions : listOfConsecutiveRunInstructions) {
4745
var secondaryLocations = consecutiveRunInstructions.stream()
4846
.skip(1)

‎iac-extensions/docker/src/main/java/org/sonar/iac/docker/checks/PackageInstallationCacheCheck.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,10 @@ private static CommandDetector buildRemoveCacheDetector(Set<String> cacheLocatio
106106
}
107107

108108
@Override
109+
protected void initializeOnFinalImage() {
110+
register(RunInstruction.class, this::checkRunInstructionFromFinalImage);
111+
}
112+
109113
public void checkRunInstructionFromFinalImage(CheckContext ctx, RunInstruction runInstruction) {
110114
List<ArgumentResolution> resolvedArgument = CheckUtils.resolveInstructionArguments(runInstruction);
111115
Set<String> commandWithMountedCache = computeCachedCommands(retrieveMountedCachePath(runInstruction));

‎iac-extensions/docker/src/main/java/org/sonar/iac/docker/checks/SecretsGenerationCheck.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,10 @@ public class SecretsGenerationCheck extends AbstractFinalImageCheck {
216216
CURL_USER_SHORT_FLAG_SPACE_PWD);
217217

218218
@Override
219+
protected void initializeOnFinalImage() {
220+
register(RunInstruction.class, this::checkRunInstructionFromFinalImage);
221+
}
222+
219223
public void checkRunInstructionFromFinalImage(CheckContext ctx, RunInstruction runInstruction) {
220224
List<ArgumentResolution> resolvedArgument = CheckUtils.resolveInstructionArguments(runInstruction);
221225

‎iac-extensions/docker/src/main/java/org/sonar/iac/docker/checks/utils/MultiStageBuildInspector.java‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,10 @@
2222
import java.util.Optional;
2323
import java.util.Set;
2424
import org.sonar.iac.docker.symbols.ArgumentResolution;
25-
import org.sonar.iac.docker.tree.TreeUtils;
2625
import org.sonar.iac.docker.tree.api.Alias;
2726
import org.sonar.iac.docker.tree.api.Argument;
2827
import org.sonar.iac.docker.tree.api.Body;
2928
import org.sonar.iac.docker.tree.api.DockerImage;
30-
import org.sonar.iac.docker.tree.api.DockerTree;
31-
import org.sonar.iac.docker.tree.api.Instruction;
3229
import org.sonar.iac.docker.tree.api.SyntaxToken;
3330

3431
public final class MultiStageBuildInspector {
@@ -49,11 +46,8 @@ public static boolean isLastStage(DockerImage dockerImage) {
4946
return dockerImage == getLastStage(body);
5047
}
5148

52-
public boolean isInFinalImage(Instruction instruction) {
53-
return TreeUtils.firstAncestorOfKind(instruction, DockerTree.Kind.DOCKERIMAGE)
54-
.map(DockerImage.class::cast)
55-
.map(instructionStage -> isLastStage(instructionStage) || isStageDependencyOfFinalStage(instructionStage))
56-
.orElse(true);
49+
public boolean isStageInFinalImage(DockerImage dockerImage) {
50+
return isLastStage(dockerImage) || isStageDependencyOfFinalStage(dockerImage);
5751
}
5852

5953
private boolean isStageDependencyOfFinalStage(DockerImage stage) {

‎iac-extensions/docker/src/test/java/org/sonar/iac/docker/checks/ConsecutiveRunInstructionCheckTest.java‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,12 @@
2121
class ConsecutiveRunInstructionCheckTest {
2222

2323
@Test
24-
void shouldVerifyCheck() {
24+
void shouldRaiseIssuesOnNonCompliantInstructions() {
2525
DockerVerifier.verify("ConsecutiveRunInstructionCheck/ConsecutiveRunInstructionCheck.dockerfile", new ConsecutiveRunInstructionCheck());
2626
}
27+
28+
@Test
29+
void shouldRaiseIssuesOnInstructionsInFinalImageOnly() {
30+
DockerVerifier.verify("ConsecutiveRunInstructionCheck/ConsecutiveRunInstructionCheck_multiStage.dockerfile", new ConsecutiveRunInstructionCheck());
31+
}
2732
}

‎iac-extensions/docker/src/test/java/org/sonar/iac/docker/checks/utils/MultiStageBuildInspectorTest.java‎

Lines changed: 17 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -22,16 +22,12 @@
2222
import org.junit.jupiter.api.Test;
2323
import org.sonar.iac.docker.tree.api.Body;
2424
import org.sonar.iac.docker.tree.api.DockerImage;
25-
import org.sonar.iac.docker.tree.api.Instruction;
26-
import org.sonar.iac.docker.tree.api.RunInstruction;
2725
import org.sonar.iac.docker.tree.impl.AliasImpl;
2826
import org.sonar.iac.docker.tree.impl.ArgumentImpl;
2927
import org.sonar.iac.docker.tree.impl.BodyImpl;
3028
import org.sonar.iac.docker.tree.impl.DockerImageImpl;
3129
import org.sonar.iac.docker.tree.impl.FromInstructionImpl;
3230
import org.sonar.iac.docker.tree.impl.LiteralImpl;
33-
import org.sonar.iac.docker.tree.impl.RunInstructionImpl;
34-
import org.sonar.iac.docker.tree.impl.SingleArgumentListImpl;
3531
import org.sonar.iac.docker.tree.impl.SyntaxTokenImpl;
3632

3733
import static org.assertj.core.api.Assertions.assertThat;
@@ -50,47 +46,39 @@ void shouldDetectLastStage() {
5046

5147
@Test
5248
void shouldDetectInstructionsInFinalImage() {
53-
var baseInstruction = createRunInstruction();
54-
var baseStage = createDockerImage("ubuntu:latest", "base", baseInstruction);
55-
var buildInstruction = createRunInstruction();
56-
var buildStage = createDockerImage("base", "build", buildInstruction);
57-
var otherInstruction = createRunInstruction();
58-
var otherStage = createDockerImage("build", "other", otherInstruction);
59-
var finalInstruction = createRunInstruction();
60-
var finalStage = createDockerImage("build", "final", finalInstruction);
49+
var baseStage = createDockerImage("ubuntu:latest", "base");
50+
var buildStage = createDockerImage("base", "build");
51+
var otherStage = createDockerImage("build", "other");
52+
var finalStage = createDockerImage("build", "final");
6153
var body = createBody(buildStage, baseStage, otherStage, finalStage);
6254
var inspector = MultiStageBuildInspector.of(body);
6355

64-
assertThat(inspector.isInFinalImage(baseInstruction)).isTrue();
65-
assertThat(inspector.isInFinalImage(buildInstruction)).isTrue();
66-
assertThat(inspector.isInFinalImage(otherInstruction)).isFalse();
67-
assertThat(inspector.isInFinalImage(finalInstruction)).isTrue();
56+
assertThat(inspector.isStageInFinalImage(baseStage)).isTrue();
57+
assertThat(inspector.isStageInFinalImage(buildStage)).isTrue();
58+
assertThat(inspector.isStageInFinalImage(otherStage)).isFalse();
59+
assertThat(inspector.isStageInFinalImage(finalStage)).isTrue();
6860
}
6961

7062
@Test
7163
void shouldDetectInstructionsInFinalImageWithoutFinalAlias() {
72-
var buildInstruction = createRunInstruction();
73-
var buildStage = createDockerImage("ubuntu:latest", "build", buildInstruction);
74-
var finalInstruction = createRunInstruction();
75-
var finalStage = createDockerImage("build", null, finalInstruction);
64+
var buildStage = createDockerImage("ubuntu:latest", "build");
65+
var finalStage = createDockerImage("build", null);
7666
var body = createBody(buildStage, finalStage);
7767
var inspector = MultiStageBuildInspector.of(body);
7868

79-
assertThat(inspector.isInFinalImage(buildInstruction)).isTrue();
80-
assertThat(inspector.isInFinalImage(finalInstruction)).isTrue();
69+
assertThat(inspector.isStageInFinalImage(buildStage)).isTrue();
70+
assertThat(inspector.isStageInFinalImage(finalStage)).isTrue();
8171
}
8272

8373
@Test
8474
void shouldDetectInstructionsInFinalImageWithCircularDependencies() {
85-
var buildInstruction = createRunInstruction();
86-
var buildStage = createDockerImage("final", "build", buildInstruction);
87-
var finalInstruction = createRunInstruction();
88-
var finalStage = createDockerImage("build", "final", finalInstruction);
75+
var buildStage = createDockerImage("final", "build");
76+
var finalStage = createDockerImage("build", "final");
8977
var body = createBody(buildStage, finalStage);
9078
var inspector = MultiStageBuildInspector.of(body);
9179

92-
assertThat(inspector.isInFinalImage(buildInstruction)).isTrue();
93-
assertThat(inspector.isInFinalImage(finalInstruction)).isTrue();
80+
assertThat(inspector.isStageInFinalImage(buildStage)).isTrue();
81+
assertThat(inspector.isStageInFinalImage(finalStage)).isTrue();
9482
}
9583

9684
private Body createBody(DockerImage... dockerImages) {
@@ -100,22 +88,9 @@ private Body createBody(DockerImage... dockerImages) {
10088
}
10189

10290
private static DockerImage createDockerImage(String imageName, @Nullable String aliasName) {
103-
return createDockerImage(imageName, aliasName, createRunInstruction());
104-
}
105-
106-
private static DockerImage createDockerImage(String imageName, @Nullable String aliasName, Instruction instruction) {
10791
var imageArgument = new ArgumentImpl(List.of(new LiteralImpl(new SyntaxTokenImpl(imageName, null, null))));
10892
var alias = aliasName != null ? new AliasImpl(null, new SyntaxTokenImpl(aliasName, null, null)) : null;
10993
var fromInstruction = new FromInstructionImpl(null, null, imageArgument, alias);
110-
var image = new DockerImageImpl(fromInstruction, List.of(instruction));
111-
instruction.setParent(image);
112-
return image;
113-
}
114-
115-
private static RunInstruction createRunInstruction() {
116-
var command = "apt-get install nginx";
117-
var commandLiteral = new LiteralImpl(new SyntaxTokenImpl(command, null, null));
118-
var commandArgument = new ArgumentImpl(List.of(commandLiteral));
119-
return new RunInstructionImpl(null, List.of(), new SingleArgumentListImpl(commandArgument));
94+
return new DockerImageImpl(fromInstruction, List.of());
12095
}
12196
}

‎iac-extensions/docker/src/test/resources/checks/ConsecutiveRunInstructionCheck/ConsecutiveRunInstructionCheck.dockerfile‎

Lines changed: 20 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1-
FROM nonCompliantCaseSimpleCase
1+
FROM scratch
2+
3+
CMD nonCompliantCaseSimpleCase
24
# Noncompliant@+1 {{Merge this RUN instruction with the consecutive ones.}}
35
RUN instruction 1
46
# ^[sc=1;ec=3]
@@ -7,7 +9,7 @@ RUN instruction 2
79
RUN instruction 3
810
# ^[sc=1;ec=3]< {{consecutive RUN instruction}}
911

10-
FROM nonCompliantRaiseSeparateIssues
12+
CMD nonCompliantRaiseSeparateIssues
1113
# Noncompliant@+1
1214
RUN instruction 1
1315
# ^[sc=1;ec=3]
@@ -20,7 +22,7 @@ RUN instruction 3
2022
RUN instruction 4
2123
# ^[sc=1;ec=3]<
2224

23-
FROM nonCompliantNotAtTheBeginningOrAtTheEnd
25+
CMD nonCompliantNotAtTheBeginningOrAtTheEnd
2426
WORKDIR something
2527
# Noncompliant@+1
2628
RUN instruction 1
@@ -29,7 +31,7 @@ RUN instruction 2
2931
# ^[sc=1;ec=3]<
3032
CMD something else
3133

32-
FROM nonCompliantMultipleForm
34+
CMD nonCompliantMultipleForm
3335
# Noncompliant@+1
3436
RUN instruction 1
3537
# ^[sc=1;ec=3]
@@ -40,7 +42,7 @@ instruction 3
4042
EOF
4143
# ^[sc=1;ec=3]@-2<
4244

43-
FROM nonCompliantRealUseCase1
45+
CMD nonCompliantRealUseCase1
4446
# Noncompliant@+1
4547
RUN curl -SL "https://nodejs.org/dist/v${NODE_VERSION}/node-v${NODE_VERSION}-linux-x64.tar.gz" --output nodejs.tar.gz
4648
# ^[sc=1;ec=3]
@@ -53,77 +55,76 @@ RUN rm nodejs.tar.gz
5355
RUN ln -s /usr/local/bin/node /usr/local/bin/nodejs
5456
# ^[sc=1;ec=3]<
5557

56-
FROM nonCompliantRealUseCase2
58+
CMD nonCompliantRealUseCase2
5759
# Noncompliant@+1
5860
RUN curl -SL "https://nodejs.org/dist/v${NODE_VERSION}/node-v${NODE_VERSION}-linux-x64.tar.gz" --output nodejs.tar.gz
5961
# ^[sc=1;ec=3]
6062
RUN echo "$NODE_DOWNLOAD_SHA nodejs.tar.gz" | sha256sum -c -
6163
# ^[sc=1;ec=3]<
6264

63-
FROM compliantBecauseTheyAreSeparated
65+
CMD compliantBecauseTheyAreSeparated
6466
RUN instruction 1
6567
WORKDIR /root
6668
RUN instruction 2
6769

68-
FROM compliantBecauseWeCompareOnlyRootInstruction
70+
CMD compliantBecauseWeCompareOnlyRootInstruction
6971
RUN instruction 1
7072
ONBUILD RUN instruction 2
7173
RUN instruction 3
7274

73-
FROM compliantBecauseWeOnlyCheckRunInstruction
75+
CMD compliantBecauseWeOnlyCheckRunInstruction
7476
RUN instruction 1
7577
CMD instruction 2
7678
CMD instruction 3
7779
RUN instruction 4
7880

79-
FROM compliantDontDetectRunInstructionInHeredoc
81+
CMD compliantDontDetectRunInstructionInHeredoc
8082
RUN <<EOF
8183
RUN instruction 1
8284
RUN instruction 2
8385
RUN instruction 3
8486
EOF
8587

86-
FROM compliantRealUseCase
88+
CMD compliantRealUseCase
8789
RUN curl -SL "https://nodejs.org/dist/v${NODE_VERSION}/node-v${NODE_VERSION}-linux-x64.tar.gz" --output nodejs.tar.gz \
8890
&& echo "$NODE_DOWNLOAD_SHA nodejs.tar.gz" | sha256sum -c - \
8991
&& tar -xzf "nodejs.tar.gz" -C /usr/local --strip-components=1 \
9092
&& rm nodejs.tar.gz \
9193
&& ln -s /usr/local/bin/node /usr/local/bin/nodejs
9294

93-
FROM scratch
95+
CMD scratch
9496
# Noncompliant@+1
9597
RUN my command
9698
RUN other command
9799

98-
FROM scratch
100+
CMD scratch
99101
# Compliant, different options cannot be merged
100102
RUN --security=insecure my command
101103
RUN other command
102104

103-
FROM scratch
105+
CMD scratch
104106
# Compliant, different options cannot be merged
105107
RUN --security=insecure my command
106108
RUN --mount=type=bind,source=/tmp,target=/tmp other command
107109

108110
# Compliant; instructions with the same options are not consecutive
109-
FROM scratch
111+
CMD scratch
110112
RUN --security=insecure my command
111113
RUN --mount=type=bind,source=/tmp,target=/tmp other command
112114
RUN --security=insecure my other command
113115

114-
FROM scratch
116+
CMD scratch
115117
# Noncompliant@+1
116118
RUN --security=insecure my command
117119
RUN --security=insecure other command
118120

119-
FROM scratch
121+
CMD scratch
120122
# Noncompliant@+1
121123
RUN --security=insecure --network=none my command
122124
RUN --network=none --security=insecure other command
123125

124-
FROM scratch
126+
CMD scratch
125127
RUN --security=insecure my command
126-
127128
# Noncompliant@+1
128129
RUN other command
129130
RUN yet another command

0 commit comments

Comments
 (0)