[Reporting] Separate internal and public API endpoints in Reporting (#…

…162288) ## Summary Closes #160827 Closes #134517 Closes #149942 In this PR, the following API endpoints are moved into an internal namespace: | New endpoint path | Previous | |---|---| | `/internal/reporting/diagnose/browser` | `/api/reporting/diagnose/browser` | | `/internal/reporting/diagnose/screenshot` | `/api/reporting/diagnose/screenshot` | | `/internal/reporting/generate/immediate/csv_searchsource` | `/api/reporting/v1/generate/immediate/csv_searchsource` | | `/internal/reporting/generate/{exportTypeId}` | `/api/reporting/generate/{exportTypeId}` | | `/internal/reporting/jobs/count` | `/api/reporting/jobs/count` | | `/internal/reporting/jobs/delete/{jobId}` | `/api/reporting/jobs/delete/{jobId}` | | `/internal/reporting/jobs/info/{jobId}` | `/api/reporting/jobs/info/{jobId}` | | `/internal/reporting/jobs/list` | `/api/reporting/jobs/list` | Support for the public APIs continues: | Public endpoint path | |---| | `/api/reporting/generate/{exportTypeId}` | | `/api/reporting/jobs/delete/{jobId}` | | `/api/reporting/jobs/download/{jobId}` | ## Other changes 1. Set access options on the routes 2. Removed API Counter functional tests, which were skipped to begin with. 3. Replaced functional tests with Jest integration tests. 4. Consolidated code in the generation routes by creating the `getJobParams` method of the `RequestHandler` class. 5. Added a new test for `getJobParams` 6. Consolidated code in the job management routes 7. Added new code for shared helpers in job management routes 8. Reorganized libs used for route handlers: ``` routes/lib/request_handler.ts => routes/common/generate/request_handler.ts routes/lib/job_management_pre_routing.ts => routes/common/jobs/job_management_pre_routing.ts routes/lib/jobs_query.ts => routes/common/jobs/jobs_query.ts routes/lib/get_document_payload.ts => routes/common/jobs/get_document_payload.ts routes/lib/get_counter.ts => routes/common/get_counter.ts routes/lib/authorized_user_pre_routing.ts => routes/common/authorized_user_pre_routing.ts routes/lib/get_user.ts => routes/common/get_user.ts ``` ## Release Note Updated API endpoint paths for Reporting to clarify which routes are public and which are not. Make sure that any custom script or application that uses Reporting endpoints only uses the public endpoints. --------- Co-authored-by: kibanamachine <[email protected]>
elastic · Aug 1, 2023 · a2e4279 · a2e4279
1 parent fcb592d
commit a2e4279
Show file tree

Hide file tree

Showing 51 changed files with 1,553 additions and 857 deletions.
diff --git a/x-pack/plugins/reporting/common/constants/index.ts b/x-pack/plugins/reporting/common/constants/index.ts
@@ -6,8 +6,8 @@
  */
 
 import { CONTENT_TYPE_CSV } from '@kbn/generate-csv/src/constants';
-import * as reportTypes from './report_types';
 import * as jobTypes from './job_types';
+import * as reportTypes from './report_types';
 
 const { PDF_JOB_TYPE, PDF_JOB_TYPE_V2, PNG_JOB_TYPE, PNG_JOB_TYPE_V2 } = jobTypes;
 
@@ -28,10 +28,6 @@ export const ALLOWED_JOB_CONTENT_TYPES = [
   'text/plain',
 ];
 
-// Re-export type definitions here for convenience.
-export * from './report_types';
-export * from './job_types';
-
 type ReportTypeDeclaration = typeof reportTypes;
 export type ReportTypes = ReportTypeDeclaration[keyof ReportTypeDeclaration];
 
@@ -62,16 +58,6 @@ export const LICENSE_TYPE_GOLD = 'gold' as const;
 export const LICENSE_TYPE_PLATINUM = 'platinum' as const;
 export const LICENSE_TYPE_ENTERPRISE = 'enterprise' as const;
 
-// Routes
-export const API_BASE_URL = '/api/reporting'; // "Generation URL" from share menu
-export const API_BASE_GENERATE = `${API_BASE_URL}/generate`;
-export const API_LIST_URL = `${API_BASE_URL}/jobs`;
-export const API_DIAGNOSE_URL = `${API_BASE_URL}/diagnose`;
-
-export const API_GET_ILM_POLICY_STATUS = `${API_BASE_URL}/ilm_policy_status`;
-export const API_MIGRATE_ILM_POLICY_URL = `${API_BASE_URL}/deprecations/migrate_ilm_policy`;
-export const API_BASE_URL_V1 = '/api/reporting/v1'; //
-
 export const ILM_POLICY_NAME = 'kibana-reporting';
 
 // Usage counter types
