Skip to content

[Core] Fix user_hash mismatch issue#9252

Merged
DanielZhangQD merged 2 commits into
masterfrom
5022
Apr 7, 2026
Merged

[Core] Fix user_hash mismatch issue#9252
DanielZhangQD merged 2 commits into
masterfrom
5022

Conversation

@DanielZhangQD
Copy link
Copy Markdown
Collaborator

@DanielZhangQD DanielZhangQD commented Apr 3, 2026

  • Fix sky api login skipping SSO when a residual service account token from a previous login is still in config, by clearing it before the first health check.
  • Fix ~/.sky/user_hash not being updated after SSO login, causing false "Cluster was created by another user" warnings on subsequent sky launch.

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)
    • Test sky api login with:
      • basic auth in ingress - local user_hash does not change
      • basic auth in api server - local user_hash is updated
      • auth with service account token - local user_hash is updated
      • auth with SSO - local user_hash is updated
  • 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)

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates the api_login flow to save the endpoint and clear residual service account tokens before the initial health check, ensuring the server correctly triggers SSO when required. It also synchronizes the local user hash with the final authenticated health check response. New unit tests have been added to verify these behaviors. Feedback was provided regarding unused variables in the newly added test cases that should be cleaned up.

Comment thread tests/unit_tests/test_sky/server/test_sdk.py Outdated
@DanielZhangQD DanielZhangQD marked this pull request as ready for review April 3, 2026 08:34
Copy link
Copy Markdown
Collaborator

@aylei aylei left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @DanielZhangQD !

@DanielZhangQD DanielZhangQD merged commit ff6e9db into master Apr 7, 2026
22 checks passed
@DanielZhangQD DanielZhangQD deleted the 5022 branch April 7, 2026 02:26
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.

2 participants