Skip to content
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.

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

[DX-1783]change swagger to add batch request #5835

Merged
merged 2 commits into from
Dec 19, 2024

Conversation

yurisasuke
Copy link
Member

@yurisasuke yurisasuke commented Dec 19, 2024

https://deploy-preview-5835--tyk-docs.netlify.app/docs/nightly/tyk-gateway-api/
DX-1783

User description

For internal users - Please add a Jira DX PR ticket to the subject!



Preview Link


Description


Screenshots (if appropriate)


Checklist

  • I have added a preview link to the PR description.
  • I have reviewed the suggestions made by our AI (PR Agent) and updated them accordingly (spelling errors, rephrasing, etc.)
  • I have reviewed the guidelines for contributing to this repository.
  • I have read the technical guidelines for contributing to this repository.
  • Make sure you have started your change off our latest master.
  • I labeled the PR

PR Type

Documentation


Description

  • Added comprehensive documentation for Tyk's batch request functionality, explaining its purpose, usage, and configuration.
  • Introduced a new endpoint /{listen_path}/tyk/batch in the Swagger file, detailing its parameters, request body, and responses.
  • Provided sample request and response examples for batch requests to illustrate usage.
  • Updated schema references to use Tyk's repository for better alignment with internal definitions.
  • Defined new schema components (BatchRequestStructure, BatchReplyUnit, and RequestDefinition) to support the batch request feature.

Changes walkthrough 📝

Relevant files
Documentation
gateway-swagger.yml
Add batch request documentation and endpoint details to Swagger file.

