Skip to content
This repository has been archived by the owner on Dec 4, 2024. It is now read-only.

Commit

Permalink
Use MARATHON_SCHEME_PORT label to define scheme of a given port
Browse files Browse the repository at this point in the history
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<N>
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.
  • Loading branch information
clems4ever committed Feb 8, 2018
1 parent cee3b3d commit b51c1d2
Show file tree
Hide file tree
Showing 7 changed files with 191 additions and 7 deletions.
1 change: 1 addition & 0 deletions src/js/components/AppPageComponent.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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} />
</TabPaneComponent>
<TabPaneComponent
Expand Down
2 changes: 2 additions & 0 deletions src/js/components/TaskListComponent.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ var TaskListComponent = React.createClass({
getTaskHealthMessage: React.PropTypes.func.isRequired,
hasHealth: React.PropTypes.bool,
itemsPerPage: React.PropTypes.number.isRequired,
labels: React.PropTypes.object.isRequired,
onTaskToggle: React.PropTypes.func.isRequired,
selectedTasks: React.PropTypes.object.isRequired,
tasks: React.PropTypes.array.isRequired,
Expand Down Expand Up @@ -70,6 +71,7 @@ var TaskListComponent = React.createClass({
onToggle={props.onTaskToggle}
sortKey={sortKey}
task={task}
labels={props.labels}
taskHealthMessage={props.getTaskHealthMessage(task.id)} />
);
})
Expand Down
28 changes: 22 additions & 6 deletions src/js/components/TaskListItemComponent.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -22,13 +24,20 @@ function joinNodes(nodes, separator = ", ") {
});
}

function getPortScheme(labels, portIndex) {
const scheme = ServiceSchemeUtil
.getServiceSchemeFromLabels(labels, portIndex);
return scheme == '' ? '//' : `${scheme}://`;
}

var TaskListItemComponent = React.createClass({
displayName: "TaskListItemComponent",

propTypes: {
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,
Expand All @@ -37,28 +46,31 @@ 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 ) {
return (<span className="text-muted">{task.host}</span>);
}

if (ports != null && ports.length === 1) {
const scheme = getPortScheme(props.labels, 0);
return (
<a className="text-muted"
href={`//${task.host}:${ports[0]}`}
href={`${scheme}${task.host}:${ports[0]}`}
target="_blank">
{`${task.host}:${ports[0]}`}
</a>
);
}

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 (
<a key={`${task.host}:${port}`}
className="text-muted"
href={`//${task.host}:${port}`}
href={`${scheme}${task.host}:${port}`}
target="_blank">
{port}
</a>
Expand Down Expand Up @@ -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 (
<a key={`${ipAddress}:${port}`}
className="text-muted" href={`//${ipAddress}:${port}`}>
className="text-muted"
href={`${scheme}${ipAddress}:${port}`}>
{`${ipAddress}:${port}`}
</a>
);
}

let portNodes = serviceDiscoveryPorts.map((port) => {
let portNodes = serviceDiscoveryPorts.map((port, i) => {
const scheme = getPortScheme(props.labels, i);
return (
<a key={`${ipAddress}:${port.number}`}
className="text-muted" href={`//${ipAddress}:${port.number}`}>
className="text-muted"
href={`${scheme}${ipAddress}:${port.number}`}>
{port.number}
</a>
);
Expand Down
2 changes: 2 additions & 0 deletions src/js/components/TaskViewComponent.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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
},

Expand Down Expand Up @@ -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} />
Expand Down
36 changes: 36 additions & 0 deletions src/js/helpers/ServiceSchemeUtil.js
Original file line number Diff line number Diff line change
@@ -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<N>` is
* in the set of labels, then the function returns the value
* of this label.
*
* Given N a port index, if `MARATHON_SCHEME_PORT0<N>` 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<N>` 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;
56 changes: 56 additions & 0 deletions src/test/units/ServiceSchemeUtil.test.js
Original file line number Diff line number Diff line change
@@ -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", "");
});
73 changes: 72 additions & 1 deletion src/test/units/TaskListItemComponent.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -20,6 +20,7 @@ describe("Task List Item component", function () {
this.component = shallow(
<TaskListItemComponent appId={"/app-1"}
hasHealth={true}
labels={{}}
taskHealthMessage="Healthy"
isActive={false}
onToggle={()=>{}}
Expand All @@ -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(
<TaskListItemComponent appId={"/app-1"}
hasHealth={true}
taskHealthMessage="Healthy"
isActive={false}
labels={{
"MARATHON_SCHEME_PORT": "https"
}}
onToggle={()=>{}}
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(
<TaskListItemComponent appId={"/app-1"}
hasHealth={true}
taskHealthMessage="Healthy"
isActive={false}
labels={{
"MARATHON_SCHEME_PORT0": "https",
"MARATHON_SCHEME_PORT2": "http"
}}
onToggle={()=>{}}
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");
});
Expand Down

0 comments on commit b51c1d2

Please sign in to comment.