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

buildx: convert vertex warnings to github annotations based on localstate #401

Merged
merged 3 commits into from
Jul 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 74 additions & 0 deletions __tests__/buildx/buildx.test.itg.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
/**
* 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.
*/

import {describe, expect, it} from '@jest/globals';
import * as fs from 'fs';
import * as path from 'path';
import * as core from '@actions/core';

import {Buildx} from '../../src/buildx/buildx';
import {Build} from '../../src/buildx/build';
import {Exec} from '../../src/exec';

const fixturesDir = path.join(__dirname, '..', 'fixtures');

// prettier-ignore
const tmpDir = path.join(process.env.TEMP || '/tmp', 'buildx-jest');

const maybe = !process.env.GITHUB_ACTIONS || (process.env.GITHUB_ACTIONS === 'true' && process.env.ImageOS && process.env.ImageOS.startsWith('ubuntu')) ? describe : describe.skip;

maybe('convertWarningsToGitHubAnnotations', () => {
it('annotate 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();

const annotations = await Buildx.convertWarningsToGitHubAnnotations(buildWarnings ?? [], [buildRef ?? '']);
expect(annotations).toBeDefined();
expect(annotations?.length).toBeGreaterThan(0);

for (const annotation of annotations ?? []) {
core.warning(annotation.message, annotation);
}
});
});
37 changes: 0 additions & 37 deletions __tests__/github.test.itg.ts
Original file line number Diff line number Diff line change
Expand Up @@ -295,40 +295,3 @@ 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);
});
});
28 changes: 28 additions & 0 deletions __tests__/util.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,34 @@ lines`;
});
});

describe('isPathRelativeTo', () => {
it('should return true for a child path directly inside the parent path', () => {
const parentPath = '/home/user/projects';
const childPath = '/home/user/projects/subproject';
expect(Util.isPathRelativeTo(parentPath, childPath)).toBe(true);
});
it('should return true for a deeply nested child path inside the parent path', () => {
const parentPath = '/home/user';
const childPath = '/home/user/projects/subproject/module';
expect(Util.isPathRelativeTo(parentPath, childPath)).toBe(true);
});
it('should return false for a child path outside the parent path', () => {
const parentPath = '/home/user/projects';
const childPath = '/home/user/otherprojects/subproject';
expect(Util.isPathRelativeTo(parentPath, childPath)).toBe(false);
});
it('should return true for a child path specified with relative segments', () => {
const parentPath = '/home/user/projects';
const childPath = '/home/user/projects/../projects/subproject';
expect(Util.isPathRelativeTo(parentPath, childPath)).toBe(true);
});
it('should return false when the child path is actually a parent path', () => {
const parentPath = '/home/user/projects/subproject';
const childPath = '/home/user/projects';
expect(Util.isPathRelativeTo(parentPath, childPath)).toBe(false);
});
});

// See: https://github.com/actions/toolkit/blob/a1b068ec31a042ff1e10a522d8fdf0b8869d53ca/packages/core/src/core.ts#L89
function getInputName(name: string): string {
return `INPUT_${name.replace(/ /g, '_').toUpperCase()}`;
Expand Down
153 changes: 153 additions & 0 deletions src/buildx/buildx.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,16 @@ import path from 'path';
import * as core from '@actions/core';
import * as semver from 'semver';

import {Git} from '../buildkit/git';
import {Docker} from '../docker/docker';
import {GitHub} from '../github';
import {Exec} from '../exec';
import {Util} from '../util';

import {VertexWarning} from '../types/buildkit/client';
import {GitURL} from '../types/buildkit/git';
import {Cert, LocalRefsOpts, LocalRefsResponse, LocalState} from '../types/buildx/buildx';
import {GitHubAnnotation} from '../types/github';

export interface BuildxOpts {
standalone?: boolean;
Expand Down Expand Up @@ -266,4 +272,151 @@ export class Buildx {

return refs;
}

public static async convertWarningsToGitHubAnnotations(warnings: Array<VertexWarning>, buildRefs: Array<string>, refsDir?: string): Promise<Array<GitHubAnnotation> | undefined> {
if (warnings.length === 0) {
return;
}
const fnGitURL = function (inp: string): GitURL | undefined {
try {
return Git.parseURL(inp);
} catch (e) {
// noop
}
};
const fnLocalState = function (ref: string): LocalState | undefined {
try {
return Buildx.localState(ref, refsDir);
} catch (e) {
core.debug(`Buildx.convertWarningsToGitHubAnnotations(${ref}): local state not found: ${e.message}`);
}
};

interface Dockerfile {
path: string;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
content?: any;
remote?: boolean;
}

const dockerfiles: Array<Dockerfile> = [];
for (const ref of buildRefs) {
const ls = fnLocalState(ref);
if (!ls) {
continue;
}

if (ls.DockerfilePath == '-') {
// exclude dockerfile from stdin
core.debug(`Buildx.convertWarningsToGitHubAnnotations(${ref}): skipping stdin Dockerfile`);
continue;
} else if (ls.DockerfilePath == '') {
ls.DockerfilePath = 'Dockerfile';
}

const gitURL = fnGitURL(ls.LocalPath);
if (gitURL) {
core.debug(`Buildx.convertWarningsToGitHubAnnotations(${ref}): git context detected: ${ls.LocalPath}`);
const remoteHost = gitURL.host.replace(/:.*/, '');
if (remoteHost !== 'github.com' && !remoteHost.endsWith('.ghe.com')) {
// we only support running actions on GitHub for now
// we might add support for GitLab in the future
core.debug(`Buildx.convertWarningsToGitHubAnnotations(${ref}): not a GitHub repo: ${remoteHost}`);
continue;
}
// if repository matches then we can link to the Dockerfile
const remoteRepo = gitURL.path.replace(/^\//, '').replace(/\.git$/, '');
if (remoteRepo !== GitHub.repository) {
core.debug(`Buildx.convertWarningsToGitHubAnnotations(${ref}): not same GitHub repo: ${remoteRepo} != ${GitHub.repository}`);
continue;
}
dockerfiles.push({
path: ls.DockerfilePath, // dockerfile path is always relative for a git context
remote: true
});
continue;
}

if (!fs.existsSync(ls.DockerfilePath)) {
core.debug(`Buildx.convertWarningsToGitHubAnnotations: Dockerfile not found from localstate ref ${ref}: ${ls.DockerfilePath}`);
continue;
}

const workspaceDir = GitHub.workspace;
// only treat dockerfile path relative to GitHub actions workspace dir
if (Util.isPathRelativeTo(workspaceDir, ls.DockerfilePath)) {
dockerfiles.push({
path: path.relative(workspaceDir, ls.DockerfilePath),
content: btoa(fs.readFileSync(ls.DockerfilePath, {encoding: 'utf-8'}))
});
} else {
core.debug(`Buildx.convertWarningsToGitHubAnnotations: skipping Dockerfile outside of workspace: ${ls.DockerfilePath}`);
}
}

if (dockerfiles.length === 0) {
core.debug(`Buildx.convertWarningsToGitHubAnnotations: no Dockerfiles found`);
return;
}
core.debug(`Buildx.convertWarningsToGitHubAnnotations: found ${dockerfiles.length} Dockerfiles: ${JSON.stringify(dockerfiles, null, 2)}`);

const annotations: Array<GitHubAnnotation> = [];
for (const warning of warnings) {
if (!warning.detail || !warning.short) {
core.debug(`Buildx.convertWarningsToGitHubAnnotations: skipping warning without detail or short`);
continue;
}

const warningSourceFilename = warning.sourceInfo?.filename;
const warningSourceData = warning.sourceInfo?.data;
if (!warningSourceFilename || !warningSourceData) {
core.debug(`Buildx.convertWarningsToGitHubAnnotations: skipping warning without source info filename or data`);
continue;
}

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's annotations don't clearly show ranges of lines, so we'll just
// show the first line: https://github.com/orgs/community/discussions/129899
const startLine = warning.range && warning.range.length > 0 ? warning.range[0]?.start.line : undefined;

// TODO: When GitHub's 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;
// }
// }

let annotated = false;
for (const dockerfile of dockerfiles) {
// a valid dockerfile path and content is required to match the warning
// source info or always assume it's valid if this is a remote git
// context as we can't read the content of the Dockerfile in this case.
if (dockerfile.remote || (dockerfile.path.endsWith(warningSourceFilename) && dockerfile.content === warningSourceData)) {
annotations.push({
title: title,
message: message,
file: dockerfile.path,
startLine: startLine
});
annotated = true;
break;
}
}
if (!annotated) {
core.debug(`Buildx.convertWarningsToGitHubAnnotations: skipping warning without matching Dockerfile ${warningSourceFilename}: ${title}`);
}
}

return annotations;
}
}
2 changes: 1 addition & 1 deletion src/buildx/history.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ export class History {
throw new Error('Docker is required to export a build record');
}
if (!(await Docker.isDaemonRunning())) {
throw new Error('Docker daemon is not running, skipping build record export');
throw new Error('Docker daemon needs to be running to export a build record');
}
if (!(await this.buildx.versionSatisfies('>=0.13.0'))) {
throw new Error('Buildx >= 0.13.0 is required to export a build record');
Expand Down
40 changes: 4 additions & 36 deletions src/github.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ 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 @@ -77,6 +76,10 @@ export class GitHub {
return `${github.context.repo.owner}/${github.context.repo.repo}`;
}

static get workspace(): string {
return process.env.GITHUB_WORKSPACE || process.cwd();
}

static get runId(): number {
return process.env.GITHUB_RUN_ID ? +process.env.GITHUB_RUN_ID : github.context.runId;
}
Expand Down Expand Up @@ -342,39 +345,4 @@ 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
});
});
}
}
Loading
Loading