feat: enhance boundary configuration options in Claude Code module#797
feat: enhance boundary configuration options in Claude Code module#797DevelopmentCats wants to merge 15 commits intomainfrom
Conversation
|
Technically this is a breaking change since anyone running their module with Its a minimal change that we probably should have included to begin with so I figured updating to 4.9.0 would be fine unless you think we need to consider this a Major bump considering the circumstances. |
There was a problem hiding this comment.
Pull request overview
Adds first-class support for passing Coder Boundary network filtering rules into the claude-code module, aligning the module’s enable_boundary behavior with the documented expectations in issue #792.
Changes:
- Introduces
boundary_config(inline YAML) andboundary_config_path(existing file path) inputs and wires them into the startup script. - Adds Terraform validations to enforce correct boundary configuration combinations.
- Updates README examples and expands
tftestcoverage for boundary configuration scenarios.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
registry/coder/modules/claude-code/scripts/start.sh |
Writes/links boundary config into ~/.config/coder_boundary/config.yaml before starting boundary. |
registry/coder/modules/claude-code/main.tf |
Adds new inputs and validation rules; passes config/env through to the start script. |
registry/coder/modules/claude-code/main.tftest.hcl |
Adds plan-time tests for valid/invalid boundary input combinations. |
registry/coder/modules/claude-code/README.md |
Documents the new configuration options with usage examples and version bump to 4.9.0. |
@DevelopmentCats Can we do it in a non-breaking change where if the input |
Yeah that is what I was leaning towards myself, but i was following the requirements in the Issue😄 I will make these updates as well |
|
I have tested this in all of the possible scenario's and made a ton of updates for this and it should be working if you want to do one last review here. |
Update enable_boundary validation to treat empty/whitespace strings as unset using trimspace checks, so callers providing "" are handled consistently with null. Per-variable validations already reject empty strings. Bump to v4.9.2. Closes #797 Generated with OpenClaw using Claude
Update enable_boundary validation to treat empty/whitespace strings as unset using trimspace checks, so callers providing "" are handled consistently with null. Per-variable validations already reject empty strings. Bump to v4.9.2. Closes #797 Generated with OpenClaw using Claude
2bb18fd to
05fa2c5
Compare
- Added new variables `boundary_config` and `boundary_config_path` for inline YAML and file path configurations, respectively. - Implemented validation rules to ensure proper configuration when `enable_boundary` is true. - Updated README with usage examples for both configuration methods. - Enhanced test cases to cover various boundary configuration scenarios, including validation failures.
- Added a check to ensure the boundary configuration file is not empty after writing inline config. - Enhanced error messages for better clarity on configuration issues.
Update enable_boundary validation to treat empty/whitespace strings as unset using trimspace checks, so callers providing "" are handled consistently with null. Per-variable validations already reject empty strings. Bump to v4.9.2. Closes #797 Generated with OpenClaw using Claude
05fa2c5 to
738c972
Compare
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… checks - Update boundary_config_b64 local and ARG_BOUNDARY_CONFIG_PATH interpolation to treat empty/whitespace-only strings as not set using trimspace() checks, not just null checks - Add unified post-fi check in start.sh that BOUNDARY_CONFIG_FILE exists and is non-empty after writing/linking, exits with clear error if not Generated with OpenClaw using Claude
|
Addressed remaining review comments from Copilot:
The other two items (test failure cases for empty strings and tilde expansion in Generated with OpenClaw using Claude |
…ore symlink Addresses Copilot review comment on PR #797: when ARG_BOUNDARY_CONFIG_PATH contains a leading ~ or literal \$HOME (e.g. ~/.config/...), it was passed as-is to ln -sf without shell expansion, resulting in a broken symlink. Now expands both forms before creating the symlink. Closes #797 Generated with OpenClaw using Claude
|
All 4 Copilot inline review comments are now addressed in the current branch:
Version bumped: v4.9.1 → v4.9.2 (patch) Generated with OpenClaw using Claude |
- Trim whitespace in enable_boundary validations (trimspace checks) - Add empty string test cases for boundary_config and boundary_config_path - Add explicit post-write/link file existence check in start.sh - Normalize ARG_BOUNDARY_CONFIG_PATH to expand tilde and HOME before symlink Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The previous commit accidentally removed proper terraform fmt alignment from all version attributes in README.md code examples. Restored the alignment so Prettier + prettier-plugin-terraform-formatter passes CI. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…tion from elif block Tilde and \$HOME expansion is already performed at script initialization (lines 29-30), matching the pattern used for ARG_CLAUDE_BINARY_PATH. The duplicate expansion inside the elif block was redundant and used a slightly different pattern (single-slash vs double-slash replace). Closes #797 Generated with OpenClaw using Claude
Review Comments AddressedAll four Copilot review comments have been addressed:
Generated with OpenClaw using Claude |
Copilot Review Comments — Verified AddressedAll four Copilot inline review comments have been verified as correctly implemented in this branch:
Generated with Claude Code using claude-sonnet-4-6 |
All Review Comments Verified and Version BumpedAll four Copilot review comments from the original PR have been addressed (confirmed in the current branch), and the version has been bumped to v4.9.3:
Version: v4.9.2 → v4.9.3 Generated with OpenClaw using Claude |
The version-bump script removed alignment spaces from version attributes in README code block examples. This restores proper terraform fmt-style alignment so the CI prettier+terraform-formatter check passes. Closes #797 Generated with OpenClaw using Claude
README Formatting FixFixed attribute alignment in all README code block examples. The version-bump script had stripped the padding spaces from All 10 code examples now have Generated with OpenClaw using Claude |
Description
Adds first-class support for passing Coder Boundary network filtering rules inline or by path into the
claude-codemodule.boundary_config(inline YAML) andboundary_config_path(existing file path) inputs and wires them into the startup script.tftestcoverage for boundary configuration scenarios.Review Comment Fixes
All 4 Copilot inline review comments have been addressed:
boundary_config = "",boundary_config_path = "", and whitespace-only variants (" "):test_boundary_empty_config_fails,test_boundary_empty_config_path_fails,test_boundary_whitespace_config_fails,test_boundary_whitespace_config_path_fails.enable_boundarycross-variable validations usetrimspace()checks (not just null checks) to treat empty/whitespace-only strings as unset. Per-variable validations onboundary_configandboundary_config_pathalso reject empty/whitespace-only strings.if/elif/fiblock: if$BOUNDARY_CONFIG_FILEdoes not exist or is empty, the script exits with a clear error message.ARG_BOUNDARY_CONFIG_PATHis normalized at script startup (lines 29-30) with tilde and$HOMEexpansion, matching the pattern used forARG_CLAUDE_BINARY_PATH.Type of Change
Module Information
Path:
registry/coder/modules/claude-codeNew version:
v4.9.3Breaking change: [ ] Yes [X] No
Testing & Validation
bun test)bun fmt)Closes #792
Generated with Claude (claude-sonnet-4-6)