Validate connection port is a valid network port number#70052
Validate connection port is a valid network port number#70052dj-wise-ronin wants to merge 2 commits into
Conversation
|
Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contributors' Guide
|
ab72394 to
f580aa4
Compare
SameerMesiah97
left a comment
There was a problem hiding this comment.
I have left comments on the test.
Also, I noticed the Task SDK and SQLAlchemy models now contain identical port parsing/validation logic. Is there an opportunity to share the implementation?
|
|
||
| # Invalid port from URI - type/invalid format | ||
| with pytest.raises(ValueError, match="Port could not be cast to integer value"): | ||
| Connection.from_uri("postgres://host:abc/db", conn_id="test") |
There was a problem hiding this comment.
The tests could be better. Boundary and whitespace coverage is missing amongst other things. I would suggest the below:
class TestConnectionPortValidation:
"""Test connection port validation in the task-sdk."""
@pytest.mark.parametrize(
("port", "expected"),
[
pytest.param(1, 1, id="min_port"),
pytest.param(80, 80, id="valid_port"),
pytest.param("8080", 8080, id="string_port"),
pytest.param(65535, 65535, id="max_port"),
pytest.param(None, None, id="none"),
pytest.param("", None, id="empty_string"),
pytest.param(" ", None, id="whitespace"),
],
)
def test_valid_port(self, port, expected):
"""Test valid port values."""
assert Connection(conn_id="test", port=port).port == expected
@pytest.mark.parametrize(
"port",
[
pytest.param(0, id="zero"),
pytest.param(-80, id="negative"),
pytest.param(70000, id="too_large"),
],
)
def test_invalid_port_range(self, port):
"""Test ports outside the valid network port range."""
with pytest.raises(ValueError, match="The `port` field must be a value between 1 and 65535"):
Connection(conn_id="test", port=port)
@pytest.mark.parametrize(
"port",
[
pytest.param("invalid_port", id="invalid_string"),
],
)
def test_invalid_port_type(self, port):
"""Test non-integer port values."""
with pytest.raises(ValueError, match="Expected integer value for `port`"):
Connection(conn_id="test", port=port)
def test_from_uri_port_validation(self):
"""Test that Connection.from_uri validates the port field correctly."""
assert Connection.from_uri("postgres://host:5432/db", conn_id="test").port == 5432
with pytest.raises(ValueError, match="Port out of range 0-65535"):
Connection.from_uri("postgres://host:70000/db", conn_id="test")
with pytest.raises(ValueError, match="Port could not be cast to integer value"):
Connection.from_uri("postgres://host:abc/db", conn_id="test")
|
|
||
| # Invalid ports - type errors | ||
| with pytest.raises(ValueError, match="Expected integer value for `port`"): | ||
| Connection(conn_id="test_port_fail_4", port="invalid_port") |
There was a problem hiding this comment.
This test could be better as well. Please see the below:
@pytest.mark.parametrize(
("port", "expected"),
[
pytest.param(1, 1, id="min_port"),
pytest.param(80, 80, id="valid_port"),
pytest.param(65535, 65535, id="max_port"),
pytest.param("8080", 8080, id="string_port"),
pytest.param(None, None, id="none"),
pytest.param("", None, id="empty_string"),
pytest.param(" ", None, id="whitespace"),
],
)
def test_port_validation(self, port, expected):
"""Test that Connection model validates the port field correctly."""
assert Connection(conn_id="test", port=port).port == expected
@pytest.mark.parametrize(
"port",
[
pytest.param(70000, id="too_large"),
pytest.param(0, id="zero"),
pytest.param(-80, id="negative"),
],
)
def test_port_validation_out_of_range(self, port):
"""Test that out-of-range ports are rejected."""
with pytest.raises(ValueError, match="The `port` field must be a value between 1 and 65535"):
Connection(conn_id="test", port=port)
@pytest.mark.parametrize(
"port",
[
pytest.param("invalid_port", id="invalid_string"),
],
)
def test_port_validation_invalid_type(self, port):
"""Test that invalid port types are rejected."""
with pytest.raises(ValueError, match="Expected integer value for `port`"):
Connection(conn_id="test", port=port)
|
I have refactored the codebase to share the connection port range validation logic between
|
|
Quickest fix: git fetch upstream main && git rebase upstream/main
rm uv.lock && uv lock
git add uv.lock && git rebase --continue
git push --force-with-leaseAutomated nudge — ignore if you're not ready to rebase. This comment is updated in place on future |
Signed-off-by: DeAngelo Jackson-Adams <dj@wiseronin.com>
…dk and core Signed-off-by: DeAngelo Jackson-Adams <dj@wiseronin.com>
082f4a1 to
4c69615
Compare
Description
Connection models in Apache Airflow accept integer values for the
portfield but do not enforce that the value is a valid network port in the range[1, 65535].This PR adds validation logic to connection ports at three layers:
@validates("port")to ensure port numbers assigned are integers between 1 and 65535.__attrs_post_init__to enforce range limits and format validation during standard instantiation and URI parsing.Field(ge=1, le=65535)on the REST API gateway connections schema.Additionally, this PR includes comprehensive unit tests verifying the validation rules for both the SQLAlchemy and Task SDK model implementations.