diff --git a/mod_ci/controllers.py b/mod_ci/controllers.py index e60c0e0d..58e142f4 100755 --- a/mod_ci/controllers.py +++ b/mod_ci/controllers.py @@ -122,6 +122,78 @@ def safe_db_commit(db, operation_description: str = "database operation") -> boo return False +# User-friendly messages for known GCP error codes +GCP_ERROR_MESSAGES = { + 'ZONE_RESOURCE_POOL_EXHAUSTED': ( + "GCP resources temporarily unavailable in the configured zone. " + "The test will be retried automatically when resources become available." + ), + 'QUOTA_EXCEEDED': ( + "GCP quota limit reached. Please wait for other tests to complete " + "or contact the administrator." + ), + 'RESOURCE_NOT_FOUND': "Required GCP resource not found. Please contact the administrator.", + 'RESOURCE_ALREADY_EXISTS': "A VM with this name already exists. Please contact the administrator.", + 'TIMEOUT': "GCP operation timed out. The test will be retried automatically.", +} + + +def parse_gcp_error(result: Dict, log=None) -> str: + """ + Parse a GCP API error response and return a user-friendly message. + + GCP errors have the structure: + { + 'error': { + 'errors': [{'code': 'ERROR_CODE', 'message': '...'}] + } + } + + For known error codes, returns a user-friendly message. + For unknown errors, logs the details server-side and returns a generic message + to avoid exposing potentially sensitive information. + + :param result: The GCP API response dictionary + :param log: Optional logger instance. If not provided, uses module logger. + :return: A user-friendly error message + """ + import logging + if log is None: + log = logging.getLogger('Platform') + + if not isinstance(result, dict): + log.error(f"GCP error (non-dict): {result}") + return "VM creation failed. Please contact the administrator." + + error = result.get('error') + if error is None: + log.error(f"GCP error (no error key): {result}") + return "VM creation failed. Please contact the administrator." + + if not isinstance(error, dict): + log.error(f"GCP error (error not dict): {error}") + return "VM creation failed. Please contact the administrator." + + errors = error.get('errors', []) + if not errors: + log.error(f"GCP error (empty errors list): {error}") + return "VM creation failed. Please contact the administrator." + + # Get the first error (usually the most relevant) + first_error = errors[0] if isinstance(errors, list) and len(errors) > 0 else {} + error_code = first_error.get('code', 'UNKNOWN') + error_message = first_error.get('message', 'No details provided') + + # Check if we have a user-friendly message for this error code + if error_code in GCP_ERROR_MESSAGES: + return GCP_ERROR_MESSAGES[error_code] + + # For unknown errors, log full details server-side but return generic message + # to avoid exposing potentially sensitive information (project names, zones, etc.) + log.error(f"GCP error ({error_code}): {error_message}") + return f"VM creation failed ({error_code}). Please contact the administrator." + + mod_ci = Blueprint('ci', __name__) @@ -782,9 +854,9 @@ def start_test(compute, app, db, repository: Repository.Repository, test, bot_to if not safe_db_commit(db, f"recording GCP instance for test {test.id}"): log.error(f"Failed to record GCP instance for test {test.id}, but VM was created") else: - error_msg = result.get('error', 'Unknown error') if isinstance(result, dict) else str(result) + error_msg = parse_gcp_error(result) log.error(f"Error creating test instance for test {test.id}, result: {result}") - mark_test_failed(db, test, repository, f"Failed to create VM: {error_msg}") + mark_test_failed(db, test, repository, error_msg) def create_instance(compute, project, zone, test, reportURL) -> Dict: diff --git a/tests/test_ci/test_controllers.py b/tests/test_ci/test_controllers.py index d9ed6152..59db5bc2 100644 --- a/tests/test_ci/test_controllers.py +++ b/tests/test_ci/test_controllers.py @@ -1,4 +1,5 @@ import json +import unittest from importlib import reload from unittest import mock from unittest.mock import MagicMock @@ -3092,3 +3093,158 @@ def test_mark_test_failed_includes_target_url(self, mock_progress, mock_update_g call_args = mock_update_github.call_args self.assertEqual(len(call_args[0]), 5) # 5 positional args including target_url self.assertIn("456", call_args[0][4]) # target_url contains test ID + + +class TestParseGcpError(unittest.TestCase): + """Tests for the parse_gcp_error helper function.""" + + def test_parse_gcp_error_zone_resource_exhausted(self): + """Test that ZONE_RESOURCE_POOL_EXHAUSTED returns user-friendly message.""" + from mod_ci.controllers import parse_gcp_error + + mock_log = MagicMock() + result = { + 'status': 'DONE', + 'error': { + 'errors': [{ + 'code': 'ZONE_RESOURCE_POOL_EXHAUSTED', + 'message': "The zone 'projects/test/zones/us-central1-a' does not " + "have enough resources available to fulfill the request." + }] + } + } + + error_msg = parse_gcp_error(result, log=mock_log) + self.assertIn("GCP resources temporarily unavailable", error_msg) + self.assertIn("retried automatically", error_msg) + # Should NOT contain raw technical details + self.assertNotIn("us-central1-a", error_msg) + + def test_parse_gcp_error_quota_exceeded(self): + """Test that QUOTA_EXCEEDED returns user-friendly message.""" + from mod_ci.controllers import parse_gcp_error + + mock_log = MagicMock() + result = { + 'error': { + 'errors': [{ + 'code': 'QUOTA_EXCEEDED', + 'message': 'Quota exceeded for resource.' + }] + } + } + + error_msg = parse_gcp_error(result, log=mock_log) + self.assertIn("quota limit reached", error_msg) + + def test_parse_gcp_error_timeout(self): + """Test that TIMEOUT returns user-friendly message.""" + from mod_ci.controllers import parse_gcp_error + + mock_log = MagicMock() + result = { + 'status': 'TIMEOUT', + 'error': { + 'errors': [{ + 'code': 'TIMEOUT', + 'message': 'Operation timed out after 1800 seconds' + }] + } + } + + error_msg = parse_gcp_error(result, log=mock_log) + self.assertIn("timed out", error_msg) + self.assertIn("retried automatically", error_msg) + + def test_parse_gcp_error_unknown_code(self): + """Test that unknown error codes return generic message and log details.""" + from mod_ci.controllers import parse_gcp_error + + mock_log = MagicMock() + result = { + 'error': { + 'errors': [{ + 'code': 'SOME_NEW_ERROR', + 'message': 'Something unexpected happened.' + }] + } + } + + error_msg = parse_gcp_error(result, log=mock_log) + # Should include error code but not the full message (security) + self.assertIn("SOME_NEW_ERROR", error_msg) + self.assertIn("contact the administrator", error_msg) + # Should NOT expose the raw error message + self.assertNotIn("Something unexpected happened", error_msg) + # Should log the full details server-side + mock_log.error.assert_called_once() + self.assertIn("SOME_NEW_ERROR", mock_log.error.call_args[0][0]) + self.assertIn("Something unexpected happened", mock_log.error.call_args[0][0]) + + def test_parse_gcp_error_logs_sensitive_info(self): + """Test that sensitive info is logged but not returned to user.""" + from mod_ci.controllers import parse_gcp_error + + mock_log = MagicMock() + result = { + 'error': { + 'errors': [{ + 'code': 'UNKNOWN_ERROR', + 'message': 'Error in project my-secret-project zone us-central1-a' + }] + } + } + + error_msg = parse_gcp_error(result, log=mock_log) + # Should NOT expose project/zone names + self.assertNotIn("my-secret-project", error_msg) + self.assertNotIn("us-central1-a", error_msg) + # But should log them server-side + mock_log.error.assert_called_once() + self.assertIn("my-secret-project", mock_log.error.call_args[0][0]) + + def test_parse_gcp_error_no_error_key(self): + """Test handling when 'error' key is missing.""" + from mod_ci.controllers import parse_gcp_error + + mock_log = MagicMock() + result = {'status': 'DONE'} + + error_msg = parse_gcp_error(result, log=mock_log) + self.assertIn("VM creation failed", error_msg) + mock_log.error.assert_called_once() + + def test_parse_gcp_error_empty_errors_list(self): + """Test handling when 'errors' list is empty.""" + from mod_ci.controllers import parse_gcp_error + + mock_log = MagicMock() + result = {'error': {'errors': []}} + + error_msg = parse_gcp_error(result, log=mock_log) + self.assertIn("VM creation failed", error_msg) + mock_log.error.assert_called_once() + + def test_parse_gcp_error_not_a_dict(self): + """Test handling when result is not a dictionary.""" + from mod_ci.controllers import parse_gcp_error + + mock_log = MagicMock() + error_msg = parse_gcp_error("some string error", log=mock_log) + self.assertIn("VM creation failed", error_msg) + # Should NOT expose the raw input + self.assertNotIn("some string error", error_msg) + # But should log it + mock_log.error.assert_called_once() + self.assertIn("some string error", mock_log.error.call_args[0][0]) + + def test_parse_gcp_error_error_not_a_dict(self): + """Test handling when 'error' value is not a dictionary.""" + from mod_ci.controllers import parse_gcp_error + + mock_log = MagicMock() + result = {'error': 'just a string'} + + error_msg = parse_gcp_error(result, log=mock_log) + self.assertIn("VM creation failed", error_msg) + mock_log.error.assert_called_once()