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

Forward headers from webhook filter responses #1261

Merged

Conversation

dlmiddlecote
Copy link
Contributor

@dlmiddlecote dlmiddlecote commented Dec 30, 2019

Update the webhook filter so that it can take a second argument, a comma-separated list of header names, that should be copied from the webhook response to the continuing request, if the webhook response is authorized.

i.e. webhook("auth-endpoint", "Headers-To-Copy,And-More")

Signed-off-by: Daniel Middlecote <dlmiddlecote@gmail.com>
filter

Signed-off-by: Daniel Middlecote <dlmiddlecote@gmail.com>
response

Signed-off-by: Daniel Middlecote <dlmiddlecote@gmail.com>
@dlmiddlecote dlmiddlecote force-pushed the feature/webhook-auth-forward-headers branch from e9ff5e9 to ff37704 Compare Dec 30, 2019
@dlmiddlecote
Copy link
Contributor Author

@dlmiddlecote dlmiddlecote commented Dec 30, 2019

Not sure why the tests failed, can't reproduce locally.

@szuecs
Copy link
Member

@szuecs szuecs commented Dec 31, 2019

From the docs and or message, I see that it’s a breaking change.
Is it a breaking change?
If so please do it non breaking.
I did not read the code yet, I can probably do it on Friday.

Travis errors are most likely not your problem and needs a retrigger in Travis.

@dlmiddlecote
Copy link
Contributor Author

@dlmiddlecote dlmiddlecote commented Dec 31, 2019

Hey @szuecs, this shouldn't be a breaking change, as the second argument to the webhook filter doesn't need to be specified, and if it's not, the filter will work as it does today. I guess the documentation doesn't convey that well enough.

@szuecs
Copy link
Member

@szuecs szuecs commented Dec 31, 2019

Can you please have both as examples, such that it’s clear in the docs?
Thanks!

…tional

Signed-off-by: Daniel Middlecote <dlmiddlecote@gmail.com>
@szuecs
Copy link
Member

@szuecs szuecs commented Jan 6, 2020

👍

1 similar comment
@aryszka
Copy link
Contributor

@aryszka aryszka commented Jan 7, 2020

👍

@aryszka aryszka merged commit 26ea79f into zalando:master Jan 7, 2020
4 checks passed
@dlmiddlecote dlmiddlecote deleted the feature/webhook-auth-forward-headers branch Jan 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants