Skip to content

Commit

Permalink
Fix .npmrc handling (#1693)
Browse files Browse the repository at this point in the history
  • Loading branch information
AaronMoat committed Sep 30, 2024
1 parent 8efb513 commit a60d919
Show file tree
Hide file tree
Showing 12 changed files with 389 additions and 13 deletions.
5 changes: 5 additions & 0 deletions .changeset/pink-seas-stare.md
Original file line number Diff line number Diff line change
@@ -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.
4 changes: 2 additions & 2 deletions docs/deep-dives/pnpm.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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`
Expand Down
8 changes: 8 additions & 0 deletions src/cli/__snapshots__/format.int.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 },
);

Expand Down
5 changes: 5 additions & 0 deletions src/cli/lint/internalLints/upgrade/patches/8.2.1/index.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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',
},
];
Original file line number Diff line number Diff line change
@@ -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,
});
});
});
});
Loading

0 comments on commit a60d919

Please sign in to comment.