Skip to content

Fix docker credentials in the skipper container and expose the push destination - #197

Merged
khizunov merged 2 commits into
upstreamfrom
anton/fix-docker-config-mount
Sep 14, 2026
Merged

khizunov merged 2 commits into
upstreamfrom
anton/fix-docker-config-mount

Conversation

@khizunov

@khizunov khizunov commented Sep 14, 2026 •

Copy link
Copy Markdown
Contributor

User description

Closes #195
Closes #196

Docker credentials (#195)

Mount the credentials from the right place. handle_volumes_bind_mount built the mount source from $HOME rather than $HOME/.docker, so on Linux the whole home directory was bind-mounted read-write at /opt/.docker (and /opt/.docker/config.json still did not exist), while on macOS the source $HOME/config.json did not exist and the mount was skipped altogether. The source is now the effective host docker config directory - DOCKER_CONFIG when set, ~/.docker otherwise - so a custom config location reaches the container too.

Also mount the credentials at their host path. The container keeps the host's HOME, so a nested skipper, helm or oras looks for $HOME/.docker/config.json rather than at DOCKER_CONFIG. The same credentials are mounted there read-only — writes belong in the DOCKER_CONFIG copy.

Honor DOCKER_CONFIG when reading the registry login. set_remote_registry_login_info read ~/.docker/config.json unconditionally. It now does the same lookup docker itself does (DOCKER_CONFIG, falling back to ~/.docker), so a skipper running inside a skipper container finds the login info for push, images -r and rmi -r.

Point every runtime at the writable copy. DOCKER_CONFIG={DOCKER_CONFIG} was injected only for the docker runtime, so under podman a docker-format credential writer (docker login, oras login) resolved to $HOME/.docker/config.json - now a read-only mount. The injection is no longer inside the runtime branch; DOCKER_CONTEXT and SKIPPER_DOCKER_GID stay docker-only.

Point helm at the mounted credentials. helm reads the docker config format but locates it through HELM_REGISTRY_CONFIG, not DOCKER_CONFIG. _run_nested now sets HELM_REGISTRY_CONFIG=/opt/.docker/config.json unless the caller defined it (is_environment_variable_defined compares the name before =, so an unrelated MY_HELM_REGISTRY_CONFIG no longer suppresses it), so helm push to an OCI registry works off the same docker login as everything else.

Registry and namespace variables (#196)

skipper run / make / shell now export SKIPPER_REGISTRY (from --registry or registry) and SKIPPER_NAMESPACE (from push.namespace), each only when configured. They are prepended to the environment list, so an explicit env / -e entry of the same name still wins. Documented in the README.

Testing

ruff check --preview skipper tests passes. pytest tests — 124 passed, 1 failed: test_run_simple_command_nested_with_multiple_env_files fails identically on upstream before this branch (macOS-only: default net bridge vs host, and /private/etc/docker symlink resolution), so it is pre-existing and unrelated.

New coverage: HELM_REGISTRY_CONFIG default and user override, exact-name environment matching, the docker config lookup honoring DOCKER_CONFIG, both credential mounts following it, a YAML-null push: section, and the injected push-destination variables. The runner tests spell out both credential mounts explicitly instead of reusing the production expression, so a change to the mounted path shows up as a failure.


Generated description

Below is a concise technical summary of the changes proposed in this PR:

graph LR
cli_("cli"):::modified
set_remote_registry_login_info_("set_remote_registry_login_info"):::modified
get_docker_config_path_("get_docker_config_path"):::added
get_docker_config_dir_("get_docker_config_dir"):::added
run_nested_("_run_nested"):::modified
is_environment_variable_defined_("is_environment_variable_defined"):::modified
handle_volumes_bind_mount_("handle_volumes_bind_mount"):::modified
HELM_("HELM"):::added
cli_ -- "CLI now stores namespace before loading registry authentication." --> set_remote_registry_login_info_
set_remote_registry_login_info_ -- "Registry authentication now respects DOCKER_CONFIG or default Docker directory." --> get_docker_config_path_
get_docker_config_path_ -- "Constructs config.json from configurable Docker directory." --> get_docker_config_dir_
run_nested_ -- "Environment checks now recognize variables defined in env files." --> is_environment_variable_defined_
run_nested_ -- "Nested containers receive configurable Docker directory and credentials." --> get_docker_config_dir_
handle_volumes_bind_mount_ -- "Mounts credentials from DOCKER_CONFIG, with home-directory fallback." --> get_docker_config_dir_
run_nested_ -- "Helm receives Docker registry credentials via HELM_REGISTRY_CONFIG." --> HELM_
classDef added stroke:#15AA7A
classDef removed stroke:#CD5270
classDef modified stroke:#EDAC4C
linkStyle default stroke:#CBD5E1,font-size:13px
Loading

Fix nested container Docker credential discovery and mounting by updating runner, utils, and Helm environment handling to honor DOCKER_CONFIG while preserving writable and read-only credential paths. Export configured push destinations through cli for run, make, and shell, and document the new variables.

TopicDetails
Push destinations Export configured SKIPPER_REGISTRY and SKIPPER_NAMESPACE values to containerized run, make, and shell flows while allowing explicit environment entries to take precedence, and document the behavior.
Modified files (3)
  • README.md
  • skipper/cli.py
  • tests/test_cli.py
Latest Contributors(2)
UserCommitDate
anton.khizunov@zadaras...Fix #196 expose the co...September 14, 2026
tomer.schwartz@zadaras...Fix #193 skipper rmi -...July 05, 2026
Container credentials Fix Docker and Helm credential propagation for nested containers by honoring DOCKER_CONFIG, mounting credentials from the effective host directory at writable and host-home paths, and applying precise environment overrides across Docker and Podman runtimes.
Modified files (5)
  • skipper/runner.py
  • skipper/utils.py
  • tests/test_runner.py
  • tests/test_runner_podman.py
  • tests/test_utils.py
Latest Contributors(2)
UserCommitDate
anton.khizunov@zadaras...Fix #195 make the host...September 14, 2026
tomer.schwartz@zadaras...Fix #193 skipper rmi -...July 05, 2026
Review this PR on Baz
Customize your next review

Comment thread skipper/runner.py Outdated
Comment thread skipper/runner.py Outdated
Comment thread skipper/cli.py Outdated
Comment thread skipper/utils.py Outdated
@khizunov
khizunov force-pushed the anton/fix-docker-config-mount branch 3 times, most recently from 10152ef to 9d2d3b9 Compare September 14, 2026 12:49
Comment thread skipper/runner.py Outdated
@khizunov
khizunov force-pushed the anton/fix-docker-config-mount branch 4 times, most recently from 11c92e2 to 211bfb8 Compare September 14, 2026 13:05
Comment thread skipper/runner.py Outdated
Comment thread skipper/runner.py Outdated
Comment thread skipper/runner.py
Comment thread skipper/runner.py
@khizunov
khizunov force-pushed the anton/fix-docker-config-mount branch from 211bfb8 to 398d5b2 Compare September 14, 2026 13:37
Comment thread skipper/utils.py
@khizunov
khizunov force-pushed the anton/fix-docker-config-mount branch from 398d5b2 to b0a7f93 Compare September 14, 2026 13:44
@khizunov
khizunov requested a review from ofir-amir September 14, 2026 13:49
@khizunov
khizunov merged commit d2206c2 into upstream Sep 14, 2026
4 checks passed
@khizunov
khizunov deleted the anton/fix-docker-config-mount branch September 14, 2026 13:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants