Support https with server certificate #423
Conversation
nolanmar511
force-pushed
the
https-fetch
branch
3 times, most recently
from
5005c25 to
f35fea4
Compare
Codecov Report
@@ Coverage Diff @@
## master #423 +/- ##
==========================================
+ Coverage 67.59% 67.68% +0.09%
==========================================
Files 36 37 +1
Lines 7607 7588 -19
==========================================
- Hits 5142 5136 -6
+ Misses 2060 2047 -13
Partials 405 405
Continue to review full report at Codecov.
|
nolanmar511
requested review from
rauls5382 and
aalexand
and removed request for
rauls5382
|
@nolanmar511 FWIW, there is a commit "Merge branch 'master' into https-fetch" in this PR, that's unexpected - the right way to update against the current master is to rebase, not to merge. The commit shows up as empty though, so perhaps it can be just removed or something. |
There was a problem hiding this comment.
I think we should rework the "Fetching profiles" section to organically (i.e. throughout all the text where applicable, not just as a separate paragraph) incorporate the documentation of the support for the profile fetch from TLS-protected server endpoints. Perhaps mention https://docs.docker.com/engine/security/https/ at some point to point to the resemblance of the flag names, server setup and the overall flow.
nolanmar511
force-pushed
the
https-fetch
branch
from
a98742d to
7f28f24
Compare
|
Rebased and updated documentation. |
| @@ -270,6 +270,18 @@ wait for the profile. | |||
| profile over http. If not specified, pprof will use heuristics to determine a | |||
| reasonable timeout. | |||
|
|
|||
| pprof also accepts options which allow a user to specify TLS certificates to | |||
There was a problem hiding this comment.
Could you rework the first paragraph in this section to mention this capability. Also, can we mention https+insecure thing as well? Basically I think the first paragraph needs a bit of holistic update to make it more of an overview with subsequent paragraphs diving into the details.
There was a problem hiding this comment.
I made a very small change to the first paragraph, but I'm having a hard time figuring this documentation -- I think I might not understand http protocols and pprof's use cases well enough right now to make this clear.
There was a problem hiding this comment.
"https+insecure" is not a protocol, it's a hack to disable the server's certificate validation on the client side.
nolanmar511
force-pushed
the
https-fetch
branch
from
7f28f24 to
0ba3779
Compare
nolanmar511
force-pushed
the
https-fetch
branch
from
0ba3779 to
a560aea
Compare
|
PTAL |
aalexand
changed the title
Support https with server certificate.
Fixes #244