You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2020/03/21 11:28:56 UTC
[GitHub] [airflow] xinbinhuang opened a new pull request #7795:
[AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
xinbinhuang opened a new pull request #7795: [AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
URL: https://github.com/apache/airflow/pull/7795
Add Secret backend for GCP Secrets Manager
---
Issue link: WILL BE INSERTED BY [boring-cyborg](https://github.com/kaxil/boring-cyborg)
Make sure to mark the boxes below before creating PR: [x]
- [ ] Description above provides context of the change
- [ ] Unit tests coverage for changes (not needed for documentation changes)
- [ ] Commits follow "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)"
- [ ] Relevant documentation is updated including usage instructions.
- [ ] I will engage committers as explained in [Contribution Workflow Example](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#contribution-workflow-example).
---
In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+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 [UPDATING.md](https://github.com/apache/airflow/blob/master/UPDATING.md).
Read the [Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines) for more information.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #7795:
[AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #7795: [AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
URL: https://github.com/apache/airflow/pull/7795#issuecomment-602132463
# [Codecov](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=h1) Report
> Merging [#7795](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/fee827ed5a5d4aea2d2e60f73443357721a2c45a&el=desc) will **decrease** coverage by `0.44%`.
> The diff coverage is `95.45%`.
[](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #7795 +/- ##
==========================================
- Coverage 86.93% 86.48% -0.45%
==========================================
Files 924 925 +1
Lines 44674 44718 +44
==========================================
- Hits 38836 38675 -161
- Misses 5838 6043 +205
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/providers/hashicorp/secrets/vault.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvaGFzaGljb3JwL3NlY3JldHMvdmF1bHQucHk=) | `76.47% <0.00%> (+11.03%)` | :arrow_up: |
| [...oviders/google/cloud/utils/credentials\_provider.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvZ29vZ2xlL2Nsb3VkL3V0aWxzL2NyZWRlbnRpYWxzX3Byb3ZpZGVyLnB5) | `93.58% <96.66%> (+1.75%)` | :arrow_up: |
| [airflow/providers/google/cloud/hooks/base.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvZ29vZ2xlL2Nsb3VkL2hvb2tzL2Jhc2UucHk=) | `97.02% <100.00%> (+1.34%)` | :arrow_up: |
| [...irflow/providers/google/secrets/secrets\_manager.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvZ29vZ2xlL3NlY3JldHMvc2VjcmV0c19tYW5hZ2VyLnB5) | `100.00% <100.00%> (ø)` | |
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0.00%> (-55.56%)` | :arrow_down: |
| [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0.00%> (-47.06%)` | :arrow_down: |
| [airflow/security/kerberos.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9zZWN1cml0eS9rZXJiZXJvcy5weQ==) | `30.43% <0.00%> (-45.66%)` | :arrow_down: |
| [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `47.18% <0.00%> (-45.08%)` | :arrow_down: |
| [airflow/executors/celery\_executor.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGVjdXRvcnMvY2VsZXJ5X2V4ZWN1dG9yLnB5) | `50.67% <0.00%> (-37.84%)` | :arrow_down: |
| [...viders/cncf/kubernetes/operators/kubernetes\_pod.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvY25jZi9rdWJlcm5ldGVzL29wZXJhdG9ycy9rdWJlcm5ldGVzX3BvZC5weQ==) | `69.69% <0.00%> (-25.26%)` | :arrow_down: |
| ... and [7 more](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=footer). Last update [fee827e...a04f148](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] mik-laj commented on a change in pull request #7795:
[AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #7795: [AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
URL: https://github.com/apache/airflow/pull/7795#discussion_r396076912
##########
File path: airflow/providers/google/cloud/utils/credentials_provider.py
##########
@@ -158,3 +161,43 @@ def provide_gcp_conn_and_credentials(
key_file_path, scopes, project_id
):
yield
+
+
+def get_credentials_and_project_id(
Review comment:
LGTM
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #7795:
[AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #7795: [AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
URL: https://github.com/apache/airflow/pull/7795#issuecomment-602132463
# [Codecov](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=h1) Report
> Merging [#7795](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/fee827ed5a5d4aea2d2e60f73443357721a2c45a&el=desc) will **decrease** coverage by `0.39%`.
> The diff coverage is `95.45%`.
[](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #7795 +/- ##
==========================================
- Coverage 86.93% 86.53% -0.40%
==========================================
Files 924 925 +1
Lines 44674 44718 +44
==========================================
- Hits 38836 38697 -139
- Misses 5838 6021 +183
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/providers/hashicorp/secrets/vault.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvaGFzaGljb3JwL3NlY3JldHMvdmF1bHQucHk=) | `76.47% <0.00%> (+11.03%)` | :arrow_up: |
| [...oviders/google/cloud/utils/credentials\_provider.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvZ29vZ2xlL2Nsb3VkL3V0aWxzL2NyZWRlbnRpYWxzX3Byb3ZpZGVyLnB5) | `93.58% <96.66%> (+1.75%)` | :arrow_up: |
| [airflow/providers/google/cloud/hooks/base.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvZ29vZ2xlL2Nsb3VkL2hvb2tzL2Jhc2UucHk=) | `97.02% <100.00%> (+1.34%)` | :arrow_up: |
| [...irflow/providers/google/secrets/secrets\_manager.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvZ29vZ2xlL3NlY3JldHMvc2VjcmV0c19tYW5hZ2VyLnB5) | `100.00% <100.00%> (ø)` | |
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0.00%> (-55.56%)` | :arrow_down: |
| [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0.00%> (-47.06%)` | :arrow_down: |
| [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `47.18% <0.00%> (-45.08%)` | :arrow_down: |
| [airflow/executors/celery\_executor.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGVjdXRvcnMvY2VsZXJ5X2V4ZWN1dG9yLnB5) | `50.67% <0.00%> (-37.84%)` | :arrow_down: |
| [...viders/cncf/kubernetes/operators/kubernetes\_pod.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvY25jZi9rdWJlcm5ldGVzL29wZXJhdG9ycy9rdWJlcm5ldGVzX3BvZC5weQ==) | `69.69% <0.00%> (-25.26%)` | :arrow_down: |
| [airflow/kubernetes/refresh\_config.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3JlZnJlc2hfY29uZmlnLnB5) | `50.98% <0.00%> (-23.53%)` | :arrow_down: |
| ... and [5 more](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=footer). Last update [fee827e...a04f148](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] kaxil commented on a change in pull request #7795:
[AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #7795: [AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
URL: https://github.com/apache/airflow/pull/7795#discussion_r396046532
##########
File path: airflow/providers/google/secrets/secrets_manager.py
##########
@@ -0,0 +1,132 @@
+# 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.
+
+"""
+Objects relating to sourcing connections from GCP Secrets Manager
+"""
+from typing import List, Optional
+
+from cached_property import cached_property
+from google.api_core.exceptions import NotFound
+from google.api_core.gapic_v1.client_info import ClientInfo
+from google.cloud.secretmanager_v1 import SecretManagerServiceClient
+
+from airflow import version
+from airflow.models import Connection
+from airflow.providers.google.cloud.utils.credentials_provider import (
+ _get_scopes, get_credentials_and_project_id,
+)
+from airflow.secrets import BaseSecretsBackend
+from airflow.utils.log.logging_mixin import LoggingMixin
+
+
+class GcpSecretsManagerSecretsBackend(BaseSecretsBackend, LoggingMixin):
Review comment:
```suggestion
class GcpSecretManager(BaseSecretsBackend, LoggingMixin):
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] xinbinhuang commented on issue #7795: [AIRFLOW-7104] Add
Secret backend for GCP Secrets Manager
Posted by GitBox <gi...@apache.org>.
xinbinhuang commented on issue #7795: [AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
URL: https://github.com/apache/airflow/pull/7795#issuecomment-602031411
@kaxil PTAL 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #7795:
[AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #7795: [AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
URL: https://github.com/apache/airflow/pull/7795#issuecomment-602132463
# [Codecov](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=h1) Report
> Merging [#7795](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/fee827ed5a5d4aea2d2e60f73443357721a2c45a&el=desc) will **decrease** coverage by `0.72%`.
> The diff coverage is `95.50%`.
[](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #7795 +/- ##
==========================================
- Coverage 86.93% 86.20% -0.73%
==========================================
Files 924 925 +1
Lines 44674 44836 +162
==========================================
- Hits 38836 38651 -185
- Misses 5838 6185 +347
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/providers/hashicorp/secrets/vault.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvaGFzaGljb3JwL3NlY3JldHMvdmF1bHQucHk=) | `76.47% <0.00%> (+11.03%)` | :arrow_up: |
| [...oviders/google/cloud/utils/credentials\_provider.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvZ29vZ2xlL2Nsb3VkL3V0aWxzL2NyZWRlbnRpYWxzX3Byb3ZpZGVyLnB5) | `93.58% <96.66%> (+1.75%)` | :arrow_up: |
| [airflow/providers/google/cloud/hooks/base.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvZ29vZ2xlL2Nsb3VkL2hvb2tzL2Jhc2UucHk=) | `97.02% <100.00%> (+1.34%)` | :arrow_up: |
| [.../providers/google/cloud/secrets/secrets\_manager.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvZ29vZ2xlL2Nsb3VkL3NlY3JldHMvc2VjcmV0c19tYW5hZ2VyLnB5) | `100.00% <100.00%> (ø)` | |
| [...flow/providers/apache/cassandra/hooks/cassandra.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYXBhY2hlL2Nhc3NhbmRyYS9ob29rcy9jYXNzYW5kcmEucHk=) | `21.51% <0.00%> (-72.16%)` | :arrow_down: |
| [...w/providers/apache/hive/operators/mysql\_to\_hive.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYXBhY2hlL2hpdmUvb3BlcmF0b3JzL215c3FsX3RvX2hpdmUucHk=) | `35.84% <0.00%> (-64.16%)` | :arrow_down: |
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0.00%> (-55.56%)` | :arrow_down: |
| [airflow/providers/postgres/operators/postgres.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvcG9zdGdyZXMvb3BlcmF0b3JzL3Bvc3RncmVzLnB5) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
| [airflow/providers/redis/operators/redis\_publish.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvcmVkaXMvb3BlcmF0b3JzL3JlZGlzX3B1Ymxpc2gucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
| [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0.00%> (-47.06%)` | :arrow_down: |
| ... and [19 more](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=footer). Last update [fee827e...3a4b925](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] kaxil commented on a change in pull request #7795:
[AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #7795: [AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
URL: https://github.com/apache/airflow/pull/7795#discussion_r396103880
##########
File path: airflow/providers/hashicorp/secrets/vault.py
##########
@@ -40,10 +40,10 @@ class VaultSecrets(BaseSecretsBackend, LoggingMixin):
[secrets]
backend = airflow.providers.hashicorp.secrets.vault.VaultSecrets
- backend_kwargs = {"path":"connections","url":"http://127.0.0.1:8200","mount_point":"airflow"}
+ backend_kwargs = {"connections_path":"conns","url":"http://127.0.0.1:8200","mount_point":"airflow"}
- For example, if your keys are under ``connections`` path in ``airflow`` mount_point, this
- would be accessible if you provide ``{"path": "connections"}`` and request
+ For example, if your keys are under ``conns`` path in ``airflow`` mount_point, this
+ would be accessible if you provide ``{"connections_path": "conns"}`` and request
conn_id ``smtp_default``.
Review comment:
I would prefer `connections_path` too, please
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #7795:
[AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #7795: [AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
URL: https://github.com/apache/airflow/pull/7795#issuecomment-602132463
# [Codecov](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=h1) Report
> Merging [#7795](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/fee827ed5a5d4aea2d2e60f73443357721a2c45a&el=desc) will **decrease** coverage by `22.54%`.
> The diff coverage is `95.45%`.
[](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #7795 +/- ##
===========================================
- Coverage 86.93% 64.38% -22.55%
===========================================
Files 924 924
Lines 44674 44705 +31
===========================================
- Hits 38836 28785 -10051
- Misses 5838 15920 +10082
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/providers/hashicorp/secrets/vault.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvaGFzaGljb3JwL3NlY3JldHMvdmF1bHQucHk=) | `76.47% <0.00%> (+11.03%)` | :arrow_up: |
| [...oviders/google/cloud/utils/credentials\_provider.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvZ29vZ2xlL2Nsb3VkL3V0aWxzL2NyZWRlbnRpYWxzX3Byb3ZpZGVyLnB5) | `93.58% <96.66%> (+1.75%)` | :arrow_up: |
| [airflow/providers/google/cloud/hooks/base.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvZ29vZ2xlL2Nsb3VkL2hvb2tzL2Jhc2UucHk=) | `97.02% <100.00%> (+1.34%)` | :arrow_up: |
| [...irflow/providers/google/secrets/secrets\_manager.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvZ29vZ2xlL3NlY3JldHMvc2VjcmV0c19tYW5hZ2VyLnB5) | `100.00% <100.00%> (ø)` | |
| [airflow/hooks/S3\_hook.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9ob29rcy9TM19ob29rLnB5) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [airflow/hooks/pig\_hook.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9ob29rcy9waWdfaG9vay5weQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [airflow/hooks/hdfs\_hook.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9ob29rcy9oZGZzX2hvb2sucHk=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [airflow/hooks/http\_hook.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9ob29rcy9odHRwX2hvb2sucHk=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [airflow/hooks/jdbc\_hook.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9ob29rcy9qZGJjX2hvb2sucHk=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [airflow/hooks/druid\_hook.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9ob29rcy9kcnVpZF9ob29rLnB5) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [499 more](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=footer). Last update [fee827e...a04f148](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] kaxil merged pull request #7795: [AIRFLOW-7104] Add
Secret backend for GCP Secrets Manager
Posted by GitBox <gi...@apache.org>.
kaxil merged pull request #7795: [AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
URL: https://github.com/apache/airflow/pull/7795
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #7795:
[AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #7795: [AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
URL: https://github.com/apache/airflow/pull/7795#issuecomment-602132463
# [Codecov](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=h1) Report
> Merging [#7795](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/fee827ed5a5d4aea2d2e60f73443357721a2c45a&el=desc) will **decrease** coverage by `0.72%`.
> The diff coverage is `95.50%`.
[](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #7795 +/- ##
==========================================
- Coverage 86.93% 86.20% -0.73%
==========================================
Files 924 925 +1
Lines 44674 44836 +162
==========================================
- Hits 38836 38651 -185
- Misses 5838 6185 +347
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/providers/hashicorp/secrets/vault.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvaGFzaGljb3JwL3NlY3JldHMvdmF1bHQucHk=) | `76.47% <0.00%> (+11.03%)` | :arrow_up: |
| [...oviders/google/cloud/utils/credentials\_provider.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvZ29vZ2xlL2Nsb3VkL3V0aWxzL2NyZWRlbnRpYWxzX3Byb3ZpZGVyLnB5) | `93.58% <96.66%> (+1.75%)` | :arrow_up: |
| [airflow/providers/google/cloud/hooks/base.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvZ29vZ2xlL2Nsb3VkL2hvb2tzL2Jhc2UucHk=) | `97.02% <100.00%> (+1.34%)` | :arrow_up: |
| [.../providers/google/cloud/secrets/secrets\_manager.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvZ29vZ2xlL2Nsb3VkL3NlY3JldHMvc2VjcmV0c19tYW5hZ2VyLnB5) | `100.00% <100.00%> (ø)` | |
| [...flow/providers/apache/cassandra/hooks/cassandra.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYXBhY2hlL2Nhc3NhbmRyYS9ob29rcy9jYXNzYW5kcmEucHk=) | `21.51% <0.00%> (-72.16%)` | :arrow_down: |
| [...w/providers/apache/hive/operators/mysql\_to\_hive.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYXBhY2hlL2hpdmUvb3BlcmF0b3JzL215c3FsX3RvX2hpdmUucHk=) | `35.84% <0.00%> (-64.16%)` | :arrow_down: |
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0.00%> (-55.56%)` | :arrow_down: |
| [airflow/providers/postgres/operators/postgres.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvcG9zdGdyZXMvb3BlcmF0b3JzL3Bvc3RncmVzLnB5) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
| [airflow/providers/redis/operators/redis\_publish.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvcmVkaXMvb3BlcmF0b3JzL3JlZGlzX3B1Ymxpc2gucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
| [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0.00%> (-47.06%)` | :arrow_down: |
| ... and [19 more](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=footer). Last update [fee827e...3a4b925](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] kaxil commented on a change in pull request #7795:
[AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #7795: [AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
URL: https://github.com/apache/airflow/pull/7795#discussion_r396103880
##########
File path: airflow/providers/hashicorp/secrets/vault.py
##########
@@ -40,10 +40,10 @@ class VaultSecrets(BaseSecretsBackend, LoggingMixin):
[secrets]
backend = airflow.providers.hashicorp.secrets.vault.VaultSecrets
- backend_kwargs = {"path":"connections","url":"http://127.0.0.1:8200","mount_point":"airflow"}
+ backend_kwargs = {"connections_path":"conns","url":"http://127.0.0.1:8200","mount_point":"airflow"}
- For example, if your keys are under ``connections`` path in ``airflow`` mount_point, this
- would be accessible if you provide ``{"path": "connections"}`` and request
+ For example, if your keys are under ``conns`` path in ``airflow`` mount_point, this
+ would be accessible if you provide ``{"connections_path": "conns"}`` and request
conn_id ``smtp_default``.
Review comment:
I would prefer `connections ` too, please
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] turbaszek commented on a change in pull request #7795:
[AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #7795: [AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
URL: https://github.com/apache/airflow/pull/7795#discussion_r396067868
##########
File path: airflow/providers/google/secrets/secrets_manager.py
##########
@@ -0,0 +1,125 @@
+# 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.
+
+"""
+Objects relating to sourcing connections from GCP Secrets Manager
+"""
+from typing import List, Optional
+
+from cached_property import cached_property
+from google.api_core.exceptions import NotFound
+from google.api_core.gapic_v1.client_info import ClientInfo
+from google.cloud.secretmanager_v1 import SecretManagerServiceClient
+
+from airflow import version
+from airflow.models import Connection
+from airflow.providers.google.cloud.utils.credentials_provider import get_credentials_and_project_id
+from airflow.secrets import BaseSecretsBackend
+from airflow.utils.log.logging_mixin import LoggingMixin
+
+
+class GcpSecretsManagerSecretsBackend(BaseSecretsBackend, LoggingMixin):
+ """
+ Retrieves Connection object from GCP Secrets Manager
+
+ Configurable via ``airflow.cfg`` as follows:
+
+ .. code-block:: ini
+
+ [secrets]
+ backend = airflow.providers.google.secrets.secrets_manager.GcpSecretsManagerSecretsBackend
+ backend_kwargs = {"connections_prefix": "airflow/connections"}
+
+ For example, if secret id is ``airflow/connections/smtp_default``, this would be accessible
+ if you provide ``{"connections_prefix": "airflow/connections"}`` and request conn_id ``smtp_default``.
+
+ :param connections_prefix: Specifies the prefix of the secret to read to get Connections.
+ :type connections_prefix: str
+ :param gcp_key_path: Path to GCP Credential JSON file;
+ use default credentials in the current environment if not provided.
+ :type gcp_key_path: str
+ :param gcp_scopes: Comma-separated string containing GCP scopes
+ :type gcp_scopes: str
+ """
+ def __init__(
+ self,
+ connections_prefix: str = "airflow/connections",
+ gcp_key_path: Optional[str] = None,
+ gcp_scopes: Optional[str] = None,
+ **kwargs
+ ):
+ self.connections_prefix = connections_prefix.rstrip("/")
+ self.credentials, self.project_id = get_credentials_and_project_id(
Review comment:
The main reason for avoiding db / API calls in the constructor (in operators) is to avoid requests during DAG parsing. However, in this case, I don't think any operator will call `GcpSecretsManagerSecretsBackend()` in its constructor
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #7795:
[AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #7795: [AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
URL: https://github.com/apache/airflow/pull/7795#issuecomment-602132463
# [Codecov](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=h1) Report
> Merging [#7795](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/fee827ed5a5d4aea2d2e60f73443357721a2c45a&el=desc) will **decrease** coverage by `0.72%`.
> The diff coverage is `95.50%`.
[](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #7795 +/- ##
==========================================
- Coverage 86.93% 86.20% -0.73%
==========================================
Files 924 925 +1
Lines 44674 44836 +162
==========================================
- Hits 38836 38652 -184
- Misses 5838 6184 +346
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/providers/hashicorp/secrets/vault.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvaGFzaGljb3JwL3NlY3JldHMvdmF1bHQucHk=) | `76.47% <0.00%> (+11.03%)` | :arrow_up: |
| [...oviders/google/cloud/utils/credentials\_provider.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvZ29vZ2xlL2Nsb3VkL3V0aWxzL2NyZWRlbnRpYWxzX3Byb3ZpZGVyLnB5) | `93.58% <96.66%> (+1.75%)` | :arrow_up: |
| [airflow/providers/google/cloud/hooks/base.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvZ29vZ2xlL2Nsb3VkL2hvb2tzL2Jhc2UucHk=) | `97.02% <100.00%> (+1.34%)` | :arrow_up: |
| [.../providers/google/cloud/secrets/secrets\_manager.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvZ29vZ2xlL2Nsb3VkL3NlY3JldHMvc2VjcmV0c19tYW5hZ2VyLnB5) | `100.00% <100.00%> (ø)` | |
| [...flow/providers/apache/cassandra/hooks/cassandra.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYXBhY2hlL2Nhc3NhbmRyYS9ob29rcy9jYXNzYW5kcmEucHk=) | `21.51% <0.00%> (-72.16%)` | :arrow_down: |
| [...w/providers/apache/hive/operators/mysql\_to\_hive.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYXBhY2hlL2hpdmUvb3BlcmF0b3JzL215c3FsX3RvX2hpdmUucHk=) | `35.84% <0.00%> (-64.16%)` | :arrow_down: |
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0.00%> (-55.56%)` | :arrow_down: |
| [airflow/providers/postgres/operators/postgres.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvcG9zdGdyZXMvb3BlcmF0b3JzL3Bvc3RncmVzLnB5) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
| [airflow/providers/redis/operators/redis\_publish.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvcmVkaXMvb3BlcmF0b3JzL3JlZGlzX3B1Ymxpc2gucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
| [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0.00%> (-47.06%)` | :arrow_down: |
| ... and [18 more](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=footer). Last update [fee827e...3a4b925](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] xinbinhuang commented on a change in pull request #7795:
[AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
Posted by GitBox <gi...@apache.org>.
xinbinhuang commented on a change in pull request #7795: [AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
URL: https://github.com/apache/airflow/pull/7795#discussion_r396081461
##########
File path: airflow/providers/google/cloud/secrets/secrets_manager.py
##########
@@ -0,0 +1,132 @@
+# 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.
+
+"""
+Objects relating to sourcing connections from GCP Secrets Manager
+"""
+from typing import List, Optional
+
+from cached_property import cached_property
+from google.api_core.exceptions import NotFound
+from google.api_core.gapic_v1.client_info import ClientInfo
+from google.cloud.secretmanager_v1 import SecretManagerServiceClient
+
+from airflow import version
+from airflow.models import Connection
+from airflow.providers.google.cloud.utils.credentials_provider import (
+ _get_scopes, get_credentials_and_project_id,
+)
+from airflow.secrets import BaseSecretsBackend
+from airflow.utils.log.logging_mixin import LoggingMixin
+
+
+class GcpSecretsManagerSecretsBackend(BaseSecretsBackend, LoggingMixin):
+ """
+ Retrieves Connection object from GCP Secrets Manager
+
+ Configurable via ``airflow.cfg`` as follows:
+
+ .. code-block:: ini
+
+ [secrets]
+ backend = airflow.providers.google.cloud.secrets.secrets_manager.GcpSecretsManagerSecretsBackend
+ backend_kwargs = {"connections_prefix": "airflow/connections"}
+
+ For example, if secret id is ``airflow/connections/smtp_default``, this would be accessible
+ if you provide ``{"connections_prefix": "airflow/connections"}`` and request conn_id ``smtp_default``.
+
+ :param connections_prefix: Specifies the prefix of the secret to read to get Connections.
+ :type connections_prefix: str
+ :param gcp_key_path: Path to GCP Credential JSON file;
+ use default credentials in the current environment if not provided.
+ :type gcp_key_path: str
+ :param gcp_scopes: Comma-separated string containing GCP scopes
+ :type gcp_scopes: str
+ """
+ def __init__(
+ self,
+ connections_prefix: str = "airflow/connections",
+ gcp_key_path: Optional[str] = None,
+ gcp_scopes: Optional[str] = None,
+ **kwargs
+ ):
+ self.connections_prefix = connections_prefix.rstrip("/")
+ self.gcp_key_path = gcp_key_path
+ self.gcp_scopes = gcp_scopes
+ self.credentials: Optional[str] = None
+ self.project_id: Optional[str] = None
+ super().__init__(**kwargs)
+
+ @cached_property
+ def client(self) -> SecretManagerServiceClient:
+ """
+ Create an authenticated KMS client
+ """
+ scopes = _get_scopes(self.gcp_scopes)
+ self.credentials, self.project_id = get_credentials_and_project_id(
+ key_path=self.gcp_key_path,
+ scopes=scopes
+ )
+ _client = SecretManagerServiceClient(
+ credentials=self.credentials,
+ client_info=ClientInfo(client_library_version='airflow_v' + version.version)
+ )
+ return _client
+
+ def build_secret_id(self, conn_id: str) -> str:
+ """
+ Given conn_id, build path for Secrets Manager
+
+ :param conn_id: connection id
+ :type conn_id: str
+ """
+ secret_id = self.connections_prefix + "/" + conn_id
+ return secret_id
+
+ def get_conn_uri(self, conn_id: str) -> Optional[str]:
+ """
+ Get secret value from Secrets Manager.
+
+ :param conn_id: connection id
+ :type conn_id: str
+ """
+ secret_id = self.build_secret_id(conn_id=conn_id)
+ # always return the latest version of the secret
+ secret_version = "latest"
+ name = self.client.secret_version_path(self.project_id, secret_id, secret_version)
+ try:
+ response = self.client.access_secret_version(name)
+ value = response.payload.data.decode('UTF-8')
+ return value
+ except NotFound:
+ self.log.info(
+ "GCP API Call Error (NotFoun): Secret ID %s not found.", secret_id
+ )
Review comment:
Good catch! `error` is more appropriate.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] xinbinhuang commented on a change in pull request #7795:
[AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
Posted by GitBox <gi...@apache.org>.
xinbinhuang commented on a change in pull request #7795: [AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
URL: https://github.com/apache/airflow/pull/7795#discussion_r396084064
##########
File path: airflow/providers/google/cloud/secrets/secrets_manager.py
##########
@@ -0,0 +1,132 @@
+# 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.
+
+"""
+Objects relating to sourcing connections from GCP Secrets Manager
+"""
+from typing import List, Optional
+
+from cached_property import cached_property
+from google.api_core.exceptions import NotFound
+from google.api_core.gapic_v1.client_info import ClientInfo
+from google.cloud.secretmanager_v1 import SecretManagerServiceClient
+
+from airflow import version
+from airflow.models import Connection
+from airflow.providers.google.cloud.utils.credentials_provider import (
+ _get_scopes, get_credentials_and_project_id,
+)
+from airflow.secrets import BaseSecretsBackend
+from airflow.utils.log.logging_mixin import LoggingMixin
+
+
+class GcpSecretsManagerSecretsBackend(BaseSecretsBackend, LoggingMixin):
+ """
+ Retrieves Connection object from GCP Secrets Manager
+
+ Configurable via ``airflow.cfg`` as follows:
+
+ .. code-block:: ini
+
+ [secrets]
+ backend = airflow.providers.google.cloud.secrets.secrets_manager.GcpSecretsManagerSecretsBackend
+ backend_kwargs = {"connections_prefix": "airflow/connections"}
+
+ For example, if secret id is ``airflow/connections/smtp_default``, this would be accessible
+ if you provide ``{"connections_prefix": "airflow/connections"}`` and request conn_id ``smtp_default``.
+
+ :param connections_prefix: Specifies the prefix of the secret to read to get Connections.
+ :type connections_prefix: str
+ :param gcp_key_path: Path to GCP Credential JSON file;
+ use default credentials in the current environment if not provided.
+ :type gcp_key_path: str
+ :param gcp_scopes: Comma-separated string containing GCP scopes
+ :type gcp_scopes: str
+ """
+ def __init__(
+ self,
+ connections_prefix: str = "airflow/connections",
+ gcp_key_path: Optional[str] = None,
+ gcp_scopes: Optional[str] = None,
+ **kwargs
+ ):
+ self.connections_prefix = connections_prefix.rstrip("/")
+ self.gcp_key_path = gcp_key_path
+ self.gcp_scopes = gcp_scopes
+ self.credentials: Optional[str] = None
+ self.project_id: Optional[str] = None
+ super().__init__(**kwargs)
+
+ @cached_property
+ def client(self) -> SecretManagerServiceClient:
+ """
+ Create an authenticated KMS client
+ """
+ scopes = _get_scopes(self.gcp_scopes)
+ self.credentials, self.project_id = get_credentials_and_project_id(
+ key_path=self.gcp_key_path,
+ scopes=scopes
+ )
+ _client = SecretManagerServiceClient(
+ credentials=self.credentials,
+ client_info=ClientInfo(client_library_version='airflow_v' + version.version)
+ )
+ return _client
+
+ def build_secret_id(self, conn_id: str) -> str:
+ """
+ Given conn_id, build path for Secrets Manager
+
+ :param conn_id: connection id
+ :type conn_id: str
+ """
+ secret_id = self.connections_prefix + "/" + conn_id
+ return secret_id
+
+ def get_conn_uri(self, conn_id: str) -> Optional[str]:
+ """
+ Get secret value from Secrets Manager.
+
+ :param conn_id: connection id
+ :type conn_id: str
+ """
+ secret_id = self.build_secret_id(conn_id=conn_id)
+ # always return the latest version of the secret
+ secret_version = "latest"
+ name = self.client.secret_version_path(self.project_id, secret_id, secret_version)
+ try:
+ response = self.client.access_secret_version(name)
+ value = response.payload.data.decode('UTF-8')
+ return value
+ except NotFound:
+ self.log.info(
+ "GCP API Call Error (NotFoun): Secret ID %s not found.", secret_id
+ )
Review comment:
Fixed as suggested
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #7795:
[AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #7795: [AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
URL: https://github.com/apache/airflow/pull/7795#issuecomment-602132463
# [Codecov](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=h1) Report
> Merging [#7795](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/fee827ed5a5d4aea2d2e60f73443357721a2c45a&el=desc) will **decrease** coverage by `22.16%`.
> The diff coverage is `97.78%`.
[](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #7795 +/- ##
===========================================
- Coverage 86.93% 64.76% -22.17%
===========================================
Files 924 924
Lines 44674 44823 +149
===========================================
- Hits 38836 29030 -9806
- Misses 5838 15793 +9955
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/providers/hashicorp/secrets/vault.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvaGFzaGljb3JwL3NlY3JldHMvdmF1bHQucHk=) | `76.47% <0.00%> (+11.03%)` | :arrow_up: |
| [...oviders/google/cloud/utils/credentials\_provider.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvZ29vZ2xlL2Nsb3VkL3V0aWxzL2NyZWRlbnRpYWxzX3Byb3ZpZGVyLnB5) | `93.58% <96.66%> (+1.75%)` | :arrow_up: |
| [airflow/bin/cli.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9iaW4vY2xpLnB5) | `96.72% <99.27%> (+1.93%)` | :arrow_up: |
| [airflow/providers/google/cloud/hooks/base.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvZ29vZ2xlL2Nsb3VkL2hvb2tzL2Jhc2UucHk=) | `97.02% <100.00%> (+1.34%)` | :arrow_up: |
| [.../providers/google/cloud/secrets/secrets\_manager.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvZ29vZ2xlL2Nsb3VkL3NlY3JldHMvc2VjcmV0c19tYW5hZ2VyLnB5) | `100.00% <100.00%> (ø)` | |
| [airflow/hooks/S3\_hook.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9ob29rcy9TM19ob29rLnB5) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [airflow/hooks/pig\_hook.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9ob29rcy9waWdfaG9vay5weQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [airflow/hooks/hdfs\_hook.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9ob29rcy9oZGZzX2hvb2sucHk=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [airflow/hooks/http\_hook.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9ob29rcy9odHRwX2hvb2sucHk=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [airflow/hooks/jdbc\_hook.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9ob29rcy9qZGJjX2hvb2sucHk=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [500 more](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=footer). Last update [fee827e...3a4b925](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] turbaszek commented on a change in pull request #7795:
[AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #7795: [AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
URL: https://github.com/apache/airflow/pull/7795#discussion_r396067167
##########
File path: airflow/providers/google/cloud/secrets/secrets_manager.py
##########
@@ -0,0 +1,132 @@
+# 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.
+
+"""
+Objects relating to sourcing connections from GCP Secrets Manager
+"""
+from typing import List, Optional
+
+from cached_property import cached_property
+from google.api_core.exceptions import NotFound
+from google.api_core.gapic_v1.client_info import ClientInfo
+from google.cloud.secretmanager_v1 import SecretManagerServiceClient
+
+from airflow import version
+from airflow.models import Connection
+from airflow.providers.google.cloud.utils.credentials_provider import (
+ _get_scopes, get_credentials_and_project_id,
+)
+from airflow.secrets import BaseSecretsBackend
+from airflow.utils.log.logging_mixin import LoggingMixin
+
+
+class GcpSecretsManagerSecretsBackend(BaseSecretsBackend, LoggingMixin):
+ """
+ Retrieves Connection object from GCP Secrets Manager
+
+ Configurable via ``airflow.cfg`` as follows:
+
+ .. code-block:: ini
+
+ [secrets]
+ backend = airflow.providers.google.cloud.secrets.secrets_manager.GcpSecretsManagerSecretsBackend
+ backend_kwargs = {"connections_prefix": "airflow/connections"}
+
+ For example, if secret id is ``airflow/connections/smtp_default``, this would be accessible
+ if you provide ``{"connections_prefix": "airflow/connections"}`` and request conn_id ``smtp_default``.
+
+ :param connections_prefix: Specifies the prefix of the secret to read to get Connections.
+ :type connections_prefix: str
+ :param gcp_key_path: Path to GCP Credential JSON file;
+ use default credentials in the current environment if not provided.
+ :type gcp_key_path: str
+ :param gcp_scopes: Comma-separated string containing GCP scopes
+ :type gcp_scopes: str
+ """
+ def __init__(
+ self,
+ connections_prefix: str = "airflow/connections",
+ gcp_key_path: Optional[str] = None,
+ gcp_scopes: Optional[str] = None,
+ **kwargs
+ ):
+ self.connections_prefix = connections_prefix.rstrip("/")
+ self.gcp_key_path = gcp_key_path
+ self.gcp_scopes = gcp_scopes
+ self.credentials: Optional[str] = None
+ self.project_id: Optional[str] = None
+ super().__init__(**kwargs)
+
+ @cached_property
+ def client(self) -> SecretManagerServiceClient:
+ """
+ Create an authenticated KMS client
+ """
+ scopes = _get_scopes(self.gcp_scopes)
+ self.credentials, self.project_id = get_credentials_and_project_id(
+ key_path=self.gcp_key_path,
+ scopes=scopes
+ )
+ _client = SecretManagerServiceClient(
+ credentials=self.credentials,
+ client_info=ClientInfo(client_library_version='airflow_v' + version.version)
+ )
+ return _client
+
+ def build_secret_id(self, conn_id: str) -> str:
+ """
+ Given conn_id, build path for Secrets Manager
+
+ :param conn_id: connection id
+ :type conn_id: str
+ """
+ secret_id = self.connections_prefix + "/" + conn_id
Review comment:
```suggestion
secret_id = f"{self.connections_prefix}/{conn_id}"
```
What do you think? :)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] xinbinhuang commented on issue #7795: [AIRFLOW-7104] Add
Secret backend for GCP Secrets Manager
Posted by GitBox <gi...@apache.org>.
xinbinhuang commented on issue #7795: [AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
URL: https://github.com/apache/airflow/pull/7795#issuecomment-602271652
Rebase with the master because of the `elasticsearch` breaking change. Travis should be green after the run.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] turbaszek commented on a change in pull request #7795:
[AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #7795: [AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
URL: https://github.com/apache/airflow/pull/7795#discussion_r396067459
##########
File path: airflow/providers/hashicorp/secrets/vault.py
##########
@@ -31,7 +31,7 @@
class VaultSecrets(BaseSecretsBackend, LoggingMixin):
- """
+ """ # noqa pylint: disable=line-too-long
Review comment:
Let's try to avoid `disable=line-too-long` :)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] xinbinhuang commented on a change in pull request #7795:
[AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
Posted by GitBox <gi...@apache.org>.
xinbinhuang commented on a change in pull request #7795: [AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
URL: https://github.com/apache/airflow/pull/7795#discussion_r396127945
##########
File path: airflow/providers/hashicorp/secrets/vault.py
##########
@@ -40,10 +40,10 @@ class VaultSecrets(BaseSecretsBackend, LoggingMixin):
[secrets]
backend = airflow.providers.hashicorp.secrets.vault.VaultSecrets
- backend_kwargs = {"path":"connections","url":"http://127.0.0.1:8200","mount_point":"airflow"}
+ backend_kwargs = {"connections_path":"conns","url":"http://127.0.0.1:8200","mount_point":"airflow"}
- For example, if your keys are under ``connections`` path in ``airflow`` mount_point, this
- would be accessible if you provide ``{"path": "connections"}`` and request
+ For example, if your keys are under ``conns`` path in ``airflow`` mount_point, this
+ would be accessible if you provide ``{"connections_path": "conns"}`` and request
conn_id ``smtp_default``.
Review comment:
Got it! I just updated the docstring with multiline values.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #7795:
[AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #7795: [AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
URL: https://github.com/apache/airflow/pull/7795#issuecomment-602132463
# [Codecov](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=h1) Report
> Merging [#7795](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/fee827ed5a5d4aea2d2e60f73443357721a2c45a&el=desc) will **decrease** coverage by `22.67%`.
> The diff coverage is `95.45%`.
[](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #7795 +/- ##
===========================================
- Coverage 86.93% 64.26% -22.68%
===========================================
Files 924 924
Lines 44674 44705 +31
===========================================
- Hits 38836 28728 -10108
- Misses 5838 15977 +10139
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/providers/hashicorp/secrets/vault.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvaGFzaGljb3JwL3NlY3JldHMvdmF1bHQucHk=) | `76.47% <0.00%> (+11.03%)` | :arrow_up: |
| [...oviders/google/cloud/utils/credentials\_provider.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvZ29vZ2xlL2Nsb3VkL3V0aWxzL2NyZWRlbnRpYWxzX3Byb3ZpZGVyLnB5) | `93.58% <96.66%> (+1.75%)` | :arrow_up: |
| [airflow/providers/google/cloud/hooks/base.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvZ29vZ2xlL2Nsb3VkL2hvb2tzL2Jhc2UucHk=) | `97.02% <100.00%> (+1.34%)` | :arrow_up: |
| [...irflow/providers/google/secrets/secrets\_manager.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvZ29vZ2xlL3NlY3JldHMvc2VjcmV0c19tYW5hZ2VyLnB5) | `100.00% <100.00%> (ø)` | |
| [airflow/hooks/S3\_hook.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9ob29rcy9TM19ob29rLnB5) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [airflow/hooks/pig\_hook.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9ob29rcy9waWdfaG9vay5weQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [airflow/hooks/hdfs\_hook.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9ob29rcy9oZGZzX2hvb2sucHk=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [airflow/hooks/http\_hook.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9ob29rcy9odHRwX2hvb2sucHk=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [airflow/hooks/jdbc\_hook.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9ob29rcy9qZGJjX2hvb2sucHk=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [airflow/hooks/druid\_hook.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9ob29rcy9kcnVpZF9ob29rLnB5) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [503 more](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=footer). Last update [fee827e...a04f148](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] mik-laj commented on a change in pull request #7795:
[AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #7795: [AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
URL: https://github.com/apache/airflow/pull/7795#discussion_r395985516
##########
File path: airflow/providers/google/cloud/utils/credentials_provider.py
##########
@@ -158,3 +161,43 @@ def provide_gcp_conn_and_credentials(
key_file_path, scopes, project_id
):
yield
+
+
+def get_credentials_and_project_id(
+ gcp_key_path: Optional[str] = None,
+ gcp_scopes: Optional[str] = None
+) -> google.auth.credentials.Credentials:
Review comment:
```suggestion
) -> Tuple[google.auth.credentials.Credentials, str]:
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] kaxil commented on a change in pull request #7795:
[AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #7795: [AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
URL: https://github.com/apache/airflow/pull/7795#discussion_r396046532
##########
File path: airflow/providers/google/secrets/secrets_manager.py
##########
@@ -0,0 +1,132 @@
+# 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.
+
+"""
+Objects relating to sourcing connections from GCP Secrets Manager
+"""
+from typing import List, Optional
+
+from cached_property import cached_property
+from google.api_core.exceptions import NotFound
+from google.api_core.gapic_v1.client_info import ClientInfo
+from google.cloud.secretmanager_v1 import SecretManagerServiceClient
+
+from airflow import version
+from airflow.models import Connection
+from airflow.providers.google.cloud.utils.credentials_provider import (
+ _get_scopes, get_credentials_and_project_id,
+)
+from airflow.secrets import BaseSecretsBackend
+from airflow.utils.log.logging_mixin import LoggingMixin
+
+
+class GcpSecretsManagerSecretsBackend(BaseSecretsBackend, LoggingMixin):
Review comment:
```suggestion
class GcpSecretManager(BaseSecretsBackend, LoggingMixin):
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io commented on issue #7795: [AIRFLOW-7104] Add
Secret backend for GCP Secrets Manager
Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #7795: [AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
URL: https://github.com/apache/airflow/pull/7795#issuecomment-602132463
# [Codecov](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=h1) Report
> Merging [#7795](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/fee827ed5a5d4aea2d2e60f73443357721a2c45a&el=desc) will **decrease** coverage by `54.91%`.
> The diff coverage is `30.68%`.
[](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #7795 +/- ##
===========================================
- Coverage 86.93% 32.01% -54.92%
===========================================
Files 924 924
Lines 44674 44705 +31
===========================================
- Hits 38836 14313 -24523
- Misses 5838 30392 +24554
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/providers/hashicorp/secrets/vault.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvaGFzaGljb3JwL3NlY3JldHMvdmF1bHQucHk=) | `23.52% <0.00%> (-41.91%)` | :arrow_down: |
| [airflow/providers/google/cloud/hooks/base.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvZ29vZ2xlL2Nsb3VkL2hvb2tzL2Jhc2UucHk=) | `47.02% <10.00%> (-48.66%)` | :arrow_down: |
| [...oviders/google/cloud/utils/credentials\_provider.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvZ29vZ2xlL2Nsb3VkL3V0aWxzL2NyZWRlbnRpYWxzX3Byb3ZpZGVyLnB5) | `30.76% <26.66%> (-61.07%)` | :arrow_down: |
| [...irflow/providers/google/secrets/secrets\_manager.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvZ29vZ2xlL3NlY3JldHMvc2VjcmV0c19tYW5hZ2VyLnB5) | `40.00% <40.00%> (ø)` | |
| [airflow/hooks/S3\_hook.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9ob29rcy9TM19ob29rLnB5) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [airflow/hooks/pig\_hook.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9ob29rcy9waWdfaG9vay5weQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [airflow/hooks/hdfs\_hook.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9ob29rcy9oZGZzX2hvb2sucHk=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [airflow/hooks/http\_hook.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9ob29rcy9odHRwX2hvb2sucHk=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [airflow/hooks/jdbc\_hook.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9ob29rcy9qZGJjX2hvb2sucHk=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [airflow/hooks/druid\_hook.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9ob29rcy9kcnVpZF9ob29rLnB5) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [799 more](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=footer). Last update [fee827e...a04f148](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] xinbinhuang commented on a change in pull request #7795:
[AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
Posted by GitBox <gi...@apache.org>.
xinbinhuang commented on a change in pull request #7795: [AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
URL: https://github.com/apache/airflow/pull/7795#discussion_r396043869
##########
File path: airflow/providers/google/cloud/utils/credentials_provider.py
##########
@@ -158,3 +161,43 @@ def provide_gcp_conn_and_credentials(
key_file_path, scopes, project_id
):
yield
+
+
+def get_credentials_and_project_id(
Review comment:
I refactor the code to move the whole logic in `base.py` into `credentials_provider.py`. Let me know how you think
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] turbaszek commented on a change in pull request #7795:
[AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #7795: [AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
URL: https://github.com/apache/airflow/pull/7795#discussion_r396086714
##########
File path: airflow/providers/hashicorp/secrets/vault.py
##########
@@ -40,10 +40,10 @@ class VaultSecrets(BaseSecretsBackend, LoggingMixin):
[secrets]
backend = airflow.providers.hashicorp.secrets.vault.VaultSecrets
- backend_kwargs = {"path":"connections","url":"http://127.0.0.1:8200","mount_point":"airflow"}
+ backend_kwargs = {"connections_path":"conns","url":"http://127.0.0.1:8200","mount_point":"airflow"}
- For example, if your keys are under ``connections`` path in ``airflow`` mount_point, this
- would be accessible if you provide ``{"path": "connections"}`` and request
+ For example, if your keys are under ``conns`` path in ``airflow`` mount_point, this
+ would be accessible if you provide ``{"connections_path": "conns"}`` and request
conn_id ``smtp_default``.
Review comment:
Can't we use `connections` and just break the lines?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] xinbinhuang commented on a change in pull request #7795:
[AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
Posted by GitBox <gi...@apache.org>.
xinbinhuang commented on a change in pull request #7795: [AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
URL: https://github.com/apache/airflow/pull/7795#discussion_r396084533
##########
File path: airflow/providers/hashicorp/secrets/vault.py
##########
@@ -40,10 +40,10 @@ class VaultSecrets(BaseSecretsBackend, LoggingMixin):
[secrets]
backend = airflow.providers.hashicorp.secrets.vault.VaultSecrets
- backend_kwargs = {"path":"connections","url":"http://127.0.0.1:8200","mount_point":"airflow"}
+ backend_kwargs = {"connections_path":"conns","url":"http://127.0.0.1:8200","mount_point":"airflow"}
- For example, if your keys are under ``connections`` path in ``airflow`` mount_point, this
- would be accessible if you provide ``{"path": "connections"}`` and request
+ For example, if your keys are under ``conns`` path in ``airflow`` mount_point, this
+ would be accessible if you provide ``{"connections_path": "conns"}`` and request
conn_id ``smtp_default``.
Review comment:
@turbaszek @kaxil I changed `connections` to `conns` in the docstring to reduce the line length < 110.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] xinbinhuang commented on a change in pull request #7795:
[AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
Posted by GitBox <gi...@apache.org>.
xinbinhuang commented on a change in pull request #7795: [AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
URL: https://github.com/apache/airflow/pull/7795#discussion_r396084081
##########
File path: airflow/providers/google/cloud/secrets/secrets_manager.py
##########
@@ -0,0 +1,132 @@
+# 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.
+
+"""
+Objects relating to sourcing connections from GCP Secrets Manager
+"""
+from typing import List, Optional
+
+from cached_property import cached_property
+from google.api_core.exceptions import NotFound
+from google.api_core.gapic_v1.client_info import ClientInfo
+from google.cloud.secretmanager_v1 import SecretManagerServiceClient
+
+from airflow import version
+from airflow.models import Connection
+from airflow.providers.google.cloud.utils.credentials_provider import (
+ _get_scopes, get_credentials_and_project_id,
+)
+from airflow.secrets import BaseSecretsBackend
+from airflow.utils.log.logging_mixin import LoggingMixin
+
+
+class GcpSecretsManagerSecretsBackend(BaseSecretsBackend, LoggingMixin):
Review comment:
Fixed as suggested
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] kaxil commented on a change in pull request #7795:
[AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #7795: [AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
URL: https://github.com/apache/airflow/pull/7795#discussion_r396046616
##########
File path: airflow/providers/hashicorp/secrets/vault.py
##########
@@ -40,10 +40,10 @@ class VaultSecrets(BaseSecretsBackend, LoggingMixin):
[secrets]
backend = airflow.providers.hashicorp.secrets.vault.VaultSecrets
- backend_kwargs = {"path":"connections","url":"http://127.0.0.1:8200","mount_point":"airflow"}
+ backend_kwargs = {"connections_path":"connections","url":"http://127.0.0.1:8200","mount_point":"airflow"}
Review comment:
Good catch
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] mik-laj commented on a change in pull request #7795:
[AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #7795: [AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
URL: https://github.com/apache/airflow/pull/7795#discussion_r396121521
##########
File path: airflow/providers/google/secrets/secrets_manager.py
##########
@@ -0,0 +1,125 @@
+# 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.
+
+"""
+Objects relating to sourcing connections from GCP Secrets Manager
+"""
+from typing import List, Optional
+
+from cached_property import cached_property
+from google.api_core.exceptions import NotFound
+from google.api_core.gapic_v1.client_info import ClientInfo
+from google.cloud.secretmanager_v1 import SecretManagerServiceClient
+
+from airflow import version
+from airflow.models import Connection
+from airflow.providers.google.cloud.utils.credentials_provider import get_credentials_and_project_id
+from airflow.secrets import BaseSecretsBackend
+from airflow.utils.log.logging_mixin import LoggingMixin
+
+
+class GcpSecretsManagerSecretsBackend(BaseSecretsBackend, LoggingMixin):
+ """
+ Retrieves Connection object from GCP Secrets Manager
+
+ Configurable via ``airflow.cfg`` as follows:
+
+ .. code-block:: ini
+
+ [secrets]
+ backend = airflow.providers.google.secrets.secrets_manager.GcpSecretsManagerSecretsBackend
+ backend_kwargs = {"connections_prefix": "airflow/connections"}
+
+ For example, if secret id is ``airflow/connections/smtp_default``, this would be accessible
+ if you provide ``{"connections_prefix": "airflow/connections"}`` and request conn_id ``smtp_default``.
+
+ :param connections_prefix: Specifies the prefix of the secret to read to get Connections.
+ :type connections_prefix: str
+ :param gcp_key_path: Path to GCP Credential JSON file;
+ use default credentials in the current environment if not provided.
+ :type gcp_key_path: str
+ :param gcp_scopes: Comma-separated string containing GCP scopes
+ :type gcp_scopes: str
+ """
+ def __init__(
+ self,
+ connections_prefix: str = "airflow/connections",
+ gcp_key_path: Optional[str] = None,
+ gcp_scopes: Optional[str] = None,
+ **kwargs
+ ):
+ self.connections_prefix = connections_prefix.rstrip("/")
+ self.credentials, self.project_id = get_credentials_and_project_id(
Review comment:
This is problematic because the backend is initialized/constructor during the initialization of the `airflow.secret` module.
If you write something similar to the code below
```python
from airflow.secret import CONFIG_SECTION
```
An HTTP request will be sent. You don't have to use this class for it to happen. It's enough to load it in memory.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] kaxil commented on a change in pull request #7795:
[AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #7795: [AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
URL: https://github.com/apache/airflow/pull/7795#discussion_r396048450
##########
File path: airflow/providers/hashicorp/secrets/vault.py
##########
@@ -40,10 +40,10 @@ class VaultSecrets(BaseSecretsBackend, LoggingMixin):
[secrets]
backend = airflow.providers.hashicorp.secrets.vault.VaultSecrets
- backend_kwargs = {"path":"connections","url":"http://127.0.0.1:8200","mount_point":"airflow"}
+ backend_kwargs = {"connections_path":"connections","url":"http://127.0.0.1:8200","mount_point":"airflow"}
Review comment:
Will re-review fully on Monday
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] mik-laj commented on a change in pull request #7795:
[AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #7795: [AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
URL: https://github.com/apache/airflow/pull/7795#discussion_r396121521
##########
File path: airflow/providers/google/secrets/secrets_manager.py
##########
@@ -0,0 +1,125 @@
+# 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.
+
+"""
+Objects relating to sourcing connections from GCP Secrets Manager
+"""
+from typing import List, Optional
+
+from cached_property import cached_property
+from google.api_core.exceptions import NotFound
+from google.api_core.gapic_v1.client_info import ClientInfo
+from google.cloud.secretmanager_v1 import SecretManagerServiceClient
+
+from airflow import version
+from airflow.models import Connection
+from airflow.providers.google.cloud.utils.credentials_provider import get_credentials_and_project_id
+from airflow.secrets import BaseSecretsBackend
+from airflow.utils.log.logging_mixin import LoggingMixin
+
+
+class GcpSecretsManagerSecretsBackend(BaseSecretsBackend, LoggingMixin):
+ """
+ Retrieves Connection object from GCP Secrets Manager
+
+ Configurable via ``airflow.cfg`` as follows:
+
+ .. code-block:: ini
+
+ [secrets]
+ backend = airflow.providers.google.secrets.secrets_manager.GcpSecretsManagerSecretsBackend
+ backend_kwargs = {"connections_prefix": "airflow/connections"}
+
+ For example, if secret id is ``airflow/connections/smtp_default``, this would be accessible
+ if you provide ``{"connections_prefix": "airflow/connections"}`` and request conn_id ``smtp_default``.
+
+ :param connections_prefix: Specifies the prefix of the secret to read to get Connections.
+ :type connections_prefix: str
+ :param gcp_key_path: Path to GCP Credential JSON file;
+ use default credentials in the current environment if not provided.
+ :type gcp_key_path: str
+ :param gcp_scopes: Comma-separated string containing GCP scopes
+ :type gcp_scopes: str
+ """
+ def __init__(
+ self,
+ connections_prefix: str = "airflow/connections",
+ gcp_key_path: Optional[str] = None,
+ gcp_scopes: Optional[str] = None,
+ **kwargs
+ ):
+ self.connections_prefix = connections_prefix.rstrip("/")
+ self.credentials, self.project_id = get_credentials_and_project_id(
Review comment:
This is problematic because the backend is initialized/constructor during the initialization of the `airflow.secret` module.
If you write something similar to the code below
```from airflow.secret import CONFIG_SECTION```
An HTTP request will be sent. You don't have to use this class for it to happen. It's enough to load it in memory.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] turbaszek commented on a change in pull request #7795:
[AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #7795: [AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
URL: https://github.com/apache/airflow/pull/7795#discussion_r396067245
##########
File path: airflow/providers/google/cloud/secrets/secrets_manager.py
##########
@@ -0,0 +1,132 @@
+# 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.
+
+"""
+Objects relating to sourcing connections from GCP Secrets Manager
+"""
+from typing import List, Optional
+
+from cached_property import cached_property
+from google.api_core.exceptions import NotFound
+from google.api_core.gapic_v1.client_info import ClientInfo
+from google.cloud.secretmanager_v1 import SecretManagerServiceClient
+
+from airflow import version
+from airflow.models import Connection
+from airflow.providers.google.cloud.utils.credentials_provider import (
+ _get_scopes, get_credentials_and_project_id,
+)
+from airflow.secrets import BaseSecretsBackend
+from airflow.utils.log.logging_mixin import LoggingMixin
+
+
+class GcpSecretsManagerSecretsBackend(BaseSecretsBackend, LoggingMixin):
+ """
+ Retrieves Connection object from GCP Secrets Manager
+
+ Configurable via ``airflow.cfg`` as follows:
+
+ .. code-block:: ini
+
+ [secrets]
+ backend = airflow.providers.google.cloud.secrets.secrets_manager.GcpSecretsManagerSecretsBackend
+ backend_kwargs = {"connections_prefix": "airflow/connections"}
+
+ For example, if secret id is ``airflow/connections/smtp_default``, this would be accessible
+ if you provide ``{"connections_prefix": "airflow/connections"}`` and request conn_id ``smtp_default``.
+
+ :param connections_prefix: Specifies the prefix of the secret to read to get Connections.
+ :type connections_prefix: str
+ :param gcp_key_path: Path to GCP Credential JSON file;
+ use default credentials in the current environment if not provided.
+ :type gcp_key_path: str
+ :param gcp_scopes: Comma-separated string containing GCP scopes
+ :type gcp_scopes: str
+ """
+ def __init__(
+ self,
+ connections_prefix: str = "airflow/connections",
+ gcp_key_path: Optional[str] = None,
+ gcp_scopes: Optional[str] = None,
+ **kwargs
+ ):
+ self.connections_prefix = connections_prefix.rstrip("/")
+ self.gcp_key_path = gcp_key_path
+ self.gcp_scopes = gcp_scopes
+ self.credentials: Optional[str] = None
+ self.project_id: Optional[str] = None
+ super().__init__(**kwargs)
+
+ @cached_property
+ def client(self) -> SecretManagerServiceClient:
+ """
+ Create an authenticated KMS client
+ """
+ scopes = _get_scopes(self.gcp_scopes)
+ self.credentials, self.project_id = get_credentials_and_project_id(
+ key_path=self.gcp_key_path,
+ scopes=scopes
+ )
+ _client = SecretManagerServiceClient(
+ credentials=self.credentials,
+ client_info=ClientInfo(client_library_version='airflow_v' + version.version)
+ )
+ return _client
+
+ def build_secret_id(self, conn_id: str) -> str:
+ """
+ Given conn_id, build path for Secrets Manager
+
+ :param conn_id: connection id
+ :type conn_id: str
+ """
+ secret_id = self.connections_prefix + "/" + conn_id
+ return secret_id
+
+ def get_conn_uri(self, conn_id: str) -> Optional[str]:
+ """
+ Get secret value from Secrets Manager.
+
+ :param conn_id: connection id
+ :type conn_id: str
+ """
+ secret_id = self.build_secret_id(conn_id=conn_id)
+ # always return the latest version of the secret
+ secret_version = "latest"
+ name = self.client.secret_version_path(self.project_id, secret_id, secret_version)
+ try:
+ response = self.client.access_secret_version(name)
+ value = response.payload.data.decode('UTF-8')
+ return value
+ except NotFound:
+ self.log.info(
+ "GCP API Call Error (NotFoun): Secret ID %s not found.", secret_id
+ )
Review comment:
Should we use `warning` or `error` instead of `info`?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] xinbinhuang commented on a change in pull request #7795:
[AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
Posted by GitBox <gi...@apache.org>.
xinbinhuang commented on a change in pull request #7795: [AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
URL: https://github.com/apache/airflow/pull/7795#discussion_r396049768
##########
File path: airflow/providers/google/secrets/secrets_manager.py
##########
@@ -0,0 +1,125 @@
+# 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.
+
+"""
+Objects relating to sourcing connections from GCP Secrets Manager
+"""
+from typing import List, Optional
+
+from cached_property import cached_property
+from google.api_core.exceptions import NotFound
+from google.api_core.gapic_v1.client_info import ClientInfo
+from google.cloud.secretmanager_v1 import SecretManagerServiceClient
+
+from airflow import version
+from airflow.models import Connection
+from airflow.providers.google.cloud.utils.credentials_provider import get_credentials_and_project_id
+from airflow.secrets import BaseSecretsBackend
+from airflow.utils.log.logging_mixin import LoggingMixin
+
+
+class GcpSecretsManagerSecretsBackend(BaseSecretsBackend, LoggingMixin):
+ """
+ Retrieves Connection object from GCP Secrets Manager
+
+ Configurable via ``airflow.cfg`` as follows:
+
+ .. code-block:: ini
+
+ [secrets]
+ backend = airflow.providers.google.secrets.secrets_manager.GcpSecretsManagerSecretsBackend
+ backend_kwargs = {"connections_prefix": "airflow/connections"}
+
+ For example, if secret id is ``airflow/connections/smtp_default``, this would be accessible
+ if you provide ``{"connections_prefix": "airflow/connections"}`` and request conn_id ``smtp_default``.
+
+ :param connections_prefix: Specifies the prefix of the secret to read to get Connections.
+ :type connections_prefix: str
+ :param gcp_key_path: Path to GCP Credential JSON file;
+ use default credentials in the current environment if not provided.
+ :type gcp_key_path: str
+ :param gcp_scopes: Comma-separated string containing GCP scopes
+ :type gcp_scopes: str
+ """
+ def __init__(
+ self,
+ connections_prefix: str = "airflow/connections",
+ gcp_key_path: Optional[str] = None,
+ gcp_scopes: Optional[str] = None,
+ **kwargs
+ ):
+ self.connections_prefix = connections_prefix.rstrip("/")
+ self.credentials, self.project_id = get_credentials_and_project_id(
Review comment:
A quick question: I understand that `get_credentials_and_project_id` can be expensive. But I don't understand the impact of putting it in the constructor vs. in other methods.
Doesn't we always call the client method after constructing an instance? Otherwise, what would be the reason for constructing the instance without calling the method?
Another confusion is that if we put it in the constructor, we only need to call the method once during instantiation, and then we can keep calling other methods without invoking the HTTP request. Wouldn't it be better than making this HTTP request every time when we call the method that contains it?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] kaxil commented on a change in pull request #7795:
[AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #7795: [AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
URL: https://github.com/apache/airflow/pull/7795#discussion_r396122238
##########
File path: airflow/providers/hashicorp/secrets/vault.py
##########
@@ -40,10 +40,10 @@ class VaultSecrets(BaseSecretsBackend, LoggingMixin):
[secrets]
backend = airflow.providers.hashicorp.secrets.vault.VaultSecrets
- backend_kwargs = {"path":"connections","url":"http://127.0.0.1:8200","mount_point":"airflow"}
+ backend_kwargs = {"connections_path":"conns","url":"http://127.0.0.1:8200","mount_point":"airflow"}
- For example, if your keys are under ``connections`` path in ``airflow`` mount_point, this
- would be accessible if you provide ``{"path": "connections"}`` and request
+ For example, if your keys are under ``conns`` path in ``airflow`` mount_point, this
+ would be accessible if you provide ``{"connections_path": "conns"}`` and request
conn_id ``smtp_default``.
Review comment:
Multiline config are handled fine as long as they are indented.
Example:
```ini
[Multiline Values]
chorus: I'm a lumberjack, and I'm okay
I sleep all night and I work all-day
```
I am also fine for you to disable long-line check for this particular case so it is easy for users to copy/paste.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] mik-laj commented on a change in pull request #7795:
[AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #7795: [AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
URL: https://github.com/apache/airflow/pull/7795#discussion_r396006374
##########
File path: airflow/providers/google/secrets/secrets_manager.py
##########
@@ -0,0 +1,125 @@
+# 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.
+
+"""
+Objects relating to sourcing connections from GCP Secrets Manager
+"""
+from typing import List, Optional
+
+from cached_property import cached_property
+from google.api_core.exceptions import NotFound
+from google.api_core.gapic_v1.client_info import ClientInfo
+from google.cloud.secretmanager_v1 import SecretManagerServiceClient
+
+from airflow import version
+from airflow.models import Connection
+from airflow.providers.google.cloud.utils.credentials_provider import get_credentials_and_project_id
+from airflow.secrets import BaseSecretsBackend
+from airflow.utils.log.logging_mixin import LoggingMixin
+
+
+class GcpSecretsManagerSecretsBackend(BaseSecretsBackend, LoggingMixin):
+ """
+ Retrieves Connection object from GCP Secrets Manager
+
+ Configurable via ``airflow.cfg`` as follows:
+
+ .. code-block:: ini
+
+ [secrets]
+ backend = airflow.providers.google.secrets.secrets_manager.GcpSecretsManagerSecretsBackend
+ backend_kwargs = {"connections_prefix": "airflow/connections"}
+
+ For example, if secret id is ``airflow/connections/smtp_default``, this would be accessible
+ if you provide ``{"connections_prefix": "airflow/connections"}`` and request conn_id ``smtp_default``.
+
+ :param connections_prefix: Specifies the prefix of the secret to read to get Connections.
+ :type connections_prefix: str
+ :param gcp_key_path: Path to GCP Credential JSON file;
+ use default credentials in the current environment if not provided.
+ :type gcp_key_path: str
+ :param gcp_scopes: Comma-separated string containing GCP scopes
+ :type gcp_scopes: str
+ """
+ def __init__(
+ self,
+ connections_prefix: str = "airflow/connections",
+ gcp_key_path: Optional[str] = None,
+ gcp_scopes: Optional[str] = None,
+ **kwargs
+ ):
+ self.connections_prefix = connections_prefix.rstrip("/")
+ self.credentials, self.project_id = get_credentials_and_project_id(
Review comment:
The get_credentials_and_project_id method is an expensive operation. This causes an HTTP request/requests to be sent. Can you move it out of the constructor?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] xinbinhuang commented on a change in pull request #7795:
[AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
Posted by GitBox <gi...@apache.org>.
xinbinhuang commented on a change in pull request #7795: [AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
URL: https://github.com/apache/airflow/pull/7795#discussion_r396121505
##########
File path: airflow/providers/hashicorp/secrets/vault.py
##########
@@ -40,10 +40,10 @@ class VaultSecrets(BaseSecretsBackend, LoggingMixin):
[secrets]
backend = airflow.providers.hashicorp.secrets.vault.VaultSecrets
- backend_kwargs = {"path":"connections","url":"http://127.0.0.1:8200","mount_point":"airflow"}
+ backend_kwargs = {"connections_path":"conns","url":"http://127.0.0.1:8200","mount_point":"airflow"}
- For example, if your keys are under ``connections`` path in ``airflow`` mount_point, this
- would be accessible if you provide ``{"path": "connections"}`` and request
+ For example, if your keys are under ``conns`` path in ``airflow`` mount_point, this
+ would be accessible if you provide ``{"connections_path": "conns"}`` and request
conn_id ``smtp_default``.
Review comment:
May be I should have asked this before. For some reason, I was under the impression that we can not break lines in the .cfg file. So can we break lines?
Something like this.
```
backend_kwargs =
{"connections_path":"connections","url":"http://127.0.0.1:8200","mount_point":"airflow"}
# or
backend_kwargs = {"connections_path":"connections",
"url":"http://127.0.0.1:8200",
"mount_point":"airflow"}
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #7795:
[AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #7795: [AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
URL: https://github.com/apache/airflow/pull/7795#issuecomment-602132463
# [Codecov](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=h1) Report
> Merging [#7795](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/fee827ed5a5d4aea2d2e60f73443357721a2c45a&el=desc) will **decrease** coverage by `0.72%`.
> The diff coverage is `95.50%`.
[](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #7795 +/- ##
==========================================
- Coverage 86.93% 86.20% -0.73%
==========================================
Files 924 925 +1
Lines 44674 44836 +162
==========================================
- Hits 38836 38652 -184
- Misses 5838 6184 +346
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/providers/hashicorp/secrets/vault.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvaGFzaGljb3JwL3NlY3JldHMvdmF1bHQucHk=) | `76.47% <0.00%> (+11.03%)` | :arrow_up: |
| [...oviders/google/cloud/utils/credentials\_provider.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvZ29vZ2xlL2Nsb3VkL3V0aWxzL2NyZWRlbnRpYWxzX3Byb3ZpZGVyLnB5) | `93.58% <96.66%> (+1.75%)` | :arrow_up: |
| [airflow/providers/google/cloud/hooks/base.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvZ29vZ2xlL2Nsb3VkL2hvb2tzL2Jhc2UucHk=) | `97.02% <100.00%> (+1.34%)` | :arrow_up: |
| [.../providers/google/cloud/secrets/secrets\_manager.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvZ29vZ2xlL2Nsb3VkL3NlY3JldHMvc2VjcmV0c19tYW5hZ2VyLnB5) | `100.00% <100.00%> (ø)` | |
| [...flow/providers/apache/cassandra/hooks/cassandra.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYXBhY2hlL2Nhc3NhbmRyYS9ob29rcy9jYXNzYW5kcmEucHk=) | `21.51% <0.00%> (-72.16%)` | :arrow_down: |
| [...w/providers/apache/hive/operators/mysql\_to\_hive.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYXBhY2hlL2hpdmUvb3BlcmF0b3JzL215c3FsX3RvX2hpdmUucHk=) | `35.84% <0.00%> (-64.16%)` | :arrow_down: |
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0.00%> (-55.56%)` | :arrow_down: |
| [airflow/providers/postgres/operators/postgres.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvcG9zdGdyZXMvb3BlcmF0b3JzL3Bvc3RncmVzLnB5) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
| [airflow/providers/redis/operators/redis\_publish.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvcmVkaXMvb3BlcmF0b3JzL3JlZGlzX3B1Ymxpc2gucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
| [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0.00%> (-47.06%)` | :arrow_down: |
| ... and [18 more](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=footer). Last update [fee827e...3a4b925](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] xinbinhuang commented on a change in pull request #7795:
[AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
Posted by GitBox <gi...@apache.org>.
xinbinhuang commented on a change in pull request #7795: [AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
URL: https://github.com/apache/airflow/pull/7795#discussion_r396043944
##########
File path: airflow/providers/google/secrets/secrets_manager.py
##########
@@ -0,0 +1,125 @@
+# 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.
+
+"""
+Objects relating to sourcing connections from GCP Secrets Manager
+"""
+from typing import List, Optional
+
+from cached_property import cached_property
+from google.api_core.exceptions import NotFound
+from google.api_core.gapic_v1.client_info import ClientInfo
+from google.cloud.secretmanager_v1 import SecretManagerServiceClient
+
+from airflow import version
+from airflow.models import Connection
+from airflow.providers.google.cloud.utils.credentials_provider import get_credentials_and_project_id
+from airflow.secrets import BaseSecretsBackend
+from airflow.utils.log.logging_mixin import LoggingMixin
+
+
+class GcpSecretsManagerSecretsBackend(BaseSecretsBackend, LoggingMixin):
+ """
+ Retrieves Connection object from GCP Secrets Manager
+
+ Configurable via ``airflow.cfg`` as follows:
+
+ .. code-block:: ini
+
+ [secrets]
+ backend = airflow.providers.google.secrets.secrets_manager.GcpSecretsManagerSecretsBackend
+ backend_kwargs = {"connections_prefix": "airflow/connections"}
+
+ For example, if secret id is ``airflow/connections/smtp_default``, this would be accessible
+ if you provide ``{"connections_prefix": "airflow/connections"}`` and request conn_id ``smtp_default``.
+
+ :param connections_prefix: Specifies the prefix of the secret to read to get Connections.
+ :type connections_prefix: str
+ :param gcp_key_path: Path to GCP Credential JSON file;
+ use default credentials in the current environment if not provided.
+ :type gcp_key_path: str
+ :param gcp_scopes: Comma-separated string containing GCP scopes
+ :type gcp_scopes: str
+ """
+ def __init__(
+ self,
+ connections_prefix: str = "airflow/connections",
+ gcp_key_path: Optional[str] = None,
+ gcp_scopes: Optional[str] = None,
+ **kwargs
+ ):
+ self.connections_prefix = connections_prefix.rstrip("/")
+ self.credentials, self.project_id = get_credentials_and_project_id(
Review comment:
I moved it inside the client now:
```python
@cached_property
def client(self) -> SecretManagerServiceClient:
"""
Create an authenticated KMS client
"""
scopes = _get_scopes(self.gcp_scopes)
self.credentials, self.project_id = get_credentials_and_project_id(
key_path=self.gcp_key_path,
scopes=scopes
)
_client = SecretManagerServiceClient(
credentials=self.credentials,
client_info=ClientInfo(client_library_version='airflow_v' + version.version)
)
return _client
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] kaxil commented on a change in pull request #7795:
[AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #7795: [AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
URL: https://github.com/apache/airflow/pull/7795#discussion_r396103880
##########
File path: airflow/providers/hashicorp/secrets/vault.py
##########
@@ -40,10 +40,10 @@ class VaultSecrets(BaseSecretsBackend, LoggingMixin):
[secrets]
backend = airflow.providers.hashicorp.secrets.vault.VaultSecrets
- backend_kwargs = {"path":"connections","url":"http://127.0.0.1:8200","mount_point":"airflow"}
+ backend_kwargs = {"connections_path":"conns","url":"http://127.0.0.1:8200","mount_point":"airflow"}
- For example, if your keys are under ``connections`` path in ``airflow`` mount_point, this
- would be accessible if you provide ``{"path": "connections"}`` and request
+ For example, if your keys are under ``conns`` path in ``airflow`` mount_point, this
+ would be accessible if you provide ``{"connections_path": "conns"}`` and request
conn_id ``smtp_default``.
Review comment:
I would prefer `connections` too, please
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] xinbinhuang commented on a change in pull request #7795:
[AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
Posted by GitBox <gi...@apache.org>.
xinbinhuang commented on a change in pull request #7795: [AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
URL: https://github.com/apache/airflow/pull/7795#discussion_r396084203
##########
File path: airflow/providers/hashicorp/secrets/vault.py
##########
@@ -31,7 +31,7 @@
class VaultSecrets(BaseSecretsBackend, LoggingMixin):
- """
+ """ # noqa pylint: disable=line-too-long
Review comment:
Fixed as suggested
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] turbaszek commented on a change in pull request #7795:
[AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #7795: [AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
URL: https://github.com/apache/airflow/pull/7795#discussion_r396067023
##########
File path: airflow/providers/google/cloud/hooks/base.py
##########
@@ -168,54 +169,20 @@ def _get_credentials_and_project_id(self) -> Tuple[google.auth.credentials.Crede
return self._cached_credentials, self._cached_project_id
key_path = self._get_field('key_path', None) # type: Optional[str]
- keyfile_dict = self._get_field('keyfile_dict', None) # type: Optional[str]
- if key_path and keyfile_dict:
- raise AirflowException(
- "The `keyfile_dict` and `key_path` fields are mutually exclusive. "
- "Please provide only one value."
- )
- if not key_path and not keyfile_dict:
- self.log.info(
- 'Getting connection using `google.auth.default()` since no key file is defined for hook.'
- )
- credentials, project_id = google.auth.default(scopes=self.scopes)
- elif key_path:
- # Get credentials from a JSON file.
- if key_path.endswith('.json'):
- self.log.debug('Getting connection using JSON key file %s', key_path)
- credentials = (
- google.oauth2.service_account.Credentials.from_service_account_file(
- key_path, scopes=self.scopes)
- )
- project_id = credentials.project_id
- elif key_path.endswith('.p12'):
- raise AirflowException(
- 'Legacy P12 key file are not supported, use a JSON key file.'
- )
- else:
- raise AirflowException('Unrecognised extension for key file.')
- else:
- # Get credentials from JSON data provided in the UI.
- try:
- if not keyfile_dict:
- raise ValueError("The keyfile_dict should be set")
- keyfile_dict_json: Dict[str, str] = json.loads(keyfile_dict)
-
- # Depending on how the JSON was formatted, it may contain
- # escaped newlines. Convert those to actual newlines.
- keyfile_dict_json['private_key'] = keyfile_dict_json['private_key'].replace(
- '\\n', '\n')
-
- credentials = (
- google.oauth2.service_account.Credentials.from_service_account_info(
- keyfile_dict_json, scopes=self.scopes)
- )
- project_id = credentials.project_id
- except json.decoder.JSONDecodeError:
- raise AirflowException('Invalid key JSON.')
-
- if self.delegate_to:
- credentials = credentials.with_subject(self.delegate_to)
+ try:
+ keyfile_dict = self._get_field('keyfile_dict', None) # type: Optional[str]
Review comment:
```suggestion
keyfile_dict: Optional[str] = self._get_field('keyfile_dict', None)
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] xinbinhuang commented on a change in pull request #7795:
[AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
Posted by GitBox <gi...@apache.org>.
xinbinhuang commented on a change in pull request #7795: [AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
URL: https://github.com/apache/airflow/pull/7795#discussion_r396049768
##########
File path: airflow/providers/google/secrets/secrets_manager.py
##########
@@ -0,0 +1,125 @@
+# 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.
+
+"""
+Objects relating to sourcing connections from GCP Secrets Manager
+"""
+from typing import List, Optional
+
+from cached_property import cached_property
+from google.api_core.exceptions import NotFound
+from google.api_core.gapic_v1.client_info import ClientInfo
+from google.cloud.secretmanager_v1 import SecretManagerServiceClient
+
+from airflow import version
+from airflow.models import Connection
+from airflow.providers.google.cloud.utils.credentials_provider import get_credentials_and_project_id
+from airflow.secrets import BaseSecretsBackend
+from airflow.utils.log.logging_mixin import LoggingMixin
+
+
+class GcpSecretsManagerSecretsBackend(BaseSecretsBackend, LoggingMixin):
+ """
+ Retrieves Connection object from GCP Secrets Manager
+
+ Configurable via ``airflow.cfg`` as follows:
+
+ .. code-block:: ini
+
+ [secrets]
+ backend = airflow.providers.google.secrets.secrets_manager.GcpSecretsManagerSecretsBackend
+ backend_kwargs = {"connections_prefix": "airflow/connections"}
+
+ For example, if secret id is ``airflow/connections/smtp_default``, this would be accessible
+ if you provide ``{"connections_prefix": "airflow/connections"}`` and request conn_id ``smtp_default``.
+
+ :param connections_prefix: Specifies the prefix of the secret to read to get Connections.
+ :type connections_prefix: str
+ :param gcp_key_path: Path to GCP Credential JSON file;
+ use default credentials in the current environment if not provided.
+ :type gcp_key_path: str
+ :param gcp_scopes: Comma-separated string containing GCP scopes
+ :type gcp_scopes: str
+ """
+ def __init__(
+ self,
+ connections_prefix: str = "airflow/connections",
+ gcp_key_path: Optional[str] = None,
+ gcp_scopes: Optional[str] = None,
+ **kwargs
+ ):
+ self.connections_prefix = connections_prefix.rstrip("/")
+ self.credentials, self.project_id = get_credentials_and_project_id(
Review comment:
A quick question: I understand that `get_credentials_and_project_id` can be expensive, and it seems to be a good practice within the project. But I don't understand the impact of putting it in the constructor vs. in other methods.
Doesn't we always call the client method after constructing an instance? Otherwise, what would be the reason for constructing the instance without calling the method?
Another confusion is that if we put it in the constructor, we only need to call the method once during instantiation, and then we can keep calling other methods without invoking the HTTP request. Wouldn't it be better than making this HTTP request every time when we call the method that contains it?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] mik-laj commented on a change in pull request #7795:
[AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #7795: [AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
URL: https://github.com/apache/airflow/pull/7795#discussion_r396122346
##########
File path: airflow/providers/google/secrets/secrets_manager.py
##########
@@ -0,0 +1,125 @@
+# 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.
+
+"""
+Objects relating to sourcing connections from GCP Secrets Manager
+"""
+from typing import List, Optional
+
+from cached_property import cached_property
+from google.api_core.exceptions import NotFound
+from google.api_core.gapic_v1.client_info import ClientInfo
+from google.cloud.secretmanager_v1 import SecretManagerServiceClient
+
+from airflow import version
+from airflow.models import Connection
+from airflow.providers.google.cloud.utils.credentials_provider import get_credentials_and_project_id
+from airflow.secrets import BaseSecretsBackend
+from airflow.utils.log.logging_mixin import LoggingMixin
+
+
+class GcpSecretsManagerSecretsBackend(BaseSecretsBackend, LoggingMixin):
+ """
+ Retrieves Connection object from GCP Secrets Manager
+
+ Configurable via ``airflow.cfg`` as follows:
+
+ .. code-block:: ini
+
+ [secrets]
+ backend = airflow.providers.google.secrets.secrets_manager.GcpSecretsManagerSecretsBackend
+ backend_kwargs = {"connections_prefix": "airflow/connections"}
+
+ For example, if secret id is ``airflow/connections/smtp_default``, this would be accessible
+ if you provide ``{"connections_prefix": "airflow/connections"}`` and request conn_id ``smtp_default``.
+
+ :param connections_prefix: Specifies the prefix of the secret to read to get Connections.
+ :type connections_prefix: str
+ :param gcp_key_path: Path to GCP Credential JSON file;
+ use default credentials in the current environment if not provided.
+ :type gcp_key_path: str
+ :param gcp_scopes: Comma-separated string containing GCP scopes
+ :type gcp_scopes: str
+ """
+ def __init__(
+ self,
+ connections_prefix: str = "airflow/connections",
+ gcp_key_path: Optional[str] = None,
+ gcp_scopes: Optional[str] = None,
+ **kwargs
+ ):
+ self.connections_prefix = connections_prefix.rstrip("/")
+ self.credentials, self.project_id = get_credentials_and_project_id(
Review comment:
I looked more closely. Even the following code is enough to initiate the backend.
```
from airflow.models.base_hook import BaseHook
```
So it is done very often.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] xinbinhuang commented on a change in pull request #7795:
[AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
Posted by GitBox <gi...@apache.org>.
xinbinhuang commented on a change in pull request #7795: [AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
URL: https://github.com/apache/airflow/pull/7795#discussion_r396084024
##########
File path: airflow/providers/google/cloud/secrets/secrets_manager.py
##########
@@ -0,0 +1,132 @@
+# 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.
+
+"""
+Objects relating to sourcing connections from GCP Secrets Manager
+"""
+from typing import List, Optional
+
+from cached_property import cached_property
+from google.api_core.exceptions import NotFound
+from google.api_core.gapic_v1.client_info import ClientInfo
+from google.cloud.secretmanager_v1 import SecretManagerServiceClient
+
+from airflow import version
+from airflow.models import Connection
+from airflow.providers.google.cloud.utils.credentials_provider import (
+ _get_scopes, get_credentials_and_project_id,
+)
+from airflow.secrets import BaseSecretsBackend
+from airflow.utils.log.logging_mixin import LoggingMixin
+
+
+class GcpSecretsManagerSecretsBackend(BaseSecretsBackend, LoggingMixin):
+ """
+ Retrieves Connection object from GCP Secrets Manager
+
+ Configurable via ``airflow.cfg`` as follows:
+
+ .. code-block:: ini
+
+ [secrets]
+ backend = airflow.providers.google.cloud.secrets.secrets_manager.GcpSecretsManagerSecretsBackend
+ backend_kwargs = {"connections_prefix": "airflow/connections"}
+
+ For example, if secret id is ``airflow/connections/smtp_default``, this would be accessible
+ if you provide ``{"connections_prefix": "airflow/connections"}`` and request conn_id ``smtp_default``.
+
+ :param connections_prefix: Specifies the prefix of the secret to read to get Connections.
+ :type connections_prefix: str
+ :param gcp_key_path: Path to GCP Credential JSON file;
+ use default credentials in the current environment if not provided.
+ :type gcp_key_path: str
+ :param gcp_scopes: Comma-separated string containing GCP scopes
+ :type gcp_scopes: str
+ """
+ def __init__(
+ self,
+ connections_prefix: str = "airflow/connections",
+ gcp_key_path: Optional[str] = None,
+ gcp_scopes: Optional[str] = None,
+ **kwargs
+ ):
+ self.connections_prefix = connections_prefix.rstrip("/")
+ self.gcp_key_path = gcp_key_path
+ self.gcp_scopes = gcp_scopes
+ self.credentials: Optional[str] = None
+ self.project_id: Optional[str] = None
+ super().__init__(**kwargs)
+
+ @cached_property
+ def client(self) -> SecretManagerServiceClient:
+ """
+ Create an authenticated KMS client
+ """
+ scopes = _get_scopes(self.gcp_scopes)
+ self.credentials, self.project_id = get_credentials_and_project_id(
+ key_path=self.gcp_key_path,
+ scopes=scopes
+ )
+ _client = SecretManagerServiceClient(
+ credentials=self.credentials,
+ client_info=ClientInfo(client_library_version='airflow_v' + version.version)
+ )
+ return _client
+
+ def build_secret_id(self, conn_id: str) -> str:
+ """
+ Given conn_id, build path for Secrets Manager
+
+ :param conn_id: connection id
+ :type conn_id: str
+ """
+ secret_id = self.connections_prefix + "/" + conn_id
Review comment:
Fixed as suggested
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] turbaszek commented on a change in pull request #7795:
[AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #7795: [AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
URL: https://github.com/apache/airflow/pull/7795#discussion_r396067305
##########
File path: airflow/providers/google/cloud/utils/credentials_provider.py
##########
@@ -20,17 +20,23 @@
Google Cloud Platform authentication.
"""
import json
+import logging
import tempfile
from contextlib import contextmanager
-from typing import Dict, Optional, Sequence
+from typing import Dict, Optional, Sequence, Tuple
from urllib.parse import urlencode
+import google.auth
+import google.oauth2.service_account
from google.auth.environment_vars import CREDENTIALS
from airflow.exceptions import AirflowException
from airflow.utils.process_utils import patch_environ
+log = logging.getLogger(__name__)
+
AIRFLOW_CONN_GOOGLE_CLOUD_DEFAULT = "AIRFLOW_CONN_GOOGLE_CLOUD_DEFAULT"
+_DEFAULT_SCOPES = ('https://www.googleapis.com/auth/cloud-platform',) # type: Sequence[str]
Review comment:
```suggestion
_DEFAULT_SCOPES: Sequence[str] = ('https://www.googleapis.com/auth/cloud-platform',)
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] xinbinhuang commented on a change in pull request #7795:
[AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
Posted by GitBox <gi...@apache.org>.
xinbinhuang commented on a change in pull request #7795: [AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
URL: https://github.com/apache/airflow/pull/7795#discussion_r396084031
##########
File path: airflow/providers/google/cloud/hooks/base.py
##########
@@ -168,54 +169,20 @@ def _get_credentials_and_project_id(self) -> Tuple[google.auth.credentials.Crede
return self._cached_credentials, self._cached_project_id
key_path = self._get_field('key_path', None) # type: Optional[str]
- keyfile_dict = self._get_field('keyfile_dict', None) # type: Optional[str]
- if key_path and keyfile_dict:
- raise AirflowException(
- "The `keyfile_dict` and `key_path` fields are mutually exclusive. "
- "Please provide only one value."
- )
- if not key_path and not keyfile_dict:
- self.log.info(
- 'Getting connection using `google.auth.default()` since no key file is defined for hook.'
- )
- credentials, project_id = google.auth.default(scopes=self.scopes)
- elif key_path:
- # Get credentials from a JSON file.
- if key_path.endswith('.json'):
- self.log.debug('Getting connection using JSON key file %s', key_path)
- credentials = (
- google.oauth2.service_account.Credentials.from_service_account_file(
- key_path, scopes=self.scopes)
- )
- project_id = credentials.project_id
- elif key_path.endswith('.p12'):
- raise AirflowException(
- 'Legacy P12 key file are not supported, use a JSON key file.'
- )
- else:
- raise AirflowException('Unrecognised extension for key file.')
- else:
- # Get credentials from JSON data provided in the UI.
- try:
- if not keyfile_dict:
- raise ValueError("The keyfile_dict should be set")
- keyfile_dict_json: Dict[str, str] = json.loads(keyfile_dict)
-
- # Depending on how the JSON was formatted, it may contain
- # escaped newlines. Convert those to actual newlines.
- keyfile_dict_json['private_key'] = keyfile_dict_json['private_key'].replace(
- '\\n', '\n')
-
- credentials = (
- google.oauth2.service_account.Credentials.from_service_account_info(
- keyfile_dict_json, scopes=self.scopes)
- )
- project_id = credentials.project_id
- except json.decoder.JSONDecodeError:
- raise AirflowException('Invalid key JSON.')
-
- if self.delegate_to:
- credentials = credentials.with_subject(self.delegate_to)
+ try:
+ keyfile_dict = self._get_field('keyfile_dict', None) # type: Optional[str]
Review comment:
Fixed as suggested
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #7795:
[AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #7795: [AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
URL: https://github.com/apache/airflow/pull/7795#issuecomment-602132463
# [Codecov](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=h1) Report
> Merging [#7795](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/fee827ed5a5d4aea2d2e60f73443357721a2c45a&el=desc) will **decrease** coverage by `0.72%`.
> The diff coverage is `95.50%`.
[](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #7795 +/- ##
==========================================
- Coverage 86.93% 86.20% -0.73%
==========================================
Files 924 925 +1
Lines 44674 44836 +162
==========================================
- Hits 38836 38652 -184
- Misses 5838 6184 +346
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/providers/hashicorp/secrets/vault.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvaGFzaGljb3JwL3NlY3JldHMvdmF1bHQucHk=) | `76.47% <0.00%> (+11.03%)` | :arrow_up: |
| [...oviders/google/cloud/utils/credentials\_provider.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvZ29vZ2xlL2Nsb3VkL3V0aWxzL2NyZWRlbnRpYWxzX3Byb3ZpZGVyLnB5) | `93.58% <96.66%> (+1.75%)` | :arrow_up: |
| [airflow/providers/google/cloud/hooks/base.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvZ29vZ2xlL2Nsb3VkL2hvb2tzL2Jhc2UucHk=) | `97.02% <100.00%> (+1.34%)` | :arrow_up: |
| [.../providers/google/cloud/secrets/secrets\_manager.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvZ29vZ2xlL2Nsb3VkL3NlY3JldHMvc2VjcmV0c19tYW5hZ2VyLnB5) | `100.00% <100.00%> (ø)` | |
| [...flow/providers/apache/cassandra/hooks/cassandra.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYXBhY2hlL2Nhc3NhbmRyYS9ob29rcy9jYXNzYW5kcmEucHk=) | `21.51% <0.00%> (-72.16%)` | :arrow_down: |
| [...w/providers/apache/hive/operators/mysql\_to\_hive.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYXBhY2hlL2hpdmUvb3BlcmF0b3JzL215c3FsX3RvX2hpdmUucHk=) | `35.84% <0.00%> (-64.16%)` | :arrow_down: |
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0.00%> (-55.56%)` | :arrow_down: |
| [airflow/providers/postgres/operators/postgres.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvcG9zdGdyZXMvb3BlcmF0b3JzL3Bvc3RncmVzLnB5) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
| [airflow/providers/redis/operators/redis\_publish.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvcmVkaXMvb3BlcmF0b3JzL3JlZGlzX3B1Ymxpc2gucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
| [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0.00%> (-47.06%)` | :arrow_down: |
| ... and [18 more](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=footer). Last update [fee827e...3a4b925](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] xinbinhuang commented on a change in pull request #7795:
[AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
Posted by GitBox <gi...@apache.org>.
xinbinhuang commented on a change in pull request #7795: [AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
URL: https://github.com/apache/airflow/pull/7795#discussion_r396084072
##########
File path: airflow/providers/google/cloud/utils/credentials_provider.py
##########
@@ -20,17 +20,23 @@
Google Cloud Platform authentication.
"""
import json
+import logging
import tempfile
from contextlib import contextmanager
-from typing import Dict, Optional, Sequence
+from typing import Dict, Optional, Sequence, Tuple
from urllib.parse import urlencode
+import google.auth
+import google.oauth2.service_account
from google.auth.environment_vars import CREDENTIALS
from airflow.exceptions import AirflowException
from airflow.utils.process_utils import patch_environ
+log = logging.getLogger(__name__)
+
AIRFLOW_CONN_GOOGLE_CLOUD_DEFAULT = "AIRFLOW_CONN_GOOGLE_CLOUD_DEFAULT"
+_DEFAULT_SCOPES = ('https://www.googleapis.com/auth/cloud-platform',) # type: Sequence[str]
Review comment:
Fixed as suggested
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #7795:
[AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #7795: [AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
URL: https://github.com/apache/airflow/pull/7795#issuecomment-602132463
# [Codecov](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=h1) Report
> Merging [#7795](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/fee827ed5a5d4aea2d2e60f73443357721a2c45a&el=desc) will **decrease** coverage by `0.72%`.
> The diff coverage is `95.50%`.
[](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #7795 +/- ##
==========================================
- Coverage 86.93% 86.20% -0.73%
==========================================
Files 924 925 +1
Lines 44674 44836 +162
==========================================
- Hits 38836 38651 -185
- Misses 5838 6185 +347
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/providers/hashicorp/secrets/vault.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvaGFzaGljb3JwL3NlY3JldHMvdmF1bHQucHk=) | `76.47% <0.00%> (+11.03%)` | :arrow_up: |
| [...oviders/google/cloud/utils/credentials\_provider.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvZ29vZ2xlL2Nsb3VkL3V0aWxzL2NyZWRlbnRpYWxzX3Byb3ZpZGVyLnB5) | `93.58% <96.66%> (+1.75%)` | :arrow_up: |
| [airflow/providers/google/cloud/hooks/base.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvZ29vZ2xlL2Nsb3VkL2hvb2tzL2Jhc2UucHk=) | `97.02% <100.00%> (+1.34%)` | :arrow_up: |
| [.../providers/google/cloud/secrets/secrets\_manager.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvZ29vZ2xlL2Nsb3VkL3NlY3JldHMvc2VjcmV0c19tYW5hZ2VyLnB5) | `100.00% <100.00%> (ø)` | |
| [...flow/providers/apache/cassandra/hooks/cassandra.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYXBhY2hlL2Nhc3NhbmRyYS9ob29rcy9jYXNzYW5kcmEucHk=) | `21.51% <0.00%> (-72.16%)` | :arrow_down: |
| [...w/providers/apache/hive/operators/mysql\_to\_hive.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYXBhY2hlL2hpdmUvb3BlcmF0b3JzL215c3FsX3RvX2hpdmUucHk=) | `35.84% <0.00%> (-64.16%)` | :arrow_down: |
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0.00%> (-55.56%)` | :arrow_down: |
| [airflow/providers/postgres/operators/postgres.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvcG9zdGdyZXMvb3BlcmF0b3JzL3Bvc3RncmVzLnB5) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
| [airflow/providers/redis/operators/redis\_publish.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvcmVkaXMvb3BlcmF0b3JzL3JlZGlzX3B1Ymxpc2gucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
| [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0.00%> (-47.06%)` | :arrow_down: |
| ... and [19 more](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=footer). Last update [fee827e...3a4b925](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] mik-laj commented on a change in pull request #7795:
[AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #7795: [AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
URL: https://github.com/apache/airflow/pull/7795#discussion_r395985652
##########
File path: airflow/providers/google/cloud/utils/credentials_provider.py
##########
@@ -158,3 +161,43 @@ def provide_gcp_conn_and_credentials(
key_file_path, scopes, project_id
):
yield
+
+
+def get_credentials_and_project_id(
Review comment:
This code is very similar to: https://github.com/apache/airflow/blob/master/airflow/providers/google/cloud/hooks/base.py#L172-L218
Can you remove code duplication, please?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] turbaszek commented on a change in pull request #7795:
[AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #7795: [AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
URL: https://github.com/apache/airflow/pull/7795#discussion_r396067305
##########
File path: airflow/providers/google/cloud/utils/credentials_provider.py
##########
@@ -20,17 +20,23 @@
Google Cloud Platform authentication.
"""
import json
+import logging
import tempfile
from contextlib import contextmanager
-from typing import Dict, Optional, Sequence
+from typing import Dict, Optional, Sequence, Tuple
from urllib.parse import urlencode
+import google.auth
+import google.oauth2.service_account
from google.auth.environment_vars import CREDENTIALS
from airflow.exceptions import AirflowException
from airflow.utils.process_utils import patch_environ
+log = logging.getLogger(__name__)
+
AIRFLOW_CONN_GOOGLE_CLOUD_DEFAULT = "AIRFLOW_CONN_GOOGLE_CLOUD_DEFAULT"
+_DEFAULT_SCOPES = ('https://www.googleapis.com/auth/cloud-platform',) # type: Sequence[str]
Review comment:
```suggestion
_DEFAULT_SCOPES: type: Sequence[str] = ('https://www.googleapis.com/auth/cloud-platform',)
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] mik-laj commented on a change in pull request #7795:
[AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #7795: [AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
URL: https://github.com/apache/airflow/pull/7795#discussion_r396076835
##########
File path: airflow/providers/google/cloud/secrets/secrets_manager.py
##########
@@ -0,0 +1,132 @@
+# 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.
+
+"""
+Objects relating to sourcing connections from GCP Secrets Manager
+"""
+from typing import List, Optional
+
+from cached_property import cached_property
+from google.api_core.exceptions import NotFound
+from google.api_core.gapic_v1.client_info import ClientInfo
+from google.cloud.secretmanager_v1 import SecretManagerServiceClient
+
+from airflow import version
+from airflow.models import Connection
+from airflow.providers.google.cloud.utils.credentials_provider import (
+ _get_scopes, get_credentials_and_project_id,
+)
+from airflow.secrets import BaseSecretsBackend
+from airflow.utils.log.logging_mixin import LoggingMixin
+
+
+class GcpSecretsManagerSecretsBackend(BaseSecretsBackend, LoggingMixin):
Review comment:
```suggestion
class CloudSecretsManagerSecretsBackend(BaseSecretsBackend, LoggingMixin):
```
We stopped using the prefix. The new prefix is 'Cloud' This one is no longer fashionable. https://docs.google.com/document/d/1_rTdJSLCt0eyrAylmmgYc3yZr-_h51fVlnvMmWqhCkY/edit
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #7795:
[AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #7795: [AIRFLOW-7104] Add Secret backend for GCP Secrets Manager
URL: https://github.com/apache/airflow/pull/7795#issuecomment-602132463
# [Codecov](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=h1) Report
> Merging [#7795](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/fee827ed5a5d4aea2d2e60f73443357721a2c45a&el=desc) will **decrease** coverage by `0.54%`.
> The diff coverage is `95.45%`.
[](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #7795 +/- ##
==========================================
- Coverage 86.93% 86.38% -0.55%
==========================================
Files 924 925 +1
Lines 44674 44718 +44
==========================================
- Hits 38836 38630 -206
- Misses 5838 6088 +250
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/providers/hashicorp/secrets/vault.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvaGFzaGljb3JwL3NlY3JldHMvdmF1bHQucHk=) | `76.47% <0.00%> (+11.03%)` | :arrow_up: |
| [...oviders/google/cloud/utils/credentials\_provider.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvZ29vZ2xlL2Nsb3VkL3V0aWxzL2NyZWRlbnRpYWxzX3Byb3ZpZGVyLnB5) | `93.58% <96.66%> (+1.75%)` | :arrow_up: |
| [airflow/providers/google/cloud/hooks/base.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvZ29vZ2xlL2Nsb3VkL2hvb2tzL2Jhc2UucHk=) | `97.02% <100.00%> (+1.34%)` | :arrow_up: |
| [...irflow/providers/google/secrets/secrets\_manager.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvZ29vZ2xlL3NlY3JldHMvc2VjcmV0c19tYW5hZ2VyLnB5) | `100.00% <100.00%> (ø)` | |
| [airflow/operators/generic\_transfer.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9vcGVyYXRvcnMvZ2VuZXJpY190cmFuc2Zlci5weQ==) | `39.28% <0.00%> (-60.72%)` | :arrow_down: |
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0.00%> (-55.56%)` | :arrow_down: |
| [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0.00%> (-47.06%)` | :arrow_down: |
| [airflow/security/kerberos.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9zZWN1cml0eS9rZXJiZXJvcy5weQ==) | `30.43% <0.00%> (-45.66%)` | :arrow_down: |
| [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `47.18% <0.00%> (-45.08%)` | :arrow_down: |
| [airflow/executors/celery\_executor.py](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGVjdXRvcnMvY2VsZXJ5X2V4ZWN1dG9yLnB5) | `50.67% <0.00%> (-37.84%)` | :arrow_down: |
| ... and [13 more](https://codecov.io/gh/apache/airflow/pull/7795/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=footer). Last update [fee827e...a04f148](https://codecov.io/gh/apache/airflow/pull/7795?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services