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

feat: Adding support to the interceptor proxy for TLS on the wire #928

Merged
merged 3 commits into from
May 1, 2024

Conversation

zorocloud
Copy link
Contributor

@zorocloud zorocloud commented Feb 20, 2024

Adding configurable TLS support to the interceptor proxy. Resolves #907

Checklist

@zorocloud zorocloud requested a review from a team as a code owner February 20, 2024 16:11
Copy link
Member

@JorTurFer JorTurFer left a comment

Choose a reason for hiding this comment

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

I like this feature, could you add an e2e test for this? I think that covering this will prevent problems in the future xD

pkg/http/server.go Show resolved Hide resolved
Copy link
Member

@JorTurFer JorTurFer left a comment

Choose a reason for hiding this comment

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

Could you add an e2e test case for this scenario @zorocloud?
We should update docs as well for explaining how to configure this

pkg/http/server.go Show resolved Hide resolved
@zorocloud
Copy link
Contributor Author

Could you add an e2e test case for this scenario @zorocloud? We should update docs as well for explaining how to configure this

Yeah no problem @JorTurFer. Planning to add them in once I get my other PR tied off. Just wanted to get the initial code up for feedback on the implementation :)

@JorTurFer
Copy link
Member

Yeah no problem @JorTurFer. Planning to add them in once I get my other PR tied off. Just wanted to get the initial code up for feedback on the implementation :)

sure! no rush, your other PR will be merged today (sorry, I've been on a trip this week)

@JorTurFer
Copy link
Member

Hello @zorocloud
We plan to cut a release (v0.8.0) during this week or the next one. By change will you have time to finish the PR before? Don't feel rush or pressure, we can deploy the release a bit, or follow the plan and cut another release (v0.8.1) with this feature and others new, we don't have a super strict release plans for the add-on.

@zorocloud
Copy link
Contributor Author

Hey @JorTurFer, I will try and get it tied off this week, but I'm happy for it to go into a follow up release if it's not done in time. May as well stick with the current plan :)

@JorTurFer
Copy link
Member

JorTurFer commented Apr 30, 2024

Hey @JorTurFer, I will try and get it tied off this week, but I'm happy for it to go into a follow up release if it's not done in time. May as well stick with the current plan :)

Well... we don't have any defined roadmap yet for the add-on, so given that you're working on this and I assume that you need the feature, we can cut a release v0.8.1 once it's the feature merged. Would it help?
We don't need to be in hurry and we can cut the release once the feature is ready, but we can ship current features. WDYT?
@tomkerkhove , do you have any objections?

@zorocloud
Copy link
Contributor Author

Hey @JorTurFer, I will try and get it tied off this week, but I'm happy for it to go into a follow up release if it's not done in time. May as well stick with the current plan :)

Well... we don't have any defined roadmap yet for the add-on, so given that you're working on this and I assume that you need the feature, we can cut a release v0.8.1 once it's the feature merged. Would it help? We don't need to be in hurry and we can cut the release once the feature is ready, but we can ship current features. WDYT? @tomkerkhove , do you have any objections?

Works for me 👍

@zorocloud
Copy link
Contributor Author

Couldn't bind 443 in the tests so just pushed a small update that should fix that. Think the tests should be good now and the PR ready for re-review whenever you have time :)

@JorTurFer
Copy link
Member

Please fix the DCO:
image
You can click on Details and get the commands to execute

@zorocloud
Copy link
Contributor Author

Please fix the DCO: image You can click on Details and get the commands to execute

Ah forgot to add that to my last commit. I've amended the commit message so we should be sweet now.

@JorTurFer
Copy link
Member

Nice, I'm going to check it now :)

Copy link
Member

@JorTurFer JorTurFer left a comment

Choose a reason for hiding this comment

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

AWESOME JOB! ❤️

@JorTurFer JorTurFer merged commit 38f50bf into kedacore:main May 1, 2024
19 checks passed
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.

Interceptor Proxy - TLS On The Wire
3 participants