Skip to content

Commit

Permalink
Adjust platform patching
Browse files Browse the repository at this point in the history
  • Loading branch information
@samchungy
samchungy committed Sep 27, 2024
1 parent e9bb2bb commit c806aae
Show file tree
Hide file tree
Showing 2 changed files with 183 additions and 10 deletions.
145 changes: 143 additions & 2 deletions src/cli/lint/internalLints/upgrade/patches/8.2.1/patchDockerImages.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,78 @@ describe('patchDockerImages', () => {
});
});

it('should patch multiple lines in Dockerfile and docker-compose files', async () => {
it('should skip a Dockerfile with multiple platforms', async () => {
jest
.mocked(fg)
.mockResolvedValueOnce(['Dockerfile'])
.mockResolvedValueOnce([]);
jest
.mocked(readFile)
.mockResolvedValueOnce(
('FROM --platform=arm64 public.ecr.aws/docker/library/node:18\n' +
'FROM --platform=amd64 public.ecr.aws/docker/library/node:18\n') as never,
);

await expect(
tryPatchDockerImages({
mode: 'format',
} as PatchConfig),
).resolves.toEqual({
result: 'skip',
reason: 'no Dockerfile or docker-compose files to patch',
});
});

it('should skip a Dockerfile with a build arg platform', async () => {
jest
.mocked(fg)
.mockResolvedValueOnce(['Dockerfile'])
.mockResolvedValueOnce([]);
jest
.mocked(readFile)
.mockResolvedValueOnce(
'FROM --platform=$BUILDPLATFORM public.ecr.aws/docker/library/node:18\n' as never,
);

await expect(
tryPatchDockerImages({
mode: 'format',
} as PatchConfig),
).resolves.toEqual({
result: 'skip',
reason: 'no Dockerfile or docker-compose files to patch',
});
});

it('should patch a Dockerfile with already patched images but invalid platform usage', async () => {
jest
.mocked(fg)
.mockResolvedValueOnce(['Dockerfile'])
.mockResolvedValueOnce([]);
jest
.mocked(readFile)
.mockResolvedValueOnce(
('FROM --platform=arm64 public.ecr.aws/docker/library/node:18\n' +
'FROM --platform=arm64 public.ecr.aws/docker/library/node:18\n') as never,
);

await expect(
tryPatchDockerImages({
mode: 'format',
} as PatchConfig),
).resolves.toEqual({
result: 'apply',
});

expect(writeFile).toHaveBeenNthCalledWith(
1,
'Dockerfile',
'FROM public.ecr.aws/docker/library/node:18\n' +
'FROM public.ecr.aws/docker/library/node:18\n',
);
});

it('should patch multiple lines in Dockerfile and docker-compose files with the same platform', async () => {
jest
.mocked(fg)
.mockResolvedValueOnce(['Dockerfile'])
Expand All @@ -109,7 +180,7 @@ describe('patchDockerImages', () => {
'\n' +
'FROM --platform=arm64 node:20-alpine AS dev-deps\n' +
'FROM --otherflag=bar --platform=arm64 node:20-alpine\n' +
'FROM --otherflag=boo --platform=${BUILDPLATFORM} --anotherflag=coo node:20-alpine\n' +
'FROM --otherflag=boo --platform=arm64 --anotherflag=coo node:20-alpine\n' +
'FROM gcr.io/distroless/nodejs20-debian12 AS runtime\n' +
'FROM --newflag node:latest\n' +
'FROM node:12:@940049cabf21bf4cd20b86641c800c2b9995e4fb85fa4698b1781239fc0f6853') as never,
Expand Down Expand Up @@ -166,4 +237,74 @@ describe('patchDockerImages', () => {
' image: public.ecr.aws/docker/library/python:3.9\n',
);
});

it('should patch multiple lines in Dockerfile and docker-compose files with multiple platforms', async () => {
jest
.mocked(fg)
.mockResolvedValueOnce(['Dockerfile'])
.mockResolvedValueOnce(['docker-compose.yml']);
jest
.mocked(readFile)
.mockResolvedValueOnce(
('# syntax=docker/dockerfile:1.10\n' +
'\n' +
'FROM --platform=amd64 node:20-alpine AS dev-deps\n' +
'FROM --otherflag=bar --platform=arm64 node:20-alpine\n' +
'FROM --otherflag=boo --platform=arm64 --anotherflag=coo node:20-alpine\n' +
'FROM gcr.io/distroless/nodejs20-debian12 AS runtime\n' +
'FROM --newflag node:latest\n' +
'FROM node:12:@940049cabf21bf4cd20b86641c800c2b9995e4fb85fa4698b1781239fc0f6853') as never,
)
.mockResolvedValueOnce(
('services:\n' +
' app:\n' +
' image: node:20-alpine\n' +
' init: true\n' +
' volumes:\n' +
' - ./:/workdir\n' +
' # Mount agent for Buildkite annotations.\n' +
' - /usr/bin/buildkite-agent:/usr/bin/buildkite-agent\n' +
' # Mount cached dependencies.\n' +
' - /workdir/node_modules\n' +
' other:\n' +
' image: python:3.9\n') as never,
);

await expect(
tryPatchDockerImages({
mode: 'format',
} as PatchConfig),
).resolves.toEqual({
result: 'apply',
});

expect(writeFile).toHaveBeenNthCalledWith(
1,
'Dockerfile',
'# syntax=docker/dockerfile:1.10\n' +
'\n' +
'FROM --platform=amd64 public.ecr.aws/docker/library/node:20-alpine AS dev-deps\n' +
'FROM --otherflag=bar --platform=arm64 public.ecr.aws/docker/library/node:20-alpine\n' +
'FROM --otherflag=boo --platform=arm64 --anotherflag=coo public.ecr.aws/docker/library/node:20-alpine\n' +
'FROM gcr.io/distroless/nodejs20-debian12 AS runtime\n' +
'FROM --newflag public.ecr.aws/docker/library/node:latest\n' +
'FROM public.ecr.aws/docker/library/node:12:@940049cabf21bf4cd20b86641c800c2b9995e4fb85fa4698b1781239fc0f6853',
);
expect(writeFile).toHaveBeenNthCalledWith(
2,
'docker-compose.yml',
'services:\n' +
' app:\n' +
' image: public.ecr.aws/docker/library/node:20-alpine\n' +
' init: true\n' +
' volumes:\n' +
' - ./:/workdir\n' +
' # Mount agent for Buildkite annotations.\n' +
' - /usr/bin/buildkite-agent:/usr/bin/buildkite-agent\n' +
' # Mount cached dependencies.\n' +
' - /workdir/node_modules\n' +
' other:\n' +
' image: public.ecr.aws/docker/library/python:3.9\n',
);
});
});
48 changes: 40 additions & 8 deletions src/cli/lint/internalLints/upgrade/patches/8.2.1/patchDockerImages.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,39 @@ const fetchFiles = async (files: string[]) =>
}),
);

