Miretpl commented on code in PR #64422:
URL: https://github.com/apache/airflow/pull/64422#discussion_r3693069677
##########
providers/git/src/airflow/providers/git/hooks/git.py:
##########
@@ -127,6 +140,26 @@ def __init__(
AirflowProviderDeprecationWarning,
stacklevel=2,
)
+ if (self.github_app_id is not None and self.github_installation_id is
None) or (
+ self.github_app_id is None and self.github_installation_id is not
None
+ ):
Review Comment:
```suggestion
if None in (self.github_app_id, self.github_installation_id):
```
##########
providers/git/src/airflow/providers/git/hooks/git.py:
##########
@@ -183,6 +216,82 @@ def _build_ssh_command(self, key_path: str | None = None)
-> str:
return " ".join(parts)
+ def _get_github_app_token(self):
Review Comment:
Missing return annotation.
##########
providers/git/src/airflow/providers/git/hooks/git.py:
##########
@@ -127,6 +140,26 @@ def __init__(
AirflowProviderDeprecationWarning,
stacklevel=2,
)
+ if (self.github_app_id is not None and self.github_installation_id is
None) or (
+ self.github_app_id is None and self.github_installation_id is not
None
+ ):
+ raise ValueError(
+ "Both 'github_app_id' and 'github_installation_id' must be
provided to use GitHub App Authentication"
+ )
+ if self.github_app_id is not None and self.github_installation_id is
not None:
+ if self.key_file and not self.private_key:
+ with open(self.key_file, encoding="utf-8") as key_file:
+ self.private_key = key_file.read()
+ if not (self.repo_url or "").startswith(("https://";, "http://";)):
+ raise ValueError(
+ f"GitHub App authentication requires an HTTPS repository
URL, but got: {self.repo_url!r}"
+ )
+ # Store the PEM separately so configure_hook_env() does not treat
it as an SSH key.
+ # Keep `private_key` populated for callers/tests that expect it to
be available,
+ # but also keep a dedicated attribute so configure_hook_env() can
avoid
+ # treating the GitHub App PEM as an SSH key.
+ self.github_app_private_key = self.private_key
+ self.auth_token = ""
Review Comment:
Shouldn't we require an empty password in the Airflow connection instead of
silently setting this to an empty string?
##########
providers/git/src/airflow/providers/git/hooks/git.py:
##########
@@ -127,6 +140,26 @@ def __init__(
AirflowProviderDeprecationWarning,
stacklevel=2,
)
+ if (self.github_app_id is not None and self.github_installation_id is
None) or (
+ self.github_app_id is None and self.github_installation_id is not
None
+ ):
+ raise ValueError(
+ "Both 'github_app_id' and 'github_installation_id' must be
provided to use GitHub App Authentication"
+ )
+ if self.github_app_id is not None and self.github_installation_id is
not None:
Review Comment:
```suggestion
else:
```
##########
providers/git/src/airflow/providers/git/hooks/git.py:
##########
@@ -183,6 +216,82 @@ def _build_ssh_command(self, key_path: str | None = None)
-> str:
return " ".join(parts)
+ def _get_github_app_token(self):
+ try:
+ from github import Auth, GithubIntegration
+ except ImportError as exc:
+ raise ImportError(
+ "The PyGithub library is required for GitHub App
authentication. Please install it with 'pip install
apache-airflow-providers-git[github]'"
+ ) from exc
+
+ auth = Auth.AppAuth(self.github_app_id, self.github_app_private_key)
+ integration = GithubIntegration(auth=auth)
+ access_token =
integration.get_access_token(installation_id=self.github_installation_id)
+ github_app_token_exp = access_token.expires_at
+ log.info(
+ "Successfully obtained GitHub App installation access token
(expires at: %s)",
+ github_app_token_exp,
+ )
+
+ return "x-access-token", access_token.token, github_app_token_exp
+
+ def _ensure_github_app_token(self) -> None:
+ if self.github_app_id is None or self.github_installation_id is None:
+ return
+
+ TOKEN_REFRESH_BUFFER = timedelta(minutes=5)
+ if (
+ self.github_app_token_exp is None
+ or self.github_app_token_exp < datetime.now(timezone.utc) +
TOKEN_REFRESH_BUFFER
+ ):
+ log.info(
+ "GitHub App token is missing or near expiry (expires at: %s).
Refreshing token.",
+ self.github_app_token_exp,
+ )
+ self.user_name, self.auth_token, self.github_app_token_exp =
self._get_github_app_token()
+
+ @contextlib.contextmanager
+ def _github_app_askpass_env(self):
Review Comment:
Missing return annotation type.
##########
providers/git/src/airflow/providers/git/hooks/git.py:
##########
@@ -240,7 +349,16 @@ def _passphrase_askpass_env(self):
@contextlib.contextmanager
def configure_hook_env(self):
- if self.private_key:
+ self._ensure_github_app_token()
Review Comment:
We could move it under if below and remove the check in
`_ensure_github_app_token` as it would be duplicated.
##########
providers/git/src/airflow/providers/git/hooks/git.py:
##########
@@ -183,6 +216,82 @@ def _build_ssh_command(self, key_path: str | None = None)
-> str:
return " ".join(parts)
+ def _get_github_app_token(self):
+ try:
+ from github import Auth, GithubIntegration
+ except ImportError as exc:
+ raise ImportError(
+ "The PyGithub library is required for GitHub App
authentication. Please install it with 'pip install
apache-airflow-providers-git[github]'"
+ ) from exc
+
+ auth = Auth.AppAuth(self.github_app_id, self.github_app_private_key)
+ integration = GithubIntegration(auth=auth)
+ access_token =
integration.get_access_token(installation_id=self.github_installation_id)
+ github_app_token_exp = access_token.expires_at
+ log.info(
+ "Successfully obtained GitHub App installation access token
(expires at: %s)",
+ github_app_token_exp,
+ )
+
+ return "x-access-token", access_token.token, github_app_token_exp
+
+ def _ensure_github_app_token(self) -> None:
+ if self.github_app_id is None or self.github_installation_id is None:
+ return
+
+ TOKEN_REFRESH_BUFFER = timedelta(minutes=5)
+ if (
+ self.github_app_token_exp is None
+ or self.github_app_token_exp < datetime.now(timezone.utc) +
TOKEN_REFRESH_BUFFER
+ ):
+ log.info(
+ "GitHub App token is missing or near expiry (expires at: %s).
Refreshing token.",
+ self.github_app_token_exp,
+ )
+ self.user_name, self.auth_token, self.github_app_token_exp =
self._get_github_app_token()
+
+ @contextlib.contextmanager
+ def _github_app_askpass_env(self):
Review Comment:
Can't we use the currently implemented `_passphrase_askpass_env`, but with a
modification for `askpass_script` 🤔?
--
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]
