Skip to content

Commit

Permalink
Merge pull request #365 from crazy-max/build-checks-annotations
Browse files Browse the repository at this point in the history
github: annotate build warnings
  • Loading branch information
crazy-max committed Jun 26, 2024
2 parents 2e70a0c + 644587f commit 7360d08
Show file tree
Hide file tree
Showing 4 changed files with 102 additions and 1 deletion.
2 changes: 1 addition & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ on:

env:
NODE_VERSION: "20"
BUILDX_VERSION: "v0.15.1"
BUILDX_VERSION: "https://github.com/docker/buildx.git#d8c9ebde1fdcf659f1fa3efa6ccc27a28b0f1564" # https://github.com/docker/buildx/pull/2551
BUILDKIT_IMAGE: "moby/buildkit:v0.14.1"

jobs:
Expand Down
28 changes: 28 additions & 0 deletions __tests__/fixtures/lint.Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
# syntax=docker/dockerfile-upstream:master

# Copyright 2024 actions-toolkit authors
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

frOM busybox as base

Check warning on line 17 in __tests__/fixtures/lint.Dockerfile

View workflow job for this annotation

GitHub Actions / test-itg (github.test.itg.ts, ubuntu-latest)

All commands within the Dockerfile should use the same casing (either upper or lower)

ConsistentInstructionCasing: Command 'frOM' should match the case of the command majority (uppercase) More info: https://docs.docker.com/go/dockerfile/rule/consistent-instruction-casing/
cOpy lint.Dockerfile .

Check warning on line 18 in __tests__/fixtures/lint.Dockerfile

View workflow job for this annotation

GitHub Actions / test-itg (github.test.itg.ts, ubuntu-latest)

All commands within the Dockerfile should use the same casing (either upper or lower)

ConsistentInstructionCasing: Command 'cOpy' should match the case of the command majority (uppercase) More info: https://docs.docker.com/go/dockerfile/rule/consistent-instruction-casing/

from scratch

Check warning on line 20 in __tests__/fixtures/lint.Dockerfile

View workflow job for this annotation

GitHub Actions / test-itg (github.test.itg.ts, ubuntu-latest)

All commands within the Dockerfile should use the same casing (either upper or lower)

ConsistentInstructionCasing: Command 'from' should match the case of the command majority (uppercase) More info: https://docs.docker.com/go/dockerfile/rule/consistent-instruction-casing/
MAINTAINER moby@example.com

Check warning on line 21 in __tests__/fixtures/lint.Dockerfile

View workflow job for this annotation

GitHub Actions / test-itg (github.test.itg.ts, ubuntu-latest)

The MAINTAINER instruction is deprecated, use a label instead to define an image author

MaintainerDeprecated: Maintainer instruction is deprecated in favor of using label More info: https://docs.docker.com/go/dockerfile/rule/maintainer-deprecated/
COPy --from=base \

Check warning on line 22 in __tests__/fixtures/lint.Dockerfile

View workflow job for this annotation

GitHub Actions / test-itg (github.test.itg.ts, ubuntu-latest)

All commands within the Dockerfile should use the same casing (either upper or lower)

ConsistentInstructionCasing: Command 'COPy' should match the case of the command majority (uppercase) More info: https://docs.docker.com/go/dockerfile/rule/consistent-instruction-casing/
/lint.Dockerfile \
/

CMD [ "echo", "Hello, Norway!" ]

Check warning on line 26 in __tests__/fixtures/lint.Dockerfile

View workflow job for this annotation

GitHub Actions / test-itg (github.test.itg.ts, ubuntu-latest)

Multiple instructions of the same type should not be used in the same stage

MultipleInstructionsDisallowed: Multiple CMD instructions should not be used in the same stage because only the last one will be used More info: https://docs.docker.com/go/dockerfile/rule/multiple-instructions-disallowed/
CMD [ "echo", "Hello, Sweden!" ]
ENTRYPOINT my-program start

