Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
d99c83d
kea_command: new module to access an ISC KEA server
mirabilos Aug 21, 2025
8b64f62
add __metaclass__ = type as the CI sanity test wants,
mirabilos Aug 22, 2025
7655dda
copy future imports from some other random module
mirabilos Aug 22, 2025
d4b8fdb
… maybe like this? for another unsupported version…
mirabilos Aug 22, 2025
9268bbd
why does this want lists to be indented TWICE‽
mirabilos Aug 22, 2025
f9dd8f1
docs say to use “yes”, yamllint says to use “true” smh
mirabilos Aug 22, 2025
13549c3
add at least a comment about the Python vs. py3k situation
mirabilos Aug 22, 2025
7e0b488
indent lists twice, which yamllint here needs for some reason
mirabilos Aug 23, 2025
a918511
Revert "… maybe like this? for another unsupported version…"
mirabilos Aug 23, 2025
d717025
port from py3k to the common subset with Python 2.7
mirabilos Aug 23, 2025
d55f534
forgot to extra-indent this
mirabilos Aug 23, 2025
a539cb1
add empty lines between the examples for readability
mirabilos Aug 23, 2025
44db03e
as requested, do not document standard return 'msg'
mirabilos Aug 23, 2025
829c503
simplify by not supporting check_mode
mirabilos Aug 23, 2025
844de31
explicitly mention needs/privileged
mirabilos Aug 23, 2025
00afb71
use community.general.attributes
mirabilos Aug 23, 2025
2a56503
add community.general.attributes.platform as well
mirabilos Aug 23, 2025
fd78a56
Merge suggested change to documentation
mirabilos Aug 23, 2025
77a6cfe
Merge ASCIIfication of message
mirabilos Aug 23, 2025
18412bb
Merge suggestion for docs formatting
mirabilos Aug 27, 2025
d41e7b8
use just one try block; add the exception message
mirabilos Aug 27, 2025
a75e00c
another exception whose text to keep
mirabilos Aug 27, 2025
205b7ce
document magic value
mirabilos Aug 27, 2025
faddbf9
document sock I/O bufsiz
mirabilos Aug 27, 2025
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
2 changes: 2 additions & 0 deletions .github/BOTMETA.yml
Original file line number Diff line number Diff line change
Expand Up @@ -803,6 +803,8 @@ files:
maintainers: Slezhuk pertoft
$modules/kdeconfig.py:
maintainers: smeso
$modules/kea_command.py:
maintainers: mirabilos
$modules/kernel_blacklist.py:
maintainers: matze
$modules/keycloak_:
Expand Down
215 changes: 215 additions & 0 deletions plugins/modules/kea_command.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,215 @@
#!/usr/bin/python
# -*- coding: utf-8 -*-
# SPDX-License-Identifier: GPL-3.0-or-later

# Copyright © Thorsten Glaser <[email protected]>
# GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt)

# note: this module has not been tested with Python, as the switch from ISC DHCPD to KEA occurs at a time where distros only ship Python3 instead.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure I understood the note. What do you mean by "has not tested with Python" ? Do you mean Python 2.7?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, exactly. I have written this in py3k and had to port it to the subset between py3k and Python later so the checks pass, but KEA is so new that any distro that ships it will already have Python deprecated and removed.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Then please be explicit about that. Also, in the aliases file in the integration test, there is a keyword you can use to prevent it from running the test in Python 2.7. Please add it (and remove the unsupported one as mentioned in the previous comment).

Copy link
Author

Choose a reason for hiding this comment

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

… huh, which one, and does that affect the EOL tests?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

And in this very line, please use "Python 2.7" instead of "Python". The unnumbered form is ambiguous, some people might read and imagine that "Python", in 2025, certainly means Python 3 (because Python 2 has been deprecated a long time ago). What is obvious to one person might be completely misleading to another. ;-)

from __future__ import (absolute_import, division, print_function)
__metaclass__ = type


DOCUMENTATION = r"""
---
module: kea_command
short_description: Submits generic command to ISC KEA server on target
description:
- Submits a command to the JSON API of an ISC KEA server running on the target and obtains the result.
- This module supports sending arbitrary commands and returns the server response unchecked;
while it would be possible to write individual modules for specific KEA service commands,
that approach would not scale, as the FOSS hooks alone provide dozens of commands.
- Between sending the command and parsing the result status, RV(ignore:changed) will register as V(true) if an error occurs,
to err on the safe side.
version_added: '11.3.0'
author: Thorsten Glaser (@mirabilos)
options:
command:
description:
- The name of the command to send, for example V(status-get).
required: true
type: str
arguments:
description:
- The arguments sent along with the command, if any.
- Use V({}) to send an empty JSONObject instead of omitting.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wouldn't it be better to make default={}? Or, leave no default and, in the code (down there somewhere):

if arguments is None:
    argumentes = {}

Whichever makes more sense to you.

Copy link
Author

Choose a reason for hiding this comment

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

I had wondered about this, but it makes a difference, and I’ve only tested the variant of not sending the JSONObject, and who knows how the KEA API will develop in the future, so let’s allow the user to set either for now.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I would not call it a JSONObject in the documentation for the end user, though. Some of your users might not be too familiar with JSON and its terminology. In the Ansible/YAML world, that thing would be a dict, even though they're amount to the same thing under the wraps.

Copy link
Author

Choose a reason for hiding this comment

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

Ah. I was addressing the KEA user here. I guess we’ll need to fit both.

What do you two think about: - Use V({}) to send an empty arguments dict/object instead of omitting.

type: dict
rv_unchanged:
description:
- A list of C(result) codes to indicate success but unchanged system state.
- Set this to V([0]) for most acquisition commands.
- Use V([3]) for C(lease4-del) and similar which have a separate code for this.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think I have missed this one before, I believe this would be:

Suggested change
- Use V([3]) for C(lease4-del) and similar which have a separate code for this.
- Use V([3]) for O(command=lease4-del) and similar which have a separate code for this.

More occurrences around the docs, please review.

Copy link
Author

Choose a reason for hiding this comment

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

Will do.

- Any C(result) codes not listed in either O(rv_unchanged) or O(rv_changed) are interpreted as indicating an error result.
- O(rv_unchanged) has precedence over O(rv_changed) if a result code is in both lists.
type: list
elements: int
default: []
rv_changed:
description:
- A list of C(result) codes to indicate success and changed system state.
- Omit this for most acquisition commands.
- Set it to V([0]) for C(lease4-del) and similar which return changed system state that way.
- Any C(result) codes not listed in either O(rv_unchanged) or O(rv_changed) are interpreted as indicating an error result.
- O(rv_unchanged) has precedence over O(rv_changed) if a result code is in both lists.
type: list
elements: int
default: []
Comment on lines +37 to +56
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wow, so the user has to map all the low level exist codes to get the changed status? That almost make using ansible.builtin.command more attractive an option :-)

To me that sounds like we need not one module, but a handful of them, so that the we can handle these exist codes accordingly.

Copy link
Author

Choose a reason for hiding this comment

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

Wow, so the user has to map all the low level exist codes to get the changed status?

Yes, as they differ between commands. KEA does not have a central command registry; they have commands common to DHCP4 and DHCP6 and DDNS, and commands specific for these, and dozens of extension libraries.

If someone really has time and interest, they can machinally create a few hundred Ansible modules that map just one command, but until then, we have to do this. And even then we still need the generic one for commands from plug-ins we don’t know about (KEA has paid-for plugins as well)…

I looked into defaulting to something more user-friendly, but the status codes diverge so much… at least, if the user does not list them it will fail, so the user will know to fix it.

The ansible.builtin.command way does not look better: you pretty much have to install socat, do something like…

   ansible.builtin.command:
     argv: ["socat", "UNIX:/run/kea/kea4-ctrl-socket", "-"]
     expand_argument_vars: false
     stdin: |
       {
         "command": "lease4-del",
         "arguments": {
           "ip-address": "{{ ipaddr }}"
         }
       }

… and still check return values. Or use one of the possible clients provided by KEA that also require you to manually format the JSON, but one is not available in any version but the one released last month, and the other is deprecated…

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok, I see :-) Thanks for clarification.

The only other way I could think of, until you said a few hundred, would be to have a preset map of command names to RV values (both changed, unchanged), so that could be automatically selected. But then again, probably only worth doing if we were talking a handful, or tens, but certainly not hundreds.

Copy link
Collaborator

Choose a reason for hiding this comment

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

If you have some patterns that are repeated abundantly in these hundred, maybe you could create a parameter, say, rv_schema or something to that effect, make it a fixed list of choices, and hopefully cover 80% of the cases with that. That parameter would then be mutually exclusive with these existing ones, so if the user uses a fixed schema they cannot pass these specific values.

Like, one such schema might be called 0chg_3unchg (please use better names than this if you go for it) and it would be equivalent to rv_changed=[0], rv_unchanged=[3], your description for the command=lease4-del.

Copy link
Author

Choose a reason for hiding this comment

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

Hm.

That honestly sounds like overcomplicating things to me, and definitely reduces legibility for saving a few bytes at best.

I also don’t have an easy way to check return values for even the currently known plugins, some even are pay-to-use…

I’d prefer to keep it simple at first.

socket:
description:
- The full pathname of the Unix Domain Socket to connect to.
- The default value is suitable for C(kea-dhcp4-server) on Debian trixie.
- This module directly interfacees via UDS; the HTTP wrappers are not supported.
type: path
default: /run/kea/kea4-ctrl-socket
extends_documentation_fragment:
- community.general.attributes
- community.general.attributes.platform
attributes:
check_mode:
support: none
diff_mode:
support: none
platform:
support: full
platforms: posix
"""

