Skip to content

Commit

Permalink
Use recommended EXEC form of commands (#2172)
Browse files Browse the repository at this point in the history
  • Loading branch information
@bwateratmsft
bwateratmsft authored Jul 17, 2020
1 parent 9eb7fbb commit c8cf547
Show file tree
Hide file tree
Showing 7 changed files with 54 additions and 39 deletions.
28 changes: 13 additions & 15 deletions src/configureWorkspace/configure.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,11 @@ import { ConfigureTelemetryProperties, genCommonDockerIgnoreFile, getSubfolderDe
import { openFilesIfRequired, registerScaffolder, scaffold, Scaffolder, ScaffolderContext, ScaffoldFile } from './scaffolding';

export interface PackageInfo {
npmStart: boolean; // has npm start
cmd: string;
fullCommand: string; // full command
cmd: string | string[];
author: string;
version: string;
artifactName: string;
main?: string;
}

interface JsonPackageContents {
Expand Down Expand Up @@ -155,12 +154,11 @@ async function getPackageJson(folderPath: string): Promise<vscode.Uri[]> {

function getDefaultPackageInfo(): PackageInfo {
return {
npmStart: true,
fullCommand: 'npm start',
cmd: 'npm start',
cmd: ['npm', 'start'],
author: 'author',
version: '0.0.1',
artifactName: ''
artifactName: '',
main: 'index.js',
};
}

Expand All @@ -175,15 +173,15 @@ async function readPackageJson(folderPath: string): Promise<{ packagePath?: stri
const json = <JsonPackageContents>JSON.parse(fse.readFileSync(packagePath, 'utf8'));

if (json.scripts && typeof json.scripts.start === "string") {
packageInfo.npmStart = true;
packageInfo.fullCommand = json.scripts.start;
packageInfo.cmd = 'npm start';
packageInfo.cmd = ['npm', 'start'];

const matches = /node (.+)/i.exec(json.scripts.start);
if (matches?.[1]) {
packageInfo.main = matches[1];
}
} else if (typeof json.main === "string") {
packageInfo.npmStart = false;
packageInfo.fullCommand = 'node' + ' ' + json.main;
packageInfo.cmd = packageInfo.fullCommand;
} else {
packageInfo.fullCommand = '';
packageInfo.cmd = ['node', json.main];
packageInfo.main = json.main;
}

if (typeof json.author === "string") {
Expand Down
2 changes: 1 addition & 1 deletion src/configureWorkspace/configureGo.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ services:
${getComposePorts(ports)}`;
}

function genDockerComposeDebug(serviceNameAndRelativePath: string, platform: string, os: string | undefined, ports: number[], { fullCommand: cmd }: Partial<PackageInfo>): string {
function genDockerComposeDebug(serviceNameAndRelativePath: string, platform: string, os: string | undefined, ports: number[], _: Partial<PackageInfo>): string {
return `version: '3.4'
services:
Expand Down
2 changes: 1 addition & 1 deletion src/configureWorkspace/configureJava.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ services:
${getComposePorts(ports)}`;
}

function genDockerComposeDebug(serviceNameAndRelativePath: string, platform: string, os: string | undefined, ports: number[], { fullCommand: cmd }: Partial<PackageInfo>): string {
function genDockerComposeDebug(serviceNameAndRelativePath: string, platform: string, os: string | undefined, ports: number[], _: Partial<PackageInfo>): string {
return `version: '3.4'
services:
Expand Down
35 changes: 26 additions & 9 deletions src/configureWorkspace/configureNode.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,24 @@ export let configureNode: IPlatformGeneratorInfo = {
initializeForDebugging,
};

function genDockerFile(serviceNameAndRelativePath: string, platform: string, os: string | undefined, ports: number[], { cmd, author, version, artifactName }: Partial<PackageInfo>): string {
function genDockerFile(serviceNameAndRelativePath: string, platform: string, os: string | undefined, ports: number[], { cmd }: Partial<PackageInfo>): string {
let exposeStatements = getExposeStatements(ports);

let cmdDirective: string;
if (Array.isArray(cmd)) {
cmdDirective = `CMD ${toCMDArray(cmd)}`;
} else {
cmdDirective = `CMD ${cmd}`;
}

return `FROM node:12.18-alpine
ENV NODE_ENV production
WORKDIR /usr/src/app
COPY ["package.json", "package-lock.json*", "npm-shrinkwrap.json*", "./"]
RUN npm install --production --silent && mv node_modules ../
COPY . .
${exposeStatements}
CMD ${cmd}`;
${cmdDirective}`;
}

function genDockerCompose(serviceNameAndRelativePath: string, platform: string, os: string | undefined, ports: number[]): string {
Expand All @@ -43,14 +50,14 @@ services:
${getComposePorts(ports)}`;
}

function genDockerComposeDebug(serviceNameAndRelativePath: string, platform: string, os: string | undefined, ports: number[], { fullCommand: cmd }: Partial<PackageInfo>): string {
function genDockerComposeDebug(serviceNameAndRelativePath: string, platform: string, os: string | undefined, ports: number[], { cmd, main }: Partial<PackageInfo>): string {
const inspectConfig = '--inspect=0.0.0.0:9229';
const cmdArray: string[] = cmd.split(' ');
if (cmdArray[0].toLowerCase() === 'node') {
cmdArray.splice(1, 0, inspectConfig);
cmd = `command: ${cmdArray.join(' ')}`;

let cmdDirective: string;
if (main) {
cmdDirective = `command: ${toCMDArray(['node', inspectConfig, main])}`;
} else {
cmd = `## set your startup file here\n command: node ${inspectConfig} index.js`;
cmdDirective = `## set your startup file here\n command: ["node", "${inspectConfig}", "index.js"]`;
}

return `version: '3.4'
Expand All @@ -62,7 +69,7 @@ services:
environment:
NODE_ENV: development
${getComposePorts(ports, 9229)}
${cmd}`;
${cmdDirective}`;
}

async function initializeForDebugging(context: IActionContext, folder: WorkspaceFolder, platformOS: PlatformOS, dockerfile: string, packageInfo: PackageInfo): Promise<void> {
Expand All @@ -75,3 +82,13 @@ async function initializeForDebugging(context: IActionContext, folder: Workspace

await dockerDebugScaffoldingProvider.initializeNodeForDebugging(scaffoldContext);
}

function toCMDArray(cmdArray: string[]): string {
return `[${cmdArray.map(part => {
if (part.startsWith('"') && part.endsWith('"')) {
return part;
}
return `"${part}"`;
}).join(', ')}]`;
}
4 changes: 2 additions & 2 deletions src/configureWorkspace/configureOther.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ function genDockerFile(serviceNameAndRelativePath: string, platform: string, os:
return `FROM docker/whalesay:latest
LABEL Name=${serviceNameAndRelativePath} Version=${version}
RUN apt-get -y update && apt-get install -y fortunes
CMD /usr/games/fortune -a | cowsay
CMD ["sh", "-c", "/usr/games/fortune -a | cowsay"]
`;
}

Expand All @@ -31,7 +31,7 @@ services:
${getComposePorts(ports)}`;
}

function genDockerComposeDebug(serviceNameAndRelativePath: string, platform: string, os: string | undefined, ports: number[], { fullCommand: cmd }: Partial<PackageInfo>): string {
function genDockerComposeDebug(serviceNameAndRelativePath: string, platform: string, os: string | undefined, ports: number[], _: Partial<PackageInfo>): string {
return `version: '3.4'
services:
Expand Down
2 changes: 1 addition & 1 deletion src/configureWorkspace/configureRuby.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ services:
${getComposePorts(ports)}`;
}

function genDockerComposeDebug(serviceNameAndRelativePath: string, platform: string, os: string | undefined, ports: number[], { fullCommand: cmd }: Partial<PackageInfo>): string {
function genDockerComposeDebug(serviceNameAndRelativePath: string, platform: string, os: string | undefined, ports: number[], _: Partial<PackageInfo>): string {
return `version: '3.4'
services:
Expand Down
20 changes: 10 additions & 10 deletions test/configure.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -463,19 +463,19 @@ suite("Configure (Add Docker files to Workspace)", function (this: Suite): void
);

assertFileContains('Dockerfile', 'EXPOSE 1234');
assertFileContains('Dockerfile', 'CMD npm start');
assertFileContains('Dockerfile', 'CMD ["npm", "start"]');

assertFileContains('docker-compose.debug.yml', '1234');
assertFileContains('docker-compose.debug.yml', '9229:9229');
assertFileContains('docker-compose.debug.yml', 'image: testoutput');
assertFileContains('docker-compose.debug.yml', 'NODE_ENV: development');
assertFileContains('docker-compose.debug.yml', 'command: node --inspect=0.0.0.0:9229 index.js');
assertFileContains('docker-compose.debug.yml', 'command: ["node", "--inspect=0.0.0.0:9229", "index.js"]');

assertFileContains('docker-compose.yml', '1234');
assertNotFileContains('docker-compose.yml', '9229:9229');
assertFileContains('docker-compose.yml', 'image: testoutput');
assertFileContains('docker-compose.yml', 'NODE_ENV: production');
assertNotFileContains('docker-compose.yml', 'command: node --inspect=0.0.0.0:9229 index.js');
assertNotFileContains('docker-compose.yml', 'command: ["node", "--inspect=0.0.0.0:9229", "index.js"]');

assertFileContains('.dockerignore', '.vscode');
});
Expand All @@ -494,7 +494,7 @@ suite("Configure (Add Docker files to Workspace)", function (this: Suite): void
);

assertFileContains('Dockerfile', 'EXPOSE 1234');
assertFileContains('Dockerfile', 'CMD npm start');
assertFileContains('Dockerfile', 'CMD ["npm", "start"]');

assertFileContains('.dockerignore', '.vscode');
});
Expand Down Expand Up @@ -534,19 +534,19 @@ suite("Configure (Add Docker files to Workspace)", function (this: Suite): void
);

assertFileContains('Dockerfile', 'EXPOSE 4321');
assertFileContains('Dockerfile', 'CMD npm start');
assertFileContains('Dockerfile', 'CMD ["npm", "start"]');

assertFileContains('docker-compose.debug.yml', '4321');
assertFileContains('docker-compose.debug.yml', '9229:9229');
assertFileContains('docker-compose.debug.yml', 'image: testoutput');
assertFileContains('docker-compose.debug.yml', 'NODE_ENV: development');
assertFileContains('docker-compose.debug.yml', 'command: node --inspect=0.0.0.0:9229 ./bin/www');
assertFileContains('docker-compose.debug.yml', 'command: ["node", "--inspect=0.0.0.0:9229", "./bin/www"]');

assertFileContains('docker-compose.yml', '4321');
assertNotFileContains('docker-compose.yml', '9229:9229');
assertFileContains('docker-compose.yml', 'image: testoutput');
assertFileContains('docker-compose.yml', 'NODE_ENV: production');
assertNotFileContains('docker-compose.yml', 'command: node --inspect=0.0.0.0:9229 ./bin/www');
assertNotFileContains('docker-compose.yml', 'command: ["node", "--inspect=0.0.0.0:9229", "./bin/www"]');

assertFileContains('.dockerignore', '.vscode');
});
Expand Down Expand Up @@ -581,7 +581,7 @@ suite("Configure (Add Docker files to Workspace)", function (this: Suite): void
['package.json', 'Dockerfile', 'docker-compose.debug.yml', 'docker-compose.yml', '.dockerignore']
);

assertNotFileContains('docker-compose.yml', 'command: node --inspect=0.0.0.0:9229 index.js');
assertNotFileContains('docker-compose.yml', 'command: ["node", "--inspect=0.0.0.0:9229", "index.js"]');
});

testInEmptyFolder("Without start script", async () => {
Expand Down Expand Up @@ -614,7 +614,7 @@ suite("Configure (Add Docker files to Workspace)", function (this: Suite): void
);

assertFileContains('Dockerfile', 'EXPOSE 4321');
assertFileContains('Dockerfile', 'CMD node ./out/dockerExtension');
assertFileContains('Dockerfile', 'CMD ["node", "./out/dockerExtension"]');
});
});

Expand Down Expand Up @@ -1348,7 +1348,7 @@ suite("Configure (Add Docker files to Workspace)", function (this: Suite): void
FROM docker/whalesay:latest
LABEL Name=testoutput Version=1.2.3
RUN apt-get -y update && apt-get install -y fortunes
CMD /usr/games/fortune -a | cowsay
CMD ["sh", "-c", "/usr/games/fortune -a | cowsay"]
`));
assert.strictEqual(composeContents, removeIndentation(`
version: '3.4'
Expand Down

0 comments on commit c8cf547

Please sign in to comment.