diff --git a/migrations/20171013003424-apps-add-newConfigJson.js b/migrations/20171013003424-apps-add-newConfigJson.js new file mode 100644 index 000000000..649c28275 --- /dev/null +++ b/migrations/20171013003424-apps-add-newConfigJson.js @@ -0,0 +1,15 @@ +'use strict'; + +exports.up = function(db, callback) { + db.runSql('ALTER TABLE apps ADD COLUMN newConfigJson TEXT', function (error) { + if (error) console.error(error); + callback(error); + }); +}; + +exports.down = function(db, callback) { + db.runSql('ALTER TABLE apps DROP COLUMN newConfigJson', function (error) { + if (error) console.error(error); + callback(error); + }); +}; diff --git a/migrations/schema.sql b/migrations/schema.sql index 4fedc9746..94dd4fd01 100644 --- a/migrations/schema.sql +++ b/migrations/schema.sql @@ -73,7 +73,8 @@ CREATE TABLE IF NOT EXISTS apps( // the following fields do not belong here, they can be removed when we use a queue for apptask lastBackupId VARCHAR(128), // used to pass backupId to restore from to apptask - oldConfigJson TEXT, // used to pass old config for apptask + oldConfigJson TEXT, // used to pass old config for apptask (configure, restore) + newConfigJson TEXT, // used to pass new config for apptask (update) PRIMARY KEY(id)); @@ -111,6 +112,7 @@ CREATE TABLE IF NOT EXISTS backups( dependsOn TEXT, /* comma separate list of objects this backup depends on */ state VARCHAR(16) NOT NULL, restoreConfigJson TEXT, /* JSON including the manifest of the backed up app */ + format VARCHAR(16) DEFAULT "tgz", PRIMARY KEY (id)); diff --git a/src/appdb.js b/src/appdb.js index b5d63f45d..015090cce 100644 --- a/src/appdb.js +++ b/src/appdb.js @@ -59,7 +59,7 @@ var assert = require('assert'), var APPS_FIELDS_PREFIXED = [ 'apps.id', 'apps.appStoreId', 'apps.installationState', 'apps.installationProgress', 'apps.runState', 'apps.health', 'apps.containerId', 'apps.manifestJson', 'apps.httpPort', 'apps.location', 'apps.dnsRecordId', - 'apps.accessRestrictionJson', 'apps.lastBackupId', 'apps.oldConfigJson', 'apps.memoryLimit', 'apps.altDomain', + 'apps.accessRestrictionJson', 'apps.lastBackupId', 'apps.oldConfigJson', 'apps.newConfigJson', 'apps.memoryLimit', 'apps.altDomain', 'apps.xFrameOptions', 'apps.sso', 'apps.debugModeJson', 'apps.robotsTxt', 'apps.enableBackup' ].join(','); var PORT_BINDINGS_FIELDS = [ 'hostPort', 'environmentVariable', 'appId' ].join(','); @@ -75,6 +75,10 @@ function postProcess(result) { result.oldConfig = safe.JSON.parse(result.oldConfigJson); delete result.oldConfigJson; + assert(result.newConfigJson === null || typeof result.newConfigJson === 'string'); + result.newConfig = safe.JSON.parse(result.newConfigJson); + delete result.newConfigJson; + assert(result.hostPorts === null || typeof result.hostPorts === 'string'); assert(result.environmentVariables === null || typeof result.environmentVariables === 'string'); @@ -305,17 +309,8 @@ function updateWithConstraints(id, app, constraints, callback) { var fields = [ ], values = [ ]; for (var p in app) { - if (p === 'manifest') { - fields.push('manifestJson = ?'); - values.push(JSON.stringify(app[p])); - } else if (p === 'oldConfig') { - fields.push('oldConfigJson = ?'); - values.push(JSON.stringify(app[p])); - } else if (p === 'accessRestriction') { - fields.push('accessRestrictionJson = ?'); - values.push(JSON.stringify(app[p])); - } else if (p === 'debugMode') { - fields.push('debugModeJson = ?'); + if (p === 'manifest' || p === 'oldConfig' || p === 'newConfig' || p === 'accessRestriction' || p === 'debugMode') { + fields.push(`${p}Json = ?`); values.push(JSON.stringify(app[p])); } else if (p !== 'portBindings') { fields.push(p + ' = ?'); diff --git a/src/apps.js b/src/apps.js index f07474636..f6d116a5c 100644 --- a/src/apps.js +++ b/src/apps.js @@ -629,7 +629,7 @@ function update(appId, data, auditSource, callback) { downloadManifest(data.appStoreId, data.manifest, function (error, appStoreId, manifest) { if (error) return callback(error); - var values = { }; + var newConfig = { }; error = manifestFormat.parse(manifest); if (error) return callback(new AppsError(AppsError.BAD_FIELD, 'Manifest error:' + error.message)); @@ -637,11 +637,11 @@ function update(appId, data, auditSource, callback) { error = checkManifestConstraints(manifest); if (error) return callback(error); - values.manifest = manifest; + newConfig.manifest = manifest; if ('portBindings' in data) { - values.portBindings = data.portBindings; - error = validatePortBindings(data.portBindings, values.manifest.tcpPorts); + newConfig.portBindings = data.portBindings; + error = validatePortBindings(data.portBindings, newConfig.manifest.tcpPorts); if (error) return callback(error); } @@ -663,24 +663,22 @@ function update(appId, data, auditSource, callback) { // prevent user from installing a app with different manifest id over an existing app // this allows cloudron install -f --app for an app installed from the appStore - if (app.manifest.id !== values.manifest.id) { + if (app.manifest.id !== newConfig.manifest.id) { if (!data.force) return callback(new AppsError(AppsError.BAD_FIELD, 'manifest id does not match. force to override')); // clear appStoreId so that this app does not get updates anymore - values.appStoreId = ''; + newConfig.appStoreId = ''; } // do not update apps in debug mode if (app.debugMode && !data.force) return callback(new AppsError(AppsError.BAD_STATE, 'debug mode enabled. force to override')); // Ensure we update the memory limit in case the new app requires more memory as a minimum - // 0 and -1 are special values for memory limit indicating unset and unlimited - if (app.memoryLimit > 0 && values.manifest.memoryLimit && app.memoryLimit < values.manifest.memoryLimit) { - values.memoryLimit = values.manifest.memoryLimit; + // 0 and -1 are special newConfig for memory limit indicating unset and unlimited + if (app.memoryLimit > 0 && newConfig.manifest.memoryLimit && app.memoryLimit < newConfig.manifest.memoryLimit) { + newConfig.memoryLimit = newConfig.manifest.memoryLimit; } - values.oldConfig = getAppConfig(app); - - appdb.setInstallationCommand(appId, data.force ? appdb.ISTATE_PENDING_FORCE_UPDATE : appdb.ISTATE_PENDING_UPDATE, values, function (error) { + appdb.setInstallationCommand(appId, data.force ? appdb.ISTATE_PENDING_FORCE_UPDATE : appdb.ISTATE_PENDING_UPDATE, { newConfig: newConfig }, function (error) { if (error && error.reason === DatabaseError.NOT_FOUND) return callback(new AppsError(AppsError.BAD_STATE)); // might be a bad guess if (error && error.reason === DatabaseError.ALREADY_EXISTS) return callback(getDuplicateErrorDetails('' /* location cannot conflict */, values.portBindings, error)); if (error) return callback(new AppsError(AppsError.INTERNAL_ERROR, error)); diff --git a/src/apptask.js b/src/apptask.js index 8452f253d..79c28fff8 100644 --- a/src/apptask.js +++ b/src/apptask.js @@ -202,13 +202,10 @@ function removeLogrotateConfig(app, callback) { shell.sudo('removeLogrotateConfig', [ CONFIGURE_LOGROTATE_CMD, 'remove', app.id ], callback); } -function verifyManifest(app, callback) { - assert.strictEqual(typeof app, 'object'); +function verifyManifest(manifest, callback) { + assert.strictEqual(typeof manifest, 'object'); assert.strictEqual(typeof callback, 'function'); - debugApp(app, 'Verifying manifest'); - - var manifest = app.manifest; var error = manifestFormat.parse(manifest); if (error) return callback(new Error(util.format('Manifest error: %s', error.message))); @@ -386,6 +383,7 @@ function updateApp(app, values, callback) { // - setup addons (requires the above volume) // - setup the container (requires image, volumes, addons) // - setup collectd (requires container id) +// restore is also handled here since restore is just an install with some oldConfig to clean up function install(app, callback) { assert.strictEqual(typeof app, 'object'); assert.strictEqual(typeof callback, 'function'); @@ -393,7 +391,9 @@ function install(app, callback) { const backupId = app.lastBackupId, isRestoring = app.installationState === appdb.ISTATE_PENDING_RESTORE; async.series([ - verifyManifest.bind(null, app), + // this protects against the theoretical possibility of an app being marked for install/restore from + // a previous version of box code + verifyManifest.bind(null, app.manifest), // teardown for re-installs updateApp.bind(null, app, { installationProgress: '10, Cleaning up old install' }), @@ -479,7 +479,6 @@ function backup(app, callback) { assert.strictEqual(typeof app, 'object'); assert.strictEqual(typeof callback, 'function'); - async.series([ updateApp.bind(null, app, { installationProgress: '10, Backing up' }), backups.backupApp.bind(null, app, app.manifest), @@ -575,46 +574,51 @@ function update(app, callback) { assert.strictEqual(typeof app, 'object'); assert.strictEqual(typeof callback, 'function'); - debugApp(app, 'Updating to %s', safe.query(app, 'manifest.version')); + debugApp(app, `Updating to ${app.newConfig.manifest.version}`); // app does not want these addons anymore // FIXME: this does not handle option changes (like multipleDatabases) - var unusedAddons = _.omit(app.oldConfig.manifest.addons, Object.keys(app.manifest.addons)); + var unusedAddons = _.omit(app.manifest.addons, Object.keys(app.newConfig.manifest.addons)); async.series([ + // this protects against the theoretical possibility of an app being marked for update from + // a previous version of box code updateApp.bind(null, app, { installationProgress: '0, Verify manifest' }), - verifyManifest.bind(null, app), - - // download new image before app is stopped. this is so we can reduce downtime - // and also not remove the 'common' layers when the old image is deleted - updateApp.bind(null, app, { installationProgress: '15, Downloading image' }), - docker.downloadImage.bind(null, app.manifest), - - // note: we cleanup first and then backup. this is done so that the app is not running should backup fail - // we cannot easily 'recover' from backup failures because we have to revert manfest and portBindings - updateApp.bind(null, app, { installationProgress: '25, Cleaning up old install' }), - removeCollectdProfile.bind(null, app), - removeLogrotateConfig.bind(null, app), - stopApp.bind(null, app), - deleteContainers.bind(null, app), - function deleteImageIfChanged(done) { - if (app.oldConfig.manifest.dockerImage === app.manifest.dockerImage) return done(); - - docker.deleteImage(app.oldConfig.manifest, done); - }, + verifyManifest.bind(null, app.newConfig.manifest), function (next) { if (app.installationState === appdb.ISTATE_PENDING_FORCE_UPDATE) return next(null); async.series([ - updateApp.bind(null, app, { installationProgress: '30, Backing up app' }), - backups.backupApp.bind(null, app, app.oldConfig.manifest) + updateApp.bind(null, app, { installationProgress: '15, Backing up app' }), + backups.backupApp.bind(null, app, app.manifest) ], next); }, + // download new image before app is stopped. this is so we can reduce downtime + // and also not remove the 'common' layers when the old image is deleted + updateApp.bind(null, app, { installationProgress: '25, Downloading image' }), + docker.downloadImage.bind(null, app.newConfig.manifest), + + // note: we cleanup first and then backup. this is done so that the app is not running should backup fail + // we cannot easily 'recover' from backup failures because we have to revert manfest and portBindings + updateApp.bind(null, app, { installationProgress: '35, Cleaning up old install' }), + removeCollectdProfile.bind(null, app), + removeLogrotateConfig.bind(null, app), + stopApp.bind(null, app), + deleteContainers.bind(null, app), + function deleteImageIfChanged(done) { + if (app.manifest.dockerImage === app.newConfig.manifest.dockerImage) return done(); + + docker.deleteImage(app.manifest, done); + }, + // only delete unused addons after backup addons.teardownAddons.bind(null, app, unusedAddons), + // switch over to the new config. manifest, memoryLimit, portBindings, appstoreId are updated here + updateApp.bind(null, app, app.newConfig), + updateApp.bind(null, app, { installationProgress: '45, Downloading icon' }), downloadIcon.bind(null, app), @@ -635,7 +639,7 @@ function update(app, callback) { // done! function (callback) { debugApp(app, 'updated'); - updateApp(app, { installationState: appdb.ISTATE_INSTALLED, installationProgress: '', health: null }, callback); + updateApp(app, { installationState: appdb.ISTATE_INSTALLED, installationProgress: '', health: null, newConfig: null }, callback); } ], function seriesDone(error) { if (error) { diff --git a/src/test/apptask-test.js b/src/test/apptask-test.js index dc3e43c49..c828fd28a 100644 --- a/src/test/apptask-test.js +++ b/src/test/apptask-test.js @@ -181,7 +181,7 @@ describe('apptask', function () { var badApp = _.extend({ }, APP); badApp.manifest = { }; - apptask._verifyManifest(badApp, function (error) { + apptask._verifyManifest(badApp.manifest, function (error) { expect(error).to.be.ok(); done(); }); @@ -192,7 +192,7 @@ describe('apptask', function () { badApp.manifest = _.extend({ }, APP.manifest); delete badApp.manifest.id; - apptask._verifyManifest(badApp, function (error) { + apptask._verifyManifest(badApp.manifest, function (error) { expect(error).to.be.ok(); done(); }); @@ -203,7 +203,7 @@ describe('apptask', function () { badApp.manifest = _.extend({ }, APP.manifest); badApp.manifest.maxBoxVersion = '0.0.0'; // max box version is too small - apptask._verifyManifest(badApp, function (error) { + apptask._verifyManifest(badApp.manifest, function (error) { expect(error).to.be.ok(); done(); }); @@ -212,7 +212,7 @@ describe('apptask', function () { it('verifies manifest', function (done) { var goodApp = _.extend({ }, APP); - apptask._verifyManifest(goodApp, function (error) { + apptask._verifyManifest(goodApp.manifest, function (error) { expect(error).to.be(null); done(); });