EXAMPLES = r"""
vars:
ipaddr: "192.168.123.45"
hwaddr: "00:00:5E:00:53:00"
tasks:

# an example for a request acquiring information
- name: Get KEA DHCP6 status
kea_command:
command: status-get
rv_unchanged: [0]
socket: /run/kea/kea6-ctrl-socket
register: kea6_status
- name: Display registered status result
ansible.builtin.debug:
msg: KEA DHCP6 running on PID {{ kea6_status.response.arguments.pid }}

# an example for requests modifying state
- name: Remove existing leases for {{ ipaddr }}, if any
kea_command:
command: lease4-del
arguments:
ip-address: "{{ ipaddr }}"
rv_changed: [0]
rv_unchanged: [3]
- name: Add DHCP lease for {{ ipaddr }}
kea_command:
command: lease4-add
arguments:
ip-address: "{{ ipaddr }}"
hw-address: "{{ hwaddr }}"
rv_changed: [0]
"""

RETURN = r"""
response:
description: The server JSON response.
returned: when available
type: dict
"""

import json
import os
import socket

from ansible.module_utils.basic import AnsibleModule


# default buffer size for socket I/O
BUFSIZ = 8192


def _parse_constant(s):
raise ValueError('Invalid JSON: "%s"' % s)


def main():
module = AnsibleModule(
argument_spec=dict(
command=dict(type='str', required=True),
arguments=dict(type='dict'),
rv_unchanged=dict(type='list', elements='int', default=[]),
rv_changed=dict(type='list', elements='int', default=[]),
socket=dict(type='path', default='/run/kea/kea4-ctrl-socket'),
),
)

cmd = {}
cmd['command'] = module.params['command']
if module.params['arguments'] is not None:
cmd['arguments'] = module.params['arguments']
cmdstr = json.dumps(cmd, ensure_ascii=True, allow_nan=False, indent=None, separators=(',', ':'), sort_keys=True)
rvok = module.params['rv_unchanged']
rvch = module.params['rv_changed']
sockfn = module.params['socket']

r = {
'changed': False
}
rsp = b''

if not os.path.exists(sockfn):
r['msg'] = 'socket (%s) does not exist' % sockfn
module.fail_json(**r)

phase = 'opening'
try:
with socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) as sock:
phase = 'connecting'
sock.connect(sockfn)
# better safe in case anything fails…
r['changed'] = True
phase = 'writing'
sock.sendall(cmdstr.encode('ASCII'))
phase = 'reading'
while True:
rspnew = sock.recv(BUFSIZ)
if len(rspnew) == 0:
break
rsp += rspnew
phase = 'closing'
except OSError as ex:
r['msg'] = 'error %s socket (%s): %s' % (phase, sockfn, str(ex))
r['exception'] = ex
Copy link
Collaborator

Choose a reason for hiding this comment

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

The return value exception is dubbed as one of the common return values, and as such, they have specific semantics. In this case exception is a misnomer, probably for some lost historical reason. The value of that RV, despite the name, is not an object of the type Exception (like ex here). It is meant to be a traceback object - associated with that exception, yes, but NOT the exception itself.

One simple way of making that work is to raise a new Exception with your text message as argument, from the exception ex. To make that work, because we still have to support Python 2 in this collection, you would have to use:

from ansible.module_utils.six import raise_from
...

    except OSError as ex:
        raise_from(Exception('error %s socket ...' % (...)), ex)

