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.

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

Add devfile validation to /workspace/devfile endpoint #13472

Merged
merged 5 commits into from
Jun 5, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}

/**
Expand Down Expand Up @@ -122,14 +128,24 @@ public WorkspaceImpl createWorkspace(

@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.validateDevfile(devfile);
metlos marked this conversation as resolved.
Show resolved Hide resolved
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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@
import javax.ws.rs.PathParam;
import javax.ws.rs.Produces;
import javax.ws.rs.QueryParam;
import javax.ws.rs.core.Context;
import javax.ws.rs.core.HttpHeaders;
import javax.ws.rs.core.Response;
import org.eclipse.che.api.core.BadRequestException;
import org.eclipse.che.api.core.ConflictException;
Expand All @@ -62,10 +64,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;
Expand All @@ -79,7 +87,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;

Expand All @@ -100,6 +107,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(
Expand All @@ -109,14 +118,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) {
skabashnyuk marked this conversation as resolved.
Show resolved Hide resolved
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
Expand Down Expand Up @@ -188,14 +201,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)
Expand All @@ -211,8 +224,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. "
Expand All @@ -231,11 +243,23 @@ public Response create(
@DefaultValue("false")
Boolean startAfterCreate,
@ApiParam("Namespace where workspace should be created") @QueryParam("namespace")
String namespace)
String namespace,
@Context HttpHeaders headers)
throws ConflictException, BadRequestException, ForbiddenException, NotFoundException,
ServerException {
requiredNotNull(devfile, "Devfile");

DevfileImpl devfileModel;
metlos marked this conversation as resolved.
Show resolved Hide resolved
try {
if (headers.getRequestHeader(HttpHeaders.CONTENT_TYPE).contains(APPLICATION_JSON)) {
skabashnyuk marked this conversation as resolved.
Show resolved Hide resolved
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) {
Expand All @@ -244,7 +268,9 @@ public Response create(

WorkspaceImpl workspace;
try {
workspace = workspaceManager.createWorkspace(devfile, namespace, attributes);
workspace =
workspaceManager.createWorkspace(
devfileModel, namespace, attributes, devfileContentProvider);
} catch (ValidationException x) {
throw new BadRequestException(x.getMessage());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -188,4 +206,9 @@ private WorkspaceConfigImpl findAvailableName(WorkspaceConfigImpl config) throws
}
return config;
}

@FunctionalInterface
private interface ValidationFunction {
JsonNode validate(String content) throws DevfileFormatException;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,15 @@
public class DevfileSchemaValidator {

private final JsonValidationService service = JsonValidationService.newInstance();
private ObjectMapper yamlReader;
private ObjectMapper jsonWriter;
private ObjectMapper yamlMapper;
private ObjectMapper jsonMapper;
private JsonSchema schema;
private ErrorMessageComposer errorMessageComposer;

@Inject
public DevfileSchemaValidator(DevfileSchemaProvider schemaProvider) {
this.yamlReader = new ObjectMapper(new YAMLFactory());
this.jsonWriter = new ObjectMapper();
this.yamlMapper = new ObjectMapper(new YAMLFactory());
this.jsonMapper = new ObjectMapper();
this.errorMessageComposer = new ErrorMessageComposer();
try {
this.schema = service.readSchema(schemaProvider.getAsReader());
Expand All @@ -52,23 +52,39 @@ public DevfileSchemaValidator(DevfileSchemaProvider schemaProvider) {
}
}

public JsonNode validateBySchema(String yamlContent) throws DevfileFormatException {
public JsonNode validateYaml(String yamlContent) throws DevfileFormatException {
return validate(yamlContent, yamlMapper);
}

public JsonNode validateJson(String jsonContent) throws DevfileFormatException {
return validate(jsonContent, jsonMapper);
}

private JsonNode validate(String content, ObjectMapper mapper) throws DevfileFormatException {
JsonNode contentNode;
try {
contentNode = yamlReader.readTree(yamlContent);
contentNode = mapper.readTree(content);
validate(contentNode);
return contentNode;
} catch (IOException e) {
throw new DevfileFormatException("Unable to validate Devfile. Error: " + e.getMessage());
}
}

private void validate(JsonNode contentNode) throws DevfileFormatException {
try {
List<Problem> validationErrors = new ArrayList<>();
ProblemHandler handler = ProblemHandler.collectingTo(validationErrors);
try (JsonReader reader =
service.createReader(
new StringReader(jsonWriter.writeValueAsString(contentNode)), schema, handler)) {
new StringReader(jsonMapper.writeValueAsString(contentNode)), schema, handler)) {
reader.read();
}
if (validationErrors.isEmpty()) {
return contentNode;
if (!validationErrors.isEmpty()) {
String error = errorMessageComposer.extractMessages(validationErrors, new StringBuilder());
throw new DevfileFormatException(
format("Devfile schema validation failed. Error: %s", error));
}
String error = errorMessageComposer.extractMessages(validationErrors, new StringBuilder());
throw new DevfileFormatException(
format("Devfile schema validation failed. Error: %s", error));
} catch (IOException e) {
throw new DevfileFormatException("Unable to validate Devfile. Error: " + e.getMessage());
}
Expand Down
Loading