Clarify support for ssl options for modules #7967
Conversation
dedemorton
added
docs
review
needs_backport
|
|
||
| [source,yaml] | ||
| ---- | ||
| - module: prometheus |
There was a problem hiding this comment.
We could use just the http module here to reduce the noise around it but mention it works for all http based modules.
There was a problem hiding this comment.
@ruflin That makes sense. I looked at the docs for the http module, but it looks like we haven't updated the config to show the ssl options: https://www.elastic.co/guide/en/beats/metricbeat/master/metricbeat-module-http.html
Can you provide an example config? I can also update the config.yml if you let me know what I need to add and where.
I guess I should also go through the other modules that have settings: ["ssl"] to make sure that the config examples are updated?
There was a problem hiding this comment.
(After a quick spot check, I can see that there are several modules with the ssl flag that need to have their config examples updated).
There was a problem hiding this comment.
@jsoriano Could you chime in here? I remember you were updating recently quite a few things related to SSL.
There was a problem hiding this comment.
Oh yes, the thing is that we may need two different ssl documentation templates, one for HTTPS and another one for plain TLS, when adding the mongodb TLS support, the template added something about HTTP (I should have reviewed this 🙁 )
There was a problem hiding this comment.
@jsoriano I'm not quite sure how to proceed here. Please advise. Thanks!
There was a problem hiding this comment.
I think there should be two different templates, one for HTTP services and another one for TLS. Services that support HTTPS would use both, services that use TLS over TCP without HTTP would use the TLS one only.
In any case maybe I'm mixing things, I'm refering to the text that is added to modules documentation when they have settings: ["ssl"] in fields.yml, let's merge this and I'll open another PR with a proposal for the other thing.
There was a problem hiding this comment.
PR for the issue with TLS/HTTP doc text in metricbeat modules #8159
There was a problem hiding this comment.
We talked about updating the example config, too. I should do this before we merge. Can someone provide an example?
There was a problem hiding this comment.
I think @ruflin refered to something like this as a generic example here:
- module: http
namespace: "myservice"
enabled: true
period: 10s
hosts: ["https://localhost"]
path: "/stats"
headers:
Authorization: "Bearer test123"
ssl.verification_mode: "none"
Not sure though if it'd be better to have a generic example or a real world example as the one with prometheus and the k8s apiserver.
|
@jsoriano I changed the example to show the http module config. I think this is ready to merge, no? |
dedemorton
removed
the
needs_backport
* Clarify support for ssl options for modules * Change example to show http module
…8478) (#8529) * Clarify support for ssl options for modules (#7967) * Clarify support for ssl options for modules * Change example to show http module * Update Elasticsearch module examples to show http in the URL (#8226) * Improve reference docs that describe how to set options dynamically (#8290) * Improve Elasticsearch output docs about indices, pipelines, and keys settings * Updates from review * Change setting name from mapping to mappings * Remove note to reviewer * Fix conditional coding (#8446) * Suppress dashboard info when dashboards aren't available (#8395) * Clarify add_docker_metadata docs (#8478)
* Clarify support for ssl options for modules * Change example to show http module
…8478) (#8528) * Clarify support for ssl options for modules (#7967) * Clarify support for ssl options for modules * Change example to show http module * Update Elasticsearch module examples to show http in the URL (#8226) * Improve reference docs that describe how to set options dynamically (#8290) * Improve Elasticsearch output docs about indices, pipelines, and keys settings * Updates from review * Change setting name from mapping to mappings * Remove note to reviewer * Fix conditional coding (#8446) * Suppress dashboard info when dashboards aren't available (#8395) * Clarify add_docker_metadata docs (#8478)
…tic#8290 elastic#8395 elastic#8446 elastic#8478) (elastic#8528) * Clarify support for ssl options for modules (elastic#7967) * Clarify support for ssl options for modules * Change example to show http module * Update Elasticsearch module examples to show http in the URL (elastic#8226) * Improve reference docs that describe how to set options dynamically (elastic#8290) * Improve Elasticsearch output docs about indices, pipelines, and keys settings * Updates from review * Change setting name from mapping to mappings * Remove note to reviewer * Fix conditional coding (elastic#8446) * Suppress dashboard info when dashboards aren't available (elastic#8395) * Clarify add_docker_metadata docs (elastic#8478)
This PR fills in some missing documentation to resolve #3948.
I used the example from the original PR without testing it (avoids eye contact), but maybe another example--perhaps from the reference.yml file--would be better. Please advise.
With these other commits/PRs, I think we can close #3948, right?