-
Notifications
You must be signed in to change notification settings - Fork 43
feat(git): log parsed gitURL and warn if local #345
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
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
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 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 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 |
---|---|---|
|
@@ -12,7 +12,6 @@ import ( | |
"github.com/coder/envbuilder/options" | ||
|
||
giturls "github.com/chainguard-dev/git-urls" | ||
"github.com/coder/envbuilder/log" | ||
"github.com/go-git/go-billy/v5" | ||
"github.com/go-git/go-git/v5" | ||
"github.com/go-git/go-git/v5/plumbing" | ||
|
@@ -47,11 +46,12 @@ type CloneRepoOptions struct { | |
// be cloned again. | ||
// | ||
// The bool returned states whether the repository was cloned or not. | ||
func CloneRepo(ctx context.Context, opts CloneRepoOptions) (bool, error) { | ||
func CloneRepo(ctx context.Context, logf func(string, ...any), opts CloneRepoOptions) (bool, error) { | ||
parsed, err := giturls.Parse(opts.RepoURL) | ||
if err != nil { | ||
return false, fmt.Errorf("parse url %q: %w", opts.RepoURL, err) | ||
} | ||
logf("Parsed Git URL as %q", parsed.Redacted()) | ||
if parsed.Hostname() == "dev.azure.com" { | ||
// Azure DevOps requires capabilities multi_ack / multi_ack_detailed, | ||
// which are not fully implemented and by default are included in | ||
|
@@ -73,6 +73,7 @@ func CloneRepo(ctx context.Context, opts CloneRepoOptions) (bool, error) { | |
transport.UnsupportedCapabilities = []capability.Capability{ | ||
capability.ThinPack, | ||
} | ||
logf("Workaround for Azure DevOps: marking thin-pack as unsupported") | ||
} | ||
|
||
err = opts.Storage.MkdirAll(opts.Path, 0o755) | ||
|
@@ -131,7 +132,7 @@ func CloneRepo(ctx context.Context, opts CloneRepoOptions) (bool, error) { | |
// clone will not be performed. | ||
// | ||
// The bool returned states whether the repository was cloned or not. | ||
func ShallowCloneRepo(ctx context.Context, opts CloneRepoOptions) error { | ||
func ShallowCloneRepo(ctx context.Context, logf func(string, ...any), opts CloneRepoOptions) error { | ||
opts.Depth = 1 | ||
opts.SingleBranch = true | ||
|
||
|
@@ -150,7 +151,7 @@ func ShallowCloneRepo(ctx context.Context, opts CloneRepoOptions) error { | |
} | ||
} | ||
|
||
cloned, err := CloneRepo(ctx, opts) | ||
cloned, err := CloneRepo(ctx, logf, opts) | ||
if err != nil { | ||
return err | ||
} | ||
|
@@ -182,14 +183,14 @@ func ReadPrivateKey(path string) (gossh.Signer, error) { | |
|
||
// LogHostKeyCallback is a HostKeyCallback that just logs host keys | ||
// and does nothing else. | ||
func LogHostKeyCallback(logger log.Func) gossh.HostKeyCallback { | ||
func LogHostKeyCallback(logger func(string, ...any)) gossh.HostKeyCallback { | ||
return func(hostname string, remote net.Addr, key gossh.PublicKey) error { | ||
var sb strings.Builder | ||
_ = knownhosts.WriteKnownHost(&sb, hostname, remote, key) | ||
// skeema/knownhosts uses a fake public key to determine the host key | ||
// algorithms. Ignore this one. | ||
if s := sb.String(); !strings.Contains(s, "fake-public-key ZmFrZSBwdWJsaWMga2V5") { | ||
logger(log.LevelInfo, "#1: 🔑 Got host key: %s", strings.TrimSpace(s)) | ||
logger("🔑 Got host key: %s", strings.TrimSpace(s)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ❤️ |
||
} | ||
return nil | ||
} | ||
|
@@ -203,6 +204,8 @@ func LogHostKeyCallback(logger log.Func) gossh.HostKeyCallback { | |
// | https?://host.tld/repo | Not Set | Set | HTTP Basic | | ||
// | https?://host.tld/repo | Set | Not Set | HTTP Basic | | ||
// | https?://host.tld/repo | Set | Set | HTTP Basic | | ||
// | file://path/to/repo | - | - | None | | ||
// | path/to/repo | - | - | None | | ||
// | All other formats | - | - | SSH | | ||
// | ||
// For SSH authentication, the default username is "git" but will honour | ||
|
@@ -214,58 +217,73 @@ func LogHostKeyCallback(logger log.Func) gossh.HostKeyCallback { | |
// If SSH_KNOWN_HOSTS is not set, the SSH auth method will be configured | ||
// to accept and log all host keys. Otherwise, host key checking will be | ||
// performed as usual. | ||
func SetupRepoAuth(options *options.Options) transport.AuthMethod { | ||
func SetupRepoAuth(logf func(string, ...any), options *options.Options) transport.AuthMethod { | ||
if options.GitURL == "" { | ||
options.Logger(log.LevelInfo, "#1: ❔ No Git URL supplied!") | ||
logf("❔ No Git URL supplied!") | ||
return nil | ||
} | ||
if strings.HasPrefix(options.GitURL, "http://") || strings.HasPrefix(options.GitURL, "https://") { | ||
parsedURL, err := giturls.Parse(options.GitURL) | ||
if err != nil { | ||
logf("❌ Failed to parse Git URL: %s", err.Error()) | ||
return nil | ||
} | ||
|
||
if parsedURL.Scheme == "http" || parsedURL.Scheme == "https" { | ||
// Special case: no auth | ||
if options.GitUsername == "" && options.GitPassword == "" { | ||
options.Logger(log.LevelInfo, "#1: 👤 Using no authentication!") | ||
logf("👤 Using no authentication!") | ||
return nil | ||
} | ||
// Basic Auth | ||
// NOTE: we previously inserted the credentials into the repo URL. | ||
// This was removed in https://github.com/coder/envbuilder/pull/141 | ||
options.Logger(log.LevelInfo, "#1: 🔒 Using HTTP basic authentication!") | ||
logf("🔒 Using HTTP basic authentication!") | ||
return &githttp.BasicAuth{ | ||
Username: options.GitUsername, | ||
Password: options.GitPassword, | ||
} | ||
} | ||
|
||
if parsedURL.Scheme == "file" { | ||
// go-git will try to fallback to using the `git` command for local | ||
// filesystem clones. However, it's more likely than not that the | ||
// `git` command is not present in the container image. Log a warning | ||
// but continue. Also, no auth. | ||
logf("🚧 Using local filesystem clone! This requires the git executable to be present!") | ||
return nil | ||
} | ||
|
||
// Generally git clones over SSH use the 'git' user, but respect | ||
// GIT_USERNAME if set. | ||
if options.GitUsername == "" { | ||
options.GitUsername = "git" | ||
} | ||
|
||
// Assume SSH auth for all other formats. | ||
options.Logger(log.LevelInfo, "#1: 🔑 Using SSH authentication!") | ||
logf("🔑 Using SSH authentication!") | ||
|
||
var signer ssh.Signer | ||
if options.GitSSHPrivateKeyPath != "" { | ||
s, err := ReadPrivateKey(options.GitSSHPrivateKeyPath) | ||
if err != nil { | ||
options.Logger(log.LevelError, "#1: ❌ Failed to read private key from %s: %s", options.GitSSHPrivateKeyPath, err.Error()) | ||
logf("❌ Failed to read private key from %s: %s", options.GitSSHPrivateKeyPath, err.Error()) | ||
} else { | ||
options.Logger(log.LevelInfo, "#1: 🔑 Using %s key!", s.PublicKey().Type()) | ||
logf("🔑 Using %s key!", s.PublicKey().Type()) | ||
signer = s | ||
} | ||
} | ||
|
||
// If no SSH key set, fall back to agent auth. | ||
if signer == nil { | ||
options.Logger(log.LevelError, "#1: 🔑 No SSH key found, falling back to agent!") | ||
logf("🔑 No SSH key found, falling back to agent!") | ||
auth, err := gitssh.NewSSHAgentAuth(options.GitUsername) | ||
if err != nil { | ||
options.Logger(log.LevelError, "#1: ❌ Failed to connect to SSH agent: %s", err.Error()) | ||
logf("❌ Failed to connect to SSH agent: " + err.Error()) | ||
return nil // nothing else we can do | ||
} | ||
if os.Getenv("SSH_KNOWN_HOSTS") == "" { | ||
options.Logger(log.LevelWarn, "#1: 🔓 SSH_KNOWN_HOSTS not set, accepting all host keys!") | ||
auth.HostKeyCallback = LogHostKeyCallback(options.Logger) | ||
logf("🔓 SSH_KNOWN_HOSTS not set, accepting all host keys!") | ||
auth.HostKeyCallback = LogHostKeyCallback(logf) | ||
} | ||
return auth | ||
} | ||
|
@@ -283,19 +301,20 @@ func SetupRepoAuth(options *options.Options) transport.AuthMethod { | |
|
||
// Duplicated code due to Go's type system. | ||
if os.Getenv("SSH_KNOWN_HOSTS") == "" { | ||
options.Logger(log.LevelWarn, "#1: 🔓 SSH_KNOWN_HOSTS not set, accepting all host keys!") | ||
auth.HostKeyCallback = LogHostKeyCallback(options.Logger) | ||
logf("🔓 SSH_KNOWN_HOSTS not set, accepting all host keys!") | ||
auth.HostKeyCallback = LogHostKeyCallback(logf) | ||
} | ||
return auth | ||
} | ||
|
||
func CloneOptionsFromOptions(options options.Options) (CloneRepoOptions, error) { | ||
func CloneOptionsFromOptions(logf func(string, ...any), options options.Options) (CloneRepoOptions, error) { | ||
caBundle, err := options.CABundle() | ||
if err != nil { | ||
return CloneRepoOptions{}, err | ||
} | ||
|
||
cloneOpts := CloneRepoOptions{ | ||
RepoURL: options.GitURL, | ||
Path: options.WorkspaceFolder, | ||
Storage: options.Filesystem, | ||
Insecure: options.Insecure, | ||
|
@@ -304,13 +323,12 @@ func CloneOptionsFromOptions(options options.Options) (CloneRepoOptions, error) | |
CABundle: caBundle, | ||
} | ||
|
||
cloneOpts.RepoAuth = SetupRepoAuth(&options) | ||
cloneOpts.RepoAuth = SetupRepoAuth(logf, &options) | ||
if options.GitHTTPProxyURL != "" { | ||
cloneOpts.ProxyOptions = transport.ProxyOptions{ | ||
URL: options.GitHTTPProxyURL, | ||
} | ||
} | ||
cloneOpts.RepoURL = options.GitURL | ||
|
||
return cloneOpts, nil | ||
} | ||
|
@@ -331,7 +349,7 @@ func (w *progressWriter) Close() error { | |
return err2 | ||
} | ||
|
||
func ProgressWriter(write func(line string)) io.WriteCloser { | ||
func ProgressWriter(write func(line string, args ...any)) io.WriteCloser { | ||
reader, writer := io.Pipe() | ||
done := make(chan struct{}) | ||
go func() { | ||
|
@@ -347,6 +365,8 @@ func ProgressWriter(write func(line string)) io.WriteCloser { | |
if line == "" { | ||
continue | ||
} | ||
// Escape % signs so that they don't get interpreted as format specifiers | ||
line = strings.Replace(line, "%", "%%", -1) | ||
write(strings.TrimSpace(line)) | ||
} | ||
} | ||
|
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this could be optional as part of options, but not a blocker.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it is optional! If it's not provided, we'll try to log and panic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, it would be via
!= nil
check or assigning a noop function.