sjyangkevin commented on PR #54043:
URL: https://github.com/apache/airflow/pull/54043#issuecomment-3148919500

   In the [provider 
check](https://github.com/apache/airflow/actions/runs/16700371139/job/47270493334?pr=54043),
 I keep seeing the following error. It looks like there is a compatibility 
issue with 3.0.3 after adding the `HITL_DETAIL` to `DagAccessEntity`. I 
attempted to use the approach here 
https://github.com/apache/airflow/pull/54043#discussion_r2249218988, but it 
doesn't look like the correct way to resolve this compatibility issue.
   
   ```
   __________________________________________________________ ERROR collecting 
providers/fab/tests/unit/fab/auth_manager/test_fab_auth_manager.py 
___________________________________________________________
   providers/fab/tests/unit/fab/auth_manager/test_fab_auth_manager.py:48: in 
<module>
       from airflow.providers.fab.auth_manager.fab_auth_manager import 
FabAuthManager
   
/usr/local/lib/python3.10/site-packages/airflow/providers/fab/auth_manager/fab_auth_manager.py:134:
 in <module>
       DagAccessEntity.HITL_DETAIL: (RESOURCE_HITL_DETAIL,),
   /usr/local/lib/python3.10/enum.py:437: in __getattr__
       raise AttributeError(name) from None
   E   AttributeError: HITL_DETAIL
   ```
   
   I would like to provide a summary regarding my approach of adding a new 
permission, and I would appreciate if I could get some guidance on how to 
properly make it available for only after 3.1.0.
   
   1. HITL is an operator, and it provides an interface on which users can 
action. This interface shows the details about the operator, and can be 
considered as a sub-entity of the DAG (similar to DAG Runs, Code, XCom, etc.). 
Therefore, the new permission can be considered as an access to the DAG's 
sub-entity, `HITL_DETAIL` in this case, and can be added to the 
`DagAccessEntity`.
   2. Then, the HITL endpoints can be configured with, e.g., 
`dependencies=[Depends(requires_access_dag(method="PUT", 
access_entity=DagAccessEntity.HITL_DETAIL))]`, to check if the user had the 
access to `HITL_DETAIL` sub-entity.
   3. The `DagAccessEntity.HITL_DETAIL` is mapped to a resource type 
`RESOURCE_HITL_DETAIL`. `RESOURCE_HITL_DETAIL` need to be defined in 
`providers/fab/src/airflow/providers/fab/www/security/permissions.py` and be 
configured in 
`providers/fab/src/airflow/providers/fab/auth_manager/security_manager/override.py`
 (with actions "can read", "can edit"). After the configuration, the resource 
type will show up in `ab_view_menu`.
   4. Then, admin user can configure the access to HITL endpoints by assigning 
permission such as "can read HITL_DETAIL" to a role and then assigning the role 
to user.
   
   I think the proper approach is to have a check for Airflow version and then 
define/register this new permission only the version is 3.1.0. However, it 
looks like we need to introduce a lot of version check across core, providers, 
and tests.. Not sure if there is a better way to implement it. Thanks!


-- 
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