Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Expose styx provider object database in YAML via admin interface #637

Merged
merged 7 commits into from
Mar 6, 2020
Merged

Expose styx provider object database in YAML via admin interface #637

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
dcd373e
Return valid yaml for admin routing objects list.
chrisgresty Feb 27, 2020
dfe03a3
Unused imports
chrisgresty Feb 27, 2020
88a031e
Return provider DB objects as YAML
chrisgresty Feb 28, 2020
52df4f7
Checkstyle
chrisgresty Feb 28, 2020
f5056c4
Test provider yaml is returned
chrisgresty Feb 28, 2020
dd8af80
Return Provider and Routing objects from admin portal as Maps instead…
chrisgresty Mar 5, 2020
9435f0b
Fix test
chrisgresty Mar 5, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
108 changes: 102 additions & 6 deletions components/proxy/src/main/java/com/hotels/styx/admin/handlers/ProviderRoutingHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,12 @@
*/
package com.hotels.styx.admin.handlers;

import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.dataformat.yaml.YAMLFactory;
import com.google.common.collect.ImmutableList;
import com.hotels.styx.StyxObjectRecord;
import com.hotels.styx.api.Eventual;
import com.hotels.styx.api.HttpInterceptor;
Expand All @@ -24,11 +30,24 @@
import com.hotels.styx.api.configuration.ObjectStore;
import com.hotels.styx.api.extension.service.spi.StyxService;
import com.hotels.styx.common.http.handler.HttpStreamer;
import com.hotels.styx.routing.config.StyxObjectDefinition;
import com.hotels.styx.routing.db.StyxObjectStore;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import reactor.core.publisher.Flux;

import java.util.List;
import java.util.stream.Collectors;

import static com.fasterxml.jackson.core.JsonParser.Feature.AUTO_CLOSE_SOURCE;
import static com.fasterxml.jackson.databind.DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES;
import static com.hotels.styx.admin.handlers.UrlPatternRouter.placeholders;
import static com.hotels.styx.api.HttpResponse.response;
import static com.hotels.styx.api.HttpResponseStatus.NOT_FOUND;
import static com.hotels.styx.api.HttpResponseStatus.OK;
import static com.hotels.styx.infrastructure.configuration.json.ObjectMappers.addStyxMixins;
import static java.nio.charset.StandardCharsets.UTF_8;

/**
* Routes admin requests to the admin endpoints of each {@link com.hotels.styx.api.extension.service.spi.StyxService}
* in the Provider {@link ObjectStore}, and to the index page that organizes and lists these endpoints.
Expand All @@ -38,6 +57,9 @@ public class ProviderRoutingHandler implements WebServiceHandler {

private static final Logger LOG = LoggerFactory.getLogger(ProviderRoutingHandler.class);
private static final int MEGABYTE = 1024 * 1024;
private static final ObjectMapper YAML_MAPPER = addStyxMixins(new ObjectMapper(new YAMLFactory()))
.configure(FAIL_ON_UNKNOWN_PROPERTIES, false)
.configure(AUTO_CLOSE_SOURCE, true);

private final String pathPrefix;
private volatile UrlPatternRouter router;
Expand Down Expand Up @@ -67,16 +89,90 @@ private void refreshRoutes(ObjectStore<? extends StyxObjectRecord<? extends Styx

private UrlPatternRouter buildRouter(ObjectStore<? extends StyxObjectRecord<? extends StyxService>> db) {
UrlPatternRouter.Builder routeBuilder = new UrlPatternRouter.Builder(pathPrefix)
.get("", new ProviderListHandler(db));
.get("", new ProviderListHandler(db))
.get("objects", (request, context) -> {
return handleRequestForAllObjects(request, context, db);
})
.get("objects/:objectName", (request, context) -> {
String name = placeholders(context).get("objectName");
return handleRequestForOneObject(request, context, db, name);
});

db.entrySet().forEach(entry -> {
String providerName = entry.getKey();
entry.getValue().getStyxService().adminInterfaceHandlers(pathPrefix + "/" + providerName)
.forEach((relPath, handler) ->
routeBuilder.get(providerName + "/" + relPath, new HttpStreamer(MEGABYTE, handler))
);
String providerName = entry.getKey();
entry.getValue().getStyxService().adminInterfaceHandlers(pathPrefix + "/" + providerName)
.forEach((relPath, handler) ->
routeBuilder.get(providerName + "/" + relPath, new HttpStreamer(MEGABYTE, handler))
);
});

return routeBuilder.build();
}

private Eventual<HttpResponse> handleRequestForAllObjects(HttpRequest request, HttpInterceptor.Context context,
ObjectStore<? extends StyxObjectRecord<? extends StyxService>> db) {
List<StyxObjectDefinition> objects = db.entrySet()
.stream()
.map(entry -> createObjectDef(entry.getKey(), entry.getValue()))
.collect(Collectors.toList());
String output = serialise(objects);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An opinionated comment, but I would inline the objects because it is only ever passed on to serialise method. ;-)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm. In this case I find it flows better as it is, because the stream construction is (in my mind) quite different to the method call, and inlining just results in my eye bouncing around a little more than I'd like.
Ideally I'd have liked to be able to follow the .collect() with .serialise() (or some construction with ::serialise) but that's not available. Visually, I think this is the next best thing. But I'll change it (and the others) if you feel moderately strongly about it.


return Eventual.of(response(OK)
.body(output, UTF_8)
.build());
}

private Eventual<HttpResponse> handleRequestForOneObject(HttpRequest request, HttpInterceptor.Context context,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The request and context arguments are never used.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not exactly sure why I left them in.

ObjectStore<? extends StyxObjectRecord<? extends StyxService>> db,
String name) {
try {
String object = db.get(name)
.map(record -> createObjectDef(name, record))
.map(ProviderRoutingHandler::serialise)
.orElseThrow(ProviderRoutingHandler.ResourceNotFoundException::new);

return Eventual.of(response(OK).body(object, UTF_8).build());
} catch (ProviderRoutingHandler.ResourceNotFoundException e) {
return Eventual.of(response(NOT_FOUND).build());
}
}

private static StyxObjectDefinition createObjectDef(String name, StyxObjectRecord<? extends StyxService> record) {
return new StyxObjectDefinition(name, record.getType(), ImmutableList.copyOf(record.getTags()), record.getConfig());
}

/**
* Serializes either a single {@link com.hotels.styx.routing.config.StyxObjectDefinition} or
* a collection of them.
*/
private static String serialise(Object object) {
JsonNode json = YAML_MAPPER
.addMixIn(StyxObjectDefinition.class, ProviderObjectDefMixin.class)
.valueToTree(object);

try {
return YAML_MAPPER.writeValueAsString(json);
} catch (JsonProcessingException e) {
throw new RuntimeException(e);
}
}

private static class ResourceNotFoundException extends RuntimeException {
}

private abstract static class ProviderObjectDefMixin {
@JsonProperty("name")
public abstract String name();

@JsonProperty("type")
public abstract String type();

@JsonProperty("tags")
public abstract List<String> tags();

@JsonProperty("config")
public abstract JsonNode config();
}


}
39 changes: 23 additions & 16 deletions components/proxy/src/main/java/com/hotels/styx/admin/handlers/RoutingObjectHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
Copyright (C) 2013-2019 Expedia Inc.
Copyright (C) 2013-2020 Expedia Inc.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
Expand All @@ -19,7 +19,6 @@
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.node.ObjectNode;
import com.fasterxml.jackson.dataformat.yaml.YAMLFactory;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
Expand All @@ -34,10 +33,10 @@
import com.hotels.styx.routing.config.RoutingObjectFactory;
import com.hotels.styx.routing.config.StyxObjectDefinition;
import com.hotels.styx.routing.db.StyxObjectStore;
import org.slf4j.Logger;

import java.io.IOException;
import java.util.List;
import java.util.stream.Collectors;

