feat: add env-file and env-from support for all job types#540
Conversation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the configuration capabilities of Ofelia by adding support for loading environment variables from external sources. This change provides users with more flexibility in managing environment variables for their jobs, making it easier to configure and deploy applications. The implementation includes thorough testing and considers various design aspects such as idempotency and graceful degradation. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
✅ Mutation Testing ResultsMutation Score: 87.18% (threshold: 60%)
What is mutation testing?Mutation testing measures test quality by introducing small changes (mutations) to the code and checking if tests detect them. A higher score means better test effectiveness.
|
There was a problem hiding this comment.
Code Review
This pull request enhances job configuration by introducing env-file and env-from options, allowing environment variables to be loaded from .env files and inherited from other Docker containers, respectively. Core logic for parsing .env files, resolving container environments, and merging these sources with explicit environment variables was added in core/envfile.go, along with comprehensive unit and integration tests. The job execution mechanisms across execjob.go, localjob.go, runjob.go, and runservice.go were updated to incorporate this new environment resolution. A review comment identified a bug in the ParseEnvFile function, noting that lines starting with an equals sign are currently parsed incorrectly and should be skipped to match Docker's behavior.
There was a problem hiding this comment.
Pull request overview
Adds support for loading job environments from external sources across Ofelia job types, enabling env-file (dotenv-style files) and env-from (copy env from a running container) with defined merge precedence.
Changes:
- Introduces
ResolveJobEnvironmentwith helpers to parse env files, resolve env from containers, and merge sources with last-wins semantics. - Wires
env-file/env-frominto exec/run/service-run/local job execution paths. - Extends Docker label decoding and adds unit/integration tests for parsing/decoding behavior.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| core/runservice.go | Uses resolved merged environment when creating service specs. |
| core/runjob.go | Uses resolved merged environment when building container configs. |
| core/localjob.go | Resolves env sources for local jobs (with warning-only behavior for env-from). |
| core/execjob.go | Resolves env sources for exec jobs in both Run and RunWithStreams. |
| core/envfile.go | Adds env-file parsing, env-from resolution via container inspection, and merge logic. |
| core/envfile_test.go | Unit tests for ParseEnvFile/MergeEnvironments/ResolveEnvFrom/ResolveJobEnvironment. |
| config/validator.go | Marks env-file / env-from as optional fields (validator allowlist). |
| cli/envfile_integration_test.go | Integration tests ensuring INI decoding supports shadow keys and env-from. |
| cli/docker-labels.go | Extends label param handling to accept JSON arrays for env-file/env-from. |
bb3e745 to
9eb9704
Compare
Add two new configuration options for loading environment variables from external sources, available on all job types (exec, run, local, service-run): - env-file: Load KEY=VALUE pairs from files (like Docker --env-file). Supports multiple files via INI shadow keys or JSON arrays in labels. Handles comments, blank lines, quoted values, export prefix, and special characters (#, ;) in values. - env-from: Copy environment variables from a running Docker container at job execution time. For local jobs, warns and skips gracefully. Merge order (last wins): env-file < env-from < environment (explicit). Environment resolution is idempotent — the original Environment field is never mutated, so cron-scheduled jobs resolve fresh each run. Closes #314 Closes #336 Closes #351 Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
- Skip lines with empty key (e.g., "=value") matching Docker behavior - Increase scanner buffer to 1MB for long values (certs, JSON blobs) - Add nil-provider guard to exported ResolveEnvFrom - Add tests for empty key and nil provider cases Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
- Add #nosec G304 for intentional user-configured file path in ParseEnvFile - Use static sentinel ErrNilContainerInspector instead of dynamic error - Fix gci import group ordering (stdlib / third-party / project) Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
The standalone gosec CLI does not recognize //nolint:gosec directives (those are golangci-lint specific). Use // #nosec G304 which is the standard gosec suppression format used throughout the codebase. Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
ca5699e to
32e5507
Compare
## Summary Release v0.23.0 with CHANGELOG update. ### Added - `env-file`: load environment variables from files for all job types (#540, closes #314) - `env-from`: copy environment variables from a running container (#540, closes #336, #351) ### Fixed - `#`/`;` in env-substituted values truncated by INI comment parsing (#539, fixes #538) - Env expansion in webhook configs, section names, and log-level pre-parse (#539) ### Security - SHA-pin all GitHub Actions (#536)
Summary
Adds two new configuration options for loading environment variables from external sources, available on all job types (exec, run, local, service-run):
env-file: LoadKEY=VALUEpairs from files (like Docker's--env-file). Supports multiple files via INI shadow keys or JSON arrays in Docker labels.env-from: Copy environment variables from a running Docker container at job execution time.Merge order (last wins):
env-file<env-from<environment(explicit always wins).Example (INI config)
Example (Docker labels)
Design decisions
j.Environmentis never mutated —ResolveJobEnvironment()returns a fresh merged slice each run, safe for cron-scheduled jobsContainerInspectorinterface (satisfied byDockerProvider) instead of coupling to full providerenv-fromlogs a warning and is skipped (no Docker provider available)exportprefix, and special characters (#,;) in valuesTest plan
ParseEnvFile,MergeEnvironments,ResolveEnvFrom,ResolveJobEnvironmentCloses #314
Closes #336
Closes #351