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

TT-13890: request debug endpoint #6862

Merged
merged 9 commits into from
Feb 5, 2025
Merged

Conversation

lghiur
Copy link
Member

@lghiur lghiur commented Feb 5, 2025

User description

TT-13890
Summary [Debugger MVP] Implement request
Type Story Story
Status In Dev
Points N/A
Labels -

TT-13890

Description

As we're building the OAS debugger screen, we need to extend the /debug endpoint to accept OAS Api definitions in the payload.

the /debug endpoint accepts now oas key in the payload.

Related Issue

Motivation and Context

How This Has Been Tested

Screenshots (if appropriate)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Refactoring or add test (improvements in base code or adds test coverage to functionality)

Checklist

  • I ensured that the documentation is up to date
  • I explained why this PR updates go.mod in detail with reasoning why it's required
  • I would like a code coverage CI quality gate exception and have explained why

PR Type

Enhancement, Tests


Description

  • Extended /debug endpoint to support OAS API definitions.

  • Updated test cases to validate OAS support in /debug.

  • Enhanced Swagger documentation with OAS examples and descriptions.

  • Refactored traceHandler to handle OAS payloads effectively.


Changes walkthrough 📝

Relevant files
Tests
gateway_test.go
Add test cases for OAS support in `/debug`                             

gateway/gateway_test.go

  • Added test cases for OAS API definitions in /debug endpoint.
  • Introduced oasSpec for testing OAS-specific scenarios.
  • Validated OAS payload handling in debug endpoint tests.
  • +8/-0     
    Enhancement
    tracing.go
    Update `traceHandler` to support OAS payloads                       

    gateway/tracing.go

  • Added OAS field to traceRequest struct.
  • Updated traceHandler to process OAS payloads.
  • Converted OAS definitions to classic API definitions.
  • +7/-2     
    Documentation
    swagger.yml
    Update Swagger documentation for OAS support                         

    swagger.yml

  • Added examples for OAS API definitions in /debug endpoint.
  • Updated summary to reflect OAS support.
  • Enhanced Swagger documentation for better clarity.
  • +66/-24 

    Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • @buger
    Copy link
    Member

    buger commented Feb 5, 2025

    I'm a bot and I 👍 this PR title. 🤖

    Copy link
    Contributor

    github-actions bot commented Feb 5, 2025

    Swagger Changes

      
      
      
        
         _        __  __
        example:
        examples:
        oas:
        oneOf:
       _| |_   _ / _|/ _|  between swagger-prev.yml
      + one map entry added:
      + one map entry added:
      + one map entry added:
      - one map entry removed:
      ± value change
     / _' | | | | |_| |_       and swagger-current.yml
     \__,_|\__, |_| |_|   returned four differences
    components.schemas.TraceRequest
    components.schemas.TraceRequest.properties
    paths./tyk/debug.post.requestBody.content.application/json
    paths./tyk/debug.post.summary
    | (_| | |_| |  _|  _|

    Copy link
    Contributor

    github-actions bot commented Feb 5, 2025

    API Changes

    no api changes detected

    Copy link
    Contributor

    github-actions bot commented Feb 5, 2025

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Possible Issue

    The new logic in the traceHandler function assumes that if traceReq.OAS is not nil, the ExtractTo method will always succeed. This could lead to unexpected behavior if ExtractTo fails or produces invalid data.

    if traceReq.OAS != nil {
    	var newDef apidef.APIDefinition
    	traceReq.OAS.ExtractTo(&newDef)
    	traceReq.Spec = &newDef
    } else if traceReq.Spec == nil {
    Documentation Update

    The Swagger documentation has been updated to include examples for both classic and OAS API definitions, but it is unclear if these examples are comprehensive enough to cover all edge cases.

    examples:
      apiDefinition:
        summary: Calling debug endpoint with classic API definition.
        value:
          request:
            method: GET
            path: /update-listen-path
          spec:
            api_id: b84fe1a04e5648927971c0557971565c
            auth:
              auth_header_name: authorization
            definition:
              key: version
              location: header
            name: Tyk Test API
            org_id: 664a14650619d40001f1f00f
            proxy:
              listen_path: /tyk-api-test/
              strip_listen_path: true
              target_url: https://httpbin.org
            use_oauth2: true
            version_data:
              not_versioned: true
              versions:
                Default:
                  name: Default
      oasApiDefinition:
        summary: Calling debug endpoint with OAS API definition.
        value:
          request:
            method: GET
            path: /get
            oas:
              info:
                title: testdebug
                version: 1.0.0
              openapi: 3.0.3
              servers:
              - url: http://localhost:8181/testdebug/
              security: []
              paths: {}
              components:
                securitySchemes: {}
              x-tyk-api-gateway:
                info:
                  dbId: 67a25ff65b60081c8731464f
                  id: d37ea0e360c245cf406d640f1dbf788d
                  orgId: 645b3db586341f751f4258aa
                  name: testdebug
                  state:
                    active: true
                    internal: false
                middleware:
                  global:
                    contextVariables:
                      enabled: true
                    trafficLogs:
                      enabled: true
                server:
                  listenPath:
                    strip: true
                    value: "/testdebug/"
                upstream:
                  url: http://httpbin.org/
    

    Copy link
    Contributor

    github-actions bot commented Feb 5, 2025

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Validate OAS object before processing

    Add validation to ensure that the OAS object in the traceReq is properly structured
    and contains necessary fields before calling ExtractTo, to prevent potential runtime
    errors.

    gateway/tracing.go [99-103]

     if traceReq.OAS != nil {
    +    if !traceReq.OAS.IsValid() { // Hypothetical validation method
    +        log.Error("Invalid OAS object")
    +        doJSONWrite(w, http.StatusBadRequest, apiError("Invalid OAS object"))
    +        return
    +    }
         var newDef apidef.APIDefinition
         traceReq.OAS.ExtractTo(&newDef)
         traceReq.Spec = &newDef
     } else if traceReq.Spec == nil {
    Suggestion importance[1-10]: 8

    __

    Why: Adding validation for the OAS object ensures that it is properly structured before calling ExtractTo, preventing potential runtime errors. This is a critical improvement for robustness and error handling in the code.

    Medium
    General
    Complete oasApiDefinition example fields

    Ensure that the oasApiDefinition example includes all required fields for a valid
    OAS object, such as paths and components, to prevent confusion or errors during
    implementation.

    swagger.yml [1831-1841]

     oas:
       info:
         title: testdebug
         version: 1.0.0
       openapi: 3.0.3
       servers:
       - url: http://localhost:8181/testdebug/
       security: []
    -  paths: {}
    +  paths:
    +    /example:
    +      get:
    +        summary: Example endpoint
    +        responses:
    +          '200':
    +            description: Successful response
       components:
    -    securitySchemes: {}
    +    securitySchemes:
    +      apiKeyAuth:
    +        type: apiKey
    +        in: header
    +        name: Authorization
    Suggestion importance[1-10]: 7

    __

    Why: Including all required fields in the oasApiDefinition example improves clarity and ensures that the example is valid and functional, reducing the likelihood of confusion or errors during implementation.

    Medium

    @lghiur lghiur merged commit ee3a332 into master Feb 5, 2025
    31 of 43 checks passed
    @lghiur lghiur deleted the TT-13890-request-debug-endpoint branch February 5, 2025 18:30
    @titpetric titpetric changed the title Tt 13890 request debug endpoint TT-13890 request debug endpoint Feb 5, 2025
    @titpetric titpetric changed the title TT-13890 request debug endpoint TT-13890: request debug endpoint Feb 5, 2025
    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.

    4 participants