Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from __future__ import annotations

import base64
import hashlib
import json
Expand All @@ -12,8 +14,12 @@
from dataclasses import dataclass
from pathlib import Path, PosixPath
from queue import Queue
from typing import TYPE_CHECKING
from urllib.parse import urlparse

if TYPE_CHECKING:
from jumpstarter.common.oci import OciCredentials

import click
import pexpect
import requests
Expand Down Expand Up @@ -137,7 +143,9 @@ def flash( # noqa: C901

if headers:
headers = self._validate_header_dict(headers)
oci_username, oci_password = self._resolve_oci_credentials(path, oci_username, oci_password)
oci_creds = self._resolve_oci_credentials(path, oci_username, oci_password)
oci_username = oci_creds.username
oci_password = oci_creds.plain_password
should_download_to_httpd = True
image_url = ""
original_http_url = None
Expand Down Expand Up @@ -1291,34 +1299,17 @@ def _validate_bearer_token(self, token: str | None) -> str | None:

return token

def _validate_oci_credentials(self, username: str | None, password: str | None) -> tuple[str | None, str | None]:
if username is not None:
username = username.strip()
if password is not None:
password = password.strip()

if username == "":
username = None
if password == "":
password = None

if bool(username) != bool(password):
raise click.ClickException(
"OCI authentication requires both OCI_USERNAME and OCI_PASSWORD "
"environment variables (or both oci_username and oci_password arguments)"
)

return username, password

def _resolve_oci_credentials(
self, path: PathBuf, username: str | None, password: str | None
) -> tuple[str | None, str | None]:
if username is None and password is None and path.startswith("oci://"):
from jumpstarter.common.oci import resolve_oci_credentials

username, password = resolve_oci_credentials(str(path))
) -> "OciCredentials":
from jumpstarter.common.oci import OciCredentials, resolve_oci_credentials

return self._validate_oci_credentials(username, password)
if username is not None or password is not None or str(path).startswith("oci://"):
try:
return resolve_oci_credentials(str(path), username=username, password=password)
except ValueError as err:
raise click.ClickException(str(err)) from err
return OciCredentials()

def _fls_oci_auth_env(self, path: PathBuf, creds_file: str | None) -> str:
if not str(path).startswith("oci://") or not creds_file:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,24 +47,24 @@ def test_validate_bearer_token_fails_invalid():
client._validate_bearer_token('token"with"quotes')


def test_validate_oci_credentials_fails_when_partial():
"""Test OCI credential validation fails when only one value is provided"""
def test_resolve_oci_credentials_fails_when_partial():
"""Test OCI credential resolution fails when only one value is provided"""
client = MockFlasherClient()

with pytest.raises(click.ClickException, match="OCI authentication requires both"):
client._validate_oci_credentials("myuser", None)
client._resolve_oci_credentials("oci://quay.io/org/image:tag", "myuser", None)

with pytest.raises(click.ClickException, match="OCI authentication requires both"):
client._validate_oci_credentials(None, "mypassword")
client._resolve_oci_credentials("oci://quay.io/org/image:tag", None, "mypassword")


def test_validate_oci_credentials_accepts_pair_and_strips_whitespace():
"""Test OCI credential validation accepts full username/password pair"""
def test_resolve_oci_credentials_accepts_pair_and_strips_whitespace():
"""Test OCI credential resolution accepts full username/password pair and strips whitespace"""
client = MockFlasherClient()

username, password = client._validate_oci_credentials(" myuser ", " mypassword ")
assert username == "myuser"
assert password == "mypassword"
creds = client._resolve_oci_credentials("oci://quay.io/org/image:tag", " myuser ", " mypassword ")
assert creds.username == "myuser"
assert creds.plain_password == "mypassword"


def test_resolve_oci_credentials_reads_env_for_oci_path(monkeypatch):
Expand All @@ -73,9 +73,9 @@ def test_resolve_oci_credentials_reads_env_for_oci_path(monkeypatch):
monkeypatch.setenv("OCI_USERNAME", "env-user")
monkeypatch.setenv("OCI_PASSWORD", "env-pass")

username, password = client._resolve_oci_credentials("oci://quay.io/org/image:tag", None, None)
assert username == "env-user"
assert password == "env-pass"
creds = client._resolve_oci_credentials("oci://quay.io/org/image:tag", None, None)
assert creds.username == "env-user"
assert creds.plain_password == "env-pass"


def test_resolve_oci_credentials_ignores_env_for_non_oci_path(monkeypatch):
Expand All @@ -84,30 +84,54 @@ def test_resolve_oci_credentials_ignores_env_for_non_oci_path(monkeypatch):
monkeypatch.setenv("OCI_USERNAME", "env-user")
monkeypatch.setenv("OCI_PASSWORD", "env-pass")

username, password = client._resolve_oci_credentials("https://example.com/image.raw.xz", None, None)
assert username is None
assert password is None
creds = client._resolve_oci_credentials("https://example.com/image.raw.xz", None, None)
assert creds.username is None
assert creds.password is None


def test_resolve_oci_credentials_partial_env_falls_through_to_auth_file(monkeypatch):
"""Partial env vars should fall through to auth file lookup, not error."""
from unittest.mock import patch

