feat: adopt provider-runtime's automatic rate-limit slow-down (v1.11.0)

README.md

@@ -39,9 +39,10 @@ The provider controller can be configured using the following environment variab
 | `GIT_PROVIDER_LEADER_ELECTION` | bool | `false` | Use leader election for the controller manager |
 | `GIT_PROVIDER_MAX_ERROR_RETRY_INTERVAL` | duration | `1m` | The maximum interval between retries when an error occurs. Should be less than half of the poll interval |
 | `GIT_PROVIDER_MIN_ERROR_RETRY_INTERVAL` | duration | `1s` | The minimum interval between retries when an error occurs. Should be less than max-error-retry-interval |
-| `GIT_PROVIDER_TIMEOUT` | duration | `4m` | The timeout time for each action. |
+| `GIT_PROVIDER_TIMEOUT` | duration | `4m` | The time all the external calls of one reconcile share, clones and pushes included. A call still running at the deadline fails. Raise it for repositories that take longer to clone and push (with the chart: `--set-string env.GIT_PROVIDER_TIMEOUT=10m`) |
 | `GIT_PROVIDER_GIT_COMMIT_AUTHOR_NAME` | string | `krateo-git-provider` | The name to use for git commits. |
 | `GIT_PROVIDER_GIT_COMMIT_AUTHOR_EMAIL` | string | `contact@krateo.io` | The email to use for git commits. |
