Skip to content

container-addon: OS-agnostic python3 install + docs/support-matrix#121

Open
sumit-badsara wants to merge 1 commit into
mainfrom
feat/container-addon-os-support
Open

container-addon: OS-agnostic python3 install + docs/support-matrix#121
sumit-badsara wants to merge 1 commit into
mainfrom
feat/container-addon-os-support

Conversation

@sumit-badsara

@sumit-badsara sumit-badsara commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Summary

Broadens the container add-on's OS support and documents it.

OS-agnostic python3 (drops the glibc-only dependsOn): the Feature previously depended on ghcr.io/devcontainers/features/python, which is glibc-only and therefore excludes Alpine. This replaces that dependency with a best-effort package-manager install of python3 in install.sh — mirroring the existing best-effort curl install — across apt/apk/dnf/microdnf/yum. Installs are guarded with || true so a failed package install never aborts the build under set -euo pipefail. The existing python3 safety-net warning is kept (it now fires only if the install truly failed), and the dependsOn block is removed from devcontainer-feature.json with the description updated to reflect the best-effort install. Net effect: adds Alpine (musl) + broad Linux support with no external feature dependency. The version field is intentionally untouched (CI rewrites it from the release tag).

New docs + support matrix (container-addon/README.md): what the add-on is, the two delivery methods (devcontainer Feature + COPY --from add-on), a supported-base-images matrix (Debian/Ubuntu, RHEL/Fedora/CentOS, Alpine ✅; distroless ❌), credential mount patterns (single-user vs any-user/su-sudo via the bundled link-unbound.sh, including the 0600-perm caveat), and an OS-agnostic consumer Dockerfile snippet.

New OS-agnostic consumer example (container-addon/consumer.Dockerfile.example): minimal COPY --from consumer that installs python3 + curl across package managers, referenced from the README.

