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

Add helper functions to print to log/print to stderr #331

Open mafredri opened 4 weeks ago

mafredri commented 4 weeks ago

While reviewing https://github.com/coder/modules/pull/329 I noticed we didn't print any errors to stderr. I suggest we add helper functions for this:

# lib
error() {
    echo "ERROR: $@" >&2
    exit 1 # (optional)
}

log() {
    echo "$@"
}

Then use them where appropriate:

log "Installing package..."
install || error "Failed to install package"

// cc @matifali

matifali commented 4 weeks ago

Great idea. To make these helper functions available across modules. We may have to make used a common module that gets nested in each module.

A similar proposal is #312

mafredri commented 4 weeks ago

We may have to make used a common module that gets nested in each module.

I’ve been thinking about this on and off myself. A common module is definitely one option, but I’ve also been entertaining the idea of using code-gen to bundle the common code into all modules. Not sure what would be the best approach.

EDIT: I say code-gen, but it could just as well be done as built-time transformation.

matifali commented 4 weeks ago

Yes. Generated code is fine too. The idea is to reduce fragmentation and optimize the UX.

mafredri commented 3 weeks ago

@matifali I've been thinking about how to solve this from a couple of perspectives:

  1. How do we include common code (lib.sh) for all modules?
  2. How can we make sure our code can be validated by shellcheck?
  3. How do we ensure TF variables are available in code
  4. How do we ensure we don't make errors where we write ${VAR} and get non-existent TF var vs intended shell var

Essentially, this is what I've come up with:

resource "coder_script" "vscode-web" {
  script = templatefile("${path.module}/script.tftpl", {
    LOADER: "/usr/bin/env bash",
    LIB: file("${path.module}/lib.sh"),
    RUN: file("${path.module}/run.sh"),
    VARS: templatefile("${path.module}/vars.tftpl", {
      PORT : var.port,
      LOG_PATH : var.log_path,
      INSTALL_PREFIX : var.install_prefix,
      EXTENSIONS : join(",", var.extensions),
      TELEMETRY_LEVEL : var.telemetry_level,
      // This is necessary otherwise the quotes are stripped!
      SETTINGS : replace(jsonencode(var.settings), "\"", "\\\""),
      OFFLINE : var.offline,
      USE_CACHED : var.use_cached,
      EXTENSIONS_DIR : var.extensions_dir,
      FOLDER : var.folder,
      AUTO_INSTALL_EXTENSIONS : var.auto_install_extensions,
    }),
  })
}

Where:

script.tftpl:

#!${LOADER}

# lib.sh
touch "$CODER_SCRIPT_DATA_DIR/lib.sh"
${LIB}

# vars.tftpl
touch "$CODER_SCRIPT_DATA_DIR/vars.tftpl"
${VARS}

# run.sh
${RUN}

(The touches are present here to avoid errors when executing run.sh content and sourcing the files. Alternatively the source lines could be stripped out but puts logic in main.tf.)

lib.sh:

#!/bin/sh

log() { echo "$@" }

run.sh:

#!/usr/bin/env bash

# shellcheck source=vscode-web/lib.sh
. "${CODER_SCRIPT_DATA_DIR}/lib.sh"
# shellcheck source=vscode-web/vars.tftpl
. "${CODER_SCRIPT_DATA_DIR}/vars.tftpl"

run_vscode_web() {
  log running... # log from lib.sh
  # ...

vars.tftpl:

#!/usr/bin/env sh
# Code generated by [insert name]. DO NOT EDIT.

# shellcheck disable=SC2269
PORT="${PORT}" # type: number, default: 13338, description: The port to run VS Code Web on.
LOG_PATH="${LOG_PATH}" # ...
INSTALL_PREFIX="${INSTALL_PREFIX}"
EXTENSIONS="${EXTENSIONS}"
TELEMETRY_LEVEL="${TELEMETRY_LEVEL}"
SETTINGS="${SETTINGS}"
OFFLINE="${OFFLINE}"
USE_CACHED="${USE_CACHED}"
EXTENSIONS_DIR="${EXTENSIONS_DIR}"
FOLDER="${FOLDER}"
AUTO_INSTALL_EXTENSIONS="${AUTO_INSTALL_EXTENSIONS}"

End result (coder script):

#!/usr/bin/env bash

# lib.sh
touch "$CODER_SCRIPT_DATA_DIR/lib.sh"
#!/bin/sh

log() { echo "$@" }

# vars.tftpl
touch "$CODER_SCRIPT_DATA_DIR/vars.tftpl"
#!/usr/bin/env sh
# Code generated by [insert name]. DO NOT EDIT.

# shellcheck disable=SC2269
PORT="${PORT}" # type: number, default: 13338, description: The port to run VS Code Web on.
LOG_PATH="${LOG_PATH}" # ...
INSTALL_PREFIX="${INSTALL_PREFIX}"
EXTENSIONS="${EXTENSIONS}"
TELEMETRY_LEVEL="${TELEMETRY_LEVEL}"
SETTINGS="${SETTINGS}"
OFFLINE="${OFFLINE}"
USE_CACHED="${USE_CACHED}"
EXTENSIONS_DIR="${EXTENSIONS_DIR}"
FOLDER="${FOLDER}"
AUTO_INSTALL_EXTENSIONS="${AUTO_INSTALL_EXTENSIONS}"

# run.sh
#!/usr/bin/env bash

# shellcheck source=vscode-web/lib.sh
. "${CODER_SCRIPT_DATA_DIR}/lib.sh"
# shellcheck source=vscode-web/vars.tftpl
. "${CODER_SCRIPT_DATA_DIR}/vars.tftpl"

run_vscode_web() {
  log running... # log from lib.sh
  # ...

Not sure if this will make it hard to understand the project structure and how/where to contribute. I have some other ideas too, but that requires introducing a pre-filter for shellcheck and supporting shfmt would be hard. Also you'd still have to be careful with $${VAR} vs ${VAR} but we could introduce a new linter for that and enforce that all tf is at the top of the file, for instance.

Also, it'd be possible to combine script.tftpl and vars.tftpl if that makes things easier, but at the same time it'd be a bit of inception (run.sh imports script.tftpl and script.tftpl embeds run.sh 😂)

matifali commented 3 weeks ago

This is an exciting approach and will greatly improve quality and maintenance. and if we can automate it with a script like bun generate, I think it will help make the adoption easy for adoption. Let it stay here fir a few weeks to get more feedback from the community.