from pydantic import SecretStr

from jumpstarter.common.oci import OciCredentials

client = MockFlasherClient()
monkeypatch.setenv("OCI_USERNAME", "env-user")
monkeypatch.delenv("OCI_PASSWORD", raising=False)

# When auth file has no match, result is (None, None) — no error
with patch("jumpstarter.common.oci.read_auth_file_credentials", return_value=(None, None)):
username, password = client._resolve_oci_credentials("oci://quay.io/org/image:tag", None, None)
assert username is None
assert password is None
# When auth file has no match, result is unauthenticated — no error
with patch("jumpstarter.common.oci.read_auth_file_credentials", return_value=OciCredentials()):
creds = client._resolve_oci_credentials("oci://quay.io/org/image:tag", None, None)
assert creds.username is None
assert creds.password is None

# When auth file has a match, those credentials are used
with patch("jumpstarter.common.oci.read_auth_file_credentials", return_value=("fileuser", "filepass")):
username, password = client._resolve_oci_credentials("oci://quay.io/org/image:tag", None, None)
assert username == "fileuser"
assert password == "filepass"
with patch(
"jumpstarter.common.oci.read_auth_file_credentials",
return_value=OciCredentials(username="fileuser", password=SecretStr("filepass")),
):
creds = client._resolve_oci_credentials("oci://quay.io/org/image:tag", None, None)
assert creds.username == "fileuser"
assert creds.plain_password == "filepass"


def test_resolve_oci_credentials_normalizes_empty_strings(monkeypatch):
"""Empty-string username/password should be treated as absent and fall through."""
from unittest.mock import patch

from jumpstarter.common.oci import OciCredentials

client = MockFlasherClient()
monkeypatch.delenv("OCI_USERNAME", raising=False)
monkeypatch.delenv("OCI_PASSWORD", raising=False)

with patch("jumpstarter.common.oci.read_auth_file_credentials", return_value=OciCredentials()) as mock_auth:
creds = client._resolve_oci_credentials("oci://quay.io/org/image:tag", "", "")
assert creds.username is None
assert creds.password is None
mock_auth.assert_called_once()


