From af1c4b0d03aef17faa6873aa89164a37f4e44c77 Mon Sep 17 00:00:00 2001 From: Paulo Castro Date: Fri, 14 Aug 2020 16:41:56 +0100 Subject: [PATCH] build: Fix --buildArg and --cache-from options (broken since v12.9.9) Change-type: patch --- lib/actions-oclif/build.ts | 20 ++---- lib/utils/docker.ts | 9 +-- tests/commands/build.spec.ts | 129 ++++++++++++++++++++++++++--------- tests/docker-mock.ts | 2 +- 4 files changed, 103 insertions(+), 57 deletions(-) diff --git a/lib/actions-oclif/build.ts b/lib/actions-oclif/build.ts index 65a2f0b3..a893592a 100644 --- a/lib/actions-oclif/build.ts +++ b/lib/actions-oclif/build.ts @@ -115,7 +115,10 @@ ${dockerignoreHelp} const logger = await Command.getLogger(); logger.logDebug('Parsing input...'); - this.translateParams(params, options); + // `build` accepts `source` as a parameter, but compose expects it as an option + options.source = params.source; + delete params.source; + await this.validateOptions(options, sdk); const app = await this.getAppAndResolveArch(options); @@ -139,21 +142,6 @@ ${dockerignoreHelp} logger.logSuccess('Build succeeded!'); } - protected translateParams(params: ArgsDef, options: FlagsDef) { - // Copy flags to those expected by other modules - options.arg = options.buildArg; - delete options.buildArg; - options['image-list'] = options['cache-from']; - delete options['cache-from']; - - // `build` accepts `[source]` as a parameter, but compose expects it - // as an option. swap them here - if (options.source == null) { - options.source = params.source; - } - delete params.source; - } - protected async validateOptions(opts: FlagsDef, sdk: BalenaSDK) { // Validate option combinations if ( diff --git a/lib/utils/docker.ts b/lib/utils/docker.ts index d8698a12..f15ff41e 100644 --- a/lib/utils/docker.ts +++ b/lib/utils/docker.ts @@ -36,10 +36,8 @@ export interface DockerConnectionCliFlags { export interface DockerCliFlags extends DockerConnectionCliFlags { tag?: string; - buildArg?: string; // maps to 'arg' - arg?: string; // Not part of command profile - 'cache-from'?: string; // maps to 'image-list' - 'image-list'?: string; // Not part of command profile + buildArg?: string[]; + 'cache-from'?: string; nocache: boolean; squash: boolean; } @@ -80,13 +78,12 @@ export const dockerCliFlags: flags.Input = { description: 'Set a build-time variable (eg. "-B \'ARG=value\'"). Can be specified multiple times.', char: 'B', - // Maps to flag `arg` + multiple: true, }), 'cache-from': flags.string({ description: `\ Comma-separated list (no spaces) of image names for build cache resolution. \ Implements the same feature as the "docker build --cache-from" option.`, - // Maps to flag `image-list` }), nocache: flags.boolean({ description: "Don't use docker layer caching when building", diff --git a/tests/commands/build.spec.ts b/tests/commands/build.spec.ts index ec6c0d04..bfe5b413 100644 --- a/tests/commands/build.spec.ts +++ b/tests/commands/build.spec.ts @@ -43,20 +43,17 @@ const commonResponseLines: { [key: string]: string[] } = { ], }; -const commonQueryParams = [ - ['t', '${tag}'], - ['buildargs', '{}'], - ['labels', ''], -]; +const commonQueryParams = { + t: '${tag}', + buildargs: '{}', + labels: '', +}; -const commonComposeQueryParams = [ - ['t', '${tag}'], - [ - 'buildargs', - '{"MY_VAR_1":"This is a variable","MY_VAR_2":"Also a variable"}', - ], - ['labels', ''], -]; +const commonComposeQueryParams = { + t: '${tag}', + buildargs: '{"MY_VAR_1":"This is a variable","MY_VAR_2":"Also a variable"}', + labels: '', +}; const hr = '----------------------------------------------------------------------'; @@ -126,7 +123,65 @@ describe('balena build', function () { commandLine: `build ${projectPath} --deviceType nuc --arch amd64 -g`, dockerMock: docker, expectedFilesByService: { main: expectedFiles }, - expectedQueryParamsByService: { main: commonQueryParams }, + expectedQueryParamsByService: { main: Object.entries(commonQueryParams) }, + expectedResponseLines, + projectPath, + responseBody, + responseCode: 200, + services: ['main'], + }); + }); + + it('should create the expected tar stream (--buildArg and --cache-from)', async () => { + const projectPath = path.join(projectsPath, 'no-docker-compose', 'basic'); + const expectedFiles: ExpectedTarStreamFiles = { + 'src/.dockerignore': { fileSize: 16, type: 'file' }, + 'src/start.sh': { fileSize: 89, type: 'file' }, + 'src/windows-crlf.sh': { + fileSize: isWindows ? 68 : 70, + testStream: isWindows ? expectStreamNoCRLF : undefined, + type: 'file', + }, + Dockerfile: { fileSize: 88, type: 'file' }, + 'Dockerfile-alt': { fileSize: 30, type: 'file' }, + }; + const expectedQueryParams = { + ...commonQueryParams, + buildargs: '{"BARG1":"b1","barg2":"B2"}', + cachefrom: '["my/img1","my/img2"]', + }; + const responseFilename = 'build-POST.json'; + const responseBody = await fs.readFile( + path.join(dockerResponsePath, responseFilename), + 'utf8', + ); + const expectedResponseLines = [ + ...commonResponseLines[responseFilename], + `[Info] No "docker-compose.yml" file found at "${projectPath}"`, + `[Info] Creating default composition with source: "${projectPath}"`, + '[Build] main Step 1/4 : FROM busybox', + ]; + if (isWindows) { + const fname = path.join(projectPath, 'src', 'windows-crlf.sh'); + if (isWindows) { + expectedResponseLines.push( + `[Info] Converting line endings CRLF -> LF for file: ${fname}`, + ); + } else { + expectedResponseLines.push( + `[Warn] CRLF (Windows) line endings detected in file: ${fname}`, + '[Warn] Windows-format line endings were detected in some files. Consider using the `--convert-eol` option.', + ); + } + } + docker.expectGetInfo({}); + await testDockerBuildStream({ + commandLine: `build ${projectPath} --deviceType nuc --arch amd64 -B BARG1=b1 -B barg2=B2 --cache-from my/img1,my/img2`, + dockerMock: docker, + expectedFilesByService: { main: expectedFiles }, + expectedQueryParamsByService: { + main: Object.entries(expectedQueryParams), + }, expectedResponseLines, projectPath, responseBody, @@ -216,7 +271,9 @@ describe('balena build', function () { commandLine: `build ${projectPath} --emulated --deviceType ${deviceType} --arch ${arch} --nogitignore`, dockerMock: docker, expectedFilesByService: { main: expectedFiles }, - expectedQueryParamsByService: { main: commonQueryParams }, + expectedQueryParamsByService: { + main: Object.entries(commonQueryParams), + }, expectedResponseLines, projectPath, responseBody, @@ -274,7 +331,7 @@ describe('balena build', function () { commandLine: `build ${projectPath} --deviceType nuc --arch amd64 --noconvert-eol -m`, dockerMock: docker, expectedFilesByService: { main: expectedFiles }, - expectedQueryParamsByService: { main: commonQueryParams }, + expectedQueryParamsByService: { main: Object.entries(commonQueryParams) }, expectedResponseLines, projectPath, responseBody, @@ -318,15 +375,19 @@ describe('balena build', function () { 'utf8', ); const expectedQueryParamsByService = { - service1: [ - ['t', '${tag}'], - [ - 'buildargs', - '{"MY_VAR_1":"This is a variable","MY_VAR_2":"Also a variable","SERVICE1_VAR":"This is a service specific variable"}', - ], - ['labels', ''], - ], - service2: [...commonComposeQueryParams, ['dockerfile', 'Dockerfile-alt']], + service1: Object.entries({ + ...commonComposeQueryParams, + buildargs: + '{"BARG1":"b1","barg2":"B2","MY_VAR_1":"This is a variable","MY_VAR_2":"Also a variable","SERVICE1_VAR":"This is a service specific variable"}', + cachefrom: '["my/img1","my/img2"]', + }), + service2: Object.entries({ + ...commonComposeQueryParams, + buildargs: + '{"BARG1":"b1","barg2":"B2","MY_VAR_1":"This is a variable","MY_VAR_2":"Also a variable"}', + cachefrom: '["my/img1","my/img2"]', + dockerfile: 'Dockerfile-alt', + }), }; const expectedResponseLines: string[] = [ ...commonResponseLines[responseFilename], @@ -356,7 +417,7 @@ describe('balena build', function () { } docker.expectGetInfo({}); await testDockerBuildStream({ - commandLine: `build ${projectPath} --deviceType nuc --arch amd64 --convert-eol -G`, + commandLine: `build ${projectPath} --deviceType nuc --arch amd64 --convert-eol -G -B BARG1=b1 -B barg2=B2 --cache-from my/img1,my/img2`, dockerMock: docker, expectedFilesByService, expectedQueryParamsByService, @@ -403,15 +464,15 @@ describe('balena build', function () { 'utf8', ); const expectedQueryParamsByService = { - service1: [ - ['t', '${tag}'], - [ - 'buildargs', + service1: Object.entries({ + ...commonComposeQueryParams, + buildargs: '{"MY_VAR_1":"This is a variable","MY_VAR_2":"Also a variable","SERVICE1_VAR":"This is a service specific variable"}', - ], - ['labels', ''], - ], - service2: [...commonComposeQueryParams, ['dockerfile', 'Dockerfile-alt']], + }), + service2: Object.entries({ + ...commonComposeQueryParams, + dockerfile: 'Dockerfile-alt', + }), }; const expectedResponseLines: string[] = [ ...commonResponseLines[responseFilename], diff --git a/tests/docker-mock.ts b/tests/docker-mock.ts index 8fd552d8..fe98213f 100644 --- a/tests/docker-mock.ts +++ b/tests/docker-mock.ts @@ -78,7 +78,7 @@ export class DockerMock extends NockMock { checkBuildRequestBody: (requestBody: string) => Promise; }) { this.optPost( - new RegExp(`^/build\\?t=${_.escapeRegExp(opts.tag)}&`), + new RegExp(`^/build\\?(|.+&)t=${_.escapeRegExp(opts.tag)}&`), opts, ).reply(async function (uri, requestBody, cb) { let error: Error | null = null;