@@ -111,6 +97,6 @@ export const REPORT_TABLE_ROW_ID = 'reportJobRow';
 // intended version is 7.14.0
 export const UNVERSIONED_VERSION = '7.14.0';
 
-// hacky endpoint: download CSV without queueing a report
-// FIXME: find a way to make these endpoints "generic" instead of hardcoded, as are the queued report export types
-export const API_GENERATE_IMMEDIATE = `${API_BASE_URL_V1}/generate/immediate/csv_searchsource`;
+export * from './job_types';
+export * from './report_types';
+export * from './routes';
diff --git a/x-pack/plugins/reporting/common/constants/routes.ts b/x-pack/plugins/reporting/common/constants/routes.ts
@@ -0,0 +1,48 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+
+const prefixInternalPath = '/internal/reporting';
+export const INTERNAL_ROUTES = {
+  MIGRATE: {
+    MIGRATE_ILM_POLICY: prefixInternalPath + '/deprecations/migrate_ilm_policy',
+    GET_ILM_POLICY_STATUS: prefixInternalPath + '/ilm_policy_status',
+  },
+  DIAGNOSE: {
+    BROWSER: prefixInternalPath + '/diagnose/browser',
+    SCREENSHOT: prefixInternalPath + '/diagnose/screenshot',
+  },
+  JOBS: {
+    COUNT: prefixInternalPath + '/jobs/count',
+    LIST: prefixInternalPath + '/jobs/list',
+    INFO_PREFIX: prefixInternalPath + '/jobs/info', // docId is added to the final path
+    DELETE_PREFIX: prefixInternalPath + '/jobs/delete', // docId is added to the final path
+    DOWNLOAD_PREFIX: prefixInternalPath + '/jobs/download', // docId is added to the final path
+  },
+  DOWNLOAD_CSV: prefixInternalPath + '/generate/immediate/csv_searchsource',
+  GENERATE_PREFIX: prefixInternalPath + '/generate', // exportTypeId is added to the final path
+};
+
+const prefixPublicPath = '/api/reporting';
+export const PUBLIC_ROUTES = {
+  /**
+   * Public endpoint for POST URL strings and automated report generation
+   * exportTypeId is added to the final path
+   */
+  GENERATE_PREFIX: prefixPublicPath + `/generate`,
+  JOBS: {
+    /**
+     * Public endpoint used by Watcher and automated report downloads
+     * jobId is added to the final path
+     */
+    DOWNLOAD_PREFIX: prefixPublicPath + `/jobs/download`,
+    /**
+     * Public endpoint potentially used to delete a report after download in automation
+     * jobId is added to the final path
+     */
+    DELETE_PREFIX: prefixPublicPath + `/jobs/delete`,
+  },
+};
diff --git a/x-pack/plugins/reporting/public/lib/reporting_api_client/hooks.ts b/x-pack/plugins/reporting/public/lib/reporting_api_client/hooks.ts
@@ -5,16 +5,13 @@
  * 2.0.
  */
 
-import { useRequest, UseRequestResponse } from '../../shared_imports';
+import { INTERNAL_ROUTES } from '../../../common/constants';
 import { IlmPolicyStatusResponse } from '../../../common/types';
-
-import { API_GET_ILM_POLICY_STATUS } from '../../../common/constants';
-
-import { useKibana } from '../../shared_imports';
+import { useKibana, useRequest, UseRequestResponse } from '../../shared_imports';
 
 export const useCheckIlmPolicyStatus = (): UseRequestResponse<IlmPolicyStatusResponse> => {
   const {
     services: { http },
   } = useKibana();
-  return useRequest(http, { path: API_GET_ILM_POLICY_STATUS, method: 'get' });
+  return useRequest(http, { path: INTERNAL_ROUTES.MIGRATE.GET_ILM_POLICY_STATUS, method: 'get' });
 };
