From 07cfd44193f6ebd552d13325858e8b5b5c131bfb Mon Sep 17 00:00:00 2001
From: Aashish Sharma <aasharma@redhat.com>
Date: Mon, 25 Jul 2022 13:48:39 +0530
Subject: [PATCH] mgr/dashboard: Show error on creating service with duplicate
 service id

Fixes: https://tracker.ceph.com/issues/56689
Signed-off-by: Aashish Sharma <aasharma@redhat.com>
---
 .../mgr/dashboard/controllers/service.py      | 27 ++++++++----
 .../service-form/service-form.component.html  |  6 ++-
 .../service-form/service-form.component.ts    | 30 ++++++++++---
 .../app/shared/api/ceph-service.service.ts    | 14 ++++++
 src/pybind/mgr/dashboard/openapi.yaml         | 44 +++++++++++++++++++
 .../mgr/dashboard/services/exception.py       |  8 ++++
 .../mgr/dashboard/services/orchestrator.py    |  6 ++-
 7 files changed, 117 insertions(+), 18 deletions(-)

diff --git a/src/pybind/mgr/dashboard/controllers/service.py b/src/pybind/mgr/dashboard/controllers/service.py
index d3ba882a1d926..afe684302b160 100644
--- a/src/pybind/mgr/dashboard/controllers/service.py
+++ b/src/pybind/mgr/dashboard/controllers/service.py
@@ -3,12 +3,11 @@ from typing import Dict, List, Optional
 import cherrypy
 from ceph.deployment.service_spec import ServiceSpec
 
-from ..exceptions import DashboardException
 from ..security import Scope
-from ..services.exception import handle_orchestrator_error
+from ..services.exception import handle_custom_error, handle_orchestrator_error
 from ..services.orchestrator import OrchClient, OrchFeature
 from . import APIDoc, APIRouter, CreatePermission, DeletePermission, Endpoint, \
-    ReadPermission, RESTController, Task
+    ReadPermission, RESTController, Task, UpdatePermission
 from .orchestrator import raise_if_no_orchestrator
 
 
@@ -50,6 +49,7 @@ class Service(RESTController):
         return [d.to_dict() for d in daemons]
 
     @CreatePermission
+    @handle_custom_error('service', exceptions=(ValueError, TypeError))
     @raise_if_no_orchestrator([OrchFeature.SERVICE_CREATE])
     @handle_orchestrator_error('service')
     @service_task('create', {'service_name': '{service_name}'})
