Skip to content

Commit

Permalink
fix(node): Docker generator should work (#23452)
Browse files Browse the repository at this point in the history
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 1b7cf42)
  • Loading branch information
ndcunningham authored and FrozenPandaz committed May 21, 2024
1 parent 31236a1 commit 40aca15
Show file tree
Hide file tree
Showing 5 changed files with 98 additions and 19 deletions.
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import 'nx/src/internal-testing-utils/mock-project-graph';

import {
readNxJson,
readProjectConfiguration,
Expand Down
2 changes: 2 additions & 0 deletions packages/node/src/generators/e2e-project/e2e-project.spec.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
70 changes: 60 additions & 10 deletions packages/node/src/generators/setup-docker/setup-docker.spec.ts
Original file line number Diff line number Diff line change
@@ -1,31 +1,52 @@
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,
projectNameAndRootFormat: 'as-provided',
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}`,
},
})
);
Expand All @@ -34,25 +55,54 @@ 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,
projectNameAndRootFormat: 'as-provided',
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}`],
},
},
};
}),
};
});
};
41 changes: 33 additions & 8 deletions packages/node/src/generators/setup-docker/setup-docker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
GeneratorCallback,
joinPathFragments,
logger,
ProjectConfiguration,
readNxJson,
readProjectConfiguration,
runTasksInSerial,
Expand All @@ -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,
Expand All @@ -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,
});
Expand Down

0 comments on commit 40aca15

Please sign in to comment.