Handle outdated webserver session timeout gracefully. (#12332)

(cherry picked from commit d4e1ff2)

airflow/settings.py

@@ -26,10 +26,11 @@
 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

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

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

tests/test_local_settings/test_local_settings.py

@@ -20,11 +20,19 @@
 import os
 import sys
 import tempfile
-import unittest
+import six
+
 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
+
+if six.PY2:
+    # Need `assertWarns` back-ported from unittest2
+    import unittest2 as unittest
+else:
+    import unittest
 
 api_client = ApiClient()
 
@@ -442,6 +450,36 @@ def test_pod_mutation_v1_pod(self):
             )
 
 
+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):

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 @@ def test_constructor_proxyfix_args(self):
         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))

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 @@ def test_should_set_sqlalchemy_engine_options(self):
             '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 @@ def test_should_set_sqlalchemy_engine_options(self):
     )
     @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)