[dashboards as code] only validate id on PUT route when creating new dashboard

src/platform/plugins/shared/dashboard/server/api/update/register_update_route.ts

@@ -12,7 +12,6 @@ import type { RequestHandlerContext } from '@kbn/core/server';
 import type { UsageCounter } from '@kbn/usage-collection-plugin/server';
 import { schema } from '@kbn/config-schema';
 import { once } from 'lodash';
-import { asCodeIdSchema } from '@kbn/as-code-shared-schemas';
 import { getRouteConfig } from '../get_route_config';
 import { getUpdateRequestBodySchema, getUpdateResponseBodySchema } from './schemas';
 import { update } from './update';
@@ -45,7 +44,9 @@ export function registerUpdateRoute(
       validate: () => ({
         request: {
           params: schema.object({
-            id: asCodeIdSchema,
+            // Can not validate id at route level
+            // existing dashboards may have invalid "as code" ids
+            id: schema.string(),
           }),
           body: getUpdateRequestBodySchema(isDashboardAppRequest),
         },

src/platform/plugins/shared/dashboard/server/api/update/update.ts

@@ -8,6 +8,7 @@
  */
 
 import type { RequestHandlerContext } from '@kbn/core/server';
+import { asCodeIdSchema } from '@kbn/as-code-shared-schemas';
 import type { DashboardSavedObjectAttributes } from '../../dashboard_saved_object';
 import { DASHBOARD_SAVED_OBJECT_TYPE } from '../../../common/constants';
 import type { DashboardUpdateRequestBody, DashboardUpdateResponseBody } from './types';
@@ -29,6 +30,25 @@ export async function update(
     isDashboardAppRequest
   );
 
+  let isCreateRequest = false;
+  try {
+    await core.savedObjects.client.resolve<DashboardSavedObjectAttributes>(
+      DASHBOARD_SAVED_OBJECT_TYPE,
+      id
+    );
+  } catch (resolveError) {
+    if (resolveError.isBoom && resolveError.output.statusCode === 404) {
+      isCreateRequest = true;
+    } else {
+      throw resolveError;
+    }
+  }
+
+  // Validate id at handler level for create requests
+  if (isCreateRequest) {
+    asCodeIdSchema.validate(id);
+  }
+
   const savedObject = await core.savedObjects.client.update<DashboardSavedObjectAttributes>(
     DASHBOARD_SAVED_OBJECT_TYPE,
     id,

src/platform/plugins/shared/dashboard/test/scout/api/tests/update_dashboard.spec.ts → src/platform/plugins/shared/dashboard/test/scout/api/tests/upsert_dashboard.spec.ts

@@ -27,6 +27,9 @@ apiTest.describe('dashboards - upsert', { tag: tags.deploymentAgnostic }, () =>
     editorCredentials = await requestAuth.getApiKeyForPrivilegedUser();
     viewerCredentials = await requestAuth.getApiKeyForViewer();
     await kbnClient.importExport.load(KBN_ARCHIVES.BASIC);
+    await kbnClient.importExport.load(
+      'src/platform/test/api_integration/fixtures/kbn_archiver/saved_objects/dashboards_api.json'
+    );
     await kbnClient.importExport.load(KBN_ARCHIVES.TAGS);
   });
 
@@ -51,41 +54,84 @@ apiTest.describe('dashboards - upsert', { tag: tags.deploymentAgnostic }, () =>
     expect(response.body.data.title).toBe('Refresh Requests (Updated)');
   });
 
-  apiTest('should create new dashboard', async ({ apiClient }) => {
-    const id = 'new-dashboard-id';
-    const title = `I'm a new dashboard`;
+  apiTest('should update existing dashboard with invalid "as code" id', async ({ apiClient }) => {
+    const id = '(my)dashboard';
     const response = await apiClient.put(`${DASHBOARD_API_PATH}/${id}`, {
       headers: {
         ...COMMON_HEADERS,
         ...editorCredentials.apiKeyHeader,
       },
       body: {
-        title: `I'm a new dashboard`,
+        title: 'Updated title',
       },
       responseType: 'json',
     });
 
-    expect(response).toHaveStatusCode(201);
+    expect(response).toHaveStatusCode(200);
     expect(response.body.id).toBe(id);
-    expect(response.body.data.title).toBe(title);
+    expect(response.body.data.title).toBe('Updated title');
   });
 
-  apiTest('validation - returns 400 when body is not empty', async ({ apiClient }) => {
-    const response = await apiClient.put(`${DASHBOARD_API_PATH}/${TEST_DASHBOARD_ID}`, {
+  apiTest('should create new dashboard', async ({ apiClient }) => {
+    const id = 'new-dashboard-id';
+    const title = `I'm a new dashboard`;
+    const response = await apiClient.put(`${DASHBOARD_API_PATH}/${id}`, {
       headers: {
         ...COMMON_HEADERS,
         ...editorCredentials.apiKeyHeader,
       },
-      body: {},
+      body: {
+        title,
+      },
       responseType: 'json',
     });
 
-    expect(response).toHaveStatusCode(400);
-    expect(response.body.message).toBe(
-      '[request body.title]: expected value of type [string] but got [undefined]'
-    );
+    expect(response).toHaveStatusCode(201);
+    expect(response.body.id).toBe(id);
+    expect(response.body.data.title).toBe(title);
   });
 
+  apiTest(
+    'validation - returns 400 when creating a new dashboard with an invalid id',
+    async ({ apiClient }) => {
+      const id = '(new)dashboard-id';
+      const response = await apiClient.put(`${DASHBOARD_API_PATH}/${id}`, {
+        headers: {
+          ...COMMON_HEADERS,
+          ...editorCredentials.apiKeyHeader,
+        },
+        body: {
+          title: `I'm a new dashboard`,
+        },
+        responseType: 'json',
+      });
+
+      expect(response).toHaveStatusCode(400);
+      expect(response.body.message).toBe(
+        'ID must contain only lowercase letters, numbers, hyphens, and underscores.'
+      );
+    }
+  );
+
+  apiTest(
+    'validation - returns 400 when body is not valid dashboard shape',
+    async ({ apiClient }) => {
+      const response = await apiClient.put(`${DASHBOARD_API_PATH}/${TEST_DASHBOARD_ID}`, {
+        headers: {
+          ...COMMON_HEADERS,
+          ...editorCredentials.apiKeyHeader,
+        },
+        body: {},
+        responseType: 'json',
+      });
+
+      expect(response).toHaveStatusCode(400);
+      expect(response.body.message).toBe(
+        '[request body.title]: expected value of type [string] but got [undefined]'
+      );
+    }
+  );
+
   apiTest('validation - returns 400 when access_control is provided', async ({ apiClient }) => {
     const response = await apiClient.put(`${DASHBOARD_API_PATH}/${TEST_DASHBOARD_ID}`, {
       headers: {
@@ -104,22 +150,6 @@ apiTest.describe('dashboards - upsert', { tag: tags.deploymentAgnostic }, () =>
     expect(response).toHaveStatusCode(400);
   });
 
-  apiTest('validation - returns 400 if panels is not an array', async ({ apiClient }) => {
-    const response = await apiClient.put(`${DASHBOARD_API_PATH}/${TEST_DASHBOARD_ID}`, {
-      headers: {
-        ...COMMON_HEADERS,
-        ...editorCredentials.apiKeyHeader,
-      },
-      body: {
-        title: 'foo',
-        panels: {},
-      },
-      responseType: 'json',
-    });
-
-    expect(response).toHaveStatusCode(400);
-  });
-
   apiTest(
     'validation - returns 403 if user does not have permission to update a dashboard',
     async ({ apiClient }) => {

src/platform/test/api_integration/fixtures/kbn_archiver/saved_objects/dashboards_api.json

@@ -0,0 +1,36 @@
+{
+  "attributes": {
+    "description": "",
+    "hits": 0,
+    "kibanaSavedObjectMeta": {
+      "searchSourceJSON": "{\"query\":{\"query\":\"\",\"language\":\"lucene\"},\"filter\":[],\"highlightAll\":true,\"version\":true}"
+    },
+    "optionsJSON": "{\"darkTheme\":false}",
+    "panelsJSON": "{}",
+    "refreshInterval": {
+      "display": "Off",
+      "pause": false,
+      "value": 0
+    },
+    "timeFrom": "Wed Sep 16 2015 22:52:17 GMT-0700",
+    "timeRestore": true,
+    "timeTo": "Fri Sep 18 2015 12:24:38 GMT-0700",
+    "title": "Dashboard with invalid as code id",
+    "version": 1
+  },
+  "coreMigrationVersion": "7.14.0",
+  "id": "(my)dashboard",
+  "migrationVersion": {
+    "dashboard": "7.11.0"
+  },
+  "references": [
+    {
+      "id": "dd7caf20-9efd-11e7-acb3-3dab96693fab",
+      "name": "1:panel_1",
+      "type": "visualization"
+    }
+  ],
+  "type": "dashboard",
+  "updated_at": "2017-09-21T18:57:40.826Z",
+  "version": "WzExLDJd"
+}