From 97cd99be776ff26baca02eb0f47e97a923165c94 Mon Sep 17 00:00:00 2001 From: Vincent <97131062+vincbeck@users.noreply.github.com> Date: Thu, 14 Dec 2023 13:49:40 -0500 Subject: [PATCH 1/6] Move config related to FAB auth manager to FAB provider --- airflow/config_templates/config.yml | 6 +++++ .../fab/auth_manager/fab_auth_manager.py | 4 ++- airflow/providers/fab/provider.yaml | 27 +++++++++++++++++++ airflow/www/extensions/init_appbuilder.py | 19 ++++++++----- .../configurations-ref.rst | 18 +++++++++++++ docs/apache-airflow-providers-fab/index.rst | 1 + tests/conftest.py | 2 +- tests/www/views/conftest.py | 2 +- tests/www/views/test_views_log.py | 2 +- tests/www/views/test_views_rate_limit.py | 4 +-- 10 files changed, 72 insertions(+), 13 deletions(-) create mode 100644 docs/apache-airflow-providers-fab/configurations-ref.rst diff --git a/airflow/config_templates/config.yml b/airflow/config_templates/config.yml index 0930fada767141..125dff6f4a03d9 100644 --- a/airflow/config_templates/config.yml +++ b/airflow/config_templates/config.yml @@ -1741,6 +1741,8 @@ webserver: type: string example: ~ default: "True" + version_deprecated: 2.9.0 + deprecation_reason: This config has been moved to fab provider. Please use the config from fab provider. session_lifetime_minutes: description: | The UI cookie lifetime in minutes. User will be logged out from UI after @@ -1817,6 +1819,8 @@ webserver: type: boolean example: ~ default: "True" + version_deprecated: 2.9.0 + deprecation_reason: This config has been moved to fab provider. Please use the config from fab provider. auth_rate_limit: description: | Rate limit for authentication endpoints. @@ -1824,6 +1828,8 @@ webserver: type: string example: ~ default: "5 per 40 second" + version_deprecated: 2.9.0 + deprecation_reason: This config has been moved to fab provider. Please use the config from fab provider. caching_hash_method: description: | The caching algorithm used by the webserver. Must be a valid hashlib function name. diff --git a/airflow/providers/fab/auth_manager/fab_auth_manager.py b/airflow/providers/fab/auth_manager/fab_auth_manager.py index 201dc050f9ad4f..599d58b325985e 100644 --- a/airflow/providers/fab/auth_manager/fab_auth_manager.py +++ b/airflow/providers/fab/auth_manager/fab_auth_manager.py @@ -508,5 +508,7 @@ def _sync_appbuilder_roles(self): # Otherwise, when the name of a view or menu is changed, the framework # will add the new Views and Menus names to the backend, but will not # delete the old ones. - if conf.getboolean("webserver", "UPDATE_FAB_PERMS"): + if conf.getboolean( + "fab", "UPDATE_FAB_PERMS", fallback=conf.getboolean("webserver", "UPDATE_FAB_PERMS") + ): self.security_manager.sync_roles() diff --git a/airflow/providers/fab/provider.yaml b/airflow/providers/fab/provider.yaml index 6687110e6a27a9..8b4046cc65c6b1 100644 --- a/airflow/providers/fab/provider.yaml +++ b/airflow/providers/fab/provider.yaml @@ -36,3 +36,30 @@ dependencies: - flask-appbuilder==4.3.10 - flask-login>=0.6.2 - google-re2>=1.0 + +config: + fab: + description: This section contains configs specific to FAB provider. + options: + auth_rate_limited: + description: | + Boolean for enabling rate limiting on authentication endpoints. + version_added: 1.0.0 + type: boolean + example: ~ + default: "True" + auth_rate_limit: + description: | + Rate limit for authentication endpoints. + version_added: 2.6.0 + type: string + example: ~ + default: "5 per 40 second" + update_fab_perms: + description: | + Update FAB permissions and sync security manager roles + on webserver startup + version_added: 1.10.7 + type: string + example: ~ + default: "True" diff --git a/airflow/www/extensions/init_appbuilder.py b/airflow/www/extensions/init_appbuilder.py index ddb44200f93b29..4514f8afa88516 100644 --- a/airflow/www/extensions/init_appbuilder.py +++ b/airflow/www/extensions/init_appbuilder.py @@ -131,9 +131,19 @@ def __init__( base_template="airflow/main.html", static_folder="static/appbuilder", static_url_path="/appbuilder", - update_perms=conf.getboolean("webserver", "UPDATE_FAB_PERMS"), - auth_rate_limited=conf.getboolean("webserver", "AUTH_RATE_LIMITED", fallback=True), - auth_rate_limit=conf.get("webserver", "AUTH_RATE_LIMIT", fallback="5 per 40 second"), + update_perms=conf.getboolean( + "fab", "UPDATE_FAB_PERMS", fallback=conf.getboolean("webserver", "UPDATE_FAB_PERMS") + ), + auth_rate_limited=conf.getboolean( + "fab", + "AUTH_RATE_LIMITED", + fallback=conf.getboolean("webserver", "AUTH_RATE_LIMITED", fallback=True), + ), + auth_rate_limit=conf.get( + "fab", + "AUTH_RATE_LIMIT", + fallback=conf.get("webserver", "AUTH_RATE_LIMIT", fallback="5 per 40 second"), + ), ): """ App-builder constructor. @@ -659,7 +669,4 @@ def init_appbuilder(app: Flask) -> AirflowAppBuilder: app=app, session=settings.Session, base_template="airflow/main.html", - update_perms=conf.getboolean("webserver", "UPDATE_FAB_PERMS"), - auth_rate_limited=conf.getboolean("webserver", "AUTH_RATE_LIMITED", fallback=True), - auth_rate_limit=conf.get("webserver", "AUTH_RATE_LIMIT", fallback="5 per 40 second"), ) diff --git a/docs/apache-airflow-providers-fab/configurations-ref.rst b/docs/apache-airflow-providers-fab/configurations-ref.rst new file mode 100644 index 00000000000000..5885c9d91b6e8d --- /dev/null +++ b/docs/apache-airflow-providers-fab/configurations-ref.rst @@ -0,0 +1,18 @@ + .. Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + .. http://www.apache.org/licenses/LICENSE-2.0 + + .. Unless required by applicable law or agreed to in writing, + software distributed under the License is distributed on an + "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + KIND, either express or implied. See the License for the + specific language governing permissions and limitations + under the License. + +.. include:: ../exts/includes/providers-configurations-ref.rst diff --git a/docs/apache-airflow-providers-fab/index.rst b/docs/apache-airflow-providers-fab/index.rst index 348a65a97abbbd..df1eb33453c2cd 100644 --- a/docs/apache-airflow-providers-fab/index.rst +++ b/docs/apache-airflow-providers-fab/index.rst @@ -34,6 +34,7 @@ :maxdepth: 1 :caption: Guides + Configuration Auth manager .. toctree:: diff --git a/tests/conftest.py b/tests/conftest.py index b48c51a0cbb8fa..780f0c471c91c9 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -602,7 +602,7 @@ def fake_sleep(seconds): def app(): from tests.test_utils.config import conf_vars - with conf_vars({("webserver", "auth_rate_limited"): "False"}): + with conf_vars({("fab", "auth_rate_limited"): "False"}): from airflow.www import app yield app.create_app(testing=True) diff --git a/tests/www/views/conftest.py b/tests/www/views/conftest.py index aa531a8267a51a..14822b5eaf5eb4 100644 --- a/tests/www/views/conftest.py +++ b/tests/www/views/conftest.py @@ -63,7 +63,7 @@ def app(examples_dag_bag): ] ) def factory(): - with conf_vars({("webserver", "auth_rate_limited"): "False"}): + with conf_vars({("fab", "auth_rate_limited"): "False"}): return create_app(testing=True) app = factory() diff --git a/tests/www/views/test_views_log.py b/tests/www/views/test_views_log.py index 16cfa2b99756c7..8d816d76956053 100644 --- a/tests/www/views/test_views_log.py +++ b/tests/www/views/test_views_log.py @@ -79,7 +79,7 @@ def log_app(backup_modules, log_path): @conf_vars( { ("logging", "logging_config_class"): "airflow_local_settings.LOGGING_CONFIG", - ("webserver", "auth_rate_limited"): "False", + ("fab", "auth_rate_limited"): "False", } ) def factory(): diff --git a/tests/www/views/test_views_rate_limit.py b/tests/www/views/test_views_rate_limit.py index ddd28259d7c4c2..540a0c9f9a1898 100644 --- a/tests/www/views/test_views_rate_limit.py +++ b/tests/www/views/test_views_rate_limit.py @@ -43,9 +43,7 @@ def app_with_rate_limit_one(examples_dag_bag): ] ) def factory(): - with conf_vars( - {("webserver", "auth_rate_limited"): "True", ("webserver", "auth_rate_limit"): "1 per 20 second"} - ): + with conf_vars({("fab", "auth_rate_limited"): "True", ("fab", "auth_rate_limit"): "1 per 20 second"}): return create_app(testing=True) app = factory() From a9b5656e27324b4d7fd7e11bae8fccc50b81bade Mon Sep 17 00:00:00 2001 From: Vincent <97131062+vincbeck@users.noreply.github.com> Date: Tue, 19 Dec 2023 11:55:07 -0500 Subject: [PATCH 2/6] Fix tests --- airflow/www/extensions/init_appbuilder.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/airflow/www/extensions/init_appbuilder.py b/airflow/www/extensions/init_appbuilder.py index bee9fd14d33d7c..991634c0f6107d 100644 --- a/airflow/www/extensions/init_appbuilder.py +++ b/airflow/www/extensions/init_appbuilder.py @@ -664,4 +664,17 @@ def init_appbuilder(app: Flask) -> AirflowAppBuilder: app=app, session=settings.Session, base_template="airflow/main.html", + update_perms=conf.getboolean( + "fab", "UPDATE_FAB_PERMS", fallback=conf.getboolean("webserver", "UPDATE_FAB_PERMS") + ), + auth_rate_limited=conf.getboolean( + "fab", + "AUTH_RATE_LIMITED", + fallback=conf.getboolean("webserver", "AUTH_RATE_LIMITED", fallback=True), + ), + auth_rate_limit=conf.get( + "fab", + "AUTH_RATE_LIMIT", + fallback=conf.get("webserver", "AUTH_RATE_LIMIT", fallback="5 per 40 second"), + ), ) From 6e059dc12e4943cab34c43330f8c335cc2abffa4 Mon Sep 17 00:00:00 2001 From: Vincent <97131062+vincbeck@users.noreply.github.com> Date: Tue, 19 Dec 2023 15:42:19 -0500 Subject: [PATCH 3/6] Tentative to fix doc build --- docs/build_docs.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/build_docs.py b/docs/build_docs.py index aaba029b60a063..0ea72f29a8a63a 100755 --- a/docs/build_docs.py +++ b/docs/build_docs.py @@ -495,7 +495,7 @@ def main(): jobs, package_build_errors, package_spelling_errors, - spellcheck_only, + False, ) # And try again in case one change spans across three-level dependencies @@ -507,7 +507,7 @@ def main(): jobs, package_build_errors, package_spelling_errors, - spellcheck_only, + False, ) if not disable_checks: From c0dabd99d0b4c14289862d88eeef8c912144fcc4 Mon Sep 17 00:00:00 2001 From: Vincent <97131062+vincbeck@users.noreply.github.com> Date: Tue, 19 Dec 2023 18:28:29 -0500 Subject: [PATCH 4/6] Revert "Tentative to fix doc build" This reverts commit 6e059dc12e4943cab34c43330f8c335cc2abffa4. --- docs/build_docs.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/build_docs.py b/docs/build_docs.py index 0ea72f29a8a63a..aaba029b60a063 100755 --- a/docs/build_docs.py +++ b/docs/build_docs.py @@ -495,7 +495,7 @@ def main(): jobs, package_build_errors, package_spelling_errors, - False, + spellcheck_only, ) # And try again in case one change spans across three-level dependencies @@ -507,7 +507,7 @@ def main(): jobs, package_build_errors, package_spelling_errors, - False, + spellcheck_only, ) if not disable_checks: From 2a46fb37e9f6c2ffec54e1f8c60877347f5f9e16 Mon Sep 17 00:00:00 2001 From: Vincent <97131062+vincbeck@users.noreply.github.com> Date: Tue, 19 Dec 2023 18:42:33 -0500 Subject: [PATCH 5/6] Update doc build to handle failure in spell checking --- docs/build_docs.py | 32 +++++++++++++++++++++++--------- 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/docs/build_docs.py b/docs/build_docs.py index aaba029b60a063..f3d3c50aed421c 100755 --- a/docs/build_docs.py +++ b/docs/build_docs.py @@ -473,8 +473,9 @@ def main(): # If only one inventory is missing, the remaining packages are correct. If we are missing # two or more inventories, it is better to try to build for all packages as the previous packages # may have failed as well. + packages_to_build = current_packages if len(priority_packages) > 1 else normal_packages package_build_errors, package_spelling_errors = build_docs_for_packages( - current_packages=current_packages if len(priority_packages) > 1 else normal_packages, + current_packages=packages_to_build, docs_only=docs_only, spellcheck_only=spellcheck_only, jobs=jobs, @@ -493,6 +494,7 @@ def main(): args, docs_only, jobs, + packages_to_build, package_build_errors, package_spelling_errors, spellcheck_only, @@ -505,6 +507,7 @@ def main(): args, docs_only, jobs, + packages_to_build, package_build_errors, package_spelling_errors, spellcheck_only, @@ -535,26 +538,37 @@ def retry_building_docs_if_needed( args, docs_only, jobs, + all_packages_to_build, package_build_errors, package_spelling_errors, spellcheck_only, ): - to_retry_packages = [ + build_to_retry_packages = [ package_name for package_name, errors in package_build_errors.items() if any(any((m in e.message) for m in ERRORS_ELIGIBLE_TO_REBUILD) for e in errors) ] + spelling_to_retry_packages = [ + package_name + for package_name, errors in package_spelling_errors.items() + if any(any((m in e.message) for m in ERRORS_ELIGIBLE_TO_REBUILD) for e in errors) + ] + to_retry_packages = build_to_retry_packages + spelling_to_retry_packages if to_retry_packages: - for package_name in to_retry_packages: - if package_name in all_build_errors: - del all_build_errors[package_name] - if package_name in all_spelling_errors: - del all_spelling_errors[package_name] + if spellcheck_only: + all_build_errors = {} + all_spelling_errors = {} + else: + for package_name in to_retry_packages: + if package_name in all_build_errors: + del all_build_errors[package_name] + if package_name in all_spelling_errors: + del all_spelling_errors[package_name] package_build_errors, package_spelling_errors = build_docs_for_packages( - current_packages=to_retry_packages, + current_packages=all_packages_to_build if spellcheck_only else to_retry_packages, docs_only=docs_only, - spellcheck_only=spellcheck_only, + spellcheck_only=False, jobs=jobs, verbose=args.verbose, ) From b939a06eae92e3f72882c1cfbd9ac6bb666e2e49 Mon Sep 17 00:00:00 2001 From: Vincent <97131062+vincbeck@users.noreply.github.com> Date: Tue, 2 Jan 2024 18:02:12 -0500 Subject: [PATCH 6/6] Increase spellcheck timeout --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a508c06f4fc449..d8a64e89368ab4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -671,7 +671,7 @@ jobs: run: aws s3 sync --delete ./files/documentation s3://apache-airflow-docs spellcheck-docs: - timeout-minutes: 60 + timeout-minutes: 130 name: "Spellcheck docs" runs-on: ${{fromJSON(needs.build-info.outputs.runs-on)}} needs: [build-info, wait-for-ci-images]