Check warning on line 28 in __tests__/fixtures/lint.Dockerfile

View workflow job for this annotation

GitHub Actions / test-itg (github.test.itg.ts, ubuntu-latest)

JSON arguments recommended for ENTRYPOINT/CMD to prevent unintended behavior related to OS signals

JSONArgsRecommended: JSON arguments recommended for ENTRYPOINT to prevent unintended behavior related to OS signals More info: https://docs.docker.com/go/dockerfile/rule/json-args-recommended/
37 changes: 37 additions & 0 deletions __tests__/github.test.itg.ts
Original file line number Diff line number Diff line change
Expand Up @@ -249,3 +249,40 @@ maybe('writeBuildSummary', () => {
});
});
});

maybe('annotateBuildWarnings', () => {
it('annoate lint issues', async () => {
const buildx = new Buildx();
const build = new Build({buildx: buildx});

fs.mkdirSync(tmpDir, {recursive: true});
await expect(
(async () => {
// prettier-ignore
const buildCmd = await buildx.getCommand([
'--builder', process.env.CTN_BUILDER_NAME ?? 'default',
'build',
'-f', path.join(fixturesDir, 'lint.Dockerfile'),
fixturesDir,
'--metadata-file', build.getMetadataFilePath()
]);
await Exec.exec(buildCmd.command, buildCmd.args, {
env: Object.assign({}, process.env, {
BUILDX_METADATA_WARNINGS: 'true'
}) as {
[key: string]: string;
}
});
})()
).resolves.not.toThrow();

const metadata = build.resolveMetadata();
expect(metadata).toBeDefined();
const buildRef = build.resolveRef(metadata);
expect(buildRef).toBeDefined();
const buildWarnings = build.resolveWarnings(metadata);
expect(buildWarnings).toBeDefined();

await GitHub.annotateBuildWarnings(path.join(fixturesDir, 'lint.Dockerfile'), buildWarnings);
});
});
36 changes: 36 additions & 0 deletions src/github.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import {jwtDecode, JwtPayload} from 'jwt-decode';

import {Util} from './util';

import {VertexWarning} from './types/buildkit/client';
import {BuildSummaryOpts, GitHubActionsRuntimeToken, GitHubActionsRuntimeTokenAC, GitHubRepo, UploadArtifactOpts, UploadArtifactResponse} from './types/github';

export interface GitHubOpts {
Expand Down Expand Up @@ -326,4 +327,39 @@ export class GitHub {
core.info(`Writing summary`);
await sum.addSeparator().write();
}

public static async annotateBuildWarnings(source: string, warnings?: Array<VertexWarning>): Promise<void> {
(warnings ?? []).forEach(warning => {
if (!warning.detail || !warning.short) {
return;
}
const title = warning.detail.map(encoded => atob(encoded)).join(' ');
let message = atob(warning.short).replace(/\s\(line \d+\)$/, '');
if (warning.url) {
// https://github.com/docker/buildx/blob/d8c9ebde1fdcf659f1fa3efa6ccc27a28b0f1564/commands/build.go#L854
message += `\nMore info: ${warning.url}`;
}

// GitHub annotations don't clearly show ranges of lines, so we'll just
// show the first line
const startLine = warning.range && warning.range.length > 0 ? warning.range[0]?.start.line : undefined;

// TODO: When GitHub annotations support showing ranges properly, we can use this code
// let startLine: number | undefined, endLine: number | undefined;
// for (const range of warning.range ?? []) {
// if (range.start.line && (!startLine || range.start.line < startLine)) {
// startLine = range.start.line;
// }
// if (range.end.line && (!endLine || range.end.line > endLine)) {
// endLine = range.end.line;
// }
// }

core.warning(message, {
title: title,
file: source,
startLine: startLine
});
});
}
}

0 comments on commit 7360d08

Please sign in to comment.