Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix FAB-related logging format interpolation #34139

Merged
merged 1 commit into from
Sep 6, 2023
Merged

Fix FAB-related logging format interpolation #34139

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 28 additions & 28 deletions airflow/auth/managers/fab/security_manager/modules/db.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ def create_db(self):
if self.count_users() == 0 and self.auth_role_public != self.auth_role_admin:
log.warning(const.LOGMSG_WAR_SEC_NO_USER)
except Exception as e:
log.error(const.LOGMSG_ERR_SEC_CREATE_DB.format(e))
log.error(const.LOGMSG_ERR_SEC_CREATE_DB, e)
exit(1)

"""
Expand All @@ -104,9 +104,9 @@ def update_role(self, role_id, name: str) -> Role | None:
role.name = name
self.get_session.merge(role)
self.get_session.commit()
log.info(const.LOGMSG_INF_SEC_UPD_ROLE.format(role))
log.info(const.LOGMSG_INF_SEC_UPD_ROLE, role)
except Exception as e:
log.error(const.LOGMSG_ERR_SEC_UPD_ROLE.format(e))
log.error(const.LOGMSG_ERR_SEC_UPD_ROLE, e)
self.get_session.rollback()
return None
return role
Expand All @@ -120,10 +120,10 @@ def add_role(self, name: str) -> Role:
role.name = name
self.get_session.add(role)
self.get_session.commit()
log.info(const.LOGMSG_INF_SEC_ADD_ROLE.format(name))
log.info(const.LOGMSG_INF_SEC_ADD_ROLE, name)
return role
except Exception as e:
log.error(const.LOGMSG_ERR_SEC_ADD_ROLE.format(e))
log.error(const.LOGMSG_ERR_SEC_ADD_ROLE, e)
self.get_session.rollback()
return role

Expand Down Expand Up @@ -187,10 +187,10 @@ def add_user(
user.password = generate_password_hash(password)
self.get_session.add(user)
self.get_session.commit()
log.info(const.LOGMSG_INF_SEC_ADD_USER.format(username))
log.info(const.LOGMSG_INF_SEC_ADD_USER, username)
return user
except Exception as e:
log.error(const.LOGMSG_ERR_SEC_ADD_USER.format(e))
log.error(const.LOGMSG_ERR_SEC_ADD_USER, e)
self.get_session.rollback()
return False

Expand Down Expand Up @@ -226,7 +226,7 @@ def add_register_user(self, username, first_name, last_name, email, password="",
self.get_session.commit()
return register_user
except Exception as e:
log.error(const.LOGMSG_ERR_SEC_ADD_REGISTER_USER.format(e))
log.error(const.LOGMSG_ERR_SEC_ADD_REGISTER_USER, e)
self.get_session.rollback()
return None

Expand Down Expand Up @@ -267,9 +267,9 @@ def update_user(self, user):
try:
self.get_session.merge(user)
self.get_session.commit()
log.info(const.LOGMSG_INF_SEC_UPD_USER.format(user))
log.info(const.LOGMSG_INF_SEC_UPD_USER, user)
except Exception as e:
log.error(const.LOGMSG_ERR_SEC_UPD_USER.format(e))
log.error(const.LOGMSG_ERR_SEC_UPD_USER, e)
self.get_session.rollback()
return False

Expand All @@ -284,7 +284,7 @@ def del_register_user(self, register_user):
self.get_session.commit()
return True
except Exception as e:
log.error(const.LOGMSG_ERR_SEC_DEL_REGISTER_USER.format(e))
log.error(const.LOGMSG_ERR_SEC_DEL_REGISTER_USER, e)
self.get_session.rollback()
return False

Expand Down Expand Up @@ -322,7 +322,7 @@ def create_action(self, name):
self.get_session.commit()
return action
except Exception as e:
log.error(const.LOGMSG_ERR_SEC_ADD_PERMISSION.format(e))
log.error(const.LOGMSG_ERR_SEC_ADD_PERMISSION, e)
self.get_session.rollback()
return action

Expand All @@ -334,7 +334,7 @@ def delete_action(self, name: str) -> bool:
"""
action = self.get_action(name)
if not action:
log.warning(const.LOGMSG_WAR_SEC_DEL_PERMISSION.format(name))
log.warning(const.LOGMSG_WAR_SEC_DEL_PERMISSION, name)
return False
try:
perms = (
Expand All @@ -343,13 +343,13 @@ def delete_action(self, name: str) -> bool:
.all()
)
if perms:
log.warning(const.LOGMSG_WAR_SEC_DEL_PERM_PVM.format(action, perms))
log.warning(const.LOGMSG_WAR_SEC_DEL_PERM_PVM, action, perms)
return False
self.get_session.delete(action)
self.get_session.commit()
return True
except Exception as e:
log.error(const.LOGMSG_ERR_SEC_DEL_PERMISSION.format(e))
log.error(const.LOGMSG_ERR_SEC_DEL_PERMISSION, e)
self.get_session.rollback()
return False

