Conversation
… installs - Add HTTPS mode option (disabled/self-signed/custom) to install TUI, persisted in deploy.options and reusable on non-interactive reruns - Support auto-generated self-signed certs (99-year validity, SAN from env or NIC auto-detection) and user-provided certs with validation (PEM format, key/cert match, expiry, encrypted key decryption) - Decrypt passphrase-protected keys to a 0600 plaintext copy mounted by nginx; passphrase stored via existing .env credential pattern - Add nexent-nginx helm chart (deployment/service/configmap/secret) terminating TLS on NodePort 30000 via error_page 497 same-port redirect; web service steps down to ClusterIP when HTTPS is enabled - Render cert/key as YAML literal blocks; store TLS as Opaque secret with server.pem/server.key keys - Update offline packaging, env example, install docs and common tests
MoeexT
requested review from
Dallas98,
WMC001,
YehongPan,
hhhhsc701 and
jeffwu-1999
as code owners
September 28, 2026 10:36
mi7ak2020-stack
approved these changes
Sep 28, 2026
|
I |
2 similar comments
|
I |
|
I |
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Unresolved critical and moderate issues affect TLS key security, service exposure, image selection, and deployment transitions.
Review effort: Lite
Findings: 4
Open (13)
Generated Helm values expose the TLS private key · New HTTPS mode still publishes the web service port · New HTTPS mode still publishes the web service port · New Web service starts before its HTTPS port mapping is applied · New Environment defaults override configured HTTPS variables · New EC private keys are rejected by RSA-only validation · New Helm Nginx image ignores the configured registry prefix · New ifconfig parsing prevents automatic SAN IP detection · New Existing certificate pair lacks matching and expiry validation · New HTTPS key passphrase is not persisted across reruns · New Compose Nginx image ignores the configured registry prefix · New Offline Compose deployment uses an unprefixed Nginx image · New Disabling HTTPS leaves the Nginx container running · New
What changed in this PR
Adds optional Nginx HTTPS termination for Docker and Kubernetes deployments, with certificate handling, validation, persistence, and offline packaging support.
Changes:
- Adds disabled, self-signed, and custom HTTPS modes.
- Adds Docker Compose and Helm Nginx proxy resources.
- Updates documentation, environment examples, deployment scripts, and tests.
| File | Summary |
|---|---|
doc/docs/zh/quick-start/installation.md |
Chinese HTTPS deployment documentation |
doc/docs/en/quick-start/installation.md |
English HTTPS deployment documentation |
deploy/tests/test_common.sh |
HTTPS configuration and certificate tests |
deploy/offline/build_offline_package.sh |
Offline HTTPS image support |
deploy/k8s/helm/nexent/charts/nexent-nginx/values.yaml |
Nginx chart defaults |
deploy/k8s/helm/nexent/charts/nexent-nginx/templates/service.yaml |
Nginx Service |
deploy/k8s/helm/nexent/charts/nexent-nginx/templates/secret.yaml |
TLS Secret |
deploy/k8s/helm/nexent/charts/nexent-nginx/templates/deployment.yaml |
Nginx Deployment |
deploy/k8s/helm/nexent/charts/nexent-nginx/templates/configmap.yaml |
Nginx configuration |
deploy/k8s/helm/nexent/charts/nexent-nginx/Chart.yaml |
Nginx subchart metadata |
deploy/k8s/helm/nexent/Chart.yaml |
Registers the Nginx dependency |
deploy/k8s/deploy.sh |
Prepares HTTPS for Helm deployment |
deploy/env/.env.example |
Documents HTTPS environment variables |
deploy/docker/deploy.sh |
Starts and manages the Docker HTTPS proxy |
deploy/docker/compose/docker-compose.yml |
Development Nginx service and port mapping |
deploy/docker/compose/docker-compose.prod.yml |
Production Nginx service and port mapping |
deploy/docker/assets/nginx/nginx.conf |
Docker Nginx TLS configuration |
deploy/common/common.sh |
Shared HTTPS configuration, validation, generation, and rendering |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+2470
to
+2477
| if [ -r "${DEPLOYMENT_HTTPS_CERT_PATH:-}" ] && [ -r "${DEPLOYMENT_HTTPS_KEY_PATH:-}" ]; then | ||
| printf ' tls:\n' | ||
| # Render PEM contents as a YAML literal block: multi-line certificates | ||
| # cannot be safely quoted on a single line. | ||
| printf ' cert: |\n' | ||
| sed 's/^/ /' "$DEPLOYMENT_HTTPS_CERT_PATH" | ||
| printf ' key: |\n' | ||
| sed 's/^/ /' "$DEPLOYMENT_HTTPS_KEY_PATH" |
| - nexent | ||
| ports: | ||
| - "3000:3000" | ||
| - "${NEXENT_WEB_PORT_MAPPING-3000:3000}" |
| - nexent | ||
| ports: | ||
| - "3000:3000" | ||
| - "${NEXENT_WEB_PORT_MAPPING-3000:3000}" |
Comment on lines
+2008
to
+2009
| # Start Nginx HTTPS reverse proxy when HTTPS is enabled | ||
| deploy_https_nginx || { |
Comment on lines
+867
to
+869
| DEPLOYMENT_HTTPS_MODE="disabled" | ||
| DEPLOYMENT_HTTPS_CERT_FILE="" | ||
| DEPLOYMENT_HTTPS_KEY_FILE="" |
Comment on lines
+2868
to
+2872
| if [ -r "$cert_file" ] && [ -r "$key_file" ]; then | ||
| if openssl x509 -in "$cert_file" -noout >/dev/null 2>&1 && openssl rsa -in "$key_file" -check -noout >/dev/null 2>&1; then | ||
| DEPLOYMENT_HTTPS_CERT_PATH="$cert_file" | ||
| DEPLOYMENT_HTTPS_KEY_PATH="$key_file" | ||
| return 0 |
Comment on lines
+2977
to
+2980
| if [ "$DEPLOYMENT_HTTPS_MODE" != "disabled" ]; then | ||
| deployment_update_env_var_file "$(deployment_env_dir)/.env" "NEXENT_HTTPS_MODE" "$DEPLOYMENT_HTTPS_MODE" | ||
| [ -n "${DEPLOYMENT_HTTPS_SAN_RESOLVED:-}" ] && deployment_update_env_var_file "$(deployment_env_dir)/.env" "NEXENT_HTTPS_SAN" "$DEPLOYMENT_HTTPS_SAN_RESOLVED" | ||
| fi |
|
|
||
|
|
||
| nexent-nginx: | ||
| image: nginx:alpine |
|
|
||
|
|
||
| nexent-nginx: | ||
| image: nginx:alpine |
Comment on lines
+1237
to
+1239
| if [ "$DEPLOYMENT_HTTPS_MODE" = "disabled" ] || [ -z "$DEPLOYMENT_HTTPS_MODE" ]; then | ||
| export NEXENT_WEB_PORT_MAPPING="3000:3000" | ||
| return 0 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
- Seed HTTPS config from NEXENT_HTTPS_* env vars before defaults - Detect SAN via ip -o -4 addr (Linux) with ifconfig fallback - Start docker nginx before core services; clean up on disabled; bind web to loopback 3001 when HTTPS is enabled - chmod 600 generated Helm values containing private keys - Support EC keys via openssl pkey with LibreSSL fallback - Route nginx image through NGINX_IMAGE and registry prefix - Validate cert/key pair and expiry when reusing self-signed certs - Persist custom key passphrase to .env
- Give HTTPS a dedicated entry: Docker port 3100, K8s NodePort 31000 - Keep the plain HTTP entry unchanged (Docker 3000 / K8s NodePort 30000); web no longer steps down to ClusterIP or a loopback-only binding - Drop the same-port error_page 497 redirect now that entries are split - Update TUI/summary wording, tests and install docs for the new ports
Offline hosts cannot pull the nginx image after delivery when the user enables HTTPS during installation. Bundle nginx:alpine with the infrastructure images instead of gating it on build-time HTTPS options, and drop the HTTPS flags from the offline build passthrough list.
Introduce NEXENT_WEB_PORT and NEXENT_HTTPS_PORT so users can change the entry ports for both Docker (host port mapping) and Kubernetes (NodePort) deployments. Defaults keep the current behavior (Docker 3000/3100, K8s 30000/31000). Also fix the deploy script overwriting a user-set web port mapping when HTTPS is disabled, and document the new variables in .env.example and the installation guides.
Reject NEXENT_WEB_PORT and NEXENT_HTTPS_PORT values outside the Kubernetes nodePort range (30000-32767) before rendering Helm values, so users get a clear error instead of a cryptic API server message. Ports are only validated when HTTPS mode needs them, and empty values keep using defaults.
The live Helm values renderer (deployment_render_helm_chart_values) only rendered the nginx image block, so the nginx Service fell back to the chart default NodePort and a user-set NEXENT_HTTPS_PORT never took effect. Render the nginx services block the same way the port-policy renderer does, and cover it with a Helm values assertion.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.


Uh oh!
There was an error while loading. Please reload this page.