Implement support for TLS/SSL on the HTTP interface #1040
Conversation
tinaselenge
force-pushed
the
ssl-support
branch
3 times, most recently
from
86bf088 to
01ad50c
Compare
tinaselenge
force-pushed
the
ssl-support
branch
10 times, most recently
from
f365d72 to
4e21120
Compare
tinaselenge
force-pushed
the
ssl-support
branch
2 times, most recently
from
9a7258c to
4bf2f2d
Compare
see-quick
left a comment
see-quick
left a comment
There was a problem hiding this comment.
Thanks for the PR 👍 . Looks good to me from the testing perspective, just a few nits to consider.
Signed-off-by: Gantigmaa Selenge <[email protected]>
Signed-off-by: Gantigmaa Selenge <[email protected]>
Signed-off-by: Gantigmaa Selenge <[email protected]>
tinaselenge
force-pushed
the
ssl-support
branch
3 times, most recently
from
a266737 to
f51569c
Compare
Signed-off-by: Gantigmaa Selenge <[email protected]>
tinaselenge
force-pushed
the
ssl-support
branch
from
f51569c to
4795f7b
Compare
see-quick
left a comment
see-quick
left a comment
There was a problem hiding this comment.
LGTM from testing POV. Thanks 👍
Reduce verbosity of RouterBuilderImpl warnings for unimplemented OpenAPI endpoints Add logging for missing required SSL configuration Signed-off-by: Gantigmaa Selenge <[email protected]>
Signed-off-by: Gantigmaa Selenge <[email protected]>
ppatierno
left a comment
ppatierno
left a comment
There was a problem hiding this comment.
I was going to approve (even with some nits) but then I read the comment about the Azure VMs and the binding port issue. I would like to have a quick chat on the community call today about that to get other opinions.
Signed-off-by: Gantigmaa Selenge <[email protected]>
|
Triaged on 27.11.2025: agreed that the default TLS port should be 8443 instead of 443 as we are using HTTP port default 8080, so 8443 makes much more sense. No need for any changes in the proposal. |
Signed-off-by: Gantigmaa Selenge <[email protected]>
|
Thanks so much for the review! @ppatierno @see-quick |
|
@katheris as agreed during the last community call, can you review this one when you have some time please? Thanks! |
katheris
left a comment
katheris
left a comment
There was a problem hiding this comment.
Thanks @tinaselenge, generally looks good but I suggested a few changes
Signed-off-by: Gantigmaa Selenge <[email protected]>
|
Thanks @katheris for the comments. I have now addressed them and also added a couple of unit test cases for HTTP configs. |
|
Thanks @tinaselenge the new changes look good, but I'll hold off approving until we get consensus on the logging discussion |
5 participants
Implements the proposal strimzi/proposals#171.
In the proposal we agreed to default port 443, however, after further discussion, it was decided to change it to 8443. This change makes more sense as the default port for non SSL server is 8080 and most Linux systems, by default, do not allow non-root users to bind to ports below 1024.
Closes #939