potiuk opened a new pull request, #70886: URL: https://github.com/apache/airflow/pull/70886
> **Draft, and stacked on [#70884](https://github.com/apache/airflow/pull/70884) → > [#70885](https://github.com/apache/airflow/pull/70885)** — all three touch > `team_command.py`. This PR's own change is the last commit. Opened for the > design discussion first: it is a **breaking change** for existing deployments. A team specific Connection or Variable is stored as `_<TEAM_NAME>___<SECRET_ID>` in the environment. While a team name may itself contain the `___` separator, that string has **no single reading**: `_a___b___c` is team `a` with id `b___c` and equally team `a___b` with id `c`, and nothing in it chooses between them. Every guard over that namespace has had to work around the ambiguity rather than resolve it — and the two previous attempts were both wrong: - comparing the id against the prefix the caller's own team builds treated a prefix match as ownership, so a caller in team `a` could read team `a___b`'s secrets; - refusing every `_<x>___<y>` id closed that, but also refused legitimate team agnostic secrets whose ids merely looked namespaced. ### Approach Remove the ambiguity at its source: `TEAM_NAME_PATTERN` drops the underscore, becoming `^[a-zA-Z0-9-]{3,50}$`. A team name can then never span the separator, so a stored id has exactly one reading, and the namespace check collapses from a scan over every possible split to a single match: ```python return re.fullmatch(rf"_{_TEAM_NAME_CHARS}{TEAM_SEP}.+", secret_id) is not None ``` Net effect on the guard is **−40 lines of reasoning replaced by one regex**, with the behaviour preserved: an id whose leading segment could not be a team name still resolves through the team agnostic lookup. ### Why this is worth the break The ambiguity is not confined to this one guard. The same `<TEAM>___<SECTION>` shape appears in team scoped configuration (`AIRFLOW__<TEAM>___<SECTION>__<KEY>`), where splitting the variable name is ambiguous for exactly the same reason. Forbidding the underscore makes every consumer of a team name parseable rather than each having to defend itself. ### Breaking change **Existing team names containing an underscore become invalid.** `airflow teams create` rejects them, and `airflow teams sync` fails when the dag bundle config contains one. There is no migration in this PR: a deployment already running teams such as `data_eng` must rename them, and renaming a team changes the environment variable names its secrets are read from. That cost is the main thing to weigh, and the reason this is a draft. Options if it is judged too disruptive: 1. accept the break in a minor release with a prominent note (what this PR does); 2. keep underscores legal and keep the split-scanning guard from #70884 — correct, but every future consumer of a team name inherits the ambiguity; 3. forbid only the `___` sequence rather than the underscore entirely — narrower, but leaves `a_b` vs `a` sharing a prefix in other string contexts. ### Test plan - [x] 70 cases pass across `test_secrets_environment_variables.py` and `test_team_command.py` - [x] `test_team_create_rejects_underscore` — `team_a` is refused at creation - [x] `test_a_team_name_cannot_span_the_separator` — asserts the pattern rejects `team-a___prod`, that `_team-a___prod___dbconn` is refused for other callers as `team-a`'s namespace, and that its owner still reaches it with the bare id - [x] `test_id_that_cannot_name_a_team_is_still_resolved` — unchanged and passing, so the narrowing from #70884 survives the simplification - [x] Team names in the existing tests moved from `team_a` to `team-a` - [x] `ruff check` / `ruff format` clean ##### Was generative AI tooling used to co-author this PR? - [X] Yes — Claude Opus 5 (1M context) Generated-by: Claude Opus 5 (1M context) following the guidelines at https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions -- 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]
