coder / modules

A collection of Terraform Modules to extend Coder templates.
https://registry.coder.com
Apache License 2.0
33 stars 33 forks source link

feat(filebrowser): check if already installed #334

Closed IamTaoChen closed 3 weeks ago

IamTaoChen commented 3 weeks ago

Add check if the filebrowser exist

IamTaoChen commented 3 weeks ago

Done

matifali commented 3 weeks ago

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 commented 3 weeks ago

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 commented 3 weeks ago

@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 commented 3 weeks ago

Now, it passed the test

IamTaoChen commented 3 weeks ago

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