Skip to content

Aws http event schema #8301

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

Merged
medikoo merged 21 commits into from
Oct 8, 2020
Merged

Aws http event schema #8301

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
684c3ea
first go at http event schema
Sep 29, 2020
b843b02
remove inline validations
Sep 29, 2020
b7236f2
remove validation tests
Sep 29, 2020
af31807
remove validation tests
Sep 29, 2020
0ea0dc8
final schema touches
Oct 1, 2020
276f688
no need for arrayOrSingle due to type coercion
Oct 1, 2020
b8e46ad
refactor to remove if/else
Oct 1, 2020
50fb4c8
use case-insensitive for path and method
Oct 2, 2020
e76e4d5
use case-insensitive for authorizer.type and integration
Oct 3, 2020
f6c4fd9
schema
Oct 6, 2020
f2c1a87
remove inline validations which are covered by schema
Oct 6, 2020
a04fb5d
remove schema tests
Oct 6, 2020
b9820ad
lint fixes
Oct 6, 2020
d8d59cf
tests fixes
Oct 6, 2020
7011b4c
Merge remote-tracking branch 'upstream/master' into aws-http-event-sc…
Oct 6, 2020
71104ac
allow path without leading slash
Oct 6, 2020
7bd1098
support case insensitive connectionType
Oct 7, 2020
34c1fe7
add statusCodes.headers
Oct 7, 2020
0cfae06
fix response.schema
Oct 7, 2020
c32a320
fix origin/origins required
Oct 7, 2020
912ab8c
small schema fixes
Oct 8, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
179 changes: 177 additions & 2 deletions lib/plugins/aws/package/compile/events/apiGateway/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,190 @@ const getMethodAuthorization = require('./lib/method/authorization');
const getMethodIntegration = require('./lib/method/integration');
const getMethodResponses = require('./lib/method/responses');

function caseInsensitive(str) {
return { type: 'string', regexp: new RegExp(`^${str}$`, 'i').toString() };
}

const contentHandlingSchema = { enum: ['CONVERT_TO_BINARY', 'CONVERT_TO_TEXT'] };

const allowedMethods = ['GET', 'POST', 'PUT', 'PATCH', 'OPTIONS', 'HEAD', 'DELETE', 'ANY'];
const methodPattern = new RegExp(`^(?:\\*|${allowedMethods.join('|')})$`, 'i');
const methodPathPattern = new RegExp(`^(?:\\*|(${allowedMethods.join('|')}) (\\/\\S*))$`, 'i');

const requestParametersSchema = {
type: 'object',
additionalProperties: {
oneOf: [
{ type: 'boolean' },
{
type: 'object',
properties: {
required: { type: 'boolean' },
mappedValue: { type: 'string' },
},
additionalProperties: false,
},
],
},
};

const authorizerSchema = {
oneOf: [
{ type: 'string' },
caseInsensitive('aws_iam'),
{ $ref: '#/definitions/awsArnString' },
{
type: 'object',
properties: {
arn: { $ref: '#/definitions/awsArn' },
authorizerId: { $ref: '#/definitions/awsCfInstruction' },
claims: { type: 'array', items: { type: 'string' } },
identitySource: { type: 'string' },
identityValidationExpression: { type: 'string' },
managedExternally: { type: 'boolean' },
name: { type: 'string' },
resultTtlInSeconds: { type: 'integer', minimum: 0, maximum: 3600 },
scopes: { type: 'array', items: { type: 'string' } },
type: {
oneOf: ['token', 'cognito_user_pools', 'request', 'aws_iam'].map(caseInsensitive),
},
},

@medikoo medikoo Oct 5, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Let's keep alphabetical order

required: [],
additionalProperties: false,
},
],
};

