remove httpPort

we can just use container IP instead of all this httpPort exporting magic.
this is also required for exposing httpPaths feature (we have to otherwise
have multiple httpPorts).

migrations/20201119072309-apps-remove-httpPort.js

@@ -0,0 +1,13 @@
+'use strict';
+
+var async = require('async');
+
+exports.up = function(db, callback) {
+    async.series([
+        db.runSql.bind(db, 'ALTER TABLE apps DROP COLUMN httpPort')
+    ], callback);
+};
+
+exports.down = function(db, callback) {
+    callback();
+};

migrations/schema.sql

@@ -65,7 +65,6 @@ CREATE TABLE IF NOT EXISTS apps(
     healthTime TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, // when the app last responded
     containerId VARCHAR(128),
     manifestJson TEXT,
-    httpPort INTEGER,                        // this is the nginx proxy port and not manifest.httpPort
     accessRestrictionJson TEXT, // { users: [ ], groups: [ ] }
     creationTime TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, // when the app was installed
     updateTime TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP,   // when the last app update was done

src/appdb.js

@@ -2,7 +2,6 @@
 
 exports = module.exports = {
     get: get,
-    getByHttpPort: getByHttpPort,
     getByContainerId: getByContainerId,
     add: add,
     exists: exists,
@@ -39,7 +38,7 @@ var assert = require('assert'),
     util = require('util');
 
 var APPS_FIELDS_PREFIXED = [ 'apps.id', 'apps.appStoreId', 'apps.installationState', 'apps.errorJson', 'apps.runState',
-    'apps.health', 'apps.containerId', 'apps.manifestJson', 'apps.httpPort', 'subdomains.subdomain AS location', 'subdomains.domain',
+    'apps.health', 'apps.containerId', 'apps.manifestJson', 'subdomains.subdomain AS location', 'subdomains.domain',
     'apps.accessRestrictionJson', 'apps.memoryLimit', 'apps.cpuShares',
     'apps.label', 'apps.tagsJson', 'apps.taskId', 'apps.reverseProxyConfigJson', 'apps.servicesConfigJson',
     'apps.sso', 'apps.debugModeJson', 'apps.enableBackup', 'apps.proxyAuth',
@@ -155,34 +154,6 @@ function get(id, callback) {
     });
 }
 
-function getByHttpPort(httpPort, callback) {
-    assert.strictEqual(typeof httpPort, 'number');
-    assert.strictEqual(typeof callback, 'function');
-
-    database.query('SELECT ' + APPS_FIELDS_PREFIXED + ','
-    + 'GROUP_CONCAT(CAST(appPortBindings.hostPort AS CHAR(6))) AS hostPorts, GROUP_CONCAT(appPortBindings.environmentVariable) AS environmentVariables, GROUP_CONCAT(appPortBindings.type) AS portTypes,'
-    + 'JSON_ARRAYAGG(appEnvVars.name) AS envNames, JSON_ARRAYAGG(appEnvVars.value) AS envValues,'
-    + 'JSON_ARRAYAGG(appMounts.volumeId) AS volumeIds, JSON_ARRAYAGG(appMounts.readOnly) AS volumeReadOnlys '
-        + ' FROM apps'
-        + '  LEFT OUTER JOIN appPortBindings ON apps.id = appPortBindings.appId'
-        + '  LEFT OUTER JOIN appEnvVars ON apps.id = appEnvVars.appId'
-        + '  LEFT OUTER JOIN subdomains ON apps.id = subdomains.appId AND subdomains.type = ?'
-        + '  LEFT OUTER JOIN appMounts ON apps.id = appMounts.appId'
-        + ' WHERE httpPort = ? GROUP BY apps.id', [ exports.SUBDOMAIN_TYPE_PRIMARY, httpPort ], function (error, result) {
-        if (error) return callback(new BoxError(BoxError.DATABASE_ERROR, error));
-        if (result.length === 0) return callback(new BoxError(BoxError.NOT_FOUND, 'App not found'));
-
-        database.query('SELECT ' + SUBDOMAIN_FIELDS + ' FROM subdomains WHERE appId = ? AND type = ?', [ result[0].id, exports.SUBDOMAIN_TYPE_REDIRECT ], function (error, alternateDomains) {
-            if (error) return callback(new BoxError(BoxError.DATABASE_ERROR, error));
-
-            result[0].alternateDomains = alternateDomains;
-            postProcess(result[0]);
-
-            callback(null, result[0]);
-        });
-    });
-}
-
 function getByContainerId(containerId, callback) {
     assert.strictEqual(typeof containerId, 'string');
     assert.strictEqual(typeof callback, 'function');

src/apphealthmonitor.js

@@ -85,8 +85,11 @@ function checkAppHealth(app, callback) {
         // non-appstore apps may not have healthCheckPath
         if (!manifest.healthCheckPath) return setHealth(app, apps.HEALTH_HEALTHY, callback);
 
+        const ip = safe.query(data, 'NetworkSettings.Networks.cloudron.IPAddress', null);
+        if (!ip) return setHealth(app, apps.HEALTH_ERROR, callback);
+
         // poll through docker network instead of nginx to bypass any potential oauth proxy
-        var healthCheckUrl = 'http://127.0.0.1:' + app.httpPort + manifest.healthCheckPath;
+        var healthCheckUrl = `http://${ip}:${manifest.httpPort}${manifest.healthCheckPath}`;
         superagent
             .get(healthCheckUrl)
             .set('Host', app.fqdn) // required for some apache configs with rewrite rules

src/apptask.js

@@ -6,7 +6,6 @@ exports = module.exports = {
     run: run,
 
     // exported for testing
-    _reserveHttpPort: reserveHttpPort,
     _configureReverseProxy: configureReverseProxy,
     _unconfigureReverseProxy: unconfigureReverseProxy,
     _createAppDir: createAppDir,
@@ -35,7 +34,6 @@ var addons = require('./addons.js'),
     eventlog = require('./eventlog.js'),
     fs = require('fs'),
     manifestFormat = require('cloudron-manifestformat'),
-    net = require('net'),
     os = require('os'),
     path = require('path'),
     paths = require('./paths.js'),
@@ -89,24 +87,6 @@ function updateApp(app, values, callback) {
     });
 }
 
-function reserveHttpPort(app, callback) {
-    assert.strictEqual(typeof app, 'object');
-    assert.strictEqual(typeof callback, 'function');
-
-    let server = net.createServer();
-    server.listen(0, function () {
-        let port = server.address().port;
-
-        updateApp(app, { httpPort: port }, function (error) {
-            server.close(function (/* closeError */) {
-                if (error) return callback(new BoxError(BoxError.NETWORK_ERROR, `Failed to allocate http port ${port}: ${error.message}`));
-
-                callback(null);
-            });
-        });
-    });
-}
-
 function configureReverseProxy(app, callback) {
     assert.strictEqual(typeof app, 'object');
     assert.strictEqual(typeof callback, 'function');
@@ -164,6 +144,7 @@ function deleteContainers(app, options, callback) {
         removeLogrotateConfig.bind(null, app),
         docker.stopContainers.bind(null, app.id),
         docker.deleteContainers.bind(null, app.id, options),
+        unconfigureReverseProxy.bind(null, app),
         updateApp.bind(null, app, { containerId: null })
     ], callback);
 }
@@ -486,7 +467,10 @@ function downloadImage(manifest, callback) {
 function startApp(app, callback){
     if (app.runState === apps.RSTATE_STOPPED) return callback();
 
-    docker.startContainer(app.id, callback);
+    async.series([
+        docker.startContainer.bind(null, app.id),
+        configureReverseProxy.bind(null, app),
+    ], callback);
 }
 
 function install(app, args, progressCallback, callback) {
@@ -506,7 +490,6 @@ function install(app, args, progressCallback, callback) {
 
         // teardown for re-installs
         progressCallback.bind(null, { percent: 10, message: 'Cleaning up old install' }),
-        unconfigureReverseProxy.bind(null, app),
         deleteContainers.bind(null, app, { managedOnly: true }),
         function teardownAddons(next) {
             // when restoring, app does not require these addons anymore. remove carefully to preserve the db passwords
@@ -532,8 +515,6 @@ function install(app, args, progressCallback, callback) {
             docker.deleteImage(oldManifest, done);
         },
 
-        reserveHttpPort.bind(null, app),
-
         progressCallback.bind(null, { percent: 20, message: 'Downloading icon' }),
         downloadIcon.bind(null, app),
 
@@ -580,9 +561,6 @@ function install(app, args, progressCallback, callback) {
         progressCallback.bind(null, { percent: 85, message: 'Waiting for DNS propagation' }),
         exports._waitForDnsPropagation.bind(null, app),
 
-        progressCallback.bind(null, { percent: 95, message: 'Configuring reverse proxy' }),
-        configureReverseProxy.bind(null, app),
-
         progressCallback.bind(null, { percent: 100, message: 'Done' }),
         updateApp.bind(null, app, { installationState: apps.ISTATE_INSTALLED, error: null, health: null })
     ], function seriesDone(error) {
@@ -660,7 +638,6 @@ function changeLocation(app, args, progressCallback, callback) {
 
     async.series([
         progressCallback.bind(null, { percent: 10, message: 'Cleaning up old install' }),
-        unconfigureReverseProxy.bind(null, app),
         deleteContainers.bind(null, app, { managedOnly: true }),
         function (next) {
             let obsoleteDomains = oldConfig.alternateDomains.filter(function (o) {
@@ -689,9 +666,6 @@ function changeLocation(app, args, progressCallback, callback) {
         progressCallback.bind(null, { percent: 80, message: 'Waiting for DNS propagation' }),
         exports._waitForDnsPropagation.bind(null, app),
 
-        progressCallback.bind(null, { percent: 90, message: 'Configuring reverse proxy' }),
-        configureReverseProxy.bind(null, app),
-
         progressCallback.bind(null, { percent: 100, message: 'Done' }),
         updateApp.bind(null, app, { installationState: apps.ISTATE_INSTALLED, error: null, health: null })
     ], function seriesDone(error) {
@@ -752,9 +726,7 @@ function configure(app, args, progressCallback, callback) {
 
     async.series([
         progressCallback.bind(null, { percent: 10, message: 'Cleaning up old install' }),
-        unconfigureReverseProxy.bind(null, app),
         deleteContainers.bind(null, app, { managedOnly: true }),
-        reserveHttpPort.bind(null, app),
 
         progressCallback.bind(null, { percent: 20, message: 'Downloading icon' }),
         downloadIcon.bind(null, app),
@@ -774,9 +746,6 @@ function configure(app, args, progressCallback, callback) {
 
         startApp.bind(null, app),
 
-        progressCallback.bind(null, { percent: 90, message: 'Configuring reverse proxy' }),
-        configureReverseProxy.bind(null, app),
-
         progressCallback.bind(null, { percent: 100, message: 'Done' }),
         updateApp.bind(null, app, { installationState: apps.ISTATE_INSTALLED, error: null, health: null })
     ], function seriesDone(error) {
@@ -789,7 +758,6 @@ function configure(app, args, progressCallback, callback) {
     });
 }
 
-// nginx configuration is skipped because app.httpPort is expected to be available
 function update(app, args, progressCallback, callback) {
     assert.strictEqual(typeof app, 'object');
     assert.strictEqual(typeof args, 'object');
@@ -978,7 +946,6 @@ function uninstall(app, args, progressCallback, callback) {
 
     async.series([
         progressCallback.bind(null, { percent: 20, message: 'Deleting container' }),
-        unconfigureReverseProxy.bind(null, app),
         deleteContainers.bind(null, app, {}),
 
         progressCallback.bind(null, { percent: 30, message: 'Teardown addons' }),

src/docker.js

@@ -1,34 +1,35 @@
 'use strict';
 
 exports = module.exports = {
-    testRegistryConfig: testRegistryConfig,
+    testRegistryConfig,
-    setRegistryConfig: setRegistryConfig,
+    setRegistryConfig,
-    injectPrivateFields: injectPrivateFields,
+    injectPrivateFields,
-    removePrivateFields: removePrivateFields,
+    removePrivateFields,
 
-    ping: ping,
+    ping,
 
-    info: info,
+    info,
-    downloadImage: downloadImage,
+    downloadImage,
-    createContainer: createContainer,
+    createContainer,
-    startContainer: startContainer,
+    startContainer,
-    restartContainer: restartContainer,
+    restartContainer,
-    stopContainer: stopContainer,
+    stopContainer,
     stopContainerByName: stopContainer,
-    stopContainers: stopContainers,
+    stopContainers,
-    deleteContainer: deleteContainer,
+    deleteContainer,
-    deleteImage: deleteImage,
+    deleteImage,
-    deleteContainers: deleteContainers,
+    deleteContainers,
-    createSubcontainer: createSubcontainer,
+    createSubcontainer,
-    getContainerIdByIp: getContainerIdByIp,
+    getContainerIdByIp,
-    inspect: inspect,
+    inspect,
+    getContainerIp,
     inspectByName: inspect,
-    execContainer: execContainer,
+    execContainer,
-    getEvents: getEvents,
+    getEvents,
-    memoryUsage: memoryUsage,
+    memoryUsage,
-    createVolume: createVolume,
+    createVolume,
-    removeVolume: removeVolume,
+    removeVolume,
-    clearVolume: clearVolume
+    clearVolume
 };
 
 var addons = require('./addons.js'),
@@ -246,11 +247,6 @@ function createSubcontainer(app, name, cmd, options, callback) {
         `${envPrefix}APP_DOMAIN=${domain}`
     ];
 
-    // docker portBindings requires ports to be exposed
-    exposedPorts[manifest.httpPort + '/tcp'] = {};
-
-    dockerPortBindings[manifest.httpPort + '/tcp'] = [ { HostIp: '127.0.0.1', HostPort: app.httpPort + '' } ];
-
     var portEnv = [];
     for (let portName in app.portBindings) {
         const hostPort = app.portBindings[portName];
@@ -259,6 +255,7 @@ function createSubcontainer(app, name, cmd, options, callback) {
 
         var containerPort = ports[portName].containerPort || hostPort;
 
+        // docker portBindings requires ports to be exposed
         exposedPorts[`${containerPort}/${portType}`] = {};
         portEnv.push(`${portName}=${hostPort}`);
 
@@ -560,6 +557,22 @@ function inspect(containerId, callback) {
     });
 }
 
+function getContainerIp(containerId, callback) {
+    assert.strictEqual(typeof containerId, 'string');
+    assert.strictEqual(typeof callback, 'function');
+
+    if (constants.TEST) return callback(null, '127.0.5.5');
+
+    inspect(containerId, function (error, result) {
+        if (error) return callback(error);
+
+        const ip = safe.query(result, 'NetworkSettings.Networks.cloudron.IPAddress', null);
+        if (!ip) return callback(new BoxError(BoxError.DOCKER_ERROR, 'Error getting container IP'));
+
+        callback(null, ip);
+    });
+}
+
 function execContainer(containerId, options, callback) {
     assert.strictEqual(typeof containerId, 'string');
     assert.strictEqual(typeof options, 'object');

src/nginxconfig.ejs

@@ -147,7 +147,7 @@ server {
 <% if ( endpoint === 'admin' ) { %>
         proxy_pass http://127.0.0.1:3000;
 <% } else if ( endpoint === 'app' ) { %>
-        proxy_pass http://127.0.0.1:<%= port %>;
+        proxy_pass http://<%= ip %>:<%= port %>;
 <% } else if ( endpoint === 'redirect' ) { %>
         return 302 https://<%= redirectTo %>$request_uri;
 <% } %>
@@ -245,7 +245,7 @@ server {
         error_page 401 = @proxy-auth-login;
     <% } %>
 
-        proxy_pass http://127.0.0.1:<%= port %>;
+        proxy_pass http://<%= ip %>:<%= port %>;
 <% } else if ( endpoint === 'redirect' ) { %>
         # redirect everything to the app. this is temporary because there is no way
         # to clear a permanent redirect on the browser

src/reverseproxy.js

@@ -38,6 +38,7 @@ var acme2 = require('./cert/acme2.js'),
     constants = require('./constants.js'),
     crypto = require('crypto'),
     debug = require('debug')('box:reverseproxy'),
+    docker = require('./docker.js'),
     domains = require('./domains.js'),
     ejs = require('ejs'),
     eventlog = require('./eventlog.js'),
@@ -160,7 +161,7 @@ function validateCertificate(location, domainObject, certificate) {
 }
 
 function reload(callback) {
-    if (process.env.BOX_ENV === 'test') return callback();
+    if (constants.TEST) return callback();
 
     shell.sudo('reload', [ RELOAD_NGINX_CMD ], {}, function (error) {
         if (error) return callback(new BoxError(BoxError.NGINX_ERROR, `Error reloading nginx: ${error.message}`));
@@ -433,6 +434,9 @@ function writeAppNginxConfig(app, bundle, callback) {
     assert.strictEqual(typeof bundle, 'object');
     assert.strictEqual(typeof callback, 'function');
 
+    docker.getContainerIp(app.containerId, function (error, ip) {
+        if (error) return callback(error);
+
         var sourceDir = path.resolve(__dirname, '..');
         var endpoint = 'app';
 
@@ -450,7 +454,8 @@ function writeAppNginxConfig(app, bundle, callback) {
             adminOrigin: settings.adminOrigin(),
             vhost: app.fqdn,
             hasIPv6: sysinfo.hasIPv6(),
-        port: app.httpPort,
+            ip,
+            port: app.manifest.httpPort,
             endpoint: endpoint,
             certFilePath: bundle.certFilePath,
             keyFilePath: bundle.keyFilePath,
@@ -473,6 +478,7 @@ function writeAppNginxConfig(app, bundle, callback) {
         }
 
         reload(callback);
+    });
 }
 
 function writeAppRedirectNginxConfig(app, fqdn, bundle, callback) {

src/test/apptask-test.js

@@ -14,7 +14,6 @@ var appdb = require('../appdb.js'),
     expect = require('expect.js'),
     fs = require('fs'),
     js2xml = require('js2xmlparser').parse,
-    net = require('net'),
     nock = require('nock'),
     paths = require('../paths.js'),
     settings = require('../settings.js'),
@@ -87,13 +86,12 @@ var APP = {
     domain: DOMAIN_0.domain,
     fqdn: DOMAIN_0.domain + '.' + 'applocation',
     manifest: MANIFEST,
-    containerId: null,
+    containerId: 'someid',
-    httpPort: 4567,
     portBindings: null,
     accessRestriction: null,
     memoryLimit: 0,
     mailboxDomain: DOMAIN_0.domain,
-    alternateDomains: []
+    alternateDomains: [],
 };
 
 var awsHostedZones;
@@ -132,28 +130,18 @@ describe('apptask', function () {
         ], done);
     });
 
-    it('reserve port', function (done) {
-        apptask._reserveHttpPort(APP, function (error) {
-            expect(error).to.not.be.ok();
-            expect(APP.httpPort).to.be.a('number');
-            var client = net.connect(APP.httpPort);
-            client.on('connect', function () { done(new Error('Port is not free:' + APP.httpPort)); });
-            client.on('error', function () { done(); });
-        });
-    });
-
     it('configure nginx correctly', function (done) {
-        apptask._configureReverseProxy(APP, function () {
+        apptask._configureReverseProxy(APP, function (error) {
             expect(fs.existsSync(paths.NGINX_APPCONFIG_DIR + '/' + APP.id + '.conf'));
-            // expect(error).to.be(null); // this fails because nginx cannot be restarted
+            expect(error).to.be(null);
             done();
         });
     });
 
     it('unconfigure nginx', function (done) {
-        apptask._unconfigureReverseProxy(APP, function () {
+        apptask._unconfigureReverseProxy(APP, function (error) {
             expect(!fs.existsSync(paths.NGINX_APPCONFIG_DIR + '/' + APP.id + '.conf'));
-            // expect(error).to.be(null); // this fails because nginx cannot be restarted
+            expect(error).to.be(null);
             done();
         });
     });

src/test/backups-test.js

@@ -112,6 +112,7 @@ describe('retention policy', function () {
         expect(b[3].keepReason).to.be(undefined);
     });
 
+    // if you are debugging this test, it's because of some timezone issue with all the hour substraction!
     it('2 daily, 1 weekly', function () {
         let b = [
             { id: '0', state: backups.BACKUP_STATE_NORMAL, creationTime: moment().toDate() },

src/test/database-test.js

@@ -397,7 +397,6 @@ describe('database', function () {
             location: 'some-location-0',
             domain: DOMAIN_0.domain,
             manifest: { version: '0.1', dockerImage: 'docker/app0', healthCheckPath: '/', httpPort: 80, title: 'app0' },
-            httpPort: null,
             containerId: null,
             portBindings: { port: { hostPort: 5678, type: 'tcp' } },
             health: null,
@@ -869,7 +868,6 @@ describe('database', function () {
             location: 'some-location-0',
             domain: DOMAIN_0.domain,
             manifest: { version: '0.1', dockerImage: 'docker/app0', healthCheckPath: '/', httpPort: 80, title: 'app0' },
-            httpPort: null,
             containerId: null,
             portBindings: { port: { hostPort: 5678, type: 'tcp' } },
             health: null,
@@ -905,7 +903,6 @@ describe('database', function () {
             location: 'some-location-1',
             domain: DOMAIN_0.domain,
             manifest: { version: '0.2', dockerImage: 'docker/app1', healthCheckPath: '/', httpPort: 80, title: 'app1' },
-            httpPort: null,
             containerId: null,
             portBindings: { },
             health: null,
@@ -1009,7 +1006,6 @@ describe('database', function () {
             APP_0.location = 'some-other-location';
             APP_0.manifest.version = '0.2';
             APP_0.accessRestriction = '';
-            APP_0.httpPort = 1337;
             APP_0.memoryLimit = 1337;
             APP_0.cpuShares = 1024;
 
@@ -1019,7 +1015,6 @@ describe('database', function () {
                 domain: APP_0.domain,
                 manifest: APP_0.manifest,
                 accessRestriction: APP_0.accessRestriction,
-                httpPort: APP_0.httpPort,
                 memoryLimit: APP_0.memoryLimit,
                 cpuShares: APP_0.cpuShares
             };
@@ -1036,15 +1031,6 @@ describe('database', function () {
             });
         });
 
-        it('getByHttpPort succeeds', function (done) {
-            appdb.getByHttpPort(APP_0.httpPort, function (error, result) {
-                expect(error).to.be(null);
-                expect(result).to.be.an('object');
-                expect(_.omit(result, ['creationTime', 'updateTime', 'ts', 'healthTime','resetTokenCreationTime'])).to.be.eql(APP_0);
-                done();
-            });
-        });
-
         it('update of nonexisting app fails', function (done) {
             appdb.update(APP_1.id, { installationState: APP_1.installationState, location: APP_1.location }, function (error) {
                 expect(error).to.be.a(BoxError);

src/test/ldap-test.js

@@ -74,7 +74,6 @@ var APP_0 = {
     location: 'some-location-0',
     domain: DOMAIN_0.domain,
     manifest: { version: '0.1', dockerImage: 'docker/app0', healthCheckPath: '/', httpPort: 80, title: 'app0' },
-    httpPort: null,
     containerId: 'someContainerId',
     portBindings: { port: 5678 },
     health: null,

src/test/updatechecker-test.js

@@ -233,7 +233,6 @@ describe('updatechecker - app - manual (email)', function () {
                 }
             }
         },
-        httpPort: null,
         containerId: null,
         portBindings: { PORT: 5678 },
         healthy: null,
@@ -342,7 +341,6 @@ describe('updatechecker - app - automatic (no email)', function () {
                 }
             }
         },
-        httpPort: null,
         containerId: null,
         portBindings: { PORT: 5678 },
         healthy: null,
@@ -407,7 +405,6 @@ describe('updatechecker - app - automatic free (email)', function () {
                 }
             }
         },
-        httpPort: null,
         containerId: null,
         portBindings: { PORT: 5678 },
         healthy: null,