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/06/18 09:04:56 UTC
[GitHub] [airflow] ashb commented on a change in pull request #9368: Adds GCP Secret Manager Hook
ashb commented on a change in pull request #9368:
URL: https://github.com/apache/airflow/pull/9368#discussion_r442076538
##########
File path: airflow/providers/google/cloud/secrets/secret_manager.py
##########
@@ -134,23 +131,12 @@ def get_variable(self, key: str) -> Optional[str]:
def _get_secret(self, path_prefix: str, secret_id: str) -> Optional[str]:
"""
- Get secret value from Parameter Store.
+ Get secret value from Secret_manager based on prefix.
Review comment:
```suggestion
Get secret value from SecretManager based on prefix.
```
maybe?
##########
File path: airflow/providers/google/PROVIDERS_CHANGES_2020.05.20.md
##########
@@ -87,7 +87,7 @@
| [01f99426f](https://github.com/apache/airflow/commit/01f99426fddd2a24552f352edcb271fa78cf3b15) | 2020-03-28 | Add download/upload operators for GCS and Google Sheets (#7866) |
| [892522f8e](https://github.com/apache/airflow/commit/892522f8e2aeedc1ad842a08aaea967b0cae077f) | 2020-03-26 | Change signature of GSheetsHook methods (#7853) |
| [bfd425157](https://github.com/apache/airflow/commit/bfd425157a746402b516f8fc9e48f4ddccd794ce) | 2020-03-26 | Improve idempotency in MLEngineHook.create_model (#7811) |
-| [f9c226343](https://github.com/apache/airflow/commit/f9c226343d94a7732da280d1dd086bf1ba291c77) | 2020-03-26 | Fix CloudSecretsManagerBackend invalid connections_prefix (#7861) |
+| [f9c226343](https://github.com/apache/airflow/commit/f9c226343d94a7732da280d1dd086bf1ba291c77) | 2020-03-26 | Fix CloudSecretManagerBackend invalid connections_prefix (#7861) |
Review comment:
This one should probably stay is it is? (it was right when then commits message was written after all)
##########
File path: tests/test_utils/gcp_system_helpers.py
##########
@@ -172,3 +173,29 @@ def grant_bucket_access(cls, bucket: str, account_email: str):
bucket_name,
]
)
+
+ @classmethod
+ def delete_secret(cls, name: str, silent: bool = False):
+ cmd = ["gcloud", "secrets", "delete", name, "--project", GoogleSystemTest._project_id(), "--quiet"]
+ cls.execute_with_ctx(cmd, key=GCP_SECRET_MANAGER_KEY, silent=silent)
+
+ @classmethod
+ def create_secret(cls, name: str, value: str):
+ with tempfile.NamedTemporaryFile(delete=False) as tmp:
+ tmp.write(value.encode("UTF-8"))
+ cmd = ["gcloud", "secrets", "create", name,
+ "--replication-policy", "automatic",
+ "--project", GoogleSystemTest._project_id(),
+ "--data-file", tmp.name]
+ cls.execute_with_ctx(cmd, key=GCP_SECRET_MANAGER_KEY)
+ os.remove(tmp.name)
Review comment:
Does this work?
```suggestion
with tempfile.NamedTemporaryFile() as tmp:
tmp.write(value.encode("UTF-8"))
tmp.flush()
cmd = ["gcloud", "secrets", "create", name,
"--replication-policy", "automatic",
"--project", GoogleSystemTest._project_id(),
"--data-file", tmp.name]
cls.execute_with_ctx(cmd, key=GCP_SECRET_MANAGER_KEY)
```
(otherwise in case of error the file leaks.)
##########
File path: airflow/providers/google/README.md
##########
@@ -560,7 +560,7 @@ All classes in Airflow 2.0 are in `airflow.providers.google` package.
| Airflow 2.0 protocols: `airflow.providers.google` package | Airflow 1.10.* previous location (usually `airflow.contrib`) |
|:--------------------------------------------------------------------------------------------------------------------------------------------------------------------|:---------------------------------------------------------------------------------------------------------------------------------------------------------------------|
-| [cloud.secrets.secrets_manager.CloudSecretsManagerBackend](https://github.com/apache/airflow/blob/master/airflow/providers/google/cloud/secrets/secrets_manager.py) | [contrib.secrets.gcp_secrets_manager.CloudSecretsManagerBackend](https://github.com/apache/airflow/blob/v1-10-stable/airflow/contrib/secrets/gcp_secrets_manager.py) |
+| [cloud.secrets.secrets_manager.CloudSecretManagerBackend](https://github.com/apache/airflow/blob/master/airflow/providers/google/cloud/secrets/secrets_manager.py) | [contrib.secrets.gcp_secrets_manager.CloudSecretManagerBackend](https://github.com/apache/airflow/blob/v1-10-stable/airflow/contrib/secrets/gcp_secrets_manager.py) |
Review comment:
```suggestion
| [cloud.secrets.secrets_manager.CloudSecretManagerBackend](https://github.com/apache/airflow/blob/master/airflow/providers/google/cloud/secrets/secrets_manager.py) | [contrib.secrets.gcp_secrets_manager.CloudSecretsManagerBackend](https://github.com/apache/airflow/blob/v1-10-stable/airflow/contrib/secrets/gcp_secrets_manager.py) |
```
----------------------------------------------------------------
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