const isInvalidPlatformFlagUsage = (contents: string) => {
const matches = [...contents.matchAll(DOCKER_IMAGE_PLATFORM_REGEX)];

if (!matches.length) {
return false;
}

const uniquePlatforms = [
...new Set(matches.map(([, , platform]) => platform as string)),
];

// Multiple --platform flags are used which indicate a multi arch build
if (uniquePlatforms.length > 1) {
return false;
}

// Avoid patching as they may be using args to set the platform
if (uniquePlatforms[0]?.startsWith('--platform=$')) {
return false;
}

return true;
};

const patchDockerImages: PatchFunction = async ({
mode,
}): Promise<PatchReturnType> => {
const [maybeDockerFilesPaths, maybeDockerComposePaths] = await Promise.all([
fg(['Dockerfile*']),
fg(['docker-compose*.yml']),
fg(['docker-compose*.y*ml']),
]);

if (!maybeDockerFilesPaths.length || !maybeDockerComposePaths.length) {
if (!maybeDockerFilesPaths.length && !maybeDockerComposePaths.length) {
return {
result: 'skip',
reason: 'no Dockerfile or docker-compose files found',
Expand All @@ -45,15 +69,14 @@ const patchDockerImages: PatchFunction = async ({

const dockerFilesToPatch = dockerFiles.filter(
({ contents }) =>
DOCKER_IMAGE_REGEX.exec(contents) ??
DOCKER_IMAGE_PLATFORM_REGEX.exec(contents),
DOCKER_IMAGE_REGEX.exec(contents) ?? isInvalidPlatformFlagUsage(contents),
);

const dockerComposeFilesToPatch = dockerComposeFiles.filter(({ contents }) =>
DOCKER_COMPOSE_IMAGE_REGEX.exec(contents),
);

if (!dockerFilesToPatch.length || !dockerComposeFilesToPatch.length) {
if (!dockerFilesToPatch.length && !dockerComposeFilesToPatch.length) {
return {
result: 'skip',
reason: 'no Dockerfile or docker-compose files to patch',
Expand All @@ -68,9 +91,18 @@ const patchDockerImages: PatchFunction = async ({

const dockerFilePatches = dockerFilesToPatch.map(
async ({ file, contents }) => {
const patchedContents = contents
.replace(DOCKER_IMAGE_REGEX, `$1$2${PUBLIC_ECR}$3$4`)
.replace(DOCKER_IMAGE_PLATFORM_REGEX, '$1');
let patchedContents = contents.replace(
DOCKER_IMAGE_REGEX,
`$1$2${PUBLIC_ECR}$3$4`,
);

if (isInvalidPlatformFlagUsage(contents)) {
patchedContents = patchedContents.replace(
DOCKER_IMAGE_PLATFORM_REGEX,
'$1',
);
}

await writeFile(file, patchedContents);
},
);
Expand Down

0 comments on commit c806aae

Please sign in to comment.