Skip to content

Commit 2986552

Browse files
author
Alexander Garagatyi
committed
CHE-22: check environment parameter on workspace start
Signed-off-by: Alexander Garagatyi <agaragatyi@codenvy.com>
1 parent 8482352 commit 2986552

6 files changed

Lines changed: 468 additions & 168 deletions

File tree

‎core/platform-api/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/DefaultWorkspaceConfigValidator.java‎

Lines changed: 64 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@
3030
@Singleton
3131
public class DefaultWorkspaceConfigValidator implements WorkspaceConfigValidator {
3232
/* should contain [3, 20] characters, first and last character is letter or digit, available characters {A-Za-z0-9.-_}*/
33-
private static final Pattern WS_NAME = Pattern.compile("[\\w][\\w\\.\\-]{1,18}[\\w]");
33+
private static final Pattern WS_NAME = Pattern.compile("[a-zA-Z0-9][-_.a-zA-Z0-9]{1,18}[a-zA-Z0-9]");
3434

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

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

6766
//environments
68-
requiredNotNull(config.getDefaultEnv(), "Workspace default environment name required");
69-
if (!config.getEnvironments().stream().anyMatch(env -> env.getName().equals(config.getDefaultEnv()))) {
70-
throw new BadRequestException("Workspace default environment configuration required");
71-
}
67+
checkArgument(!isNullOrEmpty(config.getDefaultEnv()), "Workspace default environment name required");
68+
checkArgument(config.getEnvironments()
69+
.stream()
70+
.anyMatch(env -> config.getDefaultEnv().equals(env.getName())),
71+
"Workspace default environment configuration required");
72+
7273
for (Environment environment : config.getEnvironments()) {
7374
final String envName = environment.getName();
74-
requiredNotNull(envName, "Environment name should not be null");
75+
checkArgument(!isNullOrEmpty(envName), "Environment name should be neither null nor empty");
76+
77+
checkArgument(environment.getRecipe() == null || "docker".equals(environment.getRecipe().getType()),
78+
"Couldn't start workspace '%s' from environment '%s', environment recipe has unsupported type '%s'",
79+
config.getName(),
80+
envName,
81+
environment.getRecipe() != null ? environment.getRecipe().getType() : null);
7582

7683
//machine configs
77-
if (environment.getMachineConfigs().isEmpty()) {
78-
throw new BadRequestException("Environment '" + envName + "' should contain at least 1 machine");
79-
}
84+
checkArgument(!environment.getMachineConfigs().isEmpty(), "Environment '%s' should contain at least 1 machine", envName);
85+
8086
final long devCount = environment.getMachineConfigs()
8187
.stream()
8288
.filter(MachineConfig::isDev)
8389
.count();
84-
if (devCount != 1) {
85-
throw new BadRequestException(format("Environment should contain exactly 1 dev machine, but '%s' contains '%d'",
86-
envName,
87-
devCount));
88-
}
90+
checkArgument(devCount == 1,
91+
"Environment should contain exactly 1 dev machine, but '%s' contains '%d'",
92+
envName,
93+
devCount);
8994
for (MachineConfig machineCfg : environment.getMachineConfigs()) {
90-
if (isNullOrEmpty(machineCfg.getName())) {
91-
throw new BadRequestException("Environment " + envName + " contains machine without of name");
92-
}
93-
requiredNotNull(machineCfg.getSource(), "Environment " + envName + " contains machine without of source");
94-
//TODO require type?
95+
checkArgument(!isNullOrEmpty(machineCfg.getName()), "Environment %s contains machine with null or empty name", envName);
96+
checkNotNull(machineCfg.getSource(), "Environment " + envName + " contains machine without source");
97+
checkArgument("docker".equals(machineCfg.getType()),
98+
"Type of machine %s in environment %s is not supported. Supported value is 'docker'.",
99+
machineCfg.getName(),
100+
envName);
95101
}
96102
}
97103

98104
//commands
99105
for (Command command : config.getCommands()) {
100-
requiredNotNull(command.getName(), "Workspace " + config.getName() + " contains command without of name");
101-
requiredNotNull(command.getCommandLine(), format("Command line required for command '%s' in workspace '%s'",
102-
command.getName(),
103-
config.getName()));
106+
checkArgument(!isNullOrEmpty(command.getName()),
107+
"Workspace %s contains command with null or empty name",
108+
config.getName());
109+
checkArgument(!isNullOrEmpty(command.getCommandLine()),
110+
"Command line required for command '%s' in workspace '%s'",
111+
command.getName(),
112+
config.getName());
104113
}
105114

106115
//projects
@@ -111,9 +120,31 @@ public void validate(WorkspaceConfig config) throws BadRequestException {
111120
* Checks that object reference is not null, throws {@link BadRequestException}
112121
* in the case of null {@code object} with given {@code message}.
113122
*/
114-
private void requiredNotNull(Object object, String message) throws BadRequestException {
123+
private void checkNotNull(Object object, String message) throws BadRequestException {
115124
if (object == null) {
116125
throw new BadRequestException(message);
117126
}
118127
}
128+
129+
/**
130+
* Checks that expression is true, throws {@link BadRequestException} otherwise.
131+
*
132+
* <p>Exception uses error message built from error message template and error message parameters.
133+
*/
134+
private void checkArgument(boolean expression, String errorMessageTemplate, Object... errorMessageParams) throws BadRequestException {
135+
if (!expression) {
136+
throw new BadRequestException(format(errorMessageTemplate, errorMessageParams));
137+
}
138+
}
139+
140+
/**
141+
* Checks that expression is true, throws {@link BadRequestException} otherwise.
142+
*
143+
* <p>Exception uses error message built from error message template and error message parameters.
144+
*/
145+
private void checkArgument(boolean expression, String errorMessage) throws BadRequestException {
146+
if (!expression) {
147+
throw new BadRequestException(errorMessage);
148+
}
149+
}
119150
}

‎core/platform-api/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/RuntimeWorkspaceRegistry.java‎

Lines changed: 3 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@
3838
import java.util.HashMap;
3939
import java.util.List;
4040
import java.util.Map;
41-
import java.util.Optional;
4241
import java.util.concurrent.locks.ReadWriteLock;
4342
import java.util.concurrent.locks.ReentrantReadWriteLock;
4443

@@ -65,6 +64,9 @@
6564
* second for <i>owner -> list of workspaces</i> mapping(which speeds up fetching workspaces by owner).
6665
* Maps are guarded by {@link ReentrantReadWriteLock}.
6766
*
67+
* <p>The implementation doesn't validate parameters.
68+
* They should be validated by caller of methods of this class.
69+
*
6870
* @author Yevhenii Voevodin
6971
* @author Alexander Garagatyi
7072
*/
@@ -120,7 +122,6 @@ public RuntimeWorkspaceImpl start(UsersWorkspace usersWorkspace, String envName,
120122
BadRequestException,
121123
NotFoundException {
122124
checkRegistryIsNotStopped();
123-
checkWorkspaceIsValidForStart(usersWorkspace, envName);
124125
// Prepare runtime workspace for start
125126
final RuntimeWorkspaceImpl newRuntime = RuntimeWorkspaceImpl.builder()
126127
.fromWorkspace(usersWorkspace)
@@ -367,43 +368,6 @@ private void startEnvironment(Environment environment, String workspaceId, boole
367368
}
368369
}
369370

370-
/**
371-
* Checks if workspace is valid for start.
372-
*/
373-
private void checkWorkspaceIsValidForStart(UsersWorkspace workspace, String envName) throws BadRequestException {
374-
if (workspace == null) {
375-
throw new BadRequestException("Required non-null workspace");
376-
}
377-
if (envName == null) {
378-
throw new BadRequestException(format("Couldn't start workspace '%s', environment name is null",
379-
workspace.getConfig().getName()));
380-
}
381-
final Optional<? extends Environment> environmentOptional = workspace.getConfig()
382-
.getEnvironments()
383-
.stream()
384-
.filter(env -> env.getName().equals(envName))
385-
.findFirst();
386-
if (!environmentOptional.isPresent()) {
387-
throw new BadRequestException(format("Couldn't start workspace '%s', workspace doesn't have environment '%s'",
388-
workspace.getConfig().getName(),
389-
envName));
390-
}
391-
Environment environment = environmentOptional.get();
392-
if (environment.getRecipe() != null && !"docker".equals(environment.getRecipe().getType())) {
393-
throw new BadRequestException(format("Couldn't start workspace '%s' from environment '%s', " +
394-
"environment recipe has unsupported type '%s'",
395-
workspace.getConfig().getName(),
396-
envName,
397-
environment.getRecipe().getType()));
398-
}
399-
if (findDev(environment.getMachineConfigs()) == null) {
400-
throw new BadRequestException(format("Couldn't start workspace '%s' from environment '%s', " +
401-
"environment doesn't contain dev-machine",
402-
workspace.getConfig().getName(),
403-
envName));
404-
}
405-
}
406-
407371
/**
408372
* Adds given machine to the running workspace, if the workspace exists.
409373
* Sets up this machine as dev-machine if it is dev.

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

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -399,11 +399,13 @@ public UsersWorkspaceImpl startWorkspaceByName(String workspaceName,
399399
* @see WorkspaceHooks#beforeCreate(UsersWorkspace, String)
400400
* @see RuntimeWorkspaceRegistry#start(UsersWorkspace, String)
401401
*/
402-
public RuntimeWorkspaceImpl startTemporaryWorkspace(WorkspaceConfig workspaceConfig, @Nullable String accountId) throws ServerException,
403-
BadRequestException,
404-
ForbiddenException,
405-
NotFoundException,
406-
ConflictException {
402+
public RuntimeWorkspaceImpl startTemporaryWorkspace(WorkspaceConfig workspaceConfig,
403+
@Nullable String accountId) throws ServerException,
404+
BadRequestException,
405+
ForbiddenException,
406+
NotFoundException,
407+
ConflictException {
408+
407409
final UsersWorkspaceImpl workspace = fromConfig(workspaceConfig, getCurrentUserId());
408410
workspace.setTemporary(true);
409411
// Temporary workspace is not persistent one, which means
@@ -573,7 +575,9 @@ private UsersWorkspaceImpl fromConfig(WorkspaceConfig config, String owner) thro
573575
UsersWorkspaceImpl performAsyncStart(UsersWorkspaceImpl workspace,
574576
String envName,
575577
boolean recover,
576-
@Nullable String accountId) throws ConflictException {
578+
@Nullable String accountId) throws ConflictException,
579+
BadRequestException {
580+
577581
// Runtime workspace registry performs this check as well
578582
// but this check needed here because permanent workspace start performed asynchronously
579583
// which means that even if registry won't start workspace client receives workspace object
@@ -588,6 +592,17 @@ UsersWorkspaceImpl performAsyncStart(UsersWorkspaceImpl workspace,
588592
}
589593
workspace.setTemporary(false);
590594
workspace.setStatus(WorkspaceStatus.STARTING);
595+
596+
if (envName != null && !workspace.getConfig()
597+
.getEnvironments()
598+
.stream()
599+
.anyMatch(env -> env.getName().equals(envName))) {
600+
601+
throw new BadRequestException(format("Couldn't start workspace '%s', workspace doesn't have environment '%s'",
602+
workspace.getConfig().getName(),
603+
envName));
604+
}
605+
591606
executor.execute(ThreadLocalPropagateContext.wrap(() -> {
592607
try {
593608
performSyncStart(workspace, firstNonNull(envName, workspace.getConfig().getDefaultEnv()), recover, accountId);

0 commit comments

Comments
 (0)