diff --git a/dd-smoke-tests/openfeature/src/test/groovy/datadog/smoketest/springboot/OpenFeatureProviderSmokeTest.groovy b/dd-smoke-tests/openfeature/src/test/groovy/datadog/smoketest/springboot/OpenFeatureProviderSmokeTest.groovy index ca80ca47f8b..8b1839f05aa 100644 --- a/dd-smoke-tests/openfeature/src/test/groovy/datadog/smoketest/springboot/OpenFeatureProviderSmokeTest.groovy +++ b/dd-smoke-tests/openfeature/src/test/groovy/datadog/smoketest/springboot/OpenFeatureProviderSmokeTest.groovy @@ -206,7 +206,10 @@ class OpenFeatureProviderSmokeTest extends AbstractServerSmokeTest { private static Map buildLoggedAllocations(final Map config) { final logged = [:] (config.flags as Map).each { flag, definition -> - (definition.allocations ?: []).each { allocation -> + if (!(definition instanceof Map) || !(definition.allocations instanceof List)) { + return + } + definition.allocations.findAll { it instanceof Map }.each { allocation -> logged["${flag}\u0000${allocation.key}"] = allocation.doLog == true } } diff --git a/dd-smoke-tests/openfeature/src/test/resources/ffe-system-test-data b/dd-smoke-tests/openfeature/src/test/resources/ffe-system-test-data index b42c4a104ea..ea8b5cc5ce3 160000 --- a/dd-smoke-tests/openfeature/src/test/resources/ffe-system-test-data +++ b/dd-smoke-tests/openfeature/src/test/resources/ffe-system-test-data @@ -1 +1 @@ -Subproject commit b42c4a104ea70695c8fc3516780951f7434b7906 +Subproject commit ea8b5cc5ce335109f11f3efbc5fd608f98a3ca54 diff --git a/products/feature-flagging/feature-flagging-api/build.gradle.kts b/products/feature-flagging/feature-flagging-api/build.gradle.kts index 88531ceaeed..edf5b452c3d 100644 --- a/products/feature-flagging/feature-flagging-api/build.gradle.kts +++ b/products/feature-flagging/feature-flagging-api/build.gradle.kts @@ -49,6 +49,7 @@ dependencies { compileOnly("io.opentelemetry:opentelemetry-api:1.47.0") testImplementation(project(":products:feature-flagging:feature-flagging-bootstrap")) + testImplementation(project(":products:feature-flagging:feature-flagging-lib")) testImplementation(project(":utils:config-utils")) testImplementation("io.opentelemetry:opentelemetry-api:1.47.0") testImplementation(libs.bundles.junit5) diff --git a/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/DDEvaluator.java b/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/DDEvaluator.java index fe47bbab16f..7b298adc686 100644 --- a/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/DDEvaluator.java +++ b/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/DDEvaluator.java @@ -9,7 +9,10 @@ import datadog.trace.api.featureflag.ufc.v1.ConditionConfiguration; import datadog.trace.api.featureflag.ufc.v1.ConditionOperator; import datadog.trace.api.featureflag.ufc.v1.Flag; +import datadog.trace.api.featureflag.ufc.v1.FlagMap; +import datadog.trace.api.featureflag.ufc.v1.FlagValidator; import datadog.trace.api.featureflag.ufc.v1.Rule; +import datadog.trace.api.featureflag.ufc.v1.SemanticVersion; import datadog.trace.api.featureflag.ufc.v1.ServerConfiguration; import datadog.trace.api.featureflag.ufc.v1.Shard; import datadog.trace.api.featureflag.ufc.v1.ShardRange; @@ -111,6 +114,10 @@ public ProviderEvaluation evaluate( final Flag flag = config.flags.get(key); if (flag == null) { + if (config.flags instanceof FlagMap && ((FlagMap) config.flags).isRejected(key)) { + return error( + defaultValue, ErrorCode.PARSE_ERROR, "Invalid configuration for flag " + key); + } return error(defaultValue, ErrorCode.FLAG_NOT_FOUND); } @@ -275,6 +282,13 @@ private static boolean evaluateCondition( return compareNumber(attributeValue, condition.value, (a, b) -> a <= b); case LT: return compareNumber(attributeValue, condition.value, (a, b) -> a < b); + case SEMVER_EQ: + case SEMVER_NEQ: + case SEMVER_LT: + case SEMVER_LTE: + case SEMVER_GT: + case SEMVER_GTE: + return matchesSemanticVersion(condition.operator, attributeValue, condition.value); default: return false; } @@ -283,7 +297,7 @@ private static boolean evaluateCondition( private static boolean matchesRegex(final Object attributeValue, final Object conditionValue) { // PatternSyntaxException is intentionally not caught here so it propagates to evaluate(), // which maps it to ErrorCode.PARSE_ERROR. - final Pattern pattern = Pattern.compile(String.valueOf(conditionValue)); + final Pattern pattern = FlagValidator.compileRegex(String.valueOf(conditionValue)); return pattern.matcher(String.valueOf(attributeValue)).find(); } @@ -318,6 +332,33 @@ private static boolean compareNumber( return comparator.compare(a, b); } + private static boolean matchesSemanticVersion( + final ConditionOperator operator, final Object attributeValue, final Object conditionValue) { + final int comparison; + try { + comparison = + SemanticVersion.parse(attributeValue).compareTo(SemanticVersion.parse(conditionValue)); + } catch (IllegalArgumentException ignored) { + return false; + } + switch (operator) { + case SEMVER_EQ: + return comparison == 0; + case SEMVER_NEQ: + return comparison != 0; + case SEMVER_LT: + return comparison < 0; + case SEMVER_LTE: + return comparison <= 0; + case SEMVER_GT: + return comparison > 0; + case SEMVER_GTE: + return comparison >= 0; + default: + return false; + } + } + private static boolean matchesShard(final Shard shard, final String targetingKey) { final int assignedShard = getShard(shard.salt, targetingKey, shard.totalShards); for (final ShardRange range : shard.ranges) { @@ -417,10 +458,7 @@ private static ProviderEvaluation resolveVariant( final ProviderEvaluation result = ProviderEvaluation.builder() .value(mappedValue) - .reason( - !isEmpty(allocation.rules) - ? Reason.TARGETING_MATCH.name() - : !isEmpty(split.shards) ? Reason.SPLIT.name() : Reason.STATIC.name()) + .reason(assignmentReason(allocation, split)) .variant(variant.key) .flagMetadata(metadataBuilder.build()) .build(); @@ -431,6 +469,20 @@ private static ProviderEvaluation resolveVariant( return result; } + private static String assignmentReason(final Allocation allocation, final Split split) { + if (!isEmpty(allocation.rules)) { + return Reason.TARGETING_MATCH.name(); + } + if ((allocation.startAtInstant() != null || allocation.endAtInstant() != null) + && allocation.splits.size() == 1 + && isEmpty(split.shards)) { + return Reason.DEFAULT.name(); + } + return allocation.splits.size() == 1 && isEmpty(split.shards) + ? Reason.STATIC.name() + : Reason.SPLIT.name(); + } + private static Object resolveAttribute(final String name, final EvaluationContext context) { // Special handling for "id" attribute: if not explicitly provided, use targeting key if ("id".equals(name) && !context.keySet().contains(name)) { diff --git a/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/DDEvaluatorTest.java b/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/DDEvaluatorTest.java index 155c5eb1ea8..8ac3a9874bb 100644 --- a/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/DDEvaluatorTest.java +++ b/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/DDEvaluatorTest.java @@ -17,15 +17,15 @@ import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import com.datadog.featureflag.UniversalFlagConfigParser; import com.squareup.moshi.JsonAdapter; import com.squareup.moshi.JsonDataException; -import com.squareup.moshi.JsonReader; -import com.squareup.moshi.JsonWriter; import com.squareup.moshi.Moshi; import com.squareup.moshi.Types; import datadog.trace.api.featureflag.FeatureFlaggingGateway; import datadog.trace.api.featureflag.ufc.v1.Allocation; import datadog.trace.api.featureflag.ufc.v1.Flag; +import datadog.trace.api.featureflag.ufc.v1.FlagMap; import datadog.trace.api.featureflag.ufc.v1.ServerConfiguration; import dev.openfeature.sdk.ErrorCode; import dev.openfeature.sdk.EvaluationContext; @@ -39,7 +39,6 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.time.Instant; -import java.time.format.DateTimeFormatter; import java.util.ArrayList; import java.util.Date; import java.util.HashMap; @@ -59,9 +58,7 @@ public class DDEvaluatorTest { private static final String CANONICAL_FIXTURE_PATH = "dd-smoke-tests/openfeature/src/test/resources/ffe-system-test-data"; - private static final Moshi MOSHI = new Moshi.Builder().add(Date.class, new DateAdapter()).build(); - private static final JsonAdapter CONFIG_ADAPTER = - MOSHI.adapter(ServerConfiguration.class); + private static final Moshi MOSHI = new Moshi.Builder().build(); private static final Type FIXTURE_LIST_TYPE = Types.newParameterizedType(List.class, FixtureCase.class); private static final JsonAdapter> FIXTURE_LIST_ADAPTER = @@ -214,6 +211,24 @@ public void testNoAllocations() { assertThat(details.getErrorCode(), nullValue()); } + @Test + public void testRejectedFlagsAreScopedToCurrentConfiguration() { + final FlagMap rejectedFlags = new FlagMap(); + rejectedFlags.reject("target"); + final DDEvaluator evaluator = new DDEvaluator(mock(Runnable.class)); + final EvaluationContext context = new MutableContext("target"); + + evaluator.accept(new ServerConfiguration("", "", null, rejectedFlags)); + ProviderEvaluation details = evaluator.evaluate(String.class, "target", "default", context); + assertThat(details.getReason(), equalTo(ERROR.name())); + assertThat(details.getErrorCode(), equalTo(ErrorCode.PARSE_ERROR)); + + evaluator.accept(new ServerConfiguration("", "", null, new FlagMap())); + details = evaluator.evaluate(String.class, "target", "default", context); + assertThat(details.getReason(), equalTo(ERROR.name())); + assertThat(details.getErrorCode(), equalTo(ErrorCode.FLAG_NOT_FOUND)); + } + @Test public void testAllocationDateAbiAndInstantAccessors() throws Exception { final Date startAt = Date.from(Instant.parse("2024-01-01T00:00:00Z")); @@ -319,7 +334,8 @@ private static ProviderEvaluation evaluate( } private static ServerConfiguration loadCanonicalConfiguration() throws IOException { - return CONFIG_ADAPTER.fromJson(read(fixtureRoot().resolve("ufc-config.json"))); + return UniversalFlagConfigParser.INSTANCE.deserialize( + read(fixtureRoot().resolve("ufc-config.json")).getBytes(StandardCharsets.UTF_8)); } private static List canonicalTestCases() throws IOException { @@ -433,28 +449,4 @@ private static final class FixtureResult { String variant; Map flagMetadata = emptyMap(); } - - private static final class DateAdapter extends JsonAdapter { - @Override - public Date fromJson(final JsonReader reader) throws IOException { - if (reader.peek() == JsonReader.Token.NULL) { - return reader.nextNull(); - } - try { - return Date.from( - DateTimeFormatter.ISO_OFFSET_DATE_TIME.parse(reader.nextString(), Instant::from)); - } catch (final Exception ignored) { - return null; - } - } - - @Override - public void toJson(final JsonWriter writer, final Date value) throws IOException { - if (value == null) { - writer.nullValue(); - return; - } - writer.value(value.toInstant().toString()); - } - } } diff --git a/products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/ufc/v1/ConditionOperator.java b/products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/ufc/v1/ConditionOperator.java index 088f934b126..1e864da2b6a 100644 --- a/products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/ufc/v1/ConditionOperator.java +++ b/products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/ufc/v1/ConditionOperator.java @@ -9,5 +9,11 @@ public enum ConditionOperator { NOT_MATCHES, ONE_OF, NOT_ONE_OF, - IS_NULL + IS_NULL, + SEMVER_EQ, + SEMVER_NEQ, + SEMVER_LT, + SEMVER_LTE, + SEMVER_GT, + SEMVER_GTE } diff --git a/products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/ufc/v1/FlagMap.java b/products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/ufc/v1/FlagMap.java new file mode 100644 index 00000000000..6def3101acb --- /dev/null +++ b/products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/ufc/v1/FlagMap.java @@ -0,0 +1,17 @@ +package datadog.trace.api.featureflag.ufc.v1; + +import java.util.HashMap; +import java.util.HashSet; +import java.util.Set; + +public final class FlagMap extends HashMap { + private final Set rejected = new HashSet<>(); + + public void reject(final String key) { + rejected.add(key); + } + + public boolean isRejected(final String key) { + return rejected.contains(key); + } +} diff --git a/products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/ufc/v1/FlagValidator.java b/products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/ufc/v1/FlagValidator.java new file mode 100644 index 00000000000..52879f2ebbe --- /dev/null +++ b/products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/ufc/v1/FlagValidator.java @@ -0,0 +1,203 @@ +package datadog.trace.api.featureflag.ufc.v1; + +import java.util.List; +import java.util.Map; +import java.util.regex.Pattern; + +public final class FlagValidator { + private static final String MAX_UNSIGNED_LONG = "18446744073709551615"; + + public static void validateJson(final Object rawFlag) { + if (!(rawFlag instanceof Map)) { + return; + } + final Object allocations = ((Map) rawFlag).get("allocations"); + if (!(allocations instanceof List)) { + return; + } + for (final Object allocation : (List) allocations) { + if (!(allocation instanceof Map)) { + continue; + } + final Object splits = ((Map) allocation).get("splits"); + if (!(splits instanceof List)) { + continue; + } + for (final Object split : (List) splits) { + validateJsonSplit(split); + } + } + } + + public static void validate(final Flag flag) { + if (flag == null || flag.variationType == null || flag.variations == null) { + throw new IllegalArgumentException("flag is incomplete"); + } + for (final Map.Entry entry : flag.variations.entrySet()) { + final Variant variant = entry.getValue(); + if (variant == null || !matchesType(variant.value, flag.variationType)) { + throw new IllegalArgumentException("variation has an invalid value: " + entry.getKey()); + } + } + if (flag.allocations == null) { + return; + } + for (final Allocation allocation : flag.allocations) { + if (allocation == null || allocation.splits == null) { + throw new IllegalArgumentException("allocation is incomplete"); + } + for (final Split split : allocation.splits) { + if (split == null + || split.shards == null + || !flag.variations.containsKey(split.variationKey)) { + throw new IllegalArgumentException("split is incomplete"); + } + for (final Shard shard : split.shards) { + if (shard == null || shard.totalShards <= 0 || shard.ranges == null) { + throw new IllegalArgumentException("shard is incomplete"); + } + for (final ShardRange range : shard.ranges) { + if (range == null + || range.start < 0 + || range.start >= range.end + || range.end > shard.totalShards) { + throw new IllegalArgumentException("shard range is invalid"); + } + } + } + } + if (allocation.rules == null) { + continue; + } + for (final Rule rule : allocation.rules) { + if (rule == null || rule.conditions == null) { + throw new IllegalArgumentException("rule is incomplete"); + } + for (final ConditionConfiguration condition : rule.conditions) { + validateCondition(condition); + } + } + } + } + + private static void validateCondition(final ConditionConfiguration condition) { + if (condition == null || condition.operator == null) { + throw new IllegalArgumentException("condition is incomplete"); + } + switch (condition.operator) { + case MATCHES: + case NOT_MATCHES: + if (!(condition.value instanceof String)) { + throw new IllegalArgumentException("regex comparand must be a string"); + } + compileRegex((String) condition.value); + return; + case LT: + case LTE: + case GT: + case GTE: + if (!(condition.value instanceof Number)) { + throw new IllegalArgumentException("numeric comparand must be a number"); + } + return; + case ONE_OF: + case NOT_ONE_OF: + if (!(condition.value instanceof List)) { + throw new IllegalArgumentException("membership comparand must be a list"); + } + for (final Object value : (List) condition.value) { + if (!(value instanceof String)) { + throw new IllegalArgumentException("membership comparand must contain strings"); + } + } + return; + case IS_NULL: + if (!(condition.value instanceof Boolean)) { + throw new IllegalArgumentException("null comparand must be boolean"); + } + return; + case SEMVER_EQ: + case SEMVER_NEQ: + case SEMVER_LT: + case SEMVER_LTE: + case SEMVER_GT: + case SEMVER_GTE: + SemanticVersion.parse(condition.value); + return; + default: + throw new IllegalArgumentException("unknown condition operator"); + } + } + + private static boolean matchesType(final Object value, final ValueType type) { + switch (type) { + case BOOLEAN: + return value instanceof Boolean; + case STRING: + return value instanceof String; + case INTEGER: + return value instanceof Number && ((Number) value).doubleValue() % 1 == 0; + case NUMERIC: + return value instanceof Number; + case JSON: + return true; + default: + return false; + } + } + + public static Pattern compileRegex(final String expression) { + return Pattern.compile(expression.replace("[:alnum:]", "\\p{Alnum}")); + } + + private static void validateJsonSplit(final Object rawSplit) { + if (!(rawSplit instanceof Map)) { + return; + } + final Object shards = ((Map) rawSplit).get("shards"); + if (!(shards instanceof List)) { + return; + } + for (final Object shard : (List) shards) { + if (!(shard instanceof Map)) { + continue; + } + final Map shardMap = (Map) shard; + validateJavaInteger(shardMap.get("totalShards"), true, "totalShards"); + final Object ranges = shardMap.get("ranges"); + if (!(ranges instanceof List)) { + continue; + } + for (final Object range : (List) ranges) { + if (range instanceof Map) { + validateJavaInteger(((Map) range).get("start"), false, "range start"); + validateJavaInteger(((Map) range).get("end"), false, "range end"); + } + } + } + } + + private static void validateJavaInteger( + final Object value, final boolean positive, final String field) { + if (!(value instanceof Number)) { + return; + } + final double number = ((Number) value).doubleValue(); + if (!Double.isFinite(number) + || number != Math.rint(number) + || positive && number <= 0 + || !positive && number < 0 + || number > Integer.MAX_VALUE) { + throw new IllegalArgumentException(field + " is outside the supported integer range"); + } + } + + static void validateSemanticVersionComponent(final String value) { + if (value.length() > MAX_UNSIGNED_LONG.length() + || value.length() == MAX_UNSIGNED_LONG.length() && value.compareTo(MAX_UNSIGNED_LONG) > 0) { + throw new IllegalArgumentException("semantic version component is too large"); + } + } + + private FlagValidator() {} +} diff --git a/products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/ufc/v1/SemanticVersion.java b/products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/ufc/v1/SemanticVersion.java new file mode 100644 index 00000000000..1e9793407a6 --- /dev/null +++ b/products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/ufc/v1/SemanticVersion.java @@ -0,0 +1,92 @@ +package datadog.trace.api.featureflag.ufc.v1; + +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +public final class SemanticVersion implements Comparable { + private static final Pattern PATTERN = + Pattern.compile( + "^(0|[1-9][0-9]*)\\.(0|[1-9][0-9]*)\\.(0|[1-9][0-9]*)(?:-((?:0|[1-9][0-9]*|[0-9]*[A-Za-z-][0-9A-Za-z-]*)(?:\\.(?:0|[1-9][0-9]*|[0-9]*[A-Za-z-][0-9A-Za-z-]*))*))?(?:\\+[0-9A-Za-z-]+(?:\\.[0-9A-Za-z-]+)*)?$"); + + private final String major; + private final String minor; + private final String patch; + private final String[] prerelease; + + private SemanticVersion( + final String major, final String minor, final String patch, final String prerelease) { + this.major = major; + this.minor = minor; + this.patch = patch; + this.prerelease = prerelease == null ? null : prerelease.split("\\."); + } + + public static SemanticVersion parse(final Object value) { + if (!(value instanceof String)) { + throw new IllegalArgumentException("semantic version must be a string"); + } + final Matcher matcher = PATTERN.matcher((String) value); + if (!matcher.matches()) { + throw new IllegalArgumentException("invalid semantic version: " + value); + } + FlagValidator.validateSemanticVersionComponent(matcher.group(1)); + FlagValidator.validateSemanticVersionComponent(matcher.group(2)); + FlagValidator.validateSemanticVersionComponent(matcher.group(3)); + return new SemanticVersion( + matcher.group(1), matcher.group(2), matcher.group(3), matcher.group(4)); + } + + @Override + public int compareTo(final SemanticVersion other) { + int result = compareNumeric(major, other.major); + if (result == 0) { + result = compareNumeric(minor, other.minor); + } + if (result == 0) { + result = compareNumeric(patch, other.patch); + } + if (result != 0 || prerelease == null && other.prerelease == null) { + return result; + } + if (prerelease == null) { + return 1; + } + if (other.prerelease == null) { + return -1; + } + final int count = Math.min(prerelease.length, other.prerelease.length); + for (int index = 0; index < count; index++) { + result = compareIdentifier(prerelease[index], other.prerelease[index]); + if (result != 0) { + return result; + } + } + return Integer.compare(prerelease.length, other.prerelease.length); + } + + private static int compareIdentifier(final String left, final String right) { + final boolean leftNumeric = isNumeric(left); + final boolean rightNumeric = isNumeric(right); + if (leftNumeric && rightNumeric) { + return compareNumeric(left, right); + } + if (leftNumeric != rightNumeric) { + return leftNumeric ? -1 : 1; + } + return left.compareTo(right); + } + + private static int compareNumeric(final String left, final String right) { + final int length = Integer.compare(left.length(), right.length()); + return length == 0 ? left.compareTo(right) : length; + } + + private static boolean isNumeric(final String value) { + for (int index = 0; index < value.length(); index++) { + if (!Character.isDigit(value.charAt(index))) { + return false; + } + } + return true; + } +} diff --git a/products/feature-flagging/feature-flagging-bootstrap/src/test/java/datadog/trace/api/featureflag/ufc/v1/SemanticVersionTest.java b/products/feature-flagging/feature-flagging-bootstrap/src/test/java/datadog/trace/api/featureflag/ufc/v1/SemanticVersionTest.java new file mode 100644 index 00000000000..5727947ed39 --- /dev/null +++ b/products/feature-flagging/feature-flagging-bootstrap/src/test/java/datadog/trace/api/featureflag/ufc/v1/SemanticVersionTest.java @@ -0,0 +1,38 @@ +package datadog.trace.api.featureflag.ufc.v1; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import org.junit.jupiter.api.Test; + +class SemanticVersionTest { + + @Test + void ignoresBuildMetadataForPrecedence() { + assertEquals( + 0, + SemanticVersion.parse("4.5.6-rc.1+build.42") + .compareTo(SemanticVersion.parse("4.5.6-rc.1"))); + } + + @Test + void comparesPrereleaseIdentifiersUsingSemanticVersionPrecedence() { + assertEquals( + -1, + Integer.signum( + SemanticVersion.parse("1.2.3-alpha.4") + .compareTo(SemanticVersion.parse("1.2.3-alpha.5")))); + assertEquals( + -1, + Integer.signum( + SemanticVersion.parse("1.2.3-alpha.5").compareTo(SemanticVersion.parse("1.2.3")))); + } + + @Test + void acceptsCommonMaximumAndRejectsOverflowingCoreComponent() { + SemanticVersion.parse("9007199254740991.9007199254740991.9007199254740991"); + + assertThrows( + IllegalArgumentException.class, () -> SemanticVersion.parse("18446744073709551616.0.0")); + } +} diff --git a/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/UniversalFlagConfigParser.java b/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/UniversalFlagConfigParser.java index 8951f20f866..2ccda0ff355 100644 --- a/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/UniversalFlagConfigParser.java +++ b/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/UniversalFlagConfigParser.java @@ -9,6 +9,8 @@ import datadog.remoteconfig.ConfigurationDeserializer; import datadog.trace.api.featureflag.ufc.v1.Allocation; import datadog.trace.api.featureflag.ufc.v1.Flag; +import datadog.trace.api.featureflag.ufc.v1.FlagMap; +import datadog.trace.api.featureflag.ufc.v1.FlagValidator; import datadog.trace.api.featureflag.ufc.v1.Rule; import datadog.trace.api.featureflag.ufc.v1.ServerConfiguration; import datadog.trace.api.featureflag.ufc.v1.Split; @@ -18,7 +20,6 @@ import java.lang.reflect.Type; import java.time.Instant; import java.time.format.DateTimeFormatter; -import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Set; @@ -29,11 +30,12 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -final class UniversalFlagConfigParser implements ConfigurationDeserializer { +public final class UniversalFlagConfigParser + implements ConfigurationDeserializer { private static final Logger LOGGER = LoggerFactory.getLogger(UniversalFlagConfigParser.class); - static final UniversalFlagConfigParser INSTANCE = new UniversalFlagConfigParser(); + public static final UniversalFlagConfigParser INSTANCE = new UniversalFlagConfigParser(); private static final Moshi MOSHI = new Moshi.Builder() @@ -98,17 +100,22 @@ public Map fromJson(@Nonnull final JsonReader reader) throws IOExc if (reader.peek() == JsonReader.Token.NULL) { return reader.nextNull(); } - final Map flags = new HashMap<>(); + final FlagMap flags = new FlagMap(); reader.beginObject(); while (reader.hasNext()) { final String flagKey = reader.nextName(); final Object rawFlag = reader.readJsonValue(); try { + FlagValidator.validateJson(rawFlag); final Flag flag = flagAdapter.fromJsonValue(rawFlag); if (flag != null) { + FlagValidator.validate(flag); flags.put(flagKey, flag); + } else { + flags.reject(flagKey); } } catch (JsonDataException | IllegalArgumentException error) { + flags.reject(flagKey); LOGGER.warn( "Dropping malformed FFE flag {} during remote config deserialization: {}", flagKey, diff --git a/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/RemoteConfigServiceImplTest.java b/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/RemoteConfigServiceImplTest.java index 6d14a28f796..2c1e19dfe49 100644 --- a/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/RemoteConfigServiceImplTest.java +++ b/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/RemoteConfigServiceImplTest.java @@ -31,6 +31,7 @@ import datadog.trace.api.featureflag.FeatureFlaggingGateway; import datadog.trace.api.featureflag.ufc.v1.Allocation; import datadog.trace.api.featureflag.ufc.v1.Flag; +import datadog.trace.api.featureflag.ufc.v1.FlagMap; import datadog.trace.api.featureflag.ufc.v1.ServerConfiguration; import java.io.IOException; import java.lang.annotation.Annotation; @@ -115,6 +116,7 @@ void skipsMalformedFlagAllocationsAndKeepsValidFlag() throws Exception { assertNotNull(config); assertFalse(config.flags.containsKey("malformed-flag")); + assertTrue(((FlagMap) config.flags).isRejected("malformed-flag")); assertTrue(config.flags.containsKey("valid-flag")); assertEquals("expected", config.flags.get("valid-flag").variations.get("expected").value); } @@ -221,6 +223,7 @@ void skipsUnknownOperatorFlagAndKeepsValidFlag() throws Exception { assertNotNull(config); assertFalse(config.flags.containsKey("operator-grease-flag")); + assertTrue(((FlagMap) config.flags).isRejected("operator-grease-flag")); assertTrue(config.flags.containsKey("valid-flag")); assertEquals("expected", config.flags.get("valid-flag").variations.get("expected").value); } @@ -284,6 +287,7 @@ void skipsNullFlagAndKeepsValidFlag() throws Exception { assertNotNull(config); assertFalse(config.flags.containsKey("null-flag")); + assertTrue(((FlagMap) config.flags).isRejected("null-flag")); assertTrue(config.flags.containsKey("valid-flag")); assertEquals("expected", config.flags.get("valid-flag").variations.get("expected").value); }