Skip to content

Add ssh-agent launching, and ssh-agent python client #82181

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

Closed
wants to merge 1 commit into from

Conversation

sivel
Copy link
Member

@sivel sivel commented Nov 9, 2023

SUMMARY

This PR adds:

  1. the ability to InventoryManager to spawn an ssh-agent process, or re-use an existing ssh-agent process
  2. a pure python ssh-agent client for interacting with an ssh-agent socket, for management of keys, as well as locking and unlocking the agent
  3. support to the SSH connection plugin for adding keys stored in YAML to the agent
  4. This functionality would permit any controller side plugins to interact with the agent, potentially populating it during inventory parsing, in an action plugin for forwarding an agent key to avoid transferring the key to the remote, or within a connection plugin.
ISSUE TYPE
  • Feature Pull Request
ADDITIONAL INFORMATION

Related: https://datatracker.ietf.org/doc/html/draft-miller-ssh-agent

Notes:

  • Client does not support the openssh extensions to agent constraints
  • Client does not support private key signing operations

Supersedes #52739.

@ansibot ansibot added feature This issue/PR relates to a feature request. needs_triage Needs a first human triage before being processed. labels Nov 9, 2023
@ansibot ansibot added the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Nov 9, 2023
@bcoca bcoca removed the needs_triage Needs a first human triage before being processed. label Nov 9, 2023
@ansibot ansibot removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Nov 9, 2023
@webknjaz webknjaz self-requested a review November 9, 2023 19:55
@webknjaz
Copy link
Member

webknjaz commented Nov 9, 2023

FTR, I've started a review draft, will post once I get to completing it..

@@ -161,13 +165,59 @@ def __init__(self, loader, sources=None, parse=True, cache=True):
else:
self._sources = sources

self._launch_ssh_agent()
Copy link
Member

Choose a reason for hiding this comment

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

I think this belongs does in prefork but not this early, move to strategy? (really in a connection manager as part or execution engine) as inventory does not require connection to the hosts at all. It can provide the info but should not be in charge of setting up communications or their prereqs. The good thing is that this code is easy to move later on, so I do not think it is a blocker for the PR right now.

env:
- name: ANSIBLE_PRIVATE_KEY
vars:
- name: ansible_private_key
Copy link
Member

Choose a reason for hiding this comment

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

add ansible_ssh_private_key options with higher precedence than the generic, this way we can reuse for other plugins and still have 'plugin specific' option to override or distinguish

version_added: '2.17'
private_key_passphrase:
description:
- private key passphrase, dependent on ``private_key``.
Copy link
Member

Choose a reason for hiding this comment

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

to reinforce, mention it does NOT work with private_key_file

@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Nov 22, 2023
@ansibot ansibot added the needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html label Mar 26, 2024
@bcoca bcoca mentioned this pull request May 1, 2024
@sivel sivel force-pushed the ssh_agent branch 3 times, most recently from 1667889 to 79ab013 Compare November 20, 2024 22:08
@ansibot ansibot added ci_verified Changes made in this PR are causing tests to fail. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. labels Nov 20, 2024
@ansibot

This comment was marked as outdated.

@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Dec 4, 2024
@ansibot ansibot removed ci_verified Changes made in this PR are causing tests to fail. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. labels Dec 5, 2024
@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Dec 19, 2024
@webknjaz
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@webknjaz webknjaz removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Jan 21, 2025
@ansibot ansibot removed the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Jan 21, 2025
@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Feb 4, 2025
@sivel
Copy link
Member Author

sivel commented Mar 4, 2025

Closing in favor of #84754 where this feature is being completed.

@sivel sivel closed this Mar 4, 2025
@github-project-automation github-project-automation bot moved this from Phase 2 (Dec 16 - Feb 14) to Done in ansible-core 2.19 Mar 4, 2025
@github-project-automation github-project-automation bot moved this from 🧐 @webknjaz's review queue 📋 to 🌈 Done 🦄 in 📅 Procrastinating in public Mar 4, 2025
@ansible ansible locked and limited conversation to collaborators Apr 30, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature This issue/PR relates to a feature request. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

6 participants