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

Fix config reload request url when httpsKeystore in use #514

Merged
merged 4 commits into from
Dec 2, 2021

Conversation

avidspartan1
Copy link
Contributor

@avidspartan1 avidspartan1 commented Dec 2, 2021

What this PR does / why we need it

Previously, the config reload sidecar's request URL pointed to the targetPort, which, when the httpsKeystore is in use, is set to 8443 in our configuration. The sidecar ends up POSTing HTTP->HTTPS, which results in a BadStatusLine error when the sidecar attempts a configuration reload. This PR fixes the REQ_URL environment variable's template to correctly use the defined httpsKeyStore.httpPort when the httpsKeyStore is enabled, like other uses of targetPort do.

Special notes for your reviewer

@torstenwalter @justusbunsi @nikonorovi

Checklist

  • DCO signed
  • Chart Version bumped
  • CHANGELOG.md was updated

Paul Hoisington added 2 commits December 2, 2021 13:25
…bled for config reload request url

Signed-off-by: Paul Hoisington <[email protected]>
Signed-off-by: Paul Hoisington <[email protected]>
@avidspartan1 avidspartan1 requested a review from a team as a code owner December 2, 2021 19:50
Signed-off-by: Paul Hoisington <[email protected]>
Copy link
Member

@justusbunsi justusbunsi left a comment

Choose a reason for hiding this comment

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

I am a bit irritated about my ping, tbh. But like to leave a review. 😀

@avidspartan1
Copy link
Contributor Author

Whoops - I saw your name in a recent PR merge, @justusbunsi, so I added you. Apologies - I won't ping you again!

@justusbunsi
Copy link
Member

No worries. I am happy to contribute if I can. So pings are welcome. It just was unexpected. 😀

@timja timja merged commit b555626 into jenkinsci:main Dec 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants