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

grafana_dashboard lookup plugin does not accept `validate_certs` as a parameter and does not perform accordingly #346

Closed shantanoo-desai closed 3 months ago

shantanoo-desai commented 5 months ago
SUMMARY

As a simple example, I have a Grafana Container running behind a reverse-proxy with Self-Signed Certificates which I do not want to have validated. The Grafana instance is reachable via https://<remote_machine>/grafana API

Upon using the following task:

- ansible.builtin.set_fact:
    current_dashboards: |
      {{
        lookup(
          'community.grafana.grafana_dashboard',
          'grafana_url=https://{{ inventory_hostname }}/grafana grafana_user=admin grafana_password=test search=foo',
          validate_certs=false
        )
      }}
ISSUE TYPE
COMPONENT NAME

community.grafana.grafana_dashboard lookup plugin

ANSIBLE VERSION
ansible [core 2.13.8]
  config file = /mnt/c/Users/E1401748/Development/gitlab.emrsn.org/pacedge-3.x/appstack/ansible.cfg
  configured module search path = ['/home/shantanoo/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /home/shantanoo/.local/lib/python3.8/site-packages/ansible
  ansible collection location = /home/shantanoo/.ansible/collections:/usr/share/ansible/collections
  executable location = /usr/bin/ansible
  python version = 3.8.10 (default, May 26 2023, 14:05:08) [GCC 9.4.0]
  jinja version = 3.1.2
  libyaml = True
COLLECTION VERSION
# /home/shantanoo/.ansible/collections/ansible_collections
Collection        Version
----------------- -------
community.grafana 1.7.0  
CONFIGURATION
DEFAULT_MANAGED_STR(/mnt/c/Users/E1401748/Development/gitlab.emrsn.org/pacedge-3.x/appstack/ansible.cfg) = This file is generated by Manager.%n
Generated On: %Y-%m-%d %H:%M:%S %n
DEFAULT_STDOUT_CALLBACK(/mnt/c/Users/E1401748/Development/gitlab.emrsn.org/pacedge-3.x/appstack/ansible.cfg) = yaml
OS / ENVIRONMENT

Windows Subsystem For Linux 2 - Ubuntu 20.04 LTS

STEPS TO REPRODUCE
- hosts: all
  tasks:
    - ansible.builtin.set_fact:
        dashboards: "{{ lookup('community.grafana.grafana_dashboard', 'grafana_url=https://{{ inventory_hostname}}/grafana grafana_user=admin grafana_password=test search=foo', validate_certs=false) }}"
    - debug: var=dashboards
EXPECTED RESULTS

Additional validate_certs variable should be accepted to bypass SSL certificate validation, the value is either true or false

An expectation is that the lookup plugin might behave similarly to the URL lookup plugin where explicit headers and validate_certs parameters can be explicitly set.

ACTUAL RESULTS

Throws a SSL certificate verification error:

An unhandled exception occurred while running the lookup plugin ''community.grafana.grafana_dashboard''. Error was a <class ''urllib.error.URLError''>, original message: <urlopen error [SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: self signed certificate (_ssl.c:1131)>. <urlopen error [SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: self signed certificate (_ssl.c:1131)>'
rndmh3ro commented 5 months ago

Yes, you're right. This functionality is missing. Do you want to create a PR to tackle this?

shantanoo-desai commented 5 months ago

@rndmh3ro Yes, will work on it. AFAIK this requires only changes in the module_utils/base.py argument_spec, right? Because most of the plugins will then rely on the utility to interact with the API. Or am I wrong and I need to check the lookup plugins directory and apply the changes there?

rndmh3ro commented 5 months ago

AFAIK this requires only changes in the module_utils/base.py argument_spec, right?

The argument_spec is not used in the lookup-plugin, so this will probably not work.

Or am I wrong and I need to check the lookup plugins directory and apply the changes there?

I think so, at least that's what I did some time ago in another collection.

@rrey do you have some more info here?

shantanoo-desai commented 5 months ago

@rndmh3ro no problem. Let me try working on the patch and will keep the communication open via this Issue. I will try referring to another lookup plugins from other community collections to get a better Idea.

shantanoo-desai commented 3 months ago

@rndmh3ro and @rrey Sorry it took me a while to revert back to this Issue. From what I understand:

Please advise on this topic to comply with submitting a fix for this Issue

rndmh3ro commented 3 months ago

I think adding the missing options to the GrafanaAPI class should be sufficient.