tyk-docs/assets/others/gateway-swagger.yml

  • Added detailed documentation for batch request functionality in Tyk,
    including descriptions, examples, and schema definitions.
  • Introduced a new endpoint /{listen_path}/tyk/batch for handling batch
    requests with detailed request and response structures.
  • Updated schema references to point to Tyk's repository instead of the
    OpenAPI Specification repository.
  • Added new schema components such as BatchRequestStructure,
    BatchReplyUnit, and RequestDefinition to support batch request
    documentation.
  • +298/-7 

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    Copy link
    Contributor

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
    🧪 No relevant tests
    🔒 Security concerns

    Potential Security Concern:
    The batch request endpoint allows requests without a valid key, which could be exploited if not properly secured. Ensure that appropriate validations and rate-limiting mechanisms are in place to prevent abuse.

    ⚡ Recommended focus areas for review

    Documentation Clarity
    The added documentation for batch requests is detailed but could benefit from additional clarity and consistency. For example, the repetition of the phrase "This is especially handy if clients have complex requests..." might confuse readers. Ensure the documentation is concise and avoids redundancy.

    Schema Reference Update
    The schema references have been updated to point to Tyk's repository instead of the OpenAPI Specification repository. This change should be verified to ensure it aligns with the intended functionality and does not introduce compatibility issues.

    Batch Request Endpoint
    The new batch request endpoint is well-documented, but the security implications of allowing requests without a valid key should be reviewed. Ensure this does not introduce vulnerabilities or unintended behaviors.

    Schema Definitions for Batch Requests
    The new schema definitions for BatchReplyUnit, BatchRequestStructure, and RequestDefinition should be reviewed for completeness and correctness. Ensure they cover all necessary fields and edge cases.

    Copy link
    Contributor

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    General
    Add required field validation to the BatchRequestStructure schema to ensure proper request formatting

    Ensure that the BatchRequestStructure schema includes validation for required fields
    such as requests and suppress_parallel_execution to prevent malformed requests from
    being processed.

    tyk-docs/assets/others/gateway-swagger.yml [5554-5563]

     BatchRequestStructure:
       properties:
         requests:
           items:
             $ref: '#/components/schemas/RequestDefinition'
    -      nullable: true
    +      nullable: false
           type: array
         suppress_parallel_execution:
           type: boolean
    +  required:
    +    - requests
    +    - suppress_parallel_execution
       type: object
    Suggestion importance[1-10]: 9

    Why: Adding required field validation for requests and suppress_parallel_execution ensures that malformed requests are not processed, improving the robustness and reliability of the API.

    9
    Restrict the method property in the RequestDefinition schema to valid HTTP methods

    Add validation for the method property in the RequestDefinition schema to restrict
    it to valid HTTP methods like GET, POST, etc.

    tyk-docs/assets/others/gateway-swagger.yml [5564-5577]

     RequestDefinition:
       properties:
         body:
           type: string
         headers:
           additionalProperties:
             type: string
           nullable: true
           type: object
         method:
           type: string
    +      enum:
    +        - GET
    +        - POST
    +        - PUT
    +        - DELETE
    +        - PATCH
    +        - OPTIONS
    +        - HEAD
         relative_url:
           type: string
       type: object
    Suggestion importance[1-10]: 8

    Why: Restricting the method property to valid HTTP methods enhances the schema's accuracy and prevents invalid or unsupported methods from being used, improving API reliability.

    8
    Clarify the authorization requirements for batch requests to avoid confusion

    Clarify in the description of the /tyk/batch endpoint that while the batch request
    itself does not require a valid key, each individual request within the batch must
    include valid authorization headers.

    tyk-docs/assets/others/gateway-swagger.yml [101]

    -Batch requests are created by POSTING to the `/{listen_path}/tyk/batch/` endpoint. These requests **do not require a valid key**, but their request list does.
    +Batch requests are created by POSTING to the `/{listen_path}/tyk/batch/` endpoint. While the batch request itself **does not require a valid key**, each individual request within the batch must include valid authorization headers.
    Suggestion importance[1-10]: 7

    Why: Clarifying the authorization requirements for batch requests helps avoid potential misunderstandings and ensures developers implement the API correctly, improving documentation quality.

    7
    Security
    Add validation to the relative_url property to ensure only valid paths are allowed

    Ensure that the relative_url property in the RequestDefinition schema is validated
    to prevent invalid or malicious paths from being processed.

    tyk-docs/assets/others/gateway-swagger.yml [5564-5577]

     RequestDefinition:
       properties:
         body:
           type: string
         headers:
           additionalProperties:
             type: string
           nullable: true
           type: object
         method:
           type: string
         relative_url:
           type: string
    +      pattern: '^/[a-zA-Z0-9/_-]+$'
       type: object
    Suggestion importance[1-10]: 9

    Why: Adding a pattern validation for relative_url ensures that only valid and safe paths are processed, addressing potential security concerns and improving API robustness.

    9

    Copy link

    netlify bot commented Dec 19, 2024

    PS. Pls add /docs/nightly to the end of url

    Name Link
    🔨 Latest commit b3efcb8
    🔍 Latest deploy log https://app.netlify.com/sites/tyk-docs/deploys/6764436548f03e00083a8cfa
    😎 Deploy Preview https://deploy-preview-5835--tyk-docs.netlify.app
    📱 Preview on mobile
    Toggle QR Code...

    QR Code

    Use your smartphone camera to open QR code link.

    To edit notification comments on pull requests, go to your Netlify site configuration.

    Copy link

    netlify bot commented Dec 19, 2024

    PS. Pls add /docs/nightly to the end of url

    Name Link
    🔨 Latest commit d0e36b5
    🔍 Latest deploy log https://app.netlify.com/sites/tyk-docs/deploys/67644f8f8bb4ec00088eeead
    😎 Deploy Preview https://deploy-preview-5835--tyk-docs.netlify.app
    📱 Preview on mobile
    Toggle QR Code...

    QR Code

    Use your smartphone camera to open QR code link.

    To edit notification comments on pull requests, go to your Netlify site configuration.

    @yurisasuke yurisasuke merged commit 4aad425 into master Dec 19, 2024
    9 checks passed
    @yurisasuke yurisasuke deleted the add-batch-request-to-the-oas branch December 19, 2024 16:56
    @yurisasuke
    Copy link
    Member Author

    /release to release-5.7

    Copy link

    tykbot bot commented Dec 19, 2024

    Working on it! Note that it can take a few minutes.

    tykbot bot pushed a commit that referenced this pull request Dec 19, 2024
    * change swagger to add batch request
    
    (cherry picked from commit 4aad425)
    Copy link

    tykbot bot commented Dec 19, 2024

    @yurisasuke Succesfully merged PR

    @yurisasuke
    Copy link
    Member Author

    /release to release-5.6

    Copy link

    tykbot bot commented Dec 19, 2024

    Working on it! Note that it can take a few minutes.

    tykbot bot pushed a commit that referenced this pull request Dec 19, 2024
    * change swagger to add batch request
    
    (cherry picked from commit 4aad425)
    Copy link

    tykbot bot commented Dec 19, 2024

    @yurisasuke Succesfully merged PR

    @yurisasuke yurisasuke changed the title change swagger to add batch request [DX-1783]change swagger to add batch request Dec 19, 2024
    buger added a commit that referenced this pull request Dec 19, 2024
    change swagger to add batch request (#5835)
    
    * change swagger to add batch request
    buger added a commit that referenced this pull request Dec 19, 2024
    change swagger to add batch request (#5835)
    
    * change swagger to add batch request
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    1 participant