Implement #9572 Add parameter sudo to inventory plugin iocage #9573

vbotka · 2025-01-14T13:46:21Z

SUMMARY

Implement #9572 Add parameter sudo to inventory plugin iocage.

ISSUE TYPE

Feature Pull Request

COMPONENT NAME

iocage.py

ADDITIONAL INFORMATION

Tested in Ubuntu 24.04

shell> ansible-test units --venv --python 3.12 tests/unit/plugins/inventory/test_iocage.py
Installing requirements for Python 3.12 [venv]
Unit test controller with Python 3.12
============================= test session starts ==============================
platform linux -- Python 3.12.3, pytest-8.3.4, pluggy-1.5.0
rootdir: /export/scratch/collections/ansible_collections/community/general
configfile: ../../../../../../tmp/ansible-test-h6dmhd94/ansible_test/_data/pytest/config/default.ini
plugins: mock-3.14.0, xdist-3.6.1
created: 4/4 workers
4 workers [5 items]

.....                                                                    [100%]
- generated xml file: /export/scratch/collections/ansible_collections/community/general/tests/output/junit/python3.12-controller-units.xml -
============================== 5 passed in 1.94s ===============================

changelogs/fragments/9573-iocage-inventory-sudo.yml

russoz · 2025-01-18T10:38:45Z

plugins/inventory/iocage.py

+            if env:
+                cmd_list.append('--preserve-env')


Hi @vbotka thanks for another contribution!

IIRC sudo allows you to specify which env vars should be passed. If that's feasible here, it would be better than preserving all env vars.

Thank you for pointing this out. Instead of optionally passing the selected environment I added the option sudo_preserve_env (default=true) to preserve the complete environment (default) or none. This reduces the complexity of this plugin. The administrators can:

configure the user's environment, including root, in the systems

leave env empty

disable sudo_preserve_env.

This solves the problem for now. We can add option sudo_preserve_env_list later if needed. I don't want to invest too much into this "iocage dhcp root non-sense".

Fair enough. I would only make the default for sudo_preserve_env set to false, so the default is the most secure of the two options. If an admin really needs to pass some env var, they must be explicit about it.

For the record, I consider these sudo options a workaround for the DHCP addresses. There is no reason an unprivileged user can't read a jail DHCP address like a fixed one. The problem is the dhclient lease file is not readable. For example,

# ls -la /zroot/iocage/jails/test_112/root/var/db/dhclient.leases.epair0b ---------- 1 root wheel 2835 Jan 20 17:31 /zroot/iocage/jails/test_112/root/var/db/dhclient.leases.epair0b

Therefore, the iocage developers decided to run jexec which needs root

# jexec ioc-test_112 ifconfig epair0b inet epair0b: flags=1008843<UP,BROADCAST,RUNNING,SIMPLEX,MULTICAST,LOWER_UP> metric 0 mtu 1500 options=8<VLAN_MTU> inet 10.1.0.147 netmask 0xffffff00 broadcast 10.1.0.255

A systemic approach would be to configure devd.conf inside a jail to create a public readable IP file, for example,

notify 0 { match "system" "IFNET"; match "subsystem" "epair0b"; match "type" "ADDR_ADD"; action "/usr/local/bin/devd_actions/write_ip.sh &"; };

Then, the inventory plugin could read the IP from this file.

felixfontein

Thanks for your contribution!

plugins/inventory/iocage.py

Co-authored-by: Alexei Znamensky <[email protected]>

plugins/inventory/iocage.py

changelogs/fragments/9573-iocage-inventory-sudo.yml

Co-authored-by: Felix Fontein <[email protected]>

felixfontein

Looks good from my side!

felixfontein · 2025-01-20T19:02:25Z

plugins/inventory/iocage.py

        get_properties:
            description:
              - Get jails' properties.
                Creates dictionary C(iocage_properties) for each added host.
-            type: boolean
+            type: bool


Out of curiosity, why did you change this to bool? Both bool and boolean work for plugins (only for modules you need to stick to bool).

Ansible Galaxy also doesn't recognize boolean, for example inventory iocage.

JFYI, the mess is bigger:

ansible.builtin.constructed option leading_separator type=boolean is resulting in missing default.

ansible.builtin.constructed option use_vars_plugins is missing completely. Also in "ansible-doc -t inventory community.general.iocage"

This is valid for all community.general inventory plugins which extend constructed

extends_documentation_fragment: - constructed

I don't think there is an option use_vars_plugins. I cannot find it either in ansible-core nor in community.general. Do you mean use_extra_vars?

