Feat/nginx proxy and cert handling

[dmccoystephenson]

New Base Branch

The wydot-deployment-2026 branch was created using the 2.0.1 release tag as a base. The following changes were made using the previous changes in the wydot-deployment branch as a reference.

Changes

(TBD)

[Copilot]

Pull request overview

This PR implements NGINX as an SSL/TLS termination proxy for the JPO CV Manager application, along with certificate handling infrastructure and SSL support for PostgreSQL connections. The changes enable secure HTTPS access to all services through a single domain with proper certificate management.

Key Changes:

• Added NGINX reverse proxy with SSL/TLS termination for webapp, API, and Keycloak services
• Implemented PostgreSQL SSL connection support with certificate verification
• Added Keycloak SSL verification configuration for API-to-Keycloak communication
• Updated default active upgrade limit from 1 to 4

Reviewed changes

Copilot reviewed 15 out of 16 changed files in this pull request and generated 17 comments.

Show a summary per file

File	Description
resources/nginx/nginx-ssl.conf	New NGINX configuration with SSL settings and proxy rules for all services
resources/nginx/Dockerfile	Dockerfile for building NGINX proxy container with OpenSSL support
resources/nginx/gen_dhparam.sh	Script to generate Diffie-Hellman parameters for SSL
resources/nginx/README-testing.md	Documentation for local SSL proxy testing setup
resources/nginx/README-deployment.md	Documentation for production deployment with external database
services/common/pgquery.py	Added SSL context configuration for PostgreSQL connections with CA certificate support
services/common/tests/test_pgquery.py	Updated tests to account for new connect_args parameter
services/api/src/middleware.py	Added Keycloak SSL verification configuration and fixed typo in comment
services/Dockerfile.api	Added CA certificate installation and update process
docker-compose.yml	Added nginx_proxy service, updated Keycloak configuration for proxy mode, removed exposed ports
docker-compose-addons.yml	Added PG_SSL_REQUIRED environment variable to addon services, commented out firmware manager services
sample.env	Updated endpoints to HTTPS, added SSL-related environment variables
resources/keycloak/realm.json	Changed sslRequired to "all" and updated redirect URIs to HTTPS
services/addons/images/firmware_manager/upgrade_scheduler/upgrade_scheduler.py	Changed default ACTIVE_UPGRADE_LIMIT from 1 to 4
services/addons/tests/firmware_manager/upgrade_scheduler/test_upgrade_scheduler.py	Updated test assertion for new default upgrade limit
.gitignore	Added patterns to ignore SSL certificates and keys

Comments suppressed due to low confidence (1)

services/addons/images/firmware_manager/upgrade_scheduler/upgrade_scheduler.py:40

• The error handling logic in this try-except block should return or use the default value instead of raising a ValueError. Based on the function signature and test expectations, when ACTIVE_UPGRADE_LIMIT cannot be parsed as an integer, the function should return the default value (4) rather than crashing. The current ValueError will cause the application to fail, which contradicts the intent of having a default value.

    except ValueError:
        raise ValueError(
            "The environment variable 'ACTIVE_UPGRADE_LIMIT' must be an integer."

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

When SSL verification is disabled (check_hostname = False and verify_mode = ssl.CERT_NONE), the connection is vulnerable to man-in-the-middle attacks. While the code logs a warning, this fallback creates a security risk in production. Consider failing the connection if the CA certificate is not found rather than silently disabling verification, or at minimum make this behavior configurable through an environment variable.

[Copilot]

The cvmanager_nginx_proxy service has a dependency on cvmanager_postgres which may not exist when using an external database. According to the deployment documentation (README-deployment.md), users can configure an external PostgreSQL database and should be able to exclude the local postgres container. This hard dependency will cause deployment failures in such scenarios. Consider making this dependency conditional or removing it since the nginx proxy doesn't directly interact with the database.

Suggested change

- cvmanager_postgres

[Copilot]

The RUN command has a fallback pattern 'apk add --no-network --no-cache openssl || apk add --no-cache openssl' which is confusing. The '--no-network' flag is not a standard apk option and will cause the first command to fail, then the second command without this flag will execute. If the intent is to try installation without network first, this should be documented. Otherwise, simply use 'apk add --no-cache openssl' directly.

Suggested change

	RUN apk add --no-network --no-cache openssl || apk add --no-cache openssl
	RUN apk add --no-cache openssl

[Copilot]

The command 'hostname -I' outputs all IP addresses, which may include multiple addresses on systems with multiple network interfaces. This can be confusing for users. Consider suggesting a more specific command like 'hostname -I | awk '{print $1}'' or 'ip addr show eth0 | grep inet | awk '{print $2}' | cut -d/ -f1' to get a single, relevant IP address.

Suggested change

	*Note: Replace `<WSL_IP_ADDRESS>` with your actual WSL2 IP address. You can find this by running `hostname -I` in your WSL terminal.*
	*Note: Replace `<WSL_IP_ADDRESS>` with your actual WSL2 IP address. To avoid confusion on systems with multiple IPs, you can use a more specific command such as `hostname -I | awk '{print $1}'` in your WSL terminal to get a single, relevant address.*

[Copilot]

The error log is configured to write to access.log instead of error.log. This will mix error messages with access logs, making it difficult to debug issues and monitor system health. Change this to '/var/log/nginx/error.log'.

Suggested change

	error_log /var/log/nginx/access.log;
	error_log /var/log/nginx/error.log;

[Copilot]

MIME types configuration is missing from the nginx configuration. The 'include /etc/nginx/mime.types;' directive should be added to ensure proper Content-Type headers are set for different file types. Without this, the server may serve files with incorrect or missing MIME types, which can cause browser rendering issues.

Suggested change

	keepalive_timeout 65;
	keepalive_timeout 65;
	include /etc/nginx/mime.types;

[Copilot]

Typo in the comment: "pythjon-jose" should be "python-jose".

Suggested change

	# TODO: Consider using pythjon-jose or PyJWT to locally validate the token, instead of calling the Keycloak server
	# TODO: Consider using python-jose or PyJWT to locally validate the token, instead of calling the Keycloak server

[Copilot]

Running 'update-ca-certificates' on every container startup in the CMD is inefficient. This command should only need to run once during the image build process. The certificates are already copied and updated during the build with 'RUN update-ca-certificates' at line 8. Remove 'update-ca-certificates &&' from the CMD to avoid unnecessary overhead on each container start.

Suggested change

	CMD update-ca-certificates && exec gunicorn --bind :5000 --workers 1 --threads 8 --timeout 0 main:app
	CMD exec gunicorn --bind :5000 --workers 1 --threads 8 --timeout 0 main:app

[Copilot]

There's an inconsistency in the commented-out code. Line 177 adds a volume mount for the SSL certificate to the firmware_manager_upgrade_runner service, but this mount is not present in the firmware_manager_upgrade_scheduler service (which would also need to communicate securely if enabled). If these services are being prepared for SSL support, both should have consistent certificate mounts. Additionally, since these services are commented out, consider removing them entirely or documenting why they're disabled and when they'll be re-enabled.

[Copilot]

The ssl_protocols directive currently enables TLSv1 and TLSv1.1, which are deprecated and considered insecure, allowing clients to negotiate weak protocols and ciphers. An attacker on the network could potentially force downgrade connections to these older protocol versions and weaken the confidentiality and integrity of HTTPS traffic. Restrict ssl_protocols to only strong versions such as TLSv1.2 and TLSv1.3 to prevent downgrade to insecure protocols.

Suggested change

	ssl_protocols TLSv1 TLSv1.1 TLSv1.2 TLSv1.3;
	ssl_protocols TLSv1.2 TLSv1.3;