Skip to content

Commit

Permalink
Now uses the existing infra for checking invalid schedule names.
Browse files Browse the repository at this point in the history
  • Loading branch information
Glenn Renfro committed Dec 16, 2019
1 parent 210191b commit 7abc86a
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 28 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
import io.fabric8.kubernetes.api.model.batch.CronJobList;
import io.fabric8.kubernetes.client.KubernetesClient;
import io.fabric8.kubernetes.client.KubernetesClientException;
import javax.validation.ConstraintViolationException;

import org.springframework.cloud.deployer.spi.scheduler.CreateScheduleException;
import org.springframework.cloud.deployer.spi.scheduler.ScheduleInfo;
Expand All @@ -55,8 +54,6 @@ public class KubernetesScheduler implements Scheduler {

private static final String SCHEDULE_EXPRESSION_FIELD_NAME = "spec.schedule";

private static final String SCHEDULE_METADATA_FIELD_NAME = "metadata.name";

private final KubernetesClient kubernetesClient;

private final KubernetesSchedulerProperties kubernetesSchedulerProperties;
Expand Down Expand Up @@ -85,11 +82,6 @@ public void schedule(ScheduleRequest scheduleRequest) {
throw new CreateScheduleException(invalidCronExceptionMessage, e);
}

invalidCronExceptionMessage = getExceptionMessageForField(e, SCHEDULE_METADATA_FIELD_NAME);
if (validationScheduleNameLength(invalidCronExceptionMessage)) {
throw new CreateScheduleException(invalidCronExceptionMessage, e);
}

throw new CreateScheduleException("Failed to create schedule " + scheduleRequest.getScheduleName(), e);
}
}
Expand All @@ -98,6 +90,9 @@ public void validateScheduleName(ScheduleRequest request) {
if(request.getScheduleName() == null) {
throw new CreateScheduleException("The name for the schedule request is null", null);
}
if(request.getScheduleName().length() > 52) {
throw new CreateScheduleException(String.format("because Schedule Name: '%s' has too many characters. Schedule name length must be 52 characters or less", request.getScheduleName()), null);
}
if(!Pattern.matches("^[a-z0-9]([-a-z0-9]*[a-z0-9])?$", request.getScheduleName())) {
throw new CreateScheduleException("Invalid Format for Schedule Name. Schedule name can only contain lowercase letters, numbers 0-9 and hyphens.", null);
}
Expand Down Expand Up @@ -153,19 +148,7 @@ protected CronJob createCronJob(ScheduleRequest scheduleRequest) {
String schedule = scheduleRequest.getSchedulerProperties().get(SchedulerPropertyKeys.CRON_EXPRESSION);
Assert.hasText(schedule, "The property: " + SchedulerPropertyKeys.CRON_EXPRESSION + " must be defined");

Container container;
try {
container = new ContainerCreator(this.kubernetesSchedulerProperties, scheduleRequest).build();
}
catch (ConstraintViolationException constraintViolationException) {
if (constraintViolationException.getMessage().contains("size must be between")) {
throw new CreateScheduleException(String.format("'%s' because the number of characters for the " +
"schedule name exceeds the Kubernetes maximum number of characters allowed for this field.",
scheduleRequest.getScheduleName()), constraintViolationException);
}
else
throw constraintViolationException;
}
Container container = new ContainerCreator(this.kubernetesSchedulerProperties, scheduleRequest).build();

String taskServiceAccountName = KubernetesSchedulerPropertyResolver.getTaskServiceAccountName(scheduleRequest,
this.kubernetesSchedulerProperties);
Expand Down Expand Up @@ -210,8 +193,4 @@ private void setImagePullSecret(ScheduleRequest scheduleRequest, CronJob cronJob
.add(localObjectReference);
}
}

private boolean validationScheduleNameLength(String message ) {
return (StringUtils.hasText(message) && message.contains("must be no more than") && message.endsWith("characters"));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
import io.fabric8.kubernetes.client.DefaultKubernetesClient;
import io.fabric8.kubernetes.client.KubernetesClient;
import io.fabric8.kubernetes.client.KubernetesClientException;
import javax.validation.ConstraintViolationException;
import org.junit.AfterClass;
import org.junit.ClassRule;
import org.junit.Test;
Expand Down Expand Up @@ -230,20 +231,25 @@ public void testInvalidCronSyntax() {

@Test
public void testNameTooLong() {
final String baseScheduleName = "tencharlng-scdf-itcouldbesaidthatthisislongtoowaytoo";
Map<String, String> schedulerProperties = Collections.singletonMap(CRON_EXPRESSION, "0/10 * * * *");

AppDefinition appDefinition = new AppDefinition(randomName(), null);
ScheduleRequest scheduleRequest = new ScheduleRequest(appDefinition, schedulerProperties, null, null,
"tencharlng-scdf-itcouldbesaidthatthisislongtoowaytoo-oops", testApplication());
baseScheduleName, testApplication());

//verify no validation fired.
scheduler.schedule(scheduleRequest);

scheduleRequest = new ScheduleRequest(appDefinition, schedulerProperties, null, null,
baseScheduleName + "1", testApplication());
try {
scheduler.schedule(scheduleRequest);
}
catch (CreateScheduleException createScheduleException) {
assertThat(createScheduleException.getMessage()).contains("must be no more than");
assertThat(createScheduleException.getMessage()).isEqualTo(String.format("Failed to create schedule because Schedule Name: '%s' has too many characters. Schedule name length must be 52 characters or less", baseScheduleName + "1"));
return;
}

fail();
}

Expand Down

0 comments on commit 7abc86a

Please sign in to comment.