pierrejeambrun commented on code in PR #71429:
URL: https://github.com/apache/airflow/pull/71429#discussion_r3767286879


##########
airflow-core/docs/howto/run-behind-proxy.rst:
##########
@@ -62,6 +62,7 @@ To do so, you need to set the following setting in your 
``airflow.cfg``::
   set the ``FORWARDED_ALLOW_IPS`` environment variable so Uvicorn knows who to 
trust this header from. See
   `Uvicorn's docs 
<https://www.uvicorn.org/deployment/#proxies-and-forwarded-headers>`_. For the 
full options you can pass here.
   (Please note the ``--forwarded-allow-ips`` CLI option does not exist in 
Airflow.)
+  This applies to both ``server_type = uvicorn`` and ``server_type = 
gunicorn``.

Review Comment:
   The entire section above is mentioning uvicorn a lot. I would update it to 
mention `application server`. As well as a phrase explaining the two 
application servers that Airflow supports, and I would remove this line.



##########
airflow-core/src/airflow/api_fastapi/gunicorn_app.py:
##########
@@ -286,7 +286,9 @@ def create_gunicorn_app(
         if ssl_cert_reqs is not None:
             options["cert_reqs"] = ssl_cert_reqs
 
-    if proxy_headers:
-        options["forwarded_allow_ips"] = "*"
+    if not proxy_headers:
+        # ``UvicornWorker`` leaves uvicorn's ``proxy_headers`` at its default 
of True, so
+        # trusting nobody is the only way to keep the worker off X-Forwarded-*.
+        options["forwarded_allow_ips"] = ""

Review Comment:
   I believe this can disrupt some existing installs:
   A gunicorn deployment with a TLS-terminating proxy on a different host that 
relied on the implicit "*" will now stop trusting X-Forwarded-Proto unless it 
sets FORWARDED_ALLOW_IPS
   
   Maybe we need:
   ```
     if proxy_headers:
         if "FORWARDED_ALLOW_IPS" in os.environ:
             # Operator explicitly restricted trusted proxies — previously 
ignored.
             # Leave unset so the worker reads FORWARDED_ALLOW_IPS (like 
uvicorn).
             pass
         else:
             warnings.warn(
                 "Under gunicorn the API server trusts X-Forwarded-* from all 
clients by "
                 "default. Set FORWARDED_ALLOW_IPS to restrict trusted proxies; 
this default "
                 "will become restrictive in Airflow 4.",
                 category=RemovedInAirflow4Warning,
                 stacklevel=2,
             )
             options["forwarded_allow_ips"] = "*"
   ```



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to