-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
plugins/modules/snap: improve documentation #8972
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Lincoln Wallace <[email protected]>
This comment was marked as outdated.
This comment was marked as outdated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, please see my inline comment.
Also please keep in mind that this is the documentation of the Ansible module and not a snap documentation. I'd keep the original tone which starts by explaining the action, e.g. "Set [snap] options with pattern ..." instead of "Snap options are a way to do ...".
plugins/modules/snap.py
Outdated
- Snap options are a way to configure snaps by using the C(key=value) syntax or C(snap:key=value). | ||
If a snap name is given, the option will be applied to that snap only. | ||
If the snap name is omitted, the options will be applied to all snaps listed in O(name). | ||
Options will only be applied to active snaps. | ||
Since snap applications run in a sandboxed environment, this is the method used to | ||
pass information to snapped applications regarding the desired configurations. | ||
These configurations can be applied either at the startup of the application, within the snap (via a configure hook) | ||
or during its execution if the application is designed to communicate with the snap API at runtime. | ||
- See U(https://snapcraft.io/docs/configuration-in-snaps) for more details about snap options and how to configure snaps. | ||
- See U(https://snapcraft.io/docs/supported-snap-hooks) for more details about configure hooks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Most added explanations are useful for snap developers rather than snap users. I'd keep https://snapcraft.io/docs/configuration-in-snaps as a doc reference for configuring snaps but drop everything else.
What is missing is an explanation of when the options are set. Is that after installation? In that case, snaps that have an enabled service which starts after installation may not receive the configuration until a restart. Some snap do restart their services once configuration is applied, but that is rare. Some other snaps may continue restarting until the expected config is set.
What does "Options will only be applied to active snaps." mean?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I second that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is missing is an explanation of when the options are set. Is that after installation?
Is there a situation where it's possible to set snap options before installing the snap, other than through the install hook (which in this case is irrelevant)? If I try, I get an error: snap "<snap-name>" not found
.
The install hook is irrelevant in this scenario because it's defined within the snap and doesn't depend on the user.
But anyway, the options are indeed set after the installation, as shown in the source code.
What does "Options will only be applied to active snaps." mean?
Looking at the source code, it seems that the intention of this is to say that these options will only be applied to snaps in which the state
field is present
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please see my suggestion here: #8972 (comment)
plugins/modules/snap.py
Outdated
- Confinement policy: This level of confinement is permissive, granting full system access, | ||
similar to that of traditionally packaged applications, such as Debian packages (those managed via the C(apt) command-line tool). | ||
This option corresponds to the C(--classic) argument of the C(snap install) command. | ||
It can only be specified when the task involves a single snap. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The explanation sounds like this option allows changing of the confinement. The flag is actually used to install snaps that use the classic confinement.
- Confinement policy: This level of confinement is permissive, granting full system access, | |
similar to that of traditionally packaged applications, such as Debian packages (those managed via the C(apt) command-line tool). | |
This option corresponds to the C(--classic) argument of the C(snap install) command. | |
It can only be specified when the task involves a single snap. | |
- Install a snap that has classic confinement. | |
- This level of confinement is permissive, granting full system access, | |
similar to that of traditionally packaged applications, such as Debian packages (those managed via the C(apt) command-line tool). | |
This option corresponds to the C(--classic) argument of the C(snap install) command. | |
- This option can only be specified when the task involves a single snap. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer us not to mention a specific OS as example. snap
can be used in RHEL, OpenSUSE and other distros as well. I think it is better to keep the wording neutral.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This explanation essentially mirrors the wording used in the snapcraft.io documentation: https://snapcraft.io/docs/classic-confinement.
I just wanted to clarify what it means to install using classic confinement, but I will expand on it by mentioning that this option installs a snap with classic confinement AND leave the sentence explaining what this entails.
@russoz, I will revise the wording to "traditionally packaged applications that don't use sandboxing mechanisms" and remove the part about Debian packages. Would that work for you?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This explanation essentially mirrors the wording used in the snapcraft.io documentation: snapcraft.io/docs/classic-confinement. I just wanted to clarify what it means to install using classic confinement, but I will expand on it by mentioning that this option installs a snap with classic confinement AND leave the sentence explaining what this entails.
hi @locnnil , while I appreciate your intention, I think we must be aware that the purpose of this documentation is NOT to explain how snap
-command works, but rather how to use the snap
-ansible-module. Someone who does not know how to use snap is unlikely going to use this documentation to learn it. So, it is more than OK to add references and links to more documentation about the snap
command, but too much info here might not make things better.
(Just occurred me a couple of minutes later: Copying the text adds an implicit task to us maintainers to check the source doc for updates and keeping the module docs synchronized. It's so much easier to just tell the user: read it over there!)
@russoz, I will revise the wording to "traditionally packaged applications that don't use sandboxing mechanisms" and remove the part about Debian packages. Would that work for you?
I use a couple snaps locally, I am far from being an expert on it. Although I can infer the meaning of this, based on experience, and of course we can google it (but I don't want to spend my time on that), I reckon
plugins/modules/snap.py
Outdated
- Confinement policy: This level of confinement is permissive, granting full system access, | ||
similar to that of traditionally packaged applications, such as Debian packages (those managed via the C(apt) command-line tool). | ||
This option corresponds to the C(--classic) argument of the C(snap install) command. | ||
It can only be specified when the task involves a single snap. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer us not to mention a specific OS as example. snap
can be used in RHEL, OpenSUSE and other distros as well. I think it is better to keep the wording neutral.
plugins/modules/snap.py
Outdated
- Snap options are a way to configure snaps by using the C(key=value) syntax or C(snap:key=value). | ||
If a snap name is given, the option will be applied to that snap only. | ||
If the snap name is omitted, the options will be applied to all snaps listed in O(name). | ||
Options will only be applied to active snaps. | ||
Since snap applications run in a sandboxed environment, this is the method used to | ||
pass information to snapped applications regarding the desired configurations. | ||
These configurations can be applied either at the startup of the application, within the snap (via a configure hook) | ||
or during its execution if the application is designed to communicate with the snap API at runtime. | ||
- See U(https://snapcraft.io/docs/configuration-in-snaps) for more details about snap options and how to configure snaps. | ||
- See U(https://snapcraft.io/docs/supported-snap-hooks) for more details about configure hooks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I second that.
Co-authored-by: Farshid Tavakolizadeh <[email protected]>
This comment was marked as outdated.
This comment was marked as outdated.
Signed-off-by: Lincoln Wallace <[email protected]>
@locnnil I jumped the gun a little bit, the PR is sill in draft. I should not be bothering you yet ;-) |
Signed-off-by: Lincoln Wallace <[email protected]>
This comment was marked as outdated.
This comment was marked as outdated.
Co-authored-by: Farshid Tavakolizadeh <[email protected]>
Co-authored-by: Alexei Znamensky <[email protected]>
This comment was marked as outdated.
This comment was marked as outdated.
No problem at all, I appreciate your feedback, thank you! |
Signed-off-by: Lincoln Wallace <[email protected]>
The test
The test
The test
The test
The test
The test
The test
The test
The test
The test
The test
The test
The test
The test
The test
The test
The test
The test
|
@@ -29,10 +30,9 @@ | |||
name: | |||
description: | |||
- Name of the snaps to be installed. | |||
- Any named snap accepted by the C(snap) command is valid. | |||
- - Any named snap accepted by the C(snap) command is valid. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- - Any named snap accepted by the C(snap) command is valid. | |
- Any named snap accepted by the C(snap) command is valid. |
notes: | ||
- When performing privileged operations, such as installing snap packages or setting snap options, | ||
it is necessary to escalate privileges by executing these operations as root. | ||
This can be achieved by using V(become: true) in the module. | ||
- For more details on privilege escalation in playbooks, | ||
refer to U(https://docs.ansible.com/ansible/latest/playbook_guide/playbooks_privilege_escalation.html). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple of things:
- Privilege escalation is only relevant if the user is non-root.
- For a non-privileged user, it is needed only when the user hasn't logged in to the snap store (
snap login
). For example, the user is always logged in when using a pre-built Ubuntu Core image that uses Console Conf. This can be checked withsnap whoami
. become: true
is a task parameter, not module.- I combined the bullets because this is all a single note.
notes: | |
- When performing privileged operations, such as installing snap packages or setting snap options, | |
it is necessary to escalate privileges by executing these operations as root. | |
This can be achieved by using V(become: true) in the module. | |
- For more details on privilege escalation in playbooks, | |
refer to U(https://docs.ansible.com/ansible/latest/playbook_guide/playbooks_privilege_escalation.html). | |
notes: | |
- Privileged operations, such as installing and configuring snaps, require root priviledges. | |
This is only the case if the user hasn't logged in to the Snap Store. | |
The privilege can be escalated by setting V(become: true) in the task. | |
For more details on privilege escalation in playbooks, | |
refer to U(https://docs.ansible.com/ansible/latest/playbook_guide/playbooks_privilege_escalation.html). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of U(https://docs.ansible.com/ansible/latest/playbook_guide/playbooks_privilege_escalation.html)
, please use R(the documentation on playbook privilege escalation, playbooks_privilege_escalation)
. That will generate links to the documentation of the correct Ansible version in the HTML docs.
(Probably better reformulate the whole sentence.)
SUMMARY
Improve snap module documentation.
ISSUE TYPE
COMPONENT NAME
module: community.general.snap