Merge pull request #1997 from balena-io/1992-fix-build-args

build: Fix --buildArg and --cache-from options (broken since v12.9.9)

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 (

lib/utils/docker.ts

@@ -36,10 +36,8 @@ export interface DockerConnectionCliFlags {
 
 export interface DockerCliFlags extends DockerConnectionCliFlags {
 	tag?: string;
-	buildArg?: string; // maps to 'arg'
+	buildArg?: string[];
-	arg?: string; // Not part of command profile
+	'cache-from'?: string;
-	'cache-from'?: string; // maps to 'image-list'
-	'image-list'?: string; // Not part of command profile
 	nocache: boolean;
 	squash: boolean;
 }
@@ -80,13 +78,12 @@ export const dockerCliFlags: flags.Input<DockerCliFlags> = {
 		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",

tests/commands/build.spec.ts

@@ -43,20 +43,17 @@ const commonResponseLines: { [key: string]: string[] } = {
 	],
 };
 
-const commonQueryParams = [
+const commonQueryParams = {
-	['t', '${tag}'],
+	t: '${tag}',
-	['buildargs', '{}'],
+	buildargs: '{}',
-	['labels', ''],
+	labels: '',
-];
+};
 
-const commonComposeQueryParams = [
+const commonComposeQueryParams = {
-	['t', '${tag}'],
+	t: '${tag}',
-	[
+	buildargs: '{"MY_VAR_1":"This is a variable","MY_VAR_2":"Also a variable"}',
-		'buildargs',
+	labels: '',
-		'{"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: [
+			service1: Object.entries({
-				['t', '${tag}'],
+				...commonComposeQueryParams,
-				[
+				buildargs:
-					'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"}',
-					'{"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"]',
-				],
+			}),
-				['labels', ''],
+			service2: Object.entries({
-			],
+				...commonComposeQueryParams,
-			service2: [...commonComposeQueryParams, ['dockerfile', 'Dockerfile-alt']],
+				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: [
+			service1: Object.entries({
-				['t', '${tag}'],
+				...commonComposeQueryParams,
-				[
+				buildargs:
-					'buildargs',
 					'{"MY_VAR_1":"This is a variable","MY_VAR_2":"Also a variable","SERVICE1_VAR":"This is a service specific variable"}',
-				],
+			}),
-				['labels', ''],
+			service2: Object.entries({
-			],
+				...commonComposeQueryParams,
-			service2: [...commonComposeQueryParams, ['dockerfile', 'Dockerfile-alt']],
+				dockerfile: 'Dockerfile-alt',
+			}),
 		};
 		const expectedResponseLines: string[] = [
 			...commonResponseLines[responseFilename],

tests/docker-mock.ts

@@ -78,7 +78,7 @@ export class DockerMock extends NockMock {
 		checkBuildRequestBody: (requestBody: string) => Promise<void>;
 	}) {
 		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;