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-13998] make url of X-Tyk-Upstream optional when loadBalancing is present #6860

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

pvormste
Copy link
Contributor

@pvormste pvormste commented Feb 4, 2025

User description

TT-13998
Summary [OAS Load balancer] upstream.URL is required when load balancer is enabled
Type Bug Bug
Status In Dev
Points N/A
Labels -

This PR makes upstream.url optional when upstream.loadBalancing[].targets.url is present.

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)

PR Type

Bug fix


Description

  • Made upstream.url optional when loadBalancing is present.

  • Updated OAS schema to include oneOf validation for url or loadBalancing.


Changes walkthrough 📝

Relevant files
Bug fix
x-tyk-api-gateway.json
Update OAS schema to validate `url` or `loadBalancing`     

apidef/oas/schema/x-tyk-api-gateway.json

  • Replaced required array with oneOf validation.
  • Ensured either url or loadBalancing is required.
  • Adjusted schema to align with new validation logic.
  • +7/-2     

    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 4, 2025

    Let's make that PR title a 💯 shall we? 💪

    Your PR title and story title look slightly different. Just checking in to know if it was intentional!

    Story Title [OAS Load balancer] upstream.URL is required when load balancer is enabled
    PR Title [TT-13998] make url of X-Tyk-Upstream optional when loadBalancing is present

    Check out this guide to learn more about PR best-practices.

    Copy link
    Contributor

    github-actions bot commented Feb 4, 2025

    API Changes

    no api changes detected

    Copy link
    Contributor

    github-actions bot commented Feb 4, 2025

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

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

    Schema Validation Logic

    Ensure the oneOf validation for url or loadBalancing is correctly implemented and does not introduce unintended behavior or conflicts in the schema.

    "oneOf": [
      {
        "required": ["url"]
      },
      {
        "required": ["loadBalancing"]
      }
    ]

    Copy link
    Contributor

    github-actions bot commented Feb 4, 2025

    PR Code Suggestions ✨

    No code suggestions found for the PR.

    @pvormste pvormste force-pushed the fix/TT-13998/fix-oas-make-url-optional-when-load-balancing-present branch from 57feb78 to 82f8b2e Compare February 4, 2025 13:40
    @pvormste pvormste enabled auto-merge (squash) February 4, 2025 13:41
    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.

    3 participants