diff --git a/__tests__/buildx/buildx.test.itg.ts b/__tests__/buildx/buildx.test.itg.ts new file mode 100644 index 00000000..9a251e3c --- /dev/null +++ b/__tests__/buildx/buildx.test.itg.ts @@ -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); + } + }); +}); diff --git a/__tests__/github.test.itg.ts b/__tests__/github.test.itg.ts index 5c3642ea..b720ece4 100644 --- a/__tests__/github.test.itg.ts +++ b/__tests__/github.test.itg.ts @@ -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); - }); -}); diff --git a/__tests__/util.test.ts b/__tests__/util.test.ts index 6889c1be..2a733d94 100644 --- a/__tests__/util.test.ts +++ b/__tests__/util.test.ts @@ -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()}`; diff --git a/src/buildx/buildx.ts b/src/buildx/buildx.ts index f584c87c..a8d16b29 100644 --- a/src/buildx/buildx.ts +++ b/src/buildx/buildx.ts @@ -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; @@ -266,4 +272,151 @@ export class Buildx { return refs; } + + public static async convertWarningsToGitHubAnnotations(warnings: Array, buildRefs: Array, refsDir?: string): Promise | 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 = []; + 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 = []; + 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; + } } diff --git a/src/buildx/history.ts b/src/buildx/history.ts index ae227d89..1f4fec5f 100644 --- a/src/buildx/history.ts +++ b/src/buildx/history.ts @@ -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'); diff --git a/src/github.ts b/src/github.ts index c47eac73..ca3b049a 100644 --- a/src/github.ts +++ b/src/github.ts @@ -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 { @@ -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; } @@ -342,39 +345,4 @@ export class GitHub { core.info(`Writing summary`); await sum.addSeparator().write(); } - - public static async annotateBuildWarnings(source: string, warnings?: Array): Promise { - (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 - }); - }); - } } diff --git a/src/types/github.ts b/src/types/github.ts index e1449178..2970e976 100644 --- a/src/types/github.ts +++ b/src/types/github.ts @@ -14,6 +14,7 @@ * limitations under the License. */ +import {AnnotationProperties} from '@actions/core'; import {components as OctoOpenApiTypes} from '@octokit/openapi-types'; import {JwtPayload} from 'jwt-decode'; @@ -38,6 +39,10 @@ export interface GitHubActionsRuntimeTokenAC { Permission: number; } +export interface GitHubAnnotation extends AnnotationProperties { + message: string; +} + export interface UploadArtifactOpts { filename: string; mimeType?: string; diff --git a/src/util.ts b/src/util.ts index 91fc2f66..2587acac 100644 --- a/src/util.ts +++ b/src/util.ts @@ -16,6 +16,7 @@ import crypto from 'crypto'; import fs from 'fs'; +import path from 'path'; import * as core from '@actions/core'; import * as io from '@actions/io'; import {parse} from 'csv-parse/sync'; @@ -189,4 +190,10 @@ export class Util { public static countLines(input: string): number { return input.split(/\r\n|\r|\n/).length; } + + public static isPathRelativeTo(parentPath: string, childPath: string): boolean { + const rpp = path.resolve(parentPath); + const rcp = path.resolve(childPath); + return rcp.startsWith(rpp.endsWith(path.sep) ? rpp : `${rpp}${path.sep}`); + } }