Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Added support for serverless.json. Improved error messages with filename #3647

Merged
merged 5 commits into from
May 25, 2017
Merged

Added support for serverless.json. Improved error messages with filename #3647

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a227e5b
Added support for serverless.json. Improved error messages with filen…
May 18, 2017
45a2b20
Fixed unit tests - removed filename from literal error check
May 18, 2017
9739977
Added file load tests for YAML, YML and JSON. Added precedence test.
May 19, 2017
e6f2c2d
Update other relevant parts of the codebase
pmuens May 19, 2017
081c612
Add documentation
pmuens May 25, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
40 changes: 27 additions & 13 deletions lib/classes/Service.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,23 +33,37 @@ class Service {
const options = rawOptions || {};
options.stage = options.stage || options.s;
options.region = options.region || options.r;
const servicePath = that.serverless.config.servicePath;
const servicePath = this.serverless.config.servicePath;

// skip if the service path is not found
// because the user might be creating a new service
if (!servicePath) {
return BbPromise.resolve();
}

let serverlessYmlPath = path.join(servicePath, 'serverless.yml');
// change to serverless.yaml if the file could not be found
if (!this.serverless.utils.fileExistsSync(serverlessYmlPath)) {
serverlessYmlPath = path
.join(this.serverless.config.servicePath, 'serverless.yaml');
}
// List of supported service filename variants.
// The order defines the precedence.
const serviceFilenames = [
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unit tests which check for all the possible file extensions should be added

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you're right. I will add them - also a precedence check would be beneficial.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks 👍 +1 for the precedence check!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests added.

'serverless.yaml',
'serverless.yml',
'serverless.json',
];

const serviceFilePaths = _.map(serviceFilenames, filename => path.join(servicePath, filename));
const serviceFileIndex = _.findIndex(serviceFilePaths,
filename => this.serverless.utils.fileExistsSync(filename)
);

// Set the filename if found, otherwise set the preferred variant.
const serviceFilePath = serviceFileIndex !== -1 ?
serviceFilePaths[serviceFileIndex] :
_.first(serviceFilePaths);
const serviceFilename = serviceFileIndex !== -1 ?
serviceFilenames[serviceFileIndex] :
_.first(serviceFilenames);

return that.serverless.yamlParser
.parse(serverlessYmlPath)
.parse(serviceFilePath)
.then((serverlessFileParam) => {
const serverlessFile = serverlessFileParam;
// basic service level validation
Expand All @@ -58,18 +72,18 @@ class Service {
if (ymlVersion && !semver.satisfies(version, ymlVersion)) {
const errorMessage = [
`The Serverless version (${version}) does not satisfy the`,
` "frameworkVersion" (${ymlVersion}) in serverless.yml`,
` "frameworkVersion" (${ymlVersion}) in ${serviceFilename}`,
].join('');
throw new ServerlessError(errorMessage);
}
if (!serverlessFile.service) {
throw new ServerlessError('"service" property is missing in serverless.yml');
throw new ServerlessError(`"service" property is missing in ${serviceFilename}`);
}
if (_.isObject(serverlessFile.service) && !serverlessFile.service.name) {
throw new ServerlessError('"service" is missing the "name" property in serverless.yml');
throw new ServerlessError(`"service" is missing the "name" property in ${serviceFilename}`); // eslint-disable-line max-len
}
if (!serverlessFile.provider) {
throw new ServerlessError('"provider" property is missing in serverless.yml');
throw new ServerlessError(`"provider" property is missing in ${serviceFilename}`);
}

if (typeof serverlessFile.provider !== 'object') {
Expand All @@ -84,7 +98,7 @@ class Service {
const errorMessage = [
`Provider "${serverlessFile.provider.name}" is not supported.`,
` Valid values for provider are: ${providers.join(', ')}.`,
' Please provide one of those values to the "provider" property in serverless.yml.',
` Please provide one of those values to the "provider" property in ${serviceFilename}`,
].join('');
throw new ServerlessError(errorMessage);
}
Expand Down
157 changes: 155 additions & 2 deletions lib/classes/Service.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ describe('Service', () => {
return expect(noService.load()).to.eventually.resolve;
});

it('should load from filesystem', () => {
it('should load serverless.yml from filesystem', () => {
const SUtils = new Utils();
const serverlessYml = {
service: 'new-service',
Expand Down Expand Up @@ -175,6 +175,159 @@ describe('Service', () => {
});
});

it('should load serverless.yaml from filesystem', () => {
const SUtils = new Utils();
const serverlessYml = {
service: 'new-service',
provider: {
name: 'aws',
stage: 'dev',
region: 'us-east-1',
variableSyntax: '\\${{([\\s\\S]+?)}}',
},
plugins: ['testPlugin'],
functions: {
functionA: {},
},
resources: {
aws: {
resourcesProp: 'value',
},
azure: {},
google: {},
},
package: {
exclude: ['exclude-me'],
include: ['include-me'],
artifact: 'some/path/foo.zip',
},
};

SUtils.writeFileSync(path.join(tmpDirPath, 'serverless.yaml'),
YAML.dump(serverlessYml));

const serverless = new Serverless();
serverless.init();
serverless.config.update({ servicePath: tmpDirPath });
serviceInstance = new Service(serverless);

return expect(serviceInstance.load()).to.eventually.be.fulfilled
.then(() => {
expect(serviceInstance.service).to.be.equal('new-service');
expect(serviceInstance.provider.name).to.deep.equal('aws');
expect(serviceInstance.provider.variableSyntax).to.equal('\\${{([\\s\\S]+?)}}');
expect(serviceInstance.plugins).to.deep.equal(['testPlugin']);
expect(serviceInstance.resources.aws).to.deep.equal({ resourcesProp: 'value' });
expect(serviceInstance.resources.azure).to.deep.equal({});
expect(serviceInstance.resources.google).to.deep.equal({});
expect(serviceInstance.package.exclude.length).to.equal(1);
expect(serviceInstance.package.exclude[0]).to.equal('exclude-me');
expect(serviceInstance.package.include.length).to.equal(1);
expect(serviceInstance.package.include[0]).to.equal('include-me');
expect(serviceInstance.package.artifact).to.equal('some/path/foo.zip');
});
});

it('should load serverless.json from filesystem', () => {
const SUtils = new Utils();
const serverlessJSON = {
service: 'new-service',
provider: {
name: 'aws',
stage: 'dev',
region: 'us-east-1',
variableSyntax: '\\${{([\\s\\S]+?)}}',
},
plugins: ['testPlugin'],
functions: {
functionA: {},
},
resources: {
aws: {
resourcesProp: 'value',
},
azure: {},
google: {},
},
package: {
exclude: ['exclude-me'],
include: ['include-me'],
artifact: 'some/path/foo.zip',
},
};

SUtils.writeFileSync(path.join(tmpDirPath, 'serverless.json'),
JSON.stringify(serverlessJSON));

const serverless = new Serverless();
serverless.init();
serverless.config.update({ servicePath: tmpDirPath });
serviceInstance = new Service(serverless);

return expect(serviceInstance.load()).to.eventually.be.fulfilled
.then(() => {
expect(serviceInstance.service).to.be.equal('new-service');
expect(serviceInstance.provider.name).to.deep.equal('aws');
expect(serviceInstance.provider.variableSyntax).to.equal('\\${{([\\s\\S]+?)}}');
expect(serviceInstance.plugins).to.deep.equal(['testPlugin']);
expect(serviceInstance.resources.aws).to.deep.equal({ resourcesProp: 'value' });
expect(serviceInstance.resources.azure).to.deep.equal({});
expect(serviceInstance.resources.google).to.deep.equal({});
expect(serviceInstance.package.exclude.length).to.equal(1);
expect(serviceInstance.package.exclude[0]).to.equal('exclude-me');
expect(serviceInstance.package.include.length).to.equal(1);
expect(serviceInstance.package.include[0]).to.equal('include-me');
expect(serviceInstance.package.artifact).to.equal('some/path/foo.zip');
});
});

it('should load YAML in favor of JSON', () => {
const SUtils = new Utils();
const serverlessJSON = {
provider: {
name: 'aws',
stage: 'dev',
region: 'us-east-1',
variableSyntax: '\\${{([\\s\\S]+?)}}',
},
plugins: ['testPlugin'],
functions: {
functionA: {},
},
resources: {
aws: {
resourcesProp: 'value',
},
azure: {},
google: {},
},
package: {
exclude: ['exclude-me'],
include: ['include-me'],
artifact: 'some/path/foo.zip',
},
};

serverlessJSON.service = 'JSON service';
SUtils.writeFileSync(path.join(tmpDirPath, 'serverless.json'),
JSON.stringify(serverlessJSON));

serverlessJSON.service = 'YAML service';
SUtils.writeFileSync(path.join(tmpDirPath, 'serverless.yaml'),
YAML.dump(serverlessJSON));

const serverless = new Serverless();
serverless.init();
serverless.config.update({ servicePath: tmpDirPath });
serviceInstance = new Service(serverless);

return expect(serviceInstance.load()).to.eventually.be.fulfilled
.then(() => {
// YAML should have been loaded instead of JSON
expect(serviceInstance.service).to.be.equal('YAML service');
});
});

it('should reject when the service name is missing', () => {
const SUtils = new Utils();
const serverlessYaml = {
Expand All @@ -189,7 +342,7 @@ describe('Service', () => {
serviceInstance = new Service(serverless);

return expect(serviceInstance.load()).to.eventually.be
.rejectedWith('"service" is missing the "name" property in serverless.yml');
.rejectedWith('"service" is missing the "name" property in');
});

it('should support service objects', () => {
Expand Down
2 changes: 2 additions & 0 deletions lib/classes/Utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,8 @@ class Utils {
servicePath = process.cwd();
} else if (this.serverless.utils.fileExistsSync(path.join(process.cwd(), 'serverless.yaml'))) {
servicePath = process.cwd();
} else if (this.serverless.utils.fileExistsSync(path.join(process.cwd(), 'serverless.json'))) {
servicePath = process.cwd();
}

return servicePath;
Expand Down
12 changes: 12 additions & 0 deletions lib/classes/Utils.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,18 @@ describe('Utils', () => {
expect(servicePath).to.not.equal(null);
});

it('should detect if the CWD is a service directory when using Serverless .json files', () => {
const tmpDirPath = testUtils.getTmpDirPath();
const tmpFilePath = path.join(tmpDirPath, 'serverless.json');

serverless.utils.writeFileSync(tmpFilePath, 'foo');
process.chdir(tmpDirPath);

const servicePath = serverless.utils.findServicePath();

expect(servicePath).to.not.equal(null);
});

it('should detect if the CWD is not a service directory', () => {
// just use the root of the tmpdir because findServicePath will
// also check parent directories (and may find matching tmp dirs
Expand Down
3 changes: 2 additions & 1 deletion lib/plugins/package/lib/packageService.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,9 @@ module.exports = {
'.gitignore',
'.DS_Store',
'npm-debug.log',
'serverless.yaml',
'serverless.yml',
'serverless.yaml',
'serverless.json',
'.serverless/**',
],

Expand Down
8 changes: 4 additions & 4 deletions lib/plugins/package/lib/packageService.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,8 @@ describe('#packageService()', () => {
const exclude = packagePlugin.getExcludes();
expect(exclude).to.deep.equal([
'.git/**', '.gitignore', '.DS_Store',
'npm-debug.log',
'serverless.yaml', 'serverless.yml',
'npm-debug.log', 'serverless.yml',
'serverless.yaml', 'serverless.json',
'.serverless/**', 'dir', 'file.js',
]);
});
Expand All @@ -99,8 +99,8 @@ describe('#packageService()', () => {
const exclude = packagePlugin.getExcludes(funcExcludes);
expect(exclude).to.deep.equal([
'.git/**', '.gitignore', '.DS_Store',
'npm-debug.log',
'serverless.yaml', 'serverless.yml',
'npm-debug.log', 'serverless.yml',
'serverless.yaml', 'serverless.json',
'.serverless/**', 'dir', 'file.js',
'lib', 'other.js',
]);
Expand Down