search

sensu-plugins / sensu-plugins-docker

This plugin provides native Docker instrumentation for monitoring and metrics collection, including: container status, container number, and container metrics via `docker ps`.
http://sensu-plugins.io
MIT License
35 stars 57 forks source link

Very simple TLS support #33

Closed DavidNorman closed 7 years ago

DavidNorman commented 8 years ago

Pull Request Checklist

Is this in reference to an existing issue? Yes - #24

General

This adds a simple TLS certs support for when the user has separate cert/key files.

Known Compatablity Issues

cwjohnston commented 8 years ago

Hi @DavidNorman, thanks for the pull request! I think implementing TLS support is a very good idea. I have a couple thoughts on your proposed changes:

First, I think it would be more user-friendly and consistent with other plugins if we were to use command line options instead of environment variables for enabling this behavior.

Secondly, I think its important that we support specifying the TLS verification mode via a command line flag as well. If we merge the feature without it, I'm sure support for this will be requested soon after.

Will you please consider the above suggestions? Thanks!

eheydrick commented 7 years ago

@DavidNorman any thoughts on @cwjohnston's feedback? Would be nice to have TLS support.

DavidNorman commented 7 years ago

Hi. I have moved on to another (completely different) project. Feel free to cherry pick some of the ideas here, but I have no longer the time to contribute, sorry.

majormoses commented 7 years ago

closing per: https://github.com/sensu-plugins/sensu-plugins-docker/pull/33#issuecomment-279967369