def test_fls_oci_auth_env_sources_credentials_file():
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,19 +105,9 @@ async def flash_oci(
if not oci_url.startswith("oci://"):
raise ValueError(f"OCI URL must start with oci://, got: {oci_url}")

# If explicit credentials were provided, validate immediately
if oci_username or oci_password:
if bool(oci_username) != bool(oci_password):
raise ValueError("OCI authentication requires both username and password")
else:
# Fall back to env vars, then container auth files
from jumpstarter.common.oci import resolve_oci_credentials
from jumpstarter.common.oci import resolve_oci_credentials

oci_username, oci_password = resolve_oci_credentials(oci_url)
if oci_username and oci_password:
self.logger.info("Using OCI registry credentials from environment or auth file")
elif oci_username or oci_password:
raise ValueError("OCI authentication requires both username and password")
creds = resolve_oci_credentials(oci_url, username=oci_username, password=oci_password)

target_path = str(self.parent.validate_partition(partition))

Expand All @@ -130,10 +120,10 @@ async def flash_oci(
fls_cmd = [fls_binary, "from-url", oci_url, target_path]

fls_env = None
if oci_username and oci_password:
if creds.is_authenticated:
fls_env = os.environ.copy()
fls_env["FLS_REGISTRY_USERNAME"] = oci_username
fls_env["FLS_REGISTRY_PASSWORD"] = oci_password
fls_env["FLS_REGISTRY_USERNAME"] = creds.username
fls_env["FLS_REGISTRY_PASSWORD"] = creds.plain_password

self.logger.info(f"Running fls: {' '.join(fls_cmd)}")

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

from jumpstarter_driver_qemu.driver import Qemu, QemuFlasher

from jumpstarter.common.oci import OciCredentials
from jumpstarter.common.utils import serve


Expand Down Expand Up @@ -316,7 +317,7 @@ async def test_flash_oci_no_credentials():
# Ensure OCI env vars are not set so driver doesn't pick them up
env_clean = {k: v for k, v in os.environ.items() if k not in ("OCI_USERNAME", "OCI_PASSWORD")}
with patch.dict(os.environ, env_clean, clear=True):
with patch("jumpstarter.common.oci.read_auth_file_credentials", return_value=(None, None)):
with patch("jumpstarter.common.oci.read_auth_file_credentials", return_value=OciCredentials()):
with patch("jumpstarter_driver_qemu.driver.get_fls_binary", return_value="fls"):
with patch(
"asyncio.create_subprocess_exec", new_callable=AsyncMock, return_value=mock_process
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
from __future__ import annotations

from dataclasses import dataclass
from pathlib import Path
from typing import Dict, Optional
from typing import TYPE_CHECKING, Dict, Optional

if TYPE_CHECKING:
from jumpstarter.common.oci import OciCredentials

import click
from jumpstarter_driver_composite.client import CompositeClient
Expand Down Expand Up @@ -275,24 +280,19 @@ def _flash_operation():

return self._execute_flash_operation(_flash_operation, power_off=power_off)

def _read_oci_credentials(self, oci_url: str):
def _read_oci_credentials(self, oci_url: str) -> OciCredentials:
"""Read OCI registry credentials from environment variables or auth files."""
from jumpstarter.common.oci import resolve_oci_credentials

username, password = resolve_oci_credentials(oci_url)

if bool(username) != bool(password):
raise click.ClickException("OCI authentication requires both username and password")

return username, password
return resolve_oci_credentials(oci_url)
Comment thread
raballew marked this conversation as resolved.

def _flash_oci_auto_impl(
self,
oci_url: str,
partitions: Dict[str, str] | None = None,
):
"""Core implementation of OCI flash without wrapper logic."""
oci_username, oci_password = self._read_oci_credentials(oci_url)
creds = self._read_oci_credentials(oci_url)

self.logger.info("Checking for fastboot devices on Exporter...")
detection_result = self.call("detect_fastboot_device", 5, 2.0)
Expand All @@ -307,8 +307,8 @@ def _flash_oci_auto_impl(
"flash_oci_image",
oci_url,
partitions,
oci_username,
oci_password,
creds.username,
creds.plain_password,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

[MEDIUM] creds.plain_password exercises the OciCredentials.plain_password property, but both test_flash_oci_auto_success and test_flash_oci_auto_error_cases mock resolve_oci_credentials to return OciCredentials() (empty/unauthenticated), so the password is always None in tests. The branch where plain_password actually calls get_secret_value() is never exercised.

Consider adding a test that mocks resolve_oci_credentials to return OciCredentials(username="user", password=SecretStr("pass")) and verifies that flash_oci_image is called with the plain-text password value.

AI-generated, human reviewed

)

# Display FLS output to user
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@
import click
import pytest
from jumpstarter_driver_pyserial.driver import PySerial
from pydantic import SecretStr

from .driver import RideSXDriver
from jumpstarter.common.oci import OciCredentials
from jumpstarter.common.utils import serve


Expand Down Expand Up @@ -57,20 +59,21 @@ def test_validate_partition_mappings(ridesx_client):

def test_flash_oci_auto_success(ridesx_client):
"""Test successful flash_oci_auto call"""
with patch.object(ridesx_client, "call") as mock_call:
mock_call.side_effect = [
None, # boot_to_fastboot call
{"status": "device_found", "device_id": "ABC123"},
{"status": "success"},
]
with patch("jumpstarter.common.oci.resolve_oci_credentials", return_value=OciCredentials()):
with patch.object(ridesx_client, "call") as mock_call:
mock_call.side_effect = [
None, # boot_to_fastboot call
{"status": "device_found", "device_id": "ABC123"},
{"status": "success"},
]

result = ridesx_client.flash_oci_auto("oci://quay.io/org/image:tag")
result = ridesx_client.flash_oci_auto("oci://quay.io/org/image:tag")

assert result == {"status": "success"}
# Verify flash_oci_image was called with the OCI URL
flash_call = mock_call.call_args_list[2]
assert flash_call[0][0] == "flash_oci_image"
assert flash_call[0][1] == "oci://quay.io/org/image:tag"
assert result == {"status": "success"}
# Verify flash_oci_image was called with the OCI URL
flash_call = mock_call.call_args_list[2]
assert flash_call[0][0] == "flash_oci_image"
assert flash_call[0][1] == "oci://quay.io/org/image:tag"


def test_flash_oci_auto_error_cases(ridesx_client):
Expand All @@ -84,11 +87,30 @@ def test_flash_oci_auto_error_cases(ridesx_client):
ridesx_client.flash_oci_auto("quay.io/org/image:tag")

# No device found
with patch.object(ridesx_client, "call") as mock_call:
mock_call.return_value = {"status": "no_device_found", "device_id": None}

with pytest.raises(click.ClickException, match="No fastboot devices found"):
ridesx_client.flash_oci_auto("oci://image:tag")
with patch("jumpstarter.common.oci.resolve_oci_credentials", return_value=OciCredentials()):
with patch.object(ridesx_client, "call") as mock_call:
mock_call.return_value = {"status": "no_device_found", "device_id": None}

with pytest.raises(click.ClickException, match="No fastboot devices found"):
ridesx_client.flash_oci_auto("oci://image:tag")


def test_flash_oci_auto_passes_authenticated_credentials(ridesx_client):
"""Authenticated credentials should pass username and plain password to flash_oci_image."""
creds = OciCredentials(username="myuser", password=SecretStr("mypass"))
with patch("jumpstarter.common.oci.resolve_oci_credentials", return_value=creds):
with patch.object(ridesx_client, "call") as mock_call:
mock_call.side_effect = [
None, # boot_to_fastboot
{"status": "device_found", "device_id": "ABC123"},
{"status": "success"},
]

ridesx_client.flash_oci_auto("oci://quay.io/org/image:tag")

flash_call = mock_call.call_args_list[2]
assert flash_call[0][3] == "myuser"
assert flash_call[0][4] == "mypass"


# _execute_flash_command Tests
Expand Down
Loading
Loading