-
Notifications
You must be signed in to change notification settings - Fork 230
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
Move x-ditto-pre-authenticated and X-Forwarded-User to ingress.api.annotations #1787
Move x-ditto-pre-authenticated and X-Forwarded-User to ingress.api.annotations #1787
Conversation
…roller to values.yaml config Signed-off-by: Dominik Mlasko <[email protected]>
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.
Thanks for the contribution, @mladBlum
Please update the Chart version in Chart.yaml to the next micro-version.
Signed-off-by: Dominik Mlasko <[email protected]>
@mladBlum on "master" the version is already on |
Sorry, that's on me, I should have checked it myself. |
* update kubernetes versions to test against Signed-off-by: Thomas Jäckle <[email protected]>
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.
LGTM - thanks a lot @mladBlum
I hope you verified that this works as you need it to .. ;)
I had to bump the version of helm/chart-testing-action
- otherwise the chart linting was failing.
Hey @mladBlum, could you explain what were you trying to achieve with this change? For us it broke the Ditto authentication as the pre-authenticated header is now not reaching the gateway pod. I am planning to make a PR that would revert this, but maybe there is an opportunity to achieve your goals and our goals at the same time? 😁 Oh, I see #1778. Hm, maybe there could be another way how to do it. |
Oh, ouch .. really surprising to me how many of you don't use OAuth based authentication, but rely on that pre-authentication .. |
Would be nice if we could solve this issue together. The same text you wrote applied for us before. I just placed the ditto headers into the ingress so that every ingress controller is working. I am really curious about the cause of your problem because this should be a more generic solution. What is your setup? |
Yes, we do. I am guessing you are not using the ingress controller included with the chart. For us, what I see is that The included ingress controller is In my perspective, the ConfigMap is the best place for this, so I will provide a rework of the Helm deployment that places these headers in the |
Ok, that's a little bit strange. Please add me to the merge-request with the rework so i can check the changes. |
resolves #1778