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

lxd_connection: Allow non-root users to connect to an instance #9659

Merged
merged 32 commits into from
Feb 15, 2025

Conversation

yeetypete
Copy link
Contributor

@yeetypete yeetypete commented Jan 31, 2025

SUMMARY

Currently the lxd_connection only supports connecting to an instance as root. This PR extends the plugin to allow a non-root user, configurable via the ansible_user var to connect to the instance. The option lxd_become_method controls the command used to switch users, (su by default but could also be sudo -u). The defaults ensure the old behavior so this should be a non-breaking change.

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

lxd_connection

ADDITIONAL INFORMATION

If it is beneficial I would also be happy to add this functionality to the incus_connection and lxc_connection plugins.

@ansibullbot

This comment was marked as outdated.

@ansibullbot
Copy link
Collaborator

@ansibullbot ansibullbot added WIP Work in progress ci_verified Push fixes to PR branch to re-run CI connection connection plugin feature This issue/PR relates to a feature request new_contributor Help guide this first time contributor plugins plugin (any type) and removed ci_verified Push fixes to PR branch to re-run CI labels Jan 31, 2025
@ansibullbot
Copy link
Collaborator

@yeetypete this PR contains the following merge commits:

Please rebase your branch to remove these commits.

click here for bot help

@ansibullbot ansibullbot added merge_commit This PR contains at least one merge commit. Please resolve! needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html labels Jan 31, 2025
@yeetypete yeetypete marked this pull request as ready for review January 31, 2025 19:50
@ansibullbot ansibullbot removed WIP Work in progress merge_commit This PR contains at least one merge commit. Please resolve! needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html labels Jan 31, 2025
@felixfontein 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 31, 2025
Copy link
Collaborator

@felixfontein felixfontein left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution! Please add a changelog fragment. I've also added some first comments below.

@ansibullbot

This comment was marked as outdated.

@ansibullbot ansibullbot added ci_verified Push fixes to PR branch to re-run CI needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Feb 2, 2025
@ansibullbot ansibullbot removed ci_verified Push fixes to PR branch to re-run CI needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Feb 2, 2025
Copy link
Collaborator

@russoz russoz left a comment

Choose a reason for hiding this comment

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

Other than the already existing comments, LGTM

@yeetypete
Copy link
Contributor Author

fyi I've also replicated this functionality in the incus plugin: #9743

@felixfontein felixfontein merged commit 06df717 into ansible-collections:main Feb 15, 2025
125 checks passed
Copy link

patchback bot commented Feb 15, 2025

Backport to stable-10: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-10/06df717bc62fe3371acb2462b7694828b8e8196e/pr-9659

Backported as #9751

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

@felixfontein felixfontein removed the check-before-release PR will be looked at again shortly before release and merged if possible. label Feb 15, 2025
patchback bot pushed a commit that referenced this pull request Feb 15, 2025
* fix: add support for non-root user

* fix: show correct info for connection

* fix: use build_exec_command to execute as nonroot

* unset default user

* feat: add options for setting remote user and become method

* fix: add root as default remote_user

* fix: remove ansible_ssh_user from remote_user vars

* fix: use single quotes inside f-string

* fix: ensure lxc exec comes first

* fix: line length

* fix: use -c flag with su

* Update plugins/connection/lxd.py

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

* Update plugins/connection/lxd.py

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

* Update plugins/connection/lxd.py

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

* doc: add changelog fragment

* fix: use underscore for module name in fragment

* Update 9659-lxd_connection-nonroot-user.yml

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

* fix: add put command

* feat: add get_remote_uid_gid placeholder function

* feat: complete placeholder _get_remote_uid_gid function

* fix: better logging

* fix: ensure default values are of type str

* fix: use ints for uid and gid

* fix: print put command

* fix: format

* fix: display msg for PUT

* fix: add comment about defaults

* fix: format

* fix: use os module to get uid and gid

* Revert "fix: use os module to get uid and gid"

This reverts commit bb2ba14.

* Update plugins/connection/lxd.py

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

* fix: omit uid, gid args in lxd file push if root

---------

Co-authored-by: Felix Fontein <[email protected]>
(cherry picked from commit 06df717)
@felixfontein
Copy link
Collaborator

@yeetypete thanks for your contribution!

@felixfontein
Copy link
Collaborator

@russoz thanks for reviewing!

felixfontein pushed a commit that referenced this pull request Feb 15, 2025
…t users to connect to an instance (#9751)

lxd_connection: Allow non-root users to connect to an instance (#9659)

* fix: add support for non-root user

* fix: show correct info for connection

* fix: use build_exec_command to execute as nonroot

* unset default user

* feat: add options for setting remote user and become method

* fix: add root as default remote_user

* fix: remove ansible_ssh_user from remote_user vars

* fix: use single quotes inside f-string

* fix: ensure lxc exec comes first

* fix: line length

* fix: use -c flag with su

* Update plugins/connection/lxd.py

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

* Update plugins/connection/lxd.py

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

* Update plugins/connection/lxd.py

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

* doc: add changelog fragment

* fix: use underscore for module name in fragment

* Update 9659-lxd_connection-nonroot-user.yml

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

* fix: add put command

* feat: add get_remote_uid_gid placeholder function

* feat: complete placeholder _get_remote_uid_gid function

* fix: better logging

* fix: ensure default values are of type str

* fix: use ints for uid and gid

* fix: print put command

* fix: format

* fix: display msg for PUT

* fix: add comment about defaults

* fix: format

* fix: use os module to get uid and gid

* Revert "fix: use os module to get uid and gid"

This reverts commit bb2ba14.

* Update plugins/connection/lxd.py

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

* fix: omit uid, gid args in lxd file push if root

---------

Co-authored-by: Felix Fontein <[email protected]>
(cherry picked from commit 06df717)

Co-authored-by: Peter Siegel <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-10 Automatically create a backport for the stable-10 branch connection connection plugin feature This issue/PR relates to a feature request new_contributor Help guide this first time contributor plugins plugin (any type)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants