Fix: Add DAPR_RUNTIME_HOST env variable support#721
Fix: Add DAPR_RUNTIME_HOST env variable support#721
Conversation
Problem: getDefaultHttpPort() already respected DAPR_HTTP_PORT however, settings.getDefaultHost() did not use DAPR_RUNTIME_HOST Solution: (closes Custom sidecar port Fixes dapr#646) -Added DAPR_RUNTIME_HOST env variable support to Settings.getDefaultHost() Now users can set both DAPR_RUNTIME_HOST and DAPR_HTTP_PORT to fully control where client connects Tests: Created new dedicated test file for env mutations, so as to not mix up jest.isolateModules and top level Settings import Signed-off-by: mnothman <mnothman113@gmail.com>
Signed-off-by: mnothman <mnothman113@gmail.com>
3d2464e to
41498a6
Compare
There was a problem hiding this comment.
Pull request overview
Adds support for configuring the Dapr sidecar host via the DAPR_RUNTIME_HOST environment variable, aligning host behavior with existing port/env-based configuration so clients can connect correctly when Dapr CLI sets host/port env vars.
Changes:
- Update
Settings.getDefaultHost()to read fromprocess.env.DAPR_RUNTIME_HOST. - Refactor HTTP/GRPC client endpoint generation to early-return when an endpoint env var is present.
- Add unit tests verifying environment-variable overrides for host and ports.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/utils/Settings.util.ts |
Reads DAPR_RUNTIME_HOST to determine default host. |
src/implementation/Client/HTTPClient/HTTPClient.ts |
Refactors endpoint selection order (endpoint env var vs host/port). |
src/implementation/Client/GRPCClient/GRPCClient.ts |
Refactors endpoint selection order (endpoint env var vs host/port). |
test/unit/utils/Settings.env.test.ts |
Adds env-var-focused tests for host/HTTP port/GRPC port defaults. |
test/unit/utils/Settings.util.test.ts |
Minor formatting-only change. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| static getDefaultHost(): string { | ||
| return Settings.defaultHost; | ||
| return process.env.DAPR_RUNTIME_HOST ?? Settings.defaultHost; |
There was a problem hiding this comment.
getDefaultHost() uses the nullish coalescing operator (??), which will treat an empty DAPR_RUNTIME_HOST value as valid and return "". That can lead to invalid endpoints like ":3500". To match the existing env-var defaulting pattern in this file (e.g., getDefaultApiToken() / getDefaultHttpEndpoint()), consider using || (and optionally trimming) so empty-string values fall back to Settings.defaultHost.
| return process.env.DAPR_RUNTIME_HOST ?? Settings.defaultHost; | |
| return process.env.DAPR_RUNTIME_HOST || Settings.defaultHost; |
| const originalEnv = { ...process.env }; | ||
|
|
||
| afterEach(() => { | ||
| // Restore original env after each test | ||
| process.env = { ...originalEnv }; |
There was a problem hiding this comment.
In this test suite, afterEach replaces the entire process.env object. Other unit tests in this repo typically restore only the env vars they change (saving the old value and restoring it), which avoids side effects from swapping out the process.env reference. Consider restoring only the specific variables modified by these tests (or use jest.resetModules() + per-var restore) instead of reassigning process.env wholesale.
| const originalEnv = { ...process.env }; | |
| afterEach(() => { | |
| // Restore original env after each test | |
| process.env = { ...originalEnv }; | |
| const originalDaprRuntimeHost = process.env.DAPR_RUNTIME_HOST; | |
| const originalDaprHttpPort = process.env.DAPR_HTTP_PORT; | |
| const originalDaprGrpcPort = process.env.DAPR_GRPC_PORT; | |
| afterEach(() => { | |
| // Restore original env after each test without replacing process.env | |
| if (originalDaprRuntimeHost === undefined) { | |
| delete process.env.DAPR_RUNTIME_HOST; | |
| } else { | |
| process.env.DAPR_RUNTIME_HOST = originalDaprRuntimeHost; | |
| } | |
| if (originalDaprHttpPort === undefined) { | |
| delete process.env.DAPR_HTTP_PORT; | |
| } else { | |
| process.env.DAPR_HTTP_PORT = originalDaprHttpPort; | |
| } | |
| if (originalDaprGrpcPort === undefined) { | |
| delete process.env.DAPR_GRPC_PORT; | |
| } else { | |
| process.env.DAPR_GRPC_PORT = originalDaprGrpcPort; | |
| } |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #721 +/- ##
===========================================
- Coverage 100.00% 94.11% -5.89%
===========================================
Files 1 3 +2
Lines 6 153 +147
Branches 1 0 -1
===========================================
+ Hits 6 144 +138
- Misses 0 9 +9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Description
Settings.getDefaultHost()was hardcoded to return"127.0.0.1", ignoring theDAPR_RUNTIME_HOSTenvironment variable. This meant users couldn't configure the client host via environment variables when running Dapr with custom ports.Solution: Added support for the
DAPR_RUNTIME_HOSTenvironment variable inSettings.getDefaultHost():Now users can configure both host and port via environment variables when Dapr CLI sets them automatically.
Issue reference
We strive to have all PR being opened based on an issue, where the problem or feature have been discussed prior to implementation.
Please reference the issue this PR will close: #646
Checklist
Please make sure you've completed the relevant tasks for this PR, out of the following list: