Skip to content

Conversation

@yelizhenden-mdb
Copy link
Collaborator

@yelizhenden-mdb yelizhenden-mdb commented Mar 31, 2025

Proposed changes

Jira ticket: CLOUDP-304963

  xgen-IPA-114-parameterized-paths-have-404-not-found:
    description: |
      Paths with parameters must define 404 responses.

      ##### Implementation details
      This rule checks that all endpoints with path parameters (identified by '{param}' 
      in the path) include a 404 response to handle the case when the requested resource
      is not found.

Checklist

  • I have signed the MongoDB CLA
  • I have added tests that prove my fix is effective or that my feature works

Changes to Spectral

  • I have read the README file for Spectral Updates

Further comments

@yelizhenden-mdb yelizhenden-mdb marked this pull request as ready for review March 31, 2025 16:56
@yelizhenden-mdb yelizhenden-mdb requested a review from a team as a code owner March 31, 2025 16:56
is not found.
message: '{{error}} https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-114-parameterized-paths-have-not-found'
severity: warn
given: '$.paths[*][get,put,post,delete,options,head,patch,trace]'
Copy link
Member

@wtrocki wtrocki Apr 1, 2025

Choose a reason for hiding this comment

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

[Outside PR] Delete has it's own rule covering 404. If if introduce generic rule (which I think makes sense) we should remove dedicated 404 rule for delete. Is that planned? WDYT?

Copy link
Member

@wtrocki wtrocki Apr 1, 2025

Choose a reason for hiding this comment

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

[Question] Separately - what is the purpose of mentioning all of the methods here.
I guess you wanted to exclude exceptions? If yes the same filtering can be done in the code.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, I wanted to exclude exceptions and all the stuff other than HTTP verbs. I agree that it can be handled in the code, but it is widely used approach at the moment, so I followed it

We can remove the dedicated 404 rule for delete, as you said it will be covered by this one. I will share the PR for removing it in a separate PR, if that is okay

Copy link
Member

Choose a reason for hiding this comment

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

Thank you.

Copy link
Member

@wtrocki wtrocki left a comment

Choose a reason for hiding this comment

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

LGTM with follow up recommendation.

@wtrocki wtrocki requested a review from Copilot April 1, 2025 09:18
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@yelizhenden-mdb yelizhenden-mdb merged commit fef1aba into main Apr 1, 2025
8 checks passed
@yelizhenden-mdb yelizhenden-mdb deleted the CLOUDP-304963 branch April 1, 2025 11:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants