From da1a7a38d47a0f4d3a2f23fab1fb0e56ad3319b5 Mon Sep 17 00:00:00 2001 From: Robert Austin Date: Mon, 12 Jun 2023 02:36:42 -0400 Subject: [PATCH 01/10] [Security] Fix Cypress e2e in flaky test runner (#159297) # Prevent Security Solution Cypress tests from parallelizing when run by the flaky test runner In pull request and other pipelines, the Cypress security solution runner is expected to support parallel jobs runs. Several jobs are spawned with the same command, but with different environment variables. Each jobs runs a subset of the tests based on the environment variables. These variables are BUILDKITE_PARALLEL_JOB_COUNT and BUILDKITE_PARALLEL_JOB. The Flaky Test Runner is an app that runs many instances of the same config concurrently in order to determine if any of the tests in the config are flaky. When the Flaky Test Runner runs our cypress tests, each job should run all tests instead of a subset. The flaky test runner sets the following environment variables to prevent jobs from running a subset of tests: ``` // by setting chunks vars to value 1, which means all test will run in one job CLI_NUMBER: 1, CLI_COUNT: 1, ``` These environment variables aren't respected by the Security Solution Cypress tests. This is also true for other Cypress tests that use the same code: * cypress/security_solution_investigations * cypress/osquery_cypress This PR introduces a new environment variable: RUN_ALL_TESTS, which explicitly tells the runner to run all tests instead of a subset. This causes the runner to ignore BUILDKITE_PARALLEL_JOB_COUNT and BUILDKITE_PARALLEL_JOB ### Note about other cypress tests This environment variable will be passed to other Cypress runners that aren't used or maintained by Security. Specifically: * cypress/fleet_cypress * cypress/apm_cypress Fleet does not appear to support parallelism, while APM does. In the APM script, a IS_FLAKY_TEST_RUNNER environment variable is created by reading CLI_COUNT: https://github.com/elastic/kibana/blob/09902b1055130f52ae833022633beda130d97e08/.buildkite/scripts/steps/functional/apm_cypress.sh#L13 ``` IS_FLAKY_TEST_RUNNER=${CLI_COUNT:-0} ``` The CLI_COUNT variable isnt affected by this PR, so this should still work, but it would be possible to use the same variable for both scripts. ### Other Ways we could do this We could also modify the our cypress runner to respect CLI_COUNT and CLI_NUMBER instead of the buildkite parameters. We cannot override the buildkite parameters via the step definition as that is not supported. ### Validation When running our Cypress tests via the test runner, it may not be immediately apparent that there is an issue. This PR is equivalent to main except a change to a README: https://github.com/elastic/kibana/pull/159301 Here is a flaky test runner build using that PR. It runs 25 jobs of the Investigation cypress e2e tests: https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/2368 Each job should run the same tests (so we can check if they are flaky.) However job 1 runs these: * alert_table_action_column.cy.ts * alerts_cell_actions.cy.ts image And job 2 runs these: * building_block_alerts.cy.ts image #### With the fix Here is a flaky test runner build, also 25 jobs of the Investigation cypress e2e tests, but using this PR: https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/2369 We have some failing tests, as our suite is currently flaky, however you can see that each job runs all of the tests. ### For maintainers - [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> --- .buildkite/pipelines/flaky_tests/pipeline.ts | 3 +++ .../scripts/run_cypress/parallel.ts | 25 +++++++++++-------- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/.buildkite/pipelines/flaky_tests/pipeline.ts b/.buildkite/pipelines/flaky_tests/pipeline.ts index 23a6803effe2f..24af68de09e22 100644 --- a/.buildkite/pipelines/flaky_tests/pipeline.ts +++ b/.buildkite/pipelines/flaky_tests/pipeline.ts @@ -177,6 +177,9 @@ for (const testSuite of testSuites) { // by setting chunks vars to value 1, which means all test will run in one job CLI_NUMBER: 1, CLI_COUNT: 1, + // The security solution cypress tests don't recognize CLI_NUMBER and CLI_COUNT, they use `BUILDKITE_PARALLEL_JOB_COUNT` and `BUILDKITE_PARALLEL_JOB`, which cannot be overridden here. + // Use `RUN_ALL_TESTS` to make Security Solution Cypress tests run all tests instead of a subset. + RUN_ALL_TESTS: 'true', }, }); break; diff --git a/x-pack/plugins/security_solution/scripts/run_cypress/parallel.ts b/x-pack/plugins/security_solution/scripts/run_cypress/parallel.ts index 865314134e5c0..f15d139dd78d5 100644 --- a/x-pack/plugins/security_solution/scripts/run_cypress/parallel.ts +++ b/x-pack/plugins/security_solution/scripts/run_cypress/parallel.ts @@ -41,19 +41,22 @@ import pRetry from 'p-retry'; import { renderSummaryTable } from './print_run'; import { getLocalhostRealIp } from '../endpoint/common/localhost_services'; -const retrieveIntegrations = ( - specPattern: string[], - chunksTotal: number = process.env.BUILDKITE_PARALLEL_JOB_COUNT - ? parseInt(process.env.BUILDKITE_PARALLEL_JOB_COUNT, 10) - : 1, - chunkIndex: number = process.env.BUILDKITE_PARALLEL_JOB - ? parseInt(process.env.BUILDKITE_PARALLEL_JOB, 10) - : 0 -) => { +const retrieveIntegrations = (specPattern: string[]) => { const integrationsPaths = globby.sync(specPattern); - const chunkSize = Math.ceil(integrationsPaths.length / chunksTotal); - return _.chunk(integrationsPaths, chunkSize)[chunkIndex]; + if (process.env.RUN_ALL_TESTS === 'true') { + return integrationsPaths; + } else { + const chunksTotal: number = process.env.BUILDKITE_PARALLEL_JOB_COUNT + ? parseInt(process.env.BUILDKITE_PARALLEL_JOB_COUNT, 10) + : 1; + const chunkIndex: number = process.env.BUILDKITE_PARALLEL_JOB + ? parseInt(process.env.BUILDKITE_PARALLEL_JOB, 10) + : 0; + const chunkSize = Math.ceil(integrationsPaths.length / chunksTotal); + + return _.chunk(integrationsPaths, chunkSize)[chunkIndex]; + } }; export const cli = () => { From b08c32252491466b5214ed0eccaec52dde3d8e0c Mon Sep 17 00:00:00 2001 From: Pierre Gayvallet Date: Mon, 12 Jun 2023 03:03:54 -0400 Subject: [PATCH 02/10] Allow exporting all SO types (#159289) ## Summary Fix https://github.com/elastic/kibana/issues/150079 Add support for the `*` wildcard for by-type export, allowing to more easily export all the exportable SO types ``` POST /api/saved_objects/_export { types: '*', } ``` ## Release Note The savedObjects export API now supports exporting all types using the `*` wildcard. Please refer to the documentation for more details and examples. --------- Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> --- docs/api/saved-objects/export.asciidoc | 16 ++++- .../index.ts | 6 +- .../src/{index.ts => export/constants.ts} | 7 ++- .../src/export/index.ts | 1 + .../src/export/saved_objects_exporter.test.ts | 58 ++++++++++++++++++- .../src/export/saved_objects_exporter.ts | 9 +++ .../src/routes/utils.test.ts | 15 +++++ .../src/routes/utils.ts | 11 +++- .../apis/saved_objects/export.ts | 27 +++++++++ 9 files changed, 138 insertions(+), 12 deletions(-) rename packages/core/saved-objects/core-saved-objects-import-export-server-internal/src/{index.ts => export/constants.ts} (65%) diff --git a/docs/api/saved-objects/export.asciidoc b/docs/api/saved-objects/export.asciidoc index 53910835c4989..247af44e54c8b 100644 --- a/docs/api/saved-objects/export.asciidoc +++ b/docs/api/saved-objects/export.asciidoc @@ -23,7 +23,7 @@ experimental[] Retrieve sets of saved objects that you want to import into {kib} ==== Request body `type`:: - (Optional, array|string) The saved object types to include in the export. + (Optional, array|string) The saved object types to include in the export. Use `*` to export all the types. `objects`:: (Optional, array) A list of objects to export. @@ -106,7 +106,7 @@ $ curl -X POST api/saved_objects/_export -H 'kbn-xsrf: true' -H 'Content-Type: a -------------------------------------------------- // KIBANA -Export a specific saved object and it's related objects : +Export a specific saved object and it's related objects: [source,sh] -------------------------------------------------- @@ -122,3 +122,15 @@ $ curl -X POST api/saved_objects/_export -H 'kbn-xsrf: true' -H 'Content-Type: a }' -------------------------------------------------- // KIBANA + +Export all exportable saved objects and their related objects: + +[source,sh] +-------------------------------------------------- +$ curl -X POST api/saved_objects/_export -H 'kbn-xsrf: true' -H 'Content-Type: application/json' -d ' +{ + "type": "*", + "includeReferencesDeep": true +}' +-------------------------------------------------- +// KIBANA \ No newline at end of file diff --git a/packages/core/saved-objects/core-saved-objects-import-export-server-internal/index.ts b/packages/core/saved-objects/core-saved-objects-import-export-server-internal/index.ts index 498c5ab00d6c9..b0156db86ba8a 100644 --- a/packages/core/saved-objects/core-saved-objects-import-export-server-internal/index.ts +++ b/packages/core/saved-objects/core-saved-objects-import-export-server-internal/index.ts @@ -6,9 +6,9 @@ * Side Public License, v 1. */ +export { SavedObjectsImporter, SavedObjectsImportError } from './src/import'; export { SavedObjectsExporter, SavedObjectsExportError, - SavedObjectsImporter, - SavedObjectsImportError, -} from './src'; + EXPORT_ALL_TYPES_TOKEN, +} from './src/export'; diff --git a/packages/core/saved-objects/core-saved-objects-import-export-server-internal/src/index.ts b/packages/core/saved-objects/core-saved-objects-import-export-server-internal/src/export/constants.ts similarity index 65% rename from packages/core/saved-objects/core-saved-objects-import-export-server-internal/src/index.ts rename to packages/core/saved-objects/core-saved-objects-import-export-server-internal/src/export/constants.ts index 8f22bad9f10f6..cc178ca858272 100644 --- a/packages/core/saved-objects/core-saved-objects-import-export-server-internal/src/index.ts +++ b/packages/core/saved-objects/core-saved-objects-import-export-server-internal/src/export/constants.ts @@ -6,5 +6,8 @@ * Side Public License, v 1. */ -export { SavedObjectsImporter, SavedObjectsImportError } from './import'; -export { SavedObjectsExporter, SavedObjectsExportError } from './export'; +/** + * Token used for the `type` option when exporting by type + * to specify that all (importable and exportable) types should be exported. + */ +export const EXPORT_ALL_TYPES_TOKEN = '*'; diff --git a/packages/core/saved-objects/core-saved-objects-import-export-server-internal/src/export/index.ts b/packages/core/saved-objects/core-saved-objects-import-export-server-internal/src/export/index.ts index 268bc6d007595..581e244ced7fe 100644 --- a/packages/core/saved-objects/core-saved-objects-import-export-server-internal/src/export/index.ts +++ b/packages/core/saved-objects/core-saved-objects-import-export-server-internal/src/export/index.ts @@ -8,3 +8,4 @@ export { SavedObjectsExporter } from './saved_objects_exporter'; export { SavedObjectsExportError } from './errors'; +export { EXPORT_ALL_TYPES_TOKEN } from './constants'; diff --git a/packages/core/saved-objects/core-saved-objects-import-export-server-internal/src/export/saved_objects_exporter.test.ts b/packages/core/saved-objects/core-saved-objects-import-export-server-internal/src/export/saved_objects_exporter.test.ts index 2bb65c891d837..833f0e3d08158 100644 --- a/packages/core/saved-objects/core-saved-objects-import-export-server-internal/src/export/saved_objects_exporter.test.ts +++ b/packages/core/saved-objects/core-saved-objects-import-export-server-internal/src/export/saved_objects_exporter.test.ts @@ -7,18 +7,27 @@ */ import { httpServerMock } from '@kbn/core-http-server-mocks'; -import type { SavedObject } from '@kbn/core-saved-objects-server'; +import type { SavedObject, SavedObjectsType } from '@kbn/core-saved-objects-server'; import { SavedObjectTypeRegistry } from '@kbn/core-saved-objects-base-server-internal'; import { SavedObjectsExporter } from './saved_objects_exporter'; import { savedObjectsClientMock } from '@kbn/core-saved-objects-api-server-mocks'; import { loggerMock, type MockedLogger } from '@kbn/logging-mocks'; import { Readable } from 'stream'; import { createPromiseFromStreams, createConcatStream } from '@kbn/utils'; +import { EXPORT_ALL_TYPES_TOKEN } from './constants'; async function readStreamToCompletion(stream: Readable): Promise>> { return createPromiseFromStreams([stream, createConcatStream([])]); } +const createType = (parts: Partial): SavedObjectsType => ({ + name: 'type', + namespaceType: 'single', + hidden: false, + mappings: { properties: {} }, + ...parts, +}); + const exportSizeLimit = 10000; const request = httpServerMock.createKibanaRequest(); @@ -32,13 +41,17 @@ describe('getSortedObjectsForExport()', () => { logger = loggerMock.create(); typeRegistry = new SavedObjectTypeRegistry(); savedObjectsClient = savedObjectsClientMock.create(); - exporter = new SavedObjectsExporter({ + exporter = createExporter(); + }); + + const createExporter = () => { + return new SavedObjectsExporter({ exportSizeLimit, logger, savedObjectsClient, typeRegistry, }); - }); + }; describe('#exportByTypes', () => { test('exports selected types and sorts them', async () => { @@ -986,6 +999,45 @@ describe('getSortedObjectsForExport()', () => { ] `); }); + + test(`supports the "all types" wildcard`, async () => { + typeRegistry.registerType( + createType({ + name: 'exportable_1', + management: { importableAndExportable: true }, + }) + ); + typeRegistry.registerType( + createType({ + name: 'exportable_2', + management: { importableAndExportable: true }, + }) + ); + typeRegistry.registerType( + createType({ + name: 'not_exportable', + management: { importableAndExportable: false }, + }) + ); + + exporter = createExporter(); + + savedObjectsClient.find.mockResolvedValueOnce({ + total: 0, + per_page: 1000, + page: 1, + saved_objects: [], + }); + + await exporter.exportByTypes({ request, types: [EXPORT_ALL_TYPES_TOKEN] }); + + expect(savedObjectsClient.createPointInTimeFinder).toHaveBeenCalledTimes(1); + expect(savedObjectsClient.createPointInTimeFinder).toHaveBeenCalledWith( + expect.objectContaining({ + type: ['exportable_1', 'exportable_2'], + }) + ); + }); }); describe('#exportByObjects', () => { diff --git a/packages/core/saved-objects/core-saved-objects-import-export-server-internal/src/export/saved_objects_exporter.ts b/packages/core/saved-objects/core-saved-objects-import-export-server-internal/src/export/saved_objects_exporter.ts index f5f735e3b6c5d..7d4f56aaa238f 100644 --- a/packages/core/saved-objects/core-saved-objects-import-export-server-internal/src/export/saved_objects_exporter.ts +++ b/packages/core/saved-objects/core-saved-objects-import-export-server-internal/src/export/saved_objects_exporter.ts @@ -30,6 +30,7 @@ import { getPreservedOrderComparator, type SavedObjectComparator, } from './utils'; +import { EXPORT_ALL_TYPES_TOKEN } from './constants'; /** * @internal @@ -39,6 +40,7 @@ export class SavedObjectsExporter implements ISavedObjectsExporter { readonly #exportSizeLimit: number; readonly #typeRegistry: ISavedObjectTypeRegistry; readonly #log: Logger; + readonly #exportableTypes: string[]; constructor({ savedObjectsClient, @@ -55,6 +57,9 @@ export class SavedObjectsExporter implements ISavedObjectsExporter { this.#savedObjectsClient = savedObjectsClient; this.#exportSizeLimit = exportSizeLimit; this.#typeRegistry = typeRegistry; + this.#exportableTypes = this.#typeRegistry + .getImportableAndExportableTypes() + .map((type) => type.name); } public async exportByTypes(options: SavedObjectsExportByTypeOptions) { @@ -146,6 +151,10 @@ export class SavedObjectsExporter implements ISavedObjectsExporter { hasReference, search, }: SavedObjectsExportByTypeOptions) { + if (types.includes(EXPORT_ALL_TYPES_TOKEN)) { + types = this.#exportableTypes; + } + const finder = this.#savedObjectsClient.createPointInTimeFinder({ type: types, hasReference, diff --git a/packages/core/saved-objects/core-saved-objects-server-internal/src/routes/utils.test.ts b/packages/core/saved-objects/core-saved-objects-server-internal/src/routes/utils.test.ts index c002e12363af6..0a630fe18d353 100644 --- a/packages/core/saved-objects/core-saved-objects-server-internal/src/routes/utils.test.ts +++ b/packages/core/saved-objects/core-saved-objects-server-internal/src/routes/utils.test.ts @@ -30,6 +30,7 @@ import { kibanaResponseFactory } from '@kbn/core-http-router-server-internal'; import { typeRegistryInstanceMock } from '../saved_objects_service.test.mocks'; import { httpServerMock } from '@kbn/core-http-server-mocks'; import { loggerMock, type MockedLogger } from '@kbn/logging-mocks'; +import { EXPORT_ALL_TYPES_TOKEN } from '@kbn/core-saved-objects-import-export-server-internal'; async function readStreamToCompletion(stream: Readable) { return createPromiseFromStreams([stream, createConcatStream([])]); @@ -148,6 +149,20 @@ describe('validateTypes', () => { expect(validateTypes(allowedTypes, allowedTypes)).toBeUndefined(); expect(validateTypes(['config'], allowedTypes)).toBeUndefined(); }); + it('supports the all types token', () => { + expect(validateTypes([EXPORT_ALL_TYPES_TOKEN], allowedTypes)).toBeUndefined(); + expect(validateTypes([EXPORT_ALL_TYPES_TOKEN, allowedTypes[0]], allowedTypes)).toBeUndefined(); + }); + it('returns an error message for non-allowed types even with the all types token', () => { + expect( + validateTypes( + [EXPORT_ALL_TYPES_TOKEN, 'not-allowed-type', 'not-allowed-type-2'], + allowedTypes + ) + ).toMatchInlineSnapshot( + `"Trying to export non-exportable type(s): not-allowed-type, not-allowed-type-2"` + ); + }); }); describe('validateObjects', () => { diff --git a/packages/core/saved-objects/core-saved-objects-server-internal/src/routes/utils.ts b/packages/core/saved-objects/core-saved-objects-server-internal/src/routes/utils.ts index 7daea1a33237e..454345f322df6 100644 --- a/packages/core/saved-objects/core-saved-objects-server-internal/src/routes/utils.ts +++ b/packages/core/saved-objects/core-saved-objects-server-internal/src/routes/utils.ts @@ -23,7 +23,8 @@ import { SavedObjectsExportResultDetails, SavedObjectsErrorHelpers, } from '@kbn/core-saved-objects-server'; -import { Logger } from '@kbn/logging'; +import type { Logger } from '@kbn/logging'; +import { EXPORT_ALL_TYPES_TOKEN } from '@kbn/core-saved-objects-import-export-server-internal'; export async function createSavedObjectsStreamFromNdJson(ndJsonStream: Readable) { const savedObjects = await createPromiseFromStreams([ @@ -43,7 +44,9 @@ export async function createSavedObjectsStreamFromNdJson(ndJsonStream: Readable) } export function validateTypes(types: string[], supportedTypes: string[]): string | undefined { - const invalidTypes = types.filter((t) => !supportedTypes.includes(t)); + const invalidTypes = types.filter( + (t) => t !== EXPORT_ALL_TYPES_TOKEN && !supportedTypes.includes(t) + ); if (invalidTypes.length) { return `Trying to export non-exportable type(s): ${invalidTypes.join(', ')}`; } @@ -109,6 +112,7 @@ export function throwOnGloballyHiddenTypes( ); } } + /** * @param {string[]} unsupportedTypes saved object types registered with hidden=false and hiddenFromHttpApis=true */ @@ -120,6 +124,7 @@ export function throwOnHttpHiddenTypes(unsupportedTypes: string[]) { ); } } + /** * @param {string[]} type saved object type * @param {ISavedObjectTypeRegistry} registry the saved object type registry @@ -147,6 +152,7 @@ export function throwIfAnyTypeNotVisibleByAPI( throwOnHttpHiddenTypes(unsupportedTypes); } } + export interface BulkGetItem { type: string; id: string; @@ -168,6 +174,7 @@ export interface LogWarnOnExternalRequest { req: KibanaRequest; logger: Logger; } + /** * Only log a warning when the request is internal * Allows us to silence the logs for development diff --git a/test/api_integration/apis/saved_objects/export.ts b/test/api_integration/apis/saved_objects/export.ts index 0b22ec93c2b2c..159585d7e4a75 100644 --- a/test/api_integration/apis/saved_objects/export.ts +++ b/test/api_integration/apis/saved_objects/export.ts @@ -12,6 +12,7 @@ import type { FtrProviderContext } from '../../ftr_provider_context'; function ndjsonToObject(input: string) { return input.split('\n').map((str) => JSON.parse(str)); } + export default function ({ getService }: FtrProviderContext) { const supertest = getService('supertest'); const kibanaServer = getService('kibanaServer'); @@ -53,6 +54,32 @@ export default function ({ getService }: FtrProviderContext) { }); }); + it('should support exporting all types', async () => { + await supertest + .post(`/s/${SPACE_ID}/api/saved_objects/_export`) + .send({ + type: ['*'], + excludeExportDetails: true, + }) + .expect(200) + .then((resp) => { + const objects = ndjsonToObject(resp.text); + expect(objects).to.have.length(4); + expect(objects.map((obj) => ({ id: obj.id, type: obj.type }))).to.eql([ + { id: '7.0.0-alpha1', type: 'config' }, + { + id: '91200a00-9efd-11e7-acb3-3dab96693fab', + type: 'index-pattern', + }, + { + id: 'dd7caf20-9efd-11e7-acb3-3dab96693fab', + type: 'visualization', + }, + { id: 'be3733a0-9efe-11e7-acb3-3dab96693fab', type: 'dashboard' }, + ]); + }); + }); + it('should exclude the export details if asked', async () => { await supertest .post(`/s/${SPACE_ID}/api/saved_objects/_export`) From 55bc8cf567a437581d37e6e7d1380216667f34cd Mon Sep 17 00:00:00 2001 From: Julian Gernun <17549662+jcger@users.noreply.github.com> Date: Mon, 12 Jun 2023 10:19:20 +0200 Subject: [PATCH 03/10] [RO] Incorrect Deletion of Webhook Actions in Kibana Rules (#159204) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Fixes #158167 The issue occurred because we were using a number called "index" as the React key prop. When we removed the first element, the second one took its place, but React still showed the removed element. To fix this problem, we found a solution by using the uuid field that each action item has. We now generate it for each new action we create in the action form and use that as key I was told to add @pmuellr as you might know if we are missing something 🙇 ### Test: why xpath selector? I had to use the xpath selector to fix a problem we had. The problem was between two actions we set up. They look the same, but the body content is the only different (I've attached a screenshot for more details). We use a third party component for these actions. This component doesn't have any "value" attribute and doesn't add anything besides the HTML text. I tried to find other useful details but couldn't find any. The problem comes up when we try to delete one of the actions. To fix it, we needed to check if that component was missing. We already have tools that can look for missing components, but they don't work with the xpath selector. So, I added a new function that can do this. Now, we can use the xpath selector to look for missing components and fix the problem
See Screenshot
```[tasklist] - [x] Make sure that it's ok to generate the uuid - [x] Test - [x] Do we need to backport? Versions? ``` --- test/functional/services/common/find.ts | 10 ++++++ .../action_connector_form/action_form.tsx | 5 ++- .../alert_create_flyout.ts | 35 +++++++++++++++++++ 3 files changed, 49 insertions(+), 1 deletion(-) diff --git a/test/functional/services/common/find.ts b/test/functional/services/common/find.ts index 0b405d20c4db3..682c79047a725 100644 --- a/test/functional/services/common/find.ts +++ b/test/functional/services/common/find.ts @@ -307,6 +307,16 @@ export class FindService extends FtrService { }, timeout); } + public async existsByXpath( + selector: string, + timeout: number = this.WAIT_FOR_EXISTS_TIME + ): Promise { + this.log.debug(`Find.existsByXpath('${selector}') with timeout=${timeout}`); + return await this.exists(async (drive) => { + return this.wrapAll(await drive.findElements(By.xpath(selector))); + }, timeout); + } + public async clickByCssSelectorWhenNotDisabled(selector: string, opts?: TimeoutOpt) { const timeout = opts?.timeout ?? this.defaultFindTimeout; diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_form.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_form.tsx index f5245cd85fb53..bf2a144921d9c 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_form.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_form.tsx @@ -25,6 +25,7 @@ import { RuleActionFrequency, RuleActionParam, } from '@kbn/alerting-plugin/common'; +import { v4 as uuidv4 } from 'uuid'; import { betaBadgeProps } from './beta_badge_props'; import { loadActionTypes, loadAllActions as loadConnectors } from '../../lib/action_connector_api'; import { @@ -242,6 +243,7 @@ export const ActionForm = ({ group: defaultActionGroupId, params: {}, frequency: defaultRuleFrequency, + uuid: uuidv4(), }); setActionIdByIndex(actionTypeConnectors[0].id, actions.length - 1); } else { @@ -255,6 +257,7 @@ export const ActionForm = ({ group: defaultActionGroupId, params: {}, frequency: DEFAULT_FREQUENCY, + uuid: uuidv4(), }); setActionIdByIndex(actionTypeConnectors[0].id, actions.length - 1); } @@ -427,7 +430,7 @@ export const ActionForm = ({ actionItem={actionItem} actionConnector={actionConnector} index={index} - key={`action-form-action-at-${index}`} + key={`action-form-action-at-${actionItem.uuid}`} setActionParamsProperty={setActionParamsProperty} setActionFrequencyProperty={setActionFrequencyProperty} setActionAlertsFilterProperty={setActionAlertsFilterProperty} diff --git a/x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/alert_create_flyout.ts b/x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/alert_create_flyout.ts index 273c39adbf431..e475e8e646450 100644 --- a/x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/alert_create_flyout.ts +++ b/x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/alert_create_flyout.ts @@ -85,6 +85,41 @@ export default ({ getPageObjects, getService }: FtrProviderContext) => { await testSubjects.click('rulesTab'); }); + it('should delete the right action when the same action has been added twice', async () => { + // create a new rule + const ruleName = generateUniqueKey(); + await rules.common.defineIndexThresholdAlert(ruleName); + + // create webhook connector + await testSubjects.click('.webhook-alerting-ActionTypeSelectOption'); + await testSubjects.click('createActionConnectorButton-0'); + await testSubjects.setValue('nameInput', 'webhook-test'); + await testSubjects.setValue('webhookUrlText', 'https://test.test'); + await testSubjects.setValue('webhookUserInput', 'fakeuser'); + await testSubjects.setValue('webhookPasswordInput', 'fakepassword'); + + // save rule + await find.clickByCssSelector('[data-test-subj="saveActionButtonModal"]:not(disabled)'); + await find.setValueByClass('kibanaCodeEditor', 'myUniqueKey'); + await testSubjects.click('saveRuleButton'); + + // add new action and remove first one + await testSubjects.click('ruleSidebarEditAction'); + await testSubjects.click('.webhook-alerting-ActionTypeSelectOption'); + await find.clickByCssSelector( + '[data-test-subj="alertActionAccordion-0"] [aria-label="Delete"]' + ); + + // check that the removed action is the right one + const doesExist = await find.existsByXpath(".//*[text()='myUniqueKey']"); + expect(doesExist).to.eql(false); + + // clean up created alert + const alertsToDelete = await getAlertsByName(ruleName); + await deleteAlerts(alertsToDelete.map((rule: { id: string }) => rule.id)); + expect(true).to.eql(true); + }); + it('should create an alert', async () => { const alertName = generateUniqueKey(); await rules.common.defineIndexThresholdAlert(alertName); From b9a8655e98c48a6dc12a3592089668db2ceefbbe Mon Sep 17 00:00:00 2001 From: Achyut Jhunjhunwala Date: Mon, 12 Jun 2023 12:23:13 +0200 Subject: [PATCH 04/10] Update testing document to expose times field (#159447) ## Summary Update testing documentation to expose `times` field required to run tests to check for flakiness --- x-pack/plugins/apm/dev_docs/testing.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/x-pack/plugins/apm/dev_docs/testing.md b/x-pack/plugins/apm/dev_docs/testing.md index 1fa7ac8774b08..e18ef18a2b86a 100644 --- a/x-pack/plugins/apm/dev_docs/testing.md +++ b/x-pack/plugins/apm/dev_docs/testing.md @@ -97,6 +97,12 @@ node x-pack/plugins/apm/scripts/test/e2e --server node x-pack/plugins/apm/scripts/test/e2e --runner --open ``` +### Rum tests multiple times to check for flakiness + +``` +node x-pack/plugins/apm/scripts/test/e2e --runner --times [--spec ] +``` + ### A11y checks Accessibility tests are added on the e2e with `checkA11y()`, they will run together with cypress. From e5db0db9b7ff3673bccc5e0f32612a817f98d203 Mon Sep 17 00:00:00 2001 From: Dario Gieselaar Date: Mon, 12 Jun 2023 12:52:56 +0200 Subject: [PATCH 05/10] [APM] Add dev docs for Tilt (#159287) --- .../apm/dev_docs/testing_apm_server.md | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100644 x-pack/plugins/apm/dev_docs/testing_apm_server.md diff --git a/x-pack/plugins/apm/dev_docs/testing_apm_server.md b/x-pack/plugins/apm/dev_docs/testing_apm_server.md new file mode 100644 index 0000000000000..6b33449153d39 --- /dev/null +++ b/x-pack/plugins/apm/dev_docs/testing_apm_server.md @@ -0,0 +1,41 @@ +# Testing APM Server changes with Tilt + +This document provides instructions on how to use Tilt to start a local Kubernetes (k8s) cluster for APM Server development that communicates with a local Kibana instance. This is useful when you want to test recent changes in APM Server, as the bundled or published package might not be available yet. + +## Prerequisites + +- Docker must be installed and running on your local system. +- You should have the APM Server repo checked out which contains the Tiltfile. + +## Setup + +There are two methods to start your Kubernetes cluster using Tilt: + +- With a Kibana Docker container started as part of the Kubernetes cluster. +- Connecting to a locally running Kibana instance. + +### Starting Kibana as a Docker container + +By default, when running `tilt up` without any additional configuration, Tilt will start a Kibana Docker container as part of the Kubernetes cluster: + +`$ tilt up` + +### Connecting to a locally running Kibana instance + +If you want to run Kibana locally and connect it with the Kubernetes cluster, follow these steps: + +- Start your local Kibana instance (`yarn start`) +- Navigate to the APM Server Repo and start Tilt with the '--local-kibana' flag: `$ tilt up -- --local-kibana` + +**Note**: You should start the local Kibana instance before or immediately after running the Tilt command. + +As part of the Tilt setup, a `kibana_system_user` is created with the password `changeme`. You can use this user and password for your `kibana.yml` file: + +``` +elasticsearch.username: kibana_system_user +elasticsearch.password: changeme +``` + +### Building and Uploading APM Server Package + +When you start Tilt, the APM Server package is built from the source and uploaded to Kibana automatically. This allows you to test recent changes in your APM Server. From 05f556b162f19ccebecb8a2f95c10a3255cfee6c Mon Sep 17 00:00:00 2001 From: Jonathan Budzenski Date: Mon, 12 Jun 2023 07:17:39 -0500 Subject: [PATCH 06/10] skip failing suite (#72874) --- x-pack/test/security_solution_endpoint/apps/endpoint/index.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x-pack/test/security_solution_endpoint/apps/endpoint/index.ts b/x-pack/test/security_solution_endpoint/apps/endpoint/index.ts index d1a3c92b82bac..4a20f7629e721 100644 --- a/x-pack/test/security_solution_endpoint/apps/endpoint/index.ts +++ b/x-pack/test/security_solution_endpoint/apps/endpoint/index.ts @@ -15,7 +15,8 @@ import { export default function (providerContext: FtrProviderContext) { const { loadTestFile, getService } = providerContext; - describe('endpoint', function () { + // FAILING: https://github.com/elastic/kibana/issues/72874 + describe.skip('endpoint', function () { const ingestManager = getService('ingestManager'); const log = getService('log'); const endpointTestResources = getService('endpointTestResources'); From 0a1ed74ba4294ddc471cc7de4ebb4f59360f5ef2 Mon Sep 17 00:00:00 2001 From: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Date: Mon, 12 Jun 2023 08:22:40 -0400 Subject: [PATCH 07/10] skip failing test suite (#147640) --- .../security_solution_endpoint_api_int/apis/endpoint_authz.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x-pack/test/security_solution_endpoint_api_int/apis/endpoint_authz.ts b/x-pack/test/security_solution_endpoint_api_int/apis/endpoint_authz.ts index f6c655164e249..30b8ea0166b5a 100644 --- a/x-pack/test/security_solution_endpoint_api_int/apis/endpoint_authz.ts +++ b/x-pack/test/security_solution_endpoint_api_int/apis/endpoint_authz.ts @@ -38,7 +38,8 @@ export default function ({ getService }: FtrProviderContext) { body: Record | undefined; } - describe('When attempting to call an endpoint api', () => { + // Failing: See https://github.com/elastic/kibana/issues/147640 + describe.skip('When attempting to call an endpoint api', () => { let indexedData: IndexedHostsAndAlertsResponse; let actionId = ''; let agentId = ''; From bcc4f11e0b97e991d94f5e6368dfa154f753d2ca Mon Sep 17 00:00:00 2001 From: Carlos Crespo Date: Mon, 12 Jun 2023 14:30:23 +0200 Subject: [PATCH 08/10] [Infrastructure UI] Filter control re-rendering problem fix (#159320) fixes https://github.com/elastic/kibana/issues/159317 ## Summary This PR fixes a problem in the utilization of the `ControlGroupRenderer` component in the Hosts View. The problem originated from the need to manually compare changes in the `filterPanel` object, to prevent the page from making duplicate requests https://github.com/elastic/kibana/assets/2767137/b38f5691-0519-4ae2-aab2-daaf0f72cd0d After many changes that the code has been through, the comparison mentioned above has become unnecessary. ### How to test - Start a local Kibana instance - Navigate to `Infrastructure > Hosts` - Play with the filter controls (depending on how fast the user is, it might hang for a little while, but this won't slow the whole page down) --- .../search_bar/controls_content.tsx | 34 ++++++++----------- .../components/search_bar/limit_options.tsx | 4 +-- .../search_bar/unified_search_bar.tsx | 20 ++++------- 3 files changed, 23 insertions(+), 35 deletions(-) diff --git a/x-pack/plugins/infra/public/pages/metrics/hosts/components/search_bar/controls_content.tsx b/x-pack/plugins/infra/public/pages/metrics/hosts/components/search_bar/controls_content.tsx index e2bd7d0c74dae..9338f15f3ece9 100644 --- a/x-pack/plugins/infra/public/pages/metrics/hosts/components/search_bar/controls_content.tsx +++ b/x-pack/plugins/infra/public/pages/metrics/hosts/components/search_bar/controls_content.tsx @@ -12,16 +12,15 @@ import { type ControlGroupInput, } from '@kbn/controls-plugin/public'; import { ViewMode } from '@kbn/embeddable-plugin/public'; -import { compareFilters, COMPARE_ALL_OPTIONS, Filter, Query, TimeRange } from '@kbn/es-query'; +import type { Filter, Query, TimeRange } from '@kbn/es-query'; import { DataView } from '@kbn/data-views-plugin/public'; -import { skipWhile, Subscription } from 'rxjs'; +import { Subscription } from 'rxjs'; import { useControlPanels } from '../../hooks/use_control_panels_url_state'; interface Props { dataView: DataView | undefined; timeRange: TimeRange; filters: Filter[]; - selectedOptions: Filter[]; query: Query; onFiltersChange: (filters: Filter[]) => void; } @@ -30,13 +29,11 @@ export const ControlsContent: React.FC = ({ dataView, filters, query, - selectedOptions, timeRange, onFiltersChange, }) => { const [controlPanels, setControlPanels] = useControlPanels(dataView); - const inputSubscription = useRef(); - const filterSubscription = useRef(); + const subscriptions = useRef(new Subscription()); const getInitialInput = useCallback(async () => { const initialInput: Partial = { @@ -57,27 +54,24 @@ export const ControlsContent: React.FC = ({ const loadCompleteHandler = useCallback( (controlGroup: ControlGroupAPI) => { if (!controlGroup) return; - inputSubscription.current = controlGroup.onFiltersPublished$ - .pipe( - skipWhile((newFilters) => - compareFilters(selectedOptions, newFilters, COMPARE_ALL_OPTIONS) - ) - ) - .subscribe((newFilters) => { + + subscriptions.current.add( + controlGroup.onFiltersPublished$.subscribe((newFilters) => { onFiltersChange(newFilters); - }); + }) + ); - filterSubscription.current = controlGroup - .getInput$() - .subscribe(({ panels }) => setControlPanels(panels)); + subscriptions.current.add( + controlGroup.getInput$().subscribe(({ panels }) => setControlPanels(panels)) + ); }, - [onFiltersChange, setControlPanels, selectedOptions] + [onFiltersChange, setControlPanels] ); useEffect(() => { + const currentSubscriptions = subscriptions.current; return () => { - filterSubscription.current?.unsubscribe(); - inputSubscription.current?.unsubscribe(); + currentSubscriptions.unsubscribe(); }; }, []); diff --git a/x-pack/plugins/infra/public/pages/metrics/hosts/components/search_bar/limit_options.tsx b/x-pack/plugins/infra/public/pages/metrics/hosts/components/search_bar/limit_options.tsx index 5c77556483b77..b54d187d482e3 100644 --- a/x-pack/plugins/infra/public/pages/metrics/hosts/components/search_bar/limit_options.tsx +++ b/x-pack/plugins/infra/public/pages/metrics/hosts/components/search_bar/limit_options.tsx @@ -27,7 +27,7 @@ interface Props { export const LimitOptions = ({ limit, onChange }: Props) => { const [idSelected, setIdSelected] = useState(limit as number); - const onSelected = (value: number) => { + const onSelected = (_id: string, value: number) => { setIdSelected(value); onChange(value); }; @@ -70,7 +70,7 @@ export const LimitOptions = ({ limit, onChange }: Props) => { })} idSelected={buildId(idSelected)} options={options} - onChange={(_, value: number) => onSelected(value)} + onChange={onSelected} /> diff --git a/x-pack/plugins/infra/public/pages/metrics/hosts/components/search_bar/unified_search_bar.tsx b/x-pack/plugins/infra/public/pages/metrics/hosts/components/search_bar/unified_search_bar.tsx index b2e82c9ef4c7f..b0f537fc3e874 100644 --- a/x-pack/plugins/infra/public/pages/metrics/hosts/components/search_bar/unified_search_bar.tsx +++ b/x-pack/plugins/infra/public/pages/metrics/hosts/components/search_bar/unified_search_bar.tsx @@ -5,14 +5,8 @@ * 2.0. */ -import React, { useMemo } from 'react'; -import { - compareFilters, - COMPARE_ALL_OPTIONS, - type Query, - type TimeRange, - type Filter, -} from '@kbn/es-query'; +import React, { useCallback, useMemo } from 'react'; +import type { Query, TimeRange, Filter } from '@kbn/es-query'; import { i18n } from '@kbn/i18n'; import { EuiFlexGrid, @@ -42,11 +36,12 @@ export const UnifiedSearchBar = () => { onSubmit({ limit }); }; - const onPanelFiltersChange = (panelFilters: Filter[]) => { - if (!compareFilters(searchCriteria.panelFilters, panelFilters, COMPARE_ALL_OPTIONS)) { + const onPanelFiltersChange = useCallback( + (panelFilters: Filter[]) => { onSubmit({ panelFilters }); - } - }; + }, + [onSubmit] + ); const handleRefresh = (payload: { query?: Query; dateRange: TimeRange }, isUpdate?: boolean) => { // This makes sure `onQueryChange` is only called when the submit button is clicked @@ -83,7 +78,6 @@ export const UnifiedSearchBar = () => { dataView={dataView} query={searchCriteria.query} filters={searchCriteria.filters} - selectedOptions={searchCriteria.panelFilters} onFiltersChange={onPanelFiltersChange} /> From 1e8145c925ad296d6b88a175e4229a85fa8f0457 Mon Sep 17 00:00:00 2001 From: Jonathan Budzenski Date: Mon, 12 Jun 2023 08:07:14 -0500 Subject: [PATCH 09/10] skip failing test suite (#159450) --- .../test/security_solution_endpoint/apps/integrations/index.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x-pack/test/security_solution_endpoint/apps/integrations/index.ts b/x-pack/test/security_solution_endpoint/apps/integrations/index.ts index 0da61dbbb637f..e3bf9d178beeb 100644 --- a/x-pack/test/security_solution_endpoint/apps/integrations/index.ts +++ b/x-pack/test/security_solution_endpoint/apps/integrations/index.ts @@ -15,7 +15,8 @@ import { export default function (providerContext: FtrProviderContext) { const { loadTestFile, getService } = providerContext; - describe('endpoint', function () { + // FAILING: https://github.com/elastic/kibana/issues/159450 + describe.skip('endpoint', function () { const ingestManager = getService('ingestManager'); const log = getService('log'); const endpointTestResources = getService('endpointTestResources'); From 70472fd42a6a673b9eba48c363e3425ad1a86d58 Mon Sep 17 00:00:00 2001 From: Thomas Watson Date: Mon, 12 Jun 2023 15:09:02 +0200 Subject: [PATCH 10/10] [docs] improve external plugin documentation (#159431) Co-authored-by: James Rodewig --- docs/developer/plugin/plugin-tooling.asciidoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/developer/plugin/plugin-tooling.asciidoc b/docs/developer/plugin/plugin-tooling.asciidoc index 864d99f138585..5d515c99399de 100644 --- a/docs/developer/plugin/plugin-tooling.asciidoc +++ b/docs/developer/plugin/plugin-tooling.asciidoc @@ -31,7 +31,7 @@ WARNING: {kib} distributable is not shipped with `@kbn/optimizer` anymore. You n You can leverage {kib-repo}blob/{branch}/packages/kbn-plugin-helpers[@kbn/plugin-helpers] to build a distributable archive for your plugin. The package transpiles the plugin code, adds polyfills, and links necessary js modules in the runtime. -You don't need to install the `plugin-helpers`: the `package.json` is already pre-configured if you created your plugin with `node scripts/generate_plugin` script. +You don't need to install the `plugin-helpers` dependency. If you created the plugin using `node scripts/generate_plugin` script, `package.json` is already pre-configured. To build your plugin run within your plugin folder: ["source","shell"] -----------