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

Rewrite middleware and fallthrough #515

Closed
miekg opened this issue Feb 9, 2017 · 7 comments
Closed

Rewrite middleware and fallthrough #515

miekg opened this issue Feb 9, 2017 · 7 comments

Comments

@miekg
Copy link
Member

miekg commented Feb 9, 2017

The rewrite middleware has fallthrough (at my request) to allows for falling through to the next middleware even though this middleware should be authoritative for this name.

q: should we keep something like fallthrough or just allow it? I'm not sure about the ramifications, but my gut feeling says that just allowing it can lead to weird to debug DNS issues... (and God forbid, maybe loops).
I'm also a fan of keeping things explicit; hence I think I'm in favor of keeping it.

@johnbelamaric
Copy link
Member

I think we should keep it. I'd like to add it to the k8s one too.

@johnbelamaric
Copy link
Member

Or maybe to etcd.

@miekg
Copy link
Member Author

miekg commented Feb 9, 2017

Ack. I can live with that. Maybe we should elevate it to be part of the Handler interface, as a Fallthrough method? We can then put in the NextOrFailure or something.

@miekg
Copy link
Member Author

miekg commented Feb 17, 2017

Best thing now is to document this behavior in middleware.md.

@miekg miekg added this to the 006 milestone Feb 17, 2017
@stp-ip
Copy link
Member

stp-ip commented Feb 17, 2017

Adding fallthrough in general as a highlevel option would enable workarounds like having multiple auto directives within one server. Basically abusing fallthrough and the proxy middleware.
5053 -> auto . {} + proxy -> 5054 -> auto . {} + proxy -> 5055 -> k8s + proxy -> public resolution.
Huge performance issue, but would enable various workarounds and enable testing various solutions in the wild (ignoring the performance hit) until we can implement or decide to implement it.

@miekg
Copy link
Member Author

miekg commented Feb 18, 2017 via email

@stp-ip
Copy link
Member

stp-ip commented Feb 18, 2017

One could also produce loops with the rewrite fallthrough. Debugging could be harder, but we could help by writing to the log each time a request falls through.

miekg added a commit that referenced this issue Feb 20, 2017
Update middleware.md to describe the need and use of `fallthrough* in
middlewares.

Fixes #515
miekg added a commit that referenced this issue Feb 20, 2017
While documenting the fallthrough behavior and testing it I noticed
the did not properly work. This PR does a tiny bit too much as it

- Documents fallthrough
- Fixes fallthrough in reverse
- Makes directives_generate complain on duplicate priorities
- Moved reverse *before* file in middleware.cfg
- Add a test that tests the reverse fallthrough behavior with a file
  backend

Fixes #515
miekg added a commit that referenced this issue Feb 20, 2017
* Document fallthrough and fix *reverse*

While documenting the fallthrough behavior and testing it I noticed
the did not properly work. This PR does a tiny bit too much as it

- Documents fallthrough
- Fixes fallthrough in reverse
- Makes directives_generate complain on duplicate priorities
- Moved reverse *before* file in middleware.cfg
- Add a test that tests the reverse fallthrough behavior with a file
  backend

Fixes #515

* ....and fix the tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants