-
Notifications
You must be signed in to change notification settings - Fork 24
HYPERFLEET-1146 - docs: update development env configuration #224
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -6,6 +6,20 @@ For running the binary directly on your machine (development, debugging), see th | |||||||||
|
|
||||||||||
| --- | ||||||||||
|
|
||||||||||
| ## ⚠️ Production Security Warning | ||||||||||
|
|
||||||||||
| **IMPORTANT**: Default Helm values are for development. Production requires: | ||||||||||
|
|
||||||||||
| | Setting | Default | Production | Risk if not changed | | ||||||||||
| |---------|---------|------------|---------------------| | ||||||||||
| | JWT auth | `false` | `true` | ⚠️ Unauthenticated access | | ||||||||||
| | TLS | `false` | `true` | ⚠️ Plaintext credentials | | ||||||||||
| | Database | Built-in pod | External managed | Data loss on restart | | ||||||||||
|
|
||||||||||
| See [API Operator Guide](api-operator-guide.md) for complete production configuration. | ||||||||||
|
|
||||||||||
| --- | ||||||||||
|
|
||||||||||
| ## Prerequisites | ||||||||||
|
|
||||||||||
| Before deploying, ensure you have: | ||||||||||
|
|
@@ -274,6 +288,36 @@ During upgrade, in case schema changes have occurred in the new version, a DB mi | |||||||||
| helm uninstall hyperfleet-api --namespace hyperfleet-system | ||||||||||
| ``` | ||||||||||
|
|
||||||||||
| ## Helm Values | ||||||||||
|
|
||||||||||
| ### Key Configuration Options | ||||||||||
|
|
||||||||||
| | Parameter | Description | Default | | ||||||||||
| |-----------|-------------|---------| | ||||||||||
| | `image.registry` | Container registry | `CHANGE_ME` (must be set explicitly) | | ||||||||||
| | `image.repository` | Image repository | `openshift-hyperfleet/hyperfleet-api` | | ||||||||||
| | `image.tag` | Image tag | `latest` | | ||||||||||
| | `image.pullPolicy` | Image pull policy | `Always` | | ||||||||||
| | `config.adapters.required.cluster` | Cluster adapters required for Ready state | `[]` | | ||||||||||
| | `config.adapters.required.nodepool` | Nodepool adapters required for Ready state | `[]` | | ||||||||||
| | `config.server.jwt.enabled` | Enable JWT authentication | `false` (Helm default; app default is `true`) | | ||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Warning Blocking Category: Inconsistency The "Key Configuration Options" table includes Suggested additions after the JWT row:
Suggested change
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed! I have added |
||||||||||
| | `config.server.tls.enabled` | Enable TLS on the API listener | `false` | | ||||||||||
| | `config.database.ssl.mode` | SSL mode for database connection | `disable` | | ||||||||||
| | `database.postgresql.enabled` | Enable built-in PostgreSQL | `true` | | ||||||||||
| | `database.external.enabled` | Use external database | `false` | | ||||||||||
| | `database.external.secretName` | Secret containing database credentials | `hyperfleet-db-external` | | ||||||||||
| | `serviceMonitor.enabled` | Enable Prometheus Operator ServiceMonitor | `false` | | ||||||||||
| | `serviceMonitor.interval` | Metrics scrape interval | `30s` | | ||||||||||
| | `serviceMonitor.scrapeTimeout` | Metrics scrape timeout | `10s` | | ||||||||||
| | `serviceMonitor.labels` | Additional labels for Prometheus selector | `{}` | | ||||||||||
| | `serviceMonitor.namespace` | Namespace for ServiceMonitor (if different) | `""` | | ||||||||||
| | `replicaCount` | Number of API replicas | `1` | | ||||||||||
| | `resources.limits.cpu` | CPU limit | `500m` | | ||||||||||
| | `resources.limits.memory` | Memory limit | `512Mi` | | ||||||||||
| | `podDisruptionBudget.enabled` | Enable PodDisruptionBudget | `false` | | ||||||||||
| | `podDisruptionBudget.minAvailable` | Minimum available pods during disruption | `1` | | ||||||||||
| | `podDisruptionBudget.maxUnavailable` | Maximum unavailable pods during disruption | - | | ||||||||||
|
|
||||||||||
| ### Custom Values File | ||||||||||
|
|
||||||||||
| For repeatable deployments, create a `values.yaml` file: | ||||||||||
|
|
@@ -450,6 +494,20 @@ Before deploying to production, ensure: | |||||||||
| - [ ] **Disruption**: PodDisruptionBudget enabled (`podDisruptionBudget.enabled=true`) | ||||||||||
| - [ ] **Monitoring**: ServiceMonitor enabled if using Prometheus Operator | ||||||||||
|
|
||||||||||
| ## Production Best Practices | ||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can also add:
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added. Included logging format (JSON), log level (info), tracing enablement, and identity extraction to Production Best Practices. |
||||||||||
|
|
||||||||||
| - **Environment**: Use default (ProductionEnv) for production deployments; never set `HYPERFLEET_ENV=development` | ||||||||||
| - **Database**: Use external managed database (Cloud SQL, RDS, Azure Database) with automated backups | ||||||||||
| - **Secrets**: Store all sensitive data in Kubernetes Secrets, never in ConfigMap or values.yaml | ||||||||||
| - **Authentication**: Enable JWT authentication with `config.server.jwt.enabled=true` | ||||||||||
| - **Identity**: Enable identity extraction based on JWT claims or HTTP headers as appropriate | ||||||||||
| - **Logging**: Use JSON format (`config.logging.format=json`) and set level to `info` for production | ||||||||||
| - **Tracing**: Enable distributed tracing for observability in production environments | ||||||||||
| - **Resources**: Set CPU/memory limits and use multiple replicas for high availability | ||||||||||
| - **Images**: Use specific image tags (semantic versioning) instead of `latest` | ||||||||||
| - **Disruption**: Enable PodDisruptionBudget for zero-downtime during cluster maintenance | ||||||||||
| - **Health**: Configure health probes with appropriate timeouts for your workload | ||||||||||
|
|
||||||||||
| ### Configuration File Security | ||||||||||
|
|
||||||||||
| The configuration file path — set via `--config` or `HYPERFLEET_CONFIG` — is a trust boundary. The API validates configuration **content** on startup (unknown fields are rejected, required values are enforced, TLS/JWT/timeout settings are checked) and will refuse to start with an invalid configuration. However, **path and permission safety is the operator's responsibility**. The API reads whatever file the process can access at the given path without checking permissions or ownership. | ||||||||||
|
|
||||||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -107,7 +107,7 @@ export HYPERFLEET_DATABASE_SSL_MODE=require # for remote databases | |||||
| make run-no-auth | ||||||
| ``` | ||||||
|
|
||||||
| **Note**: The default runtime environment is `production` (JWT and TLS enabled). The `make run-no-auth` target explicitly disables authentication for local development. If running the binary directly, set `HYPERFLEET_ENV=development` or use `--server-jwt-enabled=false`. | ||||||
| **Note**: The default runtime environment is `production`. For local development without authentication, use `make run-no-auth` or set `HYPERFLEET_ENV=development` see [Development Environment Configuration](#development-environment-configuration) below). | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tip nit — non-blocking suggestion Category: Bug Missing opening parenthesis before "see" — the closing
Suggested change
|
||||||
|
|
||||||
| The service starts on `localhost:8000` — see [Accessing the API](../README.md#accessing-the-api) for all available endpoints. | ||||||
|
|
||||||
|
|
@@ -451,6 +451,67 @@ make db/setup | |||||
| make test-integration | ||||||
| ``` | ||||||
|
|
||||||
| ## Development Environment Configuration | ||||||
|
|
||||||
| ### Development Environment Analysis | ||||||
|
|
||||||
| **Background**: Prior to HYPERFLEET-1133, the API defaulted to `DevelopmentEnv` (insecure). To protect production deployments, HYPERFLEET-1133 changed the default to `ProductionEnv` (secure by default). | ||||||
|
|
||||||
| **Analysis Question**: Is `e_development.go` still needed after this change? | ||||||
|
|
||||||
| **Decision**: **KEEP `e_development.go`** with improved documentation. | ||||||
|
|
||||||
| **Why keep it**: | ||||||
| - ✅ **One variable controls multiple settings** — `HYPERFLEET_ENV=development` forces JWT=false, TLS=false, SSL=disable | ||||||
| - ✅ **Convenient for scripts/CI** — One environment variable vs three separate flags | ||||||
| - ✅ **Semantic clarity** — "development mode" is clearer than remembering individual flags | ||||||
| - ✅ **Consistent with tests** — `unit_testing` and `integration_testing` use the same pattern | ||||||
| - ✅ **Production safe** — `EnvironmentDefault = ProductionEnv` prevents accidental use in production | ||||||
|
|
||||||
| **How to use**: | ||||||
|
|
||||||
| ```bash | ||||||
| # Full development mode (JWT/TLS/DB SSL all disabled) | ||||||
| HYPERFLEET_ENV=development ./bin/hyperfleet-api serve | ||||||
|
|
||||||
| # JWT-only no-auth (TLS and DB SSL keep their defaults) | ||||||
| make run-no-auth | ||||||
|
|
||||||
| # Production mode (JWT/TLS enabled, default) | ||||||
| ./bin/hyperfleet-api serve # Uses EnvironmentDefault = ProductionEnv | ||||||
| ``` | ||||||
|
|
||||||
| **⚠️ IMPORTANT**: `HYPERFLEET_ENV=development` is for **local development ONLY**. Never use in production. The development environment forces insecure settings: | ||||||
| - JWT authentication: **disabled** | ||||||
| - TLS encryption: **disabled** | ||||||
| - Database SSL: **disabled** | ||||||
|
|
||||||
| **Production deployments**: Always use `EnvironmentDefault` (production) or explicitly enable security via Helm values: | ||||||
| ```yaml | ||||||
| config: | ||||||
| server: | ||||||
| jwt: | ||||||
| enabled: true # Production requires JWT | ||||||
| tls: | ||||||
| enabled: true # Production requires TLS | ||||||
| database: | ||||||
| ssl: | ||||||
| mode: verify-full # Production requires SSL | ||||||
| ``` | ||||||
|
|
||||||
| **Alternative to `HYPERFLEET_ENV=development`**: If you prefer explicit flags over environment-based config, you can pass flags directly: | ||||||
|
|
||||||
| ```bash | ||||||
| ./bin/hyperfleet-api serve \ | ||||||
| --server-jwt-enabled=false \ | ||||||
| --server-https-enabled=false \ | ||||||
| --db-ssl-mode=disable | ||||||
| ``` | ||||||
|
|
||||||
| However, `HYPERFLEET_ENV=development` is recommended for local development as it's simpler and less error-prone. | ||||||
|
|
||||||
| --- | ||||||
|
|
||||||
| ## Related Documentation | ||||||
|
|
||||||
| - [Configuration Guide](config.md) — Complete configuration reference | ||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Warning
Blocking
Category: Bug
This new
## Helm Valuestable duplicates the existing## Helm Values Referencesection (line 376) and introduces incorrect defaults:image.repositorylisted asopenshift-hyperfleet/hyperfleet-api— actualvalues.yamldefault isCHANGE_MEimage.taglisted aslatest— actualvalues.yamldefault is""(must be set)Suggestion: remove this section and instead update the existing
## Helm Values Referencetable with the missing entries (config.server.tls.enabled,config.database.ssl.mode, and the JWT app-default annotation).