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 send-env and set-env options to ssh command #1288

Merged
merged 1 commit into from
Oct 10, 2024

Conversation

aacebedo
Copy link
Contributor

Having similar options as SetEnv and SendEnv from the ssh client can be useful.

This PR adds those options to the ssh command.

@pascalbreuninger
Copy link
Member

Hey @aacebedo, thanks for the PR!
I'm wondering how this differs from the --workspace-env flag in devpod up, except that you can set env vars per-connection.

Do you have a specific use case in mind?

@aacebedo
Copy link
Contributor Author

My UC is the following.
I start a container during a CI step through a common template and I don't know before being actually used in the step which variables will be used.

With this, I am able to inject directly the variable I need when I write it.
In addition having temporary variables valid for a single command seems better than having to create a new container for each command (when using ssh --command option)

@bkneis
Copy link
Contributor

bkneis commented Oct 3, 2024

LGTM, both options are supported by SSH so I don't see why we shouldn't support

@bkneis bkneis self-requested a review October 3, 2024 11:00
cmd/ssh.go Outdated Show resolved Hide resolved
pkg/ssh/helper.go Outdated Show resolved Hide resolved
@aacebedo aacebedo force-pushed the aacebedo/add_sendenv_option branch 2 times, most recently from 26ab28a to ab35989 Compare October 4, 2024 18:59
@aacebedo
Copy link
Contributor Author

aacebedo commented Oct 8, 2024

I fixed the comment. Will it be possible te review please?

@aacebedo
Copy link
Contributor Author

aacebedo commented Oct 9, 2024

rebased on main and test E2E tests. It is now working

@pascalbreuninger
Copy link
Member

@aacebedo merged!

@pascalbreuninger pascalbreuninger merged commit 4e94be7 into loft-sh:main Oct 10, 2024
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants