Allow NetworkService to be used without an explicit SSH username#1863
Open
ozan956 wants to merge 2 commits into
Open
Allow NetworkService to be used without an explicit SSH username#1863ozan956 wants to merge 2 commits into
NetworkService to be used without an explicit SSH username#1863ozan956 wants to merge 2 commits into
Conversation
So far, `NetworkService.username` was required and `SSHDriver` always passed it to `ssh` and `scp`. This prevented setups where the SSH username is intentionally resolved through the user's SSH configuration or by the default SSH user selection. This change makes `NetworkService.username` optional and updates `SSHDriver` to only pass `-l <username>` or `user@host` when a username is explicitly set. In addition, the client-side fallback `NetworkService` creation path no longer forces `username="root"` and now instantiates the resource with `name=None`, so ad-hoc SSH access can use the normal SSH configuration of the user. This improves compatibility with existing SSH setups and avoids hardcoding a username when it is not actually required. Add regression tests covering the no-username path in SSHDriver and the client-side fallback NetworkService creation. Signed-off-by: Ozan Durgut <ozan.durgut@analog.com>
Clarify that `NetworkService.username` is optional. If no username is configured on `NetworkService` or `SSHDriver`, labgrid lets SSH resolve the username through the local SSH configuration or the default SSH user selection. Signed-off-by: Ozan Durgut <ozan.durgut@analog.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1863 +/- ##
========================================
+ Coverage 45.8% 45.9% +0.1%
========================================
Files 182 182
Lines 14720 14738 +18
========================================
+ Hits 6743 6775 +32
+ Misses 7977 7963 -14
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Emantor
reviewed
May 31, 2026
Member
Emantor
left a comment
Emantor
left a comment
There was a problem hiding this comment.
This is going to be a breaking change for the remote infrastructure, older labgrid will expect the now optional username parameter. IMO we should still use None instead of "" which works around that.
| Arguments: | ||
| - address (str): hostname of the remote system | ||
| - username (str): username used by SSH | ||
| - username (str, default=""): optional, username used by SSH |
Member
There was a problem hiding this comment.
The default should be None, not the empty string.
| str(self.networkservice.port), "-l", self._get_username(), | ||
| self.networkservice.address] | ||
| str(self.networkservice.port)] | ||
| if self._get_username(): |
Member
There was a problem hiding this comment.
Suggested change
| if self._get_username(): | |
| if username := self._get_username(): |
| self.networkservice.address] | ||
| str(self.networkservice.port)] | ||
| if self._get_username(): | ||
| args += ["-l", self._get_username()] |
Member
There was a problem hiding this comment.
Suggested change
| args += ["-l", self._get_username()] | |
| args += ["-l", username] |
| self.networkservice.address | ||
| ] + cmd.split(" ") | ||
| "-p", str(self.networkservice.port)] | ||
| if self._get_username(): |
| @step(args=['filename', 'destination']) | ||
| def get(self, filename, destination="."): | ||
| source = f"{self.networkservice.address}:{filename}" | ||
| if self._get_username(): |
| """Starts a keepalive connection via the own or external master.""" | ||
| args = [self._ssh, *self.ssh_prefix, self.networkservice.address, "cat"] | ||
| args = [self._ssh, *self.ssh_prefix] | ||
| if self._get_username(): |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Allow
NetworkServiceto be used without an explicit SSH username.Until now,
NetworkService.usernamewas required andSSHDriveralways passedit through to
ssh,scp, etc. That prevented setups where the SSH user shouldcome from the local SSH configuration or the default SSH user resolution.
This change makes
NetworkService.usernameoptional and updatesSSHDrivertoonly pass
-l <username>oruser@hostwhen a username is explicitly set.The client-side fallback path in
labgrid.remote.clientalso no longer forcesusername="root". While adding regression coverage for that path, this alsosurfaced that the fallback resource creation needs
name=None, which is nowpassed explicitly.
I verified the change with some tests called
test_run_no_username,test_put_get_no_usernameandtest_get_ssh_no_usernamefor the no-username paths.Checklist