Skip to content
Draft
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
25 changes: 23 additions & 2 deletions crates/lib/src/podstorage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,15 @@ fn bind_storage_roots(cmd: &mut Command, storage_root: &Dir, run_root: &Dir) ->
Ok(())
}

/// Get the global authfile from the host root filesystem.
/// This is used as a fallback when the authfile is not found in the sysroot,
/// such as during upgrades where the new image may not have auth.json
/// but the running system does.
fn get_host_authfile() -> Result<Option<(camino::Utf8PathBuf, std::fs::File)>> {
let host_root = Dir::open_ambient_dir("/", cap_std::ambient_authority())?;
ostree_ext::globals::get_global_authfile(&host_root)
}

// Initialize a `podman` subprocess with:
// - storage overridden to point to to storage_root
// - Authentication (auth.json) using the bootc/ostree owned auth
Expand All @@ -133,10 +142,22 @@ fn new_podman_cmd_in(sysroot: &Dir, storage_root: &Dir, run_root: &Dir) -> Resul

// Keep this in sync with https://github.com/bootc-dev/containers-image-proxy-rs/blob/b5e0861ad5065f47eaf9cda0d48da3529cc1bc43/src/imageproxy.rs#L310
// We always override the auth to match the bootc setup.
let authfile_fd = ostree_ext::globals::get_global_authfile(sysroot)?.map(|v| v.1);
if let Some(mut fd) = authfile_fd {
// First try to get the authfile from the sysroot (e.g., upgrade image), and if not found,
// fall back to the host root. This handles the case where during an upgrade, the new image
// may not have auth.json but the running system does.
let authfile = if let Some((path, file)) = ostree_ext::globals::get_global_authfile(sysroot)? {
tracing::debug!("Using authfile from sysroot: {path}");
Some(file)
} else if let Some((path, file)) = get_host_authfile()? {
tracing::debug!("Using authfile from host root: {path}");
Some(file)
} else {
None
};
Comment on lines +148 to +156
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

This if let ... else if let ... chain can be simplified by combining the fallback logic into a single else block, which makes the code slightly more concise.

Suggested change
let authfile = if let Some((path, file)) = ostree_ext::globals::get_global_authfile(sysroot)? {
tracing::debug!("Using authfile from sysroot: {path}");
Some(file)
} else if let Some((path, file)) = get_host_authfile()? {
tracing::debug!("Using authfile from host root: {path}");
Some(file)
} else {
None
};
let authfile = if let Some((path, file)) = ostree_ext::globals::get_global_authfile(sysroot)? {
tracing::debug!("Using authfile from sysroot: {path}");
Some(file)
} else {
get_host_authfile()?.map(|(path, file)| {
tracing::debug!("Using authfile from host root: {path}");
file
})
};

if let Some(mut fd) = authfile {
std::io::copy(&mut fd, &mut tempfile)?;
} else {
tracing::debug!("No authfile found, using empty auth");
// Note that if there's no bootc-owned auth, then we force an empty authfile to ensure
// that podman doesn't fall back to searching the user-owned paths.
tempfile.write_all(b"{}")?;
Expand Down
85 changes: 85 additions & 0 deletions tmt/tests/booted/bootc_testlib.nu
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,88 @@ export def have_hostexports [] {
export def parse_cmdline [] {
open /proc/cmdline | str trim | split row " "
}

# cstor-dist configuration for authenticated registry testing
# cstor-dist serves images from containers-storage via an authenticated OCI registry endpoint
# https://github.com/ckyrouac/cstor-dist
const CSTOR_DIST_IMAGE = "ghcr.io/ckyrouac/cstor-dist:latest"
const CSTOR_DIST_USER = "testuser"
const CSTOR_DIST_PASS = "testpass"
const CSTOR_DIST_PORT = 8000

# The registry address for cstor-dist
export const CSTOR_DIST_REGISTRY = $"localhost:($CSTOR_DIST_PORT)"

# Start cstor-dist with basic auth on localhost
# Fails if cstor-dist cannot be started
export def start_cstor_dist [] {
print "Starting cstor-dist with basic auth..."

# Pull test images that cstor-dist will serve
print "Pulling test images for cstor-dist to serve..."
podman pull docker.io/library/alpine:latest
podman pull docker.io/library/busybox:latest

# Run cstor-dist container with auth enabled
# Mount the local containers storage so cstor-dist can serve images from it
let storage_path = if ("/var/lib/containers/storage" | path exists) {
"/var/lib/containers/storage"
} else {
$"($env.HOME)/.local/share/containers/storage"
}

(podman run --privileged --rm -d --name cstor-dist-auth
-p $"($CSTOR_DIST_PORT):8000"
-v $"($storage_path):/var/lib/containers/storage"
$CSTOR_DIST_IMAGE --username $CSTOR_DIST_USER --password $CSTOR_DIST_PASS)

# Wait for cstor-dist to be ready by testing HTTP connection
# Loop for up to 20 seconds
print "Waiting for cstor-dist to be ready..."
let auth_header = $"($CSTOR_DIST_USER):($CSTOR_DIST_PASS)" | encode base64
mut ready = false
for i in 1..20 {
let result = do { curl -sf -H $"Authorization: Basic ($auth_header)" $"http://($CSTOR_DIST_REGISTRY)/v2/" } | complete
if $result.exit_code == 0 {
$ready = true
break
}
print $"Attempt ($i)/20: cstor-dist not ready yet..."
sleep 1sec
}

if not $ready {
# Show container logs for debugging
print "cstor-dist failed to start. Container logs:"
podman logs cstor-dist-auth
error make { msg: "cstor-dist failed to become ready within 20 seconds" }
}

print $"cstor-dist running on ($CSTOR_DIST_REGISTRY)"
}

# Get cstor-dist auth config
export def get_cstor_auth [] {
# Base64 encode the credentials for auth.json
let auth_b64 = $"($CSTOR_DIST_USER):($CSTOR_DIST_PASS)" | encode base64
{
registry: $CSTOR_DIST_REGISTRY,
auth_b64: $auth_b64
}
}

# Configure insecure registry for cstor-dist (no TLS)
export def setup_insecure_registry [] {
mkdir /etc/containers/registries.conf.d
(echo $"[[registry]]\nlocation=\"($CSTOR_DIST_REGISTRY)\"\ninsecure=true"
| save -f /etc/containers/registries.conf.d/99-cstor-dist.conf)
}

# Set up auth.json on the running system with cstor-dist credentials
export def setup_system_auth [] {
mkdir /run/ostree
let cstor = get_cstor_auth
print $"Setting up system auth for cstor-dist at ($cstor.registry)"
let auth_json = $'{"auths": {"($cstor.registry)": {"auth": "($cstor.auth_b64)"}}}'
echo $auth_json | save -f /run/ostree/auth.json
}
85 changes: 66 additions & 19 deletions tmt/tests/booted/test-logically-bound-switch.nu
Original file line number Diff line number Diff line change
Expand Up @@ -12,27 +12,31 @@
# <verify new bound images are pulled>
# <reboot>
# <verify booted state>
#
# This test also verifies that authenticated LBIs work in two scenarios:
# 1. Auth credentials stored in the image itself
# 2. Auth credentials stored on the running system
#
# The test uses cstor-dist to serve images from containers-storage via an
# authenticated OCI registry endpoint.

use std assert
use tap.nu
use bootc_testlib.nu [CSTOR_DIST_REGISTRY, start_cstor_dist, get_cstor_auth, setup_insecure_registry, setup_system_auth]

# This code runs on *each* boot.
bootc status
let st = bootc status --json | from json
let booted = $st.status.booted.image

# The tests here aren't fetching from a registry which requires auth by default,
# but we can replicate the failure in https://github.com/bootc-dev/bootc/pull/1852
# by just injecting any auth file.
echo '{}' | save -f /run/ostree/auth.json

def initial_setup [] {
bootc image copy-to-storage
podman images
podman image inspect localhost/bootc | from json
}

def build_image [name images containers] {
# Build an image with optional auth.json baked in
def build_image [name images containers --with-auth] {
let td = mktemp -d
cd $td
mkdir usr/share/containers/systemd
Expand All @@ -59,6 +63,16 @@ RUN echo sanity check > /usr/share/bound-image-sanity-check.txt
}
}

# Optionally bake auth.json into the image
if $with_auth {
let cstor = get_cstor_auth
print "Baking auth.json into the image"
mkdir etc/ostree
let auth_json = $'{"auths": {"($cstor.registry)": {"auth": "($cstor.auth_b64)"}}}'
echo $auth_json | save etc/ostree/auth.json
echo "COPY etc/ /etc/\n" | save Dockerfile --append
}

# Build it
podman build -t $name .
# Just sanity check it
Expand All @@ -74,12 +88,13 @@ def verify_images [images containers] {
let image_names = podman --storage-opt=additionalimagestore=/usr/lib/bootc/storage images --format json | from json | select -i Names

for $image in $bound_images {
let found = $image_names | where Names == [$image.image]
# Check if the expected image name is IN the Names array (not exact match)
let found = $image_names | where { |row| $image.image in $row.Names }
assert (($found | length) > 0) $"($image.image) not found"
}

for $container in $bound_containers {
let found = $image_names | where Names == [$container.image]
let found = $image_names | where { |row| $container.image in $row.Names }
assert (($found | length) > 0) $"($container.image) not found"
}
}
Expand All @@ -89,18 +104,29 @@ def first_boot [] {

initial_setup

# build a bootc image that includes bound images
# Start cstor-dist for authenticated LBI testing
start_cstor_dist
setup_insecure_registry

# Set up auth on running system for the switch operation
# The image will also have auth baked in - both should work
setup_system_auth

# Build a bootc image that includes bound images
# Include an authenticated LBI from cstor-dist with auth baked into the image
let images = [
{ "bound": true, "image": "registry.access.redhat.com/ubi9/ubi-minimal:9.4", "name": "ubi-minimal" },
{ "bound": false, "image": "quay.io/centos-bootc/centos-bootc:stream9", "name": "centos-bootc" }
{ "bound": false, "image": "quay.io/centos-bootc/centos-bootc:stream9", "name": "centos-bootc" },
{ "bound": true, "image": $"($CSTOR_DIST_REGISTRY)/docker.io/library/alpine:latest", "name": "cstor-alpine" }
]

let containers = [{
"bound": true, "image": "docker.io/library/alpine:latest", "name": "alpine"
"bound": true, "image": "docker.io/library/alpine:latest", "name": "alpine"
}]

let image_name = "localhost/bootc-bound"
build_image $image_name $images $containers
print "Building image WITH auth.json baked in (tests auth from image)"
build_image $image_name $images $containers --with-auth
bootc switch --transport containers-storage $image_name
verify_images $images $containers
tmt-reboot
Expand All @@ -111,21 +137,39 @@ def second_boot [] {
assert equal $booted.image.transport containers-storage
assert equal $booted.image.image localhost/bootc-bound

# verify images are still there after boot
# Start cstor-dist again (container doesn't survive reboot)
start_cstor_dist
setup_insecure_registry

# Set up auth on the RUNNING SYSTEM for the upgrade
# The new image will NOT have auth baked in, so the fallback to system auth is needed
setup_system_auth

# Verify images from first switch are still there
let images = [
{ "bound": true, "image": "registry.access.redhat.com/ubi9/ubi-minimal:9.4", "name": "ubi-minimal" },
{ "bound": false, "image": "quay.io/centos-bootc/centos-bootc:stream9", "name": "centos-bootc" }
{ "bound": false, "image": "quay.io/centos-bootc/centos-bootc:stream9", "name": "centos-bootc" },
{ "bound": true, "image": $"($CSTOR_DIST_REGISTRY)/docker.io/library/alpine:latest", "name": "cstor-alpine" }
]

let containers = [{
"bound": true, "image": "docker.io/library/alpine:latest", "name": "alpine"
"bound": true, "image": "docker.io/library/alpine:latest", "name": "alpine"
}]
verify_images $images $containers

# build a new bootc image with an additional bound image
# Build a NEW bootc image WITHOUT auth baked in
# Add a DIFFERENT authenticated LBI (busybox instead of alpine)
# This tests that auth from the running system works (the fallback fix)
print "bootc upgrade with another bound image"
let image_name = "localhost/bootc-bound"
let more_images = $images | append [{ "bound": true, "image": "registry.access.redhat.com/ubi9/ubi-minimal:9.3", "name": "ubi-minimal-9-3" }]
let more_images = [
{ "bound": true, "image": "registry.access.redhat.com/ubi9/ubi-minimal:9.4", "name": "ubi-minimal" },
{ "bound": true, "image": "registry.access.redhat.com/ubi9/ubi-minimal:9.3", "name": "ubi-minimal-9-3" },
{ "bound": false, "image": "quay.io/centos-bootc/centos-bootc:stream9", "name": "centos-bootc" },
{ "bound": true, "image": $"($CSTOR_DIST_REGISTRY)/docker.io/library/alpine:latest", "name": "cstor-alpine" },
{ "bound": true, "image": $"($CSTOR_DIST_REGISTRY)/docker.io/library/busybox:latest", "name": "cstor-busybox" }
]
Comment on lines +165 to +171
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

To improve readability and reduce repetition, you can define more_images by appending to the existing $images list, which is defined just a few lines above. This approach is more concise and consistent with how this variable was constructed before this change.

    let more_images = $images | append [
        { "bound": true, "image": "registry.access.redhat.com/ubi9/ubi-minimal:9.3", "name": "ubi-minimal-9-3" },
        { "bound": true, "image": $"($CSTOR_DIST_REGISTRY)/docker.io/library/busybox:latest", "name": "cstor-busybox" }
    ]

print "Building image WITHOUT auth.json (tests auth fallback from running system)"
build_image $image_name $more_images $containers
bootc upgrade
verify_images $more_images $containers
Expand All @@ -137,14 +181,17 @@ def third_boot [] {
assert equal $booted.image.transport containers-storage
assert equal $booted.image.image localhost/bootc-bound

# No need to start cstor-dist - we're just verifying the images are in storage
let images = [
{ "bound": true, "image": "registry.access.redhat.com/ubi9/ubi-minimal:9.4", "name": "ubi-minimal" },
{ "bound": true, "image": "registry.access.redhat.com/ubi9/ubi-minimal:9.3", "name": "ubi-minimal-9-3" },
{ "bound": false, "image": "quay.io/centos-bootc/centos-bootc:stream9", "name": "centos-bootc" }
{ "bound": false, "image": "quay.io/centos-bootc/centos-bootc:stream9", "name": "centos-bootc" },
{ "bound": true, "image": $"($CSTOR_DIST_REGISTRY)/docker.io/library/alpine:latest", "name": "cstor-alpine" },
{ "bound": true, "image": $"($CSTOR_DIST_REGISTRY)/docker.io/library/busybox:latest", "name": "cstor-busybox" }
]
Comment on lines 185 to 191
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

This list of images is identical to the more_images list defined in second_boot. To avoid this duplication and improve maintainability, consider extracting this list into a shared helper function or constant that can be used in both places.

For example, you could add a helper function:

def get_final_images [] {
    [
        { "bound": true, "image": "registry.access.redhat.com/ubi9/ubi-minimal:9.4", "name": "ubi-minimal" },
        { "bound": true, "image": "registry.access.redhat.com/ubi9/ubi-minimal:9.3", "name": "ubi-minimal-9-3" },
        { "bound": false, "image": "quay.io/centos-bootc/centos-bootc:stream9", "name": "centos-bootc" },
        { "bound": true, "image": $"($CSTOR_DIST_REGISTRY)/docker.io/library/alpine:latest", "name": "cstor-alpine" },
        { "bound": true, "image": $"($CSTOR_DIST_REGISTRY)/docker.io/library/busybox:latest", "name": "cstor-busybox" }
    ]
}

Then you could use let more_images = get_final_images in second_boot and let images = get_final_images here.


let containers = [{
"bound": true, "image": "docker.io/library/alpine:latest", "name": "alpine"
"bound": true, "image": "docker.io/library/alpine:latest", "name": "alpine"
}]

verify_images $images $containers
Expand Down
Loading