amoghrajesh commented on code in PR #70755:
URL: https://github.com/apache/airflow/pull/70755#discussion_r3710058009
##########
shared/configuration/tests/configuration/test_parser.py:
##########
@@ -1124,6 +1125,150 @@ 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.
Review Comment:
```suggestion
```
##########
airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_config.py:
##########
@@ -559,3 +561,66 @@ def
test_get_config_value_masks_per_key_secrets_backend_kwarg(self, test_client)
)
assert response.status_code == 200
assert response.json()["sections"][0]["options"][0]["value"] ==
OPTION_VALUE_SENSITIVE_HIDDEN
+
+
+TEAM_NAME = "team_a"
+SECTION_TEAM_DATABASE = f"{TEAM_NAME}={SECTION_DATABASE}"
+SECTION_TEAM_CORE = f"{TEAM_NAME}={SECTION_CORE}"
+OPTION_VALUE_TEAM_SQL_ALCHEMY_CONN =
"postgresql://team_a_user:team_a_password@team-a-db/airflow"
+OPTION_VALUE_TEAM_PARALLELISM = "512"
+
+
+class TestTeamScopedOptionMasking(TestConfigEndpoint):
+ """A team scoped override of a sensitive option lives in a
``[<team>=<section>]`` section rather
+ than in the section the option is registered under. The single-option
endpoint has to resolve
+ that spelling back to the registered option, so that it redacts a team
scoped value exactly as
+ it redacts the base one."""
+
+ @pytest.fixture(autouse=True)
+ def setup_team_scoped(self) -> Generator[None, None, None]:
+ team_config = {
+ SECTION_TEAM_DATABASE: {OPTION_KEY_SQL_ALCHEMY_CONN:
OPTION_VALUE_TEAM_SQL_ALCHEMY_CONN},
+ SECTION_TEAM_CORE: {OPTION_KEY_PARALLELISM:
OPTION_VALUE_TEAM_PARALLELISM},
+ }
+
+ # Anything that is not one of the team scoped options under test keeps
being read from the
+ # real configuration - the endpoint reads [api] expose_config through
the same object.
+ real_get = conf.get
+
+ def _mock_has_option(section: str, option: str, **_) -> bool:
+ return option in team_config.get(section, {})
+
+ def _mock_get(section: str, option: str, *args, **kwargs) -> str:
+ if option in team_config.get(section, {}):
+ return team_config[section][option]
+ return real_get(section, option, *args, **kwargs)
+
+ with (
+ conf_vars(AIRFLOW_CONFIG_ENABLE_EXPOSE_CONFIG),
+ patch(
+
"airflow.api_fastapi.core_api.routes.public.config.conf.has_option",
+ new=_mock_has_option,
+ ),
+ patch(
+ "airflow.api_fastapi.core_api.routes.public.config.conf.get",
+ new=_mock_get,
+ ),
+ ):
+ yield
+
+ def test_get_config_value_masks_team_scoped_sensitive_option(self,
test_client):
+ response = test_client.get(
+
f"/config/section/{SECTION_TEAM_DATABASE}/option/{OPTION_KEY_SQL_ALCHEMY_CONN}",
+ headers=HEADERS_JSON,
+ )
+ assert response.status_code == 200
+ assert response.json()["sections"][0]["options"][0]["value"] ==
OPTION_VALUE_SENSITIVE_HIDDEN
+
+ def test_get_config_value_keeps_team_scoped_non_sensitive_option(self,
test_client):
+ """An option that is not registered as sensitive stays readable under
a team scoped section."""
Review Comment:
```suggestion
```
##########
shared/configuration/src/airflow_shared/configuration/parser.py:
##########
@@ -77,6 +77,34 @@ def _collect_kwarg_env_vars(prefix: str) -> dict[str, str]:
ConfigSectionSourcesType = dict[str, str | tuple[str, str]]
ConfigSourcesType = dict[str, ConfigSectionSourcesType]
ENV_VAR_PREFIX = "AIRFLOW__"
+# Separates the team name from the base section name in a team scoped config
file section.
+TEAM_SECTION_SEPARATOR = "="
+
+
+def team_section_name(team_name: str, section: str) -> str:
+ """
+ Build the config file section name that holds the team scoped overrides of
``section``.
+
+ :param team_name: name of the team the overrides belong to
+ :param section: base section name that is being overridden
+ :return: the team scoped section name, e.g. ``team_a=celery``
+ """
+ return f"{team_name}{TEAM_SECTION_SEPARATOR}{section}"
+
+
+def base_section_name(section: str) -> str:
Review Comment:
Do these have to be in the public API surface?
--
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]
