Skip to content

Commit 06ce8ee

Browse files
erezrokaherezrokah
committed
refactor(s3-validation): merge s3 validation schema with main schema
1 parent e100957 commit 06ce8ee

File tree

7 files changed

+217
-232
lines changed

7 files changed

+217
-232
lines changed

‎lib/apiGateway/schema.js‎

Lines changed: 62 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,18 @@
11
'use strict'
2+
const _ = require('lodash')
3+
4+
const customErrorBuilder = (type, message) => (errors) => {
5+
for (const error of errors) {
6+
switch (error.type) {
7+
case type:
8+
error.message = _.isFunction(message) ? message(error) : message
9+
break
10+
default:
11+
break
12+
}
13+
}
14+
return errors
15+
}
216

317
const Joi = require('@hapi/joi')
418

@@ -21,18 +35,7 @@ const cors = Joi.alternatives().try(
2135
allowCredentials: Joi.boolean()
2236
})
2337
.oxor('origin', 'origins') // can have one of them, but not required
24-
.error((errors) => {
25-
for (const error of errors) {
26-
switch (error.type) {
27-
case 'object.oxor':
28-
error.message = '"cors" can have "origin" or "origins" but not both'
29-
break
30-
default:
31-
break
32-
}
33-
}
34-
return errors
35-
})
38+
.error(customErrorBuilder('object.oxor', '"cors" can have "origin" or "origins" but not both'))
3639
)
3740