@@ -59,11 +59,22 @@ class Service(RESTController):
         :param service_name: The service name, e.g. 'alertmanager'.
         :return: None
         """
-        try:
-            orch = OrchClient.instance()
-            orch.services.apply(service_spec)
-        except (ValueError, TypeError) as e:
-            raise DashboardException(e, component='service')
+
+        OrchClient.instance().services.apply(service_spec, no_overwrite=True)
+
+    @UpdatePermission
+    @handle_custom_error('service', exceptions=(ValueError, TypeError))
+    @raise_if_no_orchestrator([OrchFeature.SERVICE_CREATE])
+    @handle_orchestrator_error('service')
+    @service_task('edit', {'service_name': '{service_name}'})
+    def set(self, service_spec: Dict, service_name: str):  # pylint: disable=W0613
+        """
+        :param service_spec: The service specification as JSON.
+        :param service_name: The service name, e.g. 'alertmanager'.
+        :return: None
+        """
+
+        OrchClient.instance().services.apply(service_spec, no_overwrite=False)
 
     @DeletePermission
     @raise_if_no_orchestrator([OrchFeature.SERVICE_DELETE])
diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/services/service-form/service-form.component.html b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/services/service-form/service-form.component.html
index 35f61965710e4..c9cdc70b0ccec 100644
--- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/services/service-form/service-form.component.html
+++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/services/service-form/service-form.component.html
@@ -17,7 +17,8 @@
             <select id="service_type"
                     name="service_type"
                     class="form-control"
-                    formControlName="service_type">
+                    formControlName="service_type"
+                    (change)="getServiceIds($event.target.value)">
               <option i18n
                       [ngValue]="null">-- Select a service type --</option>
               <option *ngFor="let serviceType of serviceTypes"
@@ -77,6 +78,9 @@
             <span class="invalid-feedback"
                   *ngIf="serviceForm.showError('service_id', frm, 'required')"
                   i18n>This field is required.</span>
+            <span class="invalid-feedback"
+                  *ngIf="serviceForm.showError('service_id', frm, 'uniqueName')"
+                  i18n>This service id is already in use.</span>
             <span class="invalid-feedback"
                   *ngIf="serviceForm.showError('service_id', frm, 'rgwPattern')"
                   i18n>The value does not match the pattern <strong>&lt;service_id&gt;[.&lt;realm_name&gt;.&lt;zone_name&gt;]</strong>.</span>
diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/services/service-form/service-form.component.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/services/service-form/service-form.component.ts
index 994b5ff8b3357..cfe2bb5b28dcb 100644
--- a/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/services/service-form/service-form.component.ts
+++ b/src/pybind/mgr/dashboard/frontend/src/app/ceph/cluster/services/service-form/service-form.component.ts
@@ -46,6 +46,7 @@ export class ServiceFormComponent extends CdForm implements OnInit {
   action: string;
   resource: string;
   serviceTypes: string[] = [];
+  serviceIds: string[] = [];
   hosts: any;
   labels: string[];
   labelClick = new Subject<string>();
@@ -53,6 +54,7 @@ export class ServiceFormComponent extends CdForm implements OnInit {
   pools: Array<object>;
   services: Array<CephServiceSpec> = [];
   pageURL: string;
+  serviceList: CephServiceSpec[];
 
   constructor(
     public actionLabels: ActionLabelsI18n,
@@ -109,7 +111,10 @@ export class ServiceFormComponent extends CdForm implements OnInit {
                 return !this.RGW_SVC_ID_PATTERN.test(value);
               })
             ]
-          )
+          ),
+          CdValidators.custom('uniqueName', (service_id: string) => {
+            return this.serviceIds && this.serviceIds.includes(service_id);
+          })
         ]
       ],
       placement: ['hosts'],
@@ -337,6 +342,14 @@ export class ServiceFormComponent extends CdForm implements OnInit {
         this.serviceType = params.type;
       });
     }
+
+    this.cephServiceService.list().subscribe((services: CephServiceSpec[]) => {
+      this.serviceList = services;
+      this.services = services.filter((service: any) =>
+        this.INGRESS_SUPPORTED_SERVICE_TYPES.includes(service.service_type)
+      );
+    });
+
     this.cephServiceService.getKnownTypes().subscribe((resp: Array<string>) => {
       // Remove service types:
       // osd       - This is deployed a different way.
@@ -361,11 +374,6 @@ export class ServiceFormComponent extends CdForm implements OnInit {
     this.poolService.getList().subscribe((resp: Array<object>) => {
       this.pools = resp;
     });
-    this.cephServiceService.list().subscribe((services: CephServiceSpec[]) => {
-      this.services = services.filter((service: any) =>
-        this.INGRESS_SUPPORTED_SERVICE_TYPES.includes(service.service_type)
-      );
-    });
 
     if (this.editing) {
       this.action = this.actionLabels.EDIT;
@@ -463,6 +471,12 @@ export class ServiceFormComponent extends CdForm implements OnInit {
     }
   }
 
+  getServiceIds(selectedServiceType: string) {
+    this.serviceIds = this.serviceList
+      .filter((service) => service['service_type'] === selectedServiceType)
+      .map((service) => service['service_id']);
+  }
+
   disableForEditing(serviceType: string) {
     const disableForEditKeys = ['service_type', 'service_id'];
     disableForEditKeys.forEach((key) => {
@@ -622,7 +636,9 @@ export class ServiceFormComponent extends CdForm implements OnInit {
         task: new FinishedTask(taskUrl, {
           service_name: serviceName
         }),
-        call: this.cephServiceService.create(serviceSpec)
+        call: this.editing
+          ? this.cephServiceService.update(serviceSpec)
+          : this.cephServiceService.create(serviceSpec)
       })
       .subscribe({
         error() {
diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/ceph-service.service.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/ceph-service.service.ts
index 07bd4deb9cb1f..c62dfea7c0155 100644
--- a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/ceph-service.service.ts
+++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/ceph-service.service.ts
@@ -39,6 +39,20 @@ export class CephServiceService {
     );
   }
 
+  update(serviceSpec: { [key: string]: any }) {
+    const serviceName = serviceSpec['service_id']
+      ? `${serviceSpec['service_type']}.${serviceSpec['service_id']}`
+      : serviceSpec['service_type'];
+    return this.http.put(
+      `${this.url}/${serviceName}`,
+      {
+        service_name: serviceName,
+        service_spec: serviceSpec
+      },
+      { observe: 'response' }
+    );
+  }
+
   delete(serviceName: string) {
     return this.http.delete(`${this.url}/${serviceName}`, { observe: 'response' });
   }
diff --git a/src/pybind/mgr/dashboard/openapi.yaml b/src/pybind/mgr/dashboard/openapi.yaml
index 83bd98c2ed162..8655a57364612 100644
--- a/src/pybind/mgr/dashboard/openapi.yaml
+++ b/src/pybind/mgr/dashboard/openapi.yaml
@@ -8898,6 +8898,50 @@ paths:
       - jwt: []
       tags:
       - Service
+    put:
+      description: "\n        :param service_spec: The service specification as JSON.\n\
+        \        :param service_name: The service name, e.g. 'alertmanager'.\n   \
+        \     :return: None\n        "
+      parameters:
+      - in: path
+        name: service_name
+        required: true
+        schema:
+          type: string
+      requestBody:
+        content:
+          application/json:
+            schema:
+              properties:
+                service_spec:
+                  type: string
+              required:
+              - service_spec
+              type: object
+      responses:
+        '200':
+          content:
+            application/vnd.ceph.api.v1.0+json:
+              type: object
+          description: Resource updated.
+        '202':
+          content:
+            application/vnd.ceph.api.v1.0+json:
+              type: object
+          description: Operation is still executing. Please check the task queue.
+        '400':
+          description: Operation exception. Please check the response body for details.
+        '401':
+          description: Unauthenticated access. Please login first.
+        '403':
+          description: Unauthorized access. Please check your permissions.
+        '500':
+          description: Unexpected error. Please check the response body for the stack
+            trace.
+      security:
+      - jwt: []
+      tags:
+      - Service
   /api/service/{service_name}/daemons:
     get:
       parameters:
diff --git a/src/pybind/mgr/dashboard/services/exception.py b/src/pybind/mgr/dashboard/services/exception.py
index 09851aa467e0f..ed4fbe8003d9d 100644
--- a/src/pybind/mgr/dashboard/services/exception.py
+++ b/src/pybind/mgr/dashboard/services/exception.py
@@ -122,3 +122,11 @@ def handle_error(component, http_status_code=None):
         yield
     except Exception as e:  # pylint: disable=broad-except
         raise DashboardException(e, component=component, http_status_code=http_status_code)
+
+
+@contextmanager
+def handle_custom_error(component, http_status_code=None, exceptions=()):
+    try:
+        yield
+    except exceptions as e:
+        raise DashboardException(e, component=component, http_status_code=http_status_code)
diff --git a/src/pybind/mgr/dashboard/services/orchestrator.py b/src/pybind/mgr/dashboard/services/orchestrator.py
index 4cc0a3998563a..6829108e098d5 100644
--- a/src/pybind/mgr/dashboard/services/orchestrator.py
+++ b/src/pybind/mgr/dashboard/services/orchestrator.py
@@ -128,9 +128,11 @@ class ServiceManager(ResourceManager):
             raise_if_exception(c)
 
     @wait_api_result
-    def apply(self, service_spec: Dict) -> OrchResult[List[str]]:
+    def apply(self,
+              service_spec: Dict,
+              no_overwrite: Optional[bool] = False) -> OrchResult[List[str]]:
         spec = ServiceSpec.from_json(service_spec)
-        return self.api.apply([spec])
+        return self.api.apply([spec], no_overwrite)
 
     @wait_api_result
     def remove(self, service_name: str) -> List[str]:
-- 
2.39.5