Skip to content

ssh-copy-id feature added. #1

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

Open
abdullahenesoncu wants to merge 1 commit into
base: main
Choose a base branch
from
Open

ssh-copy-id feature added. #1

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
20 changes: 18 additions & 2 deletions livedev/livedev
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ def parse_args(args):
parser.add_argument('remotes', nargs='+',
help='remote hosts and paths to use')
parser.add_argument('-v', '--verbose', action='store_true')
parser.add_argument('-s', '--ssh-copy-id', action='store_true')
return parser.parse_args(args)

def match_events(events, expected):
Expand All @@ -62,15 +63,19 @@ def parse_checksum_output(output, relative=None):
return data

class Remote(object):
def __init__(self, host, workers=1, verbose=False):
def __init__(self, host, workers=1, verbose=False, ssh_copy_id=False):
# host, path = rpath.split(':', 1)
# self.rpath = rpath
self.host = host
# self.path = path
self.workers = workers
self.verbose = verbose
self.ssh_copy_id = ssh_copy_id
self.manager = None

if self.ssh_copy_id:
self.copy_ssh_id()
Comment on lines +76 to +77
Copy link
Owner

@Staphylo Staphylo Oct 3, 2022

Choose a reason for hiding this comment

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

I would prefer avoiding operations in the constructor of the Remote object this is bad style.
Could you move this logic in the main function before if args.init line 411

This way there is no need to add a new attribute in the class that doesn't matter after init.


def __str__(self):
return self.host

Expand Down Expand Up @@ -131,6 +136,15 @@ class Remote(object):
for action in actions:
action.run(self)

def create_ssh_id_if_not_exists(self):
cmd = 'ssh-keygen -t rsa -b 2048 -P "" -f ~/.ssh/id_rsa <<< n'
subprocess.run(cmd, shell=True)

def copy_ssh_id(self):
self.create_ssh_id_if_not_exists()
cmd = 'ssh-copy-id -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -i ~/.ssh/id_rsa.pub -f '+self.host
Copy link
Owner

@Staphylo Staphylo Oct 3, 2022

Choose a reason for hiding this comment

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

Please use the array format for subprocess as it's done above.
Also do not use shell= because it's unsafe and definetly not needed here.
Last note is to use f-strings instead of string cocatenation.

subprocess.run(cmd, shell=True)
Comment on lines +144 to +146
Copy link
Owner

@Staphylo Staphylo Oct 3, 2022

Choose a reason for hiding this comment

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

I prefer if the tool gracefully exits if there is no key rather than trying to create one.
It is not the place for this tool to create a ssh key in behalf of the user, especially with the risk of overriding an existing one.
Also the defaults that you set for the key are extremely low.
I for one do not believe in rsa anymore especially not below 8192 bits.


class RemoteManager(object):
def __init__(self, remotes, dry_run):
self.remotes = remotes
Expand Down Expand Up @@ -389,7 +403,9 @@ def main(args):
if not paths:
print('Please provide valid paths to monitor')
return
remotes = [Remote(remote, workers=args.workers, verbose=args.verbose) for remote in args.remotes]
remotes = [Remote(remote, workers=args.workers,
verbose=args.verbose, ssh_copy_id=args.ssh_copy_id)
for remote in args.remotes]
rmanager = RemoteManager(remotes, dry_run=args.dry_run)

if args.init:
Expand Down