potiuk commented on code in PR #70902:
URL: https://github.com/apache/airflow/pull/70902#discussion_r3708427863
##########
airflow-core/src/airflow/secrets/environment_variables.py:
##########
@@ -20,27 +20,35 @@
from __future__ import annotations
import os
-import re
from airflow.secrets import BaseSecretsBackend
CONN_ENV_PREFIX = "AIRFLOW_CONN_"
VAR_ENV_PREFIX = "AIRFLOW_VAR_"
+# Separates the team name from the secret id in a team namespaced environment
variable
+# name: AIRFLOW_CONN__<TEAM>___<ID>.
+TEAM_SEP = "___"
+
class EnvironmentVariablesBackend(BaseSecretsBackend):
"""Retrieves Connection object and Variable from environment variable."""
def get_conn_value(self, conn_id: str, team_name: str | None = None) ->
str | None:
+ if TEAM_SEP in conn_id:
+ # Refused ahead of the team scoped lookup, not only ahead of the
team agnostic
+ # one. The scoped lookup builds PREFIX + _<TEAM>___ + <ID>, so an
id that itself
+ # contains the separator makes that string ambiguous: a caller in
team `team_a`
+ # asking for `prod___dbconn` builds exactly what team
`team_a___prod` builds for
+ # id `dbconn`. That lookup *hits*, so a guard placed after it
never runs.
Review Comment:
Applied, both of them. The block was pasted verbatim into `get_conn_value`
and `get_variable`, which is exactly the repeat-the-same-rationale case
AGENTS.md rules out — the reasoning now lives once and the second site points
at it.
---
Drafted-by: Claude Code (Opus 5); reviewed by @potiuk before posting
##########
airflow-core/src/airflow/secrets/environment_variables.py:
##########
@@ -51,39 +59,18 @@ def get_variable(self, key: str, team_name: str | None =
None) -> str | None:
:param team_name: Team name associated to the task trying to access
the variable (if any)
:return: Variable Value
"""
+ if TEAM_SEP in key:
+ # Refused ahead of the team scoped lookup, not only ahead of the
team agnostic
+ # one. The scoped lookup builds PREFIX + _<TEAM>___ + <ID>, so an
id that itself
+ # contains the separator makes that string ambiguous: a caller in
team `team_a`
+ # asking for `prod___dbconn` builds exactly what team
`team_a___prod` builds for
+ # id `dbconn`. That lookup *hits*, so a guard placed after it
never runs.
Review Comment:
Applied, both of them. The block was pasted verbatim into `get_conn_value`
and `get_variable`, which is exactly the repeat-the-same-rationale case
AGENTS.md rules out — the reasoning now lives once and the second site points
at it.
---
Drafted-by: Claude Code (Opus 5); reviewed by @potiuk before posting
--
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]