const corsSchema = {
oneOf: [
{ type: 'boolean' },
{
type: 'object',
properties: {
allowCredentials: { type: 'boolean' },
cacheControl: { type: 'string' },
headers: { type: 'array', items: { type: 'string' } },
maxAge: { type: 'integer', minimum: 1 },
methods: { type: 'array', items: { enum: allowedMethods } },
Comment thread
medikoo marked this conversation as resolved.
origin: { type: 'string' },
origins: {
type: 'array',
items: { type: 'string' },
},
},
oneOf: [{ required: ['origin'] }, { required: ['origins'] }],

@medikoo medikoo Oct 7, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we need to also introduce required: [], otherwise we make passing origin required (either via origin or origins option)

@thewizarodofoz thewizarodofoz Oct 8, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It seems to me from the code that indeed one of them is required. We also had this manual validation that I removed which supports my theory.

@medikoo medikoo Oct 8, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Indeed, it appears that if someone uses an object form, then not providing origin will fail.

Still that looks as apparent design bug, as by supporting cors: true we indicate that all properties have defaults. Intuitively object form should fall into origin: '*' if one was not set (it's e.g. the case in HTTP API, which has similar cors setting)

Anyway this is side issue (not a concern of this PR) so yes let's leave it as required.

additionalProperties: false,
},
],
};

const requestSchema = {
type: 'object',
properties: {
contentHandling: contentHandlingSchema,
method: { type: 'string', regexp: methodPattern.toString() },
parameters: {
type: 'object',
properties: {
querystrings: requestParametersSchema,
headers: requestParametersSchema,
paths: requestParametersSchema,
},
additionalProperties: false,
},
passThrough: { enum: ['NEVER', 'WHEN_NO_MATCH', 'WHEN_NO_TEMPLATES'] },
schema: {
type: 'object',
additionalProperties: { type: 'object' },
},
template: {
type: 'object',
additionalProperties: { type: 'string' },
},
uri: { type: 'string' },
},
additionalProperties: false,
};

const responseSchema = {
type: 'object',
properties: {
contentHandling: contentHandlingSchema,
headers: {
type: 'object',
additionalProperties: { type: 'string' },
},
template: { type: 'string' },
statusCodes: {
type: 'object',
propertyNames: {
type: 'string',
pattern: '^\\d{3}$',
},
additionalProperties: {
Comment thread
medikoo marked this conversation as resolved.
type: 'object',
properties: {
headers: {
type: 'object',
additionalProperties: { type: 'string' },
},
pattern: { type: 'string' },
template: {
oneOf: [
{ type: 'string' },
{
type: 'object',
additionalProperties: { type: 'string' },
},
],
},
Comment thread
medikoo marked this conversation as resolved.
},
additionalProperties: false,
},
},
},
additionalProperties: false,
};

class AwsCompileApigEvents {
constructor(serverless, options) {
this.serverless = serverless;
this.options = options;
this.provider = this.serverless.getProvider('aws');

this.serverless.configSchemaHandler.defineFunctionEvent('aws', 'http', {
// TODO: Complete schema, see https://github.com/serverless/serverless/issues/8018
anyOf: [{ type: 'string' }, { type: 'object' }],
anyOf: [
{ type: 'string', regexp: methodPathPattern.toString() },
{
type: 'object',
properties: {
async: { type: 'boolean' },
authorizer: authorizerSchema,
connectionId: { type: 'string' },
connectionType: { oneOf: ['vpc-link', 'VPC_LINK'].map(caseInsensitive) },
cors: corsSchema,
integration: {
oneOf: [
'LAMBDA_PROXY',
'LAMBDA',
'AWS',
'AWS_PROXY',
'HTTP',
'HTTP_PROXY',
'MOCK',
].map(caseInsensitive),
},
method: { type: 'string', regexp: methodPattern.toString() },
operationId: { type: 'string' },
path: { type: 'string', regexp: /^(?:\*|\/?\S*)$/.toString() },
private: { type: 'boolean' },
request: requestSchema,
response: responseSchema,
},
required: ['path', 'method'],
additionalProperties: false,
},
],
});

// used for the generated method logical ids (GET, PATCH, PUT, DELETE, OPTIONS, ...)
Expand Down
12 changes: 1 addition & 11 deletions lib/plugins/aws/package/compile/events/apiGateway/lib/cors.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,6 @@ module.exports = {
origin = origins[0];
}

if (!origin) {
const errorMessage = 'must specify either origin or origins';
throw new this.serverless.classes.Error(errorMessage);
}

const preflightHeaders = {
'Access-Control-Allow-Origin': `'${origin}'`,
'Access-Control-Allow-Headers': `'${config.headers.join(',')}'`,
Expand All @@ -43,12 +38,7 @@ module.exports = {

// Enable CORS Max Age usage if set
if (config.maxAge) {
if (config.maxAge > 0) {
preflightHeaders['Access-Control-Max-Age'] = `'${config.maxAge}'`;
} else {
const errorMessage = 'maxAge should be an integer over 0';
throw new this.serverless.classes.Error(errorMessage);
}
preflightHeaders['Access-Control-Max-Age'] = `'${config.maxAge}'`;
}

// Allow Cache-Control header if set
Expand Down
49 changes: 0 additions & 49 deletions lib/plugins/aws/package/compile/events/apiGateway/lib/cors.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,55 +237,6 @@ describe('#compileCors()', () => {
});
});

it('should throw error if no origin or origins is provided', () => {
awsCompileApigEvents.validated.corsPreflight = {
'users/update': {
headers: ['*'],
methods: ['OPTIONS', 'PUT'],
allowCredentials: false,
},
};

expect(() => awsCompileApigEvents.compileCors()).to.throw(
Error,
'must specify either origin or origins'
);
});

it('should throw error if maxAge is not an integer greater than 0', () => {
awsCompileApigEvents.validated.corsPreflight = {
'users/update': {
origin: 'http://example.com',
headers: ['*'],
methods: ['OPTIONS', 'PUT'],
allowCredentials: false,
maxAge: -1,
},
};

expect(() => awsCompileApigEvents.compileCors()).to.throw(
Error,
'maxAge should be an integer over 0'
);
});

it('should throw error if maxAge is not an integer', () => {
awsCompileApigEvents.validated.corsPreflight = {
'users/update': {
origin: 'http://example.com',
headers: ['*'],
methods: ['OPTIONS', 'PUT'],
allowCredentials: false,
maxAge: 'five',
},
};

expect(() => awsCompileApigEvents.compileCors()).to.throw(
Error,
'maxAge should be an integer over 0'
);
});

it('should add the methods resource logical id to the array of method logical ids', () => {
awsCompileApigEvents.validated.corsPreflight = {
'users/create': {
Expand Down
Loading