CLOUDP-287245: IPA-109: Validate custom method must be GET or POST #313
Conversation
| const errors = []; | ||
|
|
||
| // Iterate through each path key and its corresponding path item | ||
| for (const [pathKey, pathItem] of Object.entries(paths)) { |
There was a problem hiding this comment.
Ah didn't realize that paths is not an array, I thought that the change would only make the input contain a single path and it's pathitems, not the whole paths object. Sorry about that! This makes the function a bit harder to read IMO, feel free to revert back to the previous approach if you want. WDYT?
There was a problem hiding this comment.
Or perhaps try $.paths[*]
There was a problem hiding this comment.
It worked with $.paths[*] and using path from the context. Could you take another look and let me know how it looks?
There was a problem hiding this comment.
$.paths[*] gives you the object for each path, but it doesn't include the path keys themselves, so it's not quite enough on its own
There was a problem hiding this comment.
Thanks! Looks good! Just one question on the logic
| if (httpMethods.some((method) => !VALID_METHODS.includes(method))) { | ||
| return ERROR_RESULT; | ||
| } |
There was a problem hiding this comment.
Will this return error if the path has an exception extension?
Suggestion: use $.path[get,put,post,delete,options,head,patch,trace] to ignore any other objects than the methods, kinda like we do here:
openapi/tools/spectral/.spectral.yaml
Line 9 in 98f5b23
There was a problem hiding this comment.
Yes, it would return error with an exception extension. Good catch!
There was a problem hiding this comment.
"#PathItem[get,put,post,delete,options,head,patch,trace]" gives the object inside HTTP method. I will exclude x-xgen-IPA-exception
|
|
||
| //Exclude exception extension key | ||
| let keys = Object.keys(input); | ||
| const httpMethods = keys.filter((key) => key !== EXCEPTION_EXTENSION_KEY); |
There was a problem hiding this comment.
@lovisaberggren I excluded the exception extension key here, but in the opposite I can only check the list of predefined HTTP methods as well
There was a problem hiding this comment.
Thanks! Yeah perhaps the opposite is more stable, as there may be other extensions for example added to paths, and HTTP methods probably won't change a lot 😄
| const ERROR_MESSAGE = 'The HTTP method for custom methods must be GET or POST.'; | ||
| const ERROR_RESULT = [{ message: ERROR_MESSAGE }]; | ||
| const VALID_METHODS = ['get', 'post']; | ||
| const HTTP_METHODS = ['get', 'put', 'post', 'delete', 'options', 'head', 'patch', 'trace']; |
There was a problem hiding this comment.
[Just a note for future rules]: I expect we will have other similar rules coming up, at that point we can consider refactoring out a common method to check for valid/invalid methods to the utils package
2 participants
Proposed changes
Jira ticket: CLOUDP-287245
Implements a rule that the HTTP method for custom methods must be either GET or POST (https://docs.devprod.prod.corp.mongodb.com/ipa/109).
Checklist
Changes to Spectral
Further comments