The average SubTransport lifecycle encompass two Actions calls. Previously,
it was attempted to share the same connection across both calls. That did
not work as some Git Servers do not support multiple sessions from the same
connection. The implementation was not fully transitioned into the
"one connection per action" model, which led to connection being leaked.
The transition to RW mutex was to avoid the unnecessary blocking in the
goroutine at the start of the second action call.
It is worth mentioning that now when the context is done, the client level
resources (connection) will also be freed. This ensures that SSH connections
will not outlive the subtransport.
Signed-off-by: Paulo Gomes <paulo.gomes@weave.works>
Git repositories may be redirected to different URLs
when they are accessed via HTTP. The two most obvious
scenarios are from HTTP to HTTPS and when the .git suffix
is missing.
By improving the logging on this process users can identify
changes required to their GitRepository objects.
Signed-off-by: Paulo Gomes <paulo.gomes@weave.works>
Earlier, host key verification could potentially fail if there were
multiple entries in the known_hosts file and if the intended encryption
algorithm wasn't the first entry. This happened because we used the same
hasher object to compute the sum of all the public keys present in the
known_hosts file, which led to invalid hashes, resulting in a mismatch
when compared with the hash of the advertised public key. This is fixed,
by not creating the hasher ourselves and instead delegating that to the
function actually doing the matching, ensuring that a new hasher is used
for each comparison.
Regression introduced in v0.25.0 and reported in
https://github.com/fluxcd/image-automation-controller/issues/378
Signed-off-by: Sanskar Jaiswal <jaiswalsanskar078@gmail.com>
Gitlab only supports HTTP redirection for GET operations,
and fails POST operations targeting a repository without
the .git suffix.
Fixes: https://github.com/fluxcd/image-automation-controller/issues/379
Signed-off-by: Paulo Gomes <paulo.gomes@weave.works>
Race conditions in ssh smart subtransport caused some goroutines to
panic, resulting in crashing the whole controller, mostly evident in
image-automation-controller CI runs. Panic recovery in the main thread
do not handle goroutine panics. So, the existing panic recovery code in
libgit2 Checkout() methods weren't able to handle it.
This change groups the fields in ssh smart subtransport that may be
accessed by multiple goroutines into a new struct with a mutex. Also
adds panic recovery in the created goroutine to handle any other
possible panics.
Signed-off-by: Sunny <darkowlzz@protonmail.com>
Some scenarios could lead a goroutine to be running indefinetely within managed ssh.
Previously between the two git operations, the reconciliation
could take twice the timeout set for the Flux object.
Signed-off-by: Paulo Gomes <paulo.gomes@weave.works>
Injects transport and auth options at the transport level directly to
bypass the inbuilt credentials callback because of it's several
shortcomings. Moves some of the pre-existing logic from the reconciler
to the checkout implementation.
Signed-off-by: Sanskar Jaiswal <jaiswalsanskar078@gmail.com>
Connection caching was a feature created to resolve
upstream issues raised from concurrent ssh connections.
Some scenarios were based on multiple key exchange
operations happening at the same time.
This PR removes the connection caching, and instead:
- Services Session.StdoutPipe() as soon as possible,
as it is a known source of blocking SSH connections.
- Reuse SSH connection within the same subtransport,
eliminating the need for new handshakes when talking
with the same server.
- Simplifies the entire transport logic for better
maintainability.
Signed-off-by: Paulo Gomes <paulo.gomes@weave.works>
Enables the setting of HostKey algorithms to be used from
a client perspective. This implementation supports go-git
and libgit2 when in ManagedTransport.
Signed-off-by: Paulo Gomes <paulo.gomes@weave.works>
This commit replaces `os.MkdirTemp` with `t.TempDir` in tests. The
directory created by `t.TempDir` is automatically removed when the test
and all its subtests complete.
Prior to this commit, temporary directory created using `os.MkdirTemp`
needs to be removed manually by calling `os.RemoveAll`, which is omitted
in some tests. The error handling boilerplate e.g.
defer func() {
if err := os.RemoveAll(dir); err != nil {
t.Fatal(err)
}
}
is also tedious, but `t.TempDir` handles this for us nicely.
Reference: https://pkg.go.dev/testing#T.TempDir
Signed-off-by: Eng Zer Jun <engzerjun@gmail.com>
Previously the mutex.Lock was acquired before creating
a new connection. The lock would then hold until the
process was finished, and all network latency would be
absorbed by other goroutines trying to establish a new
connection.
Now the lock is acquired after the connection has been
created. The downside of this approach is that concurrent
goroutine may be trying to open a connection to the same
target. The loser in the race will then have to Close the
connection and use the winner's instead.
Signed-off-by: Paulo Gomes <paulo.gomes@weave.works>
Cached connections can be shared across concurrent
operations, and their disposal must take that into
account to avoid closing a connection that is stale for
one goroutine, but is still valid for another.
Signed-off-by: Paulo Gomes <paulo.gomes@weave.works>
Avoid asking for SSH credential in files, as they won't be
used. The cacheKeyAndConfig func already enforces this
behaviour.
Signed-off-by: Paulo Gomes <paulo.gomes@weave.works>
The major Git SaaS providers have repository URLs
for both HTTP and SSH that tops around 250
characters in length.
The limits chosen were a lot higher to align with use
cases in which users may have on-premise servers with
long domain names and paths.
For SSH the validation is around path length only,
which is now limited to 4096 characters, which is
at the higher end of the range in Linux.
For HTTP the validation is around the full URL
provided by the caller.
Signed-off-by: Paulo Gomes <paulo.gomes@weave.works>
Internal and upstream calls to sshSmartSubtransport.Close()
when dealing with an stale connection, may lead to misleading
errors.
Focus should instead be redirected to ensuring that Close()
releases resources and ensures that a new SubTransport can be
created, so new operations can succeed.
Signed-off-by: Paulo Gomes <paulo.gomes@weave.works>
SSH servers that block the reuse of SSH connections for
multiple SSH sessions may lead to EOF when a new session
is being created.
This fixes the issue of long-running connections resulting
in EOF for GitLab servers.
Signed-off-by: Paulo Gomes <paulo.gomes@weave.works>
Adds a flag `ssh-kex-algos` which configures the gogit and libgit2
managed clients to use the specified list of kex algos for ssh. If not
used the default list in `golang/x/crypto/ssh` is used.
Signed-off-by: Sanskar Jaiswal <jaiswalsanskar078@gmail.com>
All errors that were previously not handled are now logged through
traceLog, to further help during transport investigations.
Signed-off-by: Paulo Gomes <paulo.gomes@weave.works>
Ensure all requests are completely processed and closed,
to prove odds of the underlying connections to be reused.
The transport now is pooled and reused whenever possible.
Signed-off-by: Paulo Gomes <paulo.gomes@weave.works>
For backwards compatibility, support for HTTP redirection is enabled when targeting
the same host, and no TLS downgrade took place.
Signed-off-by: Paulo Gomes <paulo.gomes@weave.works>
The initial implementation was based off upstream, which cause
an initial request to fail, and only then the credentials would
be added into the request.
Signed-off-by: Paulo Gomes <paulo.gomes@weave.works>
libgit2 network operations are blocking and do not provide timeout nor context capabilities,
leading for several reports by users of the controllers hanging indefinitely.
By using managed transport, golang primitives such as http.Transport and net.Dial can be used
to ensure timeouts are enforced.
Co-Authored-by: Sunny <darkowlzz@protonmail.com>
Signed-off-by: Paulo Gomes <paulo.gomes@weave.works>
Sanskar Jaiswal
Paulo Gomes
Sunny
Eng Zer Jun