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.

Sign up for GitHub

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

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

Merged
merged 12 commits into from
Jan 22, 2025
Merged

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

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
3d4ba05
Add parameter sudo to inventory plugin iocage #9572
vbotka Jan 14, 2025
c2befc2
Add changelog fragment.
vbotka Jan 14, 2025
03685bb
Fix error: Expected string in description of sudo.
vbotka Jan 14, 2025
4c0798d
Fix No2 error: Expected string in description of sudo.
vbotka Jan 14, 2025
98a09cc
Fix documentation type bool
vbotka Jan 17, 2025
a5a2a7c
Update changelogs/fragments/9573-iocage-inventory-sudo.yml
vbotka Jan 19, 2025
7380684
Add option sudo_preserve_env default=true
vbotka Jan 19, 2025
dbdf233
Fix DOCUMENTATION.
vbotka Jan 19, 2025
847e5ef
Set sudo_preserve_env default=false.
vbotka Jan 20, 2025
4543dd8
Update changelogs/fragments/9573-iocage-inventory-sudo.yml
vbotka Jan 20, 2025
91c774a
Update plugins/inventory/iocage.py
vbotka Jan 20, 2025
4dd4664
Update plugins/inventory/iocage.py
vbotka Jan 20, 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 changelogs/fragments/9573-iocage-inventory-sudo.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
minor_changes:
- iocage inventory plugin - the new parameter ``sudo`` of the plugin lets the command ``iocage list -l`` to run as root on the iocage host. This is needed to get the IPv4 of a running DHCP jail (https://github.com/ansible-collections/community.general/issues/9572).
38 changes: 35 additions & 3 deletions plugins/inventory/iocage.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,14 +46,28 @@
O(host) with SSH and execute the command C(iocage list).
This option is not required if O(host) is V(localhost).
type: str
sudo:
description:
- Enable execution as root.
- This requires passwordless sudo of the command C(iocage list*).
type: bool
default: false
sudo_preserve_env:
description:
- Preserve environment if O(sudo) is enabled.
- This requires C(SETENV) sudoers tag.
type: bool
default: false
get_properties:
description:
- Get jails' properties.
Creates dictionary C(iocage_properties) for each added host.
type: boolean
type: bool
Copy link
Collaborator

Choose a reason for hiding this comment

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

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).

Copy link
Contributor Author

@vbotka vbotka Jan 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

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.

Copy link
Collaborator

Choose a reason for hiding this comment

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

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

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 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

Copy link
Collaborator

Choose a reason for hiding this comment

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

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).

Copy link
Contributor Author

@vbotka vbotka Jan 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

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).

Copy link
Collaborator

Choose a reason for hiding this comment

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

It is confusing indeed.

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

default: false
env:
description: O(user)'s environment on O(host).
description:
- O(user)'s environment on O(host).
- Enable O(sudo_preserve_env) if O(sudo) is enabled.
type: dict
default: {}
notes:
Expand Down Expand Up @@ -88,6 +102,17 @@
env:
CRYPTOGRAPHY_OPENSSL_NO_LEGACY: 1

---
# execute as root
# sudoers example 'admin ALL=(ALL) NOPASSWD:SETENV: /usr/local/bin/iocage list*'
plugin: community.general.iocage
host: 10.1.0.73
user: admin
sudo: true
sudo_preserve_env: true
env:
CRYPTOGRAPHY_OPENSSL_NO_LEGACY: 1

---
# enable cache
plugin: community.general.iocage
Expand Down Expand Up @@ -196,6 +221,8 @@ def parse(self, inventory, loader, path, cache=True):

def get_inventory(self, path):
host = self.get_option('host')
sudo = self.get_option('sudo')
sudo_preserve_env = self.get_option('sudo_preserve_env')
env = self.get_option('env')
get_properties = self.get_option('get_properties')

Expand All @@ -208,9 +235,13 @@ def get_inventory(self, path):
cmd.append("ssh")
cmd.append(f"{user}@{host}")
cmd.extend([f"{k}={v}" for k, v in env.items()])
cmd.append(self.IOCAGE)

cmd_list = cmd.copy()
if sudo:
cmd_list.append('sudo')
if sudo_preserve_env:
cmd_list.append('--preserve-env')
cmd_list.append(self.IOCAGE)
cmd_list.append('list')
cmd_list.append('--long')
try:
Expand All @@ -233,6 +264,7 @@ def get_inventory(self, path):
if get_properties:
for hostname, host_vars in results['_meta']['hostvars'].items():
cmd_get_properties = cmd.copy()
cmd_get_properties.append(self.IOCAGE)
cmd_get_properties.append("get")
cmd_get_properties.append("--all")
cmd_get_properties.append(f"{hostname}")
Expand Down
Loading