debayanCODES-1 opened a new pull request, #72271:
URL: https://github.com/apache/airflow/pull/72271
Fixes #68382
<html>
<body>
<!--StartFragment--><html><head></head><body><h1>Add validation for the
<code>port</code> field on Connection objects</h1>
<p>Fixes #68382</p>
<h2>What does this PR do?</h2>
<p>Adds validation to the <code>port</code> field on Connection objects so
that only valid TCP/UDP port numbers (integers in the range 0–65535, or
<code>None</code>) are accepted. Previously, any value — including negative
numbers, integers above 65535, non-integer strings, floats, and booleans — was
silently stored and propagated to workers without any error.</p>
<h2>Why is this change needed?</h2>
<p>A user can currently create a connection with <code>port = -1</code> or
<code>port = 99999999</code> through the CLI, the REST API, or direct model
construction, and Airflow accepts and persists it with no warning.</p>
<p>Invalid port values cause confusing failures at runtime, inside task
workers, far from where the bad value was entered. This fix surfaces the error
immediately at the point of input, regardless of which entry point is used.</p>
<h2>What are the changes?</h2>
<p><strong>1. Core model —
<code>airflow/models/connection.py</code></strong></p>
<ul>
<li>Added a <code>_validate_port(port)</code> static method.</li>
<li>Accepts integers in <code>[0, 65535]</code> and digit-strings like
<code>"8080"</code> (coerced to int, for backward compatibility with URI
parsing).</li>
<li>Rejects negative values, values above 65535, booleans, floats, and
non-numeric strings.</li>
<li>Raises <code>ValueError</code> with a consistent, human-readable
message.</li>
<li>Called in <code>__init__</code> (when port is not <code>None</code>) and
in <code>from_json</code>.</li>
</ul>
<p><strong>2. Task SDK —
<code>task-sdk/src/airflow/sdk/definitions/connection.py</code></strong></p>
<ul>
<li>Added a <code>_validate_port</code> <code>attrs</code> validator with
equivalent logic.</li>
<li>Task SDK is strictly decoupled from <code>airflow-core</code> and cannot
import from <code>airflow.models</code>, so the validator is implemented
independently using the <code>attrs</code> validator protocol.</li>
<li>Attached to the field: <code>port = attrs.field(default=None,
validator=_validate_port)</code>.</li>
</ul>
<p><strong>3. Public REST API —
<code>airflow/api_fastapi/core_api/datamodels/connections.py</code></strong></p>
<ul>
<li>Added <code>ge=0, le=65535</code> Pydantic <code>Field</code>
constraints to <code>ConnectionBody.port</code>.</li>
<li><code>ConnectionBodyPartial</code> (PATCH) and
<code>ConnectionTestRequestBody</code> (test-connection) both inherit from
<code>ConnectionBody</code>, so they receive the constraint automatically.</li>
<li>Invalid port values now return <code>422 Unprocessable Entity</code>
from the API.</li>
</ul>
<p><strong>4. Execution API —
<code>airflow/api_fastapi/execution_api/datamodels/connection.py</code></strong></p>
<ul>
<li>Added <code>ge=0, le=65535</code> to
<code>ConnectionResponse.port</code>.</li>
</ul>
<p><strong>5. CLI —
<code>airflow/cli/commands/connection_command.py</code></strong></p>
<ul>
<li>Wrapped the <code>Connection(...)</code> construction block in
<code>try/except</code> (<code>ValueError</code>,
<code>AirflowException</code>).</li>
<li>Re-raises as <code>SystemExit(f"Could not create connection.
{e}")</code> for a clean, user-readable CLI error.</li>
<li>The range check is <strong>not</strong> duplicated in the CLI — it
delegates entirely to the model and catches the resulting
<code>ValueError</code>.</li>
</ul>
<p><strong>6. Task SDK JSON schema —
<code>task-sdk/src/airflow/sdk/execution_time/schema/schema.json</code></strong></p>
<ul>
<li>Added <code>"minimum": 0</code> and <code>"maximum": 65535</code> to the
<code>port</code> integer branch in <code>ConnectionResponse</code> and
<code>ConnectionResult</code>.</li>
</ul>
<p><strong>7. OpenAPI snapshot —
<code>airflow-core/src/airflow/api_fastapi/core_api/openapi/v2-rest-api-generated.yaml</code></strong></p>
<ul>
<li>Updated <code>port</code> under <code>ConnectionBody</code> and
<code>ConnectionTestRequestBody</code> to include <code>minimum: 0</code> and
<code>maximum: 65535</code>.</li>
<li><code>ConnectionResponse.port</code> is left without bounds in the
snapshot, since responses may reflect legacy data already in the DB.</li>
</ul>
<p><strong>8. airflowctl generated client —
<code>airflow-ctl/src/airflowctl/api/datamodels/generated.py</code></strong></p>
<ul>
<li>Updated <code>ConnectionBody.port</code> and
<code>ConnectionTestRequestBody.port</code> with <code>Field(ge=0,
le=65535)</code> to match the updated OpenAPI schema.</li>
<li><code>airflow-ctl</code> interacts with Airflow only through the REST
API, so it inherits validation at the API boundary automatically; the
client-side constraint is additional defense-in-depth.</li>
</ul>
<h2>Tests</h2>
File | What is tested
-- | --
tests/unit/models/test_connection.py | Valid ports (0, 80, 5432, 65535,
None) are accepted. Invalid ports (-1, 65536, 99999999, "invalid", 80.5, True,
False) raise ValueError. from_json validates as well.
task-sdk/tests/task_sdk/definitions/test_connection.py | Same boundary cases
via the attrs validator and from_json.
tests/unit/api_fastapi/core_api/datamodels/test_connections.py |
ConnectionBody accepts valid ports and raises ValidationError on invalid ones.
tests/unit/api_fastapi/core_api/routes/public/test_connections.py | POST
/connections and PATCH /connections/{id} return 422 for out-of-range ports.
Valid boundary values (0, 65535) return 201/200.
tests/unit/cli/commands/test_connection_command.py | connections add
--conn-port=-1 exits with a SystemExit containing the message "Could not create
connection".
<h2>Notes for reviewers</h2>
<ul>
<li>The error message is uniform across all layers: <code>"Expected integer
value between 0 and 65535 for 'port', but got X instead."</code> This keeps
test assertions consistent and gives users a single, recognizable message
regardless of entry point.</li>
<li><code>bool</code> values are explicitly rejected before the
<code>isinstance(port, int)</code> check, since <code>bool</code> is a subclass
of <code>int</code> in Python (<code>True == 1</code>, <code>False ==
0</code>). Without this guard, <code>True</code> would pass as port
<code>1</code> and <code>False</code> as port <code>0</code>, which is never
intentional.</li>
<li>String coercion (<code>"8080"</code> → <code>8080</code>) is
intentional. Existing URI parsing already produces string ports in some code
paths; strict rejection would be a silent regression there.</li>
<li>The OpenAPI snapshot is hand-updated to match what the spec-generation
script produces after the <code>ConnectionBody</code> Pydantic change. If CI
runs the generation script, the output should be identical.</li>
<li>Scope is intentionally minimal — no refactoring beyond what the issue
asks for.</li>
</ul>
<hr>
</p></body></html><!--EndFragment-->
</body>
</html>
--
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]
