Merge pull request #700 from thockin/release-3.x

V3: prevent git's 'dubious ownership' error

cmd/git-sync/main.go

@@ -36,6 +36,7 @@ import (
 	"strconv"
 	"strings"
 	"sync"
+	"syscall"
 	"time"
 
 	"github.com/prometheus/client_golang/prometheus"
@@ -74,6 +75,8 @@ var flOneTime = flag.Bool("one-time", envBool("GIT_SYNC_ONE_TIME", false),
 	"exit after the first sync")
 var flMaxSyncFailures = flag.Int("max-sync-failures", envInt("GIT_SYNC_MAX_SYNC_FAILURES", 0),
 	"the number of consecutive failures allowed before aborting (the first sync must succeed, -1 will retry forever after the initial sync)")
+var flGroupWrite = flag.Bool("group-write", envBool("GIT_SYNC_GROUP_WRITE", false),
+	"ensure that all data (repo, worktrees, etc.) is group writable")
 var flChmod = flag.Int("change-permissions", envInt("GIT_SYNC_PERMISSIONS", 0),
 	"the file permissions to apply to the checked-out files (0 will not change permissions at all)")
 var flSparseCheckoutFile = flag.String("sparse-checkout-file", envString("GIT_SYNC_SPARSE_CHECKOUT_FILE", ""),
@@ -178,6 +181,8 @@ const (
 	gcAlways     = "always"
 	gcAggressive = "aggressive"
 	gcOff        = "off"
+
+	defaultDirMode = os.FileMode(0775) // subject to umask
 )
 
 func init() {
@@ -379,6 +384,13 @@ func main() {
 		}
 	}
 
+	// If the user asked for group-writable data, make sure the umask allows it.
+	if *flGroupWrite {
+		syscall.Umask(0002)
+	} else {
+		syscall.Umask(0022)
+	}
+
 	if *flAddUser {
 		if err := addUser(); err != nil {
 			handleError(false, "ERROR: can't write to /etc/passwd: %v", err)
@@ -684,7 +696,7 @@ func updateSymlink(ctx context.Context, gitRoot, link, newDir string) (string, e
 	// Make sure the link directory exists. We do this here, rather than at
 	// startup because it might be under --root and that gets wiped in some
 	// circumstances.
-	if err := os.MkdirAll(filepath.Dir(linkDir), os.FileMode(int(0755))); err != nil {
+	if err := os.MkdirAll(filepath.Dir(linkDir), defaultDirMode); err != nil {
 		return "", fmt.Errorf("error making symlink dir: %v", err)
 	}
 
@@ -818,8 +830,7 @@ func addWorktreeAndSwap(ctx context.Context, repo, gitRoot, dest, branch, rev st
 		defer source.Close()
 
 		if _, err := os.Stat(gitInfoPath); os.IsNotExist(err) {
-			fileMode := os.FileMode(int(0755))
-			err := os.Mkdir(gitInfoPath, fileMode)
+			err := os.Mkdir(gitInfoPath, defaultDirMode)
 			if err != nil {
 				return err
 			}
@@ -974,7 +985,7 @@ func cloneRepo(ctx context.Context, repo, branch, rev string, depth int, gitRoot
 		defer source.Close()
 
 		if _, err := os.Stat(gitInfoPath); os.IsNotExist(err) {
-			fileMode := os.FileMode(int(0755))
+			fileMode := os.FileMode(defaultDirMode)
 			err := os.Mkdir(gitInfoPath, fileMode)
 			if err != nil {
 				return err
@@ -1285,7 +1296,12 @@ func setupDefaultGitConfigs(ctx context.Context) error {
 		// How to manage credentials (for those modes that need it).
 		key: "credential.helper",
 		val: "cache --timeout 3600",
+	}, {
+		// Our working root is safe (avoid a "dubious ownership" error).
+		key: "safe.directory",
+		val: "*",
 	}}
+
 	for _, kv := range configs {
 		if _, err := cmdRunner.Run(ctx, "", nil, *flGitCmd, "config", "--global", kv.key, kv.val); err != nil {
 			return fmt.Errorf("error configuring git %q %q: %v", kv.key, kv.val, err)

pkg/logging/logging.go

@@ -99,7 +99,7 @@ func (l *Logger) DeleteErrorFile() {
 // writeContent writes the error content to the error file.
 func (l *Logger) writeContent(content []byte) {
 	if _, err := os.Stat(l.root); os.IsNotExist(err) {
-		fileMode := os.FileMode(0755)
+		fileMode := os.FileMode(0775) // umask applies
 		if err := os.Mkdir(l.root, fileMode); err != nil {
 			l.Logger.Error(err, "can't create the root directory", "root", l.root)
 			return

test_e2e.sh

@@ -139,6 +139,7 @@ ROOT="$DIR/root"
 function clean_root() {
     rm -rf "$ROOT"
     mkdir -p "$ROOT"
+    chmod g+rwx "$ROOT"
 }
 
 # Init SSH for test cases.
@@ -146,6 +147,7 @@ DOT_SSH="$DIR/dot_ssh"
 mkdir -p "$DOT_SSH"
 ssh-keygen -f "$DOT_SSH/id_test" -P "" >/dev/null
 cat "$DOT_SSH/id_test.pub" > "$DOT_SSH/authorized_keys"
+chmod -R g+r "$DOT_SSH"
 
 SLOW_GIT_CLONE=/slow_git_clone.sh
 SLOW_GIT_FETCH=/slow_git_fetch.sh
@@ -159,6 +161,7 @@ EXECHOOK_ENVVAL=envval
 RUNLOG="$DIR/runlog.exechook-fail-retry"
 rm -f $RUNLOG
 touch $RUNLOG
+chmod g+rw $RUNLOG
 
 function GIT_SYNC() {
     #./bin/linux_amd64/git-sync "$@"
@@ -171,7 +174,7 @@ function GIT_SYNC() {
         ${RM} \
         --label git-sync-e2e="$RUNID" \
         --network="host" \
-        -u $(id -u):$(id -g) \
+        -u git-sync:$(id -g) `# rely on GID, triggering "dubious ownership"` \
         -v "$ROOT":"$ROOT":rw \
         -v "$REPO":"$REPO":ro \
         -v "$REPO2":"$REPO2":ro \
@@ -189,6 +192,7 @@ function GIT_SYNC() {
         e2e/git-sync:"${E2E_TAG}"__$(go env GOOS)_$(go env GOARCH) \
             --v=6 \
             --add-user \
+            --group-write \
             --git-config='protocol.file.allow:always' \
             "$@"
 }
@@ -392,6 +396,7 @@ function e2e::worktree_cleanup() {
     git -C "$REPO" commit -qam "$FUNCNAME new file"
     REV=$(git -C "$REPO" rev-list -n1 HEAD)
     git -C "$REPO" worktree add -q "$ROOT"/"$REV" -b e2e --no-checkout
+    chmod g+w "$ROOT"/"$REV"
     sleep 10
     assert_file_exists "$ROOT"/link/file2
     assert_file_eq "$ROOT"/link/file2 "$FUNCNAME-ok"