diff --git a/Dockerfile.in b/Dockerfile.in index 0384235b8..ada3640d6 100644 --- a/Dockerfile.in +++ b/Dockerfile.in @@ -103,7 +103,6 @@ RUN rm -rf \ /usr/lib/git-core/git-http-push \ /usr/lib/git-core/git-imap-send \ /usr/lib/git-core/git-instaweb \ - /usr/lib/git-core/git-sh-i18n--envsubst \ /usr/lib/git-core/scalar \ /usr/lib/openssh/ssh-keysign \ /usr/lib/openssh/ssh-pkcs11-helper \ diff --git a/README.md b/README.md index e9a4b1223..4587e45be 100644 --- a/README.md +++ b/README.md @@ -312,7 +312,28 @@ OPTIONS --git-config , $GITSYNC_GIT_CONFIG Additional git config options in a comma-separated 'key:val' format. The parsed keys and values are passed to 'git config' and - must be valid syntax for that command. + must be valid syntax for that command. This is similar to + --git-config-add, but uses a single comma-separated string. + + Both keys and values can be either quoted or unquoted strings. + Within quoted keys and all values (quoted or not), the following + escape sequences are supported: + '\n' => [newline] + '\t' => [tab] + '\"' => '"' + '\,' => ',' + '\\' => '\' + To include a colon within a key (e.g. a URL) the key must be + quoted. Within unquoted values commas must be escaped. Within + quoted values commas may be escaped, but are not required to be. + Any other escape sequence is an error. + + --git-config-add + Add one git config option. The parsed key and value are passed to + 'git config' and must be valid syntax for that command. This flag + can be specified more than once. This is similar to --git-config, + but allows multiple discrete uses of the flag instead of a single + comma-separated string. Both keys and values can be either quoted or unquoted strings. Within quoted keys and all values (quoted or not), the following diff --git a/main.go b/main.go index 99fdf840a..b5dcafae5 100644 --- a/main.go +++ b/main.go @@ -41,6 +41,7 @@ import ( "syscall" "time" + "github.com/go-logr/logr/funcr" "github.com/golang-jwt/jwt/v4" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promhttp" @@ -117,7 +118,8 @@ const defaultDirMode = os.FileMode(0775) // subject to umask type repoSync struct { cmd string // the git command to run root absPath // absolute path to the root directory - repo string // remote repo to sync + remoteRepo string // remote repo to sync + localRepo absPath // absolute path to the local repo ref string // the ref to sync depth int // for shallow sync filter string // for partial clone @@ -296,9 +298,12 @@ func main() { flGitCmd := pflag.String("git", envString("git", "GITSYNC_GIT", "GIT_SYNC_GIT"), "the git command to run (subject to PATH search, mostly for testing)") - flGitConfig := pflag.String("git-config", + flGitConfigString := pflag.String("git-config", envString("", "GITSYNC_GIT_CONFIG", "GIT_SYNC_GIT_CONFIG"), "additional git config options in 'section.var1:val1,\"section.sub.var2\":\"val2\"' format") + flGitConfigList := pflag.StringArray("git-config-add", // Array vs. Slice to avoid CSV parsing + nil, + "add one additional git config option in 'section.var1:val1' format; can be repeated") flGitGC := pflag.String("git-gc", envString("always", "GITSYNC_GIT_GC", "GIT_SYNC_GIT_GC"), "git garbage collection behavior: one of 'auto', 'always', 'aggressive', or 'off'") @@ -726,7 +731,8 @@ func main() { git := &repoSync{ cmd: *flGitCmd, root: absRoot, - repo: *flRepo, + remoteRepo: *flRepo, + localRepo: absRoot.Join(".repo"), ref: *flRef, depth: *flDepth, filter: *flFilter, @@ -784,13 +790,42 @@ func main() { } // This needs to be after all other git-related config flags. - if *flGitConfig != "" { - if err := git.SetupExtraGitConfigs(ctx, *flGitConfig); err != nil { - log.Error(err, "can't set additional git configs", "configs", *flGitConfig) + if *flGitConfigString != "" { + if err := git.SetupExtraGitConfigs(ctx, *flGitConfigString, "--git-config"); err != nil { + log.Error(err, "can't set additional git configs", "configs", *flGitConfigString) + os.Exit(1) + } + } + if len(*flGitConfigList) > 0 { + str := strings.Join(*flGitConfigList, ",") + if err := git.SetupExtraGitConfigs(ctx, str, "--git-config-add"); err != nil { + log.Error(err, "can't set additional git configs", "configs", str) os.Exit(1) } } + // Log configs for debug. The -z means to produce a list of NUL-delimted KV + // pairs, where the first newline is the key/value separator and any + // additional newlines are part of the value. + if stdout, stderr, err := cmdRunner.Run(ctx, "", nil, *flGitCmd, "config", "list", "-z"); err != nil { + log.Error(err, "can't list git config") + os.Exit(1) + } else if stderr != "" { + log.V(0).Info("unexpected stderr reading git config", "stdout", stdout, "stderr", stderr) + os.Exit(1) + } else { + cfgs := strings.Split(stdout, string(rune(0))) + kvs := funcr.PseudoStruct{} // like a map but ordered + for _, cfg := range cfgs { + if cfg == "" { + continue + } + parts := strings.SplitN(cfg, "\n", 2) // any additional newlines are part of the value + kvs = append(kvs, parts[0], parts[1]) + } + log.V(0).Info("git config", "configs", kvs) + } + // The scope of the initialization context ends here, so we call cancel to release resources associated with it. cancel() @@ -1252,13 +1287,13 @@ func (git *repoSync) initRepo(ctx context.Context) error { needGitInit := false // Check out the git root, and see if it is already usable. - _, err := os.Stat(git.root.String()) + _, err := os.Stat(git.localRepo.String()) switch { case os.IsNotExist(err): // Probably the first sync. defaultDirMode ensures that this is usable // as a volume when the consumer isn't running as the same UID. - git.log.V(1).Info("repo directory does not exist, creating it", "path", git.root) - if err := os.MkdirAll(git.root.String(), defaultDirMode); err != nil { + git.log.V(1).Info("repo directory does not exist, creating it", "path", git.localRepo) + if err := os.MkdirAll(git.localRepo.String(), defaultDirMode); err != nil { return err } needGitInit = true @@ -1266,17 +1301,14 @@ func (git *repoSync) initRepo(ctx context.Context) error { return err default: // Make sure the directory we found is actually usable. - git.log.V(3).Info("repo directory exists", "path", git.root) + git.log.V(3).Info("repo directory exists", "path", git.localRepo) if git.sanityCheckRepo(ctx) { - git.log.V(4).Info("repo directory is valid", "path", git.root) + git.log.V(4).Info("repo directory is valid", "path", git.localRepo) } else { - // Maybe a previous run crashed? Git won't use this dir. We remove - // the contents rather than the dir itself, because a common use-case - // is to have a volume mounted at git.root, which makes removing it - // impossible. - git.log.V(0).Info("repo directory was empty or failed checks", "path", git.root) - if err := removeDirContents(git.root, git.log); err != nil { - return fmt.Errorf("can't wipe unusable root directory: %w", err) + // Maybe a previous run crashed? Git won't use this dir. + git.log.V(0).Info("repo directory was empty or failed checks", "path", git.localRepo) + if err := os.RemoveAll(git.localRepo.String()); err != nil { + return fmt.Errorf("can't remove unusable repo directory: %w", err) } needGitInit = true } @@ -1284,8 +1316,8 @@ func (git *repoSync) initRepo(ctx context.Context) error { if needGitInit { // Running `git init` in an existing repo is safe (according to git docs). - git.log.V(0).Info("initializing repo directory", "path", git.root) - if _, _, err := git.Run(ctx, git.root, "init", "-b", "git-sync"); err != nil { + git.log.V(0).Info("initializing repo directory", "path", git.localRepo) + if _, _, err := git.Run(ctx, git.localRepo, "init", "-b", "git-sync"); err != nil { return err } if !git.sanityCheckRepo(ctx) { @@ -1295,17 +1327,17 @@ func (git *repoSync) initRepo(ctx context.Context) error { // The "origin" remote has special meaning, like in relative-path // submodules. - if stdout, stderr, err := git.Run(ctx, git.root, "remote", "get-url", "origin"); err != nil { + if stdout, stderr, err := git.Run(ctx, git.localRepo, "remote", "get-url", "origin"); err != nil { if !strings.Contains(stderr, "No such remote") { return err } // It doesn't exist - make it. - if _, _, err := git.Run(ctx, git.root, "remote", "add", "origin", git.repo); err != nil { + if _, _, err := git.Run(ctx, git.localRepo, "remote", "add", "origin", git.remoteRepo); err != nil { return err } - } else if strings.TrimSpace(stdout) != git.repo { + } else if strings.TrimSpace(stdout) != git.remoteRepo { // It exists, but is wrong. - if _, _, err := git.Run(ctx, git.root, "remote", "set-url", "origin", git.repo); err != nil { + if _, _, err := git.Run(ctx, git.localRepo, "remote", "set-url", "origin", git.remoteRepo); err != nil { return err } } @@ -1352,32 +1384,32 @@ func hasGitLockFile(gitRoot absPath) (string, error) { // sanityCheckRepo tries to make sure that the repo dir is a valid git repository. func (git *repoSync) sanityCheckRepo(ctx context.Context) bool { - git.log.V(3).Info("sanity-checking git repo", "repo", git.root) + git.log.V(3).Info("sanity-checking git repo", "repo", git.localRepo) // If it is empty, we are done. - if empty, err := dirIsEmpty(git.root); err != nil { - git.log.Error(err, "can't list repo directory", "path", git.root) + if empty, err := dirIsEmpty(git.localRepo); err != nil { + git.log.Error(err, "can't list repo directory", "path", git.localRepo) return false } else if empty { - git.log.V(3).Info("repo directory is empty", "path", git.root) + git.log.V(3).Info("repo directory is empty", "path", git.localRepo) return false } // Check that this is actually the root of the repo. - if root, _, err := git.Run(ctx, git.root, "rev-parse", "--show-toplevel"); err != nil { - git.log.Error(err, "can't get repo toplevel", "path", git.root) + if root, _, err := git.Run(ctx, git.localRepo, "rev-parse", "--show-toplevel"); err != nil { + git.log.Error(err, "can't get repo toplevel", "path", git.localRepo) return false } else { root = strings.TrimSpace(root) - if root != git.root.String() { - git.log.Error(nil, "repo directory is under another repo", "path", git.root, "parent", root) + if root != git.localRepo.String() { + git.log.Error(nil, "repo directory is under another repo", "path", git.localRepo, "parent", root) return false } } // Consistency-check the repo. Don't use --verbose because it can be // REALLY verbose. - if _, _, err := git.Run(ctx, git.root, "fsck", "--no-progress", "--connectivity-only"); err != nil { - git.log.Error(err, "repo fsck failed", "path", git.root) + if _, _, err := git.Run(ctx, git.localRepo, "fsck", "--no-progress", "--connectivity-only"); err != nil { + git.log.Error(err, "repo fsck failed", "path", git.localRepo) return false } @@ -1399,7 +1431,7 @@ func (git *repoSync) sanityCheckRepo(ctx context.Context) bool { // files checked out - git could have died halfway through and the repo will // still pass this check. func (git *repoSync) sanityCheckWorktree(ctx context.Context, worktree worktree) bool { - git.log.V(3).Info("sanity-checking worktree", "repo", git.root, "worktree", worktree) + git.log.V(3).Info("sanity-checking worktree", "repo", git.localRepo, "worktree", worktree) // If it is empty, we are done. if empty, err := dirIsEmpty(worktree.Path()); err != nil { @@ -1439,13 +1471,6 @@ func dirIsEmpty(dir absPath) (bool, error) { return len(dirents) == 0, nil } -// removeDirContents iterated the specified dir and removes all contents. -func removeDirContents(dir absPath, log *logging.Logger) error { - return removeDirContentsIf(dir, log, func(fi os.FileInfo) (bool, error) { - return true, nil - }) -} - func removeDirContentsIf(dir absPath, log *logging.Logger, fn func(fi os.FileInfo) (bool, error)) error { dirents, err := os.ReadDir(dir.String()) if err != nil { @@ -1529,7 +1554,7 @@ func (git *repoSync) removeWorktree(ctx context.Context, worktree worktree) erro if err := os.RemoveAll(worktree.Path().String()); err != nil { return fmt.Errorf("error removing directory: %w", err) } - if _, _, err := git.Run(ctx, git.root, "worktree", "prune", "--verbose"); err != nil { + if _, _, err := git.Run(ctx, git.localRepo, "worktree", "prune", "--verbose"); err != nil { return err } return nil @@ -1550,7 +1575,7 @@ func (git *repoSync) createWorktree(ctx context.Context, hash string) (worktree, } git.log.V(1).Info("adding worktree", "path", worktree.Path(), "hash", hash) - _, _, err := git.Run(ctx, git.root, "worktree", "add", "--force", "--detach", worktree.Path().String(), hash, "--no-checkout") + _, _, err := git.Run(ctx, git.localRepo, "worktree", "add", "--force", "--detach", worktree.Path().String(), hash, "--no-checkout") if err != nil { return "", err } @@ -1568,7 +1593,7 @@ func (git *repoSync) configureWorktree(ctx context.Context, worktree worktree) e // using relative paths, so that other containers can use a different volume // mount name. var rootDotGit string - if rel, err := filepath.Rel(worktree.Path().String(), git.root.String()); err != nil { + if rel, err := filepath.Rel(worktree.Path().String(), git.localRepo.String()); err != nil { return err } else { rootDotGit = filepath.Join(rel, ".git") @@ -1580,7 +1605,7 @@ func (git *repoSync) configureWorktree(ctx context.Context, worktree worktree) e // If sparse checkout is requested, configure git for it, otherwise // unconfigure it. - gitInfoPath := filepath.Join(git.root.String(), ".git/worktrees", hash, "info") + gitInfoPath := filepath.Join(git.localRepo.String(), ".git/worktrees", hash, "info") gitSparseConfigPath := filepath.Join(gitInfoPath, "sparse-checkout") if git.sparseFile == "" { os.RemoveAll(gitSparseConfigPath) @@ -1661,13 +1686,13 @@ func (git *repoSync) cleanup(ctx context.Context) error { // Let git know we don't need those old commits any more. git.log.V(3).Info("pruning worktrees") - if _, _, err := git.Run(ctx, git.root, "worktree", "prune", "--verbose"); err != nil { + if _, _, err := git.Run(ctx, git.localRepo, "worktree", "prune", "--verbose"); err != nil { cleanupErrs = append(cleanupErrs, err) } // Expire old refs. git.log.V(3).Info("expiring unreachable refs") - if _, _, err := git.Run(ctx, git.root, "reflog", "expire", "--expire-unreachable=all", "--all"); err != nil { + if _, _, err := git.Run(ctx, git.localRepo, "reflog", "expire", "--expire-unreachable=all", "--all"); err != nil { cleanupErrs = append(cleanupErrs, err) } @@ -1683,7 +1708,7 @@ func (git *repoSync) cleanup(ctx context.Context) error { args = append(args, "--aggressive") } git.log.V(3).Info("running git garbage collection") - if _, _, err := git.Run(ctx, git.root, args...); err != nil { + if _, _, err := git.Run(ctx, git.localRepo, args...); err != nil { cleanupErrs = append(cleanupErrs, err) } } @@ -1755,7 +1780,7 @@ func (git *repoSync) currentWorktree() (worktree, error) { // and tries to clean up any detritus. This function returns whether the // current hash has changed and what the new hash is. func (git *repoSync) SyncRepo(ctx context.Context, refreshCreds func(context.Context) error) (bool, string, error) { - git.log.V(3).Info("syncing", "repo", redactURL(git.repo)) + git.log.V(3).Info("syncing", "repo", redactURL(git.remoteRepo)) if err := refreshCreds(ctx); err != nil { return false, "", fmt.Errorf("credential refresh failed: %w", err) @@ -1786,7 +1811,7 @@ func (git *repoSync) SyncRepo(ctx context.Context, refreshCreds func(context.Con // their underlying commit hashes, but has no effect if we fetched a // branch, plain tag, or hash. var remoteHash string - if output, _, err := git.Run(ctx, git.root, "rev-parse", "FETCH_HEAD^{}"); err != nil { + if output, _, err := git.Run(ctx, git.localRepo, "rev-parse", "FETCH_HEAD^{}"); err != nil { return false, "", err } else { remoteHash = strings.Trim(output, "\n") @@ -1818,14 +1843,14 @@ func (git *repoSync) SyncRepo(ctx context.Context, refreshCreds func(context.Con // Reset the repo (note: not the worktree - that happens later) to the new // ref. This makes subsequent fetches much less expensive. It uses --soft // so no files are checked out. - if _, _, err := git.Run(ctx, git.root, "reset", "--soft", remoteHash, "--"); err != nil { + if _, _, err := git.Run(ctx, git.localRepo, "reset", "--soft", remoteHash); err != nil { return false, "", err } // If we have a new hash, make a new worktree newWorktree := currentWorktree if changed { - // Create a worktree for this hash in git.root. + // Create a worktree for this hash. if wt, err := git.createWorktree(ctx, remoteHash); err != nil { return false, "", err } else { @@ -1880,11 +1905,11 @@ func (git *repoSync) SyncRepo(ctx context.Context, refreshCreds func(context.Con // fetch retrieves the specified ref from the upstream repo. func (git *repoSync) fetch(ctx context.Context, ref string) error { - git.log.V(2).Info("fetching", "ref", ref, "repo", redactURL(git.repo)) + git.log.V(2).Info("fetching", "ref", ref, "repo", redactURL(git.remoteRepo)) // Fetch the ref and do some cleanup, setting or un-setting the repo's // shallow flag as appropriate. - args := []string{"fetch", git.repo, ref, "--verbose", "--no-progress", "--prune", "--no-auto-gc"} + args := []string{"fetch", git.remoteRepo, ref, "--verbose", "--no-progress", "--prune", "--no-auto-gc"} if git.depth > 0 { args = append(args, "--depth", strconv.Itoa(git.depth)) } else { @@ -1901,7 +1926,7 @@ func (git *repoSync) fetch(ctx context.Context, ref string) error { if git.filter != "" { args = append(args, "--filter", git.filter) } - if _, _, err := git.Run(ctx, git.root, args...); err != nil { + if _, _, err := git.Run(ctx, git.localRepo, args...); err != nil { return err } @@ -1909,7 +1934,7 @@ func (git *repoSync) fetch(ctx context.Context, ref string) error { } func (git *repoSync) isShallow(ctx context.Context) (bool, error) { - boolStr, _, err := git.Run(ctx, git.root, "rev-parse", "--is-shallow-repository") + boolStr, _, err := git.Run(ctx, git.localRepo, "rev-parse", "--is-shallow-repository") if err != nil { return false, fmt.Errorf("can't determine repo shallowness: %w", err) } @@ -2057,7 +2082,7 @@ func (git *repoSync) CallAskPassURL(ctx context.Context) error { } } - if err := git.StoreCredentials(ctx, git.repo, username, password); err != nil { + if err := git.StoreCredentials(ctx, git.remoteRepo, username, password); err != nil { return err } @@ -2146,7 +2171,7 @@ func (git *repoSync) RefreshGitHubAppToken(ctx context.Context, githubBaseURL, p username := "-" password := tokenResponse.Token - if err := git.StoreCredentials(ctx, git.repo, username, password); err != nil { + if err := git.StoreCredentials(ctx, git.remoteRepo, username, password); err != nil { return err } @@ -2172,10 +2197,6 @@ func (git *repoSync) SetupDefaultGitConfigs(ctx context.Context) error { // Never prompt for a password. key: "core.askPass", val: "true", - }, { - // Mark repos as safe (avoid a "dubious ownership" error). - key: "safe.directory", - val: "*", }} for _, kv := range configs { @@ -2188,10 +2209,10 @@ func (git *repoSync) SetupDefaultGitConfigs(ctx context.Context) error { // SetupExtraGitConfigs configures the global git environment with user-provided // override settings. -func (git *repoSync) SetupExtraGitConfigs(ctx context.Context, configsFlag string) error { +func (git *repoSync) SetupExtraGitConfigs(ctx context.Context, configsFlag string, flagName string) error { configs, err := parseGitConfigs(configsFlag) if err != nil { - return fmt.Errorf("can't parse --git-config flag: %w", err) + return fmt.Errorf("can't parse %s flag: %w", flagName, err) } git.log.V(1).Info("setting additional git configs", "configs", configs) for _, kv := range configs { @@ -2543,7 +2564,28 @@ OPTIONS --git-config , $GITSYNC_GIT_CONFIG Additional git config options in a comma-separated 'key:val' format. The parsed keys and values are passed to 'git config' and - must be valid syntax for that command. + must be valid syntax for that command. This is similar to + --git-config-add, but uses a single comma-separated string. + + Both keys and values can be either quoted or unquoted strings. + Within quoted keys and all values (quoted or not), the following + escape sequences are supported: + '\n' => [newline] + '\t' => [tab] + '\"' => '"' + '\,' => ',' + '\\' => '\' + To include a colon within a key (e.g. a URL) the key must be + quoted. Within unquoted values commas must be escaped. Within + quoted values commas may be escaped, but are not required to be. + Any other escape sequence is an error. + + --git-config-add + Add one git config option. The parsed key and value are passed to + 'git config' and must be valid syntax for that command. This flag + can be specified more than once. This is similar to --git-config, + but allows multiple discrete uses of the flag instead of a single + comma-separated string. Both keys and values can be either quoted or unquoted strings. Within quoted keys and all values (quoted or not), the following diff --git a/main_test.go b/main_test.go index 13de2a4e9..8949641b8 100644 --- a/main_test.go +++ b/main_test.go @@ -17,6 +17,7 @@ limitations under the License. package main import ( + "io/fs" "os" "path/filepath" "reflect" @@ -315,7 +316,7 @@ func TestDirIsEmpty(t *testing.T) { } } -func TestRemoveDirContents(t *testing.T) { +func TestRemoveDirContentsIf(t *testing.T) { root := absPath(t.TempDir()) // Brand new should be empty. @@ -325,8 +326,12 @@ func TestRemoveDirContents(t *testing.T) { t.Errorf("expected %q to be deemed empty", root) } + fn := func(fi fs.FileInfo) (bool, error) { + return true, nil + } + // Test removal. - if err := removeDirContents(root, nil); err != nil { + if err := removeDirContentsIf(root, nil, fn); err != nil { t.Errorf("unexpected error: %v", err) } @@ -352,12 +357,12 @@ func TestRemoveDirContents(t *testing.T) { } // Test removal. - if err := removeDirContents(root, nil); err != nil { + if err := removeDirContentsIf(root, nil, fn); err != nil { t.Errorf("unexpected error: %v", err) } // Test error path. - if err := removeDirContents(root.Join("does-not-exist"), nil); err == nil { + if err := removeDirContentsIf(root.Join("does-not-exist"), nil, fn); err == nil { t.Errorf("unexpected success for non-existent dir") } } diff --git a/test_e2e.sh b/test_e2e.sh index 9571e48bf..25bdfc7e1 100755 --- a/test_e2e.sh +++ b/test_e2e.sh @@ -368,7 +368,8 @@ function GIT_SYNC() { --add-user \ --group-write \ --touch-file="$INTERLOCK" \ - --git-config='protocol.file.allow:always' \ + --git-config-add='protocol.file.allow:always' \ + --git-config-add='safe.directory:*' \ --http-bind=":$HTTP_PORT" \ --http-metrics \ --http-pprof \ @@ -3441,7 +3442,10 @@ function e2e::additional_git_configs() { --repo="file://$REPO" \ --root="$ROOT" \ --link="link" \ - --git-config='http.postBuffer:10485760,sect.k1:"a val",sect.k2:another val' + --git-config='http.postBuffer:10485760,sect.k1:"a val",sect.k2:another val' \ + --git-config-add='sect.k3:a val' \ + --git-config-add='sect.k4:"a val with quotes"' \ + --git-config-add='"sect.k5":"quoted_all"' assert_link_exists "$ROOT/link" assert_file_exists "$ROOT/link/file" assert_file_eq "$ROOT/link/file" "${FUNCNAME[0]}"