From 0c2f787680f5bf8fc30cfd105320a066ed4e048b Mon Sep 17 00:00:00 2001 From: Harikrishna Date: Tue, 24 Jun 2025 08:06:20 +0530 Subject: [PATCH 1/4] Add regional support for google secret manager hook --- .../google/cloud/hooks/secret_manager.py | 110 ++++++++++++++++-- .../google/cloud/hooks/test_secret_manager.py | 71 +++++++++-- 2 files changed, 164 insertions(+), 17 deletions(-) diff --git a/providers/google/src/airflow/providers/google/cloud/hooks/secret_manager.py b/providers/google/src/airflow/providers/google/cloud/hooks/secret_manager.py index db144acf42da6..13d40592588eb 100644 --- a/providers/google/src/airflow/providers/google/cloud/hooks/secret_manager.py +++ b/providers/google/src/airflow/providers/google/cloud/hooks/secret_manager.py @@ -47,6 +47,10 @@ class GoogleCloudSecretManagerHook(GoogleBaseHook): See https://cloud.google.com/secret-manager """ + def __init__(self, location_id: str | None = None, **kwargs) -> None: + super().__init__(**kwargs) + self.location_id = location_id + @cached_property def client(self): """ @@ -54,7 +58,16 @@ def client(self): :return: Secret Manager client. """ - return SecretManagerServiceClient(credentials=self.get_credentials(), client_info=CLIENT_INFO) + if self.location_id is not None: + return SecretManagerServiceClient( + credentials=self.get_credentials(), + client_info=CLIENT_INFO, + client_options={"api_endpoint": f"secretmanager.{self.location_id}.rep.googleapis.com"}, + ) + return SecretManagerServiceClient( + credentials=self.get_credentials(), + client_info=CLIENT_INFO, + ) def get_conn(self) -> SecretManagerServiceClient: """ @@ -64,6 +77,58 @@ def get_conn(self) -> SecretManagerServiceClient: """ return self.client + def _get_parent(self, project_id: str, location_id: str | None = None) -> str: + """ + Return parent path. + + :param project_id: Required. ID of the GCP project that owns the job. + :param location_id: Optional. Target location ID. If set to ``None`` or missing, the location_id provided for GoogleCloudSecretHook instantiation is used + For more details : https://cloud.google.com/secret-manager/docs/locations + + :return: Parent path. + """ + _location_id = location_id or self.location_id + if _location_id is not None: + return self.client.common_location_path(project_id, _location_id) + return self.client.common_project_path(project_id) + + def _get_secret_path(self, project_id: str, secret_id: str, location_id: str | None = None) -> str: + """ + Return secret path. + + :param project_id: Required. ID of the GCP project that owns the job. + :param secret_id: Required. Secret ID for which path is required. + :param location_id: Optional. Target location ID. If set to ``None`` or missing, the location_id provided for GoogleCloudSecretHook instantiation is used + For more details : https://cloud.google.com/secret-manager/docs/locations + + :return: Parent path. + """ + _location_id = location_id or self.location_id + if _location_id is not None: + # Google's client library does not provide a method to construct regional secret paths, so constructing manually. + return f"projects/{project_id}/locations/{_location_id}/secrets/{secret_id}" + return self.client.secret_path(project_id, secret_id) + + def _get_secret_version_path( + self, project_id: str, secret_id: str, secret_version: str, location_id: str | None = None + ) -> str: + """ + Return secret version path. + + :param project_id: Required. ID of the GCP project that owns the job. + :param secret_id: Required. Secret ID for which path is required. + :param secret_version: Required. Secret version for which path is required. + :param location_id: Optional. Target location ID. If set to ``None`` or missing, the location_id provided for GoogleCloudSecretHook instantiation is used + For more details : https://cloud.google.com/secret-manager/docs/locations + + :return: Parent path. + """ + _location_id = location_id or self.location_id + if _location_id is not None: + # Google's client library does not provide a method to construct regional secret version paths, so constructing manually. + return f"projects/{project_id}/locations/{_location_id}/secrets/{secret_id}/versions/{secret_version}" + return self.client.secret_version_path(project_id, secret_id, secret_version) + @GoogleBaseHook.fallback_to_default_project_id def create_secret( self, @@ -73,6 +138,7 @@ def create_secret( retry: Retry | _MethodDefault = DEFAULT, timeout: float | None = None, metadata: Sequence[tuple[str, str]] = (), + location_id: str | None = None, # I have added at end for maintaining backward compatibility. ) -> Secret: """ Create a secret. @@ -88,12 +154,20 @@ def create_secret( :param retry: Optional. Designation of what errors, if any, should be retried. :param timeout: Optional. The timeout for this request. :param metadata: Optional. Strings which should be sent along with the request as metadata. + :param location_id: Optional. Location where secret should be created. Used for creating regional secret. If set to ``None`` or missing, the location_id provided for GoogleCloudSecretHook instantiation is used + For more details : https://cloud.google.com/secret-manager/docs/locations :return: Secret object. """ - _secret = secret or {"replication": {"automatic": {}}} + if not secret: + _secret: dict | Secret = {} + if (location_id or self.location_id) is None: + _secret["replication"] = {"automatic": {}} + else: + _secret = secret + response = self.client.create_secret( request={ - "parent": f"projects/{project_id}", + "parent": self._get_parent(project_id, location_id), "secret_id": secret_id, "secret": _secret, }, @@ -113,6 +187,7 @@ def add_secret_version( retry: Retry | _MethodDefault = DEFAULT, timeout: float | None = None, metadata: Sequence[tuple[str, str]] = (), + location_id: str | None = None, # I have added at end for maintaining backward compatibility. ) -> SecretVersion: """ Add a version to the secret. @@ -128,11 +203,13 @@ def add_secret_version( :param retry: Optional. Designation of what errors, if any, should be retried. :param timeout: Optional. The timeout for this request. :param metadata: Optional. Strings which should be sent along with the request as metadata. + :param location_id: Optional. Location where secret is located. Used for adding version to regional secret. If set to ``None`` or missing, the location_id provided for GoogleCloudSecretHook instantiation is used + For more details : https://cloud.google.com/secret-manager/docs/locations :return: Secret version object. """ response = self.client.add_secret_version( request={ - "parent": f"projects/{project_id}/secrets/{secret_id}", + "parent": self._get_secret_path(project_id, secret_id, location_id), "payload": secret_payload, }, retry=retry, @@ -152,6 +229,7 @@ def list_secrets( retry: Retry | _MethodDefault = DEFAULT, timeout: float | None = None, metadata: Sequence[tuple[str, str]] = (), + location_id: str | None = None, # I have added at end for maintaining backward compatibility. ) -> ListSecretsPager: """ List secrets. @@ -168,11 +246,13 @@ def list_secrets( :param retry: Optional. Designation of what errors, if any, should be retried. :param timeout: Optional. The timeout for this request. :param metadata: Optional. Strings which should be sent along with the request as metadata. + :param location_id: Optional. The regional secrets stored in the provided location_id will be listed. If set to ``None`` or missing, the location_id provided for GoogleCloudSecretHook instantiation is used + For more details : https://cloud.google.com/secret-manager/docs/locations :return: Secret List object. """ response = self.client.list_secrets( request={ - "parent": f"projects/{project_id}", + "parent": self._get_parent(project_id, location_id), "page_size": page_size, "page_token": page_token, "filter": secret_filter, @@ -185,18 +265,22 @@ def list_secrets( return response @GoogleBaseHook.fallback_to_default_project_id - def secret_exists(self, project_id: str, secret_id: str) -> bool: + def secret_exists(self, project_id: str, secret_id: str, location_id: str | None = None) -> bool: """ Check whether secret exists. :param project_id: Required. ID of the GCP project that owns the job. If set to ``None`` or missing, the default project_id from the GCP connection is used. :param secret_id: Required. ID of the secret to find. + :param location_id: Optional. Location ID where secret is expected to be stored regionally. If set to ``None`` or missing, the location_id provided for GoogleCloudSecretHook instantiation is used + For more details : https://cloud.google.com/secret-manager/docs/locations :return: True if the secret exists, False otherwise. """ secret_filter = f"name:{secret_id}" - secret_name = self.client.secret_path(project_id, secret_id) - for secret in self.list_secrets(project_id=project_id, page_size=100, secret_filter=secret_filter): + secret_name = self._get_secret_path(project_id, secret_id, location_id) + for secret in self.list_secrets( + project_id=project_id, page_size=100, secret_filter=secret_filter, location_id=location_id + ): if secret.name.split("/")[-1] == secret_id: self.log.info("Secret %s exists.", secret_name) return True @@ -212,6 +296,7 @@ def access_secret( retry: Retry | _MethodDefault = DEFAULT, timeout: float | None = None, metadata: Sequence[tuple[str, str]] = (), + location_id: str | None = None, # I have added at end for maintaining backward compatibility. ) -> AccessSecretVersionResponse: """ Access a secret version. @@ -227,11 +312,13 @@ def access_secret( :param retry: Optional. Designation of what errors, if any, should be retried. :param timeout: Optional. The timeout for this request. :param metadata: Optional. Strings which should be sent along with the request as metadata. + :param location_id: Optional. Location ID where secret is stored regionally. If set to ``None`` or missing, the location_id provided for GoogleCloudSecretHook instantiation is used + For more details : https://cloud.google.com/secret-manager/docs/locations :return: Access secret version response object. """ response = self.client.access_secret_version( request={ - "name": self.client.secret_version_path(project_id, secret_id, secret_version), + "name": self._get_secret_version_path(project_id, secret_id, secret_version, location_id), }, retry=retry, timeout=timeout, @@ -248,6 +335,7 @@ def delete_secret( retry: Retry | _MethodDefault = DEFAULT, timeout: float | None = None, metadata: Sequence[tuple[str, str]] = (), + location_id: str | None = None, # I have added at end for maintaining backward compatibility. ) -> None: """ Delete a secret. @@ -262,9 +350,11 @@ def delete_secret( :param retry: Optional. Designation of what errors, if any, should be retried. :param timeout: Optional. The timeout for this request. :param metadata: Optional. Strings which should be sent along with the request as metadata. + :param location_id: Optional. Location ID where secret is stored regionally. If set to ``None`` or missing, the location_id provided for GoogleCloudSecretHook instantiation is used. + For more details : https://cloud.google.com/secret-manager/docs/locations :return: Access secret version response object. """ - name = self.client.secret_path(project_id, secret_id) + name = self._get_secret_path(project_id, secret_id, location_id) self.client.delete_secret( request={"name": name}, retry=retry, diff --git a/providers/google/tests/unit/google/cloud/hooks/test_secret_manager.py b/providers/google/tests/unit/google/cloud/hooks/test_secret_manager.py index 3bcafad0dcfe1..69c73d3f11087 100644 --- a/providers/google/tests/unit/google/cloud/hooks/test_secret_manager.py +++ b/providers/google/tests/unit/google/cloud/hooks/test_secret_manager.py @@ -29,6 +29,8 @@ BASE_PACKAGE = "airflow.providers.google.common.hooks.base_google." SECRETS_HOOK_PACKAGE = "airflow.providers.google.cloud.hooks.secret_manager." SECRET_ID = "test-secret-id" +REGIONAL_SECRET_LOCATION = "test-location" +SECRET_VERSION = "test-secret-version" class TestGoogleCloudSecretManagerHook: @@ -36,6 +38,52 @@ def setup_method(self, method): with patch(f"{BASE_PACKAGE}GoogleBaseHook.get_connection", return_value=MagicMock()): self.hook = GoogleCloudSecretManagerHook() + @patch(f"{SECRETS_HOOK_PACKAGE}GoogleCloudSecretManagerHook.client", new_callable=PropertyMock) + def test__get_parent(self, mock_client): + expected_value = mock_client.return_value.common_project_path.return_value + parent = self.hook._get_parent(GCP_PROJECT_ID_HOOK_UNIT_TEST) + assert expected_value == parent + mock_client.assert_called_once() + mock_client.return_value.common_project_path.assert_called_once_with(GCP_PROJECT_ID_HOOK_UNIT_TEST) + + expected_value = mock_client.return_value.common_location_path.return_value + parent = self.hook._get_parent(GCP_PROJECT_ID_HOOK_UNIT_TEST, REGIONAL_SECRET_LOCATION) + assert expected_value == parent + # mock_client.assert_called_once() # will fail as already client has been triggered due to previous case. + mock_client.return_value.common_location_path.assert_called_once_with( + GCP_PROJECT_ID_HOOK_UNIT_TEST, REGIONAL_SECRET_LOCATION + ) + + @patch(f"{SECRETS_HOOK_PACKAGE}GoogleCloudSecretManagerHook.client", new_callable=PropertyMock) + def test__get_secret_path(self, mock_client): + expected_value = mock_client.return_value.secret_path.return_value + secret_path = self.hook._get_secret_path(GCP_PROJECT_ID_HOOK_UNIT_TEST, SECRET_ID) + assert expected_value == secret_path + mock_client.assert_called_once() + mock_client.return_value.secret_path.assert_called_once_with(GCP_PROJECT_ID_HOOK_UNIT_TEST, SECRET_ID) + + expected_value = f"projects/{GCP_PROJECT_ID_HOOK_UNIT_TEST}/locations/{REGIONAL_SECRET_LOCATION}/secrets/{SECRET_ID}" + parent = self.hook._get_secret_path( + GCP_PROJECT_ID_HOOK_UNIT_TEST, SECRET_ID, REGIONAL_SECRET_LOCATION + ) + assert expected_value == parent + + @patch(f"{SECRETS_HOOK_PACKAGE}GoogleCloudSecretManagerHook.client", new_callable=PropertyMock) + def test__get_secret_version_path(self, mock_client): + expected_value = mock_client.return_value.secret_version_path.return_value + parent = self.hook._get_secret_version_path(GCP_PROJECT_ID_HOOK_UNIT_TEST, SECRET_ID, SECRET_VERSION) + assert expected_value == parent + mock_client.assert_called_once() + mock_client.return_value.secret_version_path.assert_called_once_with( + GCP_PROJECT_ID_HOOK_UNIT_TEST, SECRET_ID, SECRET_VERSION + ) + + expected_value = f"projects/{GCP_PROJECT_ID_HOOK_UNIT_TEST}/locations/{REGIONAL_SECRET_LOCATION}/secrets/{SECRET_ID}/versions/{SECRET_VERSION}" + parent = self.hook._get_secret_version_path( + GCP_PROJECT_ID_HOOK_UNIT_TEST, SECRET_ID, SECRET_VERSION, REGIONAL_SECRET_LOCATION + ) + assert expected_value == parent + @patch(f"{SECRETS_HOOK_PACKAGE}GoogleCloudSecretManagerHook.get_credentials") @patch(f"{SECRETS_HOOK_PACKAGE}SecretManagerServiceClient") def test_client(self, mock_client, mock_get_credentials): @@ -66,9 +114,10 @@ def test_get_conn(self, mock_client): (mock_secret := MagicMock(), mock_secret), # type: ignore[name-defined] ], ) + @patch(f"{SECRETS_HOOK_PACKAGE}GoogleCloudSecretManagerHook._get_parent") @patch(f"{SECRETS_HOOK_PACKAGE}GoogleCloudSecretManagerHook.client", new_callable=PropertyMock) - def test_create_secret(self, mock_client, input_secret, expected_secret): - expected_parent = f"projects/{GCP_PROJECT_ID_HOOK_UNIT_TEST}" + def test_create_secret(self, mock_client, mock_get_parent, input_secret, expected_secret): + expected_parent = mock_get_parent.return_value # f"projects/{GCP_PROJECT_ID_HOOK_UNIT_TEST}" expected_response = mock_client.return_value.create_secret.return_value mock_retry, mock_timeout, mock_metadata = MagicMock(), MagicMock(), MagicMock() @@ -83,6 +132,7 @@ def test_create_secret(self, mock_client, input_secret, expected_secret): assert actual_response == expected_response mock_client.assert_called_once() + mock_get_parent.assert_called_once_with(GCP_PROJECT_ID_HOOK_UNIT_TEST, None) mock_client.return_value.create_secret.assert_called_once_with( request={ "parent": expected_parent, @@ -94,9 +144,10 @@ def test_create_secret(self, mock_client, input_secret, expected_secret): metadata=mock_metadata, ) + @patch(f"{SECRETS_HOOK_PACKAGE}GoogleCloudSecretManagerHook._get_secret_path") @patch(f"{SECRETS_HOOK_PACKAGE}GoogleCloudSecretManagerHook.client", new_callable=PropertyMock) - def test_add_secret_version(self, mock_client): - expected_parent = f"projects/{GCP_PROJECT_ID_HOOK_UNIT_TEST}/secrets/{SECRET_ID}" + def test_add_secret_version(self, mock_client, mock_get_secret_path): + expected_parent = mock_get_secret_path.return_value expected_response = mock_client.return_value.add_secret_version.return_value mock_payload, mock_retry, mock_timeout, mock_metadata = (MagicMock() for _ in range(4)) @@ -111,6 +162,7 @@ def test_add_secret_version(self, mock_client): assert actual_response == expected_response mock_client.assert_called_once() + mock_get_secret_path.assert_called_once_with(GCP_PROJECT_ID_HOOK_UNIT_TEST, SECRET_ID, None) mock_client.return_value.add_secret_version.assert_called_once_with( request={ "parent": expected_parent, @@ -121,9 +173,10 @@ def test_add_secret_version(self, mock_client): metadata=mock_metadata, ) + @patch(f"{SECRETS_HOOK_PACKAGE}GoogleCloudSecretManagerHook._get_parent") @patch(f"{SECRETS_HOOK_PACKAGE}GoogleCloudSecretManagerHook.client", new_callable=PropertyMock) - def test_list_secrets(self, mock_client): - expected_parent = f"projects/{GCP_PROJECT_ID_HOOK_UNIT_TEST}" + def test_list_secrets(self, mock_client, mock_get_parent): + expected_parent = mock_get_parent.return_value expected_response = mock_client.return_value.list_secrets.return_value mock_filter, mock_retry, mock_timeout, mock_metadata = (MagicMock() for _ in range(4)) page_size, page_token = 20, "test-page-token" @@ -140,6 +193,7 @@ def test_list_secrets(self, mock_client): assert actual_response == expected_response mock_client.assert_called_once() + mock_get_parent.assert_called_once_with(GCP_PROJECT_ID_HOOK_UNIT_TEST, None) mock_client.return_value.list_secrets.assert_called_once_with( request={ "parent": expected_parent, @@ -184,7 +238,10 @@ def test_secret_exists( assert secret_exists_actual == secret_exists_expected mock_client.return_value.secret_path.assert_called_once_with(GCP_PROJECT_ID_HOOK_UNIT_TEST, secret_id) mock_list_secrets.assert_called_once_with( - project_id=GCP_PROJECT_ID_HOOK_UNIT_TEST, page_size=100, secret_filter=secret_filter + project_id=GCP_PROJECT_ID_HOOK_UNIT_TEST, + page_size=100, + secret_filter=secret_filter, + location_id=None, ) @patch(f"{SECRETS_HOOK_PACKAGE}GoogleCloudSecretManagerHook.client", new_callable=PropertyMock) From 99653a60016395570809069a9578ed145fad51da Mon Sep 17 00:00:00 2001 From: Harikrishna Date: Tue, 24 Jun 2025 23:00:18 +0530 Subject: [PATCH 2/4] Change property name from location_id to location --- .../google/cloud/hooks/secret_manager.py | 78 +++++++++---------- .../google/cloud/hooks/test_secret_manager.py | 2 +- 2 files changed, 40 insertions(+), 40 deletions(-) diff --git a/providers/google/src/airflow/providers/google/cloud/hooks/secret_manager.py b/providers/google/src/airflow/providers/google/cloud/hooks/secret_manager.py index 13d40592588eb..75e35b666fcfb 100644 --- a/providers/google/src/airflow/providers/google/cloud/hooks/secret_manager.py +++ b/providers/google/src/airflow/providers/google/cloud/hooks/secret_manager.py @@ -47,9 +47,9 @@ class GoogleCloudSecretManagerHook(GoogleBaseHook): See https://cloud.google.com/secret-manager """ - def __init__(self, location_id: str | None = None, **kwargs) -> None: + def __init__(self, location: str | None = None, **kwargs) -> None: super().__init__(**kwargs) - self.location_id = location_id + self.location = location @cached_property def client(self): @@ -58,11 +58,11 @@ def client(self): :return: Secret Manager client. """ - if self.location_id is not None: + if self.location is not None: return SecretManagerServiceClient( credentials=self.get_credentials(), client_info=CLIENT_INFO, - client_options={"api_endpoint": f"secretmanager.{self.location_id}.rep.googleapis.com"}, + client_options={"api_endpoint": f"secretmanager.{self.location}.rep.googleapis.com"}, ) return SecretManagerServiceClient( credentials=self.get_credentials(), @@ -77,40 +77,40 @@ def get_conn(self) -> SecretManagerServiceClient: """ return self.client - def _get_parent(self, project_id: str, location_id: str | None = None) -> str: + def _get_parent(self, project_id: str, location: str | None = None) -> str: """ Return parent path. :param project_id: Required. ID of the GCP project that owns the job. - :param location_id: Optional. Target location ID. If set to ``None`` or missing, the location_id provided for GoogleCloudSecretHook instantiation is used + :param location: Optional. Target location. If set to ``None`` or missing, the location provided for GoogleCloudSecretHook instantiation is used For more details : https://cloud.google.com/secret-manager/docs/locations :return: Parent path. """ - _location_id = location_id or self.location_id - if _location_id is not None: - return self.client.common_location_path(project_id, _location_id) + _location = location or self.location + if _location is not None: + return self.client.common_location_path(project_id, _location) return self.client.common_project_path(project_id) - def _get_secret_path(self, project_id: str, secret_id: str, location_id: str | None = None) -> str: + def _get_secret_path(self, project_id: str, secret_id: str, location: str | None = None) -> str: """ Return secret path. :param project_id: Required. ID of the GCP project that owns the job. :param secret_id: Required. Secret ID for which path is required. - :param location_id: Optional. Target location ID. If set to ``None`` or missing, the location_id provided for GoogleCloudSecretHook instantiation is used + :param location: Optional. Target location. If set to ``None`` or missing, the location provided for GoogleCloudSecretHook instantiation is used For more details : https://cloud.google.com/secret-manager/docs/locations :return: Parent path. """ - _location_id = location_id or self.location_id - if _location_id is not None: + _location = location or self.location + if _location is not None: # Google's client library does not provide a method to construct regional secret paths, so constructing manually. - return f"projects/{project_id}/locations/{_location_id}/secrets/{secret_id}" + return f"projects/{project_id}/locations/{_location}/secrets/{secret_id}" return self.client.secret_path(project_id, secret_id) def _get_secret_version_path( - self, project_id: str, secret_id: str, secret_version: str, location_id: str | None = None + self, project_id: str, secret_id: str, secret_version: str, location: str | None = None ) -> str: """ Return secret version path. @@ -118,15 +118,15 @@ def _get_secret_version_path( :param project_id: Required. ID of the GCP project that owns the job. :param secret_id: Required. Secret ID for which path is required. :param secret_version: Required. Secret version for which path is required. - :param location_id: Optional. Target location ID. If set to ``None`` or missing, the location_id provided for GoogleCloudSecretHook instantiation is used + :param location: Optional. Target location. If set to ``None`` or missing, the location provided for GoogleCloudSecretHook instantiation is used For more details : https://cloud.google.com/secret-manager/docs/locations :return: Parent path. """ - _location_id = location_id or self.location_id - if _location_id is not None: + _location = location or self.location + if _location is not None: # Google's client library does not provide a method to construct regional secret version paths, so constructing manually. - return f"projects/{project_id}/locations/{_location_id}/secrets/{secret_id}/versions/{secret_version}" + return f"projects/{project_id}/locations/{_location}/secrets/{secret_id}/versions/{secret_version}" return self.client.secret_version_path(project_id, secret_id, secret_version) @GoogleBaseHook.fallback_to_default_project_id @@ -138,7 +138,7 @@ def create_secret( retry: Retry | _MethodDefault = DEFAULT, timeout: float | None = None, metadata: Sequence[tuple[str, str]] = (), - location_id: str | None = None, # I have added at end for maintaining backward compatibility. + location: str | None = None, # I have added at end for maintaining backward compatibility. ) -> Secret: """ Create a secret. @@ -154,20 +154,20 @@ def create_secret( :param retry: Optional. Designation of what errors, if any, should be retried. :param timeout: Optional. The timeout for this request. :param metadata: Optional. Strings which should be sent along with the request as metadata. - :param location_id: Optional. Location where secret should be created. Used for creating regional secret. If set to ``None`` or missing, the location_id provided for GoogleCloudSecretHook instantiation is used + :param location: Optional. Location where secret should be created. Used for creating regional secret. If set to ``None`` or missing, the location provided for GoogleCloudSecretHook instantiation is used For more details : https://cloud.google.com/secret-manager/docs/locations :return: Secret object. """ if not secret: _secret: dict | Secret = {} - if (location_id or self.location_id) is None: + if (location or self.location) is None: _secret["replication"] = {"automatic": {}} else: _secret = secret response = self.client.create_secret( request={ - "parent": self._get_parent(project_id, location_id), + "parent": self._get_parent(project_id, location), "secret_id": secret_id, "secret": _secret, }, @@ -187,7 +187,7 @@ def add_secret_version( retry: Retry | _MethodDefault = DEFAULT, timeout: float | None = None, metadata: Sequence[tuple[str, str]] = (), - location_id: str | None = None, # I have added at end for maintaining backward compatibility. + location: str | None = None, # I have added at end for maintaining backward compatibility. ) -> SecretVersion: """ Add a version to the secret. @@ -203,13 +203,13 @@ def add_secret_version( :param retry: Optional. Designation of what errors, if any, should be retried. :param timeout: Optional. The timeout for this request. :param metadata: Optional. Strings which should be sent along with the request as metadata. - :param location_id: Optional. Location where secret is located. Used for adding version to regional secret. If set to ``None`` or missing, the location_id provided for GoogleCloudSecretHook instantiation is used + :param location: Optional. Location where secret is located. Used for adding version to regional secret. If set to ``None`` or missing, the location provided for GoogleCloudSecretHook instantiation is used For more details : https://cloud.google.com/secret-manager/docs/locations :return: Secret version object. """ response = self.client.add_secret_version( request={ - "parent": self._get_secret_path(project_id, secret_id, location_id), + "parent": self._get_secret_path(project_id, secret_id, location), "payload": secret_payload, }, retry=retry, @@ -229,7 +229,7 @@ def list_secrets( retry: Retry | _MethodDefault = DEFAULT, timeout: float | None = None, metadata: Sequence[tuple[str, str]] = (), - location_id: str | None = None, # I have added at end for maintaining backward compatibility. + location: str | None = None, # I have added at end for maintaining backward compatibility. ) -> ListSecretsPager: """ List secrets. @@ -246,13 +246,13 @@ def list_secrets( :param retry: Optional. Designation of what errors, if any, should be retried. :param timeout: Optional. The timeout for this request. :param metadata: Optional. Strings which should be sent along with the request as metadata. - :param location_id: Optional. The regional secrets stored in the provided location_id will be listed. If set to ``None`` or missing, the location_id provided for GoogleCloudSecretHook instantiation is used + :param location: Optional. The regional secrets stored in the provided location will be listed. If set to ``None`` or missing, the location provided for GoogleCloudSecretHook instantiation is used For more details : https://cloud.google.com/secret-manager/docs/locations :return: Secret List object. """ response = self.client.list_secrets( request={ - "parent": self._get_parent(project_id, location_id), + "parent": self._get_parent(project_id, location), "page_size": page_size, "page_token": page_token, "filter": secret_filter, @@ -265,21 +265,21 @@ def list_secrets( return response @GoogleBaseHook.fallback_to_default_project_id - def secret_exists(self, project_id: str, secret_id: str, location_id: str | None = None) -> bool: + def secret_exists(self, project_id: str, secret_id: str, location: str | None = None) -> bool: """ Check whether secret exists. :param project_id: Required. ID of the GCP project that owns the job. If set to ``None`` or missing, the default project_id from the GCP connection is used. :param secret_id: Required. ID of the secret to find. - :param location_id: Optional. Location ID where secret is expected to be stored regionally. If set to ``None`` or missing, the location_id provided for GoogleCloudSecretHook instantiation is used + :param location: Optional. Location where secret is expected to be stored regionally. If set to ``None`` or missing, the location provided for GoogleCloudSecretHook instantiation is used For more details : https://cloud.google.com/secret-manager/docs/locations :return: True if the secret exists, False otherwise. """ secret_filter = f"name:{secret_id}" - secret_name = self._get_secret_path(project_id, secret_id, location_id) + secret_name = self._get_secret_path(project_id, secret_id, location) for secret in self.list_secrets( - project_id=project_id, page_size=100, secret_filter=secret_filter, location_id=location_id + project_id=project_id, page_size=100, secret_filter=secret_filter, location=location ): if secret.name.split("/")[-1] == secret_id: self.log.info("Secret %s exists.", secret_name) @@ -296,7 +296,7 @@ def access_secret( retry: Retry | _MethodDefault = DEFAULT, timeout: float | None = None, metadata: Sequence[tuple[str, str]] = (), - location_id: str | None = None, # I have added at end for maintaining backward compatibility. + location: str | None = None, # I have added at end for maintaining backward compatibility. ) -> AccessSecretVersionResponse: """ Access a secret version. @@ -312,13 +312,13 @@ def access_secret( :param retry: Optional. Designation of what errors, if any, should be retried. :param timeout: Optional. The timeout for this request. :param metadata: Optional. Strings which should be sent along with the request as metadata. - :param location_id: Optional. Location ID where secret is stored regionally. If set to ``None`` or missing, the location_id provided for GoogleCloudSecretHook instantiation is used + :param location: Optional. Location where secret is stored regionally. If set to ``None`` or missing, the location provided for GoogleCloudSecretHook instantiation is used For more details : https://cloud.google.com/secret-manager/docs/locations :return: Access secret version response object. """ response = self.client.access_secret_version( request={ - "name": self._get_secret_version_path(project_id, secret_id, secret_version, location_id), + "name": self._get_secret_version_path(project_id, secret_id, secret_version, location), }, retry=retry, timeout=timeout, @@ -335,7 +335,7 @@ def delete_secret( retry: Retry | _MethodDefault = DEFAULT, timeout: float | None = None, metadata: Sequence[tuple[str, str]] = (), - location_id: str | None = None, # I have added at end for maintaining backward compatibility. + location: str | None = None, # I have added at end for maintaining backward compatibility. ) -> None: """ Delete a secret. @@ -350,11 +350,11 @@ def delete_secret( :param retry: Optional. Designation of what errors, if any, should be retried. :param timeout: Optional. The timeout for this request. :param metadata: Optional. Strings which should be sent along with the request as metadata. - :param location_id: Optional. Location ID where secret is stored regionally. If set to ``None`` or missing, the location_id provided for GoogleCloudSecretHook instantiation is used. + :param location: Optional. Location where secret is stored regionally. If set to ``None`` or missing, the location provided for GoogleCloudSecretHook instantiation is used. For more details : https://cloud.google.com/secret-manager/docs/locations :return: Access secret version response object. """ - name = self._get_secret_path(project_id, secret_id, location_id) + name = self._get_secret_path(project_id, secret_id, location) self.client.delete_secret( request={"name": name}, retry=retry, diff --git a/providers/google/tests/unit/google/cloud/hooks/test_secret_manager.py b/providers/google/tests/unit/google/cloud/hooks/test_secret_manager.py index 69c73d3f11087..4bf8a156cfe86 100644 --- a/providers/google/tests/unit/google/cloud/hooks/test_secret_manager.py +++ b/providers/google/tests/unit/google/cloud/hooks/test_secret_manager.py @@ -241,7 +241,7 @@ def test_secret_exists( project_id=GCP_PROJECT_ID_HOOK_UNIT_TEST, page_size=100, secret_filter=secret_filter, - location_id=None, + location=None, ) @patch(f"{SECRETS_HOOK_PACKAGE}GoogleCloudSecretManagerHook.client", new_callable=PropertyMock) From ff82321e8f5fb6f7f2786a99ff5ddf852e547799 Mon Sep 17 00:00:00 2001 From: Harikrishna Date: Sat, 28 Jun 2025 11:12:40 +0530 Subject: [PATCH 3/4] Remove backward compatibility comment. --- .../providers/google/cloud/hooks/secret_manager.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/providers/google/src/airflow/providers/google/cloud/hooks/secret_manager.py b/providers/google/src/airflow/providers/google/cloud/hooks/secret_manager.py index 75e35b666fcfb..2097f8b113149 100644 --- a/providers/google/src/airflow/providers/google/cloud/hooks/secret_manager.py +++ b/providers/google/src/airflow/providers/google/cloud/hooks/secret_manager.py @@ -138,7 +138,7 @@ def create_secret( retry: Retry | _MethodDefault = DEFAULT, timeout: float | None = None, metadata: Sequence[tuple[str, str]] = (), - location: str | None = None, # I have added at end for maintaining backward compatibility. + location: str | None = None, ) -> Secret: """ Create a secret. @@ -187,7 +187,7 @@ def add_secret_version( retry: Retry | _MethodDefault = DEFAULT, timeout: float | None = None, metadata: Sequence[tuple[str, str]] = (), - location: str | None = None, # I have added at end for maintaining backward compatibility. + location: str | None = None, ) -> SecretVersion: """ Add a version to the secret. @@ -229,7 +229,7 @@ def list_secrets( retry: Retry | _MethodDefault = DEFAULT, timeout: float | None = None, metadata: Sequence[tuple[str, str]] = (), - location: str | None = None, # I have added at end for maintaining backward compatibility. + location: str | None = None, ) -> ListSecretsPager: """ List secrets. @@ -296,7 +296,7 @@ def access_secret( retry: Retry | _MethodDefault = DEFAULT, timeout: float | None = None, metadata: Sequence[tuple[str, str]] = (), - location: str | None = None, # I have added at end for maintaining backward compatibility. + location: str | None = None, ) -> AccessSecretVersionResponse: """ Access a secret version. @@ -335,7 +335,7 @@ def delete_secret( retry: Retry | _MethodDefault = DEFAULT, timeout: float | None = None, metadata: Sequence[tuple[str, str]] = (), - location: str | None = None, # I have added at end for maintaining backward compatibility. + location: str | None = None, ) -> None: """ Delete a secret. From 0f9c4f973a2a802e082268c700922d2ef23707b8 Mon Sep 17 00:00:00 2001 From: Harikrishna Date: Tue, 1 Jul 2025 08:56:57 +0530 Subject: [PATCH 4/4] Fix static check failing --- .../airflow/providers/google/cloud/hooks/secret_manager.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/providers/google/src/airflow/providers/google/cloud/hooks/secret_manager.py b/providers/google/src/airflow/providers/google/cloud/hooks/secret_manager.py index 2097f8b113149..c68c01fe1c9be 100644 --- a/providers/google/src/airflow/providers/google/cloud/hooks/secret_manager.py +++ b/providers/google/src/airflow/providers/google/cloud/hooks/secret_manager.py @@ -126,7 +126,9 @@ def _get_secret_version_path( _location = location or self.location if _location is not None: # Google's client library does not provide a method to construct regional secret version paths, so constructing manually. - return f"projects/{project_id}/locations/{_location}/secrets/{secret_id}/versions/{secret_version}" + return ( + f"projects/{project_id}/locations/{_location}/secrets/{secret_id}/versions/{secret_version}" + ) return self.client.secret_version_path(project_id, secret_id, secret_version) @GoogleBaseHook.fallback_to_default_project_id