[GitHub] [airflow] ashb commented on a change in pull request #21472: WIP backend -> backends

[GitBox <gi...@apache.org>]

ashb commented on a change in pull request #21472:
URL: https://github.com/apache/airflow/pull/21472#discussion_r802943243



##########
File path: airflow/api/client/__init__.py
##########
@@ -27,14 +27,14 @@
 def get_current_api_client() -> Client:
     """Return current API Client based on current Airflow configuration"""
     api_module = import_module(conf.get('cli', 'api_client'))  # type: Any
-    auth_backend = api.load_auth()
+    auth_backends = api.load_auth()
     session = None
-    session_factory = getattr(auth_backend, 'create_client_session', None)
+    session_factory = getattr(auth_backends, 'create_client_session', None)

Review comment:
       This would need to loop over the list and check for these attributes in some form or other.

##########
File path: UPDATING.md
##########
@@ -130,6 +130,13 @@ Previously, a task’s log is dynamically rendered from the `[core] log_filename
 
 A new `log_template` table is introduced to solve this problem. This table is synchronised with the aforementioned config values every time Airflow starts, and a new field `log_template_id` is added to every DAG run to point to the format used by tasks (`NULL` indicates the first ever entry for compatibility).
 
+### `auth_backends` replaces `auth_backend` configuration setting
+
+Previously, only one backend was used to authorize use of the experimental REST API.

Review comment:
       ```suggestion
   Previously, only one backend was used to authorize use of the  REST API.
   ```
   
   As (confusingly) the auth_backend is used by both new and old APIs.
   
   
   Hmmm I wonder if this would un-intentionally make the old API available again? The Default auth backend of deny_all effectively made the old API not usable I think)

##########
File path: airflow/api_connexion/openapi/v1.yaml
##########
@@ -179,9 +179,9 @@ info:
     and it is even possible to add your own method.
 
     If you want to check which auth backend is currently set, you can use
-    `airflow config get-value api auth_backend` command as in the example below.
+    `airflow config get-value api auth_backends` command as in the example below.
     ```bash
-    $ airflow config get-value api auth_backend
+    $ airflow config get-value api auth_backends
     airflow.api.auth.backend.basic_auth
     ```
     The default is to deny all requests.

Review comment:
       Has this PR changed the default?

##########
File path: docs/apache-airflow/security/api.rst
##########
@@ -27,19 +27,24 @@ deny all requests:
 .. code-block:: ini
 
     [api]
-    auth_backend = airflow.api.auth.backend.deny_all
+    auth_backends = airflow.api.auth.backend.deny_all
 
 .. versionchanged:: 1.10.11
 
     In Airflow <1.10.11, the default setting was to allow all API requests without authentication, but this
     posed security risks for if the Webserver is publicly accessible.
 
-If you want to check which authentication backend is currently set, you can use ``airflow config get-value api auth_backend``
+.. versionchanged:: 2.?

Review comment:
       ```suggestion
   .. versionchanged:: 2.3
   ```

##########
File path: airflow/config_templates/default_test.cfg
##########
@@ -61,7 +61,7 @@ api_client = airflow.api.client.local_client
 endpoint_url = http://localhost:8080
 
 [api]

Review comment:
       ```suggestion
   ```

##########
File path: airflow/config_templates/default_test.cfg
##########
@@ -61,7 +61,7 @@ api_client = airflow.api.client.local_client
 endpoint_url = http://localhost:8080
 
 [api]
-auth_backend = airflow.api.auth.backend.default
+auth_backends = airflow.api.auth.backend.default

Review comment:
       Having this in here is un-necessary (don't ask me why we did it before), so let's remove this and the `[api]` section from here.
   ```suggestion
   ```




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org