diff --git a/README.md b/README.md index e9a4b1223..5244314f7 100644 --- a/README.md +++ b/README.md @@ -445,6 +445,23 @@ OPTIONS will take precedence. If not specified, this defaults to 10 seconds ("10s"). + --pre-publish-exechook-backoff , $GITSYNC_PRE_PUBLISH_EXECHOOK_BACKOFF + The time to wait before retrying a failed + --pre-publish-exechook-command. If not specified, this defaults to + 3 seconds ("3s"). + + --pre-publish-exechook-command , $GITSYNC_PRE_PUBLISH_EXECHOOK_COMMAND + An optional command to be executed after syncing a new hash of the + remote repository but before publishing the symlink (see --link). + This command does not take any arguments and executes with the + synced repo as its working directory. The $GITSYNC_HASH environment + variable will be set to the previous git hash that was synced. This + hook will always be invoked as it runs before any sync attempt. + + --pre-publish-exechook-timeout , $GITSYNC_PRE_PUBLISH_EXECHOOK_TIMEOUT + The timeout for the --pre-publish-exechook-command. If not + specified this defaults to 30 seconds ("30s"). + --ref , $GITSYNC_REF The git revision (branch, tag, or hash) to check out. If not specified, this defaults to "HEAD" (of the upstream repo's default diff --git a/main.go b/main.go index 99fdf840a..eb96be093 100644 --- a/main.go +++ b/main.go @@ -133,6 +133,13 @@ type repoSync struct { appTokenExpiry time.Time // time when github app auth token expires } +// syncHooks manages the refresh of credentials and hooks. +type syncHooks struct { + refreshCreds func(ctx context.Context) error + beforePublish func(hash string) error + afterPublish func(hash string) error +} + func main() { // In case we come up as pid 1, act as init. if os.Getpid() == 1 { @@ -230,6 +237,16 @@ func main() { envDuration(3*time.Second, "GITSYNC_EXECHOOK_BACKOFF", "GIT_SYNC_EXECHOOK_BACKOFF"), "the time to wait before retrying a failed exechook") + flPrePubExechookCommand := pflag.String("pre-publish-exechook-command", + envString("", "GITSYNC_PRE_PUBLISH_EXECHOOK_COMMAND"), + "an optional command to be run before syncs complete (must be idempotent)") + flPrePubExechookTimeout := pflag.Duration("pre-publish-exechook-timeout", + envDuration(30*time.Second, "GITSYNC_PRE_PUBLISH_EXECHOOK_TIMEOUT"), + "the timeout for the pre-publish-exechook") + flPrePubExechookBackoff := pflag.Duration("pre-publish-exechook-backoff", + envDuration(3*time.Second, "GITSYNC_PRE_PUBLISH_EXECHOOK_BACKOFF"), + "the time to wait before retrying a failed pre-publish-exechook") + flWebhookURL := pflag.String("webhook-url", envString("", "GITSYNC_WEBHOOK_URL", "GIT_SYNC_WEBHOOK_URL"), "a URL for optional webhook notifications when syncs complete (must be idempotent)") @@ -541,6 +558,15 @@ func main() { } } + if *flPrePubExechookCommand != "" { + if *flPrePubExechookTimeout < time.Second { + fatalConfigErrorf(log, true, "invalid flag: --pre-publish-exechook-timeout must be at least 1s") + } + if *flPrePubExechookBackoff < time.Second { + fatalConfigErrorf(log, true, "invalid flag: --pre-publish-exechook-backoff must be at least 1s") + } + } + if *flWebhookURL != "" { if *flWebhookStatusSuccess == -1 { // Back-compat: -1 and 0 mean the same things @@ -859,8 +885,10 @@ func main() { // Startup exechooks goroutine var exechookRunner *hook.HookRunner if *flExechookCommand != "" { - log := log.WithName("exechook") + logname := "exechook" + log := log.WithName(logname) exechook := hook.NewExechook( + logname, cmd.NewRunner(log), *flExechookCommand, func(hash string) string { @@ -880,6 +908,32 @@ func main() { go exechookRunner.Run(context.Background()) } + // Startup pre-publish-exechooks goroutine + var prePubExechookRunner *hook.HookRunner + if *flPrePubExechookCommand != "" { + logname := "pre-publish-exechook" + log := log.WithName(logname) + exechook := hook.NewExechook( + logname, + cmd.NewRunner(log), + *flPrePubExechookCommand, + func(hash string) string { + return git.worktreeFor(hash).Path().String() + }, + []string{}, + *flPrePubExechookTimeout, + log, + ) + prePubExechookRunner = hook.NewHookRunner( + exechook, + *flPrePubExechookBackoff, + hook.NewHookData(), + log, + *flOneTime, + ) + go prePubExechookRunner.Run(context.Background()) + } + // Setup signal notify channel sigChan := make(chan os.Signal, 1) if syncSig != 0 { @@ -931,6 +985,24 @@ func main() { return nil } + syncHooks := syncHooks{ + refreshCreds: refreshCreds, + beforePublish: func(hash string) error { + if prePubExechookRunner != nil { + prePubExechookRunner.Send(hash) + } + return nil + }, + afterPublish: func(hash string) error { + if exechookRunner != nil { + exechookRunner.Send(hash) + } + if webhookRunner != nil { + webhookRunner.Send(hash) + } + return nil + }, + } failCount := 0 syncCount := uint64(0) initialSyncDone := false @@ -948,7 +1020,7 @@ func main() { start := time.Now() ctx, cancel := context.WithTimeout(context.Background(), *flSyncTimeout) - if changed, hash, err := git.SyncRepo(ctx, refreshCreds); err != nil { + if changed, hash, err := git.SyncRepo(ctx, syncHooks); err != nil { failCount++ updateSyncMetrics(metricKeyError, start) if maxFails := getMaxFailures(); maxFails >= 0 && failCount >= maxFails { @@ -975,12 +1047,6 @@ func main() { log.V(3).Info("touched touch-file", "path", absTouchFile) } } - if webhookRunner != nil { - webhookRunner.Send(hash) - } - if exechookRunner != nil { - exechookRunner.Send(hash) - } updateSyncMetrics(metricKeySuccess, start) } else { updateSyncMetrics(metricKeyNoOp, start) @@ -999,6 +1065,11 @@ func main() { // Assumes that if hook channels are not nil, they will have at // least one value before getting closed exitCode := 0 // is 0 if all hooks succeed, else is 1 + if prePubExechookRunner != nil && changed { + if err := prePubExechookRunner.WaitForCompletion(); err != nil { + exitCode = 1 + } + } if exechookRunner != nil { if err := exechookRunner.WaitForCompletion(); err != nil { exitCode = 1 @@ -1754,10 +1825,10 @@ func (git *repoSync) currentWorktree() (worktree, error) { // SyncRepo syncs the repository to the desired ref, publishes it via the link, // 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) { +func (git *repoSync) SyncRepo(ctx context.Context, syncHooks syncHooks) (bool, string, error) { git.log.V(3).Info("syncing", "repo", redactURL(git.repo)) - if err := refreshCreds(ctx); err != nil { + if err := syncHooks.refreshCreds(ctx); err != nil { return false, "", fmt.Errorf("credential refresh failed: %w", err) } @@ -1842,7 +1913,18 @@ func (git *repoSync) SyncRepo(ctx context.Context, refreshCreds func(context.Con // If we have a new hash, update the symlink to point to the new worktree. if changed { - err := git.publishSymlink(newWorktree) + // If the previous run crashed before publishing the link, then we + // must call the pre-publish hook, and since changed is true, we will. + // we will. If the previous run crashed after publishing the link, + // then we do not need to call the pre-publish hook, and since + // changed is false, we won't. The post-publish hooks are called in + // both cases. + err := syncHooks.beforePublish(newWorktree.Hash()) + if err != nil { + return false, "", err + } + + err = git.publishSymlink(newWorktree) if err != nil { return false, "", err } @@ -1855,6 +1937,11 @@ func (git *repoSync) SyncRepo(ctx context.Context, refreshCreds func(context.Con } } + err := syncHooks.afterPublish(newWorktree.Hash()) + if err != nil { + return false, "", err + } + // Mark ourselves as "ready". setRepoReady() git.syncCount++ @@ -2515,14 +2602,15 @@ OPTIONS --exechook-command , $GITSYNC_EXECHOOK_COMMAND An optional command to be executed after syncing a new hash of the - remote repository. This command does not take any arguments and - executes with the synced repo as its working directory. The - $GITSYNC_HASH environment variable will be set to the git hash that - was synced. If, at startup, git-sync finds that the --root already - has the correct hash, this hook will still be invoked. This means - that hooks can be invoked more than one time per hash, so they - must be idempotent. This flag obsoletes --sync-hook-command, but - if sync-hook-command is specified, it will take precedence. + remote repository and publishing the symlink (see --link). This + command does not take any arguments and executes with the synced + repo as its working directory. The $GITSYNC_HASH environment + variable will be set to the git hash that was synced. If, at + startup, git-sync finds that the --root already has the correct + hash, this hook will still be invoked. This means that hooks can + be invoked more than one time per hash, so they must be idempotent. + This flag obsoletes --sync-hook-command, but if sync-hook-command + is specified, it will take precedence. --exechook-timeout , $GITSYNC_EXECHOOK_TIMEOUT The timeout for the --exechook-command. If not specifid, this @@ -2676,6 +2764,23 @@ OPTIONS will take precedence. If not specified, this defaults to 10 seconds ("10s"). + --pre-publish-exechook-backoff , $GITSYNC_PRE_PUBLISH_EXECHOOK_BACKOFF + The time to wait before retrying a failed + --pre-publish-exechook-command. If not specified, this defaults to + 3 seconds ("3s"). + + --pre-publish-exechook-command , $GITSYNC_PRE_PUBLISH_EXECHOOK_COMMAND + An optional command to be executed after syncing a new hash of the + remote repository but before publishing the symlink (see --link). + This command does not take any arguments and executes with the + synced repo as its working directory. The $GITSYNC_HASH environment + variable will be set to the previous git hash that was synced. This + hook will always be invoked as it runs before any sync attempt. + + --pre-publish-exechook-timeout , $GITSYNC_PRE_PUBLISH_EXECHOOK_TIMEOUT + The timeout for the --pre-publish-exechook-command. If not + specified this defaults to 30 seconds ("30s"). + --ref , $GITSYNC_REF The git revision (branch, tag, or hash) to check out. If not specified, this defaults to "HEAD" (of the upstream repo's default diff --git a/pkg/hook/exechook.go b/pkg/hook/exechook.go index e31d99642..c62f15ef0 100644 --- a/pkg/hook/exechook.go +++ b/pkg/hook/exechook.go @@ -27,6 +27,8 @@ import ( // Exechook implements Hook in terms of executing a command. type Exechook struct { + // Name + name string // Runner cmdrunner cmd.Runner // Command to run @@ -42,8 +44,9 @@ type Exechook struct { } // NewExechook returns a new Exechook. -func NewExechook(cmdrunner cmd.Runner, command string, getWorktree func(string) string, args []string, timeout time.Duration, log logintf) *Exechook { +func NewExechook(name string, cmdrunner cmd.Runner, command string, getWorktree func(string) string, args []string, timeout time.Duration, log logintf) *Exechook { return &Exechook{ + name: name, cmdrunner: cmdrunner, command: command, getWorktree: getWorktree, @@ -55,7 +58,7 @@ func NewExechook(cmdrunner cmd.Runner, command string, getWorktree func(string) // Name describes hook, implements Hook.Name. func (h *Exechook) Name() string { - return "exechook" + return h.name } // Do runs exechook.command, implements Hook.Do. diff --git a/pkg/hook/exechook_test.go b/pkg/hook/exechook_test.go index 0de065088..1f6b521df 100644 --- a/pkg/hook/exechook_test.go +++ b/pkg/hook/exechook_test.go @@ -29,6 +29,7 @@ func TestNotZeroReturnExechookDo(t *testing.T) { t.Run("test not zero return code", func(t *testing.T) { l := logging.New("", "", 0) ch := NewExechook( + "exechook", cmd.NewRunner(l), "false", func(string) string { return "/tmp" }, @@ -47,6 +48,7 @@ func TestZeroReturnExechookDo(t *testing.T) { t.Run("test zero return code", func(t *testing.T) { l := logging.New("", "", 0) ch := NewExechook( + "exechook", cmd.NewRunner(l), "true", func(string) string { return "/tmp" }, @@ -65,6 +67,7 @@ func TestTimeoutExechookDo(t *testing.T) { t.Run("test timeout", func(t *testing.T) { l := logging.New("", "", 0) ch := NewExechook( + "exechook", cmd.NewRunner(l), "/bin/sh", func(string) string { return "/tmp" }, diff --git a/test_e2e.sh b/test_e2e.sh index 9571e48bf..70ffde3a0 100755 --- a/test_e2e.sh +++ b/test_e2e.sh @@ -2559,6 +2559,138 @@ function e2e::exechook_startup_after_crash() { assert_file_lines_eq "$RUNLOG" 1 } +############################################## +# Test pre-publish-exechook-success +############################################## +function e2e::pre_publish_exechook_success() { + cat /dev/null > "$RUNLOG" + + # First sync + echo "${FUNCNAME[0]} 1" > "$REPO/file" + git -C "$REPO" commit -qam "${FUNCNAME[0]} 1" + + GIT_SYNC \ + --period=100ms \ + --repo="file://$REPO" \ + --root="$ROOT" \ + --link="link" \ + --pre-publish-exechook-command="/$EXECHOOK_COMMAND" \ + & + wait_for_sync "${MAXWAIT}" + assert_link_exists "$ROOT/link" + assert_file_exists "$ROOT/link/file" + assert_file_exists "$ROOT/link/exechook" + assert_file_eq "$ROOT/link/file" "${FUNCNAME[0]} 1" + assert_file_eq "$ROOT/link/exechook" "${FUNCNAME[0]} 1" + assert_file_eq "$ROOT/link/exechook-env" "$EXECHOOK_ENVKEY=$EXECHOOK_ENVVAL" + assert_file_lines_eq "$RUNLOG" 1 + + # Move forward + echo "${FUNCNAME[0]} 2" > "$REPO/file" + git -C "$REPO" commit -qam "${FUNCNAME[0]} 2" + wait_for_sync "${MAXWAIT}" + assert_link_exists "$ROOT/link" + assert_file_exists "$ROOT/link/file" + assert_file_exists "$ROOT/link/exechook" + assert_file_eq "$ROOT/link/file" "${FUNCNAME[0]} 2" + assert_file_eq "$ROOT/link/exechook" "${FUNCNAME[0]} 2" + assert_file_eq "$ROOT/link/exechook-env" "$EXECHOOK_ENVKEY=$EXECHOOK_ENVVAL" + assert_file_lines_eq "$RUNLOG" 2 +} + +############################################## +# Test pre-publish-exechook-fail-retry +############################################## +function e2e::pre_publish_exechook_fail_retry() { + cat /dev/null > "$RUNLOG" + + # First sync - return a failure to ensure that we try again + GIT_SYNC \ + --period=100ms \ + --repo="file://$REPO" \ + --root="$ROOT" \ + --link="link" \ + --pre-publish-exechook-command="/$EXECHOOK_COMMAND_FAIL" \ + --pre-publish-exechook-backoff=1s \ + & + sleep 3 # give it time to retry + + # Check that exechook was called + assert_file_lines_ge "$RUNLOG" 2 +} + +###################################################### +# Test pre-publish-exechook-success with --one-time +###################################################### +function e2e::pre_publish_exechook_success_once() { + GIT_SYNC \ + --one-time \ + --repo="file://$REPO" \ + --root="$ROOT" \ + --link="link" \ + --pre-publish-exechook-command="/$EXECHOOK_COMMAND_SLEEPY" + + wait_for_sync "${MAXWAIT}" + assert_link_exists "$ROOT/link" + assert_file_exists "$ROOT/link/file" + assert_file_exists "$ROOT/link/exechook" + assert_file_eq "$ROOT/link/file" "${FUNCNAME[0]}" + assert_file_eq "$ROOT/link/exechook" "${FUNCNAME[0]}" + assert_file_eq "$ROOT/link/exechook-env" "$EXECHOOK_ENVKEY=$EXECHOOK_ENVVAL" +} + +################################################### +# Test pre-publish-exechook-fail with --one-time +################################################### +function e2e::pre_publish_exechook_fail_once() { + cat /dev/null > "$RUNLOG" + + assert_fail \ + GIT_SYNC \ + --one-time \ + --repo="file://$REPO" \ + --root="$ROOT" \ + --link="link" \ + --pre-publish-exechook-command="/$EXECHOOK_COMMAND_FAIL_SLEEPY" \ + --pre-publish-exechook-backoff=1s + + assert_link_exists "$ROOT/link" + assert_file_exists "$ROOT/link/file" + assert_file_eq "$ROOT/link/file" "${FUNCNAME[0]}" + assert_file_lines_eq "$RUNLOG" 1 +} + +########################################################## +# Test pre-publish-exechook at startup with correct SHA +########################################################## +function e2e::pre_publish_exechook_startup_after_crash() { + GIT_SYNC \ + --one-time \ + --repo="file://$REPO" \ + --ref="$MAIN_BRANCH" \ + --root="$ROOT" \ + --link="link" + assert_link_exists "$ROOT/link" + assert_file_exists "$ROOT/link/file" + assert_file_eq "$ROOT/link/file" "${FUNCNAME[0]}" + + # No changes to repo, pre-publish-exechook-command is not called + + cat /dev/null > "$RUNLOG" + GIT_SYNC \ + --one-time \ + --repo="file://$REPO" \ + --ref="$MAIN_BRANCH" \ + --root="$ROOT" \ + --link="link" \ + --pre-publish-exechook-command="/$EXECHOOK_COMMAND" + assert_link_exists "$ROOT/link" + assert_file_exists "$ROOT/link/file" + assert_file_absent "$ROOT/link/exechook" + assert_file_eq "$ROOT/link/file" "${FUNCNAME[0]}" + assert_file_lines_eq "$RUNLOG" 0 +} + ############################################## # Test webhook success ##############################################