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

Add optional alias parameter to host config #355

Merged
merged 57 commits into from
Aug 20, 2022

Conversation

simonfelding
Copy link
Contributor

@simonfelding simonfelding commented Aug 2, 2022

this is useful for weird ssh proxies like cyberark PAM. Without this, it is difficult to identify the source of the output, as they all have the same hostname.

As a side effect, this also adds part of the future ~/.ssh/config parsing, which would be nice (as per #103). the openssh config differentiates between host and hostname.

I wrote this code that implements simple ssh config parsing with unix-like pattern matching.

    hosts = []
    host_config = []
    i = 0

    with open(Path.expanduser(Path('~/.ssh/config'))) as sshconfig:
        pattern = sys.argv[1].strip('"')
        for line in sshconfig:
            if line.startswith("Host "):
                host = line.split("Host ")[1].split('\n')[0]
                if fnmatch.fnmatch(host,pattern):
                    hostname = next(sshconfig).strip().split("HostName ")[1]
                    user = next(sshconfig).strip().split("User ")[1]
                    hosts.append({})
                    hosts[i]["hostname"] = hostname
                    hosts[i]["user"] = user
                    hosts[i]['alias'] = host
                    i = i+1
        if hosts == []:
            print(f"No hosts matched by pattern {pattern}")
            exit(1)
        for host in hosts:
            host_config.append(HostConfig(user=host['user'], password=os.environ['SSHPASS']))

@simonfelding
Copy link
Contributor Author

simonfelding commented Aug 2, 2022

I'm not entirely sure how to fix the test errors but it should be trivial :)
Edit: It was trivial, but I'm an idiot. Now it works :)

@simonfelding simonfelding changed the title Add alias parameter to host config Add optional alias parameter to host config Aug 2, 2022
@codecov
Copy link

codecov bot commented Aug 2, 2022

Codecov Report

Merging #355 (5b1be0e) into master (cd9836d) will increase coverage by 0.31%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #355      +/-   ##
==========================================
+ Coverage   99.19%   99.50%   +0.31%     
==========================================
  Files          18       18              
  Lines        1606     1613       +7     
==========================================
+ Hits         1593     1605      +12     
+ Misses         13        8       -5     
Impacted Files Coverage Δ
pssh/clients/native/parallel.py 100.00% <ø> (ø)
pssh/clients/ssh/parallel.py 100.00% <ø> (ø)
pssh/clients/ssh/single.py 99.30% <ø> (+0.69%) ⬆️
pssh/clients/base/parallel.py 99.14% <100.00%> (+0.43%) ⬆️
pssh/clients/base/single.py 99.24% <100.00%> (+0.50%) ⬆️
pssh/clients/native/single.py 100.00% <100.00%> (+0.28%) ⬆️
pssh/config.py 100.00% <100.00%> (ø)
pssh/output.py 100.00% <100.00%> (ø)
... and 2 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@pkittenis
Copy link
Member

Hi there,

Thanks for the interest and the PR.

This looks like useful functionality, and your code snippet above is a good first step for config parsing too. That one might be worth a separate PR if you're interested in making one.

I left some comments on the code.

@simonfelding
Copy link
Contributor Author

Thank you! I'll make a seperate PR for parsing a ssh config file when this one gets pulled to master :) It would be useful indeed.

I don't see any comments on the code, did you use the review function on github or what?

pssh/clients/base/parallel.py Outdated Show resolved Hide resolved
tests/native/test_parallel_client.py Outdated Show resolved Hide resolved
@simonfelding
Copy link
Contributor Author

simonfelding commented Aug 6, 2022

Removed all the references to alias in parallel configs as per your review.
I made a new single client test and it works as expected now :)

It throws an error in the host config test but I think it's actually supposed to. Shouldn't clients that fail still return parameters like alias when stop_on_errors=False?

I'm not entirely sure where in the code to fix this, maybe you can help me out here? :)

Edit:
Fixed it :)

pssh/clients/native/single.py Outdated Show resolved Hide resolved
pssh/clients/ssh/single.py Outdated Show resolved Hide resolved
pssh/config.py Outdated Show resolved Hide resolved
pssh/config.py Outdated Show resolved Hide resolved
pssh/output.py Outdated Show resolved Hide resolved
tests/test_host_config.py Outdated Show resolved Hide resolved
@pkittenis
Copy link
Member

Looks good, thank you again for the PR.

Made a small change to docstrings and accepted types for alias to only accept str types as aliases, not int, to be in line with SSH conventions.

@pkittenis pkittenis merged commit 1b44e9a into ParallelSSH:master Aug 20, 2022
@simonfelding simonfelding deleted the patch-1 branch August 20, 2022 10:34
@simonfelding
Copy link
Contributor Author

simonfelding commented Aug 20, 2022

Brilliant! I'll commit a ssh config parser in a new PR. Thank you for the pleasant collaboration 😁

SvanT pushed a commit to dathost/parallel-ssh that referenced this pull request Sep 8, 2022
* added optional alias parameter to single clients and HostConfig for configuration from parallel clients.
  This is useful for weird ssh proxies like cyberark PAM.
  Without this, it is difficult to identify the source of the output, as they all have the same host name.
* Added tests.
* Updated docstrings.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants