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

feat(cli): Add cli and context options to pass dotfile script env vars #1504

Merged

Conversation

charlysotelo
Copy link
Contributor

@charlysotelo charlysotelo commented Dec 22, 2024

My use case involves dotfile install scripts parametrized with secrets, which I don't want to commit to my dotfiles repository. Environment variables seemed like the obvious choice to pass secrets down to the agent during dotfile installation. I've done this by adding the DOTFILES_SCRIPT_ENV_FILES context option, and the equivalent --dotfiles-script-env-file option to the up command. For completeness sakes I've also added --dotfiles-script-env, which I recommend against using for secrets since this is logged.

Let me know what you think.

I do also have another branch with an --dotfiles-script-env-var option which would pass-through the corresponding environment variable defined in the environment invoking devpod-cli, but I didn't want to add bloat.

Here is me testing this PR:
Contents of dotfiles setup.sh:

【=◈︿◈=】@ C02FK0TXML7L ➜  cat dotfiles/setup.sh # https://github.com/charlysotelo/dotfiles
#!/bin/bash
echo "In dotfiles setup.sh..."
printenv

Configuring .env files and context:

【=◈︿◈=】@ C02FK0TXML7L ➜  git:(feature/dotfiles-scripts-env-v3) cat /Users/csotelo/my.env
TESTVAR1=TESTVALUE1
TESTVAR2=TESTVALUE2
【=◈︿◈=】@ C02FK0TXML7L ➜  git:(feature/dotfiles-scripts-env-v3) cat /Users/csotelo/myContext.env
CONTEXTVAR=CONTEXTVAL
【=◈︿◈=】@ C02FK0TXML7L ➜  git:(feature/dotfiles-scripts-env-v3) ./devpod-cli context set-options -o DOTFILES_SCRIPT_ENV_FILES=/Users/csotelo/myContext.env

up output, you can see the vars are defined in the install script:

【=◈︿◈=】@ C02FK0TXML7L ➜  git:(feature/dotfiles-scripts-env-v3) ./devpod-cli up --provider docker github.com/charlysotelo/devcontainer-test --ide none --dotfiles github.com/charlysotelo/dotfiles --dotfiles-script-env-file /Users/csotelo/my.env
21:32:04 info Creating devcontainer...
21:32:04 info Clone repository
21:32:04 info URL: https://github.com/charlysotelo/devcontainer-test
21:32:05 done Successfully cloned repository
21:32:05 info Couldn't find a devcontainer.json
21:32:05 info Try detecting project programming language...
21:32:05 info Detected project language 'None'
21:32:05 info Inspecting image mcr.microsoft.com/devcontainers/base:ubuntu
21:32:05 info 15a1ddff3e24ad0f5b57aaad400c0b5aff72fc7fc5f1918378e006e0c483ec78
21:32:08 info Setup container...
21:32:09 info Chown workspace...
21:32:09 info Run 'ssh devcontainer-test.devpod' to ssh into the devcontainer
21:32:09 info Dotfiles git repository github.com/charlysotelo/dotfiles specified
21:32:13 info Cloning dotfiles github.com/charlysotelo/dotfiles
21:32:14 info Failed to make install script ./install.sh executable: install script ./install.sh not found: exit status 1
21:32:14 info Failed to make install script ./install executable: install script ./install not found: exit status 1
21:32:14 info Failed to make install script ./bootstrap.sh executable: install script ./bootstrap.sh not found: exit status 1
21:32:14 info Failed to make install script ./bootstrap executable: install script ./bootstrap not found: exit status 1
21:32:14 info Failed to make install script ./script/bootstrap executable: install script ./script/bootstrap not found: exit status 1
21:32:14 info In dotfiles setup.sh...
21:32:14 info SHELL=/bin/bash
21:32:14 info DEVPOD_WORKSPACE_UID=default-de-a5e1f
21:32:14 info HOSTNAME=15a1ddff3e24
21:32:14 info SSH_AUTH_SOCK=/tmp/auth-agent1723931666/listener.sock
21:32:14 info PWD=/home/vscode/dotfiles
21:32:14 info LOGNAME=vscode
21:32:14 info HOME=/home/vscode
21:32:14 info REMOTE_CONTAINERS=true
21:32:14 info DEVPOD_WORKSPACE_ID=devcontainer-test
21:32:14 info CONTEXTVAR=CONTEXTVAL
21:32:14 info TESTVAR1=TESTVALUE1
21:32:14 info TESTVAR2=TESTVALUE2
21:32:14 info USER=vscode
21:32:14 info DEVPOD=true
21:32:14 info SHLVL=1
21:32:14 info PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
21:32:14 info MAIL=/var/mail/vscode
21:32:14 info _=/usr/bin/printenv
21:32:14 info Done setting up dotfiles into the devcontainer

@bkneis
Copy link
Contributor

bkneis commented Dec 23, 2024

Thanks for your contribution @charlysotelo! I believe this also fixes #1190. Your solution looks good to me! The only thing I am unsure on is the context option. I think the flags in up should suffice for the functionality and setting it in the context option could result in undesired environment variables in workspaces as well as less reproducible results. I think any per workspace options should be flags

@charlysotelo charlysotelo force-pushed the feature/dotfiles-scripts-env-v3 branch from fdd5ae8 to d26c5c0 Compare December 23, 2024 17:46
@charlysotelo
Copy link
Contributor Author

Thanks for the feedback @bkneis!

I've gone ahead and removed the context option. Let me know if you have any further thoughts for me.

Copy link
Member

@pascalbreuninger pascalbreuninger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@charlysotelo There are two small nitpicks from my side, other than that LGTM. Thank you!

cmd/up.go Outdated Show resolved Hide resolved
cmd/up.go Outdated Show resolved Hide resolved
@charlysotelo charlysotelo force-pushed the feature/dotfiles-scripts-env-v3 branch from ca0abef to dae820f Compare January 7, 2025 15:02
@charlysotelo charlysotelo force-pushed the feature/dotfiles-scripts-env-v3 branch from dae820f to 2fa86ea Compare January 7, 2025 15:11
@charlysotelo
Copy link
Contributor Author

@charlysotelo There are two small nitpicks from my side, other than that LGTM. Thank you!

I've gone ahead and rebased with your changes. Let me know if you have any other changes in mind. Thanks!

Copy link
Member

@pascalbreuninger pascalbreuninger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@pascalbreuninger pascalbreuninger merged commit 162ff9f into loft-sh:main Jan 8, 2025
17 of 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