feat(filebrowser): check if already installed

[IamTaoChen]

Add check if the filebrowser exist

[matifali]

Thank you for the contribution.
Can you please run bun update-version or ./scripts/update-version.sh.

[IamTaoChen]

Done

[matifali]

Hi @IamTaoChen could you also run bun install && bun fmt?
Then run bun test filebrowser/main.test.ts with docker installed to run tests for the module. Make sure they pass.

[IamTaoChen]

During testing, I encountered an error in main.tf related to the variable validation logic:

modules2 git:(main) ✗ bun test filebrowser/main.test.ts --verbose
bun test v1.1.33 (247456b6)

filebrowser/main.test.ts:
Terraform encountered problems during initialisation, including problems
with the configuration, described below.

The Terraform configuration must be valid before initialization so that
Terraform can determine which modules and providers need to be installed.
╷
│ Error: Invalid reference in variable validation
│ 
│   on main.tf line 27, in variable "agent_name":
│   27:     condition     = var.subdomain || var.agent_name != ""
│ 
│ The condition for variable "agent_name" can only refer to the variable itself, using var.agent_name.
╵

Terraform’s validation rules don’t allow a variable’s validation logic to reference other variables, such as var.subdomain. If I comment out or mask this part of the code, the test passes successfully. However, I’m unsure if masking it in this PR is the right approach.

Should I proceed by temporarily masking the validation to unblock the tests, or would you recommend another way to handle this dependency.

[matifali]

@IamTaoChen Let us remove the validation from agent_name and make it an optional variable with a default set to null.

I am actually doing some changes in https://github.com/coder/modules/blob/atif/qol-improvments/filebrowser/main.tf

Feel free to bring them in for filebrowser in your PR.

[IamTaoChen]

Now, it passed the test

[mafredri]

LGTM, thanks for your contribution!

[mafredri]

(No action, just commentary.)

This is nice, but unfortunately it's rare that users will benefit since it's usually installed to /usr/local/bin. For containers it's rarely persisted.

It'd be cool if we could store this in $CODER_SCRIPT_BIN_DIR instead. Usually this is within /tmp but it's possible to change by starting the coder agent with CODER_AGENT_SCRIPT_DATA_DIR=/path/to/persistent/dir.

This is something to consider for all modules, should we track this in a new issue @matifali?

[matifali]

This is nice, but unfortunately it's rare that users will benefit since it's usually installed to /usr/local/bin. For containers it's rarely persisted.

For air-gapped scenarios where the image already has it installed this is beneficial.

It'd be cool if we could store this in $CODER_SCRIPT_BIN_DIR instead. Usually this is within /tmp but it's possible to change by starting the coder agent with CODER_AGENT_SCRIPT_DATA_DIR=/path/to/persistent/dir.

I am a bit confused about $CODER_SCRIPT_BIN_DIR and $CODER_AGENT_SCRIPT_DATA_DIR. Do we have them already and how are the used? Or are you proposing this new concept.

[mafredri]

For air-gapped scenarios where the image already has it installed this is beneficial.

👍🏻

I am a bit confused about $CODER_SCRIPT_BIN_DIR and $CODER_AGENT_SCRIPT_DATA_DIR. Do we have them already and how are the used? Or are you proposing this new concept.

We have them.

Let's say you run the agent this way:

coder agent --script-data-dir=/home/coder/.local/coder-script

Now all coder scripts will be executed with the following environment variables:

CODER_SCRIPT_DATA_DIR=/home/coder/.local/coder-script/[UUID]
CODER_SCRIPT_BIN_DIR=/home/coder/.local/coder-script/bin

And the coder agent will automatically add /home/coder/.local/coder-script/bin to the PATH.

Even without specifying the script dir, a default will be used, you can verify this in your workspace:

ls /tmp/coder-script-data

[matifali]

This is super useful and we should start using it in our modules.

[IamTaoChen]

Hi I just noticed that /icon/filebrowser.svg doesn't exist on version of 2.16