Skip to content

Commit cb42cbb

Browse files
cronikcronik
authored
Refactor ContainerTemplate command split function (#1697)
1 parent ac0cf1c commit cb42cbb

File tree

4 files changed

+51
-82
lines changed

4 files changed

+51
-82
lines changed

‎src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateBuilder.java‎

Lines changed: 5 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,7 @@
2626

2727
import static org.csanchez.jenkins.plugins.kubernetes.ContainerTemplate.DEFAULT_WORKING_DIR;
2828
import static org.csanchez.jenkins.plugins.kubernetes.KubernetesCloud.JNLP_NAME;
29-
import static org.csanchez.jenkins.plugins.kubernetes.PodTemplateUtils.combine;
30-
import static org.csanchez.jenkins.plugins.kubernetes.PodTemplateUtils.isNullOrEmpty;
31-
import static org.csanchez.jenkins.plugins.kubernetes.PodTemplateUtils.sanitizeLabel;
32-
import static org.csanchez.jenkins.plugins.kubernetes.PodTemplateUtils.substituteEnv;
29+
import static org.csanchez.jenkins.plugins.kubernetes.PodTemplateUtils.*;
3330

3431
import edu.umd.cs.findbugs.annotations.CheckForNull;
3532
import edu.umd.cs.findbugs.annotations.NonNull;
@@ -95,8 +92,6 @@ public class PodTemplateBuilder {
9592

9693
private static final Logger LOGGER = Logger.getLogger(PodTemplateBuilder.class.getName());
9794

98-
private static final Pattern SPLIT_IN_SPACES = Pattern.compile("([^\"]\\S*|\".+?\")\\s*");
99-
10095
private static final String WORKSPACE_VOLUME_NAME = "workspace-volume";
10196
public static final Pattern FROM_DIRECTIVE = Pattern.compile("^FROM (.*)$");
10297

@@ -537,7 +532,7 @@ private Container createContainer(
537532
}
538533
}
539534
List<String> arguments =
540-
isNullOrEmpty(containerTemplate.getArgs()) ? Collections.emptyList() : parseDockerCommand(cmd);
535+
isNullOrEmpty(containerTemplate.getArgs()) ? Collections.emptyList() : splitCommandLine(cmd);
541536

542537
ContainerPort[] ports = containerTemplate.getPorts().stream()
543538
.map(entry -> entry.toPort())
@@ -547,9 +542,9 @@ private Container createContainer(
547542

548543
ContainerLivenessProbe clp = containerTemplate.getLivenessProbe();
549544
Probe livenessProbe = null;
550-
if (clp != null && parseLivenessProbe(clp.getExecArgs()) != null) {
545+
if (clp != null && splitCommandLine(clp.getExecArgs()) != null) {
551546
livenessProbe = new ProbeBuilder()
552-
.withExec(new ExecAction(parseLivenessProbe(clp.getExecArgs())))
547+
.withExec(new ExecAction(splitCommandLine(clp.getExecArgs())))
553548
.withInitialDelaySeconds(clp.getInitialDelaySeconds())
554549
.withTimeoutSeconds(clp.getTimeoutSeconds())
555550
.withFailureThreshold(clp.getFailureThreshold())
@@ -577,7 +572,7 @@ private Container createContainer(
577572
.withVolumeMounts(containerMounts.toArray(new VolumeMount[containerMounts.size()]))
578573
.addToEnv(envVars)
579574
.addToPorts(ports)
580-
.withCommand(parseDockerCommand(containerTemplate.getCommand()))
575+
.withCommand(splitCommandLine(containerTemplate.getCommand()))
581576
.withArgs(arguments)
582577
.withLivenessProbe(livenessProbe)
583578
.withTty(containerTemplate.isTtyEnabled())
@@ -615,46 +610,6 @@ private List<VolumeMount> getContainerVolumeMounts(Collection<VolumeMount> volum
615610
return containerMounts;
616611
}
617612

618-
/**
619-
* Split a command in the parts that Docker need
620-
*
621-
* @param dockerCommand
622-
* @return
623-
*/
624-
@Restricted(NoExternalUse.class)
625-
static List<String> parseDockerCommand(String dockerCommand) {
626-
if (dockerCommand == null || dockerCommand.isEmpty()) {
627-
return null;
628-
}
629-
// handle quoted arguments
630-
Matcher m = SPLIT_IN_SPACES.matcher(dockerCommand);
631-
List<String> commands = new ArrayList<String>();
632-
while (m.find()) {
633-
commands.add(substituteEnv(m.group(1).replace("\"", "")));
634-
}
635-
return commands;
636-
}
637-
638-
/**
639-
* Split a command in the parts that LivenessProbe need
640-
*
641-
* @param livenessProbeExec
642-
* @return
643-
*/
644-
@Restricted(NoExternalUse.class)
645-
static List<String> parseLivenessProbe(String livenessProbeExec) {
646-
if (StringUtils.isBlank(livenessProbeExec)) {
647-
return null;
648-
}
649-
// handle quoted arguments
650-
Matcher m = SPLIT_IN_SPACES.matcher(livenessProbeExec);
651-
List<String> commands = new ArrayList<String>();
652-
while (m.find()) {
653-
commands.add(substituteEnv(m.group(1).replace("\"", "").replace("?:\\\"", "")));
654-
}
655-
return commands;
656-
}
657-
658613
private Map<String, Quantity> getResourcesMap(String memory, String cpu, String ephemeralStorage) {
659614
Map<String, Quantity> builder = new HashMap<>();
660615
String actualMemory = substituteEnv(memory);

‎src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateUtils.java‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import java.io.IOException;
3333
import java.io.InputStream;
3434
import java.util.ArrayList;
35+
import java.util.Arrays;
3536
import java.util.Collection;
3637
import java.util.Collections;
3738
import java.util.HashMap;
@@ -53,6 +54,7 @@
5354
import java.util.stream.Collectors;
5455
import java.util.stream.Stream;
5556
import org.apache.commons.lang.StringUtils;
57+
import org.apache.tools.ant.types.Commandline;
5658
import org.csanchez.jenkins.plugins.kubernetes.model.TemplateEnvVar;
5759
import org.csanchez.jenkins.plugins.kubernetes.volumes.PodVolume;
5860
import org.csanchez.jenkins.plugins.kubernetes.volumes.workspace.WorkspaceVolume;
@@ -741,6 +743,27 @@ public static boolean validateImage(String image) {
741743
return image != null && image.matches("\\S+");
742744
}
743745

746+
/**
747+
* Split a command in the parts that Docker need
748+
*
749+
* @param commandLine docker command to parse
750+
* @return command split by arguments, {@code null} if {@code commandLine} is {@code null} or empty
751+
*/
752+
public static @Nullable List<String> splitCommandLine(@Nullable String commandLine) {
753+
if (commandLine == null || commandLine.isEmpty()) {
754+
return null;
755+
}
756+
757+
String[] args = Commandline.translateCommandline(commandLine);
758+
if (SUBSTITUTE_ENV) {
759+
for (int i = 0; i < args.length; i++) {
760+
args[i] = substituteEnv(args[i]);
761+
}
762+
}
763+
764+
return Arrays.asList(args);
765+
}
766+
744767
/**
745768
* <p>Sanitizes the input string to create a valid Kubernetes label.
746769
* <p>The input string is truncated to a maximum length of 57 characters,

‎src/test/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateBuilderTest.java‎

Lines changed: 0 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
import static org.junit.Assert.assertEquals;
99
import static org.junit.Assert.assertFalse;
1010
import static org.junit.Assert.assertNotNull;
11-
import static org.junit.Assert.assertNull;
1211
import static org.junit.Assert.assertTrue;
1312
import static org.mockito.Mockito.*;
1413

@@ -57,7 +56,6 @@
5756
import org.jvnet.hudson.test.Issue;
5857
import org.jvnet.hudson.test.JenkinsRule;
5958
import org.jvnet.hudson.test.LoggerRule;
60-
import org.jvnet.hudson.test.WithoutJenkins;
6159
import org.mockito.Mock;
6260
import org.mockito.Spy;
6361
import org.mockito.junit.MockitoJUnit;
@@ -99,36 +97,6 @@ public void setUp() {
9997
when(slave.getKubernetesCloud()).thenReturn(cloud);
10098
}
10199

102-
@WithoutJenkins
103-
@Test
104-
public void testParseDockerCommand() {
105-
assertNull(parseDockerCommand(""));
106-
assertNull(parseDockerCommand(null));
107-
assertEquals(Collections.singletonList("bash"), parseDockerCommand("bash"));
108-
assertEquals(
109-
Collections.unmodifiableList(Arrays.asList("bash", "-c", "x y")),
110-
parseDockerCommand("bash -c \"x y\""));
111-
assertEquals(Collections.unmodifiableList(Arrays.asList("a", "b", "c", "d")), parseDockerCommand("a b c d"));
112-
}
113-
114-
@WithoutJenkins
115-
@Test
116-
public void testParseLivenessProbe() {
117-
assertNull(parseLivenessProbe(""));
118-
assertNull(parseLivenessProbe(null));
119-
assertEquals(Collections.unmodifiableList(Arrays.asList("docker", "info")), parseLivenessProbe("docker info"));
120-
assertEquals(
121-
Collections.unmodifiableList(Arrays.asList("echo", "I said: 'I am alive'")),
122-
parseLivenessProbe("echo \"I said: 'I am alive'\""));
123-
assertEquals(
124-
Collections.unmodifiableList(Arrays.asList("docker", "--version")),
125-
parseLivenessProbe("docker --version"));
126-
assertEquals(
127-
Collections.unmodifiableList(
128-
Arrays.asList("curl", "-k", "--silent", "--output=/dev/null", "https://localhost:8080")),
129-
parseLivenessProbe("curl -k --silent --output=/dev/null \"https://localhost:8080\""));
130-
}
131-
132100
@Test
133101
@TestCaseName("{method}(directConnection={0})")
134102
@Parameters({"true", "false"})

‎src/test/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateUtilsTest.java‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@
8484
import org.junit.runner.RunWith;
8585
import org.jvnet.hudson.test.Issue;
8686
import org.jvnet.hudson.test.JenkinsRule;
87+
import org.jvnet.hudson.test.WithoutJenkins;
8788

8889
@RunWith(Theories.class)
8990
public class PodTemplateUtilsTest {
@@ -1162,4 +1163,26 @@ public void shouldOverrideShareProcessNamespaceIfSpecified() {
11621163
Pod result4 = combine(parent2, child2);
11631164
assertTrue(result4.getSpec().getShareProcessNamespace());
11641165
}
1166+
1167+
@WithoutJenkins
1168+
@Test
1169+
public void testSplitCommandLine() {
1170+
assertNull(PodTemplateUtils.splitCommandLine(""));
1171+
assertNull(PodTemplateUtils.splitCommandLine(null));
1172+
assertEquals(List.of("bash"), PodTemplateUtils.splitCommandLine("bash"));
1173+
assertEquals(List.of("bash", "-c", "x y"), PodTemplateUtils.splitCommandLine("bash -c \"x y\""));
1174+
assertEquals(List.of("bash", "-c", "x y"), PodTemplateUtils.splitCommandLine("bash -c 'x y'"));
1175+
assertEquals(List.of("bash", "-c", "xy"), PodTemplateUtils.splitCommandLine("bash -c 'x''y'"));
1176+
assertEquals(
1177+
List.of("bash", "-c", "\"$folder\""), PodTemplateUtils.splitCommandLine("bash -c '\"'\"$folder\"'\"'"));
1178+
assertEquals(List.of("a", "b", "c", "d"), PodTemplateUtils.splitCommandLine("a b c d"));
1179+
assertEquals(List.of("docker", "info"), PodTemplateUtils.splitCommandLine("docker info"));
1180+
assertEquals(
1181+
List.of("echo", "I said: 'I am alive'"),
1182+
PodTemplateUtils.splitCommandLine("echo \"I said: 'I am alive'\""));
1183+
assertEquals(List.of("docker", "--version"), PodTemplateUtils.splitCommandLine("docker --version"));
1184+
assertEquals(
1185+
List.of("curl", "-k", "--silent", "--output=/dev/null", "https://localhost:8080"),
1186+
PodTemplateUtils.splitCommandLine("curl -k --silent --output=/dev/null \"https://localhost:8080\""));
1187+
}
11651188
}

0 commit comments

Comments
 (0)