skypilot icon indicating copy to clipboard operation
skypilot copied to clipboard
verified publisher icon skypilot-org

[Serve] HTTPS Support

Open cblmemo opened this issue 1 year ago • 6 comments

Support HTTPS (SSL) Certificate on SkyServe Load Balancer.

Resolves (partially) #3198 . This PR only add TLS on load balancer - still need to figure out how to encrypt between LB and replica. #3458 might help this situation.

Tested (run the relevant ones):

  • [x] Code formatting: bash format.sh
  • [x] Any manual or new tests for this PR (please specify below)
    • [x] The example in this PR
  • [x] All Sky Serve smoke tests: pytest tests/test_smoke.py --serve
  • [x] Relevant individual smoke tests: pytest tests/test_smoke.py::test_skyserve_https
  • [ ] Backward compatibility tests: bash tests/backward_comaptibility_tests.sh

cblmemo avatar Mar 28 '24 06:03 cblmemo

Revamping this as a user is requesting the feature. Based on our previous discussion I disabled the update of the SSL, as it is not likely to update this - the most possible situation is the ssl file is not working and thus the service is failed anyway, so it should be fine to let users to restart the service.

cblmemo avatar Sep 05 '24 20:09 cblmemo

~~TODO: Remove http:// schema in smoke test.~~

Done!

cblmemo avatar Sep 17 '24 19:09 cblmemo

All Sky Serve smoke test passed and this is ready for another look @Michaelvll !

cblmemo avatar Sep 18 '24 00:09 cblmemo

Thanks for adding the example @cblmemo! Left several comments. Could we also add a smoke test for the HTTPS support?

We should also add a doc for this in another PR.

Also, does this mean the communication between load balancer and replicas is still going through non-secured HTTP, which might be fine for now, but would be good to mention it in comments or the doc.

Doc added in #3972

cblmemo avatar Sep 23 '24 18:09 cblmemo

@Michaelvll All comments resolved and now running the smoke test. PTAL agin!

cblmemo avatar Oct 24 '24 23:10 cblmemo

HTTPS smoke test passed!

cblmemo avatar Oct 24 '24 23:10 cblmemo

@Michaelvll PLAT, thx!

cblmemo avatar Nov 08 '24 01:11 cblmemo

Blocked by #4566. With that PR this passed all smoke tests.

cblmemo avatar Jan 15 '25 19:01 cblmemo

/smoke-test serve

cblmemo avatar Jan 17 '25 23:01 cblmemo

/smoke-test serve

cblmemo avatar Jan 19 '25 18:01 cblmemo

/smote-test serve::test_skyserve_https

cblmemo avatar Jan 19 '25 22:01 cblmemo

/smoke-test serve

cblmemo avatar Jan 19 '25 22:01 cblmemo

@zpoint , do you know the command to trigger one single smoke test?

cblmemo avatar Jan 19 '25 22:01 cblmemo

The failed smoke test passed independently. Merging now

cblmemo avatar Jan 20 '25 01:01 cblmemo

@zpoint , do you know the command to trigger one single smoke test?

Not supported now. I think it’s useful and will add a PR to support it. I’ll let you know once it’s done.

zpoint avatar Jan 20 '25 02:01 zpoint