Skip to content
This repository was archived by the owner on May 14, 2025. It is now read-only.

Commit d20f0dd

Browse files
ilayaperumalgmarkpollack
authored andcommitted
Validate app names for their length
- Before deploying/launching stream/task, validate the app names' length - For streams, the length is computed as `streamName-appName-v{version} - Rename the validation method - Add more logging and increase the version length limit to 5 digits Fixes #3005
1 parent 3696723 commit d20f0dd

File tree

3 files changed

+78
-6
lines changed

3 files changed

+78
-6
lines changed

spring-cloud-dataflow-server-core/src/main/java/org/springframework/cloud/dataflow/server/service/impl/validation/DefaultTaskValidationService.java

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2018 the original author or authors.
2+
* Copyright 2018-2019 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -26,6 +26,7 @@
2626
import org.springframework.cloud.dataflow.server.service.ValidationStatus;
2727
import org.springframework.cloud.dataflow.server.service.impl.NodeStatus;
2828
import org.springframework.cloud.dataflow.server.service.impl.TaskServiceUtils;
29+
import org.springframework.cloud.task.listener.TaskException;
2930
import org.springframework.util.Assert;
3031
import org.springframework.util.StringUtils;
3132

@@ -35,9 +36,12 @@
3536
*
3637
* @author Glenn Renfro
3738
* @author Mark Pollack
39+
* @author Ilayaperumal Gopinathan
3840
*/
3941
public class DefaultTaskValidationService extends DefaultValidationService implements TaskValidationService {
4042

43+
private static final int MAX_APPNAME_LENGTH = 63;
44+
4145
private final TaskDefinitionRepository taskDefinitionRepository;
4246

4347
private final String composedTaskRunnerName;
@@ -54,6 +58,7 @@ public DefaultTaskValidationService(AppRegistryService appRegistry,
5458

5559
@Override
5660
public ValidationStatus validateTask(String name) {
61+
validateTaskName(name);
5762
TaskDefinition definition = this.taskDefinitionRepository.findByTaskName(name);
5863
ValidationStatus validationStatus = new ValidationStatus(
5964
definition.getName(),
@@ -68,6 +73,7 @@ public ValidationStatus validateTask(String name) {
6873
String childTaskPrefix = TaskNode.getTaskPrefix(name);
6974
taskNode.getTaskApps().stream().forEach(task -> {
7075
TaskDefinition childDefinition = this.taskDefinitionRepository.findByTaskName(childTaskPrefix + task.getName());
76+
validateTaskName(childTaskPrefix + task.getName());
7177
boolean status = this.validate(childDefinition.getRegisteredAppName(), ApplicationType.task);
7278
validationStatus.getAppsStatuses().put(
7379
String.format("%s:%s", appType.name(), childDefinition.getName()),
@@ -88,4 +94,10 @@ public ValidationStatus validateTask(String name) {
8894
}
8995
return validationStatus;
9096
}
97+
98+
private void validateTaskName(String taskName) {
99+
if (taskName.length() > MAX_APPNAME_LENGTH) {
100+
throw new TaskException("The task name should not exceed "+ MAX_APPNAME_LENGTH+ " in length.");
101+
}
102+
}
91103
}

spring-cloud-dataflow-server-core/src/main/java/org/springframework/cloud/dataflow/server/stream/SkipperStreamDeployer.java

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848
import org.springframework.cloud.dataflow.registry.service.AppRegistryService;
4949
import org.springframework.cloud.dataflow.rest.SkipperStream;
5050
import org.springframework.cloud.dataflow.server.controller.NoSuchAppException;
51+
import org.springframework.cloud.dataflow.server.controller.support.InvalidStreamDefinitionException;
5152
import org.springframework.cloud.dataflow.server.repository.NoSuchStreamDefinitionException;
5253
import org.springframework.cloud.dataflow.server.repository.StreamDefinitionRepository;
5354
import org.springframework.cloud.deployer.spi.app.AppInstanceStatus;
@@ -98,6 +99,9 @@ public class SkipperStreamDeployer implements StreamDeployer {
9899

99100
private static Log logger = LogFactory.getLog(SkipperStreamDeployer.class);
100101

102+
//Assume version suffix added by skipper is 5 chars.
103+
private static final int MAX_APPNAME_LENGTH = 63-5;
104+
101105
private final SkipperClient skipperClient;
102106

103107
private final StreamDefinitionRepository streamDefinitionRepository;
@@ -200,7 +204,7 @@ private boolean streamDefinitionExists(String streamName) {
200204
}
201205

202206
public Release deployStream(StreamDeploymentRequest streamDeploymentRequest) {
203-
validateAllAppsRegistered(streamDeploymentRequest);
207+
validateStreamDeploymentRequest(streamDeploymentRequest);
204208
Map<String, String> streamDeployerProperties = streamDeploymentRequest.getStreamDeployerProperties();
205209
String packageVersion = streamDeployerProperties.get(SkipperStream.SKIPPER_PACKAGE_VERSION);
206210
Assert.isTrue(StringUtils.hasText(packageVersion), "Package Version must be set");
@@ -283,21 +287,32 @@ private String determinePlatformName(final String platformName) {
283287
}
284288
}
285289

286-
private void validateAllAppsRegistered(StreamDeploymentRequest streamDeploymentRequest) {
290+
private void validateStreamDeploymentRequest(StreamDeploymentRequest streamDeploymentRequest) {
287291
if (streamDeploymentRequest.getAppDeploymentRequests() == null
288292
|| streamDeploymentRequest.getAppDeploymentRequests().isEmpty()) {
289293
// nothing to validate.
290294
return;
291295
}
292-
296+
String streamName = streamDeploymentRequest.getStreamName();
293297
// throw as at this point we should have definition
294298
StreamDefinition streamDefinition = this.streamDefinitionRepository
295-
.findById(streamDeploymentRequest.getStreamName())
299+
.findById(streamName)
296300
.orElseThrow(() -> new NoSuchStreamDefinitionException(streamDeploymentRequest.getStreamName()));
297301

298302
for (AppDeploymentRequest adr : streamDeploymentRequest.getAppDeploymentRequests()) {
303+
String registeredAppName = getRegisteredName(streamDefinition, adr.getDefinition().getName());
304+
String appName = String.format("%s-%s-v", streamName, registeredAppName);
305+
if (appName.length() > 40) {
306+
logger.warn("The stream name plus application name [" + appName + "] is longer than 40 characters." +
307+
" This can not exceed " + MAX_APPNAME_LENGTH + " in length.");
308+
}
309+
if (appName.length() > MAX_APPNAME_LENGTH) {
310+
throw new InvalidStreamDefinitionException(
311+
String.format("The runtime application name for the app %s in the stream %s "
312+
+ "should not exceed %s in length. The runtime application name is: %s", registeredAppName, streamName, MAX_APPNAME_LENGTH, appName));
313+
}
299314
String version = this.appRegistryService.getResourceVersion(adr.getResource());
300-
validateAppVersionIsRegistered(getRegisteredName(streamDefinition, adr.getDefinition().getName()), adr, version);
315+
validateAppVersionIsRegistered(registeredAppName, adr, version);
301316
}
302317
}
303318

spring-cloud-dataflow-server-core/src/test/java/org/springframework/cloud/dataflow/server/stream/SkipperStreamDeployerTests.java

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import org.springframework.cloud.dataflow.core.StreamDefinition;
3434
import org.springframework.cloud.dataflow.registry.service.AppRegistryService;
3535
import org.springframework.cloud.dataflow.rest.SkipperStream;
36+
import org.springframework.cloud.dataflow.server.controller.support.InvalidStreamDefinitionException;
3637
import org.springframework.cloud.dataflow.server.repository.StreamDefinitionRepository;
3738
import org.springframework.cloud.dataflow.server.support.MockUtils;
3839
import org.springframework.cloud.dataflow.server.support.SkipperPackageUtils;
@@ -375,6 +376,50 @@ public void testStateOfUndeployedStream() {
375376

376377
}
377378

379+
@Test
380+
public void testStreamDeployWithLongAppName() {
381+
382+
AppRegistryService appRegistryService = mock(AppRegistryService.class);
383+
384+
when(appRegistryService.appExist(eq("time"), eq(ApplicationType.source), eq("1.2.0.RELEASE")))
385+
.thenReturn(true);
386+
when(appRegistryService.appExist(eq("log"), eq(ApplicationType.sink), eq("1.2.0.RELEASE")))
387+
.thenReturn(true);
388+
389+
AppDefinition timeAppDefinition = new AppDefinition("time", new HashMap<>());
390+
MavenResource timeResource = new MavenResource.Builder()
391+
.artifactId("time-source-rabbit").groupId("org.springframework.cloud.stream.app")
392+
.version("1.2.0.RELEASE").build();
393+
when(appRegistryService.getResourceVersion(timeResource)).thenReturn(timeResource.getVersion());
394+
AppDeploymentRequest timeAppDeploymentRequest = new AppDeploymentRequest(timeAppDefinition, timeResource);
395+
396+
List<AppDeploymentRequest> appDeploymentRequests = Arrays.asList(timeAppDeploymentRequest);
397+
398+
String streamName = "asdfkdunfdnereerejrerkjelkraerkldjkfdjfkdsjflkjdflkdjflsdflsdjfldlfdlsfjdlfjdlfjdslfdnmdfndfmdsfmndsdfafdsfmdnfdske";
399+
400+
String streamDSL = "time | log";
401+
402+
StreamDeploymentRequest streamDeploymentRequest = new StreamDeploymentRequest(streamName, streamDSL,
403+
appDeploymentRequests,
404+
new HashMap<>());
405+
406+
SkipperClient skipperClient = MockUtils.createSkipperClientMock();
407+
408+
StreamDefinitionRepository streamDefinitionRepository = mock(StreamDefinitionRepository.class);
409+
SkipperStreamDeployer skipperStreamDeployer = new SkipperStreamDeployer(skipperClient,
410+
streamDefinitionRepository, appRegistryService, mock(ForkJoinPool.class));
411+
412+
when(streamDefinitionRepository.findById(streamName)).thenReturn(Optional.of(new StreamDefinition(streamName, streamDSL)));
413+
try {
414+
skipperStreamDeployer.deployStream(streamDeploymentRequest);
415+
fail("Expected InvalidStreamDefinitionException");
416+
}
417+
catch (Exception e) {
418+
assertThat(e instanceof InvalidStreamDefinitionException).isTrue();
419+
assertThat(e.getMessage().equals("The runtime application name for the app time in the stream "+streamName+" should not exceed 63 in length. Currently it is: "+streamName+"-time-v{version-2digits}"));
420+
}
421+
}
422+
378423
private Info createInfo(StatusCode statusCode) {
379424
Info info = new Info();
380425
Status status = new Status();

0 commit comments

Comments
 (0)