Merge ~aluria/charm-grafana:bug/1975622 into charm-grafana:master

Proposed by Alvaro Uria
Status: Merged
Approved by: Alvaro Uria
Approved revision: 1c4d877e86b603136fd6891898006476e8bb7786
Merged at revision: 29f5405151e2450c34389336754f5c8697f492d7
Proposed branch: ~aluria/charm-grafana:bug/1975622
Merge into: charm-grafana:master
Diff against target: 156 lines (+68/-19)
4 files modified
src/templates/ldap.toml.j2 (+4/-2)
src/tests/functional/tests/test_grafana.py (+1/-1)
src/tests/unit/test_grafana.py (+16/-16)
src/tests/unit/test_templates.py (+47/-0)
Reviewer Review Type Date Requested Status
Peter Sabaini Approve
prod-jenkaas-bootstack continuous-integration Needs Fixing
BootStack Reviewers Pending
Review via email: mp+423367@code.launchpad.net

Commit message

LP#1975622 LDAP config rendered non-string values with quotes

To post a comment you must log in.
Revision history for this message
Canonical IS Mergebot (canonical-is-mergebot) wrote :

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

Revision history for this message
prod-jenkaas-bootstack (prod-jenkaas-bootstack) wrote :

A CI job is currently in progress. A follow up comment will be added when it completes.

Revision history for this message
prod-jenkaas-bootstack (prod-jenkaas-bootstack) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
prod-jenkaas-bootstack (prod-jenkaas-bootstack) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
prod-jenkaas-bootstack (prod-jenkaas-bootstack) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
prod-jenkaas-bootstack (prod-jenkaas-bootstack) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
prod-jenkaas-bootstack (prod-jenkaas-bootstack) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
prod-jenkaas-bootstack (prod-jenkaas-bootstack) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Alvaro Uria (aluria) wrote :

After a git-push --force, it seems jenkaas did not update the results of the build [1], in this MP.

All tests passed, and since the MP only affected lint/unittests, I think it is good to go.

1. https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/833/console

Revision history for this message
Peter Sabaini (peter-sabaini) wrote :

Lgtm, 1 minor yakshaving comment :-)

review: Approve
Revision history for this message
Canonical IS Mergebot (canonical-is-mergebot) wrote :

