diff --git a/.github/workflows/e2e-s3tests.yml b/.github/workflows/e2e-s3tests.yml index 6429d68f6..7e24d4e90 100644 --- a/.github/workflows/e2e-s3tests.yml +++ b/.github/workflows/e2e-s3tests.yml @@ -311,7 +311,7 @@ jobs: - name: Wait for RustFS ready run: | for _ in {1..120}; do - if curl -sf "http://${S3_HOST}:${S3_PORT}/health" >/dev/null 2>&1; then + if curl -sf "http://${S3_HOST}:${S3_PORT}/health/ready" >/dev/null 2>&1; then echo "RustFS is ready" exit 0 fi diff --git a/.github/workflows/mint.yml b/.github/workflows/mint.yml index 25ae02916..425e13609 100644 --- a/.github/workflows/mint.yml +++ b/.github/workflows/mint.yml @@ -155,7 +155,7 @@ jobs: - name: Wait for RustFS ready run: | for _ in {1..60}; do - if curl -sf http://127.0.0.1:9000/health >/dev/null 2>&1; then + if curl -sf http://127.0.0.1:9000/health/ready >/dev/null 2>&1; then echo "RustFS is ready" exit 0 fi diff --git a/scripts/check_test_wiring.py b/scripts/check_test_wiring.py index 4f652a14c..fc7101045 100755 --- a/scripts/check_test_wiring.py +++ b/scripts/check_test_wiring.py @@ -228,9 +228,120 @@ def check_runner_selection(root: Path) -> list[str]: def check_s3_tests_runner(root: Path) -> list[str]: runner = (root / "scripts/s3-tests/run.sh").read_text() + errors: list[str] = [] if "--showlocals" in runner: - return ["scripts/s3-tests/run.sh: pytest failure diagnostics must not dump local values"] - return [] + errors.append("scripts/s3-tests/run.sh: pytest failure diagnostics must not dump local values") + readiness = re.search( + r"test_s3_api_ready\(\) \{(?P.*?)\n\}\n\n# First, wait", + runner, + re.DOTALL, + ) + readiness_body = readiness.group("body") if readiness else "" + if ( + '"http://${S3_HOST}:${S3_PORT}/health/ready"' not in readiness_body + or re.search(r"/health(?=[\"'\s])", readiness_body) + or '[ "${READY_CODE}" != "200" ]' not in readiness_body + ): + errors.append("scripts/s3-tests/run.sh: startup must require the ready endpoint to return HTTP 200") + signed_probe = re.search( + r"if command -v awscurl\b[^\n]*; then(?P.*?)\n\s*fi\s*" + r"(?:#[^\n]*\n\s*)*return 0\s*$", + readiness_body, + re.DOTALL, + ) + signed_body = signed_probe.group("body") if signed_probe else "" + readiness_code = "\n".join(line.split("#", 1)[0] for line in readiness_body.splitlines()) + signed_code = "\n".join(line.split("#", 1)[0] for line in signed_body.splitlines()) + signed_commands = [ + command + for line in signed_body.splitlines() + if (command := line.split("#", 1)[0].strip()) + ] + signed_success = re.search( + r'if echo "\$\{RESPONSE\}" \| grep -q ".*?)\)\s*$", + signed_code, + re.DOTALL | re.MULTILINE, + ) + response_command = response_capture.group("command") if response_capture else "" + response_command = response_command.replace("\\\n", " ").strip() + response_operators = re.search( + r";|\|\||&&|(?|])\|(?!\|)|(?&])&(?![>&0-9])|\$\(|[<>]\(|`", + response_command, + ) + if ( + len(re.findall(r"\breturn\s+0\b", readiness_code)) != 2 + or len(re.findall(r"\breturn\s+0\b", signed_code)) != 1 + or len(re.findall(r"(? list[str]: + errors: list[str] = [] + for relative in (".github/workflows/e2e-s3tests.yml", ".github/workflows/mint.yml"): + path = root / relative + try: + lines = path.read_text().splitlines() + except FileNotFoundError: + errors.append(f"{relative}: missing workflow") + continue + start = next( + (index for index, line in enumerate(lines) if line.strip() == "- name: Wait for RustFS ready"), + None, + ) + if start is None: + errors.append(f"{relative}: missing RustFS readiness step") + continue + indent = len(lines[start]) - len(lines[start].lstrip()) + end = next( + ( + index + for index in range(start + 1, len(lines)) + if lines[index].strip().startswith("- name:") + and len(lines[index]) - len(lines[index].lstrip()) <= indent + ), + len(lines), + ) + readiness_step = "\n".join(lines[start:end]) + ready_branch = re.search( + r"if curl [^\n]*/health/ready[^\n]*; then(?P.*?)\n\s*fi", + readiness_step, + re.DOTALL, + ) + ready_body = ready_branch.group("body") if ready_branch else "" + ready_condition = ready_branch.group(0).splitlines()[0].rsplit("; then", 1)[0] if ready_branch else "" + step_commands = [ + command + for line in readiness_step.splitlines() + if (command := line.split("#", 1)[0].strip()) + ] + step_code = "\n".join(step_commands) + ready_code = "\n".join(line.split("#", 1)[0] for line in ready_body.splitlines()) + if ( + "/health/ready" not in readiness_step + or re.search(r"/health(?=[\"'\s])", readiness_step) + or not re.search(r"curl [^\n]*(?:-sf|-fs|--fail)", readiness_step) + or not ready_branch + or re.search(r"\|\||&&|(?|])\|(?!\|)|;|(?&])&(?![>&])", ready_condition) + or len(re.findall(r"\bexit\s+0\b", step_code)) != 1 + or not re.search(r"\bexit\s+0\b", ready_code) + or not step_commands + or step_commands[-1] != "exit 1" + ): + errors.append(f"{relative}: RustFS readiness step must fail closed on /health/ready") + return errors def profile_selection_entries(root: Path, profile: str) -> tuple[Path, list[str], dict[str, str]]: @@ -607,6 +718,7 @@ def validate(root: Path) -> list[str]: errors.extend(check_fuzz_targets(root)) errors.extend(check_runner_selection(root)) errors.extend(check_s3_tests_runner(root)) + errors.extend(check_workflow_readiness(root)) errors.extend(check_profile_definitions(root)) errors.extend(check_scheduled_alerts(root)) return errors @@ -715,19 +827,114 @@ class SelfTests(unittest.TestCase): root = Path(tmp) runner = root / "scripts/s3-tests/run.sh" runner.parent.mkdir(parents=True) - runner.write_text("tox -- -vv -ra --tb=long\n") + valid_runner = ( + "test_s3_api_ready() {\n" + ' READY_CODE=$(curl "http://${S3_HOST}:${S3_PORT}/health/ready")\n' + ' if [ "${READY_CODE}" != "200" ]; then\n' + " return 1\n" + " fi\n" + " if command -v awscurl; then\n" + " RESPONSE=$(awscurl --service s3)\n" + ' if echo "${RESPONSE}" | grep -q " None: + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + workflows = root / ".github/workflows" + workflows.mkdir(parents=True) + valid_workflow = ( + "jobs:\n" + " test:\n" + " steps:\n" + " - name: Wait for RustFS ready\n" + " run: |\n" + " for _ in {1..60}; do\n" + " if curl -sf http://127.0.0.1:9000/health/ready; then\n" + " exit 0\n" + " fi\n" + " done\n" + " exit 1\n" + " - name: Run tests\n" + " run: true\n" + ) + for name in ("e2e-s3tests.yml", "mint.yml"): + (workflows / name).write_text(valid_workflow) + self.assertEqual(check_workflow_readiness(root), []) + + (workflows / "mint.yml").write_text(valid_workflow.replace("/health/ready", "/health")) + self.assertEqual(len(check_workflow_readiness(root)), 1) + (workflows / "mint.yml").write_text( + valid_workflow.replace( + "if curl -sf http://127.0.0.1:9000/health/ready; then", + "if curl -sf http://127.0.0.1:9000/health/ready || true; then", + ) + ) + self.assertEqual(len(check_workflow_readiness(root)), 1) + (workflows / "mint.yml").write_text( + valid_workflow.replace( + "if curl -sf http://127.0.0.1:9000/health/ready; then", + "if curl -sf http://127.0.0.1:9000/health/ready || :; then", + ) + ) + self.assertEqual(len(check_workflow_readiness(root)), 1) + (workflows / "mint.yml").write_text( + valid_workflow.replace( + "if curl -sf http://127.0.0.1:9000/health/ready; then\n exit 0\n fi", + "curl -sf http://127.0.0.1:9000/health/ready || true\n exit 0", + ) + ) + self.assertEqual(len(check_workflow_readiness(root)), 1) + def test_profile_listing_enforces_selection(self) -> None: with tempfile.TemporaryDirectory() as tmp: root = Path(tmp) diff --git a/scripts/s3-tests/run.sh b/scripts/s3-tests/run.sh index 74e2289f6..bd197e0f5 100755 --- a/scripts/s3-tests/run.sh +++ b/scripts/s3-tests/run.sh @@ -584,19 +584,17 @@ check_server_ready_from_log() { # Test S3 API readiness test_s3_api_ready() { - # Step 1: Check if server is responding using /health endpoint - # /health is a probe path that bypasses readiness gate, so it can be used - # to check if the server is up and running, even if readiness gate is not ready yet - HEALTH_CODE=$(curl -s -o /dev/null -w "%{http_code}" \ + # Step 1: Require the dependency-aware readiness endpoint. + READY_CODE=$(curl -s -o /dev/null -w "%{http_code}" \ -X GET \ - "http://${S3_HOST}:${S3_PORT}/health" \ + "http://${S3_HOST}:${S3_PORT}/health/ready" \ --max-time 5 2>/dev/null || echo "000") - if [ "${HEALTH_CODE}" = "000" ]; then + if [ "${READY_CODE}" = "000" ]; then # Connection failed - server might not be running or not listening yet return 1 - elif [ "${HEALTH_CODE}" != "200" ]; then - # Health endpoint returned non-200 status, server might have issues + elif [ "${READY_CODE}" != "200" ]; then + # The service is live but its storage/IAM dependencies are not ready. return 1 fi @@ -617,16 +615,11 @@ test_s3_api_ready() { if echo "${RESPONSE}" | grep -q "503\|Service not ready"; then return 1 # Not ready yet (readiness gate is blocking S3 API) fi - # Other errors from awscurl - might be auth issues or other problems - # But server is up, so we'll consider it ready (S3 API might have other issues) - return 0 + # Authentication, server, and transport failures are not readiness. + return 1 fi - # Step 3: Fallback - if /health returns 200, server is up and readiness gate is ready - # Since /health is a probe path and returns 200, and we don't have awscurl to test S3 API, - # we can assume the server is ready. The readiness gate would have blocked /health if not ready. - # Note: Root path "/" with HEAD method returns 501 Not Implemented (S3 doesn't support HEAD on root), - # so we can't use it as a reliable test. Since /health already confirmed readiness, we return success. + # /health/ready is authoritative when the optional signed probe is absent. return 0 }