Skip to content

ShellDriver: add optional arg dest_authorized_keys #1631

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

Jump to bottom
Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

ShellDriver: add optional arg dest_authorized_keys #1631

wants to merge 2 commits into from

Conversation

pmelange
Copy link
Contributor

@pmelange pmelange commented Mar 10, 2025 •
edited
Loading

ShellDriver: add optional arg dest_authorized_keys

Add an addition optional arg "dest_authorized_keys" with the default value of ~/.ssh/authorized_keys,

An example where this might be used is when an embedded system uses dropbear as it's ssh server daemon. In such a case, the authorized_keys file is /etc/dropbear/authorized_keys

Additionally, the put_key_file takes an optional dest_authorized_keys function parameter which defaults to the above arg.

Update docs

Checklist

  • Documentation for the feature
  • Tests for the feature: runtime test with the arg set and without the arg set.
  • PR has been tested locally

Fixes: #1626

pmelange added 2 commits March 10, 2025 13:20
@pmelange
ShellDriver: add optional arg dest_authorized_keys
cf1add0 
Add an addition optional arg "dest_authorized_keys" with the default
value of ```~/.ssh/authorized_keys```,

An example where this might be used is when an embedded system uses
dropbear as it's ssh server daemon.  In such a case, the authorized_keys
file is ```/etc/dropbear/authorized_keys```

Additionally, the ```put_key_file``` takes an optional
```dest_authorized_keys``` function parameter which defaults to the above arg.

Fixes: #labgrid-project#1626

Signed-off-by: Perry Melange <[email protected]>
@pmelange
docs: ShellDriver: add documentation about the new dest_authorized_ke…
cdd396c 
…ys arg

Signed-off-by: Perry Melange <[email protected]>
@pmelange pmelange mentioned this pull request Mar 10, 2025
Open
@Emantor
Copy link
Member

Emantor commented Apr 8, 2025

Isn't a keyword argument with default for the put_ssh_key() function not enought to achieve this? Do we need the additional driver argument?

@pmelange
Copy link
Contributor Author

pmelange commented Apr 8, 2025

Isn't a keyword argument with default for the put_ssh_key() function not enought to achieve this? Do we need the additional driver argument?

put_ssh_key() is called from on_activate(). So, no. I don't think it would work as you suggested.

labgrid/labgrid/driver/shelldriver.py

Lines 70 to 75 in 0b9dd26

if self.keyfile:
keyfile_path = self.keyfile
if self.target.env:
keyfile_path = self.target.env.config.resolve_path(self.keyfile)
self._put_ssh_key(keyfile_path)

@Emantor
Copy link
Member

Emantor commented Apr 9, 2025

Making this an automatic upload on keyfile availability on activation is a design decision I'd like to revise with the original author. I'll have stern talking to with myself, your implementation looks fine and changing this is not something I want to do at this point in time. Is there a specific reason for switching to """ for the f-strings?
The logging functions will also need to be fixed to not use f-strings.

@pmelange
Copy link
Contributor Author

pmelange commented Apr 9, 2025 •
edited
Loading

Making this an automatic upload on keyfile availability on activation is a design decision I'd like to revise with the original author. I'll have stern talking to with myself, your implementation looks fine and changing this is not something I want to do at this point in time. Is there a specific reason for switching to """ for the f-strings? The logging functions will also need to be fixed to not use f-strings.

I can imagine that removing the put_ssh_key() from on_activate() will break a lot of people's code. So making that change, which is totally the decision of the maintainers, is totally fine with me as I would be aware of it and can modify my code.

I used the """ f-strings because it seems (from the code that I have looked at within labgrid) that it is the most-often style used. I personally also find that style quite readable. Could you please explain what you mean by "The logging functions will also need to be fixed to not use f-strings"?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Request: ShellDriver: put_ssh_key would benefit from a destination file attribute
2 participants
@pmelange @Emantor