ashb commented on code in PR #53801:
URL: https://github.com/apache/airflow/pull/53801#discussion_r2993990320
##########
providers/hashicorp/src/airflow/providers/hashicorp/_internal_client/vault_client.py:
##########
@@ -352,25 +348,38 @@ def _auth_gcp(self, _client: hvac.Client) -> None:
import json
import time
- import googleapiclient
+ # Determine service account email
+ service_account_email = getattr(credentials, "service_account_email",
None)
+ if not service_account_email or not isinstance(service_account_email,
str):
+ service_account_email = getattr(credentials, "client_email", None)
+
+ if not service_account_email or not isinstance(service_account_email,
str):
+ # Fallback for Compute Engine credentials if email is not yet
populated
+ try:
+ from google.auth import compute_engine
- if self.gcp_keyfile_dict:
- creds = self.gcp_keyfile_dict
- elif self.gcp_key_path:
- with open(self.gcp_key_path) as f:
- creds = json.load(f)
+ if isinstance(credentials, compute_engine.Credentials):
+ if not getattr(credentials, "service_account_email", None):
+ from google.auth import transport
- service_account = creds["client_email"]
+ credentials.refresh(transport.requests.Request())
+ service_account_email = credentials.service_account_email
+ except Exception:
+ pass
+
+ if not service_account_email:
+ raise VaultError("Could not determine service account email from
credentials")
# Generate a payload for subsequent "signJwt()" call
- # Reference:
https://googleapis.dev/python/google-auth/latest/reference/google.auth.jwt.html#google.auth.jwt.Credentials
now = int(time.time())
expires = now + 900 # 15 mins in seconds, can't be longer.
- payload = {"iat": now, "exp": expires, "sub": credentials, "aud":
f"vault/{self.role_id}"}
+ payload = {"iat": now, "exp": expires, "sub": service_account_email,
"aud": f"vault/{self.role_id}"}
Review Comment:
Hmmmm, what is the impact of this change? Was this just never working? I'm a
little bit worried this might break existing working instances.
##########
providers/hashicorp/src/airflow/providers/hashicorp/_internal_client/vault_client.py:
##########
@@ -352,25 +348,38 @@ def _auth_gcp(self, _client: hvac.Client) -> None:
import json
import time
- import googleapiclient
+ # Determine service account email
+ service_account_email = getattr(credentials, "service_account_email",
None)
+ if not service_account_email or not isinstance(service_account_email,
str):
+ service_account_email = getattr(credentials, "client_email", None)
+
+ if not service_account_email or not isinstance(service_account_email,
str):
Review Comment:
This "is not a string" check seems a bit odd -- if it's specified but of the
wrong time falling back to instance profiles is a bit unexpected. Either we
should blindly assume it's a string, or verify it's not set at all? Something
along those 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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
