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

Create Header Middleware #1236

Merged
merged 1 commit into from
Jun 13, 2017
Merged

Create Header Middleware #1236

merged 1 commit into from
Jun 13, 2017

Conversation

dtomcej
Copy link
Contributor

@dtomcej dtomcej commented Mar 4, 2017

This is a middleware based on github.com/unrolled/secure that allows us to control, and inject request and response headers.

@dtomcej
Copy link
Contributor Author

dtomcej commented Mar 4, 2017

This pull request solves/enables/improves the following tickets:

#1220
#1155
#669
#160
#812

}

// Loop through Custom request headers
if len(s.opt.CustomRequestHeaders) > 0 {
Copy link
Contributor

Choose a reason for hiding this comment

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

You don't need the length check if you for-loop over its content (note that a nil map behaves like an empty map when reading).

Same for the custom response headers below.

@emilevauge
Copy link
Member

@dtomcej Couldn't we reuse https://github.com/unrolled/secure instead of base our code on it ?

@dtomcej
Copy link
Contributor Author

dtomcej commented Mar 7, 2017

@emilevauge I ran into a couple issues:

  1. unrolled/secure utilizes a more simplistic implementation of the middleware. I would love to implement it, but would need to extend it to provide more functionality.

  2. I need to implement it as route-specific middleware, since I don't want all the middleware instances to run on each request. I want specific instances to run on their associated requests.

I have implemented a bit of code on line 717 of server.go, but would like some direction.

I want to be able to have the headers configurable through the frontend objects. This would allow us to dynamically change them, and specify headers (both request and response) per route.

If I can get part 2 working, I can then solve part 1 by wrapping it in an extension object. How would I go about with part 2?

@dtomcej dtomcej mentioned this pull request Mar 10, 2017
@timoreimann timoreimann mentioned this pull request Mar 22, 2017
@ulm0
Copy link
Contributor

ulm0 commented Mar 23, 2017

just curious, is this implemented already?

@dtomcej
Copy link
Contributor Author

dtomcej commented Mar 23, 2017

Sorry, I have been super busy. Hoping to resume work on this in the next few days.

@kaiyou
Copy link

kaiyou commented Apr 9, 2017

After running go-fmt, all tests are passing. You can find a Docker image with this branch merged and STSSeconds set to one year at tedomum/traefik:hsts, currently testing on a server with some traffic to provide useful feedback.

@dtomcej
Copy link
Contributor Author

dtomcej commented Apr 10, 2017

That is good to hear @kaiyou. I am looking to get back in on this ticket this week.

Just welcomed a son into our family last week, so its been a couple of busy weeks.

I am going to be wrapping another module in this middleware to prevent code duplication.
Should provide a lot more flexibility.

@ldez
Copy link
Contributor

ldez commented Apr 24, 2017

@dtomcej do you the time to update this PR before the upcoming feature freeze for the 1.3 release?

@ldez ldez added the priority/P1 need to be fixed in next release label Apr 25, 2017
@ldez ldez mentioned this pull request Apr 29, 2017
@Starefossen
Copy link
Contributor

Nice @dtomcej, if we could get this going in v1.3 that would be even better 👍

@ldez ldez modified the milestone: 1.4 May 10, 2017
@ldez ldez added area/middleware status/2-needs-review and removed priority/P1 need to be fixed in next release labels May 10, 2017
@ldez
Copy link
Contributor

ldez commented May 15, 2017

Due to SemaphoreCI, I close and reopen the PR.

@ldez ldez closed this May 15, 2017
@ldez ldez reopened this May 15, 2017
@emilevauge
Copy link
Member

@dtomcej as soon as you are ready, we can move on this one :)

types/types.go Outdated Show resolved Hide resolved
Copy link
Member

@emilevauge emilevauge left a comment

Choose a reason for hiding this comment

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

Awesome job @dtomcej 👏 👏
LGTM (once glide.lock issue fixed)

@ldez ldez merged commit f275e4a into traefik:master Jun 13, 2017
@jgsqware
Copy link

jgsqware commented Jun 13, 2017 via email

@andrewmclagan
Copy link

Whats the release schedule for these new merges?

@emilevauge
Copy link
Member

The 1.4 should be released in August.

@oszi
Copy link

oszi commented Jul 11, 2017

Will something like this work with docker labels?
traefik.{id}.frontend.headers.customresponseheaders.Access-Control-Allow-Origin = '*'
Or do we need to extend templates/docker.tmpl with headers?

@ldez
Copy link
Contributor

ldez commented Jul 11, 2017

@oszi Thanks for your interest in Traefik 😃

This PR is closed. Please discuss this in :

@vanthome
Copy link

vanthome commented Sep 1, 2017

Is the use of this documented somewhere?

@ldez
Copy link
Contributor

ldez commented Sep 1, 2017

@jcassee
Copy link
Contributor

jcassee commented Sep 1, 2017

It was not directly obvious to me, but the documentation for those security headers can be found in the repository of the security middleware.

@hcaz
Copy link

hcaz commented Nov 13, 2017

Hi, is there a way to remove response headers that have been added by a backend?

Similar to nginx proxy_hide_header

@mmatur
Copy link
Member

mmatur commented Nov 13, 2017

Hi @hcaz,
There is no way to remove a specific header.
You are able to add custom response header only. https://docs.traefik.io/basics/#custom-headers

@hcaz
Copy link

hcaz commented Nov 13, 2017

@mmatur Thanks for the reply!

I know I can add custom headers, I am currently doing that, but for example if I add the server header in traefik, and the backend already sends a server header then the client receives two server headers.

Do you know if there are any plans to support removing of headers?

@mmatur
Copy link
Member

mmatur commented Nov 20, 2017

@hcaz Sorry for the delay. It is plan for the next Træfik version and we will fix the duplicated headers too.

@hcaz
Copy link

hcaz commented Nov 20, 2017

@mmatur Awesome, thank you! :)

@traefik traefik locked and limited conversation to collaborators Nov 20, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area/middleware kind/enhancement a new or improved feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.