From 1bc24799b971e8fe3703c91cb554adad2d0d2c4c Mon Sep 17 00:00:00 2001
From: Peter Piela <ppiela@cray.com>
Date: Tue, 8 Mar 2016 13:19:07 -0500
Subject: [PATCH] Don't hyperlink images that are specified as URLs

Only provide hyperlinks to Image instance pages for images specified using
a uuid.

Closes-Bug: #1548928
Change-Id: I615c3288f2ca4aa790d55b5bf2e1f4f3fd9fab5f
---
 .../node-details/node-details.controller.js   | 69 ++++++++++++++-----
 .../node-details.controller.spec.js           | 31 +++++++--
 .../ironic/node-details/node-details.html     |  3 +-
 .../node-details/sections/configuration.html  | 60 +++++++++-------
 4 files changed, 113 insertions(+), 50 deletions(-)

diff --git a/ironic_ui/static/dashboard/admin/ironic/node-details/node-details.controller.js b/ironic_ui/static/dashboard/admin/ironic/node-details/node-details.controller.js
index fa58a12a..643f7fe8 100755
--- a/ironic_ui/static/dashboard/admin/ironic/node-details/node-details.controller.js
+++ b/ironic_ui/static/dashboard/admin/ironic/node-details/node-details.controller.js
@@ -47,10 +47,18 @@
     ];
 
     ctrl.basePath = basePath;
-    ctrl.init = init;
-
-    ///////////////
+    ctrl.re_uuid = /^[a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12}$/;
+    ctrl.isUuid = isUuid;
+    ctrl.getVifPortId = getVifPortId;
 
+    init();
+    
+    /**
+     * @name horizon.dashboard.admin.ironic.NodeDetailsController.init
+     * @description Initialize the controller instance based on the current page url.
+     *
+     * @return {void}
+     */
     function init() {
       // Fetch the Node ID from the URL.
       var pattern = /(.*\/admin\/ironic\/)(.+)\/(detail)?/;
@@ -61,27 +69,56 @@
       });
     }
 
+    /**
+     * @name horizon.dashboard.admin.ironic.NodeDetailsController.retrieveNode
+     * @description Retrieve the node instance for a specified node id,
+     * and store it in the controller instance.
+     *
+     * @param {string} uuid – Node name or UUID
+     * @return {promise} promise
+     */
     function retrieveNode(uuid) {
       return ironic.getNode(uuid).then(function (response) {
-        var node = response.data;
-        ctrl.node = node;
-        if (node['target_power_state']) {
-          actions.updateNode(node);
-        }
+        ctrl.node = response.data;
       });
     }
 
+    /**
+     * @name horizon.dashboard.admin.ironic.NodeDetailsController.retrievePorts
+     * @description Retrieve the ports associated with a specified node, and store
+     * them in the controller instance.
+     *
+     * @param {string} node_id – Node name or UUID
+     * @return {void}
+     */
     function retrievePorts(node_id) {
       ironic.getPortsWithNode(node_id).then(function (response) {
         ctrl.ports = response.data.items;
-        // Ensure that the vif_port_id property exists for all ports
-        angular.forEach(ctrl.ports,
-                        function(port, key) {
-                          if (angular.isUndefined(port.extra.vif_port_id)) {
-                            port.extra.vif_port_id = "";
-                          }
-                        });
-        });
+      });
+    }
+
+    /**
+     * @name horizon.dashboard.admin.ironic.NodeDetailsController.isUuid
+     * @description Test whether a string is an OpenStack UUID
+     *
+     * @param {string} str – string
+     * @return {boolean} True if the string is an OpenStack UUID, otherwise false
+     */
+    function isUuid(str) {
+      return str.match(ctrl.re_uuid) ? true : false;
+    }
+
+    /**
+     * @name horizon.dashboard.admin.ironic.NodeDetailsController.getVifPortId
+     * @description Get the vif_port_id property of a specified port
+     *
+     * @param {object} port – instance of port
+     * @return {string} Value of vif_port_id property or "" if the property does not exist
+     */
+    function getVifPortId(port) {
+      return (angular.isDefined(port.extra) &&
+              angular.isDefined(port.extra.vif_port_id)) ?
+        port.extra.vif_port_id : "";
     }
   }
 
diff --git a/ironic_ui/static/dashboard/admin/ironic/node-details/node-details.controller.spec.js b/ironic_ui/static/dashboard/admin/ironic/node-details/node-details.controller.spec.js
index 223253a0..5bbfda65 100755
--- a/ironic_ui/static/dashboard/admin/ironic/node-details/node-details.controller.spec.js
+++ b/ironic_ui/static/dashboard/admin/ironic/node-details/node-details.controller.spec.js
@@ -28,7 +28,11 @@
     }
 
     function createPort(nodeUuid, index, extra) {
-      return {uuid: portUuid(nodeUuid, index), extra: extra};
+      var port = {uuid: portUuid(nodeUuid, index)};
+      if (angular.isDefined(extra)) {
+        port.extra = extra;
+      }
+      return port;
     }
 
     function createNode(name, uuid) {
@@ -44,7 +48,7 @@
       getPortsWithNode: function (uuid) {
         var ports = [];
         for (var i = 0; i < numPorts; i++) {
-          ports.push(createPort(uuid, i, {}));
+          ports.push(createPort(uuid, i));
         }
         return $q.when({data: {items: ports}});
       }
@@ -68,7 +72,6 @@
         $location: $location,
         'horizon.dashboard.admin.ironic.actions': {},
         'horizon.dashboard.admin.basePath': '/static'});
