Add devfile validation to /workspace/devfile endpoint (#13472)

/workspace/devfile now validates the devfile against schema and
workspace manager validates the integrity of the devfile before creating
the workspace.

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

multiuser/api/che-multiuser-api-resource/src/main/java/org/eclipse/che/multiuser/resource/api/workspace/LimitsCheckingWorkspaceManager.java

@@ -35,7 +35,7 @@
 import org.eclipse.che.api.workspace.server.WorkspaceManager;
 import org.eclipse.che.api.workspace.server.WorkspaceRuntimes;
 import org.eclipse.che.api.workspace.server.WorkspaceValidator;
-import org.eclipse.che.api.workspace.server.devfile.convert.DevfileConverter;
+import org.eclipse.che.api.workspace.server.devfile.validator.DevfileIntegrityValidator;
 import org.eclipse.che.api.workspace.server.model.impl.WorkspaceConfigImpl;
 import org.eclipse.che.api.workspace.server.model.impl.WorkspaceImpl;
 import org.eclipse.che.api.workspace.server.spi.WorkspaceDao;
@@ -84,8 +84,14 @@ public LimitsCheckingWorkspaceManager(
       EnvironmentRamCalculator environmentRamCalculator,
       ResourceManager resourceManager,
       ResourcesLocks resourcesLocks,
-      DevfileConverter devfileConverter) {
-    super(workspaceDao, runtimes, eventService, accountManager, workspaceValidator);
+      DevfileIntegrityValidator devfileIntegrityValidator) {
+    super(
+        workspaceDao,
+        runtimes,
+        eventService,
+        accountManager,
+        workspaceValidator,
+        devfileIntegrityValidator);
     this.environmentRamCalculator = environmentRamCalculator;
     this.maxRamPerEnvMB = "-1".equals(maxRamPerEnv) ? -1 : Size.parseSizeToMegabytes(maxRamPerEnv);
     this.resourceManager = resourceManager;

multiuser/permission/che-multiuser-permission-workspace/src/test/java/org/eclipse/che/multiuser/permission/workspace/server/filters/WorkspacePermissionsFilterTest.java

@@ -158,7 +158,7 @@ public void shouldCheckAccountPermissionsAccessOnWorkspaceCreationFromDevfile()
             .post(SECURE_PATH + "/workspace/devfile?namespace=userok");
 
     assertEquals(response.getStatusCode(), 204);
-    verify(workspaceService).create(any(DevfileDto.class), any(), any(), eq("userok"));
+    verify(workspaceService).create(anyString(), any(), any(), eq("userok"), any());
     verify(permissionsFilter).checkAccountPermissions("userok", AccountOperation.CREATE_WORKSPACE);
     verifyZeroInteractions(subject);
   }

wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/urlfactory/URLFactoryBuilder.java

@@ -104,7 +104,7 @@ public Optional<FactoryDto> createFactoryFromDevfile(
       return Optional.empty();
     }
     try {
-      DevfileImpl devfile = devfileManager.parse(devfileYamlContent);
+      DevfileImpl devfile = devfileManager.parseYaml(devfileYamlContent);
       WorkspaceConfigImpl wsConfig =
           devfileManager.createWorkspaceConfig(devfile, fileContentProvider);
       FactoryDto factoryDto =

wsmaster/che-core-api-factory/src/test/java/org/eclipse/che/api/factory/server/urlfactory/URLFactoryBuilderTest.java

@@ -119,7 +119,7 @@ public void checkWithCustomDevfileAndRecipe() throws Exception {
     workspaceConfigImpl.setDefaultEnv("name");
 
     when(urlFetcher.fetchSafely(anyString())).thenReturn("random_content");
-    when(devfileManager.parse(anyString())).thenReturn(devfile);
+    when(devfileManager.parseYaml(anyString())).thenReturn(devfile);
     when(devfileManager.createWorkspaceConfig(any(DevfileImpl.class), any()))
         .thenReturn(workspaceConfigImpl);
 

wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/WorkspaceManager.java

@@ -42,6 +42,9 @@
 import org.eclipse.che.api.core.model.workspace.WorkspaceStatus;
 import org.eclipse.che.api.core.model.workspace.devfile.Devfile;
 import org.eclipse.che.api.core.notification.EventService;
+import org.eclipse.che.api.workspace.server.devfile.FileContentProvider;
+import org.eclipse.che.api.workspace.server.devfile.exception.DevfileFormatException;
+import org.eclipse.che.api.workspace.server.devfile.validator.DevfileIntegrityValidator;
 import org.eclipse.che.api.workspace.server.model.impl.WorkspaceConfigImpl;
 import org.eclipse.che.api.workspace.server.model.impl.WorkspaceImpl;
 import org.eclipse.che.api.workspace.server.model.impl.devfile.DevfileImpl;
@@ -73,19 +76,22 @@ public class WorkspaceManager {
   private final AccountManager accountManager;
   private final EventService eventService;
   private final WorkspaceValidator validator;
+  private final DevfileIntegrityValidator devfileIntegrityValidator;
 
   @Inject
   public WorkspaceManager(
       WorkspaceDao workspaceDao,
       WorkspaceRuntimes runtimes,
       EventService eventService,
       AccountManager accountManager,
-      WorkspaceValidator validator) {
+      WorkspaceValidator validator,
+      DevfileIntegrityValidator devfileIntegrityValidator) {
     this.workspaceDao = workspaceDao;
     this.runtimes = runtimes;
     this.accountManager = accountManager;
     this.eventService = eventService;
     this.validator = validator;
+    this.devfileIntegrityValidator = devfileIntegrityValidator;
   }
 
   /**
@@ -120,16 +126,45 @@ public WorkspaceImpl createWorkspace(
     return workspace;
   }
 
+  /**
+   * Creates a workspace out of a devfile.
+   *
+   * <p>The devfile should have been validated using the {@link
+   * DevfileIntegrityValidator#validateDevfile(Devfile)}. This method does rest of the validation
+   * and actually creates the workspace.
+   *
+   * @param devfile the devfile describing the workspace
+   * @param namespace workspace name is unique in this namespace
+   * @param attributes workspace instance attributes
+   * @param contentProvider the content provider to use for resolving content references in the
+   *     devfile
+   * @return new workspace instance
+   * @throws NullPointerException when either {@code config} or {@code namespace} is null
+   * @throws NotFoundException when account with given id was not found
+   * @throws ConflictException when any conflict occurs (e.g Workspace with such name already exists
+   *     for {@code owner})
+   * @throws ServerException when any other error occurs
+   * @throws ValidationException when incoming configuration or attributes are not valid
+   */
   @Traced
   public WorkspaceImpl createWorkspace(
-      Devfile devfile, String namespace, Map<String, String> attributes)
+      Devfile devfile,
+      String namespace,
+      Map<String, String> attributes,
+      FileContentProvider contentProvider)
       throws ServerException, NotFoundException, ConflictException, ValidationException {
     TracingTags.STACK_ID.set(() -> attributes.getOrDefault("stackId", "no stack"));
 
     requireNonNull(devfile, "Required non-null devfile");
     requireNonNull(namespace, "Required non-null namespace");
     validator.validateAttributes(attributes);
 
+    try {
+      devfileIntegrityValidator.validateContentReferences(devfile, contentProvider);
+    } catch (DevfileFormatException e) {
+      throw new ValidationException(e.getMessage(), e);
+    }
+
     WorkspaceImpl workspace =
         doCreateWorkspace(devfile, accountManager.getByName(namespace), attributes, false);
     TracingTags.WORKSPACE_ID.set(workspace.getId());

wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/WorkspaceService.java

@@ -15,7 +15,9 @@
 import static java.lang.String.format;
 import static java.util.Collections.emptyMap;
 import static java.util.stream.Collectors.toList;
+import static javax.ws.rs.core.HttpHeaders.CONTENT_TYPE;
 import static javax.ws.rs.core.MediaType.APPLICATION_JSON;
+import static javax.ws.rs.core.MediaType.APPLICATION_JSON_TYPE;
 import static org.eclipse.che.api.workspace.server.DtoConverter.asDto;
 import static org.eclipse.che.api.workspace.server.WorkspaceKeyValidator.validateKey;
 import static org.eclipse.che.api.workspace.shared.Constants.CHE_WORKSPACE_AUTO_START;
@@ -44,12 +46,14 @@
 import javax.ws.rs.DELETE;
 import javax.ws.rs.DefaultValue;
 import javax.ws.rs.GET;
+import javax.ws.rs.HeaderParam;
 import javax.ws.rs.POST;
 import javax.ws.rs.PUT;
 import javax.ws.rs.Path;
 import javax.ws.rs.PathParam;
 import javax.ws.rs.Produces;
 import javax.ws.rs.QueryParam;
+import javax.ws.rs.core.MediaType;
 import javax.ws.rs.core.Response;
 import org.eclipse.che.api.core.BadRequestException;
 import org.eclipse.che.api.core.ConflictException;
@@ -62,10 +66,16 @@
 import org.eclipse.che.api.core.model.workspace.Workspace;
 import org.eclipse.che.api.core.model.workspace.config.ServerConfig;
 import org.eclipse.che.api.core.rest.Service;
+import org.eclipse.che.api.workspace.server.devfile.DevfileManager;
+import org.eclipse.che.api.workspace.server.devfile.FileContentProvider;
+import org.eclipse.che.api.workspace.server.devfile.URLFetcher;
+import org.eclipse.che.api.workspace.server.devfile.URLFileContentProvider;
+import org.eclipse.che.api.workspace.server.devfile.exception.DevfileException;
 import org.eclipse.che.api.workspace.server.model.impl.CommandImpl;
 import org.eclipse.che.api.workspace.server.model.impl.EnvironmentImpl;
 import org.eclipse.che.api.workspace.server.model.impl.ProjectConfigImpl;
 import org.eclipse.che.api.workspace.server.model.impl.WorkspaceImpl;
+import org.eclipse.che.api.workspace.server.model.impl.devfile.DevfileImpl;
 import org.eclipse.che.api.workspace.server.token.MachineAccessForbidden;
 import org.eclipse.che.api.workspace.server.token.MachineTokenException;
 import org.eclipse.che.api.workspace.server.token.MachineTokenProvider;
@@ -79,7 +89,6 @@
 import org.eclipse.che.api.workspace.shared.dto.ServerDto;
 import org.eclipse.che.api.workspace.shared.dto.WorkspaceConfigDto;
 import org.eclipse.che.api.workspace.shared.dto.WorkspaceDto;
-import org.eclipse.che.api.workspace.shared.dto.devfile.DevfileDto;
 import org.eclipse.che.commons.annotation.Nullable;
 import org.eclipse.che.commons.env.EnvironmentContext;
 
@@ -100,6 +109,8 @@ public class WorkspaceService extends Service {
   private final String devfileRegistryUrl;
   private final String apiEndpoint;
   private final boolean cheWorkspaceAutoStart;
+  private final FileContentProvider devfileContentProvider;
+  private final DevfileManager devfileManager;
 
   @Inject
   public WorkspaceService(
@@ -109,14 +120,18 @@ public WorkspaceService(
       MachineTokenProvider machineTokenProvider,
       WorkspaceLinksGenerator linksGenerator,
       @Named(CHE_WORKSPACE_PLUGIN_REGISTRY_URL_PROPERTY) @Nullable String pluginRegistryUrl,
-      @Named(CHE_WORKSPACE_DEVFILE_REGISTRY_URL_PROPERTY) @Nullable String devfileRegistryUrl) {
+      @Named(CHE_WORKSPACE_DEVFILE_REGISTRY_URL_PROPERTY) @Nullable String devfileRegistryUrl,
+      URLFetcher urlFetcher,
+      DevfileManager devfileManager) {
     this.apiEndpoint = apiEndpoint;
     this.cheWorkspaceAutoStart = cheWorkspaceAutoStart;
     this.workspaceManager = workspaceManager;
     this.machineTokenProvider = machineTokenProvider;
     this.linksGenerator = linksGenerator;
     this.pluginRegistryUrl = pluginRegistryUrl;
     this.devfileRegistryUrl = devfileRegistryUrl;
+    this.devfileContentProvider = new URLFileContentProvider(null, urlFetcher);
+    this.devfileManager = devfileManager;
   }
 
   @POST
@@ -188,14 +203,14 @@ public Response create(
   @Beta
   @Path("/devfile")
   @POST
-  @Consumes(APPLICATION_JSON)
+  @Consumes({APPLICATION_JSON, "text/yaml", "text/x-yaml"})
   @Produces(APPLICATION_JSON)
   @ApiOperation(
       value = "Creates a new workspace based on the Devfile.",
       notes =
           "This method is in beta phase. It's strongly recommended to use `POST /devfile` instead"
               + " to get a workspace from Devfile. Workspaces created with this method are not stable yet.",
-      consumes = APPLICATION_JSON,
+      consumes = "application/json, text/yaml, text/x-yaml",
       produces = APPLICATION_JSON,
       nickname = "createFromDevfile",
       response = WorkspaceConfigDto.class)
@@ -211,8 +226,7 @@ public Response create(
     @ApiResponse(code = 500, message = "Internal server error occurred")
   })
   public Response create(
-      @ApiParam(value = "The configuration to create the new workspace", required = true)
-          DevfileDto devfile,
+      @ApiParam(value = "The devfile of the workspace to create", required = true) String devfile,
       @ApiParam(
               value =
                   "Workspace attribute defined in 'attrName:attrValue' format. "
@@ -231,11 +245,23 @@ public Response create(
           @DefaultValue("false")
           Boolean startAfterCreate,
       @ApiParam("Namespace where workspace should be created") @QueryParam("namespace")
-          String namespace)
+          String namespace,
+      @HeaderParam(CONTENT_TYPE) MediaType contentType)
       throws ConflictException, BadRequestException, ForbiddenException, NotFoundException,
           ServerException {
     requiredNotNull(devfile, "Devfile");
 
+    DevfileImpl devfileModel;
+    try {
+      if (APPLICATION_JSON_TYPE.isCompatible(contentType)) {
+        devfileModel = devfileManager.parseJson(devfile);
+      } else {
+        devfileModel = devfileManager.parseYaml(devfile);
+      }
+    } catch (DevfileException e) {
+      throw new BadRequestException(e.getMessage());
+    }
+
     final Map<String, String> attributes = parseAttrs(attrsList);
 
     if (namespace == null) {
@@ -244,7 +270,16 @@ public Response create(
 
     WorkspaceImpl workspace;
     try {
-      workspace = workspaceManager.createWorkspace(devfile, namespace, attributes);
+      workspace =
+          workspaceManager.createWorkspace(
+              devfileModel,
+              namespace,
+              attributes,
+              // create a new cache for each request so that we don't have to care about lifetime
+              // of the cache, etc. The content is cached only for the duration of this call
+              // (i.e. all the validation and provisioning of the devfile will download each
+              // referenced file only once per request)
+              FileContentProvider.cached(devfileContentProvider));
     } catch (ValidationException x) {
       throw new BadRequestException(x.getMessage());
     }

wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/devfile/DevfileManager.java

@@ -82,16 +82,34 @@ public DevfileManager(
   }
 
   /**
-   * Creates {@link DevfileImpl} from given devfile content. Performs schema and integrity
+   * Creates {@link DevfileImpl} from given devfile content in YAML. Performs schema and integrity
    * validation of input data.
    *
    * @param devfileContent raw content of devfile
    * @return Devfile object created from the source content
    * @throws DevfileFormatException when any of schema or integrity validations fail
    * @throws DevfileFormatException when any yaml parsing error occurs
    */
-  public DevfileImpl parse(String devfileContent) throws DevfileFormatException {
-    JsonNode parsed = schemaValidator.validateBySchema(devfileContent);
+  public DevfileImpl parseYaml(String devfileContent) throws DevfileFormatException {
+    return parse(devfileContent, schemaValidator::validateYaml);
+  }
+
+  /**
+   * Creates {@link DevfileImpl} from given devfile content in JSON. Performs schema and integrity
+   * validation of input data.
+   *
+   * @param devfileContent raw content of devfile
+   * @return Devfile object created from the source content
+   * @throws DevfileFormatException when any of schema or integrity validations fail
+   * @throws DevfileFormatException when any yaml parsing error occurs
+   */
+  public DevfileImpl parseJson(String devfileContent) throws DevfileFormatException {
+    return parse(devfileContent, schemaValidator::validateJson);
+  }
+
+  private DevfileImpl parse(String content, ValidationFunction validationFunction)
+      throws DevfileFormatException {
+    JsonNode parsed = validationFunction.validate(content);
 
     DevfileImpl devfile;
     try {
@@ -188,4 +206,9 @@ private WorkspaceConfigImpl findAvailableName(WorkspaceConfigImpl config) throws
     }
     return config;
   }
+
+  @FunctionalInterface
+  private interface ValidationFunction {
+    JsonNode validate(String content) throws DevfileFormatException;
+  }
 }

wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/devfile/DevfileService.java

@@ -120,7 +120,7 @@ public Response createFromYaml(String data)
 
     WorkspaceImpl workspace;
     try {
-      DevfileImpl devfile = devfileManager.parse(data);
+      DevfileImpl devfile = devfileManager.parseYaml(data);
       workspace = devfileManager.createWorkspace(devfile, urlFileContentProvider);
     } catch (DevfileException e) {
       throw new BadRequestException(e.getMessage());

wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/devfile/URLFileContentProvider.java

@@ -17,6 +17,7 @@
 import java.net.URI;
 import java.net.URISyntaxException;
 import org.eclipse.che.api.workspace.server.devfile.exception.DevfileException;
+import org.eclipse.che.commons.annotation.Nullable;
 
 /**
  * A simple implementation of the FileContentProvider that merely uses the function resolve relative
@@ -27,7 +28,7 @@ public class URLFileContentProvider implements FileContentProvider {
   private final URI devfileLocation;
   private final URLFetcher urlFetcher;
 
-  public URLFileContentProvider(URI devfileLocation, URLFetcher urlFetcher) {
+  public URLFileContentProvider(@Nullable URI devfileLocation, URLFetcher urlFetcher) {
     this.devfileLocation = devfileLocation;
     this.urlFetcher = urlFetcher;
   }