diff --git a/dashboard/src/js/client.js b/dashboard/src/js/client.js index 5ee82d5b7..410d3bcd2 100644 --- a/dashboard/src/js/client.js +++ b/dashboard/src/js/client.js @@ -957,7 +957,7 @@ angular.module('Application').service('Client', ['$http', '$interval', '$timeout subdomain: config.subdomain, domain: config.domain, secondaryDomains: config.secondaryDomains, - portBindings: config.portBindings, + ports: config.ports, accessRestriction: config.accessRestriction, cert: config.cert, key: config.key, @@ -979,7 +979,7 @@ angular.module('Application').service('Client', ['$http', '$interval', '$timeout subdomain: config.subdomain, domain: config.domain, secondaryDomains: config.secondaryDomains, - portBindings: config.portBindings, + ports: config.ports, backupId: config.backupId, overwriteDns: !!config.overwriteDns }; diff --git a/dashboard/src/views/app.html b/dashboard/src/views/app.html index 72d919856..3967aab80 100644 --- a/dashboard/src/views/app.html +++ b/dashboard/src/views/app.html @@ -626,17 +626,17 @@

{{ clone.error.port }}
-
+
-
@@ -932,18 +932,18 @@
{{ location.error.port }}
-
+
-
diff --git a/dashboard/src/views/app.js b/dashboard/src/views/app.js index 0709dd756..9cf0d3bbd 100644 --- a/dashboard/src/views/app.js +++ b/dashboard/src/views/app.js @@ -305,9 +305,9 @@ angular.module('Application').controller('AppController', ['$scope', '$location' secondaryDomains: {}, redirectDomains: [], aliasDomains: [], - portBindings: {}, - portBindingsEnabled: {}, - portBindingsInfo: {}, + ports: {}, + portsEnabled: {}, + portInfo: {}, addRedirectDomain: function (event) { event.preventDefault(); @@ -369,18 +369,18 @@ angular.module('Application').controller('AppController', ['$scope', '$location' }; }); - $scope.location.portBindingsInfo = angular.extend({}, app.manifest.tcpPorts, app.manifest.udpPorts); // Portbinding map only for information + $scope.location.portInfo = angular.extend({}, app.manifest.tcpPorts, app.manifest.udpPorts); // Portbinding map only for information $scope.location.redirectDomains = app.redirectDomains.map(function (a) { return { subdomain: a.subdomain, domain: $scope.domains.filter(function (d) { return d.domain === a.domain; })[0] };}); $scope.location.aliasDomains = app.aliasDomains.map(function (a) { return { subdomain: a.subdomain, domain: $scope.domains.filter(function (d) { return d.domain === a.domain; })[0] };}); // fill the portBinding structures. There might be holes in the app.portBindings, which signalizes a disabled port - for (var env in $scope.location.portBindingsInfo) { + for (var env in $scope.location.portInfo) { if (app.portBindings && app.portBindings[env]) { - $scope.location.portBindings[env] = app.portBindings[env]; - $scope.location.portBindingsEnabled[env] = true; + $scope.location.ports[env] = app.portBindings[env].hostPort; + $scope.location.portsEnabled[env] = true; } else { - $scope.location.portBindings[env] = $scope.location.portBindingsInfo[env].defaultValue || 0; - $scope.location.portBindingsEnabled[env] = false; + $scope.location.ports[env] = $scope.location.portInfo[env].defaultValue || 0; + $scope.location.portsEnabled[env] = false; } } }, @@ -400,11 +400,11 @@ angular.module('Application').controller('AppController', ['$scope', '$location' }; } - // only use enabled ports from portBindings - var portBindings = {}; - for (var env in $scope.location.portBindings) { - if ($scope.location.portBindingsEnabled[env]) { - portBindings[env] = $scope.location.portBindings[env]; + // only use enabled ports + var ports = {}; + for (var env in $scope.location.ports) { + if ($scope.location.portsEnabled[env]) { + ports[env] = $scope.location.ports[env]; } } @@ -412,7 +412,7 @@ angular.module('Application').controller('AppController', ['$scope', '$location' overwriteDns: !!overwriteDns, subdomain: $scope.location.subdomain, domain: $scope.location.domain.domain, - portBindings: portBindings, + ports: ports, secondaryDomains: secondaryDomains, redirectDomains: $scope.location.redirectDomains.map(function (a) { return { subdomain: a.subdomain, domain: a.domain.domain };}), aliasDomains: $scope.location.aliasDomains.map(function (a) { return { subdomain: a.subdomain, domain: a.domain.domain };}) @@ -1794,9 +1794,9 @@ angular.module('Application').controller('AppController', ['$scope', '$location' secondaryDomains: {}, needsOverwrite: false, overwriteDns: false, - portBindings: {}, - portBindingsInfo: {}, - portBindingsEnabled: {}, + ports: {}, + portsEnabled: {}, + portInfo: {}, show: function (backup) { var app = $scope.app; @@ -1818,11 +1818,11 @@ angular.module('Application').controller('AppController', ['$scope', '$location' }; } - $scope.clone.portBindingsInfo = angular.extend({}, backup.manifest.tcpPorts, backup.manifest.udpPorts); // Portbinding map only for information + $scope.clone.portInfo = angular.extend({}, backup.manifest.tcpPorts, backup.manifest.udpPorts); // Portbinding map only for information // set default ports - for (var env in $scope.clone.portBindingsInfo) { - $scope.clone.portBindings[env] = $scope.clone.portBindingsInfo[env].defaultValue || 0; - $scope.clone.portBindingsEnabled[env] = true; + for (var env in $scope.clone.portInfo) { + $scope.clone.ports[env] = $scope.clone.portInfo[env].defaultValue || 0; + $scope.clone.portsEnabled[env] = true; } $('#appCloneModal').modal('show'); @@ -1839,11 +1839,11 @@ angular.module('Application').controller('AppController', ['$scope', '$location' }; } - // only use enabled ports from portBindings - var finalPortBindings = {}; - for (var env in $scope.clone.portBindings) { - if ($scope.clone.portBindingsEnabled[env]) { - finalPortBindings[env] = $scope.clone.portBindings[env]; + // only use enabled ports + var finalPorts = {}; + for (var env in $scope.clone.ports) { + if ($scope.clone.portsEnabled[env]) { + finalPorts[env] = $scope.clone.ports[env]; } } @@ -1851,7 +1851,7 @@ angular.module('Application').controller('AppController', ['$scope', '$location' subdomain: $scope.clone.subdomain, domain: $scope.clone.domain.domain, secondaryDomains: secondaryDomains, - portBindings: finalPortBindings, + ports: finalPorts, backupId: $scope.clone.backup.id, overwriteDns: $scope.clone.overwriteDns }; diff --git a/dashboard/src/views/appstore.html b/dashboard/src/views/appstore.html index 6b29ad283..855bc89d5 100644 --- a/dashboard/src/views/appstore.html +++ b/dashboard/src/views/appstore.html @@ -69,17 +69,17 @@
{{ appInstall.error.port }}
-
+
-
diff --git a/dashboard/src/views/appstore.js b/dashboard/src/views/appstore.js index d69f7eee5..a66a6c1e6 100644 --- a/dashboard/src/views/appstore.js +++ b/dashboard/src/views/appstore.js @@ -122,7 +122,8 @@ angular.module('Application').controller('AppStoreController', ['$scope', '$tran subdomain: '', domain: null, // object and not the string secondaryDomains: {}, - portBindings: {}, + ports: {}, + portsEnabled: {}, mediaLinks: [], certificateFile: null, certificateFileName: '', @@ -147,7 +148,7 @@ angular.module('Application').controller('AppStoreController', ['$scope', '$tran $scope.appInstall.subdomain = ''; $scope.appInstall.domain = null; $scope.appInstall.secondaryDomains = {}; - $scope.appInstall.portBindings = {}; + $scope.appInstall.ports = {}; $scope.appInstall.state = 'appInfo'; $scope.appInstall.mediaLinks = []; $scope.appInstall.certificateFile = null; @@ -218,9 +219,9 @@ angular.module('Application').controller('AppStoreController', ['$scope', '$tran }; } - $scope.appInstall.portBindingsInfo = angular.extend({}, $scope.appInstall.app.manifest.tcpPorts, $scope.appInstall.app.manifest.udpPorts); // Portbinding map only for information - $scope.appInstall.portBindings = {}; // This is the actual model holding the env:port pair - $scope.appInstall.portBindingsEnabled = {}; // This is the actual model holding the enabled/disabled flag + $scope.appInstall.portInfo = angular.extend({}, $scope.appInstall.app.manifest.tcpPorts, $scope.appInstall.app.manifest.udpPorts); // Portbinding map only for information + $scope.appInstall.ports = {}; // This holds the env:port pair + $scope.appInstall.portsEnabled = {}; // This holds the enabled/disabled flag var manifest = app.manifest; $scope.appInstall.optionalSso = !!manifest.optionalSso; @@ -232,8 +233,8 @@ angular.module('Application').controller('AppStoreController', ['$scope', '$tran // set default ports var allPorts = angular.extend({}, $scope.appInstall.app.manifest.tcpPorts, $scope.appInstall.app.manifest.udpPorts); for (var env in allPorts) { - $scope.appInstall.portBindings[env] = allPorts[env].defaultValue || 0; - $scope.appInstall.portBindingsEnabled[env] = true; + $scope.appInstall.ports[env] = allPorts[env].defaultValue || 0; + $scope.appInstall.portsEnabled[env] = true; } $('#appInstallModal').modal('show'); @@ -253,11 +254,11 @@ angular.module('Application').controller('AppStoreController', ['$scope', '$tran }; } - // only use enabled ports from portBindings - var finalPortBindings = {}; - for (var env in $scope.appInstall.portBindings) { - if ($scope.appInstall.portBindingsEnabled[env]) { - finalPortBindings[env] = $scope.appInstall.portBindings[env]; + // only use enabled ports from ports + var finalPorts = {}; + for (var env in $scope.appInstall.ports) { + if ($scope.appInstall.portsEnabled[env]) { + finalPorts[env] = $scope.appInstall.ports[env]; } } @@ -273,7 +274,7 @@ angular.module('Application').controller('AppStoreController', ['$scope', '$tran subdomain: $scope.appInstall.subdomain || '', domain: $scope.appInstall.domain.domain, secondaryDomains: secondaryDomains, - portBindings: finalPortBindings, + ports: finalPorts, accessRestriction: finalAccessRestriction, cert: $scope.appInstall.certificateFile, key: $scope.appInstall.keyFile, diff --git a/migrations/20240716214414-appPortBindings-make-count-not-null.js b/migrations/20240716214414-appPortBindings-make-count-not-null.js new file mode 100644 index 000000000..8655b0873 --- /dev/null +++ b/migrations/20240716214414-appPortBindings-make-count-not-null.js @@ -0,0 +1,9 @@ +'use strict'; + +exports.up = async function(db) { + await db.runSql('UPDATE appPortBindings SET count=1 WHERE count IS NULL'); + await db.runSql('ALTER TABLE appPortBindings MODIFY count INTEGER NOT NULL DEFAULT 1'); +}; + +exports.down = async function(/* db */) { +}; diff --git a/migrations/schema.sql b/migrations/schema.sql index 68732816f..885586ac6 100644 --- a/migrations/schema.sql +++ b/migrations/schema.sql @@ -118,7 +118,7 @@ CREATE TABLE IF NOT EXISTS appPortBindings( type VARCHAR(8) NOT NULL DEFAULT "tcp", environmentVariable VARCHAR(128) NOT NULL, appId VARCHAR(128) NOT NULL, - count INTEGER DEFAULT 1, + count INTEGER NOT NULL DEFAULT 1, FOREIGN KEY(appId) REFERENCES apps(id), PRIMARY KEY(hostPort)); diff --git a/src/apps.js b/src/apps.js index 1647755f6..4a5d57afc 100644 --- a/src/apps.js +++ b/src/apps.js @@ -134,11 +134,10 @@ exports = module.exports = { // exported for testing _checkForPortBindingConflict: checkForPortBindingConflict, - _validatePortBindings: validatePortBindings, + _validatePorts: validatePorts, _validateAccessRestriction: validateAccessRestriction, _validateUpstreamUri: validateUpstreamUri, _validateLocations: validateLocations, - _translatePortBindings: translatePortBindings, _parseCrontab: parseCrontab, _clear: clear }; @@ -196,8 +195,9 @@ const LOCATION_FIELDS = [ 'appId', 'subdomain', 'domain', 'type', 'certificateJs const CHECKVOLUME_CMD = path.join(__dirname, 'scripts/checkvolume.sh'); -function validatePortBindings(portBindings, manifest) { - assert.strictEqual(typeof portBindings, 'object'); +// ports is a map of envvar -> hostPort +function validatePorts(ports, manifest) { + assert.strictEqual(typeof ports, 'object'); assert.strictEqual(typeof manifest, 'object'); // keep the public ports in sync with firewall rules in setup/start/cloudron-firewall.sh @@ -236,45 +236,48 @@ function validatePortBindings(portBindings, manifest) { 853 // dns over tls ]; - if (!portBindings) return null; + if (!ports) return null; const tcpPorts = manifest.tcpPorts || {}; const udpPorts = manifest.udpPorts || {}; - for (const portName in portBindings) { - if (!/^[A-Z0-9_]+$/.test(portName)) return new BoxError(BoxError.BAD_FIELD, `${portName} is not a valid environment variable in portBindings`); + for (const portName in ports) { + if (!/^[A-Z0-9_]+$/.test(portName)) return new BoxError(BoxError.BAD_FIELD, `${portName} is not a valid environment variable in ports`); - const hostPort = portBindings[portName]; - if (!Number.isInteger(hostPort)) return new BoxError(BoxError.BAD_FIELD, `${hostPort} is not an integer in ${portName} portBindings`); - if (RESERVED_PORTS.indexOf(hostPort) !== -1) return new BoxError(BoxError.BAD_FIELD, `Port ${hostPort} for ${portName} is reserved in portBindings`); - if (RESERVED_PORT_RANGES.find(range => (hostPort >= range[0] && hostPort <= range[1]))) return new BoxError(BoxError.BAD_FIELD, `Port ${hostPort} for ${portName} is reserved in portBindings`); - if (ALLOWED_PORTS.indexOf(hostPort) === -1 && (hostPort <= 1023 || hostPort > 65535)) return new BoxError(BoxError.BAD_FIELD, `${hostPort} for ${portName} is not in permitted range in portBindings`); + const hostPort = ports[portName]; + if (!Number.isInteger(hostPort)) return new BoxError(BoxError.BAD_FIELD, `${hostPort} is not an integer in ${portName} ports`); + if (RESERVED_PORTS.indexOf(hostPort) !== -1) return new BoxError(BoxError.BAD_FIELD, `Port ${hostPort} for ${portName} is reserved in ports`); + if (RESERVED_PORT_RANGES.find(range => (hostPort >= range[0] && hostPort <= range[1]))) return new BoxError(BoxError.BAD_FIELD, `Port ${hostPort} for ${portName} is reserved in ports`); + if (ALLOWED_PORTS.indexOf(hostPort) === -1 && (hostPort <= 1023 || hostPort > 65535)) return new BoxError(BoxError.BAD_FIELD, `${hostPort} for ${portName} is not in permitted range in ports`); - // it is OK if there is no 1-1 mapping between values in manifest.tcpPorts and portBindings. missing values implies the service is disabled + // it is OK if there is no 1-1 mapping between values in manifest.tcpPorts and ports. missing values implies the service is disabled const portSpec = tcpPorts[portName] || udpPorts[portName]; if (!portSpec) return new BoxError(BoxError.BAD_FIELD, `Invalid portBinding ${portName}`); if (portSpec.readOnly && portSpec.defaultValue !== hostPort) return new BoxError(BoxError.BAD_FIELD, `portBinding ${portName} is readOnly and cannot have a different value that the default`); + if ((hostPort + (portSpec.portCount || 1)) > 65535) return new BoxError(BoxError.BAD_FIELD, `${hostPort}+${portSpec.portCount} for ${portName} exceeds valid port range`); } return null; } -function translatePortBindings(portBindings, manifest) { - assert.strictEqual(typeof portBindings, 'object'); +// translates the REST API ports (envvar -> hostPort) to database portBindings (envvar -> { hostPort, count, type }) +function translateToPortBindings(ports, manifest) { + assert.strictEqual(typeof ports, 'object'); assert.strictEqual(typeof manifest, 'object'); - if (!portBindings) return null; + const portBindings = {}; - const result = {}; - const tcpPorts = manifest.tcpPorts || { }; + if (!ports) return portBindings; - for (const portName in portBindings) { + const tcpPorts = manifest.tcpPorts || {}; + + for (const portName in ports) { const portType = portName in tcpPorts ? exports.PORT_TYPE_TCP : exports.PORT_TYPE_UDP; - const portCount = portBindings[portName].portCount || (portName in tcpPorts ? manifest.tcpPorts[portName].portCount : manifest.udpPorts[portName].portCount); - result[portName] = { hostPort: portBindings[portName], type: portType, portCount: portCount || 1 }; + const portCount = portName in tcpPorts ? tcpPorts[portName].portCount : manifest.udpPorts[portName].portCount; // since count is optional, this can be undefined + portBindings[portName] = { hostPort: ports[portName], type: portType, count: portCount || 1 }; } - return result; + return portBindings; } function validateSecondaryDomains(secondaryDomains, manifest) { @@ -543,9 +546,8 @@ function getDuplicateErrorDetails(errorMessage, locations, portBindings) { } } - // check if any of the port bindings conflict for (const portName in portBindings) { - if (portBindings[portName] === parseInt(match[1])) return new BoxError(BoxError.ALREADY_EXISTS, `Port ${match[1]} is in use`); + if (portBindings[portName].hostPort === parseInt(match[1])) return new BoxError(BoxError.ALREADY_EXISTS, `Port ${match[1]} is in use`); } if (match[2] === 'apps_storageVolume') { @@ -647,7 +649,7 @@ function postProcess(result) { assert(result.hostPorts === null || typeof result.hostPorts === 'string'); assert(result.environmentVariables === null || typeof result.environmentVariables === 'string'); - result.portBindings = { }; + result.portBindings = {}; const hostPorts = result.hostPorts === null ? [ ] : result.hostPorts.split(','); const environmentVariables = result.environmentVariables === null ? [ ] : result.environmentVariables.split(','); const portTypes = result.portTypes === null ? [ ] : result.portTypes.split(','); @@ -659,7 +661,7 @@ function postProcess(result) { delete result.portCounts; for (let i = 0; i < environmentVariables.length; i++) { - result.portBindings[environmentVariables[i]] = { hostPort: parseInt(hostPorts[i], 10), type: portTypes[i], portCount: parseInt(portCounts[i], 10) }; + result.portBindings[environmentVariables[i]] = { hostPort: parseInt(hostPorts[i], 10), type: portTypes[i], count: parseInt(portCounts[i], 10) }; } assert(result.accessRestrictionJson === null || typeof result.accessRestrictionJson === 'string'); @@ -740,15 +742,11 @@ function postProcess(result) { result.taskId = result.taskId ? String(result.taskId) : null; } +// attaches computed properties function attachProperties(app, domainObjectMap) { assert.strictEqual(typeof app, 'object'); assert.strictEqual(typeof domainObjectMap, 'object'); - const result = {}; - for (const portName in app.portBindings) { - result[portName] = app.portBindings[portName].hostPort; - } - app.portBindings = result; app.iconUrl = app.hasIcon || app.hasAppStoreIcon ? `/api/v1/apps/${app.id}/icon` : null; app.fqdn = dns.fqdn(app.subdomain, app.domain); app.secondaryDomains.forEach(function (ad) { ad.fqdn = dns.fqdn(ad.subdomain, ad.domain); }); @@ -804,26 +802,26 @@ async function checkForPortBindingConflict(portBindings, options) { if (existingPortBindings.length === 0) return; - const tcpPorts = existingPortBindings.filter((p) => p.type === 'tcp'); - const udpPorts = existingPortBindings.filter((p) => p.type === 'udp'); + const tcpPortBindings = existingPortBindings.filter((p) => p.type === 'tcp'); + const udpPortBindings = existingPortBindings.filter((p) => p.type === 'udp'); for (const portName in portBindings) { - const p = portBindings[portName]; - const testPorts = p.type === 'tcp' ? tcpPorts : udpPorts; + const portBinding = portBindings[portName]; + const existingPortBinding = portBinding.type === 'tcp' ? tcpPortBindings : udpPortBindings; - const found = testPorts.find((e) => { + const found = existingPortBinding.find((epb) => { // if one is true we dont have a conflict // a1 <----> a2 b1 <-------> b2 // b1 <------> b2 a1 <-----> a2 - const a2 = (e.hostPort + e.count - 1); - const b1 = p.hostPort; - const b2 = (p.hostPort + p.portCount -1); - const a1 = e.hostPort; + const a2 = (epb.hostPort + epb.count - 1); + const b1 = portBinding.hostPort; + const b2 = (portBinding.hostPort + portBinding.count - 1); + const a1 = epb.hostPort; return !((a2 < b1) || (b2 < a1)); }); - if (found) throw new BoxError(BoxError.CONFLICT, `Conflicting port ${p.hostPort}`); + if (found) throw new BoxError(BoxError.CONFLICT, `Conflicting ${portBinding.type} port ${portBinding.hostPort}`); } } @@ -834,11 +832,9 @@ async function add(id, appStoreId, manifest, subdomain, domain, portBindings, da assert.strictEqual(typeof manifest.version, 'string'); assert.strictEqual(typeof subdomain, 'string'); assert.strictEqual(typeof domain, 'string'); - assert.strictEqual(typeof portBindings, 'object'); + assert(portBindings && typeof portBindings === 'object'); assert(data && typeof data === 'object'); - portBindings = portBindings || { }; - const manifestJson = JSON.stringify(manifest), accessRestriction = data.accessRestriction || null, accessRestrictionJson = JSON.stringify(accessRestriction), @@ -863,7 +859,7 @@ async function add(id, appStoreId, manifest, subdomain, domain, portBindings, da enableRedis = 'enableRedis' in data ? data.enableRedis : true, icon = data.icon || null; - await checkForPortBindingConflict(portBindings, {}); + await checkForPortBindingConflict(portBindings, { appId: null }); const queries = []; @@ -885,7 +881,7 @@ async function add(id, appStoreId, manifest, subdomain, domain, portBindings, da Object.keys(portBindings).forEach(function (env) { queries.push({ query: 'INSERT INTO appPortBindings (environmentVariable, hostPort, type, appId, count) VALUES (?, ?, ?, ?, ?)', - args: [ env, portBindings[env].hostPort, portBindings[env].type, id, portBindings[env].portCount ] + args: [ env, portBindings[env].hostPort, portBindings[env].type, id, portBindings[env].count ] }); }); @@ -953,14 +949,14 @@ async function updateWithConstraints(id, app, constraints) { const queries = [ ]; if ('portBindings' in app) { - const portBindings = app.portBindings || { }; + const portBindings = app.portBindings; await checkForPortBindingConflict(portBindings, { appId: id }); // replace entries by app id queries.push({ query: 'DELETE FROM appPortBindings WHERE appId = ?', args: [ id ] }); Object.keys(portBindings).forEach(function (env) { - const values = [ portBindings[env].hostPort, portBindings[env].type, env, id, portBindings[env].portCount ]; + const values = [ portBindings[env].hostPort, portBindings[env].type, env, id, portBindings[env].count ]; queries.push({ query: 'INSERT INTO appPortBindings (hostPort, type, environmentVariable, appId, count) VALUES(?, ?, ?, ?, ?)', args: values }); }); } @@ -1342,9 +1338,9 @@ async function install(data, auditSource) { error = await checkManifest(manifest); if (error) throw error; - error = validatePortBindings(data.portBindings || null, manifest); + error = validatePorts(data.ports || null, manifest); if (error) throw error; - const portBindings = translatePortBindings(data.portBindings || null, manifest); + const portBindings = translateToPortBindings(data.ports || null, manifest); error = validateAccessRestriction(accessRestriction); if (error) throw error; @@ -1906,16 +1902,16 @@ async function setLocation(app, data, auditSource) { subdomain: data.subdomain.toLowerCase(), domain: data.domain.toLowerCase(), // these are intentionally reset, if not set - portBindings: null, + portBindings: {}, secondaryDomains: [], redirectDomains: [], aliasDomains: [] }; - if ('portBindings' in data) { - error = validatePortBindings(data.portBindings || null, app.manifest); + if ('ports' in data) { + error = validatePorts(data.ports || null, app.manifest); if (error) throw error; - values.portBindings = translatePortBindings(data.portBindings || null, app.manifest); + values.portBindings = translateToPortBindings(data.ports || null, app.manifest); } // rename the auto-created mailbox to match the new location @@ -1954,7 +1950,7 @@ async function setLocation(app, data, auditSource) { }; const [taskError, taskId] = await safe(addTask(appId, exports.ISTATE_PENDING_LOCATION_CHANGE, task, auditSource)); if (taskError && taskError.reason !== BoxError.ALREADY_EXISTS) throw taskError; - if (taskError && taskError.reason === BoxError.ALREADY_EXISTS) throw getDuplicateErrorDetails(taskError.message, locations, data.portBindings); + if (taskError && taskError.reason === BoxError.ALREADY_EXISTS) throw getDuplicateErrorDetails(taskError.message, locations, values.portBindings); values.fqdn = dns.fqdn(values.subdomain, values.domain); values.secondaryDomains.forEach(function (ad) { ad.fqdn = dns.fqdn(ad.subdomain, ad.domain); }); @@ -2359,9 +2355,9 @@ async function clone(app, data, user, auditSource) { error = await checkManifest(manifest); if (error) throw error; - error = validatePortBindings(data.portBindings || null, manifest); + error = validatePorts(data.ports || null, manifest); if (error) throw error; - const portBindings = translatePortBindings(data.portBindings || null, manifest); + const portBindings = translateToPortBindings(data.ports || null, manifest); // should we copy the original app's mailbox settings instead? const mailboxName = manifest.addons?.sendmail ? mailboxNameForSubdomain(subdomain, manifest) : null; diff --git a/src/docker.js b/src/docker.js index 7b5de4367..808dd93c6 100644 --- a/src/docker.js +++ b/src/docker.js @@ -285,11 +285,9 @@ async function createSubcontainer(app, name, cmd, options) { const portEnv = []; for (const portName in app.portBindings) { - const hostPort = app.portBindings[portName]; - const portType = (manifest.tcpPorts && portName in manifest.tcpPorts) ? 'tcp' : 'udp'; - const ports = portType == 'tcp' ? manifest.tcpPorts : manifest.udpPorts; - const containerPort = ports[portName].containerPort || hostPort; - const portCount = ports[portName].portCount || 1; + const { hostPort, type:portType, count:portCount } = app.portBindings[portName]; + const portSpec = portType == 'tcp' ? manifest.tcpPorts : manifest.udpPorts; + const containerPort = portSpec[portName].containerPort || hostPort; const hostIps = hostPort === 53 ? await getAddressesForPort53() : [ '0.0.0.0', '::0' ]; // port 53 is special because it is possibly taken by systemd-resolved portEnv.push(`${portName}=${hostPort}`); @@ -298,7 +296,7 @@ async function createSubcontainer(app, name, cmd, options) { // docker portBindings requires ports to be exposed for (let i = 0; i < portCount; ++i) { exposedPorts[`${containerPort+i}/${portType}`] = {}; - dockerPortBindings[`${containerPort+i}/${portType}`] = hostIps.map(hip => { return { HostIp: hip, HostPort: (hostPort + i) + '' }; }); + dockerPortBindings[`${containerPort+i}/${portType}`] = hostIps.map(hip => { return { HostIp: hip, HostPort: String(hostPort + i) }; }); } } @@ -348,7 +346,7 @@ async function createSubcontainer(app, name, cmd, options) { }, Memory: memoryLimit, MemorySwap: -1, // Unlimited swap - PortBindings: isAppContainer ? dockerPortBindings : { }, + PortBindings: isAppContainer ? dockerPortBindings : {}, PublishAllPorts: false, ReadonlyRootfs: app.debugMode ? !!app.debugMode.readonlyRootfs : true, RestartPolicy: { diff --git a/src/routes/apps.js b/src/routes/apps.js index 1eed0eae9..e4fe53227 100644 --- a/src/routes/apps.js +++ b/src/routes/apps.js @@ -144,7 +144,7 @@ async function install(req, res, next) { if (typeof data.accessRestriction !== 'object') return next(new HttpError(400, 'accessRestriction is required')); // optional - if (('portBindings' in data) && typeof data.portBindings !== 'object') return next(new HttpError(400, 'portBindings must be an object')); + if (('ports' in data) && typeof data.ports !== 'object') return next(new HttpError(400, 'ports must be an object')); if ('icon' in data && typeof data.icon !== 'string') return next(new HttpError(400, 'icon is not a string')); if ('label' in data && typeof data.label !== 'string') return next(new HttpError(400, 'label must be a string')); @@ -468,7 +468,7 @@ async function setLocation(req, res, next) { if (typeof req.body.subdomain !== 'string') return next(new HttpError(400, 'subdomain must be string')); // subdomain may be an empty string if (typeof req.body.domain !== 'string') return next(new HttpError(400, 'domain must be string')); - if ('portBindings' in req.body && typeof req.body.portBindings !== 'object') return next(new HttpError(400, 'portBindings must be an object')); + if ('ports' in req.body && typeof req.body.ports !== 'object') return next(new HttpError(400, 'ports must be an object')); if ('secondaryDomains' in req.body) { if (!req.body.secondaryDomains || typeof req.body.secondaryDomains !== 'object') return next(new HttpError(400, 'secondaryDomains must be an object')); @@ -607,7 +607,7 @@ async function clone(req, res, next) { if (typeof data.backupId !== 'string') return next(new HttpError(400, 'backupId must be a string')); if (typeof data.subdomain !== 'string') return next(new HttpError(400, 'subdomain is required')); if (typeof data.domain !== 'string') return next(new HttpError(400, 'domain is required')); - if (('portBindings' in data) && typeof data.portBindings !== 'object') return next(new HttpError(400, 'portBindings must be an object')); + if (('ports' in data) && typeof data.ports !== 'object') return next(new HttpError(400, 'ports must be an object')); if ('secondaryDomains' in data) { if (!data.secondaryDomains || typeof data.secondaryDomains !== 'object') return next(new HttpError(400, 'secondaryDomains must be an object')); diff --git a/src/test/apps-test.js b/src/test/apps-test.js index 03093d3a0..26a336356 100644 --- a/src/test/apps-test.js +++ b/src/test/apps-test.js @@ -21,7 +21,7 @@ describe('Apps', function () { describe('checkForPortBindingConflict', function () { before(async function () { - await apps.add(app.id, app.appStoreId, app.manifest, app.subdomain, app.domain, [{ hostPort: 40000, type: 'tcp', portCount: 100 }, { hostPort: 50000, type: 'udp', portCount: 1 }], app); + await apps.add(app.id, app.appStoreId, app.manifest, app.subdomain, app.domain, [{ hostPort: 40000, type: 'tcp', count: 100 }, { hostPort: 50000, type: 'udp', count: 1 }], app); }); after(async function () { @@ -29,26 +29,26 @@ describe('Apps', function () { }); it('throws on exact conflict', async function () { - let [error] = await safe(apps._checkForPortBindingConflict([{ hostPort: 40000, type: 'tcp', portCount: 1 }], {})); + let [error] = await safe(apps._checkForPortBindingConflict([{ hostPort: 40000, type: 'tcp', count: 1 }], {})); expect(error.reason).to.equal(BoxError.CONFLICT); - [error] = await safe(apps._checkForPortBindingConflict([{ hostPort: 50000, type: 'udp', portCount: 1 }], {})); + [error] = await safe(apps._checkForPortBindingConflict([{ hostPort: 50000, type: 'udp', count: 1 }], {})); expect(error.reason).to.equal(BoxError.CONFLICT); }); it('throws on range conflict', async function () { - let [error] = await safe(apps._checkForPortBindingConflict([{ hostPort: 40080, type: 'tcp', portCount: 40 }], {})); + let [error] = await safe(apps._checkForPortBindingConflict([{ hostPort: 40080, type: 'tcp', count: 40 }], {})); expect(error.reason).to.equal(BoxError.CONFLICT); - [error] = await safe(apps._checkForPortBindingConflict([{ hostPort: 49995, type: 'udp', portCount: 20 }], {})); + [error] = await safe(apps._checkForPortBindingConflict([{ hostPort: 49995, type: 'udp', count: 20 }], {})); expect(error.reason).to.equal(BoxError.CONFLICT); }); it('succeeds without conflict', async function () { - await apps._checkForPortBindingConflict([{ hostPort: 39995, type: 'tcp', portCount: 2 }], {}); - await apps._checkForPortBindingConflict([{ hostPort: 45000, type: 'tcp', portCount: 1 }], {}); - await apps._checkForPortBindingConflict([{ hostPort: 49995, type: 'udp', portCount: 2 }], {}); - await apps._checkForPortBindingConflict([{ hostPort: 50001, type: 'udp', portCount: 1 }], {}); + await apps._checkForPortBindingConflict([{ hostPort: 39995, type: 'tcp', count: 2 }], {}); + await apps._checkForPortBindingConflict([{ hostPort: 45000, type: 'tcp', count: 1 }], {}); + await apps._checkForPortBindingConflict([{ hostPort: 49995, type: 'udp', count: 2 }], {}); + await apps._checkForPortBindingConflict([{ hostPort: 50001, type: 'udp', count: 1 }], {}); }); }); @@ -71,29 +71,29 @@ describe('Apps', function () { describe('validatePortBindings', function () { it('does not allow invalid host port', function () { - expect(apps._validatePortBindings({ PORT: -1 }, { tcpPorts: { PORT: 5000 } })).to.be.an(Error); - expect(apps._validatePortBindings({ PORT: 0 }, { tcpPorts: { PORT: 5000 } })).to.be.an(Error); - expect(apps._validatePortBindings({ PORT: 'text' }, { tcpPorts: { PORT: 5000 } })).to.be.an(Error); - expect(apps._validatePortBindings({ PORT: 65536 }, { tcpPorts: { PORT: 5000 } })).to.be.an(Error); - expect(apps._validatePortBindings({ PORT: 470 }, { tcpPorts: { PORT: 5000 } })).to.be.an(Error); + expect(apps._validatePorts({ PORT: -1 }, { tcpPorts: { PORT: 5000 } })).to.be.an(Error); + expect(apps._validatePorts({ PORT: 0 }, { tcpPorts: { PORT: 5000 } })).to.be.an(Error); + expect(apps._validatePorts({ PORT: 'text' }, { tcpPorts: { PORT: 5000 } })).to.be.an(Error); + expect(apps._validatePorts({ PORT: 65536 }, { tcpPorts: { PORT: 5000 } })).to.be.an(Error); + expect(apps._validatePorts({ PORT: 470 }, { tcpPorts: { PORT: 5000 } })).to.be.an(Error); }); it('does not allow ports not as part of manifest', function () { - expect(apps._validatePortBindings({ PORT: 1567 }, { tcpPorts: { } })).to.be.an(Error); - expect(apps._validatePortBindings({ PORT: 1567 }, { tcpPorts: { port3: null } })).to.be.an(Error); + expect(apps._validatePorts({ PORT: 1567 }, { tcpPorts: { } })).to.be.an(Error); + expect(apps._validatePorts({ PORT: 1567 }, { tcpPorts: { port3: null } })).to.be.an(Error); }); it('does not allow reserved ports', function () { - expect(apps._validatePortBindings({ PORT: 443 }, { tcpPorts: { PORT: 5000 } })).to.be.an(Error); - expect(apps._validatePortBindings({ PORT: 50000 }, { tcpPorts: { PORT: 5000 } })).to.be.an(Error); - expect(apps._validatePortBindings({ PORT: 51000 }, { tcpPorts: { PORT: 5000 } })).to.be.an(Error); - expect(apps._validatePortBindings({ PORT: 50100 }, { tcpPorts: { PORT: 5000 } })).to.be.an(Error); + expect(apps._validatePorts({ PORT: 443 }, { tcpPorts: { PORT: 5000 } })).to.be.an(Error); + expect(apps._validatePorts({ PORT: 50000 }, { tcpPorts: { PORT: 5000 } })).to.be.an(Error); + expect(apps._validatePorts({ PORT: 51000 }, { tcpPorts: { PORT: 5000 } })).to.be.an(Error); + expect(apps._validatePorts({ PORT: 50100 }, { tcpPorts: { PORT: 5000 } })).to.be.an(Error); }); it('allows valid bindings', function () { - expect(apps._validatePortBindings({ PORT: 1024 }, { tcpPorts: { PORT: 5000 } })).to.be(null); + expect(apps._validatePorts({ PORT: 1024 }, { tcpPorts: { PORT: 5000 } })).to.be(null); - expect(apps._validatePortBindings({ + expect(apps._validatePorts({ PORT1: 4033, PORT2: 3242, PORT3: 1234 diff --git a/src/test/common.js b/src/test/common.js index a3893cbae..4e44b9868 100644 --- a/src/test/common.js +++ b/src/test/common.js @@ -132,7 +132,7 @@ const app = { fqdn: domain.domain + '.' + 'applocation', manifest, containerId: 'someid', - portBindings: null, + portBindings: {}, accessRestriction: null, memoryLimit: 0, mailboxDomain: domain.domain, @@ -153,7 +153,7 @@ const proxyApp = { fqdn: domain.domain + '.' + 'proxylocation', manifest: proxyAppManifest, containerId: '', - portBindings: null, + portBindings: {}, accessRestriction: null, memoryLimit: 0, mailboxDomain: domain.domain,