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

Give control for the reverse proxy path #18

Open
whitlockjc opened this issue Apr 19, 2016 · 18 comments
Open

Give control for the reverse proxy path #18

whitlockjc opened this issue Apr 19, 2016 · 18 comments

Comments

@whitlockjc
Copy link
Contributor

whitlockjc commented Apr 19, 2016

Right now, the generated nginx will retain the originally requested path when performing the reverse proxy to the k8s pod. We need a way to allow the user to configure this and here are three scenarios:

  1. Retain the originally requested path: /nodejs/x -> /nodejs/x
  2. Remove the base path: /nodejs/x -> /x
  3. Simple base path rewriting: /nodejs/x -> /java/x

The current publicPaths is in the form of {CONTAINER_PORT}:{PATH} and to facilitate this, we could use {CONTAINER_PORT}:{PATH}[:{TARGET_PATH}]. Here are examples to solve these use cases:

  1. 3000:/nodejs or 3000:/nodejs:
  2. 3000:/nodejs:/
  3. 3000:/nodejs:/java

Since : is a valid URI character, the separator for the third optional part might need to be changed but this should illustrate the problem and suggested fix.

@tnine
Copy link
Contributor

tnine commented Apr 19, 2016

I'd suggest just using an arrow of => to perform the input to output mapping. To me it's intuitive, and it's an easy parse separator.

@whitlockjc
Copy link
Contributor Author

I don't disagree but I do want it to be somewhat intuitive in that spaces matter so it can't be 3000:test.github.com -> /, it would need to be 3000:test.github.com->/ That seems ugly to me but let me think about it more.

On a different note, I really wish we could break this up into an object but without a custom type, we either overload the value as serialized JSON or we continue with simple tokenization like have now.

@mpnally
Copy link

mpnally commented Jun 13, 2016

I'd like us to start with a definition of the problem we are solving. What are the customer scenarios? Why are people motivated to change paths, rather than passing them through?

@whitlockjc
Copy link
Contributor Author

Some applications will be sensitive the the path it receives. Not only this but there is a discrepancy amongst proxies. For example, Apigee Edge will remove the proxy's base path from the target request while our router will not. Since we cannot predict how people will deploy applications to Kubernetes and we should not force applications be written to be deployed for Kubernetes, we should just let the application author configure how this themselves.

@mpnally
Copy link

mpnally commented Jun 14, 2016

I'd like to understand the customer's problem in more detail. Our ingress controller is designed to allow each namespace to appear on the outside as a single domain (e.g. acme.com), or possibly a set of related domains (e.g. acme.com and acme-canary-test.com). Within that domain, there are paths (e.g. /foo, /bar) that are mapped to deployments of "components". The obvious approach is that the "component" that handles /foo should implement /foo. We are proposing to allow the case where the "component" implements /baz, but it is exposed as /foo from the namespace. Why? Is it because we are reusing a legacy component that already implements /baz? Are we solving some other problem? [I'm not arguing that we don't need the feature—I just want to know why we need it.]

@whitlockjc
Copy link
Contributor Author

I agree that if an application says it handles /foo that it should be written to be aware of /foo in the application's request path but sometimes, like during the hackathon we recently held, these public/private paths are really just used for namespacing and the target application may or may not be written to handle the base path used in the public/private path. Since we cannot guarantee this, at some point we will need to let the person creating the deployment specification to handle the situation where they might not want to have the public/private path in the target Pod request. A good example of this is a WebSockets application.

@mpnally
Copy link

mpnally commented Jun 14, 2016

"sometimes [...] these public/private paths are really just used for namespacing".
Is that a usage of paths that we encourage/support? Or is that just a misunderstanding by users that should be addressed in documentation?

@whitlockjc
Copy link
Contributor Author

Well, if you have multiple applications for the same hostname, the publicPath is really just a unique way to access the shared hostname and get traffic associated with your application instead of some other application deployed for the same hostname.

@mpnally
Copy link

mpnally commented Jun 14, 2016

If your goal is just to expose your application, why not just put it in its own namespace at '/'? Why mess with al this paths stuff at all?

@whitlockjc
Copy link
Contributor Author

