Skip to content

Conversation

@leodido
Copy link
Contributor

@leodido leodido commented Nov 20, 2025

Problem

When exportToCache is enabled, Docker images are exported in OCI layout format (not loaded into Docker daemon). The container filesystem extraction code uses checkImageExists() which calls docker image inspect, failing for OCI layout images with:

image not found - build may have failed silently

This blocks packages that use container extraction (e.g., extracting files from a Docker image to use in subsequent builds) when SLSA L3 caching is enabled.

Root Cause

The PostProcess function in Docker package builds always validates the image exists using checkImageExists(), which assumes the image is in the Docker daemon. With OCI layout export, the image is written to image.tar in the build directory and never loaded into the daemon.

Solution

Add checkOCILayoutExists() function to validate OCI layout images by checking for image.tar in the build directory. Update PostProcess to use the appropriate validation based on the exportToCache flag:

  • exportToCache=false → use checkImageExists() (Docker daemon)
  • exportToCache=true → use checkOCILayoutExists() (OCI layout)

Changes

  • pkg/leeway/build.go:

    • Add checkOCILayoutExists() function (lines 2040-2065)
    • Update PostProcess to use appropriate check based on exportToCache flag (lines 2793-2825)
  • pkg/leeway/build_oci_test.go:

    • Add TestCheckOCILayoutExists with 4 test cases covering:
      • Valid OCI layout
      • Missing image.tar
      • Empty image.tar
      • Invalid file type
  • pkg/leeway/build_integration_test.go:

    • Add TestDockerPackage_ContainerExtraction_Integration with 2 subtests:
      • with_docker_daemon: Docker daemon path
      • with_oci_layout: OCI layout path
    • Fix TestDockerPackage_ExportToCache_Integration to create docker-container builder
    • Fix TestDockerPackage_CacheRoundTrip_Integration to create docker-container builder
    • Fix test expectations for OCI export behavior
    • Add comprehensive comments explaining what each test validates and SLSA L3 relevance
  • .github/workflows/integration-tests.yaml:

    • Add workflow_dispatch trigger for manual test runs
  • README.md:

    • Document docker-container builder requirement for OCI export
    • Clarify that docker/setup-buildx-action uses docker-container by default

Test Results

All integration tests passing:

  • TestDockerPackage_ExportToCache_Integration (3 subtests) - OCI layout export functionality
  • TestDockerPackage_CacheRoundTrip_Integration - Complete cache workflow
  • TestDockerPackage_OCILayout_Determinism_Integration - Deterministic builds (critical for SLSA L3)
  • TestDockerPackage_OCILayout_SLSA_Integration ⭐ - PRIMARY SLSA L3 TEST - End-to-end provenance generation
  • TestDockerPackage_ContainerExtraction_Integration (2 subtests) - Container extraction with both paths

Both Docker daemon and OCI layout paths verified working correctly with no "image not found" errors.

Related Issues

Part of the OCI layout + SLSA L3 work:

How to Test

Run the integration tests:

go test -tags=integration -v ./pkg/leeway/ -run ".*Integration" -timeout 10m

All tests should pass, demonstrating container extraction works with both Docker daemon and OCI layout.

Documentation

Updated README.md with OCI export requirements and docker-container builder setup instructions.

/hold

@leodido leodido changed the title fix: container extract fix: support container extraction with OCI layout export Nov 20, 2025
@leodido leodido force-pushed the ldd/fix-oci-container-extract branch from 94722ee to a0e44fa Compare November 20, 2025 21:29
@leodido leodido marked this pull request as ready for review November 20, 2025 21:30
@leodido leodido self-assigned this Nov 20, 2025
@leodido leodido requested a review from kylos101 November 20, 2025 21:32
@leodido leodido force-pushed the ldd/fix-oci-container-extract branch 2 times, most recently from 21605e7 to b12cf22 Compare November 20, 2025 21:44
@leodido leodido force-pushed the ldd/fix-oci-container-extract branch from 74c78ec to 44c5410 Compare November 20, 2025 21:50
Copy link
Member

@geropl geropl left a comment

Choose a reason for hiding this comment

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

LGTM ✔️

@leodido leodido changed the base branch from ldd/fix-oci-slsa-inspect to main November 21, 2025 08:51
leodido and others added 3 commits November 21, 2025 08:57
When exportToCache is enabled, images are in OCI layout format (not Docker daemon).
Container extraction code was checking Docker daemon with 'docker image inspect',
which fails for OCI layout images.

Changes:
- Add checkOCILayoutExists() to validate OCI layout before extraction
- Update PostProcess to use appropriate check based on exportToCache flag
- Add unit tests for OCI layout validation (4 test cases)
- Add integration test for both Docker daemon and OCI layout paths
- Add workflow_dispatch trigger to integration tests workflow

All tests passing:
- Unit tests: TestCheckOCILayoutExists (4/4)
- Integration: TestDockerPackage_ContainerExtraction_Integration (2/2)

Co-authored-by: Ona <no-reply@ona.com>
Document docker-container builder requirement for OCI export.
Clarify that docker/setup-buildx-action uses docker-container by default.
Explain what each test validates and its SLSA L3 relevance.
@leodido leodido force-pushed the ldd/fix-oci-container-extract branch from 44c5410 to 6add4dd Compare November 21, 2025 08:57
@leodido
Copy link
Contributor Author

leodido commented Nov 21, 2025

Rebased on main

✅ Successfully rebased on latest main branch after PR #289 was merged.

Rebase details:

  • Base branch changed from ldd/extract-digest-from-oci-layout to main
  • No conflicts encountered (automatic rebase)
  • All commits preserved with new commit hashes
  • Branch force-pushed to update PR

Commit history:

6add4dd docs: add comprehensive comments to integration tests
e530a03 docs: add OCI export requirements to README
e03499a fix: support container extraction with OCI layout export

Status:

  • ✅ Mergeable: true
  • ✅ All test cases and comments preserved
  • ✅ Ready for review and merge

@leodido leodido merged commit 723a1fe into main Nov 21, 2025
7 checks passed
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.

3 participants