ephraimbuddy commented on code in PR #60280:
URL: https://github.com/apache/airflow/pull/60280#discussion_r2675344560
##########
providers/git/src/airflow/providers/git/bundles/git.py:
##########
@@ -27,13 +27,59 @@
from git.exc import BadName, GitCommandError, InvalidGitRepositoryError,
NoSuchPathError
from tenacity import retry, retry_if_exception_type, stop_after_attempt
-from airflow.dag_processing.bundles.base import BaseDagBundle
+from airflow.dag_processing.bundles.base import BaseDagBundle,
get_bundle_permissions
from airflow.providers.common.compat.sdk import AirflowException
from airflow.providers.git.hooks.git import GitHook
log = structlog.get_logger(__name__)
+def _apply_permissions_recursively(path: Path) -> None:
+ """
+ Apply configured bundle permissions to a directory tree.
+
+ This ensures that when user impersonation is used, the impersonated user
+ can access the cloned repository files.
+
+ :param path: The root path to apply permissions to recursively
+ """
+ folder_perms, file_perms = get_bundle_permissions()
+ with suppress(OSError):
+ for root, dirs, files in os.walk(path):
+ root_path = Path(root)
+ with suppress(OSError):
+ root_path.chmod(folder_perms)
+ for d in dirs:
+ with suppress(OSError):
+ (root_path / d).chmod(folder_perms)
+ for f in files:
+ with suppress(OSError):
+ (root_path / f).chmod(file_perms)
+
+
+def _configure_git_safe_directory(path: Path) -> None:
+ """
+ Add path to git safe.directory to allow cross-user access.
+
+ Git 2.35.2+ refuses to operate on repositories owned by different users
+ without explicit safe directory configuration. This is needed when using
+ user impersonation (run_as_user) where the repository is created by one
+ user but accessed by another.
Review Comment:
What if the run_as_user is configured in only one task?
##########
providers/git/tests/unit/git/bundles/test_git.py:
##########
@@ -976,3 +976,52 @@ def test_submodule_fetch_error_raises_runtime_error(
bundle.initialize()
mock_rmtree.assert_not_called()
+
+ @mock.patch("airflow.providers.git.bundles.git.GitHook")
+ def test_clone_applies_permissions(self, mock_githook, git_repo,
bundle_temp_dir):
+ """Test that cloning applies configured permissions to repository
directories."""
+ import stat
+
+ repo_path, repo = git_repo
+ mock_githook.return_value.repo_url = repo_path
+
+ with conf_vars(
+ {
+ ("dag_processor", "dag_bundle_new_folder_permissions"):
"0o775",
+ ("dag_processor", "dag_bundle_new_file_permissions"): "0o664",
+ }
+ ):
Review Comment:
You should have included the other PR and mark this as dependent on that one
--
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]
