Fix .npmrc handling

.changeset/pink-seas-stare.md

@@ -0,0 +1,5 @@
+---
+'skuba': minor
+---
+
+template, format: Mount `.npmrc` files in `/tmp/` rather than `<workdir>/tmp/`, to avoid accidental inclusion in commits or published artifacts, as per the original intent of this handling.

docs/deep-dives/pnpm.md

@@ -287,7 +287,7 @@ This migration guide assumes that your project was scaffolded with a **skuba** t
     ```diff
       seek-oss/private-npm#v1.2.0:
         env: NPM_READ_TOKEN
-    +   output-path: tmp/
+    +   output-path: /tmp/
     ```
 
     ```diff
@@ -301,7 +301,7 @@ This migration guide assumes that your project was scaffolded with a **skuba** t
     +     - pnpm-lock.yaml
         dockerfile: Dockerfile.dev-deps
     -   secrets: id=npm,src=.npmrc
-    +   secrets: id=npm,src=tmp/.npmrc
+    +   secrets: id=npm,src=/tmp/.npmrc
     ```
 
 15. Run `pnpm install --offline` and replace `yarn` with `pnpm` in `.buildkite/pipeline.yml`

src/cli/__snapshots__/format.int.test.ts.snap

@@ -27,6 +27,8 @@ Patch skipped: Remove version field from docker-compose files - no docker-compos
 
 Patch skipped: Update docker image references to use public.ecr.aws and remove --platform flag - no Dockerfile or docker-compose files found
 
+Patch skipped: Move .npmrc mounts from tmp/.npmrc to /tmp/.npmrc - no Buildkite files found
+
 skuba update complete.
 
 Refreshed .gitignore. refresh-config-files
@@ -109,6 +111,8 @@ Patch skipped: Remove version field from docker-compose files - no docker-compos
 
 Patch skipped: Update docker image references to use public.ecr.aws and remove --platform flag - no Dockerfile or docker-compose files found
 
+Patch skipped: Move .npmrc mounts from tmp/.npmrc to /tmp/.npmrc - no Buildkite files found
+
 skuba update complete.
 
 Refreshed .gitignore. refresh-config-files
@@ -186,6 +190,8 @@ Patch skipped: Remove version field from docker-compose files - no docker-compos
 
 Patch skipped: Update docker image references to use public.ecr.aws and remove --platform flag - no Dockerfile or docker-compose files found
 
+Patch skipped: Move .npmrc mounts from tmp/.npmrc to /tmp/.npmrc - no Buildkite files found
+
 skuba update complete.
 
 Refreshed .gitignore. refresh-config-files
@@ -234,6 +240,8 @@ Patch skipped: Remove version field from docker-compose files - no docker-compos
 
 Patch skipped: Update docker image references to use public.ecr.aws and remove --platform flag - no Dockerfile or docker-compose files found
 
+Patch skipped: Move .npmrc mounts from tmp/.npmrc to /tmp/.npmrc - no Buildkite files found
+
 skuba update complete.
 
 Refreshed .gitignore. refresh-config-files

src/cli/lint/internalLints/upgrade/patches/8.2.1/collapseDuplicateMergeKeys.ts

@@ -10,7 +10,7 @@ const collapseDuplicateMergeKeys: PatchFunction = async ({
   mode,
 }): Promise<PatchReturnType> => {
   const buildkiteFiles = await glob(
-    ['.buildkite/**/*.yml', '.buildkite/**/*.yaml'],
+    ['{apps/*/,packages/*/,./}.buildkite/**/*.y*ml'],
     { onlyFiles: true },
   );
 

src/cli/lint/internalLints/upgrade/patches/8.2.1/index.ts

@@ -1,6 +1,7 @@
 import type { Patches } from '../..';
 
 import { tryCollapseDuplicateMergeKeys } from './collapseDuplicateMergeKeys';
+import { tryMoveNpmrcMounts } from './moveNpmrcMounts';
 import { tryPatchDockerComposeFiles } from './patchDockerCompose';
 import { tryPatchDockerImages } from './patchDockerImages';
 import { tryUpgradeESLint } from './upgradeESLint';
@@ -23,4 +24,8 @@ export const patches: Patches = [
     description:
       'Update docker image references to use public.ecr.aws and remove --platform flag',
   },
+  {
+    apply: tryMoveNpmrcMounts,
+    description: 'Move .npmrc mounts from tmp/.npmrc to /tmp/.npmrc',
+  },
 ];

