You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "jeff3071 (via GitHub)" <gi...@apache.org> on 2025/08/22 01:44:17 UTC
[PR] Feat/GitHub auth [airflow]
jeff3071 opened a new pull request, #54812:
URL: https://github.com/apache/airflow/pull/54812
<!--
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to you under the Apache License, Version 2.0 (the
"License"); you may not use this file except in compliance
with the License. You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
-->
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of an existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
closes: #39457
Summary
---
This PR add support for GitHub App Installation Authentication in `GithubHook`.
If `access_token` is not provided, read `key_path`, `app_id`, `installation_id`, and optional `token_permissions` from extra field.
Other Change
---
- Use airflow.sdk.timezone to replace airflow.utils.timezone.
Related Document
---
[Github app document](https://docs.github.com/en/apps)
[Example app installation authentication](https://pygithub.readthedocs.io/en/stable/examples/Authentication.html#app-installation-authentication)
<!-- Please keep an empty line above the dashes. -->
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-guidelines)** for more information.
In case of fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in a newsfragment file, named `{pr_number}.significant.rst` or `{issue_number}.significant.rst`, in [airflow-core/newsfragments](https://github.com/apache/airflow/tree/main/airflow-core/newsfragments).
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add Support for GitHub App Installation Authentication in `GithubHook` [airflow]
Posted by "Lee-W (via GitHub)" <gi...@apache.org>.
Lee-W merged PR #54812:
URL: https://github.com/apache/airflow/pull/54812
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add Support for GitHub App Installation Authentication in `GithubHook` [airflow]
Posted by "jason810496 (via GitHub)" <gi...@apache.org>.
jason810496 commented on code in PR #54812:
URL: https://github.com/apache/airflow/pull/54812#discussion_r2544362533
##########
providers/github/src/airflow/providers/github/hooks/github.py:
##########
@@ -55,17 +54,33 @@ def get_conn(self) -> GithubClient:
conn = self.get_connection(self.github_conn_id)
access_token = conn.password
host = conn.host
-
- # Currently the only method of authenticating to GitHub in Airflow is via a token. This is not the
- # only means available, but raising an exception to enforce this method for now.
- # TODO: When/If other auth methods are implemented this exception should be removed/modified.
- if not access_token:
- raise AirflowException("An access token is required to authenticate to GitHub.")
+ extras = conn.extra_dejson or {}
+
+ if access_token:
+ auth: Auth.Auth = Auth.Token(access_token)
+ elif extras:
+ if key_path := extras.get("key_path"):
+ if not key_path.endswith(".pem"):
+ raise RuntimeError("Unrecognised key file")
Review Comment:
```suggestion
raise RuntimeError("Unrecognised key file: expected a .pem private key")
```
##########
providers/github/src/airflow/providers/github/hooks/github.py:
##########
@@ -55,17 +54,33 @@ def get_conn(self) -> GithubClient:
conn = self.get_connection(self.github_conn_id)
access_token = conn.password
host = conn.host
-
- # Currently the only method of authenticating to GitHub in Airflow is via a token. This is not the
- # only means available, but raising an exception to enforce this method for now.
- # TODO: When/If other auth methods are implemented this exception should be removed/modified.
- if not access_token:
- raise AirflowException("An access token is required to authenticate to GitHub.")
+ extras = conn.extra_dejson or {}
+
+ if access_token:
+ auth: Auth.Auth = Auth.Token(access_token)
+ elif extras:
+ if key_path := extras.get("key_path"):
+ if not key_path.endswith(".pem"):
+ raise RuntimeError("Unrecognised key file")
+ with open(key_path) as key_file:
+ private_key = key_file.read()
+
+ app_id = extras.get("app_id")
+ installation_id = extras.get("installation_id")
+ if not isinstance(installation_id, int):
+ raise RuntimeError("The provided installation_id should be integer.")
+ if not isinstance(app_id, (str | int)):
+ raise RuntimeError("The provided app_id should be integer or string.")
+ token_permissions = extras.get("token_permissions", None)
+
+ auth = Auth.AppAuth(app_id, private_key).get_installation_auth(installation_id, token_permissions)
Review Comment:
It seems it might raise variable not define error for `private_key`, as `private_key` is only defined in `if key_path := extras.get("key_path"):` branch. The error occur if there isn't `key_path` in extra.
##########
providers/github/docs/connections/github.rst:
##########
@@ -40,3 +40,27 @@ Host (optional)
.. code-block::
https://{hostname}/api/v3
+
Review Comment:
Just to make double check that we support GitHub Authentication by either
1. `access_token`
2. `extras.get("key_path")` (SSH Private Key)
If this is the case, how about mentioning the second approach for authentication?
Since we only mention the first approach above. Or maybe we can change the `Extra Parameters` section name to something like `Authenticate with Private Key` ?
##########
providers/github/src/airflow/providers/github/hooks/github.py:
##########
@@ -55,17 +54,33 @@ def get_conn(self) -> GithubClient:
conn = self.get_connection(self.github_conn_id)
access_token = conn.password
host = conn.host
-
- # Currently the only method of authenticating to GitHub in Airflow is via a token. This is not the
- # only means available, but raising an exception to enforce this method for now.
- # TODO: When/If other auth methods are implemented this exception should be removed/modified.
- if not access_token:
- raise AirflowException("An access token is required to authenticate to GitHub.")
+ extras = conn.extra_dejson or {}
+
+ if access_token:
+ auth: Auth.Auth = Auth.Token(access_token)
+ elif extras:
+ if key_path := extras.get("key_path"):
+ if not key_path.endswith(".pem"):
+ raise RuntimeError("Unrecognised key file")
Review Comment:
Would it be better to replace all the `RuntimeError`with `ValueError` for validating the attributes?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add Support for GitHub App Installation Authentication in `GithubHook` [airflow]
Posted by "jeff3071 (via GitHub)" <gi...@apache.org>.
jeff3071 commented on PR #54812:
URL: https://github.com/apache/airflow/pull/54812#issuecomment-3562687344
> Thanks for the update! Please feel free to resolve the addressed comments.
>
> Sorry I should raise the nit in previous review: How about adding unit test for `get_conn` method to check the new branch logic? Thanks!
Thanks for addressing this!
I add some unit test to check `get_conn` method.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add Support for GitHub App Installation Authentication in `GithubHook` [airflow]
Posted by "Lee-W (via GitHub)" <gi...@apache.org>.
Lee-W commented on PR #54812:
URL: https://github.com/apache/airflow/pull/54812#issuecomment-3552468135
still need some fix on CI failure. will take a look tomorrow
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Feat/GitHub auth [airflow]
Posted by "Lee-W (via GitHub)" <gi...@apache.org>.
Lee-W commented on code in PR #54812:
URL: https://github.com/apache/airflow/pull/54812#discussion_r2488405349
##########
providers/github/docs/connections/github.rst:
##########
@@ -40,3 +40,26 @@ Host (optional)
.. code-block::
https://{hostname}/api/v3
+
+Extra Parameters
+----------------
+
+You can authenticate using a GitHub App installation by setting the extra field of your connection, instead of using a token.
+
+- ``key_path``: Path to the private key file used for GitHub App authentication.
+- ``app_id``: The application ID.
+- ``installation_id``: The ID of the app installation.
+- ``token_permissions``: A dictionary of permissions. - Properties of permissions - https://docs.github.com/en/rest/apps/apps?apiVersion=2022-11-28#create-an-installation-access-token-for-an-app
+
+
+Example::
+
+ {
+ "key_path": "FAKE_KEY.pem",
+ "app_id": "123456s",
+ "installation_id": 123456789,
+ "token_permissions": {
+ "issues":"write",
+ "contents":"read"
+ }
+ }
Review Comment:
```suggestion
.. code-block:: json
{
"key_path": "FAKE_KEY.pem",
"app_id": "123456s",
"installation_id": 123456789,
"token_permissions": {
"issues":"write",
"contents":"read"
}
}
```
##########
providers/github/src/airflow/providers/github/hooks/github.py:
##########
@@ -55,17 +54,34 @@ def get_conn(self) -> GithubClient:
conn = self.get_connection(self.github_conn_id)
access_token = conn.password
host = conn.host
-
- # Currently the only method of authenticating to GitHub in Airflow is via a token. This is not the
- # only means available, but raising an exception to enforce this method for now.
- # TODO: When/If other auth methods are implemented this exception should be removed/modified.
- if not access_token:
- raise AirflowException("An access token is required to authenticate to GitHub.")
+ extras = conn.extra_dejson or {}
+
+ if access_token:
+ auth: Auth.Auth = Auth.Token(access_token)
+ elif extras:
+ key_path = extras.get("key_path")
+ if key_path:
+ if not key_path.endswith(".pem"):
+ raise RuntimeError("Unrecognised key file")
+ with open(key_path) as key_file:
+ private_key = key_file.read()
+
+ app_id = extras.get("app_id")
+ installation_id = extras.get("installation_id")
+ if not isinstance(installation_id, int):
+ raise RuntimeError("The provided installation_id should be integer.")
+ if not isinstance(app_id, str) and not isinstance(app_id, int):
Review Comment:
```suggestion
if not isinstance(app_id, (str, int)):
```
##########
providers/github/src/airflow/providers/github/hooks/github.py:
##########
@@ -55,17 +54,34 @@ def get_conn(self) -> GithubClient:
conn = self.get_connection(self.github_conn_id)
access_token = conn.password
host = conn.host
-
- # Currently the only method of authenticating to GitHub in Airflow is via a token. This is not the
- # only means available, but raising an exception to enforce this method for now.
- # TODO: When/If other auth methods are implemented this exception should be removed/modified.
- if not access_token:
- raise AirflowException("An access token is required to authenticate to GitHub.")
+ extras = conn.extra_dejson or {}
+
+ if access_token:
+ auth: Auth.Auth = Auth.Token(access_token)
+ elif extras:
+ key_path = extras.get("key_path")
+ if key_path:
+ if not key_path.endswith(".pem"):
+ raise RuntimeError("Unrecognised key file")
+ with open(key_path) as key_file:
+ private_key = key_file.read()
Review Comment:
```suggestion
if (key_path := extras.get("key_path")):
if not key_path.endswith(".pem"):
raise RuntimeError("Unrecognised key file")
with open(key_path) as key_file:
private_key = key_file.read()
```
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add Support for GitHub App Installation Authentication in `GithubHook` [airflow]
Posted by "Lee-W (via GitHub)" <gi...@apache.org>.
Lee-W commented on code in PR #54812:
URL: https://github.com/apache/airflow/pull/54812#discussion_r2544549771
##########
providers/github/src/airflow/providers/github/hooks/github.py:
##########
@@ -55,17 +54,33 @@ def get_conn(self) -> GithubClient:
conn = self.get_connection(self.github_conn_id)
access_token = conn.password
host = conn.host
-
- # Currently the only method of authenticating to GitHub in Airflow is via a token. This is not the
- # only means available, but raising an exception to enforce this method for now.
- # TODO: When/If other auth methods are implemented this exception should be removed/modified.
- if not access_token:
- raise AirflowException("An access token is required to authenticate to GitHub.")
+ extras = conn.extra_dejson or {}
+
+ if access_token:
+ auth: Auth.Auth = Auth.Token(access_token)
+ elif extras:
+ if key_path := extras.get("key_path"):
+ if not key_path.endswith(".pem"):
+ raise RuntimeError("Unrecognised key file")
+ with open(key_path) as key_file:
+ private_key = key_file.read()
+
+ app_id = extras.get("app_id")
+ installation_id = extras.get("installation_id")
+ if not isinstance(installation_id, int):
+ raise RuntimeError("The provided installation_id should be integer.")
+ if not isinstance(app_id, (str | int)):
+ raise RuntimeError("The provided app_id should be integer or string.")
+ token_permissions = extras.get("token_permissions", None)
+
+ auth = Auth.AppAuth(app_id, private_key).get_installation_auth(installation_id, token_permissions)
Review Comment:
good catch 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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add Support for GitHub App Installation Authentication in `GithubHook` [airflow]
Posted by "jeff3071 (via GitHub)" <gi...@apache.org>.
jeff3071 commented on code in PR #54812:
URL: https://github.com/apache/airflow/pull/54812#discussion_r2545956957
##########
providers/github/src/airflow/providers/github/hooks/github.py:
##########
@@ -55,17 +54,33 @@ def get_conn(self) -> GithubClient:
conn = self.get_connection(self.github_conn_id)
access_token = conn.password
host = conn.host
-
- # Currently the only method of authenticating to GitHub in Airflow is via a token. This is not the
- # only means available, but raising an exception to enforce this method for now.
- # TODO: When/If other auth methods are implemented this exception should be removed/modified.
- if not access_token:
- raise AirflowException("An access token is required to authenticate to GitHub.")
+ extras = conn.extra_dejson or {}
+
+ if access_token:
+ auth: Auth.Auth = Auth.Token(access_token)
+ elif extras:
+ if key_path := extras.get("key_path"):
+ if not key_path.endswith(".pem"):
+ raise RuntimeError("Unrecognised key file")
Review Comment:
I have replace all `RuntimeError` with `ValueError`. Thank!
##########
providers/github/src/airflow/providers/github/hooks/github.py:
##########
@@ -55,17 +54,33 @@ def get_conn(self) -> GithubClient:
conn = self.get_connection(self.github_conn_id)
access_token = conn.password
host = conn.host
-
- # Currently the only method of authenticating to GitHub in Airflow is via a token. This is not the
- # only means available, but raising an exception to enforce this method for now.
- # TODO: When/If other auth methods are implemented this exception should be removed/modified.
- if not access_token:
- raise AirflowException("An access token is required to authenticate to GitHub.")
+ extras = conn.extra_dejson or {}
+
+ if access_token:
+ auth: Auth.Auth = Auth.Token(access_token)
+ elif extras:
+ if key_path := extras.get("key_path"):
+ if not key_path.endswith(".pem"):
+ raise RuntimeError("Unrecognised key file")
Review Comment:
Fix
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add Support for GitHub App Installation Authentication in `GithubHook` [airflow]
Posted by "jeff3071 (via GitHub)" <gi...@apache.org>.
jeff3071 commented on code in PR #54812:
URL: https://github.com/apache/airflow/pull/54812#discussion_r2545958347
##########
providers/github/src/airflow/providers/github/hooks/github.py:
##########
@@ -55,17 +54,33 @@ def get_conn(self) -> GithubClient:
conn = self.get_connection(self.github_conn_id)
access_token = conn.password
host = conn.host
-
- # Currently the only method of authenticating to GitHub in Airflow is via a token. This is not the
- # only means available, but raising an exception to enforce this method for now.
- # TODO: When/If other auth methods are implemented this exception should be removed/modified.
- if not access_token:
- raise AirflowException("An access token is required to authenticate to GitHub.")
+ extras = conn.extra_dejson or {}
+
+ if access_token:
+ auth: Auth.Auth = Auth.Token(access_token)
+ elif extras:
+ if key_path := extras.get("key_path"):
+ if not key_path.endswith(".pem"):
+ raise RuntimeError("Unrecognised key file")
+ with open(key_path) as key_file:
+ private_key = key_file.read()
+
+ app_id = extras.get("app_id")
+ installation_id = extras.get("installation_id")
+ if not isinstance(installation_id, int):
+ raise RuntimeError("The provided installation_id should be integer.")
+ if not isinstance(app_id, (str | int)):
+ raise RuntimeError("The provided app_id should be integer or string.")
+ token_permissions = extras.get("token_permissions", None)
+
+ auth = Auth.AppAuth(app_id, private_key).get_installation_auth(installation_id, token_permissions)
Review Comment:
Great catch thank!
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add Support for GitHub App Installation Authentication in `GithubHook` [airflow]
Posted by "jeff3071 (via GitHub)" <gi...@apache.org>.
jeff3071 commented on code in PR #54812:
URL: https://github.com/apache/airflow/pull/54812#discussion_r2545969210
##########
providers/github/docs/connections/github.rst:
##########
@@ -40,3 +40,27 @@ Host (optional)
.. code-block::
https://{hostname}/api/v3
+
Review Comment:
Sorry for misleading.
Now we can support both access token authentication and github app installation authentication.
Github app installation authentication can offers more granular permission model compared to access tokens.
I update the connection docs. 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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add Support for GitHub App Installation Authentication in `GithubHook` [airflow]
Posted by "Lee-W (via GitHub)" <gi...@apache.org>.
Lee-W commented on code in PR #54812:
URL: https://github.com/apache/airflow/pull/54812#discussion_r2544537544
##########
providers/github/src/airflow/providers/github/hooks/github.py:
##########
@@ -55,17 +54,33 @@ def get_conn(self) -> GithubClient:
conn = self.get_connection(self.github_conn_id)
access_token = conn.password
host = conn.host
-
- # Currently the only method of authenticating to GitHub in Airflow is via a token. This is not the
- # only means available, but raising an exception to enforce this method for now.
- # TODO: When/If other auth methods are implemented this exception should be removed/modified.
- if not access_token:
- raise AirflowException("An access token is required to authenticate to GitHub.")
+ extras = conn.extra_dejson or {}
+
+ if access_token:
+ auth: Auth.Auth = Auth.Token(access_token)
+ elif extras:
+ if key_path := extras.get("key_path"):
+ if not key_path.endswith(".pem"):
+ raise RuntimeError("Unrecognised key file")
Review Comment:
Yep, sounds like a better option
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org