repsheet / repsheet-nginx

The nginx module for Repsheet
Apache License 2.0
82 stars 11 forks source link

Add $repsheet, $repsheet_reason, store request state on request ctx #38

Closed thetristan closed 8 years ago

thetristan commented 8 years ago

This change removes the need to pre-set any variables in the nginx configuration by updating the module to provide both the $repsheet and $repsheet_reason variables. Due to how the proxy module works, setting proxy_set_header X-Repsheet $repsheet; is still required for passing the marking header to any upstream proxies.

I also updated the module to properly store it's state on the module's request context (instead of on the location config which is shared between requests), and I removed the guard that prevented the module from running for internal redirects (these subrequests cause a request to clear its state).

It doesn't look like there are any tests that test to ensure the variables set by this module (essentially the marking headers) are set/proxied correctly -- is that intentional?

abedra commented 8 years ago

Thanks @thetristan! I'll take a look at these changes today.

abedra commented 8 years ago

This looks great! I'll merge now and do some digging to make sure nothing broke as well. As far as testing the marking headers, it's a bit tricky. I'd welcome a good way to test that but haven't figured out the best approach yet.