From ef49090c31efa8a791ee5558289f0ffb4672b968 Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Mon, 3 Mar 2025 15:56:10 -0500 Subject: [PATCH] feat: Add codejail darklaunch toggle. This adds a toggle for running codejail in both remote and local configurations for testing purposes. https://github.com/edx/edx-arch-experiments/issues/895 --- xmodule/capa/safe_exec/remote_exec.py | 24 +++++++ xmodule/capa/safe_exec/safe_exec.py | 36 +++++++++- .../capa/safe_exec/tests/test_safe_exec.py | 67 ++++++++++++++++++- 3 files changed, 124 insertions(+), 3 deletions(-) diff --git a/xmodule/capa/safe_exec/remote_exec.py b/xmodule/capa/safe_exec/remote_exec.py index a7aa7f8344..5b231eb156 100644 --- a/xmodule/capa/safe_exec/remote_exec.py +++ b/xmodule/capa/safe_exec/remote_exec.py @@ -29,11 +29,35 @@ ENABLE_CODEJAIL_REST_SERVICE = SettingToggle( "ENABLE_CODEJAIL_REST_SERVICE", default=False, module_name=__name__ ) +# .. toggle_name: ENABLE_CODEJAIL_DARKLAUNCH +# .. toggle_implementation: SettingToggle +# .. toggle_default: False +# .. toggle_description: Turn on to send requests to both the codejail service and the installed codejail library for +# testing and evaluation purposes. The results from the installed codejail library will be the ones used. +# .. toggle_warning: This toggle will only behave as expected when ENABLE_CODEJAIL_REST_SERVICE is not enabled and when +# CODE_JAIL_REST_SERVICE_REMOTE_EXEC, CODE_JAIL_REST_SERVICE_HOST, CODE_JAIL_REST_SERVICE_READ_TIMEOUT, +# and CODE_JAIL_REST_SERVICE_CONNECT_TIMEOUT are configured. +# .. toggle_use_cases: temporary +# .. toggle_creation_date: 2025-04-03 +# .. toggle_target_removal_date: 2025-05-01 +ENABLE_CODEJAIL_DARKLAUNCH = SettingToggle( + "ENABLE_CODEJAIL_DARKLAUNCH", default=False, module_name=__name__ +) + def is_codejail_rest_service_enabled(): return ENABLE_CODEJAIL_REST_SERVICE.is_enabled() +def is_codejail_in_darklaunch(): + """ + Returns whether codejail dark launch is enabled. + + Codejail dark launch can only be enabled if ENABLE_CODEJAIL_REST_SERVICE is not enabled. + """ + return not is_codejail_rest_service_enabled() and ENABLE_CODEJAIL_DARKLAUNCH.is_enabled() + + def get_remote_exec(*args, **kwargs): """Get remote exec function based on setting and executes it.""" remote_exec_function_name = settings.CODE_JAIL_REST_SERVICE_REMOTE_EXEC diff --git a/xmodule/capa/safe_exec/safe_exec.py b/xmodule/capa/safe_exec/safe_exec.py index d843a02831..5b1b2a88bd 100644 --- a/xmodule/capa/safe_exec/safe_exec.py +++ b/xmodule/capa/safe_exec/safe_exec.py @@ -1,13 +1,18 @@ """Capa's specialized use of codejail.safe_exec.""" +import copy import hashlib +import logging from codejail.safe_exec import SafeExecException, json_safe from codejail.safe_exec import not_safe_exec as codejail_not_safe_exec from codejail.safe_exec import safe_exec as codejail_safe_exec -from edx_django_utils.monitoring import function_trace +from edx_django_utils.monitoring import function_trace, record_exception from . import lazymod -from .remote_exec import is_codejail_rest_service_enabled, get_remote_exec +from .remote_exec import is_codejail_rest_service_enabled, is_codejail_in_darklaunch, get_remote_exec + +log = logging.getLogger(__name__) + # Establish the Python environment for Capa. # Capa assumes float-friendly division always. @@ -155,6 +160,7 @@ def safe_exec( emsg, exception = get_remote_exec(data) else: + # Decide which code executor to use. if unsafely: exec_fn = codejail_not_safe_exec @@ -178,6 +184,32 @@ def safe_exec( else: emsg = None + # Run the code in both the remote codejail service as well as the local codejail + # when in darklaunch mode. + if is_codejail_in_darklaunch(): + try: + # Create a copy so the originals are not modified as part of this call. + darklaunch_globals = copy.deepcopy(globals_dict) + data = { + "code": code_prolog + LAZY_IMPORTS + code, + "globals_dict": darklaunch_globals, + "python_path": python_path, + "limit_overrides_context": limit_overrides_context, + "slug": slug, + "unsafely": unsafely, + "extra_files": extra_files, + } + remote_emsg, _remote_exception = get_remote_exec(data) + log.info( + f"Remote execution in darklaunch mode produces: {darklaunch_globals} or exception: {remote_emsg}" + ) + log.info(f"Local execution in darklaunch mode produces: {globals_dict} or exception: {emsg}") + except Exception as e: # pragma: no cover # pylint: disable=broad-except + # Swallows all exceptions and logs it in monitoring so that dark launch doesn't cause issues during + # deploy. + log.exception("Error occurred while trying to remote exec in dark launch mode.") + record_exception() + # Put the result back in the cache. This is complicated by the fact that # the globals dict might not be entirely serializable. if cache: diff --git a/xmodule/capa/safe_exec/tests/test_safe_exec.py b/xmodule/capa/safe_exec/tests/test_safe_exec.py index 36d2cc9657..7cb74c9402 100644 --- a/xmodule/capa/safe_exec/tests/test_safe_exec.py +++ b/xmodule/capa/safe_exec/tests/test_safe_exec.py @@ -6,6 +6,7 @@ import os import os.path import textwrap import unittest +from unittest.mock import patch import pytest import random2 as random @@ -20,7 +21,7 @@ from six.moves import range from openedx.core.djangolib.testing.utils import skip_unless_lms from xmodule.capa.safe_exec import safe_exec, update_hash -from xmodule.capa.safe_exec.remote_exec import is_codejail_rest_service_enabled +from xmodule.capa.safe_exec.remote_exec import is_codejail_in_darklaunch, is_codejail_rest_service_enabled class TestSafeExec(unittest.TestCase): # lint-amnesty, pylint: disable=missing-class-docstring @@ -125,6 +126,70 @@ class TestSafeOrNot(unittest.TestCase): # lint-amnesty, pylint: disable=missing assert "SystemExit" in str(cm) +class TestCodeJailDarkLaunch(unittest.TestCase): + """ + Test that the behavior of the dark launched code behaves as expected. + """ + @patch('xmodule.capa.safe_exec.safe_exec.get_remote_exec') + @patch('xmodule.capa.safe_exec.safe_exec.codejail_safe_exec') + def test_default_code_execution(self, local_exec, remote_exec): + + # Test default only runs local exec. + g = {} + safe_exec('a=1', g) + assert local_exec.called + assert not remote_exec.called + + @override_settings(ENABLE_CODEJAIL_REST_SERVICE=True) + @patch('xmodule.capa.safe_exec.safe_exec.get_remote_exec') + @patch('xmodule.capa.safe_exec.safe_exec.codejail_safe_exec') + def test_code_execution_only_codejail_service(self, local_exec, remote_exec): + # Set return values to empty values to indicate no error. + remote_exec.return_value = (None, None) + # Test with only the service enabled. + g = {} + safe_exec('a=1', g) + assert not local_exec.called + assert remote_exec.called + + @override_settings(ENABLE_CODEJAIL_DARKLAUNCH=True) + @patch('xmodule.capa.safe_exec.safe_exec.get_remote_exec') + @patch('xmodule.capa.safe_exec.safe_exec.codejail_safe_exec') + def test_code_execution_darklaunch(self, local_exec, remote_exec): + # Set return values to empty values to indicate no error. + remote_exec.return_value = (None, None) + g = {} + + # Verify that incorrect config runs only remote and not both. + with override_settings(ENABLE_CODEJAIL_REST_SERVICE=True): + safe_exec('a=1', g) + assert not local_exec.called + assert remote_exec.called + + local_exec.reset_mock() + remote_exec.reset_mock() + + # Set up side effects to mimic the real behavior of modifying the globals_dict. + def local_side_effect(*args, **kwargs): + test_globals = args[1] + test_globals['test'] = 'local_test' + + def remote_side_effect(*args, **kwargs): + test_globals = args[0]['globals_dict'] + test_globals['test'] = 'remote_test' + + local_exec.side_effect = local_side_effect + remote_exec.side_effect = remote_side_effect + + assert is_codejail_in_darklaunch() + safe_exec('a=1', g) + + assert local_exec.called + assert remote_exec.called + # Verify that the local/default behavior currently wins out. + assert g['test'] == 'local_test' + + class TestLimitConfiguration(unittest.TestCase): """ Test that resource limits can be configured and overriden via Django settings.