-      ctrl.init();
 
       scope.$apply();
     }));
@@ -84,7 +87,7 @@
 
     it('should have a node', function () {
       expect(ctrl.node).toBeDefined();
-      expect(ctrl.node).toEqual(createNode(nodeName, nodeUuid))
+      expect(ctrl.node).toEqual(createNode(nodeName, nodeUuid));
     });
 
     it('should have ports', function () {
@@ -93,11 +96,27 @@
 
       var ports = [];
       for (var i = 0; i < ctrl.ports.length; i++) {
-        ports.push(createPort(ctrl.node.uuid, i, {vif_port_id: ''}));
+        ports.push(createPort(ctrl.node.uuid, i));
       }
       expect(ctrl.ports).toEqual(ports);
     });
 
-  });
+    it('should have a uuid regular expression pattern', function () {
+      expect(ctrl.re_uuid).toBeDefined();
+    });
 
+    it('should have an isUuid function', function () {
+      expect(ctrl.isUuid).toBeDefined();
+      expect(ctrl.isUuid(ctrl.node.uuid)).toEqual(true);
+      expect(ctrl.isUuid("not a uuid")).toEqual(false);
+    });
+
+    it('should have a getVifPortId function', function () {
+      expect(ctrl.getVifPortId).toBeDefined();
+      expect(ctrl.getVifPortId(createPort(ctrl.node.uuid, 1))).toEqual("");
+      var extra = {vif_port_id: "port_uuid"};
+      expect(ctrl.getVifPortId(createPort(ctrl.node.uuid, 1, extra))).
+        toEqual("port_uuid");
+    });
+  })
 })();
diff --git a/ironic_ui/static/dashboard/admin/ironic/node-details/node-details.html b/ironic_ui/static/dashboard/admin/ironic/node-details/node-details.html
index 044bf063..ca0d525f 100644
--- a/ironic_ui/static/dashboard/admin/ironic/node-details/node-details.html
+++ b/ironic_ui/static/dashboard/admin/ironic/node-details/node-details.html
@@ -1,6 +1,5 @@
 <div class="detail-page"
-     ng-controller="horizon.dashboard.admin.ironic.NodeDetailsController as ctrl"
-     ng-init="ctrl.init()">
+     ng-controller="horizon.dashboard.admin.ironic.NodeDetailsController as ctrl">
 
   <div class="pull-right">
     <action-list dropdown>
diff --git a/ironic_ui/static/dashboard/admin/ironic/node-details/sections/configuration.html b/ironic_ui/static/dashboard/admin/ironic/node-details/sections/configuration.html
index c39e7c21..d08bb67d 100644
--- a/ironic_ui/static/dashboard/admin/ironic/node-details/sections/configuration.html
+++ b/ironic_ui/static/dashboard/admin/ironic/node-details/sections/configuration.html
@@ -6,13 +6,13 @@
     <hr class="header_rule">
     <dl class="dl-horizontal">
       <dt translate>Node ID</dt>
-      <dd>{$ ctrl.node['uuid'] $}</dd>
+      <dd>{$ ctrl.node.uuid $}</dd>
       <dt translate>Chassis ID</dt>
-      <dd>{$ ctrl.node['chassis_uuid'] $}</dd>
+      <dd>{$ ctrl.node.chassis_uuid $}</dd>
       <dt translate>Created At</dt>
-      <dd>{$ ctrl.node['created_at'] $}</dd>
+      <dd>{$ ctrl.node.created_at $}</dd>
       <dt translate>Extra</dt>
-      <dd>{$ ctrl.node['extra'] $}</dd>
+      <dd>{$ ctrl.node.extra $}</dd>
     </dl>
   </div>
 
@@ -23,12 +23,12 @@
     <dl class="dl-horizontal">
       <dt ng-repeat-start="port in ctrl.ports">
         {$ 'MAC ' + (1 + $index) $}</dt>
-      <dd ng-if="port.extra.vif_port_id">
-        <a href="/admin/networks/ports/{$ port.extra.vif_port_id $}/detail">
+      <dd ng-if="vif_port_id = ctrl.getVifPortId(port)">
+        <a href="/admin/networks/ports/{$ vif_port_id $}/detail">
           {$ port.address $}
         </a>
       </dd>
-      <dd ng-if="!port.extra.vif_port_id">
+      <dd ng-if="!vif_port_id">
         {$ port.address $}
       </dd>
       <p ng-repeat-end></p>
@@ -43,13 +43,13 @@
     <hr class="header_rule">
     <dl class="dl-horizontal">
       <dt translate>Memory</dt>