The missing default is likely because boolean is not recognized as the same as bool and there's a check treating falsy values as an empty string... I'm wondering if the same happens with 0 and integer...

This is definitely a bug in galaxy_ng.

The check for boolean is missing here: https://github.com/ansible/ansible-hub-ui/blob/master/src/components/render-plugin-doc.tsx#L695

I don't think there is an option use_vars_plugins.

See

shell> ansible-doc -t inventory ansible.builtin.constructed

quoting:

use_vars_plugins Normally, for performance reasons, vars plugins get executed after the inventory sources complete the base inventory, this option allows for getting vars related to hosts/groups from those plugins. The host_group_vars (enabled by default) 'vars plugin' is the one responsible for reading host_vars/ and group_vars/ directories. This will execute all vars plugins, even those that are not supposed to execute at the 'inventory' stage. See vars plugins docs for details on 'stage'. Implicit groups, such as 'all' or 'ungrouped', need to be explicitly defined in any previous inventory to apply the corresponding group_vars default: false type: boolean

Ah, you mean the ansible.builtin.constructed inventory plugin, not the ansible.builtin.constructed docs fragment. The latter does not have this option, only the former. That's also why the iocage inventory plugin doesn't have this option either, because it's not part of the shared code, but restricted to the constructed inventory plugin (which also uses the shared code).

Right. It's counterintuitive. When I'm used to writing a constructed configuration I don't expect any option to be missing without warning. When used, it's silently ignored unless you enable "strict: true" (default: false).

It is confusing indeed.

About the boolean bug: I created ansible/ansible-hub-ui#5415 to fix it.

russoz

LGTM

patchback · 2025-01-22T19:04:03Z

Backport to stable-10: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-10/8f29976102d4ada43c1a639abab398183915cf34/pr-9573

Backported as #9605

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

felixfontein · 2025-01-22T19:04:10Z

@vbotka thanks for your contribution!
@russoz thanks for reviewing!

* Add parameter sudo to inventory plugin iocage #9572 * Add changelog fragment. * Fix error: Expected string in description of sudo. * Fix No2 error: Expected string in description of sudo. * Fix documentation type bool * Update changelogs/fragments/9573-iocage-inventory-sudo.yml Co-authored-by: Alexei Znamensky <[email protected]> * Add option sudo_preserve_env default=true * Fix DOCUMENTATION. * Set sudo_preserve_env default=false. * Update changelogs/fragments/9573-iocage-inventory-sudo.yml Co-authored-by: Felix Fontein <[email protected]> * Update plugins/inventory/iocage.py Co-authored-by: Felix Fontein <[email protected]> * Update plugins/inventory/iocage.py Co-authored-by: Felix Fontein <[email protected]> --------- Co-authored-by: Alexei Znamensky <[email protected]> Co-authored-by: Felix Fontein <[email protected]> (cherry picked from commit 8f29976)

… sudo to inventory plugin iocage (#9605) Implement #9572 Add parameter sudo to inventory plugin iocage (#9573) * Add parameter sudo to inventory plugin iocage #9572 * Add changelog fragment. * Fix error: Expected string in description of sudo. * Fix No2 error: Expected string in description of sudo. * Fix documentation type bool * Update changelogs/fragments/9573-iocage-inventory-sudo.yml Co-authored-by: Alexei Znamensky <[email protected]> * Add option sudo_preserve_env default=true * Fix DOCUMENTATION. * Set sudo_preserve_env default=false. * Update changelogs/fragments/9573-iocage-inventory-sudo.yml Co-authored-by: Felix Fontein <[email protected]> * Update plugins/inventory/iocage.py Co-authored-by: Felix Fontein <[email protected]> * Update plugins/inventory/iocage.py Co-authored-by: Felix Fontein <[email protected]> --------- Co-authored-by: Alexei Znamensky <[email protected]> Co-authored-by: Felix Fontein <[email protected]> (cherry picked from commit 8f29976) Co-authored-by: Vladimir Botka <[email protected]>

Add parameter sudo to inventory plugin iocage ansible-collections#9572

3d4ba05

ansibullbot added feature This issue/PR relates to a feature request inventory inventory plugin plugins plugin (any type) labels Jan 14, 2025

vbotka added 2 commits January 14, 2025 14:59

Add changelog fragment.

c2befc2

Fix error: Expected string in description of sudo.

03685bb

This comment was marked as outdated.

Sign in to view

ansibullbot added the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR label Jan 14, 2025

Fix No2 error: Expected string in description of sudo.

4c0798d

ansibullbot added the needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI label Jan 14, 2025

felixfontein added check-before-release PR will be looked at again shortly before release and merged if possible. backport-10 Automatically create a backport for the stable-10 branch labels Jan 14, 2025

ansibullbot removed needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Jan 14, 2025

Fix documentation type bool

98a09cc

russoz reviewed Jan 18, 2025

View reviewed changes

felixfontein reviewed Jan 18, 2025

View reviewed changes

plugins/inventory/iocage.py Outdated Show resolved Hide resolved

plugins/inventory/iocage.py Show resolved Hide resolved

vbotka and others added 4 commits January 19, 2025 12:53

Update changelogs/fragments/9573-iocage-inventory-sudo.yml

a5a2a7c

Co-authored-by: Alexei Znamensky <[email protected]>

Add option sudo_preserve_env default=true

7380684

Fix DOCUMENTATION.

dbdf233

Set sudo_preserve_env default=false.

847e5ef

felixfontein reviewed Jan 20, 2025

View reviewed changes

plugins/inventory/iocage.py Show resolved Hide resolved

plugins/inventory/iocage.py Show resolved Hide resolved

changelogs/fragments/9573-iocage-inventory-sudo.yml Outdated Show resolved Hide resolved

vbotka and others added 3 commits January 20, 2025 19:47

Update changelogs/fragments/9573-iocage-inventory-sudo.yml

4543dd8

Co-authored-by: Felix Fontein <[email protected]>

Update plugins/inventory/iocage.py

91c774a

Co-authored-by: Felix Fontein <[email protected]>

Update plugins/inventory/iocage.py

4dd4664

Co-authored-by: Felix Fontein <[email protected]>

felixfontein approved these changes Jan 20, 2025

View reviewed changes

felixfontein mentioned this pull request Jan 21, 2025

Plugin option choices: both 'bool' and 'boolean' are valid types for plugins, and should be treated the same ansible/ansible-hub-ui#5415

Open

russoz approved these changes Jan 22, 2025

View reviewed changes

felixfontein removed the check-before-release PR will be looked at again shortly before release and merged if possible. label Jan 22, 2025

felixfontein merged commit 8f29976 into ansible-collections:main Jan 22, 2025
138 checks passed

patchback bot mentioned this pull request Jan 22, 2025

[PR #9573/8f299761 backport][stable-10] Implement #9572 Add parameter sudo to inventory plugin iocage #9605

Merged

vbotka deleted the feature-iocage-sudo branch January 25, 2025 11:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement #9572 Add parameter sudo to inventory plugin iocage #9573

Implement #9572 Add parameter sudo to inventory plugin iocage #9573

vbotka commented Jan 14, 2025

This comment was marked as outdated.

russoz Jan 18, 2025

vbotka Jan 19, 2025 •

edited

Loading

russoz Jan 20, 2025

vbotka Jan 20, 2025

vbotka Jan 20, 2025 •

edited

Loading

felixfontein left a comment

felixfontein left a comment

felixfontein Jan 20, 2025

vbotka Jan 20, 2025 •

edited

Loading

felixfontein Jan 21, 2025

felixfontein Jan 21, 2025

vbotka Jan 21, 2025

felixfontein Jan 21, 2025

vbotka Jan 21, 2025 •

edited

Loading

felixfontein Jan 21, 2025

russoz left a comment

patchback bot commented Jan 22, 2025 •

edited

Loading

felixfontein commented Jan 22, 2025

Implement #9572 Add parameter sudo to inventory plugin iocage #9573

Implement #9572 Add parameter sudo to inventory plugin iocage #9573

Conversation

vbotka commented Jan 14, 2025

SUMMARY

ISSUE TYPE

COMPONENT NAME

ADDITIONAL INFORMATION

This comment was marked as outdated.

Choose a reason for hiding this comment

vbotka Jan 19, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vbotka Jan 20, 2025 • edited Loading

Choose a reason for hiding this comment

felixfontein left a comment

Choose a reason for hiding this comment

felixfontein left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vbotka Jan 20, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vbotka Jan 21, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

russoz left a comment

Choose a reason for hiding this comment

patchback bot commented Jan 22, 2025 • edited Loading

Backport to stable-10: 💚 backport PR created

felixfontein commented Jan 22, 2025

vbotka Jan 19, 2025 •

edited

Loading

vbotka Jan 20, 2025 •

edited

Loading

vbotka Jan 20, 2025 •

edited

Loading

vbotka Jan 21, 2025 •

edited

Loading

patchback bot commented Jan 22, 2025 •

edited

Loading