Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
---
version: "Unreleased"
version: "v1.0.1"
---

### 🧪 Experiments Added
Expand Down Expand Up @@ -43,6 +43,10 @@ A regression introduced in v0.99.4 caused `read_terragrunt_config()` to fail to

Chaining dependencies with exposed includes no longer produces a spurious "Could not convert include to the execution ctx to evaluate additional locals" error during partial parsing.

#### Provider cache fixed on Windows for remote URLs

The provider cache failed on Windows with `CreateFile https://...: The filename, directory name, or volume label syntax is incorrect` because remote download URLs were passed to `os.Stat`, and the colon in `https:` is invalid Windows path syntax. The fix skips the filesystem existence check when the download URL is a remote URL (`://`), going directly to the download path.

Comment on lines +48 to +49
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Tighten Line 48 wording to match the actual code path.

Line 48 says the URL was passed to os.Stat, but the implementation guards vfs.FileExists (which calls fs.Stat underneath). Updating this avoids implying a direct os.Stat call in provider_cache.go.

Suggested docs wording
-The provider cache failed on Windows with `CreateFile https://...: The filename, directory name, or volume label syntax is incorrect` because remote download URLs were passed to `os.Stat`, and the colon in `https:` is invalid Windows path syntax. The fix skips the filesystem existence check when the download URL is a remote URL (`://`), going directly to the download path.
+The provider cache failed on Windows with `CreateFile https://...: The filename, directory name, or volume label syntax is incorrect` because remote download URLs were being treated like local filesystem paths during existence checks (`vfs.FileExists`/`fs.Stat`), and the colon in `https:` is invalid Windows path syntax. The fix skips the filesystem existence check when the download URL is remote (`://`) and proceeds directly with provider download/caching.

As per coding guidelines docs/**/*.md*: documentation should be clear and accurate.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
The provider cache failed on Windows with `CreateFile https://...: The filename, directory name, or volume label syntax is incorrect` because remote download URLs were passed to `os.Stat`, and the colon in `https:` is invalid Windows path syntax. The fix skips the filesystem existence check when the download URL is a remote URL (`://`), going directly to the download path.
The provider cache failed on Windows with `CreateFile https://...: The filename, directory name, or volume label syntax is incorrect` because remote download URLs were being treated like local filesystem paths during existence checks (`vfs.FileExists`/`fs.Stat`), and the colon in `https:` is invalid Windows path syntax. The fix skips the filesystem existence check when the download URL is remote (`://`) and proceeds directly with provider download/caching.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/src/data/changelog/v1.0.1.mdx` around lines 48 - 49, Update the sentence
on line 48 to accurately reflect the code path: replace the claim that the URL
was passed to os.Stat with wording that the remote URL reached the
vfs.FileExists check (which calls fs.Stat underneath) in provider_cache.go;
mention vfs.FileExists and fs.Stat to make the documentation precise about where
the filesystem existence check is skipped for URLs containing "://".

### ⚙️ Process Updates

---
Expand Down
22 changes: 18 additions & 4 deletions internal/tf/cache/services/provider_cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"path"
"path/filepath"
"slices"
"strings"
"sync"
"time"

Expand Down Expand Up @@ -318,12 +319,12 @@ func (cache *ProviderCache) warmUp(ctx context.Context) error {
return errors.Errorf("not found provider download url")
}

downloadURLExists, err := vfs.FileExists(fs, cache.DownloadURL)
downloadURLIsLocalFile, err := cache.isLocalFile(fs, cache.DownloadURL)
if err != nil {
return errors.New(err)
}

if downloadURLExists {
if downloadURLIsLocalFile {
cache.archivePath = cache.DownloadURL
} else {
if err := util.DoWithRetry(ctx, fmt.Sprintf("Fetching provider %s", cache.Provider), maxRetriesFetchFile, retryDelayFetchFile, cache.logger, log.DebugLevel, func(ctx context.Context) error {
Expand Down Expand Up @@ -404,6 +405,17 @@ func (cache *ProviderCache) removeArchive() error {
return nil
}

// isLocalFile checks whether the given path refers to an existing local file.
// Remote URLs (containing "://") are never checked against the filesystem because
// on Windows the colon in "https:" is invalid path syntax and causes an error.
func (cache *ProviderCache) isLocalFile(fs vfs.FS, path string) (bool, error) {
if strings.Contains(path, "://") {
return false, nil
}

return vfs.FileExists(fs, path)
}

func (cache *ProviderCache) acquireLockFile(ctx context.Context) (*util.Lockfile, error) {
lockfile := util.NewLockfile(cache.lockfilePath)

Expand Down Expand Up @@ -670,8 +682,10 @@ func (service *ProviderService) startProviderCaching(ctx context.Context, cache
service.logger.Warnf("Failed to clean up package dir %q: %v", cache.packageDir, err)
}

if err := service.FS().Remove(cache.archivePath); err != nil {
service.logger.Warnf("Failed to clean up archive %q: %v", cache.archivePath, err)
if cache.archiveCached {
if err := service.FS().Remove(cache.archivePath); err != nil {
service.logger.Warnf("Failed to clean up archive %q: %v", cache.archivePath, err)
}
}

return cache.err
Expand Down
10 changes: 10 additions & 0 deletions test/fixtures/provider-cache/windows-remote-url/main.tf
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
terraform {
required_providers {
null = {
source = "hashicorp/null"
version = "3.2.4"
}
}
}

resource "null_resource" "example" {}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# Intentionally empty
22 changes: 22 additions & 0 deletions test/integration_windows_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ const (
testFixtureLocalRelativeArgsWindowsDownloadPath = "fixtures/download/local-windows"
testFixtureManifestRemoval = "fixtures/manifest-removal"
testFixtureTflintNoIssuesFound = "fixtures/tflint/no-issues-found"
testFixtureProviderCacheWindowsRemoteURL = "fixtures/provider-cache/windows-remote-url"

tempDir = `C:\tmp`
)
Expand Down Expand Up @@ -228,6 +229,27 @@ func TestWindowsScaffoldRef(t *testing.T) {
assert.NoError(t, err)
}

// TestWindowsProviderCacheWithRemoteURL verifies that provider caching works on Windows
// when providers are fetched from remote URLs. Previously, passing a URL like
// "https://github.com/..." to os.Stat caused a "syntax is incorrect" error on Windows
// because the colon in "https:" is invalid Windows path syntax.
func TestWindowsProviderCacheWithRemoteURL(t *testing.T) {
t.Parallel()

helpers.CleanupTerraformFolder(t, testFixtureProviderCacheWindowsRemoteURL)
tmpEnvPath := helpers.CopyEnvironment(t, testFixtureProviderCacheWindowsRemoteURL)
rootPath := filepath.Join(tmpEnvPath, testFixtureProviderCacheWindowsRemoteURL)

providerCacheDir := filepath.Join(t.TempDir(), "provider-cache-windows-test")

helpers.RunTerragrunt(t, fmt.Sprintf("terragrunt run init --provider-cache --provider-cache-dir %s --non-interactive --working-dir %s", providerCacheDir, rootPath))

// Verify the provider was cached
entries, err := os.ReadDir(providerCacheDir)
require.NoError(t, err)
assert.NotEmpty(t, entries, "provider cache dir should not be empty after init")
}

func CopyEnvironmentToPath(t *testing.T, environmentPath, targetPath string) {
if err := os.MkdirAll(targetPath, 0777); err != nil {
t.Fatalf("Failed to create temp dir %s due to error %v", targetPath, err)
Expand Down
Loading