Skip to content
This repository has been archived by the owner on Dec 5, 2022. It is now read-only.

Configurable header forwarding for fragment requests #97

Closed
jmaicher opened this issue Jan 5, 2017 · 7 comments · Fixed by #110
Closed

Configurable header forwarding for fragment requests #97

jmaicher opened this issue Jan 5, 2017 · 7 comments · Fixed by #110

Comments

@jmaicher
Copy link

jmaicher commented Jan 5, 2017

The forwarded headers are currently hard-coded in https://github.com/zalando/tailor/blob/master/lib/filter-headers.js#L16.

This could be configurable via Tailor(options) with sane defaults. My particular use case is that tailor runs in a private, trusted network where headers are used for authentication.

I'd be happy to contribute it myself if considered useful.

@lmineiro
Copy link
Contributor

lmineiro commented Jan 5, 2017

This would be a great contribution. Thanks in advance.

@vigneshshanmugam
Copy link
Collaborator

vigneshshanmugam commented Jan 5, 2017

@jmaicher Thanks for opening the issue.

This could be configurable via Tailor(options) with sane defaults. My particular use case is that tailor runs in a private, trusted network where headers are used for authentication.

The only problem with adding the headers in the options list is that

  • It will end in a huge list if in future fragments starts understanding CSP and other security related headers.
  • You cant conditionally add/remove header based on GET/POST or any other logic.

The other option which I can think of is requestFragment starts accepting the filterHeader function as part of a argument and you could tune the header function as per your needs.

I am open to discussion. Let me know your thoughts.
/cc @lmineiro @grassator

@grassator
Copy link
Contributor

I would go for accepting filterHeader function, the only thing is that I don't like having 4 params to that function, so I would add a check for the type of the first argument and support passing object with those props instead.

@vigneshshanmugam
Copy link
Collaborator

@jmaicher thoughts?

@lmineiro
Copy link
Contributor

lmineiro commented Jan 9, 2017

What about building the filtered request before calling requestFragment?
This would keep that function even cleaner, leaving to whatever happens between receiving the edge request and calling the fragment the required transformation/header filtering.
Composition and testing also seems simpler, but I admit my lack of knowledge of this particular "ecosystem"

@jmaicher
Copy link
Author

jmaicher commented Jan 9, 2017

@vigneshshanmugam I like the general idea of a filterHeader function, but I'm not sure yet what arguments it should get?! Where do I get the request method from?

@grassator 👍 on keeping the interface clean. Regarding the first argument check, is your intention to avoid breaking changes in the public API? If not, I'd rather go with just an argument object to keep the code cleaner.

@lmineiro We also need to consider that requestFragment is already configurable from the outside, giving users full control over how requests are constructed and dispatched. Of course, we could think about splitting this concern into a factory and dispatch function, but I'm not sure it's worth it.

@vigneshshanmugam
Copy link
Collaborator

@jmaicher Please have a look at the PR #110

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

4 participants