ansible-collections / community.grafana

Grafana Collection for Ansible
http://galaxy.ansible.com/community/grafana
GNU General Public License v3.0
123 stars 78 forks source link

fix(lookup/grafana_dashboard): add custom certs verification logic #356

Closed shantanoo-desai closed 3 months ago

shantanoo-desai commented 3 months ago
SUMMARY

This fix adds the validate_certs, ca_path options to the lookup plugin. Both parameters comply with the get_url functionality of ansible-core and provides additional utility to perform lookup of dashboards from Grafana instances that are configured with Self-Signed Certificates.

validate_certs option value defaults to true - following the pattern of url lookup plugin from the Core.

ca_path option value is set explicitly when using the plugin else defaults to None.

Fixes #346

ISSUE TYPE
COMPONENT NAME

Lookup Plugin for Grafana Dashboards

ADDITIONAL INFORMATION

Tested with a Grafana Docker image with Self-Signed Certificates as well as a local Certificate Authority. The Local CA along with a self-signed certificate is used to form a chain.crt and validated as ca_path option for the plugin.

shantanoo-desai commented 3 months ago

/cc @rndmh3ro

codecov[bot] commented 3 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 65.18%. Comparing base (4d0af1a) to head (fefb2e6).

:exclamation: Current head fefb2e6 differs from pull request most recent head 2fcf0c2. Consider uploading reports for the commit 2fcf0c2 to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #356 +/- ## =========================================== + Coverage 23.42% 65.18% +41.76% =========================================== Files 15 9 -6 Lines 1601 968 -633 Branches 336 133 -203 =========================================== + Hits 375 631 +256 + Misses 1219 309 -910 - Partials 7 28 +21 ``` | [Flag](https://app.codecov.io/gh/ansible-collections/community.grafana/pull/356/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ansible-collections) | Coverage Δ | | |---|---|---| | [sanity](https://app.codecov.io/gh/ansible-collections/community.grafana/pull/356/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ansible-collections) | `?` | | | [units](https://app.codecov.io/gh/ansible-collections/community.grafana/pull/356/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ansible-collections) | `65.18% <ø> (?)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ansible-collections#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

shantanoo-desai commented 3 months ago

@Nemental One of the CI Sanity check for development fails for a test file which doesn't belong to my PR. The other Lint problem will be solved by me now. I am not sure why the particular test file for Sanity Develop Job fails?

Edit: also I used the wrong number for the fragments file (used issue number instead of PR number)

Tasks

Nemental commented 3 months ago

Hey @shantanoo-desai Thanks for your PR! I'll do a final review of the changes tomorrow, but so far it looks pretty good. The remaining sanity failings have already been fixed in another PR.

shantanoo-desai commented 3 months ago

Update

the two Integration Tests seem to fail with HTTP 429 Too Many Requests on the Grafana Endpoint when trying to retrieve some dashboards. I am assuming the tests would just need to be re-run and nothing from my patch should actually break the tests.

shantanoo-desai commented 3 months ago

@Nemental and @rndmh3ro The PR is rebased to main branch and the changes have been made. The Sanity tests still seem to fail however. Because of the failure strategy in the CI system, I am not sure why some Sanity Checks fail on some branches and some don't.

Nemental commented 3 months ago

The remaining sanity workflow failures are not related with your changes and have already been fixed in #354 :)