Http api schema validation

[fredericbarthelet]

Closes: #8021

[codecov-commenter]

Codecov Report

Merging #8068 into master will increase coverage by 0.05%.
The diff coverage is 50.00%.

@@            Coverage Diff             @@
##           master    #8068      +/-   ##
==========================================
+ Coverage   88.37%   88.42%   +0.05%     
==========================================
  Files         248      248              
  Lines        9446     9445       -1     
==========================================
+ Hits         8348     8352       +4     
+ Misses       1098     1093       -5     

Impacted Files	Coverage Δ
.../classes/ConfigSchemaHandler/normalizeAjvErrors.js	95.79% <0.00%> (-1.64%)	⬇️
lib/plugins/aws/provider/awsProvider.js	92.77% <ø> (ø)
...lugins/aws/package/compile/events/httpApi/index.js	92.57% <66.66%> (+2.06%)	⬆️
lib/classes/ConfigSchemaHandler/index.js	96.55% <100.00%> (+3.52%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 20d9c64...8f6cc2c. Read the comment docs.

[medikoo]

@fredericbarthelet that's outstanding, thank you! It's great to see missing schemas being filled in.

Please see my remarks, I've proposed some additional fine tuning to what you've proposed, so it's as bulletproof as it can be (I hope)

[medikoo]

It'll be nice to make it more restrictive with regexp pattern.

I guess we can improve regexp at methodPathPattern so it's:

const methodPathPattern = new RegExp(
  `^(\\*|(?:${Array.from(allowedMethods).join('|')}))( .+)?$`,
  'i'
);

And define property with regexp setting

Also having that, it'll be nice to simplify validation logic in resolveConfiguration, as over there we may blindly assume that format is respected. So we should cover only cases which are not handled by schema

[fredericbarthelet]

@medikoo, noted, will update accordingly :)
One last thing on that matter : some error messages lose in clarity
Suppose you define for a function hello the httpApi attribute equals to the string myPath, the following error messages are displayed :

• before schema validation : Invalid "<method> <path>" route in function hello for httpApi event in serverless.yml
• after schema validation : Configuration error at 'functions.hello.events[0].httpApi': should pass "regexp" keyword validation

One way to compensate would be to define custom JSON Schema error messages based on validation keywords. WDYT ?

[medikoo]

@fredericbarthelet that's a very good point.

In light of that I think we should improve messaging of regex keyword errors. It easily can be done here:

serverless/lib/classes/ConfigSchemaHandler/normalizeAjvErrors.js

Lines 210 to 233 in 20d9c64

	switch (error.keyword) {
	case 'additionalProperties':
	if (error.dataPath === '.functions') {
	error.message = `name '${error.params.additionalProperty}' must be alphanumeric`;
	} else {
	error.message = `unrecognized property '${error.params.additionalProperty}'`;
	}
	break;
	case 'anyOf':
	if (isEventTypeDataPath(error.dataPath)) {
	error.message = 'unsupported function event';
	break;
	}
	// fallthrough
	case 'oneOf':
	error.message = 'unsupported configuration format';
	break;
	case 'enum':
	if (error.params.allowedValues.every(value => typeof value === 'string')) {
	error.message += ` [${error.params.allowedValues.join(', ')}]`;
	}
	break;
	default:
	}

Let's replace the message, with e.g. doesn't match expected format (if you feel you can come up with something better, please propose :)

[fredericbarthelet]

@medikoo, I added a custom message for regexp keyword : value '${error.data}' does not satisfy pattern ${error.schema}. The resulting error message in the scenario described above, now much clearer, is :

Configuration error at 'functions.hello.events[0].httpApi': value "myPath" does not satisfy pattern /^(?:*|(GET|POST|PUT|PATCH|OPTIONS|HEAD|DELETE) (/\S*))$/i

WDYT ?

[medikoo]

Looks good. I wonder whether listing pattern, won't be too noisy, but it's definitely very informative, so I'd say it's great!

[fredericbarthelet]

@medikoo, thanks for your review. I updated my PR using your feedbacks.

[medikoo]

@fredericbarthelet looks great! I've proposed just one last minor suggestion, and we should be good to go

[fredericbarthelet]

@medikoo, PR updated with custom error message and regexp pattern for path property :)

[medikoo]

Thank you @fredericbarthelet that looks great!