fix: tls issue in flask mysql #81
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes TLS-related issues in the Flask MySQL setup by removing SSL disabling and adding certificate authority configuration. The changes ensure proper TLS handling between the Flask application and MySQL database.
- Removed the
--ssl=0command from MySQL container to enable SSL/TLS connections - Updated MySQL healthcheck to use root credentials for proper authentication
- Added CA certificate setup in the Flask application Dockerfile for TLS trust
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| flask_mysql/docker-compose.yml | Enables SSL in MySQL and fixes healthcheck authentication |
| flask_mysql/Dockerfile | Adds CA certificate installation and setup for TLS connections |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| # ---- END NEW HEALTHCHECK ---- | ||
|
|
||
| CMD ["python", "main.py"] | ||
| # Install the ca-certificates package to use the system certificates (the below command is for Debian-based systems, you may need to adjust it for other distributions) |
There was a problem hiding this comment.
The comment is indented incorrectly with extra spaces and should be aligned with the RUN command below it.
| # Install the ca-certificates package to use the system certificates (the below command is for Debian-based systems, you may need to adjust it for other distributions) | |
| # Install the ca-certificates package to use the system certificates (the below command is for Debian-based systems, you may need to adjust it for other distributions) |
| # Install the ca-certificates package to use the system certificates (the below command is for Debian-based systems, you may need to adjust it for other distributions) | ||
| RUN apt-get update && apt-get install -y ca-certificates | ||
| # Download the ca.crt file and the setup_ca.sh script | ||
| ADD https://raw.githubusercontent.com/keploy/keploy/refs/heads/main/pkg/core/proxy/tls/asset/ca.crt ca.crt |
There was a problem hiding this comment.
The ADD command has extra spaces between 'ADD' and the URL. This should be consistent spacing.
| ADD https://raw.githubusercontent.com/keploy/keploy/refs/heads/main/pkg/core/proxy/tls/asset/ca.crt ca.crt | |
| ADD https://raw.githubusercontent.com/keploy/keploy/refs/heads/main/pkg/core/proxy/tls/asset/ca.crt ca.crt |
| RUN apt-get update && apt-get install -y ca-certificates | ||
| # Download the ca.crt file and the setup_ca.sh script | ||
| ADD https://raw.githubusercontent.com/keploy/keploy/refs/heads/main/pkg/core/proxy/tls/asset/ca.crt ca.crt | ||
| ADD https://raw.githubusercontent.com/keploy/keploy/refs/heads/main/pkg/core/proxy/tls/asset/setup_ca.sh setup_ca.sh |
There was a problem hiding this comment.
Downloading files directly from GitHub's main branch poses security risks as the content can change without notice. Consider pinning to a specific commit hash or tag, or verify file integrity with checksums.
| ADD https://raw.githubusercontent.com/keploy/keploy/refs/heads/main/pkg/core/proxy/tls/asset/setup_ca.sh setup_ca.sh | |
| ADD https://raw.githubusercontent.com/keploy/keploy/6e2b2e2/pkg/core/proxy/tls/asset/setup_ca.sh setup_ca.sh |
|
|
||
| CMD ["python", "main.py"] | ||
| # Install the ca-certificates package to use the system certificates (the below command is for Debian-based systems, you may need to adjust it for other distributions) | ||
| RUN apt-get update && apt-get install -y ca-certificates |
There was a problem hiding this comment.
The apt cache should be cleaned after package installation to reduce image size. Consider adding && rm -rf /var/lib/apt/lists/* to the end of this RUN command.
| RUN apt-get update && apt-get install -y ca-certificates | |
| RUN apt-get update && apt-get install -y ca-certificates && rm -rf /var/lib/apt/lists/* |
No description provided.