-
-
Notifications
You must be signed in to change notification settings - Fork 135
fix: normalize legacy triple-slash vendor URIs for go-getter compatibility #1504
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
aknysh
merged 56 commits into
main
from
feature/dev-3639-failed-to-pull-vendors-on-latest-atmos
Oct 2, 2025
Merged
Changes from 5 commits
Commits
Show all changes
56 commits
Select commit
Hold shift + click to select a range
e1d7241
fix: handle triple-slash vendor URIs after go-getter v1.7.9 update
osterman 38b3e93
refactor: rename vendor triple-slash test files to be more descriptive
osterman 5ed9378
docs: improve triple-slash pattern documentation
osterman 27e12d8
test: add unit tests for normalizeVendorURI and fix missing test func…
osterman f0e312f
docs: clarify triple-slash pattern as root of repository
osterman c51c3e1
fix: normalize vendor URIs to handle triple-slash pattern correctly
osterman f4bfec5
fix: address linting comment formatting
osterman 80b7742
fix: improve vendor URI normalization for triple-slash patterns
osterman a857141
Merge branch 'main' into feature/dev-3639-failed-to-pull-vendors-on-l…
osterman b03ef7d
Merge branch 'main' into feature/dev-3639-failed-to-pull-vendors-on-l…
osterman 32065af
test: address CodeRabbit feedback and improve test coverage
osterman ea903d5
style: add missing period to comment per linting requirements
osterman 22bafc6
test: update golden snapshots for vendor pull tests with new debug logs
osterman 1f7cf9a
Merge branch 'main' into feature/dev-3639-failed-to-pull-vendors-on-l…
osterman c7d1082
Add Atmos CLI performance profiling with multiple profile types (#1534)
aknysh a43ab81
fix: merge commands from all sources preserving precedence hierarchy …
osterman 7c6d4cd
chore(deps): bump github.com/aws/aws-sdk-go-v2/service/ssm (#1539)
dependabot[bot] 8f7cac3
chore(deps): bump github.com/aws/aws-sdk-go-v2/feature/s3/manager (#1…
dependabot[bot] 1e5b7bf
fix: Improve test infrastructure and fix critical environment variabl…
osterman b5d5313
fix: final linting adjustments after merge resolution
osterman 37b7e00
Merge branch 'main' of https://github.com/cloudposse/atmos into featu…
osterman caba443
fix: improve Windows path handling with UNC volume preservation and s…
osterman 071e50d
fix: add nolint directive for godot linter issue
osterman 86b75bd
Merge branch 'feature/dev-3639-failed-to-pull-vendors-on-latest-atmos…
osterman ab9d262
Merge remote-tracking branch 'origin/main' into feature/dev-3639-fail…
osterman 626fe9f
test: remove obsolete UNC path tests after merge
osterman 0fcd54b
test: add YAML-based precondition system for vendor pull tests
osterman e2fd0d5
Merge branch 'main' into feature/dev-3639-failed-to-pull-vendors-on-l…
osterman ca2825b
refactor: remove no-op adjustSubdir and restore integration tests
osterman 3e3d2ae
fix: resolve merge conflicts and apply pending changes
osterman 0a4e523
fix: enhance sanitizeImport and restore test best practices
osterman 7ba97f3
fix: add case-insensitive path matching for Windows CI snapshots
osterman 0274681
Merge remote-tracking branch 'origin/main' into feature/dev-3639-fail…
osterman 59a093b
fix: only replace backslashes on Windows in sanitizeOutput
osterman 24abf9a
fix: use regex to replace backslashes in path contexts only
osterman 656daf5
fix: add github_token precondition to slow vendor tests
osterman e881711
fix: generalize github_token precondition message
osterman 14decd6
refactor: replace heuristic vendor URI parsing with go-getter API
osterman 046f2ad
refactor: extract named helper functions in vendor URI detection
osterman 109bee8
docs: fix PRD to compare against main branch baseline
osterman 6c09ee1
fix: replace broad .Contains() checks with precise URL parsing
osterman 51c1f0f
fix: add SCP-style Git URL detection and security edge case tests
osterman 180a981
docs: clarify needsDoubleSlashDot function logic
osterman 7dd398b
docs: reorganize vendor URL examples by platform and update repos
osterman 9e971ed
docs: add language identifiers to fenced code blocks in PRD
osterman eae6225
test: add comprehensive coverage for triple-slash normalization helpers
osterman 86a5bf9
fix: correct hasSubdirectoryDelimiter to skip scheme separators
osterman fa26be5
Merge branch 'main' into feature/dev-3639-failed-to-pull-vendors-on-l…
osterman 201c215
fix: rename profiler-prefixed config errors to appropriate names
osterman b362f1d
fix: handle URIs ending with // in appendDoubleSlashDot
osterman b0d429d
fix: use ErrMerge sentinel for config loading error wrapping
osterman 42e1d13
refactor: move import errors to centralized errors package
osterman 774c6f9
refactor: sort resolved import paths for deterministic logging
osterman 767662e
Revert "refactor: sort resolved import paths for deterministic logging"
osterman 065ed3d
refactor: use t.Cleanup and t.Setenv for test teardown
osterman b8b270b
docs: add PR review thread response instructions to CLAUDE.md
osterman File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,222 @@ | ||
| package exec | ||
|
|
||
| import ( | ||
| "os" | ||
| "path/filepath" | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/assert" | ||
| "github.com/stretchr/testify/require" | ||
|
|
||
| "github.com/cloudposse/atmos/tests" | ||
| "github.com/spf13/cobra" | ||
| ) | ||
|
|
||
| // TestVendorPullWithTripleSlashPattern tests the vendor pull command with the triple-slash pattern | ||
| // which indicates cloning from the root of a repository (e.g., github.com/repo.git///?ref=v1.0). | ||
| // This pattern was broken after go-getter v1.7.9 due to changes in subdirectory path handling. | ||
| func TestVendorPullWithTripleSlashPattern(t *testing.T) { | ||
| // Check for GitHub access with rate limit check | ||
| rateLimits := tests.RequireGitHubAccess(t) | ||
| if rateLimits != nil && rateLimits.Remaining < 10 { | ||
| t.Skipf("Insufficient GitHub API requests remaining (%d). Test may require ~10 requests", rateLimits.Remaining) | ||
| } | ||
|
|
||
| // Store original environment variables | ||
| originalCliPath := os.Getenv("ATMOS_CLI_CONFIG_PATH") | ||
| originalBasePath := os.Getenv("ATMOS_BASE_PATH") | ||
| originalLogLevel := os.Getenv("ATMOS_LOGS_LEVEL") | ||
|
|
||
| // Set debug logging to see what's happening | ||
| os.Setenv("ATMOS_LOGS_LEVEL", "Debug") | ||
|
|
||
| // Capture the starting working directory | ||
| startingDir, err := os.Getwd() | ||
| require.NoError(t, err, "Failed to get the current working directory") | ||
|
|
||
| defer func() { | ||
| // Restore original environment variables | ||
| if originalCliPath != "" { | ||
| os.Setenv("ATMOS_CLI_CONFIG_PATH", originalCliPath) | ||
| } else { | ||
| os.Unsetenv("ATMOS_CLI_CONFIG_PATH") | ||
| } | ||
|
|
||
| if originalBasePath != "" { | ||
| os.Setenv("ATMOS_BASE_PATH", originalBasePath) | ||
| } else { | ||
| os.Unsetenv("ATMOS_BASE_PATH") | ||
| } | ||
|
|
||
| if originalLogLevel != "" { | ||
| os.Setenv("ATMOS_LOGS_LEVEL", originalLogLevel) | ||
| } else { | ||
| os.Unsetenv("ATMOS_LOGS_LEVEL") | ||
| } | ||
|
|
||
| // Change back to the original working directory after the test | ||
| if err := os.Chdir(startingDir); err != nil { | ||
| t.Fatalf("Failed to change back to the starting directory: %v", err) | ||
| } | ||
| }() | ||
|
|
||
| // Define the test directory | ||
| testDir := "../../tests/fixtures/scenarios/vendor-triple-slash" | ||
|
|
||
| // Change to the test directory | ||
| err = os.Chdir(testDir) | ||
| require.NoError(t, err, "Failed to change to test directory") | ||
|
|
||
| // Set up the command | ||
| cmd := &cobra.Command{} | ||
| cmd.PersistentFlags().String("base-path", "", "Base path for Atmos project") | ||
| cmd.PersistentFlags().StringSlice("config", []string{}, "Paths to configuration file") | ||
| cmd.PersistentFlags().StringSlice("config-path", []string{}, "Path to configuration directory") | ||
|
|
||
| flags := cmd.Flags() | ||
| flags.String("component", "s3-bucket", "") | ||
| flags.String("stack", "", "") | ||
| flags.String("tags", "", "") | ||
| flags.Bool("dry-run", false, "") | ||
| flags.Bool("everything", false, "") | ||
|
|
||
| // Execute vendor pull command | ||
| err = ExecuteVendorPullCommand(cmd, []string{}) | ||
| require.NoError(t, err, "Vendor pull command should execute without error") | ||
|
|
||
| // Check that the target directory was created | ||
| targetDir := filepath.Join("components", "terraform", "s3-bucket") | ||
| assert.DirExists(t, targetDir, "Target directory should be created") | ||
|
|
||
| // Test that the expected files were pulled from the repository | ||
| // According to the bug report, these files should be present but are not being pulled | ||
| expectedFiles := []string{ | ||
| // Main terraform files that should match "**/*.tf" | ||
| filepath.Join(targetDir, "main.tf"), | ||
| filepath.Join(targetDir, "outputs.tf"), | ||
| filepath.Join(targetDir, "variables.tf"), | ||
| filepath.Join(targetDir, "versions.tf"), | ||
|
|
||
| // Documentation files that should match "**/README.md" | ||
| filepath.Join(targetDir, "README.md"), | ||
|
|
||
| // License file that should match "**/LICENSE" | ||
| filepath.Join(targetDir, "LICENSE"), | ||
|
|
||
| // Module files that should match "**/modules/**" | ||
| // The terraform-aws-s3-bucket repository has modules subdirectory | ||
| filepath.Join(targetDir, "modules", "notification", "main.tf"), | ||
| filepath.Join(targetDir, "modules", "notification", "variables.tf"), | ||
| filepath.Join(targetDir, "modules", "notification", "outputs.tf"), | ||
| filepath.Join(targetDir, "modules", "notification", "versions.tf"), | ||
| } | ||
|
|
||
| // Check that files were actually pulled (not just an empty directory) | ||
| // This is the main assertion that should fail based on the bug report | ||
| for _, file := range expectedFiles { | ||
| assert.FileExists(t, file, "File should be pulled from repository: %s", file) | ||
| } | ||
|
|
||
| // Clean up: Remove the created directory and its contents | ||
| defer func() { | ||
| if err := os.RemoveAll(targetDir); err != nil { | ||
| t.Logf("Failed to clean up target directory: %v", err) | ||
| } | ||
| }() | ||
| } | ||
|
|
||
| // TestVendorPullWithMultipleVendorFiles tests that vendor pull works correctly | ||
| // even when there are multiple vendor YAML files in the same directory. | ||
| // This could be a potential cause of the issue where the vendor process | ||
| // gets confused by multiple configuration files. | ||
| func TestVendorPullWithMultipleVendorFiles(t *testing.T) { | ||
coderabbitai[bot] marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| // Check for GitHub access with rate limit check | ||
| rateLimits := tests.RequireGitHubAccess(t) | ||
| if rateLimits != nil && rateLimits.Remaining < 10 { | ||
| t.Skipf("Insufficient GitHub API requests remaining (%d). Test may require ~10 requests", rateLimits.Remaining) | ||
| } | ||
|
|
||
| // Store original environment variables | ||
| originalCliPath := os.Getenv("ATMOS_CLI_CONFIG_PATH") | ||
| originalBasePath := os.Getenv("ATMOS_BASE_PATH") | ||
| originalLogLevel := os.Getenv("ATMOS_LOGS_LEVEL") | ||
|
|
||
| // Set debug logging to see what's happening | ||
| os.Setenv("ATMOS_LOGS_LEVEL", "Debug") | ||
|
|
||
| // Capture the starting working directory | ||
| startingDir, err := os.Getwd() | ||
| require.NoError(t, err, "Failed to get the current working directory") | ||
|
|
||
| defer func() { | ||
| // Restore original environment variables | ||
| if originalCliPath != "" { | ||
| os.Setenv("ATMOS_CLI_CONFIG_PATH", originalCliPath) | ||
| } else { | ||
| os.Unsetenv("ATMOS_CLI_CONFIG_PATH") | ||
| } | ||
|
|
||
| if originalBasePath != "" { | ||
| os.Setenv("ATMOS_BASE_PATH", originalBasePath) | ||
| } else { | ||
| os.Unsetenv("ATMOS_BASE_PATH") | ||
| } | ||
|
|
||
| if originalLogLevel != "" { | ||
| os.Setenv("ATMOS_LOGS_LEVEL", originalLogLevel) | ||
| } else { | ||
| os.Unsetenv("ATMOS_LOGS_LEVEL") | ||
| } | ||
|
|
||
| // Change back to the original working directory after the test | ||
| if err := os.Chdir(startingDir); err != nil { | ||
| t.Fatalf("Failed to change back to the starting directory: %v", err) | ||
| } | ||
| }() | ||
|
|
||
| // Define the test directory | ||
| testDir := "../../tests/fixtures/scenarios/vendor-triple-slash" | ||
|
|
||
| // Change to the test directory | ||
| err = os.Chdir(testDir) | ||
| require.NoError(t, err, "Failed to change to test directory") | ||
|
|
||
| // Verify that multiple vendor files exist in the directory | ||
| vendorFiles := []string{"vendor.yaml", "vendor-test.yaml"} | ||
| for _, file := range vendorFiles { | ||
| assert.FileExists(t, file, "Vendor file should exist: %s", file) | ||
| } | ||
|
|
||
| // Set up the command | ||
| cmd := &cobra.Command{} | ||
| cmd.PersistentFlags().String("base-path", "", "Base path for Atmos project") | ||
| cmd.PersistentFlags().StringSlice("config", []string{}, "Paths to configuration file") | ||
| cmd.PersistentFlags().StringSlice("config-path", []string{}, "Path to configuration directory") | ||
|
|
||
| flags := cmd.Flags() | ||
| flags.String("component", "", "") | ||
| flags.String("stack", "", "") | ||
| flags.String("tags", "aws", "") | ||
| flags.Bool("dry-run", false, "") | ||
| flags.Bool("everything", false, "") | ||
|
|
||
| // Execute vendor pull command with tags filter | ||
| err = ExecuteVendorPullCommand(cmd, []string{}) | ||
| require.NoError(t, err, "Vendor pull command should execute without error even with multiple vendor files") | ||
|
|
||
| // Check that the s3-bucket component was pulled (it has the 'aws' tag) | ||
| targetDir := filepath.Join("components", "terraform", "s3-bucket") | ||
| assert.DirExists(t, targetDir, "Target directory for s3-bucket should be created") | ||
|
|
||
| // Verify that at least some files were pulled | ||
| entries, err := os.ReadDir(targetDir) | ||
| assert.NoError(t, err, "Should be able to read target directory") | ||
| assert.NotEmpty(t, entries, "Target directory should not be empty - files should have been pulled") | ||
|
|
||
| // Clean up: Remove the created directory and its contents | ||
| defer func() { | ||
| if err := os.RemoveAll(targetDir); err != nil { | ||
| t.Logf("Failed to clean up target directory: %v", err) | ||
| } | ||
| }() | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.