Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove ANSI control chars from GitHub step Summary and PR comment #1312

Open
wants to merge 17 commits into
base: main
Choose a base branch
from
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

- fix: Make `pulumi login` respect configuration in `Pulumi.yaml`
([#1299](https://github.com/pulumi/actions/pull/1299))
- feat: Strip ansi control characters from GitHub step Summary and PR comment
([#1312](https://github.com/pulumi/actions/pull/1312))

---

Expand Down
1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@
"@actions/tool-cache": "^2.0.1",
"@pulumi/pulumi": "3.134.0",
"actions-parsers": "^1.0.2",
"ansi-to-html": "^0.7.2",
"dedent": "^0.7.0",
"envalid": "^7.3.1",
"got": "^11.8.6",
Expand Down
30 changes: 14 additions & 16 deletions src/libs/__tests__/pr.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,12 @@ describe('pr.ts', () => {

await handlePullRequestMessage(defaultOptions, projectName, 'test');
expect(createComment).toHaveBeenCalledWith({
body: '#### :tropical_drink: `preview` on myFirstProject/staging\n\n<details>\n<summary>Pulumi report</summary>\n\n\n<pre>\ntest\n</pre>\n\n</details>',
body: `#### :tropical_drink: \`preview\` on ${projectName}/${defaultOptions.stackName}\n\n<details>\n<summary>Pulumi report</summary>\n\n\n<pre>\ntest\n</pre>\n\n</details>`,
issue_number: 123,
});
});

it('should convert ansi control character to html and add to pull request message', async () => {
it('should convert ansi control character to plain text and add to pull request message', async () => {
// @ts-ignore
gh.context = {
payload: {
Expand All @@ -69,7 +69,7 @@ describe('pr.ts', () => {

await handlePullRequestMessage(defaultOptions, projectName, '\x1b[30mblack\x1b[37mwhite');
expect(createComment).toHaveBeenCalledWith({
body: '#### :tropical_drink: `preview` on myFirstProject/staging\n\n<details>\n<summary>Pulumi report</summary>\n\n\n<pre>\n<span style="color:#000">black<span style="color:#AAA">white</span></span>\n</pre>\n\n</details>',
body: `#### :tropical_drink: \`preview\` on ${projectName}/${defaultOptions.stackName}\n\n<details>\n<summary>Pulumi report</summary>\n\n\n<pre>\nblackwhite\n</pre>\n\n</details>`,
issue_number: 123,
});
});
Expand All @@ -93,7 +93,7 @@ describe('pr.ts', () => {

await handlePullRequestMessage(options, projectName, 'test');
expect(createComment).toHaveBeenCalledWith({
body: '#### :tropical_drink: `preview` on myFirstProject/staging\n\n<details>\n<summary>Pulumi report</summary>\n\n\n<pre>\ntest\n</pre>\n\n</details>',
body: `#### :tropical_drink: \`preview\` on ${projectName}/${options.stackName}\n\n<details>\n<summary>Pulumi report</summary>\n\n\n<pre>\ntest\n</pre>\n\n</details>`,
issue_number: 87,
});
});
Expand All @@ -120,7 +120,7 @@ describe('pr.ts', () => {

await handlePullRequestMessage(options, projectName, 'test');
expect(createComment).toHaveBeenCalledWith({
body: '#### :tropical_drink: `preview` on myFirstProject/staging\n\n<details>\n<summary>Pulumi report</summary>\n\n\n<pre>\ntest\n</pre>\n\n</details>',
body: `#### :tropical_drink: \`preview\` on ${projectName}/${options.stackName}\n\n<details>\n<summary>Pulumi report</summary>\n\n\n<pre>\ntest\n</pre>\n\n</details>`,
issue_number: 87,
});
});
Expand Down Expand Up @@ -158,15 +158,14 @@ describe('pr.ts', () => {
},
},
};
const alwaysIncludeSummaryOptions = {
command: 'preview',
stackName: 'staging',

const options: Config = {
...defaultOptions,
alwaysIncludeSummary: true,
options: {},
} as Config;
};

await handlePullRequestMessage(
alwaysIncludeSummaryOptions,
options,
projectName,
'a'.repeat(65_000) + '\n' + 'this is at the end and should be in the output',
);
Expand All @@ -182,17 +181,16 @@ describe('pr.ts', () => {
// @ts-ignore
gh.context = { repo: {} };

const options = {
command: 'preview',
stackName: 'staging',
const options: Config = {
...defaultOptions,
commentOnPrNumber: 123,
editCommentOnPr: true,
} as Config;
};

await handlePullRequestMessage(options, projectName, 'test');
expect(updateComment).toHaveBeenCalledWith({
comment_id: 2,
body: '#### :tropical_drink: `preview` on myFirstProject/staging\n\n<details>\n<summary>Pulumi report</summary>\n\n\n<pre>\ntest\n</pre>\n\n</details>',
body: `#### :tropical_drink: \`preview\` on ${projectName}/${options.stackName}\n\n<details>\n<summary>Pulumi report</summary>\n\n\n<pre>\ntest\n</pre>\n\n</details>`,
});
});
});
112 changes: 112 additions & 0 deletions src/libs/__tests__/summary.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
import * as fs from 'fs'
import path from 'path'
import * as core from '@actions/core';
import { SUMMARY_ENV_VAR } from '@actions/core/lib/summary'
import { Config } from '../../config';
import { handleSummaryMessage } from '../summary';

const testDirectoryPath = path.join(__dirname, 'test')
const testFilePath = path.join(testDirectoryPath, 'test-summary.md')

async function getSummary(): Promise<string> {
const file = await fs.promises.readFile(testFilePath, {encoding: 'utf8'});
return file.replace(/\r\n/g, '\n');
}

const projectName = 'myFirstProject';
const defaultOptions = {
command: 'preview',
stackName: 'staging',
options: {},
} as Config;

describe('summary.ts', () => {
beforeEach(async () => {
process.env[SUMMARY_ENV_VAR] = testFilePath;
await fs.promises.mkdir(testDirectoryPath, {recursive: true});
await fs.promises.writeFile(testFilePath, '', {encoding: 'utf8'});
core.summary.emptyBuffer();
})

afterAll(async () => {
await fs.promises.unlink(testFilePath);
})

it('throws if summary env var is undefined', async () => {
process.env[SUMMARY_ENV_VAR] = undefined;
const write = core.summary.addRaw('123').write();
await expect(write).rejects.toThrow();
})

it('throws if summary file does not exist', async () => {
await fs.promises.unlink(testFilePath);
const write = core.summary.addRaw('123').write();
await expect(write).rejects.toThrow();
})

it('should add only heading with empty code block to summary', async () => {
const message = '';
const expected = `<h1>Pulumi ${projectName}/${defaultOptions.stackName} results</h1>\n<pre lang="diff"><code></pre>\n`;

await handleSummaryMessage(defaultOptions, projectName, message);
const summary = await getSummary()

expect(summary).toBe(expected);
})

it('should convert ansi control character to plain text and add to summary', async () => {
const message = '\x1b[30mblack\x1b[37mwhite';
const expected = `<h1>Pulumi ${projectName}/${defaultOptions.stackName} results</h1>\n<pre lang="diff"><code>blackwhite</code></pre>\n`;

await handleSummaryMessage(defaultOptions, projectName, message);
const summary = await getSummary()

expect(summary).toBe(expected);
})

it('should trim the output when the output is larger than 1 MiB', async () => {
const message = 'this is at the begining and should not be in the output' + 'a'.repeat(1_048_576) + 'this is at the end and should be in the output';

await handleSummaryMessage(defaultOptions, projectName, message);
const summary = await getSummary()

expect(Buffer.byteLength(summary, 'utf8')).toBeLessThan(1_048_576);
expect(summary).toContain('this is at the begining and should not be in the output')
expect(summary).toContain('The output was too long and trimmed.');
expect(summary).not.toContain('this is at the end and should be in the output');
expect(summary).not.toContain('The output was too long and trimmed from the front.');
})

it('should trim the output from front when the output is larger than 1 MiB and config is set', async () => {
const message = 'this is at the begining and should not be in the output' + 'a'.repeat(1_048_576) + 'this is at the end and should be in the output';

const options: Config = {
...defaultOptions,
alwaysIncludeSummary: true,
};

await handleSummaryMessage(options, projectName, message);
const summary = await getSummary()

expect(Buffer.byteLength(summary, 'utf8')).toBeLessThan(1_048_576);
expect(summary).toContain('this is at the end and should be in the output');
expect(summary).toContain('The output was too long and trimmed from the front.');
expect(summary).not.toContain('this is at the begining and should not be in the output')
expect(summary).not.toContain('The output was too long and trimmed.');
})

it('should replace first leading space with non-breaking space character to preserve the formatting', async () => {
const message =
`begin
some leading spaces in this line
`;

const expected = `<h1>Pulumi ${projectName}/${defaultOptions.stackName} results</h1>\n<pre lang="diff"><code>begin\n&nbsp; some leading spaces in this line\n</code></pre>\n`;

await handleSummaryMessage(defaultOptions, projectName, message);
const summary = await getSummary()

expect(summary).toBe(expected);
})

})
39 changes: 18 additions & 21 deletions src/libs/pr.ts
Original file line number Diff line number Diff line change
@@ -1,47 +1,40 @@
import * as core from '@actions/core';
import { context, getOctokit } from '@actions/github';
import AnsiToHtml from 'ansi-to-html';
import dedent from 'dedent';
import invariant from 'ts-invariant';
import { Config } from '../config';

function ansiToHtml(
function trimOutput(
message: string,
maxLength: number,
alwaysIncludeSummary: boolean,
): [string, boolean] {
/**
* Converts an ansi string to html by for example removing color escape characters.
* message: ansi string to convert
* maxLength: Maximum number of characters of final message incl. HTML tags
* Trim message to maxLength
* message: string to trim
* maxLength: Maximum number of characters of final message
* alwaysIncludeSummary: if true, trim message from front (if trimming is needed), otherwise from end
*
* return message as html and information if message was trimmed because of length
* return message and information if message was trimmed
*/
const convert = new AnsiToHtml();
let trimmed = false;

let htmlBody: string = convert.toHtml(message);
// Check if message exceeds max characters
if (message.length > maxLength) {

// Check if htmlBody exceeds max characters
if (htmlBody.length > maxLength) {

// trim input message by number of exceeded characters from front or back as configured
const dif: number = htmlBody.length - maxLength;
// Trim input message by number of exceeded characters from front or back as configured
const dif: number = message.length - maxLength;

if (alwaysIncludeSummary) {
message = message.substring(dif, htmlBody.length);
message = message.substring(dif, message.length);
} else {
message = message.substring(0, message.length - dif);
}

trimmed = true;

// convert trimmed message to html
htmlBody = convert.toHtml(message);
}

return [htmlBody, trimmed];
return [message, trimmed];
}

export async function handlePullRequestMessage(
Expand All @@ -57,14 +50,18 @@ export async function handlePullRequestMessage(
alwaysIncludeSummary,
} = config;

// GitHub limits comment characters to 65535, use lower max to keep buffer for variable values
// Remove ANSI symbols from output because they are not supported in GitHub PR message
const regex = RegExp(`\x1B(?:[@-Z\\-_]|[[0-?]*[ -/]*[@-~])`, 'g');
output = output.replace(regex, '');

// GitHub limits PR comment characters to 65_535, use lower max to keep buffer for variable values
const MAX_CHARACTER_COMMENT = 64_000;

const heading = `#### :tropical_drink: \`${command}\` on ${projectName}/${stackName}`;

const summary = '<summary>Pulumi report</summary>';

const [htmlBody, trimmed]: [string, boolean] = ansiToHtml(output, MAX_CHARACTER_COMMENT, alwaysIncludeSummary);
const [message, trimmed]: [string, boolean] = trimOutput(output, MAX_CHARACTER_COMMENT, alwaysIncludeSummary);

const body = dedent`
${heading}
Expand All @@ -77,7 +74,7 @@ export async function handlePullRequestMessage(
: ''
}
<pre>
${htmlBody}
${message}
</pre>
${trimmed && !alwaysIncludeSummary
? ':warning: **Warn**: The output was too long and trimmed.'
Expand Down
74 changes: 74 additions & 0 deletions src/libs/summary.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
import * as core from '@actions/core';
import { Config } from '../config';

function trimOutput(
message: string,
maxSize: number,
alwaysIncludeSummary: boolean,
): [string, boolean] {
/**
* Trim message to maxSize in bytes
* message: string to trim
* maxSize: Maximum number of bytes of final message
* alwaysIncludeSummary: if true, trim message from front (if trimming is needed), otherwise from end
*
* return message and information if message was trimmed
*/
let trimmed = false;

const messageSize = Buffer.byteLength(message, 'utf8');

// Check if message exceeds max size
if (messageSize > maxSize) {

// Trim input message by number of exceeded bytes from front or back as configured
const dif: number = messageSize - maxSize;

if (alwaysIncludeSummary) {
message = Buffer.from(message).subarray(dif, messageSize).toString()
} else {
message = Buffer.from(message).subarray(0, messageSize - dif).toString()
}

trimmed = true;
}

return [message, trimmed];
}

export async function handleSummaryMessage(
config: Config,
projectName: string,
output: string,
): Promise<void> {
const {
stackName,
alwaysIncludeSummary,
} = config;

// Remove ANSI symbols from output because they are not supported in GitHub step Summary
const regex_ansi = RegExp(`\x1B(?:[@-Z\\-_]|[[0-?]*[ -/]*[@-~])`, 'g');
output = output.replace(regex_ansi, '');

// Replace the first leading space in each line with a non-breaking space character to preserve the formatting
const regex_space = RegExp(`^[ ]`, 'gm');
output = output.replace(regex_space, '&nbsp;');

// GitHub limits step Summary to 1 MiB (1_048_576 bytes), use lower max to keep buffer for variable values
const MAX_SUMMARY_SIZE_BYTES = 1_000_000;

const [message, trimmed]: [string, boolean] = trimOutput(output, MAX_SUMMARY_SIZE_BYTES, alwaysIncludeSummary);

let heading = `Pulumi ${projectName}/${stackName} results`;

if (trimmed && alwaysIncludeSummary) {
heading += ' :warning: **Warn**: The output was too long and trimmed from the front.';
} else if (trimmed && !alwaysIncludeSummary) {
heading += ' :warning: **Warn**: The output was too long and trimmed.';
}

await core.summary
.addHeading(heading)
.addCodeBlock(message, "diff")
.write();
}
6 changes: 2 additions & 4 deletions src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
import { environmentVariables } from './libs/envs';
import { handlePullRequestMessage } from './libs/pr';
import * as pulumiCli from './libs/pulumi-cli';
import { handleSummaryMessage } from './libs/summary';
import { login } from './login';

const main = async () => {
Expand Down Expand Up @@ -124,10 +125,7 @@ const runAction = async (config: Config): Promise<void> => {
}

if (config.commentOnSummary) {
await core.summary
.addHeading(`Pulumi ${config.stackName} results`)
.addCodeBlock(output, "diff")
.write();
handleSummaryMessage(config, projectName, output)
}

if (config.remove && config.command === 'destroy') {
Expand Down