Skip to content

Commit

Permalink
fix(migrate): re-work migrate step to run knex-migrator in a subprocess
Browse files Browse the repository at this point in the history 
closes TryGhost#349
- move knex-migrator to run in a child process, which allows us to initialize a sqlite database during a production ubuntu install
  • Loading branch information
@acburdine
acburdine committed Jul 15, 2017
1 parent 3a450e1 commit 53d89fd
Show file tree
Hide file tree
Showing 3 changed files with 189 additions and 129 deletions.
45 changes: 28 additions & 17 deletions lib/tasks/migrate.js
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
'use strict';
const path = require('path');
const KnexMigrator = require('knex-migrator');
const execa = require('execa');

const errors = require('../errors');
const shouldUseGhostUser = require('../utils/use-ghost-user');

module.exports = function runMigrations(context) {
let config = context.instance.config;
Expand All @@ -12,22 +13,34 @@ module.exports = function runMigrations(context) {
}

let transports = config.get('logging.transports', null);
// TODO: revisit just hiding migration output altogether
config.set('logging.transports', []).save();
config.set('logging.transports', ['file']).save();

let knexMigrator = new KnexMigrator({
knexMigratorFilePath: path.join(process.cwd(), 'current')
});
let contentDir = path.join(context.instance.dir, 'content');
let currentDir = path.join(context.instance.dir, 'current');
let knexMigratorPromise;

return knexMigrator.isDatabaseOK().catch((error) => {
if (error.code === 'DB_NOT_INITIALISED' ||
error.code === 'MIGRATION_TABLE_IS_MISSING') {
return knexMigrator.init();
} else if (error.code === 'DB_NEEDS_MIGRATION') {
return knexMigrator.migrate();
}
let args = ['--init', '--mgpath', currentDir];

// If we're using sqlite and the ghost user owns the content folder, then
// we should run sudo, otherwise run normally
if (shouldUseGhostUser(contentDir)) {
let knexMigratorPath = path.resolve(__dirname, '../../node_modules/.bin/knex-migrator-migrate');
knexMigratorPromise = context.ui.sudo(`-E -u ghost ${knexMigratorPath} ${args.join(' ')}`, {
stdio: ['inherit', 'inherit', 'pipe']
});
} else {
knexMigratorPromise = execa('knex-migrator-migrate', args, {
preferLocal: true,
localDir: __dirname,
stdout: 'ignore',
stderr: 'pipe'
});
}

if (error.code === 'ENOTFOUND') {
return knexMigratorPromise.then(() => {
config.set('logging.transports', transports).save();
}).catch((error) => {
if (error.stderr.match(/CODE\: ENOTFOUND/)) {
// Database not found
error = new errors.ConfigError({
config: {
Expand All @@ -36,7 +49,7 @@ module.exports = function runMigrations(context) {
message: 'Invalid database host',
environment: context.instance.system.environment
});
} else if (error.code === 'ER_ACCESS_DENIED_ERROR') {
} else if (error.stderr.match(/CODE\: ER_ACCESS_DENIED_ERROR/)) {
error = new errors.ConfigError({
config: {
'database.connection.user': config.get('database.connection.user'),
Expand All @@ -49,8 +62,6 @@ module.exports = function runMigrations(context) {

config.set('logging.transports', transports).save();
return Promise.reject(error);
}).then(() => {
config.set('logging.transports', transports).save();
});
}

155 changes: 43 additions & 112 deletions test/unit/tasks/migrate-spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,6 @@ const errors = require('../../../lib/errors');

const migratePath = '../../../lib/tasks/migrate';

// Used for testing
class TestError extends Error {
constructor(code) {
super('an error');
this.code = code;
}
}

function getConfigStub(noContentPath) {
let config = {
get: sinon.stub(),
Expand All @@ -28,157 +20,96 @@ function getConfigStub(noContentPath) {
}

describe('Unit: Tasks > Migrate', function () {
it('sets content path if it does not exist', function () {
it('runs direct command if useGhostUser returns false', function () {
let config = getConfigStub(true);
let migrate = proxyquire(migratePath, {
'knex-migrator': class KnexMigrator {
isDatabaseOK() { return Promise.resolve() }
}
});

migrate({ instance: { config: config, dir: '/test/' } }).then(() => {
expect(config.has.calledOnce).to.be.true;
expect(config.set.called).to.be.true;
expect(config.set.args[0]).to.deep.equal(['paths.contentPath', '/test/content']);
expect(config.save.called).to.be.true;
});
});

it('disables stdout log in config and re-enables it after completion', function () {
let config = getConfigStub();
config.get.withArgs('logging.transports', null).returns(['stdout', 'file']);
let dbOkStub = sinon.stub().resolves();
let execaStub = sinon.stub().resolves();
let useGhostUserStub = sinon.stub().returns(false);

let migrate = proxyquire(migratePath, {
'knex-migrator': class KnexMigrator {
isDatabaseOK() { return dbOkStub(); }
}
execa: execaStub,
'../utils/use-ghost-user': useGhostUserStub
});

return migrate({ instance: { config: config } }).then(() => {
expect(dbOkStub.calledOnce).to.be.true;
expect(config.get.calledOnce).to.be.true;
expect(config.set.calledTwice).to.be.true;
expect(config.set.args[0]).to.deep.equal(['logging.transports', []]);
expect(config.set.args[1]).to.deep.equal(['logging.transports', ['stdout', 'file']]);
expect(config.save.calledTwice).to.be.true;
});
});
let sudoStub = sinon.stub().resolves();

it('runs init if database is not initialized', function () {
let config = getConfigStub();
config.get.withArgs('logging.transports', null).returns(['stdout', 'file'])
let dbOkStub = sinon.stub().returns(Promise.reject(new TestError('DB_NOT_INITIALISED')));
let initStub = sinon.stub().resolves();

let migrate = proxyquire(migratePath, {
'knex-migrator': class KnexMigrator {
isDatabaseOK() { return dbOkStub() }
init() { return initStub() }
}
});

return migrate({ instance: { config: config } }).then(() => {
expect(dbOkStub.calledOnce).to.be.true;
expect(initStub.calledOnce).to.be.true;
expect(config.set.calledTwice).to.be.true;
expect(config.set.args[0]).to.deep.equal(['logging.transports', []]);
expect(config.set.args[1]).to.deep.equal(['logging.transports', ['stdout', 'file']]);
});
});

it('runs init if migration table is missing', function () {
let config = getConfigStub();
config.get.withArgs('logging.transports', null).returns(['stdout', 'file'])
let dbOkStub = sinon.stub().returns(Promise.reject(new TestError('MIGRATION_TABLE_IS_MISSING')));
let initStub = sinon.stub().resolves();

let migrate = proxyquire(migratePath, {
'knex-migrator': class KnexMigrator {
isDatabaseOK() { return dbOkStub() }
init() { return initStub() }
}
});

return migrate({ instance: { config: config } }).then(() => {
expect(dbOkStub.calledOnce).to.be.true;
expect(initStub.calledOnce).to.be.true;
expect(config.set.calledTwice).to.be.true;
expect(config.set.args[0]).to.deep.equal(['logging.transports', []]);
expect(config.set.args[1]).to.deep.equal(['logging.transports', ['stdout', 'file']]);
return migrate({ instance: { config: config, dir: '/some-dir' }, ui: { sudo: sudoStub } }).then(() => {
expect(useGhostUserStub.calledOnce).to.be.true;
expect(useGhostUserStub.args[0][0]).to.equal('/some-dir/content');
expect(execaStub.calledOnce).to.be.true;
expect(sudoStub.called).to.be.false;
expect(config.has.calledOnce).to.be.true;
expect(config.set.calledThrice).to.be.true;
expect(config.set.args[0]).to.deep.equal(['paths.contentPath', '/some-dir/content']);
expect(config.set.args[1]).to.deep.equal(['logging.transports', ['file']]);
expect(config.set.args[2]).to.deep.equal(['logging.transports', ['stdout', 'file']]);
expect(config.save.called).to.be.true;
});
});

it('runs migrate if db needs migration', function () {
it('runs sudo command if useGhostUser returns true', function () {
let config = getConfigStub();
config.get.withArgs('logging.transports', null).returns(['stdout', 'file']);
let dbOkStub = sinon.stub().returns(Promise.reject(new TestError('DB_NEEDS_MIGRATION')));
let migrateStub = sinon.stub().resolves();
let execaStub = sinon.stub().resolves();
let useGhostUserStub = sinon.stub().returns(true);

let migrate = proxyquire(migratePath, {
'knex-migrator': class KnexMigrator {
isDatabaseOK() { return dbOkStub() }
migrate() { return migrateStub() }
}
execa: execaStub,
'../utils/use-ghost-user': useGhostUserStub
});

return migrate({ instance: { config: config } }).then(() => {
expect(dbOkStub.calledOnce).to.be.true;
expect(migrateStub.calledOnce).to.be.true;
expect(config.set.calledTwice).to.be.true;
expect(config.set.args[0]).to.deep.equal(['logging.transports', []]);
let sudoStub = sinon.stub().resolves();

return migrate({ instance: { config: config, dir: '/some-dir' }, ui: { sudo: sudoStub } }).then(() => {
expect(useGhostUserStub.calledOnce).to.be.true;
expect(useGhostUserStub.args[0][0]).to.equal('/some-dir/content');
expect(execaStub.calledOnce).to.be.false;
expect(sudoStub.called).to.be.true;
expect(config.set.args[0]).to.deep.equal(['logging.transports', ['file']]);
expect(config.set.args[1]).to.deep.equal(['logging.transports', ['stdout', 'file']]);
});
});

it('throws config error with db host if database not found', function () {
let config = getConfigStub();
config.get.withArgs('logging.transports', null).returns(['stdout', 'file']);
let dbOkStub = sinon.stub().returns(Promise.reject(new TestError('ENOTFOUND')));
let execaStub = sinon.stub().returns(Promise.reject({stderr: 'CODE: ENOTFOUND'}));
let useGhostUserStub = sinon.stub().returns(false);

let migrate = proxyquire(migratePath, {
'knex-migrator': class KnexMigrator {
isDatabaseOK() { return dbOkStub() }
}
execa: execaStub,
'../utils/use-ghost-user': useGhostUserStub
});

return migrate({ instance: {
config: config,
system: { environment: 'testing' }
} }).then(() => {
return migrate({ instance: { config: config, dir: '/some-dir', system: { environment: 'testing' } } }).then(() => {
expect(false, 'error should have been thrown').to.be.true;
}).catch((error) => {
expect(error).to.be.an.instanceof(errors.ConfigError);
expect(error.options.config).to.have.key('database.connection.host');
expect(dbOkStub.calledOnce).to.be.true;
expect(config.set.calledTwice).to.be.true;
expect(config.set.args[0]).to.deep.equal(['logging.transports', []]);
expect(config.set.args[0]).to.deep.equal(['logging.transports', ['file']]);
expect(config.set.args[1]).to.deep.equal(['logging.transports', ['stdout', 'file']]);
});
});

it('throws config error with db user if access denied error', function () {
let config = getConfigStub();
config.get.withArgs('logging.transports', null).returns(['stdout', 'file'])
let dbOkStub = sinon.stub().returns(Promise.reject(new TestError('ER_ACCESS_DENIED_ERROR')));
config.get.withArgs('logging.transports', null).returns(['stdout', 'file']);
let execaStub = sinon.stub().returns(Promise.reject({stderr: 'CODE: ER_ACCESS_DENIED_ERROR' }));
let useGhostUserStub = sinon.stub().returns(false);

let migrate = proxyquire(migratePath, {
'knex-migrator': class KnexMigrator {
isDatabaseOK() { return dbOkStub() }
}
execa: execaStub,
'../utils/use-ghost-user': useGhostUserStub
});

return migrate({ instance: {
config: config,
system: { environment: 'testing' }
} }).then(() => {
return migrate({ instance: { config: config, dir: '/some-dir', system: { environment: 'testing' } } }).then(() => {
expect(false, 'error should have been thrown').to.be.true;
}).catch((error) => {
expect(error).to.be.an.instanceof(errors.ConfigError);
expect(error.options.config).to.have.all.keys('database.connection.user', 'database.connection.password');
expect(dbOkStub.calledOnce).to.be.true;
expect(config.set.calledTwice).to.be.true;
expect(config.set.args[0]).to.deep.equal(['logging.transports', []]);
expect(config.set.args[0]).to.deep.equal(['logging.transports', ['file']]);
expect(config.set.args[1]).to.deep.equal(['logging.transports', ['stdout', 'file']]);
});
});
Expand Down
Loading

0 comments on commit 53d89fd

Please sign in to comment.