Skip to content

Conversation

@kevinmingtarja
Copy link
Collaborator

@kevinmingtarja kevinmingtarja commented Sep 26, 2025

Fixes #7159.

The new managed jobs controller code uses the local API server in consolidation mode, but if you are using SSO or basic auth, it somehow needs to authenticate, even though it's running on the same host, just in different processes.

This PR fixes this by bypassing the oauth2 and basic auth middleware, if consolidation mode is enabled AND the request comes from a loopback address. We also handle the case where there is a proxy running in the same host (in which case the request will come from a loopback address), by checking for common headers added by proxies such as X-Forwarded-For, Forwarded, etc.

We also considered having the API server open a Unix domain socket, and similarly have requests to the socket be treated as internal (no auth needed), however it doesn't seem like there is a good way to do this in uvicorn while being able to share worker processes with the existing server.

Tested (run the relevant ones):

  • Code formatting: install pre-commit (auto-check on commit) or bash format.sh
  • Any manual or new tests for this PR (please specify below)
    • New smoke test test_loopback_access_with_basic_auth
    • A few unit tests
    • Tested locally using a local oauth2 proxy and a local API server with SKYPILOT_AUTH_OAUTH2_PROXY_ENABLED=true
    • Tested manually on a remote API server with okta oauth login
  • All smoke tests: /smoke-test (CI) or pytest tests/test_smoke.py (local)
  • Relevant individual tests: /smoke-test -k test_name (CI) or pytest tests/test_smoke.py::test_name (local)
  • Backward compatibility: /quicktest-core (CI) or pytest tests/smoke_tests/test_backward_compat.py (local)

@kevinmingtarja
Copy link
Collaborator Author

/smoke-test -k test_loopback_access_with_basic_auth

@kevinmingtarja
Copy link
Collaborator Author

/smoke-test -k test_loopback_access_with_basic_auth

@kevinmingtarja
Copy link
Collaborator Author

/smoke-test -k test_loopback_access_with_basic_auth
/smoke-test -k test_loopback_access_with_basic_auth --kubernetes

@kevinmingtarja
Copy link
Collaborator Author

/smoke-test -k test_loopback_access_with_basic_auth
/smoke-test -k test_loopback_access_with_basic_auth --kubernetes

@kevinmingtarja kevinmingtarja changed the title [Core] Authenticate requests from loopback address when consolidation mode is enabled [Jobs] Authenticate requests from loopback address when consolidation mode is enabled Sep 26, 2025
@kevinmingtarja
Copy link
Collaborator Author

/quicktest-core --base-branch master

@kevinmingtarja
Copy link
Collaborator Author

/smoke-test

@Michaelvll
Copy link
Collaborator

/smoke-test --remote-server

@Michaelvll
Copy link
Collaborator

/smoke-test --remote-server --kubernetes

smoke_tests_utils.run_one_test(test)


@pytest.mark.no_remote_server
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Skipping as this test requires restarting the API server.

@Michaelvll
Copy link
Collaborator

/smoke-test --remote-server
/smoke-test --remote-server --kubernetes
/quicktest-core --base-branch 0.10.1

@kevinmingtarja
Copy link
Collaborator Author

/quicktest-core --base-branch v0.10.1

@Michaelvll Michaelvll added this to the 0.10.4 milestone Oct 3, 2025
@kevinmingtarja kevinmingtarja merged commit ce3847b into master Oct 4, 2025
21 of 23 checks passed
@kevinmingtarja kevinmingtarja deleted the consolidation-mode-auth-fix branch October 4, 2025 02:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cannot run managed job in a remote Kubernetes in-cluster environment using nightly image

4 participants