From b65a4dcb4688a1df22abe2d63580bf64927f94f4 Mon Sep 17 00:00:00 2001 From: Jonnatan Jossemar Cordero Date: Tue, 5 Jul 2022 22:50:24 -0600 Subject: [PATCH 1/2] Add .dockerignore support for blobs and dirs The current .dockerignore file support is limited to the files listed on the context that comes from the options. On said scenario some of those could be a directory holding files that should be ignored by a blob rule. Also, I refactored it to not rely on readSync. Signed-off-by: Jonnatan Jossemar Cordero --- .gitignore | 1 + lib/docker.js | 85 ++++++++----------- lib/util.js | 36 ++++++++ package-lock.json | 6 ++ test/docker.js | 25 +++++- test/fixtures/dockerignore/.dockerignore | 6 ++ test/fixtures/dockerignore/Dockerfile | 5 ++ test/fixtures/dockerignore/MC-hammer.txt | 1 + test/fixtures/dockerignore/empty.txt | 0 .../fixtures/dockerignore/ignore-dir/.gitkeep | 0 .../dockerignore/ignore-dir/empty.txt | 0 test/util.js | 54 ++++++++++-- 12 files changed, 163 insertions(+), 56 deletions(-) create mode 100644 test/fixtures/dockerignore/.dockerignore create mode 100644 test/fixtures/dockerignore/Dockerfile create mode 100644 test/fixtures/dockerignore/MC-hammer.txt create mode 100644 test/fixtures/dockerignore/empty.txt create mode 100644 test/fixtures/dockerignore/ignore-dir/.gitkeep create mode 100644 test/fixtures/dockerignore/ignore-dir/empty.txt diff --git a/.gitignore b/.gitignore index b07a075..62180ed 100644 --- a/.gitignore +++ b/.gitignore @@ -4,3 +4,4 @@ npm-debug.log *~ *.log .idea +.vscode diff --git a/lib/docker.js b/lib/docker.js index a5ae1c2..b9edcd4 100644 --- a/lib/docker.js +++ b/lib/docker.js @@ -1,7 +1,5 @@ var EventEmitter = require('events').EventEmitter, Modem = require('docker-modem'), - tar = require('tar-fs'), - zlib = require('zlib'), Container = require('./container'), Image = require('./image'), Volume = require('./volume'), @@ -14,11 +12,7 @@ var EventEmitter = require('events').EventEmitter, Node = require('./node'), Exec = require('./exec'), util = require('./util'), - extend = util.extend, - path = require('path'), - extend = util.extend, - ignore = require('@balena/dockerignore'), - fs = require('fs'); + extend = util.extend; var Docker = function(opts) { if (!(this instanceof Docker)) return new Docker(opts); @@ -262,35 +256,40 @@ Docker.prototype.buildImage = function(file, opts, callback) { opts = null; } - function build(file) { - var optsf = { - path: '/build?', - method: 'POST', - file: file, - options: opts, - abortSignal: opts && opts.abortSignal, - isStream: true, - statusCodes: { - 200: true, - 500: 'server error' - } - }; + var optsf = { + path: '/build?', + method: 'POST', + file: undefined, + options: opts, + abortSignal: opts && opts.abortSignal, + isStream: true, + statusCodes: { + 200: true, + 500: 'server error' + } + }; - if (opts) { - if (opts.registryconfig) { - optsf.registryconfig = optsf.options.registryconfig; - delete optsf.options.registryconfig; - } + if (opts) { + if (opts.registryconfig) { + optsf.registryconfig = optsf.options.registryconfig; + delete optsf.options.registryconfig; + } - //undocumented? - if (opts.authconfig) { - optsf.authconfig = optsf.options.authconfig; - delete optsf.options.authconfig; - } + //undocumented? + if (opts.authconfig) { + optsf.authconfig = optsf.options.authconfig; + delete optsf.options.authconfig; } + } + + if (callback === undefined) { + const prepareCtxPromise = new self.modem.Promise(function(resolve, _) { + util.prepareBuildContext(file, resolve) + }); - if (callback === undefined) { + return prepareCtxPromise.then((ctx)=> { return new self.modem.Promise(function(resolve, reject) { + optsf.file = ctx; self.modem.dial(optsf, function(err, data) { if (err) { return reject(err); @@ -298,28 +297,14 @@ Docker.prototype.buildImage = function(file, opts, callback) { resolve(data); }); }); - } else { + }) + } else { + util.prepareBuildContext(file, (ctx) => { + optsf.file = ctx; self.modem.dial(optsf, function(err, data) { callback(err, data); }); - } - } - - if (file && file.context) { - let entries = file.src - - const dockerignorePath = path.join(file.context, '.dockerignore') - if (fs.existsSync(dockerignorePath)) { - const dockerIgnore = ignore({ ignorecase: false }).add(fs.readFileSync(dockerignorePath).toString()) - entries = (entries || []).filter(dockerIgnore.createFilter()) - } - - var pack = tar.pack(file.context, { - entries: file.src.slice() - }); - return build(pack.pipe(zlib.createGzip())); - } else { - return build(file); + }) } }; diff --git a/lib/util.js b/lib/util.js index 13cca97..e16f3aa 100644 --- a/lib/util.js +++ b/lib/util.js @@ -1,3 +1,9 @@ +var DockerIgnore = require('@balena/dockerignore'); +var fs = require('fs'); +var path = require('path'); +var tar = require('tar-fs'); +var zlib = require('zlib'); + // https://github.com/HenrikJoreteg/extend-object/blob/v0.1.0/extend-object.js var arr = []; @@ -68,3 +74,33 @@ module.exports.parseRepositoryTag = function(input) { repository: input }; }; + + +module.exports.prepareBuildContext = function(file, next) { + if (file && file.context) { + fs.readFile(path.join(file.context, '.dockerignore'), (err, data) => { + let ignoreFn; + let filterFn; + + if (!err) { + const dockerIgnore = DockerIgnore({ ignorecase: false }).add(data.toString()); + + filterFn = dockerIgnore.createFilter(); + ignoreFn = (path) => { + return !filterFn(path); + } + } + + const entries = file.src.slice() || [] + + const pack = tar.pack(file.context, { + entries: filterFn ? entries.filter(filterFn) : entries, + ignore: ignoreFn // Only works on directories + }); + + next(pack.pipe(zlib.createGzip())); + }) + } else { + next(file); + } +} diff --git a/package-lock.json b/package-lock.json index 23f99ab..91a253d 100644 --- a/package-lock.json +++ b/package-lock.json @@ -9,6 +9,7 @@ "version": "3.3.3", "license": "Apache-2.0", "dependencies": { + "@balena/dockerignore": "^1.0.2", "docker-modem": "^3.0.0", "tar-fs": "~2.0.1" }, @@ -22,6 +23,11 @@ "node": ">= 8.0" } }, + "node_modules/@balena/dockerignore": { + "version": "1.0.2", + "resolved": "https://registry.npmjs.org/@balena/dockerignore/-/dockerignore-1.0.2.tgz", + "integrity": "sha512-wMue2Sy4GAVTk6Ic4tJVcnfdau+gx2EnG7S+uAEe+TWJFqE4YoWN4/H8MSLj4eYJKxGg26lZwboEniNiNwZQ6Q==" + }, "node_modules/ansi-colors": { "version": "3.2.3", "resolved": "https://registry.npmjs.org/ansi-colors/-/ansi-colors-3.2.3.tgz", diff --git a/test/docker.js b/test/docker.js index e296a78..c31effe 100644 --- a/test/docker.js +++ b/test/docker.js @@ -3,6 +3,7 @@ var Bluebird = require('bluebird'), expect = require('chai').expect, assert = require('assert'), + path = require('path'), Docker = require('../lib/docker'); var docker = require('./spec_helper').docker; @@ -128,7 +129,29 @@ describe("#docker", function() { docker.buildImage({ context: __dirname, src: ['Dockerfile'] - }, {}, handler); + }, { t: 'multiple-files' }, handler); + }); + + it("should build image from multiple files while respecting the .dockerignore file", function(done) { + this.timeout(60000); + + function handler(err, stream) { + expect(err).to.be.null; + expect(stream).to.be.ok; + + stream.pipe(process.stdout, { + end: true + }); + + stream.on('end', function() { + done(); + }); + } + + docker.buildImage({ + context: path.join(__dirname, 'fixtures', 'dockerignore'), + src: ['Dockerfile', 'MC-hammer.txt', 'ignore-dir', 'foo.txt'] + }, { t: 'honor-dockerignore' }, handler); }); it("should build image from multiple files while respecting the dockerignore file", function(done) { diff --git a/test/fixtures/dockerignore/.dockerignore b/test/fixtures/dockerignore/.dockerignore new file mode 100644 index 0000000..5cbc018 --- /dev/null +++ b/test/fixtures/dockerignore/.dockerignore @@ -0,0 +1,6 @@ +# Ignore dirs +ignore-dir +# Ignore blobs +*.txt +# Override rules +!MC-hammer.txt diff --git a/test/fixtures/dockerignore/Dockerfile b/test/fixtures/dockerignore/Dockerfile new file mode 100644 index 0000000..d2e2544 --- /dev/null +++ b/test/fixtures/dockerignore/Dockerfile @@ -0,0 +1,5 @@ +FROM scratch + +COPY . . + +CMD ["bash"] diff --git a/test/fixtures/dockerignore/MC-hammer.txt b/test/fixtures/dockerignore/MC-hammer.txt new file mode 100644 index 0000000..8335e70 --- /dev/null +++ b/test/fixtures/dockerignore/MC-hammer.txt @@ -0,0 +1 @@ +Can't touch this diff --git a/test/fixtures/dockerignore/empty.txt b/test/fixtures/dockerignore/empty.txt new file mode 100644 index 0000000..e69de29 diff --git a/test/fixtures/dockerignore/ignore-dir/.gitkeep b/test/fixtures/dockerignore/ignore-dir/.gitkeep new file mode 100644 index 0000000..e69de29 diff --git a/test/fixtures/dockerignore/ignore-dir/empty.txt b/test/fixtures/dockerignore/ignore-dir/empty.txt new file mode 100644 index 0000000..e69de29 diff --git a/test/util.js b/test/util.js index a8479c7..5a273c9 100644 --- a/test/util.js +++ b/test/util.js @@ -1,11 +1,16 @@ var expect = require('chai').expect; var util = require('../lib/util'); +var path = require('path'); +var zlib = require('zlib'); +var tar = require('tar-fs'); +var fs = require('fs'); +var os = require('os'); -describe('util', function() { +describe('util', function () { - describe('#parseRepositoryTag', function() { + describe('#parseRepositoryTag', function () { function validate(input, expected) { - it('should parse "' + input + '"', function() { + it('should parse "' + input + '"', function () { expect(util.parseRepositoryTag(input)).to.eql(expected); }); } @@ -46,11 +51,50 @@ describe('util', function() { }); // https://github.com/HenrikJoreteg/extend-object/blob/v0.1.0/test.js - describe('.extend', function() { + describe('.extend', function () { it('accepts multiple object arguments', function () { var start = {}; - expect(util.extend(start, {name: 'test'}, {hello: 'test'})).to.deep.equal({name: 'test', hello: 'test'}); + expect(util.extend(start, { name: 'test' }, { hello: 'test' })).to.deep.equal({ name: 'test', hello: 'test' }); expect(start).to.eql(util.extend(start, {})); }); }); + + describe('.prepareBuildContext', function () { + + it("should pass the options through when there is no context", function () { + const dummy = {}; + util.prepareBuildContext(dummy, function (ctx) { + expect(ctx).to.be.equal(dummy); + }) + }); + + it("bundle the context and source as a single tar.gz stream", function (done) { + this.timeout(60000); + + function handler(stream) { + expect(stream).to.be.ok; + + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'dockerode-')); + const z = zlib.createGunzip(); + + stream + .pipe(z) + .pipe(tar.extract(tmp), { end: true }) + .on('finish', function () { + const files = fs.readdirSync(tmp); + + expect(files.length).to.be.equal(2); + expect(files).to.have.members(['Dockerfile', 'MC-hammer.txt']); + + fs.rm(tmp, { recursive: true }); + done(); + }); + } + + util.prepareBuildContext({ + context: path.join(__dirname, 'fixtures', 'dockerignore'), + src: ['Dockerfile', 'MC-hammer.txt', 'ignore-dir', 'foo.txt'] + }, handler); + }); + }); }); From 2312b40fbe1dfac014e4df366bf4047160464d8a Mon Sep 17 00:00:00 2001 From: Jonnatan Jossemar Cordero Date: Sat, 13 Aug 2022 12:39:28 -0600 Subject: [PATCH 2/2] Groom the Vagrantfile It was evident that few people were using this file, anyway I could not resist the urge of updating it. This commit is not relevant for the main issue I'm aiming to cover and can be move to another PR. Signed-off-by: Jonnatan Jossemar Cordero --- Vagrantfile | 44 ++++++++++++++++++++++++++++++++------------ 1 file changed, 32 insertions(+), 12 deletions(-) diff --git a/Vagrantfile b/Vagrantfile index 1edf12c..474ae4f 100644 --- a/Vagrantfile +++ b/Vagrantfile @@ -1,8 +1,9 @@ # -*- mode: ruby -*- # vi: set ft=ruby : -BOX_NAME = ENV['BOX_NAME'] || "ubuntu/xenial64" +BOX_NAME = ENV['BOX_NAME'] || "ubuntu/focal64" SSH_PRIVKEY_PATH = ENV["SSH_PRIVKEY_PATH"] +NODE_MAJOR_VERSION = "14" $script = <