starred/terravision

Fork 0

mirror of https://github.com/patrickchugh/terravision.git synced 2026-05-09 08:25:25 +02:00

) #186

New issue

Closed

opened 2026-05-06 12:38:04 +02:00 by BreizhHardware · 0 comments

BreizhHardware commented

2026-05-06 12:38:04 +02:00

Owner

📋 Pull Request Information

Original PR: https://github.com/patrickchugh/terravision/pull/164
Author: @j-srodka
Created: 1/28/2026
Status: ✅ Merged
Merged: 2/7/2026
Merged by: @patrickchugh

Base: main ← Head: fix/subdirectory-notation-support

📝 Commits (2)

fbc1e2b Fix FileNotFoundError for modules with subdirectory notation (//)
5303d61 Fix module cache issue and enhance URL parsing for all source types

📊 Changes

1 file changed (+40 additions, -12 deletions)

View changed files

📝 modules/gitlibs.py (+40 -12)

📄 Description

Pull Request: Fix FileNotFoundError for modules with subdirectory notation (//)

Description

This PR fixes issue #163 where Terravision crashes with FileNotFoundError when processing Terraform modules that use the // subdirectory notation.

Updates (v2): Enhanced fix to address additional feedback:

Fixed module cache issue on second run
Added proper support for git:: prefixed URLs
Improved URL parsing for all Terraform module source types

Problem

When processing modules like terraform-aws-modules/ecs/aws//modules/cluster, Terravision had two issues:

Incorrect cache paths: Sanitizing the entire URL including the subfolder portion resulted in path mismatches and FileNotFoundError
Cache not used on second run: Main source modules weren't being cached, causing issues when running the same command twice

Example of the Bug:

Input: terraform-aws-modules/ecs/aws//modules/cluster

Current (incorrect) behavior:

Cache dir: terraform-aws-modules_ecs_aws__modules_cluster;module_name;
Lookup path: terraform-aws-modules_ecs_aws__modules_cluster;module_name;/cluster
Result: FileNotFoundError ❌

Expected behavior:

Cache dir: terraform-aws-modules_ecs_aws;module_name;
Lookup path: terraform-aws-modules_ecs_aws;module_name;/modules/cluster
Result: Module files found successfully ✅

Changes Made

1. Enhanced `clone_files()` function - Base URL extraction

Original issue: Simple split("//") incorrectly parsed URLs with protocols (e.g., git::https:// split at protocol's //)

Fix: Added intelligent URL parsing that handles:

git:: prefixed URLs: git::https://github.com/repo.git//subfolder
- Strips git:: prefix
- Finds protocol end (https://)
- Only then looks for subfolder separator //
HTTPS URLs: https://github.com/repo.git//subfolder
- Finds protocol end after https://
- Looks for subfolder // only in remaining part
Registry modules: terraform-aws-modules/ecs/aws//modules/cluster
- Simple split on // (no protocol to avoid)

2. Fixed cache behavior for main modules

Original issue: Cache check had and module != "main" preventing main source caching

Fix: Removed condition so ALL modules (including "main") use cache on subsequent runs

# Before
if os.path.exists(codepath) and module != "main":
    return _handle_cached_module(...)

# After  
if os.path.exists(codepath):
    return _handle_cached_module(...)

3. Updated `_handle_cached_module()` function

Added subfolder parameter to function signature
Include subfolder in the returned path for cached modules
Updated docstring to reflect new parameter

Impact

This fix affects any Terraform configuration using modules with subdirectory notation:

✅ Registry modules: terraform-aws-modules/ecs/aws//modules/cluster
✅ Git URLs with git:: prefix: git::https://github.com/example/repo.git//path/to/module
✅ Git URLs with ssh: git::ssh://git@github.com/example/repo.git//path/to/module
✅ HTTPS URLs: https://github.com/example/repo.git//path/to/module
✅ GitHub shorthand: github.com/example/repo//path/to/module

This is a very common pattern, especially with official terraform-aws-modules collections.

Testing

Test 1: Maintainer's example

Source: https://github.com/patrickchugh/testcase-bastion.git//examples

✅ First run: Successfully clones and processes files
✅ Second run: Uses cache correctly (no FileNotFoundError)

Test 2: Registry modules with subdirectories

Tested with real-world Terraform configurations:

terraform-aws-modules/ecs/aws//modules/cluster ✅
terraform-aws-modules/ecs/aws//modules/service ✅
terraform-aws-modules/route53/aws//modules/records ✅
terraform-aws-modules/cloudwatch/aws//modules/log-metric-filter ✅

All modules were successfully processed without FileNotFoundError.

Test 3: git:: prefixed URLs

✅ git::https://github.com/terraform-aws-modules/terraform-aws-vpc.git//examples/simple-vpc

Test 4: Local testing

Successfully tested locally on macOS (darwin_arm64) with:

No FileNotFoundError on first or second run
Proper cache directory creation
Correct module path resolution

Backwards Compatibility

✅ This change is fully backwards compatible:

Modules without // work exactly as before
No changes to API or function signatures (only internal logic and one parameter addition to a private function)
Existing cache directories are not affected

Files Changed

modules/gitlibs.py: Fixed path handling for subdirectory notation

Closes #163

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/patrickchugh/terravision/pull/164 **Author:** [@j-srodka](https://github.com/j-srodka) **Created:** 1/28/2026 **Status:** ✅ Merged **Merged:** 2/7/2026 **Merged by:** [@patrickchugh](https://github.com/patrickchugh) **Base:** `main` ← **Head:** `fix/subdirectory-notation-support` --- ### 📝 Commits (2) - [`fbc1e2b`](https://github.com/patrickchugh/terravision/commit/fbc1e2b4d19491f3693a75056eaac4d5e0078f40) Fix FileNotFoundError for modules with subdirectory notation (//) - [`5303d61`](https://github.com/patrickchugh/terravision/commit/5303d61f2ee3f39e05380586fcd036989e29f9c3) Fix module cache issue and enhance URL parsing for all source types ### 📊 Changes **1 file changed** (+40 additions, -12 deletions) <details> <summary>View changed files</summary> 📝 `modules/gitlibs.py` (+40 -12) </details> ### 📄 Description # Pull Request: Fix FileNotFoundError for modules with subdirectory notation (//) ## Description This PR fixes issue #163 where Terravision crashes with `FileNotFoundError` when processing Terraform modules that use the `//` subdirectory notation. **Updates (v2)**: Enhanced fix to address additional feedback: - Fixed module cache issue on second run - Added proper support for `git::` prefixed URLs - Improved URL parsing for all Terraform module source types ## Problem When processing modules like `terraform-aws-modules/ecs/aws//modules/cluster`, Terravision had two issues: 1. **Incorrect cache paths**: Sanitizing the entire URL including the subfolder portion resulted in path mismatches and `FileNotFoundError` 2. **Cache not used on second run**: Main source modules weren't being cached, causing issues when running the same command twice ### Example of the Bug: **Input**: `terraform-aws-modules/ecs/aws//modules/cluster` **Current (incorrect) behavior**: - Cache dir: `terraform-aws-modules_ecs_aws__modules_cluster;module_name;` - Lookup path: `terraform-aws-modules_ecs_aws__modules_cluster;module_name;/cluster` - **Result**: FileNotFoundError ❌ **Expected behavior**: - Cache dir: `terraform-aws-modules_ecs_aws;module_name;` - Lookup path: `terraform-aws-modules_ecs_aws;module_name;/modules/cluster` - **Result**: Module files found successfully ✅ ## Changes Made ### 1. Enhanced `clone_files()` function - Base URL extraction **Original issue**: Simple `split("//")` incorrectly parsed URLs with protocols (e.g., `git::https://` split at protocol's `//`) **Fix**: Added intelligent URL parsing that handles: - **git:: prefixed URLs**: `git::https://github.com/repo.git//subfolder` - Strips `git::` prefix - Finds protocol end (`https://`) - Only then looks for subfolder separator `//` - **HTTPS URLs**: `https://github.com/repo.git//subfolder` - Finds protocol end after `https://` - Looks for subfolder `//` only in remaining part - **Registry modules**: `terraform-aws-modules/ecs/aws//modules/cluster` - Simple split on `//` (no protocol to avoid) ### 2. Fixed cache behavior for main modules **Original issue**: Cache check had `and module != "main"` preventing main source caching **Fix**: Removed condition so ALL modules (including "main") use cache on subsequent runs ```python # Before if os.path.exists(codepath) and module != "main": return _handle_cached_module(...) # After if os.path.exists(codepath): return _handle_cached_module(...) ``` ### 3. Updated `_handle_cached_module()` function - Added `subfolder` parameter to function signature - Include subfolder in the returned path for cached modules - Updated docstring to reflect new parameter ## Impact This fix affects any Terraform configuration using modules with subdirectory notation: - ✅ Registry modules: `terraform-aws-modules/ecs/aws//modules/cluster` - ✅ Git URLs with git:: prefix: `git::https://github.com/example/repo.git//path/to/module` - ✅ Git URLs with ssh: `git::ssh://git@github.com/example/repo.git//path/to/module` - ✅ HTTPS URLs: `https://github.com/example/repo.git//path/to/module` - ✅ GitHub shorthand: `github.com/example/repo//path/to/module` This is a very common pattern, especially with official `terraform-aws-modules` collections. ## Testing ### Test 1: Maintainer's example **Source**: `https://github.com/patrickchugh/testcase-bastion.git//examples` - ✅ First run: Successfully clones and processes files - ✅ Second run: Uses cache correctly (no FileNotFoundError) ### Test 2: Registry modules with subdirectories Tested with real-world Terraform configurations: - `terraform-aws-modules/ecs/aws//modules/cluster` ✅ - `terraform-aws-modules/ecs/aws//modules/service` ✅ - `terraform-aws-modules/route53/aws//modules/records` ✅ - `terraform-aws-modules/cloudwatch/aws//modules/log-metric-filter` ✅ All modules were successfully processed without `FileNotFoundError`. ### Test 3: git:: prefixed URLs - ✅ `git::https://github.com/terraform-aws-modules/terraform-aws-vpc.git//examples/simple-vpc` ### Test 4: Local testing Successfully tested locally on macOS (darwin_arm64) with: - No FileNotFoundError on first or second run - Proper cache directory creation - Correct module path resolution ## Backwards Compatibility ✅ This change is fully backwards compatible: - Modules without `//` work exactly as before - No changes to API or function signatures (only internal logic and one parameter addition to a private function) - Existing cache directories are not affected ## Files Changed - `modules/gitlibs.py`: Fixed path handling for subdirectory notation --- **Closes #163** --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

BreizhHardware

2026-05-06 12:38:04 +02:00

closed this issue
added the
pull-request
label

No labels

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

starred/terravision#186

No description provided.

Rows
Columns