src/cli/lint/internalLints/upgrade/patches/8.2.1/moveNpmrcMounts.test.ts

@@ -0,0 +1,289 @@
+// eslint-disable-next-line no-restricted-imports -- fs-extra is mocked
+import fsp from 'fs/promises';
+
+import memfs, { vol } from 'memfs';
+
+import type { PatchConfig } from '../..';
+import { configForPackageManager } from '../../../../../../utils/packageManager';
+
+import { tryMoveNpmrcMounts } from './moveNpmrcMounts';
+
+const volToJson = () => vol.toJSON(process.cwd(), undefined, true);
+
+jest.mock('fs-extra', () => memfs);
+jest.mock('fast-glob', () => ({
+  glob: (pat: any, opts: any) =>
+    jest.requireActual('fast-glob').glob(pat, { ...opts, fs: memfs }),
+}));
+
+beforeEach(() => vol.reset());
+
+describe('moveNpmrcMounts', () => {
+  const baseArgs = {
+    manifest: {} as PatchConfig['manifest'],
+    packageManager: configForPackageManager('pnpm'),
+  };
+
+  afterEach(() => jest.resetAllMocks());
+
+  describe.each(['lint', 'format'] as const)('%s', (mode) => {
+    it('should not need to modify any of the template pipelines', async () => {
+      for (const template of await fsp.readdir('template')) {
+        const pipelineFile = `template/${template}/.buildkite/pipeline.yml`;
+        try {
+          await fsp.stat(pipelineFile);
+        } catch {
+          continue;
+        }
+
+        const contents = await fsp.readFile(pipelineFile, 'utf-8');
+
+        vol.fromJSON({
+          '.buildkite/pipeline.yml': contents,
+        });
+
+        await expect(
+          tryMoveNpmrcMounts({
+            ...baseArgs,
+            mode,
+          }),
+        ).resolves.toEqual({
+          result: 'skip',
+          reason: 'no .npmrc mounts found need to be updated',
+        });
+
+        expect(volToJson()).toEqual({
+          '.buildkite/pipeline.yml': contents,
+        });
+      }
+    });
+
+    it('should skip if no Buildkite files are found', async () => {
+      await expect(
+        tryMoveNpmrcMounts({
+          ...baseArgs,
+          mode,
+        }),
+      ).resolves.toEqual({
+        result: 'skip',
+        reason: 'no Buildkite files found',
+      });
+
+      expect(volToJson()).toEqual({});
+    });
+
+    it('should skip on a pipeline without mounts', async () => {
+      const input = `steps:
+  - label: 'My Step'
+    command: echo 'Hello, world!'
+`;
+
+      vol.fromJSON({
+        '.buildkite/pipeline.yml': input,
+      });
+
+      await expect(
+        tryMoveNpmrcMounts({
+          ...baseArgs,
+          mode,
+        }),
+      ).resolves.toEqual({
+        result: 'skip',
+        reason: 'no .npmrc mounts found need to be updated',
+      });
+
+      expect(volToJson()).toEqual({
+        '.buildkite/pipeline.yml': input,
+      });
+    });
+
+    it('should fix an incorrect mount', async () => {
+      const input = `configs:
+  plugins:
+    - &docker-ecr-cache
+      seek-oss/docker-ecr-cache#v2.2.0:
+        cache-on:
+          - .npmrc
+          - package.json#.packageManager
+          - pnpm-lock.yaml
+        dockerfile: Dockerfile.dev-deps
+        ecr-name: build-cache/my-service
+        secrets: id=npm,src=tmp/.npmrc
+
+    - &private-npm
+      seek-oss/private-npm#v1.2.0:
+        env: NPM_READ_TOKEN
+        output-path: tmp/
+
+steps: []`;
+
+      vol.fromJSON({
+        '.buildkite/pipeline.yml': input,
+      });
+
+      await expect(
+        tryMoveNpmrcMounts({
+          ...baseArgs,
+          mode,
+        }),
+      ).resolves.toEqual({ result: 'apply' });
+
+      expect(volToJson()).toEqual({
+        '.buildkite/pipeline.yml':
+          mode === 'lint'
+            ? input
+            : `configs:
+  plugins:
+    - &docker-ecr-cache
+      seek-oss/docker-ecr-cache#v2.2.0:
+        cache-on:
+          - .npmrc
+          - package.json#.packageManager
+          - pnpm-lock.yaml
+        dockerfile: Dockerfile.dev-deps
+        ecr-name: build-cache/my-service
+        secrets: id=npm,src=/tmp/.npmrc
+
+    - &private-npm
+      seek-oss/private-npm#v1.2.0:
+        env: NPM_READ_TOKEN
+        output-path: /tmp/
+
+steps: []`,
+      });
+    });
+
+    it('should fix an incorrect mount with comments', async () => {
+      const input = `configs:
+  plugins:
+    - &docker-ecr-cache
+      seek-oss/docker-ecr-cache#v2.2.0:
+        cache-on:
+          - .npmrc
+          - package.json#.packageManager
+          - pnpm-lock.yaml
+        dockerfile: Dockerfile.dev-deps
+        ecr-name: build-cache/my-service
+        secrets: id=npm,src=tmp/.npmrc     # hello
+
+    - &private-npm
+      seek-oss/private-npm#v1.2.0:
+        env: NPM_READ_TOKEN
+        output-path: tmp/#world
+
+steps: []`;
+
+      vol.fromJSON({
+        '.buildkite/pipeline.yml': input,
+      });
+
+      await expect(
+        tryMoveNpmrcMounts({
+          ...baseArgs,
+          mode,
+        }),
+      ).resolves.toEqual({ result: 'apply' });
+
+      expect(volToJson()).toEqual({
+        '.buildkite/pipeline.yml':
+          mode === 'lint'
+            ? input
+            : `configs:
+  plugins:
+    - &docker-ecr-cache
+      seek-oss/docker-ecr-cache#v2.2.0:
+        cache-on:
+          - .npmrc
+          - package.json#.packageManager
+          - pnpm-lock.yaml
+        dockerfile: Dockerfile.dev-deps
+        ecr-name: build-cache/my-service
+        secrets: id=npm,src=/tmp/.npmrc     # hello
+
+    - &private-npm
+      seek-oss/private-npm#v1.2.0:
+        env: NPM_READ_TOKEN
+        output-path: /tmp/#world
+
+steps: []`,
+      });
+    });
+
+    it('should skip a mount without tmp', async () => {
+      const input = `configs:
+  plugins:
+    - &docker-ecr-cache
+      seek-oss/docker-ecr-cache#v2.2.0:
+        cache-on:
+          - .npmrc
+          - package.json#.packageManager
+          - pnpm-lock.yaml
+        dockerfile: Dockerfile.dev-deps
+        ecr-name: build-cache/my-service
+        secrets: id=npm,src=.npmrc
+
+    - &private-npm
+      seek-oss/private-npm#v1.2.0:
+        env: NPM_READ_TOKEN
+
+steps: []`;
+
+      vol.fromJSON({
+        '.buildkite/pipeline.yml': input,
+      });
+
+      await expect(
+        tryMoveNpmrcMounts({
+          ...baseArgs,
+          mode,
+        }),
+      ).resolves.toEqual({
+        result: 'skip',
+        reason: 'no .npmrc mounts found need to be updated',
+      });
+
+      expect(volToJson()).toEqual({
+        '.buildkite/pipeline.yml': input,
+      });
+    });
+
+    it('should skip a /tmp/ mount', async () => {
+      const input = `configs:
+  plugins:
+    - &docker-ecr-cache
+      seek-oss/docker-ecr-cache#v2.2.0:
+        cache-on:
+          - .npmrc
+          - package.json#.packageManager
+          - pnpm-lock.yaml
+        dockerfile: Dockerfile.dev-deps
+        ecr-name: build-cache/my-service
+        secrets: id=npm,src=/tmp/.npmrc
+
+    - &private-npm
+      seek-oss/private-npm#v1.2.0:
+        env: NPM_READ_TOKEN
+        output-path: /tmp/
+
+steps: []`;
+
+      vol.fromJSON({
+        'packages/stuff/.buildkite/pipeline.yaml': input,
+      });
+
+      await expect(
+        tryMoveNpmrcMounts({
+          ...baseArgs,
+          mode,
+        }),
+      ).resolves.toEqual({
+        result: 'skip',
+        reason: 'no .npmrc mounts found need to be updated',
+      });
+
+      expect(volToJson()).toEqual({
+        'packages/stuff/.buildkite/pipeline.yaml': input,
+      });
+    });
+  });
+});