diff --git a/docusaurus/docs/commands.mdx b/docusaurus/docs/commands.mdx index 98ca0de..1d8730e 100644 --- a/docusaurus/docs/commands.mdx +++ b/docusaurus/docs/commands.mdx @@ -638,7 +638,7 @@ This document provides a comprehensive reference for all available CLI commands Create a cluster with custom settings: $ v8x cluster create mycluster --cloud-account-id 16 --settings - '{"autoscaler_enabled":true}' + '{"kubeflow_enabled":true}' Create a cluster with an app: $ v8x cluster create mycluster --cloud-account-id 16 --app @@ -681,7 +681,7 @@ This document provides a comprehensive reference for all available CLI commands │ [default: slurm] │ │ --settings -s <str> Cluster settings as JSON │ │ string (e.g., │ -│ '{"autoscaler_enabled":tr… │ +│ '{"kubeflow_enabled":tr… │ │ --config -c <str> Cluster config as JSON │ │ string (e.g., │ │ '{"key":"value"}'). │ @@ -883,7 +883,7 @@ This document provides a comprehensive reference for all available CLI commands v8x cluster update my-cluster --status ready # Replace all settings - v8x cluster update my-cluster --settings '{"autoscaler_enabled": true}' + v8x cluster update my-cluster --settings '{"kubeflow_enabled": true}' # Merge new settings with existing settings v8x cluster update my-cluster --settings '{"new_key": "value"}' --merge diff --git a/pyproject.toml b/pyproject.toml index 4544595..9aa0887 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -28,7 +28,7 @@ dependencies = [ "pyyaml>=6.0.3", "pyopenssl>=25.3.0", "qrcode>=8.0.0", - "vantage-sdkpy>=0.1.23", + "vantage-sdkpy>=0.1.24", ] [tool.hatch.build.targets.wheel] diff --git a/tests/unit/test_cluster_sdk_delegation.py b/tests/unit/test_cluster_sdk_delegation.py index 6e6af0a..c30e9b0 100644 --- a/tests/unit/test_cluster_sdk_delegation.py +++ b/tests/unit/test_cluster_sdk_delegation.py @@ -962,7 +962,7 @@ async def test_disable_service_heavy_cascade_delegates_to_workflow_sdk( async def test_cluster_update_triggers_workflow_sdk(monkeypatch: pytest.MonkeyPatch): ctx = _ctx() cluster_with_settings = SimpleNamespace(client_id="client-1") - merged_settings = {"autoscaler_enabled": True} + merged_settings = {"kubeflow_enabled": True} monkeypatch.setattr( "v8x.commands.cluster.update._merge_and_validate_settings", AsyncMock(return_value=(cluster_with_settings, merged_settings)), @@ -998,7 +998,7 @@ async def test_cluster_update_triggers_workflow_sdk(monkeypatch: pytest.MonkeyPa cluster_name="cluster-a", description=None, status=None, - settings='{"autoscaler_enabled": true}', + settings='{"kubeflow_enabled": true}', settings_file=None, config=None, config_file=None, diff --git a/tests/unit/test_lxd_slurm_provider_command.py b/tests/unit/test_lxd_slurm_provider_command.py index ab2f8bb..6bb5840 100644 --- a/tests/unit/test_lxd_slurm_provider_command.py +++ b/tests/unit/test_lxd_slurm_provider_command.py @@ -76,7 +76,8 @@ def fake_run(cmd: list[str], **_: object) -> subprocess.CompletedProcess[str]: vantage_cluster_ctx=_cluster_ctx( { "default_control_plane_compute_pool": { - "name": "control-plane", "instance_type": "control-plane-smx" + "name": "control-plane", + "instance_type": "control-plane-smx", } } ), @@ -142,3 +143,81 @@ def fake_run(cmd: list[str], **_: object) -> subprocess.CompletedProcess[str]: assert cmd[cmd.index("--containerd-device") + 1] == "/dev/disk/by-id/custom-containerd" assert cmd[cmd.index("--containerd-disk-size") + 1] == "250" assert cmd[cmd.index("--containerd-storage-pool") + 1] == "fast-pool" + + +def test_provider_command_reads_organization_from_the_nested_keycloak_block(monkeypatch) -> None: + """The gateway Istio org-claim policy depends on these two flags. + + vdeployer nests keycloak settings, so this read moved from + settings["keycloak_organization_id"] to settings["keycloak"]["organization_id"]. + Renaming only the *write* sites would have left this returning None and + dropped --organization-id/--organization-name from the command -- disabling + the auth policy with no error and no log line. + """ + captured: dict[str, list[str]] = {} + + def fake_run(cmd: list[str], **_: object) -> subprocess.CompletedProcess[str]: + captured["cmd"] = cmd + return subprocess.CompletedProcess(cmd, 0, stdout="", stderr="") + + monkeypatch.setattr(lxd_app.subprocess, "run", fake_run) + + lxd_app._run_vantage_provider_provision( + ctx=_ctx(), + vantage_cluster_ctx=_cluster_ctx( + {"keycloak": {"organization_id": "org-uuid", "organization_name": "Acme"}} + ), + binary_path=Path("/tmp/vantage-provider"), + ) + + cmd = captured["cmd"] + assert cmd[cmd.index("--organization-id") + 1] == "org-uuid" + assert cmd[cmd.index("--organization-name") + 1] == "Acme" + + +def test_provider_command_omits_organization_flags_when_absent(monkeypatch) -> None: + """No keycloak block at all must not raise -- older clusters have none.""" + captured: dict[str, list[str]] = {} + + def fake_run(cmd: list[str], **_: object) -> subprocess.CompletedProcess[str]: + captured["cmd"] = cmd + return subprocess.CompletedProcess(cmd, 0, stdout="", stderr="") + + monkeypatch.setattr(lxd_app.subprocess, "run", fake_run) + + lxd_app._run_vantage_provider_provision( + ctx=_ctx(), + vantage_cluster_ctx=_cluster_ctx({}), + binary_path=Path("/tmp/vantage-provider"), + ) + + cmd = captured["cmd"] + assert "--organization-id" not in cmd + assert "--organization-name" not in cmd + + +def test_provider_command_ignores_the_stale_flat_organization_keys(monkeypatch) -> None: + """A pre-nesting settings blob must not silently keep working here. + + If this passed, it would mean the read still honours the flat keys and the + migration is incomplete somewhere. + """ + captured: dict[str, list[str]] = {} + + def fake_run(cmd: list[str], **_: object) -> subprocess.CompletedProcess[str]: + captured["cmd"] = cmd + return subprocess.CompletedProcess(cmd, 0, stdout="", stderr="") + + monkeypatch.setattr(lxd_app.subprocess, "run", fake_run) + + lxd_app._run_vantage_provider_provision( + ctx=_ctx(), + vantage_cluster_ctx=_cluster_ctx( + {"keycloak_organization_id": "org-uuid", "keycloak_organization_name": "Acme"} + ), + binary_path=Path("/tmp/vantage-provider"), + ) + + cmd = captured["cmd"] + assert "--organization-id" not in cmd + assert "--organization-name" not in cmd diff --git a/tests/unit/test_vdeployer_nested_settings.py b/tests/unit/test_vdeployer_nested_settings.py new file mode 100644 index 0000000..78b7452 --- /dev/null +++ b/tests/unit/test_vdeployer_nested_settings.py @@ -0,0 +1,84 @@ +"""vdeployer settings must be sent in the nested shape it now expects. + +`vdeployer` moved keycloak_client_id / client_secret / organization_id / +organization_name under a nested `keycloak:` config class as a clean break -- +no pydantic aliases. Any flat key v8x sends now binds to nothing and is dropped +by `extra="ignore"`, silently. `vdeployer` derives cluster_name from client_id +and raises ValueError building the tunnel URL without it, so a regression here +breaks deploys rather than degrading them. + +These assert the wire shape at the boundary, which is the thing that has to stay +right; the flat form produced no error anywhere, which is why it needs a test +rather than trust. +""" + +from __future__ import annotations + +from types import SimpleNamespace + +import pytest + +from v8x.commands.cluster.utils import build_vdeployer_settings + + +def _cluster(**overrides): + base = { + "name": "mycluster", + "client_id": "cid-abc", + "client_secret": "sec-xyz", + "sssd_binder_password": "pw", + "creation_parameters": {}, + } + base.update(overrides) + return SimpleNamespace(**base) + + +def _persona(org_id: str | None = "org-uuid"): + return SimpleNamespace(identity_data=SimpleNamespace(org_id=org_id)) + + +def test_keycloak_credentials_are_sent_nested_not_flat() -> None: + out = build_vdeployer_settings({}, "lxd", _cluster(), _persona()) + + assert out["keycloak"] == { + "client_id": "cid-abc", + "client_secret": "sec-xyz", + "organization_id": "org-uuid", + } + assert not [k for k in out if k.startswith("keycloak_")], ( + "flat keycloak_* keys bind to nothing on vdeployer and are dropped silently" + ) + + +def test_a_caller_supplied_keycloak_block_survives() -> None: + """Merged, not assigned over -- organization_name has no other source.""" + out = build_vdeployer_settings( + {"keycloak": {"organization_name": "Acme"}}, "lxd", _cluster(), _persona() + ) + + assert out["keycloak"]["organization_name"] == "Acme" + assert out["keycloak"]["client_id"] == "cid-abc" + + +def test_the_callers_settings_dict_is_not_mutated() -> None: + """`dict(settings)` is a shallow copy, so a nested block must be rebuilt.""" + caller = {"keycloak": {"organization_name": "Acme"}} + build_vdeployer_settings(caller, "lxd", _cluster(), _persona()) + + assert caller == {"keycloak": {"organization_name": "Acme"}} + + +def test_unrelated_settings_pass_through() -> None: + out = build_vdeployer_settings({"kubeflow_enabled": True}, "aws", _cluster(), _persona()) + + assert out["kubeflow_enabled"] is True + assert out["provider"] == "aws" + assert out["cluster_name"] == "mycluster" + + +def test_missing_org_context_still_aborts() -> None: + """The nesting change must not have weakened this guard.""" + from vantage_sdk.exceptions import Abort + + with pytest.raises(Abort): + build_vdeployer_settings({}, "lxd", _cluster(), _persona(org_id=None)) diff --git a/uv.lock b/uv.lock index 7f6e988..7747182 100644 --- a/uv.lock +++ b/uv.lock @@ -1645,7 +1645,7 @@ requires-dist = [ { name = "snick", specifier = ">=3.0.0" }, { name = "textual", specifier = ">=7.5.0" }, { name = "typer", specifier = ">=0.21.1" }, - { name = "vantage-sdkpy", specifier = ">=0.1.23" }, + { name = "vantage-sdkpy", specifier = ">=0.1.24" }, { name = "websockets", specifier = ">=16.0" }, ] provides-extras = ["dev"] @@ -1658,7 +1658,7 @@ dev = [ [[package]] name = "vantage-sdkpy" -version = "0.1.23" +version = "0.1.24" source = { registry = "https://pypi.org/simple" } dependencies = [ { name = "aiohttp" }, @@ -1677,9 +1677,9 @@ dependencies = [ { name = "snick" }, { name = "typer" }, ] -sdist = { url = "https://files.pythonhosted.org/packages/1f/a1/5bededa9dcd53482d816b7d929207d1958f4dd13a6bfc0b71dc2af02a3b5/vantage_sdkpy-0.1.23.tar.gz", hash = "sha256:09e79005320f1cc90b4e3d7a2e5087f790eb52023900459c6eefc091d1d230f5", size = 66804, upload-time = "2026-07-26T20:27:57.887Z" } +sdist = { url = "https://files.pythonhosted.org/packages/6c/84/2af96d2b71f9472022203546924f3b3cd641c32ad9ee1c46c4da1a943e7f/vantage_sdkpy-0.1.24.tar.gz", hash = "sha256:9ed1e5a78d51a1de8b4a00c58a7972a67bb4e4c896909b22a05af0889a4c7c25", size = 67097, upload-time = "2026-07-26T22:39:58.288Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/77/33/7d4318ddc48e458c1c5eb1af21d782cbf90c6fe6a5ffdcb4ca75302a30f0/vantage_sdkpy-0.1.23-py3-none-any.whl", hash = "sha256:4473b167fba64b2d1b1c114fbfdc7bbdf77f5179c85638b951893e7cfc121875", size = 106230, upload-time = "2026-07-26T20:27:56.383Z" }, + { url = "https://files.pythonhosted.org/packages/6d/18/2daecff763a6ae1b2a4ad7010230c50c1e4fe7ce2b658994416c82b06ccc/vantage_sdkpy-0.1.24-py3-none-any.whl", hash = "sha256:2b7d9db213943900107ae42aee7fd1de0b8806c5336044e44c9e96dcd7622f82", size = 106543, upload-time = "2026-07-26T22:39:56.954Z" }, ] [[package]] diff --git a/v8x/apps/lxd/slurm/app.py b/v8x/apps/lxd/slurm/app.py index 779ecf4..9b6d90a 100644 --- a/v8x/apps/lxd/slurm/app.py +++ b/v8x/apps/lxd/slurm/app.py @@ -19,7 +19,7 @@ import subprocess import urllib.parse from pathlib import Path -from typing import Annotated, Optional +from typing import Annotated, Any, Optional import httpx import typer @@ -53,8 +53,16 @@ logger = logging.getLogger("v8x.apps.lxd.slurm") -def _asc(ctx: VantageClusterContext, key: str, default: object = None) -> object: - """Read a key from the nested ``vantage_lxd_autoscaler`` settings block.""" +def _asc(ctx: VantageClusterContext, key: str, default: object = None) -> Any: + """Read a key from the nested ``vantage_lxd_autoscaler`` settings block. + + Returns ``Any`` rather than ``object`` because the value comes out of an + untyped settings dict and callers legitimately feed it to ``int()`` and to + string contexts. ``object`` was not more truthful -- it only moved the + problem to the call sites, where pyright rejected + ``int(_asc(...) or DEFAULT)`` as "object is incompatible with + SupportsInt". + """ return ctx.settings.get("vantage_lxd_autoscaler", {}).get(key, default) @@ -300,10 +308,11 @@ def _run_vantage_provider_provision( # noqa: C901 cmd.extend(["--extra-api-san", extra_sans]) # Organization info for gateway-level Istio auth policy - if org_id := vantage_cluster_ctx.settings.get("keycloak_organization_id"): + keycloak_settings = vantage_cluster_ctx.settings.get("keycloak") or {} + if org_id := keycloak_settings.get("organization_id"): cmd.extend(["--organization-id", org_id]) - if org_name := vantage_cluster_ctx.settings.get("keycloak_organization_name"): + if org_name := keycloak_settings.get("organization_name"): cmd.extend(["--organization-name", org_name]) if ctx.obj.cloud_config_metadata.get("dev_mode"): @@ -829,9 +838,14 @@ async def _trigger_vdeployer_deploy( # Build settings from the cluster's stored creation_parameters.settings deploy_settings = dict(vctx.settings) - deploy_settings["keycloak_client_id"] = vctx.client_id - deploy_settings["keycloak_client_secret"] = vctx.client_secret - deploy_settings["keycloak_organization_id"] = vctx.org_id + # vdeployer nests these under `keycloak:`; the flat keys it used to + # accept now bind to nothing and are dropped by `extra="ignore"`. + deploy_settings["keycloak"] = { + **(deploy_settings.get("keycloak") or {}), + "client_id": vctx.client_id, + "client_secret": vctx.client_secret, + "organization_id": vctx.org_id, + } deploy_settings["sssd_binder_password"] = vctx.sssd_binder_password deploy_settings["vantage_url"] = ctx.obj.settings.vantage_url if vctx.jupyterhub_token: diff --git a/v8x/commands/cluster/create.py b/v8x/commands/cluster/create.py index f723537..3a8ae98 100644 --- a/v8x/commands/cluster/create.py +++ b/v8x/commands/cluster/create.py @@ -245,7 +245,7 @@ async def create_cluster( # noqa: C901 typer.Option( "--settings", "-s", - help="Cluster settings as JSON string (e.g., '{\"autoscaler_enabled\":true}').", + help="Cluster settings as JSON string (e.g., '{\"kubeflow_enabled\":true}').", ), ] = None, config: Annotated[ @@ -330,7 +330,7 @@ async def create_cluster( # noqa: C901 $ v8x cluster create mycluster --cloud-account my-aws-account Create a cluster with custom settings: - $ v8x cluster create mycluster --cloud-account-id 16 --settings '{"autoscaler_enabled":true}' + $ v8x cluster create mycluster --cloud-account-id 16 --settings '{"kubeflow_enabled":true}' Create a cluster with an app: $ v8x cluster create mycluster --cloud-account-id 16 --app slurm-lxd-localhost @@ -553,8 +553,11 @@ async def create_cluster( # noqa: C901 # Pass organization identity to vdeployer for Istio auth policies if ctx.obj.persona and ctx.obj.persona.identity_data: identity = ctx.obj.persona.identity_data - vdeployer_settings_dict["keycloak_organization_id"] = identity.org_id - vdeployer_settings_dict["keycloak_organization_name"] = identity.org_name + vdeployer_settings_dict["keycloak"] = { + **(vdeployer_settings_dict.get("keycloak") or {}), + "organization_id": identity.org_id, + "organization_name": identity.org_name, + } # Set vantage_url from CLI context (applies to all providers) vdeployer_settings_dict["vantage_url"] = ctx.obj.settings.vantage_url @@ -595,15 +598,15 @@ async def create_cluster( # noqa: C901 "http://" ) vdeployer_settings_dict["local_registry"] = local_registry - asc.setdefault("cloud_init", {})["local_registry"] = ( - cloud_account_attributes["local_registry"] - ) + asc.setdefault("cloud_init", {})["local_registry"] = cloud_account_attributes[ + "local_registry" + ] cloud_init = asc.setdefault("cloud_init", {}) if "vantage_node_security_binary_url" in cloud_account_attributes: - cloud_init["node_security_binary_url"] = ( - cloud_account_attributes["vantage_node_security_binary_url"] - ) + cloud_init["node_security_binary_url"] = cloud_account_attributes[ + "vantage_node_security_binary_url" + ] if "dev_mode" in cloud_account_attributes: cloud_init["dev_mode"] = cloud_account_attributes["dev_mode"] diff --git a/v8x/commands/cluster/update.py b/v8x/commands/cluster/update.py index ae68691..f31edde 100644 --- a/v8x/commands/cluster/update.py +++ b/v8x/commands/cluster/update.py @@ -235,7 +235,7 @@ async def update_cluster( v8x cluster update my-cluster --status ready # Replace all settings - v8x cluster update my-cluster --settings '{"autoscaler_enabled": true}' + v8x cluster update my-cluster --settings '{"kubeflow_enabled": true}' # Merge new settings with existing settings v8x cluster update my-cluster --settings '{"new_key": "value"}' --merge diff --git a/v8x/commands/cluster/utils.py b/v8x/commands/cluster/utils.py index aa71d4e..131a230 100644 --- a/v8x/commands/cluster/utils.py +++ b/v8x/commands/cluster/utils.py @@ -99,9 +99,14 @@ def build_vdeployer_settings( vdeployer_settings = dict(settings) vdeployer_settings["provider"] = provider vdeployer_settings["cluster_name"] = cluster.name - vdeployer_settings["keycloak_client_id"] = cluster.client_id - vdeployer_settings["keycloak_client_secret"] = cluster.client_secret - vdeployer_settings["keycloak_organization_id"] = org_id + # vdeployer nests these under `keycloak:`; the flat keys it used to + # accept now bind to nothing and are dropped by `extra="ignore"`. + vdeployer_settings["keycloak"] = { + **(vdeployer_settings.get("keycloak") or {}), + "client_id": cluster.client_id, + "client_secret": cluster.client_secret, + "organization_id": org_id, + } vdeployer_settings["sssd_binder_password"] = cluster.sssd_binder_password creation_params = getattr(cluster, "creation_parameters", {}) or {} diff --git a/v8x/commands/vdeployer_web/deploy.py b/v8x/commands/vdeployer_web/deploy.py index e42646f..7bc9607 100644 --- a/v8x/commands/vdeployer_web/deploy.py +++ b/v8x/commands/vdeployer_web/deploy.py @@ -231,13 +231,18 @@ async def _deploy_no_wan( # Add required fields settings_dict["provider"] = provider settings_dict["cluster_name"] = cluster_name - settings_dict["keycloak_client_id"] = client_id - settings_dict["keycloak_client_secret"] = client_secret settings_dict["sssd_binder_password"] = sssd_password # Use org_id from parameter or fall back to current profile keycloak_org_id = org_id or ctx.obj.persona.identity_data.org_id - settings_dict["keycloak_organization_id"] = keycloak_org_id + # vdeployer nests these under `keycloak:`; the flat keys it used to + # accept now bind to nothing and are dropped by `extra="ignore"`. + settings_dict["keycloak"] = { + **(settings_dict.get("keycloak") or {}), + "client_id": client_id, + "client_secret": client_secret, + "organization_id": keycloak_org_id, + } # Construct the vdeployer-web URL vdeployer_url = get_vdeployer_web_url( @@ -361,10 +366,13 @@ async def _deploy_from_api(ctx: typer.Context, cluster_name: str): log_message="Cluster missing sssd_binder_password", ) - settings_dict["keycloak_client_id"] = cluster.client_id - settings_dict["keycloak_client_secret"] = cluster.client_secret - # settings_dict["keycloak_organization_id"] = ctx.obj.persona.identity_data.org_id - # settings_dict["keycloak_organization_name"] = ctx.obj.persona.identity_data.org_name + # vdeployer nests these under `keycloak:`; the flat keys it used to + # accept now bind to nothing and are dropped by `extra="ignore"`. + settings_dict["keycloak"] = { + **(settings_dict.get("keycloak") or {}), + "client_id": cluster.client_id, + "client_secret": cluster.client_secret, + } settings_dict["sssd_binder_password"] = cluster.sssd_binder_password # Pass the JupyterHub service token from creation_parameters so vdeployer