From 40aca15c52ded17f3f7a0d9f3d941afe8d6dc24c Mon Sep 17 00:00:00 2001 From: Nicholas Cunningham Date: Thu, 16 May 2024 15:15:40 -0600 Subject: [PATCH] fix(node): Docker generator should work (#23452) Also fixes the unit tests for node package ## Currently When you generate a docker file using either the node generator or the node:setup-docker generator using inferred targets would create the DockerFile but the COPY command would be incorrect. It would resemble something similar to ``` //... COPY acme //etc... ``` ## Expected Now it generates the correct command. ``` //... COPY dist/acme acme/ //etc... ``` closes: #23365 (cherry picked from commit 1b7cf426c229e282e7597541ba52f501d75ffa1a) --- .../application/application.legacy.spec.ts | 2 + .../e2e-project/e2e-project.spec.ts | 2 + .../setup-docker/files/Dockerfile__tmpl__ | 2 +- .../setup-docker/setup-docker.spec.ts | 70 ++++++++++++++++--- .../generators/setup-docker/setup-docker.ts | 41 ++++++++--- 5 files changed, 98 insertions(+), 19 deletions(-) diff --git a/packages/node/src/generators/application/application.legacy.spec.ts b/packages/node/src/generators/application/application.legacy.spec.ts index 38616eb0d80f9..9a56a513d9f22 100644 --- a/packages/node/src/generators/application/application.legacy.spec.ts +++ b/packages/node/src/generators/application/application.legacy.spec.ts @@ -1,3 +1,5 @@ +import 'nx/src/internal-testing-utils/mock-project-graph'; + import { readNxJson, readProjectConfiguration, diff --git a/packages/node/src/generators/e2e-project/e2e-project.spec.ts b/packages/node/src/generators/e2e-project/e2e-project.spec.ts index 8b993a68b03b9..b2b142c03a6a5 100644 --- a/packages/node/src/generators/e2e-project/e2e-project.spec.ts +++ b/packages/node/src/generators/e2e-project/e2e-project.spec.ts @@ -1,3 +1,5 @@ +import 'nx/src/internal-testing-utils/mock-project-graph'; + import { Tree } from '@nx/devkit'; import { createTreeWithEmptyWorkspace } from '@nx/devkit/testing'; import { applicationGenerator } from '../application/application'; diff --git a/packages/node/src/generators/setup-docker/files/Dockerfile__tmpl__ b/packages/node/src/generators/setup-docker/files/Dockerfile__tmpl__ index f6752d0547dc5..6203ee33f9f45 100644 --- a/packages/node/src/generators/setup-docker/files/Dockerfile__tmpl__ +++ b/packages/node/src/generators/setup-docker/files/Dockerfile__tmpl__ @@ -14,7 +14,7 @@ WORKDIR /app RUN addgroup --system <%= project %> && \ adduser --system -G <%= project %> <%= project %> -COPY <%= buildLocation %> <%= project %> +COPY <%= buildLocation %> <%= project %>/ RUN chown -R <%= project %>:<%= project %> . # You can remove this install step if you build with `--bundle` option. diff --git a/packages/node/src/generators/setup-docker/setup-docker.spec.ts b/packages/node/src/generators/setup-docker/setup-docker.spec.ts index aa2e8a0c4dde4..8a1d4e4e853ca 100644 --- a/packages/node/src/generators/setup-docker/setup-docker.spec.ts +++ b/packages/node/src/generators/setup-docker/setup-docker.spec.ts @@ -1,16 +1,35 @@ -import { readProjectConfiguration, Tree } from '@nx/devkit'; +import 'nx/src/internal-testing-utils/mock-project-graph'; + +import { + ProjectConfiguration, + readProjectConfiguration, + Tree, +} from '@nx/devkit'; import { createTreeWithEmptyWorkspace } from '@nx/devkit/testing'; -import { applicationGenerator } from '../application/application'; + describe('setupDockerGenerator', () => { let tree: Tree; beforeEach(async () => { tree = createTreeWithEmptyWorkspace(); + + jest.resetModules(); }); describe('integrated', () => { it('should create docker assets when --docker is passed', async () => { + const projectName = 'integreated-api'; + // Since we mock the project graph, we need to mock the project configuration as well + mockReadCachedProjectConfiguration({ + name: projectName, + root: projectName, + }); + + const { applicationGenerator } = await import( + '../application/application' + ); + await applicationGenerator(tree, { - name: 'api', + name: projectName, framework: 'express', e2eTestRunner: 'none', docker: true, @@ -18,14 +37,16 @@ describe('setupDockerGenerator', () => { addPlugin: true, }); - const project = readProjectConfiguration(tree, 'api'); + const project = readProjectConfiguration(tree, projectName); - expect(tree.exists('api/Dockerfile')).toBeTruthy(); + const dockerFile = tree.read(`${projectName}/Dockerfile`, 'utf8'); + expect(tree.exists(`${projectName}/Dockerfile`)).toBeTruthy(); + expect(dockerFile).toContain(`COPY dist/${projectName} ${projectName}/`); expect(project.targets).toEqual( expect.objectContaining({ 'docker-build': { dependsOn: ['build'], - command: 'docker build -f api/Dockerfile . -t api', + command: `docker build -f ${projectName}/Dockerfile . -t ${projectName}`, }, }) ); @@ -34,8 +55,14 @@ describe('setupDockerGenerator', () => { describe('standalone', () => { it('should create docker assets when --docker is passed', async () => { + const projectName = 'standalone-api'; + mockReadCachedProjectConfiguration({ name: projectName, root: '' }); + + const { applicationGenerator } = await import( + '../application/application' + ); await applicationGenerator(tree, { - name: 'api', + name: projectName, framework: 'fastify', rootProject: true, docker: true, @@ -43,16 +70,39 @@ describe('setupDockerGenerator', () => { addPlugin: true, }); - const project = readProjectConfiguration(tree, 'api'); - expect(tree.exists('Dockerfile')).toBeTruthy(); + const project = readProjectConfiguration(tree, projectName); + const dockerFile = tree.read(`Dockerfile`, 'utf8'); + + expect(tree.exists(`Dockerfile`)).toBeTruthy(); + expect(dockerFile).toContain(`COPY dist/${projectName} ${projectName}/`); expect(project.targets).toEqual( expect.objectContaining({ 'docker-build': { dependsOn: ['build'], - command: 'docker build -f Dockerfile . -t api', + command: `docker build -f Dockerfile . -t ${projectName}`, }, }) ); }); }); }); + +const mockReadCachedProjectConfiguration = ( + projectConfig: ProjectConfiguration +) => { + jest.mock('nx/src/project-graph/project-graph', () => { + return { + ...jest.requireActual('nx/src/project-graph/project-graph'), + readCachedProjectConfiguration: jest.fn(() => { + return { + root: projectConfig.root, + targets: { + build: { + outputs: [`dist/${projectConfig.name}`], + }, + }, + }; + }), + }; + }); +}; diff --git a/packages/node/src/generators/setup-docker/setup-docker.ts b/packages/node/src/generators/setup-docker/setup-docker.ts index 5176b71fe9d1d..f0c3277cbfeb3 100644 --- a/packages/node/src/generators/setup-docker/setup-docker.ts +++ b/packages/node/src/generators/setup-docker/setup-docker.ts @@ -4,6 +4,7 @@ import { GeneratorCallback, joinPathFragments, logger, + ProjectConfiguration, readNxJson, readProjectConfiguration, runTasksInSerial, @@ -13,6 +14,8 @@ import { import { SetUpDockerOptions } from './schema'; import { join } from 'path'; +import { interpolate } from 'nx/src/tasks-runner/utils'; +import { readCachedProjectConfiguration } from 'nx/src/project-graph/project-graph'; function normalizeOptions( tree: Tree, @@ -27,21 +30,43 @@ function normalizeOptions( } function addDocker(tree: Tree, options: SetUpDockerOptions) { - const project = readProjectConfiguration(tree, options.project); - if (!project || !options.targetName) { + // Inferred targets are only available in the project graph + const projectConfig = readCachedProjectConfiguration(options.project); + + if ( + !projectConfig || + !projectConfig.targets || + !projectConfig.targets[options.buildTarget] + ) { return; } - if (tree.exists(joinPathFragments(project.root, 'DockerFile'))) { + // Returns an string like {workspaceRoot}/dist/apps/{projectName} + // Non crystalized projects would return {options.outputPath} + const tokenizedOutputPath = + projectConfig.targets[`${options.buildTarget}`]?.outputs?.[0]; + const maybeBuildOptions = + projectConfig.targets[`${options.buildTarget}`]?.options; + + if (tree.exists(joinPathFragments(projectConfig.root, 'DockerFile'))) { logger.info( - `Skipping setup since a Dockerfile already exists inside ${project.root}` + `Skipping setup since a Dockerfile already exists inside ${projectConfig.root}` + ); + } else if (!tokenizedOutputPath) { + logger.error( + `Skipping setup since the output path for the build target ${options.buildTarget} is not defined.` ); } else { - const outputPath = - project.targets[`${options.buildTarget}`]?.options.outputPath; - generateFiles(tree, join(__dirname, './files'), project.root, { + const outputPath = interpolate(tokenizedOutputPath, { + projectName: projectConfig.name, + projectRoot: projectConfig.root, + workspaceRoot: '', + options: maybeBuildOptions || '', + }); + + generateFiles(tree, join(__dirname, './files'), projectConfig.root, { tmpl: '', - app: project.sourceRoot, + app: projectConfig.sourceRoot, buildLocation: outputPath, project: options.project, });