Repository: skypilot-org/skypilot
Pull request #6037 (2025-06-19 00:17:45)

[auth] read client id/secret from k8s secret
<!-- Describe the changes in this PR -->
Add a config option so that the OAuth Client ID/Client Secret used the oauth2-proxy can be specified in a kubernetes secret rather than in the helm values.
If using this feature, the secret should be defined as follows:
```
apiVersion: v1
kind: Secret
metadata:
  name: oauth2-proxy-credentials
  namespace: skypilot
data:
  client-id: XXXXXXX
  client-secret: XXXXXX
```
Where the namespace and name match those specified in helm

<!-- Describe the tests ran -->
<!-- Unit tests (tests/test_*.py) are part of GitHub CI; below are tests that launch on the cloud. -->

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)
- [ ] 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)

<!-- CI commands (/-prefixed) can only be triggered by repo members -->



Patch:
diff --git a/charts/skypilot/templates/oauth2-proxy-deployment.yaml b/charts/skypilot/templates/oauth2-proxy-deployment.yaml
index 6fd12809a..fc88bc94f 100644
--- a/charts/skypilot/templates/oauth2-proxy-deployment.yaml
+++ b/charts/skypilot/templates/oauth2-proxy-deployment.yaml
@@ -3,12 +3,13 @@
 {{- if not (index $oauth2 "oidc-issuer-url") -}}
 {{- fail "ingress.oauth2-proxy.oidc-issuer-url is required when OAuth2 proxy is enabled" -}}
 {{- end -}}
-{{- if eq (index $oauth2 "client-id") "" -}}
-{{- fail "ingress.oauth2-proxy.client-id is required when OAuth2 proxy is enabled" -}}
+{{- if and (not (index $oauth2 "client-details-from-secret")) (eq (index $oauth2 "client-id") "") -}}
+{{- fail "ingress.oauth2-proxy.client-id is required when OAuth2 proxy is enabled and client-details-from-secret is not set" -}}
 {{- end -}}
-{{- if eq (index $oauth2 "client-secret") "" -}}
-{{- fail "ingress.oauth2-proxy.client-secret is required when OAuth2 proxy is enabled" -}}
+{{- if and (not (index $oauth2 "client-details-from-secret")) (eq (index $oauth2 "client-secret") "") -}}
+{{- fail "ingress.oauth2-proxy.client-secret is required when OAuth2 proxy is enabled and client-details-from-secret is not set" -}}
 {{- end -}}
+
 apiVersion: apps/v1
 kind: Deployment
 metadata:
@@ -60,9 +61,23 @@ spec:
         {{- end }}
         env:
         - name: OAUTH2_PROXY_CLIENT_ID
+          {{- if (index .Values.ingress "oauth2-proxy" "client-details-from-secret") }}
+          valueFrom:
+            secretKeyRef:
+              name: {{ index .Values.ingress "oauth2-proxy" "client-details-from-secret" }}
+              key: client-id
+          {{- else }}
           value: {{ index .Values.ingress "oauth2-proxy" "client-id" | quote }}
+          {{- end }}
         - name: OAUTH2_PROXY_CLIENT_SECRET
+          {{- if (index .Values.ingress "oauth2-proxy" "client-details-from-secret") }}
+          valueFrom:
+            secretKeyRef:
+              name: {{ index .Values.ingress "oauth2-proxy" "client-details-from-secret" }}
+              key: client-secret
+          {{- else }}
           value: {{ index .Values.ingress "oauth2-proxy" "client-secret" | quote }}
+          {{- end }}
         - name: SERVICE_NAME
           value: {{ .Release.Name }}
         - name: OAUTH2_PROXY_COOKIE_SECRET
diff --git a/charts/skypilot/values.yaml b/charts/skypilot/values.yaml
index cd00eeeca..0b19814a4 100644
--- a/charts/skypilot/values.yaml
+++ b/charts/skypilot/values.yaml
@@ -142,6 +142,12 @@ ingress:
     client-id: ""
     # Client Secret from OIDC provider (Okta) (required when enabled=true)
     client-secret: ""
+    # Alternative: Get both client ID and client secret from a Kubernetes secret
+    # If set to a secret name, both client-id and client-secret values above are ignored
+    # The secret must contain keys named "client-id" and "client-secret"
+    # If empty/null, the above client-id and client-secret values are used instead
+    # See https://docs.skypilot.co/en/latest/reference/api-server/examples/api-server-auth-proxy.html#auth-proxy-client-secret
+    client-details-from-secret: ""
     # Use HTTPS (set to true for production environments)
     use-https: false
     # Email domains to allow (use "*" for all domains)
diff --git a/docs/source/examples/managed-jobs.rst b/docs/source/examples/managed-jobs.rst
index 62d01dd47..5ffe276b5 100644
--- a/docs/source/examples/managed-jobs.rst
+++ b/docs/source/examples/managed-jobs.rst
@@ -196,39 +196,6 @@ The SkyPilot dashboard, ``sky dashboard`` has a **Jobs** page that shows all man
 The UI shows the same information as the CLI ``sky jobs queue -au``.
 
 
-.. _job-priority:
-
-Job queueing and priority
--------------------------
-
-Job queueing and priority are supported by managed jobs.
-
-
-Specify job priority by setting the ``job.priority`` field in the :ref:`SkyPilot YAML <yaml-spec-job-priority>`.
-
-.. code-block:: yaml
-
-  job:
-    # Priority of the job, between 0 and 1000 (default: 500).
-    #
-    # A higher value means that the job is higher priority. High priority jobs
-    # are scheduled sooner and will block lower priority jobs from starting
-    # until the high priority jobs have started.
-    priority: 500
-
-
-All jobs are submitted to a queue. When the scheduler is selecting the next job
-to schedule, it will select the highest priority job.
-
-If a high priority job is still launching, lower priority jobs will not be
-scheduled (i.e., stay in pending).
-
-.. image:: https://i.imgur.com/8H8ictY.png
-  :width: 800
-  :alt: Job queueing and priority
-
-
-
 .. _spot-jobs:
 
 Running on spot instances
diff --git a/docs/source/reference/api-server/examples/api-server-auth-proxy.rst b/docs/source/reference/api-server/examples/api-server-auth-proxy.rst
index 6c46aa6f2..2c3d73a91 100644
--- a/docs/source/reference/api-server/examples/api-server-auth-proxy.rst
+++ b/docs/source/reference/api-server/examples/api-server-auth-proxy.rst
@@ -118,6 +118,23 @@ Use ``helm upgrade`` to redeploy the API server helm chart with the ``skypilot-o
       --set ingress.oauth2-proxy.client-id=<CLIENT ID> \
       --set ingress.oauth2-proxy.client-secret=<CLIENT SECRET>
 
+.. _auth-proxy-client-secret:
+
+For better security, you can also store the client details in a Kubernetes secret instead of passing them as Helm values:
+
+.. code-block:: console
+
+    $ # Create a secret with your Okta credentials
+    $ kubectl create secret generic oauth2-proxy-credentials -n $NAMESPACE \
+      --from-literal=client-id=<CLIENT ID> \
+      --from-literal=client-secret=<CLIENT SECRET>
+
+    $ # Deploy using the secret
+    $ helm upgrade -n $NAMESPACE $RELEASE_NAME skypilot/skypilot-nightly --devel --reuse-values \
+      --set ingress.oauth2-proxy.enabled=true \
+      --set ingress.oauth2-proxy.oidc-issuer-url=https://<OKTA URL>.okta.com \
+      --set ingress.oauth2-proxy.client-details-from-secret=oauth2-proxy-credentials
+
 To make sure it's working, visit your endpoint URL in a browser. You should be redirected to Okta to sign in.
 
 Now, you can use ``sky api login -e <ENDPOINT>`` to go though the login flow for the CLI.
diff --git a/docs/source/reference/yaml-spec.rst b/docs/source/reference/yaml-spec.rst
index 2dee76ce8..416a826e0 100644
--- a/docs/source/reference/yaml-spec.rst
+++ b/docs/source/reference/yaml-spec.rst
@@ -1308,35 +1308,3 @@ Port to run your service on each replica.
 
   resources:
     ports: 8080
-
-Managed jobs
-============
-
-When creating a managed job, you can add an optional ``job`` section to your SkyPilot YAML for additional configuration.
-
-Syntax
-
-.. parsed-literal::
-
-  job:
-    :ref:`priority <yaml-spec-job-priority>`: 200
-
-
-Fields
-----------
-
-.. _yaml-spec-job-priority:
-
-``job.priority``
-~~~~~~~~~~~~~~~~
-
-Priority of the job, between 0 and 1000 (default: 500).
-
-Set the queuing priority of the job. A higher value means that the job is higher
-priority. High priority jobs are scheduled sooner and will block lower priority
-jobs from starting until the high priority jobs have started.
-
-.. code-block:: yaml
-
-  job:
-    priority: 200
diff --git a/sky/client/cli/command.py b/sky/client/cli/command.py
index afba9a55c..7c381b435 100644
--- a/sky/client/cli/command.py
+++ b/sky/client/cli/command.py
@@ -3763,11 +3763,13 @@ def jobs():
               type=str,
               help='Recovery strategy to use for managed jobs.')
 @click.option('--priority',
-              type=click.IntRange(0, 1000),
+              type=click.IntRange(constants.MIN_PRIORITY,
+                                  constants.MAX_PRIORITY),
               default=None,
               show_default=True,
-              help=('Job priority from 0 to 1000. A higher number is higher '
-                    'priority. Default is 500.'))
+              help=f'Job priority from ({constants.MIN_PRIORITY} '
+              f'to {constants.MAX_PRIORITY}). '
+              f'Default: {constants.DEFAULT_PRIORITY}.')
 @click.option(
     '--detach-run',
     '-d',
diff --git a/sky/jobs/constants.py b/sky/jobs/constants.py
index 5c91e5d72..caad4c0e3 100644
--- a/sky/jobs/constants.py
+++ b/sky/jobs/constants.py
@@ -70,5 +70,3 @@ DASHBOARD_SETUP_CMD = (
     f'(nohup {skylet_constants.SKY_PYTHON_CMD} -m sky.jobs.dashboard.dashboard '
     '>> ~/.sky/job-dashboard.log 2>&1 &); '
     'fi')
-
-DEFAULT_PRIORITY = 500
diff --git a/sky/jobs/scheduler.py b/sky/jobs/scheduler.py
index a4197b75d..ba93733cb 100644
--- a/sky/jobs/scheduler.py
+++ b/sky/jobs/scheduler.py
@@ -331,10 +331,13 @@ if __name__ == '__main__':
     parser.add_argument('--env-file',
                         type=str,
                         help='The path to the controller env file.')
-    parser.add_argument('--priority',
-                        type=int,
-                        default=500,
-                        help='Job priority (0-1000). Default: 500.')
+    parser.add_argument(
+        '--priority',
+        type=int,
+        default=constants.DEFAULT_PRIORITY,
+        help=
+        f'Job priority ({constants.MIN_PRIORITY} to {constants.MAX_PRIORITY}).'
+        f' Default: {constants.DEFAULT_PRIORITY}.')
     args = parser.parse_args()
     submit_job(args.job_id, args.dag_yaml, args.user_yaml_path, args.env_file,
                args.priority)
diff --git a/sky/jobs/server/core.py b/sky/jobs/server/core.py
index c4c9e00d4..6079c7a18 100644
--- a/sky/jobs/server/core.py
+++ b/sky/jobs/server/core.py
@@ -145,10 +145,13 @@ def launch(
             priority = task_priority
 
     if priority is None:
-        priority = managed_job_constants.DEFAULT_PRIORITY
+        priority = skylet_constants.DEFAULT_PRIORITY
 
-    if priority < 0 or priority > 1000:
-        raise ValueError(f'Priority must be between 0 and 1000, got {priority}')
+    if (priority < skylet_constants.MIN_PRIORITY or
+            priority > skylet_constants.MAX_PRIORITY):
+        raise ValueError(
+            f'Priority must be between {skylet_constants.MIN_PRIORITY}'
+            f' and {skylet_constants.MAX_PRIORITY}, got {priority}')
 
     dag_utils.fill_default_config_in_dag_for_job_launch(dag)
 
diff --git a/sky/jobs/state.py b/sky/jobs/state.py
index 9c9a98675..ed01b02fb 100644
--- a/sky/jobs/state.py
+++ b/sky/jobs/state.py
@@ -113,7 +113,7 @@ def create_table(cursor, conn):
 
     # `job_info` contains the mapping from job_id to the job_name, as well as
     # information used by the scheduler.
-    cursor.execute("""\
+    cursor.execute(f"""\
         CREATE TABLE IF NOT EXISTS job_info (
         spot_job_id INTEGER PRIMARY KEY AUTOINCREMENT,
         name TEXT,
@@ -123,7 +123,7 @@ def create_table(cursor, conn):
         env_file_path TEXT,
         user_hash TEXT,
         workspace TEXT DEFAULT NULL,
-        priority INTEGER DEFAULT 500,
+        priority INTEGER DEFAULT {constants.DEFAULT_PRIORITY},
         entrypoint TEXT DEFAULT NULL,
         original_user_yaml_path TEXT DEFAULT NULL)""")
 
@@ -148,12 +148,13 @@ def create_table(cursor, conn):
                                  'TEXT DEFAULT NULL',
                                  value_to_replace_existing_entries='default')
 
-    db_utils.add_column_to_table(cursor,
-                                 conn,
-                                 'job_info',
-                                 'priority',
-                                 'INTEGER',
-                                 value_to_replace_existing_entries=500)
+    db_utils.add_column_to_table(
+        cursor,
+        conn,
+        'job_info',
+        'priority',
+        'INTEGER',
+        value_to_replace_existing_entries=constants.DEFAULT_PRIORITY)
 
     db_utils.add_column_to_table(cursor, conn, 'job_info', 'entrypoint', 'TEXT')
     db_utils.add_column_to_table(cursor, conn, 'job_info',
diff --git a/sky/jobs/utils.py b/sky/jobs/utils.py
index eab88e5d2..22a7f1ac9 100644
--- a/sky/jobs/utils.py
+++ b/sky/jobs/utils.py
@@ -945,7 +945,7 @@ def dump_managed_job_queue() -> str:
     # Figure out what the highest priority blocking job is. We need to know in
     # order to determine if other jobs are blocked by a higher priority job, or
     # just by the limited controller resources.
-    highest_blocking_priority = 0
+    highest_blocking_priority = constants.MIN_PRIORITY
     for job in jobs:
         if job['schedule_state'] not in (
                 # LAUNCHING and ALIVE_BACKOFF jobs will block other jobs with
diff --git a/sky/resources.py b/sky/resources.py
index 608aa3e1c..f02792358 100644
--- a/sky/resources.py
+++ b/sky/resources.py
@@ -225,7 +225,7 @@ class Resources:
           autostop: the autostop configuration to use. For launched resources,
             may or may not correspond to the actual current autostop config.
           priority: the priority for this resource configuration. Must be an
-            integer from 0 to 1000, where higher values indicate higher priority.
+            integer from -1000 to 1000, where higher values indicate higher priority.
             If None, no priority is set.
           volumes: the volumes to mount on the instance.
           _docker_login_config: the docker configuration to use. This includes
@@ -631,7 +631,7 @@ class Resources:
     def priority(self) -> Optional[int]:
         """The priority for this resource configuration.
 
-        Higher values indicate higher priority. Valid range is 0-1000.
+        Higher values indicate higher priority. Valid range is -1000 to 1000.
         """
         return self._priority
 
@@ -824,14 +824,15 @@ class Resources:
         """Sets the priority for this resource configuration.
 
         Args:
-            priority: Priority value from 0 to 1000, where higher values
+            priority: Priority value from -1000 to 1000, where higher values
                 indicate higher priority. If None, no priority is set.
         """
         if priority is not None:
-            if not 0 <= priority <= 1000:
+            if not constants.MIN_PRIORITY <= priority <= constants.MAX_PRIORITY:
                 with ux_utils.print_exception_no_traceback():
-                    raise ValueError(f'Priority must be between 0 and 1000. '
-                                     f'Found: {priority}')
+                    raise ValueError(
+                        f'Priority must be between {constants.MIN_PRIORITY} and'
+                        f' {constants.MAX_PRIORITY}. Found: {priority}')
         self._priority = priority
 
     def _set_volumes(
diff --git a/sky/skylet/constants.py b/sky/skylet/constants.py
index 69267935b..aa9a7a4e0 100644
--- a/sky/skylet/constants.py
+++ b/sky/skylet/constants.py
@@ -468,3 +468,7 @@ MEMORY_SIZE_PATTERN = (
     f'{"|".join([unit.lower() for unit in MEMORY_SIZE_UNITS])}'
     ')?$/i')
 MEMORY_SIZE_PLUS_PATTERN = f'{MEMORY_SIZE_PATTERN[:-3]}+?$/i'
+
+MIN_PRIORITY = -1000
+MAX_PRIORITY = 1000
+DEFAULT_PRIORITY = 0
diff --git a/sky/utils/schemas.py b/sky/utils/schemas.py
index e465b83a5..25c60484b 100644
--- a/sky/utils/schemas.py
+++ b/sky/utils/schemas.py
@@ -291,8 +291,8 @@ def _get_single_resources_schema():
             'autostop': _AUTOSTOP_SCHEMA,
             'priority': {
                 'type': 'integer',
-                'minimum': 0,
-                'maximum': 1000,
+                'minimum': constants.MIN_PRIORITY,
+                'maximum': constants.MAX_PRIORITY,
             },
             # The following fields are for internal use only. Should not be
             # specified in the task config.


Test patch:
diff --git a/tests/common_test_fixtures.py b/tests/common_test_fixtures.py
index 26c3a3d41..3846fa3f7 100644
--- a/tests/common_test_fixtures.py
+++ b/tests/common_test_fixtures.py
@@ -283,7 +283,7 @@ def mock_job_table_one_job(monkeypatch):
             'task_id': 0,
             'task_name': 'test_task',
             'job_duration': 20,
-            'priority': 500,
+            'priority': constants.DEFAULT_PRIORITY,
         }
         return 0, message_utils.encode_payload([job_data]), ''
 
diff --git a/tests/unit_tests/test_resources.py b/tests/unit_tests/test_resources.py
index e1e36c51a..19105b944 100644
--- a/tests/unit_tests/test_resources.py
+++ b/tests/unit_tests/test_resources.py
@@ -877,14 +877,14 @@ def test_priority_basic():
     assert r.priority is None
 
     # Test with valid priority values
-    r = Resources(priority=0)
-    assert r.priority == 0
+    r = Resources(priority=constants.MIN_PRIORITY)
+    assert r.priority == constants.MIN_PRIORITY
 
-    r = Resources(priority=500)
-    assert r.priority == 500
+    r = Resources(priority=constants.DEFAULT_PRIORITY)
+    assert r.priority == constants.DEFAULT_PRIORITY
 
-    r = Resources(priority=1000)
-    assert r.priority == 1000
+    r = Resources(priority=constants.MAX_PRIORITY)
+    assert r.priority == constants.MAX_PRIORITY
 
     # Test with None explicitly
     r = Resources(priority=None)
@@ -894,12 +894,13 @@ def test_priority_basic():
 def test_priority_validation():
     """Test priority field validation with invalid values."""
     # Test invalid priority - below range
-    with pytest.raises(ValueError, match='Priority must be between 0 and 1000'):
-        Resources(priority=-1)
+    error_message = f'Priority must be between {constants.MIN_PRIORITY} and {constants.MAX_PRIORITY}'
+    with pytest.raises(ValueError, match=error_message):
+        Resources(priority=constants.MIN_PRIORITY - 1)
 
     # Test invalid priority - above range
-    with pytest.raises(ValueError, match='Priority must be between 0 and 1000'):
-        Resources(priority=1001)
+    with pytest.raises(ValueError, match=error_message):
+        Resources(priority=constants.MAX_PRIORITY + 1)
 
 
 def test_priority_yaml_serialization():
@@ -955,7 +956,7 @@ def test_priority_copy():
 def test_priority_with_any_of():
     """Test priority field works with any_of configuration."""
     config = {
-        'priority': 500,  # Base priority
+        'priority': constants.DEFAULT_PRIORITY,  # Base priority
         'any_of': [
             {
                 'cpus': 8,
@@ -980,7 +981,7 @@ def test_priority_with_any_of():
     r_cpus4 = next((r for r in resources_list if r.cpus == '4'), None)
 
     assert r_cpus8 is not None
-    assert r_cpus8.priority == 500  # Base priority
+    assert r_cpus8.priority == constants.DEFAULT_PRIORITY  # Base priority
 
     assert r_cpus4 is not None
     assert r_cpus4.priority == 800  # Override priority
diff --git a/tests/unit_tests/test_sky/utils/test_schemas.py b/tests/unit_tests/test_sky/utils/test_schemas.py
index 4c1a2e77d..08351d9bd 100644
--- a/tests/unit_tests/test_sky/utils/test_schemas.py
+++ b/tests/unit_tests/test_sky/utils/test_schemas.py
@@ -3,6 +3,7 @@ import unittest
 
 import jsonschema
 
+from sky.skylet import constants
 from sky.utils import schemas
 
 
@@ -140,10 +141,10 @@ class TestResourcesSchema(unittest.TestCase):
         # Invalid priority configurations
         invalid_priority_configs = [
             {
-                'priority': -1
+                'priority': constants.MIN_PRIORITY - 1
             },  # Below minimum
             {
-                'priority': 1001
+                'priority': constants.MAX_PRIORITY + 1
             },  # Above maximum
             {
                 'priority': 'high'