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.

Sign up for GitHub

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

Jump to bottom

#1883 return pre-authenticated configuration to location-snippet #1834

Merged
merged 2 commits into from
Dec 2, 2023
Merged

#1883 return pre-authenticated configuration to location-snippet #1834

merged 2 commits into from
Dec 2, 2023

Conversation

RockyMM
Copy link
Contributor

@RockyMM RockyMM commented Dec 1, 2023

Necessary for the Ingress-Nginx Controller. Fixes #1833

@RockyMM
eclipse-ditto#1883 return pre-authenticated configuration to location…
c74b43f 
…-snippet

Necessary for the Ingress-Nginx Controller
@RockyMM
Copy link
Contributor Author

RockyMM commented Dec 1, 2023

Hm, should I increase the chart version?

@RockyMM
Copy link
Contributor Author

RockyMM commented Dec 1, 2023

@mladBlum take a look, I think this change should not affect you.

@RockyMM RockyMM closed this Dec 1, 2023
@RockyMM RockyMM reopened this Dec 1, 2023
@RockyMM
Copy link
Contributor Author

RockyMM commented Dec 1, 2023 •
edited
Loading

Hm, should I increase the chart version?

The next semver should be 3.4.2-1 3.4.3-0. I kept the appVersion at 3.4.2. Maybe there is a way to only release the Helm chart? @thjaeckle, what do you think?

@thjaeckle
Copy link
Member

@RockyMM do you know if we can use version 3.4.2.1 in Helm? The forth digit would be nice in this case ..

@thjaeckle
Copy link
Member

@RockyMM do you know if we can use version 3.4.2.1 in Helm? The forth digit would be nice in this case ..

Probably not .. so 3.4.3-0 would be fine for me (if that passes linting) - as a "pre-release of 3.4.3".
Otherwise, just bump it to 3.4.3 - then Chart and appVersion are no longer in sync until the next minor release 3.5.0.

@RockyMM
Updating chart version
f30c829 
But keeping the app version
@RockyMM RockyMM force-pushed the pre-authenticated-proxy-headers-compromise branch from 3a5b8e1 to f30c829 Compare December 1, 2023 17:55
@thjaeckle
Copy link
Member

@kalinkostashki could you review? I know you also were looking into solutions..

kalinkostashki
kalinkostashki approved these changes Dec 2, 2023
View reviewed changes
Copy link
Contributor

@kalinkostashki kalinkostashki left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@thjaeckle thjaeckle merged commit d615bbf into eclipse-ditto:master Dec 2, 2023
9 checks passed
@RockyMM RockyMM deleted the pre-authenticated-proxy-headers-compromise branch December 4, 2023 09:04
@thjaeckle thjaeckle added this to the 3.4.3 milestone Dec 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kalinkostashki kalinkostashki kalinkostashki approved these changes

@thjaeckle thjaeckle thjaeckle approved these changes

Assignees

@RockyMM RockyMM

Labels
bug Helm chart
Projects
Ditto Planning
Status: Done
Milestone
3.4.3
Development

Successfully merging this pull request may close these issues.

Regression: When using BASIC auth the pre-authenticated header is not forwarded to gateway pod
3 participants
@RockyMM @thjaeckle @kalinkostashki