Add certificates files for docker engine daemon socket TLS #44
Conversation
Add certificates files for docker engine daemon socket TLS for production environment (see https://docs.docker.com/engine/security/https/)
Add certificates files for docker engine daemon socket TLS for production environment (see https://docs.docker.com/engine/security/https/)
|
Add modifications to check_swarm.py script |
|
Thanks for the PR. I am reviewing this change and looking into the test results. I will keep you posted. |
There was a problem hiding this comment.
Hey, finally had a few minutes. Looks good. A couple things I noticed.
- I think we should display a more helpful message if the use gives us non-nonsensical arguments. Otherwise they may me left scratching their heads for a while.
- Some of the tests are broken. I think fixing there is as simple as adding the https handler in the
check_dockerpytest fixtures. - It would be even better to also move the https handler initialization into into a seperate function (taking
argsas a argument). This would ensure we are initializing appropriately. If you are strapped for time I can look into that.
| # Add Certificate files for secure connection: | ||
| if args.secure_connection: | ||
| ctx = ssl.SSLContext(ssl.PROTOCOL_TLSv1_2) | ||
| if args.cafile and args.certfile and args.keyfile: |
There was a problem hiding this comment.
We should display a useful message if an incomplete set of options are specified.
if args.cafile and args.certfile and args.keyfile:
...use handler with context
elif allargs.cafile or args.certfile or args.keyfile:
display error about argument usage "cafile, c, and certfile, and keyfile must be used together"
else:
use default handler
| # Add Certificate files for secure connection: | ||
| if args.secure_connection: | ||
| ctx = ssl.SSLContext(ssl.PROTOCOL_TLSv1_2) | ||
| if args.cafile and args.certfile and args.keyfile: |
|
This (still) works for the latest release. Would like to see it merged to master. |
|
Me too however there are still quality issues that need to be addressed. The tests need to pass and a error message should be added for bad argument usage. I don't have a test environment for this daemon configuration. I would help but I have no good simulation of your environment to use for testing. Perhaps you can update the Here are some hints on how I do testing. https://github.com/timdaman/check_docker/blob/master/DEVLOPEMENT.md |
|
@rbw I could use some. The biggest issue is I don't have a Docker server setup with cert auth. I could try to make one but I figure someone who has something in the field should probably be the person to do that since they know what it should look like in operation. Then there is writing up the tests and polishing the args, I can help with some of that but assistance is most welcome. |
Add certificates files for docker engine daemon socket TLS for production environment (see https://docs.docker.com/engine/security/https/)