Bug 5092: improve https_port ssl-bump documentation #1981
Conversation
rousskov
left a comment
rousskov
left a comment
There was a problem hiding this comment.
PR title promises to improve https_port documentation. PR code changes http_port documentation instead.
rousskov
added
the
S-waiting-for-author
kinkie
added
backport-to-v7
rousskov
left a comment
rousskov
left a comment
There was a problem hiding this comment.
PR title promises to improve https_port documentation. PR code changes http_port documentation instead.
The above problem remains unaddressed.
From Bug 5092: I got a log message saying "FATAL: ssl-bump on https_port requires tproxy/intercept which is missing." and "FATAL: tproxy/intercept on https_port requires ssl-bump which is missing.". However, none of these information has been mentioned in [documentation]"
The current PR changes do not address Bug 5092!
rousskov
added
S-waiting-for-author
|
moved text to https_port, made it more generic |
kinkie
added
S-waiting-for-reviewer
Except that FWIW, IMO we should merge the port options down to a "port" directive with one block of documentation for how the listener options interact. Not insisting on that here though, it is a long-term thing. |
| The ssl_bump option is required to fully enable | ||
| bumping of CONNECT requests. |
There was a problem hiding this comment.
Current PR effectively deletes this useful information, from both http_port (where it was explicitly present) and from https_port (where it was present by reference). Instead, this PR adds very generic statements that are true for lots of directives (but are added for one) followed by a very specific error handling description that, I bet, does not yet match actual (varying/inconsistent/questionable) Squid behavior in some of the claimed cases.
PR title promises to improve "https_port ssl-bump" documentation. PR code does not really change that particular documentation.
From Bug 5092: I got a log message saying "FATAL: ssl-bump on https_port requires tproxy/intercept which is missing." and "FATAL: tproxy/intercept on https_port requires ssl-bump which is missing.". However, none of these information has been mentioned in [documentation]"
IMO, current PR changes do not really address Bug 5092. At the risk of saying the obvious, Bug 5092 can be rephrased as follows: https_port documentation does not make it clear that
- https_port supports "intercept" and "tproxy" modes, but only when "ssl-bump" mode is also enabled.
- https_port supports "ssl-bump" mode, but only when either "intercept" or "tproxy" mode is also enabled.
Current PR does imply that some options depend on others and says that some bad option combinations are fatal, but we can safely assume that the target audience, including the bug reporter, already know that general software configuration principle. IMHO, admins want to know primary listening port mode requirements at the planning stage, before they spend time configuring their test clients and network to check their setup. This is not about some kind of obscure, rarely seen option combination. These limitations directly affect how admins design their solutions.
rousskov
added
S-waiting-for-author
3 participants
No description provided.