Skip to content

Conversation

@Blacksmoke16
Copy link
Contributor

@Blacksmoke16 Blacksmoke16 commented Dec 15, 2025

Summary by CodeRabbit

  • New Features
    • The router compose command now supports environment variable substitution in configuration files using ${VARIABLE_NAME} syntax to reference environment values.
    • Missing variables are automatically replaced with empty strings.
    • Supports multiple variables and adjacent variable references within configuration strings.

✏️ Tip: You can customize this high-level summary in your review settings.

Checklist

Resolves #2002

Will follow up with a docs PR soon

@coderabbitai
Copy link

coderabbitai bot commented Dec 15, 2025

Walkthrough

The changes add environment variable interpolation support to the router compose command. A new expandEnvVars() utility function replaces ${VAR_NAME} patterns with corresponding environment variable values before YAML parsing, along with comprehensive test coverage validating the behavior.

Changes

Cohort / File(s) Summary
Core Implementation
cli/src/commands/router/commands/compose.ts
Adds expandEnvVars() function to replace ${VAR_NAME} patterns with environment variable values; applies expansion to YAML content before parsing
Test Suite
cli/test/expand-env-vars.test.ts
Adds comprehensive tests for expandEnvVars() covering variable replacement, missing variables, multiple/adjacent variables, unchanged strings, and Authorization header scenarios; manages environment state per test

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

  • Verify the regex pattern for ${VAR_NAME} correctly matches the intended syntax and doesn't introduce unintended expansions
  • Confirm that missing environment variables safely default to empty strings as intended
  • Review the integration point in the YAML parsing flow to ensure the preprocessing step doesn't disrupt error handling or existing functionality

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: adding environment variable interpolation support to the router compose input file.
Linked Issues check ✅ Passed The PR implements environment variable interpolation for router compose input files, directly addressing the requirement in issue #2002 to support ENV var interpolation similar to config.yaml.
Out of Scope Changes check ✅ Passed All changes are directly related to implementing environment variable interpolation in the router compose input file, with no unrelated modifications detected.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link

codecov bot commented Dec 15, 2025

Codecov Report

❌ Patch coverage is 71.42857% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 29.20%. Comparing base (923cd4f) to head (4c4ccc7).
⚠️ Report is 7 commits behind head on main.

Files with missing lines Patch % Lines
cli/src/commands/router/commands/compose.ts 71.42% 2 Missing ⚠️

❌ Your patch check has failed because the patch coverage (71.42%) is below the target coverage (90.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##            main    #2413       +/-   ##
==========================================
+ Coverage   1.49%   29.20%   +27.70%     
==========================================
  Files        292      127      -165     
  Lines      46926    11051    -35875     
  Branches     431      254      -177     
==========================================
+ Hits         703     3227     +2524     
+ Misses     45940     7822    -38118     
+ Partials     283        2      -281     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
cli/src/commands/router/commands/compose.ts (1)

33-41: Consider documenting edge cases.

While the implementation is solid, you may want to document two behaviors for users:

  1. Missing variables: Undefined environment variables are replaced with empty strings (rather than causing errors or warnings).
  2. YAML special characters: Environment variable values containing YAML special characters (quotes, colons, etc.) could break YAML syntax. Users should ensure proper quoting in their input files.

For example, add to the JSDoc:

 /**
  * Expands environment variables in a string using ${VAR_NAME} syntax.
  * Consistent with Go's os.ExpandEnv() used in router config.yaml.
+ * 
+ * @param content - The string content to expand
+ * @returns The content with all ${VAR_NAME} patterns replaced by their values,
+ *          or empty strings for undefined variables
+ * @note Ensure environment variable values are properly escaped for YAML context
  */
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4400c5d and 4c4ccc7.

📒 Files selected for processing (2)
  • cli/src/commands/router/commands/compose.ts (2 hunks)
  • cli/test/expand-env-vars.test.ts (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Analyze (go)
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (5)
cli/test/expand-env-vars.test.ts (3)

1-14: LGTM! Clean test setup.

The environment snapshot and restore pattern is appropriate. The ESLint disable is necessary for the test strings containing ${VAR} patterns.


16-40: Excellent test coverage.

The test cases cover all essential scenarios including edge cases like adjacent variables and the important behavior of replacing missing variables with empty strings.


42-49: Great practical test case!

This test validates the primary use case described in the PR objectives: interpolating sensitive authorization tokens in headers. The multi-line YAML-like format makes the test realistic.

cli/src/commands/router/commands/compose.ts (2)

195-197: LGTM! Integration point is correct.

The expansion is applied at the right time—after file read but before YAML parsing. This ensures that both main subgraphs and feature flag configurations benefit from environment variable interpolation, directly addressing the PR objective of handling sensitive introspection headers.


33-41: Implementation correctly follows Go's os.ExpandEnv behavior.

The regex pattern appropriately handles ${VAR_NAME} syntax, and the use of process.env[varName] ?? '' correctly returns an empty string for undefined variables, matching Go's documented behavior. The code is secure with no injection risks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support ENV var interpolation within router compose input file

1 participant