From 191d1699a1d3ad31d90cc9ac74343b205a217043 Mon Sep 17 00:00:00 2001 From: Valerij Fredriksen Date: Wed, 22 Sep 2021 15:24:27 +0200 Subject: Add getAndSet to MutableBoolean --- vespajlib/src/main/java/com/yahoo/lang/MutableBoolean.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/vespajlib/src/main/java/com/yahoo/lang/MutableBoolean.java b/vespajlib/src/main/java/com/yahoo/lang/MutableBoolean.java index 17501b17bd0..877620547ba 100644 --- a/vespajlib/src/main/java/com/yahoo/lang/MutableBoolean.java +++ b/vespajlib/src/main/java/com/yahoo/lang/MutableBoolean.java @@ -22,6 +22,12 @@ public class MutableBoolean { public void orSet(boolean value) { this.value |= value; } + public boolean getAndSet(boolean newValue) { + boolean prev = value; + value = newValue; + return prev; + } + @Override public String toString() { return Boolean.toString(value); } -- cgit v1.2.3 From 916787e32fa8a99b8a13110e0cb730ecda195117 Mon Sep 17 00:00:00 2001 From: Valerij Fredriksen Date: Wed, 22 Sep 2021 15:32:51 +0200 Subject: Use collection factory methods --- flags/src/main/java/com/yahoo/vespa/flags/FetchVector.java | 8 +++----- flags/src/main/java/com/yahoo/vespa/flags/FlagDefinition.java | 4 +--- flags/src/main/java/com/yahoo/vespa/flags/json/FlagData.java | 11 ++++------- flags/src/main/java/com/yahoo/vespa/flags/json/Rule.java | 5 ++--- 4 files changed, 10 insertions(+), 18 deletions(-) diff --git a/flags/src/main/java/com/yahoo/vespa/flags/FetchVector.java b/flags/src/main/java/com/yahoo/vespa/flags/FetchVector.java index ede7bd6a109..178fdc90d20 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/FetchVector.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/FetchVector.java @@ -4,9 +4,7 @@ package com.yahoo.vespa.flags; import com.yahoo.vespa.flags.json.DimensionHelper; import javax.annotation.concurrent.Immutable; -import java.util.Collections; import java.util.EnumMap; -import java.util.HashMap; import java.util.Map; import java.util.Objects; import java.util.Optional; @@ -72,15 +70,15 @@ public class FetchVector { private final Map map; public FetchVector() { - this.map = Collections.emptyMap(); + this.map = Map.of(); } public static FetchVector fromMap(Map map) { - return new FetchVector(new HashMap<>(map)); + return new FetchVector(map); } private FetchVector(Map map) { - this.map = Collections.unmodifiableMap(map); + this.map = Map.copyOf(map); } public Optional getValue(Dimension dimension) { diff --git a/flags/src/main/java/com/yahoo/vespa/flags/FlagDefinition.java b/flags/src/main/java/com/yahoo/vespa/flags/FlagDefinition.java index d01ca64cb9f..90e0022c634 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/FlagDefinition.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/FlagDefinition.java @@ -3,8 +3,6 @@ package com.yahoo.vespa.flags; import javax.annotation.concurrent.Immutable; import java.time.Instant; -import java.util.Arrays; -import java.util.Collections; import java.util.List; /** @@ -35,7 +33,7 @@ public class FlagDefinition { this.expiresAt = expiresAt; this.description = description; this.modificationEffect = modificationEffect; - this.dimensions = Collections.unmodifiableList(Arrays.asList(dimensions)); + this.dimensions = List.of(dimensions); } public UnboundFlag getUnboundFlag() { diff --git a/flags/src/main/java/com/yahoo/vespa/flags/json/FlagData.java b/flags/src/main/java/com/yahoo/vespa/flags/json/FlagData.java index c4079380a8c..88da3e75a84 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/json/FlagData.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/json/FlagData.java @@ -14,9 +14,6 @@ import com.yahoo.vespa.flags.json.wire.WireRule; import javax.annotation.concurrent.Immutable; import java.io.InputStream; import java.io.OutputStream; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collections; import java.util.List; import java.util.Optional; import java.util.stream.Collectors; @@ -35,16 +32,16 @@ public class FlagData { private final FetchVector defaultFetchVector; public FlagData(FlagId id) { - this(id, new FetchVector(), Collections.emptyList()); + this(id, new FetchVector(), List.of()); } public FlagData(FlagId id, FetchVector defaultFetchVector, Rule... rules) { - this(id, defaultFetchVector, Arrays.asList(rules)); + this(id, defaultFetchVector, List.of(rules)); } public FlagData(FlagId id, FetchVector defaultFetchVector, List rules) { this.id = id; - this.rules = Collections.unmodifiableList(new ArrayList<>(rules)); + this.rules = List.copyOf(rules); this.defaultFetchVector = defaultFetchVector; } @@ -136,7 +133,7 @@ public class FlagData { } private static List rulesFromWire(List wireRules) { - if (wireRules == null) return Collections.emptyList(); + if (wireRules == null) return List.of(); return wireRules.stream().map(Rule::fromWire).collect(Collectors.toList()); } diff --git a/flags/src/main/java/com/yahoo/vespa/flags/json/Rule.java b/flags/src/main/java/com/yahoo/vespa/flags/json/Rule.java index b7d60889419..eae6952d329 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/json/Rule.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/json/Rule.java @@ -6,7 +6,6 @@ import com.yahoo.vespa.flags.JsonNodeRawFlag; import com.yahoo.vespa.flags.RawFlag; import com.yahoo.vespa.flags.json.wire.WireRule; -import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.Optional; @@ -20,11 +19,11 @@ public class Rule { private final Optional valueToApply; public Rule(Optional valueToApply, Condition... andConditions) { - this(valueToApply, Arrays.asList(andConditions)); + this(valueToApply, List.of(andConditions)); } public Rule(Optional valueToApply, List andConditions) { - this.andConditions = andConditions; + this.andConditions = List.copyOf(andConditions); this.valueToApply = valueToApply; } -- cgit v1.2.3 From f33a49faf68ea482d77536c3ff5a3af31a70fe94 Mon Sep 17 00:00:00 2001 From: Valerij Fredriksen Date: Wed, 22 Sep 2021 15:34:03 +0200 Subject: Create serializer for user flags --- .../restapi/user/UserFlagsSerializer.java | 86 +++++++++++++ .../restapi/user/UserFlagsSerializerTest.java | 133 +++++++++++++++++++++ .../java/com/yahoo/vespa/flags/FetchVector.java | 4 + .../java/com/yahoo/vespa/flags/json/Condition.java | 24 ++-- .../java/com/yahoo/vespa/flags/json/FlagData.java | 4 + .../com/yahoo/vespa/flags/json/ListCondition.java | 15 +++ .../vespa/flags/json/RelationalCondition.java | 15 +++ .../main/java/com/yahoo/vespa/flags/json/Rule.java | 14 +++ 8 files changed, 288 insertions(+), 7 deletions(-) create mode 100644 controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/user/UserFlagsSerializer.java create mode 100644 controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/UserFlagsSerializerTest.java diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/user/UserFlagsSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/user/UserFlagsSerializer.java new file mode 100644 index 00000000000..44d537883f9 --- /dev/null +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/user/UserFlagsSerializer.java @@ -0,0 +1,86 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.controller.restapi.user; + +import com.yahoo.config.provision.TenantName; +import com.yahoo.lang.MutableBoolean; +import com.yahoo.slime.Cursor; +import com.yahoo.slime.SlimeUtils; +import com.yahoo.vespa.flags.FetchVector; +import com.yahoo.vespa.flags.FlagDefinition; +import com.yahoo.vespa.flags.FlagId; +import com.yahoo.vespa.flags.Flags; +import com.yahoo.vespa.flags.RawFlag; +import com.yahoo.vespa.flags.UnboundFlag; +import com.yahoo.vespa.flags.json.Condition; +import com.yahoo.vespa.flags.json.FlagData; +import com.yahoo.vespa.flags.json.Rule; + +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.Set; +import java.util.function.Predicate; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +/** + * @author freva + */ +public class UserFlagsSerializer { + static void toSlime(Cursor cursor, Map rawFlagData, + Set authorizedForTenantNames, boolean isOperator, String userEmail) { + FetchVector resolveVector = FetchVector.fromMap(Map.of(FetchVector.Dimension.CONSOLE_USER_EMAIL, userEmail)); + List filteredFlagData = Flags.getAllFlags().stream() + // Only include flags that have CONSOLE_USER_EMAIL dimension, this should be replaced with more explicit + // 'target' annotation if/when that is added to flag definition + .filter(fd -> fd.getDimensions().contains(FetchVector.Dimension.CONSOLE_USER_EMAIL)) + .map(FlagDefinition::getUnboundFlag) + .map(flag -> filteredFlagData(flag, Optional.ofNullable(rawFlagData.get(flag.id())), authorizedForTenantNames, isOperator, resolveVector)) + .collect(Collectors.toUnmodifiableList()); + + byte[] bytes = FlagData.serializeListToUtf8Json(filteredFlagData); + SlimeUtils.copyObject(SlimeUtils.jsonToSlime(bytes).get(), cursor); + } + + private static FlagData filteredFlagData(UnboundFlag definition, Optional original, + Set authorizedForTenantNames, boolean isOperator, FetchVector resolveVector) { + MutableBoolean encounteredEmpty = new MutableBoolean(false); + Optional defaultValue = Optional.of(definition.serializer().serialize(definition.defaultValue())); + // Include the original rules from flag DB and the default value from code if there is no default rule in DB + List rules = Stream.concat(original.stream().flatMap(fd -> fd.rules().stream()), Stream.of(new Rule(defaultValue))) + // Exclude rules that do not match the resolveVector + .filter(rule -> rule.partialMatch(resolveVector)) + // Re-create each rule with value explicitly set, either from DB or default from code and + // a filtered set of conditions + .map(rule -> new Rule(rule.getValueToApply().or(() -> defaultValue), + rule.conditions().stream() + .flatMap(condition -> filteredCondition(condition, authorizedForTenantNames, isOperator, resolveVector).stream()) + .collect(Collectors.toUnmodifiableList()))) + // We can stop as soon as we hit the first rule that has no conditions + .takeWhile(rule -> !encounteredEmpty.getAndSet(rule.conditions().isEmpty())) + .collect(Collectors.toUnmodifiableList()); + + return new FlagData(definition.id(), new FetchVector(), rules); + } + + private static Optional filteredCondition(Condition condition, Set authorizedForTenantNames, + boolean isOperator, FetchVector resolveVector) { + // If the condition is one of the conditions that we resolve on the server, e.g. email, we do not need to + // propagate it back to the user + if (resolveVector.hasDimension(condition.dimension())) return Optional.empty(); + + // For the other dimensions, filter the values down to an allowed subset + switch (condition.dimension()) { + case TENANT_ID: return valueSubset(condition, tenant -> isOperator || authorizedForTenantNames.contains(TenantName.from(tenant))); + case APPLICATION_ID: return valueSubset(condition, appId -> isOperator || authorizedForTenantNames.stream().anyMatch(tenant -> appId.startsWith(tenant.value() + ":"))); + default: throw new IllegalArgumentException("Dimension " + condition.dimension() + " is not supported for user flags"); + } + } + + private static Optional valueSubset(Condition condition, Predicate predicate) { + Condition.CreateParams createParams = condition.toCreateParams(); + return Optional.of(createParams + .withValues(createParams.values().stream().filter(predicate).collect(Collectors.toUnmodifiableList())) + .createAs(condition.type())); + } +} diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/UserFlagsSerializerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/UserFlagsSerializerTest.java new file mode 100644 index 00000000000..8625628b74e --- /dev/null +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/UserFlagsSerializerTest.java @@ -0,0 +1,133 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.controller.restapi.user; + +import com.fasterxml.jackson.annotation.JsonIgnoreProperties; +import com.fasterxml.jackson.annotation.JsonProperty; +import com.yahoo.config.provision.TenantName; +import com.yahoo.slime.Slime; +import com.yahoo.slime.SlimeUtils; +import com.yahoo.test.json.JsonTestHelper; +import com.yahoo.vespa.flags.FetchVector; +import com.yahoo.vespa.flags.FlagId; +import com.yahoo.vespa.flags.Flags; +import com.yahoo.vespa.flags.JsonNodeRawFlag; +import com.yahoo.vespa.flags.json.Condition; +import com.yahoo.vespa.flags.json.FlagData; +import com.yahoo.vespa.flags.json.Rule; +import org.junit.Test; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Optional; +import java.util.Set; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +import static com.yahoo.vespa.flags.FetchVector.Dimension.APPLICATION_ID; +import static com.yahoo.vespa.flags.FetchVector.Dimension.CONSOLE_USER_EMAIL; +import static com.yahoo.vespa.flags.FetchVector.Dimension.TENANT_ID; + +/** + * @author freva + */ +public class UserFlagsSerializerTest { + + @Test + public void user_flag_test() throws IOException { + String email1 = "alice@domain.tld"; + String email2 = "bob@domain.tld"; + + try (Flags.Replacer ignored = Flags.clearFlagsForTesting()) { + Flags.defineStringFlag("string-id", "default value", List.of("owner"), "1970-01-01", "2100-01-01", "desc", "mod", CONSOLE_USER_EMAIL); + Flags.defineIntFlag("int-id", 123, List.of("owner"), "1970-01-01", "2100-01-01", "desc", "mod", CONSOLE_USER_EMAIL, TENANT_ID, APPLICATION_ID); + Flags.defineDoubleFlag("double-id", 3.14d, List.of("owner"), "1970-01-01", "2100-01-01", "desc", "mod"); + Flags.defineListFlag("list-id", List.of("a"), String.class, List.of("owner"), "1970-01-01", "2100-01-01", "desc", "mod", CONSOLE_USER_EMAIL); + Flags.defineJacksonFlag("jackson-id", new ExampleJacksonClass(123, "abc"), ExampleJacksonClass.class, + List.of("owner"), "1970-01-01", "2100-01-01", "desc", "mod", CONSOLE_USER_EMAIL, TENANT_ID); + + Map flagData = Stream.of( + flagData("string-id", rule("\"value1\"", condition(CONSOLE_USER_EMAIL, Condition.Type.WHITELIST, email1))), + flagData("int-id", rule("456")), + flagData("list-id", + rule("[\"value1\"]", condition(CONSOLE_USER_EMAIL, Condition.Type.WHITELIST, email1), condition(APPLICATION_ID, Condition.Type.BLACKLIST, "tenant1:video:default", "tenant1:video:default", "tenant2:music:default")), + rule("[\"value2\"]", condition(CONSOLE_USER_EMAIL, Condition.Type.WHITELIST, email2)), + rule("[\"value1\",\"value3\"]", condition(APPLICATION_ID, Condition.Type.BLACKLIST, "tenant1:video:default", "tenant1:video:default", "tenant2:music:default"))), + flagData("jackson-id", rule("{\"integer\":456,\"string\":\"xyz\"}", condition(CONSOLE_USER_EMAIL, Condition.Type.WHITELIST, email1), condition(TENANT_ID, Condition.Type.WHITELIST, "tenant1", "tenant3"))) + ).collect(Collectors.toMap(FlagData::id, fd -> fd)); + + // double-id is not here as it does not have CONSOLE_USER_EMAIL dimension + assertUserFlags("{\"flags\":[" + + "{\"id\":\"int-id\",\"rules\":[{\"value\":456}]}," + // Default from DB + "{\"id\":\"jackson-id\",\"rules\":[{\"conditions\":[{\"type\":\"whitelist\",\"dimension\":\"tenant\"}],\"value\":{\"integer\":456,\"string\":\"xyz\"}},{\"value\":{\"integer\":123,\"string\":\"abc\"}}]}," + // Resolved for email + // Resolved for email, but conditions are empty since this user is not authorized for any tenants + "{\"id\":\"list-id\",\"rules\":[{\"conditions\":[{\"type\":\"blacklist\",\"dimension\":\"application\"}],\"value\":[\"value1\"]},{\"conditions\":[{\"type\":\"blacklist\",\"dimension\":\"application\"}],\"value\":[\"value1\",\"value3\"]},{\"value\":[\"a\"]}]}," + + "{\"id\":\"string-id\",\"rules\":[{\"value\":\"value1\"}]}]}", // resolved for email + flagData, Set.of(), false, email1); + + // Same as the first one, but user is authorized for tenant1 + assertUserFlags("{\"flags\":[" + + "{\"id\":\"int-id\",\"rules\":[{\"value\":456}]}," + // Default from DB + "{\"id\":\"jackson-id\",\"rules\":[{\"conditions\":[{\"type\":\"whitelist\",\"dimension\":\"tenant\",\"values\":[\"tenant1\"]}],\"value\":{\"integer\":456,\"string\":\"xyz\"}},{\"value\":{\"integer\":123,\"string\":\"abc\"}}]}," + // Resolved for email + // Resolved for email, but conditions have filtered out tenant2 + "{\"id\":\"list-id\",\"rules\":[{\"conditions\":[{\"type\":\"blacklist\",\"dimension\":\"application\",\"values\":[\"tenant1:video:default\",\"tenant1:video:default\"]}],\"value\":[\"value1\"]},{\"conditions\":[{\"type\":\"blacklist\",\"dimension\":\"application\",\"values\":[\"tenant1:video:default\",\"tenant1:video:default\"]}],\"value\":[\"value1\",\"value3\"]},{\"value\":[\"a\"]}]}," + + "{\"id\":\"string-id\",\"rules\":[{\"value\":\"value1\"}]}]}", // resolved for email + flagData, Set.of("tenant1"), false, email1); + + // As operator no conditions are filtered, but the email precondition is applied + assertUserFlags("{\"flags\":[" + + "{\"id\":\"int-id\",\"rules\":[{\"value\":456}]}," + // Default from DB + "{\"id\":\"jackson-id\",\"rules\":[{\"value\":{\"integer\":123,\"string\":\"abc\"}}]}," + // Default from code, no DB values match + // Includes last value from DB which is not conditioned on email and the default from code + "{\"id\":\"list-id\",\"rules\":[{\"conditions\":[{\"type\":\"blacklist\",\"dimension\":\"application\",\"values\":[\"tenant1:video:default\",\"tenant1:video:default\",\"tenant2:music:default\"]}],\"value\":[\"value1\",\"value3\"]},{\"value\":[\"a\"]}]}," + + "{\"id\":\"string-id\",\"rules\":[{\"value\":\"default value\"}]}]}", // Default from code + flagData, Set.of(), true, "operator@domain.tld"); + } + } + + private static FlagData flagData(String id, Rule... rules) { + return new FlagData(new FlagId(id), new FetchVector(), rules); + } + + private static Rule rule(String data, Condition... conditions) { + return new Rule(Optional.ofNullable(data).map(JsonNodeRawFlag::fromJson), conditions); + } + + private static Condition condition(FetchVector.Dimension dimension, Condition.Type type, String... values) { + return new Condition.CreateParams(dimension).withValues(values).createAs(type); + } + + private static void assertUserFlags(String expected, Map rawFlagData, + Set authorizedForTenantNames, boolean isOperator, String userEmail) throws IOException { + Slime slime = new Slime(); + UserFlagsSerializer.toSlime(slime.setObject(), rawFlagData, authorizedForTenantNames.stream().map(TenantName::from).collect(Collectors.toSet()), isOperator, userEmail); + JsonTestHelper.assertJsonEquals(expected, + new String(SlimeUtils.toJsonBytes(slime), StandardCharsets.UTF_8)); + } + + @JsonIgnoreProperties(ignoreUnknown = true) + private static class ExampleJacksonClass { + @JsonProperty("integer") public final int integer; + @JsonProperty("string") public final String string; + private ExampleJacksonClass(@JsonProperty("integer") int integer, @JsonProperty("string") String string) { + this.integer = integer; + this.string = string; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + ExampleJacksonClass that = (ExampleJacksonClass) o; + return integer == that.integer && + Objects.equals(string, that.string); + } + + @Override + public int hashCode() { + return Objects.hash(integer, string); + } + } +} \ No newline at end of file diff --git a/flags/src/main/java/com/yahoo/vespa/flags/FetchVector.java b/flags/src/main/java/com/yahoo/vespa/flags/FetchVector.java index 178fdc90d20..5b3b2a94beb 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/FetchVector.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/FetchVector.java @@ -91,6 +91,10 @@ public class FetchVector { public boolean isEmpty() { return map.isEmpty(); } + public boolean hasDimension(FetchVector.Dimension dimension) { + return map.containsKey(dimension); + } + /** * Returns a new FetchVector, identical to {@code this} except for its value in {@code dimension}. * Dimension is removed if the value is null. diff --git a/flags/src/main/java/com/yahoo/vespa/flags/json/Condition.java b/flags/src/main/java/com/yahoo/vespa/flags/json/Condition.java index 46961fbd8cc..f73e0033773 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/json/Condition.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/json/Condition.java @@ -52,6 +52,16 @@ public interface Condition extends Predicate { public FetchVector.Dimension dimension() { return dimension; } public List values() { return values; } public Optional predicate() { return predicate; } + + public Condition createAs(Condition.Type type) { + switch (type) { + case WHITELIST: return WhitelistCondition.create(this); + case BLACKLIST: return BlacklistCondition.create(this); + case RELATIONAL: return RelationalCondition.create(this); + } + + throw new IllegalArgumentException("Unknown type '" + type + "'"); + } } static Condition fromWire(WireCondition wireCondition) { @@ -70,14 +80,14 @@ public interface Condition extends Predicate { params.withPredicate(wireCondition.predicate); } - switch (type) { - case WHITELIST: return WhitelistCondition.create(params); - case BLACKLIST: return BlacklistCondition.create(params); - case RELATIONAL: return RelationalCondition.create(params); - } - - throw new IllegalArgumentException("Unknown type '" + type + "'"); + return params.createAs(type); } + Condition.Type type(); + + FetchVector.Dimension dimension(); + + CreateParams toCreateParams(); + WireCondition toWire(); } diff --git a/flags/src/main/java/com/yahoo/vespa/flags/json/FlagData.java b/flags/src/main/java/com/yahoo/vespa/flags/json/FlagData.java index 88da3e75a84..eea61eb71ef 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/json/FlagData.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/json/FlagData.java @@ -49,6 +49,10 @@ public class FlagData { return id; } + public List rules() { + return rules; + } + public boolean isEmpty() { return rules.isEmpty() && defaultFetchVector.isEmpty(); } public Optional resolve(FetchVector fetchVector) { diff --git a/flags/src/main/java/com/yahoo/vespa/flags/json/ListCondition.java b/flags/src/main/java/com/yahoo/vespa/flags/json/ListCondition.java index c2c76529833..136857bea5f 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/json/ListCondition.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/json/ListCondition.java @@ -26,6 +26,21 @@ public abstract class ListCondition implements Condition { } } + @Override + public Type type() { + return type; + } + + @Override + public FetchVector.Dimension dimension() { + return dimension; + } + + @Override + public CreateParams toCreateParams() { + return new CreateParams(dimension).withValues(values); + } + @Override public boolean test(FetchVector fetchVector) { boolean listContainsValue = fetchVector.getValue(dimension).map(values::contains).orElse(false); diff --git a/flags/src/main/java/com/yahoo/vespa/flags/json/RelationalCondition.java b/flags/src/main/java/com/yahoo/vespa/flags/json/RelationalCondition.java index db2f0a3a197..4ed3e49029f 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/json/RelationalCondition.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/json/RelationalCondition.java @@ -47,6 +47,21 @@ public class RelationalCondition implements Condition { this.dimension = dimension; } + @Override + public Type type() { + return Type.RELATIONAL; + } + + @Override + public FetchVector.Dimension dimension() { + return dimension; + } + + @Override + public CreateParams toCreateParams() { + return new CreateParams(dimension).withPredicate(relationalPredicate.toWire()); + } + @Override public boolean test(FetchVector fetchVector) { return fetchVector.getValue(dimension).map(predicate::test).orElse(false); diff --git a/flags/src/main/java/com/yahoo/vespa/flags/json/Rule.java b/flags/src/main/java/com/yahoo/vespa/flags/json/Rule.java index eae6952d329..0d50f1e283f 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/json/Rule.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/json/Rule.java @@ -27,10 +27,24 @@ public class Rule { this.valueToApply = valueToApply; } + public List conditions() { + return andConditions; + } + + /** Returns true if all the conditions satisfy the given fetch vector */ public boolean match(FetchVector fetchVector) { return andConditions.stream().allMatch(condition -> condition.test(fetchVector)); } + /** + * Returns true if all the conditions on dimensions set in the fetch vector are satisfied. + * Conditions on dimensions not specified in the given fetch vector are ignored. + */ + public boolean partialMatch(FetchVector fetchVector) { + return andConditions.stream() + .allMatch(condition -> !fetchVector.hasDimension(condition.dimension()) || condition.test(fetchVector)); + } + public Optional getValueToApply() { return valueToApply; } -- cgit v1.2.3 From f549625e7c8a212d5165a3e402e5f9fd74fed451 Mon Sep 17 00:00:00 2001 From: Valerij Fredriksen Date: Wed, 22 Sep 2021 15:35:04 +0200 Subject: Expose user flags in /user/v1/user response --- .../controller/restapi/user/UserApiHandler.java | 12 ++- .../hosted/controller/restapi/ContainerTester.java | 1 - .../controller/restapi/user/UserApiOnPremTest.java | 58 ++++++----- .../controller/restapi/user/UserApiTest.java | 116 +++++++++++---------- .../responses/user-with-applications-athenz.json | 3 +- .../responses/user-with-applications-cloud.json | 3 +- .../user/responses/user-without-applications.json | 3 +- .../user-without-trial-capacity-cloud.json | 3 +- .../src/main/java/com/yahoo/vespa/flags/Flags.java | 7 +- 9 files changed, 112 insertions(+), 94 deletions(-) diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiHandler.java index 044b7b76d1e..157f57b3bea 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiHandler.java @@ -19,6 +19,7 @@ import com.yahoo.slime.Slime; import com.yahoo.slime.SlimeStream; import com.yahoo.slime.SlimeUtils; import com.yahoo.text.Text; +import com.yahoo.vespa.configserver.flags.FlagsDb; import com.yahoo.vespa.flags.BooleanFlag; import com.yahoo.vespa.flags.FetchVector; import com.yahoo.vespa.flags.FlagSource; @@ -69,15 +70,17 @@ public class UserApiHandler extends LoggingRequestHandler { private final UserManagement users; private final Controller controller; + private final FlagsDb flagsDb; private final BooleanFlag enable_public_signup_flow; private final IntFlag maxTrialTenants; private final BooleanFlag enabledHorizonDashboard; @Inject - public UserApiHandler(Context parentCtx, UserManagement users, Controller controller, FlagSource flagSource) { + public UserApiHandler(Context parentCtx, UserManagement users, Controller controller, FlagSource flagSource, FlagsDb flagsDb) { super(parentCtx); this.users = users; this.controller = controller; + this.flagsDb = flagsDb; this.enable_public_signup_flow = PermanentFlags.ENABLE_PUBLIC_SIGNUP_FLOW.bindTo(flagSource); this.maxTrialTenants = PermanentFlags.MAX_TRIAL_TENANTS.bindTo(flagSource); this.enabledHorizonDashboard = Flags.ENABLED_HORIZON_DASHBOARD.bindTo(flagSource); @@ -170,6 +173,7 @@ public class UserApiHandler extends LoggingRequestHandler { root.setBool("isPublic", controller.system().isPublic()); root.setBool("isCd", controller.system().isCd()); + // TODO (freva): Remove after users have migrated to use 'flags' root.setBool(enable_public_signup_flow.id().toString(), enable_public_signup_flow.with(FetchVector.Dimension.CONSOLE_USER_EMAIL, user.email()).value()); root.setBool("hasTrialCapacity", hasTrialCapacity()); @@ -197,6 +201,8 @@ public class UserApiHandler extends LoggingRequestHandler { operatorRoles.forEach(role -> operator.addString(role.definition().name())); } + UserFlagsSerializer.toSlime(root, flagsDb.getAllFlagData(), tenantRolesByTenantName.keySet(), !operatorRoles.isEmpty(), user.email()); + return new SlimeJsonResponse(slime); } @@ -249,7 +255,7 @@ public class UserApiHandler extends LoggingRequestHandler { }); } - private void toSlime(Cursor userObject, User user) { + private static void toSlime(Cursor userObject, User user) { if (user.name() != null) userObject.setString("name", user.name()); userObject.setString("email", user.email()); if (user.nickname() != null) userObject.setString("nickname", user.nickname()); @@ -376,7 +382,7 @@ public class UserApiHandler extends LoggingRequestHandler { return Exceptions.uncheck(() -> SlimeUtils.jsonToSlime(IOUtils.readBytes(request.getData(), 1 << 10)).get()); } - private Type require(String name, Function mapper, Inspector object) { + private static Type require(String name, Function mapper, Inspector object) { if ( ! object.field(name).valid()) throw new IllegalArgumentException("Missing field '" + name + "'."); return mapper.apply(object.field(name)); } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerTester.java index 10f143a8e96..1d844859c37 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerTester.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerTester.java @@ -26,7 +26,6 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Paths; import java.util.Optional; -import java.util.function.Consumer; import java.util.function.Supplier; import java.util.regex.Pattern; diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiOnPremTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiOnPremTest.java index acd481030e2..c884eae8afc 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiOnPremTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiOnPremTest.java @@ -5,6 +5,8 @@ import com.yahoo.application.container.handler.Request; import com.yahoo.vespa.athenz.api.AthenzDomain; import com.yahoo.vespa.athenz.api.AthenzIdentity; import com.yahoo.vespa.athenz.utils.AthenzIdentities; +import com.yahoo.vespa.flags.Flags; +import com.yahoo.vespa.flags.PermanentFlags; import com.yahoo.vespa.hosted.controller.ControllerTester; import com.yahoo.vespa.hosted.controller.api.integration.user.User; import com.yahoo.vespa.hosted.controller.restapi.ContainerTester; @@ -25,40 +27,42 @@ public class UserApiOnPremTest extends ControllerContainerTest { @Test public void userMetadataOnPremTest() { - ContainerTester tester = new ContainerTester(container, responseFiles); - ControllerTester controller = new ControllerTester(tester); - User user = new User("dev@domail", "Joe Developer", "dev", null); + try (Flags.Replacer ignored = Flags.clearFlagsForTesting(PermanentFlags.MAX_TRIAL_TENANTS.id(), PermanentFlags.ENABLE_PUBLIC_SIGNUP_FLOW.id())) { + ContainerTester tester = new ContainerTester(container, responseFiles); + ControllerTester controller = new ControllerTester(tester); + User user = new User("dev@domail", "Joe Developer", "dev", null); - controller.createTenant("tenant1", "domain1", 1L); - controller.createApplication("tenant1", "app1", "default"); - controller.createApplication("tenant1", "app2", "default"); - controller.createApplication("tenant1", "app2", "myinstance"); - controller.createApplication("tenant1", "app3"); + controller.createTenant("tenant1", "domain1", 1L); + controller.createApplication("tenant1", "app1", "default"); + controller.createApplication("tenant1", "app2", "default"); + controller.createApplication("tenant1", "app2", "myinstance"); + controller.createApplication("tenant1", "app3"); - controller.createTenant("tenant2", "domain2", 2L); - controller.createApplication("tenant2", "app2", "test"); + controller.createTenant("tenant2", "domain2", 2L); + controller.createApplication("tenant2", "app2", "test"); - controller.createTenant("tenant3", "domain3", 3L); - controller.createApplication("tenant3", "app1"); + controller.createTenant("tenant3", "domain3", 3L); + controller.createApplication("tenant3", "app1"); - controller.createTenant("sandbox", "domain4", 4L); - controller.createApplication("sandbox", "app1", "default"); - controller.createApplication("sandbox", "app2", "default"); - controller.createApplication("sandbox", "app2", "dev"); + controller.createTenant("sandbox", "domain4", 4L); + controller.createApplication("sandbox", "app1", "default"); + controller.createApplication("sandbox", "app2", "default"); + controller.createApplication("sandbox", "app2", "dev"); - AthenzIdentity operator = AthenzIdentities.from("vespa.alice"); - controller.athenzDb().addHostedOperator(operator); - AthenzIdentity tenantAdmin = AthenzIdentities.from("domain1.bob"); - Stream.of("domain1", "domain2", "domain4") - .map(AthenzDomain::new) - .map(controller.athenzDb()::getOrCreateDomain) - .forEach(d -> d.admin(AthenzIdentities.from("domain1.bob"))); + AthenzIdentity operator = AthenzIdentities.from("vespa.alice"); + controller.athenzDb().addHostedOperator(operator); + AthenzIdentity tenantAdmin = AthenzIdentities.from("domain1.bob"); + Stream.of("domain1", "domain2", "domain4") + .map(AthenzDomain::new) + .map(controller.athenzDb()::getOrCreateDomain) + .forEach(d -> d.admin(AthenzIdentities.from("domain1.bob"))); - tester.assertResponse(createUserRequest(user, operator), - new File("user-without-applications.json")); + tester.assertResponse(createUserRequest(user, operator), + new File("user-without-applications.json")); - tester.assertResponse(createUserRequest(user, tenantAdmin), - new File("user-with-applications-athenz.json")); + tester.assertResponse(createUserRequest(user, tenantAdmin), + new File("user-with-applications-athenz.json")); + } } private Request createUserRequest(User user, AthenzIdentity identity) { diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiTest.java index 03f1d75a50b..9198369a3ad 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiTest.java @@ -4,6 +4,7 @@ package com.yahoo.vespa.hosted.controller.restapi.user; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.SystemName; import com.yahoo.config.provision.TenantName; +import com.yahoo.vespa.flags.Flags; import com.yahoo.vespa.flags.InMemoryFlagSource; import com.yahoo.vespa.flags.PermanentFlags; import com.yahoo.vespa.hosted.controller.ControllerTester; @@ -205,65 +206,68 @@ public class UserApiTest extends ControllerContainerCloudTest { @Test public void userMetadataTest() { - ContainerTester tester = new ContainerTester(container, responseFiles); - ((InMemoryFlagSource) tester.controller().flagSource()) - .withBooleanFlag(PermanentFlags.ENABLE_PUBLIC_SIGNUP_FLOW.id(), true); - ControllerTester controller = new ControllerTester(tester); - Set operator = Set.of(Role.hostedOperator(), Role.hostedSupporter(), Role.hostedAccountant()); - User user = new User("dev@domail", "Joe Developer", "dev", null); - - tester.assertResponse(request("/user/v1/user") - .roles(operator) - .user(user), - new File("user-without-applications.json")); - - controller.createTenant("tenant1", Tenant.Type.cloud); - controller.createApplication("tenant1", "app1", "default"); - controller.createApplication("tenant1", "app2", "default"); - controller.createApplication("tenant1", "app2", "myinstance"); - controller.createApplication("tenant1", "app3"); - - controller.createTenant("tenant2", Tenant.Type.cloud); - controller.createApplication("tenant2", "app2", "test"); - - controller.createTenant("tenant3", Tenant.Type.cloud); - controller.createApplication("tenant3", "app1"); - - controller.createTenant("sandbox", Tenant.Type.cloud); - controller.createApplication("sandbox", "app1", "default"); - controller.createApplication("sandbox", "app2", "default"); - controller.createApplication("sandbox", "app2", "dev"); - - // Should still be empty because none of the roles explicitly refer to any of the applications - tester.assertResponse(request("/user/v1/user") - .roles(operator) - .user(user), - new File("user-without-applications.json")); - - // Empty applications because tenant dummy does not exist - tester.assertResponse(request("/user/v1/user") - .roles(Set.of(Role.administrator(TenantName.from("tenant1")), - Role.developer(TenantName.from("tenant2")), - Role.developer(TenantName.from("sandbox")), - Role.reader(TenantName.from("sandbox")))) - .user(user), - new File("user-with-applications-cloud.json")); + try (Flags.Replacer ignored = Flags.clearFlagsForTesting(PermanentFlags.MAX_TRIAL_TENANTS.id(), PermanentFlags.ENABLE_PUBLIC_SIGNUP_FLOW.id())) { + ContainerTester tester = new ContainerTester(container, responseFiles); + ((InMemoryFlagSource) tester.controller().flagSource()) + .withBooleanFlag(PermanentFlags.ENABLE_PUBLIC_SIGNUP_FLOW.id(), true); + ControllerTester controller = new ControllerTester(tester); + Set operator = Set.of(Role.hostedOperator(), Role.hostedSupporter(), Role.hostedAccountant()); + User user = new User("dev@domail", "Joe Developer", "dev", null); + + tester.assertResponse(request("/user/v1/user") + .roles(operator) + .user(user), + new File("user-without-applications.json")); + + controller.createTenant("tenant1", Tenant.Type.cloud); + controller.createApplication("tenant1", "app1", "default"); + controller.createApplication("tenant1", "app2", "default"); + controller.createApplication("tenant1", "app2", "myinstance"); + controller.createApplication("tenant1", "app3"); + + controller.createTenant("tenant2", Tenant.Type.cloud); + controller.createApplication("tenant2", "app2", "test"); + + controller.createTenant("tenant3", Tenant.Type.cloud); + controller.createApplication("tenant3", "app1"); + + controller.createTenant("sandbox", Tenant.Type.cloud); + controller.createApplication("sandbox", "app1", "default"); + controller.createApplication("sandbox", "app2", "default"); + controller.createApplication("sandbox", "app2", "dev"); + + // Should still be empty because none of the roles explicitly refer to any of the applications + tester.assertResponse(request("/user/v1/user") + .roles(operator) + .user(user), + new File("user-without-applications.json")); + + // Empty applications because tenant dummy does not exist + tester.assertResponse(request("/user/v1/user") + .roles(Set.of(Role.administrator(TenantName.from("tenant1")), + Role.developer(TenantName.from("tenant2")), + Role.developer(TenantName.from("sandbox")), + Role.reader(TenantName.from("sandbox")))) + .user(user), + new File("user-with-applications-cloud.json")); + } } @Test public void maxTrialTenants() { - ContainerTester tester = new ContainerTester(container, responseFiles); - ((InMemoryFlagSource) tester.controller().flagSource()) - .withIntFlag(PermanentFlags.MAX_TRIAL_TENANTS.id(), 1) - .withBooleanFlag(PermanentFlags.ENABLE_PUBLIC_SIGNUP_FLOW.id(), true); - ControllerTester controller = new ControllerTester(tester); - Set operator = Set.of(Role.hostedOperator(), Role.hostedSupporter(), Role.hostedAccountant()); - User user = new User("dev@domail", "Joe Developer", "dev", null); - - controller.createTenant("tenant1", Tenant.Type.cloud); - - tester.assertResponse( - request("/user/v1/user").user(user), - new File("user-without-trial-capacity-cloud.json")); + try (Flags.Replacer ignored = Flags.clearFlagsForTesting(PermanentFlags.MAX_TRIAL_TENANTS.id(), PermanentFlags.ENABLE_PUBLIC_SIGNUP_FLOW.id())) { + ContainerTester tester = new ContainerTester(container, responseFiles); + ((InMemoryFlagSource) tester.controller().flagSource()) + .withIntFlag(PermanentFlags.MAX_TRIAL_TENANTS.id(), 1) + .withBooleanFlag(PermanentFlags.ENABLE_PUBLIC_SIGNUP_FLOW.id(), true); + ControllerTester controller = new ControllerTester(tester); + User user = new User("dev@domail", "Joe Developer", "dev", null); + + controller.createTenant("tenant1", Tenant.Type.cloud); + + tester.assertResponse( + request("/user/v1/user").user(user), + new File("user-without-trial-capacity-cloud.json")); + } } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/responses/user-with-applications-athenz.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/responses/user-with-applications-athenz.json index 5d3a38334ad..0a416600b2c 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/responses/user-with-applications-athenz.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/responses/user-with-applications-athenz.json @@ -31,5 +31,6 @@ "reader" ] } - } + }, + "flags": [{"id":"enable-public-signup-flow","rules":[{"value":false}]}] } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/responses/user-with-applications-cloud.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/responses/user-with-applications-cloud.json index e883993cb53..4e179ad83c5 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/responses/user-with-applications-cloud.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/responses/user-with-applications-cloud.json @@ -29,5 +29,6 @@ ], "enabled-horizon-dashboard":false } - } + }, + "flags": [{"id":"enable-public-signup-flow","rules":[{"value":false}]}] } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/responses/user-without-applications.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/responses/user-without-applications.json index 3bf999b490b..7eb445140e7 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/responses/user-without-applications.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/responses/user-without-applications.json @@ -14,5 +14,6 @@ "hostedOperator", "hostedSupporter", "hostedAccountant" - ] + ], + "flags": [{"id":"enable-public-signup-flow","rules":[{"value":false}]}] } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/responses/user-without-trial-capacity-cloud.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/responses/user-without-trial-capacity-cloud.json index 27242424579..3c1edab8cfc 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/responses/user-without-trial-capacity-cloud.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/responses/user-without-trial-capacity-cloud.json @@ -9,5 +9,6 @@ "nickname": "dev", "verified":false }, - "tenants": {} + "tenants": {}, + "flags": [{"id":"enable-public-signup-flow","rules":[{"value":false}]}] } \ No newline at end of file diff --git a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java index 015e063fe11..aa7f42fede6 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java @@ -416,8 +416,8 @@ public class Flags { * *

NOT thread-safe. Tests using this cannot run in parallel. */ - public static Replacer clearFlagsForTesting() { - return new Replacer(); + public static Replacer clearFlagsForTesting(FlagId... flagsToKeep) { + return new Replacer(flagsToKeep); } public static class Replacer implements AutoCloseable { @@ -425,10 +425,11 @@ public class Flags { private final TreeMap savedFlags; - private Replacer() { + private Replacer(FlagId... flagsToKeep) { verifyAndSetFlagsCleared(true); this.savedFlags = Flags.flags; Flags.flags = new TreeMap<>(); + List.of(flagsToKeep).forEach(id -> Flags.flags.put(id, savedFlags.get(id))); } @Override -- cgit v1.2.3 From db492ef523f0fda105e55f40699522ec6f828672 Mon Sep 17 00:00:00 2001 From: Valerij Fredriksen Date: Wed, 22 Sep 2021 16:05:35 +0200 Subject: Disallow combing CONSOLE_USER_EMAIL dimension with some other dimensions --- .../main/java/com/yahoo/vespa/flags/FlagDefinition.java | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/flags/src/main/java/com/yahoo/vespa/flags/FlagDefinition.java b/flags/src/main/java/com/yahoo/vespa/flags/FlagDefinition.java index 90e0022c634..7ddbd85a904 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/FlagDefinition.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/FlagDefinition.java @@ -3,7 +3,9 @@ package com.yahoo.vespa.flags; import javax.annotation.concurrent.Immutable; import java.time.Instant; +import java.util.EnumSet; import java.util.List; +import java.util.Set; /** * @author hakonhall @@ -26,7 +28,6 @@ public class FlagDefinition { String description, String modificationEffect, FetchVector.Dimension... dimensions) { - validate(owners, createdAt, expiresAt); this.unboundFlag = unboundFlag; this.owners = owners; this.createdAt = createdAt; @@ -34,6 +35,7 @@ public class FlagDefinition { this.description = description; this.modificationEffect = modificationEffect; this.dimensions = List.of(dimensions); + validate(owners, createdAt, expiresAt, this.dimensions); } public UnboundFlag getUnboundFlag() { @@ -58,13 +60,14 @@ public class FlagDefinition { public Instant getExpiresAt() { return expiresAt; } - private static void validate(List owners, Instant createdAt, Instant expiresAt) { + private static void validate(List owners, Instant createdAt, Instant expiresAt, List dimensions) { if (expiresAt.isBefore(createdAt)) { throw new IllegalArgumentException( String.format( "Flag cannot expire before its creation date (createdAt='%s', expiresAt='%s')", createdAt, expiresAt)); } + if (owners == PermanentFlags.OWNERS) { if (!createdAt.equals(PermanentFlags.CREATED_AT) || !expiresAt.equals(PermanentFlags.EXPIRES_AT)) { throw new IllegalArgumentException("Invalid creation or expiration date for permanent flag"); @@ -72,5 +75,15 @@ public class FlagDefinition { } else if (owners.isEmpty()) { throw new IllegalArgumentException("Owner(s) must be specified"); } + + if (dimensions.contains(FetchVector.Dimension.CONSOLE_USER_EMAIL)) { + Set disallowedCombinations = EnumSet.allOf(FetchVector.Dimension.class); + disallowedCombinations.remove(FetchVector.Dimension.CONSOLE_USER_EMAIL); + disallowedCombinations.remove(FetchVector.Dimension.APPLICATION_ID); + disallowedCombinations.remove(FetchVector.Dimension.TENANT_ID); + disallowedCombinations.retainAll(dimensions); + if (!disallowedCombinations.isEmpty()) + throw new IllegalArgumentException("Dimension " + FetchVector.Dimension.CONSOLE_USER_EMAIL + " cannot be combined with " + disallowedCombinations); + } } } -- cgit v1.2.3 From 01a79eb963ea6c65efa3ba21807d3ced2f79defc Mon Sep 17 00:00:00 2001 From: Valerij Fredriksen Date: Wed, 22 Sep 2021 16:25:51 +0200 Subject: Add CONSOLE_USER_EMAIL to enabled-horizon-dashboard --- flags/src/main/java/com/yahoo/vespa/flags/Flags.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java index aa7f42fede6..736bdc4d8c3 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java @@ -13,6 +13,7 @@ import java.util.Optional; import java.util.TreeMap; import static com.yahoo.vespa.flags.FetchVector.Dimension.APPLICATION_ID; +import static com.yahoo.vespa.flags.FetchVector.Dimension.CONSOLE_USER_EMAIL; import static com.yahoo.vespa.flags.FetchVector.Dimension.HOSTNAME; import static com.yahoo.vespa.flags.FetchVector.Dimension.TENANT_ID; import static com.yahoo.vespa.flags.FetchVector.Dimension.VESPA_VERSION; @@ -278,7 +279,7 @@ public class Flags { List.of("olaa"), "2021-09-13", "2021-12-31", "Enable Horizon dashboard", "Takes effect immediately", - TENANT_ID + TENANT_ID, CONSOLE_USER_EMAIL ); public static final UnboundBooleanFlag ENABLE_ONPREM_TENANT_S3_ARCHIVE = defineFeatureFlag( -- cgit v1.2.3