Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Grafana datasource module : tls_ca_cert or tls_skip_verify options #36945

Merged
merged 5 commits into from
Apr 9, 2018
Merged

Grafana datasource module : tls_ca_cert or tls_skip_verify options #36945

merged 5 commits into from
Apr 9, 2018

Conversation

seuf
Copy link
Contributor

@seuf seuf commented Mar 2, 2018

SUMMARY

Allow to set tls_ca_cert or skip verify for grafana datasources

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

grafana_datasource module

ANSIBLE VERSION

Here is my ansible --version

ansible 2.4.2.0
  config file = /etc/ansible/ansible.cfg
  configured module search path = [u'/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/lib/python2.7/site-packages/ansible
  executable location = /usr/bin/ansible
  python version = 2.7.14 (default, Dec 11 2017, 16:08:01) [GCC 7.2.1 20170915 (Red Hat 7.2.1-2)]
ADDITIONAL INFORMATION

With the release of grafana 5.0.0, the datasource tls certificate are verified by default.
this pull request allow to set the tls_ca_cert option to set the certificate authority to validate the datasource certificate or tls_skip_verify to skip the datasource certificate.

@ansibot
Copy link
Contributor

ansibot commented Mar 2, 2018

cc @tsalle
click here for bot help

@ansibot ansibot added bug This issue/PR relates to a bug. ci_verified Changes made in this PR are causing tests to fail. module This issue/PR relates to a module. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. needs_triage Needs a first human triage before being processed. support:community This issue/PR relates to code supported by the Ansible community. labels Mar 2, 2018
@s-hertel s-hertel removed the needs_triage Needs a first human triage before being processed. label Mar 2, 2018
@ansibot ansibot added community_review In order to be merged, this PR must follow the community review workflow. and removed ci_verified Changes made in this PR are causing tests to fail. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Mar 2, 2018
@seuf seuf changed the title Allow to set tls_ca_cert or skip verify for grafana datasources Grafana datasource module : tls_ca_cert or tls_skip_verify options Mar 2, 2018
@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Mar 10, 2018
@ansibot ansibot added needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed community_review In order to be merged, this PR must follow the community review workflow. labels Mar 18, 2018
@seuf seuf force-pushed the grafana_datasource_tls_ca_cert_verify branch from 35e77fa to 0b99de0 Compare March 19, 2018 08:57
@ansibot ansibot added ci_verified Changes made in this PR are causing tests to fail. needs_info This issue requires further information. Please answer any outstanding questions. and removed needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. labels Mar 19, 2018
@seuf seuf force-pushed the grafana_datasource_tls_ca_cert_verify branch from 0b99de0 to 25fe64b Compare March 19, 2018 09:07
@ansibot ansibot added ci_verified Changes made in this PR are causing tests to fail. and removed ci_verified Changes made in this PR are causing tests to fail. labels Mar 19, 2018
@seuf seuf force-pushed the grafana_datasource_tls_ca_cert_verify branch from 25fe64b to c78d17d Compare March 19, 2018 09:33
@ansibot ansibot removed the ci_verified Changes made in this PR are causing tests to fail. label Mar 19, 2018
@seuf
Copy link
Contributor Author

seuf commented Mar 19, 2018

Hi, since the grafana_datasource module documentation has been updated (cf. #37516) I've rebased this pr and also updated the examples.
@gundalow Could you review this pr ?

@ansibot ansibot removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Mar 19, 2018
@seuf
Copy link
Contributor Author

seuf commented Mar 20, 2018

cc @dagwieers . I see you have cleaned up the documentation for this module.
This pr also fix the examples..

@ansibot ansibot removed the needs_info This issue requires further information. Please answer any outstanding questions. label Mar 20, 2018
@ansibot ansibot added the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Mar 20, 2018
@seuf
Copy link
Contributor Author

seuf commented Mar 26, 2018

@dagwieers Is this ok for you ? I will probably need to rebase the grafana zabbix datasource PR #36948 when this one will be merged..

@seuf seuf force-pushed the grafana_datasource_tls_ca_cert_verify branch from 354d5fa to 238fde3 Compare March 30, 2018 12:33
@seuf
Copy link
Contributor Author

seuf commented Mar 30, 2018

PR rebased from devel branch with examples more accurate. ping @dagwieers

@ansibot ansibot added stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. stale_review Updates were made after the last review and the last review is more than 7 days old. labels Apr 7, 2018
@seuf
Copy link
Contributor Author

seuf commented Apr 9, 2018

PR ready.. Can you approve it / ship it ? @dagwieers

@dagwieers
Copy link
Contributor

dagwieers commented Apr 9, 2018 •
edited

@seuf I am not directly involved with Grafana development so I prefer @tsalle or another Grafana dev approves this. If none of them respond within a certain period, we could think of increasing the number of authors/dev for the grafana modules so active developmentis not stalling.

Update: Hmm, it seems @tsalle is not an existing Github user ?

@ansibot ansibot added community_review In order to be merged, this PR must follow the community review workflow. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. stale_review Updates were made after the last review and the last review is more than 7 days old. labels Apr 9, 2018
@dagwieers
Copy link
Contributor

@seuf It appeared you authored this module, but ansibot only knows about @tsalle (which isn't an actual Github account). So what I would do is remove the @tsalle reference from the source and add yourself as the author, either in the code, or in .github/BOTMETA.yml so that you have shipit-rights.

Or you could form a group of trusted Grafana devs, and add the group to BOTMETA.yml. This way you can work as a group and approve each other's PRs on these Grafana modules. If you plan to work together on a roadmap for future releases, you might even want to add some pages to the Community Wiki: https://github.com/ansible/community/wiki

@seuf
Copy link
Contributor Author

seuf commented Apr 9, 2018

@tsalle is my Twitter account.. I can change the author in the source to my @seuf github account..

@dagwieers
Copy link
Contributor

dagwieers commented Apr 9, 2018 •
edited

@seuf You should, the format expects a Github account. Then you can merge your own PRs. And if you updated this PR, I will merge everything.

@seuf
Copy link
Contributor Author

seuf commented Apr 9, 2018

github account fixed.
I will do the same on the other grafana modules (dashboard and plugin)

@dagwieers
Copy link
Contributor

The author-entry is actually more important than the Copyright entry. So I took the liberty to edit it myself in Github.

@dagwieers dagwieers merged commit 72120c3 into ansible:devel Apr 9, 2018
ilicmilan pushed a commit to ilicmilan/ansible that referenced this pull request Nov 7, 2018
@seuf @ilicmilan
Grafana datasource module : tls_ca_cert or tls_skip_verify options (a…
a4dc5eb 
…nsible#36945)

* Allow to set tls_ca_cert or skip verify for grafana datasources

* version_added in documentation for new options tls_skip_verify

* Added default value for tls_skip_verify option in doc

* Fixed author git account

* Updated author
@dagwieers dagwieers added the grafana Grafana community label Feb 7, 2019
@ansible ansible locked and limited conversation to collaborators Apr 27, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@dagwieers dagwieers dagwieers approved these changes

Assignees
No one assigned
Labels
bug This issue/PR relates to a bug. community_review In order to be merged, this PR must follow the community review workflow. grafana Grafana community module This issue/PR relates to a module. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. support:community This issue/PR relates to code supported by the Ansible community.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@seuf @ansibot @dagwieers @s-hertel