Skip to content

fix(devcontainer): preserve readonly compose mounts#767

Open
heyarny wants to merge 1 commit into
skevetter:mainfrom
heyarny:fix-compose-readonly-mounts
Open

fix(devcontainer): preserve readonly compose mounts#767
heyarny wants to merge 1 commit into
skevetter:mainfrom
heyarny:fix-compose-readonly-mounts

Conversation

@heyarny

@heyarny heyarny commented Jun 21, 2026

Copy link
Copy Markdown

Summary

  • Preserve readonly and ro mount options when devcontainer mounts are converted into generated Docker Compose service volumes.
  • Add coverage for compose-mode mount conversion so readonly and ro become read-only volumes while valued options like readonly=true remain unsupported.

Root Cause

config.Mount stores mount options that are not modeled directly in Other. The non-compose Docker path serializes those options back into the mount string, but the compose override path rebuilt ServiceVolumeConfig with only Type, Source, and Target, dropping readonly/ro before Docker Compose saw them.

Validation

  • go test ./pkg/devcontainer/...

Fixes #766.

Summary by CodeRabbit

  • Bug Fixes

    • Fixed Docker Compose configuration generation to preserve read-only mount intent. Mounts specified as readonly, ro, or readonly=true are now correctly marked as read-only in the generated service volumes.
  • Tests

    • Added coverage validating that read-only mount options are retained across multiple formats and generate the expected volume ReadOnly settings.

@coderabbitai

coderabbitai Bot commented Jun 21, 2026

Copy link
Copy Markdown

Review Change Stack

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: 1c0d457a-e1e9-462c-b758-dbc815ddc728

📥 Commits

Reviewing files that changed from the base of the PR and between a8a4145 and 88f0444.

📒 Files selected for processing (2)
  • pkg/devcontainer/compose.go
  • pkg/devcontainer/compose_test.go
🚧 Files skipped from review as they are similar to previous changes (2)
  • pkg/devcontainer/compose.go
  • pkg/devcontainer/compose_test.go

📝 Walkthrough

Walkthrough

generateDockerComposeUpProject in compose.go now sets ServiceVolumeConfig.ReadOnly for each mount by calling a new helper isReadOnlyMount, which checks mount.Other for "readonly" or "ro". A new test verifies all three common read-only option forms produce the correct ReadOnly values on the generated Compose volumes.

Changes

Read-only mount propagation for Docker Compose devcontainers

Layer / File(s) Summary
Read-only mount detection and volume wiring
pkg/devcontainer/compose.go
readOnlyMountOption constant defines "readonly" for option detection; isReadOnlyMount helper inspects mount.Other for "readonly" or "ro" tokens; generateDockerComposeUpProject calls isReadOnlyMount to populate ReadOnly on each ServiceVolumeConfig.
Test coverage for read-only mount option forms
pkg/devcontainer/compose_test.go
TestGenerateDockerComposeUpProjectPreservesReadOnlyMountOptions exercises readonly, ro, and readonly=true option forms, asserting 3 volumes with the first two read-only and the third writable.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% 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 'fix(devcontainer): preserve readonly compose mounts' clearly summarizes the main change—preserving readonly mount options in Docker Compose devcontainer configurations.
Linked Issues check ✅ Passed The PR fully addresses the requirements from issue #766 by implementing readonly mount option preservation and adding corresponding test coverage.
Out of Scope Changes check ✅ Passed All changes are directly related to preserving readonly mount options during devcontainer mount conversion to Docker Compose ServiceVolumeConfig.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@heyarny heyarny force-pushed the fix-compose-readonly-mounts branch from 3ddd7e6 to a8a4145 Compare June 21, 2026 11:50
@heyarny heyarny force-pushed the fix-compose-readonly-mounts branch from a8a4145 to 88f0444 Compare June 21, 2026 11:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: dockerComposeFile devcontainers ignore readonly bind mounts from devcontainer.json

1 participant