Skip to content

allow adding env variables via CLI #11338

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

Closed
matifali opened this issue Dec 25, 2023 · 5 comments
Closed

allow adding env variables via CLI #11338

matifali opened this issue Dec 25, 2023 · 5 comments
Assignees
Labels
cli Area: CLI

Comments

@matifali
Copy link
Member

matifali commented Dec 25, 2023

Context

Coder now has a resource named coder_env that can be used to inject environment variables into the workspace.

There is one use case where a module author would like to do this conditionally depending on if another binary is in the PATH. This could be used to install and configure vscode extensions for the code-server if it is already installed and inject the environment variables for automatic login.
One example is the JFrog VS Code extension that supports auto-login if we set the following environment variables.

JFROG_IDE_URL="https://jfrog.example.com"
JFROG_IDE_ACCESS_TOKEN="XXXXXXXXXXXXXXXXXXXXXXX"

Suggestion

Provide a CLI command coder env that can be used to set/unset environment variables within a coder_script.

This can then be used to set the environment variables conditionally.

if command -v code-server > /dev/null 2>&1; then
  code-server --install-extension jfrog.jfrog-vs code-extension
  coder env set JFROG_IDE_URL --body "https://jfrog.example.com"
  coder env set JFROG_IDE_ACCESS_TOKEN --body "XXXXXXXXXXXXXXXXXXXXXXX"
else
  echo "🤔 code-server is not installed, skipping JFrog extension installation."
fi

The syntax is based on the same pattern as gh variable set.

cc: @bpmct @mafredri @kylecarbs

Tip

As a bonus, this can also allow modifying $PATH, e.g.,
coder env set PATH --body "/tmp/code-server/bin/coder-server:$PATH"

@mafredri
Copy link
Member

@matifali Regarding implementation, should we limit the ability to invoke coder [agent] env set to coder_scripts only?

This would let us implement it in a way where the envs are ephemeral, i.e. they don't stick around if the agent is restarted (they need to be re-set by script). We also don't need any API endpoints on coderd to implement it.

Optionally we can also allow manual invocation within the workspace, but I think if a user needs that, dotfiles are probably a more appropriate location.

If we allow for other use-cases wrt setting envs, I'm afraid it might lead to confusion of when they apply and when they don't. For instance, does a workspace stop/start wipe the env, or persist it? Does it apply to all agents or only one? Etc.

@matifali
Copy link
Member Author

@mafredri Good questions.

should we limit the ability to invoke coder [agent] env set to coder_scripts only?

I think we should not limit the use but can assume that they will be primarily set in coder_script resources.

coder [agent] env set

What does this mean? It's not a good UX if a user manually specifies the agent name. If I am correct, coder_script or a coder_worksapce is linked to a single coder_agent, so it should be sufficient for a user to use coder env set only.

For instance, does a workspace stop/start wipe the env, or persist it?

A stop is usually stopping/destroying a VM or container so the envs will be destroyed. The user or the coder_script should set them again on the next start. We should not treat them as files, and I think it's okay to not persist them in this case.

Optionally we can also allow manual invocation within the workspace, but I think if a user needs that, dotfiles are probably a more appropriate location.

I agree. We should not block this use-case.

@mafredri
Copy link
Member

mafredri commented Jan 26, 2024

What does this mean? It's not a good UX if a user manually specifies the agent name. If I am correct, coder_script or a coder_worksapce is linked to a single coder_agent, so it should be sufficient for a user to use coder env set only.

Oh, sorry it was unclear, I was not referring to specifying the agent, instead I was suggesting we might put the command under coder agent env set instead of coder env set because we don't want to support it outside the workspace. (I think this might also have the benefit of giving better understanding of how it's applied, i.e. only to this running agent.)

I don't think it'd be nice UX if we had the command there, but it said "this can only be run inside the workspace", but then again, maybe that's OK.

I agree. We should not block this use-case.

By not blocking, are you referring to that we should allow manual coder agent env set invocations by the user?

@matifali
Copy link
Member Author

By not blocking, are you referring to that we should allow manual coder agent env set invocations by the user?

Yes

Oh, sorry it was unclear, I was not referring to specifying the agent, instead I was suggesting we might put the command under coder agent env set instead of coder env set because we don't want to support it outside the workspace. (I think this might also have the benefit of giving better understanding of how it's applied, i.e. only to this running agent.)

I still prefer the coder env set We can exit with a message that it's only allowed in a workspace. But probably you are right. coder env set can be used later once we decide to add user-scoped variables. So its ok to go with coder agent env set. your choice :)

What do you think @bpmct?

@mafredri
Copy link
Member

mafredri commented Feb 5, 2024

After a discussion on Slack we came to the conclusion that there's no real need for this at this time. It's preferable to set environments declaratively in Terraform. Setting them in run-time introduces a race between starting processes and scripts modifying the environment as changes won't propagate to old processes.

Use-cases like refreshing tokens can be by wrapping binaries that need to perform authentication. Such use-cases can be amended by #11131.

If the need arises, we can re-open.

@mafredri mafredri closed this as completed Feb 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli Area: CLI
Projects
None yet
Development

No branches or pull requests

2 participants