import static com.fasterxml.jackson.core.JsonParser.Feature.AUTO_CLOSE_SOURCE;
import static com.fasterxml.jackson.databind.DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES;
Expand All @@ -49,14 +48,11 @@
import static com.hotels.styx.api.HttpResponseStatus.OK;
import static com.hotels.styx.infrastructure.configuration.json.ObjectMappers.addStyxMixins;
import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.stream.Collectors.joining;
import static org.slf4j.LoggerFactory.getLogger;

/**
* Provides admin interface access to Styx routing configuration.
*/
public class RoutingObjectHandler implements WebServiceHandler {
private static final Logger LOGGER = getLogger(RoutingObjectHandler.class);

private static final ObjectMapper YAML_MAPPER = addStyxMixins(new ObjectMapper(new YAMLFactory()))
.configure(FAIL_ON_UNKNOWN_PROPERTIES, false)
Expand All @@ -68,10 +64,11 @@ public class RoutingObjectHandler implements WebServiceHandler {
public RoutingObjectHandler(StyxObjectStore<RoutingObjectRecord> routeDatabase, RoutingObjectFactory.Context routingObjectFactoryContext) {
urlRouter = new UrlPatternRouter.Builder()
.get("/admin/routing/objects", (request, context) -> {
String output = routeDatabase.entrySet()
List<StyxObjectDefinition> objects = routeDatabase.entrySet()
.stream()
.map(entry -> serialise(entry.getKey(), entry.getValue()))
.collect(joining("\n"));
.map(entry -> createObjectDef(entry.getKey(), entry.getValue()))
.collect(Collectors.toList());
String output = serialise(objects);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opinionated: I would inline objects because it is only ever passed, unconditionally, as an argument to objects. Therefore it just adds noise. :-)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possibly below, too.


return Eventual.of(response(OK)
.body(output, UTF_8)
Expand All @@ -82,7 +79,8 @@ public RoutingObjectHandler(StyxObjectStore<RoutingObjectRecord> routeDatabase,

try {
String object = routeDatabase.get(name)
.map(record -> serialise(name, record))
.map(record -> createObjectDef(name, record))
.map(RoutingObjectHandler::serialise)
.orElseThrow(ResourceNotFoundException::new);

return Eventual.of(response(OK).body(object, UTF_8).build());
Expand Down Expand Up @@ -117,15 +115,21 @@ public RoutingObjectHandler(StyxObjectStore<RoutingObjectRecord> routeDatabase,
.build();
}

private static String serialise(String name, RoutingObjectRecord record) {
JsonNode node = YAML_MAPPER
.addMixIn(StyxObjectDefinition.class, RoutingObjectDefMixin.class)
.valueToTree(new StyxObjectDefinition(name, record.getType(), ImmutableList.copyOf(record.getTags()), record.getConfig()));
private static StyxObjectDefinition createObjectDef(String name, RoutingObjectRecord record) {
return new StyxObjectDefinition(name, record.getType(), ImmutableList.copyOf(record.getTags()), record.getConfig());
}

((ObjectNode) node).set("config", record.getConfig());
/**
* Serializes either a single {@link com.hotels.styx.routing.config.StyxObjectDefinition} or
* a collection of them.
*/
private static String serialise(Object object) {
JsonNode json = YAML_MAPPER
.addMixIn(StyxObjectDefinition.class, RoutingObjectDefMixin.class)
.valueToTree(object);

try {
return YAML_MAPPER.writeValueAsString(node);
return YAML_MAPPER.writeValueAsString(json);
} catch (JsonProcessingException e) {
throw new RuntimeException(e);
}
Expand All @@ -148,6 +152,9 @@ private abstract static class RoutingObjectDefMixin {

@JsonProperty("tags")
public abstract List<String> tags();

@JsonProperty("config")
public abstract JsonNode config();
}

}
Expand Down
30 changes: 14 additions & 16 deletions components/proxy/src/test/kotlin/com/hotels/styx/admin/handlers/RoutingObjectHandlerTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,22 +135,20 @@ class RoutingObjectHandlerTest : FeatureSpec({
it!!.status() shouldBe OK
it.bodyAs(UTF_8).trim() shouldBe """
---
name: "conditionRouter"
type: "ConditionRouter"
tags: []
config:
routes:
- condition: "path() == \"/bar\""
destination: "b"
fallback: "fb"

---
name: "staticResponse"
type: "StaticResponseHandler"
tags: []
config:
status: 200
content: "Hello, world!"
- name: "conditionRouter"
type: "ConditionRouter"
tags: []
config:
routes:
- condition: "path() == \"/bar\""
destination: "b"
fallback: "fb"
- name: "staticResponse"
type: "StaticResponseHandler"
tags: []
config:
status: 200
content: "Hello, world!"
""".trimIndent().trim()
}

Expand Down
39 changes: 39 additions & 0 deletions system-tests/ft-suite/src/test/kotlin/com/hotels/styx/admin/ProviderAdminInterfaceSpec.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,45 @@ class ProviderAdminInterfaceSpec : FeatureSpec() {
body shouldContain "/admin/providers/myMonitor/status"
body shouldContain "/admin/providers/mySecondMonitor/status"
}

scenario("YAML configuration for all providers is available") {
val body = styxServer.adminRequest("/admin/providers/objects")
.bodyAs(UTF_8)
body shouldContain """
- name: "mySecondMonitor"
type: "HealthCheckMonitor"
tags: []
config:
objects: "bbb"
path: "/healthCheck/y"
timeoutMillis: 250
intervalMillis: 500
healthyThreshold: 3
unhealthyThreshold: 2
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should add here another object. So that the list would render at least two or more objects.

""".trimIndent()

body shouldContain """ - name: "myMonitor" """.trim()
body shouldContain """ - name: "originsFileLoader" """.trim()
}

scenario("YAML configuration for a single provider is available") {
val body = styxServer.adminRequest("/admin/providers/objects/myMonitor")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A small nag: IMHO the url should be /admin/provider/objects (The provider component should be in singular). /admin/providers/objects sounds wrong because both providers and objects are plural.

Do you think we could change it as a part of this PR, or would this warrant a separate PR?

Copy link
Contributor Author

@chrisgresty chrisgresty Mar 5, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a number of related URLs that could be changed like this - raised a new issue #641 to cover this.

.bodyAs(UTF_8)
body shouldNotContain """ name: "mySecondMonitor" """.trim()
body shouldNotContain """ name: "originsFileLoader" """.trim()
body shouldContain """
name: "myMonitor"
type: "HealthCheckMonitor"
tags: []
config:
objects: "aaa"
path: "/healthCheck/x"
timeoutMillis: 250
intervalMillis: 500
healthyThreshold: 3
unhealthyThreshold: 2
""".trimIndent()
}
}

feature("Endpoints for dynamically added Styx services are available in the Admin interface") {
Expand Down
28 changes: 13 additions & 15 deletions system-tests/ft-suite/src/test/kotlin/com/hotels/styx/routing/RoutingRestApiSpec.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
Copyright (C) 2013-2019 Expedia Inc.
Copyright (C) 2013-2020 Expedia Inc.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -120,20 +120,18 @@ class RoutingRestApiSpec : StringSpec() {
it!!.status() shouldBe OK
it.bodyAs(UTF_8).trim() shouldBe """
---
name: "responder"
type: "StaticResponseHandler"
tags: []
config:
status: 200
content: "Responder"

---
name: "root"
type: "StaticResponseHandler"
tags: []
config:
status: 200
content: "Root"
- name: "responder"
type: "StaticResponseHandler"
tags: []
config:
status: 200
content: "Responder"
- name: "root"
type: "StaticResponseHandler"
tags: []
config:
status: 200
content: "Root"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm... 🤔

So conceptually this is a map. Also in styx yaml configuration this is represented as a map. But now this implementation renders it as a list.

For consistency, I would prefer the admin interface also to render it as a map.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, of course. I should have noticed that the config yaml uses maps, not lists here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the case where we are requesting a single object, should it be as now (with name as an attribute) or in a map format (with name as the key)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eh, forget that, it should be the single object payload with no name: attribute and no key.

""".trimIndent().trim()
}
}
Expand Down