Skip to content

Commit

Permalink
CHE-22: check environment parameter on workspace start
Browse files Browse the repository at this point in the history 
Signed-off-by: Alexander Garagatyi <agaragatyi@codenvy.com>
  • Loading branch information
Alexander Garagatyi committed Feb 26, 2016
1 parent cebb19f commit 166c2f5
Show file tree
Hide file tree
Showing 6 changed files with 536 additions and 168 deletions.
97 changes: 64 additions & 33 deletions ...e/src/main/java/org/eclipse/che/api/workspace/server/DefaultWorkspaceConfigValidator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
@Singleton
public class DefaultWorkspaceConfigValidator implements WorkspaceConfigValidator {
/* should contain [3, 20] characters, first and last character is letter or digit, available characters {A-Za-z0-9.-_}*/
private static final Pattern WS_NAME = Pattern.compile("[\\w][\\w\\.\\-]{1,18}[\\w]");
private static final Pattern WS_NAME = Pattern.compile("[a-zA-Z0-9][-_.a-zA-Z0-9]{1,18}[a-zA-Z0-9]");

/**
* Checks that workspace configuration is valid.
Expand All @@ -49,58 +49,67 @@ public class DefaultWorkspaceConfigValidator implements WorkspaceConfigValidator
@Override
public void validate(WorkspaceConfig config) throws BadRequestException {
// configuration object itself
requiredNotNull(config.getName(), "Workspace name required");
if (!WS_NAME.matcher(config.getName()).matches()) {
throw new BadRequestException("Incorrect workspace name, it must be between 3 and 20 characters and may contain digits, " +
"latin letters, underscores, dots, dashes and should start and end only with digits, " +
"latin letters or underscores");
}
checkNotNull(config.getName(), "Workspace name required");
checkArgument(WS_NAME.matcher(config.getName()).matches(),
"Incorrect workspace name, it must be between 3 and 20 characters and may contain digits, " +
"latin letters, underscores, dots, dashes and should start and end only with digits, " +
"latin letters or underscores");

//attributes
for (String attributeName : config.getAttributes().keySet()) {
//attribute name should not be empty and should not start with codenvy
if (attributeName.trim().isEmpty() || attributeName.toLowerCase().startsWith("codenvy")) {
throw new BadRequestException(format("Attribute name '%s' is not valid", attributeName));
}
checkArgument(attributeName != null && !attributeName.trim().isEmpty() && !attributeName.toLowerCase().startsWith("codenvy"),
"Attribute name '%s' is not valid",
attributeName);
}

//environments
requiredNotNull(config.getDefaultEnv(), "Workspace default environment name required");
if (!config.getEnvironments().stream().anyMatch(env -> env.getName().equals(config.getDefaultEnv()))) {
throw new BadRequestException("Workspace default environment configuration required");
}
checkArgument(!isNullOrEmpty(config.getDefaultEnv()), "Workspace default environment name required");
checkArgument(config.getEnvironments()
.stream()
.anyMatch(env -> config.getDefaultEnv().equals(env.getName())),
"Workspace default environment configuration required");

for (Environment environment : config.getEnvironments()) {
final String envName = environment.getName();
requiredNotNull(envName, "Environment name should not be null");
checkArgument(!isNullOrEmpty(envName), "Environment name should be neither null nor empty");

checkArgument(environment.getRecipe() == null || "docker".equals(environment.getRecipe().getType()),
"Couldn't start workspace '%s' from environment '%s', environment recipe has unsupported type '%s'",
config.getName(),
envName,
environment.getRecipe() != null ? environment.getRecipe().getType() : null);

//machine configs
if (environment.getMachineConfigs().isEmpty()) {
throw new BadRequestException("Environment '" + envName + "' should contain at least 1 machine");
}
checkArgument(!environment.getMachineConfigs().isEmpty(), "Environment '%s' should contain at least 1 machine", envName);

final long devCount = environment.getMachineConfigs()
.stream()
.filter(MachineConfig::isDev)
.count();
if (devCount != 1) {
throw new BadRequestException(format("Environment should contain exactly 1 dev machine, but '%s' contains '%d'",
envName,
devCount));
}
checkArgument(devCount == 1,
"Environment should contain exactly 1 dev machine, but '%s' contains '%d'",
envName,
devCount);
for (MachineConfig machineCfg : environment.getMachineConfigs()) {
if (isNullOrEmpty(machineCfg.getName())) {
throw new BadRequestException("Environment " + envName + " contains machine without of name");
}
requiredNotNull(machineCfg.getSource(), "Environment " + envName + " contains machine without of source");
//TODO require type?
checkArgument(!isNullOrEmpty(machineCfg.getName()), "Environment %s contains machine with null or empty name", envName);
checkNotNull(machineCfg.getSource(), "Environment " + envName + " contains machine without source");
checkArgument("docker".equals(machineCfg.getType()),
"Type of machine %s in environment %s is not supported. Supported value is 'docker'.",
machineCfg.getName(),
envName);
}
}

//commands
for (Command command : config.getCommands()) {
requiredNotNull(command.getName(), "Workspace " + config.getName() + " contains command without of name");
requiredNotNull(command.getCommandLine(), format("Command line required for command '%s' in workspace '%s'",
command.getName(),
config.getName()));
checkArgument(!isNullOrEmpty(command.getName()),
"Workspace %s contains command with null or empty name",
config.getName());
checkArgument(!isNullOrEmpty(command.getCommandLine()),
"Command line required for command '%s' in workspace '%s'",
command.getName(),
config.getName());
}

//projects
Expand All @@ -111,9 +120,31 @@ public void validate(WorkspaceConfig config) throws BadRequestException {
* Checks that object reference is not null, throws {@link BadRequestException}
* in the case of null {@code object} with given {@code message}.
*/
private void requiredNotNull(Object object, String message) throws BadRequestException {
private void checkNotNull(Object object, String message) throws BadRequestException {
if (object == null) {
throw new BadRequestException(message);
}
}

/**
* Checks that expression is true, throws {@link BadRequestException} otherwise.
*
* <p>Exception uses error message built from error message template and error message parameters.
*/
private void checkArgument(boolean expression, String errorMessageTemplate, Object... errorMessageParams) throws BadRequestException {
if (!expression) {
throw new BadRequestException(format(errorMessageTemplate, errorMessageParams));
}
}

/**
* Checks that expression is true, throws {@link BadRequestException} otherwise.
*
* <p>Exception uses error message built from error message template and error message parameters.
*/
private void checkArgument(boolean expression, String errorMessage) throws BadRequestException {
if (!expression) {
throw new BadRequestException(errorMessage);
}
}
}
42 changes: 3 additions & 39 deletions ...orkspace/src/main/java/org/eclipse/che/api/workspace/server/RuntimeWorkspaceRegistry.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.concurrent.locks.ReadWriteLock;
import java.util.concurrent.locks.ReentrantReadWriteLock;

Expand All @@ -65,6 +64,9 @@
* second for <i>owner -> list of workspaces</i> mapping(which speeds up fetching workspaces by owner).
* Maps are guarded by {@link ReentrantReadWriteLock}.
*
* <p>The implementation doesn't validate parameters.
* They should be validated by caller of methods of this class.
*
* @author Yevhenii Voevodin
* @author Alexander Garagatyi
*/
Expand Down Expand Up @@ -120,7 +122,6 @@ public RuntimeWorkspaceImpl start(UsersWorkspace usersWorkspace, String envName,
BadRequestException,
NotFoundException {
checkRegistryIsNotStopped();
checkWorkspaceIsValidForStart(usersWorkspace, envName);
// Prepare runtime workspace for start
final RuntimeWorkspaceImpl newRuntime = RuntimeWorkspaceImpl.builder()
.fromWorkspace(usersWorkspace)
Expand Down Expand Up @@ -367,43 +368,6 @@ private void startEnvironment(Environment environment, String workspaceId, boole
}
}

/**
* Checks if workspace is valid for start.
*/
private void checkWorkspaceIsValidForStart(UsersWorkspace workspace, String envName) throws BadRequestException {
if (workspace == null) {
throw new BadRequestException("Required non-null workspace");
}
if (envName == null) {
throw new BadRequestException(format("Couldn't start workspace '%s', environment name is null",
workspace.getConfig().getName()));
}
final Optional<? extends Environment> environmentOptional = workspace.getConfig()
.getEnvironments()
.stream()
.filter(env -> env.getName().equals(envName))
.findFirst();
if (!environmentOptional.isPresent()) {
throw new BadRequestException(format("Couldn't start workspace '%s', workspace doesn't have environment '%s'",
workspace.getConfig().getName(),
envName));
}
Environment environment = environmentOptional.get();
if (environment.getRecipe() != null && !"docker".equals(environment.getRecipe().getType())) {
throw new BadRequestException(format("Couldn't start workspace '%s' from environment '%s', " +
"environment recipe has unsupported type '%s'",
workspace.getConfig().getName(),
envName,
environment.getRecipe().getType()));
}
if (findDev(environment.getMachineConfigs()) == null) {
throw new BadRequestException(format("Couldn't start workspace '%s' from environment '%s', " +
"environment doesn't contain dev-machine",
workspace.getConfig().getName(),
envName));
}
}

/**
* Adds given machine to the running workspace, if the workspace exists.
* Sets up this machine as dev-machine if it is dev.
Expand Down
29 changes: 23 additions & 6 deletions ...re-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/WorkspaceManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -399,11 +399,13 @@ public UsersWorkspaceImpl startWorkspaceByName(String workspaceName,
* @see WorkspaceHooks#beforeCreate(UsersWorkspace, String)
* @see RuntimeWorkspaceRegistry#start(UsersWorkspace, String)
*/
public RuntimeWorkspaceImpl startTemporaryWorkspace(WorkspaceConfig workspaceConfig, @Nullable String accountId) throws ServerException,
BadRequestException,
ForbiddenException,
NotFoundException,
ConflictException {
public RuntimeWorkspaceImpl startTemporaryWorkspace(WorkspaceConfig workspaceConfig,
@Nullable String accountId) throws ServerException,
BadRequestException,
ForbiddenException,
NotFoundException,
ConflictException {

final UsersWorkspaceImpl workspace = fromConfig(workspaceConfig, getCurrentUserId());
workspace.setTemporary(true);
// Temporary workspace is not persistent one, which means
Expand Down Expand Up @@ -573,7 +575,9 @@ private UsersWorkspaceImpl fromConfig(WorkspaceConfig config, String owner) thro
UsersWorkspaceImpl performAsyncStart(UsersWorkspaceImpl workspace,
String envName,
boolean recover,
@Nullable String accountId) throws ConflictException {
@Nullable String accountId) throws ConflictException,
BadRequestException {

// Runtime workspace registry performs this check as well
// but this check needed here because permanent workspace start performed asynchronously
// which means that even if registry won't start workspace client receives workspace object
Expand All @@ -588,6 +592,19 @@ UsersWorkspaceImpl performAsyncStart(UsersWorkspaceImpl workspace,
}
workspace.setTemporary(false);
workspace.setStatus(WorkspaceStatus.STARTING);

if (envName != null && !workspace.getConfig()
.getEnvironments()
.stream()
.filter(env -> env.getName().equals(envName))
.findFirst()
.isPresent()) {

throw new BadRequestException(format("Couldn't start workspace '%s', workspace doesn't have environment '%s'",
workspace.getConfig().getName(),
envName));
}

executor.execute(ThreadLocalPropagateContext.wrap(() -> {
try {
performSyncStart(workspace, firstNonNull(envName, workspace.getConfig().getDefaultEnv()), recover, accountId);
Expand Down
Loading

0 comments on commit 166c2f5

Please sign in to comment.