fix(headers): avoid retrofit and okhttp adding duplicate headers #1114
Conversation
|
For some requests the duplicate headers break Fiat auth calls which show up like the following, however I am unable to determine why some requests break and not others: |
|
I suspect you're right about this. Would you mind adding a test that demonstrates it? |
dmart
force-pushed
the
duplicate-headers
branch
4 times, most recently
from
8a681f5 to
1fe7107
Compare
...-retrofit/src/test/kotlin/com/netflix/spinnaker/kork/retrofit/RetrofitServiceProviderTest.kt
Outdated
Show resolved
Hide resolved
...-retrofit/src/test/kotlin/com/netflix/spinnaker/kork/retrofit/RetrofitServiceProviderTest.kt
Outdated
Show resolved
Hide resolved
| import com.netflix.spinnaker.security.AuthenticatedRequest | ||
| import retrofit.RequestInterceptor | ||
|
|
||
| class SpinnakerRequestInterceptor implements RequestInterceptor { |
There was a problem hiding this comment.
Comments like this one in MdcCopyingAsyncTaskExecutor could use a tweak now that SpinnakerRequestHeaderInterceptor is doing the copying.
I also see some other places that use SpinnakerRequestInterceptor that are going to need adjusting:
- echo: IgorConfig, Front50Config
- fiat
- orca
Can you make draft PRs to resolve those, or...maybe we need some more discussion about how to resolve them?
There was a problem hiding this comment.
Maybe removing the bean entirely is not the way to go, perhaps marking it deprecated and simply having intercept noop so we don't break other projects? I don't love either solution.
There was a problem hiding this comment.
Yeah, I don't love that either. I'd love to at least explore what it would be like to remove it. Or at least what the path is to stop using it.
dmart
force-pushed
the
duplicate-headers
branch
from
d952d69 to
9025bbd
Compare
With the addition of the okHttp3 request interceptor `SpinnakerRequestHeaderInterceptor` in spinnaker#1004 having retrofit1 add `X-SPINNAKER-*` headers is now redundant, this results in the headers being added twice to all retrofit1 client requests.
dmart
force-pushed
the
duplicate-headers
branch
from
e097f5f to
e44aa56
Compare
With the addition of the okHttp3 request interceptor
SpinnakerRequestHeaderInterceptorin #1004 having retrofit1 addX-SPINNAKER-*headers is now redundant, this results in the headers being added twice to all retrofit1 client requests.