From 68c097eae34c826df22295e1d7b80d3047295bd6 Mon Sep 17 00:00:00 2001 From: dkirchan <55240027+dkirchan@users.noreply.github.com> Date: Thu, 7 Nov 2024 15:17:54 +0200 Subject: [PATCH] [Security Solution][Serverless] Github tickets / notifications (#197265) ## Summary This PR is accompanied with the [PR in kibana-operations repo](https://github.com/elastic/kibana-operations/pull/236) and integrates the security solution quality gate pipelines with the functionality to create ticket when a test fails and then notify the respective **codeowner** team with a _Test failed alert_. Also when a test fails a second time and a ticket exist a new comment is added in the same issue that the test failed again in a given pipeline. Last a similar flow exist when a test is skipped by a team. **Specifications:** - In the `failed_test_reporter_cli.ts` [a new field is introduced ](https://github.com/elastic/kibana/compare/main...security-mki-github-tickets#diff-bb801c18fd2e1a3a36a3b39fbf02c1abe337c46c201ad5a01239a9d2501d4b56R47)`prependTitle` which is initialized by the environmental variable `process.env.PREPEND_FAILURE_TITLE` if exists, in order to add a custom text of preference before the issue title. The scope of this is to be able to give the team the opportunity to add some tags or any convention agreed within the team in the issue created in order to easily understand the context without opening the ticket. If this field is not initialized, the normal existing flow proceeds. [Here](https://github.com/elastic/kibana/issues/197133) an example of the prependTitle usage can be seen where the tags `[MKI][QA]` are prepended in order to identify where did the test fail, having the same exactly tests running on both CI and MKI. This means that a github issue with the exact same title would be created for both cases if this prepend title field would not exist. - In the [junit_transformer/lib.ts](https://github.com/elastic/kibana/compare/main...security-mki-github-tickets#diff-31c5651af613c7d02139f3e9fccd00ddb997f2502523372dd19db9e0659a66d6R278) a new functionality is introduced to cover an existing issue. The existing issue is: the fact that we are retrying the failed test in cypress in the `parallel_serverless` script, leads to two junit xml result files completely identical with only difference the execution time and the timestamps. This change will take one by one the xml outputs, transform them exactly as it was happening before and then save the file, but also remove the time and timestamp fields, convert it to a string and add it to a "state" list. Then in a next file if it is already parsed and saved to the list having the exact same results within, it deletes the file instead of saving it transformed. The problem before this fix was the fact that when two xml outputs existed, a github ticket was created and when parsing-uploading the second file it was immediately failing for a second time as well. This means that `new-failure` notification was never triggered after the github actions flow split, and the existing failure was always triggered, something that most teams have disabled. With the fix, the identical files are parsed but only once uploaded and the new failure flow works again. (cherry picked from commit 9353497d0adea457c13643bc8fa1a26a05422e8f) --- .../failed_tests_reporter_cli.ts | 11 ++++- .../report_failure.test.ts | 47 +++++++++++++++++++ .../failed_tests_reporter/report_failure.ts | 10 +++- .../scripts/junit_transformer/lib.ts | 38 +++++++++++++++ 4 files changed, 103 insertions(+), 3 deletions(-) diff --git a/packages/kbn-failed-test-reporter-cli/failed_tests_reporter/failed_tests_reporter_cli.ts b/packages/kbn-failed-test-reporter-cli/failed_tests_reporter/failed_tests_reporter_cli.ts index d86f7f5213125..36466c3e3637e 100644 --- a/packages/kbn-failed-test-reporter-cli/failed_tests_reporter/failed_tests_reporter_cli.ts +++ b/packages/kbn-failed-test-reporter-cli/failed_tests_reporter/failed_tests_reporter_cli.ts @@ -44,6 +44,7 @@ run( let branch: string = ''; let pipeline: string = ''; + let prependTitle: string = ''; if (updateGithub) { let isPr = false; @@ -52,6 +53,7 @@ run( pipeline = process.env.BUILDKITE_PIPELINE_SLUG || ''; isPr = process.env.BUILDKITE_PULL_REQUEST === 'true'; updateGithub = process.env.REPORT_FAILED_TESTS_TO_GITHUB === 'true'; + prependTitle = process.env.PREPEND_FAILURE_TITLE || ''; } else { // JOB_NAME is formatted as `elastic+kibana+7.x` in some places and `elastic+kibana+7.x/JOB=kibana-intake,node=immutable` in others const jobNameSplit = (process.env.JOB_NAME || '').split(/\+|\//); @@ -154,7 +156,14 @@ run( continue; } - const newIssue = await createFailureIssue(buildUrl, failure, githubApi, branch, pipeline); + const newIssue = await createFailureIssue( + buildUrl, + failure, + githubApi, + branch, + pipeline, + prependTitle + ); existingIssues.addNewlyCreated(failure, newIssue); pushMessage('Test has not failed recently on tracked branches'); if (updateGithub) { diff --git a/packages/kbn-failed-test-reporter-cli/failed_tests_reporter/report_failure.test.ts b/packages/kbn-failed-test-reporter-cli/failed_tests_reporter/report_failure.test.ts index dce47461377c4..c7dbd2db77319 100644 --- a/packages/kbn-failed-test-reporter-cli/failed_tests_reporter/report_failure.test.ts +++ b/packages/kbn-failed-test-reporter-cli/failed_tests_reporter/report_failure.test.ts @@ -60,6 +60,53 @@ describe('createFailureIssue()', () => { } `); }); + + it('creates new github issue with title prepended', async () => { + const api = new GithubApi(); + + await createFailureIssue( + 'https://build-url', + { + classname: 'some.classname', + failure: 'this is the failure text', + name: 'test name', + time: '2018-01-01T01:00:00Z', + likelyIrrelevant: false, + }, + api, + 'main', + 'kibana-on-merge', + '[MKI][QA]' + ); + + expect(api.createIssue).toMatchInlineSnapshot(` + [MockFunction] { + "calls": Array [ + Array [ + "Failing test: [MKI][QA] some.classname - test name", + "A test failed on a tracked branch + + \`\`\` + this is the failure text + \`\`\` + + First failure: [kibana-on-merge - main](https://build-url) + + ", + Array [ + "failed-test", + ], + ], + ], + "results": Array [ + Object { + "type": "return", + "value": undefined, + }, + ], + } + `); + }); }); describe('updateFailureIssue()', () => { diff --git a/packages/kbn-failed-test-reporter-cli/failed_tests_reporter/report_failure.ts b/packages/kbn-failed-test-reporter-cli/failed_tests_reporter/report_failure.ts index 182bde666c250..d941f94b6b24d 100644 --- a/packages/kbn-failed-test-reporter-cli/failed_tests_reporter/report_failure.ts +++ b/packages/kbn-failed-test-reporter-cli/failed_tests_reporter/report_failure.ts @@ -17,9 +17,15 @@ export async function createFailureIssue( failure: TestFailure, api: GithubApi, branch: string, - pipeline: string + pipeline: string, + prependTitle: string = '' ) { - const title = `Failing test: ${failure.classname} - ${failure.name}`; + // PrependTitle is introduced to provide some clarity by prepending the failing test title + // in order to give the whole info in the title according to each team's preference. + const title = + prependTitle && prependTitle.trim() !== '' + ? `Failing test: ${prependTitle} ${failure.classname} - ${failure.name}` + : `Failing test: ${failure.classname} - ${failure.name}`; // Github API body length maximum is 65536 characters // Let's keep consistency with Mocha output that is truncated to 8192 characters diff --git a/x-pack/plugins/security_solution/scripts/junit_transformer/lib.ts b/x-pack/plugins/security_solution/scripts/junit_transformer/lib.ts index 725baa689cd20..e09a057978bce 100644 --- a/x-pack/plugins/security_solution/scripts/junit_transformer/lib.ts +++ b/x-pack/plugins/security_solution/scripts/junit_transformer/lib.ts @@ -18,6 +18,20 @@ import { PathReporter } from 'io-ts/lib/PathReporter'; import globby from 'globby'; import del from 'del'; +// Function to remove specific fields from an XML object in order to +// compare them as strings. +// eslint-disable-next-line @typescript-eslint/no-explicit-any +function removeFields(obj: any, fieldsToRemove: string[]): any { + for (const key in obj) { + if (fieldsToRemove.includes(key)) { + delete obj[key]; + } else if (typeof obj[key] === 'object') { + obj[key] = removeFields(obj[key], fieldsToRemove); // Recursively remove fields + } + } + return obj; +} + /** * Updates the `name` and `classname` attributes of each testcase. * `name` will have the value of `classname` appended to it. This makes sense because they each contain part of the bdd spec. @@ -174,6 +188,10 @@ export async function command({ flags, log }: CommandArgs) { throw createFlagError('please provide a single --reportName flag'); } + // Going to be used in order to store results as string in order to compare + // and remove duplicated reports. + const xmlResultFiles: string[] = []; + for (const path of await globby(flags.pathPattern)) { // Read the file const source: string = await fs.readFile(path, 'utf8'); @@ -242,6 +260,26 @@ ${boilerplate} rootDirectory: flags.rootDirectory, }); + // We need to check if a XML Junit report is duplicate + // Only if we remove the time and timestamp and the rest of the + // report as a string is completely identical. + const fieldsToRemove = ['time', 'timestamp']; + const tempReport = await parseStringPromise(reportString); + const truncatedReport = removeFields(tempReport, fieldsToRemove); + + // Rebuild the XML to compare (optional, if you want to compare XML strings) + const builder = new Builder(); + const rebuildComparableReport = builder.buildObject(truncatedReport); + + // Compare the cleaned and rebuilt XML objects or strings + if (xmlResultFiles.includes(rebuildComparableReport)) { + // If the report is a duplicate, we need to remove the file + // in order to be excluded from the uploaded results. + await del(path, { force: true }); + continue; + } + xmlResultFiles.push(rebuildComparableReport); + // If the writeInPlace flag was passed, overwrite the original file, otherwise log the output to stdout if (flags.writeInPlace) { log.info(`Wrote transformed junit report to ${path}`);