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.

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

Ansible INI module #88

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
9 changes: 9 additions & 0 deletions cfbs.json
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,15 @@
"append enable.cf services/init.cf"
]
},
"promise-type-ini": {
"description": "A Custom CFEngine promise module, installing, and using the Ansible INI module to provide an INI promise type for INI files",
"subdirectory": "promise-types/ini",
"dependencies": ["library-for-promise-types-in-python"],
"steps": [
"copy ini.py modules/promises/",
"append enable.cf services/init.cf"
]
},
"uninstall-packages": {
"description": "Allows you to specify a list of packages you want uninstalled on your hosts.",
"subdirectory": "security/uninstall-packages",
Expand Down
3 changes: 3 additions & 0 deletions libraries/python/cfengine.py
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,9 @@ def _handle_evaluate(self, promiser, attributes, request):
except Exception as e:
self.log_critical(
"{error_type}: {error}".format(error_type=type(e).__name__, error=e)
+ e.message
Copy link
Contributor

Choose a reason for hiding this comment

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

seems like this would read easier and fit in one line.

Copy link
Contributor

Choose a reason for hiding this comment

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

then again, what does black say? yeah, black seems to agree with me. Maybe run all your python files through black

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, this is black. But I'm guessing it's a version thing then. I used 19. but don't know what your standard is

Copy link
Contributor

Choose a reason for hiding this comment

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

oh dang. the opinionated tool changes its opinions... :( I have 23.3.0 from brew on macos. Anyhow, I do like it better on one line when it fits since it is a single expression, essentially a ternary expression.

if hasattr(e, "message")
else ""
)
self._add_traceback_to_response()
self._result = Result.ERROR
Expand Down
30 changes: 30 additions & 0 deletions promise-types/ini/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
# Ansible INI promise type

## Synopsis

* *Name*: `Ansible INI - Custom Promise Module`
* *Version*: `0.0.1`
* *Description*: Manage configuration files through the Ansible INI module in CFEngine.

## Requirements

* Python installed on the system
* `ansible` pip package
* Correct path to the `ini_file.py` in the custom promise module

## Attributes

See [anible_ini module](https://docs.ansible.com/ansible/latest/collections/community/general/ini_file_module.html).

## Example

```cfengine3
bundle agent main
{
ini:
oleorhagen marked this conversation as resolved.
Show resolved Hide resolved
"/path/to/file.ini"
section => "foo",
option => "bar",
value => "baz";
}
```
6 changes: 6 additions & 0 deletions promise-types/ini/enable.cf
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
promise agent ini
oleorhagen marked this conversation as resolved.
Show resolved Hide resolved
# @brief Define ini promise type
{
path => "$(sys.workdir)/modules/promises/ini.py";
interpreter => "/usr/bin/python3";
}
20 changes: 20 additions & 0 deletions promise-types/ini/example.cf
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
promise agent ini
Copy link
Member

Choose a reason for hiding this comment

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

@oleorhagen sorry that we don't have a good automatic formatter for policy language (yet). Please take a look at the style guide here and try to align with it (doesn't have to be perfect):

https://docs.cfengine.com/docs/master/examples-tutorials-writing-and-serving-policy-policy-style.html#top

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Did my best. See latest push

oleorhagen marked this conversation as resolved.
Show resolved Hide resolved
# @brief Define ini promise type
oleorhagen marked this conversation as resolved.
Show resolved Hide resolved
{
path => "$(sys.workdir)/modules/promises/ini.py";
interpreter => "/usr/bin/python3";
Comment on lines +4 to +5
Copy link
Member

Choose a reason for hiding this comment

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

We could enable setting environment variables for the module, or add another field of "data" to pass to the module here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I like the data field idea

}

bundle agent main
{

meta:
"bundle_version" string => "0.0.1";
"promise_type" string => "ini";
Comment on lines +11 to +13
Copy link
Member

@olehermanse olehermanse Feb 26, 2024

Choose a reason for hiding this comment

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

Using the meta promise type won't do anything for the promise module.

You could use the meta attribute per promise:

  ini:
    "hello.ini"
      meta => { "report", "some=value" };

But it seems to me that the information you want to pass is more static, so would fit better if we make some room for it in the promise agent block.

Edit: This is currently not possible, the meta attribute does not get passed to the module. See custom promise type docs


ini:
oleorhagen marked this conversation as resolved.
Show resolved Hide resolved
"/tmp/ini/test.ini"
section => "foo",
option => "bar",
value => "baz";
}
105 changes: 105 additions & 0 deletions promise-types/ini/ini.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
"""A CFEngine custom promise module for INI files"""

import json
import subprocess
import sys

from cfengine import PromiseModule, ValidationError, Result


class AnsiballINIModule(PromiseModule):
def __init__(self):
super().__init__("ansible_ini_promise_module", "0.0.1")

self.add_attribute("path", str, default_to_promiser=True)

def validate_attributes(self, promiser, attributes, meta):
# Just pass the attributes on transparently to Ansible INI The Ansible
# module will report if the missing parameters are not in Ansible attributes

return True

def validate_promise(self, promiser: str, attributes: dict, meta: dict) -> None:
self.log_error(
"Validating the ansible ini promise: %s %s %s"
% (promiser, attributes, meta)
)
if not meta.get("promise_type"):
raise ValidationError("Promise type not specified")

assert meta.get("promise_type") == "ini"
oleorhagen marked this conversation as resolved.
Show resolved Hide resolved

def evaluate_promise(self, promiser: str, attributes: dict, meta: dict):
self.log_error(
"Evaluating the ansible ini promise %s, %s, %s"
% (promiser, attributes, meta)
)

if "module_path" not in attributes:
Copy link
Contributor

Choose a reason for hiding this comment

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

Related to the TODO above where validation "does not work" here we provide a reasonable default? Doesn't seem like it actually since we are assuming here python3.11 which is a wild assumption for many different platforms.

Can we be smarter here about the path?

I had to pip3 install ansible in order to get ini_file.py. pip3 install ansible-core didn't do enough.

If I do pip3 show ansible I get an idea of where to look that is dependent on the install which might be better and something we can do programmatically via python?

? pip3 show ansible
Name: ansible
Version: 9.2.0
Summary: Radically simple IT automation
Home-page: https://ansible.com/
Author: Ansible, Inc.
Author-email: [email protected]
License: GPL-3.0-or-later
Location: /usr/local/lib/python3.11/site-packages
Requires: ansible-core
Required-by: 

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I get your point, and I agree. However, I won't have time to finish this, and it seems the other modules too write in their Readme's that the prerequisites need to be installed.

I fiddled around with how to provide this, and tried a few different things:

  1. Having the path in the promise agent ini setup, but this apparently is not valid syntax
  2. Having it in the meta section in the promise - but this does not get passed to the custom promise module (only
  3. Hardcoding it as I've done now.

Dynamically would have been the sweetest though

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had to pip3 install ansible in order to get ini_file.py. pip3 install ansible-core didn't do enough.

This is true, if we do not provide the file ourselves (Like in the original implementation where it was downloaded).

we could consider adding the file to the repo, and have it installed just as we do the custom promise module ini.py itself?

Copy link
Contributor

Choose a reason for hiding this comment

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

Right on. Two things then:

  • we need to update the requirements to include ansible since that's easier than having folks have to figure out where to get it.
  • make the path dynamic with some python code if possible, doesn't have to be done by you :)

attributes.setdefault(
"module_path",
"/tmp/ini_file.py",
)

# NOTE: INI module specific - should not be passed on to Ansible
module_path = attributes["module_path"]
del attributes["module_path"]

# NOTE - needed because 'default_to_promiser' is not respected
attributes.setdefault("path", promiser)

self.log_error(
"Evaluating the ansible ini promise %s, %s, %s"
% (promiser, attributes, meta)
)

proc = subprocess.run(
[
"python",
module_path,
],
input=json.dumps({"ANSIBLE_MODULE_ARGS": attributes}).encode("utf-8"),
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
)

if not proc:
self.log_error("Failed to run the ansible module")
return (
Result.NOT_KEPT,
[],
)

if proc.returncode != 0:
self.log_error("Failed to run the ansible module")
self.log_error("Ansible INI module returned(stdout): %s" % proc.stdout)
self.log_error("Ansible INI module returned(stderr): %s" % proc.stderr)
return (
Result.NOT_KEPT,
[],
)

self.log_debug("Received output: %s (stdout)" % proc.stdout)
oleorhagen marked this conversation as resolved.
Show resolved Hide resolved
self.log_debug("Received output: (stderr): %s" % proc.stderr)

try:
d = json.loads(proc.stdout.decode("UTF-8").strip())
if d.get("changed", False):
self.log_info(
"Edited content of '%s' (%s)" % (promiser, d.get("msg", ""))
)
except Exception as e:
self.log_error(
"Failed to decode the JSON returned from the Ansible INI module. Error: %s"
% e
)
return (Result.NOT_KEPT, [])

return (
Result.KEPT,
[],
)


if __name__ == "__main__":
AnsiballINIModule().start()