From af2ae5b533dc20aea4d36f478df771b929b93863 Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Sat, 31 Oct 2020 10:26:27 -0700 Subject: [PATCH 1/3] Make the --root flag required, no default BREAKING CHANGE The default of $HOME has caused problems for people playing with git-sync, so this change makes it a required argument. --- README.md | 2 +- cmd/git-sync/main.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 1970d20..8c4de3b 100644 --- a/README.md +++ b/README.md @@ -87,7 +87,7 @@ 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 --dest 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_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" | diff --git a/cmd/git-sync/main.go b/cmd/git-sync/main.go index 84a943e..7ceff3b 100644 --- a/cmd/git-sync/main.go +++ b/cmd/git-sync/main.go @@ -62,7 +62,7 @@ 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") var flDest = pflag.String("dest", envString("GIT_SYNC_DEST", ""), "the name of (a symlink to) a directory in which to check-out files under --root (defaults to the leaf dir of --repo)") @@ -1136,7 +1136,7 @@ OPTIONS --root , $GIT_SYNC_ROOT The root directory for git-sync operations, under which --dest will - be created. (default: $HOME/git) + be created. This flag is required. --ssh, $GIT_SYNC_SSH Use SSH for git authentication and operations. From 0e802450ee229032af038e4060df70f80f08a960 Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Sat, 31 Oct 2020 10:30:31 -0700 Subject: [PATCH 2/3] Don't allow --dest to start with a dot BREAKING CHANGE It's useful to reserve names that start with a dot. --- README.md | 4 +-- cmd/git-sync/main.go | 49 ++++++++++++++++++++-------------- demo/config/deployment.yaml | 2 +- docs/ssh.md | 4 +-- test_e2e.sh | 52 ++++++++++++++++++------------------- 5 files changed, 61 insertions(+), 50 deletions(-) diff --git a/README.md b/README.md index 8c4de3b..0ff2b6e 100644 --- a/README.md +++ b/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 | "" | -| 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_ROOT | `--root` | the root directory for git-sync operations, under which --link will be created | "" | +| 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 | diff --git a/cmd/git-sync/main.go b/cmd/git-sync/main.go index 7ceff3b..0bd8996 100644 --- a/cmd/git-sync/main.go +++ b/cmd/git-sync/main.go @@ -63,9 +63,9 @@ var flSubmodules = pflag.String("submodules", envString("GIT_SYNC_SUBMODULES", " "git submodule behavior: one of 'recursive', 'shallow', or 'off'") var flRoot = pflag.String("root", envString("GIT_SYNC_ROOT", ""), - "the root directory for git-sync operations, under which --dest will be created") -var flDest = pflag.String("dest", envString("GIT_SYNC_DEST", ""), - "the name of (a symlink to) a directory in which to check-out files under --root (defaults to the leaf dir of --repo)") + "the root directory for git-sync operations, under which --link will be created") +var flLink = pflag.String("link", envString("GIT_SYNC_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)") 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 == "" { - parts := strings.Split(strings.Trim(*flRepo, "/"), "/") - *flDest = parts[len(parts)-1] + if *flDest != "" { + *flLink = *flDest } - - if strings.Contains(*flDest, "/") { - fmt.Fprintf(os.Stderr, "ERROR: --dest must be a leaf name, not a path\n") + if *flLink == "" { + parts := strings.Split(strings.Trim(*flRepo, "/"), "/") + *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 , $GIT_SYNC_DEST + --link , $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 , $GIT_SYNC_GIT The git command to run (subject to PATH search, mostly for testing). @@ -1135,7 +1146,7 @@ OPTIONS The git revision (tag or hash) to check out. (default: HEAD) --root , $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. This flag is required. --ssh, $GIT_SYNC_SSH diff --git a/demo/config/deployment.yaml b/demo/config/deployment.yaml index b529f65..76155f8 100644 --- a/demo/config/deployment.yaml +++ b/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 diff --git a/docs/ssh.md b/docs/ssh.md index a191834..b805e37 100644 --- a/docs/ssh.md +++ b/docs/ssh.md @@ -83,7 +83,7 @@ as user ID "65533" which is created for running git-sync as non-root. args: - "-ssh" - "-repo=git@github.com:foo/bar" - - "-dest=bar" + - "-link=bar" - "-branch=master" volumeMounts: - name: git-secret @@ -142,7 +142,7 @@ spec: args: - "-ssh" - "-repo=git@github.com:torvalds/linux" - - "-dest=linux" + - "-link=linux" - "-branch=master" - "-depth=1" securityContext: diff --git a/test_e2e.sh b/test_e2e.sh index dbb9bef..2ecc8ee 100755 --- a/test_e2e.sh +++ b/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 From a2fa6892964300b882591bce8474936dadbd647f Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Fri, 6 Nov 2020 20:12:44 -0800 Subject: [PATCH 3/3] Fix SSH docs to use 2 dashes --- docs/ssh.md | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/docs/ssh.md b/docs/ssh.md index b805e37..9b93e59 100644 --- a/docs/ssh.md +++ b/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" - - "-repo=git@github.com:foo/bar" - - "-link=bar" - - "-branch=master" + - "--ssh" + - "--repo=git@github.com:foo/bar" + - "--link=bar" + - "--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" - - "-repo=git@github.com:torvalds/linux" - - "-link=linux" - - "-branch=master" - - "-depth=1" + - "--ssh" + - "--repo=git@github.com:torvalds/linux" + - "--link=linux" + - "--branch=master" + - "--depth=1" securityContext: runAsUser: 65533 # git-sync user volumeMounts: