Skip to content
Draft
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions plugins/module_utils/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

from __future__ import absolute_import, division, print_function
from ansible.module_utils.urls import url_argument_spec
from ansible.module_utils.six import string_types

__metaclass__ = type

Expand Down Expand Up @@ -52,3 +53,9 @@

def grafana_mutually_exclusive():
return [["url_username", "grafana_api_key"]]


def dict_str(value):
if isinstance(value, (dict, string_types)):
return value
raise TypeError

Check warning on line 61 in plugins/module_utils/base.py

View check run for this annotation

Codecov / codecov/patch

plugins/module_utils/base.py#L60-L61

Added lines #L60 - L61 were not covered by tests
Comment on lines +69 to +74
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moving dict_str to module_utils is fine, but not absolutely necessary, since this function isn't used multiple times.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did it as maybe other parts could use it as well. OK to keep?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

30 changes: 23 additions & 7 deletions plugins/modules/grafana_dashboard.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,8 @@
description:
- The path to the json file containing the Grafana dashboard to import or export.
- A http URL is also accepted (since 2.10).
- Required if C(state) is C(export) or C(present).
- Required if C(state) is C(export).
- Mutually exclusive with C(content) and C(dashboard_id).
aliases: [ dashboard_url ]
type: str
overwrite:
Expand All @@ -77,6 +78,7 @@
dashboard_id:
description:
- Public Grafana.com dashboard id to import
- Mutually exclusive with C(content) and C(path).
version_added: "1.0.0"
type: str
dashboard_revision:
Expand All @@ -90,6 +92,11 @@
- Set a commit message for the version history.
- Only used when C(state) is C(present).
type: str
content:
description:
- Grafana dashboard content.
- Mutually exclusive with C(path) and C(dashboard_id).
version_added: "2.3.0"
extends_documentation_fragment:
- community.grafana.basic_auth
- community.grafana.api_key
Expand Down Expand Up @@ -157,6 +164,7 @@
from ansible_collections.community.grafana.plugins.module_utils.base import (
grafana_argument_spec,
clean_url,
dict_str,
)

__metaclass__ = type
Expand Down Expand Up @@ -367,11 +375,18 @@
)
payload = json.loads(r.read())
else:
try:
with open(data["path"], "r", encoding="utf-8") as json_file:
payload = json.load(json_file)
except Exception as e:
raise GrafanaAPIException("Can't load json file %s" % to_native(e))
if data["path"]:
try:
with open(data["path"], "r", encoding="utf-8") as json_file:
payload = json.load(json_file)
except Exception as e:
raise GrafanaAPIException("Can't load json file %s" % to_native(e))

Check warning on line 383 in plugins/modules/grafana_dashboard.py

View check run for this annotation

Codecov / codecov/patch

plugins/modules/grafana_dashboard.py#L382-L383

Added lines #L382 - L383 were not covered by tests
else:
content = data["content"]

Check warning on line 385 in plugins/modules/grafana_dashboard.py

View check run for this annotation

Codecov / codecov/patch

plugins/modules/grafana_dashboard.py#L385

Added line #L385 was not covered by tests
if type(content) == dict:
payload = content

Check warning on line 387 in plugins/modules/grafana_dashboard.py

View check run for this annotation

Codecov / codecov/patch

plugins/modules/grafana_dashboard.py#L387

Added line #L387 was not covered by tests
else:
payload = json.loads(content)

Check warning on line 389 in plugins/modules/grafana_dashboard.py

View check run for this annotation

Codecov / codecov/patch

plugins/modules/grafana_dashboard.py#L389

Added line #L389 was not covered by tests

# Check that the dashboard JSON is nested under the 'dashboard' key
if "dashboard" not in payload:
Expand Down Expand Up @@ -638,6 +653,7 @@
dashboard_revision=dict(type="str", default="1"),
overwrite=dict(type="bool", default=False),
commit_message=dict(type="str"),
content=dict(type=dict_str),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://docs.ansible.com/ansible/latest/dev_guide/developing_program_flow_modules.html#argument-spec

Suggested change
content=dict(type=dict_str),
content=dict(type="raw"),

I think it's better, if not mandatory, to stick with the officially specified argument types. With raw, you can pass the content unvalidated and then perform the actual validation of the content immediately afterward.

Copy link
Author

@Hipska Hipska May 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, might need help on that. I copied the usage from here: https://github.com/ansible-collections/kubernetes.core/blob/94c1f57f36a08c1ac245556824410c52d929ec21/plugins/module_utils/args_common.py#L85

I think it is like that to be able to have ansible-lint to check the argument type?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For example, you can set the type="raw" and then add your try-isinstance and except-TypeError to the already existing try/except block. Something like this (not tested):

    try:
        if not isinstance(module.params["content"], (dict, string_types)):
            raise TypeError("content must be either a string or a dictionary")
        if module.params["state"] == "present":
            result = grafana_create_dashboard(module, module.params)
        elif module.params["state"] == "absent":
            result = grafana_delete_dashboard(module, module.params)
        else:
            result = grafana_export_dashboard(module, module.params)
    except (TypeError, GrafanaAPIException, GrafanaMalformedJson) as e:
        module.fail_json(failed=True, msg="error : %s" % to_native(e))
    except GrafanaDeleteException as e:
        module.fail_json(
            failed=True, msg="error : Can't delete dashboard : %s" % to_native(e)
        )
    except GrafanaExportException as e:
        module.fail_json(
            failed=True, msg="error : Can't export dashboard : %s" % to_native(e)
        )

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rndmh3ro Maybe you can share your opinion :)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rndmh3ro Please advise, so we can have this landed..

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rndmh3ro any update please?

)
module = AnsibleModule(
argument_spec=argument_spec,
Expand All @@ -649,7 +665,7 @@
mutually_exclusive=[
["url_username", "grafana_api_key"],
["uid", "slug"],
["path", "dashboard_id"],
["path", "dashboard_id", "content"],
["org_id", "org_name"],
],
)
Expand Down
Loading