From 6be85f2ca51c86b34b7d6ec831603531c4683ca6 Mon Sep 17 00:00:00 2001 From: Bjørn Christian Seime Date: Thu, 7 Nov 2019 11:46:33 +0100 Subject: Implement /system-flags/v1 handler --- controller-server/pom.xml | 12 ++ .../restapi/systemflags/FlagsClient.java | 181 +++++++++++++++++++ .../restapi/systemflags/FlagsClientException.java | 26 +++ .../systemflags/SystemFlagsDeployResult.java | 200 +++++++++++++++++++++ .../restapi/systemflags/SystemFlagsDeployer.java | 106 +++++++++++ .../restapi/systemflags/SystemFlagsHandler.java | 67 +++++++ .../systemflags/SystemFlagsDeployerTest.java | 76 ++++++++ .../system-flags/existing-prod.us-east-3.json | 8 + .../system-flags/existing-prod.us-west-1.json | 8 + .../resources/system-flags/flags/my-flag/main.json | 8 + .../flags/my-flag/main.prod.us-east-3.json | 8 + 11 files changed, 700 insertions(+) create mode 100644 controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/FlagsClient.java create mode 100644 controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/FlagsClientException.java create mode 100644 controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/SystemFlagsDeployResult.java create mode 100644 controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/SystemFlagsDeployer.java create mode 100644 controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/SystemFlagsHandler.java create mode 100644 controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/SystemFlagsDeployerTest.java create mode 100644 controller-server/src/test/resources/system-flags/existing-prod.us-east-3.json create mode 100644 controller-server/src/test/resources/system-flags/existing-prod.us-west-1.json create mode 100644 controller-server/src/test/resources/system-flags/flags/my-flag/main.json create mode 100644 controller-server/src/test/resources/system-flags/flags/my-flag/main.prod.us-east-3.json (limited to 'controller-server') diff --git a/controller-server/pom.xml b/controller-server/pom.xml index ae756eae1fb..0c545020449 100644 --- a/controller-server/pom.xml +++ b/controller-server/pom.xml @@ -172,6 +172,18 @@ test + + org.assertj + assertj-core + test + + + + org.mockito + mockito-core + test + + diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/FlagsClient.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/FlagsClient.java new file mode 100644 index 00000000000..9d21281ce9f --- /dev/null +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/FlagsClient.java @@ -0,0 +1,181 @@ +// 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.systemflags; + +import com.fasterxml.jackson.databind.ObjectMapper; +import com.yahoo.vespa.athenz.api.AthenzIdentity; +import com.yahoo.vespa.athenz.identity.ServiceIdentityProvider; +import com.yahoo.vespa.athenz.tls.AthenzIdentityVerifier; +import com.yahoo.vespa.flags.FlagId; +import com.yahoo.vespa.flags.json.FlagData; +import com.yahoo.vespa.hosted.controller.api.systemflags.v1.FlagsTarget; +import com.yahoo.vespa.hosted.controller.api.systemflags.v1.wire.WireErrorResponse; +import org.apache.http.HttpEntity; +import org.apache.http.HttpResponse; +import org.apache.http.HttpStatus; +import org.apache.http.client.ResponseHandler; +import org.apache.http.client.config.RequestConfig; +import org.apache.http.client.methods.HttpDelete; +import org.apache.http.client.methods.HttpGet; +import org.apache.http.client.methods.HttpPut; +import org.apache.http.client.methods.HttpUriRequest; +import org.apache.http.client.utils.URIBuilder; +import org.apache.http.entity.ContentType; +import org.apache.http.entity.StringEntity; +import org.apache.http.impl.client.CloseableHttpClient; +import org.apache.http.impl.client.DefaultHttpRequestRetryHandler; +import org.apache.http.impl.client.HttpClientBuilder; +import org.apache.http.util.EntityUtils; + +import javax.net.ssl.HostnameVerifier; +import javax.net.ssl.SSLSession; +import java.io.IOException; +import java.io.UncheckedIOException; +import java.net.URI; +import java.net.URISyntaxException; +import java.time.Duration; +import java.util.List; +import java.util.Set; + +import static java.util.stream.Collectors.toSet; + +/** + * A client for /flags/v1 rest api on configserver and controller. + * + * @author bjorncs + */ +class FlagsClient { + + private static final String FLAGS_V1_PATH = "/flags/v1"; + + private static final ObjectMapper mapper = new ObjectMapper(); + + private final CloseableHttpClient client; + + FlagsClient(ServiceIdentityProvider identityProvider, Set targets) { + this.client = createClient(identityProvider, targets); + } + + List listFlagData(FlagsTarget target) throws FlagsException, UncheckedIOException { + HttpGet request = new HttpGet(createUri(target, "/data")); + return executeRequest(request, response -> { + verifySuccess(response, target, null); + return FlagData.deserializeList(EntityUtils.toByteArray(response.getEntity())); + }); + } + + void putFlagData(FlagsTarget target, FlagData flagData) throws FlagsException, UncheckedIOException { + HttpPut request = new HttpPut(createUri(target, "/data/" + flagData.id().toString())); + request.setEntity(jsonContent(flagData.serializeToJson())); + executeRequest(request, response -> { + verifySuccess(response, target, flagData.id()); + return null; + }); + } + + void deleteFlagData(FlagsTarget target, FlagId flagId) throws FlagsException, UncheckedIOException { + HttpDelete request = new HttpDelete(createUri(target, "/data/" + flagId.toString())); + executeRequest(request, response -> { + verifySuccess(response, target, flagId); + return null; + }); + } + + + private static CloseableHttpClient createClient(ServiceIdentityProvider identityProvider, Set targets) { + return HttpClientBuilder.create() + .setUserAgent("controller-flags-v1-client") + .setRetryHandler(new DefaultHttpRequestRetryHandler(5, /*retry on non-idempotent requests*/true)) + .setSslcontext(identityProvider.getIdentitySslContext()) + .setSSLHostnameVerifier(new FlagTargetsHostnameVerifier(targets)) + .setDefaultRequestConfig(RequestConfig.custom() + .setConnectTimeout((int) Duration.ofSeconds(10).toMillis()) + .setConnectionRequestTimeout((int) Duration.ofSeconds(10).toMillis()) + .setSocketTimeout((int) Duration.ofSeconds(20).toMillis()) + .build()) + .setMaxConnPerRoute(2) + .setMaxConnTotal(100) + .build(); + } + + private T executeRequest(HttpUriRequest request, ResponseHandler handler) { + try { + return client.execute(request, handler); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + } + + private static URI createUri(FlagsTarget target, String subPath) { + try { + return new URIBuilder(target.endpoint()).setPath(FLAGS_V1_PATH + subPath).build(); + } catch (URISyntaxException e) { + throw new RuntimeException(e); // should never happen + } + } + + private static void verifySuccess(HttpResponse response, FlagsTarget target, FlagId flagId) throws IOException { + if (!success(response)) { + String message = getErrorMessage(response); + throw new FlagsException(response.getStatusLine().getStatusCode(), target, flagId, message); + } + } + + private static String getErrorMessage(HttpResponse response) throws IOException { + HttpEntity entity = response.getEntity(); + String content = EntityUtils.toString(entity); + if (ContentType.get(entity).getMimeType().equals(ContentType.APPLICATION_JSON.getMimeType())) { + WireErrorResponse errorResponse = mapper.readValue(content, WireErrorResponse.class); + return errorResponse.message; + } + return content; + } + + private static boolean success(HttpResponse response) { + return response.getStatusLine().getStatusCode() == HttpStatus.SC_OK; + } + + private static StringEntity jsonContent(String json) { + return new StringEntity(json, ContentType.APPLICATION_JSON); + } + + private static class FlagTargetsHostnameVerifier implements HostnameVerifier { + + private final AthenzIdentityVerifier athenzVerifier; + + FlagTargetsHostnameVerifier(Set targets) { + this.athenzVerifier = createAthenzIdentityVerifier(targets); + } + + private static AthenzIdentityVerifier createAthenzIdentityVerifier(Set targets) { + Set identities = targets.stream() + .flatMap(target -> target.athenzHttpsIdentity().stream()) + .collect(toSet()); + return new AthenzIdentityVerifier(identities); + } + + @Override + public boolean verify(String hostname, SSLSession session) { + return "localhost".equals(hostname) /* for controllers */ || athenzVerifier.verify(hostname, session); + } + } + + static class FlagsException extends RuntimeException { + + private FlagsException(int statusCode, FlagsTarget target, String responseMessage) { + this(statusCode, target, null, responseMessage); + } + + private FlagsException(int statusCode, FlagsTarget target, FlagId flagId, String responseMessage) { + super(createErrorMessage(statusCode, target, flagId, responseMessage)); + } + + private static String createErrorMessage(int statusCode, FlagsTarget target, FlagId flagId, String responseMessage) { + StringBuilder builder = new StringBuilder() + .append("Received '").append(statusCode).append("' from '").append(target.endpoint().getHost()).append("'"); + if (flagId != null) { + builder.append("' for flag '").append(flagId).append("'"); + } + return builder.append(": ").append(responseMessage).toString(); + } + } +} diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/FlagsClientException.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/FlagsClientException.java new file mode 100644 index 00000000000..864ad332696 --- /dev/null +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/FlagsClientException.java @@ -0,0 +1,26 @@ +// 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.systemflags; + +import java.util.OptionalInt; + +/** + * @author bjorncs + */ +class FlagsClientException extends RuntimeException { + + private final int responseCode; + + FlagsClientException(int responseCode, String message) { + super(message); + this.responseCode = responseCode; + } + + FlagsClientException(String message, Throwable cause) { + super(message, cause); + this.responseCode = -1; + } + + OptionalInt responseCode() { + return responseCode > 0 ? OptionalInt.of(responseCode) : OptionalInt.empty(); + } +} diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/SystemFlagsDeployResult.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/SystemFlagsDeployResult.java new file mode 100644 index 00000000000..ae1cb6321bd --- /dev/null +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/SystemFlagsDeployResult.java @@ -0,0 +1,200 @@ +// 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.systemflags; + +import com.fasterxml.jackson.databind.JsonNode; +import com.yahoo.vespa.flags.FlagId; +import com.yahoo.vespa.flags.json.FlagData; +import com.yahoo.vespa.hosted.controller.api.systemflags.v1.FlagsTarget; +import com.yahoo.vespa.hosted.controller.api.systemflags.v1.wire.WireSystemFlagsDeployResult; +import com.yahoo.vespa.hosted.controller.api.systemflags.v1.wire.WireSystemFlagsDeployResult.WireFlagDataChange; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Optional; +import java.util.Set; + +import static java.util.stream.Collectors.toList; + +/** + * @author bjorncs + */ +class SystemFlagsDeployResult { + + private final List flagChanges; + + SystemFlagsDeployResult(List flagChanges) { this.flagChanges = flagChanges; } + + List flagChanges() { + return flagChanges; + } + + static SystemFlagsDeployResult merge(List results) { + Map> targetsForOperation = new HashMap<>(); + + for (SystemFlagsDeployResult result : results) { + for (FlagDataChange change : result.flagChanges()) { + FlagDataOperation operation = new FlagDataOperation(change); + targetsForOperation.computeIfAbsent(operation, k -> new HashSet<>()) + .addAll(change.targets()); + } + } + + List mergedResult = new ArrayList<>(); + targetsForOperation.forEach( + (operation, targets) -> mergedResult.add(operation.toFlagDataChange(targets))); + return new SystemFlagsDeployResult(mergedResult); + } + + WireSystemFlagsDeployResult toWire() { + var wireResult = new WireSystemFlagsDeployResult(); + wireResult.changes = new ArrayList<>(); + for (FlagDataChange change : flagChanges) { + var wireChange = new WireFlagDataChange(); + wireChange.flagId = change.flagId().toString(); + wireChange.operation = change.operation().asString(); + wireChange.targets = change.targets().stream().map(FlagsTarget::asString).collect(toList()); + wireChange.data = change.data().map(FlagData::toWire).orElse(null); + wireChange.previousData = change.previousData().map(FlagData::toWire).orElse(null); + } + return wireResult; + } + + static class FlagDataChange { + + private final FlagId flagId; + private final Set targets; + private final OperationType operationType; + private final FlagData data; + private final FlagData previousData; + + private FlagDataChange( + FlagId flagId, Set targets, OperationType operationType, FlagData data, FlagData previousData) { + this.flagId = flagId; + this.targets = targets; + this.operationType = operationType; + this.data = data; + this.previousData = previousData; + } + + static FlagDataChange created(FlagId flagId, Set targets, FlagData data) { + return new FlagDataChange(flagId, targets, OperationType.CREATE, data, null); + } + + static FlagDataChange deleted(FlagId flagId, Set targets) { + return new FlagDataChange(flagId, targets, OperationType.DELETE, null, null); + } + + static FlagDataChange updated(FlagId flagId, Set targets, FlagData data, FlagData previousData) { + return new FlagDataChange(flagId, targets, OperationType.UPDATE, data, previousData); + } + + FlagId flagId() { + return flagId; + } + + Set targets() { + return targets; + } + + OperationType operation() { + return operationType; + } + + Optional data() { + return Optional.ofNullable(data); + } + + Optional previousData() { + return Optional.ofNullable(previousData); + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + FlagDataChange that = (FlagDataChange) o; + return Objects.equals(flagId, that.flagId) && + Objects.equals(targets, that.targets) && + operationType == that.operationType && + Objects.equals(data, that.data) && + Objects.equals(previousData, that.previousData); + } + + @Override + public int hashCode() { + return Objects.hash(flagId, targets, operationType, data, previousData); + } + + @Override + public String toString() { + return "FlagDataChange{" + + "flagId=" + flagId + + ", targets=" + targets + + ", operationType=" + operationType + + ", data=" + data + + ", previousData=" + previousData + + '}'; + } + } + + enum OperationType { + CREATE("create"), DELETE("delete"), UPDATE("update"); + + private final String stringValue; + + OperationType(String stringValue) { this.stringValue = stringValue; } + + String asString() { return stringValue; } + + static OperationType fromString(String stringValue) { + return Arrays.stream(values()) + .filter(v -> v.stringValue.equals(stringValue)) + .findAny() + .orElseThrow(() -> new IllegalArgumentException("Unknown string value: " + stringValue)); + } + } + + private static class FlagDataOperation { + final FlagId flagId; + final OperationType operationType; + final FlagData data; + final FlagData previousData; + final JsonNode jsonData; // needed for FlagData equality check + final JsonNode jsonPreviousData; // needed for FlagData equality check + + + FlagDataOperation(FlagDataChange change) { + this.flagId = change.flagId(); + this.operationType = change.operation(); + this.data = change.data().orElse(null); + this.previousData = change.previousData().orElse(null); + this.jsonData = Optional.ofNullable(data).map(FlagData::toJsonNode).orElse(null); + this.jsonPreviousData = Optional.ofNullable(previousData).map(FlagData::toJsonNode).orElse(null); + } + + FlagDataChange toFlagDataChange(Set targets) { + return new FlagDataChange(flagId, targets, operationType, data, previousData); + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + FlagDataOperation that = (FlagDataOperation) o; + return Objects.equals(flagId, that.flagId) && + operationType == that.operationType && + Objects.equals(jsonData, that.jsonData) && + Objects.equals(jsonPreviousData, that.jsonPreviousData); + } + + @Override + public int hashCode() { + return Objects.hash(flagId, operationType, jsonData, jsonPreviousData); + } + } +} diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/SystemFlagsDeployer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/SystemFlagsDeployer.java new file mode 100644 index 00000000000..2e783d5fcb3 --- /dev/null +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/SystemFlagsDeployer.java @@ -0,0 +1,106 @@ +// 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.systemflags; + +import com.yahoo.concurrent.DaemonThreadFactory; +import com.yahoo.vespa.athenz.identity.ServiceIdentityProvider; +import com.yahoo.vespa.flags.FlagId; +import com.yahoo.vespa.flags.json.FlagData; +import com.yahoo.vespa.hosted.controller.api.systemflags.v1.FlagsTarget; +import com.yahoo.vespa.hosted.controller.api.systemflags.v1.SystemFlagsDataArchive; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorCompletionService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.function.Function; +import java.util.stream.Collectors; + +import static com.yahoo.vespa.hosted.controller.restapi.systemflags.SystemFlagsDeployResult.FlagDataChange; + +/** + * Deploy a flags data archive to all targets in a given system + * + * @author bjorncs + */ +class SystemFlagsDeployer { + + private final FlagsClient client; + private final Set targets; + private final ExecutorCompletionService completionService = + new ExecutorCompletionService<>(Executors.newCachedThreadPool(new DaemonThreadFactory("system-flags-deployer-"))); + + + SystemFlagsDeployer(ServiceIdentityProvider identityProvider, Set targets) { + this(new FlagsClient(identityProvider, targets), targets); + } + + SystemFlagsDeployer(FlagsClient client, Set targets) { + this.client = client; + this.targets = targets; + } + + SystemFlagsDeployResult deployFlags(SystemFlagsDataArchive archive, boolean dryRun) { + for (FlagsTarget target : targets) { + completionService.submit(() -> deployFlags(target, archive.flagData(target), dryRun)); + } + List results = new ArrayList<>(); + Future future; + try { + while (results.size() < targets.size() && (future = completionService.take()) != null) { + try { + results.add(future.get()); + } catch (ExecutionException e) { + // TODO Handle errors + throw new RuntimeException(e); + } + } + return SystemFlagsDeployResult.merge(results); + } catch (InterruptedException e) { + throw new RuntimeException(e); + } + } + + // TODO Handle http status code 4xx/5xx (e.g for unknown flag id) + private SystemFlagsDeployResult deployFlags(FlagsTarget target, Set flagData, boolean dryRun) { + Map wantedFlagData = lookupTable(flagData); + Map currentFlagData = lookupTable(client.listFlagData(target)); + + List result = new ArrayList<>(); + + wantedFlagData.forEach((id, data) -> { + if (currentFlagData.containsKey(id)) { + FlagData currentData = currentFlagData.get(id); + if (currentData.toJsonNode().equals(data.toJsonNode())) { + return; // noop + } + result.add(FlagDataChange.updated(id, Set.of(target), data, currentData)); + } else { + result.add(FlagDataChange.created(id, Set.of(target), data)); + } + if (!dryRun) { + client.putFlagData(target, data); + } + }); + + currentFlagData.forEach((id, data) -> { + if (!wantedFlagData.containsKey(id)) { + if (!dryRun) { + client.deleteFlagData(target, id); + } + result.add(FlagDataChange.deleted(id, Set.of(target))); + } + }); + + return new SystemFlagsDeployResult(result); + } + + private static Map lookupTable(Collection data) { + return data.stream().collect(Collectors.toMap(FlagData::id, Function.identity())); + } + +} diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/SystemFlagsHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/SystemFlagsHandler.java new file mode 100644 index 00000000000..8e5376f9b9c --- /dev/null +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/SystemFlagsHandler.java @@ -0,0 +1,67 @@ +// 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.systemflags; + +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.container.logging.AccessLog; +import com.yahoo.restapi.ErrorResponse; +import com.yahoo.restapi.JacksonJsonResponse; +import com.yahoo.restapi.Path; +import com.yahoo.vespa.athenz.identity.ServiceIdentityProvider; +import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneRegistry; +import com.yahoo.vespa.hosted.controller.api.systemflags.v1.FlagsTarget; +import com.yahoo.vespa.hosted.controller.api.systemflags.v1.SystemFlagsDataArchive; + +import java.util.concurrent.Executor; + +/** + * Handler implementation for '/system-flags/v1', an API for controlling system-wide feature flags + * + * @author bjorncs + */ +@SuppressWarnings("unused") // Request handler listed in controller's services.xml +class SystemFlagsHandler extends LoggingRequestHandler { + + private static final String API_PREFIX = "/system-flags/v1"; + + private final SystemFlagsDeployer deployer; + + @Inject + public SystemFlagsHandler(ZoneRegistry zoneRegistry, + ServiceIdentityProvider identityProvider, + Executor executor, + AccessLog accessLog) { + super(executor, accessLog); + this.deployer = new SystemFlagsDeployer(identityProvider, FlagsTarget.getAllTargetsInSystem(zoneRegistry)); + } + + @Override + public HttpResponse handle(HttpRequest request) { + switch (request.getMethod()) { + case PUT: + return put(request); + default: + return ErrorResponse.methodNotAllowed("Method '" + request.getMethod() + "' is unsupported"); + } + } + + private HttpResponse put(HttpRequest request) { + Path path = new Path(request.getUri()); + if (path.matches(API_PREFIX + "/deploy")) return deploy(request); + return ErrorResponse.notFoundError("Nothing at " + path); + } + + private HttpResponse deploy(HttpRequest request) { + // TODO Error handling + String contentType = request.getHeader("Content-Type"); + if (!contentType.equalsIgnoreCase("application/zip")) { + return ErrorResponse.badRequest("Invalid content type: " + contentType); + } + SystemFlagsDataArchive archive = SystemFlagsDataArchive.fromZip(request.getData()); + SystemFlagsDeployResult result = deployer.deployFlags(archive, request.getBooleanProperty("dry-run")); + return new JacksonJsonResponse<>(200, result.toWire()); + } + +} diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/SystemFlagsDeployerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/SystemFlagsDeployerTest.java new file mode 100644 index 00000000000..b64d609404b --- /dev/null +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/SystemFlagsDeployerTest.java @@ -0,0 +1,76 @@ +// 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.systemflags; + +import com.yahoo.config.provision.SystemName; +import com.yahoo.vespa.flags.FlagId; +import com.yahoo.vespa.flags.json.FlagData; +import com.yahoo.vespa.hosted.controller.api.systemflags.v1.FlagsTarget; +import com.yahoo.vespa.hosted.controller.api.systemflags.v1.SystemFlagsDataArchive; +import com.yahoo.vespa.hosted.controller.integration.ZoneApiMock; +import com.yahoo.vespa.hosted.controller.integration.ZoneRegistryMock; +import org.junit.Test; + +import java.io.IOException; +import java.util.List; +import java.util.Set; + +import static com.yahoo.vespa.hosted.controller.restapi.systemflags.SystemFlagsDeployResult.FlagDataChange; +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +/** + * @author bjorncs + */ +public class SystemFlagsDeployerTest { + + private static final SystemName SYSTEM = SystemName.main; + + @Test + public void deploys_flag_data_to_targets() throws IOException { + ZoneApiMock prodUsWest1Zone = ZoneApiMock.fromId("prod.us-west-1"); + ZoneApiMock prodUsEast3Zone = ZoneApiMock.fromId("prod.us-east-3"); + ZoneRegistryMock registry = new ZoneRegistryMock(SYSTEM).setZones(prodUsWest1Zone, prodUsEast3Zone); + + FlagsTarget controllerTarget = FlagsTarget.forController(SYSTEM); + FlagsTarget prodUsWest1Target = FlagsTarget.forConfigserver(registry, prodUsWest1Zone.getId()); + FlagsTarget prodUsEast3Target = FlagsTarget.forConfigserver(registry, prodUsEast3Zone.getId()); + + FlagsClient flagsClient = mock(FlagsClient.class); + when(flagsClient.listFlagData(controllerTarget)).thenReturn(List.of()); + when(flagsClient.listFlagData(prodUsWest1Target)).thenReturn(List.of(flagData("existing-prod.us-west-1.json"))); + FlagData existingProdUsEast3Data = flagData("existing-prod.us-east-3.json"); + when(flagsClient.listFlagData(prodUsEast3Target)).thenReturn(List.of(existingProdUsEast3Data)); + + FlagData defaultData = flagData("flags/my-flag/main.json"); + FlagData prodUsEast3Data = flagData("flags/my-flag/main.prod.us-east-3.json"); + SystemFlagsDataArchive archive = new SystemFlagsDataArchive.Builder() + .addFile("main.json", defaultData) + .addFile("main.prod.us-east-3.json", prodUsEast3Data) + .build(); + + SystemFlagsDeployer deployer = + new SystemFlagsDeployer(flagsClient, Set.of(controllerTarget, prodUsWest1Target, prodUsEast3Target)); + + + SystemFlagsDeployResult result = deployer.deployFlags(archive, false); + + verify(flagsClient).putFlagData(controllerTarget, defaultData); + verify(flagsClient).putFlagData(prodUsEast3Target, prodUsEast3Data); + verify(flagsClient, never()).putFlagData(prodUsWest1Target, defaultData); + List changes = result.flagChanges(); + FlagId flagId = new FlagId("my-flag"); + assertThat(changes).containsOnly( + FlagDataChange.created(flagId, Set.of(controllerTarget), defaultData), + FlagDataChange.updated(flagId, Set.of(prodUsEast3Target), prodUsEast3Data, existingProdUsEast3Data)); + + } + + private static FlagData flagData(String filename) throws IOException { + return FlagData.deserializeUtf8Json( + SystemFlagsDeployerTest.class.getResourceAsStream("/system-flags/" + filename).readAllBytes()); + } + +} \ No newline at end of file diff --git a/controller-server/src/test/resources/system-flags/existing-prod.us-east-3.json b/controller-server/src/test/resources/system-flags/existing-prod.us-east-3.json new file mode 100644 index 00000000000..8db6c423e4d --- /dev/null +++ b/controller-server/src/test/resources/system-flags/existing-prod.us-east-3.json @@ -0,0 +1,8 @@ +{ + "id" : "my-flag", + "rules" : [ + { + "value" : "prod.us-east-3.original" + } + ] +} \ No newline at end of file diff --git a/controller-server/src/test/resources/system-flags/existing-prod.us-west-1.json b/controller-server/src/test/resources/system-flags/existing-prod.us-west-1.json new file mode 100644 index 00000000000..70fa0624a21 --- /dev/null +++ b/controller-server/src/test/resources/system-flags/existing-prod.us-west-1.json @@ -0,0 +1,8 @@ +{ + "id" : "my-flag", + "rules" : [ + { + "value" : "default" + } + ] +} \ No newline at end of file diff --git a/controller-server/src/test/resources/system-flags/flags/my-flag/main.json b/controller-server/src/test/resources/system-flags/flags/my-flag/main.json new file mode 100644 index 00000000000..70fa0624a21 --- /dev/null +++ b/controller-server/src/test/resources/system-flags/flags/my-flag/main.json @@ -0,0 +1,8 @@ +{ + "id" : "my-flag", + "rules" : [ + { + "value" : "default" + } + ] +} \ No newline at end of file diff --git a/controller-server/src/test/resources/system-flags/flags/my-flag/main.prod.us-east-3.json b/controller-server/src/test/resources/system-flags/flags/my-flag/main.prod.us-east-3.json new file mode 100644 index 00000000000..e7e8a318a6f --- /dev/null +++ b/controller-server/src/test/resources/system-flags/flags/my-flag/main.prod.us-east-3.json @@ -0,0 +1,8 @@ +{ + "id" : "my-flag", + "rules" : [ + { + "value" : "us-east-3" + } + ] +} \ No newline at end of file -- cgit v1.2.3 From a6c73c4606d49fe3441f8d73a0fba43a27597791 Mon Sep 17 00:00:00 2001 From: Bjørn Christian Seime Date: Fri, 8 Nov 2019 10:59:42 +0100 Subject: Rename 'ConfigserverFlagsTarget' -> 'ConfigServerFlagsTarget' --- .../systemflags/v1/ConfigServerFlagsTarget.java | 51 ++++++++++++++++++++++ .../systemflags/v1/ConfigserverFlagsTarget.java | 51 ---------------------- .../controller/api/systemflags/v1/FlagsTarget.java | 6 +-- .../systemflags/v1/SystemFlagsDataArchiveTest.java | 2 +- .../systemflags/SystemFlagsDeployerTest.java | 4 +- 5 files changed, 57 insertions(+), 57 deletions(-) create mode 100644 controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/ConfigServerFlagsTarget.java delete mode 100644 controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/ConfigserverFlagsTarget.java (limited to 'controller-server') diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/ConfigServerFlagsTarget.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/ConfigServerFlagsTarget.java new file mode 100644 index 00000000000..0304d1a5949 --- /dev/null +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/ConfigServerFlagsTarget.java @@ -0,0 +1,51 @@ +// 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.api.systemflags.v1; + +import com.yahoo.config.provision.SystemName; +import com.yahoo.config.provision.zone.ZoneId; +import com.yahoo.vespa.athenz.api.AthenzIdentity; + +import java.net.URI; +import java.util.List; +import java.util.Objects; +import java.util.Optional; + +import static com.yahoo.vespa.hosted.controller.api.systemflags.v1.FlagsTarget.defaultFile; +import static com.yahoo.vespa.hosted.controller.api.systemflags.v1.FlagsTarget.environmentFile; +import static com.yahoo.vespa.hosted.controller.api.systemflags.v1.FlagsTarget.systemFile; +import static com.yahoo.vespa.hosted.controller.api.systemflags.v1.FlagsTarget.zoneFile; + +/** + * @author bjorncs + */ +class ConfigServerFlagsTarget implements FlagsTarget { + private final SystemName system; + private final ZoneId zone; + private final URI endpoint; + private final AthenzIdentity identity; + + ConfigServerFlagsTarget(SystemName system, ZoneId zone, URI endpoint, AthenzIdentity identity) { + this.system = Objects.requireNonNull(system); + this.zone = Objects.requireNonNull(zone); + this.endpoint = Objects.requireNonNull(endpoint); + this.identity = Objects.requireNonNull(identity); + } + + @Override public List flagDataFilesPrioritized() { return List.of(zoneFile(system, zone), environmentFile(system, zone.environment()), systemFile(system), defaultFile()); } + @Override public URI endpoint() { return endpoint; } + @Override public Optional athenzHttpsIdentity() { return Optional.of(identity); } + @Override public String asString() { return String.format("%s.%s", system.value(), zone.value()); } + + @Override public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + ConfigServerFlagsTarget that = (ConfigServerFlagsTarget) o; + return system == that.system && + Objects.equals(zone, that.zone) && + Objects.equals(endpoint, that.endpoint) && + Objects.equals(identity, that.identity); + } + + @Override public int hashCode() { return Objects.hash(system, zone, endpoint, identity); } +} + diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/ConfigserverFlagsTarget.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/ConfigserverFlagsTarget.java deleted file mode 100644 index 6863b68ba72..00000000000 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/ConfigserverFlagsTarget.java +++ /dev/null @@ -1,51 +0,0 @@ -// 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.api.systemflags.v1; - -import com.yahoo.config.provision.SystemName; -import com.yahoo.config.provision.zone.ZoneId; -import com.yahoo.vespa.athenz.api.AthenzIdentity; - -import java.net.URI; -import java.util.List; -import java.util.Objects; -import java.util.Optional; - -import static com.yahoo.vespa.hosted.controller.api.systemflags.v1.FlagsTarget.defaultFile; -import static com.yahoo.vespa.hosted.controller.api.systemflags.v1.FlagsTarget.environmentFile; -import static com.yahoo.vespa.hosted.controller.api.systemflags.v1.FlagsTarget.systemFile; -import static com.yahoo.vespa.hosted.controller.api.systemflags.v1.FlagsTarget.zoneFile; - -/** - * @author bjorncs - */ -class ConfigserverFlagsTarget implements FlagsTarget { - private final SystemName system; - private final ZoneId zone; - private final URI endpoint; - private final AthenzIdentity identity; - - ConfigserverFlagsTarget(SystemName system, ZoneId zone, URI endpoint, AthenzIdentity identity) { - this.system = Objects.requireNonNull(system); - this.zone = Objects.requireNonNull(zone); - this.endpoint = Objects.requireNonNull(endpoint); - this.identity = Objects.requireNonNull(identity); - } - - @Override public List flagDataFilesPrioritized() { return List.of(zoneFile(system, zone), environmentFile(system, zone.environment()), systemFile(system), defaultFile()); } - @Override public URI endpoint() { return endpoint; } - @Override public Optional athenzHttpsIdentity() { return Optional.of(identity); } - @Override public String asString() { return String.format("%s.%s", system.value(), zone.value()); } - - @Override public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - ConfigserverFlagsTarget that = (ConfigserverFlagsTarget) o; - return system == that.system && - Objects.equals(zone, that.zone) && - Objects.equals(endpoint, that.endpoint) && - Objects.equals(identity, that.identity); - } - - @Override public int hashCode() { return Objects.hash(system, zone, endpoint, identity); } -} - diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/FlagsTarget.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/FlagsTarget.java index 7b63cfbdcf2..1b7f84d1ec7 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/FlagsTarget.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/FlagsTarget.java @@ -41,7 +41,7 @@ public interface FlagsTarget { SystemName system = registry.system(); Set targets = new HashSet<>(); for (ZoneApi zone : registry.zones().reachable().zones()) { - targets.add(forConfigserver(registry, zone.getId())); + targets.add(forConfigServer(registry, zone.getId())); } targets.add(forController(system)); return targets; @@ -51,8 +51,8 @@ public interface FlagsTarget { return new ControllerFlagsTarget(systemName); } - static FlagsTarget forConfigserver(ZoneRegistry registry, ZoneId zoneId) { - return new ConfigserverFlagsTarget( + static FlagsTarget forConfigServer(ZoneRegistry registry, ZoneId zoneId) { + return new ConfigServerFlagsTarget( registry.system(), zoneId, registry.getConfigServerVipUri(zoneId), registry.getConfigServerHttpsIdentity(zoneId)); } diff --git a/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/SystemFlagsDataArchiveTest.java b/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/SystemFlagsDataArchiveTest.java index eb0e90f8ca1..35fec04e4c5 100644 --- a/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/SystemFlagsDataArchiveTest.java +++ b/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/SystemFlagsDataArchiveTest.java @@ -45,7 +45,7 @@ public class SystemFlagsDataArchiveTest { private static FlagsTarget devUsEast1CfgTarget = createConfigserverTarget(Environment.dev, "us-east-1"); private static FlagsTarget createConfigserverTarget(Environment environment, String region) { - return new ConfigserverFlagsTarget( + return new ConfigServerFlagsTarget( SYSTEM, ZoneId.from(environment, RegionName.from(region)), URI.create("https://cfg-" + region), diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/SystemFlagsDeployerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/SystemFlagsDeployerTest.java index b64d609404b..42e33bc2f8f 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/SystemFlagsDeployerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/SystemFlagsDeployerTest.java @@ -35,8 +35,8 @@ public class SystemFlagsDeployerTest { ZoneRegistryMock registry = new ZoneRegistryMock(SYSTEM).setZones(prodUsWest1Zone, prodUsEast3Zone); FlagsTarget controllerTarget = FlagsTarget.forController(SYSTEM); - FlagsTarget prodUsWest1Target = FlagsTarget.forConfigserver(registry, prodUsWest1Zone.getId()); - FlagsTarget prodUsEast3Target = FlagsTarget.forConfigserver(registry, prodUsEast3Zone.getId()); + FlagsTarget prodUsWest1Target = FlagsTarget.forConfigServer(registry, prodUsWest1Zone.getId()); + FlagsTarget prodUsEast3Target = FlagsTarget.forConfigServer(registry, prodUsEast3Zone.getId()); FlagsClient flagsClient = mock(FlagsClient.class); when(flagsClient.listFlagData(controllerTarget)).thenReturn(List.of()); -- cgit v1.2.3 From 95bda19dcb52647c5b8fa84fa9b6e244987c9021 Mon Sep 17 00:00:00 2001 From: Bjørn Christian Seime Date: Fri, 8 Nov 2019 11:19:56 +0100 Subject: Include error code from response in exception message --- .../restapi/systemflags/FlagsClient.java | 32 +++++++++++----------- 1 file changed, 16 insertions(+), 16 deletions(-) (limited to 'controller-server') diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/FlagsClient.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/FlagsClient.java index 9d21281ce9f..d11e17ce634 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/FlagsClient.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/FlagsClient.java @@ -115,19 +115,20 @@ class FlagsClient { private static void verifySuccess(HttpResponse response, FlagsTarget target, FlagId flagId) throws IOException { if (!success(response)) { - String message = getErrorMessage(response); - throw new FlagsException(response.getStatusLine().getStatusCode(), target, flagId, message); + throw createFlagsException(response, target, flagId); } } - private static String getErrorMessage(HttpResponse response) throws IOException { + private static FlagsException createFlagsException(HttpResponse response, FlagsTarget target, FlagId flagId) throws IOException { HttpEntity entity = response.getEntity(); String content = EntityUtils.toString(entity); + int statusCode = response.getStatusLine().getStatusCode(); if (ContentType.get(entity).getMimeType().equals(ContentType.APPLICATION_JSON.getMimeType())) { - WireErrorResponse errorResponse = mapper.readValue(content, WireErrorResponse.class); - return errorResponse.message; + WireErrorResponse error = mapper.readValue(content, WireErrorResponse.class); + return new FlagsException(statusCode, target, flagId, error.errorCode, error.message); + } else { + return new FlagsException(statusCode, target, flagId, null, content); } - return content; } private static boolean success(HttpResponse response) { @@ -161,21 +162,20 @@ class FlagsClient { static class FlagsException extends RuntimeException { - private FlagsException(int statusCode, FlagsTarget target, String responseMessage) { - this(statusCode, target, null, responseMessage); + private FlagsException(int statusCode, FlagsTarget target, FlagId flagId, String errorCode, String errorMessage) { + super(createErrorMessage(statusCode, target, flagId, errorCode, errorMessage)); } - private FlagsException(int statusCode, FlagsTarget target, FlagId flagId, String responseMessage) { - super(createErrorMessage(statusCode, target, flagId, responseMessage)); - } - - private static String createErrorMessage(int statusCode, FlagsTarget target, FlagId flagId, String responseMessage) { - StringBuilder builder = new StringBuilder() - .append("Received '").append(statusCode).append("' from '").append(target.endpoint().getHost()).append("'"); + private static String createErrorMessage(int statusCode, FlagsTarget target, FlagId flagId, String errorCode, String errorMessage) { + StringBuilder builder = new StringBuilder().append("Received ").append(statusCode); + if (errorCode != null) { + builder.append('/').append(errorCode); + } + builder.append(" from '").append(target.endpoint().getHost()).append("'"); if (flagId != null) { builder.append("' for flag '").append(flagId).append("'"); } - return builder.append(": ").append(responseMessage).toString(); + return builder.append(": ").append(errorMessage).toString(); } } } -- cgit v1.2.3 From 4b632e8a210a6487975ffd9b2959c4f4af673b5d Mon Sep 17 00:00:00 2001 From: Bjørn Christian Seime Date: Fri, 8 Nov 2019 11:42:05 +0100 Subject: Move dryrun deploy to separate path --- .../controller/api/systemflags/v1/wire/SystemFlagsV1Api.java | 10 +++++++--- .../controller/restapi/systemflags/SystemFlagsHandler.java | 7 ++++--- 2 files changed, 11 insertions(+), 6 deletions(-) (limited to 'controller-server') diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/wire/SystemFlagsV1Api.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/wire/SystemFlagsV1Api.java index 2b7119a5a13..1107b70c01c 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/wire/SystemFlagsV1Api.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/wire/SystemFlagsV1Api.java @@ -2,11 +2,9 @@ package com.yahoo.vespa.hosted.controller.api.systemflags.v1.wire; import javax.ws.rs.Consumes; -import javax.ws.rs.DefaultValue; import javax.ws.rs.PUT; import javax.ws.rs.Path; import javax.ws.rs.Produces; -import javax.ws.rs.QueryParam; import javax.ws.rs.core.MediaType; import java.io.InputStream; @@ -20,6 +18,12 @@ public interface SystemFlagsV1Api { @Produces(MediaType.APPLICATION_JSON) @Consumes("application/zip") @Path("/deploy") - WireSystemFlagsDeployResult deploy(@QueryParam("dryRun") @DefaultValue("false") boolean dryRun, InputStream inputStream); + WireSystemFlagsDeployResult deploy(InputStream inputStream); + + @PUT + @Produces(MediaType.APPLICATION_JSON) + @Consumes("application/zip") + @Path("/dryrun") + WireSystemFlagsDeployResult dryrun(InputStream inputStream); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/SystemFlagsHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/SystemFlagsHandler.java index 8e5376f9b9c..08bb7628080 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/SystemFlagsHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/SystemFlagsHandler.java @@ -49,18 +49,19 @@ class SystemFlagsHandler extends LoggingRequestHandler { private HttpResponse put(HttpRequest request) { Path path = new Path(request.getUri()); - if (path.matches(API_PREFIX + "/deploy")) return deploy(request); + if (path.matches(API_PREFIX + "/deploy")) return deploy(request, /*dryRun*/false); + if (path.matches(API_PREFIX + "/dryrun")) return deploy(request, /*dryRun*/true); return ErrorResponse.notFoundError("Nothing at " + path); } - private HttpResponse deploy(HttpRequest request) { + private HttpResponse deploy(HttpRequest request, boolean dryRun) { // TODO Error handling String contentType = request.getHeader("Content-Type"); if (!contentType.equalsIgnoreCase("application/zip")) { return ErrorResponse.badRequest("Invalid content type: " + contentType); } SystemFlagsDataArchive archive = SystemFlagsDataArchive.fromZip(request.getData()); - SystemFlagsDeployResult result = deployer.deployFlags(archive, request.getBooleanProperty("dry-run")); + SystemFlagsDeployResult result = deployer.deployFlags(archive, dryRun); return new JacksonJsonResponse<>(200, result.toWire()); } -- cgit v1.2.3