Skip to content
This repository has been archived by the owner on Jul 2, 2020. It is now read-only.

Substitute headers #46

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

PilchardFriendly
Copy link

There are some cases where the upstream client would like to control which headers are sent to the downstream servers.

An example of this is where prior authentication has occurred, and so unique Authorisation headers must be sent separately to primary, secondary and candidate servers.

In the PR, I've designed it so that, for HTTP(S) protocols only, headers will be rewritten (or dropped) if they are prefixed by "primary_", "secondary_", "candidate_".

For instance, the header "primary_Authorization: Bearer ajdskhasqqjkh19qw1h2" will be sent to only primary, as "Authorization: Bearer ajdskhasqqjkh19qw1h2".

This maintains diffy's stateless communications, but supports testing through Postman, etc.

@PilchardFriendly
Copy link
Author

Travis CI seems to have a borked setup - builds pass on 4/6 environments.

@PilchardFriendly
Copy link
Author

Looks like something wrong with java 1.7.0_91 - this has been EOLd on 25 Feb 2015. Is it still necessary? If so - we can have a look at what is causing Travis to coredump.

inclusion <- inclusions if name.startsWith(inclusion + "_")
} yield name->name.substring(inclusion.length+1)

incomingHeaders.filter( removeExclusion).foreach( headers.remove )
Copy link
Contributor

@puneetkhanduri puneetkhanduri Jul 8, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can ignore this but I am leaving it in to share my thought process through the review.
Consider making this slightly procedural and reducing the total lines of code:

incomingHeaders foreach { name =>
  val shouldExclude = exclusion exists { ex => name.startsWith(s"$ex_") }
  if (shouldExclude) {
    headers.remove(name)
  }

  val shouldInclude = inclusion exists { in => name.startsWith(s"$in_") }
  if (shouldInclude) {
    renameHeaders(headers)(name, name.substring(inclusion.length + 1))
  }
}

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My reflections - Since the header set is mutable, we should apply all the changes in a single operation.
this makes sense. It's counter to splitting the filters, unless we just compose aggregates of headerEffects.

@codecov-io
Copy link

Current coverage is 37.51%

Merging #46 into master will increase coverage by 1.01%

@@             master        #46   diff @@
==========================================
  Files            29         31     +2   
  Lines           789        821    +32   
  Methods         772        801    +29   
  Messages          0          0          
  Branches         17         20     +3   
==========================================
+ Hits            288        308    +20   
- Misses          501        513    +12   
  Partials          0          0          

Powered by Codecov. Last updated by 8586972...625182d

@CLAassistant
Copy link

CLAassistant commented Jul 18, 2019

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

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

Successfully merging this pull request may close these issues.

4 participants