✨ Local config and creds. #75
Conversation
Signed-off-by: Jeff Ortel <[email protected]>
Signed-off-by: Jeff Ortel <[email protected]>
Signed-off-by: Jeff Ortel <[email protected]>
WalkthroughThe changes refactor identity and environment handling across repository, SSH, and command execution modules. Identity management is simplified from lists to single identities, with new flexible option mechanisms for repository creation. Environment variables are now explicitly set for command execution, and configuration files are written to the current working directory instead of the home directory. Several method signatures are updated to reflect these changes. Changes
Sequence Diagram(s)sequenceDiagram
participant Main
participant RepoFactory as repository.New
participant SCM
participant AddonStore
participant Command
participant OS
Main->>RepoFactory: New(destDir, remote, option...)
RepoFactory->>SCM: create SCM instance
loop For each option
RepoFactory->>SCM: Use(option)
alt option is api.Ref
SCM->>AddonStore: Fetch Identity by Ref
SCM->>SCM: Set Identity
else option is api.Identity
SCM->>SCM: Set Identity
else
SCM->>RepoFactory: Return error
end
end
Main->>SCM: Use SCM (e.g., Fetch, Commit)
SCM->>Command: Prepare command with Env
Command->>OS: Execute with set Env
Possibly related issues
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
CodeRabbit Configuration File (
|
Signed-off-by: Jeff Ortel <[email protected]>
Signed-off-by: Jeff Ortel <[email protected]>
Signed-off-by: Jeff Ortel <[email protected]>
Signed-off-by: Jeff Ortel <[email protected]>
Signed-off-by: Jeff Ortel <[email protected]>
Signed-off-by: Jeff Ortel <[email protected]>
Signed-off-by: Jeff Ortel <[email protected]>
Signed-off-by: Jeff Ortel <[email protected]>
Signed-off-by: Jeff Ortel <[email protected]>
Signed-off-by: Jeff Ortel <[email protected]>
Signed-off-by: Jeff Ortel <[email protected]>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
repository/git.go (1)
84-96: Inconsistent command creation in Branch method.The fallback command still uses
command.Newdirectly instead ofr.git(), which means it won't have the necessary environment variables set (HOME, GIT_TERMINAL_PROMPT, etc.).Apply this diff to fix the inconsistency:
func (r *Git) Branch(ref string) (err error) { cmd := r.git() cmd.Dir = r.Path cmd.Options.Add("checkout", ref) err = cmd.Run() if err != nil { - cmd = command.New("/usr/bin/git") + cmd = r.git() cmd.Dir = r.Path cmd.Options.Add("checkout", "-b", ref) } r.Remote.Branch = ref err = cmd.Run() return }
🧹 Nitpick comments (3)
repository/maven.go (1)
46-48: Consider using the cleaner defer pattern.While explicitly ignoring the close error is acceptable here, the more idiomatic Go pattern would be:
- defer func() { - _ = f.Close() - }() + defer f.Close()repository/subversion.go (1)
199-199: Minor: Comment grammar improvement.-// writeConfig writes configuration file. +// writeConfig writes the configuration file.repository/factory.go (1)
16-18: Consider handling the error fromos.Getwd().The init function ignores the error from
os.Getwd(). If the working directory cannot be determined,Dirwill remain an empty string, which could cause issues when constructing file paths.Consider logging or handling the error:
func init() { - Dir, _ = os.Getwd() + var err error + Dir, err = os.Getwd() + if err != nil { + // Log the error or use a default directory + Dir = "." + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
cmd/main.go(1 hunks)command/cmd.go(2 hunks)repository/factory.go(3 hunks)repository/git.go(10 hunks)repository/maven.go(1 hunks)repository/subversion.go(6 hunks)ssh/ssh.go(6 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
repository/subversion.go (3)
ssh/ssh.go (2)
Agent(30-31)Dir(20-20)repository/factory.go (1)
Dir(13-13)command/cmd.go (1)
Options(93-93)
repository/git.go (3)
ssh/ssh.go (2)
Agent(30-31)Dir(20-20)repository/factory.go (2)
Dir(13-13)New(28-63)command/cmd.go (3)
Options(93-93)Command(28-35)New(22-25)
🔇 Additional comments (17)
command/cmd.go (1)
32-32: LGTM! Clean implementation of environment variable support.The addition of the
Envfield and its usage inRunWithenables proper environment isolation for SCM operations, which is essential for managing multiple repository instances concurrently.Also applies to: 67-67
cmd/main.go (1)
12-12: LGTM! Correctly updated to match the new repository.New signature.The call properly reflects the API change from accepting identity slices to using variadic options.
repository/maven.go (1)
50-55: LGTM! Clean refactor to use embedded Identity.The direct use of
r.Identitysimplifies the code and aligns with the broader refactoring to use a single embedded identity instead of dynamic lookups.ssh/ssh.go (5)
25-26: LGTM! Directory initialization supports instance isolation.Using the current working directory instead of the home directory enables proper isolation between multiple repository instances.
38-38: LGTM! Explicit HOME environment ensures SSH agent isolation.Setting HOME to the instance-specific directory prevents SSH configuration conflicts between concurrent repository operations.
84-84: Good improvement to return the script path.The signature change to return the script path is cleaner than relying on a hardcoded path assumption.
Also applies to: 116-116
93-97: LGTM! Proper environment setup for non-interactive SSH key addition.The explicit environment variable setup correctly configures SSH_ASKPASS for automated password entry.
129-134: Good improvements to script writing logic.The deferred close and simplified string concatenation make the code cleaner and more maintainable.
repository/subversion.go (4)
51-56: LGTM! Consistent identity refactoring.The changes properly use the embedded
r.Identityfield, aligning with the simplified identity management approach across all SCM implementations.Also applies to: 61-61, 66-66
145-145: LGTM! Environment isolation for SVN commands.Setting HOME ensures SVN configuration and credentials are isolated per repository instance.
238-240: LGTM! Clean refactor of writePassword method.The method now properly uses the embedded identity, and the parameter removal simplifies the API while maintaining functionality.
Also applies to: 249-251, 306-311
202-202: LGTM! Path changes support instance isolation.Using
Dir(current working directory) instead of the home directory ensures proper isolation of SVN configuration between repository instances.Also applies to: 258-258
repository/factory.go (2)
23-63: Well-implemented factory pattern with flexible options.The refactored
Newfunction with variadic options provides a clean and extensible API for SCM configuration. The error handling ensures proper validation and option application.
81-115: Robust option handling with comprehensive type support.The
Usemethod implementation properly handles all documented option types with appropriate nil checks and clear error messages for invalid options.repository/git.go (3)
141-151: Well-designed git command factory method.The
git()method correctly sets up the environment for isolated repository operations by:
- Setting HOME to the instance-specific directory
- Disabling terminal prompts for non-interactive operation
- Enabling trace logs for debugging
- Preserving existing environment variables
171-190: Proper isolation of git configuration.The configuration is correctly written to the instance-specific directory, and the credential helper path is properly configured to use the local credentials file.
209-250: Clean refactoring of credential management.The simplified method signature and direct use of
r.Identitymakes the code cleaner and more maintainable. The credentials are properly isolated to the instance-specific directory.
Signed-off-by: Jeff Ortel <[email protected]>
Signed-off-by: Jeff Ortel <[email protected]>
Signed-off-by: Jeff Ortel <[email protected]>
Signed-off-by: Jeff Ortel <[email protected]>
Signed-off-by: Jeff Ortel <[email protected]>
Signed-off-by: Jeff Ortel <[email protected]>
This spot missed. Related to: konveyor/tackle2-addon#75 - handle working with multiple repositories concurrently in 0.8. ``` - '[RULESET] fetching: id=30 (__Target(Test)-ac32d015-93df-11f0-8d81-4661cd220ef3)' - '[GIT] Home (directory): /addon/.git/8c35e7c2' - '[GIT] Cloning: https://github.com/abrugaro/book-server' - '[CMD] /usr/bin/git succeeded.' ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved error handling when associating identities with ruleset repositories so identity lookup failures are reported early and prevent partial repository setup. * **Refactor** * Repository initialization updated to a more flexible options-based pattern, improving extensibility while preserving existing behavior for users. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Jeff Ortel <[email protected]>
This spot missed. Related to: konveyor/tackle2-addon#75 - handle working with multiple repositories concurrently in 0.8. ``` - '[RULESET] fetching: id=30 (__Target(Test)-ac32d015-93df-11f0-8d81-4661cd220ef3)' - '[GIT] Home (directory): /addon/.git/8c35e7c2' - '[GIT] Cloning: https://github.com/abrugaro/book-server' - '[CMD] /usr/bin/git succeeded.' ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved error handling when associating identities with ruleset repositories so identity lookup failures are reported early and prevent partial repository setup. * **Refactor** * Repository initialization updated to a more flexible options-based pattern, improving extensibility while preserving existing behavior for users. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Jeff Ortel <[email protected]> Signed-off-by: Cherry Picker <[email protected]>
This spot missed. Related to: konveyor/tackle2-addon#75 - handle working with multiple repositories concurrently in 0.8. Tested. ``` - '[RULESET] fetching: id=30 (__Target(Test)-ac32d015-93df-11f0-8d81-4661cd220ef3)' - '[GIT] Home (directory): /addon/.git/8c35e7c2' - '[GIT] Cloning: https://github.com/abrugaro/book-server' - '[CMD] /usr/bin/git succeeded.' ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved error handling when associating identities with ruleset repositories so identity lookup failures are reported early and prevent partial repository setup. * **Refactor** * Repository initialization updated to a more flexible options-based pattern, improving extensibility while preserving existing behavior for users. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Jeff Ortel <[email protected]> Signed-off-by: Cherry Picker <[email protected]> Signed-off-by: Jeff Ortel <[email protected]> Signed-off-by: Cherry Picker <[email protected]> Co-authored-by: Jeff Ortel <[email protected]>
2 participants
problem
This is needed to support working with multiple repositories simultaneously.
solution
The configuration and credentials cannot be written into the HOME directory. Further, each instance of
the SCM needs to have their own.
Add command.Command.Env support.
Update ssh, git, subversion to write credentials and run with HOME=dir where dir is specific to each instance.
The ssh.Agent writes keys in .ssh/ with identity ID prefix.
closes #76
Summary by CodeRabbit
New Features
Refactor