From b51c1d2be33a0dd446325f387d1ebdf00d6ffa12 Mon Sep 17 00:00:00 2001 From: Clement Michaud Date: Wed, 17 Jan 2018 19:48:29 +0100 Subject: [PATCH] Use MARATHON_SCHEME_PORT label to define scheme of a given port The links in the tasks view use the same scheme as the Marathon UI web application but in some cases users expose a service with a different scheme on a given port. In order to correctly redirect the user clicking on the link of a given port, one can use MARATHON_SCHEME_PORT and MARATHON_SCHEME_PORT labels to define the scheme of the all the ports and/or the scheme of port with index N. The available values for those labels are 'http' and 'https'. If no label is provided, the scheme is the same as Marathon UI scheme to avoid breaking compatibility. --- src/js/components/AppPageComponent.jsx | 1 + src/js/components/TaskListComponent.jsx | 2 + src/js/components/TaskListItemComponent.jsx | 28 ++++++-- src/js/components/TaskViewComponent.jsx | 2 + src/js/helpers/ServiceSchemeUtil.js | 36 ++++++++++ src/test/units/ServiceSchemeUtil.test.js | 56 +++++++++++++++ src/test/units/TaskListItemComponent.test.js | 73 +++++++++++++++++++- 7 files changed, 191 insertions(+), 7 deletions(-) create mode 100644 src/js/helpers/ServiceSchemeUtil.js create mode 100644 src/test/units/ServiceSchemeUtil.test.js diff --git a/src/js/components/AppPageComponent.jsx b/src/js/components/AppPageComponent.jsx index 4abdfc59b..4a7ec11b9 100644 --- a/src/js/components/AppPageComponent.jsx +++ b/src/js/components/AppPageComponent.jsx @@ -352,6 +352,7 @@ var AppPageComponent = React.createClass({ fetchState={state.fetchState} getTaskHealthMessage={this.getTaskHealthMessage} hasHealth={Object.keys(model.healthChecks).length > 0} + labels={model.labels} tasks={model.tasks} /> ); }) diff --git a/src/js/components/TaskListItemComponent.jsx b/src/js/components/TaskListItemComponent.jsx index 0f7001cfa..992eab5d5 100644 --- a/src/js/components/TaskListItemComponent.jsx +++ b/src/js/components/TaskListItemComponent.jsx @@ -7,6 +7,8 @@ import AppsStore from "../stores/AppsStore"; import HealthStatus from "../constants/HealthStatus"; import TaskStatus from "../constants/TaskStatus"; import TaskFileDownloadComponent from "../components/TaskFileDownloadComponent"; +import ServiceSchemeUtil from "../helpers/ServiceSchemeUtil"; +import { watch } from "fs"; function joinNodes(nodes, separator = ", ") { var lastIndex = nodes.length - 1; @@ -22,6 +24,12 @@ function joinNodes(nodes, separator = ", ") { }); } +function getPortScheme(labels, portIndex) { + const scheme = ServiceSchemeUtil + .getServiceSchemeFromLabels(labels, portIndex); + return scheme == '' ? '//' : `${scheme}://`; +} + var TaskListItemComponent = React.createClass({ displayName: "TaskListItemComponent", @@ -29,6 +37,7 @@ var TaskListItemComponent = React.createClass({ appId: React.PropTypes.string.isRequired, hasHealth: React.PropTypes.bool, isActive: React.PropTypes.bool.isRequired, + labels: React.PropTypes.object.isRequired, onToggle: React.PropTypes.func.isRequired, sortKey: React.PropTypes.string, task: React.PropTypes.object.isRequired, @@ -37,6 +46,7 @@ var TaskListItemComponent = React.createClass({ getHostAndPorts: function () { var task = this.props.task; + var props = this.props; var ports = task.ports; if (ports == null || ports.length === 0 ) { @@ -44,9 +54,10 @@ var TaskListItemComponent = React.createClass({ } if (ports != null && ports.length === 1) { + const scheme = getPortScheme(props.labels, 0); return ( {`${task.host}:${ports[0]}`} @@ -54,11 +65,12 @@ var TaskListItemComponent = React.createClass({ } if (ports != null && ports.length > 1) { - let portNodes = ports.map(function (port) { + let portNodes = ports.map(function (port, i) { + const scheme = getPortScheme(props.labels, i); return ( {port} @@ -98,18 +110,22 @@ var TaskListItemComponent = React.createClass({ let ipAddress = address.ipAddress; if (serviceDiscoveryPorts.length === 1) { let port = serviceDiscoveryPorts[0].number; + const scheme = getPortScheme(props.labels, 0); return ( + className="text-muted" + href={`${scheme}${ipAddress}:${port}`}> {`${ipAddress}:${port}`} ); } - let portNodes = serviceDiscoveryPorts.map((port) => { + let portNodes = serviceDiscoveryPorts.map((port, i) => { + const scheme = getPortScheme(props.labels, i); return ( + className="text-muted" + href={`${scheme}${ipAddress}:${port.number}`}> {port.number} ); diff --git a/src/js/components/TaskViewComponent.jsx b/src/js/components/TaskViewComponent.jsx index 5a1d473d1..f25918ec8 100644 --- a/src/js/components/TaskViewComponent.jsx +++ b/src/js/components/TaskViewComponent.jsx @@ -18,6 +18,7 @@ var TaskViewComponent = React.createClass({ fetchState: React.PropTypes.number.isRequired, getTaskHealthMessage: React.PropTypes.func.isRequired, hasHealth: React.PropTypes.bool, + labels: React.PropTypes.object.isRequired, tasks: React.PropTypes.array.isRequired }, @@ -228,6 +229,7 @@ var TaskViewComponent = React.createClass({ hasHealth={this.props.hasHealth} onTaskToggle={this.onTaskToggle} itemsPerPage={this.state.itemsPerPage} + labels={this.props.labels} selectedTasks={this.state.selectedTasks} tasks={this.props.tasks} toggleAllTasks={this.toggleAllTasks} /> diff --git a/src/js/helpers/ServiceSchemeUtil.js b/src/js/helpers/ServiceSchemeUtil.js new file mode 100644 index 000000000..9eea7aaa9 --- /dev/null +++ b/src/js/helpers/ServiceSchemeUtil.js @@ -0,0 +1,36 @@ +const BASE_SCHEME_LABEL = "MARATHON_SCHEME_PORT"; + +var ServiceSchemeUtil = { + /* + * Returns service scheme of the n-th port. + * + * Given N a port index, if `MARATHON_SCHEME_PORT` is + * in the set of labels, then the function returns the value + * of this label. + * + * Given N a port index, if `MARATHON_SCHEME_PORT0` is + * not in the set of labels, then the value associated with + * `MARATHON_SCHEME_PORT` is returned. + * + * Given N a port index, if `MARATHON_SCHEME_PORT0` and + * `MARATHON_SCHEME_PORT` are not in the set of labels, the + * function returns the `http` as the default scheme. + */ + getServiceSchemeFromLabels(labels, n) { + function getScheme(labelValue) { + return (labelValue === "http" || labelValue === "https") + ? labelValue + : ''; + } + + const labelKey = ("" + BASE_SCHEME_LABEL + n); + if (labels && labelKey in labels) + return getScheme(labels[labelKey]); + else if (labels && BASE_SCHEME_LABEL in labels) + return getScheme(labels[BASE_SCHEME_LABEL]); + + return ''; + } +}; + +export default ServiceSchemeUtil; \ No newline at end of file diff --git a/src/test/units/ServiceSchemeUtil.test.js b/src/test/units/ServiceSchemeUtil.test.js new file mode 100644 index 000000000..236722ceb --- /dev/null +++ b/src/test/units/ServiceSchemeUtil.test.js @@ -0,0 +1,56 @@ +import {expect} from "chai"; +import ServiceSchemeUtil from "../../js/helpers/ServiceSchemeUtil"; + +function verifyPortSchemeWithDefault(serviceScheme, serviceScheme0, + expectedSchemePort0, expectedSchemePort1) { + describe("MARATHON_SCHEME_PORT is set to " + serviceScheme, function() { + describe("MARATHON_SCHEME_PORT0 is set to " + serviceScheme0, function() { + it("detect scheme of port 0", function() { + expect(ServiceSchemeUtil.getServiceSchemeFromLabels({ + "MARATHON_SCHEME_PORT": serviceScheme, + "MARATHON_SCHEME_PORT0": serviceScheme0, + }, 0)).to.eq(expectedSchemePort0); + }); + + it("detect scheme of port 1", function() { + expect(ServiceSchemeUtil.getServiceSchemeFromLabels({ + "MARATHON_SCHEME_PORT": serviceScheme, + "MARATHON_SCHEME_PORT0": serviceScheme0, + }, 1)).to.eq(expectedSchemePort1); + }); + }); + }); +} + +function verifyPortSchemeWithoutDefault(serviceScheme0, + expectedSchemePort0, expectedSchemePort1) { + describe("MARATHON_SCHEME_PORT is not set", function() { + describe("MARATHON_SCHEME_PORT0 is set to " + serviceScheme0, function() { + it("detect scheme of port 0", function() { + expect(ServiceSchemeUtil.getServiceSchemeFromLabels({ + "MARATHON_SCHEME_PORT0": serviceScheme0, + }, 0)).to.eq(expectedSchemePort0); + }); + + it("detect scheme of port 1", function() { + expect(ServiceSchemeUtil.getServiceSchemeFromLabels({ + "MARATHON_SCHEME_PORT0": serviceScheme0, + }, 1)).to.eq(expectedSchemePort1); + }); + }); + }); +} + +describe("ServiceSchemeUtil", function () { + // default scheme scheme port 0 expected port 0 expected port 1 + verifyPortSchemeWithDefault("http", "http", "http", "http"); + verifyPortSchemeWithDefault("http", "https", "https", "http"); + verifyPortSchemeWithDefault("https", "http", "http", "https"); + verifyPortSchemeWithDefault("https", "https", "https", "https"); + + // scheme port 0 expected port 0 expected port 1 + verifyPortSchemeWithoutDefault("http", "http", ""); + verifyPortSchemeWithoutDefault("https", "https", ""); + verifyPortSchemeWithoutDefault("http", "http", ""); + verifyPortSchemeWithoutDefault("https", "https", ""); +}); diff --git a/src/test/units/TaskListItemComponent.test.js b/src/test/units/TaskListItemComponent.test.js index 6bcbfd83a..5b1262d32 100644 --- a/src/test/units/TaskListItemComponent.test.js +++ b/src/test/units/TaskListItemComponent.test.js @@ -11,7 +11,7 @@ describe("Task List Item component", function () { appId: "/app-1", id: "task-123", host: "host-1", - ports: [1, 2, 3], + ports: [8081, 8082, 8083], status: "status-0", updatedAt: "2015-06-29T14:11:58.709Z", version: "2015-06-29T13:54:24.171Z" @@ -20,6 +20,7 @@ describe("Task List Item component", function () { this.component = shallow( {}} @@ -37,6 +38,76 @@ describe("Task List Item component", function () { ).to.equal("task-123"); }); + describe("task url are correct", function() { + function getNthPortLink(component, n) { + return component.find("td") + .at(1).children() + .at(2).children() + .at(2 + n).children().first().props().href + } + + it("has a HTTP task url when app does not have scheme label", function() { + expect(getNthPortLink(this.component, 0)).to.equal("//host-1:8081"); + expect(getNthPortLink(this.component, 1)).to.equal("//host-1:8082"); + expect(getNthPortLink(this.component, 2)).to.equal("//host-1:8083"); + }); + + it("has only https schemes", function() { + var model = { + appId: "/app-1", + id: "task-123", + host: "host-1", + ports: [8081, 8082, 8083], + status: "status-0", + updatedAt: "2015-06-29T14:11:58.709Z", + version: "2015-06-29T13:54:24.171Z" + }; + + this.component = shallow( + {}} + task={model} /> + ); + expect(getNthPortLink(this.component, 0)).to.equal("https://host-1:8081"); + expect(getNthPortLink(this.component, 1)).to.equal("https://host-1:8082"); + expect(getNthPortLink(this.component, 2)).to.equal("https://host-1:8083"); + }) + + it("has different schemes depending on the port", function() { + var model = { + appId: "/app-1", + id: "task-123", + host: "host-1", + ports: [8081, 8082, 8083], + status: "status-0", + updatedAt: "2015-06-29T14:11:58.709Z", + version: "2015-06-29T13:54:24.171Z" + }; + + this.component = shallow( + {}} + task={model} /> + ); + expect(getNthPortLink(this.component, 0)).to.equal("https://host-1:8081"); + expect(getNthPortLink(this.component, 1)).to.equal("//host-1:8082"); + expect(getNthPortLink(this.component, 2)).to.equal("http://host-1:8083"); + }) + }) + it("has correct health message", function () { expect(this.component.find("td").at(2).text()).to.equal("Healthy"); });