The point is that as a generic router, we have no right to tell an application author that they have to alter their application that runs just fine outside of Kubernetes, or in Kubernetes when accessed directly without our router, to update their application just so it plays well with the hard coded approach to routing paths that we have. I agree that in most cases, especially in a managed environment, we can assume one way but this is a generic router that has no concept of these things. That's why I think we can make our router the better by support this.

@mpnally
Copy link

mpnally commented Jun 14, 2016

Now you are saying things that I understand. The user scenario you are
postulating is that I wrote an application as either a stand-alone
application or as a component in a different system. I now want to use it
as a component in a new system, where the basePaths it supports are
different. I do not want to change the code, so I want to map the new
basePaths to the 'legacy' basePaths. I would call the user scenarios
"repurposing a legacy application" and "repurposing a component from
another system".

On Tue, Jun 14, 2016 at 3:21 PM, Jeremy Whitlock [email protected]
wrote:

The point is that as a generic router, we have no right to tell an
application author that they have to alter their application that runs just
fine outside of Kubernetes, or in Kubernetes when accessed directly without
our router, to update their application just so it plays well with the hard
coded approach to routing paths that we have. I agree that in most cases,
especially in a managed environment, we can assume one way but this is a
generic router that has no concept of these things. That's why I think we
can make our router the better by support this.


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#18 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/ACoA6hlP5WfWsRBwjWtsO8I6Jengqeyjks5qLylWgaJpZM4ILHu1
.

@whitlockjc
Copy link
Contributor Author

Glad I got us on the same page. Yes, that's the idea. And it came up quite a bit in our internal hackathon. When people were pointed to this as a potential answer, they were very happy with this.

@mpnally
Copy link

mpnally commented Jun 15, 2016

I don't really like this idea, because it only works in a narrow set of circumstances. Suppose, for example, that the application includes self-links in all resources (something all apps should do, IMO). In other words, if I POST an new resource to /issues, the response will be {"self": "http://github.com/30x/ingress/issues/1234", ...}. If github mapped paths the way you describe—e.g. /issues=>/reports, then this application would break. This can be fixed by also passing the basePath as an environment variable to the application. The application can then write code that maps ALL the URLs on input (not just the one on the first line of the request), and on output, but this adds complexity. All of this could be done if it was really solving an important problem, but I still don't understand what that problem is.

@whitlockjc
Copy link
Contributor Author

I agree that it won't work for everything but for the cases it does, it means the author not having to rewrite their client/server and/or contend with other tenants. I agree with you but in the cases where it does work, we're not hurting anything giving them more control. As long as we don't label this as a silver bullet that makes magic happen, it should be limited in scope and all the ownership and risk is on the application author and not us.

@mpnally
Copy link

mpnally commented Jun 15, 2016

The problem with that is that it is difficult to explain when it will and
will not work. I do not think we have established that this is a
high-priority feature. I think we should leave it on the backlog.

On Wed, Jun 15, 2016 at 9:42 AM, Jeremy Whitlock [email protected]
wrote:

I agree that it won't work for everything but for the cases it does, it
means the author not having to rewrite their client/server and/or contend
with other tenants. I agree with you but in the cases where it does work,
we're not hurting anything giving them more control. As long as we don't
label this as a silver bullet that makes magic happen, it should be limited
in scope and all the ownership and risk is on the application author and
not us.


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#18 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/ACoA6uiBxJTyIZVJYOcHuo7-hO1g2ADaks5qMCtagaJpZM4ILHu1
.

@mpnally
Copy link

mpnally commented Dec 1, 2016

Ironic that I was arguing against this, and am now suggesting we do it :)

In your original issue description, you said there are 3 cases, but I think Nginx models this as just 2 cases. The cases are:

  1. The path is forwarded unaltered
  2. A path prefix is replaced by a different prefix. The second prefix can be the null string

I like your -> syntax. If we are using this syntax, would it not be more logical to put the port on the right? In other words, instead of writing 3000:/nodejs, I would write /nodejs -> 3000:. And instead of 3000:/nodejs/x -> /x, I would write /nodejs/x -> 3000:/x

@whitlockjc
Copy link
Contributor Author

You're right, the last two examples are really the same. Let me think about your proposed syntax and I'll get back to you.

@AdamMagaluk
Copy link
Contributor

I like the original path on the left syntax, feels more intuitive.

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

No branches or pull requests

4 participants