This is an automated email from the ASF dual-hosted git repository. kaxilnaik pushed a commit to branch v1-10-test in repository https://gitbox.apache.org/repos/asf/airflow.git
commit 4086736828ca7a26dc29459002bcaa301a08670c Author: James Timmins <[email protected]> AuthorDate: Mon Nov 16 11:38:28 2020 -0800 Handle outdated webserver session timeout gracefully. (#12332) (cherry picked from commit d4e1ff290f59f365163b0d969840f72746364e8e) --- airflow/settings.py | 33 +++++++++++++++++++++++- airflow/www/app.py | 4 +++ airflow/www_rbac/app.py | 19 +------------- tests/test_local_settings/test_local_settings.py | 31 ++++++++++++++++++++++ tests/www/test_app.py | 33 ++++++++++++++++++++++++ tests/www_rbac/test_app.py | 20 +------------- 6 files changed, 102 insertions(+), 38 deletions(-) diff --git a/airflow/settings.py b/airflow/settings.py index 0f35dc5..f4cca1a 100644 --- a/airflow/settings.py +++ b/airflow/settings.py @@ -26,10 +26,11 @@ import atexit import json import logging import os -import pendulum import sys +import warnings from typing import Any +import pendulum from sqlalchemy import create_engine, exc from sqlalchemy.orm import scoped_session, sessionmaker from sqlalchemy.pool import NullPool @@ -380,6 +381,36 @@ def prepare_syspath(): sys.path.append(PLUGINS_FOLDER) +def get_session_lifetime_config(): + """Gets session timeout configs and handles outdated configs gracefully.""" + session_lifetime_minutes = conf.get('webserver', 'session_lifetime_minutes', fallback=None) + session_lifetime_days = conf.get('webserver', 'session_lifetime_days', fallback=None) + uses_deprecated_lifetime_configs = session_lifetime_days or conf.get( + 'webserver', 'force_logout_after', fallback=None + ) + + minutes_per_day = 24 * 60 + default_lifetime_minutes = '43200' + if uses_deprecated_lifetime_configs and session_lifetime_minutes == default_lifetime_minutes: + warnings.warn( + '`session_lifetime_days` option from `[webserver]` section has been ' + 'renamed to `session_lifetime_minutes`. The new option allows to configure ' + 'session lifetime in minutes. The `force_logout_after` option has been removed ' + 'from `[webserver]` section. Please update your configuration.', + category=DeprecationWarning, + ) + if session_lifetime_days: + session_lifetime_minutes = minutes_per_day * int(session_lifetime_days) + + if not session_lifetime_minutes: + session_lifetime_days = 30 + session_lifetime_minutes = minutes_per_day * session_lifetime_days + + logging.info('User session lifetime is set to %s minutes.', session_lifetime_minutes) + + return int(session_lifetime_minutes) + + def import_local_settings(): try: import airflow_local_settings diff --git a/airflow/www/app.py b/airflow/www/app.py index 7d0dae7..58e82b9 100644 --- a/airflow/www/app.py +++ b/airflow/www/app.py @@ -60,6 +60,7 @@ def create_app(config=None, testing=False): x_prefix=conf.getint("webserver", "PROXY_FIX_X_PREFIX", fallback=1) ) app.secret_key = conf.get('webserver', 'SECRET_KEY') + app.config['PERMANENT_SESSION_LIFETIME'] = datetime.timedelta(minutes=settings.get_session_lifetime_config()) app.config['LOGIN_DISABLED'] = not conf.getboolean( 'webserver', 'AUTHENTICATE') @@ -70,6 +71,9 @@ def create_app(config=None, testing=False): if config: app.config.from_mapping(config) + if 'SQLALCHEMY_ENGINE_OPTIONS' not in app.config: + app.config['SQLALCHEMY_ENGINE_OPTIONS'] = settings.prepare_engine_args() + csrf.init_app(app) app.config['TESTING'] = testing diff --git a/airflow/www_rbac/app.py b/airflow/www_rbac/app.py index 0b1a8c7..a2ebf7b 100644 --- a/airflow/www_rbac/app.py +++ b/airflow/www_rbac/app.py @@ -18,7 +18,6 @@ # under the License. # import logging -import sys import socket from datetime import timedelta from typing import Any @@ -62,23 +61,7 @@ def create_app(config=None, session=None, testing=False, app_name="Airflow"): x_prefix=conf.getint("webserver", "PROXY_FIX_X_PREFIX", fallback=1) ) app.secret_key = conf.get('webserver', 'SECRET_KEY') - - if conf.has_option('webserver', 'SESSION_LIFETIME_DAYS') or conf.has_option( - 'webserver', 'FORCE_LOG_OUT_AFTER' - ): - logging.error( - '`SESSION_LIFETIME_DAYS` option from `webserver` section has been ' - 'renamed to `SESSION_LIFETIME_MINUTES`. New option allows to configure ' - 'session lifetime in minutes. FORCE_LOG_OUT_AFTER option has been removed ' - 'from `webserver` section. Please update your configuration.' - ) - # Stop gunicorn server https://github.com/benoitc/gunicorn/blob/20.0.4/gunicorn/arbiter.py#L526 - # sys.exit(4) - else: - session_lifetime_minutes = conf.getint('webserver', 'SESSION_LIFETIME_MINUTES', fallback=43200) - logging.info('User session lifetime is set to %s minutes.', session_lifetime_minutes) - - app.config['PERMANENT_SESSION_LIFETIME'] = timedelta(minutes=session_lifetime_minutes) + app.config['PERMANENT_SESSION_LIFETIME'] = timedelta(minutes=settings.get_session_lifetime_config()) app.config.from_pyfile(settings.WEBSERVER_CONFIG, silent=True) app.config['SQLALCHEMY_TRACK_MODIFICATIONS'] = False diff --git a/tests/test_local_settings/test_local_settings.py b/tests/test_local_settings/test_local_settings.py index e69bd05..56fb93e 100644 --- a/tests/test_local_settings/test_local_settings.py +++ b/tests/test_local_settings/test_local_settings.py @@ -25,6 +25,7 @@ from airflow.kubernetes import pod_generator from kubernetes.client import ApiClient import kubernetes.client.models as k8s from tests.compat import MagicMock, Mock, mock, call, patch +from tests.test_utils.config import conf_vars api_client = ApiClient() @@ -442,6 +443,36 @@ class LocalSettingsTest(unittest.TestCase): ) +class TestUpdatedConfigNames(unittest.TestCase): + @conf_vars( + {("webserver", "session_lifetime_days"): '5', ("webserver", "session_lifetime_minutes"): '43200'} + ) + def test_updates_deprecated_session_timeout_config_val_when_new_config_val_is_default(self): + from airflow import settings + + with self.assertWarns(DeprecationWarning): + session_lifetime_config = settings.get_session_lifetime_config() + minutes_in_five_days = 5 * 24 * 60 + self.assertEqual(session_lifetime_config, minutes_in_five_days) + + @conf_vars( + {("webserver", "session_lifetime_days"): '5', ("webserver", "session_lifetime_minutes"): '43201'} + ) + def test_uses_updated_session_timeout_config_when_val_is_not_default(self): + from airflow import settings + + session_lifetime_config = settings.get_session_lifetime_config() + self.assertEqual(session_lifetime_config, 43201) + + @conf_vars({("webserver", "session_lifetime_days"): ''}) + def test_uses_updated_session_timeout_config_by_default(self): + from airflow import settings + + session_lifetime_config = settings.get_session_lifetime_config() + default_timeout_minutes = 30 * 24 * 60 + self.assertEqual(session_lifetime_config, default_timeout_minutes) + + class TestStatsWithAllowList(unittest.TestCase): def setUp(self): diff --git a/tests/www/test_app.py b/tests/www/test_app.py index 56ec213..83df598 100644 --- a/tests/www/test_app.py +++ b/tests/www/test_app.py @@ -17,10 +17,14 @@ # specific language governing permissions and limitations # under the License. import unittest +from datetime import timedelta +import pytest from werkzeug.middleware.proxy_fix import ProxyFix +from airflow.settings import Session from airflow.www_rbac import app as application +from tests.compat import mock from tests.test_utils.config import conf_vars @@ -54,3 +58,32 @@ class TestApp(unittest.TestCase): self.assertEqual(app.wsgi_app.x_host, 5) self.assertEqual(app.wsgi_app.x_port, 6) self.assertEqual(app.wsgi_app.x_prefix, 7) + + @conf_vars({ + ('core', 'sql_alchemy_pool_enabled'): 'True', + ('core', 'sql_alchemy_pool_size'): '3', + ('core', 'sql_alchemy_max_overflow'): '5', + ('core', 'sql_alchemy_pool_recycle'): '120', + ('core', 'sql_alchemy_pool_pre_ping'): 'True', + }) + @mock.patch("airflow.www.app.app", None) + @pytest.mark.backend("mysql", "postgres") + def test_should_set_sqlalchemy_engine_options(self): + app, _ = application.create_app(session=Session, testing=True) + engine_params = { + 'pool_size': 3, + 'pool_recycle': 120, + 'pool_pre_ping': True, + 'max_overflow': 5 + } + self.assertEqual(app.config['SQLALCHEMY_ENGINE_OPTIONS'], engine_params) + + @conf_vars( + { + ('webserver', 'session_lifetime_minutes'): '3600', + } + ) + @mock.patch("airflow.www.app.app", None) + def test_should_set_permanent_session_timeout(self): + app, _ = application.create_app(session=Session, testing=True) + self.assertEqual(app.config['PERMANENT_SESSION_LIFETIME'], timedelta(minutes=3600)) diff --git a/tests/www_rbac/test_app.py b/tests/www_rbac/test_app.py index a0607cb..a1ac2d3 100644 --- a/tests/www_rbac/test_app.py +++ b/tests/www_rbac/test_app.py @@ -16,15 +16,12 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. -import json import unittest from datetime import timedelta import pytest -import six from werkzeug.middleware.proxy_fix import ProxyFix -from airflow.configuration import conf from airflow.settings import Session from airflow.www_rbac import app as application from tests.compat import mock @@ -79,8 +76,6 @@ class TestApp(unittest.TestCase): 'pool_pre_ping': True, 'max_overflow': 5 } - if six.PY2: - engine_params = json.dumps(engine_params) self.assertEqual(app.config['SQLALCHEMY_ENGINE_OPTIONS'], engine_params) @conf_vars( @@ -90,18 +85,5 @@ class TestApp(unittest.TestCase): ) @mock.patch("airflow.www_rbac.app.app", None) def test_should_set_permanent_session_timeout(self): - app, _ = application.cached_appbuilder(testing=True) + app, _ = application.create_app(session=Session, testing=True) self.assertEqual(app.config['PERMANENT_SESSION_LIFETIME'], timedelta(minutes=3600)) - - @conf_vars( - { - ('webserver', 'session_lifetime_days'): '30', - ('webserver', 'force_log_out_after'): '30', - } - ) - @mock.patch("airflow.www_rbac.app.app", None) - def test_should_stop_app_when_removed_options_are_provided(self): - with self.assertRaises(SystemExit) as e: - conf.remove_option('webserver', 'session_lifetime_minutes') - application.cached_appbuilder(testing=True) - self.assertEqual(e.exception.code, 4)
