Skip to content

Commit

Permalink
[Security Solution][Serverless] Github tickets / notifications (elast…
Browse files Browse the repository at this point in the history
…ic#197265)

## Summary

This PR is accompanied with the [PR in kibana-operations
repo](elastic/kibana-operations#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](elastic#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](elastic/kibana@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.
  • Loading branch information
dkirchan authored and mbondyra committed Nov 8, 2024
1 parent 193b533 commit 8668946
Show file tree
Hide file tree
Showing 4 changed files with 103 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ run(

let branch: string = '';
let pipeline: string = '';
let prependTitle: string = '';
if (updateGithub) {
let isPr = false;

Expand All @@ -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(/\+|\//);
Expand Down Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
<!-- kibanaCiData = {\\"failed-test\\":{\\"test.class\\":\\"some.classname\\",\\"test.name\\":\\"test name\\",\\"test.failCount\\":1}} -->",
Array [
"failed-test",
],
],
],
"results": Array [
Object {
"type": "return",
"value": undefined,
},
],
}
`);
});
});

describe('updateFailureIssue()', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
38 changes: 38 additions & 0 deletions x-pack/plugins/security_solution/scripts/junit_transformer/lib.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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');
Expand Down Expand Up @@ -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}`);
Expand Down

0 comments on commit 8668946

Please sign in to comment.