Merge pull request #401 from crazy-max/build-warnings-localstate

buildx: convert vertex warnings to github annotations based on localstate

__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);
+    }
+  });
+});

__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);
-  });
-});

__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()}`;

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<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;
+  }
 }

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');

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<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
-      });
-    });
-  }
 }

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;

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}`);
+  }
 }