-
Notifications
You must be signed in to change notification settings - Fork 186
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
sso-proxy: refactor reverse proxy into separate file w/ tests #188
Conversation
fb84b74
to
5af96bd
Compare
9523d52
to
09df2b1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great @jphines!
09df2b1
to
75db6a3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this lgtm!
internal/proxy/reverse_proxy.go
Outdated
} | ||
|
||
if config.FlushInterval != 0 { | ||
reverseProxy.FlushInterval = config.FlushInterval |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we have a default reverse proxy flush interval?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't. 0
specifies no periodic flushing according to https://golang.org/pkg/net/http/httputil/ which is fine default behavior imho
internal/proxy/reverse_proxy.go
Outdated
return handler, nil | ||
} | ||
|
||
// Director implements the Director func providerd in the httputil reverse proxy. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
providerd -> provided
internal/proxy/reverse_proxy.go
Outdated
} | ||
|
||
// Director implements the Director func providerd in the httputil reverse proxy. | ||
// This implements a variety fo director behavior based on configuration defined |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fo -> of
fb05af4
to
4d788a6
Compare
Problem
Refactor reverse proxy and upstream into separate file w/ tests. Cleans up some of the reverse proxy logic.
This should make it easier to test reverse proxy specific features/changes in the future and help isolate the reverse proxy from the authentication/authorization portions of sso.
A split out from #187