From 83e64be5d6730ec1f22ade806192bf1eab54751a Mon Sep 17 00:00:00 2001 From: Jill Guyonnet Date: Wed, 21 Feb 2024 09:14:10 +0000 Subject: [PATCH] [Fleet] Add retry logic to serverless API check (#176808) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Closes https://github.com/elastic/kibana/issues/176352 Closes https://github.com/elastic/kibana/issues/176399 https://github.com/elastic/kibana/pull/175315 added the possibility to configure new Fleet Server hosts in serverless, with the constraint that the host URL must match the default URL. The API integration tests written to test this have been flaky, due to failure retrieving the default Fleet Server host or default Elasticsearch output from saved objects. This PR adds retry in the tests. Note: I have tried adding retry logic in the API handlers but kept hitting test flakiness. This fix has been tested using the Flaky Test Runner Pipeline, with 48/49 test runs for observability and security project types: 🟢 https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/5218 🟢 https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/5222 🟢 https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/5225 ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [x] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed --- .../routes/fleet_server_hosts/handler.ts | 1 + .../server/routes/output/handler.test.ts | 16 +++-- .../test_suites/common/fleet/default_setup.ts | 65 +++++++++++++++++++ .../test_suites/observability/fleet/config.ts | 19 +----- .../test_suites/observability/fleet/fleet.ts | 21 ++++-- .../test_suites/security/fleet/config.ts | 19 +----- .../test_suites/security/fleet/fleet.ts | 21 ++++-- 7 files changed, 114 insertions(+), 48 deletions(-) create mode 100644 x-pack/test_serverless/api_integration/test_suites/common/fleet/default_setup.ts diff --git a/x-pack/plugins/fleet/server/routes/fleet_server_hosts/handler.ts b/x-pack/plugins/fleet/server/routes/fleet_server_hosts/handler.ts index 7e6c14506f3da0..0b564b5e573a8e 100644 --- a/x-pack/plugins/fleet/server/routes/fleet_server_hosts/handler.ts +++ b/x-pack/plugins/fleet/server/routes/fleet_server_hosts/handler.ts @@ -36,6 +36,7 @@ async function checkFleetServerHostsWriteAPIsAllowed( return; } + // Fleet Server hosts must have the default host URL in serverless. const serverlessDefaultFleetServerHost = await getFleetServerHost( soClient, SERVERLESS_DEFAULT_FLEET_SERVER_HOST_ID diff --git a/x-pack/plugins/fleet/server/routes/output/handler.test.ts b/x-pack/plugins/fleet/server/routes/output/handler.test.ts index c24863d1cc08fa..26dd6099e474f2 100644 --- a/x-pack/plugins/fleet/server/routes/output/handler.test.ts +++ b/x-pack/plugins/fleet/server/routes/output/handler.test.ts @@ -144,11 +144,19 @@ describe('output handler', () => { it('should return error on put elasticsearch output in serverless if host url is different from default', async () => { jest.spyOn(appContextService, 'getCloud').mockReturnValue({ isServerlessEnabled: true } as any); + // The original output should provide the output type + jest.spyOn(outputService, 'get').mockImplementation((_, id: string) => { + if (id === SERVERLESS_DEFAULT_OUTPUT_ID) { + return { hosts: ['http://elasticsearch:9200'] } as any; + } else { + return { id: 'output1', type: 'elasticsearch' } as any; + } + }); const res = await putOutputHandler( mockContext, { - body: { type: 'elasticsearch', hosts: ['http://localhost:8080'] }, + body: { hosts: ['http://localhost:8080'] }, params: { outputId: 'output1' }, } as any, mockResponse as any @@ -169,7 +177,7 @@ describe('output handler', () => { const res = await putOutputHandler( mockContext, { - body: { type: 'elasticsearch', hosts: ['http://elasticsearch:9200'] }, + body: { hosts: ['http://elasticsearch:9200'] }, params: { outputId: 'output1' }, } as any, mockResponse as any @@ -184,7 +192,7 @@ describe('output handler', () => { const res = await putOutputHandler( mockContext, { - body: { type: 'elasticsearch', name: 'Renamed output' }, + body: { name: 'Renamed output' }, params: { outputId: 'output1' }, } as any, mockResponse as any @@ -201,7 +209,7 @@ describe('output handler', () => { const res = await putOutputHandler( mockContext, { - body: { type: 'elasticsearch', hosts: ['http://localhost:8080'] }, + body: { hosts: ['http://localhost:8080'] }, params: { outputId: 'output1' }, } as any, mockResponse as any diff --git a/x-pack/test_serverless/api_integration/test_suites/common/fleet/default_setup.ts b/x-pack/test_serverless/api_integration/test_suites/common/fleet/default_setup.ts new file mode 100644 index 00000000000000..fe0ff5343bfb71 --- /dev/null +++ b/x-pack/test_serverless/api_integration/test_suites/common/fleet/default_setup.ts @@ -0,0 +1,65 @@ +/* + * 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. + */ + +import { FtrProviderContext } from '../../../ftr_provider_context'; + +const defaultFleetServerHostId = 'default-fleet-server'; +const defaultFleetServerHostUrl = 'https://localhost:8220'; +const defaultElasticsearchOutputId = 'es-default-output'; +const defaultElasticsearchOutputHostUrl = 'https://localhost:9200'; + +export async function expectDefaultFleetServer({ getService }: FtrProviderContext) { + const supertest = getService('supertest'); + const retry = getService('retry'); + + await retry.waitForWithTimeout('get default fleet server', 30_000, async () => { + const { body, status } = await supertest.get( + `/api/fleet/fleet_server_hosts/${defaultFleetServerHostId}` + ); + if (status === 200 && body.item.host_urls.includes(defaultFleetServerHostUrl)) { + return true; + } else { + throw new Error(`Expected default Fleet Server id ${defaultFleetServerHostId} to exist`); + } + }); +} + +export async function expectDefaultElasticsearchOutput({ getService }: FtrProviderContext) { + const supertest = getService('supertest'); + const retry = getService('retry'); + + await retry.waitForWithTimeout('get default Elasticsearch output', 30_000, async () => { + const { body, status } = await supertest.get( + `/api/fleet/outputs/${defaultElasticsearchOutputId}` + ); + if (status === 200 && body.item.hosts.includes(defaultElasticsearchOutputHostUrl)) { + return true; + } else { + throw new Error( + `Expected default Elasticsearch output id ${defaultElasticsearchOutputId} to exist` + ); + } + }); +} + +export const kbnServerArgs = [ + '--xpack.cloud.serverless.project_id=ftr_fake_project_id', + `--xpack.fleet.fleetServerHosts=[${JSON.stringify({ + id: defaultFleetServerHostId, + name: 'Default Fleet Server', + is_default: true, + host_urls: [defaultFleetServerHostUrl], + })}]`, + `--xpack.fleet.outputs=[${JSON.stringify({ + id: defaultElasticsearchOutputId, + name: 'Default Output', + type: 'elasticsearch', + is_default: true, + is_default_monitoring: true, + hosts: [defaultElasticsearchOutputHostUrl], + })}]`, +]; diff --git a/x-pack/test_serverless/api_integration/test_suites/observability/fleet/config.ts b/x-pack/test_serverless/api_integration/test_suites/observability/fleet/config.ts index b3c3918c4dfd52..fa83bef9cedb9c 100644 --- a/x-pack/test_serverless/api_integration/test_suites/observability/fleet/config.ts +++ b/x-pack/test_serverless/api_integration/test_suites/observability/fleet/config.ts @@ -7,6 +7,7 @@ import { createTestConfig } from '../../../config.base'; import { services } from '../apm_api_integration/common/services'; +import { kbnServerArgs } from '../../common/fleet/default_setup'; export default createTestConfig({ serverlessProject: 'oblt', @@ -21,21 +22,5 @@ export default createTestConfig({ // https://github.com/elastic/project-controller/blob/main/internal/project/observability/config/elasticsearch.yml esServerArgs: ['xpack.ml.dfa.enabled=false', 'xpack.ml.nlp.enabled=false'], - kbnServerArgs: [ - '--xpack.cloud.serverless.project_id=ftr_fake_project_id', - `--xpack.fleet.fleetServerHosts=[${JSON.stringify({ - id: 'default-fleet-server', - name: 'Default Fleet Server', - is_default: true, - host_urls: ['https://localhost:8220'], - })}]`, - `--xpack.fleet.outputs=[${JSON.stringify({ - id: 'es-default-output', - name: 'Default Output', - type: 'elasticsearch', - is_default: true, - is_default_monitoring: true, - hosts: ['https://localhost:9200'], - })}]`, - ], + kbnServerArgs, }); diff --git a/x-pack/test_serverless/api_integration/test_suites/observability/fleet/fleet.ts b/x-pack/test_serverless/api_integration/test_suites/observability/fleet/fleet.ts index e6b55bd99a29b0..913c0667895c3f 100644 --- a/x-pack/test_serverless/api_integration/test_suites/observability/fleet/fleet.ts +++ b/x-pack/test_serverless/api_integration/test_suites/observability/fleet/fleet.ts @@ -7,14 +7,19 @@ import expect from 'expect'; import { FtrProviderContext } from '../../../ftr_provider_context'; +import { + expectDefaultElasticsearchOutput, + expectDefaultFleetServer, +} from '../../common/fleet/default_setup'; -export default function ({ getService }: FtrProviderContext) { - const svlCommonApi = getService('svlCommonApi'); - const supertest = getService('supertest'); +export default function (ctx: FtrProviderContext) { + const svlCommonApi = ctx.getService('svlCommonApi'); + const supertest = ctx.getService('supertest'); - // FLAKY: https://github.com/elastic/kibana/issues/176858 - describe.skip('fleet', function () { + describe('fleet', function () { it('rejects request to create a new fleet server hosts if host url is different from default', async () => { + await expectDefaultFleetServer(ctx); + const { body, status } = await supertest .post('/api/fleet/fleet_server_hosts') .set(svlCommonApi.getInternalRequestHeader()) @@ -33,6 +38,8 @@ export default function ({ getService }: FtrProviderContext) { }); it('accepts request to create a new fleet server hosts if host url is same as default', async () => { + await expectDefaultFleetServer(ctx); + const { body, status } = await supertest .post('/api/fleet/fleet_server_hosts') .set(svlCommonApi.getInternalRequestHeader()) @@ -51,6 +58,8 @@ export default function ({ getService }: FtrProviderContext) { }); it('rejects request to create a new elasticsearch output if host is different from default', async () => { + await expectDefaultElasticsearchOutput(ctx); + const { body, status } = await supertest .post('/api/fleet/outputs') .set(svlCommonApi.getInternalRequestHeader()) @@ -71,6 +80,8 @@ export default function ({ getService }: FtrProviderContext) { }); it('accepts request to create a new elasticsearch output if host url is same as default', async () => { + await expectDefaultElasticsearchOutput(ctx); + const { body, status } = await supertest .post('/api/fleet/outputs') .set(svlCommonApi.getInternalRequestHeader()) diff --git a/x-pack/test_serverless/api_integration/test_suites/security/fleet/config.ts b/x-pack/test_serverless/api_integration/test_suites/security/fleet/config.ts index 2805cfa7c607a1..eeb86e176a506e 100644 --- a/x-pack/test_serverless/api_integration/test_suites/security/fleet/config.ts +++ b/x-pack/test_serverless/api_integration/test_suites/security/fleet/config.ts @@ -6,6 +6,7 @@ */ import { createTestConfig } from '../../../config.base'; +import { kbnServerArgs } from '../../common/fleet/default_setup'; export default createTestConfig({ serverlessProject: 'security', @@ -19,21 +20,5 @@ export default createTestConfig({ // https://github.com/elastic/project-controller/blob/main/internal/project/security/config/elasticsearch.yml esServerArgs: ['xpack.ml.nlp.enabled=false'], - kbnServerArgs: [ - '--xpack.cloud.serverless.project_id=ftr_fake_project_id', - `--xpack.fleet.fleetServerHosts=[${JSON.stringify({ - id: 'default-fleet-server', - name: 'Default Fleet Server', - is_default: true, - host_urls: ['https://localhost:8220'], - })}]`, - `--xpack.fleet.outputs=[${JSON.stringify({ - id: 'es-default-output', - name: 'Default Output', - type: 'elasticsearch', - is_default: true, - is_default_monitoring: true, - hosts: ['https://localhost:9200'], - })}]`, - ], + kbnServerArgs, }); diff --git a/x-pack/test_serverless/api_integration/test_suites/security/fleet/fleet.ts b/x-pack/test_serverless/api_integration/test_suites/security/fleet/fleet.ts index 3c8dad02fafc8e..913c0667895c3f 100644 --- a/x-pack/test_serverless/api_integration/test_suites/security/fleet/fleet.ts +++ b/x-pack/test_serverless/api_integration/test_suites/security/fleet/fleet.ts @@ -7,14 +7,19 @@ import expect from 'expect'; import { FtrProviderContext } from '../../../ftr_provider_context'; +import { + expectDefaultElasticsearchOutput, + expectDefaultFleetServer, +} from '../../common/fleet/default_setup'; -export default function ({ getService }: FtrProviderContext) { - const svlCommonApi = getService('svlCommonApi'); - const supertest = getService('supertest'); +export default function (ctx: FtrProviderContext) { + const svlCommonApi = ctx.getService('svlCommonApi'); + const supertest = ctx.getService('supertest'); - // FLAKY: https://github.com/elastic/kibana/issues/176754 - describe.skip('fleet', function () { + describe('fleet', function () { it('rejects request to create a new fleet server hosts if host url is different from default', async () => { + await expectDefaultFleetServer(ctx); + const { body, status } = await supertest .post('/api/fleet/fleet_server_hosts') .set(svlCommonApi.getInternalRequestHeader()) @@ -33,6 +38,8 @@ export default function ({ getService }: FtrProviderContext) { }); it('accepts request to create a new fleet server hosts if host url is same as default', async () => { + await expectDefaultFleetServer(ctx); + const { body, status } = await supertest .post('/api/fleet/fleet_server_hosts') .set(svlCommonApi.getInternalRequestHeader()) @@ -51,6 +58,8 @@ export default function ({ getService }: FtrProviderContext) { }); it('rejects request to create a new elasticsearch output if host is different from default', async () => { + await expectDefaultElasticsearchOutput(ctx); + const { body, status } = await supertest .post('/api/fleet/outputs') .set(svlCommonApi.getInternalRequestHeader()) @@ -71,6 +80,8 @@ export default function ({ getService }: FtrProviderContext) { }); it('accepts request to create a new elasticsearch output if host url is same as default', async () => { + await expectDefaultElasticsearchOutput(ctx); + const { body, status } = await supertest .post('/api/fleet/outputs') .set(svlCommonApi.getInternalRequestHeader())