From c33719864490cb7ff08321c215706ea1f1c6dd4c Mon Sep 17 00:00:00 2001 From: jaymode Date: Tue, 13 Oct 2020 13:15:38 -0600 Subject: [PATCH 1/3] Update logstash pipeline management to use system index APIs This change updates the logstash pipeline management plugin to use pipeline management APIs in Elasticsearch rather than directly accessing the .logstash index. In Elasticsearch 8.0, direct access to system indices will no longer be allowed when using standard APIs. Given this change, a new set of APIs has been created specifically for the management of Logstash pipelines and this change makes use of the APIs. Relates elastic/elasticsearch#53350 --- .../fetch_all_from_scroll.test.ts | 71 ------------------- .../fetch_all_from_scroll.ts | 34 --------- .../server/lib/fetch_all_from_scroll/index.ts | 7 -- .../server/models/pipeline/pipeline.test.ts | 21 +++--- .../server/models/pipeline/pipeline.ts | 14 ++-- .../pipeline_list_item.test.ts | 19 +++-- .../pipeline_list_item/pipeline_list_item.ts | 12 ++-- .../logstash/server/routes/pipeline/delete.ts | 8 +-- .../logstash/server/routes/pipeline/load.ts | 26 +++---- .../logstash/server/routes/pipeline/save.ts | 8 +-- .../server/routes/pipelines/delete.ts | 8 +-- .../logstash/server/routes/pipelines/list.ts | 30 ++++---- x-pack/plugins/logstash/server/types.ts | 6 -- 13 files changed, 68 insertions(+), 196 deletions(-) delete mode 100755 x-pack/plugins/logstash/server/lib/fetch_all_from_scroll/fetch_all_from_scroll.test.ts delete mode 100755 x-pack/plugins/logstash/server/lib/fetch_all_from_scroll/fetch_all_from_scroll.ts delete mode 100755 x-pack/plugins/logstash/server/lib/fetch_all_from_scroll/index.ts diff --git a/x-pack/plugins/logstash/server/lib/fetch_all_from_scroll/fetch_all_from_scroll.test.ts b/x-pack/plugins/logstash/server/lib/fetch_all_from_scroll/fetch_all_from_scroll.test.ts deleted file mode 100755 index 8cd6b70d475701..00000000000000 --- a/x-pack/plugins/logstash/server/lib/fetch_all_from_scroll/fetch_all_from_scroll.test.ts +++ /dev/null @@ -1,71 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License; - * you may not use this file except in compliance with the Elastic License. - */ -import { fetchAllFromScroll } from './fetch_all_from_scroll'; - -describe('fetch_all_from_scroll', () => { - let stubCallWithRequest: jest.Mock; - - beforeEach(() => { - stubCallWithRequest = jest.fn(); - stubCallWithRequest - .mockResolvedValueOnce({ - hits: { - hits: ['newhit'], - }, - _scroll_id: 'newScrollId', - }) - .mockResolvedValueOnce({ - hits: { - hits: [], - }, - }); - }); - - describe('#fetchAllFromScroll', () => { - describe('when the passed-in response has no hits', () => { - const mockResponse = { - hits: { - hits: [], - }, - }; - - it('should return an empty array of hits', async () => { - const hits = await fetchAllFromScroll(mockResponse as any, stubCallWithRequest); - expect(hits).toEqual([]); - }); - - it('should not call callWithRequest', async () => { - await fetchAllFromScroll(mockResponse as any, stubCallWithRequest); - expect(stubCallWithRequest).toHaveBeenCalledTimes(0); - }); - }); - - describe('when the passed-in response has some hits', () => { - const mockResponse = { - hits: { - hits: ['foo', 'bar'], - }, - _scroll_id: 'originalScrollId', - }; - - it('should return the hits from the response', async () => { - const hits = await fetchAllFromScroll(mockResponse as any, stubCallWithRequest); - expect(hits).toEqual(['foo', 'bar', 'newhit']); - }); - - it('should call callWithRequest', async () => { - await fetchAllFromScroll(mockResponse as any, stubCallWithRequest); - expect(stubCallWithRequest).toHaveBeenCalledTimes(2); - - const firstCallWithRequestCallArgs = stubCallWithRequest.mock.calls[0]; - expect(firstCallWithRequestCallArgs[1].body.scroll_id).toBe('originalScrollId'); - - const secondCallWithRequestCallArgs = stubCallWithRequest.mock.calls[1]; - expect(secondCallWithRequestCallArgs[1].body.scroll_id).toBe('newScrollId'); - }); - }); - }); -}); diff --git a/x-pack/plugins/logstash/server/lib/fetch_all_from_scroll/fetch_all_from_scroll.ts b/x-pack/plugins/logstash/server/lib/fetch_all_from_scroll/fetch_all_from_scroll.ts deleted file mode 100755 index f7cfc4d4fa12a9..00000000000000 --- a/x-pack/plugins/logstash/server/lib/fetch_all_from_scroll/fetch_all_from_scroll.ts +++ /dev/null @@ -1,34 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License; - * you may not use this file except in compliance with the Elastic License. - */ -import { LegacyAPICaller } from 'src/core/server'; -import { SearchResponse } from 'elasticsearch'; - -import { ES_SCROLL_SETTINGS } from '../../../common/constants'; -import { Hits } from '../../types'; - -export async function fetchAllFromScroll( - response: SearchResponse, - callWithRequest: LegacyAPICaller, - hits: Hits = [] -): Promise { - const newHits = response.hits?.hits || []; - const scrollId = response._scroll_id; - - if (newHits.length > 0) { - hits.push(...newHits); - - const innerResponse = await callWithRequest('scroll', { - body: { - scroll: ES_SCROLL_SETTINGS.KEEPALIVE, - scroll_id: scrollId, - }, - }); - - return await fetchAllFromScroll(innerResponse, callWithRequest, hits); - } - - return hits; -} diff --git a/x-pack/plugins/logstash/server/lib/fetch_all_from_scroll/index.ts b/x-pack/plugins/logstash/server/lib/fetch_all_from_scroll/index.ts deleted file mode 100755 index a6865a50633392..00000000000000 --- a/x-pack/plugins/logstash/server/lib/fetch_all_from_scroll/index.ts +++ /dev/null @@ -1,7 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License; - * you may not use this file except in compliance with the Elastic License. - */ - -export { fetchAllFromScroll } from './fetch_all_from_scroll'; diff --git a/x-pack/plugins/logstash/server/models/pipeline/pipeline.test.ts b/x-pack/plugins/logstash/server/models/pipeline/pipeline.test.ts index 82ce0d72e2052c..695531bb695101 100755 --- a/x-pack/plugins/logstash/server/models/pipeline/pipeline.test.ts +++ b/x-pack/plugins/logstash/server/models/pipeline/pipeline.test.ts @@ -10,8 +10,7 @@ describe('pipeline', () => { describe('Pipeline', () => { describe('fromUpstreamJSON factory method', () => { const upstreamJSON = { - _id: 'apache', - _source: { + apache: { description: 'this is an apache pipeline', pipeline_metadata: { version: 1, @@ -21,25 +20,23 @@ describe('pipeline', () => { pipeline: 'input {} filter { grok {} }\n output {}', }, }; + const upstreamId = 'apache'; it('returns correct Pipeline instance', () => { const pipeline = Pipeline.fromUpstreamJSON(upstreamJSON); - expect(pipeline.id).toBe(upstreamJSON._id); - expect(pipeline.description).toBe(upstreamJSON._source.description); - expect(pipeline.username).toBe(upstreamJSON._source.username); - expect(pipeline.pipeline).toBe(upstreamJSON._source.pipeline); + expect(pipeline.id).toBe(upstreamId); + expect(pipeline.description).toBe(upstreamJSON.apache.description); + expect(pipeline.username).toBe(upstreamJSON.apache.username); + expect(pipeline.pipeline).toBe(upstreamJSON.apache.pipeline); }); - it('throws if pipeline argument does not contain an id property', () => { - const badJSON = { - // no _id - _source: upstreamJSON._source, - }; + it('throws if pipeline argument does not contain id as a key', () => { + const badJSON = {}; const testFromUpstreamJsonError = () => { return Pipeline.fromUpstreamJSON(badJSON); }; expect(testFromUpstreamJsonError).toThrowError( - /upstreamPipeline argument must contain an id property/i + /upstreamPipeline argument must contain pipeline id as a key/i ); }); }); diff --git a/x-pack/plugins/logstash/server/models/pipeline/pipeline.ts b/x-pack/plugins/logstash/server/models/pipeline/pipeline.ts index 0b7c3888b6f03c..02eefd839e1fff 100755 --- a/x-pack/plugins/logstash/server/models/pipeline/pipeline.ts +++ b/x-pack/plugins/logstash/server/models/pipeline/pipeline.ts @@ -93,21 +93,21 @@ export class Pipeline { // generate Pipeline object from elasticsearch response static fromUpstreamJSON(upstreamPipeline: Record) { - if (!upstreamPipeline._id) { + if (Object.keys(upstreamPipeline).length !== 1) { throw badRequest( i18n.translate( 'xpack.logstash.upstreamPipelineArgumentMustContainAnIdPropertyErrorMessage', { - defaultMessage: 'upstreamPipeline argument must contain an id property', + defaultMessage: 'upstreamPipeline argument must contain pipeline id as a key', } ) ); } - const id = get(upstreamPipeline, '_id') as string; - const description = get(upstreamPipeline, '_source.description') as string; - const username = get(upstreamPipeline, '_source.username') as string; - const pipeline = get(upstreamPipeline, '_source.pipeline') as string; - const settings = get(upstreamPipeline, '_source.pipeline_settings') as Record; + const id = Object.keys(upstreamPipeline).pop() as string; + const description = get(upstreamPipeline, id + '.description') as string; + const username = get(upstreamPipeline, id + '.username') as string; + const pipeline = get(upstreamPipeline, id + '.pipeline') as string; + const settings = get(upstreamPipeline, id + '.pipeline_settings') as Record; const opts: PipelineOptions = { id, description, username, pipeline, settings }; diff --git a/x-pack/plugins/logstash/server/models/pipeline_list_item/pipeline_list_item.test.ts b/x-pack/plugins/logstash/server/models/pipeline_list_item/pipeline_list_item.test.ts index c557e84443b02a..7a3654794b5901 100755 --- a/x-pack/plugins/logstash/server/models/pipeline_list_item/pipeline_list_item.test.ts +++ b/x-pack/plugins/logstash/server/models/pipeline_list_item/pipeline_list_item.test.ts @@ -9,8 +9,7 @@ import { PipelineListItem } from './pipeline_list_item'; describe('pipeline_list_item', () => { describe('PipelineListItem', () => { const upstreamJSON = { - _id: 'apache', - _source: { + apache: { description: 'this is an apache pipeline', last_modified: '2017-05-14T02:50:51.250Z', pipeline_metadata: { @@ -20,24 +19,22 @@ describe('pipeline_list_item', () => { username: 'elastic', pipeline: 'input {} filter { grok {} }\n output {}', }, - _index: 'index', - _type: 'type', - _score: 100, }; + const upstreamId = 'apache'; describe('fromUpstreamJSON factory method', () => { it('returns correct PipelineListItem instance', () => { - const pipelineListItem = PipelineListItem.fromUpstreamJSON(upstreamJSON); - expect(pipelineListItem.id).toBe(upstreamJSON._id); - expect(pipelineListItem.description).toBe(upstreamJSON._source.description); - expect(pipelineListItem.username).toBe(upstreamJSON._source.username); - expect(pipelineListItem.last_modified).toBe(upstreamJSON._source.last_modified); + const pipelineListItem = PipelineListItem.fromUpstreamJSON(upstreamId, upstreamJSON); + expect(pipelineListItem.id).toBe(upstreamId); + expect(pipelineListItem.description).toBe(upstreamJSON.apache.description); + expect(pipelineListItem.username).toBe(upstreamJSON.apache.username); + expect(pipelineListItem.last_modified).toBe(upstreamJSON.apache.last_modified); }); }); describe('downstreamJSON getter method', () => { it('returns the downstreamJSON JSON', () => { - const pipelineListItem = PipelineListItem.fromUpstreamJSON(upstreamJSON); + const pipelineListItem = PipelineListItem.fromUpstreamJSON(upstreamId, upstreamJSON); const expectedDownstreamJSON = { id: 'apache', description: 'this is an apache pipeline', diff --git a/x-pack/plugins/logstash/server/models/pipeline_list_item/pipeline_list_item.ts b/x-pack/plugins/logstash/server/models/pipeline_list_item/pipeline_list_item.ts index eeb197a58f51da..07326dc745e180 100755 --- a/x-pack/plugins/logstash/server/models/pipeline_list_item/pipeline_list_item.ts +++ b/x-pack/plugins/logstash/server/models/pipeline_list_item/pipeline_list_item.ts @@ -5,7 +5,7 @@ */ import { get } from 'lodash'; -import { Hit, PipelineListItemOptions } from '../../types'; +import { PipelineListItemOptions } from '../../types'; export class PipelineListItem { public readonly id: string; @@ -34,12 +34,12 @@ export class PipelineListItem { * Takes the json GET response from ES and constructs a pipeline model to be used * in Kibana downstream */ - static fromUpstreamJSON(pipeline: Hit) { + static fromUpstreamJSON(id: string, pipeline: Record) { const opts = { - id: pipeline._id, - description: get(pipeline, '_source.description') as string, - last_modified: get(pipeline, '_source.last_modified') as string, - username: get(pipeline, '_source.username') as string, + id, + description: get(pipeline, id + '.description') as string, + last_modified: get(pipeline, id + '.last_modified') as string, + username: get(pipeline, id + '.username') as string, }; return new PipelineListItem(opts); diff --git a/x-pack/plugins/logstash/server/routes/pipeline/delete.ts b/x-pack/plugins/logstash/server/routes/pipeline/delete.ts index 4aeae3e0ae2d5c..d94b3b94b1df02 100644 --- a/x-pack/plugins/logstash/server/routes/pipeline/delete.ts +++ b/x-pack/plugins/logstash/server/routes/pipeline/delete.ts @@ -5,7 +5,6 @@ */ import { schema } from '@kbn/config-schema'; import { IRouter } from 'src/core/server'; -import { INDEX_NAMES } from '../../../common/constants'; import { wrapRouteWithLicenseCheck } from '../../../../licensing/server'; import { checkLicense } from '../../lib/check_license'; @@ -25,10 +24,9 @@ export function registerPipelineDeleteRoute(router: IRouter) { router.handleLegacyErrors(async (context, request, response) => { const client = context.logstash!.esClient; - await client.callAsCurrentUser('delete', { - index: INDEX_NAMES.PIPELINES, - id: request.params.id, - refresh: 'wait_for', + await client.callAsCurrentUser('transport.request', { + path: '/_logstash/pipeline/' + encodeURIComponent(request.params.id), + method: 'DELETE', }); return response.noContent(); diff --git a/x-pack/plugins/logstash/server/routes/pipeline/load.ts b/x-pack/plugins/logstash/server/routes/pipeline/load.ts index fec9097114d261..25bde7c1b89220 100644 --- a/x-pack/plugins/logstash/server/routes/pipeline/load.ts +++ b/x-pack/plugins/logstash/server/routes/pipeline/load.ts @@ -6,7 +6,6 @@ import { schema } from '@kbn/config-schema'; import { IRouter } from 'src/core/server'; -import { INDEX_NAMES } from '../../../common/constants'; import { Pipeline } from '../../models/pipeline'; import { wrapRouteWithLicenseCheck } from '../../../../licensing/server'; import { checkLicense } from '../../lib/check_license'; @@ -26,20 +25,21 @@ export function registerPipelineLoadRoute(router: IRouter) { router.handleLegacyErrors(async (context, request, response) => { const client = context.logstash!.esClient; - const result = await client.callAsCurrentUser('get', { - index: INDEX_NAMES.PIPELINES, - id: request.params.id, - _source: ['description', 'username', 'pipeline', 'pipeline_settings'], - ignore: [404], - }); + try { + const result = await client.callAsCurrentUser('transport.request', { + path: '/_logstash/pipeline/' + encodeURIComponent(request.params.id), + method: 'GET', + }); + return response.ok({ + body: Pipeline.fromUpstreamJSON(result).downstreamJSON, + }); + } catch (err) { + if (err.statusCode === 404) { + return response.notFound(); + } - if (!result.found) { - return response.notFound(); + throw err; } - - return response.ok({ - body: Pipeline.fromUpstreamJSON(result).downstreamJSON, - }); }) ) ); diff --git a/x-pack/plugins/logstash/server/routes/pipeline/save.ts b/x-pack/plugins/logstash/server/routes/pipeline/save.ts index 755a82e670a2a7..e5f28bda1974c0 100644 --- a/x-pack/plugins/logstash/server/routes/pipeline/save.ts +++ b/x-pack/plugins/logstash/server/routes/pipeline/save.ts @@ -7,7 +7,6 @@ import { schema } from '@kbn/config-schema'; import { i18n } from '@kbn/i18n'; import { IRouter } from 'src/core/server'; -import { INDEX_NAMES } from '../../../common/constants'; import { Pipeline } from '../../models/pipeline'; import { wrapRouteWithLicenseCheck } from '../../../../licensing/server'; import { SecurityPluginSetup } from '../../../../security/server'; @@ -41,11 +40,10 @@ export function registerPipelineSaveRoute(router: IRouter, security?: SecurityPl const client = context.logstash!.esClient; const pipeline = Pipeline.fromDownstreamJSON(request.body, request.params.id, username); - await client.callAsCurrentUser('index', { - index: INDEX_NAMES.PIPELINES, - id: pipeline.id, + await client.callAsCurrentUser('transport.request', { + path: '/_logstash/pipeline/' + encodeURIComponent(pipeline.id), + method: 'PUT', body: pipeline.upstreamJSON, - refresh: 'wait_for', }); return response.noContent(); diff --git a/x-pack/plugins/logstash/server/routes/pipelines/delete.ts b/x-pack/plugins/logstash/server/routes/pipelines/delete.ts index 4b2471c89fe07c..4eb3cae1d0956a 100644 --- a/x-pack/plugins/logstash/server/routes/pipelines/delete.ts +++ b/x-pack/plugins/logstash/server/routes/pipelines/delete.ts @@ -7,15 +7,13 @@ import { schema } from '@kbn/config-schema'; import { LegacyAPICaller, IRouter } from 'src/core/server'; import { wrapRouteWithLicenseCheck } from '../../../../licensing/server'; -import { INDEX_NAMES } from '../../../common/constants'; import { checkLicense } from '../../lib/check_license'; async function deletePipelines(callWithRequest: LegacyAPICaller, pipelineIds: string[]) { const deletePromises = pipelineIds.map((pipelineId) => { - return callWithRequest('delete', { - index: INDEX_NAMES.PIPELINES, - id: pipelineId, - refresh: 'wait_for', + return callWithRequest('transport.request', { + path: '/_logstash/pipeline/' + encodeURIComponent(pipelineId), + method: 'DELETE', }) .then((success) => ({ success })) .catch((error) => ({ error })); diff --git a/x-pack/plugins/logstash/server/routes/pipelines/list.ts b/x-pack/plugins/logstash/server/routes/pipelines/list.ts index de2266b89e3d03..ba668ddd11e79b 100644 --- a/x-pack/plugins/logstash/server/routes/pipelines/list.ts +++ b/x-pack/plugins/logstash/server/routes/pipelines/list.ts @@ -4,27 +4,26 @@ * you may not use this file except in compliance with the Elastic License. */ import { i18n } from '@kbn/i18n'; -import { SearchResponse } from 'elasticsearch'; import { LegacyAPICaller, IRouter } from 'src/core/server'; import { wrapRouteWithLicenseCheck } from '../../../../licensing/server'; -import { INDEX_NAMES, ES_SCROLL_SETTINGS } from '../../../common/constants'; import { PipelineListItem } from '../../models/pipeline_list_item'; -import { fetchAllFromScroll } from '../../lib/fetch_all_from_scroll'; import { checkLicense } from '../../lib/check_license'; async function fetchPipelines(callWithRequest: LegacyAPICaller) { const params = { - index: INDEX_NAMES.PIPELINES, - scroll: ES_SCROLL_SETTINGS.KEEPALIVE, - body: { - size: ES_SCROLL_SETTINGS.PAGE_SIZE, - }, - ignore: [404], + path: '/_logstash/pipeline', + method: 'GET', }; - const response = await callWithRequest>('search', params); - return fetchAllFromScroll(response, callWithRequest); + try { + return await callWithRequest('transport.request', params); + } catch (err) { + if (err.statusCode === 404) { + return {}; + } + throw err; + } } export function registerPipelinesListRoute(router: IRouter) { @@ -38,10 +37,13 @@ export function registerPipelinesListRoute(router: IRouter) { router.handleLegacyErrors(async (context, request, response) => { try { const client = context.logstash!.esClient; - const pipelinesHits = await fetchPipelines(client.callAsCurrentUser); + const pipelinesRecord = (await fetchPipelines(client.callAsCurrentUser)) as Record< + string, + any + >; - const pipelines = pipelinesHits.map((pipeline) => { - return PipelineListItem.fromUpstreamJSON(pipeline).downstreamJSON; + const pipelines = Object.keys(pipelinesRecord).map((key) => { + return PipelineListItem.fromUpstreamJSON(key, pipelinesRecord).downstreamJSON; }); return response.ok({ body: { pipelines } }); diff --git a/x-pack/plugins/logstash/server/types.ts b/x-pack/plugins/logstash/server/types.ts index 29198407013abe..d9fd109b209430 100644 --- a/x-pack/plugins/logstash/server/types.ts +++ b/x-pack/plugins/logstash/server/types.ts @@ -3,14 +3,8 @@ * or more contributor license agreements. Licensed under the Elastic License; * you may not use this file except in compliance with the Elastic License. */ -import { SearchResponse } from 'elasticsearch'; import { ILegacyScopedClusterClient } from 'src/core/server'; -type UnwrapArray = T extends Array ? U : never; - -export type Hits = SearchResponse['hits']['hits']; -export type Hit = UnwrapArray; - export interface PipelineListItemOptions { id: string; description: string; From 8eb4cc2fae181270b0425a00c29226e97c0d336c Mon Sep 17 00:00:00 2001 From: Kaise Cheng Date: Thu, 15 Oct 2020 15:28:18 +0200 Subject: [PATCH 2/3] updated privileges. fixed ui tests --- x-pack/plugins/logstash/server/plugin.ts | 6 +-- .../logstash/server/routes/pipeline/load.ts | 24 ++++----- .../logstash/server/routes/pipelines/list.ts | 23 +++----- .../logstash/pipelines/fixtures/list.json | 52 +++++++++---------- .../functional/apps/logstash/pipeline_list.js | 22 ++++---- x-pack/test/functional/config.js | 7 +-- 6 files changed, 57 insertions(+), 77 deletions(-) diff --git a/x-pack/plugins/logstash/server/plugin.ts b/x-pack/plugins/logstash/server/plugin.ts index 0347a606a80441..4a6d476551db0b 100644 --- a/x-pack/plugins/logstash/server/plugin.ts +++ b/x-pack/plugins/logstash/server/plugin.ts @@ -44,10 +44,8 @@ export class LogstashPlugin implements Plugin { }, privileges: [ { - requiredClusterPrivileges: [], - requiredIndexPrivileges: { - ['.logstash']: ['read'], - }, + requiredClusterPrivileges: ['manage_logstash_pipelines'], + requiredIndexPrivileges: {}, ui: [], }, ], diff --git a/x-pack/plugins/logstash/server/routes/pipeline/load.ts b/x-pack/plugins/logstash/server/routes/pipeline/load.ts index 25bde7c1b89220..69d16fb82d8691 100644 --- a/x-pack/plugins/logstash/server/routes/pipeline/load.ts +++ b/x-pack/plugins/logstash/server/routes/pipeline/load.ts @@ -25,21 +25,19 @@ export function registerPipelineLoadRoute(router: IRouter) { router.handleLegacyErrors(async (context, request, response) => { const client = context.logstash!.esClient; - try { - const result = await client.callAsCurrentUser('transport.request', { - path: '/_logstash/pipeline/' + encodeURIComponent(request.params.id), - method: 'GET', - }); - return response.ok({ - body: Pipeline.fromUpstreamJSON(result).downstreamJSON, - }); - } catch (err) { - if (err.statusCode === 404) { - return response.notFound(); - } + const result = await client.callAsCurrentUser('transport.request', { + path: '/_logstash/pipeline/' + encodeURIComponent(request.params.id), + method: 'GET', + ignore: [404], + }); - throw err; + if (result[request.params.id] === undefined) { + return response.notFound(); } + + return response.ok({ + body: Pipeline.fromUpstreamJSON(result).downstreamJSON, + }); }) ) ); diff --git a/x-pack/plugins/logstash/server/routes/pipelines/list.ts b/x-pack/plugins/logstash/server/routes/pipelines/list.ts index ba668ddd11e79b..2bb8be653dfe3f 100644 --- a/x-pack/plugins/logstash/server/routes/pipelines/list.ts +++ b/x-pack/plugins/logstash/server/routes/pipelines/list.ts @@ -14,16 +14,10 @@ async function fetchPipelines(callWithRequest: LegacyAPICaller) { const params = { path: '/_logstash/pipeline', method: 'GET', + ignore: [404], }; - try { - return await callWithRequest('transport.request', params); - } catch (err) { - if (err.statusCode === 404) { - return {}; - } - throw err; - } + return await callWithRequest('transport.request', params); } export function registerPipelinesListRoute(router: IRouter) { @@ -37,14 +31,13 @@ export function registerPipelinesListRoute(router: IRouter) { router.handleLegacyErrors(async (context, request, response) => { try { const client = context.logstash!.esClient; - const pipelinesRecord = (await fetchPipelines(client.callAsCurrentUser)) as Record< - string, - any - >; + const pipelinesRecord = (await fetchPipelines(client.callAsCurrentUser)) as Record; - const pipelines = Object.keys(pipelinesRecord).map((key) => { - return PipelineListItem.fromUpstreamJSON(key, pipelinesRecord).downstreamJSON; - }); + const pipelines = Object.keys(pipelinesRecord) + .sort() + .map((key) => { + return PipelineListItem.fromUpstreamJSON(key, pipelinesRecord).downstreamJSON; + }); return response.ok({ body: { pipelines } }); } catch (err) { diff --git a/x-pack/test/api_integration/apis/logstash/pipelines/fixtures/list.json b/x-pack/test/api_integration/apis/logstash/pipelines/fixtures/list.json index bee5c0aa2dee21..b4c973f22ca28d 100644 --- a/x-pack/test/api_integration/apis/logstash/pipelines/fixtures/list.json +++ b/x-pack/test/api_integration/apis/logstash/pipelines/fixtures/list.json @@ -1,11 +1,5 @@ { "pipelines": [ - { - "id": "tweets_and_beats", - "description": "ingest tweets and beats", - "last_modified": "2017-08-02T18:59:07.724Z", - "username": "elastic" - }, { "id": "empty_pipeline_1", "description": "an empty pipeline", @@ -13,124 +7,130 @@ "username": "elastic" }, { - "id": "empty_pipeline_2", + "id": "empty_pipeline_10", "description": "an empty pipeline", "last_modified": "2017-08-02T18:57:32.907Z", "username": "elastic" }, { - "id": "empty_pipeline_3", + "id": "empty_pipeline_11", "description": "an empty pipeline", "last_modified": "2017-08-02T18:57:32.907Z", "username": "elastic" }, { - "id": "empty_pipeline_4", + "id": "empty_pipeline_12", "description": "an empty pipeline", "last_modified": "2017-08-02T18:57:32.907Z", "username": "elastic" }, { - "id": "empty_pipeline_5", + "id": "empty_pipeline_13", "description": "an empty pipeline", "last_modified": "2017-08-02T18:57:32.907Z", "username": "elastic" }, { - "id": "empty_pipeline_6", + "id": "empty_pipeline_14", "description": "an empty pipeline", "last_modified": "2017-08-02T18:57:32.907Z", "username": "elastic" }, { - "id": "empty_pipeline_7", + "id": "empty_pipeline_15", "description": "an empty pipeline", "last_modified": "2017-08-02T18:57:32.907Z", "username": "elastic" }, { - "id": "empty_pipeline_8", + "id": "empty_pipeline_16", "description": "an empty pipeline", "last_modified": "2017-08-02T18:57:32.907Z", "username": "elastic" }, { - "id": "empty_pipeline_9", + "id": "empty_pipeline_17", "description": "an empty pipeline", "last_modified": "2017-08-02T18:57:32.907Z", "username": "elastic" }, { - "id": "empty_pipeline_10", + "id": "empty_pipeline_18", "description": "an empty pipeline", "last_modified": "2017-08-02T18:57:32.907Z", "username": "elastic" }, { - "id": "empty_pipeline_11", + "id": "empty_pipeline_19", "description": "an empty pipeline", "last_modified": "2017-08-02T18:57:32.907Z", "username": "elastic" }, { - "id": "empty_pipeline_12", + "id": "empty_pipeline_2", "description": "an empty pipeline", "last_modified": "2017-08-02T18:57:32.907Z", "username": "elastic" }, { - "id": "empty_pipeline_13", + "id": "empty_pipeline_20", "description": "an empty pipeline", "last_modified": "2017-08-02T18:57:32.907Z", "username": "elastic" }, { - "id": "empty_pipeline_14", + "id": "empty_pipeline_21", "description": "an empty pipeline", "last_modified": "2017-08-02T18:57:32.907Z", "username": "elastic" }, { - "id": "empty_pipeline_15", + "id": "empty_pipeline_3", "description": "an empty pipeline", "last_modified": "2017-08-02T18:57:32.907Z", "username": "elastic" }, { - "id": "empty_pipeline_16", + "id": "empty_pipeline_4", "description": "an empty pipeline", "last_modified": "2017-08-02T18:57:32.907Z", "username": "elastic" }, { - "id": "empty_pipeline_17", + "id": "empty_pipeline_5", "description": "an empty pipeline", "last_modified": "2017-08-02T18:57:32.907Z", "username": "elastic" }, { - "id": "empty_pipeline_18", + "id": "empty_pipeline_6", "description": "an empty pipeline", "last_modified": "2017-08-02T18:57:32.907Z", "username": "elastic" }, { - "id": "empty_pipeline_19", + "id": "empty_pipeline_7", "description": "an empty pipeline", "last_modified": "2017-08-02T18:57:32.907Z", "username": "elastic" }, { - "id": "empty_pipeline_20", + "id": "empty_pipeline_8", "description": "an empty pipeline", "last_modified": "2017-08-02T18:57:32.907Z", "username": "elastic" }, { - "id": "empty_pipeline_21", + "id": "empty_pipeline_9", "description": "an empty pipeline", "last_modified": "2017-08-02T18:57:32.907Z", "username": "elastic" + }, + { + "id": "tweets_and_beats", + "description": "ingest tweets and beats", + "last_modified": "2017-08-02T18:59:07.724Z", + "username": "elastic" } ] } diff --git a/x-pack/test/functional/apps/logstash/pipeline_list.js b/x-pack/test/functional/apps/logstash/pipeline_list.js index dd6313f7b9b38d..8ea3a9c49af79a 100644 --- a/x-pack/test/functional/apps/logstash/pipeline_list.js +++ b/x-pack/test/functional/apps/logstash/pipeline_list.js @@ -42,16 +42,8 @@ export default function ({ getService, getPageObjects }) { expect(time).to.be.a('string').match(/ ago$/); } - const expectedRows = [ - { - selected: false, - id: 'tweets_and_beats', - description: 'ingest tweets and beats', - username: 'elastic', - }, - ]; - - for (let emptyPipelineId = 1; emptyPipelineId <= 19; ++emptyPipelineId) { + let expectedRows = []; + for (let emptyPipelineId = 1; emptyPipelineId <= 21; ++emptyPipelineId) { expectedRows.push({ selected: false, id: `empty_pipeline_${emptyPipelineId}`, @@ -59,6 +51,10 @@ export default function ({ getService, getPageObjects }) { username: 'elastic', }); } + expectedRows = expectedRows.sort((a, b) => { + return a.id.localeCompare(b.id); + }); + expectedRows.pop(); expect(rowsWithoutTime).to.eql(expectedRows); }); @@ -145,14 +141,14 @@ export default function ({ getService, getPageObjects }) { expect(rowsWithoutTime).to.eql([ { selected: false, - id: 'empty_pipeline_20', + id: 'empty_pipeline_9', description: 'an empty pipeline', username: 'elastic', }, { selected: false, - id: 'empty_pipeline_21', - description: 'an empty pipeline', + id: 'tweets_and_beats', + description: 'ingest tweets and beats', username: 'elastic', }, ]); diff --git a/x-pack/test/functional/config.js b/x-pack/test/functional/config.js index 2072f4aa1c571c..27c45dcbc7c805 100644 --- a/x-pack/test/functional/config.js +++ b/x-pack/test/functional/config.js @@ -472,12 +472,7 @@ export default async function ({ readConfigFile }) { logstash_read_user: { elasticsearch: { - indices: [ - { - names: ['.logstash*'], - privileges: ['read'], - }, - ], + cluster: ['manage_logstash_pipelines'], }, }, From c4455babe3a3cc131868959d6380eb41fe8f4296 Mon Sep 17 00:00:00 2001 From: Kaise Cheng Date: Thu, 15 Oct 2020 16:32:21 +0200 Subject: [PATCH 3/3] fix eslint --- x-pack/plugins/logstash/server/routes/pipelines/list.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/logstash/server/routes/pipelines/list.ts b/x-pack/plugins/logstash/server/routes/pipelines/list.ts index 2bb8be653dfe3f..78690d3091cbd7 100644 --- a/x-pack/plugins/logstash/server/routes/pipelines/list.ts +++ b/x-pack/plugins/logstash/server/routes/pipelines/list.ts @@ -31,7 +31,10 @@ export function registerPipelinesListRoute(router: IRouter) { router.handleLegacyErrors(async (context, request, response) => { try { const client = context.logstash!.esClient; - const pipelinesRecord = (await fetchPipelines(client.callAsCurrentUser)) as Record; + const pipelinesRecord = (await fetchPipelines(client.callAsCurrentUser)) as Record< + string, + any + >; const pipelines = Object.keys(pipelinesRecord) .sort()