Use MARATHON_SCHEME_PORT label to define scheme of a given port

clems4ever · clems4ever · commit 800a10699f19 · 2018-02-08T13:53:57.000+01:00
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&lt;N&gt;
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.
diff --git 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} />
         </TabPaneComponent>
         <TabPaneComponent
diff --git a/src/js/components/TaskListComponent.jsx b/src/js/components/TaskListComponent.jsx
@@ -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,
@@ -70,6 +71,7 @@ var TaskListComponent = React.createClass({
             onToggle={props.onTaskToggle}
             sortKey={sortKey}
             task={task}
+            labels={props.labels}
             taskHealthMessage={props.getTaskHealthMessage(task.id)} />
         );
       })
diff --git a/src/js/components/TaskListItemComponent.jsx b/src/js/components/TaskListItemComponent.jsx
@@ -7,6 +7,7 @@ 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";
 
 function joinNodes(nodes, separator = ", ") {
   var lastIndex = nodes.length - 1;
@@ -22,13 +23,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,
@@ -37,28 +45,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>
@@ -98,18 +109,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>
           );
diff --git 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
@@ -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;
diff --git a/src/test/units/ServiceSchemeUtil.test.js 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
@@ -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(
       <TaskListItemComponent appId={"/app-1"}
                              hasHealth={true}
+                             labels={{}}
                              taskHealthMessage="Healthy"
                              isActive={false}
                              onToggle={()=>{}}
@@ -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");
   });