Out of scope (documented): Windows containers (different managed-settings path C:\ProgramData\ClaudeCode\…, no POSIX sh, python not python3 — separate future track), distroless (no package manager), and other AI tools beyond Claude Code (Cursor/Codex/Copilot hooks exist in this repo but aren't packaged here yet).

Stacking

This is a distinct PR stacked on top of feat/container-addon-symlink-creds (PR #120) — the symlink-creds branch is used only as the base to keep this diff clean (it bundles link-unbound.sh, which the docs reference). The base should move to main after the symlink PR merges.

Validation

  • bash -n / sh -n on install.sh, link-unbound.sh, and the consumer Dockerfile RUN body — all pass.
  • All JSON parses (devcontainer-feature.json, both managed-settings.json).
  • Feature dir still has no committed unbound.py (CI copies the canonical hook in at publish time).
  • claude-code/hooks/unbound.py is untouched.

🤖 Generated with Claude Code

Greptile Summary

This PR replaces the glibc-only dependsOn Python Feature with a self-contained best-effort python3 install block in install.sh, extending Alpine/musl support to the devcontainer Feature. It also adds a README and consumer Dockerfile example documenting the OS support matrix, credential mount patterns, and out-of-scope cases.

  • install.sh: New python3 install block (apt/apk/dnf/microdnf/yum, guarded with || true) mirrors the existing curl pattern; dependsOn is cleanly removed from devcontainer-feature.json.
  • container-addon/README.md + consumer.Dockerfile.example: New documentation covering supported bases, credential mount patterns (single-user and su-sudo), and a minimal OS-agnostic consumer Dockerfile snippet.

Confidence Score: 4/5

Safe to merge; the logic is correct and the || true guards are properly scoped around the full && chains under set -euo pipefail.

The install block correctly mirrors the established curl pattern, the dependsOn removal is clean, and the safety-net warning is preserved. The two findings are a verbosity inconsistency in apt-get update flags and missing dnf/yum cache cleanup in the consumer example — both affect build log noise and image layer size rather than correctness.

The apt-get verbosity in install.sh and the dnf/yum cleanup gap in consumer.Dockerfile.example (and the matching README snippet) are worth a second look before this ships as a reference example.

Important Files Changed

Filename Overview
devcontainer-feature/src/unbound-hooks/install.sh New python3 best-effort install block mirrors the existing curl pattern but uses verbose apt-get output where curl uses -qq, causing noisy build logs on Debian/Ubuntu bases.
container-addon/consumer.Dockerfile.example New OS-agnostic consumer example Dockerfile; apt-get case cleans up package lists but dnf/microdnf/yum cases omit cache cleanup, leaving package manager cache in the image layer.
devcontainer-feature/src/unbound-hooks/devcontainer-feature.json Clean removal of the glibc-only dependsOn python Feature; description updated to reflect best-effort install; no other fields changed.
container-addon/README.md New documentation file; support matrix, credential mount patterns, and out-of-scope notes are accurate and well-structured; the inline Dockerfile snippet shares the same dnf cleanup omission as consumer.Dockerfile.example.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[install.sh runs at image build] --> B{python3 on PATH?}
    B -- yes --> E{curl on PATH?}
    B -- no --> C{Detect package manager}
    C -- apt-get --> C1[apt-get install python3 with true]
    C -- apk --> C2[apk add python3 with true]
    C -- dnf/microdnf/yum --> C3[pkg install python3 with true]
    C -- none --> C4[no-op]
    C1 & C2 & C3 & C4 --> E2{python3 on PATH now?}
    E2 -- no --> W1[WARNING: hooks fail open]
    E2 -- yes --> E
    W1 --> E
    E -- no --> H{Detect package manager for curl}
    H -- apt-get --> H1[apt-get install curl with true]
    H -- apk/dnf/etc --> H2[pkg install curl with true]
    H1 & H2 --> I{curl on PATH?}
    I -- no --> W2[WARNING: gateway calls fail open]
    I -- yes --> G[Install hook + managed-settings + link-unbound.sh]
    W2 --> G
    E -- yes --> G
Loading

Reviews (1): Last reviewed commit: "container-addon: make Feature OS-agnosti..." | Re-trigger Greptile

Greptile also left 2 inline comments on this PR.

Broaden OS support for the container hooks add-on and document it.

- devcontainer-feature: drop the glibc-only dependsOn on the official
  python feature and install python3 best-effort across apt/apk/dnf/
  microdnf/yum (mirroring the existing curl install), guarded with
  `|| true` so a failed package install never aborts the build. This
  adds Alpine (musl) plus broad Linux support. The existing python3
  safety-net warning is kept (now fires only if install truly failed),
  and the description is updated to drop the external-feature claim.

- container-addon/README.md: new docs covering the two delivery methods
  (Feature + COPY --from add-on), a supported-base-images matrix, the
  credential mount patterns (single-user vs su/sudo via link-unbound.sh,
  incl. the 0600-perm caveat), an OS-agnostic consumer Dockerfile, and
  Windows/distroless/other-tools out-of-scope notes.

- container-addon/consumer.Dockerfile.example: minimal OS-agnostic
  COPY --from consumer example, referenced from the README.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
# the build under `set -e` — the safety-net warning below fires only if it truly failed.
if ! command -v python3 >/dev/null 2>&1; then
echo "unbound-hooks: WARNING — python3 not found on PATH despite the python dependency;" >&2
if command -v apt-get >/dev/null 2>&1; then apt-get update 2>&1 && apt-get install -y --no-install-recommends python3 2>&1 || true

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 The python3 apt-get block uses apt-get update 2>&1 (verbose) while the existing curl block on line 40 uses apt-get update -qq (quiet). On a Debian/Ubuntu base that lacks python3, the update output streams to the build log at full verbosity, inconsistent with the curl install below. Aligning to -qq keeps build noise uniform and matches the established pattern.

Suggested change
if command -v apt-get >/dev/null 2>&1; then apt-get update 2>&1 && apt-get install -y --no-install-recommends python3 2>&1 || true
if command -v apt-get >/dev/null 2>&1; then apt-get update -qq && apt-get install -y -qq --no-install-recommends python3 2>&1 || true

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Comment on lines +16 to +18
elif command -v dnf >/dev/null 2>&1; then dnf install -y python3 curl; \
elif command -v microdnf >/dev/null 2>&1; then microdnf install -y python3 curl; \
elif command -v yum >/dev/null 2>&1; then yum install -y python3 curl; \

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 The apt-get branch cleans up package lists (rm -rf /var/lib/apt/lists/*), but the dnf, microdnf, and yum branches leave their cache in the image layer. On RHEL/Fedora/CentOS bases this can add tens of MBs to the final image. The same omission exists in the matching snippet in container-addon/README.md.

Suggested change
elif command -v dnf >/dev/null 2>&1; then dnf install -y python3 curl; \
elif command -v microdnf >/dev/null 2>&1; then microdnf install -y python3 curl; \
elif command -v yum >/dev/null 2>&1; then yum install -y python3 curl; \
elif command -v dnf >/dev/null 2>&1; then dnf install -y python3 curl && dnf clean all; \
elif command -v microdnf >/dev/null 2>&1; then microdnf install -y python3 curl && microdnf clean all; \
elif command -v yum >/dev/null 2>&1; then yum install -y python3 curl && yum clean all; \

@sumit-badsara sumit-badsara changed the base branch from feat/container-addon-symlink-creds to main June 9, 2026 21:12
@sumit-badsara sumit-badsara requested a review from a team June 9, 2026 21:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant