Merge pull request #300 from thockin/v4-breakings

Change some flags (breaking)

README.md

@@ -87,8 +87,8 @@ docker run -d \
 | GIT_SYNC_REV                    | `--rev`                    | the git revision (tag or hash) to check out                                                                                                                                                                                                   | "HEAD"                        |
 | GIT_SYNC_DEPTH                  | `--depth`                  | use a shallow clone with a history truncated to the specified number of commits                                                                                                                                                               | 0                             |
 | GIT_SYNC_SUBMODULES             | `--submodules`             | git submodule behavior: one of 'recursive', 'shallow', or 'off'                                                                                                                                                                               | recursive                     |
-| GIT_SYNC_ROOT                   | `--root`                   | the root directory for git-sync operations, under which --dest will be created                                                                                                                                                                | "$HOME/git"                   |
+| GIT_SYNC_ROOT                   | `--root`                   | the root directory for git-sync operations, under which --link will be created                                                                                                                                                                | ""                            |
-| GIT_SYNC_DEST                   | `--dest`                   | the name of (a symlink to) a directory in which to check-out files under --root (defaults to the leaf dir of --repo)                                                                                                                          | ""                            |
+| GIT_SYNC_LINK                   | `--link`                   | the name of a symlink, under --root, which points to a directory in which --repo is checked out (defaults to the leaf dir of --repo)                                                                                                          | ""                            |
 | GIT_SYNC_PERIOD                 | `--period`                 | how long to wait between syncs, must be >= 10ms                                                                                                                                                                                               | "10s"                         |
 | GIT_SYNC_SYNC_TIMEOUT           | `--sync-timeout`           | the total time allowed for one complete sync, must be >= 10ms                                                                                                                                                                                 | "120s"                        |
 | GIT_SYNC_ONE_TIME               | `--one-time`               | exit after the first sync                                                                                                                                                                                                                     | false                         |

cmd/git-sync/main.go

@@ -62,10 +62,10 @@ var flDepth = pflag.Int("depth", envInt("GIT_SYNC_DEPTH", 0),
 var flSubmodules = pflag.String("submodules", envString("GIT_SYNC_SUBMODULES", "recursive"),
 	"git submodule behavior: one of 'recursive', 'shallow', or 'off'")
 
-var flRoot = pflag.String("root", envString("GIT_SYNC_ROOT", envString("HOME", "")+"/git"),
+var flRoot = pflag.String("root", envString("GIT_SYNC_ROOT", ""),
-	"the root directory for git-sync operations, under which --dest will be created")
+	"the root directory for git-sync operations, under which --link will be created")
-var flDest = pflag.String("dest", envString("GIT_SYNC_DEST", ""),
+var flLink = pflag.String("link", envString("GIT_SYNC_LINK", ""),
-	"the name of (a symlink to) a directory in which to check-out files under --root (defaults to the leaf dir of --repo)")
+	"the name of a symlink, under --root, which points to a directory in which --repo is checked out (defaults to the leaf dir of --repo)")
 var flPeriod = pflag.Duration("period", envDuration("GIT_SYNC_PERIOD", 10*time.Second),
 	"how long to wait between syncs, must be >= 10ms; --wait overrides this")
 var flSyncTimeout = pflag.Duration("sync-timeout", envDuration("GIT_SYNC_SYNC_TIMEOUT", 120*time.Second),
@@ -128,10 +128,13 @@ var flWait = pflag.Float64("wait", envFloat("GIT_SYNC_WAIT", 0),
 	"DEPRECATED: use --period instead")
 var flTimeout = pflag.Int("timeout", envInt("GIT_SYNC_TIMEOUT", 0),
 	"DEPRECATED: use --sync-timeout instead")
+var flDest = pflag.String("dest", envString("GIT_SYNC_DEST", ""),
+	"DEPRECATED: use --link instead")
 
 func init() {
 	pflag.CommandLine.MarkDeprecated("wait", "use --period instead")
 	pflag.CommandLine.MarkDeprecated("timeout", "use --sync-timeout instead")
+	pflag.CommandLine.MarkDeprecated("dest", "use --link instead")
 }
 
 var log = glogr.New()
@@ -305,13 +308,20 @@ func main() {
 		os.Exit(1)
 	}
 
-	if *flDest == "" {
+	if *flDest != "" {
-		parts := strings.Split(strings.Trim(*flRepo, "/"), "/")
+		*flLink = *flDest
-		*flDest = parts[len(parts)-1]
 	}
-
+	if *flLink == "" {
-	if strings.Contains(*flDest, "/") {
+		parts := strings.Split(strings.Trim(*flRepo, "/"), "/")
-		fmt.Fprintf(os.Stderr, "ERROR: --dest must be a leaf name, not a path\n")
+		*flLink = parts[len(parts)-1]
+	}
+	if strings.Contains(*flLink, "/") {
+		fmt.Fprintf(os.Stderr, "ERROR: --link must not contain '/'\n")
+		pflag.Usage()
+		os.Exit(1)
+	}
+	if strings.HasPrefix(*flLink, ".") {
+		fmt.Fprintf(os.Stderr, "ERROR: --link must not start with '.'\n")
 		pflag.Usage()
 		os.Exit(1)
 	}
@@ -486,7 +496,7 @@ func main() {
 	for {
 		start := time.Now()
 		ctx, cancel := context.WithTimeout(context.Background(), *flSyncTimeout)
-		if changed, hash, err := syncRepo(ctx, *flRepo, *flBranch, *flRev, *flDepth, *flRoot, *flDest, *flAskPassURL, *flSubmodules); err != nil {
+		if changed, hash, err := syncRepo(ctx, *flRepo, *flBranch, *flRev, *flDepth, *flRoot, *flLink, *flAskPassURL, *flSubmodules); err != nil {
 			updateSyncMetrics(metricKeyError, start)
 			if *flMaxSyncFailures != -1 && failCount >= *flMaxSyncFailures {
 				// Exit after too many retries, maybe the error is not recoverable.
@@ -616,7 +626,7 @@ func setRepoReady() {
 }
 
 // addWorktreeAndSwap creates a new worktree and calls updateSymlink to swap the symlink to point to the new worktree
-func addWorktreeAndSwap(ctx context.Context, gitRoot, dest, branch, rev string, depth int, hash string, submoduleMode string) error {
+func addWorktreeAndSwap(ctx context.Context, gitRoot, link, branch, rev string, depth int, hash string, submoduleMode string) error {
 	log.V(0).Info("syncing git", "rev", rev, "hash", hash)
 
 	args := []string{"fetch", "-f", "--tags"}
@@ -700,7 +710,7 @@ func addWorktreeAndSwap(ctx context.Context, gitRoot, dest, branch, rev string,
 	}
 
 	// Flip the symlink.
-	oldWorktree, err := updateSymlink(ctx, gitRoot, dest, worktreePath)
+	oldWorktree, err := updateSymlink(ctx, gitRoot, link, worktreePath)
 	if err != nil {
 		return err
 	}
@@ -789,9 +799,9 @@ func revIsHash(ctx context.Context, rev, gitRoot string) (bool, error) {
 	return strings.HasPrefix(output, rev), nil
 }
 
-// syncRepo syncs the branch of a given repository to the destination at the given rev.
+// syncRepo syncs the branch of a given repository to the link at the given rev.
 // returns (1) whether a change occured, (2) the new hash, and (3) an error if one happened
-func syncRepo(ctx context.Context, repo, branch, rev string, depth int, gitRoot, dest string, authURL string, submoduleMode string) (bool, string, error) {
+func syncRepo(ctx context.Context, repo, branch, rev string, depth int, gitRoot, link string, authURL string, submoduleMode string) (bool, string, error) {
 	if authURL != "" {
 		// For ASKPASS Callback URL, the credentials behind is dynamic, it needs to be
 		// re-fetched each time.
@@ -802,7 +812,7 @@ func syncRepo(ctx context.Context, repo, branch, rev string, depth int, gitRoot,
 		askpassCount.WithLabelValues(metricKeySuccess).Inc()
 	}
 
-	target := filepath.Join(gitRoot, dest)
+	target := filepath.Join(gitRoot, link)
 	gitRepoPath := filepath.Join(target, ".git")
 	var hash string
 	_, err := os.Stat(gitRepoPath)
@@ -833,7 +843,7 @@ func syncRepo(ctx context.Context, repo, branch, rev string, depth int, gitRoot,
 		hash = remote
 	}
 
-	return true, hash, addWorktreeAndSwap(ctx, gitRoot, dest, branch, rev, depth, hash, submoduleMode)
+	return true, hash, addWorktreeAndSwap(ctx, gitRoot, link, branch, rev, depth, hash, submoduleMode)
 }
 
 // getRevs returns the local and upstream hashes for rev.
@@ -1083,9 +1093,10 @@ OPTIONS
             Create a shallow clone with history truncated to the specified
             number of commits.
 
-    --dest <string>, $GIT_SYNC_DEST
+    --link <string>, $GIT_SYNC_LINK
             The name of the final symlink (under --root) which will point to the
-            current git worktree. (default: the leaf dir of --repo)
+            current git worktree. This must be a filename, not a path, and may
+            not start with a period. (default: the leaf dir of --repo)
 
     --git <string>, $GIT_SYNC_GIT
             The git command to run (subject to PATH search, mostly for testing).
@@ -1135,8 +1146,8 @@ OPTIONS
             The git revision (tag or hash) to check out. (default: HEAD)
 
     --root <string>, $GIT_SYNC_ROOT
-            The root directory for git-sync operations, under which --dest will
+            The root directory for git-sync operations, under which --link will
-            be created. (default: $HOME/git)
+            be created. This flag is required.
 
     --ssh, $GIT_SYNC_SSH
             Use SSH for git authentication and operations.

demo/config/deployment.yaml

@@ -21,7 +21,7 @@ spec:
         env:
         - name: GIT_SYNC_REPO
           value: https://github.com/kubernetes/git-sync.git
-        - name: GIT_SYNC_DEST
+        - name: GIT_SYNC_LINK
           value: git-sync
       - name: hugo
         image: k8s.gcr.io/hugo

docs/ssh.md

@@ -69,9 +69,9 @@ Secret (e.g. "git-creds" used in both above examples).
 ## Step 3: Configure git-sync container
 
 In your git-sync container configuration, mount the Secret volume at
-"/etc/git-secret". Ensure that the `-repo` flag (or the GIT_SYNC_REPO
+"/etc/git-secret". Ensure that the `--repo` flag (or the GIT_SYNC_REPO
 environment variable) is set to use the SSH protocol (e.g.
-git@github.com/foo/bar) , and set the `-ssh` flags (or set GIT_SYNC_SSH to
+git@github.com/foo/bar) , and set the `--ssh` flags (or set GIT_SYNC_SSH to
 "true").  You will also need to set your container's `securityContext` to run
 as user ID "65533" which is created for running git-sync as non-root.
 
@@ -81,10 +81,10 @@ as user ID "65533" which is created for running git-sync as non-root.
       - name: git-sync
         image: k8s.gcr.io/git-sync:v3.1.5
         args:
-         - "-ssh"
+         - "--ssh"
-         - "-repo=git@github.com:foo/bar"
+         - "--repo=git@github.com:foo/bar"
-         - "-dest=bar"
+         - "--link=bar"
-         - "-branch=master"
+         - "--branch=master"
         volumeMounts:
         - name: git-secret
           mountPath: /etc/git-secret
@@ -140,11 +140,11 @@ spec:
       - name: git-sync
         image: k8s.gcr.io/git-sync:v3.1.5
         args:
-         - "-ssh"
+         - "--ssh"
-         - "-repo=git@github.com:torvalds/linux"
+         - "--repo=git@github.com:torvalds/linux"
-         - "-dest=linux"
+         - "--link=linux"
-         - "-branch=master"
+         - "--branch=master"
-         - "-depth=1"
+         - "--depth=1"
         securityContext:
           runAsUser: 65533 # git-sync user
         volumeMounts:

test_e2e.sh

@@ -156,7 +156,7 @@ GIT_SYNC \
     --branch=master \
     --rev=HEAD \
     --root="$ROOT" \
-    --dest="link" \
+    --link="link" \
     > "$DIR"/log."$TESTCASE" 2>&1
 assert_link_exists "$ROOT"/link
 assert_file_exists "$ROOT"/link/file
@@ -175,7 +175,7 @@ GIT_SYNC \
     --period=100ms \
     --repo="file://$REPO" \
     --root="$ROOT" \
-    --dest="link" \
+    --link="link" \
     > "$DIR"/log."$TESTCASE" 2>&1 &
 sleep 3
 assert_link_exists "$ROOT"/link
@@ -210,7 +210,7 @@ GIT_SYNC \
     --branch=master \
     --rev=HEAD \
     --root="$ROOT" \
-    --dest="link" \
+    --link="link" \
     > "$DIR"/log."$TESTCASE" 2>&1 &
 sleep 3
 assert_link_exists "$ROOT"/link
@@ -247,7 +247,7 @@ GIT_SYNC \
     --repo="file://$REPO" \
     --branch="$BRANCH" \
     --root="$ROOT" \
-    --dest="link" \
+    --link="link" \
     > "$DIR"/log."$TESTCASE" 2>&1 &
 sleep 3
 assert_link_exists "$ROOT"/link
@@ -287,7 +287,7 @@ GIT_SYNC \
     --repo="file://$REPO" \
     --rev="$TAG" \
     --root="$ROOT" \
-    --dest="link" \
+    --link="link" \
     > "$DIR"/log."$TESTCASE" 2>&1 &
 sleep 3
 assert_link_exists "$ROOT"/link
@@ -332,7 +332,7 @@ GIT_SYNC \
     --repo="file://$REPO" \
     --rev="$TAG" \
     --root="$ROOT" \
-    --dest="link" \
+    --link="link" \
     > "$DIR"/log."$TESTCASE" 2>&1 &
 sleep 3
 assert_link_exists "$ROOT"/link
@@ -376,7 +376,7 @@ GIT_SYNC \
     --repo="file://$REPO" \
     --rev="$REV" \
     --root="$ROOT" \
-    --dest="link" \
+    --link="link" \
     > "$DIR"/log."$TESTCASE" 2>&1 &
 sleep 3
 assert_link_exists "$ROOT"/link
@@ -411,7 +411,7 @@ GIT_SYNC \
     --repo="file://$REPO" \
     --rev="$REV" \
     --root="$ROOT" \
-    --dest="link" \
+    --link="link" \
     > "$DIR"/log."$TESTCASE" 2>&1
 assert_link_exists "$ROOT"/link
 assert_file_exists "$ROOT"/link/file
@@ -430,7 +430,7 @@ GIT_SYNC \
     --one-time \
     --repo="file://$REPO" \
     --root="$ROOT" \
-    --dest="link" \
+    --link="link" \
     > "$DIR"/log."$TESTCASE" 2>&1
 assert_link_exists "$ROOT"/link
 assert_file_exists "$ROOT"/link/file
@@ -442,7 +442,7 @@ GIT_SYNC \
     --one-time \
     --repo="file://$REPO" \
     --root="$ROOT" \
-    --dest="link" \
+    --link="link" \
     > "$DIR"/log."$TESTCASE" 2>&1
 assert_link_exists "$ROOT"/link
 assert_file_exists "$ROOT"/link/file
@@ -463,7 +463,7 @@ GIT_SYNC \
     --sync-timeout=1s \
     --repo="file://$REPO" \
     --root="$ROOT" \
-    --dest="link" \
+    --link="link" \
     > "$DIR"/log."$TESTCASE" 2>&1 || true
 # check for failure
 assert_file_absent "$ROOT"/link/file
@@ -474,7 +474,7 @@ GIT_SYNC \
     --sync-timeout=16s \
     --repo="file://$REPO" \
     --root="$ROOT" \
-    --dest="link" \
+    --link="link" \
     > "$DIR"/log."$TESTCASE" 2>&1 &
 sleep 10
 assert_link_exists "$ROOT"/link
@@ -503,7 +503,7 @@ GIT_SYNC \
     --repo="file://$REPO" \
     --depth="$expected_depth" \
     --root="$ROOT" \
-    --dest="link" \
+    --link="link" \
     > "$DIR"/log."$TESTCASE" 2>&1 &
 sleep 3
 assert_link_exists "$ROOT"/link
@@ -553,7 +553,7 @@ GIT_SYNC \
     --branch=master \
     --rev=HEAD \
     --root="$ROOT" \
-    --dest="link" \
+    --link="link" \
     > "$DIR"/log."$TESTCASE" 2>&1 || true
 # check for failure
 assert_file_absent "$ROOT"/link/file
@@ -567,7 +567,7 @@ GIT_SYNC \
     --branch=master \
     --rev=HEAD \
     --root="$ROOT" \
-    --dest="link" \
+    --link="link" \
     > "$DIR"/log."$TESTCASE" 2>&1
 assert_link_exists "$ROOT"/link
 assert_file_exists "$ROOT"/link/file
@@ -598,7 +598,7 @@ GIT_SYNC \
     --branch=master \
     --rev=HEAD \
     --root="$ROOT" \
-    --dest="link" \
+    --link="link" \
     > "$DIR"/log."$TESTCASE" 2>&1 || true
 # check for failure
 assert_file_absent "$ROOT"/link/file
@@ -618,7 +618,7 @@ GIT_SYNC \
     --branch=master \
     --rev=HEAD \
     --root="$ROOT" \
-    --dest="link" \
+    --link="link" \
     > "$DIR"/log."$TESTCASE" 2>&1
 assert_link_exists "$ROOT"/link
 assert_file_exists "$ROOT"/link/file
@@ -637,7 +637,7 @@ GIT_SYNC \
     --period=100ms \
     --repo="file://$REPO" \
     --root="$ROOT" \
-    --dest="link" \
+    --link="link" \
     --sync-hook-command="$SYNC_HOOK_COMMAND" \
     > "$DIR"/log."$TESTCASE" 2>&1 &
 sleep 3
@@ -672,7 +672,7 @@ GIT_SYNC \
     --root="$ROOT" \
     --webhook-url="http://127.0.0.1:$NCPORT" \
     --webhook-success-status=200 \
-    --dest="link" \
+    --link="link" \
     > "$DIR"/log."$TESTCASE" 2>&1 &
 # check that basic call works
 { (echo -e "HTTP/1.1 200 OK\r\n" | nc -q1 -l $NCPORT > /dev/null) &}
@@ -715,7 +715,7 @@ GIT_SYNC \
     --root="$ROOT" \
     --webhook-url="http://127.0.0.1:$NCPORT" \
     --webhook-success-status=-1 \
-    --dest="link" \
+    --link="link" \
     > "$DIR"/log."$TESTCASE" 2>&1 &
 # check that basic call works
 { (echo -e "HTTP/1.1 404 Not Found\r\n" | nc -q1 -l $NCPORT > /dev/null) &}
@@ -743,7 +743,7 @@ GIT_SYNC \
     --http-bind=":$BINDPORT" \
     --http-metrics \
     --http-pprof \
-    --dest="link" \
+    --link="link" \
     > "$DIR"/log."$TESTCASE" 2>&1 &
 while ! curl --silent --output /dev/null http://localhost:$BINDPORT; do
     # do nothing, just wait for the HTTP to come up
@@ -801,7 +801,7 @@ GIT_SYNC \
     --period=100ms \
     --repo="file://$REPO" \
     --root="$ROOT" \
-    --dest="link" \
+    --link="link" \
     > "$DIR"/log."$TESTCASE" 2>&1 &
 sleep 3
 assert_link_exists "$ROOT"/link
@@ -889,7 +889,7 @@ GIT_SYNC \
     --repo="file://$REPO" \
     --depth="$expected_depth" \
     --root="$ROOT" \
-    --dest="link" \
+    --link="link" \
     > "$DIR"/log."$TESTCASE" 2>&1 &
 sleep 3
 assert_link_exists "$ROOT"/link
@@ -963,7 +963,7 @@ GIT_SYNC \
     --period=100ms \
     --repo="file://$REPO" \
     --root="$ROOT" \
-    --dest="link" \
+    --link="link" \
     --submodules=off \
     > "$DIR"/log."$TESTCASE" 2>&1 &
 sleep 3
@@ -1005,7 +1005,7 @@ GIT_SYNC \
     --period=100ms \
     --repo="file://$REPO" \
     --root="$ROOT" \
-    --dest="link" \
+    --link="link" \
     --submodules=shallow \
     > "$DIR"/log."$TESTCASE" 2>&1 &
 sleep 3
@@ -1039,7 +1039,7 @@ GIT_SYNC \
     --branch=master \
     --rev=HEAD \
     --root="$ROOT" \
-    --dest="link" \
+    --link="link" \
     --ssh \
     --ssh-known-hosts=false \
     > "$DIR"/log."$TESTCASE" 2>&1