Skip to content

useruid: drop unnecessary dockerfile:1.4 frontend directive#48

Merged
bilby91 merged 1 commit into
mainfrom
fix/uid-reconcile-syntax-directive
May 12, 2026
Merged

useruid: drop unnecessary dockerfile:1.4 frontend directive#48
bilby91 merged 1 commit into
mainfrom
fix/uid-reconcile-syntax-directive

Conversation

@bilby91
Copy link
Copy Markdown
Member

@bilby91 bilby91 commented May 12, 2026

Summary

  • The generated uid_reconcile Dockerfile in useruid.go declared # syntax=docker/dockerfile:1.4, but the instructions it actually uses (ARG, FROM $ARG, USER, COPY, RUN) are all handled by buildkit's built-in frontend. The directive forces buildkit to pull docker/dockerfile:* from a registry before parsing, which hangs indefinitely in environments whose mirror routes docker.io through a broken upstream (observed on DAP workspaces routing through an ECR docker-hub mirror).
  • Drops the directive. Adds a test guard so it can't be reintroduced.

This is a narrow, minimum-risk fix. The companion PR #47 makes BuildKit the default builder (and applies the same fix to feature/dockerfile.go); without this PR, the same hang would surface for any uid_reconcile build after #47 lands.

Test plan

  • Existing useruid unit tests pass (go test ./...)
  • New assertion in TestGenerateUIDDockerfile_ContainsKeyDirectives rejects any # syntax= directive

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Generated Dockerfiles no longer include the BuildKit syntax directive. This change prevents unnecessary registry fetches during the build parsing phase, resulting in improved build performance and reliability.
  • Tests

    • Enhanced test coverage to verify that generated Dockerfiles correctly exclude the BuildKit syntax directive and rely on BuildKit's built-in frontend.

Review Change Stack

The generated uid_reconcile Dockerfile only uses ARG, FROM $ARG, USER,
COPY, and RUN — all handled by buildkit's built-in frontend. The
`# syntax=docker/dockerfile:1.4` declaration forced buildkit to pull
docker/dockerfile:* from a registry before parsing, which hangs
indefinitely in environments whose mirror routes docker.io through a
broken upstream (observed on DAP workspaces routing through an ECR
docker-hub mirror).

Adds a test guard so the directive can't be reintroduced.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 12, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 1d3dbdab-879c-4b14-a0bf-484a72f96864

📥 Commits

Reviewing files that changed from the base of the PR and between 398d0c5 and 27d2db1.

📒 Files selected for processing (2)
  • useruid.go
  • useruid_test.go

📝 Walkthrough

Walkthrough

The generateUIDDockerfile function now removes the # syntax=docker/dockerfile:1.4 directive from generated Dockerfiles and documents that the output relies on BuildKit's built-in frontend to avoid parsing-time registry fetches. A test assertion verifies the directive is absent.

Changes

BuildKit Syntax Directive Removal

Layer / File(s) Summary
Remove syntax directive and verify absence
useruid.go, useruid_test.go
The generateUIDDockerfile function removes the # syntax=docker/dockerfile:1.4\n header and adds documentation explaining that the Dockerfile relies on BuildKit's built-in frontend (ARG, FROM $ARG, USER, COPY, RUN). The test TestGenerateUIDDockerfile_ContainsKeyDirectives now verifies no # syntax= directive is present in the generated output.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Poem

A Dockerfile's parsed with no extra fight,
When syntax directives vanish from sight,
BuildKit needs not fetch from afar,
Our UID logic shines like a star! 🐰✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and specifically describes the main change: removing the unnecessary dockerfile:1.4 frontend directive from the generated Dockerfile.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/uid-reconcile-syntax-directive

Comment @coderabbitai help to get the list of available commands and usage tips.

@bilby91 bilby91 merged commit dbbd79d into main May 12, 2026
5 checks passed
@bilby91 bilby91 mentioned this pull request May 12, 2026
2 tasks
bilby91 added a commit that referenced this pull request May 12, 2026
Documents the BuildKit-switch / pre-pull bundle (#47), the
useruid frontend-directive drop (#48), the BuildCompletedEvent
duration fix (#46), and the dependabot CI bumps (#40-#42) under
their respective Keep a Changelog sections.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@bilby91 bilby91 deleted the fix/uid-reconcile-syntax-directive branch May 14, 2026 13:04
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