Skip to content

feat(devcontainers-cli): support pinned and mirrored installs - #1047

Open
Edd88-pixel wants to merge 3 commits into
coder:mainfrom
Edd88-pixel:feat/devcontainers-cli-pinned-mirrored-installs
Open

feat(devcontainers-cli): support pinned and mirrored installs#1047
Edd88-pixel wants to merge 3 commits into
coder:mainfrom
Edd88-pixel:feat/devcontainers-cli-pinned-mirrored-installs

Conversation

@Edd88-pixel

Copy link
Copy Markdown
Collaborator

Why

The module always installed the latest @devcontainers/cli release from the package manager's configured public source. That made reproducible installs and restricted-network workspaces difficult, while global installs could depend on elevated permissions.

Changes

  • add validated inputs for an npm version or dist-tag and an optional npm-compatible registry URL
  • install into Coder's agent-managed binary directory without sudo
  • preserve existing package-manager detection and skip installation when devcontainer is already available
  • propagate package-manager failures and verify the installed command is on PATH
  • document pinned, mirrored, and air-gapped usage
  • add Terraform tests, deterministic npm/pnpm/Yarn runtime coverage, and a real pinned npm installation test

Type of Change

  • New module
  • New template
  • Bug fix
  • Feature/enhancement
  • Documentation
  • Other

Module Information

Path: registry/coder/modules/devcontainers-cli
New version: v1.2.0
Breaking change: [ ] Yes [x] No

Validation

  • terraform test — 4 passed
  • bun test registry/coder/modules/devcontainers-cli/main.test.ts — 9 passed
  • real npm installation of @devcontainers/cli@0.80.0 in node:22-alpine, followed by devcontainer --version
  • terraform validate
  • repository ShellCheck validation
  • repository README validation
  • targeted Prettier and Terraform formatting checks
  • tsc --noEmit

Related Issues

Related to #427 and #308.

@Edd88-pixel
Edd88-pixel marked this pull request as ready for review August 12, 2026 10:30
@Edd88-pixel

Copy link
Copy Markdown
Collaborator Author

@matifali could you review this please ?

@Edd88-pixel

Copy link
Copy Markdown
Collaborator Author

@DevelopmentCats Can you retry ?

@Edd88-pixel
Edd88-pixel requested a review from matifali August 22, 2026 17:13
#!/usr/bin/env sh
# shellcheck shell=sh

# shellcheck disable=SC2016 # Terraform replaces this literal placeholder.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I wonder why we had to use this?
If you do not want terraform to replace a literal. use $${LITERAL} or $LITERAL

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

These placeholders are intentionally replaced by templatefile; the base64 encoding keeps the rendered shell values safe. The SC2016 suppressions are only needed because I used single quotes.

We need Terraform’s templatefile to substitute the value while rendering the script, so using $${LITERAL} would leave the required value unset.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I understand. If you use ${LITERAL} , Terraform's templatefile()will replace it but if you use $${LITERAL} or $LITERAL it wont.

exit 1
fi

INSTALL_PREFIX=$(dirname "$CODER_SCRIPT_BIN_DIR")

@matifali matifali Aug 24, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should we allow provding an input to control this?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Do you mean allowing callers to provide a custom install prefix? I currently derive it from CODER_SCRIPT_BIN_DIR so npm/Yarn install into the Coder-managed bin directory already on PATH, while pnpm uses it as PNPM_HOME. I’d prefer to keep that fixed unless there is a specific use case, but I’m happy to adjust if that’s what you meant.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yes. Sorry I forgot to complete what I was typing. Its not a blocker

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