Skip to content

fix: fallback to docker-container driver when default builder uses docker driver#76

Closed
dephiros wants to merge 3 commits intouseblacksmith:mainfrom
dephiros:main
Closed

fix: fallback to docker-container driver when default builder uses docker driver#76
dephiros wants to merge 3 commits intouseblacksmith:mainfrom
dephiros:main

Conversation

@dephiros
Copy link
Copy Markdown

@dephiros dephiros commented Mar 23, 2026

Problem

When the Blacksmith remote builder setup fails (e.g. sticky disk API timeout), the fallback path at src/main.ts:644 checks for an existing builder via toolkit.builder.inspect(). The default builder always exists and uses the docker driver, so the condition if (builder) is always true.

This means the code never reaches the else branch that creates a docker-container builder. The docker driver doesn't support attestation (provenance/sbom), causing builds to fail with:

ERROR: Attestation is not supported for the docker driver.
Switch to a different driver, or turn on the containerd image store, and try again.

Fix

Check the builder's driver property, not just its existence. If the existing builder uses the docker driver, create a docker-container builder instead, which supports all buildx features including attestation.

- if (builder) {
-   core.info(\`Found configured builder: \${builder.name}\`);
+ if (builder && builder.driver !== "docker") {
+   core.info(\`Found configured builder: \${builder.name} (driver: \${builder.driver})\`);

Open with Devin

…ailable

When the Blacksmith remote builder setup fails (e.g. sticky disk API timeout),
the fallback path checks for an existing builder. The 'default' builder always
exists and uses the 'docker' driver, which doesn't support attestation
(provenance/sbom). This causes builds to fail with:
  ERROR: Attestation is not supported for the docker driver.

Fix: check the builder's driver, not just its existence. If the existing
builder uses the 'docker' driver, create a 'docker-container' builder
instead, which supports all buildx features including attestation.
Copy link
Copy Markdown

@eltonkl eltonkl left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution!

The logging statement on src/main.ts line 656 is not reflected in dist/index.js yet.

Could you update dist/index.js slightly?

sed -i '' 's/Created and set a local builder for use/Created and set a local docker-container builder/g' dist/index.js

@dephiros
Copy link
Copy Markdown
Author

Thanks for the contribution!

Thanks for the review @eltonkl and sorry for missing that since I had to update the dist file manually(missing private deps locally)
Updated ✅

@dephiros dephiros requested a review from eltonkl March 25, 2026 09:00
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 2 additional findings.

Open in Devin Review

@adityamaru
Copy link
Copy Markdown
Contributor

@dephiros this seems like a good change but I would rather y'all not fallback at all! Let me also look into what error you're seeing when interacting with the stickydisk API

# Conflicts:
#	dist/index.js
@dephiros
Copy link
Copy Markdown
Author

I saw this PR should be superseded by #86.
Thank you for getting the fix in 🙏 🎉

@dephiros dephiros closed this Mar 27, 2026
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