diff options
33 files changed, 371 insertions, 392 deletions
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 1f7053e0ad3..f4f2308f261 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,32 +71,28 @@ public class ProxyServer implements Runnable { defaultTimingValues = tv; } - private ProxyServer(Spec spec, ConfigSourceSet source, - ConfigProxyStatistics statistics, TimingValues timingValues, - boolean delayedResponseHandling, MemoryCache memoryCache, - ConfigSourceClient configClient) { + private ProxyServer(Spec spec, ConfigSourceSet source, TimingValues timingValues, + boolean delayedResponseHandling, MemoryCache memoryCache, ConfigSourceClient configClient) { this.delayedResponses = new 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, source, statistics, defaultTimingValues(), delayedResponseHandling, + return new ProxyServer(null, source, defaultTimingValues(), delayedResponseHandling, memoryCache, configSourceClient); } @@ -119,7 +114,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. @@ -154,12 +148,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; } @@ -168,7 +161,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() { @@ -201,14 +194,9 @@ 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); - ProxyServer proxyServer = new ProxyServer(new Spec(null, port), configSources, statistics, + ProxyServer proxyServer = new ProxyServer(new Spec(null, port), configSources, defaultTimingValues(), true, new MemoryCache(), null); // catch termination and interrupt signal proxyServer.setupSignalHandler(); @@ -219,18 +207,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; } } @@ -243,10 +227,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() { @@ -259,7 +239,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<ConfigCacheKey, Subscriber> 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<DelayedResponse> 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 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<FlagData> 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<FlagId, FlagData> getAllFlags(); -} 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/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<String> 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 @@ <preprocess:include file='model-integration.xml' required='true' /> <component id="com.yahoo.vespa.configserver.flags.ConfigServerFlagSource" bundle="configserver-flags"/> - <component id="com.yahoo.vespa.configserver.flags.db.FlagsDbImpl" bundle="configserver-flags"/> + <component id="com.yahoo.vespa.flags.persistence.FlagsDb" bundle="flags"/> <preprocess:include file='metrics-packets.xml' required='false' /> <preprocess:include file='container.xml' required='false' /> @@ -92,10 +92,6 @@ <handler id='com.yahoo.vespa.config.server.http.status.StatusHandler' bundle='configserver'> <binding>http://*/status</binding> </handler> - <handler id='com.yahoo.vespa.config.server.http.flags.FlagsHandler' bundle='configserver'> - <binding>http://*/flags/v1</binding> - <binding>http://*/flags/v1/*</binding> - </handler> <handler id='com.yahoo.vespa.config.server.http.v2.TenantHandler' bundle='configserver'> <binding>http://*/application/v2/tenant/</binding> <binding>http://*/application/v2/tenant/*</binding> 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..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; @@ -555,27 +554,6 @@ public class ApplicationSerializer { private List<AssignedRotation> assignedRotationsFromSlime(DeploymentSpec deploymentSpec, Inspector root) { final var assignedRotations = new LinkedHashMap<EndpointId, AssignedRotation>(); - // 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()); @@ -590,22 +568,6 @@ public class ApplicationSerializer { return List.copyOf(assignedRotations.values()); } - private List<RotationId> rotationListFromSlime(Inspector field) { - final var rotations = new ArrayList<RotationId>(); - - 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<RotationId> 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(); } 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/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() - ); - } } 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 { " </rotations>\n" + " </config>\n" + " <component id='com.yahoo.vespa.flags.InMemoryFlagSource'/>\n" + + " <component id='com.yahoo.vespa.flags.persistence.FlagsDb'/>\n" + + " <component id='com.yahoo.vespa.curator.mock.MockCurator'/>\n" + " <component id='com.yahoo.vespa.hosted.controller.persistence.MockCuratorDb'/>\n" + " <component id='com.yahoo.vespa.hosted.controller.athenz.mock.AthenzClientFactoryMock'/>\n" + " <component id='com.yahoo.vespa.hosted.controller.api.integration.dns.MemoryNameService'/>\n" + @@ -112,6 +114,10 @@ public class ControllerContainerTest { " <binding>http://*/zone/v2</binding>\n" + " <binding>http://*/zone/v2/*</binding>\n" + " </handler>\n" + + " <handler id='com.yahoo.vespa.hosted.controller.restapi.flags.AuditedFlagsHandler'>\n" + + " <binding>http://*/flags/v1</binding>\n" + + " <binding>http://*/flags/v1/*</binding>\n" + + " </handler>\n" + variablePartXml() + "</container>"; } 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); + } + +} 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 @@ <classifier>no_aop</classifier> <scope>provided</scope> </dependency> - + <dependency> + <groupId>com.yahoo.vespa</groupId> + <artifactId>container-dev</artifactId> + <version>${project.version}</version> + <scope>provided</scope> + </dependency> + <dependency> + <groupId>com.yahoo.vespa</groupId> + <artifactId>zkfacade</artifactId> + <version>${project.version}</version> + <scope>provided</scope> + </dependency> <dependency> <groupId>junit</groupId> <artifactId>junit</artifactId> 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) { diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/DefinedFlag.java b/flags/src/main/java/com/yahoo/vespa/flags/http/DefinedFlag.java index 92397fc84a7..8234e9df725 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/DefinedFlag.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/http/DefinedFlag.java @@ -1,12 +1,11 @@ // 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; +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.config.server.http.HttpConfigResponse; import com.yahoo.vespa.flags.FlagDefinition; import com.yahoo.vespa.flags.json.DimensionHelper; @@ -42,6 +41,7 @@ public class DefinedFlag extends HttpResponse { @Override public String getContentType() { - return HttpConfigResponse.JSON_CONTENT_TYPE; + return "application/json"; } + } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/DefinedFlags.java b/flags/src/main/java/com/yahoo/vespa/flags/http/DefinedFlags.java index 9604c51ee4b..e1db7dda6e0 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/DefinedFlags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/http/DefinedFlags.java @@ -1,11 +1,10 @@ // 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; +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.config.server.http.HttpConfigResponse; import com.yahoo.vespa.flags.FlagDefinition; import java.io.IOException; @@ -18,8 +17,7 @@ import java.util.List; */ public class DefinedFlags extends HttpResponse { private static ObjectMapper mapper = new ObjectMapper(); - private static final Comparator<FlagDefinition> sortByFlagId = - (left, right) -> left.getUnboundFlag().id().compareTo(right.getUnboundFlag().id()); + private static final Comparator<FlagDefinition> sortByFlagId = Comparator.comparing(flagDefinition -> flagDefinition.getUnboundFlag().id()); private final List<FlagDefinition> flags; @@ -40,6 +38,6 @@ public class DefinedFlags extends HttpResponse { @Override public String getContentType() { - return HttpConfigResponse.JSON_CONTENT_TYPE; + 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/configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/FlagDataListResponse.java b/flags/src/main/java/com/yahoo/vespa/flags/http/FlagDataListResponse.java index b33fc7c2b04..5af97007997 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/FlagDataListResponse.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/http/FlagDataListResponse.java @@ -1,12 +1,11 @@ // 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; +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.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; @@ -54,6 +53,6 @@ public class FlagDataListResponse extends HttpResponse { @Override public String getContentType() { - return HttpConfigResponse.JSON_CONTENT_TYPE; + return "application/json"; } } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/FlagDataResponse.java b/flags/src/main/java/com/yahoo/vespa/flags/http/FlagDataResponse.java index 054b218ff2d..f6e81e030c7 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/FlagDataResponse.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/http/FlagDataResponse.java @@ -1,9 +1,8 @@ // 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; +package com.yahoo.vespa.flags.http; 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; @@ -26,6 +25,6 @@ public class FlagDataResponse extends HttpResponse { @Override public String getContentType() { - return HttpConfigResponse.JSON_CONTENT_TYPE; + return "application/json"; } } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/FlagsHandler.java b/flags/src/main/java/com/yahoo/vespa/flags/http/FlagsHandler.java index 00f3d457d3d..76f74cbe931 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/FlagsHandler.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/http/FlagsHandler.java @@ -1,19 +1,17 @@ // 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; +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.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.vespa.flags.persistence.FlagsDb; import com.yahoo.yolean.Exceptions; import java.io.UncheckedIOException; @@ -25,7 +23,8 @@ import java.util.Objects; * * @author hakonhall */ -public class FlagsHandler extends HttpHandler { +public class FlagsHandler extends LoggingRequestHandler { + private final FlagsDb flagsDb; @Inject @@ -35,28 +34,44 @@ public class FlagsHandler extends HttpHandler { } @Override - protected HttpResponse handleGET(HttpRequest request) { + 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)); - throw new NotFoundException("Nothing at path '" + path + "'"); + return ErrorResponse.notFoundError("Nothing at path '" + path + "'"); } - @Override - protected HttpResponse handlePUT(HttpRequest request) { + private 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 + "'"); + return ErrorResponse.notFoundError("Nothing at path '" + path + "'"); } - @Override - protected HttpResponse handleDELETE(HttpRequest request) { + private 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 + "'"); + return ErrorResponse.notFoundError("Nothing at path '" + path + "'"); } private String flagsV1Uri(HttpRequest request) { @@ -66,9 +81,11 @@ public class FlagsHandler extends HttpHandler { } private HttpResponse getDefinedFlag(FlagId flagId) { - FlagDefinition definition = Flags.getFlag(flagId) - .orElseThrow(() -> new NotFoundException("Flag " + flagId + " not defined")); - return new DefinedFlag(definition); + 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) { @@ -77,8 +94,11 @@ public class FlagsHandler extends HttpHandler { } private HttpResponse getFlagData(FlagId flagId) { - FlagData data = flagsDb.getValue(flagId).orElseThrow(() -> new NotFoundException("Flag " + flagId + " not set")); - return new FlagDataResponse(data); + 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) { @@ -86,7 +106,7 @@ public class FlagsHandler extends HttpHandler { try { data = FlagData.deserialize(request.getData()); } catch (UncheckedIOException e) { - return HttpErrorResponse.badRequest("Failed to deserialize request data: " + Exceptions.toMessageString(e)); + return ErrorResponse.badRequest("Failed to deserialize request data: " + Exceptions.toMessageString(e)); } if (!isForce(request)) { @@ -105,16 +125,14 @@ public class FlagsHandler extends HttpHandler { 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)")); + 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/configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/OKResponse.java b/flags/src/main/java/com/yahoo/vespa/flags/http/OKResponse.java index 87c02ae56f1..d094e2d5734 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/OKResponse.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/http/OKResponse.java @@ -1,9 +1,8 @@ // 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; +package com.yahoo.vespa.flags.http; import com.yahoo.container.jdisc.EmptyResponse; import com.yahoo.jdisc.Response; -import com.yahoo.vespa.config.server.http.HttpConfigResponse; /** * @author hakonhall @@ -15,6 +14,6 @@ public class OKResponse extends EmptyResponse { @Override public String getContentType() { - return HttpConfigResponse.JSON_CONTENT_TYPE; + 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<String> 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/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/db/FlagsDbImpl.java b/flags/src/main/java/com/yahoo/vespa/flags/persistence/FlagsDb.java index 5058358ba03..2ed762f2895 100644 --- a/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/db/FlagsDbImpl.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/persistence/FlagsDb.java @@ -1,9 +1,8 @@ // 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; +package com.yahoo.vespa.flags.persistence; 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; @@ -20,14 +19,15 @@ import java.util.stream.Collectors; /** * @author hakonhall */ -public class FlagsDbImpl implements FlagsDb { +public class 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) { + public FlagsDb(Curator curator) { this.curator = curator; curator.create(ROOT_PATH); ExecutorService executorService = Executors.newFixedThreadPool(1); @@ -35,28 +35,28 @@ public class FlagsDbImpl implements FlagsDb { cache.start(); } - @Override + /** Get the String value of the flag. */ public Optional<FlagData> getValue(FlagId flagId) { return Optional.ofNullable(cache.getCurrentData(getZkPathFor(flagId))) - .map(ChildData::getData) - .map(FlagData::deserializeUtf8Json); + .map(ChildData::getData) + .map(FlagData::deserializeUtf8Json); } - @Override + /** Set the String value of the flag. */ public void setValue(FlagId flagId, FlagData data) { curator.set(getZkPathFor(flagId), data.serializeToUtf8Json()); } - @Override + /** Get all flags that have been set. */ public Map<FlagId, FlagData> getAllFlags() { List<ChildData> dataList = cache.getCurrentData(); return dataList.stream() - .map(ChildData::getData) - .map(FlagData::deserializeUtf8Json) - .collect(Collectors.toMap(FlagData::id, Function.identity())); + .map(ChildData::getData) + .map(FlagData::deserializeUtf8Json) + .collect(Collectors.toMap(FlagData::id, Function.identity())); } - @Override + /** Remove the flag value if it exists. */ public void removeValue(FlagId flagId) { curator.delete(getZkPathFor(flagId)); } @@ -64,4 +64,5 @@ public class FlagsDbImpl implements FlagsDb { 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/configserver/src/test/java/com/yahoo/vespa/config/server/http/flags/FlagsHandlerTest.java b/flags/src/test/java/com/yahoo/vespa/flags/http/FlagsHandlerTest.java index 5ae6ce9820b..8ae1008ba22 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/flags/FlagsHandlerTest.java +++ b/flags/src/test/java/com/yahoo/vespa/flags/http/FlagsHandlerTest.java @@ -1,25 +1,26 @@ // 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; +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.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 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 com.yahoo.yolean.Exceptions.uncheck; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.is; @@ -37,7 +38,7 @@ public class FlagsHandlerTest { private static final String FLAGS_V1_URL = "https://foo.com:4443/flags/v1"; - private final FlagsDbImpl flagsDb = new FlagsDbImpl(new MockCurator()); + private final FlagsDb flagsDb = new FlagsDb(new MockCurator()); private final FlagsHandler handler = new FlagsHandler(FlagsHandler.testOnlyContext(), flagsDb); @Test @@ -161,7 +162,7 @@ public class FlagsHandlerTest { @Test public void testForcing() { - assertThat(handle(Method.PUT, "/data/" + new FlagId("undef"), "", 404), + 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), @@ -191,10 +192,12 @@ public class FlagsHandlerTest { HttpResponse response = handler.handle(request); assertEquals(expectedStatus, response.getStatus()); assertEquals("application/json", response.getContentType()); - return uncheck(() -> SessionHandlerTest.getRenderedString(response)); + 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)); } -}
\ No newline at end of file +} diff --git a/configserver-flags/src/test/java/com/yahoo/vespa/configserver/flags/db/FlagsDbImplTest.java b/flags/src/test/java/com/yahoo/vespa/flags/persistence/FlagsDbTest.java index ecc9bacb081..5102305af90 100644 --- a/configserver-flags/src/test/java/com/yahoo/vespa/configserver/flags/db/FlagsDbImplTest.java +++ b/flags/src/test/java/com/yahoo/vespa/flags/persistence/FlagsDbTest.java @@ -1,5 +1,5 @@ // 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; +package com.yahoo.vespa.flags.persistence; import com.yahoo.vespa.curator.mock.MockCurator; import com.yahoo.vespa.flags.FetchVector; @@ -23,11 +23,11 @@ import static org.junit.Assert.assertTrue; /** * @author hakonhall */ -public class FlagsDbImplTest { +public class FlagsDbTest { @Test public void test() { MockCurator curator = new MockCurator(); - FlagsDbImpl db = new FlagsDbImpl(curator); + 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); @@ -39,8 +39,8 @@ public class FlagsDbImplTest { assertTrue(dataCopy.isPresent()); assertEquals("{\"id\":\"id\",\"rules\":[{\"conditions\":[{\"type\":\"whitelist\",\"dimension\":\"hostname\"," + - "\"values\":[\"host1\"]}],\"value\":13}],\"attributes\":{\"zone\":\"zone-a\"}}", - dataCopy.get().serializeToJson()); + "\"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); @@ -53,4 +53,4 @@ public class FlagsDbImplTest { db.removeValue(flagId2); assertFalse(db.getValue(flagId2).isPresent()); } -}
\ No newline at end of file +} |