+| `GIT_PROVIDER_RATE_LIMIT_GOVERNOR` | bool | `false` (the chart sets `true`) | Slow reconciles down per remote and credential when a git server signals a rate limit |
 
 ## CRD Reference
 To view the generated CR configuration schema, visit [this link](https://doc.crds.dev/github.com/krateoplatformops/git-provider).

go.mod

@@ -13,7 +13,7 @@ require (
 	github.com/go-git/go-git/v5 v5.19.2
 	github.com/go-logr/logr v1.4.4
 	github.com/krateo-platformops/plumbing v1.14.3
-	github.com/krateo-platformops/provider-runtime v1.8.2
+	github.com/krateo-platformops/provider-runtime v1.11.0
 	github.com/moby/moby/api v1.52.0
 	github.com/moby/moby/client v0.1.0
 	github.com/pkg/errors v0.9.1

go.sum

@@ -149,8 +149,8 @@ github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
 github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE=
 github.com/krateo-platformops/plumbing v1.14.3 h1:g60QTgbhIsZVTFq9JZveNx2QZauVyFeWHUAVxahxKRU=
 github.com/krateo-platformops/plumbing v1.14.3/go.mod h1:JMHwVbS2vdA/OSm3Gi3QCDTntFjPmrGtqZ6LQuhciZQ=
-github.com/krateo-platformops/provider-runtime v1.8.2 h1:X3OxWfdsZfxOAR6IkdVD/cobCUo04mJKaxjHhSZmOQU=
-github.com/krateo-platformops/provider-runtime v1.8.2/go.mod h1:3j80GRyb7mfg+FweV0Ev1Si/0n72lZEujb3JyYL3BMQ=
+github.com/krateo-platformops/provider-runtime v1.11.0 h1:PLhAYZ2uzoUrw7S7HvQ3KXc2/htJ2mvboevJe+chqkE=
+github.com/krateo-platformops/provider-runtime v1.11.0/go.mod h1:Ah+zDm8QQEI4zm8epsDQnWTGNzoi2jnwivKsxTX2IrY=
 github.com/kylelemons/godebug v1.1.0 h1:RPNrshWIDI6G2gRW9EHilWtl7Z6Sb1BR0xunSBf0SNc=
 github.com/kylelemons/godebug v1.1.0/go.mod h1:9/0rRGxNHcop5bhtWyNeEfOS8JIWk580+fNqagV/RAw=
 github.com/mailru/easyjson v0.9.0 h1:PrnmzHw7262yW8sTBwxi1PdJA3Iw/EKBa8psRf7d9a4=

helm/git-provider/values.schema.json

@@ -164,6 +164,10 @@
           "type": "string",
           "title": "Poll interval (e.g. 3m)"
         },
+        "GIT_PROVIDER_RATE_LIMIT_GOVERNOR": {
+          "type": "string",
+          "title": "Slow down when a git server signals a rate limit"
+        },
         "GIT_PROVIDER_SERVICE_PORT_HACK": {
           "type": "string",
           "title": "Service port hack"

helm/git-provider/values.yaml

@@ -80,5 +80,6 @@ affinity: {}
 env:
   GIT_PROVIDER_DEBUG: "false"
   GIT_PROVIDER_POLL_INTERVAL: "3m"
+  GIT_PROVIDER_RATE_LIMIT_GOVERNOR: "true"
   GIT_PROVIDER_SERVICE_PORT_HACK: "false"
   HOME: "/tmp"

internal/clients/git/git.go

@@ -20,7 +20,6 @@ import (
 
 	"github.com/go-git/go-git/v5/plumbing/cache"
 	gitclient "github.com/go-git/go-git/v5/plumbing/transport/client"
-	githttp "github.com/go-git/go-git/v5/plumbing/transport/http"
 	"github.com/go-git/go-git/v5/storage"
 
 	"github.com/go-git/go-billy/v5"
@@ -90,11 +89,15 @@ func normalizeEmptyReasonError(err error) error {
 // krateo.io/external-create-pending annotation and writing the create result,
 // the managed resource is wedged forever on "cannot determine creation result".
 //
-// Operations that need no custom client take the read lock and share go-git's
-// default https client; only the cookie-jar and capability paths write, under
-// the exclusive lock, and they restore the defaults before releasing it.
+// Operations that need no custom client take the read lock and share
+// defaultClient; only the cookie-jar and capability paths write, under the
+// exclusive lock, and they restore the defaults before releasing it.
 var transportMu sync.RWMutex
 
+// remoteListTimeout bounds listing a remote's refs. It is the timeout go-git's
+// Remote.List applies itself; ListContext applies none.
+const remoteListTimeout = 10 * time.Second
+
 type Repo struct {
 	rawURL      string
 	auth        transport.AuthMethod
@@ -132,12 +135,12 @@ type IndexOptions struct {
 	ToPath     string
 }
 
-// cookieJarClient builds the HTTP client carrying the given git cookie.
+// cookieJarClient builds the go-git client carrying the given git cookie.
 //
 // It returns (nil, nil) when there is no usable cookie, which means the caller
-// must leave the global transport registry completely alone - go-git's default
-// https client is already installed and is exactly what we want.
-func cookieJarClient(cookie []byte) (*http.Client, error) {
+// must leave the global transport registry completely alone - defaultClient is
+// already installed and is exactly what we want.
+func cookieJarClient(cookie []byte) (*signalClient, error) {
 	cookie = bytes.Trim(cookie, "\n")
 	if len(cookie) == 0 {
 		return nil, nil
@@ -173,7 +176,7 @@ func cookieJarClient(cookie []byte) (*http.Client, error) {
 		},
 	)
 
-	return &http.Client{Jar: jar}, nil
+	return newSignalClient(jar), nil
 }
 
 // azureDevOps reports whether the URL points at Azure DevOps, which needs the
@@ -202,9 +205,9 @@ func lockTransport(cookie []byte, mutatesGlobals bool) (func(), error) {
 	}
 
 	if custom == nil && !mutatesGlobals {
-		// Nothing to install: go-git's default https client is already in the
-		// registry. Hold the read lock so that no writer can mutate the map
-		// out from under go-git while it reads it.
+		// Nothing to install: defaultClient is already in the registry. Hold
+		// the read lock so that no writer can mutate the map out from under
+		// go-git while it reads it.
 		transportMu.RLock()
 		return transportMu.RUnlock, nil
 	}
@@ -214,30 +217,31 @@ func lockTransport(cookie []byte, mutatesGlobals bool) (func(), error) {
 		return transportMu.Unlock, nil
 	}
 
-	gitclient.InstallProtocol("https", githttp.NewClient(custom))
+	gitclient.InstallProtocol("https", custom)
 	return func() {
-		// Restore the exact singleton go-git ships in its registry, rather than
-		// allocating an equivalent-but-different client on every release.
-		gitclient.InstallProtocol("https", githttp.DefaultClient)
+		// Restore the singleton rather than allocating an equivalent client on
+		// every release. It has to be ours, not go-git's: restoring go-git's
+		// would silently stop every later operation from reporting signals.
+		gitclient.InstallProtocol("https", defaultClient)
 		transportMu.Unlock()
 	}, nil
 }
 
 // GetLatestCommitRemote lists the remote refs and returns the tip of the
 // requested branch. It takes the transport lock; use getLatestCommitRemote when
 // the caller already holds it.
-func GetLatestCommitRemote(opts ListOptions) (*string, error) {
+func GetLatestCommitRemote(ctx context.Context, opts ListOptions) (*string, error) {
 	release, err := lockTransport(opts.GitCookies, false)
 	if err != nil {
 		return nil, err
 	}
 	defer release()
 
-	return getLatestCommitRemote(opts)
+	return getLatestCommitRemote(ctx, opts)
 }
 
 // getLatestCommitRemote assumes the caller already holds the transport lock.
-func getLatestCommitRemote(opts ListOptions) (*string, error) {
+func getLatestCommitRemote(ctx context.Context, opts ListOptions) (*string, error) {
 	tmpDir, err := os.MkdirTemp(opts.HomeDir, "git-provider-list-*")
 	if err != nil {
 		return nil, fmt.Errorf("failed to create temporary directory: %w", err)
@@ -272,7 +276,10 @@ func getLatestCommitRemote(opts ListOptions) (*string, error) {
 		return nil, err
 	}
 
-	refs, err := remote.List(&git.ListOptions{
+	ctx, cancel := context.WithTimeout(ctx, remoteListTimeout)
+	defer cancel()
+
+	refs, err := remote.ListContext(ctx, &git.ListOptions{
 		Auth:            opts.Auth,
 		InsecureSkipTLS: opts.Insecure,
 	})
@@ -293,7 +300,7 @@ func restoreUnsupportedCapabilities(oldUnsupportedCaps []capability.Capability)
 	transport.UnsupportedCapabilities = oldUnsupportedCaps
 }
 
-func isInGitCommitHistory(ctx context.Context, opts ListOptions, hash string) (bool, error) {
+func IsInGitCommitHistory(ctx context.Context, opts ListOptions, hash string) (bool, error) {
 	log := contexttools.LoggerFromCtx(ctx, logging.NewNopLogger())
 
 	tmpDir, err := os.MkdirTemp(opts.HomeDir, "git-provider-history-*")
@@ -349,7 +356,7 @@ func isInGitCommitHistory(ctx context.Context, opts ListOptions, hash string) (b
 		}
 	}
 
-	res.repo, err = git.Clone(res.storer, res.fs, &cloneOpts)
+	res.repo, err = git.CloneContext(ctx, res.storer, res.fs, &cloneOpts)
 	if err != nil {
 		if strings.Contains(err.Error(), "couldn't find remote ref") {
 			log.Warn("Branch not found in remote repository", "branch", opts.Branch, "url", opts.URL)
@@ -439,7 +446,7 @@ func IsFuncInGitCommitHistory(ctx context.Context, opts ListOptions, f func(comm
 		}
 	}
 
-	res.repo, err = git.Clone(res.storer, res.fs, &cloneOpts)
+	res.repo, err = git.CloneContext(ctx, res.storer, res.fs, &cloneOpts)
 	if err != nil {
 		if strings.Contains(err.Error(), "couldn't find remote ref") {
 			log.Warn("Branch not found in remote repository", "branch", opts.Branch, "url", opts.URL)
@@ -523,11 +530,7 @@ func (s *Repo) UpdateIndex(idx *IndexOptions) error {
 	}
 	return nil
 }
-func Clone(opts CloneOptions) (*Repo, error) {
-	return clone(context.Background(), opts)
-}
-
-func clone(ctx context.Context, opts CloneOptions) (*Repo, error) {
+func Clone(ctx context.Context, opts CloneOptions) (*Repo, error) {
 	tmpDir, err := os.MkdirTemp(opts.HomeDir, "git-provider-clone-*")
 	if err != nil {
 		return nil, fmt.Errorf("failed to create temporary directory: %w", err)
@@ -583,7 +586,7 @@ func clone(ctx context.Context, opts CloneOptions) (*Repo, error) {
 		InsecureSkipTLS: opts.Insecure,
 	}
 	isOrphan := true
-	_, err = getLatestCommitRemote(ListOptions{
+	_, err = getLatestCommitRemote(ctx, ListOptions{
 		URL:        opts.URL,
 		Auth:       opts.Auth,
 		Insecure:   opts.Insecure,
@@ -607,7 +610,7 @@ func clone(ctx context.Context, opts CloneOptions) (*Repo, error) {
 		}
 		res.isNewBranch = ptr.To(true)
 	}
-	res.repo, err = git.Clone(res.storer, res.fs, &cloneOpts)
+	res.repo, err = git.CloneContext(ctx, res.storer, res.fs, &cloneOpts)
 	if err != nil {
 		return nil, fmt.Errorf("failed to clone repository: %w", normalizeEmptyReasonError(err))
 	}
@@ -620,14 +623,6 @@ func clone(ctx context.Context, opts CloneOptions) (*Repo, error) {
 	return res, err
 }
 
-func IsInGitCommitHistory(opts ListOptions, hash string) (bool, error) {
-	return isInGitCommitHistory(context.Background(), opts, hash)
-}
-
-func IsInGitCommitHistoryContext(ctx context.Context, opts ListOptions, hash string) (bool, error) {
-	return isInGitCommitHistory(ctx, opts, hash)
-}
-
 // Exists stats the local filesystem: it never opens a transport, so it must not
 // touch the global transport registry.
 func (s *Repo) Exists(path string) (bool, error) {
@@ -763,7 +758,7 @@ func (s *Repo) Commit(path, msg string, opt *IndexOptions) (plumbing.Hash, error
 	return hash, nil
 }
 
-func (s *Repo) Push(downstream, branch string, insecure bool) error {
+func (s *Repo) Push(ctx context.Context, downstream, branch string, insecure bool) error {
 	release, err := lockTransport(s.cookie, false)
 	if err != nil {
 		return err
@@ -772,7 +767,7 @@ func (s *Repo) Push(downstream, branch string, insecure bool) error {
 
 	//Push the code to the remote
 	if len(branch) == 0 {
-		return s.repo.Push(&git.PushOptions{
+		return s.repo.PushContext(ctx, &git.PushOptions{
 			RemoteName:      downstream,
 			Auth:            s.auth,
 			InsecureSkipTLS: insecure,
@@ -806,7 +801,7 @@ func (s *Repo) Push(downstream, branch string, insecure bool) error {
 		}
 	}
 
-	err = s.repo.Push(&git.PushOptions{
+	err = s.repo.PushContext(ctx, &git.PushOptions{
 		RemoteName:      downstream,
 		Force:           false,
 		Auth:            s.auth,
@@ -821,7 +816,7 @@ func (s *Repo) Push(downstream, branch string, insecure bool) error {
 	return nil
 }
 
-func Pull(s *Repo, insecure bool) error {
+func Pull(ctx context.Context, s *Repo, insecure bool) error {
 	release, err := lockTransport(s.cookie, false)
 	if err != nil {
 		return err
@@ -834,7 +829,7 @@ func Pull(s *Repo, insecure bool) error {
 		return err
 	}
 
-	err = wt.Pull(&git.PullOptions{
+	err = wt.PullContext(ctx, &git.PullOptions{
 		RemoteName:      "origin",
 		Auth:            s.auth,
 		InsecureSkipTLS: insecure,