Skip to content

Commit 4f04466

Browse files
Paulo Gomesstefanprodan
authored andcommitted
Update source-controller
- Panic recovery for Git operations. - Improved SSH connection management without use of caching. - Enforce context timeout for managed SSH. - Remove dependency to callback functions. - Add support for hashed known_hosts. Signed-off-by: Paulo Gomes <[email protected]>
1 parent 31fe653 commit 4f04466

File tree

3 files changed

+41
-25
lines changed

3 files changed

+41
-25
lines changed

Dockerfile

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ WORKDIR /workspace
7272
COPY main.go main.go
7373
COPY controllers/ controllers/
7474
COPY pkg/ pkg/
75+
COPY internal/ internal/
7576

7677
COPY --from=musl-tool-chain /workspace/build /workspace/build
7778

controllers/imageupdateautomation_controller.go

Lines changed: 39 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -250,31 +250,20 @@ func (r *ImageUpdateAutomationReconciler) Reconcile(ctx context.Context, req ctr
250250
return failWithError(err)
251251
}
252252

253-
repositoryURL := origin.Spec.URL
253+
// managed GIT transport only affects the libgit2 implementation
254254
if managed.Enabled() {
255-
// At present only HTTP connections have the ability to define remote options.
256-
// Although this can be easily extended by ensuring that the fake URL below uses the
257-
// target ssh scheme, and the libgit2/managed/ssh.go pulls that information accordingly.
258-
//
259-
// This is due to the fact the key libgit2 remote callbacks do not take place for HTTP
260-
// whilst most still work for SSH.
261-
if strings.HasPrefix(repositoryURL, "http") {
262-
if access.auth != nil && len(access.auth.CAFile) > 0 {
263-
// Due to the lack of the callback feature, a fake target URL is created to allow
264-
// for the smart sub transport be able to pick the options specific for this
265-
// GitRepository object.
266-
// The URL should use unique information that do not collide in a multi tenant
267-
// deployment.
268-
repositoryURL = fmt.Sprintf("http://%s/%s/%d", auto.Name, auto.UID, auto.Generation)
269-
managed.AddTransportOptions(repositoryURL,
270-
managed.TransportOptions{
271-
TargetURL: repositoryURL,
272-
CABundle: access.auth.CAFile,
273-
})
274-
275-
// We remove the options from memory, to avoid accumulating unused options over time.
276-
defer managed.RemoveTransportOptions(repositoryURL)
277-
}
255+
// We set the TransportOptionsURL of this set of authentication options here by constructing
256+
// a unique URL that won't clash in a multi tenant environment. This unique URL is used by
257+
// libgit2 managed transports. This enables us to bypass the inbuilt credentials callback in
258+
// libgit2, which is inflexible and unstable.
259+
// NB: The Transport Options URL must be unique, therefore it must use the object under
260+
// reconciliation details, instead of the repository it depends on.
261+
if strings.HasPrefix(origin.Spec.URL, "http") {
262+
access.auth.TransportOptionsURL = fmt.Sprintf("http://%s/%s/%d", auto.Name, auto.UID, auto.Generation)
263+
} else if strings.HasPrefix(origin.Spec.URL, "ssh") {
264+
access.auth.TransportOptionsURL = fmt.Sprintf("ssh://%s/%s/%d", auto.Name, auto.UID, auto.Generation)
265+
} else {
266+
return failWithError(fmt.Errorf("git repository URL '%s' has invalid transport type, supported types are: http, https, ssh", origin.Spec.URL))
278267
}
279268
}
280269

@@ -287,6 +276,20 @@ func (r *ImageUpdateAutomationReconciler) Reconcile(ctx context.Context, req ctr
287276
}
288277
defer repo.Free()
289278

279+
if managed.Enabled() {
280+
// Checkout removes TransportOptions before returning, therefore this
281+
// must happen after cloneInto.
282+
// TODO(pjbgf): Git consolidation should improve the API workflow.
283+
managed.AddTransportOptions(access.auth.TransportOptionsURL, managed.TransportOptions{
284+
TargetURL: origin.Spec.URL,
285+
AuthOpts: access.auth,
286+
ProxyOptions: &libgit2.ProxyOptions{Type: libgit2.ProxyTypeAuto},
287+
Context: cloneCtx,
288+
})
289+
290+
defer managed.RemoveTransportOptions(access.auth.TransportOptionsURL)
291+
}
292+
290293
// When there's a push spec, the pushed-to branch is where commits
291294
// shall be made
292295

@@ -732,6 +735,13 @@ var errRemoteBranchMissing = errors.New("remote branch missing")
732735
// switchToBranch switches to a branch after fetching latest from upstream.
733736
// If the branch does not exist, it is created using the head as the starting point.
734737
func switchToBranch(repo *libgit2.Repository, ctx context.Context, branch string, access repoAccess) error {
738+
callbacks := access.remoteCallbacks(ctx)
739+
if managed.Enabled() {
740+
// Override callbacks with dummy ones as they are not needed within Managed Transport.
741+
// However, not setting them may lead to git2go panicing.
742+
callbacks = managed.RemoteCallbacks()
743+
}
744+
735745
branchRef := fmt.Sprintf("origin/%s", branch)
736746
remoteBranch, err := repo.LookupBranch(branchRef, libgit2.BranchRemote)
737747
if err != nil && !libgit2.IsErrorCode(err, libgit2.ErrorCodeNotFound) {
@@ -806,6 +816,11 @@ func push(ctx context.Context, path, branch string, access repoAccess) error {
806816
defer origin.Free()
807817

808818
callbacks := access.remoteCallbacks(ctx)
819+
if managed.Enabled() {
820+
// Override callbacks with dummy ones as they are not needed within Managed Transport.
821+
// However, not setting them may lead to git2go panicing.
822+
callbacks = managed.RemoteCallbacks()
823+
}
809824

810825
// calling repo.Push will succeed even if a reference update is
811826
// rejected; to detect this case, this callback is supplied.

controllers/update_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1593,7 +1593,7 @@ func createSSHIdentitySecret(kClient client.Client, name, namespace, repoURL str
15931593
if err != nil {
15941594
return err
15951595
}
1596-
knownhosts, err := ssh.ScanHostKey(url.Host, 5*time.Second)
1596+
knownhosts, err := ssh.ScanHostKey(url.Host, 5*time.Second, []string{}, false)
15971597
if err != nil {
15981598
return err
15991599
}

0 commit comments

Comments
 (0)