Set SSH working directory #738
Conversation
cmd/ssh.go
Outdated
| @@ -243,7 +244,7 @@ func (cmd *SSHCmd) startTunnel(ctx context.Context, devPodConfig *config.Config, | |||
| defer writer.Close() | |||
|
|
|||
| log.Debugf("Run outer container tunnel") | |||
| command := fmt.Sprintf("'%s' helper ssh-server --track-activity --stdio", agent.ContainerDevPodHelperLocation) | |||
| command := fmt.Sprintf("'%s' helper ssh-server --track-activity --stdio --workdir '%s'", agent.ContainerDevPodHelperLocation, filepath.Join("workspaces", workspaceName)) | |||
There was a problem hiding this comment.
On second thought filepath.Join("workspaces", workspaceName) is wrong because it's not an absolute path, but it's probably not right anyway... What would be the right way to get the directory?
There was a problem hiding this comment.
ContainerWorkspaceFolder is the only thing I could find but it seems available at a very different point in the lifecycle.
There was a problem hiding this comment.
You will need to make workspace dir absolute /workspaces.
I guess it's fine as a default, when users set workspaceFolder in the devcontainer.json, it won't be able to find this path and fall back to the default
There was a problem hiding this comment.
I think at this time in cmd we already know the workspaceFolder, we could already use that?
There was a problem hiding this comment.
yeah but we don't have a parsed devcontainer.json if I'm not mistaken
There was a problem hiding this comment.
That seems correct to me there doesn't seem to be a parsed devcontainer.json, only a devpod config.
I guess it's fine as a default, when users set workspaceFolder in the devcontainer.json, it won't be able to find this path and fall back to the default
Ideally it would correctly use that path if set... but it may require larger changes than this PR.
cmd/ssh.go
Outdated
| @@ -243,7 +244,7 @@ func (cmd *SSHCmd) startTunnel(ctx context.Context, devPodConfig *config.Config, | |||
| defer writer.Close() | |||
|
|
|||
| log.Debugf("Run outer container tunnel") | |||
| command := fmt.Sprintf("'%s' helper ssh-server --track-activity --stdio", agent.ContainerDevPodHelperLocation) | |||
| command := fmt.Sprintf("'%s' helper ssh-server --track-activity --stdio --workdir '%s'", agent.ContainerDevPodHelperLocation, filepath.Join("workspaces", workspaceName)) | |||
There was a problem hiding this comment.
You will need to make workspace dir absolute /workspaces.
I guess it's fine as a default, when users set workspaceFolder in the devcontainer.json, it won't be able to find this path and fall back to the default
|
Should be ready now. Thanks! Do you have an idea when this might be released? |
|
@frangio thank you. Not quite sure tbh but looking at the changes we've accumulated it's not too far away |
|
Do you think the E2E test failures are caused by these changes? |
Yep, will have a look tomorrow |
|
We need to check if Fleet is still working after this check as they expect the SSH root to be in HOME as thats the default for SSH I think. |
|
@FabianKramm fleet stilled worked for me and tests are passing now |
|
@frangio PR is ready to be merged now, thanks for your contribution. |
|
@frangio It's released in |
Fixes #730
I didn't go as far as adding a
--workdirargument todevpod sshas suggested in #730 (comment) because it isn't strictly necessary.Looking forward to feedback.
Should this be tested in any way?