[
https://issues.apache.org/jira/browse/AIRFLOW-3397?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16717206#comment-16717206
]
ASF GitHub Bot commented on AIRFLOW-3397:
-----------------------------------------
BrechtDeVlieger opened a new pull request #4305: [AIRFLOW-3397] Fix integrety
error in rbac AirflowSecurityManager
URL: https://github.com/apache/incubator-airflow/pull/4305
This was caused by the variable `role` being shadowed in a loop statement.
Make sure you have checked _all_ steps below.
### Jira
- [x] My PR addresses the following [Airflow
Jira](https://issues.apache.org/jira/browse/AIRFLOW/) issues and references
them in the PR title. For example, "\[AIRFLOW-XXX\] My Airflow PR"
- https://issues.apache.org/jira/browse/AIRFLOW-XXX
- In case you are fixing a typo in the documentation you can prepend your
commit with \[AIRFLOW-XXX\], code changes always need a Jira issue.
### Description
- [x] Here are some details about my PR, including screenshots of any UI
changes:
This PR fixes a bug in rbac AirflowSecurityManager where a variable was
shadowed by a loop variable.
### Tests
- [ ] My PR does not need testing for this extremely good reason:
No significant changes were made, only a variable name was changed to fix a
shadowing bug.
### Commits
- [x] My commits all reference Jira issues in their subject lines, and I
have squashed multiple commits if they address the same issue. In addition, my
commits follow the guidelines from "[How to write a good git commit
message](http://chris.beams.io/posts/git-commit/)":
1. Subject is separated from body by a blank line
1. Subject is limited to 50 characters (not including Jira issue reference)
1. Subject does not end with a period
1. Subject uses the imperative mood ("add", not "adding")
1. Body wraps at 72 characters
1. Body explains "what" and "why", not "how"
### Documentation
- [x] In case of new functionality, my PR adds documentation that describes
how to use it.
- When adding new operators/hooks/sensors, the autoclass documentation
generation needs to be added.
- All the public functions and the classes in the PR contain docstrings
that explain what it does
### Code Quality
- [x] Passes `flake8`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Adding a new role to rbac makes airflow crash
> ---------------------------------------------
>
> Key: AIRFLOW-3397
> URL: https://issues.apache.org/jira/browse/AIRFLOW-3397
> Project: Apache Airflow
> Issue Type: Bug
> Components: security
> Affects Versions: 2.0.0
> Reporter: Brecht De Vlieger
> Priority: Major
>
> Here are the steps to reproduce:
> - Start airflow with rbac enabled
> - Create a new role
> - Wait until one of the gunicorn workers is restarted
> - Airflow crashes with the following exception:
> ```
> [2018-11-26 09:59:17,353] \{__init__.py:51} INFO - Using executor
> KubernetesExecutor
> [2018-11-26 09:59:17,617] \{models.py:286} INFO - Filling up the DagBag from
> /root/airflow/dags
> [2018-11-26 09:59:18,134] \{security.py:435} INFO - Start syncing user roles.
> [2018-11-26 09:59:18,270] \{security.py:194} INFO - Existing permissions for
> the role:Viewer within the database will persist.
> /usr/local/lib/python2.7/dist-packages/flask_appbuilder/fields.py:152:
> UserWarning: allow_blank=True does not do anything for
> QuerySelectMultipleField.
> warnings.warn('allow_blank=True does not do anything for
> QuerySelectMultipleField.')
> [2018-11-26 09:59:18,384] \{security.py:194} INFO - Existing permissions for
> the role:User within the database will persist.
> [2018-11-26 09:59:18,491] \{security.py:194} INFO - Existing permissions for
> the role:Op within the database will persist.
> [2018-11-26 09:59:18,491] \{security.py:344} INFO - Fetching a set of all
> permission, view_menu from FAB meta-table
> 172.17.0.1 - - [26/Nov/2018:09:59:18 +0000] "GET /roles/list/ HTTP/1.1" 200
> 48607 "http://localhost/home"; "Mozilla/5.0 (X11; Ubuntu; Linux x86_64;
> rv:63.0) Gecko/20100101 Firefox/63.0"
> [2018-11-26 09:59:18 +0000] [37] [ERROR] Exception in worker process
> Traceback (most recent call last):
> File "/usr/local/lib/python2.7/dist-packages/gunicorn/arbiter.py", line
> 583, in spawn_worker
> worker.init_process()
> File "/usr/local/lib/python2.7/dist-packages/gunicorn/workers/base.py",
> line 129, in init_process
> self.load_wsgi()
> File "/usr/local/lib/python2.7/dist-packages/gunicorn/workers/base.py",
> line 138, in load_wsgi
> self.wsgi = self.app.wsgi()
> File "/usr/local/lib/python2.7/dist-packages/gunicorn/app/base.py", line
> 67, in wsgi
> self.callable = self.load()
> File "/usr/local/lib/python2.7/dist-packages/gunicorn/app/wsgiapp.py", line
> 52, in load
> return self.load_wsgiapp()
> File "/usr/local/lib/python2.7/dist-packages/gunicorn/app/wsgiapp.py", line
> 41, in load_wsgiapp
> return util.import_app(self.app_uri)
> File "/usr/local/lib/python2.7/dist-packages/gunicorn/util.py", line 362,
> in import_app
> app = eval(obj, vars(mod))
> File "<string>", line 1, in <module>
> File "/usr/local/lib/python2.7/dist-packages/airflow/www_rbac/app.py", line
> 207, in cached_app
> app, _ = create_app(config, session, testing)
> File "/usr/local/lib/python2.7/dist-packages/airflow/www_rbac/app.py", line
> 167, in create_app
> security_manager.sync_roles()
> File "/usr/local/lib/python2.7/dist-packages/airflow/www_rbac/security.py",
> line 443, in sync_roles
> self.create_custom_dag_permission_view()
> File "/usr/local/lib/python2.7/dist-packages/airflow/www_rbac/security.py",
> line 406, in create_custom_dag_permission_view
> self.get_session.execute(ab_perm_view_role.insert(), update_perm_views)
> File "/usr/local/lib/python2.7/dist-packages/sqlalchemy/orm/scoping.py",
> line 157, in do
> return getattr(self.registry(), name)(*args, **kwargs)
> File "/usr/local/lib/python2.7/dist-packages/sqlalchemy/orm/session.py",
> line 1160, in execute
> bind, close_with_result=True).execute(clause, params or {})
> File "/usr/local/lib/python2.7/dist-packages/sqlalchemy/engine/base.py",
> line 945, in execute
> return meth(self, multiparams, params)
> File "/usr/local/lib/python2.7/dist-packages/sqlalchemy/sql/elements.py",
> line 263, in _execute_on_connection
> return connection._execute_clauseelement(self, multiparams, params)
> File "/usr/local/lib/python2.7/dist-packages/sqlalchemy/engine/base.py",
> line 1053, in _execute_clauseelement
> compiled_sql, distilled_params
> File "/usr/local/lib/python2.7/dist-packages/sqlalchemy/engine/base.py",
> line 1189, in _execute_context
> context)
> File "/usr/local/lib/python2.7/dist-packages/sqlalchemy/engine/base.py",
> line 1402, in _handle_dbapi_exception
> exc_info
> File "/usr/local/lib/python2.7/dist-packages/sqlalchemy/util/compat.py",
> line 203, in raise_from_cause
> reraise(type(exception), exception, tb=exc_tb, cause=cause)
> File "/usr/local/lib/python2.7/dist-packages/sqlalchemy/engine/base.py",
> line 1159, in _execute_context
> context)
> File "/usr/local/lib/python2.7/dist-packages/sqlalchemy/engine/default.py",
> line 467, in do_executemany
> cursor.executemany(statement, parameters)
> IntegrityError: (psycopg2.IntegrityError) insert or update on table
> "ab_permission_view_role" violates foreign key constraint
> "ab_permission_view_role_role_id_fkey"
> DETAIL: Key (role_id)=(347) is not present in table "ab_role".
> [SQL: "INSERT INTO ab_permission_view_role (id, permission_view_id, role_id)
> VALUES (nextval('ab_permission_view_role_id_seq'), %(permission_view_id)s,
> %(role_id)s)"] [parameters: (\{'permission_view_id': 36, 'role_id': 347},
> \{'permission_view_id': 37, 'role_id': 347}, \{'permission_view_id': 38,
> 'role_id': 347}, \{'permission_view_id': 39, 'role_id': 347},
> \{'permission_view_id': 40, 'role_id': 347}, \{'permission_view_id': 41,
> 'role_id': 347}, \{'permission_view_id': 42, 'role_id': 347},
> \{'permission_view_id': 43, 'role_id': 347} ... displaying 10 of 54 total
> bound parameter sets ... \{'permission_view_id': 122, 'role_id': 347},
> \{'permission_view_id': 123, 'role_id': 347})]
> [2018-11-26 09:59:18 +0000] [37] [INFO] Worker exiting (pid: 37)
> [2018-11-26 09:59:19 +0000] [33] [INFO] Worker exiting (pid: 33)
> [2018-11-26 09:59:19 +0000] [15] [INFO] Worker exiting (pid: 15)
> [2018-11-26 09:59:19 +0000] [16] [INFO] Worker exiting (pid: 16)
> [2018-11-26 09:59:19 +0000] [29] [INFO] Worker exiting (pid: 29)
> [2018-11-26 09:59:19 +0000] [8] [INFO] Shutting down: Master
> [2018-11-26 09:59:19 +0000] [8] [INFO] Reason: Worker failed to boot.
> [2018-11-26 09:59:19,963] \{cli.py:818} ERROR - [0 / 0] some workers seem to
> have died and gunicorndid not restart them as expected
> ```
> Solution:
> This is caused because of name shadowing in a for loop.
> The following diff solves the problem:
> ```
> diff --git a/airflow/www_rbac/security.py b/airflow/www_rbac/security.py
> index 8f9b6287..369ec71c 100644
> --- a/airflow/www_rbac/security.py
> +++ b/airflow/www_rbac/security.py
> @@ -395,8 +395,8 @@ class AirflowSecurityManager(SecurityManager):
> existing_perm_view_by_user =
> self.get_session.query(ab_perm_view_role)\
> .filter(ab_perm_view_role.columns.role_id == role.id)
>
> - existing_perms_views = set([role.permission_view_id
> - for role in
> existing_perm_view_by_user])
> + existing_perms_views = set([pv.permission_view_id
> + for pv in
> existing_perm_view_by_user])
> missing_perm_views = all_perm_views - existing_perms_views
>
> for perm_view_id in missing_perm_views:
> ```
> I have no time atm to create a PR, but I can do it later if required.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