diff --git a/x-pack/plugins/reporting/public/lib/reporting_api_client/reporting_api_client.test.ts b/x-pack/plugins/reporting/public/lib/reporting_api_client/reporting_api_client.test.ts
@@ -25,9 +25,9 @@ describe('ReportingAPIClient', () => {
   });
 
   describe('getReportURL', () => {
-    it('should generate report URL', () => {
+    it('should generate the internal report download URL', () => {
       expect(apiClient.getReportURL('123')).toMatchInlineSnapshot(
-        `"/base/path/api/reporting/jobs/download/123"`
+        `"/base/path/internal/reporting/jobs/download/123"`
       );
     });
   });
@@ -52,10 +52,7 @@ describe('ReportingAPIClient', () => {
     it('should send a delete request', async () => {
       await apiClient.deleteReport('123');
 
-      expect(httpClient.delete).toHaveBeenCalledWith(
-        expect.stringContaining('/delete/123'),
-        expect.any(Object)
-      );
+      expect(httpClient.delete).toHaveBeenCalledWith(expect.stringContaining('/delete/123'));
     });
   });
 
@@ -112,10 +109,7 @@ describe('ReportingAPIClient', () => {
     it('should send a get request', async () => {
       await apiClient.getInfo('123');
 
-      expect(httpClient.get).toHaveBeenCalledWith(
-        expect.stringContaining('/info/123'),
-        expect.any(Object)
-      );
+      expect(httpClient.get).toHaveBeenCalledWith(expect.stringContaining('/info/123'));
     });
 
     it('should return a job instance', async () => {
@@ -174,7 +168,7 @@ describe('ReportingAPIClient', () => {
   describe('getReportingJobPath', () => {
     it('should generate a job path', () => {
       expect(
-        apiClient.getReportingJobPath('pdf', {
+        apiClient.getReportingPublicJobPath('pdf', {
           browserTimezone: 'UTC',
           objectType: 'something',
           title: 'some title',
@@ -293,21 +287,15 @@ describe('ReportingAPIClient', () => {
     it('should send a post request', async () => {
       await apiClient.verifyBrowser();
 
-      expect(httpClient.post).toHaveBeenCalledWith(
-        expect.stringContaining('/diagnose/browser'),
-        expect.any(Object)
-      );
+      expect(httpClient.post).toHaveBeenCalledWith(expect.stringContaining('/diagnose/browser'));
     });
   });
 
   describe('verifyScreenCapture', () => {
     it('should send a post request', async () => {
       await apiClient.verifyScreenCapture();
 
-      expect(httpClient.post).toHaveBeenCalledWith(
-        expect.stringContaining('/diagnose/screenshot'),
-        expect.any(Object)
-      );
+      expect(httpClient.post).toHaveBeenCalledWith(expect.stringContaining('/diagnose/screenshot'));
     });
   });
 

diff --git a/x-pack/plugins/reporting/public/lib/reporting_api_client/reporting_api_client.ts b/x-pack/plugins/reporting/public/lib/reporting_api_client/reporting_api_client.ts
@@ -4,20 +4,17 @@
  * 2.0; you may not use this file except in compliance with the Elastic License
  * 2.0.
  */
+import type { HttpFetchQuery } from '@kbn/core/public';
+import { HttpSetup, IUiSettingsClient } from '@kbn/core/public';
 import { i18n } from '@kbn/i18n';
+import rison from '@kbn/rison';
 import moment from 'moment';
 import { stringify } from 'query-string';
-import rison from '@kbn/rison';
-import type { HttpFetchQuery } from '@kbn/core/public';
-import { HttpSetup, IUiSettingsClient } from '@kbn/core/public';
 import { buildKibanaPath } from '../../../common/build_kibana_path';
 import {
-  API_BASE_GENERATE,
-  API_BASE_URL,
-  API_GENERATE_IMMEDIATE,
-  API_LIST_URL,
-  API_MIGRATE_ILM_POLICY_URL,
   getRedirectAppPath,
+  INTERNAL_ROUTES,
+  PUBLIC_ROUTES,
   REPORTING_MANAGEMENT_HOME,
 } from '../../../common/constants';
 import {
@@ -46,7 +43,7 @@ export interface DiagnoseResponse {
 interface IReportingAPI {
   // Helpers
   getReportURL(jobId: string): string;
-  getReportingJobPath<T>(exportType: string, jobParams: BaseParams & T): string; // Return a URL to queue a job, with the job params encoded in the query string of the URL. Used for copying POST URL
+  getReportingPublicJobPath<T>(exportType: string, jobParams: BaseParams & T): string; // Return a URL to queue a job, with the job params encoded in the query string of the URL. Used for copying POST URL
   createReportingJob<T>(exportType: string, jobParams: BaseParams & T): Promise<Job>; // Sends a request to queue a job, with the job params in the POST body
   getServerBasePath(): string; // Provides the raw server basePath to allow it to be stripped out from relativeUrls in job params
 
@@ -96,9 +93,13 @@ export class ReportingAPIClient implements IReportingAPI {
     return href;
   }
 
+  /**
+   * Get the internal URL
+   */
   public getReportURL(jobId: string) {
-    const apiBaseUrl = this.http.basePath.prepend(API_LIST_URL);
-    const downloadLink = `${apiBaseUrl}/download/${jobId}`;
+    const downloadLink = this.http.basePath.prepend(
+      `${INTERNAL_ROUTES.JOBS.DOWNLOAD_PREFIX}/${jobId}`
+    );
 
     return downloadLink;
   }
@@ -110,9 +111,7 @@ export class ReportingAPIClient implements IReportingAPI {
   }
 
   public async deleteReport(jobId: string) {
-    return await this.http.delete<void>(`${API_LIST_URL}/delete/${jobId}`, {
-      asSystemRequest: true,
-    });
+    return await this.http.delete<void>(`${INTERNAL_ROUTES.JOBS.DELETE_PREFIX}/${jobId}`);
   }
 
   public async list(page = 0, jobIds: string[] = []) {
@@ -122,7 +121,7 @@ export class ReportingAPIClient implements IReportingAPI {
       query.ids = jobIds.slice(0, 10).join(',');
     }
 
-    const jobQueueEntries: ReportApiJSON[] = await this.http.get(`${API_LIST_URL}/list`, {
+    const jobQueueEntries: ReportApiJSON[] = await this.http.get(INTERNAL_ROUTES.JOBS.LIST, {
       query,
       asSystemRequest: true,
     });
@@ -131,7 +130,7 @@ export class ReportingAPIClient implements IReportingAPI {
   }
 
   public async total() {
-    return await this.http.get<number>(`${API_LIST_URL}/count`, {
+    return await this.http.get<number>(INTERNAL_ROUTES.JOBS.COUNT, {
       asSystemRequest: true,
     });
   }
@@ -151,47 +150,50 @@ export class ReportingAPIClient implements IReportingAPI {
   }
 
   public async getInfo(jobId: string) {
-    const report: ReportApiJSON = await this.http.get(`${API_LIST_URL}/info/${jobId}`, {
-      asSystemRequest: true,
-    });
+    const report: ReportApiJSON = await this.http.get(
+      `${INTERNAL_ROUTES.JOBS.INFO_PREFIX}/${jobId}`
+    );
     return new Job(report);
   }
 
   public async findForJobIds(jobIds: JobId[]) {
-    const reports: ReportApiJSON[] = await this.http.fetch(`${API_LIST_URL}/list`, {
+    const reports: ReportApiJSON[] = await this.http.fetch(INTERNAL_ROUTES.JOBS.LIST, {
       query: { page: 0, ids: jobIds.join(',') },
       method: 'GET',
     });
     return reports.map((report) => new Job(report));
   }
 
-  public getReportingJobPath(exportType: string, jobParams: BaseParams) {
+  /**
+   * Returns a string for the public API endpoint used to automate the generation of reports
+   * This string must be shown when the user selects the option to view/copy the POST URL
+   */
+  public getReportingPublicJobPath(exportType: string, jobParams: BaseParams) {
     const params = stringify({
       jobParams: rison.encode(jobParams),
     });
-    return `${this.http.basePath.prepend(API_BASE_GENERATE)}/${exportType}?${params}`;
+    return `${this.http.basePath.prepend(PUBLIC_ROUTES.GENERATE_PREFIX)}/${exportType}?${params}`;
   }
 
+  /**
+   * Calls the internal API to generate a report job on-demand
+   */
   public async createReportingJob(exportType: string, jobParams: BaseParams) {
     const jobParamsRison = rison.encode(jobParams);
     const resp: { job: ReportApiJSON } = await this.http.post(
-      `${API_BASE_GENERATE}/${exportType}`,
+      `${INTERNAL_ROUTES.GENERATE_PREFIX}/${exportType}`,
       {
         method: 'POST',
-        body: JSON.stringify({
-          jobParams: jobParamsRison,
-        }),
+        body: JSON.stringify({ jobParams: jobParamsRison }),
       }
     );
-
     add(resp.job.id);
-
     return new Job(resp.job);
   }
 
   public async createImmediateReport(baseParams: BaseParams) {
     const { objectType: _objectType, ...params } = baseParams; // objectType is not needed for immediate download api
-    return this.http.post(`${API_GENERATE_IMMEDIATE}`, {
+    return this.http.post(INTERNAL_ROUTES.DOWNLOAD_CSV, {
       asResponse: true,
       body: JSON.stringify(params),
     });
@@ -217,23 +219,19 @@ export class ReportingAPIClient implements IReportingAPI {
     this.http.basePath.prepend(REPORTING_MANAGEMENT_HOME);
 
   public getDownloadLink: DownloadReportFn = (jobId: JobId) =>
-    this.http.basePath.prepend(`${API_LIST_URL}/download/${jobId}`);
+    this.http.basePath.prepend(`${INTERNAL_ROUTES.JOBS.DOWNLOAD_PREFIX}/${jobId}`);
 
   public getServerBasePath = () => this.http.basePath.serverBasePath;
 
   public verifyBrowser() {
-    return this.http.post<DiagnoseResponse>(`${API_BASE_URL}/diagnose/browser`, {
-      asSystemRequest: true,
-    });
+    return this.http.post<DiagnoseResponse>(INTERNAL_ROUTES.DIAGNOSE.BROWSER);
   }
 
   public verifyScreenCapture() {
-    return this.http.post<DiagnoseResponse>(`${API_BASE_URL}/diagnose/screenshot`, {
-      asSystemRequest: true,
-    });
+    return this.http.post<DiagnoseResponse>(INTERNAL_ROUTES.DIAGNOSE.SCREENSHOT);
   }
 
   public migrateReportingIndicesIlmPolicy() {
-    return this.http.put(`${API_MIGRATE_ILM_POLICY_URL}`);
+    return this.http.put(INTERNAL_ROUTES.MIGRATE.MIGRATE_ILM_POLICY);
   }
 }
diff --git a/...s/reporting/public/share_context_menu/reporting_panel_content/reporting_panel_content.tsx b/...s/reporting/public/share_context_menu/reporting_panel_content/reporting_panel_content.tsx
@@ -84,11 +84,11 @@ class ReportingPanelContentUi extends Component<Props, State> {
   }
 
   private getAbsoluteReportGenerationUrl = (props: Props) => {
-    const relativePath = this.props.apiClient.getReportingJobPath(
+    const relativePath = this.props.apiClient.getReportingPublicJobPath(
       props.reportType,
       this.props.apiClient.getDecoratedJobParams(this.props.getJobParams(true))
     );
-    return url.resolve(window.location.href, relativePath); // FIXME: '(from: string, to: string): string' is deprecated
+    return url.resolve(window.location.href, relativePath);
   };
 
   public componentDidUpdate(_prevProps: Props, prevState: State) {

diff --git a/x-pack/plugins/reporting/server/deprecations/migrate_existing_indices_ilm_policy.test.ts b/x-pack/plugins/reporting/server/deprecations/migrate_existing_indices_ilm_policy.test.ts
@@ -52,7 +52,7 @@ describe("Migrate existing indices' ILM policy deprecations", () => {
           "correctiveActions": Object {
             "api": Object {
               "method": "PUT",
-              "path": "/api/reporting/deprecations/migrate_ilm_policy",
+              "path": "/internal/reporting/deprecations/migrate_ilm_policy",
             },
             "manualSteps": Array [
               "Update all reporting indices to use the \\"kibana-reporting\\" policy using the index settings API.",

diff --git a/x-pack/plugins/reporting/server/deprecations/migrate_existing_indices_ilm_policy.ts b/x-pack/plugins/reporting/server/deprecations/migrate_existing_indices_ilm_policy.ts
@@ -7,7 +7,7 @@
 
 import { i18n } from '@kbn/i18n';
 import { DeprecationsDetails, GetDeprecationsContext } from '@kbn/core/server';
-import { API_MIGRATE_ILM_POLICY_URL, ILM_POLICY_NAME } from '../../common/constants';
+import { INTERNAL_ROUTES, ILM_POLICY_NAME } from '../../common/constants';
 import { ReportingCore } from '../core';
 import { deprecations } from '../lib/deprecations';
 
@@ -54,7 +54,7 @@ export const getDeprecationsInfo = async (
           ],
           api: {
             method: 'PUT',
-            path: API_MIGRATE_ILM_POLICY_URL,
+            path: INTERNAL_ROUTES.MIGRATE.MIGRATE_ILM_POLICY,
           },
         },
       },

diff --git a/...s/lib/authorized_user_pre_routing.test.ts → ...ommon/authorized_user_pre_routing.test.ts b/...s/lib/authorized_user_pre_routing.test.ts → ...ommon/authorized_user_pre_routing.test.ts
diff --git a/...routes/lib/authorized_user_pre_routing.ts → ...tes/common/authorized_user_pre_routing.ts b/...routes/lib/authorized_user_pre_routing.ts → ...tes/common/authorized_user_pre_routing.ts