-      <dd>{$ ctrl.node['properties']['memory_mb'] + ' MB' $}</dd>
+      <dd>{$ ctrl.node.properties.memory_mb + ' MB' $}</dd>
       <dt translate>CPU Arch</dt>
-      <dd>{$ ctrl.node['properties']['cpu_arch'] $}</dd>
+      <dd>{$ ctrl.node.properties.cpu_arch $}</dd>
       <dt translate>Local GB</dt>
-      <dd>{$ ctrl.node['properties']['local_gb'] $}</dd>
+      <dd>{$ ctrl.node.properties.local_gb $}</dd>
       <dt translate>CPUs</dt>
-      <dd>{$ ctrl.node['properties']['cpus'] $}</dd>
+      <dd>{$ ctrl.node.properties.cpus $}</dd>
     </dl>
   </div>
 
@@ -57,29 +57,37 @@
   <div class="col-md-6 status detail">
     <h4 translate>Driver Info</h4>
     <hr class="header_rule">
-    <div ng-switch="ctrl.node['driver']">
+    <div ng-switch="ctrl.node.driver">
       <dl ng-switch-when="pxe_ssh" class="dl-horizontal">
         <dt translate>Driver</dt>
-        <dd>{$ ctrl.node['driver'] $}</dd>
+        <dd>{$ ctrl.node.driver $}</dd>
         <dt translate>SSH Port</dt>
-        <dd>{$ ctrl.node['driver_info']['ssh_port'] $}</dd>
+        <dd>{$ ctrl.node.driver_info.ssh_port $}</dd>
         <dt translate>SSH Username</dt>
-        <dd>{$ ctrl.node['driver_info']['ssh_username'] $}</dd>
+        <dd>{$ ctrl.node.driver_info.ssh_username $}</dd>
         <dt translate>Deploy Kernel</dt>
         <dd>
-          <a href="/admin/images/{$ ctrl.node['driver_info']['deploy_kernel'] $}/detail">
-            {$ ctrl.node['driver_info']['deploy_kernel'] $}
+          <a ng-if="deploy_kernel_is_uuid = ctrl.isUuid(ctrl.node.driver_info.deploy_kernel)"
+             href="/admin/images/{$ ctrl.node.driver_info.deploy_kernel $}/detail">
+            {$ ctrl.node.driver_info.deploy_kernel $}
           </a>
+          <span ng-if="!deploy_kernel_is_uuid">
+            {$ ctrl.node.driver_info.deploy_kernel $}
+          </span>
         </dd>
         <dt translate>Deploy Ramdisk</dt>
         <dd>
-          <a href="/admin/images/{$ ctrl.node['driver_info']['deploy_ramdisk'] $}/detail">
-            {$ ctrl.node['driver_info']['deploy_ramdisk'] $}
+          <a ng-if="deploy_ramdisk_is_uuid = ctrl.isUuid(ctrl.node.driver_info.deploy_ramdisk)"
+             href="/admin/images/{$ ctrl.node.driver_info.deploy_ramdisk $}/detail">
+            {$ ctrl.node.driver_info.deploy_ramdisk $}
           </a>
+          <span ng-if="!deploy_ramdisk_is_uuid">
+            {$ ctrl.node.driver_info.deploy_ramdisk $}
+          </span>
         </dd>
       </dl>
       <dl ng-switch-default class="dl-horizontal">
-        <dt ng-repeat-start="(id, value) in ctrl.node['driver_info']">{$ id $}</dt>
+        <dt ng-repeat-start="(id, value) in ctrl.node.driver_info">{$ id $}</dt>
         <dd ng-repeat-end>{$ value $}</dd>
       </dl>
     </div>
@@ -92,7 +100,7 @@
     <h4 translate>Capabilities</h4>
     <hr class="header_rule">
     <dl class="dl-horizontal">
-      <dd>{$ ctrl.node['capabilities'] $}</dd>
+      <dd>{$ ctrl.node.capabilities $}</dd>
     </dl>
   </div>
 
@@ -102,17 +110,17 @@
     <hr class="header_rule">
     <dl class="dl-horizontal">
       <dt translate>Instance Name</dt>
-      <dd>{$ ctrl.node['instance_info']['display_name'] $}</dd>
+      <dd>{$ ctrl.node.instance_info.display_name $}</dd>
       <dt translate>Ramdisk</dt>
       <dd>
-        <a href="/admin/images/{$ ctrl.node['instance_info']['ramdisk'] $}/detail">
-          {$ ctrl.node['instance_info']['ramdisk'] $}
+        <a href="/admin/images/{$ ctrl.node.instance_info.ramdisk $}/detail">
+          {$ ctrl.node.instance_info.ramdisk $}
         </a>
       </dd>
       <dt translate>Kernel</dt>
       <dd>
-        <a href="/admin/images/{$ ctrl.node['instance_info']['kernel'] $}/detail">
-          {$ ctrl.node['instance_info']['kernel'] $}
+        <a href="/admin/images/{$ ctrl.node.instance_info.kernel $}/detail">
+          {$ ctrl.node.instance_info.kernel $}
         </a>
       </dd>
     </dl>