From c78f48ec75e9c15b90d78a0c99e5d193f0691a60 Mon Sep 17 00:00:00 2001 From: Martin Polden Date: Thu, 11 Jul 2019 13:44:36 +0200 Subject: Remove disable-chef feature flag --- flags/src/main/java/com/yahoo/vespa/flags/Flags.java | 7 ------- 1 file changed, 7 deletions(-) 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 0981d7b84e2..c160bf4dc4e 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java @@ -151,13 +151,6 @@ public class Flags { "Takes effect on deployment through controller", APPLICATION_ID); - public static final UnboundBooleanFlag DISABLE_CHEF = defineFeatureFlag( - "disable-chef", false, - "Stops and disables chef-client", - "Takes effect on next host-admin tick", - HOSTNAME, NODE_TYPE); - - /** WARNING: public for testing: All flags should be defined in {@link Flags}. */ public static UnboundBooleanFlag defineFeatureFlag(String flagId, boolean defaultValue, String description, String modificationEffect, FetchVector.Dimension... dimensions) { -- cgit v1.2.3 From 2f95dbec5a6d82f2eda41dbeea594eb41f135223 Mon Sep 17 00:00:00 2001 From: Øyvind Grønnesby Date: Thu, 11 Jul 2019 14:08:16 +0200 Subject: Do not read legacy rotation/endpoint fields --- .../persistence/ApplicationSerializer.java | 21 ------------ .../persistence/ApplicationSerializerTest.java | 39 ++-------------------- 2 files changed, 2 insertions(+), 58 deletions(-) diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java index 0c045eb7253..135586972b1 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java @@ -555,27 +555,6 @@ public class ApplicationSerializer { private List assignedRotationsFromSlime(DeploymentSpec deploymentSpec, Inspector root) { final var assignedRotations = new LinkedHashMap(); - // Add the legacy rotation field to the set - this needs to be first - // TODO: Remove when we retire the rotations field - final var legacyRotation = legacyRotationFromSlime(root.field(deprecatedRotationField)); - if (legacyRotation.isPresent() && deploymentSpec.globalServiceId().isPresent()) { - final var clusterId = new ClusterSpec.Id(deploymentSpec.globalServiceId().get()); - final var regions = deploymentSpec.zones().stream().flatMap(zone -> zone.region().stream()).collect(Collectors.toSet()); - assignedRotations.putIfAbsent(EndpointId.default_(), new AssignedRotation(clusterId, EndpointId.default_(), legacyRotation.get(), regions)); - } - - // Now add the same entries from "stupid" list of rotations - // TODO: Remove when we retire the rotations field - final var rotations = rotationListFromSlime(root.field(rotationsField)); - for (var rotation : rotations) { - final var regions = deploymentSpec.zones().stream().flatMap(zone -> zone.region().stream()).collect(Collectors.toSet()); - if (deploymentSpec.globalServiceId().isPresent()) { - final var clusterId = new ClusterSpec.Id(deploymentSpec.globalServiceId().get()); - assignedRotations.putIfAbsent(EndpointId.default_(), new AssignedRotation(clusterId, EndpointId.default_(), rotation, regions)); - } - } - - // Last - add the actual entries we want. Do _not_ remove this during clean-up root.field(assignedRotationsField).traverse((ArrayTraverser) (idx, inspector) -> { final var clusterId = new ClusterSpec.Id(inspector.field(assignedRotationClusterField).asString()); final var endpointId = EndpointId.of(inspector.field(assignedRotationEndpointField).asString()); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java index 7b39b0d53a4..aca4f750649 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java @@ -253,9 +253,8 @@ public class ApplicationSerializerTest { // ok if no error } - /** TODO: Test can be removed after June 2019 - once legacy field for single rotation is retired */ @Test - public void testParsingLegacyRotationElement() throws IOException { + public void testParsingAssignedRotations() throws IOException { // Use the 'complete-application.json' as a baseline final var applicationJson = Files.readAllBytes(testData.resolve("complete-application.json")); final var slime = SlimeUtils.jsonToSlime(applicationJson); @@ -267,12 +266,6 @@ public class ApplicationSerializerTest { // Add the necessary fields to the Slime representation of the application final var cursor = slime.get(); - cursor.setString("rotation", "single-rotation"); - - final var rotations = cursor.setArray("endpoints"); - rotations.addString("multiple-rotation-1"); - rotations.addString("multiple-rotation-2"); - final var assignedRotations = cursor.setArray("assignedRotations"); final var assignedRotation = assignedRotations.addObject(); assignedRotation.setString("clusterId", "foobar"); @@ -282,51 +275,23 @@ public class ApplicationSerializerTest { // Parse and test the output from parsing contains both legacy rotation and multiple rotations final var application = applicationSerializer.fromSlime(slime); - // Since only one AssignedEndpoint can be "default", we make sure that we are ignoring the - // multiple-rotation entries as the globalServiceId will override them assertEquals( List.of( - new RotationId("single-rotation"), new RotationId("assigned-rotation") ), application.rotations() ); assertEquals( - Optional.of(new RotationId("single-rotation")), application.legacyRotation() + Optional.of(new RotationId("assigned-rotation")), application.legacyRotation() ); - // The same goes here for AssignedRotations with "default" EndpointId as in the .rotations() test above. - // Note that we are only using Set.of() on "assigned-rotation" because in this test we do not have access - // to a deployment.xml that describes the zones a rotation should map to. assertEquals( List.of( - new AssignedRotation(new ClusterSpec.Id("foo"), EndpointId.of("default"), new RotationId("single-rotation"), regions), new AssignedRotation(new ClusterSpec.Id("foobar"), EndpointId.of("nice-endpoint"), new RotationId("assigned-rotation"), Set.of()) ), application.assignedRotations() ); } - @Test - public void testParsingOnlyLegacyRotationElement() throws IOException { - // Use the 'complete-application.json' as a baseline - final var applicationJson = Files.readAllBytes(testData.resolve("complete-application.json")); - final var slime = SlimeUtils.jsonToSlime(applicationJson); - - // Add the necessary fields to the Slime representation of the application - final var cursor = slime.get(); - - cursor.setString("rotation", "single-rotation"); - - // Parse and test the output from parsing contains both legacy rotation and multiple rotations - final var application = applicationSerializer.fromSlime(slime); - - assertEquals( - List.of( - new RotationId("single-rotation") - ), - application.rotations() - ); - } } -- cgit v1.2.3 From 664dbf7a8f4eabff904a688f77b8b7c5a2ffc184 Mon Sep 17 00:00:00 2001 From: Øyvind Grønnesby Date: Thu, 11 Jul 2019 14:17:00 +0200 Subject: Remove two unused methods --- .../controller/persistence/ApplicationSerializer.java | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java index 135586972b1..d49587963dc 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java @@ -42,7 +42,6 @@ import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.LinkedHashMap; -import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Optional; @@ -569,22 +568,6 @@ public class ApplicationSerializer { return List.copyOf(assignedRotations.values()); } - private List rotationListFromSlime(Inspector field) { - final var rotations = new ArrayList(); - - field.traverse((ArrayTraverser) (idx, inspector) -> { - final var rotation = new RotationId(inspector.asString()); - rotations.add(rotation); - }); - - return rotations; - } - - // TODO: Remove after June 2019 once the 'rotation' field is gone from storage - private Optional legacyRotationFromSlime(Inspector field) { - return field.valid() ? optionalString(field).map(RotationId::new) : Optional.empty(); - } - private OptionalLong optionalLong(Inspector field) { return field.valid() ? OptionalLong.of(field.asLong()) : OptionalLong.empty(); } -- cgit v1.2.3 From b81b21546cdff92d360cbdf7dda27e6ed7bc7170 Mon Sep 17 00:00:00 2001 From: Martin Polden Date: Thu, 11 Jul 2019 14:13:23 +0200 Subject: Decouple flags REST API from config server --- .../configserver/flags/ConfigServerFlagSource.java | 1 + .../yahoo/vespa/configserver/flags/FlagsDb.java | 25 --- .../vespa/configserver/flags/db/FlagsDbImpl.java | 67 ------- .../configserver/flags/db/ZooKeeperFlagSource.java | 2 +- .../flags/ConfigServerFlagSourceTest.java | 3 +- .../configserver/flags/db/FlagsDbImplTest.java | 56 ------ .../config/server/http/flags/DefinedFlag.java | 47 ----- .../config/server/http/flags/DefinedFlags.java | 45 ----- .../server/http/flags/FlagDataListResponse.java | 59 ------ .../config/server/http/flags/FlagDataResponse.java | 31 ---- .../config/server/http/flags/FlagsHandler.java | 120 ------------ .../vespa/config/server/http/flags/OKResponse.java | 20 -- .../vespa/config/server/http/flags/V1Response.java | 34 ---- .../main/resources/configserver-app/services.xml | 6 +- .../config/server/http/flags/FlagsHandlerTest.java | 200 -------------------- flags/pom.xml | 13 +- .../com/yahoo/vespa/flags/http/DefinedFlag.java | 47 +++++ .../com/yahoo/vespa/flags/http/DefinedFlags.java | 43 +++++ .../com/yahoo/vespa/flags/http/ErrorResponse.java | 66 +++++++ .../vespa/flags/http/FlagDataListResponse.java | 58 ++++++ .../yahoo/vespa/flags/http/FlagDataResponse.java | 30 +++ .../com/yahoo/vespa/flags/http/FlagsHandler.java | 138 ++++++++++++++ .../com/yahoo/vespa/flags/http/OKResponse.java | 19 ++ .../yahoo/vespa/flags/http/SlimeJsonResponse.java | 38 ++++ .../com/yahoo/vespa/flags/http/V1Response.java | 46 +++++ .../com/yahoo/vespa/flags/persistence/FlagsDb.java | 68 +++++++ .../vespa/flags/persistence/package-info.java | 8 + .../yahoo/vespa/flags/http/FlagsHandlerTest.java | 203 +++++++++++++++++++++ .../yahoo/vespa/flags/persistence/FlagsDbTest.java | 56 ++++++ 29 files changed, 837 insertions(+), 712 deletions(-) delete mode 100644 configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/FlagsDb.java delete mode 100644 configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/db/FlagsDbImpl.java delete mode 100644 configserver-flags/src/test/java/com/yahoo/vespa/configserver/flags/db/FlagsDbImplTest.java delete mode 100644 configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/DefinedFlag.java delete mode 100644 configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/DefinedFlags.java delete mode 100644 configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/FlagDataListResponse.java delete mode 100644 configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/FlagDataResponse.java delete mode 100644 configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/FlagsHandler.java delete mode 100644 configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/OKResponse.java delete mode 100644 configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/V1Response.java delete mode 100644 configserver/src/test/java/com/yahoo/vespa/config/server/http/flags/FlagsHandlerTest.java create mode 100644 flags/src/main/java/com/yahoo/vespa/flags/http/DefinedFlag.java create mode 100644 flags/src/main/java/com/yahoo/vespa/flags/http/DefinedFlags.java create mode 100644 flags/src/main/java/com/yahoo/vespa/flags/http/ErrorResponse.java create mode 100644 flags/src/main/java/com/yahoo/vespa/flags/http/FlagDataListResponse.java create mode 100644 flags/src/main/java/com/yahoo/vespa/flags/http/FlagDataResponse.java create mode 100644 flags/src/main/java/com/yahoo/vespa/flags/http/FlagsHandler.java create mode 100644 flags/src/main/java/com/yahoo/vespa/flags/http/OKResponse.java create mode 100644 flags/src/main/java/com/yahoo/vespa/flags/http/SlimeJsonResponse.java create mode 100644 flags/src/main/java/com/yahoo/vespa/flags/http/V1Response.java create mode 100644 flags/src/main/java/com/yahoo/vespa/flags/persistence/FlagsDb.java create mode 100644 flags/src/main/java/com/yahoo/vespa/flags/persistence/package-info.java create mode 100644 flags/src/test/java/com/yahoo/vespa/flags/http/FlagsHandlerTest.java create mode 100644 flags/src/test/java/com/yahoo/vespa/flags/persistence/FlagsDbTest.java diff --git a/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/ConfigServerFlagSource.java b/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/ConfigServerFlagSource.java index b1ffc05e70c..90709951dec 100644 --- a/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/ConfigServerFlagSource.java +++ b/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/ConfigServerFlagSource.java @@ -5,6 +5,7 @@ import com.google.inject.Inject; import com.yahoo.vespa.configserver.flags.db.BootstrapFlagSource; import com.yahoo.vespa.configserver.flags.db.ZooKeeperFlagSource; import com.yahoo.vespa.flags.OrderedFlagSource; +import com.yahoo.vespa.flags.persistence.FlagsDb; import java.nio.file.FileSystem; import java.nio.file.FileSystems; diff --git a/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/FlagsDb.java b/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/FlagsDb.java deleted file mode 100644 index 2c29ae0b818..00000000000 --- a/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/FlagsDb.java +++ /dev/null @@ -1,25 +0,0 @@ -// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.configserver.flags; - -import com.yahoo.vespa.flags.FlagId; -import com.yahoo.vespa.flags.json.FlagData; - -import java.util.Map; -import java.util.Optional; - -/** - * @author hakonhall - */ -public interface FlagsDb { - /** Get the String value of the flag. */ - Optional getValue(FlagId flagId); - - /** Set the String value of the flag. */ - void setValue(FlagId flagId, FlagData data); - - /** Remove the flag value if it exists. */ - void removeValue(FlagId flagId); - - /** Get all flags that have been set. */ - Map getAllFlags(); -} diff --git a/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/db/FlagsDbImpl.java b/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/db/FlagsDbImpl.java deleted file mode 100644 index 5058358ba03..00000000000 --- a/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/db/FlagsDbImpl.java +++ /dev/null @@ -1,67 +0,0 @@ -// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.configserver.flags.db; - -import com.google.inject.Inject; -import com.yahoo.path.Path; -import com.yahoo.vespa.configserver.flags.FlagsDb; -import com.yahoo.vespa.curator.Curator; -import com.yahoo.vespa.flags.FlagId; -import com.yahoo.vespa.flags.json.FlagData; -import org.apache.curator.framework.recipes.cache.ChildData; - -import java.util.List; -import java.util.Map; -import java.util.Optional; -import java.util.concurrent.ExecutorService; -import java.util.concurrent.Executors; -import java.util.function.Function; -import java.util.stream.Collectors; - -/** - * @author hakonhall - */ -public class FlagsDbImpl implements FlagsDb { - private static final Path ROOT_PATH = Path.fromString("/flags/v1"); - - private final Curator curator; - private final Curator.DirectoryCache cache; - - @Inject - public FlagsDbImpl(Curator curator) { - this.curator = curator; - curator.create(ROOT_PATH); - ExecutorService executorService = Executors.newFixedThreadPool(1); - this.cache = curator.createDirectoryCache(ROOT_PATH.getAbsolute(), true, false, executorService); - cache.start(); - } - - @Override - public Optional getValue(FlagId flagId) { - return Optional.ofNullable(cache.getCurrentData(getZkPathFor(flagId))) - .map(ChildData::getData) - .map(FlagData::deserializeUtf8Json); - } - - @Override - public void setValue(FlagId flagId, FlagData data) { - curator.set(getZkPathFor(flagId), data.serializeToUtf8Json()); - } - - @Override - public Map getAllFlags() { - List dataList = cache.getCurrentData(); - return dataList.stream() - .map(ChildData::getData) - .map(FlagData::deserializeUtf8Json) - .collect(Collectors.toMap(FlagData::id, Function.identity())); - } - - @Override - public void removeValue(FlagId flagId) { - curator.delete(getZkPathFor(flagId)); - } - - private static Path getZkPathFor(FlagId flagId) { - return ROOT_PATH.append(flagId.toString()); - } -} diff --git a/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/db/ZooKeeperFlagSource.java b/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/db/ZooKeeperFlagSource.java index 4a9d604b4bd..e6ab3f5b387 100644 --- a/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/db/ZooKeeperFlagSource.java +++ b/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/db/ZooKeeperFlagSource.java @@ -1,7 +1,7 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.configserver.flags.db; -import com.yahoo.vespa.configserver.flags.FlagsDb; +import com.yahoo.vespa.flags.persistence.FlagsDb; import com.yahoo.vespa.flags.FetchVector; import com.yahoo.vespa.flags.FlagId; import com.yahoo.vespa.flags.FlagSource; diff --git a/configserver-flags/src/test/java/com/yahoo/vespa/configserver/flags/ConfigServerFlagSourceTest.java b/configserver-flags/src/test/java/com/yahoo/vespa/configserver/flags/ConfigServerFlagSourceTest.java index d0d1d61628c..b2f891326fc 100644 --- a/configserver-flags/src/test/java/com/yahoo/vespa/configserver/flags/ConfigServerFlagSourceTest.java +++ b/configserver-flags/src/test/java/com/yahoo/vespa/configserver/flags/ConfigServerFlagSourceTest.java @@ -7,6 +7,7 @@ import com.yahoo.vespa.flags.FetchVector; import com.yahoo.vespa.flags.FlagId; import com.yahoo.vespa.flags.Flags; import com.yahoo.vespa.flags.RawFlag; +import com.yahoo.vespa.flags.persistence.FlagsDb; import com.yahoo.vespa.test.file.TestFileSystem; import org.junit.After; import org.junit.Before; @@ -103,4 +104,4 @@ public class ConfigServerFlagSourceTest { assertFalse(rawFlag2.isPresent()); verify(flagsDb, times(1)).getValue(flagId2); } -} \ No newline at end of file +} diff --git a/configserver-flags/src/test/java/com/yahoo/vespa/configserver/flags/db/FlagsDbImplTest.java b/configserver-flags/src/test/java/com/yahoo/vespa/configserver/flags/db/FlagsDbImplTest.java deleted file mode 100644 index ecc9bacb081..00000000000 --- a/configserver-flags/src/test/java/com/yahoo/vespa/configserver/flags/db/FlagsDbImplTest.java +++ /dev/null @@ -1,56 +0,0 @@ -// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.configserver.flags.db; - -import com.yahoo.vespa.curator.mock.MockCurator; -import com.yahoo.vespa.flags.FetchVector; -import com.yahoo.vespa.flags.FlagId; -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.util.Map; -import java.util.Optional; - -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.notNullValue; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; - -/** - * @author hakonhall - */ -public class FlagsDbImplTest { - @Test - public void test() { - MockCurator curator = new MockCurator(); - FlagsDbImpl db = new FlagsDbImpl(curator); - - Condition condition1 = new Condition(Condition.Type.WHITELIST, FetchVector.Dimension.HOSTNAME, "host1"); - Rule rule1 = new Rule(Optional.of(JsonNodeRawFlag.fromJson("13")), condition1); - FlagId flagId = new FlagId("id"); - FlagData data = new FlagData(flagId, new FetchVector().with(FetchVector.Dimension.ZONE_ID, "zone-a"), rule1); - db.setValue(flagId, data); - - Optional dataCopy = db.getValue(flagId); - assertTrue(dataCopy.isPresent()); - - assertEquals("{\"id\":\"id\",\"rules\":[{\"conditions\":[{\"type\":\"whitelist\",\"dimension\":\"hostname\"," + - "\"values\":[\"host1\"]}],\"value\":13}],\"attributes\":{\"zone\":\"zone-a\"}}", - dataCopy.get().serializeToJson()); - - FlagId flagId2 = new FlagId("id2"); - FlagData data2 = new FlagData(flagId2, new FetchVector().with(FetchVector.Dimension.ZONE_ID, "zone-a"), rule1); - db.setValue(flagId2, data2); - Map flags = db.getAllFlags(); - assertThat(flags.size(), equalTo(2)); - assertThat(flags.get(flagId), notNullValue()); - assertThat(flags.get(flagId2), notNullValue()); - - db.removeValue(flagId2); - assertFalse(db.getValue(flagId2).isPresent()); - } -} \ No newline at end of file diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/DefinedFlag.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/DefinedFlag.java deleted file mode 100644 index 92397fc84a7..00000000000 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/DefinedFlag.java +++ /dev/null @@ -1,47 +0,0 @@ -// Copyright 2019 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.config.server.http.flags; - -import com.fasterxml.jackson.databind.ObjectMapper; -import com.fasterxml.jackson.databind.node.ArrayNode; -import com.fasterxml.jackson.databind.node.ObjectNode; -import com.yahoo.container.jdisc.HttpResponse; -import com.yahoo.jdisc.Response; -import com.yahoo.vespa.config.server.http.HttpConfigResponse; -import com.yahoo.vespa.flags.FlagDefinition; -import com.yahoo.vespa.flags.json.DimensionHelper; - -import java.io.IOException; -import java.io.OutputStream; - -/** - * @author hakonhall - */ -public class DefinedFlag extends HttpResponse { - private static ObjectMapper mapper = new ObjectMapper(); - - private final FlagDefinition flagDefinition; - - public DefinedFlag(FlagDefinition flagDefinition) { - super(Response.Status.OK); - this.flagDefinition = flagDefinition; - } - - @Override - public void render(OutputStream outputStream) throws IOException { - ObjectNode rootNode = mapper.createObjectNode(); - renderFlagDefinition(flagDefinition, rootNode); - mapper.writeValue(outputStream, rootNode); - } - - static void renderFlagDefinition(FlagDefinition flagDefinition, ObjectNode definitionNode) { - definitionNode.put("description", flagDefinition.getDescription()); - definitionNode.put("modification-effect", flagDefinition.getModificationEffect()); - ArrayNode dimensionsNode = definitionNode.putArray("dimensions"); - flagDefinition.getDimensions().forEach(dimension -> dimensionsNode.add(DimensionHelper.toWire(dimension))); - } - - @Override - public String getContentType() { - return HttpConfigResponse.JSON_CONTENT_TYPE; - } -} diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/DefinedFlags.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/DefinedFlags.java deleted file mode 100644 index 9604c51ee4b..00000000000 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/DefinedFlags.java +++ /dev/null @@ -1,45 +0,0 @@ -// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.config.server.http.flags; - -import com.fasterxml.jackson.databind.ObjectMapper; -import com.fasterxml.jackson.databind.node.ObjectNode; -import com.yahoo.container.jdisc.HttpResponse; -import com.yahoo.jdisc.Response; -import com.yahoo.vespa.config.server.http.HttpConfigResponse; -import com.yahoo.vespa.flags.FlagDefinition; - -import java.io.IOException; -import java.io.OutputStream; -import java.util.Comparator; -import java.util.List; - -/** - * @author hakonhall - */ -public class DefinedFlags extends HttpResponse { - private static ObjectMapper mapper = new ObjectMapper(); - private static final Comparator sortByFlagId = - (left, right) -> left.getUnboundFlag().id().compareTo(right.getUnboundFlag().id()); - - private final List flags; - - public DefinedFlags(List flags) { - super(Response.Status.OK); - this.flags = flags; - } - - @Override - public void render(OutputStream outputStream) throws IOException { - ObjectNode rootNode = mapper.createObjectNode(); - flags.stream().sorted(sortByFlagId).forEach(flagDefinition -> { - ObjectNode definitionNode = rootNode.putObject(flagDefinition.getUnboundFlag().id().toString()); - DefinedFlag.renderFlagDefinition(flagDefinition, definitionNode); - }); - mapper.writeValue(outputStream, rootNode); - } - - @Override - public String getContentType() { - return HttpConfigResponse.JSON_CONTENT_TYPE; - } -} diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/FlagDataListResponse.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/FlagDataListResponse.java deleted file mode 100644 index b33fc7c2b04..00000000000 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/FlagDataListResponse.java +++ /dev/null @@ -1,59 +0,0 @@ -// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.config.server.http.flags; - -import com.fasterxml.jackson.databind.ObjectMapper; -import com.fasterxml.jackson.databind.node.ArrayNode; -import com.fasterxml.jackson.databind.node.ObjectNode; -import com.yahoo.container.jdisc.HttpResponse; -import com.yahoo.jdisc.Response; -import com.yahoo.vespa.config.server.http.HttpConfigResponse; -import com.yahoo.vespa.flags.FlagId; -import com.yahoo.vespa.flags.json.FlagData; -import com.yahoo.vespa.flags.json.wire.WireFlagDataList; - -import java.io.OutputStream; -import java.util.Map; -import java.util.TreeMap; - -import static com.yahoo.yolean.Exceptions.uncheck; - -/** - * @author hakonhall - */ -public class FlagDataListResponse extends HttpResponse { - private static ObjectMapper mapper = new ObjectMapper(); - - private final String flagsV1Uri; - private final TreeMap flags; - private final boolean recursive; - - public FlagDataListResponse(String flagsV1Uri, Map flags, boolean recursive) { - super(Response.Status.OK); - this.flagsV1Uri = flagsV1Uri; - this.flags = new TreeMap<>(flags); - this.recursive = recursive; - } - - @Override - public void render(OutputStream outputStream) { - if (recursive) { - WireFlagDataList list = new WireFlagDataList(); - flags.values().forEach(flagData -> list.flags.add(flagData.toWire())); - list.serializeToOutputStream(outputStream); - } else { - ObjectNode rootNode = mapper.createObjectNode(); - ArrayNode flagsArray = rootNode.putArray("flags"); - flags.forEach((flagId, flagData) -> { - ObjectNode object = flagsArray.addObject(); - object.put("id", flagId.toString()); - object.put("url", flagsV1Uri + "/data/" + flagId.toString()); - }); - uncheck(() -> mapper.writeValue(outputStream, rootNode)); - } - } - - @Override - public String getContentType() { - return HttpConfigResponse.JSON_CONTENT_TYPE; - } -} diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/FlagDataResponse.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/FlagDataResponse.java deleted file mode 100644 index 054b218ff2d..00000000000 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/FlagDataResponse.java +++ /dev/null @@ -1,31 +0,0 @@ -// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.config.server.http.flags; - -import com.yahoo.container.jdisc.HttpResponse; -import com.yahoo.jdisc.Response; -import com.yahoo.vespa.config.server.http.HttpConfigResponse; -import com.yahoo.vespa.flags.json.FlagData; - -import java.io.OutputStream; - -/** - * @author hakonhall - */ -public class FlagDataResponse extends HttpResponse { - private final FlagData data; - - FlagDataResponse(FlagData data) { - super(Response.Status.OK); - this.data = data; - } - - @Override - public void render(OutputStream outputStream) { - data.serializeToOutputStream(outputStream); - } - - @Override - public String getContentType() { - return HttpConfigResponse.JSON_CONTENT_TYPE; - } -} diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/FlagsHandler.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/FlagsHandler.java deleted file mode 100644 index 00f3d457d3d..00000000000 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/FlagsHandler.java +++ /dev/null @@ -1,120 +0,0 @@ -// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.config.server.http.flags; - -import com.google.inject.Inject; -import com.yahoo.container.jdisc.HttpRequest; -import com.yahoo.container.jdisc.HttpResponse; -import com.yahoo.container.jdisc.LoggingRequestHandler; -import com.yahoo.restapi.Path; -import com.yahoo.vespa.config.server.http.HttpErrorResponse; -import com.yahoo.vespa.config.server.http.HttpHandler; -import com.yahoo.vespa.config.server.http.NotFoundException; -import com.yahoo.vespa.configserver.flags.FlagsDb; -import com.yahoo.vespa.flags.FlagDefinition; -import com.yahoo.vespa.flags.FlagId; -import com.yahoo.vespa.flags.Flags; -import com.yahoo.vespa.flags.json.FlagData; -import com.yahoo.yolean.Exceptions; - -import java.io.UncheckedIOException; -import java.net.URI; -import java.util.Objects; - -/** - * Handles /flags/v1 requests - * - * @author hakonhall - */ -public class FlagsHandler extends HttpHandler { - private final FlagsDb flagsDb; - - @Inject - public FlagsHandler(LoggingRequestHandler.Context context, FlagsDb flagsDb) { - super(context); - this.flagsDb = flagsDb; - } - - @Override - protected HttpResponse handleGET(HttpRequest request) { - Path path = new Path(request.getUri()); - if (path.matches("/flags/v1")) return new V1Response(flagsV1Uri(request), "data", "defined"); - if (path.matches("/flags/v1/data")) return getFlagDataList(request); - if (path.matches("/flags/v1/data/{flagId}")) return getFlagData(findFlagId(request, path)); - if (path.matches("/flags/v1/defined")) return new DefinedFlags(Flags.getAllFlags()); - if (path.matches("/flags/v1/defined/{flagId}")) return getDefinedFlag(findFlagId(request, path)); - throw new NotFoundException("Nothing at path '" + path + "'"); - } - - @Override - protected HttpResponse handlePUT(HttpRequest request) { - Path path = new Path(request.getUri()); - if (path.matches("/flags/v1/data/{flagId}")) return putFlagData(request, findFlagId(request, path)); - throw new NotFoundException("Nothing at path '" + path + "'"); - } - - @Override - protected HttpResponse handleDELETE(HttpRequest request) { - Path path = new Path(request.getUri()); - if (path.matches("/flags/v1/data/{flagId}")) return deleteFlagData(findFlagId(request, path)); - throw new NotFoundException("Nothing at path '" + path + "'"); - } - - private String flagsV1Uri(HttpRequest request) { - URI uri = request.getUri(); - String port = uri.getPort() < 0 ? "" : ":" + uri.getPort(); - return uri.getScheme() + "://" + uri.getHost() + port + "/flags/v1"; - } - - private HttpResponse getDefinedFlag(FlagId flagId) { - FlagDefinition definition = Flags.getFlag(flagId) - .orElseThrow(() -> new NotFoundException("Flag " + flagId + " not defined")); - return new DefinedFlag(definition); - } - - private HttpResponse getFlagDataList(HttpRequest request) { - return new FlagDataListResponse(flagsV1Uri(request), flagsDb.getAllFlags(), - Objects.equals(request.getProperty("recursive"), "true")); - } - - private HttpResponse getFlagData(FlagId flagId) { - FlagData data = flagsDb.getValue(flagId).orElseThrow(() -> new NotFoundException("Flag " + flagId + " not set")); - return new FlagDataResponse(data); - } - - private HttpResponse putFlagData(HttpRequest request, FlagId flagId) { - FlagData data; - try { - data = FlagData.deserialize(request.getData()); - } catch (UncheckedIOException e) { - return HttpErrorResponse.badRequest("Failed to deserialize request data: " + Exceptions.toMessageString(e)); - } - - if (!isForce(request)) { - FlagDefinition definition = Flags.getFlag(flagId).get(); // FlagId has been validated in findFlagId() - data.validate(definition.getUnboundFlag().serializer()); - } - - flagsDb.setValue(flagId, data); - return new OKResponse(); - } - - private HttpResponse deleteFlagData(FlagId flagId) { - flagsDb.removeValue(flagId); - return new OKResponse(); - } - - private FlagId findFlagId(HttpRequest request, Path path) { - FlagId flagId = new FlagId(path.get("flagId")); - - if (!isForce(request)) { - Flags.getFlag(flagId).orElseThrow(() -> - new NotFoundException("There is no flag '" + flagId + "' (use ?force=true to override)")); - } - - return flagId; - } - - private boolean isForce(HttpRequest request) { - return Objects.equals(request.getProperty("force"), "true"); - } -} diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/OKResponse.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/OKResponse.java deleted file mode 100644 index 87c02ae56f1..00000000000 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/OKResponse.java +++ /dev/null @@ -1,20 +0,0 @@ -// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.config.server.http.flags; - -import com.yahoo.container.jdisc.EmptyResponse; -import com.yahoo.jdisc.Response; -import com.yahoo.vespa.config.server.http.HttpConfigResponse; - -/** - * @author hakonhall - */ -public class OKResponse extends EmptyResponse { - public OKResponse() { - super(Response.Status.OK); - } - - @Override - public String getContentType() { - return HttpConfigResponse.JSON_CONTENT_TYPE; - } -} diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/V1Response.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/V1Response.java deleted file mode 100644 index 3594c801ca8..00000000000 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/V1Response.java +++ /dev/null @@ -1,34 +0,0 @@ -// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.config.server.http.flags; - -import com.yahoo.jdisc.Response; -import com.yahoo.slime.Cursor; -import com.yahoo.slime.Slime; -import com.yahoo.text.Utf8; -import com.yahoo.vespa.config.SlimeUtils; -import com.yahoo.vespa.config.server.http.HttpConfigResponse; -import com.yahoo.vespa.config.server.http.StaticResponse; - -import java.util.Arrays; -import java.util.List; - -import static com.yahoo.yolean.Exceptions.uncheck; - -/** - * @author hakonhall - */ -public class V1Response extends StaticResponse { - public V1Response(String flagsV1Uri, String... names) { - super(Response.Status.OK, HttpConfigResponse.JSON_CONTENT_TYPE, generateBody(flagsV1Uri, Arrays.asList(names))); - } - - private static String generateBody(String flagsV1Uri, List names) { - Slime slime = new Slime(); - Cursor root = slime.setObject(); - names.forEach(name -> { - Cursor data = root.setObject(name); - data.setString("url", flagsV1Uri + "/" + name); - }); - return Utf8.toString(uncheck(() -> SlimeUtils.toJsonBytes(slime))); - } -} diff --git a/configserver/src/main/resources/configserver-app/services.xml b/configserver/src/main/resources/configserver-app/services.xml index cfacd6ff8c9..b429e220c33 100644 --- a/configserver/src/main/resources/configserver-app/services.xml +++ b/configserver/src/main/resources/configserver-app/services.xml @@ -54,7 +54,7 @@ - + @@ -92,10 +92,6 @@ http://*/status - - http://*/flags/v1 - http://*/flags/v1/* - http://*/application/v2/tenant/ http://*/application/v2/tenant/* diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/flags/FlagsHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/flags/FlagsHandlerTest.java deleted file mode 100644 index 5ae6ce9820b..00000000000 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/flags/FlagsHandlerTest.java +++ /dev/null @@ -1,200 +0,0 @@ -// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.config.server.http.flags; - -import com.yahoo.container.jdisc.HttpRequest; -import com.yahoo.container.jdisc.HttpResponse; -import com.yahoo.jdisc.http.HttpRequest.Method; -import com.yahoo.text.Utf8; -import com.yahoo.vespa.config.server.http.SessionHandlerTest; -import com.yahoo.vespa.configserver.flags.db.FlagsDbImpl; -import com.yahoo.vespa.curator.mock.MockCurator; -import com.yahoo.vespa.flags.FetchVector; -import com.yahoo.vespa.flags.FlagId; -import com.yahoo.vespa.flags.Flags; -import com.yahoo.vespa.flags.UnboundBooleanFlag; -import org.junit.Test; - -import java.io.ByteArrayInputStream; -import java.io.InputStream; -import java.util.stream.Collectors; -import java.util.stream.Stream; - -import static com.yahoo.yolean.Exceptions.uncheck; -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.is; -import static org.junit.Assert.assertEquals; - -/** - * @author hakonhall - */ -public class FlagsHandlerTest { - private static final UnboundBooleanFlag FLAG1 = Flags.defineFeatureFlag( - "id1", false, "desc1", "mod1"); - private static final UnboundBooleanFlag FLAG2 = Flags.defineFeatureFlag( - "id2", true, "desc2", "mod2", - FetchVector.Dimension.HOSTNAME, FetchVector.Dimension.APPLICATION_ID); - - private static final String FLAGS_V1_URL = "https://foo.com:4443/flags/v1"; - - private final FlagsDbImpl flagsDb = new FlagsDbImpl(new MockCurator()); - private final FlagsHandler handler = new FlagsHandler(FlagsHandler.testOnlyContext(), flagsDb); - - @Test - public void testV1() { - String expectedResponse = "{" + - Stream.of("data", "defined") - .map(name -> "\"" + name + "\":{\"url\":\"https://foo.com:4443/flags/v1/" + name + "\"}") - .collect(Collectors.joining(",")) + - "}"; - verifySuccessfulRequest(Method.GET, "", "", expectedResponse); - verifySuccessfulRequest(Method.GET, "/", "", expectedResponse); - } - - @Test - public void testDefined() { - try (Flags.Replacer replacer = Flags.clearFlagsForTesting()) { - fixUnusedWarning(replacer); - Flags.defineFeatureFlag("id", false, "desc", "mod", FetchVector.Dimension.HOSTNAME); - verifySuccessfulRequest(Method.GET, "/defined", "", - "{\"id\":{\"description\":\"desc\",\"modification-effect\":\"mod\",\"dimensions\":[\"hostname\"]}}"); - - verifySuccessfulRequest(Method.GET, "/defined/id", "", - "{\"description\":\"desc\",\"modification-effect\":\"mod\",\"dimensions\":[\"hostname\"]}"); - } - } - - private void fixUnusedWarning(Flags.Replacer replacer) { } - - @Test - public void testData() { - // PUT flag with ID id1 - verifySuccessfulRequest(Method.PUT, "/data/" + FLAG1.id(), - "{\n" + - " \"id\": \"id1\",\n" + - " \"rules\": [\n" + - " {\n" + - " \"value\": true\n" + - " }\n" + - " ]\n" + - "}", - ""); - - // GET on ID id1 should return the same as the put. - verifySuccessfulRequest(Method.GET, "/data/" + FLAG1.id(), - "", "{\"id\":\"id1\",\"rules\":[{\"value\":true}]}"); - - // List all flags should list only id1 - verifySuccessfulRequest(Method.GET, "/data", - "", "{\"flags\":[{\"id\":\"id1\",\"url\":\"https://foo.com:4443/flags/v1/data/id1\"}]}"); - - // Should be identical to above: suffix / on path should be ignored - verifySuccessfulRequest(Method.GET, "/data/", - "", "{\"flags\":[{\"id\":\"id1\",\"url\":\"https://foo.com:4443/flags/v1/data/id1\"}]}"); - - // Verify absent port => absent in response - assertThat(handleWithPort(Method.GET, -1, "/data", "", 200), - is("{\"flags\":[{\"id\":\"id1\",\"url\":\"https://foo.com/flags/v1/data/id1\"}]}")); - - // PUT id2 - verifySuccessfulRequest(Method.PUT, "/data/" + FLAG2.id(), - "{\n" + - " \"id\": \"id2\",\n" + - " \"rules\": [\n" + - " {\n" + - " \"conditions\": [\n" + - " {\n" + - " \"type\": \"whitelist\",\n" + - " \"dimension\": \"hostname\",\n" + - " \"values\": [ \"host1\", \"host2\" ]\n" + - " },\n" + - " {\n" + - " \"type\": \"blacklist\",\n" + - " \"dimension\": \"application\",\n" + - " \"values\": [ \"app1\", \"app2\" ]\n" + - " }\n" + - " ],\n" + - " \"value\": true\n" + - " }\n" + - " ],\n" + - " \"attributes\": {\n" + - " \"zone\": \"zone1\"\n" + - " }\n" + - "}\n", - ""); - - // GET on id2 should now return what was put - verifySuccessfulRequest(Method.GET, "/data/" + FLAG2.id(), "", - "{\"id\":\"id2\",\"rules\":[{\"conditions\":[{\"type\":\"whitelist\",\"dimension\":\"hostname\",\"values\":[\"host1\",\"host2\"]},{\"type\":\"blacklist\",\"dimension\":\"application\",\"values\":[\"app1\",\"app2\"]}],\"value\":true}],\"attributes\":{\"zone\":\"zone1\"}}"); - - // The list of flag data should return id1 and id2 - verifySuccessfulRequest(Method.GET, "/data", - "", - "{\"flags\":[{\"id\":\"id1\",\"url\":\"https://foo.com:4443/flags/v1/data/id1\"},{\"id\":\"id2\",\"url\":\"https://foo.com:4443/flags/v1/data/id2\"}]}"); - - // Putting (overriding) id1 should work silently - verifySuccessfulRequest(Method.PUT, "/data/" + FLAG1.id(), - "{\n" + - " \"id\": \"id1\",\n" + - " \"rules\": [\n" + - " {\n" + - " \"value\": false\n" + - " }\n" + - " ]\n" + - "}\n", - ""); - - // Verify PUT - verifySuccessfulRequest(Method.GET, "/data/" + FLAG1.id(), "", "{\"id\":\"id1\",\"rules\":[{\"value\":false}]}"); - - // Get all recursivelly displays all flag data - verifySuccessfulRequest(Method.GET, "/data?recursive=true", "", - "{\"flags\":[{\"id\":\"id1\",\"rules\":[{\"value\":false}]},{\"id\":\"id2\",\"rules\":[{\"conditions\":[{\"type\":\"whitelist\",\"dimension\":\"hostname\",\"values\":[\"host1\",\"host2\"]},{\"type\":\"blacklist\",\"dimension\":\"application\",\"values\":[\"app1\",\"app2\"]}],\"value\":true}],\"attributes\":{\"zone\":\"zone1\"}}]}"); - - // Deleting both flags - verifySuccessfulRequest(Method.DELETE, "/data/" + FLAG1.id(), "", ""); - verifySuccessfulRequest(Method.DELETE, "/data/" + FLAG2.id(), "", ""); - - // And the list of data flags should now be empty - verifySuccessfulRequest(Method.GET, "/data", "", "{\"flags\":[]}"); - } - - @Test - public void testForcing() { - assertThat(handle(Method.PUT, "/data/" + new FlagId("undef"), "", 404), - containsString("There is no flag 'undef'")); - - assertThat(handle(Method.PUT, "/data/" + new FlagId("undef") + "?force=true", "", 400), - containsString("No content to map due to end-of-input")); - - assertThat(handle(Method.PUT, "/data/" + FLAG1.id(), "{}", 400), - containsString("Flag ID missing")); - - assertThat(handle(Method.PUT, "/data/" + FLAG1.id(), "{\"id\": \"id1\",\"rules\": [{\"value\":\"string\"}]}", 400), - containsString("Wrong type of JsonNode: STRING")); - - assertThat(handle(Method.PUT, "/data/" + FLAG1.id() + "?force=true", "{\"id\": \"id1\",\"rules\": [{\"value\":\"string\"}]}", 200), - is("")); - } - - private void verifySuccessfulRequest(Method method, String pathSuffix, String requestBody, String expectedResponseBody) { - assertThat(handle(method, pathSuffix, requestBody, 200), is(expectedResponseBody)); - } - - private String handle(Method method, String pathSuffix, String requestBody, int expectedStatus) { - return handleWithPort(method, 4443, pathSuffix, requestBody, expectedStatus); - } - - private String handleWithPort(Method method, int port, String pathSuffix, String requestBody, int expectedStatus) { - String uri = "https://foo.com" + (port < 0 ? "" : ":" + port) + "/flags/v1" + pathSuffix; - HttpRequest request = HttpRequest.createTestRequest(uri, method, makeInputStream(requestBody)); - HttpResponse response = handler.handle(request); - assertEquals(expectedStatus, response.getStatus()); - assertEquals("application/json", response.getContentType()); - return uncheck(() -> SessionHandlerTest.getRenderedString(response)); - } - - private InputStream makeInputStream(String content) { - return new ByteArrayInputStream(Utf8.toBytes(content)); - } -} \ No newline at end of file diff --git a/flags/pom.xml b/flags/pom.xml index c1e9eca20ab..7ef082cc1bc 100644 --- a/flags/pom.xml +++ b/flags/pom.xml @@ -59,7 +59,18 @@ no_aop provided - + + com.yahoo.vespa + container-dev + ${project.version} + provided + + + com.yahoo.vespa + zkfacade + ${project.version} + provided + junit junit diff --git a/flags/src/main/java/com/yahoo/vespa/flags/http/DefinedFlag.java b/flags/src/main/java/com/yahoo/vespa/flags/http/DefinedFlag.java new file mode 100644 index 00000000000..8234e9df725 --- /dev/null +++ b/flags/src/main/java/com/yahoo/vespa/flags/http/DefinedFlag.java @@ -0,0 +1,47 @@ +// Copyright 2019 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.flags.http; + +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.node.ArrayNode; +import com.fasterxml.jackson.databind.node.ObjectNode; +import com.yahoo.container.jdisc.HttpResponse; +import com.yahoo.jdisc.Response; +import com.yahoo.vespa.flags.FlagDefinition; +import com.yahoo.vespa.flags.json.DimensionHelper; + +import java.io.IOException; +import java.io.OutputStream; + +/** + * @author hakonhall + */ +public class DefinedFlag extends HttpResponse { + private static ObjectMapper mapper = new ObjectMapper(); + + private final FlagDefinition flagDefinition; + + public DefinedFlag(FlagDefinition flagDefinition) { + super(Response.Status.OK); + this.flagDefinition = flagDefinition; + } + + @Override + public void render(OutputStream outputStream) throws IOException { + ObjectNode rootNode = mapper.createObjectNode(); + renderFlagDefinition(flagDefinition, rootNode); + mapper.writeValue(outputStream, rootNode); + } + + static void renderFlagDefinition(FlagDefinition flagDefinition, ObjectNode definitionNode) { + definitionNode.put("description", flagDefinition.getDescription()); + definitionNode.put("modification-effect", flagDefinition.getModificationEffect()); + ArrayNode dimensionsNode = definitionNode.putArray("dimensions"); + flagDefinition.getDimensions().forEach(dimension -> dimensionsNode.add(DimensionHelper.toWire(dimension))); + } + + @Override + public String getContentType() { + return "application/json"; + } + +} diff --git a/flags/src/main/java/com/yahoo/vespa/flags/http/DefinedFlags.java b/flags/src/main/java/com/yahoo/vespa/flags/http/DefinedFlags.java new file mode 100644 index 00000000000..e1db7dda6e0 --- /dev/null +++ b/flags/src/main/java/com/yahoo/vespa/flags/http/DefinedFlags.java @@ -0,0 +1,43 @@ +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.flags.http; + +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.node.ObjectNode; +import com.yahoo.container.jdisc.HttpResponse; +import com.yahoo.jdisc.Response; +import com.yahoo.vespa.flags.FlagDefinition; + +import java.io.IOException; +import java.io.OutputStream; +import java.util.Comparator; +import java.util.List; + +/** + * @author hakonhall + */ +public class DefinedFlags extends HttpResponse { + private static ObjectMapper mapper = new ObjectMapper(); + private static final Comparator sortByFlagId = Comparator.comparing(flagDefinition -> flagDefinition.getUnboundFlag().id()); + + private final List flags; + + public DefinedFlags(List flags) { + super(Response.Status.OK); + this.flags = flags; + } + + @Override + public void render(OutputStream outputStream) throws IOException { + ObjectNode rootNode = mapper.createObjectNode(); + flags.stream().sorted(sortByFlagId).forEach(flagDefinition -> { + ObjectNode definitionNode = rootNode.putObject(flagDefinition.getUnboundFlag().id().toString()); + DefinedFlag.renderFlagDefinition(flagDefinition, definitionNode); + }); + mapper.writeValue(outputStream, rootNode); + } + + @Override + public String getContentType() { + return "application/json"; + } +} diff --git a/flags/src/main/java/com/yahoo/vespa/flags/http/ErrorResponse.java b/flags/src/main/java/com/yahoo/vespa/flags/http/ErrorResponse.java new file mode 100644 index 00000000000..969903093a4 --- /dev/null +++ b/flags/src/main/java/com/yahoo/vespa/flags/http/ErrorResponse.java @@ -0,0 +1,66 @@ +// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.flags.http; + +import com.yahoo.slime.Cursor; +import com.yahoo.slime.Slime; + +import static com.yahoo.jdisc.Response.Status.BAD_REQUEST; +import static com.yahoo.jdisc.Response.Status.FORBIDDEN; +import static com.yahoo.jdisc.Response.Status.INTERNAL_SERVER_ERROR; +import static com.yahoo.jdisc.Response.Status.METHOD_NOT_ALLOWED; +import static com.yahoo.jdisc.Response.Status.NOT_FOUND; +import static com.yahoo.jdisc.Response.Status.UNAUTHORIZED; + +/** + * A HTTP JSON response containing an error code and a message + * + * @author bratseth + */ +public class ErrorResponse extends SlimeJsonResponse { + + public enum errorCodes { + NOT_FOUND, + BAD_REQUEST, + FORBIDDEN, + METHOD_NOT_ALLOWED, + INTERNAL_SERVER_ERROR, + UNAUTHORIZED + } + + public ErrorResponse(int statusCode, String errorType, String message) { + super(statusCode, asSlimeMessage(errorType, message)); + } + + private static Slime asSlimeMessage(String errorType, String message) { + Slime slime = new Slime(); + Cursor root = slime.setObject(); + root.setString("error-code", errorType); + root.setString("message", message); + return slime; + } + + public static ErrorResponse notFoundError(String message) { + return new ErrorResponse(NOT_FOUND, errorCodes.NOT_FOUND.name(), message); + } + + public static ErrorResponse internalServerError(String message) { + return new ErrorResponse(INTERNAL_SERVER_ERROR, errorCodes.INTERNAL_SERVER_ERROR.name(), message); + } + + public static ErrorResponse badRequest(String message) { + return new ErrorResponse(BAD_REQUEST, errorCodes.BAD_REQUEST.name(), message); + } + + public static ErrorResponse forbidden(String message) { + return new ErrorResponse(FORBIDDEN, errorCodes.FORBIDDEN.name(), message); + } + + public static ErrorResponse unauthorized(String message) { + return new ErrorResponse(UNAUTHORIZED, errorCodes.UNAUTHORIZED.name(), message); + } + + public static ErrorResponse methodNotAllowed(String message) { + return new ErrorResponse(METHOD_NOT_ALLOWED, errorCodes.METHOD_NOT_ALLOWED.name(), message); + } + +} diff --git a/flags/src/main/java/com/yahoo/vespa/flags/http/FlagDataListResponse.java b/flags/src/main/java/com/yahoo/vespa/flags/http/FlagDataListResponse.java new file mode 100644 index 00000000000..5af97007997 --- /dev/null +++ b/flags/src/main/java/com/yahoo/vespa/flags/http/FlagDataListResponse.java @@ -0,0 +1,58 @@ +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.flags.http; + +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.node.ArrayNode; +import com.fasterxml.jackson.databind.node.ObjectNode; +import com.yahoo.container.jdisc.HttpResponse; +import com.yahoo.jdisc.Response; +import com.yahoo.vespa.flags.FlagId; +import com.yahoo.vespa.flags.json.FlagData; +import com.yahoo.vespa.flags.json.wire.WireFlagDataList; + +import java.io.OutputStream; +import java.util.Map; +import java.util.TreeMap; + +import static com.yahoo.yolean.Exceptions.uncheck; + +/** + * @author hakonhall + */ +public class FlagDataListResponse extends HttpResponse { + private static ObjectMapper mapper = new ObjectMapper(); + + private final String flagsV1Uri; + private final TreeMap flags; + private final boolean recursive; + + public FlagDataListResponse(String flagsV1Uri, Map flags, boolean recursive) { + super(Response.Status.OK); + this.flagsV1Uri = flagsV1Uri; + this.flags = new TreeMap<>(flags); + this.recursive = recursive; + } + + @Override + public void render(OutputStream outputStream) { + if (recursive) { + WireFlagDataList list = new WireFlagDataList(); + flags.values().forEach(flagData -> list.flags.add(flagData.toWire())); + list.serializeToOutputStream(outputStream); + } else { + ObjectNode rootNode = mapper.createObjectNode(); + ArrayNode flagsArray = rootNode.putArray("flags"); + flags.forEach((flagId, flagData) -> { + ObjectNode object = flagsArray.addObject(); + object.put("id", flagId.toString()); + object.put("url", flagsV1Uri + "/data/" + flagId.toString()); + }); + uncheck(() -> mapper.writeValue(outputStream, rootNode)); + } + } + + @Override + public String getContentType() { + return "application/json"; + } +} diff --git a/flags/src/main/java/com/yahoo/vespa/flags/http/FlagDataResponse.java b/flags/src/main/java/com/yahoo/vespa/flags/http/FlagDataResponse.java new file mode 100644 index 00000000000..f6e81e030c7 --- /dev/null +++ b/flags/src/main/java/com/yahoo/vespa/flags/http/FlagDataResponse.java @@ -0,0 +1,30 @@ +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.flags.http; + +import com.yahoo.container.jdisc.HttpResponse; +import com.yahoo.jdisc.Response; +import com.yahoo.vespa.flags.json.FlagData; + +import java.io.OutputStream; + +/** + * @author hakonhall + */ +public class FlagDataResponse extends HttpResponse { + private final FlagData data; + + FlagDataResponse(FlagData data) { + super(Response.Status.OK); + this.data = data; + } + + @Override + public void render(OutputStream outputStream) { + data.serializeToOutputStream(outputStream); + } + + @Override + public String getContentType() { + return "application/json"; + } +} diff --git a/flags/src/main/java/com/yahoo/vespa/flags/http/FlagsHandler.java b/flags/src/main/java/com/yahoo/vespa/flags/http/FlagsHandler.java new file mode 100644 index 00000000000..76f74cbe931 --- /dev/null +++ b/flags/src/main/java/com/yahoo/vespa/flags/http/FlagsHandler.java @@ -0,0 +1,138 @@ +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.flags.http; + +import com.google.inject.Inject; +import com.yahoo.container.jdisc.HttpRequest; +import com.yahoo.container.jdisc.HttpResponse; +import com.yahoo.container.jdisc.LoggingRequestHandler; +import com.yahoo.log.LogLevel; +import com.yahoo.restapi.Path; +import com.yahoo.vespa.flags.FlagDefinition; +import com.yahoo.vespa.flags.FlagId; +import com.yahoo.vespa.flags.Flags; +import com.yahoo.vespa.flags.json.FlagData; +import com.yahoo.vespa.flags.persistence.FlagsDb; +import com.yahoo.yolean.Exceptions; + +import java.io.UncheckedIOException; +import java.net.URI; +import java.util.Objects; + +/** + * Handles /flags/v1 requests + * + * @author hakonhall + */ +public class FlagsHandler extends LoggingRequestHandler { + + private final FlagsDb flagsDb; + + @Inject + public FlagsHandler(LoggingRequestHandler.Context context, FlagsDb flagsDb) { + super(context); + this.flagsDb = flagsDb; + } + + @Override + public HttpResponse handle(HttpRequest request) { + try { + switch (request.getMethod()) { + case GET: return handleGET(request); + case DELETE: return handleDELETE(request); + case PUT: return handlePUT(request); + default: return ErrorResponse.methodNotAllowed("Method '" + request.getMethod() + "' is not supported"); + } + } + catch (IllegalArgumentException e) { + return ErrorResponse.badRequest(Exceptions.toMessageString(e)); + } + catch (RuntimeException e) { + log.log(LogLevel.WARNING, "Unexpected error handling '" + request.getUri() + "'", e); + return ErrorResponse.internalServerError(Exceptions.toMessageString(e)); + } + } + + private HttpResponse handleGET(HttpRequest request) { + Path path = new Path(request.getUri()); + if (path.matches("/flags/v1")) return new V1Response(flagsV1Uri(request), "data", "defined"); + if (path.matches("/flags/v1/data")) return getFlagDataList(request); + if (path.matches("/flags/v1/data/{flagId}")) return getFlagData(findFlagId(request, path)); + if (path.matches("/flags/v1/defined")) return new DefinedFlags(Flags.getAllFlags()); + if (path.matches("/flags/v1/defined/{flagId}")) return getDefinedFlag(findFlagId(request, path)); + return ErrorResponse.notFoundError("Nothing at path '" + path + "'"); + } + + private HttpResponse handlePUT(HttpRequest request) { + Path path = new Path(request.getUri()); + if (path.matches("/flags/v1/data/{flagId}")) return putFlagData(request, findFlagId(request, path)); + return ErrorResponse.notFoundError("Nothing at path '" + path + "'"); + } + + private HttpResponse handleDELETE(HttpRequest request) { + Path path = new Path(request.getUri()); + if (path.matches("/flags/v1/data/{flagId}")) return deleteFlagData(findFlagId(request, path)); + return ErrorResponse.notFoundError("Nothing at path '" + path + "'"); + } + + private String flagsV1Uri(HttpRequest request) { + URI uri = request.getUri(); + String port = uri.getPort() < 0 ? "" : ":" + uri.getPort(); + return uri.getScheme() + "://" + uri.getHost() + port + "/flags/v1"; + } + + private HttpResponse getDefinedFlag(FlagId flagId) { + var definedFlag = Flags.getFlag(flagId).map(DefinedFlag::new); + if (definedFlag.isPresent()) { + return definedFlag.get(); + } + return ErrorResponse.notFoundError("Flag " + flagId + " not defined"); + } + + private HttpResponse getFlagDataList(HttpRequest request) { + return new FlagDataListResponse(flagsV1Uri(request), flagsDb.getAllFlags(), + Objects.equals(request.getProperty("recursive"), "true")); + } + + private HttpResponse getFlagData(FlagId flagId) { + var data = flagsDb.getValue(flagId).map(FlagDataResponse::new); + if (data.isPresent()) { + return data.get(); + } + return ErrorResponse.notFoundError("Flag " + flagId + " not set"); + } + + private HttpResponse putFlagData(HttpRequest request, FlagId flagId) { + FlagData data; + try { + data = FlagData.deserialize(request.getData()); + } catch (UncheckedIOException e) { + return ErrorResponse.badRequest("Failed to deserialize request data: " + Exceptions.toMessageString(e)); + } + + if (!isForce(request)) { + FlagDefinition definition = Flags.getFlag(flagId).get(); // FlagId has been validated in findFlagId() + data.validate(definition.getUnboundFlag().serializer()); + } + + flagsDb.setValue(flagId, data); + return new OKResponse(); + } + + private HttpResponse deleteFlagData(FlagId flagId) { + flagsDb.removeValue(flagId); + return new OKResponse(); + } + + private FlagId findFlagId(HttpRequest request, Path path) { + FlagId flagId = new FlagId(path.get("flagId")); + if (!isForce(request) && Flags.getFlag(flagId).isEmpty()) { + throw new IllegalArgumentException("There is no flag '" + flagId + "' (use ?force=true to override)"); + } + return flagId; + } + + private boolean isForce(HttpRequest request) { + return Objects.equals(request.getProperty("force"), "true"); + } + +} diff --git a/flags/src/main/java/com/yahoo/vespa/flags/http/OKResponse.java b/flags/src/main/java/com/yahoo/vespa/flags/http/OKResponse.java new file mode 100644 index 00000000000..d094e2d5734 --- /dev/null +++ b/flags/src/main/java/com/yahoo/vespa/flags/http/OKResponse.java @@ -0,0 +1,19 @@ +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.flags.http; + +import com.yahoo.container.jdisc.EmptyResponse; +import com.yahoo.jdisc.Response; + +/** + * @author hakonhall + */ +public class OKResponse extends EmptyResponse { + public OKResponse() { + super(Response.Status.OK); + } + + @Override + public String getContentType() { + return "application/json"; + } +} diff --git a/flags/src/main/java/com/yahoo/vespa/flags/http/SlimeJsonResponse.java b/flags/src/main/java/com/yahoo/vespa/flags/http/SlimeJsonResponse.java new file mode 100644 index 00000000000..dd71795ae43 --- /dev/null +++ b/flags/src/main/java/com/yahoo/vespa/flags/http/SlimeJsonResponse.java @@ -0,0 +1,38 @@ +// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.flags.http; + +import com.yahoo.container.jdisc.HttpResponse; +import com.yahoo.slime.JsonFormat; +import com.yahoo.slime.Slime; + +import java.io.IOException; +import java.io.OutputStream; + +/** + * A generic Json response using Slime for JSON encoding + * + * @author bratseth + */ +public class SlimeJsonResponse extends HttpResponse { + + private final Slime slime; + + public SlimeJsonResponse(Slime slime) { + super(200); + this.slime = slime; + } + + public SlimeJsonResponse(int statusCode, Slime slime) { + super(statusCode); + this.slime = slime; + } + + @Override + public void render(OutputStream stream) throws IOException { + new JsonFormat(true).encode(stream, slime); + } + + @Override + public String getContentType() { return "application/json"; } + +} diff --git a/flags/src/main/java/com/yahoo/vespa/flags/http/V1Response.java b/flags/src/main/java/com/yahoo/vespa/flags/http/V1Response.java new file mode 100644 index 00000000000..e8ff0bd99a4 --- /dev/null +++ b/flags/src/main/java/com/yahoo/vespa/flags/http/V1Response.java @@ -0,0 +1,46 @@ +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.flags.http; + +import com.yahoo.container.jdisc.HttpResponse; +import com.yahoo.jdisc.Response; +import com.yahoo.slime.Cursor; +import com.yahoo.slime.JsonFormat; +import com.yahoo.slime.Slime; + +import java.io.IOException; +import java.io.OutputStream; +import java.util.List; + +/** + * @author hakonhall + */ +public class V1Response extends HttpResponse { + + private final Slime slime; + + public V1Response(String flagsV1Uri, String... names) { + super(Response.Status.OK); + this.slime = generateBody(flagsV1Uri, List.of(names)); + } + + @Override + public void render(OutputStream stream) throws IOException { + new JsonFormat(true).encode(stream, slime); + } + + @Override + public String getContentType() { + return "application/json"; + } + + private static Slime generateBody(String flagsV1Uri, List names) { + Slime slime = new Slime(); + Cursor root = slime.setObject(); + names.forEach(name -> { + Cursor data = root.setObject(name); + data.setString("url", flagsV1Uri + "/" + name); + }); + return slime; + } + +} diff --git a/flags/src/main/java/com/yahoo/vespa/flags/persistence/FlagsDb.java b/flags/src/main/java/com/yahoo/vespa/flags/persistence/FlagsDb.java new file mode 100644 index 00000000000..2ed762f2895 --- /dev/null +++ b/flags/src/main/java/com/yahoo/vespa/flags/persistence/FlagsDb.java @@ -0,0 +1,68 @@ +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.flags.persistence; + +import com.google.inject.Inject; +import com.yahoo.path.Path; +import com.yahoo.vespa.curator.Curator; +import com.yahoo.vespa.flags.FlagId; +import com.yahoo.vespa.flags.json.FlagData; +import org.apache.curator.framework.recipes.cache.ChildData; + +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.function.Function; +import java.util.stream.Collectors; + +/** + * @author hakonhall + */ +public class FlagsDb { + + private static final Path ROOT_PATH = Path.fromString("/flags/v1"); + + private final Curator curator; + private final Curator.DirectoryCache cache; + + @Inject + public FlagsDb(Curator curator) { + this.curator = curator; + curator.create(ROOT_PATH); + ExecutorService executorService = Executors.newFixedThreadPool(1); + this.cache = curator.createDirectoryCache(ROOT_PATH.getAbsolute(), true, false, executorService); + cache.start(); + } + + /** Get the String value of the flag. */ + public Optional getValue(FlagId flagId) { + return Optional.ofNullable(cache.getCurrentData(getZkPathFor(flagId))) + .map(ChildData::getData) + .map(FlagData::deserializeUtf8Json); + } + + /** Set the String value of the flag. */ + public void setValue(FlagId flagId, FlagData data) { + curator.set(getZkPathFor(flagId), data.serializeToUtf8Json()); + } + + /** Get all flags that have been set. */ + public Map getAllFlags() { + List dataList = cache.getCurrentData(); + return dataList.stream() + .map(ChildData::getData) + .map(FlagData::deserializeUtf8Json) + .collect(Collectors.toMap(FlagData::id, Function.identity())); + } + + /** Remove the flag value if it exists. */ + public void removeValue(FlagId flagId) { + curator.delete(getZkPathFor(flagId)); + } + + private static Path getZkPathFor(FlagId flagId) { + return ROOT_PATH.append(flagId.toString()); + } + +} diff --git a/flags/src/main/java/com/yahoo/vespa/flags/persistence/package-info.java b/flags/src/main/java/com/yahoo/vespa/flags/persistence/package-info.java new file mode 100644 index 00000000000..d4753ed1756 --- /dev/null +++ b/flags/src/main/java/com/yahoo/vespa/flags/persistence/package-info.java @@ -0,0 +1,8 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +/** + * @author mpolden + */ +@ExportPackage +package com.yahoo.vespa.flags.persistence; + +import com.yahoo.osgi.annotation.ExportPackage; diff --git a/flags/src/test/java/com/yahoo/vespa/flags/http/FlagsHandlerTest.java b/flags/src/test/java/com/yahoo/vespa/flags/http/FlagsHandlerTest.java new file mode 100644 index 00000000000..8ae1008ba22 --- /dev/null +++ b/flags/src/test/java/com/yahoo/vespa/flags/http/FlagsHandlerTest.java @@ -0,0 +1,203 @@ +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.flags.http; + +import com.yahoo.container.jdisc.HttpRequest; +import com.yahoo.container.jdisc.HttpResponse; +import com.yahoo.jdisc.http.HttpRequest.Method; +import com.yahoo.text.Utf8; +import com.yahoo.vespa.curator.mock.MockCurator; +import com.yahoo.vespa.flags.FetchVector; +import com.yahoo.vespa.flags.FlagId; +import com.yahoo.vespa.flags.Flags; +import com.yahoo.vespa.flags.UnboundBooleanFlag; +import com.yahoo.vespa.flags.persistence.FlagsDb; +import com.yahoo.yolean.Exceptions; +import org.junit.Test; + +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.InputStream; +import java.nio.charset.StandardCharsets; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.is; +import static org.junit.Assert.assertEquals; + +/** + * @author hakonhall + */ +public class FlagsHandlerTest { + private static final UnboundBooleanFlag FLAG1 = Flags.defineFeatureFlag( + "id1", false, "desc1", "mod1"); + private static final UnboundBooleanFlag FLAG2 = Flags.defineFeatureFlag( + "id2", true, "desc2", "mod2", + FetchVector.Dimension.HOSTNAME, FetchVector.Dimension.APPLICATION_ID); + + private static final String FLAGS_V1_URL = "https://foo.com:4443/flags/v1"; + + private final FlagsDb flagsDb = new FlagsDb(new MockCurator()); + private final FlagsHandler handler = new FlagsHandler(FlagsHandler.testOnlyContext(), flagsDb); + + @Test + public void testV1() { + String expectedResponse = "{" + + Stream.of("data", "defined") + .map(name -> "\"" + name + "\":{\"url\":\"https://foo.com:4443/flags/v1/" + name + "\"}") + .collect(Collectors.joining(",")) + + "}"; + verifySuccessfulRequest(Method.GET, "", "", expectedResponse); + verifySuccessfulRequest(Method.GET, "/", "", expectedResponse); + } + + @Test + public void testDefined() { + try (Flags.Replacer replacer = Flags.clearFlagsForTesting()) { + fixUnusedWarning(replacer); + Flags.defineFeatureFlag("id", false, "desc", "mod", FetchVector.Dimension.HOSTNAME); + verifySuccessfulRequest(Method.GET, "/defined", "", + "{\"id\":{\"description\":\"desc\",\"modification-effect\":\"mod\",\"dimensions\":[\"hostname\"]}}"); + + verifySuccessfulRequest(Method.GET, "/defined/id", "", + "{\"description\":\"desc\",\"modification-effect\":\"mod\",\"dimensions\":[\"hostname\"]}"); + } + } + + private void fixUnusedWarning(Flags.Replacer replacer) { } + + @Test + public void testData() { + // PUT flag with ID id1 + verifySuccessfulRequest(Method.PUT, "/data/" + FLAG1.id(), + "{\n" + + " \"id\": \"id1\",\n" + + " \"rules\": [\n" + + " {\n" + + " \"value\": true\n" + + " }\n" + + " ]\n" + + "}", + ""); + + // GET on ID id1 should return the same as the put. + verifySuccessfulRequest(Method.GET, "/data/" + FLAG1.id(), + "", "{\"id\":\"id1\",\"rules\":[{\"value\":true}]}"); + + // List all flags should list only id1 + verifySuccessfulRequest(Method.GET, "/data", + "", "{\"flags\":[{\"id\":\"id1\",\"url\":\"https://foo.com:4443/flags/v1/data/id1\"}]}"); + + // Should be identical to above: suffix / on path should be ignored + verifySuccessfulRequest(Method.GET, "/data/", + "", "{\"flags\":[{\"id\":\"id1\",\"url\":\"https://foo.com:4443/flags/v1/data/id1\"}]}"); + + // Verify absent port => absent in response + assertThat(handleWithPort(Method.GET, -1, "/data", "", 200), + is("{\"flags\":[{\"id\":\"id1\",\"url\":\"https://foo.com/flags/v1/data/id1\"}]}")); + + // PUT id2 + verifySuccessfulRequest(Method.PUT, "/data/" + FLAG2.id(), + "{\n" + + " \"id\": \"id2\",\n" + + " \"rules\": [\n" + + " {\n" + + " \"conditions\": [\n" + + " {\n" + + " \"type\": \"whitelist\",\n" + + " \"dimension\": \"hostname\",\n" + + " \"values\": [ \"host1\", \"host2\" ]\n" + + " },\n" + + " {\n" + + " \"type\": \"blacklist\",\n" + + " \"dimension\": \"application\",\n" + + " \"values\": [ \"app1\", \"app2\" ]\n" + + " }\n" + + " ],\n" + + " \"value\": true\n" + + " }\n" + + " ],\n" + + " \"attributes\": {\n" + + " \"zone\": \"zone1\"\n" + + " }\n" + + "}\n", + ""); + + // GET on id2 should now return what was put + verifySuccessfulRequest(Method.GET, "/data/" + FLAG2.id(), "", + "{\"id\":\"id2\",\"rules\":[{\"conditions\":[{\"type\":\"whitelist\",\"dimension\":\"hostname\",\"values\":[\"host1\",\"host2\"]},{\"type\":\"blacklist\",\"dimension\":\"application\",\"values\":[\"app1\",\"app2\"]}],\"value\":true}],\"attributes\":{\"zone\":\"zone1\"}}"); + + // The list of flag data should return id1 and id2 + verifySuccessfulRequest(Method.GET, "/data", + "", + "{\"flags\":[{\"id\":\"id1\",\"url\":\"https://foo.com:4443/flags/v1/data/id1\"},{\"id\":\"id2\",\"url\":\"https://foo.com:4443/flags/v1/data/id2\"}]}"); + + // Putting (overriding) id1 should work silently + verifySuccessfulRequest(Method.PUT, "/data/" + FLAG1.id(), + "{\n" + + " \"id\": \"id1\",\n" + + " \"rules\": [\n" + + " {\n" + + " \"value\": false\n" + + " }\n" + + " ]\n" + + "}\n", + ""); + + // Verify PUT + verifySuccessfulRequest(Method.GET, "/data/" + FLAG1.id(), "", "{\"id\":\"id1\",\"rules\":[{\"value\":false}]}"); + + // Get all recursivelly displays all flag data + verifySuccessfulRequest(Method.GET, "/data?recursive=true", "", + "{\"flags\":[{\"id\":\"id1\",\"rules\":[{\"value\":false}]},{\"id\":\"id2\",\"rules\":[{\"conditions\":[{\"type\":\"whitelist\",\"dimension\":\"hostname\",\"values\":[\"host1\",\"host2\"]},{\"type\":\"blacklist\",\"dimension\":\"application\",\"values\":[\"app1\",\"app2\"]}],\"value\":true}],\"attributes\":{\"zone\":\"zone1\"}}]}"); + + // Deleting both flags + verifySuccessfulRequest(Method.DELETE, "/data/" + FLAG1.id(), "", ""); + verifySuccessfulRequest(Method.DELETE, "/data/" + FLAG2.id(), "", ""); + + // And the list of data flags should now be empty + verifySuccessfulRequest(Method.GET, "/data", "", "{\"flags\":[]}"); + } + + @Test + public void testForcing() { + assertThat(handle(Method.PUT, "/data/" + new FlagId("undef"), "", 400), + containsString("There is no flag 'undef'")); + + assertThat(handle(Method.PUT, "/data/" + new FlagId("undef") + "?force=true", "", 400), + containsString("No content to map due to end-of-input")); + + assertThat(handle(Method.PUT, "/data/" + FLAG1.id(), "{}", 400), + containsString("Flag ID missing")); + + assertThat(handle(Method.PUT, "/data/" + FLAG1.id(), "{\"id\": \"id1\",\"rules\": [{\"value\":\"string\"}]}", 400), + containsString("Wrong type of JsonNode: STRING")); + + assertThat(handle(Method.PUT, "/data/" + FLAG1.id() + "?force=true", "{\"id\": \"id1\",\"rules\": [{\"value\":\"string\"}]}", 200), + is("")); + } + + private void verifySuccessfulRequest(Method method, String pathSuffix, String requestBody, String expectedResponseBody) { + assertThat(handle(method, pathSuffix, requestBody, 200), is(expectedResponseBody)); + } + + private String handle(Method method, String pathSuffix, String requestBody, int expectedStatus) { + return handleWithPort(method, 4443, pathSuffix, requestBody, expectedStatus); + } + + private String handleWithPort(Method method, int port, String pathSuffix, String requestBody, int expectedStatus) { + String uri = "https://foo.com" + (port < 0 ? "" : ":" + port) + "/flags/v1" + pathSuffix; + HttpRequest request = HttpRequest.createTestRequest(uri, method, makeInputStream(requestBody)); + HttpResponse response = handler.handle(request); + assertEquals(expectedStatus, response.getStatus()); + assertEquals("application/json", response.getContentType()); + var outputStream = new ByteArrayOutputStream(); + Exceptions.uncheck(() -> response.render(outputStream)); + return outputStream.toString(StandardCharsets.UTF_8); + } + + private InputStream makeInputStream(String content) { + return new ByteArrayInputStream(Utf8.toBytes(content)); + } +} diff --git a/flags/src/test/java/com/yahoo/vespa/flags/persistence/FlagsDbTest.java b/flags/src/test/java/com/yahoo/vespa/flags/persistence/FlagsDbTest.java new file mode 100644 index 00000000000..5102305af90 --- /dev/null +++ b/flags/src/test/java/com/yahoo/vespa/flags/persistence/FlagsDbTest.java @@ -0,0 +1,56 @@ +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.flags.persistence; + +import com.yahoo.vespa.curator.mock.MockCurator; +import com.yahoo.vespa.flags.FetchVector; +import com.yahoo.vespa.flags.FlagId; +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.util.Map; +import java.util.Optional; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.notNullValue; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +/** + * @author hakonhall + */ +public class FlagsDbTest { + @Test + public void test() { + MockCurator curator = new MockCurator(); + FlagsDb db = new FlagsDb(curator); + + Condition condition1 = new Condition(Condition.Type.WHITELIST, FetchVector.Dimension.HOSTNAME, "host1"); + Rule rule1 = new Rule(Optional.of(JsonNodeRawFlag.fromJson("13")), condition1); + FlagId flagId = new FlagId("id"); + FlagData data = new FlagData(flagId, new FetchVector().with(FetchVector.Dimension.ZONE_ID, "zone-a"), rule1); + db.setValue(flagId, data); + + Optional dataCopy = db.getValue(flagId); + assertTrue(dataCopy.isPresent()); + + assertEquals("{\"id\":\"id\",\"rules\":[{\"conditions\":[{\"type\":\"whitelist\",\"dimension\":\"hostname\"," + + "\"values\":[\"host1\"]}],\"value\":13}],\"attributes\":{\"zone\":\"zone-a\"}}", + dataCopy.get().serializeToJson()); + + FlagId flagId2 = new FlagId("id2"); + FlagData data2 = new FlagData(flagId2, new FetchVector().with(FetchVector.Dimension.ZONE_ID, "zone-a"), rule1); + db.setValue(flagId2, data2); + Map flags = db.getAllFlags(); + assertThat(flags.size(), equalTo(2)); + assertThat(flags.get(flagId), notNullValue()); + assertThat(flags.get(flagId2), notNullValue()); + + db.removeValue(flagId2); + assertFalse(db.getValue(flagId2).isPresent()); + } +} -- cgit v1.2.3 From ce05a635b0cadb7fecdc06a92d90ab6477ae529b Mon Sep 17 00:00:00 2001 From: Martin Polden Date: Fri, 12 Jul 2019 09:37:31 +0200 Subject: Audit log /flags/v1 on controller --- .../restapi/flags/AuditedFlagsHandler.java | 30 ++++++++++++ .../hosted/controller/restapi/ContainerTester.java | 1 - .../restapi/ControllerContainerTest.java | 6 +++ .../restapi/flags/AuditedFlagsApiTest.java | 57 ++++++++++++++++++++++ 4 files changed, 93 insertions(+), 1 deletion(-) create mode 100644 controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/flags/AuditedFlagsHandler.java create mode 100644 controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/flags/AuditedFlagsApiTest.java diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/flags/AuditedFlagsHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/flags/AuditedFlagsHandler.java new file mode 100644 index 00000000000..12b59db756f --- /dev/null +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/flags/AuditedFlagsHandler.java @@ -0,0 +1,30 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.controller.restapi.flags; + +import com.yahoo.container.jdisc.HttpRequest; +import com.yahoo.container.jdisc.HttpResponse; +import com.yahoo.vespa.flags.http.FlagsHandler; +import com.yahoo.vespa.flags.persistence.FlagsDb; +import com.yahoo.vespa.hosted.controller.Controller; +import com.yahoo.vespa.hosted.controller.auditlog.AuditLogger; + +/** + * An extension of {@link FlagsHandler} which logs requests to the audit log. + * + * @author mpolden + */ +public class AuditedFlagsHandler extends FlagsHandler { + + private final AuditLogger auditLogger; + + public AuditedFlagsHandler(Context context, Controller controller, FlagsDb flagsDb) { + super(context, flagsDb); + auditLogger = controller.auditLogger(); + } + + @Override + public HttpResponse handle(HttpRequest request) { + return super.handle(auditLogger.log(request)); + } + +} 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 c7be543dd00..b32cbbcb926 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 @@ -7,7 +7,6 @@ import com.yahoo.application.container.handler.Response; import com.yahoo.component.ComponentSpecification; import com.yahoo.component.Version; import com.yahoo.config.provision.zone.ZoneApi; -import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.container.http.filter.FilterChainRepository; import com.yahoo.jdisc.http.filter.SecurityRequestFilter; import com.yahoo.jdisc.http.filter.SecurityRequestFilterChain; diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ControllerContainerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ControllerContainerTest.java index 11aa132b478..eaafb08f0de 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ControllerContainerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ControllerContainerTest.java @@ -60,6 +60,8 @@ public class ControllerContainerTest { " \n" + " \n" + " \n" + + " \n" + + " \n" + " \n" + " \n" + " \n" + @@ -112,6 +114,10 @@ public class ControllerContainerTest { " http://*/zone/v2\n" + " http://*/zone/v2/*\n" + " \n" + + " \n" + + " http://*/flags/v1\n" + + " http://*/flags/v1/*\n" + + " \n" + variablePartXml() + ""; } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/flags/AuditedFlagsApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/flags/AuditedFlagsApiTest.java new file mode 100644 index 00000000000..b4ef98cc7f6 --- /dev/null +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/flags/AuditedFlagsApiTest.java @@ -0,0 +1,57 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.controller.restapi.flags; + +import com.yahoo.application.container.handler.Request; +import com.yahoo.vespa.athenz.api.AthenzIdentity; +import com.yahoo.vespa.athenz.api.AthenzUser; +import com.yahoo.vespa.hosted.controller.auditlog.AuditLog; +import com.yahoo.vespa.hosted.controller.restapi.ContainerControllerTester; +import com.yahoo.vespa.hosted.controller.restapi.ControllerContainerTest; +import org.junit.Before; +import org.junit.Test; + +import static org.junit.Assert.assertEquals; + +/** + * @author mpolden + */ +public class AuditedFlagsApiTest extends ControllerContainerTest { + + private static final String responses = "src/test/java/com/yahoo/vespa/hosted/controller/restapi/flags/responses/"; + private static final AthenzIdentity operator = AthenzUser.fromUserId("operatorUser"); + + private ContainerControllerTester tester; + + @Before + public void before() { + addUserToHostedOperatorRole(operator); + tester = new ContainerControllerTester(container, responses); + } + + @Test + public void test_audit_logging() { + var body = "{\n" + + " \"id\": \"id1\",\n" + + " \"rules\": [\n" + + " {\n" + + " \"value\": true\n" + + " }\n" + + " ]\n" + + "}"; + assertResponse(new Request("http://localhost:8080/flags/v1/data/id1?force=true", body, Request.Method.PUT), + "", 200); + var log = tester.controller().auditLogger().readLog(); + assertEquals(1, log.entries().size()); + var entry = log.entries().get(0); + assertEquals(operator.getFullName(), entry.principal()); + assertEquals(AuditLog.Entry.Method.PUT, entry.method()); + assertEquals("/flags/v1/data/id1?force=true", entry.resource()); + assertEquals(body, log.entries().get(0).data().get()); + } + + private void assertResponse(Request request, String body, int statusCode) { + addIdentityToRequest(request, operator); + tester.assertResponse(request, body, statusCode); + } + +} -- cgit v1.2.3 From 8d8783a3f13dfaa134c11888516dfa5c0b507aae Mon Sep 17 00:00:00 2001 From: Harald Musum Date: Fri, 12 Jul 2019 10:42:31 +0200 Subject: Remove event logging from config proxy Statistics in config proxy are logged as events in vespa log, we want to move away from that to using real metrics if we need something like this in the future. AFAIK these events have not proved useful for anything in a long time. --- .../vespa/config/proxy/ConfigProxyRpcServer.java | 1 - .../vespa/config/proxy/ConfigProxyStatistics.java | 104 --------------------- .../com/yahoo/vespa/config/proxy/ProxyServer.java | 49 +++------- .../vespa/config/proxy/RpcConfigSourceClient.java | 6 -- .../yahoo/vespa/config/proxy/ProxyServerTest.java | 25 +---- .../config/proxy/RpcConfigSourceClientTest.java | 5 +- 6 files changed, 15 insertions(+), 175 deletions(-) delete mode 100644 config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ConfigProxyStatistics.java diff --git a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ConfigProxyRpcServer.java b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ConfigProxyRpcServer.java index 5ffc7293742..e815fd9a5f9 100644 --- a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ConfigProxyRpcServer.java +++ b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ConfigProxyRpcServer.java @@ -133,7 +133,6 @@ public class ConfigProxyRpcServer implements Runnable, TargetWatcher, RpcServer dispatchRpcRequest(req, () -> { JRTServerConfigRequest request = JRTServerConfigRequestV3.createFromRequest(req); if (isProtocolVersionSupported(request)) { - proxyServer.getStatistics().incRpcRequests(); req.target().addWatcher(this); getConfigImpl(request); return; diff --git a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ConfigProxyStatistics.java b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ConfigProxyStatistics.java deleted file mode 100644 index 314a4b0cb11..00000000000 --- a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ConfigProxyStatistics.java +++ /dev/null @@ -1,104 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.config.proxy; - -import com.yahoo.log.LogLevel; -import com.yahoo.log.event.Event; - -/** - * Statistics/metrics for config proxy. - * //TODO Use metrics framework - * - * @author hmusum - */ -class ConfigProxyStatistics implements Runnable { - static final long defaultEventInterval = 5 * 60; // in seconds - - private final long eventInterval; // in seconds - private boolean stopped; - private long lastRun = System.currentTimeMillis(); - - /* Number of RPC getConfig requests */ - private long rpcRequests = 0; - private long processedRequests = 0; - private long errors = 0; - private long delayedResponses = 0; - - ConfigProxyStatistics() { - this(defaultEventInterval); - } - - ConfigProxyStatistics(long eventInterval) { - this.eventInterval = eventInterval; - } - - // Send events every eventInterval seconds - public void run() { - while (true) { - try { - Thread.sleep(1000); - } catch (InterruptedException e) { - ProxyServer.log.log(LogLevel.WARNING, e.getMessage()); - } - if (stopped) { - return; - } - ProxyServer.log.log(LogLevel.SPAM, "Running ConfigProxyStatistics"); - // Only send events every eventInterval seconds - if ((System.currentTimeMillis() - lastRun) > eventInterval * 1000) { - lastRun = System.currentTimeMillis(); - sendEvents(); - } - } - } - - private void sendEvents() { - Event.count("rpc_requests", rpcRequests()); - Event.count("processed_messages", processedRequests()); - Event.count("errors", errors()); - Event.value("delayed_responses", delayedResponses()); - } - - void stop() { - stopped = true; - } - - Long getEventInterval() { - return eventInterval; - } - - void incRpcRequests() { - rpcRequests++; - } - - void incProcessedRequests() { - processedRequests++; - } - - void incErrorCount() { - errors++; - } - - long processedRequests() { - return processedRequests; - } - - long rpcRequests() { - return rpcRequests; - } - - long errors() { - return errors; - } - - long delayedResponses() { - return delayedResponses; - } - - void delayedResponses(long count) { - delayedResponses = count; - } - - void decDelayedResponses() { - delayedResponses--; - } -} diff --git a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ProxyServer.java b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ProxyServer.java index 4bf3bd5a786..c819192eb62 100644 --- a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ProxyServer.java +++ b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ProxyServer.java @@ -38,7 +38,7 @@ public class ProxyServer implements Runnable { private static final int JRT_TRANSPORT_THREADS = 4; static final String DEFAULT_PROXY_CONFIG_SOURCES = "tcp/localhost:19070"; - final static Logger log = Logger.getLogger(ProxyServer.class.getName()); + private final static Logger log = Logger.getLogger(ProxyServer.class.getName()); private final AtomicBoolean signalCaught = new AtomicBoolean(false); // Scheduled executor that periodically checks for requests that have timed out and response should be returned to clients @@ -52,7 +52,6 @@ public class ProxyServer implements Runnable { private volatile ConfigSourceClient configClient; - private final ConfigProxyStatistics statistics; private final TimingValues timingValues; private final MemoryCache memoryCache; private static final double timingValuesRatio = 0.8; @@ -72,33 +71,29 @@ public class ProxyServer implements Runnable { defaultTimingValues = tv; } - private ProxyServer(Spec spec, DelayedResponses delayedResponses, ConfigSourceSet source, - ConfigProxyStatistics statistics, TimingValues timingValues, - boolean delayedResponseHandling, MemoryCache memoryCache, - ConfigSourceClient configClient) { + private ProxyServer(Spec spec, DelayedResponses delayedResponses, ConfigSourceSet source, TimingValues timingValues, + boolean delayedResponseHandling, MemoryCache memoryCache, ConfigSourceClient configClient) { this.delayedResponses = delayedResponses; this.configSource = source; log.log(LogLevel.DEBUG, "Using config source '" + source); - this.statistics = statistics; this.timingValues = timingValues; this.delayedResponseHandling = delayedResponseHandling; this.memoryCache = memoryCache; this.rpcServer = createRpcServer(spec); - this.configClient = createClient(rpcServer, statistics, delayedResponses, source, timingValues, memoryCache, configClient); + this.configClient = createClient(rpcServer, delayedResponses, source, timingValues, memoryCache, configClient); this.fileDistributionAndUrlDownload = new FileDistributionAndUrlDownload(supervisor, source); } static ProxyServer createTestServer(ConfigSourceSet source) { - return createTestServer(source, null, new MemoryCache(), new ConfigProxyStatistics()); + return createTestServer(source, null, new MemoryCache()); } static ProxyServer createTestServer(ConfigSourceSet source, ConfigSourceClient configSourceClient, - MemoryCache memoryCache, - ConfigProxyStatistics statistics) { + MemoryCache memoryCache) { final boolean delayedResponseHandling = false; return new ProxyServer(null, new DelayedResponses(), - source, statistics, defaultTimingValues(), delayedResponseHandling, + source, defaultTimingValues(), delayedResponseHandling, memoryCache, configSourceClient); } @@ -120,7 +115,6 @@ public class ProxyServer implements Runnable { } RawConfig resolveConfig(JRTServerConfigRequest req) { - statistics.incProcessedRequests(); // Calling getConfig() will either return with an answer immediately or // create a background thread that retrieves config from the server and // calls updateSubscribers when new config is returned from the config source. @@ -155,12 +149,11 @@ public class ProxyServer implements Runnable { } } - private ConfigSourceClient createClient(RpcServer rpcServer, ConfigProxyStatistics statistics, - DelayedResponses delayedResponses, + private ConfigSourceClient createClient(RpcServer rpcServer, DelayedResponses delayedResponses, ConfigSourceSet source, TimingValues timingValues, MemoryCache memoryCache, ConfigSourceClient client) { return (client == null) - ? new RpcConfigSourceClient(rpcServer, source, statistics, memoryCache, timingValues, delayedResponses) + ? new RpcConfigSourceClient(rpcServer, source, memoryCache, timingValues, delayedResponses) : client; } @@ -169,7 +162,7 @@ public class ProxyServer implements Runnable { } private RpcConfigSourceClient createRpcClient() { - return new RpcConfigSourceClient(rpcServer, configSource, statistics, memoryCache, timingValues, delayedResponses); + return new RpcConfigSourceClient(rpcServer, configSource, memoryCache, timingValues, delayedResponses); } private void setupSignalHandler() { @@ -202,15 +195,10 @@ public class ProxyServer implements Runnable { port = Integer.parseInt(args[0]); } Event.started("configproxy"); - ConfigProxyStatistics statistics = new ConfigProxyStatistics(properties.eventInterval); - Thread t = new Thread(statistics); - t.setName("Metrics generator"); - t.setDaemon(true); - t.start(); ConfigSourceSet configSources = new ConfigSourceSet(properties.configSources); DelayedResponses delayedResponses = new DelayedResponses(); - ProxyServer proxyServer = new ProxyServer(new Spec(null, port), delayedResponses, configSources, statistics, + ProxyServer proxyServer = new ProxyServer(new Spec(null, port), delayedResponses, configSources, defaultTimingValues(), true, new MemoryCache(), null); // catch termination and interrupt signal proxyServer.setupSignalHandler(); @@ -221,18 +209,14 @@ public class ProxyServer implements Runnable { } static Properties getSystemProperties() { - // Read system properties - long eventInterval = Long.getLong("eventinterval", ConfigProxyStatistics.defaultEventInterval); final String[] inputConfigSources = System.getProperty("proxyconfigsources", DEFAULT_PROXY_CONFIG_SOURCES).split(","); - return new Properties(eventInterval, inputConfigSources); + return new Properties(inputConfigSources); } static class Properties { - final long eventInterval; final String[] configSources; - Properties(long eventInterval, String[] configSources) { - this.eventInterval = eventInterval; + Properties(String[] configSources) { this.configSources = configSources; } } @@ -245,10 +229,6 @@ public class ProxyServer implements Runnable { return timingValues; } - ConfigProxyStatistics getStatistics() { - return statistics; - } - // Cancels all config instances and flushes the cache. When this method returns, // the cache will not be updated again before someone calls getConfig(). private synchronized void flush() { @@ -261,9 +241,6 @@ public class ProxyServer implements Runnable { if (rpcServer != null) rpcServer.shutdown(); if (delayedResponseScheduler != null) delayedResponseScheduler.cancel(true); flush(); - if (statistics != null) { - statistics.stop(); - } fileDistributionAndUrlDownload.close(); } diff --git a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/RpcConfigSourceClient.java b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/RpcConfigSourceClient.java index c9f43ac48e2..d809a3c97ed 100644 --- a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/RpcConfigSourceClient.java +++ b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/RpcConfigSourceClient.java @@ -38,7 +38,6 @@ class RpcConfigSourceClient implements ConfigSourceClient { private final HashMap activeSubscribers = new HashMap<>(); private final Object activeSubscribersLock = new Object(); private final MemoryCache memoryCache; - private final ConfigProxyStatistics statistics; private final DelayedResponses delayedResponses; private final TimingValues timingValues; @@ -48,13 +47,11 @@ class RpcConfigSourceClient implements ConfigSourceClient { RpcConfigSourceClient(RpcServer rpcServer, ConfigSourceSet configSourceSet, - ConfigProxyStatistics statistics, MemoryCache memoryCache, TimingValues timingValues, DelayedResponses delayedResponses) { this.rpcServer = rpcServer; this.configSourceSet = configSourceSet; - this.statistics = statistics; this.memoryCache = memoryCache; this.delayedResponses = delayedResponses; this.timingValues = timingValues; @@ -122,7 +119,6 @@ class RpcConfigSourceClient implements ConfigSourceClient { // happens at the same time DelayedResponse delayedResponse = new DelayedResponse(request); delayedResponses.add(delayedResponse); - statistics.delayedResponses(delayedResponses.size()); final ConfigCacheKey configCacheKey = new ConfigCacheKey(input.getKey(), input.getDefMd5()); RawConfig cachedConfig = memoryCache.get(configCacheKey); @@ -139,7 +135,6 @@ class RpcConfigSourceClient implements ConfigSourceClient { // unless another thread already did it ret = cachedConfig; } - statistics.decDelayedResponses(); } if (!cachedConfig.isError() && cachedConfig.getGeneration() > 0) { needToGetConfig = false; @@ -220,7 +215,6 @@ class RpcConfigSourceClient implements ConfigSourceClient { */ public void updateSubscribers(RawConfig config) { log.log(LogLevel.DEBUG, () -> "Config updated for " + config.getKey() + "," + config.getGeneration()); - if (config.isError()) { statistics.incErrorCount(); } DelayQueue responseDelayQueue = delayedResponses.responses(); log.log(LogLevel.SPAM, () -> "Delayed response queue: " + responseDelayQueue); if (responseDelayQueue.size() == 0) { diff --git a/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/ProxyServerTest.java b/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/ProxyServerTest.java index 9d6d0ca2a39..803f5c85b5c 100644 --- a/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/ProxyServerTest.java +++ b/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/ProxyServerTest.java @@ -23,7 +23,6 @@ public class ProxyServerTest { private final MemoryCache memoryCache = new MemoryCache(); private final MockConfigSource source = new MockConfigSource(); private MockConfigSourceClient client = new MockConfigSourceClient(source, memoryCache); - private final ConfigProxyStatistics statistics = new ConfigProxyStatistics(); private ProxyServer proxy; static final RawConfig fooConfig = ConfigTester.fooConfig; @@ -42,7 +41,7 @@ public class ProxyServerTest { source.clear(); source.put(fooConfig.getKey(), createConfigWithNextConfigGeneration(fooConfig, 0)); source.put(errorConfigKey, createConfigWithNextConfigGeneration(fooConfig, ErrorCode.UNKNOWN_DEFINITION)); - proxy = ProxyServer.createTestServer(source, client, memoryCache, statistics); + proxy = ProxyServer.createTestServer(source, client, memoryCache); } @After @@ -64,27 +63,6 @@ public class ProxyServerTest { assertThat(res.getPayload().toString(), is(ConfigTester.fooPayload.toString())); assertEquals(1, memoryCache.size()); assertThat(memoryCache.get(new ConfigCacheKey(fooConfig.getKey(), fooConfig.getDefMd5())), is(res)); - - - assertEquals(1, statistics.processedRequests()); - assertEquals(0, statistics.rpcRequests()); - assertEquals(0, statistics.errors()); - assertEquals(0, statistics.delayedResponses()); - - statistics.incProcessedRequests(); - statistics.incRpcRequests(); - statistics.incErrorCount(); - statistics.delayedResponses(1); - - assertEquals(2, statistics.processedRequests()); - assertEquals(1, statistics.rpcRequests()); - assertEquals(1, statistics.errors()); - assertEquals(1, statistics.delayedResponses()); - - statistics.decDelayedResponses(); - assertEquals(0, statistics.delayedResponses()); - - assertEquals(ConfigProxyStatistics.defaultEventInterval, statistics.getEventInterval().longValue()); } /** @@ -228,7 +206,6 @@ public class ProxyServerTest { @Test public void testReadingSystemProperties() { ProxyServer.Properties properties = ProxyServer.getSystemProperties(); - assertThat(properties.eventInterval, is(ConfigProxyStatistics.defaultEventInterval)); assertThat(properties.configSources.length, is(1)); assertThat(properties.configSources[0], is(ProxyServer.DEFAULT_PROXY_CONFIG_SOURCES)); } diff --git a/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/RpcConfigSourceClientTest.java b/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/RpcConfigSourceClientTest.java index 7f762955b92..35f1dd8fcd8 100644 --- a/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/RpcConfigSourceClientTest.java +++ b/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/RpcConfigSourceClientTest.java @@ -18,7 +18,6 @@ import static org.junit.Assert.assertEquals; public class RpcConfigSourceClientTest { private MockRpcServer rpcServer; - private ConfigProxyStatistics statistics; private DelayedResponses delayedResponses; private RpcConfigSourceClient rpcConfigSourceClient; @@ -29,10 +28,9 @@ public class RpcConfigSourceClientTest { @Before public void setup() { rpcServer = new MockRpcServer(); - statistics = new ConfigProxyStatistics(); delayedResponses = new DelayedResponses(); rpcConfigSourceClient = - new RpcConfigSourceClient(rpcServer, new MockConfigSource(), statistics, + new RpcConfigSourceClient(rpcServer, new MockConfigSource(), new MemoryCache(), ProxyServer.defaultTimingValues(), delayedResponses); } @@ -57,7 +55,6 @@ public class RpcConfigSourceClientTest { public void errorResponse() { configUpdatedSendResponse(ProxyServerTest.errorConfig); assertSentResponses(0); - assertEquals(1, statistics.errors()); } @Test -- cgit v1.2.3