From 067257df2bf5cd554810303136f54a188144369f Mon Sep 17 00:00:00 2001
From: Chaim Lev-Ari <chiptus@users.noreply.github.com>
Date: Mon, 14 Dec 2020 05:27:05 +0200
Subject: [PATCH] fix(services): prevent adding volume without source and
 target (#4538)

* feat(services): check that target mounts are non empty

* feat(services): prevent creating service when no source

* refactor(services): remove ng-form

* fix(services): check that every volume is valid
---
 app/assets/css/app.css                        |  4 ++
 .../create/createServiceController.js         |  6 +++
 .../views/services/create/createservice.html  | 47 ++++++++++++-------
 .../views/services/edit/includes/mounts.html  |  7 ++-
 .../views/services/edit/serviceController.js  |  6 +++
 5 files changed, 51 insertions(+), 19 deletions(-)

diff --git a/app/assets/css/app.css b/app/assets/css/app.css
index d42b10ee0..4f9ca5881 100644
--- a/app/assets/css/app.css
+++ b/app/assets/css/app.css
@@ -1042,3 +1042,7 @@ json-tree .branch-preview {
   background-color: #337ab7;
 }
 /* !spinkit override */
+
+.w-full {
+  width: 100%;
+}
diff --git a/app/docker/views/services/create/createServiceController.js b/app/docker/views/services/create/createServiceController.js
index 2edddb658..a59d74207 100644
--- a/app/docker/views/services/create/createServiceController.js
+++ b/app/docker/views/services/create/createServiceController.js
@@ -520,6 +520,12 @@ angular.module('portainer.docker').controller('CreateServiceController', [
       return true;
     }
 
+    $scope.volumesAreValid = volumesAreValid;
+    function volumesAreValid() {
+      const volumes = $scope.formValues.Volumes;
+      return volumes.every((volume) => volume.Target && volume.Source);
+    }
+
     $scope.create = function createService() {
       var accessControlData = $scope.formValues.AccessControlData;
 
diff --git a/app/docker/views/services/create/createservice.html b/app/docker/views/services/create/createservice.html
index cc304b3e1..2dcdea2de 100644
--- a/app/docker/views/services/create/createservice.html
+++ b/app/docker/views/services/create/createservice.html
@@ -123,7 +123,7 @@
               <button
                 type="button"
                 class="btn btn-primary btn-sm"
-                ng-disabled="state.actionInProgress || !formValues.RegistryModel.Image"
+                ng-disabled="state.actionInProgress || !formValues.RegistryModel.Image || !volumesAreValid()"
                 ng-click="create()"
                 button-spinner="state.actionInProgress"
               >
@@ -298,13 +298,16 @@
                       <!-- volume-line1 -->
                       <div class="col-sm-12 form-inline">
                         <!-- container-path -->
-                        <div class="input-group input-group-sm col-sm-6">
-                          <span class="input-group-addon">container</span>
-                          <input type="text" class="form-control" ng-model="volume.Target" placeholder="e.g. /path/in/container" />
+                        <div class="input-group col-sm-6">
+                          <div class="input-group input-group-sm w-full">
+                            <span class="input-group-addon">container</span>
+                            <input type="text" class="form-control" ng-model="volume.Target" placeholder="e.g. /path/in/container" />
+                          </div>
+                          <div class="small text-warning" ng-show="!volume.Target"> <i class="fa fa-exclamation-triangle" aria-hidden="true"></i> Target is required. </div>
                         </div>
                         <!-- !container-path -->
                         <!-- volume-type -->
-                        <div class="input-group col-sm-5" style="margin-left: 5px;">
+                        <div class="input-group col-sm-5" style="margin-left: 5px; vertical-align: top;">
                           <div class="btn-group btn-group-sm" ng-if="allowBindMounts">
                             <label class="btn btn-primary" ng-model="volume.Type" uib-btn-radio="'volume'" ng-click="volume.name = ''">Volume</label>
                             <label class="btn btn-primary" ng-model="volume.Type" uib-btn-radio="'bind'" ng-click="volume.Id = ''">Bind</label>
@@ -318,27 +321,35 @@
                       <!-- !volume-line1 -->
                       <!-- volume-line2 -->
                       <div class="col-sm-12 form-inline" style="margin-top: 5px;">
-                        <i class="fa fa-long-arrow-alt-right" aria-hidden="true"></i>
+                        <div style="height: 30px; display: inline-block; vertical-align: top; display: inline-flex; align-items: center;">
+                          <i class="fa fa-long-arrow-alt-right" aria-hidden="true"></i>
+                        </div>
                         <!-- volume -->
-                        <div class="input-group input-group-sm col-sm-6" ng-if="volume.Type === 'volume'">
-                          <span class="input-group-addon">volume</span>
-                          <select
-                            class="form-control"
-                            ng-model="volume.Source"
-                            ng-options="vol as ((vol.Id|truncate:30) + ' - ' + (vol.Driver|truncate:30)) for vol in availableVolumes"
-                          >
-                            <option selected disabled hidden value="">Select a volume</option>
-                          </select>
+                        <div class="col-sm-6 input-group" ng-if="volume.Type === 'volume'" style="float: none; padding: 0;">
+                          <div class="input-group input-group-sm w-full">
+                            <span class="input-group-addon">volume</span>
+                            <select
+                              class="form-control"
+                              ng-model="volume.Source"
+                              ng-options="vol as ((vol.Id|truncate:30) + ' - ' + (vol.Driver|truncate:30)) for vol in availableVolumes"
+                            >
+                              <option selected disabled hidden value="">Select a volume</option>
+                            </select>
+                          </div>
+                          <div class="small text-warning" ng-show="!volume.Source"> <i class="fa fa-exclamation-triangle" aria-hidden="true"></i> Source is required. </div>
                         </div>
                         <!-- !volume -->
                         <!-- bind -->
                         <div class="input-group input-group-sm col-sm-6" ng-if="volume.Type === 'bind'">
-                          <span class="input-group-addon">host</span>
-                          <input type="text" class="form-control" ng-model="volume.Source" placeholder="e.g. /path/on/host" />
+                          <div class="input-group input-group-sm w-full">
+                            <span class="input-group-addon">host</span>
+                            <input type="text" class="form-control" ng-model="volume.Source" placeholder="e.g. /path/on/host" />
+                          </div>
+                          <div class="small text-warning" ng-show="!volume.Source"> <i class="fa fa-exclamation-triangle" aria-hidden="true"></i> Source is required. </div>
                         </div>
                         <!-- !bind -->
                         <!-- read-only -->
-                        <div class="input-group input-group-sm col-sm-5" style="margin-left: 5px;">
+                        <div class="input-group input-group-sm col-sm-5" style="margin-left: 5px; vertical-align: top;">
                           <div class="btn-group btn-group-sm">
                             <label class="btn btn-primary" ng-model="volume.ReadOnly" uib-btn-radio="false">Writable</label>
                             <label class="btn btn-primary" ng-model="volume.ReadOnly" uib-btn-radio="true">Read-only</label>
diff --git a/app/docker/views/services/edit/includes/mounts.html b/app/docker/views/services/edit/includes/mounts.html
index c23de2644..0dea0a45a 100644
--- a/app/docker/views/services/edit/includes/mounts.html
+++ b/app/docker/views/services/edit/includes/mounts.html
@@ -42,12 +42,14 @@
               <input
                 type="text"
                 class="form-control"
+                name=""
                 ng-model="mount.Source"
                 placeholder="e.g. /tmp/portainer/data"
                 ng-change="updateMount(service, mount)"
                 ng-disabled="isUpdating || (!isAdmin && !allowBindMounts && mount.Type === 'bind')"
                 ng-if="mount.Type === 'bind'"
               />
+              <div class="col-sm-12 small text-warning" ng-show="!mount.Source"> <i class="fa fa-exclamation-triangle" aria-hidden="true"></i> Source is required. </div>
             </td>
             <td>
               <input
@@ -59,6 +61,7 @@
                 ng-disabled="isUpdating"
                 disable-authorization="DockerServiceUpdate"
               />
+              <div class="col-sm-12 small text-warning" ng-show="!mount.Target"> <i class="fa fa-exclamation-triangle" aria-hidden="true"></i> Target is required. </div>
             </td>
             <td authorization="DockerServiceUpdate">
               <input type="checkbox" class="form-control" ng-model="mount.ReadOnly" ng-change="updateMount(service, mount)" ng-disabled="isUpdating" />
@@ -77,7 +80,9 @@
     <rd-widget-footer authorization="DockerServiceUpdate">
       <div class="btn-toolbar" role="toolbar">
         <div class="btn-group" role="group">
-          <button type="button" class="btn btn-primary btn-sm" ng-disabled="!hasChanges(service, ['ServiceMounts'])" ng-click="updateService(service)">Apply changes</button>
+          <button type="button" class="btn btn-primary btn-sm" ng-disabled="!mountsAreValid() || !hasChanges(service, ['ServiceMounts'])" ng-click="updateService(service)">
+            Apply changes
+          </button>
           <button type="button" class="btn btn-default btn-sm dropdown-toggle" data-toggle="dropdown" aria-haspopup="true" aria-expanded="false">
             <span class="caret"></span>
           </button>
diff --git a/app/docker/views/services/edit/serviceController.js b/app/docker/views/services/edit/serviceController.js
index f3cd5ae87..b73938d74 100644
--- a/app/docker/views/services/edit/serviceController.js
+++ b/app/docker/views/services/edit/serviceController.js
@@ -378,6 +378,12 @@ angular.module('portainer.docker').controller('ServiceController', [
       return hasChanges;
     };
 
+    $scope.mountsAreValid = mountsAreValid;
+    function mountsAreValid() {
+      const mounts = $scope.service.ServiceMounts;
+      return mounts.every((mount) => mount.Source && mount.Target);
+    }
+
     function buildChanges(service) {
       var config = ServiceHelper.serviceToConfig(service.Model);
       config.Name = service.Name;