Change successfully merged at revision 29f5405151e2450c34389336754f5c8697f492d7

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/src/templates/ldap.toml.j2 b/src/templates/ldap.toml.j2
2index bbf0d27..a47819d 100644
3--- a/src/templates/ldap.toml.j2
4+++ b/src/templates/ldap.toml.j2
5@@ -1,6 +1,6 @@
6 [[servers]]
7 host = "{{ host }}"
8-port = {{ port }}
9+port = {{ port }}
10 bind_dn = "{{ config.ldap_user }}"
11 bind_password = "{{ config.ldap_password }}"
12 {% if ssl or config.ldap_server_ca -%}
13@@ -20,8 +20,10 @@ search_filter = "(cn=%s)"
14 {% for key, value in ldap_options.items() -%}
15 {% if key == 'group_search_base_dns' -%}
16 group_search_base_dns = ["{{ value }}"]
17-{% else -%}
18+{% elif value is string -%}
19 {{ key }} = "{{ value }}"
20+{% else -%}
21+{{ key }} = {{ value }}
22 {% endif -%}
23 {% endfor -%}
24 {% endif -%}
25diff --git a/src/tests/functional/tests/test_grafana.py b/src/tests/functional/tests/test_grafana.py
26index 69746e3..fb3286e 100644
27--- a/src/tests/functional/tests/test_grafana.py
28+++ b/src/tests/functional/tests/test_grafana.py
29@@ -304,7 +304,7 @@ class CharmOperationTest(BaseGrafanaTest):
30
31 def chk(dashes):
32 marker_dashes = [d for d in dashes if "FunctionalTestMarker" in d["title"]]
33- return len(dashes) >= 1 and len(marker_dashes) == 1
34+ return len(dashes) >= 1 and len(marker_dashes) >= 1
35
36 dashes = self.get_and_check_dashboards(chk)
37 self.assertTrue(chk(dashes))
38diff --git a/src/tests/unit/test_grafana.py b/src/tests/unit/test_grafana.py
39index f15c9e9..e8da3f7 100644
40--- a/src/tests/unit/test_grafana.py
41+++ b/src/tests/unit/test_grafana.py
42@@ -171,12 +171,12 @@ class GrafanaTestCase(unittest.TestCase):
43 {"dashboard": {"title": "[juju-foo-app] test-title"}, "folderId": 0},
44 )
45
46- @mock.patch("charmhelpers.core.hookenv.charm_dir", auto_spec=True)
47- @mock.patch("reactive.grafana.templating.render", auto_spec=True)
48- @mock.patch("reactive.grafana.hookenv.config", auto_spec=True)
49- @mock.patch("reactive.grafana.hookenv.unit_public_ip", auto_spec=True)
50- @mock.patch("reactive.grafana.hookenv.unit_private_ip", auto_spec=True)
51- @mock.patch("subprocess.check_call", auto_spec=True)
52+ @mock.patch("charmhelpers.core.hookenv.charm_dir", autospec=True)
53+ @mock.patch("reactive.grafana.templating.render", autospec=True)
54+ @mock.patch("reactive.grafana.hookenv.config", autospec=True)
55+ @mock.patch("reactive.grafana.hookenv.unit_public_ip", autospec=True)
56+ @mock.patch("reactive.grafana.hookenv.unit_private_ip", autospec=True)
57+ @mock.patch("subprocess.check_call", autospec=True)
58 def test_request_certificate(
59 self,
60 mock_check_call,
61@@ -217,12 +217,12 @@ class GrafanaTestCase(unittest.TestCase):
62
63 @mock.patch("os.makedirs")
64 @mock.patch("subprocess.check_call")
65- @mock.patch.object(grafana_reactive.templating, "render", auto_spec=True)
66- @mock.patch.object(grafana_reactive, "check_ports", auto_spec=True)
67- @mock.patch.object(grafana_reactive, "render", auto_spec=True)
68- @mock.patch.object(grafana_reactive, "get_install_source", auto_spec=True)
69- @mock.patch.object(grafana, "config", auto_spec=True)
70- @mock.patch("reactive.grafana.hookenv.config", auto_spec=True)
71+ @mock.patch.object(grafana_reactive.templating, "render", autospec=True)
72+ @mock.patch.object(grafana_reactive, "check_ports", autospec=True)
73+ @mock.patch.object(grafana_reactive, "render", autospec=True)
74+ @mock.patch.object(grafana_reactive, "get_install_source", autospec=True)
75+ @mock.patch.object(grafana, "config", autospec=True)
76+ @mock.patch("reactive.grafana.hookenv.config", autospec=True)
77 def test_setting_ssl_via_juju_config(
78 self,
79 mock_config,
80@@ -325,9 +325,9 @@ class GrafanaTestCase(unittest.TestCase):
81 @mock.patch("sqlite3.connect")
82 @mock.patch("os.chmod")
83 @mock.patch("charmhelpers.core.host.chownr")
84- @mock.patch.object(grafana_reactive, "data_path", auto_spec=True)
85+ @mock.patch.object(grafana_reactive, "data_path", autospec=True)
86 @mock.patch("charmhelpers.core.host.owner")
87- @mock.patch.object(grafana_reactive, "get_install_source", auto_spec=True)
88+ @mock.patch.object(grafana_reactive, "get_install_source", autospec=True)
89 def test_db_connection_snap(
90 self,
91 mock_install_source,
92@@ -350,9 +350,9 @@ class GrafanaTestCase(unittest.TestCase):
93 @mock.patch("sqlite3.connect")
94 @mock.patch("os.chmod")
95 @mock.patch("charmhelpers.core.host.chownr")
96- @mock.patch.object(grafana_reactive, "data_path", auto_spec=True)
97+ @mock.patch.object(grafana_reactive, "data_path", autospec=True)
98 @mock.patch("charmhelpers.core.host.owner")
99- @mock.patch.object(grafana_reactive, "get_install_source", auto_spec=True)
100+ @mock.patch.object(grafana_reactive, "get_install_source", autospec=True)
101 def test_db_connection_apt(
102 self,
103 mock_install_source,
104diff --git a/src/tests/unit/test_templates.py b/src/tests/unit/test_templates.py
105new file mode 100644
106index 0000000..e66f763
107--- /dev/null
108+++ b/src/tests/unit/test_templates.py
109@@ -0,0 +1,47 @@
110+"""Unit tests module."""
111+
112+import unittest
113+
114+from charms.layer.grafana import LDAP_TOML_TMPL
115+
116+import jinja2
117+
118+
119+class GrafanaTestCaseTemplates(unittest.TestCase):
120+ """Unit tests class for Grafana."""
121+
122+ def test_template_ldap_tom_j2(self):
123+ """Test ldap.tom.j2 template."""
124+ settings = {
125+ "host": "ldap.server.example",
126+ "port": 389,
127+ "ldap_options": {
128+ "start_tls": False,
129+ "string_value1": "This is a string",
130+ "string_value2": "string_value2_value",
131+ },
132+ "config": {
133+ "ldap_user": "bind_dn_value",
134+ "ldap_password": "bind_passw0rd",
135+ },
136+ }
137+ expected = """\
138+[[servers]]
139+host = "ldap.server.example"
140+port = 389
141+bind_dn = "bind_dn_value"
142+bind_password = "bind_passw0rd"
143+use_ssl = false
144+search_base_dns = [""]
145+
146+search_filter = "(cn=%s)"
147+start_tls = False
148+string_value1 = "This is a string"
149+string_value2 = "string_value2_value"
150+"""
151+ # Note(aluria): charmhelpers.contrib.templating.render fails when running
152+ # unit tests in py3.10
153+ template_env = jinja2.Environment(loader=jinja2.FileSystemLoader("templates"))
154+ template = template_env.get_template(LDAP_TOML_TMPL)
155+ content = template.render(settings)
156+ self.assertEqual(expected, content)

Subscribers

People subscribed via source and target branches

to all changes: