Patch Pnpm Dockerfile (#1714)

Co-authored-by: Aaron Moat <[email protected]>
seek-oss · Oct 19, 2024 · 91af3b6 · 91af3b6
1 parent 498139c
commit 91af3b6
Show file tree

Hide file tree

Showing 11 changed files with 401 additions and 7 deletions.
diff --git a/.changeset/strange-cameras-brake.md b/.changeset/strange-cameras-brake.md
@@ -0,0 +1,7 @@
+---
+'skuba': minor
+---
+
+lint, format, template: Use pinned `pnpm` version in Dockerfiles
+
+This fixes an issue where `pnpm` commands in Dockerfiles incorrectly use the latest pnpm version instead of the pinned version.
diff --git a/docs/deep-dives/pnpm.md b/docs/deep-dives/pnpm.md
@@ -211,9 +211,10 @@ This migration guide assumes that your project was scaffolded with a **skuba** t
       FROM --platform=arm64 node:20-alpine AS dev-deps
 
     + RUN --mount=type=bind,source=package.json,target=package.json \
-    + corepack enable pnpm && corepack install
+    +     corepack enable pnpm && corepack install
 
-    + RUN pnpm config set store-dir /root/.pnpm-store
+    + RUN --mount=type=bind,source=package.json,target=package.json \
+    +     pnpm config set store-dir /root/.pnpm-store
 
       WORKDIR /workdir
 
@@ -223,6 +224,7 @@ This migration guide assumes that your project was scaffolded with a **skuba** t
     - RUN --mount=type=secret,id=npm,dst=/workdir/.npmrc \
     -     yarn install --frozen-lockfile --ignore-optional --non-interactive
     + RUN --mount=type=bind,source=.npmrc,target=.npmrc \
+    +     --mount=type=bind,source=package.json,target=package.json \
     +     --mount=type=bind,source=pnpm-lock.yaml,target=pnpm-lock.yaml \
     +     --mount=type=secret,id=npm,dst=/root/.npmrc,required=true \
     +     pnpm fetch

diff --git a/src/cli/__snapshots__/format.int.test.ts.snap b/src/cli/__snapshots__/format.int.test.ts.snap
@@ -29,6 +29,8 @@ Patch skipped: Update docker image references to use public.ecr.aws and remove -
 
 Patch skipped: Move .npmrc mounts from tmp/.npmrc to /tmp/.npmrc - no Buildkite files found
 
+Patch skipped: Use pinned pnpm version in Dockerfiles - no Dockerfiles found
+
 skuba update complete.
 
 Refreshed .gitignore. refresh-config-files
@@ -113,6 +115,8 @@ Patch skipped: Update docker image references to use public.ecr.aws and remove -
 
 Patch skipped: Move .npmrc mounts from tmp/.npmrc to /tmp/.npmrc - no Buildkite files found
 
+Patch skipped: Use pinned pnpm version in Dockerfiles - no Dockerfiles found
+
 skuba update complete.
 
 Refreshed .gitignore. refresh-config-files
@@ -192,6 +196,8 @@ Patch skipped: Update docker image references to use public.ecr.aws and remove -
 
 Patch skipped: Move .npmrc mounts from tmp/.npmrc to /tmp/.npmrc - no Buildkite files found
 
+Patch skipped: Use pinned pnpm version in Dockerfiles - no Dockerfiles found
+
 skuba update complete.
 
 Refreshed .gitignore. refresh-config-files
@@ -242,6 +248,8 @@ Patch skipped: Update docker image references to use public.ecr.aws and remove -
 
 Patch skipped: Move .npmrc mounts from tmp/.npmrc to /tmp/.npmrc - no Buildkite files found
 
+Patch skipped: Use pinned pnpm version in Dockerfiles - no Dockerfiles found
+
 skuba update complete.
 
 Refreshed .gitignore. refresh-config-files

diff --git a/src/cli/lint/internalLints/upgrade/patches/9.0.1/index.ts b/src/cli/lint/internalLints/upgrade/patches/9.0.1/index.ts
@@ -0,0 +1,10 @@
+import type { Patches } from '../..';
+
+import { tryPatchPnpmDockerImages } from './patchPnpmDockerImages';
+
+export const patches: Patches = [
+  {
+    apply: tryPatchPnpmDockerImages,
+    description: 'Use pinned pnpm version in Dockerfiles',
+  },
+];
diff --git a/src/cli/lint/internalLints/upgrade/patches/9.0.1/patchPnpmDockerImages.test.ts b/src/cli/lint/internalLints/upgrade/patches/9.0.1/patchPnpmDockerImages.test.ts
@@ -0,0 +1,274 @@
+import fg from 'fast-glob';
+import { readFile, writeFile } from 'fs-extra';
+
+import type { PatchConfig } from '../..';
+
+import { tryPatchPnpmDockerImages } from './patchPnpmDockerImages';
+
+jest.mock('fast-glob');
+jest.mock('fs-extra');
+
+describe('patchPnpmDockerImages', () => {
+  afterEach(() => jest.resetAllMocks());
+
+  it('should skip if no Dockerfiles found', async () => {
+    jest.mocked(fg).mockResolvedValueOnce([]);
+    await expect(
+      tryPatchPnpmDockerImages({
+        mode: 'format',
+      } as PatchConfig),
+    ).resolves.toEqual({
+      result: 'skip',
+      reason: 'no Dockerfiles found',
+    });
+  });
+
+  it('should skip if no Dockerfiles to patch', async () => {
+    jest.mocked(fg).mockResolvedValueOnce(['Dockerfile']);
+    jest.mocked(readFile).mockResolvedValueOnce(`beep` as never);
+
+    await expect(
+      tryPatchPnpmDockerImages({
+        mode: 'format',
+      } as PatchConfig),
+    ).resolves.toEqual({
+      result: 'skip',
+      reason: 'no Dockerfiles to patch',
+    });
+  });
+
+  it('should return apply and not modify files if mode is lint', async () => {
+    jest.mocked(fg).mockResolvedValueOnce(['Dockerfile']);
+    jest
+      .mocked(readFile)
+      .mockResolvedValueOnce(
+        `RUN pnpm config set store-dir /root/.pnpm-store` as never,
+      );
+
+    await expect(
+      tryPatchPnpmDockerImages({
+        mode: 'lint',
+      } as PatchConfig),
+    ).resolves.toEqual({
+      result: 'apply',
+    });
+
+    expect(writeFile).not.toHaveBeenCalled();
+  });
+
+  it('should patch Dockerfiles', async () => {
+    jest.mocked(fg).mockResolvedValueOnce(['Dockerfile']);
+    jest.mocked(readFile).mockResolvedValueOnce(
+      `# syntax=docker/dockerfile:1.10
+
+FROM public.ecr.aws/docker/library/node:20-alpine AS dev-deps
+
+RUN --mount=type=bind,source=package.json,target=package.json \\
+    corepack enable pnpm && corepack install
+
+RUN pnpm config set store-dir /root/.pnpm-store
+
+WORKDIR /workdir
+
+RUN --mount=type=bind,source=.npmrc,target=.npmrc \\
+    --mount=type=bind,source=pnpm-lock.yaml,target=pnpm-lock.yaml \\
+    --mount=type=secret,id=npm,dst=/root/.npmrc,required=true \\
+    pnpm fetch
+` as never,
+    );
+
+    await expect(
+      tryPatchPnpmDockerImages({
+        mode: 'format',
+      } as PatchConfig),
+    ).resolves.toEqual({
+      result: 'apply',
+    });
+
+    expect(writeFile).toHaveBeenNthCalledWith(
+      1,
+      'Dockerfile',
+      `# syntax=docker/dockerfile:1.10
+
+FROM public.ecr.aws/docker/library/node:20-alpine AS dev-deps
+
+RUN --mount=type=bind,source=package.json,target=package.json \\
+    corepack enable pnpm && corepack install
+
+RUN --mount=type=bind,source=package.json,target=package.json \\
+    pnpm config set store-dir /root/.pnpm-store
+
+WORKDIR /workdir
+
+RUN --mount=type=bind,source=.npmrc,target=.npmrc \\
+    --mount=type=bind,source=package.json,target=package.json \\
+    --mount=type=bind,source=pnpm-lock.yaml,target=pnpm-lock.yaml \\
+    --mount=type=secret,id=npm,dst=/root/.npmrc,required=true \\
+    pnpm fetch
+`,
+    );
+  });
+
+  it('should patch Dockerfiles with extra mounts', async () => {
+    jest.mocked(fg).mockResolvedValueOnce(['Dockerfile']);
+    jest.mocked(readFile).mockResolvedValueOnce(
+      `RUN --mount=type=bind,source=.npmrc,target=.npmrc \\
+    --mount=type=bind,source=patches,target=patches \\
+    --mount=type=bind,source=pnpm-lock.yaml,target=pnpm-lock.yaml \\
+    --mount=type=secret,id=npm,dst=/root/.npmrc,required=true \\
+    pnpm fetch
+` as never,
+    );
+
+    await expect(
+      tryPatchPnpmDockerImages({
+        mode: 'format',
+      } as PatchConfig),
+    ).resolves.toEqual({
+      result: 'apply',
+    });
+
+    expect(writeFile).toHaveBeenNthCalledWith(
+      1,
+      'Dockerfile',
+      `RUN --mount=type=bind,source=.npmrc,target=.npmrc \\
+    --mount=type=bind,source=package.json,target=package.json \\
+    --mount=type=bind,source=patches,target=patches \\
+    --mount=type=bind,source=pnpm-lock.yaml,target=pnpm-lock.yaml \\
+    --mount=type=secret,id=npm,dst=/root/.npmrc,required=true \\
+    pnpm fetch
+`,
+    );
+  });
+
+  it('should fix Dockerfiles with only the config store line to fix', async () => {
+    jest.mocked(fg).mockResolvedValueOnce(['Dockerfile']);
+    jest.mocked(readFile).mockResolvedValueOnce(
+      `# syntax=docker/dockerfile:1.10
+
+FROM public.ecr.aws/docker/library/node:20-alpine AS dev-deps
+
+RUN --mount=type=bind,source=package.json,target=package.json \\
+    corepack enable pnpm && corepack install
+
+RUN pnpm config set store-dir /root/.pnpm-store
+` as never,
+    );
+
+    await expect(
+      tryPatchPnpmDockerImages({
+        mode: 'format',
+      } as PatchConfig),
+    ).resolves.toEqual({
+      result: 'apply',
+    });
+
+    expect(writeFile).toHaveBeenNthCalledWith(
+      1,
+      'Dockerfile',
+      `# syntax=docker/dockerfile:1.10
+
+FROM public.ecr.aws/docker/library/node:20-alpine AS dev-deps
+
+RUN --mount=type=bind,source=package.json,target=package.json \\
+    corepack enable pnpm && corepack install
+
+RUN --mount=type=bind,source=package.json,target=package.json \\
+    pnpm config set store-dir /root/.pnpm-store
+`,
+    );
+  });
+
+  it('should fix Dockerfiles with only pnpm fetch to fix', async () => {
+    jest.mocked(fg).mockResolvedValueOnce(['Dockerfile']);
+    jest.mocked(readFile).mockResolvedValueOnce(
+      `RUN --mount=type=bind,source=.npmrc,target=.npmrc \\
+    --mount=type=bind,source=pnpm-lock.yaml,target=pnpm-lock.yaml \\
+    --mount=type=secret,id=npm,dst=/root/.npmrc,required=true \\
+    pnpm fetch
+` as never,
+    );
+
+    await expect(
+      tryPatchPnpmDockerImages({
+        mode: 'format',
+      } as PatchConfig),
+    ).resolves.toEqual({
+      result: 'apply',
+    });
+
+    expect(writeFile).toHaveBeenNthCalledWith(
+      1,
+      'Dockerfile',
+      `RUN --mount=type=bind,source=.npmrc,target=.npmrc \\
+    --mount=type=bind,source=package.json,target=package.json \\
+    --mount=type=bind,source=pnpm-lock.yaml,target=pnpm-lock.yaml \\
+    --mount=type=secret,id=npm,dst=/root/.npmrc,required=true \\
+    pnpm fetch
+`,
+    );
+  });
+
+  it('should fix Dockerfiles with an alternative pnpm install syntax', async () => {
+    jest.mocked(fg).mockResolvedValueOnce(['Dockerfile']);
+    jest.mocked(readFile).mockResolvedValueOnce(
+      `RUN --mount=type=bind,source=.npmrc,target=.npmrc \\
+    --mount=type=bind,source=pnpm-lock.yaml,target=pnpm-lock.yaml \\
+    --mount=type=secret,id=npm,dst=/root/.npmrc,required=true \\
+    pnpm install
+` as never,
+    );
+
+    await expect(
+      tryPatchPnpmDockerImages({
+        mode: 'format',
+      } as PatchConfig),
+    ).resolves.toEqual({
+      result: 'apply',
+    });
+
+    expect(writeFile).toHaveBeenNthCalledWith(
+      1,
+      'Dockerfile',
+      `RUN --mount=type=bind,source=.npmrc,target=.npmrc \\
+    --mount=type=bind,source=package.json,target=package.json \\
+    --mount=type=bind,source=pnpm-lock.yaml,target=pnpm-lock.yaml \\
+    --mount=type=secret,id=npm,dst=/root/.npmrc,required=true \\
+    pnpm install
+`,
+    );
+  });
+
+  it('should not try to patch already patched Dockerfiles', async () => {
+    jest.mocked(fg).mockResolvedValueOnce(['Dockerfile']);
+    jest.mocked(readFile).mockResolvedValueOnce(
+      `# syntax=docker/dockerfile:1.10
+
+FROM public.ecr.aws/docker/library/node:20-alpine AS dev-deps
+
+RUN --mount=type=bind,source=package.json,target=package.json \\
+    corepack enable pnpm && corepack install
+
+RUN --mount=type=bind,source=package.json,target=package.json \\
+    pnpm config set store-dir /root/.pnpm-store
+
+WORKDIR /workdir
+
+RUN --mount=type=bind,source=.npmrc,target=.npmrc \\
+    --mount=type=bind,source=package.json,target=package.json \\
+    --mount=type=bind,source=pnpm-lock.yaml,target=pnpm-lock.yaml \\
+    --mount=type=secret,id=npm,dst=/root/.npmrc,required=true \\
+    pnpm fetch
+` as never,
+    );
+
+    await expect(
+      tryPatchPnpmDockerImages({
+        mode: 'format',
+      } as PatchConfig),
+    ).resolves.toEqual({
+      result: 'skip',
+      reason: 'no Dockerfiles to patch',
+    });
+  });
+});