Convert SCP-style URLs (no explicit scheme) into proper SSH URLs #1061
Convert SCP-style URLs (no explicit scheme) into proper SSH URLs #1061Listener430 wants to merge 79 commits intomainfrom
Conversation
📝 WalkthroughWalkthroughThis pull request refactors the Git detection and vendoring process within Atmos. It renames and restructures the custom Git detector to support multiple hosting services, adds new helper methods for URL scheme validation, normalization, and token injection, and introduces a custom getter for post-clone cleanup. Several YAML configuration files, tests, and documentation updates are also included to better support SSH/HTTPS vendoring, new environment variables for Bitbucket and GitLab, and ensure secure handling of credentials. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Detector as CustomGitDetector
participant Parser as Git URL Parser
participant Resolver as Token Resolver
participant Getter as CustomGitGetter
Client->>Detector: Detect(src)
Detector->>Detector: ensureScheme(src)
Detector->>Detector: rewriteSCPURL(src)
Detector->>Parser: Parse & normalize URL
Detector->>Resolver: Resolve token for host
Resolver-->>Detector: Return token & username
Detector->>Detector: injectToken(validated URL, host)
Detector-->>Client: Return final URL, validity flag, error
Client->>Getter: Get(dst, final URL)
Getter->>Getter: Perform clone operation
Getter->>Getter: removeSymlinks(root)
Getter-->>Client: Return clone operation status
Possibly related PRs
Suggested labels
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 golangci-lint (1.62.2)Error: can't load config: the Go language version (go1.23) used to build golangci-lint is lower than the targeted Go version (1.24.0) Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
internal/exec/go_getter_utils.go (5)
15-15: Consider using a more descriptive alias
Short aliases likelmight hamper clarity. Renaming tologorloggercould improve readability.
70-70: Suggest clarifying the field name
sourcecould be named more explicitly, such assourceURIororiginalSource, for improved code clarity.
90-112: Add optional support for custom ports
SCP-style URLs sometimes specify a custom port (e.g.,git@host:port/repo). The current regex won’t match those.- scpPattern := regexp.MustCompile(`^(([\w.-]+)@)?([\w.-]+\.[\w.-]+):([\w./-]+)(\.git)?(.*)$`) + scpPattern := regexp.MustCompile(`^(([\w.-]+)@)?([\w.-]+\.[\w.-]+)(:[0-9]+)?:([\w./-]+)(\.git)?(.*)$`)
136-138: Convert TBC comment into a TODO
Documenting pending enhancements is good. Consider adding a// TODO:or opening an issue.Would you like me to open an issue for broadening token injection support?
139-164: Ensure consistent config toggles
Only GitHub token injection is governed byInjectGithubToken. Consider adding similar toggles for Bitbucket and GitLab for uniformity.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
internal/exec/go_getter_utils.go(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (17)
- GitHub Check: [mock-macos] tests/fixtures/scenarios/complete
- GitHub Check: [mock-macos] examples/demo-vendoring
- GitHub Check: [mock-macos] examples/demo-context
- GitHub Check: [mock-macos] examples/demo-component-versions
- GitHub Check: [mock-macos] examples/demo-atlantis
- GitHub Check: [mock-windows] examples/demo-context
- GitHub Check: [mock-windows] examples/demo-component-versions
- GitHub Check: [mock-windows] examples/demo-atlantis
- GitHub Check: [mock-linux] tests/fixtures/scenarios/complete
- GitHub Check: [mock-linux] examples/demo-vendoring
- GitHub Check: [mock-linux] examples/demo-context
- GitHub Check: Acceptance Tests (windows-latest, windows)
- GitHub Check: Acceptance Tests (ubuntu-latest, linux)
- GitHub Check: Docker Lint
- GitHub Check: [k3s] demo-helmfile
- GitHub Check: [localstack] demo-localstack
- GitHub Check: Summary
🔇 Additional comments (11)
internal/exec/go_getter_utils.go (11)
11-11: Looks good
No immediate concerns. Theregexpimport is necessary for the new SCP-style detection.
61-61: Confirmed
Addinggit::sshto the list of valid schemes aligns with go-getter usage.
66-68: Renaming is consistent
This rename clarifies support for multiple Git hosting platforms.
81-88: Excellent documentation
The inline comments clearly explain how SCP-style URLs are handled.
115-115: Duplicate concern: potential credential exposure
Similar to the previous logging ofsrc, sensitive info may be leaked.
123-126: Clear error handling
Returning an error when no SSH agent is available is straightforward.
128-133: Non-standard host scenario well-handled
Skipping token injection for unrecognized hosts is logical.
166-175: Token injection logic is robust
Injecting the token only if credentials aren’t already present is correct.
177-185: Subdir detection
Appending//.for top-level repos is a known go-getter approach. Looks good.
187-197: Default shallow clone
Specifyingdepth=1improves performance but may break use cases needing full history. Confirm this suits your workflows.
208-208: Registering the new CustomGitDetector
Ensuring the custom detector runs first is correct.
|
To test vendoring of SSH style URLs, without SSH key, test instead |
|
Windows tests are failing: === RUN TestCLICommands/atmos_vendor_pull_(no_tty)
cli_test.go:901: Stderr diff mismatch for "D:\\a\\atmos\\atmos\\tests\\snapshots\\TestCLICommands_atmos_vendor_pull_(no_tty).stdout.golden":
--- expected
+++ actual
@@ -1,7 +1,12 @@
INFO Vendoring from 'vendor.yaml'
WARN No TTY detected. Falling back to basic output. This can happen when no terminal is attached or when commands are pipelined.
-INFO ✓ github/stargazers (main)
-INFO ✓ weather (main)
-INFO ✓ ipinfo (main)
+ERRO Failed to vendor github/stargazers: error : failed to download package: subdir "examples%5Cdemo-library%5Cgithub%5Cstargazers" not found
+INFO x github/stargazers (main)
+ERRO Failed to vendor weather: error : failed to download package: subdir "examples%5Cdemo-library%5Cweather" not found
+INFO x weather (main)
+ERRO Failed to vendor ipinfo: error : failed to download package: subdir "examples%5Cdemo-library%5Cipinfo" not found
+INFO x ipinfo (main)
+INFO Vendored 0 components. Failed to vendor 3 components.
+ |
Have added the test, but it is in disabled state now as current dry run implementation in internal\exec\vendor_component_utils.go exits before hitting custom detectors and go getters. Let me know there are any ojectiosn to adjust vendor_component_utils.go code for the dry run to operate differently. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/test-cases/demo-vendoring.yaml (1)
58-59: Clean up YAML formatting.Fix the following formatting issues:
- Line 58: Remove trailing spaces
- Line 59: Add newline at end of file
- - "No SSH authentication method found" - exit_code: 0 \ No newline at end of file + - "No SSH authentication method found" + exit_code: 0 +🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 58-58: trailing spaces
(trailing-spaces)
[error] 59-59: no new line character at the end of file
(new-line-at-end-of-file)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
internal/exec/go_getter_utils.go(3 hunks)tests/snapshots/TestCLICommands_atmos_vendor_pull_ssh.stderr.golden(1 hunks)tests/test-cases/demo-vendoring.yaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- internal/exec/go_getter_utils.go
🧰 Additional context used
🪛 YAMLlint (1.35.1)
tests/test-cases/demo-vendoring.yaml
[error] 58-58: trailing spaces
(trailing-spaces)
[error] 59-59: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: Summary
🔇 Additional comments (1)
tests/test-cases/demo-vendoring.yaml (1)
43-59: LGTM! Well-structured test case for SSH URL handling.The test case effectively validates:
- SSH-style URL handling
- Dry-run behavior
- Error messaging for missing SSH authentication
- Debug logging output
This aligns perfectly with the PR objectives for SCP-style URL support.
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 58-58: trailing spaces
(trailing-spaces)
[error] 59-59: no new line character at the end of file
(new-line-at-end-of-file)
Co-authored-by: Erik Osterman (CEO @ Cloud Posse) <erik@cloudposse.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
website/docs/cli/commands/vendor/vendor-pull.mdx (5)
111-118: Clarify SSH Vendoring Behavior
The introduction clearly explains SSH support for non-public repositories. To further guide users, consider explicitly noting that while SSH is convenient for local development, credentials (such as tokens) are not injected for SSH sources. This distinction reinforces why automated environments should prefer HTTPS.
128-133: URL Rewriting Clarity
The example showing the rewritten URL is concise and correct. Consider adding a brief explanation—perhaps as an inline note—on why the?depth=1parameter is appended (e.g., to minimize clone time and optimize performance).
172-203: Authentication & Token Usage Details
The detailed tables for GitHub, Bitbucket, and GitLab authentication priorities are comprehensive and invaluable. As a minor improvement, consider using typographic quotes (as hinted by the static analysis) for values like"x-access-token"and"x-token-auth"to enhance readability and consistency.🧰 Tools
🪛 LanguageTool
[typographical] ~189-~189: Consider using a typographic close quote here.
Context: ...e and does not accept dummy values like"x-access-token". | **Default Username ...(EN_QUOTES)
[typographical] ~194-~194: Consider using a typographic close quote here.
Context: ...s set, it is used. - If neither is set,"x-token-auth"is used. ### GitLab | En...(EN_QUOTES)
204-211: Git URL Resolution Rules
The rules for resolving Git URLs are clearly defined and align with the PR objectives. One minor nitpick: replacing the plain ellipsis with a typographic ellipsis (…) in the example could improve stylistic consistency, as suggested by static analysis.🧰 Tools
🪛 LanguageTool
[style] ~206-~206: Consider using a different verb to strengthen your wording.
Context: ...lved When resolving Git sources, Atmos follows these rules: 1. If a *full HTTPS URL...(FOLLOW_OBEY)
[style] ~208-~208: Consider using the typographical ellipsis character here instead.
Context: ...1. If a full HTTPS URL is provided (git::https://github.com/...), it is used as-is. No token data is i...(ELLIPSIS)
222-232: Examples Section
The examples provided are diverse and clearly demonstrate different usage scenarios. To further support users, you might consider including a specific example that shows the transformation of an SCP-style URL into its SSH format.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
website/docs/cli/commands/vendor/vendor-pull.mdx(1 hunks)
🧰 Additional context used
🪛 LanguageTool
website/docs/cli/commands/vendor/vendor-pull.mdx
[typographical] ~189-~189: Consider using a typographic close quote here.
Context: ...e and does not accept dummy values like "x-access-token". | **Default Username ...
(EN_QUOTES)
[typographical] ~194-~194: Consider using a typographic close quote here.
Context: ...s set, it is used. - If neither is set, "x-token-auth" is used. ### GitLab | En...
(EN_QUOTES)
[style] ~206-~206: Consider using a different verb to strengthen your wording.
Context: ...lved When resolving Git sources, Atmos follows these rules: 1. If a *full HTTPS URL...
(FOLLOW_OBEY)
[style] ~208-~208: Consider using the typographical ellipsis character here instead.
Context: ...1. If a full HTTPS URL is provided (git::https://github.com/...), it is used as-is. No token data is i...
(ELLIPSIS)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: website-deploy-preview
- GitHub Check: Analyze (go)
- GitHub Check: Build (ubuntu-latest, linux)
- GitHub Check: Lint (golangci)
- GitHub Check: Summary
🔇 Additional comments (9)
website/docs/cli/commands/vendor/vendor-pull.mdx (9)
120-127: SCP-style Sources Explanation
This section effectively illustrates the traditional SCP-style format and its use of a colon as a separator. It might be helpful to add a brief note that Atmos automatically converts these sources into proper SSH URLs behind the scenes.
136-144: Explicit SSH Scheme Handling
This segment aptly differentiates between SCP-style and explicitly defined SSH URLs, noting that explicitssh://URLs bypass rewriting. Excellent clarity here.
145-152: Invalid URL Format Notice
The note regarding invalid URLs (e.g., usinggithub.com:...without the proper prefix) is well-articulated and useful. It clearly warns users about go-getter’s misinterpretation of such formats.
153-158: Scheme Default Explanation
The explanation that URLs lacking an explicit scheme default to HTTPS—and trigger automatic credential injection—is clear and straightforward. No changes needed here.
159-171: HTTPS Vendoring Section
This section provides a clear walkthrough with examples on how HTTPS URLs are resolved and transformed (including the addition of?depth=1). The content is consistent and informative.
212-216: Configuration Reference Note
The note directing users to further details on configuration is succinct and helpful.
218-220: Help Command Tip
The tip advising users to runatmos vendor pull --helpis clear and useful.
234-245: Vendoring Manifest Selection Note
The explanation regarding how Atmos chooses betweenvendor.yamlandcomponent.yamlis detailed and clear.🧰 Tools
🪛 LanguageTool
[grammar] ~239-~239: The word ‘vendor’ is a noun or an adjective. A verb or adverb is missing or misspelled here, or maybe a comma is missing.
Context: ...componentis not specified, Atmos will vendor all the artifacts defined in thevendo...(PRP_MD_NN)
[style] ~240-~240: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...efined in thevendor.yamlmanifest. If the flag--componentis passed in, At...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[grammar] ~240-~240: The word ‘vendor’ is a noun or an adjective. A verb or adverb is missing or misspelled here, or maybe a comma is missing.
Context: ...--componentis passed in, Atmos will vendor only that component - Ifvendor.yaml...(PRP_MD_NN)
[grammar] ~240-~240: Please add a punctuation mark at the end of paragraph.
Context: ... passed in, Atmos will vendor only that component - Ifvendor.yamlis not found, Atmos...(PUNCTUATION_PARAGRAPH_END)
[style] ~242-~242: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...mlmanifest in the component's folder. Ifcomponent.yaml` is not found, an err...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[grammar] ~243-~243: Please add a punctuation mark at the end of paragraph.
Context: ... flag--componentis required in this case ::: ## Flags | Flag | Descr...(PUNCTUATION_PARAGRAPH_END)
247-255: Flags Table
The flags table is well-organized and comprehensively illustrates the available options for the command.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
website/docs/cli/commands/vendor/vendor-pull.mdx (3)
189-200: Bitbucket Authentication Details with Minor Typographic Suggestion
The Bitbucket section is well structured with a table, clearly listing the environment variables and the default username rules. As a minor improvement, consider using typographic (curly) quotes for the default username"x-token-auth"to align with our style guidelines.🧰 Tools
🪛 LanguageTool
[typographical] ~196-~196: Consider using a typographic close quote here.
Context: ...e and does not accept dummy values like"x-access-token". | **Default Username ...(EN_QUOTES)
201-208: GitLab Authentication Section Consistency
This section is clear and mirrors the style used for GitHub and Bitbucket. Similar to the Bitbucket section, you might consider replacing the straight quotes around"oauth2"with typographic quotes for improved readability.🧰 Tools
🪛 LanguageTool
[typographical] ~201-~201: Consider using a typographic close quote here.
Context: ...s set, it is used. - If neither is set,"x-token-auth"is used. ### GitLab | En...(EN_QUOTES)
209-217: Refinement of Git URL Resolution Rules
The numbered list explaining how Atmos resolves Git sources is informative. As a nitpick, consider rephrasing the introductory sentence (e.g., "Atmos resolves Git sources using the following rules:") to strengthen clarity. Also, replacing the sequence of three dots (...) in the HTTPS URL example with a typographical ellipsis ( … ) could enhance the presentation per our static analysis hints.🧰 Tools
🪛 LanguageTool
[style] ~213-~213: Consider using a different verb to strengthen your wording.
Context: ...lved When resolving Git sources, Atmos follows these rules: 1. If a *full HTTPS URL...(FOLLOW_OBEY)
[style] ~215-~215: Consider using the typographical ellipsis character here instead.
Context: ...1. If a full HTTPS URL is provided (git::https://github.com/...), it is used as-is. No token data is i...(ELLIPSIS)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
website/docs/cli/commands/vendor/vendor-pull.mdx(1 hunks)
🧰 Additional context used
🪛 LanguageTool
website/docs/cli/commands/vendor/vendor-pull.mdx
[typographical] ~196-~196: Consider using a typographic close quote here.
Context: ...e and does not accept dummy values like "x-access-token". | **Default Username ...
(EN_QUOTES)
[typographical] ~201-~201: Consider using a typographic close quote here.
Context: ...s set, it is used. - If neither is set, "x-token-auth" is used. ### GitLab | En...
(EN_QUOTES)
[style] ~213-~213: Consider using a different verb to strengthen your wording.
Context: ...lved When resolving Git sources, Atmos follows these rules: 1. If a *full HTTPS URL...
(FOLLOW_OBEY)
[style] ~215-~215: Consider using the typographical ellipsis character here instead.
Context: ...1. If a full HTTPS URL is provided (git::https://github.com/...), it is used as-is. No token data is i...
(ELLIPSIS)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: Analyze (go)
- GitHub Check: Summary
🔇 Additional comments (8)
website/docs/cli/commands/vendor/vendor-pull.mdx (8)
111-118: Addition of the SSH Vendoring Introduction
The new section clearly explains that Atmos supports SSH-based vendoring for non-public Git repositories and establishes the context for local development. It provides a concise overview along with a helpful tip for automated systems.
120-127: Clear Explanation of SCP-style Sources
This section clearly details how traditional SCP-style sources (using a colon) are supported and provides a relevant example showing the expected syntax. The example helps readers understand the input format.
128-133: Documentation of URL Rewriting Behavior
The explanation of how Atmos rewrites the SCP-style URL into a proper SSH URL (adding thedepth=1parameter) is well presented. Ensure that the significance ofdepth=1is documented elsewhere if not already.
134-134: Default Username Injection for GitHub
The note that Atmos automatically injects the default usernamegitwhen none is provided forgithub.comis clear and aligns with our design.
135-143: Explicit SSH Scheme Usage
The section explaining that URLs already starting with thessh://scheme remain unmodified is straightforward and provides a concrete example. This helps clarify behavior for users preferring explicit schemes.
145-158: Important Notes on Invalid URL Formats
The warnings about using an invalid SCP-style URL (without the proper prefix) and about the default HTTPS behavior are precise and useful. This section efficiently prevents common pitfalls related to URL formatting.
159-171: HTTPS Vendoring Clarification
The explanation of how repository names without an explicit scheme are automatically resolved to HTTPS with credential injection is clear. The before-and-after examples effectively illustrate the transformation.
172-188: Comprehensive Authentication Instructions for GitHub
The GitHub authentication section uses a clean definition list to outline the priority of environment variables. It succinctly covers token usage and the default username strategy.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
website/docs/cli/commands/vendor/vendor-pull.mdx (3)
111-159: New SSH Vendoring Section Clarity and Completeness
The newly added "Vendoring from SSH" section is well structured. It clearly differentiates between SCP-style sources and explicit SSH sources and provides concrete examples of URL rewriting. One suggestion is to include a brief note about SSH key requirements for local development to help users understand potential caveats when using SSH in automated pipelines.
216-216: Wording Suggestion for Git Source Resolution
Consider revising the sentence "When resolving Git sources, Atmos follows these rules:" to a more active voice—perhaps "Atmos resolves Git sources using the following rules:"—to strengthen the tone.🧰 Tools
🪛 LanguageTool
[style] ~216-~216: Consider using a different verb to strengthen your wording.
Context: ...lved When resolving Git sources, Atmos follows these rules: 1. If a *full HTTPS URL...(FOLLOW_OBEY)
218-218: Typographical Ellipsis
The example URL uses three consecutive dots (...). For improved readability and typographical polish, consider replacing them with the typographical ellipsis character (…).🧰 Tools
🪛 LanguageTool
[style] ~218-~218: Consider using the typographical ellipsis character here instead.
Context: ...1. If a full HTTPS URL is provided (git::https://github.com/...), it is used as-is. No token data is i...(ELLIPSIS)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
internal/exec/vendor_utils.go(2 hunks)website/docs/cli/commands/vendor/vendor-pull.mdx(1 hunks)
🧰 Additional context used
🪛 LanguageTool
website/docs/cli/commands/vendor/vendor-pull.mdx
[style] ~216-~216: Consider using a different verb to strengthen your wording.
Context: ...lved When resolving Git sources, Atmos follows these rules: 1. If a *full HTTPS URL...
(FOLLOW_OBEY)
[style] ~218-~218: Consider using the typographical ellipsis character here instead.
Context: ...1. If a full HTTPS URL is provided (git::https://github.com/...), it is used as-is. No token data is i...
(ELLIPSIS)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: Build (ubuntu-latest, linux)
- GitHub Check: Analyze (go)
- GitHub Check: Lint (golangci)
- GitHub Check: website-deploy-preview
- GitHub Check: Summary
🔇 Additional comments (2)
internal/exec/vendor_utils.go (2)
22-23: Good addition of dedicated stderr logger.Creating a separate logger for stderr output is a clean approach that helps maintain separation between different output streams. This will keep stdout clean of detailed debug messages during vendoring operations, making it easier to parse program output in scripts or CI/CD pipelines.
514-514: Updated log statement to use the new StderrLogger.The log statement has been correctly updated to use the new StderrLogger.Debug instead of log.Debug, which aligns with the goal of separating detailed vendoring messages to stderr.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
pkg/utils/url_utils_test.go (2)
43-49: Proper error handling test.This test correctly validates that the function returns an error when given an invalid URL format. Consider adding a check for the specific error message to ensure the exact expected error is returned.
func TestMaskBasicAuth_InvalidURL(t *testing.T) { input := "://invalid-url" _, err := MaskBasicAuth(input) if err == nil { t.Errorf("Expected error for invalid URL, but got none") + } else if !strings.Contains(err.Error(), "parse") { + t.Errorf("Expected URL parsing error, got: %v", err) } }Don't forget to add
stringsto your imports if you implement this suggestion.
1-50: Consider adding test cases for SCP-style URLs.Since the PR's main objective is to handle SCP-style URLs (like
git@github.com:user/repo.git), consider adding specific test cases that verify the MaskBasicAuth function correctly handles these formats.func TestMaskBasicAuth_SCPStyleURL(t *testing.T) { input := "git@github.com:user/repo.git" expected := "git@github.com:user/repo.git" // or whatever the expected masking should be masked, err := MaskBasicAuth(input) if err != nil { t.Fatalf("Expected no error, got %v", err) } if masked != expected { t.Errorf("Expected masked URL %q, got %q", expected, masked) } }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pkg/utils/url_utils_test.go(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: Build (macos-latest, macos)
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: Build (ubuntu-latest, linux)
- GitHub Check: Lint (golangci)
- GitHub Check: website-deploy-preview
- GitHub Check: Analyze (go)
- GitHub Check: Summary
🔇 Additional comments (4)
pkg/utils/url_utils_test.go (4)
1-6: Good package structure and imports.The test file is properly organized with the correct package name matching the implementation. The imports are kept minimal with just the testing package, which is sufficient for these tests.
7-17: Well-structured test for username and password masking.This test effectively validates that URLs with both username and password have their credentials masked. The test includes proper error handling and clear assertion messages.
19-29: Good coverage for username-only scenario.The test correctly handles the case where a URL contains only a username without a password, ensuring it gets properly masked.
31-41: Appropriate test for URLs without credentials.This test verifies that URLs without any authentication information remain unchanged, which is an important edge case to verify.
what
Done:
This is a spin off of #984 that futher extends custom detector logic
Testing
Use this to run only test cases relevant for this PR
$ go test -v -run '^TestCLICommands/(atmos_vendor_pull_using_SSH|atmos_vendor_pull_with_custom_detector_and_handling_credentials_leakage)$' github.com/cloudposse/atmos/testsnon-standard SCP-style links handling
Token injections were tested wtih bitbucket and gitlab (http) for private and public repos + ssh vendoring for both.
Listing them here as there are no dedicated tests/repos available for testing at bitbucket/gitlab.
why
git::git@github.com:cloudposse/terraform-null-label.git?ref={{.Version}}references
Summary by CodeRabbit
New Features
Documentation
Refactor
Bug Fixes