Expand Down Expand Up @@ -383,7 +383,7 @@ def create_resource(self, name) -> Resource:
self.get_session.commit()
return resource
except Exception as e:
log.error(const.LOGMSG_ERR_SEC_ADD_VIEWMENU.format(e))
log.error(const.LOGMSG_ERR_SEC_ADD_VIEWMENU, e)
self.get_session.rollback()
return resource

Expand All @@ -404,7 +404,7 @@ def delete_resource(self, name: str) -> bool:
"""
resource = self.get_resource(name)
if not resource:
log.warning(const.LOGMSG_WAR_SEC_DEL_VIEWMENU.format(name))
log.warning(const.LOGMSG_WAR_SEC_DEL_VIEWMENU, name)
return False
try:
perms = (
Expand All @@ -413,13 +413,13 @@ def delete_resource(self, name: str) -> bool:
.all()
)
if perms:
log.warning(const.LOGMSG_WAR_SEC_DEL_VIEWMENU_PVM.format(resource, perms))
log.warning(const.LOGMSG_WAR_SEC_DEL_VIEWMENU_PVM, resource, perms)
return False
self.get_session.delete(resource)
self.get_session.commit()
return True
except Exception as e:
log.error(const.LOGMSG_ERR_SEC_DEL_PERMISSION.format(e))
log.error(const.LOGMSG_ERR_SEC_DEL_PERMISSION, e)
self.get_session.rollback()
return False

Expand Down Expand Up @@ -481,10 +481,10 @@ def create_permission(self, action_name, resource_name) -> Permission | None:
try:
self.get_session.add(perm)
self.get_session.commit()
log.info(const.LOGMSG_INF_SEC_ADD_PERMVIEW.format(perm))
log.info(const.LOGMSG_INF_SEC_ADD_PERMVIEW, perm)
return perm
except Exception as e:
log.error(const.LOGMSG_ERR_SEC_ADD_PERMVIEW.format(e))
log.error(const.LOGMSG_ERR_SEC_ADD_PERMVIEW, e)
self.get_session.rollback()
return None

Expand All @@ -507,7 +507,7 @@ def delete_permission(self, action_name: str, resource_name: str) -> None:
self.get_session.query(self.role_model).filter(self.role_model.permissions.contains(perm)).first()
)
if roles:
log.warning(const.LOGMSG_WAR_SEC_DEL_PERMVIEW.format(resource_name, action_name, roles))
log.warning(const.LOGMSG_WAR_SEC_DEL_PERMVIEW, resource_name, action_name, roles)
return
try:
# delete permission on resource
Expand All @@ -516,9 +516,9 @@ def delete_permission(self, action_name: str, resource_name: str) -> None:
# if no more permission on permission view, delete permission
if not self.get_session.query(self.permission_model).filter_by(action=perm.action).all():
self.delete_action(perm.action.name)
log.info(const.LOGMSG_INF_SEC_DEL_PERMVIEW.format(action_name, resource_name))
log.info(const.LOGMSG_INF_SEC_DEL_PERMVIEW, action_name, resource_name)
except Exception as e:
log.error(const.LOGMSG_ERR_SEC_DEL_PERMVIEW.format(e))
log.error(const.LOGMSG_ERR_SEC_DEL_PERMVIEW, e)
self.get_session.rollback()

def add_permission_to_role(self, role: Role, permission: Permission | None) -> None:
Expand All @@ -534,9 +534,9 @@ def add_permission_to_role(self, role: Role, permission: Permission | None) -> N
role.permissions.append(permission)
self.get_session.merge(role)
self.get_session.commit()
log.info(const.LOGMSG_INF_SEC_ADD_PERMROLE.format(permission, role.name))
log.info(const.LOGMSG_INF_SEC_ADD_PERMROLE, permission, role.name)
except Exception as e:
log.error(const.LOGMSG_ERR_SEC_ADD_PERMROLE.format(e))
log.error(const.LOGMSG_ERR_SEC_ADD_PERMROLE, e)
self.get_session.rollback()

def remove_permission_from_role(self, role: Role, permission: Permission) -> None:
Expand All @@ -551,7 +551,7 @@ def remove_permission_from_role(self, role: Role, permission: Permission) -> Non
role.permissions.remove(permission)
self.get_session.merge(role)
self.get_session.commit()
log.info(const.LOGMSG_INF_SEC_DEL_PERMROLE.format(permission, role.name))
log.info(const.LOGMSG_INF_SEC_DEL_PERMROLE, permission, role.name)
except Exception as e:
log.error(const.LOGMSG_ERR_SEC_DEL_PERMROLE.format(e))
log.error(const.LOGMSG_ERR_SEC_DEL_PERMROLE, e)
self.get_session.rollback()
16 changes: 8 additions & 8 deletions airflow/www/extensions/init_appbuilder.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ def dynamic_class_import(class_path):
return reduce(getattr, tmp[1:], package)
except Exception as e:
log.exception(e)
log.error(LOGMSG_ERR_FAB_ADDON_IMPORT.format(class_path, e))
log.error(LOGMSG_ERR_FAB_ADDON_IMPORT, class_path, e)


class AirflowAppBuilder:
Expand Down Expand Up @@ -352,10 +352,10 @@ def _add_addon_views(self):
addon_class.register_views()
addon_class.post_process()
self.addon_managers[addon] = addon_class
log.info(LOGMSG_INF_FAB_ADDON_ADDED.format(str(addon)))
log.info(LOGMSG_INF_FAB_ADDON_ADDED, addon)
except Exception as e:
log.exception(e)
log.error(LOGMSG_ERR_FAB_ADDON_PROCESS.format(addon, e))
log.error(LOGMSG_ERR_FAB_ADDON_PROCESS, addon, e)

def _check_and_init(self, baseview):
if hasattr(baseview, "datamodel"):
Expand Down Expand Up @@ -443,7 +443,7 @@ def add_view(
appbuilder.add_link("google", href="www.google.com", icon = "fa-google-plus")
"""
baseview = self._check_and_init(baseview)
log.info(LOGMSG_INF_FAB_ADD_VIEW.format(baseview.__class__.__name__, name))
log.info(LOGMSG_INF_FAB_ADD_VIEW, baseview.__class__.__name__, name)

if not self._view_exists(baseview):
baseview.appbuilder = self
Expand Down Expand Up @@ -544,7 +544,7 @@ def add_view_no_menu(self, baseview, endpoint=None, static_folder=None):
:param baseview: A BaseView type class instantiated.
"""
baseview = self._check_and_init(baseview)
log.info(LOGMSG_INF_FAB_ADD_VIEW.format(baseview.__class__.__name__, ""))
log.info(LOGMSG_INF_FAB_ADD_VIEW, baseview.__class__.__name__, "")

if not self._view_exists(baseview):
baseview.appbuilder = self
Expand All @@ -554,7 +554,7 @@ def add_view_no_menu(self, baseview, endpoint=None, static_folder=None):
self.register_blueprint(baseview, endpoint=endpoint, static_folder=static_folder)
self._add_permission(baseview)
else:
log.warning(LOGMSG_WAR_FAB_VIEW_EXISTS.format(baseview.__class__.__name__))
log.warning(LOGMSG_WAR_FAB_VIEW_EXISTS, baseview.__class__.__name__)
return baseview

def security_cleanup(self):
Expand Down Expand Up @@ -620,15 +620,15 @@ def _add_permission(self, baseview, update_perms=False):
self.sm.add_permissions_view(baseview.base_permissions, baseview.class_permission_name)
except Exception as e:
log.exception(e)
log.error(LOGMSG_ERR_FAB_ADD_PERMISSION_VIEW.format(str(e)))
log.error(LOGMSG_ERR_FAB_ADD_PERMISSION_VIEW, e)

def _add_permissions_menu(self, name, update_perms=False):
if self.update_perms or update_perms:
try:
self.sm.add_permissions_menu(name)
except Exception as e:
log.exception(e)
log.error(LOGMSG_ERR_FAB_ADD_PERMISSION_MENU.format(str(e)))
log.error(LOGMSG_ERR_FAB_ADD_PERMISSION_MENU, e)

def _add_menu_permissions(self, update_perms=False):
if self.update_perms or update_perms:
Expand Down
22 changes: 11 additions & 11 deletions airflow/www/fab_security/manager.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -490,15 +490,15 @@ def auth_user_db(self, username, password):
"c0976a03d2f18f680bfff877c9a965db9eedc51bc0be87c",
"password",
)
log.info(LOGMSG_WAR_SEC_LOGIN_FAILED.format(username))
log.info(LOGMSG_WAR_SEC_LOGIN_FAILED, username)
return None
elif check_password_hash(user.password, password):
self._rotate_session_id()
self.update_user_auth_stat(user, True)
return user
else:
self.update_user_auth_stat(user, False)
log.info(LOGMSG_WAR_SEC_LOGIN_FAILED.format(username))
log.info(LOGMSG_WAR_SEC_LOGIN_FAILED, username)
return None

def _search_ldap(self, ldap, con, username):
Expand Down Expand Up @@ -678,7 +678,7 @@ def auth_user_ldap(self, username, password):
try:
con.start_tls_s()
except Exception:
log.error(LOGMSG_ERR_SEC_AUTH_LDAP_TLS.format(self.auth_ldap_server))
log.error(LOGMSG_ERR_SEC_AUTH_LDAP_TLS, self.auth_ldap_server)
return None

# Define variables, so we can check if they are set in later steps
Expand Down Expand Up @@ -706,7 +706,7 @@ def auth_user_ldap(self, username, password):

# If search failed, go away
if user_dn is None:
log.info(LOGMSG_WAR_SEC_NOLDAP_OBJ.format(username))
log.info(LOGMSG_WAR_SEC_NOLDAP_OBJ, username)
return None

# Bind with user_dn/password (validates credentials)
Expand All @@ -715,7 +715,7 @@ def auth_user_ldap(self, username, password):
self.update_user_auth_stat(user, False)

# Invalid credentials, go away
log.info(LOGMSG_WAR_SEC_LOGIN_FAILED.format(username))
log.info(LOGMSG_WAR_SEC_LOGIN_FAILED, username)
return None

# Flow 2 - (Direct Search Bind):
Expand Down Expand Up @@ -746,7 +746,7 @@ def auth_user_ldap(self, username, password):
self.update_user_auth_stat(user, False)

# Invalid credentials, go away
log.info(LOGMSG_WAR_SEC_LOGIN_FAILED.format(bind_username))
log.info(LOGMSG_WAR_SEC_LOGIN_FAILED, bind_username)
return None

# Search for `username` (if AUTH_LDAP_SEARCH is set)
Expand All @@ -760,7 +760,7 @@ def auth_user_ldap(self, username, password):

# If search failed, go away
if user_dn is None:
log.info(LOGMSG_WAR_SEC_NOLDAP_OBJ.format(username))
log.info(LOGMSG_WAR_SEC_NOLDAP_OBJ, username)
return None

# Sync the user's roles
Expand All @@ -785,7 +785,7 @@ def auth_user_ldap(self, username, password):

# If user registration failed, go away
if not user:
log.info(LOGMSG_ERR_SEC_ADD_REGISTER_USER.format(username))
log.info(LOGMSG_ERR_SEC_ADD_REGISTER_USER, username)
return None

# LOGIN SUCCESS (only if user is now registered)
Expand All @@ -801,7 +801,7 @@ def auth_user_ldap(self, username, password):
if isinstance(e, dict):
msg = getattr(e, "message", None)
if (msg is not None) and ("desc" in msg):
log.error(LOGMSG_ERR_SEC_AUTH_LDAP.format(e.message["desc"]))
log.error(LOGMSG_ERR_SEC_AUTH_LDAP, e.message["desc"])
return None
else:
log.error(e)
Expand All @@ -815,7 +815,7 @@ def auth_user_oid(self, email):
"""
user = self.find_user(email=email)
if user is None or (not user.is_active):
log.info(LOGMSG_WAR_SEC_LOGIN_FAILED.format(email))
log.info(LOGMSG_WAR_SEC_LOGIN_FAILED, email)
return None
else:
self._rotate_session_id()
Expand Down Expand Up @@ -845,7 +845,7 @@ def auth_user_remote_user(self, username):
# If user does not exist on the DB and not auto user registration,
# or user is inactive, go away.
elif user is None or (not user.is_active):
log.info(LOGMSG_WAR_SEC_LOGIN_FAILED.format(username))
log.info(LOGMSG_WAR_SEC_LOGIN_FAILED, username)
return None

self._rotate_session_id()
Expand Down