HYPERFLEET-1133 - fix: make production the default runtime environment#191
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Central YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR changes the default runtime environment constant from DevelopmentEnv to ProductionEnv, adds a test asserting EnvironmentDefault is ProductionEnv (with Gomega dot-import), and updates docs to state the binary defaults to production (JWT/TLS enabled) and how to run with authentication disabled for local development. Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
Risk Score: 2 —
|
| Signal | Detail | Points |
|---|---|---|
| PR size | 13 lines | +0 |
| Sensitive paths | cmd/ | +2 |
| Test coverage | Tests cover changed packages | +0 |
Computed by hyperfleet-risk-scorer
There was a problem hiding this comment.
🧹 Nitpick comments (1)
cmd/hyperfleet-api/environments/framework_test.go (1)
56-59: ⚡ Quick winConsider a behavioral test, not just constant equality.
This guards the constant, which is a fine regression check. But the actual security guarantee from this PR is that an unset
HYPERFLEET_ENVresolves to a config with JWT and TLS enabled. A test that initializes the environment with noHYPERFLEET_ENVset and asserts JWT/TLS-enabled on the resolved config would catch regressions inOverrideConfig()wiring that a const comparison cannot.Want me to draft a test that loads defaults with
HYPERFLEET_ENVunset and asserts JWT/TLS are enabled?🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/hyperfleet-api/environments/framework_test.go` around lines 56 - 59, Add a behavioral test that unsets HYPERFLEET_ENV, constructs the environment/config via the same codepath used in production (call the function that resolves/overrides config—e.g., OverrideConfig or the environment loading function used in tests) and assert that the resulting config has JWT and TLS enabled (e.g., cfg.JWT.Enabled == true and cfg.TLS.Enabled == true). Use os.Unsetenv("HYPERFLEET_ENV") at the start of the test, then call the environment resolution function referenced in the codebase (OverrideConfig / the environment loader used alongside EnvironmentDefault) and use the existing test helpers (RegisterTestingT/Expect) to check JWT and TLS flags so regressions in wiring are caught.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@cmd/hyperfleet-api/environments/framework_test.go`:
- Around line 56-59: Add a behavioral test that unsets HYPERFLEET_ENV,
constructs the environment/config via the same codepath used in production (call
the function that resolves/overrides config—e.g., OverrideConfig or the
environment loading function used in tests) and assert that the resulting config
has JWT and TLS enabled (e.g., cfg.JWT.Enabled == true and cfg.TLS.Enabled ==
true). Use os.Unsetenv("HYPERFLEET_ENV") at the start of the test, then call the
environment resolution function referenced in the codebase (OverrideConfig / the
environment loader used alongside EnvironmentDefault) and use the existing test
helpers (RegisterTestingT/Expect) to check JWT and TLS flags so regressions in
wiring are caught.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 6f9ed444-ec1b-44d5-a58d-687c1d311a52
📒 Files selected for processing (1)
cmd/hyperfleet-api/environments/framework_test.go
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rh-amarin The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
6740304
into
openshift-hyperfleet:main
Summary
Changes the default runtime environment from
developmenttoproductionto enforce secure-by-default behavior.Problem: When
HYPERFLEET_ENVis not set, the server currently defaults todevelopmentmode, which silently disables JWT authentication and TLS. This creates a security risk — any deployment that omitsthe environment variable runs without authentication.
Solution: Set
EnvironmentDefault = ProductionEnvso that missing configuration results in a secured service, not an open one.Changes
EnvironmentDefaultfromDevelopmentEnvtoProductionEnvHow Environment Configuration Works
The application follows a two-stage configuration process:
OverrideConfig()to adjust settings per environmentThis means the codebase is secure by default — environments must explicitly opt-out of security features.
Environment Configuration Differences
e_production.go(new default):