Skip to content

fix: preserve kubernetes discovery env for worker command steps#1924

Merged
yottahmd merged 1 commit intomainfrom
1922-bugfix-env
Apr 1, 2026
Merged

fix: preserve kubernetes discovery env for worker command steps#1924
yottahmd merged 1 commit intomainfrom
1922-bugfix-env

Conversation

@yottahmd
Copy link
Copy Markdown
Collaborator

@yottahmd yottahmd commented Apr 1, 2026 •
edited by coderabbitai Bot
Loading

Summary

  • allow KUBERNETES_ discovery env vars through the filtered base env used for step execution
  • preserve the existing isolated step env model for unrelated host env vars
  • add regression coverage for base env loading, exec context env composition, and worker task start/retry execution

Testing

  • go test ./internal/cmn/config ./internal/core/exec ./internal/service/worker

Related to #1922

Summary by CodeRabbit

  • New Features
    • Added support for Kubernetes environment variables. Variables starting with KUBERNETES_ (such as service host and port information) are now preserved and passed to child processes, enabling better Kubernetes in-cluster discovery support.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 1, 2026 •
edited
Loading

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e42faacb-2212-4e7e-8381-ba483e2fed79

📥 Commits

Reviewing files that changed from the base of the PR and between b53e219 and d44ef60.

📒 Files selected for processing (4)
  • internal/cmn/config/env.go
  • internal/cmn/config/env_test.go
  • internal/core/exec/context_test.go
  • internal/service/worker/handler_test.go
📝 Walkthrough

Walkthrough

The pull request adds Kubernetes in-cluster discovery environment variables to the list of allowed environment variable prefixes. Variables starting with KUBERNETES_ are now eligible for inclusion in the BaseEnv passed to child processes. Tests verify that Kubernetes environment variables are preserved through multiple execution layers.

Changes

Cohort / File(s) Summary
Core Configuration
internal/cmn/config/env.go		 Added "KUBERNETES_" to defaultPrefixes in environment variable filtering logic to permit Kubernetes service discovery variables through LoadBaseEnv().
Environment Loading Tests
internal/cmn/config/env_test.go		 Extended test matrix to verify KUBERNETES_SERVICE_HOST, KUBERNETES_SERVICE_PORT, and KUBERNETES_SERVICE_PORT_HTTPS are retained in LoadBaseEnv() output.
Execution Context Tests
internal/core/exec/context_test.go		 Updated TestNewContext_AllEnvsUsesFilteredBaseEnv to confirm Kubernetes environment variables persist in context's AllEnvs() output alongside other filtered variables.
Worker Handler Tests
internal/service/worker/handler_test.go		 Added two TestTaskHandler subtests validating that Kubernetes discovery variables are preserved across worker task execution for both OPERATION_START and OPERATION_RETRY scenarios, with assertions on environment variable retention and task completion status.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: adding Kubernetes discovery environment variables to the filtered base environment for worker command steps.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 1922-bugfix-env

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.

@yottahmd yottahmd merged commit 1af0090 into main Apr 1, 2026
5 checks passed
@yottahmd yottahmd deleted the 1922-bugfix-env branch April 1, 2026 06:49
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 1, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 68.78%. Comparing base (b53e219) to head (d44ef60).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1924   +/-   ##
=======================================
  Coverage   68.77%   68.78%           
=======================================
  Files         457      457           
  Lines       57637    57637           
=======================================
+ Hits        39638    39643    +5     
+ Misses      14388    14381    -7     
- Partials     3611     3613    +2
Files with missing lines Coverage Δ
internal/cmn/config/env.go 89.18% <ø> (ø)

... and 11 files with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b53e219...d44ef60. Read the comment docs.

🚀 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.

@coderabbitai coderabbitai Bot mentioned this pull request Apr 1, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@yottahmd