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

added proxy config for complex proxy rules #907

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

Conversation

i3ene
Copy link

@i3ene i3ene commented Nov 14, 2024

Description

Added an additional --proxy-config parameter that takes in a .json file (with path).
The use of it overrides any other proxy configuration! (Well, it actually disables the other options, not overrides it, but this could also be implemented.)
This allows to write individual rules like:

{
  "/api/*": {
    "target": "localhost:8090",
    "changeOrigin": true,
    "secure": false,
    "ws": false,
    "pathRewrite": {
      "^/api": ""
    }
  },
  "*": {
    "target": "localhost:8070",
    "changeOrigin": false
  }
}

Relevant Issues

This addresses #439, #901 and #906.
I wanted to make a first approach with this to maybe incorporate more functionality different people need. It does rely on a file and therefor isn't CLI only configurable, but with the complexity some rules can have, I think it is not feasible.

Tasks

There are still a few things to do:

  • Write tests
  • Create documentation
  • Default proxy (use default target and port) behavior if no rule matches

@KernelDeimos
Copy link
Contributor

This is a nice addition! I agree that trying to configure this in the CLI would be cumbersome; I can't imagine many people would prefer to do it that way.

This might take some time to review for potential security concerns. I would also feel more comfortable if there are other contributors willing to take a look at this as well. I can't think of any issues on my first look, but there's always something.

@KernelDeimos
Copy link
Contributor

KernelDeimos commented Mar 1, 2025

Hi, this has been inactive for a bit but it needs tests. If you're still interested in having this merged that would be a great help. In either case I'll try to come back to this in a few days and give it a proper review, maybe I'll add the test myself.

@i3ene
Copy link
Author

i3ene commented Mar 2, 2025

Sorry for not responding so long, I'll try to incorporate some tests in the next few days! I'm still behind this functionality and think it would help a lot of others.

@i3ene
Copy link
Author

i3ene commented Mar 2, 2025

Some discussion:
Should the rules be written as real Regex, or should we use simplified patterns?
At the moment I try to simplify it as to make it more usable. For example:

Simplified Actual Regex
/some/path/* /some/path/.*
/some/$/path /some/[^/]*/path (not implemented yet)

@KernelDeimos
Copy link
Contributor

I see the problem you're solving there. I think it should be either just regex or a convention that's common for paths in a well-known platform or framework. Maybe glob syntax?

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.

2 participants