From 73bd73ca6691cb24d0f30183b7e691205bc46586 Mon Sep 17 00:00:00 2001 From: Marco Kleinlein Date: Thu, 24 Nov 2022 08:47:50 +0100 Subject: [PATCH 01/36] Exposed variablesMeta in serverless --- lib/serverless.js | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/serverless.js b/lib/serverless.js index 90965dce2e8..d277bb575fe 100644 --- a/lib/serverless.js +++ b/lib/serverless.js @@ -99,6 +99,7 @@ class Serverless { // Old variables resolver is dropped, yet some plugins access service properties through // `variables` class. Below patch ensures those plugins won't get broken this.variables = { service: this.service }; + this.variablesMeta = config.variablesMeta; this.pluginManager = new PluginManager(this); this.configSchemaHandler = new ConfigSchemaHandler(this); From b08bdc9d23d07b7e52bcf3d656bd50c605976d7e Mon Sep 17 00:00:00 2001 From: Marco Kleinlein Date: Thu, 24 Nov 2022 09:11:55 +0100 Subject: [PATCH 02/36] Added variables meta to serverless --- scripts/serverless.js | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/scripts/serverless.js b/scripts/serverless.js index 333adf0aa19..24d4f4605f9 100755 --- a/scripts/serverless.js +++ b/scripts/serverless.js @@ -598,6 +598,7 @@ processSpanPromise = (async () => { commands[0] === 'plugin' || Boolean(variablesMeta && !variablesMeta.size), commands, options, + variablesMeta, }); try { @@ -643,7 +644,7 @@ processSpanPromise = (async () => { // Validate result command and options if (hasFinalCommandSchema) require('../lib/cli/ensure-supported-command')(configuration); if (isHelpRequest) return; - if (!_.get(variablesMeta, 'size')) return; + if (!_.get(serverless.variablesMeta, 'size')) return; if (commandSchema) { resolverConfiguration.options = filterSupportedOptions(options, { @@ -678,12 +679,12 @@ processSpanPromise = (async () => { // Pre-resolve to eventually pick not yet resolved AWS auth related properties processLog.debug('resolve variables'); await resolveVariables(resolverConfiguration); - if (!variablesMeta.size) return; + if (!serverless.variablesMeta.size) return; if ( eventuallyReportVariableResolutionErrors( configurationPath, configuration, - variablesMeta + serverless.variablesMeta ) ) { return; @@ -727,9 +728,13 @@ processSpanPromise = (async () => { // Having all source resolvers configured, resolve variables processLog.debug('resolve all variables'); await resolveVariables(resolverConfiguration); - if (!variablesMeta.size) return; + if (!serverless.variablesMeta.size) return; if ( - eventuallyReportVariableResolutionErrors(configurationPath, configuration, variablesMeta) + eventuallyReportVariableResolutionErrors( + configurationPath, + configuration, + serverless.variablesMeta + ) ) { return; } @@ -737,10 +742,12 @@ processSpanPromise = (async () => { // Do not confirm on unresolved sources with partially resolved configuration if (resolverConfiguration.propertyPathsToResolve) return; - processLog.debug('uresolved variables meta: %o', variablesMeta); + processLog.debug('uresolved variables meta: %o', serverless.variablesMeta); // Report unrecognized variable sources found in variables configured in service config const unresolvedSources = - require('../lib/configuration/variables/resolve-unresolved-source-types')(variablesMeta); + require('../lib/configuration/variables/resolve-unresolved-source-types')( + serverless.variablesMeta + ); const recognizedSourceNames = new Set(Object.keys(resolverConfiguration.sources)); const unrecognizedSourceNames = Array.from(unresolvedSources.keys()).filter( From bb9223ed5a991d258183b9b012c0e1369ea94106 Mon Sep 17 00:00:00 2001 From: Marco Kleinlein Date: Thu, 24 Nov 2022 10:27:00 +0100 Subject: [PATCH 03/36] Added extendConfiguration --- lib/serverless.js | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/lib/serverless.js b/lib/serverless.js index d277bb575fe..21716f55fbd 100644 --- a/lib/serverless.js +++ b/lib/serverless.js @@ -2,6 +2,7 @@ const path = require('path'); const os = require('os'); +const _ = require('lodash'); const ensureString = require('type/string/ensure'); const ensureValue = require('type/value/ensure'); const ensureArray = require('type/array/ensure'); @@ -23,6 +24,7 @@ const eventuallyUpdate = require('./utils/eventually-update'); const commmandsSchema = require('./cli/commands-schema'); const resolveCliInput = require('./cli/resolve-input'); const isDashboardEnabled = require('./configuration/is-dashboard-enabled'); +const { parseEntries } = require('./configuration/variables/resolve-meta'); // Old local fallback is triggered in older versions by Serverless constructor directly const isStackFromOldLocalFallback = RegExp.prototype.test.bind( @@ -216,6 +218,27 @@ class Serverless { logDeprecation(code, message) { return this._logDeprecation(`EXT_${ensureString(code)}`, ensureString(message)); } + + extendConfiguration(configurationPathKeys, value) { + const oldConfig = _.get(configurationPathKeys, this.configurationInput); + if (oldConfig === undefined) { + _.set(this.configurationInput, configurationPathKeys, value); + } else { + const mergedConfig = _.merge(oldConfig, value); + _.set(this.configurationInput, configurationPathKeys, mergedConfig); + } + + const metaPathPrefix = configurationPathKeys.replace('.', '\0'); + const filteredEntries = Object.entries(this.variablesMeta).filter( + (entry) => !entry[0].startsWith(metaPathPrefix) + ); + this.variablesMeta = Object.fromEntries(filteredEntries); + parseEntries(this.configurationInput, configurationPathKeys.split('.'), new Map()).forEach( + (entryValue, key) => { + this.variablesMeta.set(key, entryValue); + } + ); + } } module.exports = Serverless; From 17b32eb8b2473282ccd40ddc517d7c0b019680eb Mon Sep 17 00:00:00 2001 From: Marco Kleinlein Date: Thu, 24 Nov 2022 16:15:58 +0100 Subject: [PATCH 04/36] Fixed merging --- lib/serverless.js | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/lib/serverless.js b/lib/serverless.js index 21716f55fbd..e6d649e889c 100644 --- a/lib/serverless.js +++ b/lib/serverless.js @@ -220,12 +220,23 @@ class Serverless { } extendConfiguration(configurationPathKeys, value) { - const oldConfig = _.get(configurationPathKeys, this.configurationInput); + this.cli.consoleLog(`Extending configuration at ${configurationPathKeys}`) + + const isObject = (obj) => { + return typeof obj === 'object' && !Array.isArray(obj) && obj !== null + } + + const oldConfig = _.get(this.configurationInput, configurationPathKeys); if (oldConfig === undefined) { _.set(this.configurationInput, configurationPathKeys, value); - } else { + } else if ( Array.isArray(oldConfig) && Array.isArray(value) ) { + const mergedConfig = oldConfig.concat(value); + _.set(this.configurationInput, configurationPathKeys, mergedConfig); + } else if ( isObject(oldConfig) && isObject(value) ) { const mergedConfig = _.merge(oldConfig, value); _.set(this.configurationInput, configurationPathKeys, mergedConfig); + } else { + throw Error('Cannot extend without matching types.') } const metaPathPrefix = configurationPathKeys.replace('.', '\0'); From d771c756483942405504bf02f9c1df16540bfe6e Mon Sep 17 00:00:00 2001 From: Marco Kleinlein Date: Thu, 24 Nov 2022 16:16:51 +0100 Subject: [PATCH 05/36] Fixed variablesMeta handling --- lib/serverless.js | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/lib/serverless.js b/lib/serverless.js index e6d649e889c..d52d86559a7 100644 --- a/lib/serverless.js +++ b/lib/serverless.js @@ -220,31 +220,34 @@ class Serverless { } extendConfiguration(configurationPathKeys, value) { - this.cli.consoleLog(`Extending configuration at ${configurationPathKeys}`) + this.cli.consoleLog(`Extending configuration at ${configurationPathKeys}`); const isObject = (obj) => { - return typeof obj === 'object' && !Array.isArray(obj) && obj !== null - } + return typeof obj === 'object' && !Array.isArray(obj) && obj !== null; + }; const oldConfig = _.get(this.configurationInput, configurationPathKeys); if (oldConfig === undefined) { _.set(this.configurationInput, configurationPathKeys, value); - } else if ( Array.isArray(oldConfig) && Array.isArray(value) ) { + } else if (Array.isArray(oldConfig) && Array.isArray(value)) { const mergedConfig = oldConfig.concat(value); _.set(this.configurationInput, configurationPathKeys, mergedConfig); - } else if ( isObject(oldConfig) && isObject(value) ) { + } else if (isObject(oldConfig) && isObject(value)) { const mergedConfig = _.merge(oldConfig, value); _.set(this.configurationInput, configurationPathKeys, mergedConfig); } else { - throw Error('Cannot extend without matching types.') + throw Error('Cannot extend without matching types.'); } const metaPathPrefix = configurationPathKeys.replace('.', '\0'); - const filteredEntries = Object.entries(this.variablesMeta).filter( - (entry) => !entry[0].startsWith(metaPathPrefix) - ); - this.variablesMeta = Object.fromEntries(filteredEntries); - parseEntries(this.configurationInput, configurationPathKeys.split('.'), new Map()).forEach( + for (const key of this.variablesMeta.keys()) { + if (key.startsWith(metaPathPrefix)) { + this.variablesMeta.delete(key); + } + } + + const mergedConfig = _.get(this.configurationInput, configurationPathKeys); + parseEntries(Object.entries(mergedConfig), configurationPathKeys.split('.'), new Map()).forEach( (entryValue, key) => { this.variablesMeta.set(key, entryValue); } From 910b4e48824b8fe69415fe0788148c512f620952 Mon Sep 17 00:00:00 2001 From: Marco Kleinlein Date: Fri, 25 Nov 2022 14:03:44 +0100 Subject: [PATCH 06/36] Cleaned up feature code --- lib/serverless.js | 35 ++++++++++++++++++++++++----------- 1 file changed, 24 insertions(+), 11 deletions(-) diff --git a/lib/serverless.js b/lib/serverless.js index d52d86559a7..a6ea08d46ca 100644 --- a/lib/serverless.js +++ b/lib/serverless.js @@ -24,7 +24,7 @@ const eventuallyUpdate = require('./utils/eventually-update'); const commmandsSchema = require('./cli/commands-schema'); const resolveCliInput = require('./cli/resolve-input'); const isDashboardEnabled = require('./configuration/is-dashboard-enabled'); -const { parseEntries } = require('./configuration/variables/resolve-meta'); +const parseEntries = require('./configuration/variables/resolve-meta').parseEntries; // Old local fallback is triggered in older versions by Serverless constructor directly const isStackFromOldLocalFallback = RegExp.prototype.test.bind( @@ -119,6 +119,7 @@ class Serverless { this.serverlessDirPath = path.join(os.homedir(), '.serverless'); this.isStandaloneExecutable = isStandaloneExecutable; this.triggeredDeprecations = logDeprecation.triggeredDeprecations; + this.isConfigurationExtendable = true; // TODO: Remove once "@serverless/dashboard-plugin" is integrated into this repository this._commandsSchema = commmandsSchema; @@ -139,6 +140,7 @@ class Serverless { await this.service.load(this.processedInput.options); // load all plugins await this.pluginManager.loadAllPlugins(this.service.plugins); + this.isConfigurationExtendable = false; // give the CLI the plugins and commands so that it can print out // information such as options when the user enters --help this.cli.setLoadedPlugins(this.pluginManager.getPlugins()); @@ -219,35 +221,46 @@ class Serverless { return this._logDeprecation(`EXT_${ensureString(code)}`, ensureString(message)); } - extendConfiguration(configurationPathKeys, value) { - this.cli.consoleLog(`Extending configuration at ${configurationPathKeys}`); + extendConfiguration(configurationPath, value) { + if (!this.isConfigurationExtendable) { + throw new ServerlessError( + 'ExtendConfiguration cannot be used after init.', + 'INVALID_EXTEND_AFTER_INIT' + ); + } + value = JSON.parse(JSON.stringify(value)); const isObject = (obj) => { return typeof obj === 'object' && !Array.isArray(obj) && obj !== null; }; - const oldConfig = _.get(this.configurationInput, configurationPathKeys); + const oldConfig = _.get(this.configurationInput, configurationPath); if (oldConfig === undefined) { - _.set(this.configurationInput, configurationPathKeys, value); + _.set(this.configurationInput, configurationPath, value); } else if (Array.isArray(oldConfig) && Array.isArray(value)) { const mergedConfig = oldConfig.concat(value); - _.set(this.configurationInput, configurationPathKeys, mergedConfig); + _.set(this.configurationInput, configurationPath, mergedConfig); } else if (isObject(oldConfig) && isObject(value)) { const mergedConfig = _.merge(oldConfig, value); - _.set(this.configurationInput, configurationPathKeys, mergedConfig); + _.set(this.configurationInput, configurationPath, mergedConfig); + } else if (typeof oldConfig === typeof value && typeof oldConfig !== 'object') { + _.set(this.configurationInput, configurationPath, value); } else { - throw Error('Cannot extend without matching types.'); + throw new ServerlessError( + `Configuration cannot be extended at ${configurationPath}. Expected: ${typeof oldConfig}, Received: ${typeof value}`, + 'INVALID_EXTEND_WITH_TYPE_MISMATCH' + ); } - const metaPathPrefix = configurationPathKeys.replace('.', '\0'); + const metaPathPrefix = configurationPath.replace(/\./g, '\0'); for (const key of this.variablesMeta.keys()) { if (key.startsWith(metaPathPrefix)) { this.variablesMeta.delete(key); } } - const mergedConfig = _.get(this.configurationInput, configurationPathKeys); - parseEntries(Object.entries(mergedConfig), configurationPathKeys.split('.'), new Map()).forEach( + const mergedConfig = _.get(this.configurationInput, configurationPath); + parseEntries(Object.entries(mergedConfig), configurationPath.split('.'), new Map()).forEach( (entryValue, key) => { this.variablesMeta.set(key, entryValue); } From 5de376ea7a1e3fa582cc2c7446d045b4b4ccff8d Mon Sep 17 00:00:00 2001 From: Marco Kleinlein Date: Fri, 25 Nov 2022 14:08:18 +0100 Subject: [PATCH 07/36] Added tests for extendConfiguration --- test/unit/lib/serverless.test.js | 180 +++++++++++++++++++++++++++++++ 1 file changed, 180 insertions(+) diff --git a/test/unit/lib/serverless.test.js b/test/unit/lib/serverless.test.js index 38d550d8612..b929402ec8b 100644 --- a/test/unit/lib/serverless.test.js +++ b/test/unit/lib/serverless.test.js @@ -1,8 +1,11 @@ 'use strict'; const chai = require('chai'); +const sinon = require('sinon'); +const proxyquire = require('proxyquire').noCallThru(); chai.use(require('chai-as-promised')); +chai.use(require('sinon-chai')); const { expect } = chai; @@ -174,4 +177,181 @@ describe('test/unit/lib/serverless.test.js', () => { expect(serverless.config).to.have.property('servicePath'); }); }); + + describe('Extend configuration', () => { + let serverless; + let parseEntriesStub; + + before(() => { + parseEntriesStub = sinon.stub().returns(new Map([])); + const ServerlessProxy = proxyquire('../../../lib/serverless.js', { + './configuration/variables/resolve-meta': { + parseEntries: parseEntriesStub, + }, + }); + + serverless = new ServerlessProxy({ + commands: ['print'], + options: {}, + serviceDir: null, + variablesMeta: new Map([]), + }); + + serverless.pluginManager = sinon.createStubInstance(PluginManager); + serverless.classes.CLI = sinon.stub(serverless.classes.CLI); + }); + + after(() => { + sinon.restore(); + }); + + beforeEach(async () => { + serverless.isConfigurationExtendable = true; + serverless.variablesMeta = new Map([]); + }); + + it('Should add configuration path if it not exists', async () => { + const initialConfig = { + 'pre-existing': { + path: true, + }, + }; + + const path = 'path.to.insert'; + const value = { + newKey: 'value', + }; + serverless.configurationInput = JSON.parse(JSON.stringify(initialConfig)); + serverless.extendConfiguration(path, value); + + expect(serverless.configurationInput).to.deep.include(initialConfig); + expect(serverless.configurationInput).to.deep.nested.include({ [path]: value }); + }); + + it('Should merge configuration object if it exists', async () => { + const path = 'pre-existing'; + const initialConfig = { + [path]: { + path: true, + }, + }; + + const value = { + newKey: 'value', + }; + serverless.configurationInput = JSON.parse(JSON.stringify(initialConfig)); + serverless.extendConfiguration(path, value); + + expect(serverless.configurationInput[path]).to.deep.include( + Object.assign(initialConfig[path], value) + ); + }); + + it('Should extend configuration array if it exists', async () => { + const path = 'pre-existing'; + const initialConfig = { + [path]: [1, 2, 3, 4], + }; + + const value = [4, 5, 6]; + serverless.configurationInput = JSON.parse(JSON.stringify(initialConfig)); + serverless.extendConfiguration(path, value); + + expect(serverless.configurationInput[path]) + .to.be.an('array') + .that.includes.members(initialConfig[path]); + expect(serverless.configurationInput[path]).to.include.members(value); + expect(serverless.configurationInput[path]).to.have.length( + value.length + initialConfig[path].length + ); + }); + + it('Should assign trivial types', async () => { + const path = 'pre-existing'; + const initialConfig = { + [path]: 'some string', + }; + const value = 'other string'; + serverless.configurationInput = JSON.parse(JSON.stringify(initialConfig)); + serverless.extendConfiguration(path, value); + expect(serverless.configurationInput[path]).to.equal(value); + }); + + it('Should deeply copy the new value', async () => { + const path = 'level1'; + const subpath = 'level2'; + const subvalue = { + level3: 'copy', + }; + const value = { + [subpath]: subvalue, + }; + serverless.configurationInput = {}; + serverless.extendConfiguration(path, value); + + expect(serverless.configurationInput[path]).to.not.equal(value); + expect(serverless.configurationInput[path][subpath]).to.not.equal(subvalue); + }); + + it('Should throw if types do not match', async () => { + const path = 'pre-existing'; + const initialConfig = { + [path]: [1, 2, 3, 4], + }; + const value = {}; + serverless.configurationInput = JSON.parse(JSON.stringify(initialConfig)); + + expect(() => serverless.extendConfiguration(path, value)).to.throw(ServerlessError); + }); + + it('Should throw if called after init', async () => { + await serverless.init(); + expect(() => serverless.extendConfiguration('', {})).to.throw(ServerlessError); + }); + + it('Should clean variables meta in configurationPath', async () => { + const path = 'path.to.insert'; + const value = { + newKey: 'value', + }; + serverless.configurationInput = {}; + const metaToKeep = ['path.to.keep'].map((_) => _.replace(/\./g, '\0')); + const metaToDelete = [path, `${path}.child`].map((_) => _.replace(/\./g, '\0')); + const variablesMeta = metaToKeep.concat(metaToDelete); + + const keyStub = sinon.stub(serverless.variablesMeta, 'keys'); + keyStub.returns(variablesMeta); + + const deleteStub = sinon.stub(serverless.variablesMeta, 'delete'); + + serverless.extendConfiguration(path, value); + + metaToDelete.forEach((meta) => { + expect(deleteStub).to.have.been.calledWith(meta); + }); + }); + + it('Should add variables to variables meta', async () => { + const path = 'path.to.insert'; + const value = { + newKey: 'value', + }; + serverless.configurationInput = {}; + + const resolvedVariables = { + var1: {}, + var2: {}, + }; + const fakeReturn = new Map(Object.entries(resolvedVariables)); + parseEntriesStub.returns(fakeReturn); + + const setStub = sinon.stub(serverless.variablesMeta, 'set'); + + serverless.extendConfiguration(path, value); + + Object.getOwnPropertyNames(resolvedVariables).forEach((varName) => { + expect(setStub).to.have.been.calledWith(varName, sinon.match.any); + }); + }); + }); }); From ae55f45e10656b0b1257c04e2fff018149b3e847 Mon Sep 17 00:00:00 2001 From: Marco Kleinlein Date: Mon, 28 Nov 2022 17:06:51 +0100 Subject: [PATCH 08/36] Added try/catch for deep copy --- lib/serverless.js | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/lib/serverless.js b/lib/serverless.js index a6ea08d46ca..8768cda1aca 100644 --- a/lib/serverless.js +++ b/lib/serverless.js @@ -228,7 +228,14 @@ class Serverless { 'INVALID_EXTEND_AFTER_INIT' ); } - value = JSON.parse(JSON.stringify(value)); + try { + value = JSON.parse(JSON.stringify(value)); + } catch (error) { + throw new ServerlessError( + 'ExtendConfiguration called with invalid data. Value is not json-serializable. Error: ${error}', + 'INVALID_EXTEND_VALUE' + ); + } const isObject = (obj) => { return typeof obj === 'object' && !Array.isArray(obj) && obj !== null; From 161b2125e50a2f15d55999af1e4d632d36bd3f61 Mon Sep 17 00:00:00 2001 From: Marco Kleinlein <87859452+mklenbw@users.noreply.github.com> Date: Mon, 28 Nov 2022 17:17:11 +0100 Subject: [PATCH 09/36] Changed configurationPath to key array --- lib/serverless.js | 10 ++++------ test/unit/lib/serverless.test.js | 18 ++++++++++-------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/lib/serverless.js b/lib/serverless.js index 8768cda1aca..9a10321cc93 100644 --- a/lib/serverless.js +++ b/lib/serverless.js @@ -221,7 +221,7 @@ class Serverless { return this._logDeprecation(`EXT_${ensureString(code)}`, ensureString(message)); } - extendConfiguration(configurationPath, value) { + extendConfiguration(configurationPathKeys, value) { if (!this.isConfigurationExtendable) { throw new ServerlessError( 'ExtendConfiguration cannot be used after init.', @@ -241,6 +241,8 @@ class Serverless { return typeof obj === 'object' && !Array.isArray(obj) && obj !== null; }; + const configurationPath = configurationPathKeys.join('.'); + const oldConfig = _.get(this.configurationInput, configurationPath); if (oldConfig === undefined) { _.set(this.configurationInput, configurationPath, value); @@ -267,11 +269,7 @@ class Serverless { } const mergedConfig = _.get(this.configurationInput, configurationPath); - parseEntries(Object.entries(mergedConfig), configurationPath.split('.'), new Map()).forEach( - (entryValue, key) => { - this.variablesMeta.set(key, entryValue); - } - ); + parseEntries(Object.entries(mergedConfig), configurationPathKeys, this.variablesMeta); } } diff --git a/test/unit/lib/serverless.test.js b/test/unit/lib/serverless.test.js index b929402ec8b..c203ee124ba 100644 --- a/test/unit/lib/serverless.test.js +++ b/test/unit/lib/serverless.test.js @@ -222,7 +222,7 @@ describe('test/unit/lib/serverless.test.js', () => { newKey: 'value', }; serverless.configurationInput = JSON.parse(JSON.stringify(initialConfig)); - serverless.extendConfiguration(path, value); + serverless.extendConfiguration(path.split('.'), value); expect(serverless.configurationInput).to.deep.include(initialConfig); expect(serverless.configurationInput).to.deep.nested.include({ [path]: value }); @@ -240,7 +240,7 @@ describe('test/unit/lib/serverless.test.js', () => { newKey: 'value', }; serverless.configurationInput = JSON.parse(JSON.stringify(initialConfig)); - serverless.extendConfiguration(path, value); + serverless.extendConfiguration(path.split('.'), value); expect(serverless.configurationInput[path]).to.deep.include( Object.assign(initialConfig[path], value) @@ -255,7 +255,7 @@ describe('test/unit/lib/serverless.test.js', () => { const value = [4, 5, 6]; serverless.configurationInput = JSON.parse(JSON.stringify(initialConfig)); - serverless.extendConfiguration(path, value); + serverless.extendConfiguration(path.split('.'), value); expect(serverless.configurationInput[path]) .to.be.an('array') @@ -273,7 +273,7 @@ describe('test/unit/lib/serverless.test.js', () => { }; const value = 'other string'; serverless.configurationInput = JSON.parse(JSON.stringify(initialConfig)); - serverless.extendConfiguration(path, value); + serverless.extendConfiguration(path.split('.'), value); expect(serverless.configurationInput[path]).to.equal(value); }); @@ -287,7 +287,7 @@ describe('test/unit/lib/serverless.test.js', () => { [subpath]: subvalue, }; serverless.configurationInput = {}; - serverless.extendConfiguration(path, value); + serverless.extendConfiguration(path.split('.'), value); expect(serverless.configurationInput[path]).to.not.equal(value); expect(serverless.configurationInput[path][subpath]).to.not.equal(subvalue); @@ -301,7 +301,9 @@ describe('test/unit/lib/serverless.test.js', () => { const value = {}; serverless.configurationInput = JSON.parse(JSON.stringify(initialConfig)); - expect(() => serverless.extendConfiguration(path, value)).to.throw(ServerlessError); + expect(() => serverless.extendConfiguration(path.split('.'), value)).to.throw( + ServerlessError + ); }); it('Should throw if called after init', async () => { @@ -324,7 +326,7 @@ describe('test/unit/lib/serverless.test.js', () => { const deleteStub = sinon.stub(serverless.variablesMeta, 'delete'); - serverless.extendConfiguration(path, value); + serverless.extendConfiguration(path.split('.'), value); metaToDelete.forEach((meta) => { expect(deleteStub).to.have.been.calledWith(meta); @@ -347,7 +349,7 @@ describe('test/unit/lib/serverless.test.js', () => { const setStub = sinon.stub(serverless.variablesMeta, 'set'); - serverless.extendConfiguration(path, value); + serverless.extendConfiguration(path.split('.'), value); Object.getOwnPropertyNames(resolvedVariables).forEach((varName) => { expect(setStub).to.have.been.calledWith(varName, sinon.match.any); From 95777f740d8f3b54ef9499a2680e65bbfae6eef1 Mon Sep 17 00:00:00 2001 From: Marco Kleinlein <87859452+mklenbw@users.noreply.github.com> Date: Mon, 28 Nov 2022 17:21:41 +0100 Subject: [PATCH 10/36] Ensure array --- lib/serverless.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/serverless.js b/lib/serverless.js index 9a10321cc93..049107a9c2c 100644 --- a/lib/serverless.js +++ b/lib/serverless.js @@ -222,6 +222,8 @@ class Serverless { } extendConfiguration(configurationPathKeys, value) { + ensureArray(configurationPathKeys, { ensureItem: ensureString }); + if (!this.isConfigurationExtendable) { throw new ServerlessError( 'ExtendConfiguration cannot be used after init.', From 08ba6888de2913d4dff1a311f543e2c7fd0d045d Mon Sep 17 00:00:00 2001 From: Marco Kleinlein <87859452+mklenbw@users.noreply.github.com> Date: Mon, 28 Nov 2022 17:46:51 +0100 Subject: [PATCH 11/36] Ensure array length Introduces newConfig to remove redundant get --- lib/serverless.js | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/lib/serverless.js b/lib/serverless.js index 049107a9c2c..ce8aa5980bd 100644 --- a/lib/serverless.js +++ b/lib/serverless.js @@ -223,6 +223,12 @@ class Serverless { extendConfiguration(configurationPathKeys, value) { ensureArray(configurationPathKeys, { ensureItem: ensureString }); + if (configurationPathKeys.length < 1) { + throw new ServerlessError( + 'ExtendConfiguration cannot be used at root. ConfigurationPathKeys needs to contain at least one element.', + 'INVALID_EXTEND_AFTER_INIT' + ); + } if (!this.isConfigurationExtendable) { throw new ServerlessError( @@ -245,17 +251,16 @@ class Serverless { const configurationPath = configurationPathKeys.join('.'); + let newConfig; const oldConfig = _.get(this.configurationInput, configurationPath); if (oldConfig === undefined) { - _.set(this.configurationInput, configurationPath, value); + newConfig = value; } else if (Array.isArray(oldConfig) && Array.isArray(value)) { - const mergedConfig = oldConfig.concat(value); - _.set(this.configurationInput, configurationPath, mergedConfig); + newConfig = oldConfig.concat(value); } else if (isObject(oldConfig) && isObject(value)) { - const mergedConfig = _.merge(oldConfig, value); - _.set(this.configurationInput, configurationPath, mergedConfig); + newConfig = _.merge(oldConfig, value); } else if (typeof oldConfig === typeof value && typeof oldConfig !== 'object') { - _.set(this.configurationInput, configurationPath, value); + newConfig = value; } else { throw new ServerlessError( `Configuration cannot be extended at ${configurationPath}. Expected: ${typeof oldConfig}, Received: ${typeof value}`, @@ -263,15 +268,20 @@ class Serverless { ); } + _.set(this.configurationInput, configurationPath, newConfig); + const metaPathPrefix = configurationPath.replace(/\./g, '\0'); for (const key of this.variablesMeta.keys()) { if (key.startsWith(metaPathPrefix)) { this.variablesMeta.delete(key); } } - - const mergedConfig = _.get(this.configurationInput, configurationPath); - parseEntries(Object.entries(mergedConfig), configurationPathKeys, this.variablesMeta); + if (typeof newConfig !== 'object') { + const lastKey = configurationPathKeys.pop(); + parseEntries({ [lastKey]: newConfig }, configurationPathKeys, this.variablesMeta); + } else { + parseEntries(Object.entries(newConfig), configurationPathKeys, this.variablesMeta); + } } } From 1eecaf54d902fd429e0f34bef86d6a7b14b37ec3 Mon Sep 17 00:00:00 2001 From: Marco Kleinlein <87859452+mklenbw@users.noreply.github.com> Date: Mon, 28 Nov 2022 17:49:04 +0100 Subject: [PATCH 12/36] Revert variable path change --- scripts/serverless.js | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/scripts/serverless.js b/scripts/serverless.js index 24d4f4605f9..c181e789307 100755 --- a/scripts/serverless.js +++ b/scripts/serverless.js @@ -644,7 +644,7 @@ processSpanPromise = (async () => { // Validate result command and options if (hasFinalCommandSchema) require('../lib/cli/ensure-supported-command')(configuration); if (isHelpRequest) return; - if (!_.get(serverless.variablesMeta, 'size')) return; + if (!_.get(variablesMeta, 'size')) return; if (commandSchema) { resolverConfiguration.options = filterSupportedOptions(options, { @@ -679,12 +679,12 @@ processSpanPromise = (async () => { // Pre-resolve to eventually pick not yet resolved AWS auth related properties processLog.debug('resolve variables'); await resolveVariables(resolverConfiguration); - if (!serverless.variablesMeta.size) return; + if (!variablesMeta.size) return; if ( eventuallyReportVariableResolutionErrors( configurationPath, configuration, - serverless.variablesMeta + variablesMeta ) ) { return; @@ -728,13 +728,9 @@ processSpanPromise = (async () => { // Having all source resolvers configured, resolve variables processLog.debug('resolve all variables'); await resolveVariables(resolverConfiguration); - if (!serverless.variablesMeta.size) return; + if (!variablesMeta.size) return; if ( - eventuallyReportVariableResolutionErrors( - configurationPath, - configuration, - serverless.variablesMeta - ) + eventuallyReportVariableResolutionErrors(configurationPath, configuration, variablesMeta) ) { return; } @@ -742,12 +738,10 @@ processSpanPromise = (async () => { // Do not confirm on unresolved sources with partially resolved configuration if (resolverConfiguration.propertyPathsToResolve) return; - processLog.debug('uresolved variables meta: %o', serverless.variablesMeta); + processLog.debug('uresolved variables meta: %o', variablesMeta); // Report unrecognized variable sources found in variables configured in service config const unresolvedSources = - require('../lib/configuration/variables/resolve-unresolved-source-types')( - serverless.variablesMeta - ); + require('../lib/configuration/variables/resolve-unresolved-source-types')(variablesMeta); const recognizedSourceNames = new Set(Object.keys(resolverConfiguration.sources)); const unrecognizedSourceNames = Array.from(unresolvedSources.keys()).filter( From 2628e75cc53659c71f740540d9d4f0211aa7f1da Mon Sep 17 00:00:00 2001 From: Marco Kleinlein <87859452+mklenbw@users.noreply.github.com> Date: Tue, 29 Nov 2022 12:55:39 +0100 Subject: [PATCH 13/36] Revert variable path change --- lib/serverless.js | 38 +++------- test/unit/lib/serverless.test.js | 117 +++++++++++-------------------- 2 files changed, 51 insertions(+), 104 deletions(-) diff --git a/lib/serverless.js b/lib/serverless.js index ce8aa5980bd..f141995ab87 100644 --- a/lib/serverless.js +++ b/lib/serverless.js @@ -222,10 +222,12 @@ class Serverless { } extendConfiguration(configurationPathKeys, value) { - ensureArray(configurationPathKeys, { ensureItem: ensureString }); + ensureArray(configurationPathKeys, { + ensureItem: ensureString, + }); if (configurationPathKeys.length < 1) { throw new ServerlessError( - 'ExtendConfiguration cannot be used at root. ConfigurationPathKeys needs to contain at least one element.', + 'ConfigurationPathKeys needs to contain at least one element.', 'INVALID_EXTEND_AFTER_INIT' ); } @@ -245,42 +247,20 @@ class Serverless { ); } - const isObject = (obj) => { - return typeof obj === 'object' && !Array.isArray(obj) && obj !== null; - }; - const configurationPath = configurationPathKeys.join('.'); + _.set(this.configurationInput, configurationPath, value); - let newConfig; - const oldConfig = _.get(this.configurationInput, configurationPath); - if (oldConfig === undefined) { - newConfig = value; - } else if (Array.isArray(oldConfig) && Array.isArray(value)) { - newConfig = oldConfig.concat(value); - } else if (isObject(oldConfig) && isObject(value)) { - newConfig = _.merge(oldConfig, value); - } else if (typeof oldConfig === typeof value && typeof oldConfig !== 'object') { - newConfig = value; - } else { - throw new ServerlessError( - `Configuration cannot be extended at ${configurationPath}. Expected: ${typeof oldConfig}, Received: ${typeof value}`, - 'INVALID_EXTEND_WITH_TYPE_MISMATCH' - ); - } - - _.set(this.configurationInput, configurationPath, newConfig); - - const metaPathPrefix = configurationPath.replace(/\./g, '\0'); + const metaPathPrefix = configurationPathKeys.join('\0'); for (const key of this.variablesMeta.keys()) { if (key.startsWith(metaPathPrefix)) { this.variablesMeta.delete(key); } } - if (typeof newConfig !== 'object') { + if (typeof value !== 'object') { const lastKey = configurationPathKeys.pop(); - parseEntries({ [lastKey]: newConfig }, configurationPathKeys, this.variablesMeta); + parseEntries({ [lastKey]: value }, configurationPathKeys, this.variablesMeta); } else { - parseEntries(Object.entries(newConfig), configurationPathKeys, this.variablesMeta); + parseEntries(Object.entries(value), configurationPathKeys, this.variablesMeta); } } } diff --git a/test/unit/lib/serverless.test.js b/test/unit/lib/serverless.test.js index c203ee124ba..1c38f86587f 100644 --- a/test/unit/lib/serverless.test.js +++ b/test/unit/lib/serverless.test.js @@ -2,7 +2,6 @@ const chai = require('chai'); const sinon = require('sinon'); -const proxyquire = require('proxyquire').noCallThru(); chai.use(require('chai-as-promised')); chai.use(require('sinon-chai')); @@ -182,20 +181,27 @@ describe('test/unit/lib/serverless.test.js', () => { let serverless; let parseEntriesStub; - before(() => { + before(async () => { parseEntriesStub = sinon.stub().returns(new Map([])); - const ServerlessProxy = proxyquire('../../../lib/serverless.js', { - './configuration/variables/resolve-meta': { - parseEntries: parseEntriesStub, - }, - }); - - serverless = new ServerlessProxy({ - commands: ['print'], - options: {}, - serviceDir: null, - variablesMeta: new Map([]), - }); + try { + await runServerless({ + noService: true, + command: 'info', + modulesCacheStub: { + './lib/configuration/variables/resolve-meta': { + parseEntries: parseEntriesStub, + }, + }, + hooks: { + beforeInstanceInit: (serverlessInstance) => { + serverless = serverlessInstance; + throw Error('Stop serverless before init.'); + }, + }, + }); + } catch (error) { + // Not an error + } serverless.pluginManager = sinon.createStubInstance(PluginManager); serverless.classes.CLI = sinon.stub(serverless.classes.CLI); @@ -228,7 +234,7 @@ describe('test/unit/lib/serverless.test.js', () => { expect(serverless.configurationInput).to.deep.nested.include({ [path]: value }); }); - it('Should merge configuration object if it exists', async () => { + it('Should assign configuration object if it exists', async () => { const path = 'pre-existing'; const initialConfig = { [path]: { @@ -242,28 +248,7 @@ describe('test/unit/lib/serverless.test.js', () => { serverless.configurationInput = JSON.parse(JSON.stringify(initialConfig)); serverless.extendConfiguration(path.split('.'), value); - expect(serverless.configurationInput[path]).to.deep.include( - Object.assign(initialConfig[path], value) - ); - }); - - it('Should extend configuration array if it exists', async () => { - const path = 'pre-existing'; - const initialConfig = { - [path]: [1, 2, 3, 4], - }; - - const value = [4, 5, 6]; - serverless.configurationInput = JSON.parse(JSON.stringify(initialConfig)); - serverless.extendConfiguration(path.split('.'), value); - - expect(serverless.configurationInput[path]) - .to.be.an('array') - .that.includes.members(initialConfig[path]); - expect(serverless.configurationInput[path]).to.include.members(value); - expect(serverless.configurationInput[path]).to.have.length( - value.length + initialConfig[path].length - ); + expect(serverless.configurationInput[path]).to.deep.include(value); }); it('Should assign trivial types', async () => { @@ -293,24 +278,6 @@ describe('test/unit/lib/serverless.test.js', () => { expect(serverless.configurationInput[path][subpath]).to.not.equal(subvalue); }); - it('Should throw if types do not match', async () => { - const path = 'pre-existing'; - const initialConfig = { - [path]: [1, 2, 3, 4], - }; - const value = {}; - serverless.configurationInput = JSON.parse(JSON.stringify(initialConfig)); - - expect(() => serverless.extendConfiguration(path.split('.'), value)).to.throw( - ServerlessError - ); - }); - - it('Should throw if called after init', async () => { - await serverless.init(); - expect(() => serverless.extendConfiguration('', {})).to.throw(ServerlessError); - }); - it('Should clean variables meta in configurationPath', async () => { const path = 'path.to.insert'; const value = { @@ -333,27 +300,27 @@ describe('test/unit/lib/serverless.test.js', () => { }); }); - it('Should add variables to variables meta', async () => { - const path = 'path.to.insert'; - const value = { - newKey: 'value', - }; - serverless.configurationInput = {}; - - const resolvedVariables = { - var1: {}, - var2: {}, - }; - const fakeReturn = new Map(Object.entries(resolvedVariables)); - parseEntriesStub.returns(fakeReturn); - - const setStub = sinon.stub(serverless.variablesMeta, 'set'); - - serverless.extendConfiguration(path.split('.'), value); - - Object.getOwnPropertyNames(resolvedVariables).forEach((varName) => { - expect(setStub).to.have.been.calledWith(varName, sinon.match.any); + it('Should throw if called after init', async () => { + let localServerless; + await runServerless({ + config: { + service: 'test', + provider: { + name: 'aws', + }, + }, + command: 'print', + hooks: { + beforeInstanceInit: (serverlessInstance) => { + localServerless = serverlessInstance; + const { hooks: lifecycleHooks } = serverlessInstance.pluginManager; + for (const hookName of Object.keys(lifecycleHooks)) { + delete lifecycleHooks[hookName]; + } + }, + }, }); + expect(() => localServerless.extendConfiguration([], {})).to.throw(ServerlessError); }); }); }); From c9ad3f5e05b2f66c502bc5738fb3d5fb59d0d52c Mon Sep 17 00:00:00 2001 From: Marco Kleinlein <87859452+mklenbw@users.noreply.github.com> Date: Tue, 6 Dec 2022 09:21:33 +0100 Subject: [PATCH 14/36] Resolved review comments --- lib/serverless.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/serverless.js b/lib/serverless.js index f141995ab87..252245ab578 100644 --- a/lib/serverless.js +++ b/lib/serverless.js @@ -222,7 +222,7 @@ class Serverless { } extendConfiguration(configurationPathKeys, value) { - ensureArray(configurationPathKeys, { + configurationPathKeys = ensureArray(configurationPathKeys, { ensureItem: ensureString, }); if (configurationPathKeys.length < 1) { @@ -247,8 +247,7 @@ class Serverless { ); } - const configurationPath = configurationPathKeys.join('.'); - _.set(this.configurationInput, configurationPath, value); + _.set(this.configurationInput, configurationPathKeys, value); const metaPathPrefix = configurationPathKeys.join('\0'); for (const key of this.variablesMeta.keys()) { @@ -256,7 +255,7 @@ class Serverless { this.variablesMeta.delete(key); } } - if (typeof value !== 'object') { + if (!_.isObject(value)) { const lastKey = configurationPathKeys.pop(); parseEntries({ [lastKey]: value }, configurationPathKeys, this.variablesMeta); } else { From 98409532c9715b14b055f27c32472a307bf87fe4 Mon Sep 17 00:00:00 2001 From: Marco Kleinlein <87859452+mklenbw@users.noreply.github.com> Date: Tue, 6 Dec 2022 09:25:08 +0100 Subject: [PATCH 15/36] Added pending zero to testing --- test/unit/lib/serverless.test.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/unit/lib/serverless.test.js b/test/unit/lib/serverless.test.js index 1c38f86587f..89a55029e15 100644 --- a/test/unit/lib/serverless.test.js +++ b/test/unit/lib/serverless.test.js @@ -285,7 +285,9 @@ describe('test/unit/lib/serverless.test.js', () => { }; serverless.configurationInput = {}; const metaToKeep = ['path.to.keep'].map((_) => _.replace(/\./g, '\0')); - const metaToDelete = [path, `${path}.child`].map((_) => _.replace(/\./g, '\0')); + const metaToDelete = [path, `${path}.child`, `${path}.zero\0`].map((_) => + _.replace(/\./g, '\0') + ); const variablesMeta = metaToKeep.concat(metaToDelete); const keyStub = sinon.stub(serverless.variablesMeta, 'keys'); From f0f18e19b81205ee11ccabd838b400bb41363241 Mon Sep 17 00:00:00 2001 From: Marco Kleinlein <87859452+mklenbw@users.noreply.github.com> Date: Tue, 6 Dec 2022 13:28:25 +0100 Subject: [PATCH 16/36] Added condition to ensure properties with same prefix are kept --- lib/serverless.js | 2 +- test/unit/lib/serverless.test.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/serverless.js b/lib/serverless.js index 252245ab578..cc68f4c10e1 100644 --- a/lib/serverless.js +++ b/lib/serverless.js @@ -251,7 +251,7 @@ class Serverless { const metaPathPrefix = configurationPathKeys.join('\0'); for (const key of this.variablesMeta.keys()) { - if (key.startsWith(metaPathPrefix)) { + if (key === metaPathPrefix || key.startsWith(`${metaPathPrefix}\0`)) { this.variablesMeta.delete(key); } } diff --git a/test/unit/lib/serverless.test.js b/test/unit/lib/serverless.test.js index 89a55029e15..648ee1f7a86 100644 --- a/test/unit/lib/serverless.test.js +++ b/test/unit/lib/serverless.test.js @@ -284,7 +284,7 @@ describe('test/unit/lib/serverless.test.js', () => { newKey: 'value', }; serverless.configurationInput = {}; - const metaToKeep = ['path.to.keep'].map((_) => _.replace(/\./g, '\0')); + const metaToKeep = ['path.to.keep', `${path}suffix`].map((_) => _.replace(/\./g, '\0')); const metaToDelete = [path, `${path}.child`, `${path}.zero\0`].map((_) => _.replace(/\./g, '\0') ); From 7d75b798a125b1a6d443a0e1ac7f0f653f6c569d Mon Sep 17 00:00:00 2001 From: Marco Kleinlein <87859452+mklenbw@users.noreply.github.com> Date: Mon, 12 Dec 2022 09:27:42 +0100 Subject: [PATCH 17/36] Removed unused parameter --- scripts/serverless.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/scripts/serverless.js b/scripts/serverless.js index c181e789307..9401ba2de36 100755 --- a/scripts/serverless.js +++ b/scripts/serverless.js @@ -594,8 +594,6 @@ processSpanPromise = (async () => { configuration, serviceDir, configurationFilename, - isConfigurationResolved: - commands[0] === 'plugin' || Boolean(variablesMeta && !variablesMeta.size), commands, options, variablesMeta, From 1d3fc6ff03cafbcf1cd4a834fabc98f1b2249801 Mon Sep 17 00:00:00 2001 From: Marco Kleinlein <87859452+mklenbw@users.noreply.github.com> Date: Mon, 12 Dec 2022 11:02:44 +0100 Subject: [PATCH 18/36] Always prepare resolverConfiguration; Reset resolve paths if config was extended --- scripts/serverless.js | 45 +++++++++++++++++++++++++++---------------- 1 file changed, 28 insertions(+), 17 deletions(-) diff --git a/scripts/serverless.js b/scripts/serverless.js index 9401ba2de36..ad1eb325a3d 100755 --- a/scripts/serverless.js +++ b/scripts/serverless.js @@ -296,29 +296,31 @@ processSpanPromise = (async () => { } let envVarNamesNeededForDotenvResolution; + resolverConfiguration = { + serviceDir, + configuration, + variablesMeta, + sources: { + env: require('../lib/configuration/variables/sources/env'), + file: require('../lib/configuration/variables/sources/file'), + opt: require('../lib/configuration/variables/sources/opt'), + self: require('../lib/configuration/variables/sources/self'), + strToBool: require('../lib/configuration/variables/sources/str-to-bool'), + sls: require('../lib/configuration/variables/sources/instance-dependent/get-sls')(), + }, + options: filterSupportedOptions(options, { commandSchema, providerName }), + fulfilledSources: new Set(['file', 'self', 'strToBool']), + propertyPathsToResolve: new Set(['provider\0name', 'provider\0stage', 'useDotenv']), + variableSourcesInConfig, + }; + if (variablesMeta.size) { processLog.debug('resolve variables in core properties'); // Some properties are configured with variables // Resolve eventual variables in `provider.stage` and `useDotEnv` // (required for reliable .env resolution) - resolverConfiguration = { - serviceDir, - configuration, - variablesMeta, - sources: { - env: require('../lib/configuration/variables/sources/env'), - file: require('../lib/configuration/variables/sources/file'), - opt: require('../lib/configuration/variables/sources/opt'), - self: require('../lib/configuration/variables/sources/self'), - strToBool: require('../lib/configuration/variables/sources/str-to-bool'), - sls: require('../lib/configuration/variables/sources/instance-dependent/get-sls')(), - }, - options: filterSupportedOptions(options, { commandSchema, providerName }), - fulfilledSources: new Set(['file', 'self', 'strToBool']), - propertyPathsToResolve: new Set(['provider\0name', 'provider\0stage', 'useDotenv']), - variableSourcesInConfig, - }; + if (isInteractiveSetup) resolverConfiguration.fulfilledSources.add('opt'); await resolveVariables(resolverConfiguration); @@ -589,6 +591,10 @@ processSpanPromise = (async () => { } } + const isConfigurationExtended = function (oldSize) { + return oldSize < variablesMeta.size; + }.bind(null, variablesMeta.size); + processLog.debug('construct Serverless instance'); serverless = new Serverless({ configuration, @@ -605,6 +611,11 @@ processSpanPromise = (async () => { processLog.debug('initialize Serverless instance'); await serverless.init(); + if (isConfigurationExtended() && resolverConfiguration.propertyPathsToResolve) { + processLog.debug('resolve variables in all properties'); + delete resolverConfiguration.propertyPathsToResolve; + } + // IIFE for maintenance convenience await (async () => { if (!configuration) return; From 18b3a5777d8690aa274c4bf580f8c5fc74deb788 Mon Sep 17 00:00:00 2001 From: Marco Kleinlein <87859452+mklenbw@users.noreply.github.com> Date: Mon, 12 Dec 2022 11:10:21 +0100 Subject: [PATCH 19/36] Run tests on fixture --- .../plugin/extend-config-plugin/index.js | 35 ++++ test/unit/lib/serverless.test.js | 197 ++++++++---------- 2 files changed, 120 insertions(+), 112 deletions(-) create mode 100644 test/fixtures/programmatic/plugin/extend-config-plugin/index.js diff --git a/test/fixtures/programmatic/plugin/extend-config-plugin/index.js b/test/fixtures/programmatic/plugin/extend-config-plugin/index.js new file mode 100644 index 00000000000..f8c9244b42b --- /dev/null +++ b/test/fixtures/programmatic/plugin/extend-config-plugin/index.js @@ -0,0 +1,35 @@ +'use strict'; + +module.exports = class TestPlugin { + constructor(serverless, options, utils) { + this.serverless = serverless; + this.options = options; + this.utils = utils; + + this.target = this.serverless.configurationInput.custom.extendConfig.target; + this.value = this.serverless.configurationInput.custom.extendConfig.value; + if (typeof this.value === 'string') { + this.value = this.value.replace(/%/g, '$'); + } + + this.hook = this.serverless.configurationInput.custom.extendConfig.hook; + if (this.hook !== undefined) { + this.hooks = { + [this.hook]: () => this.extend(), + }; + } else { + this.hook = 'async init'; + } + } + + async asyncInit() { + if (this.hooks === undefined) { + this.extend(); + } + } + + extend() { + this.utils.log(`Excuting "${this.hook}" for extension of "${this.target}" with: ${this.value}`); + this.serverless.extendConfiguration(this.target, this.value); + } +}; diff --git a/test/unit/lib/serverless.test.js b/test/unit/lib/serverless.test.js index 648ee1f7a86..160e02ab41d 100644 --- a/test/unit/lib/serverless.test.js +++ b/test/unit/lib/serverless.test.js @@ -1,10 +1,8 @@ 'use strict'; const chai = require('chai'); -const sinon = require('sinon'); chai.use(require('chai-as-promised')); -chai.use(require('sinon-chai')); const { expect } = chai; @@ -19,6 +17,11 @@ const ConfigSchemaHandler = require('../../../lib/classes/config-schema-handler' const CLI = require('../../../lib/classes/cli'); const ServerlessError = require('../../../lib/serverless-error'); const runServerless = require('../../utils/run-serverless'); +const spawn = require('child-process-ext/spawn'); +const programmaticFixturesEngine = require('../../fixtures/programmatic'); +const path = require('path'); +const yaml = require('js-yaml'); +const _ = require('lodash'); describe('Serverless', () => { let serverless; @@ -178,151 +181,121 @@ describe('test/unit/lib/serverless.test.js', () => { }); describe('Extend configuration', () => { - let serverless; - let parseEntriesStub; + let awsRegion; + const serverlessPath = path.resolve(__dirname, '../../../scripts/serverless.js'); + + const runFixture = async (extendConfig, customExt) => { + const configExt = { + plugins: ['./extend-config-plugin/index.js'], + provider: { + stage: 'dev', + }, + custom: Object.assign( + { + extendConfig, + }, + customExt + ), + }; - before(async () => { - parseEntriesStub = sinon.stub().returns(new Map([])); + const { servicePath: serviceDir } = await programmaticFixturesEngine.setup('plugin', { + configExt, + }); try { - await runServerless({ - noService: true, - command: 'info', - modulesCacheStub: { - './lib/configuration/variables/resolve-meta': { - parseEntries: parseEntriesStub, - }, - }, - hooks: { - beforeInstanceInit: (serverlessInstance) => { - serverless = serverlessInstance; - throw Error('Stop serverless before init.'); - }, - }, + const serverlessProcess = await spawn('node', [serverlessPath, 'print'], { + cwd: serviceDir, }); + return yaml.load(String(serverlessProcess.stdoutBuffer)); } catch (error) { - // Not an error + return String(error.stdoutBuffer); } + }; - serverless.pluginManager = sinon.createStubInstance(PluginManager); - serverless.classes.CLI = sinon.stub(serverless.classes.CLI); + before(() => { + awsRegion = process.env.AWS_REGION; + process.env.AWS_REGION = 'us-east-1'; }); after(() => { - sinon.restore(); - }); - - beforeEach(async () => { - serverless.isConfigurationExtendable = true; - serverless.variablesMeta = new Map([]); + process.env.AWS_REGION = awsRegion; }); - it('Should add configuration path if it not exists', async () => { - const initialConfig = { - 'pre-existing': { - path: true, + it('Extends configuration with given values', async () => { + const extendConfig = { + value: { + deep: 'config', }, + target: ['custom', 'target', 'value'], }; - const path = 'path.to.insert'; - const value = { - newKey: 'value', - }; - serverless.configurationInput = JSON.parse(JSON.stringify(initialConfig)); - serverless.extendConfiguration(path.split('.'), value); + const configuration = await runFixture(extendConfig, {}); + expect(configuration).to.be.an('object', configuration); - expect(serverless.configurationInput).to.deep.include(initialConfig); - expect(serverless.configurationInput).to.deep.nested.include({ [path]: value }); + const targetValue = _.get(configuration, extendConfig.target); + expect(targetValue).to.deep.equal(extendConfig.value); }); - it('Should assign configuration object if it exists', async () => { - const path = 'pre-existing'; - const initialConfig = { - [path]: { - path: true, + it('Overwrites existing values', async () => { + const customExt = { + target: { + someProperty: 'value', }, }; - const value = { - newKey: 'value', + const extendConfig = { + value: { + deep: 'config', + }, + target: ['custom', 'target', 'value'], }; - serverless.configurationInput = JSON.parse(JSON.stringify(initialConfig)); - serverless.extendConfiguration(path.split('.'), value); - expect(serverless.configurationInput[path]).to.deep.include(value); - }); + const configuration = await runFixture(extendConfig, customExt); + expect(configuration).to.be.an('object', configuration); - it('Should assign trivial types', async () => { - const path = 'pre-existing'; - const initialConfig = { - [path]: 'some string', - }; - const value = 'other string'; - serverless.configurationInput = JSON.parse(JSON.stringify(initialConfig)); - serverless.extendConfiguration(path.split('.'), value); - expect(serverless.configurationInput[path]).to.equal(value); + const targetValue = _.get(configuration, extendConfig.target); + expect(targetValue).to.deep.equal(extendConfig.value); }); - it('Should deeply copy the new value', async () => { - const path = 'level1'; - const subpath = 'level2'; - const subvalue = { - level3: 'copy', + it('Resolves variables in extended values', async () => { + const extendConfig = { + value: '%{self:custom.extendConfig.target}', + target: ['custom', 'target'], }; - const value = { - [subpath]: subvalue, - }; - serverless.configurationInput = {}; - serverless.extendConfiguration(path.split('.'), value); - expect(serverless.configurationInput[path]).to.not.equal(value); - expect(serverless.configurationInput[path][subpath]).to.not.equal(subvalue); + const configuration = await runFixture(extendConfig, {}); + expect(configuration).to.be.an('object', configuration); + + const targetValue = _.get(configuration, extendConfig.target); + expect(targetValue).to.deep.equal(extendConfig.target); }); - it('Should clean variables meta in configurationPath', async () => { - const path = 'path.to.insert'; - const value = { - newKey: 'value', + it('Throws when extending at root', async () => { + const extendConfig = { + value: { + deep: 'config', + }, + target: [], }; - serverless.configurationInput = {}; - const metaToKeep = ['path.to.keep', `${path}suffix`].map((_) => _.replace(/\./g, '\0')); - const metaToDelete = [path, `${path}.child`, `${path}.zero\0`].map((_) => - _.replace(/\./g, '\0') - ); - const variablesMeta = metaToKeep.concat(metaToDelete); - - const keyStub = sinon.stub(serverless.variablesMeta, 'keys'); - keyStub.returns(variablesMeta); - const deleteStub = sinon.stub(serverless.variablesMeta, 'delete'); + const configuration = await runFixture(extendConfig, {}); + expect(configuration).to.be.a('string', configuration); - serverless.extendConfiguration(path.split('.'), value); - - metaToDelete.forEach((meta) => { - expect(deleteStub).to.have.been.calledWith(meta); - }); + expect(configuration).to.include('needs to contain at least one element', configuration); }); - it('Should throw if called after init', async () => { - let localServerless; - await runServerless({ - config: { - service: 'test', - provider: { - name: 'aws', - }, + it('Throws when called after init', async () => { + const extendConfig = { + hook: 'initialize', + value: { + deep: 'config', }, - command: 'print', - hooks: { - beforeInstanceInit: (serverlessInstance) => { - localServerless = serverlessInstance; - const { hooks: lifecycleHooks } = serverlessInstance.pluginManager; - for (const hookName of Object.keys(lifecycleHooks)) { - delete lifecycleHooks[hookName]; - } - }, - }, - }); - expect(() => localServerless.extendConfiguration([], {})).to.throw(ServerlessError); + target: ['custom', 'target'], + }; + + const configuration = await runFixture(extendConfig, {}); + expect(configuration).to.be.a('string', configuration); + + expect(configuration).to.include('cannot be used after init', configuration); }); }); }); From 5c0700740a2df4fddd76f9d34b61b8465019f7dd Mon Sep 17 00:00:00 2001 From: Marco Kleinlein <87859452+mklenbw@users.noreply.github.com> Date: Mon, 12 Dec 2022 11:11:05 +0100 Subject: [PATCH 20/36] Fixed minor errors --- lib/serverless.js | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/lib/serverless.js b/lib/serverless.js index cc68f4c10e1..7310514325d 100644 --- a/lib/serverless.js +++ b/lib/serverless.js @@ -101,7 +101,7 @@ class Serverless { // Old variables resolver is dropped, yet some plugins access service properties through // `variables` class. Below patch ensures those plugins won't get broken this.variables = { service: this.service }; - this.variablesMeta = config.variablesMeta; + this.variablesMeta = config.variablesMeta || new Map([]); this.pluginManager = new PluginManager(this); this.configSchemaHandler = new ConfigSchemaHandler(this); @@ -228,7 +228,7 @@ class Serverless { if (configurationPathKeys.length < 1) { throw new ServerlessError( 'ConfigurationPathKeys needs to contain at least one element.', - 'INVALID_EXTEND_AFTER_INIT' + 'INVALID_EXTEND_AT_ROOT' ); } @@ -248,7 +248,6 @@ class Serverless { } _.set(this.configurationInput, configurationPathKeys, value); - const metaPathPrefix = configurationPathKeys.join('\0'); for (const key of this.variablesMeta.keys()) { if (key === metaPathPrefix || key.startsWith(`${metaPathPrefix}\0`)) { @@ -257,10 +256,9 @@ class Serverless { } if (!_.isObject(value)) { const lastKey = configurationPathKeys.pop(); - parseEntries({ [lastKey]: value }, configurationPathKeys, this.variablesMeta); - } else { - parseEntries(Object.entries(value), configurationPathKeys, this.variablesMeta); + value = { [lastKey]: value }; } + parseEntries(Object.entries(value), configurationPathKeys, this.variablesMeta); } } From 68e20ab3af836b93dfe85d6c681bddd0f9106875 Mon Sep 17 00:00:00 2001 From: Marco Kleinlein <87859452+mklenbw@users.noreply.github.com> Date: Tue, 13 Dec 2022 12:58:02 +0100 Subject: [PATCH 21/36] Added comment --- lib/serverless.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/serverless.js b/lib/serverless.js index 7310514325d..53f7fbc97af 100644 --- a/lib/serverless.js +++ b/lib/serverless.js @@ -101,6 +101,10 @@ class Serverless { // Old variables resolver is dropped, yet some plugins access service properties through // `variables` class. Below patch ensures those plugins won't get broken this.variables = { service: this.service }; + + // `config.variablesMeta` will not be provided if the initial resolution of variables failed. + // We're ensuring it locally not to disrupt configuration extensions as eventually done by + // the plugins (which are still loaded in spite of the error, if e.g. help output was requested) this.variablesMeta = config.variablesMeta || new Map([]); this.pluginManager = new PluginManager(this); this.configSchemaHandler = new ConfigSchemaHandler(this); From 7e3f5c7b49034dd7d1fc8a08abd803523360699c Mon Sep 17 00:00:00 2001 From: Marco Kleinlein <87859452+mklenbw@users.noreply.github.com> Date: Tue, 13 Dec 2022 13:01:18 +0100 Subject: [PATCH 22/36] Changed error text --- lib/serverless.js | 2 +- test/unit/lib/serverless.test.js | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/serverless.js b/lib/serverless.js index 53f7fbc97af..a40747ee1cf 100644 --- a/lib/serverless.js +++ b/lib/serverless.js @@ -238,7 +238,7 @@ class Serverless { if (!this.isConfigurationExtendable) { throw new ServerlessError( - 'ExtendConfiguration cannot be used after init.', + 'Cannot extend configuration: It can only be extended during initialization phase.', 'INVALID_EXTEND_AFTER_INIT' ); } diff --git a/test/unit/lib/serverless.test.js b/test/unit/lib/serverless.test.js index 160e02ab41d..dac9a982671 100644 --- a/test/unit/lib/serverless.test.js +++ b/test/unit/lib/serverless.test.js @@ -295,7 +295,10 @@ describe('test/unit/lib/serverless.test.js', () => { const configuration = await runFixture(extendConfig, {}); expect(configuration).to.be.a('string', configuration); - expect(configuration).to.include('cannot be used after init', configuration); + expect(configuration).to.include( + 'It can only be extended during initialization phase', + configuration + ); }); }); }); From e6205bb795504b20074598f13b936583163b50c2 Mon Sep 17 00:00:00 2001 From: Marco Kleinlein <87859452+mklenbw@users.noreply.github.com> Date: Tue, 13 Dec 2022 13:11:51 +0100 Subject: [PATCH 23/36] Changed error type --- lib/serverless.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/serverless.js b/lib/serverless.js index a40747ee1cf..3766957c9b6 100644 --- a/lib/serverless.js +++ b/lib/serverless.js @@ -230,14 +230,14 @@ class Serverless { ensureItem: ensureString, }); if (configurationPathKeys.length < 1) { - throw new ServerlessError( + throw new Error( 'ConfigurationPathKeys needs to contain at least one element.', 'INVALID_EXTEND_AT_ROOT' ); } if (!this.isConfigurationExtendable) { - throw new ServerlessError( + throw new Error( 'Cannot extend configuration: It can only be extended during initialization phase.', 'INVALID_EXTEND_AFTER_INIT' ); @@ -245,7 +245,7 @@ class Serverless { try { value = JSON.parse(JSON.stringify(value)); } catch (error) { - throw new ServerlessError( + throw new Error( 'ExtendConfiguration called with invalid data. Value is not json-serializable. Error: ${error}', 'INVALID_EXTEND_VALUE' ); From 202a12b7fd07c356c4a3ed4f6f23ca8fae742735 Mon Sep 17 00:00:00 2001 From: Marco Kleinlein <87859452+mklenbw@users.noreply.github.com> Date: Tue, 13 Dec 2022 13:13:04 +0100 Subject: [PATCH 24/36] Removed ids as we are using default errors --- lib/serverless.js | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/lib/serverless.js b/lib/serverless.js index 3766957c9b6..31741d4de07 100644 --- a/lib/serverless.js +++ b/lib/serverless.js @@ -230,24 +230,19 @@ class Serverless { ensureItem: ensureString, }); if (configurationPathKeys.length < 1) { - throw new Error( - 'ConfigurationPathKeys needs to contain at least one element.', - 'INVALID_EXTEND_AT_ROOT' - ); + throw new Error('ConfigurationPathKeys needs to contain at least one element.'); } if (!this.isConfigurationExtendable) { throw new Error( - 'Cannot extend configuration: It can only be extended during initialization phase.', - 'INVALID_EXTEND_AFTER_INIT' + 'Cannot extend configuration: It can only be extended during initialization phase.' ); } try { value = JSON.parse(JSON.stringify(value)); } catch (error) { throw new Error( - 'ExtendConfiguration called with invalid data. Value is not json-serializable. Error: ${error}', - 'INVALID_EXTEND_VALUE' + 'ExtendConfiguration called with invalid data. Value is not json-serializable. Error: ${error}' ); } From 0879b0e07baf155719c5ad402af42c975206754e Mon Sep 17 00:00:00 2001 From: Marco Kleinlein <87859452+mklenbw@users.noreply.github.com> Date: Tue, 13 Dec 2022 13:15:16 +0100 Subject: [PATCH 25/36] Changed error texts --- lib/serverless.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/serverless.js b/lib/serverless.js index 31741d4de07..a3381234ebc 100644 --- a/lib/serverless.js +++ b/lib/serverless.js @@ -230,7 +230,9 @@ class Serverless { ensureItem: ensureString, }); if (configurationPathKeys.length < 1) { - throw new Error('ConfigurationPathKeys needs to contain at least one element.'); + throw new Error( + 'Cannot extend configuration: ConfigurationPathKeys needs to contain at least one element.' + ); } if (!this.isConfigurationExtendable) { @@ -241,9 +243,7 @@ class Serverless { try { value = JSON.parse(JSON.stringify(value)); } catch (error) { - throw new Error( - 'ExtendConfiguration called with invalid data. Value is not json-serializable. Error: ${error}' - ); + throw new Error(`Cannot extend configuration: Received non JSON value: ${value}`); } _.set(this.configurationInput, configurationPathKeys, value); From 23062544c4736d13d86304ad6002d096c09b7cfb Mon Sep 17 00:00:00 2001 From: Marco Kleinlein <87859452+mklenbw@users.noreply.github.com> Date: Tue, 13 Dec 2022 13:20:25 +0100 Subject: [PATCH 26/36] Add resolverConfiguraiton after init if config is extended --- scripts/serverless.js | 69 ++++++++++++++++++++++++++----------------- 1 file changed, 42 insertions(+), 27 deletions(-) diff --git a/scripts/serverless.js b/scripts/serverless.js index ad1eb325a3d..3cd66bc44a0 100755 --- a/scripts/serverless.js +++ b/scripts/serverless.js @@ -296,30 +296,29 @@ processSpanPromise = (async () => { } let envVarNamesNeededForDotenvResolution; - resolverConfiguration = { - serviceDir, - configuration, - variablesMeta, - sources: { - env: require('../lib/configuration/variables/sources/env'), - file: require('../lib/configuration/variables/sources/file'), - opt: require('../lib/configuration/variables/sources/opt'), - self: require('../lib/configuration/variables/sources/self'), - strToBool: require('../lib/configuration/variables/sources/str-to-bool'), - sls: require('../lib/configuration/variables/sources/instance-dependent/get-sls')(), - }, - options: filterSupportedOptions(options, { commandSchema, providerName }), - fulfilledSources: new Set(['file', 'self', 'strToBool']), - propertyPathsToResolve: new Set(['provider\0name', 'provider\0stage', 'useDotenv']), - variableSourcesInConfig, - }; - if (variablesMeta.size) { processLog.debug('resolve variables in core properties'); // Some properties are configured with variables // Resolve eventual variables in `provider.stage` and `useDotEnv` // (required for reliable .env resolution) + resolverConfiguration = { + serviceDir, + configuration, + variablesMeta, + sources: { + env: require('../lib/configuration/variables/sources/env'), + file: require('../lib/configuration/variables/sources/file'), + opt: require('../lib/configuration/variables/sources/opt'), + self: require('../lib/configuration/variables/sources/self'), + strToBool: require('../lib/configuration/variables/sources/str-to-bool'), + sls: require('../lib/configuration/variables/sources/instance-dependent/get-sls')(), + }, + options: filterSupportedOptions(options, { commandSchema, providerName }), + fulfilledSources: new Set(['file', 'self', 'strToBool']), + propertyPathsToResolve: new Set(['provider\0name', 'provider\0stage', 'useDotenv']), + variableSourcesInConfig, + }; if (isInteractiveSetup) resolverConfiguration.fulfilledSources.add('opt'); await resolveVariables(resolverConfiguration); @@ -591,10 +590,6 @@ processSpanPromise = (async () => { } } - const isConfigurationExtended = function (oldSize) { - return oldSize < variablesMeta.size; - }.bind(null, variablesMeta.size); - processLog.debug('construct Serverless instance'); serverless = new Serverless({ configuration, @@ -611,11 +606,6 @@ processSpanPromise = (async () => { processLog.debug('initialize Serverless instance'); await serverless.init(); - if (isConfigurationExtended() && resolverConfiguration.propertyPathsToResolve) { - processLog.debug('resolve variables in all properties'); - delete resolverConfiguration.propertyPathsToResolve; - } - // IIFE for maintenance convenience await (async () => { if (!configuration) return; @@ -654,6 +644,31 @@ processSpanPromise = (async () => { if (hasFinalCommandSchema) require('../lib/cli/ensure-supported-command')(configuration); if (isHelpRequest) return; if (!_.get(variablesMeta, 'size')) return; + if (!resolverConfiguration) { + // There were no variables in the initial configuration, yet it was extended by + // the plugins with ones. + // In this case we need to ensure `resolverConfiguration` which initially was not setup + resolverConfiguration = { + serviceDir, + configuration, + variablesMeta, + sources: { + env: require('../lib/configuration/variables/sources/env'), + file: require('../lib/configuration/variables/sources/file'), + opt: require('../lib/configuration/variables/sources/opt'), + self: require('../lib/configuration/variables/sources/self'), + strToBool: require('../lib/configuration/variables/sources/str-to-bool'), + sls: require('../lib/configuration/variables/sources/instance-dependent/get-sls')(), + }, + options: filterSupportedOptions(options, { commandSchema, providerName }), + fulfilledSources: new Set(['env', 'file', 'self', 'strToBool']), + propertyPathsToResolve: + commands[0] === 'plugin' + ? new Set(['plugin', 'provider\0name', 'provider\0stage', 'useDotenv']) + : null, + variableSourcesInConfig, + }; + } if (commandSchema) { resolverConfiguration.options = filterSupportedOptions(options, { From 4a0cea012307af7029dc87b55d96ec5b324c8da2 Mon Sep 17 00:00:00 2001 From: Marco Kleinlein <87859452+mklenbw@users.noreply.github.com> Date: Tue, 13 Dec 2022 13:56:07 +0100 Subject: [PATCH 27/36] Simplified tests and fixture --- .../plugin/extend-config-plugin/index.js | 39 +++--- test/unit/lib/serverless.test.js | 126 +++++------------- 2 files changed, 52 insertions(+), 113 deletions(-) diff --git a/test/fixtures/programmatic/plugin/extend-config-plugin/index.js b/test/fixtures/programmatic/plugin/extend-config-plugin/index.js index f8c9244b42b..b5c9236083c 100644 --- a/test/fixtures/programmatic/plugin/extend-config-plugin/index.js +++ b/test/fixtures/programmatic/plugin/extend-config-plugin/index.js @@ -6,30 +6,31 @@ module.exports = class TestPlugin { this.options = options; this.utils = utils; - this.target = this.serverless.configurationInput.custom.extendConfig.target; - this.value = this.serverless.configurationInput.custom.extendConfig.value; - if (typeof this.value === 'string') { - this.value = this.value.replace(/%/g, '$'); - } - - this.hook = this.serverless.configurationInput.custom.extendConfig.hook; - if (this.hook !== undefined) { - this.hooks = { - [this.hook]: () => this.extend(), - }; - } else { - this.hook = 'async init'; - } + this.hooks = { + initialize: () => this.extendAfterInit(), + }; } async asyncInit() { - if (this.hooks === undefined) { - this.extend(); + const configExt = { + var: 'value', + }; + this.serverless.extendConfiguration(['custom', 'extend', 'value'], configExt); + this.serverless.extendConfiguration(['custom', 'extend', 'preexist'], configExt); + this.serverless.extendConfiguration(['custom', 'extend', 'ref'], '${self:custom.extend.value}'); + + try { + this.serverless.extendConfiguration([], { custom: {} }); + } catch (error) { + // ignore this } } - extend() { - this.utils.log(`Excuting "${this.hook}" for extension of "${this.target}" with: ${this.value}`); - this.serverless.extendConfiguration(this.target, this.value); + extendAfterInit() { + try { + this.serverless.extendConfiguration(['custom', 'extend', 'afterInit'], 'value'); + } catch (error) { + // ignore this + } } }; diff --git a/test/unit/lib/serverless.test.js b/test/unit/lib/serverless.test.js index dac9a982671..dbac22d02b5 100644 --- a/test/unit/lib/serverless.test.js +++ b/test/unit/lib/serverless.test.js @@ -4,7 +4,7 @@ const chai = require('chai'); chai.use(require('chai-as-promised')); -const { expect } = chai; +const { expect, assert } = chai; const Serverless = require('../../../lib/serverless'); const semverRegex = require('semver-regex'); @@ -182,123 +182,61 @@ describe('test/unit/lib/serverless.test.js', () => { describe('Extend configuration', () => { let awsRegion; + // see fixture plugin in ./test/fixtures/programmatic/plugin/extend-config-plugin/index.js + const targetValuePath = ['custom', 'extend', 'value']; + const targetValuePreexistPath = ['custom', 'extend', 'preexist']; + const targetValueAfterInitPath = ['custom', 'extend', 'afterInit']; + const targetRefPath = ['custom', 'extend', 'ref']; + const serverlessPath = path.resolve(__dirname, '../../../scripts/serverless.js'); - const runFixture = async (extendConfig, customExt) => { + before(() => { + awsRegion = process.env.AWS_REGION; + process.env.AWS_REGION = 'us-east-1'; + }); + + after(() => { + process.env.AWS_REGION = awsRegion; + }); + + it('Extends configuration with given values', async () => { + const customExt = { custom: {} }; const configExt = { plugins: ['./extend-config-plugin/index.js'], provider: { stage: 'dev', }, - custom: Object.assign( - { - extendConfig, - }, - customExt - ), + custom: {}, }; + _.set(customExt, targetValuePreexistPath, 'test_value'); const { servicePath: serviceDir } = await programmaticFixturesEngine.setup('plugin', { configExt, }); + let configuration; try { const serverlessProcess = await spawn('node', [serverlessPath, 'print'], { cwd: serviceDir, }); - return yaml.load(String(serverlessProcess.stdoutBuffer)); + configuration = yaml.load(String(serverlessProcess.stdoutBuffer)); } catch (error) { - return String(error.stdoutBuffer); + const errorMessage = String(error.stdoutBuffer); + assert.fail(errorMessage); } - }; - before(() => { - awsRegion = process.env.AWS_REGION; - process.env.AWS_REGION = 'us-east-1'; - }); - - after(() => { - process.env.AWS_REGION = awsRegion; - }); - - it('Extends configuration with given values', async () => { - const extendConfig = { - value: { - deep: 'config', - }, - target: ['custom', 'target', 'value'], - }; - - const configuration = await runFixture(extendConfig, {}); expect(configuration).to.be.an('object', configuration); - const targetValue = _.get(configuration, extendConfig.target); - expect(targetValue).to.deep.equal(extendConfig.value); - }); + const targetValue = _.get(configuration, targetValuePath); + expect(targetValue).to.not.be.undefined; - it('Overwrites existing values', async () => { - const customExt = { - target: { - someProperty: 'value', - }, - }; - - const extendConfig = { - value: { - deep: 'config', - }, - target: ['custom', 'target', 'value'], - }; - - const configuration = await runFixture(extendConfig, customExt); - expect(configuration).to.be.an('object', configuration); - - const targetValue = _.get(configuration, extendConfig.target); - expect(targetValue).to.deep.equal(extendConfig.value); - }); - - it('Resolves variables in extended values', async () => { - const extendConfig = { - value: '%{self:custom.extendConfig.target}', - target: ['custom', 'target'], - }; - - const configuration = await runFixture(extendConfig, {}); - expect(configuration).to.be.an('object', configuration); - - const targetValue = _.get(configuration, extendConfig.target); - expect(targetValue).to.deep.equal(extendConfig.target); - }); - - it('Throws when extending at root', async () => { - const extendConfig = { - value: { - deep: 'config', - }, - target: [], - }; - - const configuration = await runFixture(extendConfig, {}); - expect(configuration).to.be.a('string', configuration); - - expect(configuration).to.include('needs to contain at least one element', configuration); - }); - - it('Throws when called after init', async () => { - const extendConfig = { - hook: 'initialize', - value: { - deep: 'config', - }, - target: ['custom', 'target'], - }; + const targetValueAfterInit = _.get(configuration, targetValueAfterInitPath); + expect(targetValueAfterInit).to.be.undefined; - const configuration = await runFixture(extendConfig, {}); - expect(configuration).to.be.a('string', configuration); + const refValue = _.get(configuration, targetRefPath); + expect(refValue).to.deep.equal(targetValue); - expect(configuration).to.include( - 'It can only be extended during initialization phase', - configuration - ); + const preexistValue = _.get(configuration, targetValuePreexistPath); + expect(preexistValue).to.deep.equal(targetValue); }); }); }); From c96ff53e69a9f614983cda8384b8a4ff9b5f2bf5 Mon Sep 17 00:00:00 2001 From: Marco Kleinlein <87859452+mklenbw@users.noreply.github.com> Date: Tue, 13 Dec 2022 14:48:30 +0100 Subject: [PATCH 28/36] Moved paths to fixture --- .../plugin/extend-config-plugin/index.js | 17 ++++++++++--- test/unit/lib/serverless.test.js | 25 ++++++++----------- 2 files changed, 24 insertions(+), 18 deletions(-) diff --git a/test/fixtures/programmatic/plugin/extend-config-plugin/index.js b/test/fixtures/programmatic/plugin/extend-config-plugin/index.js index b5c9236083c..eda7d672fe3 100644 --- a/test/fixtures/programmatic/plugin/extend-config-plugin/index.js +++ b/test/fixtures/programmatic/plugin/extend-config-plugin/index.js @@ -1,5 +1,12 @@ 'use strict'; +const pluginConfig = { + targetValuePath: ['custom', 'extend', 'value'], + overwriteValuePath: ['custom', 'extend', 'overwrite'], + afterInitValuePath: ['custom', 'extend', 'afterInit'], + refValuePath: ['custom', 'extend', 'ref'], +}; + module.exports = class TestPlugin { constructor(serverless, options, utils) { this.serverless = serverless; @@ -15,9 +22,9 @@ module.exports = class TestPlugin { const configExt = { var: 'value', }; - this.serverless.extendConfiguration(['custom', 'extend', 'value'], configExt); - this.serverless.extendConfiguration(['custom', 'extend', 'preexist'], configExt); - this.serverless.extendConfiguration(['custom', 'extend', 'ref'], '${self:custom.extend.value}'); + this.serverless.extendConfiguration(pluginConfig.targetValuePath, configExt); + this.serverless.extendConfiguration(pluginConfig.overwriteValuePath, configExt); + this.serverless.extendConfiguration(pluginConfig.refValuePath, '${self:custom.extend.value}'); try { this.serverless.extendConfiguration([], { custom: {} }); @@ -28,9 +35,11 @@ module.exports = class TestPlugin { extendAfterInit() { try { - this.serverless.extendConfiguration(['custom', 'extend', 'afterInit'], 'value'); + this.serverless.extendConfiguration(pluginConfig.afterInitValuePath, 'value'); } catch (error) { // ignore this } } }; + +module.exports.pluginConfig = pluginConfig; diff --git a/test/unit/lib/serverless.test.js b/test/unit/lib/serverless.test.js index dbac22d02b5..8a6883dda37 100644 --- a/test/unit/lib/serverless.test.js +++ b/test/unit/lib/serverless.test.js @@ -182,11 +182,8 @@ describe('test/unit/lib/serverless.test.js', () => { describe('Extend configuration', () => { let awsRegion; - // see fixture plugin in ./test/fixtures/programmatic/plugin/extend-config-plugin/index.js - const targetValuePath = ['custom', 'extend', 'value']; - const targetValuePreexistPath = ['custom', 'extend', 'preexist']; - const targetValueAfterInitPath = ['custom', 'extend', 'afterInit']; - const targetRefPath = ['custom', 'extend', 'ref']; + const pluginConfig = + require('../../fixtures/programmatic/plugin/extend-config-plugin').pluginConfig; const serverlessPath = path.resolve(__dirname, '../../../scripts/serverless.js'); @@ -208,7 +205,7 @@ describe('test/unit/lib/serverless.test.js', () => { }, custom: {}, }; - _.set(customExt, targetValuePreexistPath, 'test_value'); + _.set(customExt, pluginConfig.overwriteValuePath, 'test_value'); const { servicePath: serviceDir } = await programmaticFixturesEngine.setup('plugin', { configExt, @@ -226,17 +223,17 @@ describe('test/unit/lib/serverless.test.js', () => { expect(configuration).to.be.an('object', configuration); - const targetValue = _.get(configuration, targetValuePath); - expect(targetValue).to.not.be.undefined; + const targetValue = _.get(configuration, pluginConfig.targetValuePath); + expect(targetValue, 'Target value should not be undefined').to.not.be.undefined; - const targetValueAfterInit = _.get(configuration, targetValueAfterInitPath); - expect(targetValueAfterInit).to.be.undefined; + const afterInitValue = _.get(configuration, pluginConfig.afterInitValuePath); + expect(afterInitValue, 'afterInitValue should be undefined').to.be.undefined; - const refValue = _.get(configuration, targetRefPath); - expect(refValue).to.deep.equal(targetValue); + const refValue = _.get(configuration, pluginConfig.refValuePath); + expect(refValue).to.deep.equal(targetValue, 'refValue should equal targetValue'); - const preexistValue = _.get(configuration, targetValuePreexistPath); - expect(preexistValue).to.deep.equal(targetValue); + const overwriteValue = _.get(configuration, pluginConfig.overwriteValuePath); + expect(overwriteValue).to.deep.equal(targetValue, 'overwriteValue should equal targetValue'); }); }); }); From b85712b77881fc128e8ad55b0d2fa6d278087a53 Mon Sep 17 00:00:00 2001 From: Marco Kleinlein <87859452+mklenbw@users.noreply.github.com> Date: Tue, 13 Dec 2022 14:53:11 +0100 Subject: [PATCH 29/36] Added check for pathKeys not being an array --- .../programmatic/plugin/extend-config-plugin/index.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/fixtures/programmatic/plugin/extend-config-plugin/index.js b/test/fixtures/programmatic/plugin/extend-config-plugin/index.js index eda7d672fe3..1dcc8a6b0ad 100644 --- a/test/fixtures/programmatic/plugin/extend-config-plugin/index.js +++ b/test/fixtures/programmatic/plugin/extend-config-plugin/index.js @@ -31,6 +31,12 @@ module.exports = class TestPlugin { } catch (error) { // ignore this } + + try { + this.serverless.extendConfiguration('custom.target.invalid', {}); + } catch (error) { + // ignore this + } } extendAfterInit() { From 532cf5e4996a259cee048f002cfaaad7061a6127 Mon Sep 17 00:00:00 2001 From: Marco Kleinlein <87859452+mklenbw@users.noreply.github.com> Date: Thu, 15 Dec 2022 10:36:30 +0100 Subject: [PATCH 30/36] Fixed searchpath --- scripts/serverless.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/serverless.js b/scripts/serverless.js index 3cd66bc44a0..3d69e55ba45 100755 --- a/scripts/serverless.js +++ b/scripts/serverless.js @@ -664,7 +664,7 @@ processSpanPromise = (async () => { fulfilledSources: new Set(['env', 'file', 'self', 'strToBool']), propertyPathsToResolve: commands[0] === 'plugin' - ? new Set(['plugin', 'provider\0name', 'provider\0stage', 'useDotenv']) + ? new Set(['plugins', 'provider\0name', 'provider\0stage', 'useDotenv']) : null, variableSourcesInConfig, }; From 1a48db8b52233a52ab1f85c764b2be4384c1e4bd Mon Sep 17 00:00:00 2001 From: Marco Kleinlein <87859452+mklenbw@users.noreply.github.com> Date: Thu, 15 Dec 2022 10:41:21 +0100 Subject: [PATCH 31/36] Fail with throw --- test/unit/lib/serverless.test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/unit/lib/serverless.test.js b/test/unit/lib/serverless.test.js index 8a6883dda37..e2eb73603cd 100644 --- a/test/unit/lib/serverless.test.js +++ b/test/unit/lib/serverless.test.js @@ -4,7 +4,7 @@ const chai = require('chai'); chai.use(require('chai-as-promised')); -const { expect, assert } = chai; +const { expect } = chai; const Serverless = require('../../../lib/serverless'); const semverRegex = require('semver-regex'); @@ -218,7 +218,7 @@ describe('test/unit/lib/serverless.test.js', () => { configuration = yaml.load(String(serverlessProcess.stdoutBuffer)); } catch (error) { const errorMessage = String(error.stdoutBuffer); - assert.fail(errorMessage); + throw Error(errorMessage); } expect(configuration).to.be.an('object', configuration); From 3781996de1b625c205b8cf1592109ac386cb9401 Mon Sep 17 00:00:00 2001 From: Marco Kleinlein <87859452+mklenbw@users.noreply.github.com> Date: Thu, 15 Dec 2022 10:43:48 +0100 Subject: [PATCH 32/36] Remove aws-region --- test/unit/lib/serverless.test.js | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/test/unit/lib/serverless.test.js b/test/unit/lib/serverless.test.js index e2eb73603cd..0819a9eee44 100644 --- a/test/unit/lib/serverless.test.js +++ b/test/unit/lib/serverless.test.js @@ -181,21 +181,11 @@ describe('test/unit/lib/serverless.test.js', () => { }); describe('Extend configuration', () => { - let awsRegion; const pluginConfig = require('../../fixtures/programmatic/plugin/extend-config-plugin').pluginConfig; const serverlessPath = path.resolve(__dirname, '../../../scripts/serverless.js'); - before(() => { - awsRegion = process.env.AWS_REGION; - process.env.AWS_REGION = 'us-east-1'; - }); - - after(() => { - process.env.AWS_REGION = awsRegion; - }); - it('Extends configuration with given values', async () => { const customExt = { custom: {} }; const configExt = { From 799f5f7cad91173af818318ed34cc58482a3a052 Mon Sep 17 00:00:00 2001 From: Marco Kleinlein <87859452+mklenbw@users.noreply.github.com> Date: Thu, 15 Dec 2022 10:49:26 +0100 Subject: [PATCH 33/36] Removed try/catch for test --- test/unit/lib/serverless.test.js | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/test/unit/lib/serverless.test.js b/test/unit/lib/serverless.test.js index 0819a9eee44..5931502a417 100644 --- a/test/unit/lib/serverless.test.js +++ b/test/unit/lib/serverless.test.js @@ -200,16 +200,10 @@ describe('test/unit/lib/serverless.test.js', () => { const { servicePath: serviceDir } = await programmaticFixturesEngine.setup('plugin', { configExt, }); - let configuration; - try { - const serverlessProcess = await spawn('node', [serverlessPath, 'print'], { - cwd: serviceDir, - }); - configuration = yaml.load(String(serverlessProcess.stdoutBuffer)); - } catch (error) { - const errorMessage = String(error.stdoutBuffer); - throw Error(errorMessage); - } + const serverlessProcess = await spawn('node', [serverlessPath, 'print'], { + cwd: serviceDir, + }); + const configuration = yaml.load(String(serverlessProcess.stdoutBuffer)); expect(configuration).to.be.an('object', configuration); From 5e0e9a095099f9d8600bbf86b5f267cc918ec7f6 Mon Sep 17 00:00:00 2001 From: Marco Kleinlein <87859452+mklenbw@users.noreply.github.com> Date: Thu, 15 Dec 2022 10:52:32 +0100 Subject: [PATCH 34/36] Removed assertion --- test/unit/lib/serverless.test.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/unit/lib/serverless.test.js b/test/unit/lib/serverless.test.js index 5931502a417..365633e0934 100644 --- a/test/unit/lib/serverless.test.js +++ b/test/unit/lib/serverless.test.js @@ -205,8 +205,6 @@ describe('test/unit/lib/serverless.test.js', () => { }); const configuration = yaml.load(String(serverlessProcess.stdoutBuffer)); - expect(configuration).to.be.an('object', configuration); - const targetValue = _.get(configuration, pluginConfig.targetValuePath); expect(targetValue, 'Target value should not be undefined').to.not.be.undefined; From ea29c16d30812432cc0c9aaaaef5d6078a223ecb Mon Sep 17 00:00:00 2001 From: Marco Kleinlein <87859452+mklenbw@users.noreply.github.com> Date: Thu, 15 Dec 2022 13:35:52 +0100 Subject: [PATCH 35/36] Added documentation --- docs/guides/plugins/custom-configuration.md | 6 +- .../guides/plugins/extending-configuration.md | 70 +++++++++++++++++++ docs/menu.json | 3 +- 3 files changed, 75 insertions(+), 4 deletions(-) create mode 100644 docs/guides/plugins/extending-configuration.md diff --git a/docs/guides/plugins/custom-configuration.md b/docs/guides/plugins/custom-configuration.md index 3f1cf75133a..886c148f49a 100644 --- a/docs/guides/plugins/custom-configuration.md +++ b/docs/guides/plugins/custom-configuration.md @@ -1,8 +1,8 @@ @@ -12,7 +12,7 @@ layout: Doc -# Extending the configuration +# Extending the configuration schema Plugin can extend the `serverless.yml` syntax with custom configuration: diff --git a/docs/guides/plugins/extending-configuration.md b/docs/guides/plugins/extending-configuration.md new file mode 100644 index 00000000000..85ba0550b0b --- /dev/null +++ b/docs/guides/plugins/extending-configuration.md @@ -0,0 +1,70 @@ + + + + +### [Read this on the main serverless docs site](https://www.serverless.com/framework/docs/guides/plugins/extending-configuration) + + + +# Extending and overriding configuration + +Plugins can extend and override the internal configuration. + +To do so, plugins may use the `serverless.extendConfiguration(...)` method. +This is only allowed at pre-init stage of serverless. +The method also takes care of resolving all variables in the given value. But it **does not validate you input** nor the target. Improper usage can cause serverless to fail. + +The `serverless.extendConfiguration(configurationPathKeys, value)` method takes two arguments. + +| Argument | Type | Description | +| ----------------------- | ------------------------- | ------------------------------------------------------------------ | +| `configurationPathKeys` | string[] | Path of the configuration property to set; must not be empty | +| `value` | string \| object \| array | New value of the configuration property in `configurationPathKeys` | + +If configuration in `configurationPathKeys` **does exist** the value will be overwritten. +If configuration in `configurationPathKeys` **does not exist** the whole path will be created. + +You can use it in the `asyncInit()` method of your plugin. + +```js +class MyPlugin { + constructor(serverless) { + this.serverless = serverless; + } + + async asyncInit() { + const value = { + myKey: 'myValue', + }; + this.serverless.extendConfiguration(['custom', 'myPlugin'], value); + } +} + +module.exports = MyPlugin; +``` + +If your plugin needs merging you need to take care of it yourself. + +```js +class MyPlugin { + constructor(serverless) { + this.serverless = serverless; + } + + async asyncInit() { + const currentConfig = this.serverless.configurationInput.custom.myPlugin; + const value = Object.assign(currentConfig, { + myKey: 'myValue', + }); + this.serverless.extendConfiguration(['custom', 'myPlugin'], value); + } +} + +module.exports = MyPlugin; +``` diff --git a/docs/menu.json b/docs/menu.json index 8d03d422989..652a39ef37c 100644 --- a/docs/menu.json +++ b/docs/menu.json @@ -50,7 +50,8 @@ "CLI Output": "guides/plugins/cli-output", "Custom Commands": "guides/plugins/custom-commands", "Custom Variables": "guides/plugins/custom-variables", - "Extending the Configuration": "guides/plugins/custom-configuration" + "Extending the Configuration schema": "guides/plugins/custom-configuration", + "Extending and overriding configuration": "guides/plugins/extending-configuration" } }, "Examples and Tutorials": "examples-and-tutorials", From a3d4fe94484cf1fa4fa13531975213aa0441c7b2 Mon Sep 17 00:00:00 2001 From: Marco Kleinlein <87859452+mklenbw@users.noreply.github.com> Date: Tue, 20 Dec 2022 14:19:07 +0100 Subject: [PATCH 36/36] Clearified usage --- docs/guides/plugins/extending-configuration.md | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/docs/guides/plugins/extending-configuration.md b/docs/guides/plugins/extending-configuration.md index 85ba0550b0b..73f185a820f 100644 --- a/docs/guides/plugins/extending-configuration.md +++ b/docs/guides/plugins/extending-configuration.md @@ -30,15 +30,13 @@ The `serverless.extendConfiguration(configurationPathKeys, value)` method takes If configuration in `configurationPathKeys` **does exist** the value will be overwritten. If configuration in `configurationPathKeys` **does not exist** the whole path will be created. -You can use it in the `asyncInit()` method of your plugin. +You can use it in plugin constructor, or if for some reason configuration extension is resolved asynchronously you may resort to `asyncInit()` method ```js class MyPlugin { constructor(serverless) { this.serverless = serverless; - } - async asyncInit() { const value = { myKey: 'myValue', }; @@ -55,9 +53,7 @@ If your plugin needs merging you need to take care of it yourself. class MyPlugin { constructor(serverless) { this.serverless = serverless; - } - async asyncInit() { const currentConfig = this.serverless.configurationInput.custom.myPlugin; const value = Object.assign(currentConfig, { myKey: 'myValue',