3841
const authorizerId = Joi.alternatives().try(
@@ -71,18 +74,9 @@ const proxy = Joi.object({
7174
authorizationScopes
7275
})
7376
.oxor('authorizerId', 'authorizationScopes') // can have one of them, but not required
74-
.error((errors) => {
75-
for (const error of errors) {
76-
switch (error.type) {
77-
case 'object.oxor':
78-
error.message = 'cannot set both "authorizerId" and "authorizationScopes"'
79-
break
80-
default:
81-
break
82-
}
83-
}
84-
return errors
85-
})
77+
.error(
78+
customErrorBuilder('object.oxor', 'cannot set both "authorizerId" and "authorizationScopes"')
79+
)
8680
.required()
8781

8882
const stringOrRef = Joi.alternatives().try([
@@ -92,45 +86,62 @@ const stringOrRef = Joi.alternatives().try([
9286
})
9387
])
9488

89+
const key = Joi.alternatives().try([
90+
Joi.string(),
91+
Joi.object()
92+
.keys({
93+
pathParam: Joi.string(),
94+
queryStringParam: Joi.string()
95+
})
96+
.xor('pathParam', 'queryStringParam')
97+
.error(
98+
customErrorBuilder(
99+
'object.xor',
100+
'key must contain "pathParam" or "queryStringParam" but not both'
101+
)
102+
)
103+
])
104+
95105
const allowedProxies = ['kinesis', 'sqs', 's3', 'sns']
96106

97107
const proxiesSchemas = {
98108
kinesis: Joi.object({
99109
kinesis: proxy.append({ streamName: stringOrRef.required() })
100110
}),
101-
s3: Joi.object({ s3: proxy }),
111+
s3: Joi.object({
112+
s3: proxy.append({
113+
action: Joi.string()
114+
.valid('GetObject', 'PutObject', 'DeleteObject')
115+
.required(),
116+
bucket: stringOrRef.required(),
117+
key: key.required()
118+
})
119+
}),
102120
sns: Joi.object({ sns: proxy }),
103121
sqs: Joi.object({ sqs: proxy.append({ requestParameters }) })
104122
}
105123

106124
const schema = Joi.array()
107125
.items(...allowedProxies.map((proxyKey) => proxiesSchemas[proxyKey]))
108-
.error((errors) => {
109-
for (const error of errors) {
110-
switch (error.type) {
111-
case 'array.includes':
112-
// get a detailed error why the proxy object failed the schema validation
113-
// Joi default message is `"value" at position <i> does not match any of the allowed types`
114-
const proxyKey = Object.keys(error.context.value)[0]
115-
if (proxiesSchemas[proxyKey]) {
116-
// e.g. value is { kinesis: { path: '/kinesis', method: 'xxxx' } }
117-
const { error: proxyError } = Joi.validate(
118-
error.context.value,
119-
proxiesSchemas[proxyKey]
120-
)
121-
error.message = proxyError.message
122-
} else {
123-
// e.g. value is { xxxxx: { path: '/kinesis', method: 'post' } }
124-
error.message = `Invalid APIG proxy "${proxyKey}". This plugin supported Proxies are: ${allowedProxies.join(
125-
', '
126-
)}.`
127-
}
128-
break
129-
default:
130-
break
126+
.error(
127+
customErrorBuilder('array.includes', (error) => {
128+
// get a detailed error why the proxy object failed the schema validation
129+
// Joi default message is `"value" at position <i> does not match any of the allowed types`
130+
const proxyKey = Object.keys(error.context.value)[0]
131+
132+
let message = ''
133+
if (proxiesSchemas[proxyKey]) {
134+
// e.g. value is { kinesis: { path: '/kinesis', method: 'xxxx' } }
135+
const { error: proxyError } = Joi.validate(error.context.value, proxiesSchemas[proxyKey])
136+
message = proxyError.message
137+
} else {
138+
// e.g. value is { xxxxx: { path: '/kinesis', method: 'post' } }
139+
message = `Invalid APIG proxy "${proxyKey}". This plugin supported Proxies are: ${allowedProxies.join(
140+
', '
141+
)}.`
131142
}
132-
}
133-
return errors
134-
})
143+
return message
144+
})
145+
)
135146

136147
module.exports = schema

‎lib/apiGateway/validate.test.js‎

Lines changed: 155 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -580,7 +580,7 @@ describe('#validateServiceProxies()', () => {
580580

581581
const proxiesToTest = [
582582
{ proxy: 'kinesis', props: { streamName: 'streamName' } },
583-
{ proxy: 's3', props: {} },
583+
{ proxy: 's3', props: { bucket: 'bucket', action: 'PutObject', key: 'myKey' } },
584584
{ proxy: 'sns', props: {} }
585585
]
586586
proxiesToTest.forEach(({ proxy, props }) => {
@@ -697,6 +697,160 @@ describe('#validateServiceProxies()', () => {
697697
})
698698
})
699699

700+
describe('s3', () => {
701+
const getProxy = (key, value, ...missing) => {
702+
const proxy = {
703+
action: 'GetObject',
704+
bucket: 'myBucket',
705+
key: 'myKey',
706+
path: 's3',
707+
method: 'post'
708+
}
709+
710+
if (key) {
711+
proxy[key] = value
712+
}
713+
714+
missing.forEach((k) => delete proxy[k])
715+
return proxy
716+
}
717+
718+
const shouldError = (message, key, value, ...missing) => {
719+
serverlessApigatewayServiceProxy.serverless.service.custom = {
720+
apiGatewayServiceProxies: [
721+
{
722+
s3: getProxy(key, value, ...missing)
723+
}
724+
]
725+
}
726+
727+
expect(() => serverlessApigatewayServiceProxy.validateServiceProxies()).to.throw(
728+
serverless.classes.Error,
729+
message
730+
)
731+
}
732+
733+
const shouldSucceed = (key, value) => {
734+
serverlessApigatewayServiceProxy.serverless.service.custom = {
735+
apiGatewayServiceProxies: [
736+
{
737+
s3: getProxy(key, value)
738+
}
739+
]
740+
}
741+
742+
expect(() => serverlessApigatewayServiceProxy.validateServiceProxies()).to.not.throw(
743+
serverless.classes.Error
744+
)
745+
}
746+
747+
it('should error if the "bucket" property is missing', () => {
748+
shouldError(
749+
'child "s3" fails because [child "bucket" fails because ["bucket" is required]]',
750+
null,
751+
null,
752+
'bucket'
753+
)
754+
})
755+
756+
it('should succeed if the "bucket" property is string or AWS Ref function', () => {
757+
shouldSucceed('bucket', 'x')
758+
shouldSucceed('bucket', { Ref: 'x' })
759+
})
760+
761+
it('should error if the "bucket" property if AWS Ref function is invalid', () => {
762+
shouldError(
763+
'child "s3" fails because [child "bucket" fails because ["bucket" must be a string, child "Ref" fails because ["Ref" is required]]]',
764+
'bucket',
765+
{ xxx: 's3Bucket' }
766+
)
767+
shouldError(
768+
'child "s3" fails because [child "bucket" fails because ["bucket" must be a string, child "Ref" fails because ["Ref" must be a string]]]',
769+
'bucket',
770+
{ Ref: ['s3Bucket', 'Arn'] }
771+
)
772+
shouldError(
773+
'child "s3" fails because [child "bucket" fails because ["bucket" must be a string, "bucket" must be an object]]',
774+
'bucket',
775+
['xx', 'yy']
776+
)
777+
shouldError(
778+
'child "s3" fails because [child "bucket" fails because ["bucket" must be a string, child "Ref" fails because ["Ref" is required]]]',
779+
'bucket',
780+
{ 'Fn::GetAtt': ['x', 'Arn'] }
781+
)
782+
})
783+
784+
it('should error if the "action" property is missing', () => {
785+
shouldError(
786+
'child "s3" fails because [child "action" fails because ["action" is required]]',
787+
null,
788+
null,
789+
'action'
790+
)
791+
})
792+
793+
it('should error if the "action" property is not one of the allowed values', () => {
794+
shouldError(
795+
'child "s3" fails because [child "action" fails because ["action" must be a string]]',
796+
'action',
797+
['x']
798+
) // arrays
799+
shouldError(
800+
'child "s3" fails because [child "action" fails because ["action" must be a string]]',
801+
'action',
802+
{ Ref: 'x' }
803+
) // object
804+
shouldError(
805+
'child "s3" fails because [child "action" fails because ["action" must be one of [GetObject, PutObject, DeleteObject]]]',
806+
'action',
807+
'ListObjects'
808+
) // invalid actions
809+
})
810+
811+
it('should succeed if the "action" property is one of the allowed values', () => {
812+
shouldSucceed('action', 'GetObject')
813+
shouldSucceed('action', 'PutObject')
814+
shouldSucceed('action', 'DeleteObject')
815+
})
816+
817+
it('should error the "key" property is missing', () => {
818+
shouldError(
819+
'child "s3" fails because [child "key" fails because ["key" is required]]',
820+
null,
821+
null,
822+
'key'
823+
)
824+
})
825+
826+
it('should succeed if the "key" property is string or valid object', () => {
827+
shouldSucceed('key', 'myKey')
828+
shouldSucceed('key', { pathParam: 'myKey' })
829+
shouldSucceed('key', { queryStringParam: 'myKey' })
830+
})
831+
832+
it('should error if the "key" property specifies both pathParam and queryStringParam', () => {
833+
shouldError(
834+
'child "s3" fails because [child "key" fails because ["key" must be a string, key must contain "pathParam" or "queryStringParam" but not both]]',
835+
'key',
836+
{ pathParam: 'myKey', queryStringParam: 'myKey' }
837+
)
838+
})
839+
840+
it('should error if the "key" property is not a string or valid object', () => {
841+
shouldError(
842+
'child "s3" fails because [child "key" fails because ["key" must be a string, "key" must be an object]]',
843+
'key',
844+
['x']
845+
)
846+
shouldError(
847+
'child "s3" fails because [child "key" fails because ["key" must be a string, "param" is not allowed, "key" must contain at least one of [pathParam, queryStringParam]]]',
848+
'key',
849+
{ param: 'myKey' }
850+
)
851+
})
852+
})
853+
700854
describe('sqs', () => {
701855
it('should throw if requestParameters is not a string to string', () => {
702856
serverlessApigatewayServiceProxy.serverless.service.custom = {

‎lib/index.js‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ const compileSqsServiceProxy = require('./package/sqs/compileSqsServiceProxy')
2323
// S3
2424
const compileMethodsToS3 = require('./package/s3/compileMethodsToS3')
2525
const compileIamRoleToS3 = require('./package/s3/compileIamRoleToS3')
26-
const validateS3ServiceProxy = require('./package/s3/validateS3ServiceProxy')
2726
const compileS3ServiceProxy = require('./package/s3/compileS3ServiceProxy')
2827
// SNS
2928
const compileMethodsToSns = require('./package/sns/compileMethodsToSns')
@@ -56,7 +55,6 @@ class ServerlessApigatewayServiceProxy {
5655
compileMethodsToS3,
5756
compileIamRoleToS3,
5857
compileS3ServiceProxy,
59-
validateS3ServiceProxy,
6058
compileMethodsToSns,
6159
compileIamRoleToSns,
6260
validateSnsServiceProxy,

‎lib/package/s3/compileS3ServiceProxy.js‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
module.exports = {
44
async compileS3ServiceProxy() {
5-
this.validateS3ServiceProxy()
65
this.compileIamRoleToS3()
76
this.compileMethodsToS3()
87
}

‎lib/package/s3/schema.js‎

Lines changed: 0 additions & 30 deletions
This file was deleted.

‎lib/package/s3/validateS3ServiceProxy.js‎

Lines changed: 0 additions & 18 deletions
This file was deleted.

0 commit comments

Comments
 (0)