potiuk commented on code in PR #70755:
URL: https://github.com/apache/airflow/pull/70755#discussion_r3708428151
##########
shared/configuration/src/airflow_shared/configuration/parser.py:
##########
@@ -562,6 +590,68 @@ def sensitive_config_values(self) -> set[tuple[str, str]]:
sensitive.update(depr_section, depr_option)
return sensitive
+ def _names_sensitive_team_env_var(self, env_var: str) -> bool:
+ """
+ Check whether an environment variable name is a team scoped override
of a sensitive option.
+
+ Team scoped variables are named ``AIRFLOW__<TEAM>___<SECTION>__<KEY>``
(see
+ :meth:`_env_var_name`). A team name may contain underscores itself, so
the team name is not
+ parsed out of the variable name; the name is matched against the tail
that each option
+ registered as sensitive contributes instead. The ``_CMD`` /
``_SECRET`` fallbacks are
+ matched too, because - unlike for a base section - they are never
resolved into their value.
+
+ :param env_var: environment variable name
+ :return: True if the variable holds a team scoped value of an option
registered as sensitive
+ """
+ env_var = env_var.upper()
+ if not env_var.startswith(ENV_VAR_PREFIX):
+ return False
+ # Every tail matched below starts with the ``___`` separating the team
name from the
+ # section, so a name not containing it cannot be a team scoped one.
+ if "___" not in env_var:
+ return False
+ for section, key in self.sensitive_config_values:
+ option_tail = self._env_var_name(section,
key).removeprefix(ENV_VAR_PREFIX)
+ for tail in (f"___{option_tail}", f"___{option_tail}_CMD",
f"___{option_tail}_SECRET"):
+ # The team name sits between the prefix and the tail, so it
must not be empty.
+ if env_var.endswith(tail) and len(env_var) >
len(ENV_VAR_PREFIX) + len(tail):
+ return True
+ return False
+
+ def is_sensitive_option(self, section: str, key: str) -> bool:
+ """
+ Check whether the value of ``key`` in ``section`` is registered as
sensitive.
+
+ Options are registered as sensitive under their base section name,
while a team scoped
+ override of the very same option is held by a ``<team
name>=<section>`` config file section
+ or by an ``AIRFLOW__<TEAM>___<SECTION>__<KEY>`` environment variable.
Both of those
+ spellings are resolved back to the base option here, so that a team
scoped value is treated
+ exactly like the base one. A name that does not resolve to a
registered option is not
+ sensitive - so this only ever recognises more options as sensitive,
never fewer.
+
+ :param section: section name, either a base one or a team scoped one
+ :param key: option name
+ :return: True if the value of the option should be treated as sensitive
+ """
+ section = section.lower()
+ key = key.lower()
+ if (section, key) in self.sensitive_config_values:
+ return True
+ base_section = base_section_name(section)
+ if base_section != section:
+ if (base_section, key) in self.sensitive_config_values:
+ return True
+ # A team scoped ``_cmd`` / ``_secret`` fallback is not resolved
into its value, so it
+ # stays in the output as configured and has to be recognised on
its own.
+ for fallback_suffix in ("_cmd", "_secret"):
+ if not key.endswith(fallback_suffix):
+ continue
+ if (base_section, key.removesuffix(fallback_suffix)) in
self.sensitive_config_values:
+ return True
+ # A team scoped environment variable is reported under the section and
key its name splits
+ # into, which is neither the base nor the team scoped section name.
Review Comment:
Trimmed — `_include_envs` now just says it covers the cmd/secret variants
too and points at `is_sensitive_option`, which is where the reasoning belongs.
---
Drafted-by: Claude Code (Opus 5); reviewed by @potiuk before posting
##########
shared/configuration/tests/configuration/test_parser.py:
##########
@@ -1124,6 +1125,152 @@ def test_team_env_var_format(self):
):
assert test_conf.get("my_section", "my_key", team_name="my_team")
== "team_value"
+ @pytest.mark.parametrize(
+ ("section", "expected"),
+ [
+ pytest.param("celery", "celery", id="base_section"),
+ pytest.param("team_a=celery", "celery", id="team_scoped_section"),
+ pytest.param("team-a=celery", "celery", id="team_name_with_dash"),
+ pytest.param("a=team=celery", "celery",
id="separator_inside_team_name"),
+ ],
+ )
+ def test_base_section_name(self, section, expected):
+ """The base section name is recovered from a team scoped section
name."""
+ assert base_section_name(section) == expected
+
+ def test_is_sensitive_option_resolves_team_scoped_names(self):
+ """A team scoped spelling of a registered sensitive option is
recognised as sensitive."""
+ test_conf = AirflowConfigParser()
+ test_conf.sensitive_config_values.add(("test", "sensitive_key"))
+
+ assert test_conf.is_sensitive_option("test", "sensitive_key")
+ assert test_conf.is_sensitive_option("team_a=test", "sensitive_key")
+ assert test_conf.is_sensitive_option("team-a=test", "sensitive_key")
+ # A team name is never able to hide the base section it overrides, not
even when it
+ # contains the separator itself.
+ assert test_conf.is_sensitive_option("a=team=test", "sensitive_key")
+ # The section and key an AIRFLOW__<TEAM>___<SECTION>__<KEY> variable
name splits into.
+ assert test_conf.is_sensitive_option("team_a", "_test__sensitive_key")
+
+ # An option that is not registered as sensitive stays readable under
any spelling.
Review Comment:
Dropped two of the three; the remaining one sits on the first test that
makes the point.
---
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]
