Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion server/opensandbox_server/services/docker.py
Original file line number Diff line number Diff line change
Expand Up @@ -1695,7 +1695,8 @@ def renew_expiration(
"message": f"Sandbox {sandbox_id} does not have automatic expiration enabled.",
},
)
if self._get_tracked_expiration(sandbox_id, labels) is None:
current_expiration = self._get_tracked_expiration(sandbox_id, labels)
if current_expiration is None:
raise HTTPException(
status_code=status.HTTP_409_CONFLICT,
detail={
Expand All @@ -1705,6 +1706,14 @@ def renew_expiration(
),
},
)
if new_expiration <= current_expiration:
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve renew-intent no-op behavior for equal expirations

The new strict check rejects new_expiration == current_expiration, but the renew-intent controller intentionally computes new_expires = max(now + extend, current) (see server/opensandbox_server/integrations/renew_intent/controller.py), so while a sandbox still has more than extend seconds left this path submits an equal timestamp. After this change those internal renew attempts now fail with 400, and in proxy-only mode that means last_success_monotonic is never updated (consumer.py cooldown is success-based), which can trigger repeated renew calls and warning logs on every access until close to expiry. Either adjust renew-intent to request a strictly later time or keep equality idempotent for this internal flow.

Useful? React with 👍 / 👎.

raise HTTPException(
status_code=status.HTTP_400_BAD_REQUEST,
detail={
"code": SandboxErrorCodes.INVALID_EXPIRATION,
"message": "New expiration time must be after the current expiresAt time.",
},
)

# Persist the new timeout in memory; it will also be respected on restart via _restore_existing_sandboxes
self._schedule_expiration(sandbox_id, new_expiration)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -660,6 +660,15 @@ def renew_expiration(
},
)

if new_expiration <= current_expiration:
raise HTTPException(
status_code=status.HTTP_400_BAD_REQUEST,
detail={
"code": SandboxErrorCodes.INVALID_EXPIRATION,
"message": "New expiration time must be after the current expiresAt time.",
},
)

# Update BatchSandbox spec.expireTime field
self.workload_provider.update_expiration(
sandbox_id=sandbox_id,
Expand Down
22 changes: 21 additions & 1 deletion server/tests/k8s/test_kubernetes_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -643,11 +643,12 @@ def test_renew_expiration_succeeds(self, k8s_service, mock_workload):

Purpose: Verify that sandbox expiration can be successfully renewed
"""
current_expiration = datetime.now(timezone.utc) + timedelta(hours=1)
new_expiration = datetime.now(timezone.utc) + timedelta(hours=2)

k8s_service.workload_provider.get_workload.return_value = mock_workload
k8s_service.workload_provider.update_expiration.return_value = None
k8s_service.workload_provider.get_expiration.return_value = new_expiration
k8s_service.workload_provider.get_expiration.return_value = current_expiration

from opensandbox_server.api.schema import RenewSandboxExpirationRequest
request = RenewSandboxExpirationRequest(expires_at=new_expiration)
Expand Down Expand Up @@ -694,3 +695,22 @@ def test_renew_returns_409_when_sandbox_has_no_expiration(self, k8s_service):
assert exc_info.value.status_code == 409
assert "does not have automatic expiration" in exc_info.value.detail["message"]
k8s_service.workload_provider.update_expiration.assert_not_called()

def test_renew_rejects_time_before_current_expires_at(self, k8s_service):
"""Renew is rejected when the new expiresAt is not after the current expiresAt (spec alignment)."""
current_expiration = datetime.now(timezone.utc) + timedelta(hours=2)
# Request a time that is in the future but earlier than the current expiration
earlier_time = datetime.now(timezone.utc) + timedelta(hours=1)

k8s_service.workload_provider.get_workload.return_value = MagicMock()
k8s_service.workload_provider.get_expiration.return_value = current_expiration

from opensandbox_server.api.schema import RenewSandboxExpirationRequest
request = RenewSandboxExpirationRequest(expires_at=earlier_time)

with pytest.raises(HTTPException) as exc_info:
k8s_service.renew_expiration("test-sandbox-id", request)

assert exc_info.value.status_code == 400
assert "after the current expiresAt" in exc_info.value.detail["message"]
k8s_service.workload_provider.update_expiration.assert_not_called()
29 changes: 29 additions & 0 deletions server/tests/test_docker_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -827,6 +827,35 @@ def test_renew_expiration_rejects_manual_cleanup_sandbox():
assert exc_info.value.detail["message"] == "Sandbox manual-id does not have automatic expiration enabled."


def test_renew_expiration_rejects_time_before_current_expires_at():
"""Renew must be rejected when the new expiresAt is before the current expiresAt (spec alignment)."""
service = DockerSandboxService(config=_app_config())
current_expiration = datetime.now(timezone.utc) + timedelta(hours=2)
# Request a time that is in the future but earlier than the current expiration
earlier_time = datetime.now(timezone.utc) + timedelta(hours=1)

container = MagicMock()
container.attrs = {
"Config": {
"Labels": {
SANDBOX_ID_LABEL: "sbx-001",
SANDBOX_EXPIRES_AT_LABEL: current_expiration.isoformat(),
}
}
}
request = MagicMock(expires_at=earlier_time)

with (
patch.object(service, "_get_container_by_sandbox_id", return_value=container),
patch.object(service, "_get_tracked_expiration", return_value=current_expiration),
):
with pytest.raises(HTTPException) as exc_info:
service.renew_expiration("sbx-001", request)

assert exc_info.value.status_code == status.HTTP_400_BAD_REQUEST
assert "after the current expiresAt" in exc_info.value.detail["message"]


@pytest.mark.asyncio
@patch("opensandbox_server.services.docker.docker")
async def test_create_sandbox_async_returns_provisioning(mock_docker):
Expand Down
Loading