Or you can obtain the traceback object yourself and pass it as RV exception.
Personally, I think since you are handling the error and passing an informative message about it, there is no need to return exception nor traceback. But that's your call.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Alternatively change r['exception'] = ex to r['exception'] = traceback.format_exc() (and add import traceback further up).

Copy link
Author

Choose a reason for hiding this comment

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

Is that another Python vs. py3k thing? The builtin command module certainly doesn’t do that when it does:

    try:
        os.chdir(chdir)
    except OSError as ex:
        r['msg'] = 'Unable to change directory before execution.'
        module.fail_json(**r, exception=ex)

(It’s one of the two-and-a-half I derived from.)

Copy link
Collaborator

Choose a reason for hiding this comment

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

No, this is an Ansible thing. Exceptions must not be passed as exception objects to module.fail_json, but as stack traces. The latest ansible-core version might support this, but we also support older versions, and they do not. (I did thest that.)

Copy link
Author

Choose a reason for hiding this comment

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

Ah, okay. Thank you for all the fixes, I’ll work them in once I justifiably find time for this again (need to get paid work done first).

module.fail_json(**r)

# 15 is the length of the minimum response {"response":0} as formatted by KEA
if len(rsp) < 15:
r['msg'] = 'unrealistically short response ' + repr(rsp)
module.fail_json(**r)

try:
r['response'] = json.loads(rsp, parse_constant=_parse_constant)
except ValueError as ex:
r['msg'] = 'error parsing JSON response: ' + str(ex)
r['exception'] = ex
module.fail_json(**r)
if not isinstance(r['response'], dict):
r['msg'] = 'bogus JSON response (JSONObject expected)'
module.fail_json(**r)
if 'result' not in r['response']:
r['msg'] = 'bogus JSON response (missing result)'
module.fail_json(**r)
res = r['response']['result']
if not isinstance(res, int):
r['msg'] = 'bogus JSON response (non-integer result)'
module.fail_json(**r)

if res in rvok:
r['changed'] = False
elif res not in rvch:
r['msg'] = 'failure result (code %d)' % res
module.fail_json(**r)

module.exit_json(**r)


if __name__ == '__main__':
main()
15 changes: 15 additions & 0 deletions tests/integration/targets/kea_command/aliases
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# SPDX-License-Identifier: GPL-3.0-or-later

# Copyright © Thorsten Glaser <[email protected]>
# GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt)

# needs to be run in a debian:trixie container
unsupported

# sets up networks and services
needs/root
needs/privileged
destructive

# run this with:
# ansible-test integration --docker quay.io/ansible-community/test-image:debian-13-trixie --python 3.13 --docker-privileged -v unsupported/kea_command
138 changes: 138 additions & 0 deletions tests/integration/targets/kea_command/tasks/main.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
# SPDX-License-Identifier: GPL-3.0-or-later
---
####################################################################
# WARNING: These are designed specifically for Ansible tests #
# and should not be used as examples of how to write Ansible roles #
####################################################################

# Copyright © Thorsten Glaser <[email protected]>
# GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt)

- name: Ensure this is run on the right distro and version the integration test was written for
ansible.builtin.assert:
that:
- ansible_distribution_major_version | int == 13
- ansible_distribution | lower == 'debian'
fail_msg: "This test was written for a Debian 13 (trixie) container due to uses of its default configuration"
quiet: true
Comment on lines +11 to +17
Copy link
Collaborator

Choose a reason for hiding this comment

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

Easy as that:

Suggested change
- name: Ensure this is run on the right distro and version the integration test was written for
ansible.builtin.assert:
that:
- ansible_distribution_major_version | int == 13
- ansible_distribution | lower == 'debian'
fail_msg: "This test was written for a Debian 13 (trixie) container due to uses of its default configuration"
quiet: true
- name: Ensure this is run on the right distro and version the integration test was written for
ansible.builtin.meta: end_play
when: ansible_os_family != 'Debian' and ansible_distribution_major_version | int != 13

This way we can enable the test and, whenever Debian trixie is enabled in the CI, it will already work.

Copy link
Collaborator

Choose a reason for hiding this comment

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

We won't have Debian Trixie as a VM, so it won't work :) But there are other OSes that we do have as a VM that should work already now (with potentially little changes, like Ubuntu 24.04).

Copy link
Collaborator

Choose a reason for hiding this comment

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

That sounds... final. Any reason not to use Trixie in tests?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Tje ansible-core testing infrastructure does not provide Debian VMs. We can run containers though (and we will run Debian Trixie containers).

