Remove go-getter from GetContent and switch Docker image to bci-busybox#4841
Conversation
There was a problem hiding this comment.
Pull request overview
This PR replaces go-getter’s default git implementation (which shells out to the git binary and relied on global env vars) with a custom go-git–based getter, and adds/updates integration test infrastructure to validate Git content fetching over HTTPS (custom CA / insecure) and SSH.
Changes:
- Replace go-getter’s default
GitGetter/HttpGetterwith custom implementations (including a new go-git–based git getter). - Remove mutex/env-var based synchronization previously used for git-over-HTTPS TLS configuration.
- Add Gogs HTTPS testcontainer utilities and new bundlereader integration tests for
GetContent(HTTPS + SSH scenarios).
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/bundlereader/loaddirectory.go | Swaps in custom getters and simplifies the get() wrapper now that git is handled by go-git. |
| internal/bundlereader/loaddirectory_internal_test.go | Replaces mutex/env-var tests with unit tests for the new go-git getter detection behavior. |
| internal/bundlereader/gitgetter.go | New go-git–based getter.Getter implementation (clone, auth, ref handling). |
| integrationtests/utils/gogs.go | New shared helper for starting/configuring a Gogs container with HTTPS + CA extraction. |
| integrationtests/gitjob/git/git_test.go | Reuses shared SSH key generation helper and minor error formatting cleanup. |
| integrationtests/gitcloner/clone_test.go | Refactors Gogs setup/URL/key helpers into integration test utils. |
| integrationtests/bundlereader/suite_test.go | Adds Ginkgo suite bootstrap for bundlereader integration tests. |
| integrationtests/bundlereader/getcontent_test.go | New integration tests for bundlereader.GetContent over HTTPS (CA/insecure/auth/ref/subdir) and SSH. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
There was a problem hiding this comment.
Pull request overview
This PR replaces go-getter’s default Git implementation (which shells out to the git binary and previously required process-wide env var/mutex handling for TLS tweaks) with a custom go-git–based getter, and adds/updates tests to validate HTTPS/SSH cloning behavior (including known_hosts support).
Changes:
- Replace go-getter’s
GitGetterwith a new internalgoGitGetterimplementation (go-git based). - Extend
AuthwithSSHKnownHostssupport and wire it into SSH cloning. - Refactor/add integration test utilities for Gogs and introduce new integration coverage for
bundlereader.GetContent.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/bundlereader/loaddirectory.go | Switches go-getter to use custom HTTP + go-git getters; removes env var/mutex-based git-over-HTTPS handling. |
| internal/bundlereader/loaddirectory_internal_test.go | Replaces mutex/env-var tests with goGitGetter.Detect unit tests. |
| internal/bundlereader/gitgetter.go | Adds the new go-git–based getter, query-param parsing, and SSH/HTTPS auth handling. |
| internal/bundlereader/auth.go | Adds SSHKnownHosts and includes it in Auth.Hash(). |
| integrationtests/utils/gogs.go | New shared helpers for starting/configuring a Gogs container (HTTPS + SSH). |
| integrationtests/gitjob/git/git_test.go | Uses shared SSH key generation helper; minor error construction cleanup. |
| integrationtests/gitcloner/clone_test.go | Refactors to shared Gogs helpers; removes duplicated container/key/tempdir helpers. |
| integrationtests/bundlereader/suite_test.go | Adds a Ginkgo suite for bundlereader integration tests. |
| integrationtests/bundlereader/getcontent_test.go | New integration tests covering GetContent via HTTPS (CA/insecure) and SSH (key + known_hosts). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
2e91d63 to
e8b712b
Compare
e8b712b to
040335a
Compare
There was a problem hiding this comment.
Pull request overview
This PR removes the go-getter-based git cloning path from bundlereader.GetContent and replaces it with a go-git implementation, aiming to eliminate process-global TLS env-var hacks and improve SSH host-key validation behavior.
Changes:
- Replaces
go-getterfetching inbundlereaderwith explicitgit/http/localfetch implementations (including shorthand URL detection and//subdirhandling). - Adds a shared SSH auth helper that supports strict host-key checks when
known_hostsdata is provided. - Refactors/extends integration tests and removes
go-getterdependencies from module files.
Reviewed changes
Copilot reviewed 19 out of 21 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/git/netutils.go | Switches SSH auth construction to a helper that can apply known_hosts validation. |
| internal/ssh/auth.go | Adds NewSSHPublicKeys helper to centralize SSH auth + host-key callback selection. |
| internal/cmd/cli/gitcloner/cloner.go | Introduces custom-CA handling via go-git protocol registry override. |
| internal/bundlereader/source.go | Adds parsing/detection logic for sources (forced schemes, shorthands, //subdir). |
| internal/bundlereader/loaddirectory.go | Replaces go-getter-based retrieval with fetchToDir + scheme-specific download logic. |
| internal/bundlereader/loaddirectory_test.go | Updates unit tests (TempDir usage; removes sshkey query-param behavior test). |
| internal/bundlereader/loaddirectory_internal_test.go | Replaces mutex/env-var tests with parser/query-param/safeJoin unit tests. |
| internal/bundlereader/httpdownload.go | Adds direct HTTP downloader and archive extraction utilities. |
| internal/bundlereader/gitclone.go | Adds go-git clone implementation with support for ref, sshkey, depth, and TLS options. |
| internal/bundlereader/gitclone_test.go | Adds tests around exclusive CA-bundle behavior for HTTPS git cloning. |
| internal/bundlereader/auth.go | Extends Auth with SSHKnownHosts and includes it in hashing. |
| integrationtests/utils/gogs.go | Consolidates Gogs helpers (HTTPS/SSH URLs, keypair creation, known_hosts entry). |
| integrationtests/gitjob/git/git_test.go | Uses consolidated Gogs SSH keypair helper; minor error handling cleanup. |
| integrationtests/gitcloner/clone_test.go | Refactors to shared Gogs utilities; reduces duplicated container/key code. |
| integrationtests/bundlereader/suite_test.go | Adds a Ginkgo suite for bundlereader integration tests. |
| integrationtests/bundlereader/getcontent_test.go | Adds integration coverage for GetContent via HTTPS/SSH, CA bundles, refs, and subdirs. |
| go.mod / go.sum | Removes github.com/hashicorp/go-getter/v2 (and related transitive deps) from main module. |
| integrationtests/go.mod / integrationtests/go.sum | Removes go-getter-related dependencies from integration test module. |
| e2e/single-cluster/go_getter_custom_ca_test.go | Adjusts in-repo chart reference to use //subdir notation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
040335a to
1291daf
Compare
There was a problem hiding this comment.
Pull request overview
This PR replaces bundlereader.GetContent’s go-getter based git fetching (which depended on the system git binary + process-global env vars) with a go-git implementation, removing the global clone mutex/env-var hacks and improving SSH host key validation.
Changes:
- Reworked
GetContentfetching to use new scheme detection +gitDownload/httpDownloadimplementations (go-git + net/http), including support for?ref=,?sshkey=,?depth=, and//subdir. - Centralized SSH auth creation to support strict known_hosts validation when provided.
- Consolidated gogs integration-test helpers and removed go-getter dependencies from modules/sums.
Reviewed changes
Copilot reviewed 19 out of 21 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/git/netutils.go | Switches SSH auth creation to shared helper that respects provided known_hosts. |
| internal/ssh/auth.go | Adds helper to build go-git SSH auth with optional known_hosts enforcement. |
| internal/cmd/cli/gitcloner/cloner.go | Introduces custom-CA HTTPS protocol installation guarded by RWMutex; reuses new SSH auth helper. |
| internal/bundlereader/source.go | Adds source parsing/shorthand detection and //subdir parsing. |
| internal/bundlereader/loaddirectory.go | Replaces go-getter usage with fetchToDir + new git/http/local fetching and directory copying. |
| internal/bundlereader/loaddirectory_test.go | Updates temp dir usage and removes go-getter/sshkey URL param test. |
| internal/bundlereader/loaddirectory_internal_test.go | Replaces old mutex/env-var tests with parser/query-param/safeJoin unit tests. |
| internal/bundlereader/httpdownload.go | Adds HTTP download + archive extraction implementation. |
| internal/bundlereader/gitclone.go | Adds go-git based clone with TLS/CA handling and ref/sshkey/depth query support. |
| internal/bundlereader/gitclone_test.go | Adds unit test ensuring exclusive CA bundle behavior for HTTPS clones. |
| internal/bundlereader/auth.go | Extends auth hash inputs to include SSHKnownHosts. |
| integrationtests/utils/gogs.go | Consolidates gogs container/keypair/known_hosts helpers for integration tests. |
| integrationtests/go.mod | Removes indirect go-getter-related dependencies from integration test module. |
| integrationtests/go.sum | Removes go-getter-related checksums from integration test module. |
| integrationtests/gitjob/git/git_test.go | Uses consolidated SSH keypair helper; minor error construction tweak. |
| integrationtests/gitcloner/clone_test.go | Uses consolidated gogs helpers and keypair generator; removes duplicated container setup. |
| integrationtests/bundlereader/suite_test.go | Adds Ginkgo suite entry point for bundlereader integration tests. |
| integrationtests/bundlereader/getcontent_test.go | Adds integration tests for GetContent against HTTPS/SSH gogs (CA bundle, ref, subdir, known_hosts). |
| go.mod | Removes go-getter dependency from main module. |
| go.sum | Removes go-getter-related checksums from main module. |
| e2e/single-cluster/go_getter_custom_ca_test.go | Updates git URL usage in fleet.yaml to use //subdir semantics. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
c5ab803 to
a8b2fc5
Compare
There was a problem hiding this comment.
Pull request overview
This PR removes the go-getter dependency from internal/bundlereader.GetContent and replaces it with native implementations based on go-git (git sources) and the Go stdlib HTTP stack (HTTP archives/files), while preserving Fleet-specific URL semantics like ?ref=, ?sshkey=, ?depth=, and //subdir.
Changes:
- Implemented source parsing/shorthand detection and scheme-specific fetching (
git,http,local) for bundle content. - Added go-git cloning with query-param handling and coordinated custom-CA HTTPS transport installation via RWMutex.
- Updated unit/integration/e2e tests and removed
go-getter(and related transitive deps) fromgo.mod/go.sum.
Reviewed changes
Copilot reviewed 21 out of 23 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/git/netutils.go | Switch SSH auth construction to centralized helper and prefer secret known_hosts with fallback. |
| internal/ssh/auth.go | Add helper to build go-git SSH auth and optionally enforce known_hosts. |
| internal/cmd/cli/gitcloner/cloner.go | Add exclusive custom-CA HTTPS transport installation via go-git protocol registry. |
| internal/cmd/cli/gitcloner/cloner_test.go | Add tests validating custom-CA install/restoration behavior. |
| internal/bundlereader/source.go | New source parsing: forced schemes, //subdir, and shorthand detection. |
| internal/bundlereader/loaddirectory.go | Replace go-getter usage with fetchToDir + scheme-specific download/copy logic. |
| internal/bundlereader/loaddirectory_test.go | Adjust tests for new content-fetch path and tempdir handling. |
| internal/bundlereader/loaddirectory_internal_test.go | Replace mutex/env-var tests with parser/query/subdir/safe-join tests. |
| internal/bundlereader/httpdownload.go | New HTTP downloader and archive extraction with path-safety checks. |
| internal/bundlereader/httpdownload_internal_test.go | Add extraction-focused internal tests (unsupported tar types, symlink traversal, zip streaming). |
| internal/bundlereader/gitclone.go | New go-git clone implementation with ref/sshkey/depth parsing and custom CA handling. |
| internal/bundlereader/gitclone_test.go | Add tests for CA bundle behavior and protocol restoration. |
| internal/bundlereader/auth.go | Extend bundlereader Auth with SSHKnownHosts and include it in hashing. |
| integrationtests/utils/gogs.go | Add shared Gogs HTTPS/SSH + cert/known_hosts utilities for integration tests. |
| integrationtests/go.sum | Remove go-getter-related transitive sums from integration tests module. |
| integrationtests/go.mod | Remove go-getter-related transitive requirements from integration tests module. |
| integrationtests/gitjob/git/git_test.go | Reuse shared SSH keypair helper; simplify error creation. |
| integrationtests/gitcloner/clone_test.go | Reuse Gogs utilities; add integration regression test for CA transport restoration. |
| integrationtests/bundlereader/suite_test.go | Add new Ginkgo suite for bundlereader integration tests. |
| integrationtests/bundlereader/getcontent_test.go | Add integration coverage for GetContent git HTTPS/SSH, ref, and subdir behavior. |
| go.sum | Remove go-getter-related transitive sums from main module. |
| go.mod | Remove go-getter dependency and related transitive requirements from main module. |
| e2e/single-cluster/go_getter_custom_ca_test.go | Adjust test chart URL to use //subdir path form. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
fe0bef1 to
60da5fe
Compare
|
However, Bitbucket dropped Mercurial support in 2020, so all Bitbucket repositories are git today. The network request is no longer needed to determine the VCS type, which means a Since this PR aims to preserve go-getter URL semantics, omitting Bitbucket shorthand is a silent regression: any |
|
One more compatibility note on the HTTP archive path in This is probably fine for the majority of users, but worth considering before merging: an HTTP source pointing at a If re-adding the formats is not in scope, it would be good to document the supported extensions so users are not caught off guard during an upgrade. |
|
|
|
go-getter stripped |
8634593 to
71604e2
Compare
I have added the checksum verification except that we do not support the referencing checksum files because this would add quite some complexity without much gain. We can add this in a later pr if explicitly requested. |
71604e2 to
1ef32e8
Compare
Move the gogs container setup, URL helpers, and SSH key generation out of gitcloner/clone_test.go and gitjob/git/git_test.go into a shared integrationtests/utils/gogs.go package. The helpers are used by both test suites; centralising them removes the duplication and makes them available for the bundlereader integration tests added in the next commit.
Add integration tests exercising GetContent for a range of scenarios that were supported before the go-getter removal: HTTPS with InsecureSkipVerify, no-TLS certificate rejection, private repos with embedded credentials, branch selection via ?ref=, subdir extraction via //, and plain SSH with a private key.
Centralise SSH private-key loading and public-key auth creation into internal/ssh so both bundlereader and gitcloner share a single implementation. pkg/git/netutils.go gains SSHPublicKeysFromFile for callers in pkg/git that already use a file path directly. Prefer known_hosts from the secret; fall back to the cluster-wide value. Add unit tests for NewSSHPublicKeys.
Remove the go-getter dependency from bundlereader and replace its use with go-git for git sources and stdlib HTTP for HTTP archive extraction. CA bundles augment the system pool rather than replacing it; go-git creates an isolated transport per clone so concurrent clones with different CAs need no mutex. SCP-style SSH sources accept any username, and forced-scheme prefixes (git::, ssh::) are normalised to proper URLs. Source shorthands cover GitHub, GitLab, and Bitbucket. HTTP archives (tar.gz, tar.bz2, tar.xz, tar.zst, tar, gz, bz2, xz, zst, zip) are extracted inline; ?checksum=<type>:<hex> verifies the body via a TeeReader; ?archive= overrides extension detection. Symlink and path traversal in tar and zip archives are rejected. Error messages redact any embedded credentials in clone URLs.
Pass FLEET_KNOWN_HOSTS and FLEET_INSECURE_SKIP_TLS_VERIFY through to the bundlereader.Auth so that fleet apply CLI and the GitOps reconciler honour the same TLS and SSH host-key settings as the rest of the pipeline. Default the SSH username to 'git' when the URL has no user component, matching the controller behaviour. Add tests for createAuthFromOpts and addAuthToOpts, and update the gitjob test to assert that InsecureSkipTLSverify is forwarded.
Fall back to a transport with stdlib-matching defaults if DefaultTransport is not a *http.Transport, to avoid a panic when another component has replaced the global default. The fallback sets Proxy: http.ProxyFromEnvironment, 30s dial/keep-alive, and the standard idle/TLS/expect-continue timeouts so runtime behaviour stays close to the baseline.
go-git's HTTP transport does not accept *ssh.PublicKeys, so applying SSH auth to an https:// URL causes the clone to fail with a confusing transport error. Guard the SSH auth path with an isSSH check. If ?sshkey= is explicitly provided for an HTTP(S) URL, return a clear error. If Auth.SSHPrivateKey is set alongside an HTTP(S) URL, silently ignore it and fall through to HTTP basic auth.
1ef32e8 to
c73552b
Compare
Remove openssh-clients and git-core: all git and SSH operations use go-git and golang.org/x/crypto/ssh; no system git or ssh binary is called anywhere in the controller. Switch the base image from bci-base to bci-busybox, matching the agent image. Drop useradd; bci-busybox lacks useradd, and USER 1000 without prior user-creation is already the pattern used by Dockerfile.agent.
bundlereader.GetContent previously used
github.com/hashicorp/go-getterfor git and HTTP fetches, which shelled out to thegitbinary and controlled TLS only through process-global environment variables. This PR removes go-getter from go.mod entirely and replaces all fetching with native go-git and stdlib HTTP, preserving Fleet's?ref=,?sshkey=,?depth=, and//subdirsemantics.SSHKnownHostsis provided; previously all SSH clones usedInsecureIgnoreHostKeyunconditionally.CABundleis applied per-clone viaCloneOptions.CABundle. go-git creates an isolated transport per session, so concurrent clones with different CAs are fully independent — no mutex needed.setGitAuthnow only applies SSH auth when the URL scheme issshorgit. An HTTPS URL combined with?sshkey=returns a clear error instead of failing with a confusing transport error.extractQueryParamspreserves the original byte encoding and ordering of non-Fleet query parameters instead of round-tripping through url.Values, so presigned URLs are not corrupted.tar.gz/tgz,tar.bz2/tbz2,tar.zst/tzst/tar.zstd/zstd,tar.xz/txz,tar, gz,bz2,zst,xz, andzip. Unknown extensions are written as plain files.?checksum=<type>:<hex>verifies the body viaTeeReaderwithout buffering. Supported types:md5,sha1,sha256,sha512. Thefilevariant is not supported — it would require a recursive HTTP fetch with no meaningful security benefit over an inline hash.?archive=<ext>overrides format detection for URLs with no recognisable extension. Both ?checksum= and?archive=are stripped before the upstream request is made.ssh://URLs.addAuthToOptsinfleet applynow readsFLEET_KNOWN_HOSTSdirectly into opts.Auth.SSHKnownHosts instead of writing it to a temp file and mutatingGIT_SSH_COMMAND.transportForAuth(Helm chart download) guards the http.DefaultTransport.(*http.Transport) assertion to avoid a panic when another component has replaced the global default.bci-busyboximage with static tini. (before:377 MB- after:220 MB)Known difference to go-getter implementation
checksum=file:http://example.com/sha.txt) because it would add quite some complexity. Could be done in a later pr if there is relevant interest.Additional go modules
Refers #4877