Handle the containerCommand and containerArgs attributes of the machine

defined with the dockerimage recipe.

Note that this only works in kubernetes and openshift environments.

Signed-off-by: Lukas Krejci <lkrejci@redhat.com>

core/che-core-api-model/src/main/java/org/eclipse/che/api/core/model/workspace/config/MachineConfig.java

@@ -37,6 +37,25 @@ public interface MachineConfig {
    */
   String MEMORY_REQUEST_ATTRIBUTE = "memoryRequestBytes";
 
+  /**
+   * Name of the attribute from {@link #getAttributes()} which, if present, defines the entrypoint
+   * command to be executed in the machine/container.
+   *
+   * <p>The format is a YAML list of strings, e.g. {@code ['/bin/sh', '-c']}
+   */
+  String CONTAINER_COMMAND_ATTRIBUTE = "containerCommand";
+
+  /**
+   * Name of the attribute from {@link #getAttributes()} which, if present, defines the command line
+   * arguments of the entrypoint command specified using the {@link #CONTAINER_COMMAND_ATTRIBUTE}.
+   *
+   * <p>If {@link #CONTAINER_COMMAND_ATTRIBUTE} is not present, the default command defined in the
+   * image is used and the arguments are provided to it.
+   *
+   * <p>The format is a YAML list of strings, e.g. {@code ['-f', '--yes']}
+   */
+  String CONTAINER_ARGS_ATTRIBUTE = "containerArgs";
+
   /**
    * Returns configured installers.
    *

infrastructures/docker/environment/src/main/java/org/eclipse/che/workspace/infrastructure/docker/environment/dockerimage/DockerImageEnvironment.java

@@ -11,6 +11,8 @@
  */
 package org.eclipse.che.workspace.infrastructure.docker.environment.dockerimage;
 
+import static java.lang.String.format;
+
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
@@ -19,7 +21,11 @@ import org.eclipse.che.api.workspace.server.spi.environment.InternalEnvironment;
 import org.eclipse.che.api.workspace.server.spi.environment.InternalMachineConfig;
 import org.eclipse.che.api.workspace.server.spi.environment.InternalRecipe;
 
-/** @author Sergii Leshchenko */
+/**
+ * Represents an environment based on a docker image. It must be declared with exactly 1 machine.
+ *
+ * @author Sergii Leshchenko
+ */
 public class DockerImageEnvironment extends InternalEnvironment {
   public static final String TYPE = "dockerimage";
 
@@ -30,10 +36,21 @@ public class DockerImageEnvironment extends InternalEnvironment {
       InternalRecipe recipe,
       Map<String, InternalMachineConfig> machines,
       List<Warning> warnings) {
-    super(recipe, machines, warnings);
+    super(recipe, checkSingleEntry(machines), warnings);
     this.dockerImage = dockerImage;
   }
 
+  private static <K, V> Map<K, V> checkSingleEntry(Map<K, V> map) {
+    if (map.size() == 1) {
+      return map;
+    } else {
+      throw new IllegalArgumentException(
+          format(
+              "A docker image environment must contain precisely 1 machine configuration but found %d.",
+              map.size()));
+    }
+  }
+
   @Override
   public DockerImageEnvironment setType(String type) {
     return (DockerImageEnvironment) super.setType(type);

infrastructures/docker/environment/src/main/java/org/eclipse/che/workspace/infrastructure/docker/environment/dockerimage/DockerImageEnvironmentFactory.java

@@ -64,7 +64,7 @@ public class DockerImageEnvironmentFactory
       @Nullable InternalRecipe recipe,
       Map<String, InternalMachineConfig> machines,
       List<Warning> warnings)
-      throws InfrastructureException, ValidationException {
+      throws ValidationException {
     checkNotNull(recipe, "Null recipe is not supported by docker image environment factory");
     if (!DockerImageEnvironment.TYPE.equals(recipe.getType())) {
       throw new ValidationException(
@@ -77,11 +77,30 @@ public class DockerImageEnvironmentFactory
 
     checkArgument(dockerImage != null, "Docker image should not be null.");
 
+    ensureSingleMachine(machines);
+
     addRamAttributes(machines);
 
     return new DockerImageEnvironment(dockerImage, recipe, machines, warnings);
   }
 
+  private void ensureSingleMachine(Map<String, InternalMachineConfig> machines)
+      throws ValidationException {
+    int nofMachines = machines.size();
+    if (nofMachines == 0) {
+      // we create a "fake" machine definition where the rest of the code can put additional
+      // definitions, if needed.
+      InternalMachineConfig emptyConfig = new InternalMachineConfig();
+      // let's just call the machine after the type. The name doesn't matter that much anyway.
+      machines.put(DockerImageEnvironment.TYPE, emptyConfig);
+    } else if (nofMachines > 1) {
+      throw new ValidationException(
+          format(
+              "Docker image environment only supports a single machine definition but found %d.",
+              nofMachines));
+    }
+  }
+
   private void addRamAttributes(Map<String, InternalMachineConfig> machines) {
     for (InternalMachineConfig machineConfig : machines.values()) {
       memoryProvisioner.provision(machineConfig, 0L, 0L);

infrastructures/docker/environment/src/test/java/org/eclipse/che/workspace/infrastructure/docker/environment/dockerimage/DockerImageEnvironmentFactoryTest.java

@@ -19,11 +19,18 @@ import static org.mockito.Mockito.when;
 
 import com.google.common.collect.ImmutableMap;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.Map;
+import org.eclipse.che.api.core.ValidationException;
 import org.eclipse.che.api.installer.server.InstallerRegistry;
-import org.eclipse.che.api.workspace.server.spi.environment.*;
+import org.eclipse.che.api.workspace.server.spi.environment.InternalMachineConfig;
+import org.eclipse.che.api.workspace.server.spi.environment.InternalRecipe;
+import org.eclipse.che.api.workspace.server.spi.environment.MachineConfigsValidator;
+import org.eclipse.che.api.workspace.server.spi.environment.MemoryAttributeProvisioner;
+import org.eclipse.che.api.workspace.server.spi.environment.RecipeRetriever;
 import org.mockito.Mock;
 import org.mockito.testng.MockitoTestNGListener;
+import org.testng.Assert;
 import org.testng.annotations.BeforeMethod;
 import org.testng.annotations.Listeners;
 import org.testng.annotations.Test;
@@ -60,4 +67,18 @@ public class DockerImageEnvironmentFactoryTest {
 
     verify(memoryProvisioner).provision(any(), eq(0L), eq(0L));
   }
+
+  @Test
+  public void testInsertsEmptyMachineConfigIfNoMachines() throws Exception {
+    DockerImageEnvironment env = factory.doCreate(recipe, new HashMap<>(), Collections.emptyList());
+    Assert.assertEquals(1, env.getMachines().size());
+  }
+
+  @Test(expectedExceptions = ValidationException.class)
+  public void shouldFailIfMoreThanOneMachineConfigProvided() throws Exception {
+    Map<String, InternalMachineConfig> machines =
+        ImmutableMap.of("one", new InternalMachineConfig(), "two", new InternalMachineConfig());
+
+    factory.doCreate(recipe, machines, Collections.emptyList());
+  }
 }

infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/environment/convert/DockerImageEnvironmentConverter.java

@@ -19,13 +19,14 @@ import io.fabric8.kubernetes.api.model.ContainerBuilder;
 import io.fabric8.kubernetes.api.model.Pod;
 import io.fabric8.kubernetes.api.model.PodBuilder;
 import java.util.HashMap;
-import java.util.Iterator;
 import java.util.Map;
 import javax.inject.Singleton;
 import org.eclipse.che.api.workspace.server.spi.InfrastructureException;
-import org.eclipse.che.api.workspace.server.spi.InternalInfrastructureException;
+import org.eclipse.che.api.workspace.server.spi.environment.InternalMachineConfig;
 import org.eclipse.che.workspace.infrastructure.docker.environment.dockerimage.DockerImageEnvironment;
 import org.eclipse.che.workspace.infrastructure.kubernetes.environment.KubernetesEnvironment;
+import org.eclipse.che.workspace.infrastructure.kubernetes.environment.util.EntryPoint;
+import org.eclipse.che.workspace.infrastructure.kubernetes.environment.util.EntryPointParser;
 
 /**
  * Converts {@link DockerImageEnvironment} to {@link KubernetesEnvironment}.
@@ -39,15 +40,27 @@ public class DockerImageEnvironmentConverter {
   static final String POD_NAME = "dockerimage";
   static final String CONTAINER_NAME = "container";
 
+  private EntryPointParser entryPointParser = new EntryPointParser();
+
   public KubernetesEnvironment convert(DockerImageEnvironment environment)
       throws InfrastructureException {
-    final Iterator<String> iterator = environment.getMachines().keySet().iterator();
-    if (!iterator.hasNext()) {
-      throw new InternalInfrastructureException(
-          "DockerImage environment must contain at least one machine configuration");
-    }
-    final String machineName = iterator.next();
+
     final String dockerImage = environment.getRecipe().getContent();
+
+    Map.Entry<String, InternalMachineConfig> e =
+        environment.getMachines().entrySet().iterator().next();
+
+    InternalMachineConfig machine = e.getValue();
+    String machineName = e.getKey();
+
+    ContainerBuilder container =
+        new ContainerBuilder()
+            .withImage(dockerImage)
+            .withName(CONTAINER_NAME)
+            .withImagePullPolicy("Always");
+
+    applyEntryPoint(machine, container);
+
     final Map<String, String> annotations = new HashMap<>();
     annotations.put(format(MACHINE_NAME_ANNOTATION_FMT, CONTAINER_NAME), machineName);
 
@@ -58,18 +71,22 @@ public class DockerImageEnvironmentConverter {
             .withAnnotations(annotations)
             .endMetadata()
             .withNewSpec()
-            .withContainers(
-                new ContainerBuilder()
-                    .withImage(dockerImage)
-                    .withName(CONTAINER_NAME)
-                    .withImagePullPolicy("Always")
-                    .build())
+            .withContainers(container.build())
             .endSpec()
             .build();
+
     return KubernetesEnvironment.builder(environment)
         .setMachines(environment.getMachines())
         .setInternalRecipe(environment.getRecipe())
-        .setPods(ImmutableMap.of(POD_NAME, pod))
+        .setPods(ImmutableMap.of(machineName, pod))
         .build();
   }
+
+  private void applyEntryPoint(InternalMachineConfig machineConfig, ContainerBuilder bld)
+      throws InfrastructureException {
+    EntryPoint ep = entryPointParser.parse(machineConfig.getAttributes());
+
+    bld.withCommand(ep.getCommand());
+    bld.withArgs(ep.getArguments());
+  }
 }

infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/environment/util/EntryPoint.java

@@ -0,0 +1,60 @@
+/*
+ * Copyright (c) 2012-2018 Red Hat, Inc.
+ * This program and the accompanying materials are made
+ * available under the terms of the Eclipse Public License 2.0
+ * which is available at https://www.eclipse.org/legal/epl-2.0/
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *
+ * Contributors:
+ *   Red Hat, Inc. - initial API and implementation
+ */
+package org.eclipse.che.workspace.infrastructure.kubernetes.environment.util;
+
+import com.google.common.collect.ImmutableList;
+import java.util.List;
+import java.util.Objects;
+
+/** Represents an entry-point definition parsed from a string using the {@link EntryPointParser}. */
+public final class EntryPoint {
+
+  private final List<String> command;
+  private final List<String> arguments;
+
+  public EntryPoint(List<String> command, List<String> arguments) {
+    this.command = ImmutableList.copyOf(command);
+    this.arguments = ImmutableList.copyOf(arguments);
+  }
+
+  /** @return unmodifiable list representing the command of the entrypoint */
+  public List<String> getCommand() {
+    return command;
+  }
+
+  /** @return unmodifiable list representing the arguments of the entrypoint */
+  public List<String> getArguments() {
+    return arguments;
+  }
+
+  @Override
+  public boolean equals(Object o) {
+    if (this == o) {
+      return true;
+    }
+    if (o == null || getClass() != o.getClass()) {
+      return false;
+    }
+    EntryPoint that = (EntryPoint) o;
+    return Objects.equals(command, that.command) && Objects.equals(arguments, that.arguments);
+  }
+
+  @Override
+  public int hashCode() {
+    return Objects.hash(command, arguments);
+  }
+
+  @Override
+  public String toString() {
+    return "EntryPoint{" + "command=" + command + ", arguments=" + arguments + '}';
+  }
+}

infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/environment/util/EntryPointParser.java

@@ -0,0 +1,67 @@
+/*
+ * Copyright (c) 2012-2018 Red Hat, Inc.
+ * This program and the accompanying materials are made
+ * available under the terms of the Eclipse Public License 2.0
+ * which is available at https://www.eclipse.org/legal/epl-2.0/
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *
+ * Contributors:
+ *   Red Hat, Inc. - initial API and implementation
+ */
+package org.eclipse.che.workspace.infrastructure.kubernetes.environment.util;
+
+import static java.lang.String.format;
+import static java.util.Collections.emptyList;
+
+import com.fasterxml.jackson.core.type.TypeReference;
+import com.fasterxml.jackson.dataformat.yaml.YAMLMapper;
+import java.io.IOException;
+import java.util.List;
+import java.util.Map;
+import org.eclipse.che.api.core.model.workspace.config.MachineConfig;
+import org.eclipse.che.api.workspace.server.spi.InfrastructureException;
+
+/** Can be used to parse container entry-point definition specified as a YAML list of strings. */
+public final class EntryPointParser {
+  private final YAMLMapper mapper = new YAMLMapper();
+
+  /**
+   * Parses the attributes contained in the provided machine config and produces an entry point
+   * definition.
+   *
+   * <p>This method looks for the values of the {@link MachineConfig#CONTAINER_COMMAND_ATTRIBUTE}
+   * and {@link MachineConfig#CONTAINER_ARGS_ATTRIBUTE} attributews in the provided map and
+   * constructs an Entrypoint instance parsed out of the contents of those attributes.
+   *
+   * @param machineAttributes the attributes of a machine to extract the entry point info from
+   * @return an entry point definition, never null
+   * @throws InfrastructureException on failure to parse the command or arguments
+   */
+  public EntryPoint parse(Map<String, String> machineAttributes) throws InfrastructureException {
+    String command = machineAttributes.get(MachineConfig.CONTAINER_COMMAND_ATTRIBUTE);
+    String args = machineAttributes.get(MachineConfig.CONTAINER_ARGS_ATTRIBUTE);
+
+    List<String> commandList =
+        command == null
+            ? emptyList()
+            : parseAsList(command, MachineConfig.CONTAINER_COMMAND_ATTRIBUTE);
+    List<String> argList =
+        args == null ? emptyList() : parseAsList(args, MachineConfig.CONTAINER_ARGS_ATTRIBUTE);
+
+    return new EntryPoint(commandList, argList);
+  }
+
+  private List<String> parseAsList(String data, String attributeName)
+      throws InfrastructureException {
+    try {
+      return mapper.readValue(data, new TypeReference<List<String>>() {});
+    } catch (IOException e) {
+      throw new InfrastructureException(
+          format(
+              "Failed to parse the attribute %s as a YAML list. The value was %s",
+              attributeName, data),
+          e.getCause());
+    }
+  }
+}

infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/environment/convert/DockerImageEnvironmentConverterTest.java

@@ -12,21 +12,24 @@
 package org.eclipse.che.workspace.infrastructure.kubernetes.environment.convert;
 
 import static java.lang.String.format;
-import static java.util.Collections.emptyMap;
+import static java.util.Arrays.asList;
+import static java.util.Collections.singletonList;
 import static org.eclipse.che.workspace.infrastructure.kubernetes.Constants.MACHINE_NAME_ANNOTATION_FMT;
 import static org.eclipse.che.workspace.infrastructure.kubernetes.environment.convert.DockerImageEnvironmentConverter.CONTAINER_NAME;
 import static org.eclipse.che.workspace.infrastructure.kubernetes.environment.convert.DockerImageEnvironmentConverter.POD_NAME;
+import static org.mockito.Mockito.lenient;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 import static org.testng.AssertJUnit.assertEquals;
 
 import com.google.common.collect.ImmutableMap;
+import io.fabric8.kubernetes.api.model.Container;
 import io.fabric8.kubernetes.api.model.ContainerBuilder;
 import io.fabric8.kubernetes.api.model.Pod;
 import io.fabric8.kubernetes.api.model.PodBuilder;
 import java.util.HashMap;
 import java.util.Map;
-import org.eclipse.che.api.workspace.server.spi.InfrastructureException;
+import org.eclipse.che.api.core.model.workspace.config.MachineConfig;
 import org.eclipse.che.api.workspace.server.spi.environment.InternalMachineConfig;
 import org.eclipse.che.api.workspace.server.spi.environment.InternalRecipe;
 import org.eclipse.che.workspace.infrastructure.docker.environment.dockerimage.DockerImageEnvironment;
@@ -55,8 +58,8 @@ public class DockerImageEnvironmentConverterTest {
   @BeforeMethod
   public void setup() throws Exception {
     converter = new DockerImageEnvironmentConverter();
-    when(recipe.getContent()).thenReturn(RECIPE_CONTENT);
-    when(recipe.getType()).thenReturn(RECIPE_TYPE);
+    lenient().when(recipe.getContent()).thenReturn(RECIPE_CONTENT);
+    lenient().when(recipe.getType()).thenReturn(RECIPE_TYPE);
     machines = ImmutableMap.of(MACHINE_NAME, mock(InternalMachineConfig.class));
     final Map<String, String> annotations = new HashMap<>();
     annotations.put(format(MACHINE_NAME_ANNOTATION_FMT, CONTAINER_NAME), MACHINE_NAME);
@@ -89,13 +92,32 @@ public class DockerImageEnvironmentConverterTest {
     assertEquals(machines, actual.getMachines());
   }
 
-  @Test(
-      expectedExceptions = InfrastructureException.class,
-      expectedExceptionsMessageRegExp =
-          "DockerImage environment must contain at least one machine configuration")
-  public void throwsValidationExceptionWhenNoMachineConfigProvided() throws Exception {
-    when(dockerEnv.getMachines()).thenReturn(emptyMap());
+  @Test
+  public void shouldUseMachineConfigIfProvided() throws Exception {
+    // given
+    Map<String, String> attributes = new HashMap<>(2);
+    attributes.put(MachineConfig.CONTAINER_COMMAND_ATTRIBUTE, "[/teh/script]");
+    attributes.put(MachineConfig.CONTAINER_ARGS_ATTRIBUTE, "['teh', 'argz']");
 
-    converter.convert(dockerEnv);
+    InternalMachineConfig machineConfig = mock(InternalMachineConfig.class);
+    when(machineConfig.getAttributes()).thenReturn(attributes);
+
+    Map<String, InternalMachineConfig> machines = new HashMap<>(1);
+    machines.put(MACHINE_NAME, machineConfig);
+
+    when(dockerEnv.getMachines()).thenReturn(machines);
+    when(dockerEnv.getRecipe()).thenReturn(recipe);
+
+    Container ctn = pod.getSpec().getContainers().get(0);
+    ctn.setCommand(singletonList("/teh/script"));
+    ctn.setArgs(asList("teh", "argz"));
+
+    // when
+    KubernetesEnvironment env = converter.convert(dockerEnv);
+
+    // then
+    assertEquals(pod, env.getPodsCopy().values().iterator().next());
+    assertEquals(recipe, env.getRecipe());
+    assertEquals(machines, env.getMachines());
   }
 }

infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/environment/util/EntryPointParserTest.java

@@ -0,0 +1,86 @@
+/*
+ * Copyright (c) 2012-2018 Red Hat, Inc.
+ * This program and the accompanying materials are made
+ * available under the terms of the Eclipse Public License 2.0
+ * which is available at https://www.eclipse.org/legal/epl-2.0/
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *
+ * Contributors:
+ *   Red Hat, Inc. - initial API and implementation
+ */
+package org.eclipse.che.workspace.infrastructure.kubernetes.environment.util;
+
+import static java.util.Arrays.asList;
+import static java.util.Collections.emptyList;
+import static org.testng.AssertJUnit.assertEquals;
+
+import java.util.HashMap;
+import java.util.Map;
+import org.eclipse.che.api.core.model.workspace.config.MachineConfig;
+import org.eclipse.che.api.workspace.server.spi.InfrastructureException;
+import org.testng.annotations.DataProvider;
+import org.testng.annotations.Test;
+
+public class EntryPointParserTest {
+
+  @Test
+  public void shouldParseCommandAndArgFromYAMLList() throws Exception {
+    Map<String, String> cfg = new HashMap<>();
+    cfg.put(MachineConfig.CONTAINER_COMMAND_ATTRIBUTE, "['/bin/sh', '''yaml quoting ftw''']");
+    cfg.put(MachineConfig.CONTAINER_ARGS_ATTRIBUTE, "['x', 'y', '''z']");
+
+    EntryPointParser parser = new EntryPointParser();
+
+    EntryPoint ep = parser.parse(cfg);
+
+    assertEquals(asList("/bin/sh", "'yaml quoting ftw'"), ep.getCommand());
+    assertEquals(asList("x", "y", "'z"), ep.getArguments());
+  }
+
+  @Test
+  public void shouldParseCommandFromYAMLList() throws Exception {
+    Map<String, String> cfg = new HashMap<>();
+    cfg.put(MachineConfig.CONTAINER_COMMAND_ATTRIBUTE, "['/bin/sh', '''yaml quoting ftw''']");
+
+    EntryPointParser parser = new EntryPointParser();
+
+    EntryPoint ep = parser.parse(cfg);
+
+    assertEquals(asList("/bin/sh", "'yaml quoting ftw'"), ep.getCommand());
+    assertEquals(emptyList(), ep.getArguments());
+  }
+
+  @Test
+  public void shouldParseArgsFromYAMLList() throws Exception {
+    Map<String, String> cfg = new HashMap<>();
+    cfg.put(MachineConfig.CONTAINER_ARGS_ATTRIBUTE, "['x', 'y', '''z', --yes]");
+
+    EntryPointParser parser = new EntryPointParser();
+
+    EntryPoint ep = parser.parse(cfg);
+
+    assertEquals(emptyList(), ep.getCommand());
+    assertEquals(asList("x", "y", "'z", "--yes"), ep.getArguments());
+  }
+
+  @Test(dataProvider = "invalidEntryProvider", expectedExceptions = InfrastructureException.class)
+  public void shouldFailOnOtherYAMLDataType(String invalidEntry) throws InfrastructureException {
+    Map<String, String> cfg = new HashMap<>();
+    cfg.put(MachineConfig.CONTAINER_ARGS_ATTRIBUTE, invalidEntry);
+
+    EntryPointParser parser = new EntryPointParser();
+    parser.parse(cfg);
+  }
+
+  @DataProvider
+  public static Object[][] invalidEntryProvider() {
+    return new Object[][] {
+      new String[] {"key: value"},
+      new String[] {"42"},
+      new String[] {"true"},
+      new String[] {"string value"},
+      new String[] {"[a, b, [c]]"}
+    };
+  }
+}