The tests for this modlue do not run in containers, though.

Copy link
Author

Choose a reason for hiding this comment

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

Ow. If they don’t run in containers, there’s little chance I can do the things needed to start the KEA services in there.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@felixfontein I gave the thumbs up to your last comment, but then later it hit me: why? why won't the tests run in containers? I am not familiar with KEA, so I am assuming something in this tool that prevents that from happening? Glancing over the test code I could not see anything else that would justify that - but I really just glanced quickly, I may have missed something.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@russoz the tests set up a new networking device, that isn't supported in "normal" containers (without additional capabilities, or privileged mode): #10709 (comment)

Since we want that containerized tests "just work" if you run ansible-test integration --docker <right image> <test name>, without having to supply specific additional options to give more capabilities to the container (and especially without making it a privileged container), it won't run in CI this way.


- name: Install prerequisites
ansible.builtin.apt:
name:
- iproute2
state: present
install_recommends: false
update_cache: true

- name: Networking setup, interface
ansible.builtin.command:
cmd: "ip link add eth666 type dummy"
changed_when: true

- name: Networking setup, IPv4
ansible.builtin.command:
cmd: "ip addr add 192.0.2.1/24 dev eth666"
changed_when: true

- name: Networking setup, link
ansible.builtin.command:
cmd: "ip link set up dev eth666"
changed_when: true

- name: Install KEA servers for DHCP and DHCPv6
ansible.builtin.apt:
name:
- kea-dhcp4-server
- kea-dhcp6-server
state: present
install_recommends: false

- name: Set up dhcp4 server, network
ansible.builtin.lineinfile:
firstmatch: true
insertafter: '"interfaces-config": [{]'
line: '"interfaces": [ "eth666" ]'
path: /etc/kea/kea-dhcp4.conf
search_string: '"interfaces": ['

- name: Set up dhcp4 server, hooks
ansible.builtin.lineinfile:
firstmatch: true
insertbefore: '"subnet4": '
line: '"hooks-libraries": [ { "library": "libdhcp_lease_cmds.so" } ],'
path: /etc/kea/kea-dhcp4.conf
regexp: '^ *"hooks-libraries":'

- name: Ensure the dhcp4 server is (re)started
ansible.builtin.service:
name: kea-dhcp4-server
state: restarted

- name: Ensure the dhcp6 server is (re)started
ansible.builtin.service:
name: kea-dhcp6-server
state: restarted

# an example for a request acquiring information
- name: Get KEA DHCP6 status
kea_command:
command: status-get
rv_unchanged: [0]
socket: /run/kea/kea6-ctrl-socket
register: kea6_status
- name: Display registered status result
ansible.builtin.debug:
msg: KEA DHCP6 running on PID {{ kea6_status.response.arguments.pid }}

# ensure socket option works
- name: Get KEA DHCP4 status
kea_command:
command: status-get
rv_unchanged: [0]
socket: /run/kea/kea4-ctrl-socket
register: kea4_status
- name: Ensure dhcp4 and dhcp6 PIDs are different
ansible.builtin.assert:
that:
- kea4_status.response.arguments.pid is integer
- kea4_status.response.arguments.pid > 0
- kea6_status.response.arguments.pid is integer
- kea6_status.response.arguments.pid > 0
- kea4_status.response.arguments.pid != kea6_status.response.arguments.pid
fail_msg: 'PIDs are invalid or do not differ (4: {{ kea4_status.response.arguments.pid }}, 6: {{ kea6_status.response.arguments.pid }})'
success_msg: 'PIDs differ (4: {{ kea4_status.response.arguments.pid }}, 6: {{ kea6_status.response.arguments.pid }})'

# an example for requests modifying state
- name: Remove existing leases for 192.0.2.66, if any
kea_command:
command: lease4-del
arguments:
ip-address: "192.0.2.66"
rv_changed: [0]
rv_unchanged: [3]
- name: Add DHCP lease for 192.0.2.66
kea_command:
command: lease4-add
arguments:
ip-address: "192.0.2.66"
hw-address: "00:00:5E:00:53:00"
rv_changed: [0]
register: lease_add

- name: An unknown command
kea_command:
command: get-status
rv_unchanged: [0]
register: uc_status
ignore_errors: true

- name: Check results
ansible.builtin.assert:
that:
- kea6_status is not changed
- kea6_status is not failed
- kea4_status is not changed
- kea4_status is not failed
- lease_add is changed
- lease_add is not failed
- uc_status is failed