Remove tls conditions from source code #320
Conversation
Signed-off-by: Mykola Morhun <mmorhun@redhat.com>
mmorhun
requested review from
davidfestal,
tomgeorge,
sleshchenko and
AndrienkoAleksandr
mmorhun
requested review from
l0rd,
nickboldt,
tolusha and
vparfonov
as code owners
|
How long ago did we announce this was deprecated? @l0rd isn't the policy something like 3 sprints after deprecation, THEN we can remove API? I'm all for removing cruft but do we need to at least WARN people since this DOES affect users and customers? |
I agree with @nickboldt. And I have to disagree with myself of just a couple of weeks ago. I mean removing unsecure HTTP support is NOT removing a fully working API but an API that got (partially) broken 6 months ago, so that's not a big concern but...we agreed that we would remove unsecure HTTP support only after we had improved self-signed certificates scenarios UX and even if eclipse-che/che#16764 got fixed:
In other words a broken, unsecure Che is still better than no-Che at all. Let's wait 3 sprints to get some feedback on automatic TLS configuration and eclipse-che/che#12914 resolution. In the meantime we can start warning the community on che-dev mailing list. @mmorhun @tolusha are you ok with that? That means putting this work on hold and can be frustrating but it looks the better thing to do right now. |
|
@l0rd But what I don't understand what is the difference between |
@tolusha that was because @nickboldt was asking: "isn't the policy something like 3 sprints after deprecation, THEN we can remove API?" (the "API" that we want to remove is the no-TLS option). And my reply was that is true, we should first deprecate, wait and then remove, but in this case the situation is slightly different because the no-TLS "API" is broken since 6 months (i.e. Che Theia webview doesn't work). |
|
@mmorhun: PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
@mmorhun: The following tests failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
|
@mmorhun: The following test failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
|
@mmorhun: PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
@mmorhun: The following tests failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
|
PR is heavily outdated. |
Signed-off-by: Mykola Morhun mmorhun@redhat.com
What does this PR do?
Removes tls support flag from source code which makes it impossible to deploy Eclipse Che without TLS any more.
tlsSupportfields is still present in Che custom resource, however it is only for backward compatibility and the flag has no effect any more.What issues does this PR fix or reference?
eclipse-che/che#17090
Tests
Testing was done on CRC with the folowing scenario:
chectl cacert:exporthelpful here)