From f4cbdc3a3b84225181d7bcd7d0e92676358da340 Mon Sep 17 00:00:00 2001 From: PandaMan Date: Wed, 25 Feb 2026 09:47:12 +0800 Subject: [PATCH] fix(api): MinIO health check use dynamic scheme and verify (Closes #13159 and #13158) (#13197) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Fixes MinIO SSL/TLS support in two places: the MinIO **client** connection and the **health check** used by the Admin/Service Health dashboard. Both now respect the `secure` and `verify` settings from the MinIO configuration. Closes #13158 Closes #13159 --- ## Problem **#13158 – MinIO client:** The client in `rag/utils/minio_conn.py` was hardcoded with `secure=False`, so RAGFlow could not connect to MinIO over HTTPS even when `secure: true` was set in config. There was also no way to disable certificate verification for self-signed certs. **#13159 – MinIO health check:** In `api/utils/health_utils.py`, the MinIO liveness check always used `http://` for the health URL. When MinIO was configured with SSL, the health check failed and the dashboard showed "timeout" even though MinIO was reachable over HTTPS. --- ## Solution ### MinIO client (`rag/utils/minio_conn.py`) - Read `MINIO.secure` (default `false`) and pass it into the `Minio()` constructor so HTTPS is used when configured. - Add `_build_minio_http_client()` that reads `MINIO.verify` (default `true`). When `verify` is false, return an `urllib3.PoolManager` with `cert_reqs=ssl.CERT_NONE` and pass it as `http_client` to `Minio()` so self-signed certificates are accepted. - Support string values for `secure` and `verify` (e.g. `"true"`, `"false"`). ### MinIO health check (`api/utils/health_utils.py`) - Add `_minio_scheme_and_verify()` to derive URL scheme (http/https) and the `verify` flag from `MINIO.secure` and `MINIO.verify`. - Update `check_minio_alive()` to use the correct scheme, pass `verify` into `requests.get(..., verify=verify)`, and use `timeout=10`. ### Config template (`docker/service_conf.yaml.template`) - Add commented optional MinIO keys `secure` and `verify` (and env vars `MINIO_SECURE`, `MINIO_VERIFY`) so deployers know they can enable HTTPS and optional cert verification. ### Tests - **`test/unit_test/utils/test_health_utils_minio.py`** – Tests for `_minio_scheme_and_verify()` and `check_minio_alive()` (scheme, verify, status codes, timeout, errors). - **`test/unit_test/utils/test_minio_conn_ssl.py`** – Tests for `_build_minio_http_client()` (verify true/false/missing, string values, `CERT_NONE` when verify is false). --- ## Testing - Unit tests added/updated as above; run with the project's test runner. - Manually: configure MinIO with HTTPS and `secure: true` (and optionally `verify: false` for self-signed); confirm client operations work and the Service Health dashboard shows MinIO as alive instead of timeout. --- api/utils/health_utils.py | 32 +++- docker/service_conf.yaml.template | 4 + rag/utils/minio_conn.py | 30 +++- .../utils/test_health_utils_minio.py | 146 ++++++++++++++++++ test/unit_test/utils/test_minio_conn_ssl.py | 63 ++++++++ 5 files changed, 267 insertions(+), 8 deletions(-) create mode 100644 test/unit_test/utils/test_health_utils_minio.py create mode 100644 test/unit_test/utils/test_minio_conn_ssl.py diff --git a/api/utils/health_utils.py b/api/utils/health_utils.py index 7456ed0f88..f8222a291f 100644 --- a/api/utils/health_utils.py +++ b/api/utils/health_utils.py @@ -233,14 +233,40 @@ def get_mysql_status(): } +def _minio_scheme_and_verify(): + """ + Determine URL scheme (http/https) and SSL verify flag for MinIO health check. + Uses MINIO.secure for scheme and MINIO.verify for certificate verification + (e.g. self-signed certs when verify is False). + """ + secure = settings.MINIO.get("secure", False) + if isinstance(secure, str): + secure = secure.lower() in ("true", "1", "yes") + scheme = "https" if secure else "http" + verify = settings.MINIO.get("verify", True) + if isinstance(verify, str): + verify = verify.lower() not in ("false", "0", "no") + elif isinstance(verify, bool): + pass + else: + verify = bool(verify) + return scheme, verify + + def check_minio_alive(): + """ + Check MinIO service liveness via /minio/health/live. + Uses http or https and optional certificate verification based on + MINIO.secure and MINIO.verify configuration. + """ start_time = timer() try: - response = requests.get(f'http://{settings.MINIO["host"]}/minio/health/live') + scheme, verify = _minio_scheme_and_verify() + url = f"{scheme}://{settings.MINIO['host']}/minio/health/live" + response = requests.get(url, timeout=10, verify=verify) if response.status_code == 200: return {"status": "alive", "message": f"Confirm elapsed: {(timer() - start_time) * 1000.0:.1f} ms."} - else: - return {"status": "timeout", "message": f"Confirm elapsed: {(timer() - start_time) * 1000.0:.1f} ms."} + return {"status": "timeout", "message": f"Confirm elapsed: {(timer() - start_time) * 1000.0:.1f} ms."} except Exception as e: return { "status": "timeout", diff --git a/docker/service_conf.yaml.template b/docker/service_conf.yaml.template index f283f08530..4fac37f55d 100644 --- a/docker/service_conf.yaml.template +++ b/docker/service_conf.yaml.template @@ -19,6 +19,10 @@ minio: host: '${MINIO_HOST:-minio}:9000' bucket: '${MINIO_BUCKET:-}' prefix_path: '${MINIO_PREFIX_PATH:-}' + # optional: set to true for HTTPS (SSL/TLS). Used by MinIO client and health check. + # secure: ${MINIO_SECURE:-false} + # optional: set to false to allow self-signed certificates (e.g. in development). + # verify: ${MINIO_VERIFY:-true} es: hosts: 'http://${ES_HOST:-es01}:9200' username: '${ES_USER:-elastic}' diff --git a/rag/utils/minio_conn.py b/rag/utils/minio_conn.py index 595a00d1ca..3800b77774 100644 --- a/rag/utils/minio_conn.py +++ b/rag/utils/minio_conn.py @@ -15,15 +15,29 @@ # import logging +import ssl import time from minio import Minio from minio.commonconfig import CopySource from minio.error import S3Error, ServerError, InvalidResponseError from io import BytesIO +import urllib3 from common.decorator import singleton from common import settings +def _build_minio_http_client(): + """ + Build an optional urllib3 HTTP client for MinIO when using SSL/TLS. + Respects MINIO.verify (default True) to allow self-signed certificates + when set to False. + """ + verify = settings.MINIO.get("verify", True) + if verify is True or verify == "true" or verify == "1": + return None + return urllib3.PoolManager(cert_reqs=ssl.CERT_NONE) + + @singleton class RAGFlowMinio: def __init__(self): @@ -83,11 +97,17 @@ class RAGFlowMinio: pass try: - self.conn = Minio(settings.MINIO["host"], - access_key=settings.MINIO["user"], - secret_key=settings.MINIO["password"], - secure=False - ) + secure = settings.MINIO.get("secure", False) + if isinstance(secure, str): + secure = secure.lower() in ("true", "1", "yes") + http_client = _build_minio_http_client() + self.conn = Minio( + settings.MINIO["host"], + access_key=settings.MINIO["user"], + secret_key=settings.MINIO["password"], + secure=secure, + http_client=http_client, + ) except Exception: logging.exception( "Fail to connect %s " % settings.MINIO["host"]) diff --git a/test/unit_test/utils/test_health_utils_minio.py b/test/unit_test/utils/test_health_utils_minio.py new file mode 100644 index 0000000000..176ace64dd --- /dev/null +++ b/test/unit_test/utils/test_health_utils_minio.py @@ -0,0 +1,146 @@ +# +# Copyright 2025 The InfiniFlow Authors. All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +""" +Unit tests for MinIO health check (check_minio_alive) and scheme/verify helpers. +Covers SSL/HTTPS and certificate verification (issues #13158, #13159). +""" +from unittest.mock import patch, Mock + + +class TestMinioSchemeAndVerify: + """Test _minio_scheme_and_verify helper.""" + + @patch("api.utils.health_utils.settings") + def test_scheme_http_when_secure_false(self, mock_settings): + mock_settings.MINIO = {"host": "minio:9000", "secure": False} + from api.utils.health_utils import _minio_scheme_and_verify + scheme, verify = _minio_scheme_and_verify() + assert scheme == "http" + assert verify is True + + @patch("api.utils.health_utils.settings") + def test_scheme_https_when_secure_true(self, mock_settings): + mock_settings.MINIO = {"host": "minio:9000", "secure": True} + from api.utils.health_utils import _minio_scheme_and_verify + scheme, verify = _minio_scheme_and_verify() + assert scheme == "https" + assert verify is True + + @patch("api.utils.health_utils.settings") + def test_scheme_https_when_secure_string_true(self, mock_settings): + mock_settings.MINIO = {"host": "minio:9000", "secure": "true"} + from api.utils.health_utils import _minio_scheme_and_verify + scheme, verify = _minio_scheme_and_verify() + assert scheme == "https" + + @patch("api.utils.health_utils.settings") + def test_verify_false_for_self_signed(self, mock_settings): + mock_settings.MINIO = {"host": "minio:9000", "secure": True, "verify": False} + from api.utils.health_utils import _minio_scheme_and_verify + scheme, verify = _minio_scheme_and_verify() + assert scheme == "https" + assert verify is False + + @patch("api.utils.health_utils.settings") + def test_verify_string_false(self, mock_settings): + mock_settings.MINIO = {"host": "minio:9000", "verify": "false"} + from api.utils.health_utils import _minio_scheme_and_verify + _, verify = _minio_scheme_and_verify() + assert verify is False + + @patch("api.utils.health_utils.settings") + def test_default_verify_true_when_key_missing(self, mock_settings): + mock_settings.MINIO = {"host": "minio:9000"} + from api.utils.health_utils import _minio_scheme_and_verify + _, verify = _minio_scheme_and_verify() + assert verify is True + + +class TestCheckMinioAlive: + """Test check_minio_alive with mocked requests and settings.""" + + @patch("api.utils.health_utils.requests.get") + @patch("api.utils.health_utils.settings") + def test_returns_alive_when_http_200(self, mock_settings, mock_get): + mock_settings.MINIO = {"host": "minio:9000", "secure": False} + mock_response = Mock() + mock_response.status_code = 200 + mock_get.return_value = mock_response + from api.utils.health_utils import check_minio_alive + result = check_minio_alive() + assert result["status"] == "alive" + assert "elapsed" in result["message"] + mock_get.assert_called_once() + call_args = mock_get.call_args + assert call_args[0][0] == "http://minio:9000/minio/health/live" + assert call_args[1]["verify"] is True + + @patch("api.utils.health_utils.requests.get") + @patch("api.utils.health_utils.settings") + def test_uses_https_when_secure_true(self, mock_settings, mock_get): + mock_settings.MINIO = {"host": "minio:9000", "secure": True} + mock_response = Mock() + mock_response.status_code = 200 + mock_get.return_value = mock_response + from api.utils.health_utils import check_minio_alive + check_minio_alive() + call_args = mock_get.call_args + assert call_args[0][0] == "https://minio:9000/minio/health/live" + + @patch("api.utils.health_utils.requests.get") + @patch("api.utils.health_utils.settings") + def test_passes_verify_false_for_self_signed(self, mock_settings, mock_get): + mock_settings.MINIO = {"host": "minio:9000", "secure": True, "verify": False} + mock_response = Mock() + mock_response.status_code = 200 + mock_get.return_value = mock_response + from api.utils.health_utils import check_minio_alive + check_minio_alive() + call_args = mock_get.call_args + assert call_args[1]["verify"] is False + + @patch("api.utils.health_utils.requests.get") + @patch("api.utils.health_utils.settings") + def test_returns_timeout_on_non_200(self, mock_settings, mock_get): + mock_settings.MINIO = {"host": "minio:9000"} + mock_response = Mock() + mock_response.status_code = 503 + mock_get.return_value = mock_response + from api.utils.health_utils import check_minio_alive + result = check_minio_alive() + assert result["status"] == "timeout" + + @patch("api.utils.health_utils.requests.get") + @patch("api.utils.health_utils.settings") + def test_returns_timeout_on_request_exception(self, mock_settings, mock_get): + mock_settings.MINIO = {"host": "minio:9000"} + mock_get.side_effect = ConnectionError("Connection refused") + from api.utils.health_utils import check_minio_alive + result = check_minio_alive() + assert result["status"] == "timeout" + assert "error" in result["message"] + + @patch("api.utils.health_utils.requests.get") + @patch("api.utils.health_utils.settings") + def test_request_uses_timeout(self, mock_settings, mock_get): + mock_settings.MINIO = {"host": "minio:9000"} + mock_response = Mock() + mock_response.status_code = 200 + mock_get.return_value = mock_response + from api.utils.health_utils import check_minio_alive + check_minio_alive() + call_args = mock_get.call_args + assert call_args[1]["timeout"] == 10 diff --git a/test/unit_test/utils/test_minio_conn_ssl.py b/test/unit_test/utils/test_minio_conn_ssl.py new file mode 100644 index 0000000000..5fc87d3304 --- /dev/null +++ b/test/unit_test/utils/test_minio_conn_ssl.py @@ -0,0 +1,63 @@ +# +# Copyright 2025 The InfiniFlow Authors. All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +""" +Unit tests for MinIO client SSL/secure configuration (_build_minio_http_client). +Covers issue #13158. +""" +import ssl +from unittest.mock import patch + + +class TestBuildMinioHttpClient: + """Test _build_minio_http_client helper.""" + + @patch("rag.utils.minio_conn.settings") + def test_returns_none_when_verify_true(self, mock_settings): + mock_settings.MINIO = {"verify": True} + from rag.utils.minio_conn import _build_minio_http_client + client = _build_minio_http_client() + assert client is None + + @patch("rag.utils.minio_conn.settings") + def test_returns_none_when_verify_missing(self, mock_settings): + mock_settings.MINIO = {} + from rag.utils.minio_conn import _build_minio_http_client + client = _build_minio_http_client() + assert client is None + + @patch("rag.utils.minio_conn.settings") + def test_returns_pool_manager_when_verify_false(self, mock_settings): + mock_settings.MINIO = {"verify": False} + from rag.utils.minio_conn import _build_minio_http_client + client = _build_minio_http_client() + assert client is not None + assert hasattr(client, "connection_pool_kw") + assert client.connection_pool_kw.get("cert_reqs") == ssl.CERT_NONE + + @patch("rag.utils.minio_conn.settings") + def test_returns_pool_manager_when_verify_string_false(self, mock_settings): + mock_settings.MINIO = {"verify": "false"} + from rag.utils.minio_conn import _build_minio_http_client + client = _build_minio_http_client() + assert client is not None + assert client.connection_pool_kw.get("cert_reqs") == ssl.CERT_NONE + + @patch("rag.utils.minio_conn.settings") + def test_returns_none_when_verify_string_1(self, mock_settings): + mock_settings.MINIO = {"verify": "1"} + from rag.utils.minio_conn import _build_minio_http_client + client = _build_minio_http_client() + assert client is None