Skip to content

HYPERFLEET-1133 - fix: make production the default runtime environment#191

Open
ldornele wants to merge 3 commits into
openshift-hyperfleet:mainfrom
ldornele:HYPERFLEET-1133
Open

HYPERFLEET-1133 - fix: make production the default runtime environment#191
ldornele wants to merge 3 commits into
openshift-hyperfleet:mainfrom
ldornele:HYPERFLEET-1133

Conversation

@ldornele
Copy link
Copy Markdown
Contributor

Summary

Changes the default runtime environment from development to production to enforce secure-by-default behavior.

Problem: When HYPERFLEET_ENV is not set, the server currently defaults to development mode, which silently disables JWT authentication and TLS. This creates a security risk — any deployment that omits
the environment variable runs without authentication.

Solution: Set EnvironmentDefault = ProductionEnv so that missing configuration results in a secured service, not an open one.

Changes

  • cmd/hyperfleet-api/environments/types.go:18 — Changed EnvironmentDefault from DevelopmentEnv to ProductionEnv
  • docs/development.md — Updated local development section to document the new default and how to disable auth explicitly

How Environment Configuration Works

The application follows a two-stage configuration process:

  1. Base configuration loads first with JWT and TLS enabled by default
  2. Environment-specific overrides apply via OverrideConfig() to adjust settings per environment

This means the codebase is secure by default — environments must explicitly opt-out of security features.

Environment Configuration Differences

e_production.go (new default):

func (e *productionEnvImpl) OverrideConfig(c *config.ApplicationConfig) error {
    return nil  // No overrides — keeps JWT and TLS enabled from base config
}
✅ Result: JWT enabled, TLS enabled (respects base configuration defaults)

e_development.go (must now be set explicitly):
func (e *devEnvImpl) OverrideConfig(c *config.ApplicationConfig) error {
    c.Server.JWT.Enabled = false  // Explicitly disable JWT
    c.Server.TLS.Enabled = false  // Explicitly disable TLS

    if c.Database.SSL.Mode == "" {
        c.Database.SSL.Mode = SSLModeDisable
    }   
    
    return nil
}
⚠️  Result: JWT disabled, TLS disabled (explicitly overrides base config for local development)

Key difference: The base configuration starts with security enabled. Production environment keeps these defaults unchanged, while development environment explicitly disables authentication and TLS for local
convenience. With this change, forgetting to set HYPERFLEET_ENV now results in a secure default instead of an insecure one.

@openshift-ci openshift-ci Bot requested review from crizzo71 and kuudori May 29, 2026 18:43
@openshift-ci
Copy link
Copy Markdown

openshift-ci Bot commented May 29, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign ciaranroche for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 29, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: 31ffd50f-acee-468c-9c1e-38469e455e63

📥 Commits

Reviewing files that changed from the base of the PR and between 288dc00 and e82bac4.

📒 Files selected for processing (1)
  • cmd/hyperfleet-api/environments/framework_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
  • cmd/hyperfleet-api/environments/framework_test.go

📝 Walkthrough

Summary by CodeRabbit

  • Configuration Changes

    • Default runtime environment changed from development to production; JWT and TLS are enabled by default.
  • Documentation

    • Clarified default environment behavior and added instructions for running locally without authentication.
  • Tests

    • Added a test that verifies the default environment is production.

Walkthrough

The 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)

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 (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately reflects the main change: making production the default runtime environment instead of development.
Description check ✅ Passed The description is well-related to the changeset, explaining the security rationale, the configuration mechanism, and how the three files were modified.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Sec-02: Secrets In Log Output ✅ Passed PR introduces no log statements containing tokens, passwords, credentials, or secrets. Production files (types.go, development.md) contain no logging code or sensitive data leaks.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
✨ Simplify code
  • Create PR with simplified code

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

@hyperfleet-ci-bot
Copy link
Copy Markdown

hyperfleet-ci-bot Bot commented May 29, 2026

Risk Score: 2 — risk/medium

Signal Detail Points
PR size 13 lines +0
Sensitive paths cmd/ +2
Test coverage Tests cover changed packages +0

Computed by hyperfleet-risk-scorer

Copy link
Copy Markdown

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

🧹 Nitpick comments (1)
cmd/hyperfleet-api/environments/framework_test.go (1)

56-59: ⚡ Quick win

Consider 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_ENV resolves to a config with JWT and TLS enabled. A test that initializes the environment with no HYPERFLEET_ENV set and asserts JWT/TLS-enabled on the resolved config would catch regressions in OverrideConfig() wiring that a const comparison cannot.

Want me to draft a test that loads defaults with HYPERFLEET_ENV unset 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

📥 Commits

Reviewing files that changed from the base of the PR and between 1ab24ee and 288dc00.

📒 Files selected for processing (1)
  • cmd/hyperfleet-api/environments/framework_test.go

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant