Skip to content
This repository has been archived by the owner on Jan 7, 2018. It is now read-only.

Support For Dynamic Headers #7

Merged
merged 6 commits into from
Feb 3, 2015
Merged

Conversation

darylrobbins
Copy link
Contributor

Add support for dynamic HTTP header values by using a handlebars template as the heaver value instead of a static value. This allows header values to be derived from other headers or other attributes of the request.

For example:
x-original-proto: {{headers.x-forwarded-proto}}

* master:
  Retry connecting to mongodb on startup failures.
  Whoops, remove some debug stuff from f3bc4c4
  Fix some race conditions in tests that led to periodic failures.
  Improve error handling during tests if redis or mongo connections fail.
  Flip order of test matchers to properly reflect "expected" output.
  Standardize on CircleCI for testing.
  Let the import_nginx_logs script uncompress gzipped logs on the fly.
  Revamp logging to use Bunyan and cleanup how Rollbar errors are logged.
  Experimenting with sending more detailed error logs to rollbar.
  Begin refactoring log collection process.
@GUI
Copy link
Member

GUI commented Feb 3, 2015

Thanks! This looks great, and this seems like like a handy feature. Just a couple of quick comments.

  • Can you add a test? That would just help ensure we don't accidentally break this functionality in the future.
  • There may be some very slight performance implications of this change, since all header overrides are now being treated as a handlebars templates (so even if handlebars aren't being used in the header value, it's still being parsed as one). However, you did exactly the right thing with pre-compiling the handlebars template inside the configReload method, so I think this is about as optimized as it's going to get with a handlebars approach (which might be overkill for renaming a header, but it is flexible and also jives with how we've handled templateizing some of the other settings). Anyway, I digress--I'm good with this, and it seems like a solid approach.

Thanks again! And if you don't have time to add a test, or you're not comfortable in the testing code, I'd be happy to add one, but it might be tomorrow before I can do that.

@GUI GUI added this to the v0.7 milestone Feb 3, 2015
@GUI GUI added the enhancement label Feb 3, 2015
@darylrobbins
Copy link
Contributor Author

I will see if I can get a test going tomorrow morning.

I figured it wasn't worth handling templates and non-templates separately. I have to believe Handlebars generates a compiled template for static text that doesn't result in much overhead. I wouldn't be surprised if they have optimizations for this special case.

I guess we'll have to think about how we implement this feature in Lua land as well.

- add test specs
- handle sub-url headers
- only set dynamic header if it evaluates to a value
- accidentally resetting cache settings
@darylrobbins
Copy link
Contributor Author

I have added a reasonably complete test suite for dynamic headers, exercising some of their more advanced capabilities. I have the per API and sub-url cases covered. Are there any others that may not be covered by the api_matcher? (i.e. global headers across all APIs?)

I made a design decision that if the handlebars template evaluates to nothing, the header will not be set. So, this would allow you to set/override a header only if another header had a value for example. For example, use x-forwarded-proto as-is unless x-original-proto has a value, in which case use it. You also have access to use default values through handlebars (example includes in the tests). Given this decision, you would not be able to remove a header though using the dynamic headers. Thoughts?

GUI added a commit that referenced this pull request Feb 3, 2015
@GUI GUI merged commit e962160 into NREL:master Feb 3, 2015
@GUI
Copy link
Member

GUI commented Feb 3, 2015

Fantastic, thanks so much for the pull request and additional tests!

This will be part of the v0.7 package releases, that I meant to get to last week, but will try to definitely get to this week.

@darylrobbins darylrobbins deleted the dynamic-header branch February 3, 2015 14:23
GUI added a commit that referenced this pull request Jul 18, 2015
Configurable system-wide URL redirects
GUI added a commit that referenced this pull request Jul 27, 2015
Simpler query builder interface for constructing admin analytics queries
@sandeeepk
Copy link

I have to add Headers in Global RequestSetting but there is no to add in Analytics Path These are all doing in Api Umberlla

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
3 participants