upstream/opentofu
1
0
Fork 0
mirror of https://github.com/opentofu/opentofu.git synced 2026-02-18 18:17:54 -05:00
Code Issues Projects Releases Packages Wiki Activity Actions 6

Improved missing subpath detection in module installation (#3184)

Browse source 
Signed-off-by: James Humphries <james@james-humphries.co.uk>
Signed-off-by: James Humphries <James@james-humphries.co.uk>
Co-authored-by: Andrei Ciobanu <andrei.ciobanu@opentofu.org>
This commit is contained in:
James Humphries 2025-08-27 11:06:38 +01:00 committed by GitHub
parent c29311ee82
commit 1da126365c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 115 additions and 19 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

53
internal/initwd/module_install.go
View file

@ -85,24 +85,44 @@ func NewModuleInstaller(modsDir string, loader *configload.Loader, registryClien
}
}
// isSubDirNonExistant checks if the error is due to a non-existent subdirectory
// isSubDirNonExistent checks if the error is due to a non-existent subdirectory
// within an otherwise valid module. This helps distinguish between a genuine
// OpenTofu bug when failing to get the module and a user configuration error where they've specified a
// submodule path that doesn't exist.
func isSubDirNonExistant(modDir string) bool {
parent := filepath.Dir(modDir)
if parent == modDir || parent == "." || parent == "/" {
return false
func isSubDirNonExistent(modDir string) (isNonExistent bool, missingDir string) {
modDir = filepath.Clean(modDir)
_, err := os.Stat(modDir)
// If the directory exists, this isn't a missing subdir case
if err == nil {
return false, ""
}
// If it's not a "does not exist" error, return false (unexpected error)
if !os.IsNotExist(err) {
return false, ""
}
// Check if parent directory exists but the subdir doesn't
if _, err := os.Stat(parent); err == nil {
if _, err := os.Stat(modDir); os.IsNotExist(err) {
return true
var missingParts []string
current := modDir
for {
parent := filepath.Dir(current)
if parent == current {
return false, "" // reached root without finding it
}
}
return false
missingParts = append([]string{filepath.Base(current)}, missingParts...)
info, err := os.Stat(parent)
if err == nil && info.IsDir() { // return the missing parts!
if len(missingParts) > 0 {
return true, missingParts[0]
}
return true, filepath.Base(current)
}
current = parent
}
}
// InstallModules analyses the root module in the given directory and installs
@ -816,15 +836,17 @@ func (i *ModuleInstaller) installRegistryModule(ctx context.Context, req *config
// Finally we are ready to try actually loading the module.
mod, mDiags := i.loader.Parser().LoadConfigDir(modDir, req.Call)
if mod == nil {
isMissingSubDir, missingDir := isSubDirNonExistent(modDir)
// nil indicates missing or unreadable directory, so we'll
// discard the returned diags and return a more specific
// error message here.
if subDir != "" && isSubDirNonExistant(modDir) {
if subDir != "" && isMissingSubDir {
// This may be a user error, or a submodule may have been removed between unpinned versions of the module (ie a module update)
diags = diags.Append(&hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Module subdirectory not found",
Detail: fmt.Sprintf("The subdirectory %q does not exist in module %q. Please verify that the subdirectory path is correct.", subDir, instPath),
Detail: fmt.Sprintf("Cannot find directory %q in module %q. The requested subdirectory was %q.", missingDir, instPath, subDir),
Subject: req.CallRange.Ptr(),
})
} else {
@ -941,12 +963,13 @@ func (i *ModuleInstaller) installGoGetterModule(ctx context.Context, req *config
// nil indicates missing or unreadable directory, so we'll
// discard the returned diags and return a more specific
// error message here.
if addr.Subdir != "" && isSubDirNonExistant(modDir) {
isNonExistent, missingDir := isSubDirNonExistent(modDir)
if addr.Subdir != "" && isNonExistent {
// This is a user configuration error - they referenced a submodule that doesn't exist
diags = diags.Append(&hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Module subdirectory not found",
Detail: fmt.Sprintf("The subdirectory %q does not exist in the downloaded module. Verify that the subdirectory path is correct.", addr.Subdir),
Detail: fmt.Sprintf("Cannot find directory %q in the module path %q. The requested subdirectory was %q.", missingDir, instPath, addr.Subdir),
Subject: req.CallRange.Ptr(),
})
} else {

81
internal/initwd/module_install_test.go
View file

@ -167,7 +167,7 @@ func TestModuleInstaller_packageEscapeError(t *testing.T) {
t.Fatal(err)
}
final := bytes.ReplaceAll(template, []byte("%%BASE%%"), []byte(filepath.ToSlash(dir)))
err = os.WriteFile(rootFilename, final, 0644)
err = os.WriteFile(rootFilename, final, 0o644)
if err != nil {
t.Fatal(err)
}
@ -207,7 +207,7 @@ func TestModuleInstaller_explicitPackageBoundary(t *testing.T) {
t.Fatal(err)
}
final := bytes.ReplaceAll(template, []byte("%%BASE%%"), []byte(filepath.ToSlash(dir)))
err = os.WriteFile(rootFilename, final, 0644)
err = os.WriteFile(rootFilename, final, 0o644)
if err != nil {
t.Fatal(err)
}
@ -619,7 +619,6 @@ func TestLoaderInstallModules_registry(t *testing.T) {
gotTraces[path] = varDesc
})
assertResultDeepEqual(t, gotTraces, wantTraces)
}
func TestLoaderInstallModules_goGetter(t *testing.T) {
@ -747,7 +746,6 @@ func TestLoaderInstallModules_goGetter(t *testing.T) {
gotTraces[path] = varDesc
})
assertResultDeepEqual(t, gotTraces, wantTraces)
}
func TestModuleInstaller_fromTests(t *testing.T) {
@ -898,6 +896,81 @@ func TestLoadInstallModules_registryFromTest(t *testing.T) {
}
}
func TestIsSubDirNonExistent(t *testing.T) {
tmpDir := t.TempDir()
// Create test directory structure
existingDir := filepath.Join(tmpDir, "existing")
nestedExistingDir := filepath.Join(existingDir, "nested", "deep")
err := os.MkdirAll(nestedExistingDir, 0o755)
if err != nil {
t.Fatal(err)
}
tests := []struct {
name string
path string
wantExists bool
wantMissingDir string
}{
{
name: "directory exists",
path: existingDir,
wantExists: false,
wantMissingDir: "",
},
{
name: "nested directory exists",
path: nestedExistingDir,
wantExists: false,
wantMissingDir: "",
},
{
name: "single missing directory with existing parent",
path: filepath.Join(existingDir, "missing"),
wantExists: true,
wantMissingDir: "missing",
},
{
name: "nested missing directory",
path: filepath.Join(existingDir, "missing", "nested"),
wantExists: true,
wantMissingDir: "missing",
},
{
name: "deeply nested missing with existing parent",
path: filepath.Join(nestedExistingDir, "missing"),
wantExists: true,
wantMissingDir: "missing",
},
{
name: "multiple missing levels",
path: filepath.Join(existingDir, "missing", "also", "missingtoo"),
wantExists: true,
wantMissingDir: "missing",
},
{
name: "completely non-existent path",
path: filepath.Join(tmpDir, "completely", "missing", "path"),
wantExists: true,
wantMissingDir: "completely",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
gotExists, gotMissingDir := isSubDirNonExistent(tt.path)
if gotExists != tt.wantExists {
t.Errorf("isSubDirNonExistent() exists = %v, want %v", gotExists, tt.wantExists)
}
if gotMissingDir != tt.wantMissingDir {
t.Errorf("isSubDirNonExistent() missingDir = %q, want %q", gotMissingDir, tt.wantMissingDir)
}
})
}
}
// TestModuleInstaller_nonExistentSubmodule ensures that the error message returned when a module does not exist in an existing module
// is not that it's a bug in OpenTofu. See issue https://github.com/opentofu/opentofu/issues/3142 for more information
//