pierrejeambrun commented on code in PR #68777:
URL: https://github.com/apache/airflow/pull/68777#discussion_r3529354261
##########
airflow-core/src/airflow/models/connection.py:
##########
@@ -202,6 +202,13 @@ def __init__(
mask_secret(quote(self.password))
self.team_name = team_name
+ @staticmethod
+ def _validate_port(port: int | None, conn_id: str | None = None) -> None:
+ """Validate that port is within the valid TCP/UDP range (0-65535)."""
+ if port is not None and not (0 <= port <= 65535):
+ conn_msg = f" for connection {conn_id!r}" if conn_id else ""
+ raise ValueError(f"Port must be between 0 and 65535{conn_msg}, got
{port}")
+
Review Comment:
Unreachable, to remove
##########
airflow-core/tests/unit/models/test_connection.py:
##########
@@ -540,3 +540,54 @@ def test_get_conn_id_to_team_name_mapping(self,
testing_team: Team, session: Ses
"test_conn2": None,
}
clear_db_connections()
+
+ def test_port_validation_valid_ports(self):
+ """Test that valid port numbers (0-65535) are accepted."""
+ for port in [0, 1, 80, 443, 3306, 5432, 8080, 65535]:
+ conn = Connection(conn_id=f"test_{port}", conn_type="test",
port=port)
+ assert conn.port == port
+
+ def test_port_validation_invalid_negative_port(self):
+ """Test that negative port numbers are rejected."""
+ with pytest.raises(ValueError, match="Port must be between 0 and
65535"):
+ Connection(conn_id="test_neg", conn_type="test", port=-1)
+
+ def test_port_validation_invalid_port_too_large(self):
+ """Test that port numbers > 65535 are rejected."""
+ with pytest.raises(ValueError, match="Port must be between 0 and
65535"):
+ Connection(conn_id="test_large", conn_type="test", port=65536)
+
+ def test_port_validation_invalid_port_very_large(self):
+ """Test that very large port numbers are rejected."""
+ with pytest.raises(ValueError, match="Port must be between 0 and
65535"):
+ Connection(conn_id="test_very_large", conn_type="test",
port=99999999)
+
+ def test_port_validation_none_allowed(self):
+ """Test that None port is allowed (optional field)."""
+ conn = Connection(conn_id="test_none", conn_type="test", port=None)
+ assert conn.port is None
+
+ def test_port_validation_from_uri_valid_port(self):
+ """Test that valid ports from URI are accepted."""
+ conn = Connection(uri="postgres://user:pass@host:5432/db",
conn_id="test_uri")
+ assert conn.port == 5432
+
+ def test_port_validation_from_uri_invalid_port(self):
+ """Test that invalid ports from URI are rejected."""
+ with pytest.raises(ValueError, match="Port must be between 0 and
65535"):
+ Connection(uri="postgres://user:pass@host:99999/db",
conn_id="test_uri_invalid")
+
+ def test_port_validation_from_json_valid_port(self):
+ """Test that valid ports from JSON are accepted."""
+ conn = Connection.from_json('{"conn_type": "postgres", "port":
"5432"}', conn_id="test_json")
+ assert conn.port == 5432
+
+ def test_port_validation_from_json_invalid_port(self):
+ """Test that invalid ports from JSON are rejected."""
+ with pytest.raises(ValueError, match="Port must be between 0 and
65535"):
+ Connection.from_json('{"conn_type": "postgres", "port": "99999"}',
conn_id="test_json_invalid")
+
+ def test_port_validation_from_json_negative_port(self):
+ """Test that negative ports from JSON are rejected."""
+ with pytest.raises(ValueError, match="Port must be between 0 and
65535"):
+ Connection.from_json('{"conn_type": "postgres", "port": "-1"}',
conn_id="test_json_neg")
Review Comment:
This is testing the models but we don't update the model anymore.
##########
airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_connections.py:
##########
@@ -2062,3 +2062,30 @@ def test_post_should_fail_with_non_json_object_as_extra(
"method": "POST",
},
)
+
+
+class TestConnectionBodyPortValidation:
+ """Test port validation in ConnectionBody model."""
+
+ @pytest.mark.parametrize(
+ "port",
+ [0, 1, 80, 443, 3306, 5432, 8080, 65535],
+ )
+ def test_valid_ports(self, port):
+ """Test that valid port numbers (0-65535) are accepted."""
+ body = ConnectionBody(connection_id="test", conn_type="test",
port=port)
+ assert body.port == port
+
+ def test_none_port_allowed(self):
+ """Test that None port is allowed (optional field)."""
+ body = ConnectionBody(connection_id="test", conn_type="test",
port=None)
+ assert body.port is None
+
+ @pytest.mark.parametrize(
+ "port",
+ [-1, 65536, 99999, 99999999],
+ )
+ def test_invalid_ports(self, port):
+ """Test that invalid port numbers are rejected."""
+ with pytest.raises(ValueError, match="Port must be between 0 and
65535"):
+ ConnectionBody(connection_id="test", conn_type="test", port=port)
Review Comment:
No, that's a serializer test, it should be in the appropriate file.
Or test the full view / route. (patch connection endpoint, create connection
endpoint directly) and check responses.
--
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]
