mweinelt / kea-exporter

Export Kea Metrics in the Prometheus Exposition Format
MIT License
34 stars 17 forks source link

feat: add mTLS #39

Closed M0NsTeRRR closed 8 months ago

M0NsTeRRR commented 9 months ago

This PR adds the possibility to connect to a control agent with mTLS enabled (cert-required). I've also added a missing dependency. I haven't included the option to pass a custom certificate authority since you can use the REQUESTS_CA_BUNDLE environment variable for that purpose. However, I can add it if you think it's not sufficient.

PS: I've pinned the requests dependency using ~= to prevent installation issues in case of a major release. What are your thoughts on using this approach for all dependencies ?

mweinelt commented 8 months ago

Cool!

I haven't included the option to pass a custom certificate authority since you can use the REQUESTS_CA_BUNDLE environment variable for that purpose. However, I can add it if you think it's not sufficient.

I think we can do this step by step. If someone requests it, it won't be a hassle to add it. But also some short explanation in README.md might help.

PS: I've pinned the requests dependency using ~= to prevent installation issues in case of a major release. What are your thoughts on using this approach for all dependencies ?

I think using a semantic pin where possible and keep the dependencies updated is a good mix to satisfy most consumers.

M0NsTeRRR commented 8 months ago

I've added the 2 new flags in the documentation with some lines about https validation. I've updated dependencies to follow semver.

mweinelt commented 8 months ago

You can either squash the "fix: black" commit into the "feat: add mTLS" commit, or I can squash the whole PR. WDYT?

M0NsTeRRR commented 8 months ago

I can but on github you can "Squash and merge" to only have one commit and you can override the commit message here to fit repository best practice.

mweinelt commented 8 months ago

Thank you!