Skip to content
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import com.netflix.maestro.models.parameter.ParamDefinition;
import com.netflix.maestro.models.signal.SignalDependenciesDefinition;
import com.netflix.maestro.models.signal.SignalOutputsDefinition;
import com.netflix.maestro.validations.MaestroNameConstraint;
import com.netflix.maestro.validations.MaestroReferenceIdConstraint;
import com.netflix.maestro.validations.SignalDependenciesDefinitionConstraint;
import com.netflix.maestro.validations.SignalOutputsDefinitionConstraint;
Expand All @@ -44,7 +45,7 @@ public abstract class AbstractStep implements Step {
private String id;

@Getter(onMethod = @__({@Override}))
@Size(max = Constants.NAME_LENGTH_LIMIT)
@MaestroNameConstraint(required = false, enforcePattern = false)
Comment thread
jakhani marked this conversation as resolved.
Outdated
private String name;

@Getter(onMethod = @__({@Override}))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import com.netflix.maestro.models.trigger.TimeTrigger;
import com.netflix.maestro.utils.MapHelper;
import com.netflix.maestro.validations.MaestroIdConstraint;
import com.netflix.maestro.validations.MaestroNameConstraint;
import com.netflix.maestro.validations.SignalTriggerConstraint;
import com.netflix.maestro.validations.TagListConstraint;
import com.netflix.maestro.validations.TimeTriggerConstraint;
Expand Down Expand Up @@ -73,7 +74,7 @@ public class Workflow {
* Name of the workflow. Can be absent by user. Can be filled by workflow id if absent - use
* helper method in WorkflowHelper.
*/
@Size(max = Constants.NAME_LENGTH_LIMIT)
@MaestroNameConstraint(required = false, enforcePattern = false)
private final String name;

@Size(max = Constants.FIELD_SIZE_LIMIT)
Expand All @@ -96,7 +97,7 @@ public class Workflow {
@Builder(toBuilder = true)
Workflow(
String id,
@Size(max = Constants.NAME_LENGTH_LIMIT) String name,
String name,
@Size(max = Constants.FIELD_SIZE_LIMIT) String description,
@Valid TagList tags,
ParsableLong timeout,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
/*
* Copyright 2024 Netflix, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
* an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
* specific language governing permissions and limitations under the License.
*/
package com.netflix.maestro.utils;

import com.netflix.maestro.models.Constants;

/** Interface exposing configurable validation limits used by constraint validators. */
public interface ValidationLimits {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May be MaestroNameIdValidationLimits or MaestroNameIdValidationConfigs


/**
* Default instance backed by compile-time {@link Constants} values. Used by validators when no
* configured {@link ValidationLimits} bean is injected (e.g. in unit tests without Spring). Keeps
* each limit independently defaulted so id and name limits remain decoupled.
*/
ValidationLimits DEFAULTS =
new ValidationLimits() {
@Override
public int getIdLengthLimit() {
return Constants.ID_LENGTH_LIMIT;
}

@Override
public int getNameLengthLimit() {
return Constants.NAME_LENGTH_LIMIT;
}
};

/** Returns the maximum allowed length for Maestro IDs. */
int getIdLengthLimit();

/** Returns the maximum allowed length for Maestro names. */
int getNameLengthLimit();
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
package com.netflix.maestro.validations;

import com.netflix.maestro.models.Constants;
import com.netflix.maestro.utils.ValidationLimits;
import jakarta.inject.Inject;
import jakarta.validation.Constraint;
import jakarta.validation.ConstraintValidator;
import jakarta.validation.ConstraintValidatorContext;
Expand Down Expand Up @@ -44,6 +46,8 @@
class MaestroIdValidator implements ConstraintValidator<MaestroIdConstraint, String> {
private static final Pattern ID_PATTERN = Pattern.compile("[_a-zA-Z0-9][.\\-_a-zA-Z0-9]*+");

@Inject private ValidationLimits validationLimits;

@Override
public void initialize(MaestroIdConstraint constraint) {}

Expand All @@ -56,12 +60,15 @@ public boolean isValid(String id, ConstraintValidatorContext context) {
return false;
}

if (id.length() > Constants.ID_LENGTH_LIMIT) {
ValidationLimits limits =
validationLimits != null ? validationLimits : ValidationLimits.DEFAULTS;
int idLimit = limits.getIdLengthLimit();
if (id.length() > idLimit) {
context
.buildConstraintViolationWithTemplate(
String.format(
"[maestro id] cannot be more than id length limit %s - rejected length is [%s] for value [%s]",
Constants.ID_LENGTH_LIMIT, id.length(), id))
idLimit, id.length(), id))
.addConstraintViolation();
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@

import com.netflix.maestro.models.Constants;
import com.netflix.maestro.utils.StringUtils;
import com.netflix.maestro.utils.ValidationLimits;
import jakarta.inject.Inject;
import jakarta.validation.Constraint;
import jakarta.validation.ConstraintValidator;
import jakarta.validation.ConstraintValidatorContext;
Expand Down Expand Up @@ -41,35 +43,65 @@
/** input constraint payload. */
Class<? extends Payload>[] payload() default {};

/**
* Whether the name field is required. When false, null or blank values are treated as absent and
* pass validation. Defaults to true.
*/
boolean required() default true;

/**
* Whether to enforce the regex pattern and reserved prefix/suffix checks. When false, only the
* length limit is enforced. Defaults to true.
*/
boolean enforcePattern() default true;

/** Maestro id/name validator. */
class MaestroNameValidator implements ConstraintValidator<MaestroNameConstraint, String> {
private static final Pattern NAME_PATTERN =
Pattern.compile("[\\w \\.,:@&='(){}$+\\[\\]\\-\\/\\\\]+");

@Inject private ValidationLimits validationLimits;

private boolean required;
private boolean enforcePattern;

@Override
public void initialize(MaestroNameConstraint constraint) {}
public void initialize(MaestroNameConstraint constraint) {
this.required = constraint.required();
this.enforcePattern = constraint.enforcePattern();
}

@Override
public boolean isValid(String id, ConstraintValidatorContext context) {
if (id == null || StringUtils.checkTrimEmpty(id)) {
if (!required) {
return true;
}
context
.buildConstraintViolationWithTemplate(
"[maestro name] cannot be null or empty or blank spaces")
.addConstraintViolation();
return false;
}

if (id.length() > Constants.NAME_LENGTH_LIMIT) {
ValidationLimits limits =
validationLimits != null ? validationLimits : ValidationLimits.DEFAULTS;
int nameLimit = limits.getNameLengthLimit();
if (id.length() > nameLimit) {
context
.buildConstraintViolationWithTemplate(
String.format(
"[maestro name] cannot be more than name length limit %s "
+ "- rejected length is [%s] for value [%s]",
Constants.NAME_LENGTH_LIMIT, id.length(), id))
nameLimit, id.length(), id))
.addConstraintViolation();
return false;
}

if (!enforcePattern) {
return true;
}

if (!NAME_PATTERN.matcher(id).matches()) {
context
.buildConstraintViolationWithTemplate(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
package com.netflix.maestro.validations;

import com.netflix.maestro.models.Constants;
import com.netflix.maestro.utils.ValidationLimits;
import jakarta.inject.Inject;
import jakarta.validation.Constraint;
import jakarta.validation.ConstraintValidator;
import jakarta.validation.ConstraintValidatorContext;
Expand Down Expand Up @@ -45,6 +47,8 @@ class MaestroIdValidator implements ConstraintValidator<MaestroReferenceIdConstr
private static final Pattern ID_PATTERN = Pattern.compile("[_a-zA-Z][.\\-_a-zA-Z0-9]*+");
private static final String REJECTED_VALUE = "- rejected value is [%s]";

@Inject private ValidationLimits validationLimits;

@Override
public void initialize(MaestroReferenceIdConstraint constraint) {}

Expand All @@ -58,13 +62,16 @@ public boolean isValid(String id, ConstraintValidatorContext context) {
return false;
}

if (id.length() > Constants.ID_LENGTH_LIMIT) {
ValidationLimits limits =
validationLimits != null ? validationLimits : ValidationLimits.DEFAULTS;
int idLimit = limits.getIdLengthLimit();
if (id.length() > idLimit) {
context
.buildConstraintViolationWithTemplate(
String.format(
"[maestro id or name reference] cannot be more than id length limit %s "
+ "- rejected length is [%s] for value [%s]",
Constants.ID_LENGTH_LIMIT, id.length(), id))
idLimit, id.length(), id))
.addConstraintViolation();
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,14 @@ public void isIdTooLong() {
new TestId(new String(new char[Constants.ID_LENGTH_LIMIT + 1]).replace("\0", "a")));
assertEquals(1, violations.size());
ConstraintViolation<TestId> violation = violations.iterator().next();
assertEquals(129, ((String) violation.getInvalidValue()).length());
assertEquals(Constants.ID_LENGTH_LIMIT + 1, ((String) violation.getInvalidValue()).length());
assertEquals(
String.format(
"[maestro id] cannot be more than id length limit 128 "
"[maestro id] cannot be more than id length limit %s "
+ "- rejected length is [%s] for value [%s]",
129, new String(new char[Constants.ID_LENGTH_LIMIT + 1]).replace("\0", "a")),
Constants.ID_LENGTH_LIMIT,
Constants.ID_LENGTH_LIMIT + 1,
new String(new char[Constants.ID_LENGTH_LIMIT + 1]).replace("\0", "a")),
violation.getMessage());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,15 @@ private static class TestName {
}
}

private static class TestOptionalName {
@MaestroNameConstraint(required = false, enforcePattern = false)
String name;

TestOptionalName(String name) {
this.name = name;
}
}

@Test
public void isValidName() {
Set<ConstraintViolation<TestName>> violations =
Expand Down Expand Up @@ -86,12 +95,14 @@ public void isNameTooLong() {
new TestName(new String(new char[Constants.NAME_LENGTH_LIMIT + 1]).replace("\0", "a")));
assertEquals(1, violations.size());
ConstraintViolation<TestName> violation = violations.iterator().next();
assertEquals(257, ((String) violation.getInvalidValue()).length());
assertEquals(Constants.NAME_LENGTH_LIMIT + 1, ((String) violation.getInvalidValue()).length());
assertEquals(
String.format(
"[maestro name] cannot be more than name length limit 256 "
"[maestro name] cannot be more than name length limit %s "
+ "- rejected length is [%s] for value [%s]",
257, new String(new char[Constants.NAME_LENGTH_LIMIT + 1]).replace("\0", "a")),
Constants.NAME_LENGTH_LIMIT,
Constants.NAME_LENGTH_LIMIT + 1,
new String(new char[Constants.NAME_LENGTH_LIMIT + 1]).replace("\0", "a")),
violation.getMessage());
}

Expand Down Expand Up @@ -126,6 +137,41 @@ public void isNameUsingReservedPrefix() {
violation.getMessage());
}

@Test
public void isOptionalNameInvalidCharsAllowed() {
Set<ConstraintViolation<TestOptionalName>> violations =
validator.validate(new TestOptionalName("~foo`bar"));
assertEquals(0, violations.size());
}

@Test
public void isOptionalNameReservedPrefixAllowed() {
Set<ConstraintViolation<TestOptionalName>> violations =
validator.validate(new TestOptionalName("maestro_foo"));
assertEquals(0, violations.size());
}

@Test
public void isOptionalNameNullAllowed() {
Set<ConstraintViolation<TestOptionalName>> violations =
validator.validate(new TestOptionalName(null));
assertEquals(0, violations.size());
}

@Test
public void isOptionalNameEmptyAllowed() {
Set<ConstraintViolation<TestOptionalName>> violations =
validator.validate(new TestOptionalName(""));
assertEquals(0, violations.size());
}

@Test
public void isOptionalNameBlankAllowed() {
Set<ConstraintViolation<TestOptionalName>> violations =
validator.validate(new TestOptionalName(" "));
assertEquals(0, violations.size());
}

@Test
public void isNameUsingReservedSuffix() {
Set<ConstraintViolation<TestName>> violations = validator.validate(new TestName("foo_maestro"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,12 +62,14 @@ public void isIdTooLong() {
new TestId(new String(new char[Constants.ID_LENGTH_LIMIT + 1]).replace("\0", "a")));
assertEquals(1, violations.size());
ConstraintViolation<TestId> violation = violations.iterator().next();
assertEquals(129, ((String) violation.getInvalidValue()).length());
assertEquals(Constants.ID_LENGTH_LIMIT + 1, ((String) violation.getInvalidValue()).length());
assertEquals(
String.format(
"[maestro id or name reference] cannot be more than id length limit 128 "
"[maestro id or name reference] cannot be more than id length limit %s "
+ "- rejected length is [%s] for value [%s]",
129, new String(new char[Constants.ID_LENGTH_LIMIT + 1]).replace("\0", "a")),
Constants.ID_LENGTH_LIMIT,
Constants.ID_LENGTH_LIMIT + 1,
new String(new char[Constants.ID_LENGTH_LIMIT + 1]).replace("\0", "a")),
violation.getMessage());
}

Expand Down
Loading
Loading