use smb volume for case insensitive jobs #472
Conversation
|
Is there any way we can test this? For instance making one of these script tests we could test it's possible to mount a case-insensitive filesystem, but more importantly ensure it doesn't break going forward. |
There was a problem hiding this comment.
Pull Request Overview
This PR introduces support for case-insensitive file operations during NuGet updates by leveraging SMB/CIFS mounts. The key changes include:
- Adding a new method to Job to check the "use_case_insensitive_filesystem" experiment flag.
- Creating a storage container and volumes in the updater to mount SMB/CIFS shares with optional case-insensitive behavior.
- Adjusting environment variables and clone directory paths based on the case-sensitivity setting.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| internal/model/job.go | Adds a method to check the case insensitivity experimental feature. |
| internal/infra/updater.go | Implements storage container and volume setups for case-insensitive mounts and updates environment variables accordingly. |
| internal/infra/run.go | Updates cloning and shell execution to account for different container mount paths. |
cb6c77d to
ac2e2e2
Compare
659dcc8 to
61cc328
Compare
|
@jakecoffman I added @schmittjoseph Addressed feedback in additional commit.
|
Problem
More mature .NET repos with NuGet packages are commonly only built on Windows with a case-insensitive filesystem. This can cause issues with the dependabot updater if there is a mismatch between the files in the repo and their casing in import statements. Consider a
.csprojwith a line like<Import Project="build/versions.props" />but in the repo and on disk the file is namedbuild/Versions.props(notice the upper-caseV). Since dependabot runs in a Linux host, all operations will fail because the requested files could not be loaded. This same issue can arise if a NuGet package consumed by a repo has package-internal<Import>elements that don't match the casing of the file in the package. All of these scenarios have been encountered.The Fix
To address this, dependabot needs to run on a case-insensitive filesystem, but that would add certain requirements for the Docker host that's running the update job. To avoid this, we can use some functionality built into Docker itself, namely its ability to natively mount SMB/CIFS shares for use as volumes to bind to another container. The SMB/CIFS sharing protocol was picked because it has an optional
nocasemount option that makes all operations case-insensitive.To accomplish this, the Docker host needs access to a SMB/CIFS share, but to again not impose additional requirements on the Docker host, a separate image is used,
ghcr.io.dependabot/dependabot-storage. That container only hosts a SMB/CIFS share on the default ports with a hardcoded username and password. The default Docker configuration doesn't allow guest access so user/pass had to be used isntead.The changes to this codebase
Immediately before the updater container is created (e.g., from the
dependabot-updater-nugetimage), a new container is created from thedependabot-storageimage and it is given no external internet access. The host (this code) then mounts the SMB/CIFS share twice. The first mount is with the standard options which is then given to the updater in the environment variable$DEPENDABOT_REPO_CONTENTS_PATH. The second mount is exactly the same as the first, except with thenocaseoption and it is exposed to the updater in the environment variable$DEPENDABOT_CASE_INSENSITIVE_REPO_CONTENTS_PATH.The job the proceeds as normal and the container cleanup operations have been amended to also remove the storage container and both volume mounts.
All of this work is behind a new feature flag
use_case_insensitive_filesystem.This experiment is not expected to be enabled for non-NuGet jobs, but I've done some testing anyway and found no change in behavior because the updater reads its version of
$DEPENDABOT_REPO_CONTENTS_PATHand proceeds as normal. Nothing is done with the..._CASE_INSENSITIVE_...variable.The changes here will also require a change to
dependabot/dependabot-core(PR pending) where the updater is aware of both directory paths. It performs all update work against the case-insensitive one then normalizes all file paths (e.g. the contents ofcreate_pull_request) using the case-sensitive one.