diff --git a/CLAUDE.md b/CLAUDE.md index ce2b18d5..420bb80d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -60,7 +60,7 @@ pkg/gitprovider/ — Per-provider adapters: github.go, gitlab.go, bitb Factory: NewProvider() dispatches on GitServer.Spec.GitProvider pkg/client/jira/ — Jira REST client (go-jira wrapper + mockable Client interface) pkg/gerrit/ — SSH-based Gerrit client -pkg/git/ — go-git wrapper + factory (supports SSH/HTTPS auth) +pkg/git/ — go-git wrapper + factory (supports SSH/HTTPS auth); Clone and Push run the git binary (cli.go) unless GIT_CLI_ENABLED=false pkg/webhook/ — Admission webhooks: Codebase and CodebaseBranch validation pkg/objectmodifier/ — Applies default values / label patches before reconcile logic pkg/util/ — Shared helpers (workdir, timeouts, URL utilities, templates) @@ -100,6 +100,7 @@ Deletion runs a shorter chain: `DeleteWebHook` → `Cleaner`. - `WATCH_NAMESPACE` — namespace the operator watches (required) - `CODEBASE_BRANCH_MAX_CONCURRENT_RECONCILES` — concurrency for CodebaseBranch reconciler (default 1) - `ENABLE_WEBHOOKS` — set to `false` to disable admission webhooks +- `GIT_CLI_ENABLED` — set to `false` to clone and push with go-git instead of the git binary - `TELEMETRY_ENABLED` / `TELEMETRY_DELAY` — opt-in usage telemetry ### Adding a new chain handler diff --git a/Dockerfile b/Dockerfile index 40858f09..574ac0dd 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,4 @@ -FROM alpine:3.18.12 +FROM alpine:3.24 ARG TARGETARCH @@ -9,9 +9,9 @@ ENV ASSETS_DIR=/usr/local/bin \ USER_NAME=codebase-operator \ USER_UID=1001 -RUN apk add --no-cache ca-certificates=20241121-r1 \ - openssh-client==9.3_p2-r3 \ - git==2.40.4-r0 +# pkg/git/cli.go runs git and ssh. The ~= pins match the minors the FROM image ships: bump them with the FROM tag. +RUN apk upgrade --no-cache && \ + apk add --no-cache git~=2.54 openssh-client~=10.3 RUN adduser -h ${HOME} -s /bin/ash -D -u ${USER_UID} codebase-operator diff --git a/cmd/main.go b/cmd/main.go index 5e12df07..cac19044 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -5,6 +5,8 @@ import ( "crypto/tls" "flag" "os" + "os/exec" + "os/user" "path/filepath" "strconv" "time" @@ -393,6 +395,8 @@ func main() { setupLog.Info("Stale branch checker is disabled", "env", branchStaleCheckIntervalEnv) } + checkGitEngine() + setupLog.Info("starting manager") ctx := ctrl.SetupSignalHandler() @@ -408,6 +412,38 @@ func main() { } } +// checkGitEngine logs what clones and pushes repositories and exits when it is the git binary and git is missing. +func checkGitEngine() { + if value, ok := os.LookupEnv(gitproviderv2.CLIEnabledEnv); ok { + if _, err := strconv.ParseBool(value); err != nil { + setupLog.Error(err, "Invalid value, clone and push keep the git binary", "env", gitproviderv2.CLIEnabledEnv) + } + } + + if !gitproviderv2.CLIEnabled() { + setupLog.Info("Clone and push run go-git", "env", gitproviderv2.CLIEnabledEnv) + return + } + + if _, err := exec.LookPath("git"); err != nil { + setupLog.Error(err, "Clone and push need the git binary; set env to false to use go-git", + "env", gitproviderv2.CLIEnabledEnv) + os.Exit(1) + } + + if _, err := exec.LookPath("ssh"); err != nil { + setupLog.Error(err, "Clone and push over SSH need the ssh binary", "env", gitproviderv2.CLIEnabledEnv) + } + + // ssh exits when its UID has no passwd entry; the image's entrypoint adds one for a process in group 0. + if _, err := user.LookupId(strconv.Itoa(os.Getuid())); err != nil { + setupLog.Error(err, "Clone and push over SSH will fail: run the pod with group 0, or set env to false", + "env", gitproviderv2.CLIEnabledEnv) + } + + setupLog.Info("Clone and push run the git binary", "env", gitproviderv2.CLIEnabledEnv) +} + func getMaxConcurrentReconciles(envVar string) int { val, exists := os.LookupEnv(envVar) if !exists { diff --git a/deploy-templates/Chart.yaml b/deploy-templates/Chart.yaml index 45358132..802bf110 100644 --- a/deploy-templates/Chart.yaml +++ b/deploy-templates/Chart.yaml @@ -35,8 +35,8 @@ annotations: artifacthub.io/images: | - name: codebase-operator:2.36.0 image: epamedp/codebase-operator:2.36.0 - - name: alpine:3.18.12 - image: alpine:3.18.12 + - name: alpine:3.24 + image: alpine:3.24 artifacthub.io/operatorCapabilities: Full Lifecycle artifacthub.io/crds: | - kind: CDStageDeploy diff --git a/deploy-templates/README.md b/deploy-templates/README.md index 1ffd7452..fc680433 100644 --- a/deploy-templates/README.md +++ b/deploy-templates/README.md @@ -30,6 +30,7 @@ A Helm chart for KubeRocketCI Codebase Operator | envs[0].name | string | `"RECONCILATION_PERIOD"` | | | envs[0].value | string | `"360"` | | | envs[1] | object | `{"name":"CODEBASE_BRANCH_MAX_CONCURRENT_RECONCILES","value":"3"}` | Maximum number of parallel reconciliation codebasebranches | +| gitCliEnabled | bool | `true` | Clone and push repositories with the git binary. Over SSH the pod's UID needs a passwd entry, which the image adds for any UID in group 0, as OpenShift assigns. Set to false to use go-git, which clones and pushes inside the operator's process: onboarding a large repository can then exceed the memory limit. | | global.gatewayApi | object | `{"gatewayName":"main-gateway","gatewayNamespace":"envoy-gateway-system"}` | Gateway API parent Gateway that EventListener HTTPRoutes attach to (used when ingressController=envoy) | | global.gatewayApi.gatewayName | string | `"main-gateway"` | Name of the parent Gateway resource | | global.gatewayApi.gatewayNamespace | string | `"envoy-gateway-system"` | Namespace of the parent Gateway resource | diff --git a/deploy-templates/templates/deployment.yaml b/deploy-templates/templates/deployment.yaml index 26fa26b8..e9049209 100644 --- a/deploy-templates/templates/deployment.yaml +++ b/deploy-templates/templates/deployment.yaml @@ -89,6 +89,8 @@ spec: value: {{ .Values.enableWebhooks | quote }} - name: BRANCH_STALE_CHECK_INTERVAL value: {{ .Values.branchStaleCheckInterval | quote }} + - name: GIT_CLI_ENABLED + value: {{ .Values.gitCliEnabled | quote }} - name: SSH_KNOWN_HOSTS value: /etc/codebase-operator/ssh/ssh_known_hosts {{- if .Values.caCerts.enabled }} diff --git a/deploy-templates/values.yaml b/deploy-templates/values.yaml index 2e9fc015..b5682a3f 100644 --- a/deploy-templates/values.yaml +++ b/deploy-templates/values.yaml @@ -89,6 +89,12 @@ enableWebhooks: true # Accepts Go duration strings (e.g. 24h, 30m); "0" disables the check. branchStaleCheckInterval: 24h +# -- Clone and push repositories with the git binary. Over SSH the pod's UID needs a passwd +# entry, which the image adds for any UID in group 0, as OpenShift assigns. Set to false to +# use go-git, which clones and pushes inside the operator's process: onboarding a large +# repository can then exceed the memory limit. +gitCliEnabled: true + # SSH host key verification. Every SSH connection the operator makes is verified # against these entries; there is no way to disable verification. GitServers that # authenticate with a token over HTTPS are unaffected. diff --git a/pkg/git/checkout_remote_test.go b/pkg/git/checkout_remote_test.go index 6c0e71fe..81c6342a 100644 --- a/pkg/git/checkout_remote_test.go +++ b/pkg/git/checkout_remote_test.go @@ -74,7 +74,7 @@ func workdirHead(t *testing.T, workDir string) *plumbing.Reference { } // A branch pushed to origin after the workdir was cloned is fetched into -// refs/heads by Checkout's refs/*:refs/* refspec; the existence check must look +// refs/heads by Checkout's refs/heads/* refspec; the existence check must look // there, or the checkout collides on Create with the ref the fetch just wrote. func TestCheckout_RemoteBranchCreatedAfterClone(t *testing.T) { originDir, workDir := buildOriginWithClone(t) @@ -123,6 +123,45 @@ func TestCheckout_BranchAbsentEverywhere(t *testing.T) { require.Equal(t, "brand-new", workdirHead(t, workDir).Name().Short()) } +// The checkout fetches take branches and tags only: a pull request ref, which +// refs/* would match, stays on the remote. +func TestCheckout_FetchesOnlyBranchesAndTags(t *testing.T) { + tests := []struct { + name string + checkout func(ctx context.Context, gp *GitProvider, workDir string) error + }{ + { + name: "Checkout with remote", + checkout: func(ctx context.Context, gp *GitProvider, workDir string) error { + return gp.Checkout(ctx, workDir, "late-branch", true) + }, + }, + { + name: "CheckoutRemoteBranch", + checkout: func(ctx context.Context, gp *GitProvider, workDir string) error { + return gp.CheckoutRemoteBranch(ctx, workDir, "late-branch") + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + originDir, workDir := buildOriginWithClone(t) + seedCommit(t, originDir, "pull-request", "+refs/heads/master:refs/pull/1/head") + seedCommit(t, originDir, "late", "+refs/heads/master:refs/heads/late-branch") + + require.NoError(t, tt.checkout(t.Context(), NewGitProvider(Config{}), workDir)) + require.Equal(t, "late-branch", workdirHead(t, workDir).Name().Short()) + + repo, err := gogit.PlainOpen(workDir) + require.NoError(t, err) + + _, err = repo.Reference("refs/pull/1/head", false) + require.ErrorIs(t, err, plumbing.ErrReferenceNotFound) + }) + } +} + // TestCheckout_ForcePushedRemoteBranch: a rewritten upstream branch must not // fail the fetch of a cached workdir; the checkout lands on the new tip. func TestCheckout_ForcePushedRemoteBranch(t *testing.T) { diff --git a/pkg/git/cli.go b/pkg/git/cli.go new file mode 100644 index 00000000..1b1acfbe --- /dev/null +++ b/pkg/git/cli.go @@ -0,0 +1,430 @@ +package v2 + +import ( + "bytes" + "context" + "crypto/ed25519" + "crypto/x509" + "encoding/pem" + "errors" + "fmt" + "net/url" + "os" + "os/exec" + "path/filepath" + "slices" + "strconv" + "strings" + "time" + + "github.com/go-git/go-git/v5" + "github.com/go-git/go-git/v5/plumbing" + "github.com/go-git/go-git/v5/plumbing/storer" + "github.com/go-git/go-git/v5/plumbing/transport" + ssh2 "golang.org/x/crypto/ssh" + ctrl "sigs.k8s.io/controller-runtime" + + "github.com/epam/edp-codebase-operator/v2/pkg/sshhostkey" +) + +const ( + // maxErrorOutput bounds the git output an error carries into the Codebase status. + maxErrorOutput = 2048 + + // The image's CA bundle and directory, where Go looks when SSL_CERT_FILE or SSL_CERT_DIR is unset. + systemCertFile = "/etc/ssl/certs/ca-certificates.crt" + systemCertDir = "/etc/ssl/certs" +) + +// credentialHelper answers git's credential requests with the token for KRCI_GIT_REMOTE, a scheme and host, +// and for nothing else. +const credentialHelper = `#!/bin/sh +[ "$1" = get ] || exit 0 +while IFS== read -r key value; do + case $key in + protocol) protocol=$value ;; + host) host=$value ;; + esac +done +[ "$protocol://$host" = "$KRCI_GIT_REMOTE" ] || exit 0 +printf 'username=%s\npassword=%s\n' "$KRCI_GIT_USERNAME" "$KRCI_GIT_PASSWORD" +` + +// inheritedGitEnv are the only GIT_ variables git gets from the operator's environment: where it finds its +// configuration files. Any other could point git at another repository (GIT_DIR, GIT_WORK_TREE) or turn +// TLS verification off (GIT_SSL_NO_VERIFY, GIT_CONFIG_PARAMETERS). +var inheritedGitEnv = []string{"GIT_CONFIG_GLOBAL", "GIT_CONFIG_SYSTEM", "GIT_CONFIG_NOSYSTEM"} + +// CLIProvider runs Clone and Push with the git binary in a child process; every other method is GitProvider's. +type CLIProvider struct { + *GitProvider +} + +// NewCLIProvider creates a CLIProvider with the given configuration. +func NewCLIProvider(cfg Config) *CLIProvider { + return &CLIProvider{GitProvider: NewGitProvider(cfg)} +} + +// Clone clones a repository to the specified destination with every branch and tag. +func (p *CLIProvider) Clone(ctx context.Context, repoURL, destination string) error { + return p.withEnv(repoURL, func(env []string) error { + clone := func() error { + // "--" keeps a repository URL from the Codebase spec from being read as an option. + return run(ctx, env, "", repoURL, "clone", "--quiet", "--no-checkout", "--", repoURL, destination) + } + + fetchAndCheckout := func() error { return fetchAllAndCheckout(ctx, env, repoURL, destination) } + + return cloneThenFetch(ctx, repoURL, destination, clone, fetchAndCheckout) + }) +} + +// fetchAllAndCheckout is the second half of CLIProvider.Clone, see cloneThenFetch. +func fetchAllAndCheckout(ctx context.Context, env []string, repoURL, destination string) error { + // git clones an empty repository, or one whose HEAD names a missing branch, without an error; + // rev-parse --verify --quiet exits 1 for both, and go-git returns these two errors. + if err := run(ctx, env, destination, repoURL, "rev-parse", "--verify", "--quiet", "HEAD"); err != nil { + var exitErr *exec.ExitError + if !errors.As(err, &exitErr) || exitErr.ExitCode() != 1 { + return fmt.Errorf("failed to resolve HEAD of the clone: %w", err) + } + + if hasRefs(destination) { + return fmt.Errorf("failed to clone repository: remote HEAD: %w", plumbing.ErrReferenceNotFound) + } + + return fmt.Errorf("failed to clone repository: %w", transport.ErrEmptyRemoteRepository) + } + + // --update-head-ok: the refspec also covers the branch HEAD points to. + err := run(ctx, env, destination, repoURL, + "fetch", "--quiet", "--update-head-ok", "origin", refSpecFetchAllBranches, refSpecFetchAllTags) + if err != nil { + return fmt.Errorf("failed to fetch all branches and tags: %w", err) + } + + // go-git, which commits on this checkout later, applies no .gitattributes + // conversion; without this file git would, and go-git would see the files as changed. + attributes := filepath.Join(destination, ".git", "info", "attributes") + if err := os.MkdirAll(filepath.Dir(attributes), 0o755); err != nil { + return fmt.Errorf("failed to create %s: %w", filepath.Dir(attributes), err) + } + + if err := os.WriteFile(attributes, []byte("* -text -ident -filter -working-tree-encoding\n"), 0o600); err != nil { + return fmt.Errorf("failed to write %s: %w", attributes, err) + } + + if err := run(ctx, env, destination, repoURL, "reset", "--quiet", "--hard"); err != nil { + return fmt.Errorf("failed to check out the default branch: %w", err) + } + + return nil +} + +// hasRefs reports whether the repository at dir holds a ref other than HEAD. +func hasRefs(dir string) bool { + repo, err := git.PlainOpen(dir) + if err != nil { + return false + } + + refs, err := repo.References() + if err != nil { + return false + } + + found := false + + _ = refs.ForEach(func(ref *plumbing.Reference) error { + if ref.Name() == plumbing.HEAD { + return nil + } + + found = true + + return storer.ErrStop + }) + + return found +} + +// Push pushes the refspecs to origin; without refspecs it pushes every branch, as go-git does. +func (p *CLIProvider) Push(ctx context.Context, directory string, refspecs ...string) error { + log := ctrl.LoggerFrom(ctx).WithValues("directory", directory) + log.Info("Pushing changes") + + repo, err := git.PlainOpen(directory) + if err != nil { + return fmt.Errorf("failed to open repository at %q: %w", directory, err) + } + + if len(refspecs) == 0 { + refspecs = []string{RefSpecPushAllBranches} + } + + remoteURL := originURL(repo) + + err = p.withEnv(remoteURL, func(env []string) error { + push := func(options ...string) error { + args := slices.Concat([]string{"push", "--quiet"}, options, []string{"--", "origin"}, refspecs) + + return run(ctx, env, directory, remoteURL, args...) + } + + // --atomic: every ref or none, as with go-git, which checks every update before it sends one; + // the next reconcile would take a partial push for a complete one. A server without atomic + // push gets a plain one. + err := push("--atomic") + if err != nil && strings.Contains(err.Error(), "does not support --atomic push") { + err = push() + } + + return err + }) + if err != nil { + return fmt.Errorf("failed to push: %w", err) + } + + log.Info("Changes pushed successfully") + + return nil +} + +// withEnv runs fn with the environment that passes the provider's credentials for remoteURL to git, +// never its arguments; the key and the CA bundle live in a temporary directory for the call. +func (p *CLIProvider) withEnv(remoteURL string, fn func(env []string) error) error { + tmp, err := os.MkdirTemp("", "git-cli-") + if err != nil { + return fmt.Errorf("failed to create a directory for git credentials: %w", err) + } + + defer func() { _ = os.RemoveAll(tmp) }() + + env, err := p.env(tmp, remoteURL) + if err != nil { + return err + } + + return fn(env) +} + +// run runs git in dir with env; remoteURL names the host in a host key error. +func run(ctx context.Context, env []string, dir, remoteURL string, args ...string) error { + cmd := exec.CommandContext(ctx, "git", args...) + cmd.Dir = dir + cmd.Env = slices.Concat(inheritedEnv(), env) + // After a cancel kills git, its ssh or git-remote-https child can still hold the output pipe. + cmd.WaitDelay = 10 * time.Second + + out, err := cmd.CombinedOutput() + if err != nil { + return sshhostkey.EnrichOutput(gitError(args[0], out, err), string(out), sshTarget(remoteURL)) + } + + return nil +} + +// gitError keeps the end of git's output, where git reports the failure, and +// marks rejected credentials as go-git does. +func gitError(command string, out []byte, err error) error { + msg := string(bytes.TrimSpace(out)) + + if len(msg) > maxErrorOutput { + msg = "..." + strings.ToValidUTF8(msg[len(msg)-maxErrorOutput:], "") + } + + if msg != "" { + err = fmt.Errorf("git %s: %s: %w", command, msg, err) + } else { + err = fmt.Errorf("git %s: %w", command, err) + } + + // With prompts off, git reports a rejected HTTP credential as a missing one. + if strings.Contains(msg, "terminal prompts disabled") || strings.Contains(msg, "Authentication failed") { + return fmt.Errorf("%w: %w", transport.ErrAuthenticationRequired, err) + } + + return err +} + +// inheritedEnv is the operator's environment without the GIT_ variables outside inheritedGitEnv. +func inheritedEnv() []string { + return slices.DeleteFunc(os.Environ(), func(kv string) bool { + name, _, _ := strings.Cut(kv, "=") + + return strings.HasPrefix(name, "GIT_") && !slices.Contains(inheritedGitEnv, name) + }) +} + +// env passes the credentials for remoteURL to git through its environment, never its arguments. +func (p *CLIProvider) env(tmp, remoteURL string) ([]string, error) { + env := []string{ + // Repository URLs come from the Codebase spec: allow the transports go-git + // supports, never a remote helper such as ext::, which runs a command. + "GIT_ALLOW_PROTOCOL=file:git:http:https:ssh", + "GIT_TERMINAL_PROMPT=0", + } + + sshCommand := []string{ + "ssh", "-o", "BatchMode=yes", + "-o", "ConnectTimeout=30", "-o", "ServerAliveInterval=30", "-o", "ServerAliveCountMax=4", + } + + if p.config.SSHKey != "" { + keyFile := filepath.Join(tmp, "id") + if err := writeSSHKey(keyFile, p.config.SSHKey); err != nil { + return nil, err + } + + // -l: the GitServer's user, which go-git uses too; Gerrit URLs carry no user. + sshCommand = append(sshCommand, "-i", keyFile, "-o", "IdentitiesOnly=yes", "-l", p.config.SSHUser) + } + + env = append(env, "GIT_SSH_COMMAND="+shellJoin(append(sshCommand, sshhostkey.OpenSSHOptions()...))) + + gitConfig := [][2]string{ + // The operator runs as PID 1 and does not reap the maintenance process git can detach after a fetch. + {"maintenance.auto", "false"}, + // Bounds on git's memory: each delta thread holds a cache, and its search window holds whole + // objects, so the threads are capped (or the memory grows with the node's CPUs), the window + // is capped in bytes, and blobs over the threshold skip the delta search and are streamed. + {"pack.threads", "2"}, + {"pack.windowMemory", "100m"}, + {"core.bigFileThreshold", "64m"}, + {"http.sslVerify", "true"}, + } + + // git asks the credential helper per host, so after a redirect to another host, or to plain http, + // the helper gives it nothing, as go-git sends nothing there. The empty value drops other helpers. + if u, err := url.Parse(remoteURL); err == nil && (u.Scheme == "https" || u.Scheme == "http") && p.config.Token != "" { + helper := filepath.Join(tmp, "credential-helper") + if err := os.WriteFile(helper, []byte(credentialHelper), 0o700); err != nil { + return nil, fmt.Errorf("failed to write the git credential helper: %w", err) + } + + auth := p.getTokenAuth() + env = append(env, + "KRCI_GIT_REMOTE="+u.Scheme+"://"+u.Host, "KRCI_GIT_USERNAME="+auth.Username, "KRCI_GIT_PASSWORD="+auth.Password) + gitConfig = append(gitConfig, [2]string{"credential.helper", ""}, [2]string{"credential.helper", helper}) + } + + env = append(env, "GIT_CONFIG_COUNT="+strconv.Itoa(len(gitConfig))) + for i, kv := range gitConfig { + env = append(env, fmt.Sprintf("GIT_CONFIG_KEY_%d=%s", i, kv[0]), fmt.Sprintf("GIT_CONFIG_VALUE_%d=%s", i, kv[1])) + } + + bundle, err := caBundle(tmp) + if err != nil { + return nil, err + } + + if bundle != "" { + env = append(env, "GIT_SSL_CAINFO="+bundle) + } + + return env, nil +} + +// writeSSHKey writes the key re-encoded through x/crypto, which go-git parses keys with: ssh rejects +// spellings go-git accepts, such as CRLF line endings, surrounding blank lines or a missing final newline. +func writeSSHKey(keyFile, sshKey string) error { + key, err := ssh2.ParseRawPrivateKey([]byte(sshKey)) + if err != nil { + return fmt.Errorf("failed to parse SSH private key: %w", err) + } + + // ParseRawPrivateKey returns a pointer to the ed25519 key type that MarshalPrivateKey takes as a value. + if k, ok := key.(*ed25519.PrivateKey); ok { + key = *k + } + + block, err := ssh2.MarshalPrivateKey(key, "") + if err != nil { + return fmt.Errorf("failed to encode the SSH key for git: %w", err) + } + + if err := os.WriteFile(keyFile, pem.EncodeToMemory(block), 0o600); err != nil { + return fmt.Errorf("failed to write the SSH key for git: %w", err) + } + + return nil +} + +// caBundle gives git the certificates Go trusts, as one file for GIT_SSL_CAINFO: git reads neither +// SSL_CERT_FILE nor SSL_CERT_DIR. Like Go, it takes SSL_CERT_FILE or the system bundle, plus every +// certificate in the SSL_CERT_DIR directories or the system one. It returns "" when neither variable +// is set, because git then trusts the system bundle already. +func caBundle(tmp string) (string, error) { + certFile, certDirs := os.Getenv("SSL_CERT_FILE"), filepath.SplitList(os.Getenv("SSL_CERT_DIR")) + if certFile == "" && len(certDirs) == 0 { + return "", nil + } + + if certFile == "" { + certFile = systemCertFile + } + + if len(certDirs) == 0 { + certDirs = []string{systemCertDir} + } + + files := []string{certFile} + + for _, dir := range certDirs { + entries, err := os.ReadDir(dir) + if err != nil { + continue + } + + for _, entry := range entries { + files = append(files, filepath.Join(dir, entry.Name())) + } + } + + var bundle bytes.Buffer + + seen := map[string]bool{} + + for _, file := range files { + // Entries that are not files, such as the ..data directory of a mounted secret, fail to read. + data, err := os.ReadFile(file) + if err != nil { + continue + } + + for block, rest := pem.Decode(data); block != nil; block, rest = pem.Decode(rest) { + // OpenSSL rejects the whole file for one block that does not parse; Go skips the block. + if block.Type != "CERTIFICATE" || seen[string(block.Bytes)] { + continue + } + + if _, err := x509.ParseCertificate(block.Bytes); err != nil { + continue + } + + seen[string(block.Bytes)] = true + _ = pem.Encode(&bundle, block) + } + } + + if bundle.Len() == 0 { + return "", nil + } + + bundleFile := filepath.Join(tmp, "ca-certificates.crt") + if err := os.WriteFile(bundleFile, bundle.Bytes(), 0o600); err != nil { + return "", fmt.Errorf("failed to write the CA bundle for git: %w", err) + } + + return bundleFile, nil +} + +// shellJoin quotes args for GIT_SSH_COMMAND, which git runs through the shell. +func shellJoin(args []string) string { + quoted := make([]string, len(args)) + for i, arg := range args { + quoted[i] = "'" + strings.ReplaceAll(arg, "'", `'\''`) + "'" + } + + return strings.Join(quoted, " ") +} diff --git a/pkg/git/cli_test.go b/pkg/git/cli_test.go new file mode 100644 index 00000000..ffa977f0 --- /dev/null +++ b/pkg/git/cli_test.go @@ -0,0 +1,891 @@ +package v2 + +import ( + "crypto/ed25519" + "crypto/rand" + "crypto/x509" + "crypto/x509/pkix" + "encoding/base64" + "encoding/pem" + "errors" + "fmt" + "math/big" + "net/http" + "net/http/cgi" + "net/http/httptest" + "os" + "os/exec" + "path/filepath" + "slices" + "strconv" + "strings" + "sync/atomic" + "testing" + "time" + "unicode/utf8" + + gogit "github.com/go-git/go-git/v5" + "github.com/go-git/go-git/v5/plumbing" + "github.com/go-git/go-git/v5/plumbing/transport" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + ssh2 "golang.org/x/crypto/ssh" + + codebaseApi "github.com/epam/edp-codebase-operator/v2/api/v1" + "github.com/epam/edp-codebase-operator/v2/pkg/sshhostkey" +) + +// testSSHKey returns a new ed25519 private key in the OpenSSH format. +func testSSHKey(t *testing.T) string { + t.Helper() + + _, key, err := ed25519.GenerateKey(rand.Reader) + require.NoError(t, err) + + block, err := ssh2.MarshalPrivateKey(key, "") + require.NoError(t, err) + + return string(pem.EncodeToMemory(block)) +} + +// testCertPEM returns a new self-signed CA certificate. +func testCertPEM(t *testing.T) []byte { + t.Helper() + + pub, key, err := ed25519.GenerateKey(rand.Reader) + require.NoError(t, err) + + template := &x509.Certificate{ + SerialNumber: big.NewInt(1), + Subject: pkix.Name{CommonName: "test CA"}, + NotBefore: time.Now(), + NotAfter: time.Now().Add(time.Hour), + IsCA: true, + BasicConstraintsValid: true, + } + + der, err := x509.CreateCertificate(rand.Reader, template, template, pub, key) + require.NoError(t, err) + + return pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: der}) +} + +// hermeticGit keeps the developer's git configuration out of the CLI tests. +func hermeticGit(t *testing.T) { + t.Helper() + + t.Setenv("GIT_CONFIG_GLOBAL", os.DevNull) + t.Setenv("GIT_CONFIG_NOSYSTEM", "1") +} + +// onPath puts an executable script named name first on PATH. +func onPath(t *testing.T, name, script string) { + t.Helper() + + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, name), []byte(script), 0o700)) + t.Setenv("PATH", dir+string(filepath.ListSeparator)+os.Getenv("PATH")) +} + +// onPathGit puts a git stand-in first on PATH; the script reaches the real git as $REAL_GIT. +func onPathGit(t *testing.T, script string) { + t.Helper() + + realGit, err := exec.LookPath("git") + require.NoError(t, err) + + t.Setenv("REAL_GIT", realGit) + onPath(t, "git", script) +} + +// initOrigin creates a bare repository with the branches master and feature and the tag v1. +func initOrigin(t *testing.T, dir string) { + t.Helper() + + repo, err := gogit.PlainInit(dir, true) + require.NoError(t, err) + + master := seedCommit(t, dir, "base", "+refs/heads/master:refs/heads/master") + seedCommit(t, dir, "feature", "+refs/heads/master:refs/heads/feature") + + _, err = repo.CreateTag("v1", master, nil) + require.NoError(t, err) +} + +// localRefs lists the branches and tags of a repository. +func localRefs(t *testing.T, dir string) []string { + t.Helper() + + repo, err := gogit.PlainOpen(dir) + require.NoError(t, err) + + iter, err := repo.References() + require.NoError(t, err) + + var refs []string + + require.NoError(t, iter.ForEach(func(ref *plumbing.Reference) error { + if ref.Name().IsBranch() || ref.Name().IsTag() { + refs = append(refs, ref.Name().String()) + } + + return nil + })) + + slices.Sort(refs) + + return refs +} + +// runGit runs git in dir and returns its output. +func runGit(t *testing.T, dir string, args ...string) string { + t.Helper() + + out, err := exec.Command("git", append([]string{"-C", dir}, args...)...).CombinedOutput() + require.NoError(t, err, string(out)) + + return string(out) +} + +// gitBackend serves the bare repositories under root through git http-backend. +func gitBackend(t *testing.T, root, user string) http.Handler { + t.Helper() + + execPath, err := exec.Command("git", "--exec-path").Output() + require.NoError(t, err) + + return &cgi.Handler{ + Path: filepath.Join(strings.TrimSpace(string(execPath)), "git-http-backend"), + // REMOTE_USER makes http-backend accept pushes; PATH stays the one without + // the stand-ins that tests put on it later. + Env: []string{ + "GIT_PROJECT_ROOT=" + root, "GIT_HTTP_EXPORT_ALL=1", "REMOTE_USER=" + user, + "GIT_CONFIG_GLOBAL=" + os.DevNull, "GIT_CONFIG_NOSYSTEM=1", "PATH=" + os.Getenv("PATH"), + }, + } +} + +// gitHTTPServer serves the bare repositories under root over HTTPS through +// git http-backend, behind Basic authentication. +func gitHTTPServer(t *testing.T, root, user, password string) *httptest.Server { + t.Helper() + + backend := gitBackend(t, root, user) + + srv := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if u, p, ok := r.BasicAuth(); !ok || u != user || p != password { + w.Header().Set("WWW-Authenticate", `Basic realm="git"`) + http.Error(w, "unauthorized", http.StatusUnauthorized) + + return + } + + backend.ServeHTTP(w, r) + })) + t.Cleanup(srv.Close) + + return srv +} + +func TestNewGitProviderFactory(t *testing.T) { + tests := []struct { + name string + value string + unset bool + wantCLI bool + }{ + {name: "unset selects the git binary", unset: true, wantCLI: true}, + {name: "true selects the git binary", value: "true", wantCLI: true}, + {name: "false selects go-git", value: "false", wantCLI: false}, + {name: "an invalid value keeps the default", value: "maybe", wantCLI: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Setenv(CLIEnabledEnv, tt.value) + + if tt.unset { + require.NoError(t, os.Unsetenv(CLIEnabledEnv)) + } + + _, isCLI := NewGitProviderFactory(Config{}).(*CLIProvider) + assert.Equal(t, tt.wantCLI, isCLI) + assert.Equal(t, tt.wantCLI, CLIEnabled()) + }) + } +} + +func TestCLIProvider_Clone(t *testing.T) { + hermeticGit(t) + + origin := t.TempDir() + initOrigin(t, origin) + + clone := func(p Git) ([]string, string) { + dir := filepath.Join(t.TempDir(), "clone") + require.NoError(t, p.Clone(t.Context(), origin, dir)) + assert.Empty(t, runGit(t, dir, "status", "--porcelain"), "the working tree must match HEAD") + + return localRefs(t, dir), workdirHead(t, dir).Name().Short() + } + + wantRefs, wantHead := clone(NewGitProvider(Config{})) + gotRefs, gotHead := clone(NewCLIProvider(Config{})) + + assert.Equal(t, []string{"refs/heads/feature", "refs/heads/master", "refs/tags/v1"}, wantRefs) + assert.Equal(t, wantRefs, gotRefs, "the CLI clone must hold the same local refs as the go-git clone") + assert.Equal(t, wantHead, gotHead) +} + +// A push to the default branch while the clone runs must not leave the working +// tree behind the branch, or the next commit would revert that push. +func TestCLIProvider_Clone_BranchMovedDuringClone(t *testing.T) { + hermeticGit(t) + + origin := t.TempDir() + initOrigin(t, origin) + + moved := seedCommit(t, origin, "moved", "+refs/heads/master:refs/keep/moved") + + // After the real clone, origin's master moves to a commit the clone has not seen. + t.Setenv("ORIGIN", origin) + t.Setenv("MOVED", moved.String()) + onPathGit(t, `#!/bin/sh +"$REAL_GIT" "$@" || exit +[ "$1" = clone ] && "$REAL_GIT" --git-dir="$ORIGIN" update-ref refs/heads/master "$MOVED" +exit 0 +`) + + dir := filepath.Join(t.TempDir(), "clone") + require.NoError(t, NewCLIProvider(Config{}).Clone(t.Context(), origin, dir)) + + assert.Equal(t, moved, workdirHead(t, dir).Hash()) + assert.Empty(t, runGit(t, dir, "status", "--porcelain"), "the working tree must match the moved branch") + + content, err := os.ReadFile(filepath.Join(dir, "f.txt")) + require.NoError(t, err) + assert.Equal(t, "moved", string(content)) +} + +// go-git commits on the CLI checkout later and applies no .gitattributes +// conversion, so the checkout must not apply one either. +func TestCLIProvider_Clone_NoAttributeConversion(t *testing.T) { + hermeticGit(t) + + origin := t.TempDir() + _, err := gogit.PlainInit(origin, true) + require.NoError(t, err) + + seed := t.TempDir() + runGit(t, seed, "init", "--quiet", "-b", "master") + require.NoError(t, os.WriteFile(filepath.Join(seed, ".gitattributes"), []byte("*.bat text eol=crlf\n"), 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(seed, "gradlew.bat"), []byte("echo one\necho two\n"), 0o600)) + runGit(t, seed, "add", ".") + runGit(t, seed, "-c", "user.name=t", "-c", "user.email=t@t", "commit", "--quiet", "-m", "init") + runGit(t, seed, "push", "--quiet", origin, "master") + + dir := filepath.Join(t.TempDir(), "clone") + require.NoError(t, NewCLIProvider(Config{}).Clone(t.Context(), origin, dir)) + + content, err := os.ReadFile(filepath.Join(dir, "gradlew.bat")) + require.NoError(t, err) + assert.Equal(t, "echo one\necho two\n", string(content), "the file must hold the committed bytes, as with go-git") +} + +func TestCLIProvider_Push(t *testing.T) { + hermeticGit(t) + + tests := []struct { + name string + refspecs []string + want []string + }{ + { + name: "no refspec pushes every branch, as go-git does", + want: []string{"refs/heads/feature", "refs/heads/master"}, + }, + { + name: "branches and tags", + refspecs: []string{RefSpecPushAllBranches, RefSpecPushAllTags}, + want: []string{"refs/heads/feature", "refs/heads/master", "refs/tags/v1"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + p := NewCLIProvider(Config{}) + + origin := t.TempDir() + initOrigin(t, origin) + + workDir := filepath.Join(t.TempDir(), "clone") + require.NoError(t, p.Clone(t.Context(), origin, workDir)) + + target := t.TempDir() + _, err := gogit.PlainInit(target, true) + require.NoError(t, err) + require.NoError(t, p.AddRemoteLink(t.Context(), workDir, target)) + + require.NoError(t, p.Push(t.Context(), workDir, tt.refspecs...)) + assert.Equal(t, tt.want, localRefs(t, target)) + + require.NoError(t, p.Push(t.Context(), workDir, tt.refspecs...), "an up-to-date push is not an error") + }) + } +} + +func TestCLIProvider_Errors(t *testing.T) { + hermeticGit(t) + + p := NewCLIProvider(Config{}) + + t.Run("missing repository", func(t *testing.T) { + err := p.Clone(t.Context(), filepath.Join(t.TempDir(), "missing"), filepath.Join(t.TempDir(), "clone")) + require.ErrorContains(t, err, "failed to clone repository") + }) + + t.Run("a URL is never read as an option", func(t *testing.T) { + // Read as an option, this URL would make git clone the destination with "touch" as upload-pack. + origin := t.TempDir() + initOrigin(t, origin) + + marker := filepath.Join(t.TempDir(), "marker") + require.Error(t, p.Clone(t.Context(), "--upload-pack=touch "+marker, origin)) + assert.NoFileExists(t, marker) + }) + + t.Run("empty repository fails as with go-git and leaves no clone", func(t *testing.T) { + origin := t.TempDir() + _, err := gogit.PlainInit(origin, true) + require.NoError(t, err) + + for _, gp := range []Git{NewGitProvider(Config{}), p} { + dir := filepath.Join(t.TempDir(), "clone") + require.ErrorIs(t, gp.Clone(t.Context(), origin, dir), transport.ErrEmptyRemoteRepository) + assert.NoDirExists(t, dir) + } + }) + + t.Run("a failed fetch leaves no partial clone", func(t *testing.T) { + onPathGit(t, "#!/bin/sh\n[ \"$1\" = fetch ] && exit 1\nexec \"$REAL_GIT\" \"$@\"\n") + + origin := t.TempDir() + initOrigin(t, origin) + + dir := filepath.Join(t.TempDir(), "clone") + require.ErrorContains(t, p.Clone(t.Context(), origin, dir), "failed to fetch all branches and tags") + assert.NoDirExists(t, dir) + }) + + t.Run("a killed clone leaves no partial clone", func(t *testing.T) { + // git killed mid-clone never runs its own cleanup and leaves the .git it has written. + onPathGit(t, "#!/bin/sh\nif [ \"$1\" = clone ]; then for last; do :; done; mkdir -p \"$last/.git\"; kill -9 $$; fi\n"+ + "exec \"$REAL_GIT\" \"$@\"\n") + + origin := t.TempDir() + initOrigin(t, origin) + + dir := filepath.Join(t.TempDir(), "clone") + require.ErrorContains(t, p.Clone(t.Context(), origin, dir), "failed to clone repository") + assert.NoDirExists(t, dir) + }) + + t.Run("a HEAD that names a missing branch is not an empty repository", func(t *testing.T) { + origin := t.TempDir() + initOrigin(t, origin) + runGit(t, origin, "update-ref", "-d", "refs/heads/master") + + dir := filepath.Join(t.TempDir(), "clone") + err := p.Clone(t.Context(), origin, dir) + require.ErrorIs(t, err, plumbing.ErrReferenceNotFound) + assert.NotErrorIs(t, err, transport.ErrEmptyRemoteRepository) + assert.NoDirExists(t, dir) + }) + + t.Run("rejected push", func(t *testing.T) { + origin := t.TempDir() + initOrigin(t, origin) + + workDir := filepath.Join(t.TempDir(), "clone") + require.NoError(t, p.Clone(t.Context(), origin, workDir)) + + seedCommit(t, origin, "unrelated", "+refs/heads/master:refs/heads/master") + require.ErrorContains(t, p.Push(t.Context(), workDir, RefSpecPushAllBranches), "rejected") + }) + + t.Run("push outside a repository", func(t *testing.T) { + require.ErrorContains(t, p.Push(t.Context(), t.TempDir()), "failed to open repository") + }) +} + +func TestCLIProvider_HTTPS(t *testing.T) { + hermeticGit(t) + + const token = "glpat-secret" + + root := t.TempDir() + initOrigin(t, filepath.Join(root, "repo.git")) + + srv := gitHTTPServer(t, root, "oauth2", token) + repoURL := srv.URL + "/repo.git" + + trusted := t.TempDir() + certPEM := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: srv.Certificate().Raw}) + require.NoError(t, os.WriteFile(filepath.Join(trusted, "ca.crt"), certPEM, 0o600)) + + tests := []struct { + name string + token string + certDir string + env map[string]string + wantErr string + wantErrIs error + }{ + {name: "token over a CA from SSL_CERT_DIR", token: token, certDir: trusted}, + { + name: "rejected token", token: "glpat-wrong", certDir: trusted, + wantErr: "failed to clone repository", wantErrIs: transport.ErrAuthenticationRequired, + }, + {name: "CA missing from SSL_CERT_DIR", token: token, wantErr: "certificate"}, + { + name: "GIT_SSL_NO_VERIFY does not turn verification off", token: token, + env: map[string]string{"GIT_SSL_NO_VERIFY": "1"}, wantErr: "certificate", + }, + { + name: "GIT_CONFIG_PARAMETERS does not turn verification off", token: token, + env: map[string]string{"GIT_CONFIG_PARAMETERS": "'http.sslverify'='false'"}, wantErr: "certificate", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Setenv("SSL_CERT_DIR", tt.certDir) + + for k, v := range tt.env { + t.Setenv(k, v) + } + + argv := filepath.Join(t.TempDir(), "argv") + t.Setenv("ARGV", argv) + onPathGit(t, "#!/bin/sh\necho \"$@\" >> \"$ARGV\"\nexec \"$REAL_GIT\" \"$@\"\n") + + p := NewCLIProvider(Config{GitProvider: codebaseApi.GitProviderGitlab, Token: tt.token}) + workDir := filepath.Join(t.TempDir(), "clone") + secrets := []string{tt.token, base64.StdEncoding.EncodeToString([]byte("oauth2:" + tt.token))} + + err := p.Clone(t.Context(), repoURL, workDir) + if tt.wantErr == "" { + require.NoError(t, err) + require.NoError(t, p.CreateChildBranch(t.Context(), workDir, "master", "pushed")) + require.NoError(t, p.Push(t.Context(), workDir, RefSpecPushAllBranches)) + assert.Contains(t, localRefs(t, filepath.Join(root, "repo.git")), "refs/heads/pushed") + } else { + require.ErrorContains(t, err, tt.wantErr) + + if tt.wantErrIs != nil { + require.ErrorIs(t, err, tt.wantErrIs) + } + + for _, secret := range secrets { + assert.NotContains(t, err.Error(), secret) + } + } + + args, err := os.ReadFile(argv) + require.NoError(t, err) + + for _, secret := range secrets { + assert.NotContains(t, string(args), secret, "credentials must never be in git's arguments") + } + }) + } +} + +// The SSH path, with an ssh stand-in that records its arguments and the key +// file's mode, then runs the remote git command locally. +func TestCLIProvider_SSH(t *testing.T) { + hermeticGit(t) + t.Setenv("SSH_KNOWN_HOSTS", "/etc/codebase-operator/ssh/ssh_known_hosts") + + argv := filepath.Join(t.TempDir(), "argv") + t.Setenv("ARGV", argv) + onPath(t, "ssh", `#!/bin/sh +for arg; do + echo "$arg" >> "$ARGV" + [ "$prev" = -i ] && echo "key mode $(stat -c %a "$arg" 2>/dev/null || stat -f %Lp "$arg")" >> "$ARGV" + prev=$arg +done +exec sh -c "$arg" +`) + + origin := t.TempDir() + initOrigin(t, origin) + + // Like a Gerrit URL: no user in it. + repoURL := "ssh://git.example.com:29418" + filepath.ToSlash(origin) + p := NewCLIProvider(Config{SSHKey: testSSHKey(t), SSHUser: "edp-ci"}) + + workDir := filepath.Join(t.TempDir(), "clone") + require.NoError(t, p.Clone(t.Context(), repoURL, workDir)) + require.NoError(t, p.CreateChildBranch(t.Context(), workDir, "master", "pushed")) + require.NoError(t, p.Push(t.Context(), workDir, RefSpecPushAllBranches)) + assert.Contains(t, localRefs(t, origin), "refs/heads/pushed") + + recorded, err := os.ReadFile(argv) + require.NoError(t, err) + + args := strings.Split(string(recorded), "\n") + assert.Subset(t, args, []string{ + "BatchMode=yes", "IdentitiesOnly=yes", "key mode 600", + "ConnectTimeout=30", "ServerAliveInterval=30", "ServerAliveCountMax=4", + }) + assert.Subset(t, args, sshhostkey.OpenSSHOptions()) + assert.Equal(t, "edp-ci", args[slices.Index(args, "-l")+1], "ssh must log in as the GitServer user") +} + +// A host key failure reported by ssh reaches the caller with the known_hosts guidance. +func TestCLIProvider_SSH_HostKeyFailure(t *testing.T) { + hermeticGit(t) + t.Setenv("SSH_KNOWN_HOSTS", "/etc/codebase-operator/ssh/ssh_known_hosts") + onPath(t, "ssh", "#!/bin/sh\necho 'Host key verification failed.' >&2\nexit 255\n") + + p := NewCLIProvider(Config{SSHKey: testSSHKey(t), SSHUser: "edp-ci"}) + err := p.Clone(t.Context(), "ssh://gerrit.example.com:29418/proj", filepath.Join(t.TempDir(), "clone")) + + require.ErrorContains(t, err, "not present in known_hosts (/etc/codebase-operator/ssh/ssh_known_hosts)") + require.ErrorContains(t, err, "ssh-keyscan -p 29418 gerrit.example.com") +} + +// The temporary SSH key and CA bundle of a git run are gone after it, whether it succeeded or failed. +func TestCLIProvider_RemovesCredentials(t *testing.T) { + hermeticGit(t) + + tmp := t.TempDir() + t.Setenv("TMPDIR", tmp) + + certs := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(certs, "ca.crt"), testCertPEM(t), 0o600)) + t.Setenv("SSL_CERT_DIR", certs) + + // What TMPDIR holds while git runs, which proves the credentials were written there. + seen := filepath.Join(t.TempDir(), "seen") + t.Setenv("SEEN", seen) + onPathGit(t, "#!/bin/sh\nls -R \"$TMPDIR\" >> \"$SEEN\"\nexec \"$REAL_GIT\" \"$@\"\n") + + origin := t.TempDir() + initOrigin(t, origin) + + p := NewCLIProvider(Config{SSHKey: testSSHKey(t), Token: "token"}) + require.NoError(t, p.Clone(t.Context(), origin, filepath.Join(t.TempDir(), "clone"))) + require.Error(t, p.Clone(t.Context(), filepath.Join(t.TempDir(), "missing"), filepath.Join(t.TempDir(), "clone"))) + + during, err := os.ReadFile(seen) + require.NoError(t, err) + assert.Contains(t, string(during), "ca-certificates.crt") + assert.Contains(t, strings.Fields(string(during)), "id") + + entries, err := os.ReadDir(tmp) + require.NoError(t, err) + assert.Empty(t, entries) +} + +// GIT_ALLOW_PROTOCOL overrides protocol..allow, so ext:: stays refused even +// when the git configuration allows it. +func TestCLIProvider_RefusesExtTransport(t *testing.T) { + hermeticGit(t) + + gitConfig := filepath.Join(t.TempDir(), "gitconfig") + require.NoError(t, os.WriteFile(gitConfig, []byte("[protocol \"ext\"]\n\tallow = always\n"), 0o600)) + t.Setenv("GIT_CONFIG_GLOBAL", gitConfig) + + marker := filepath.Join(t.TempDir(), "marker") + extURL := "ext::sh -c touch% " + marker + " %S x" + err := NewCLIProvider(Config{}).Clone(t.Context(), extURL, filepath.Join(t.TempDir(), "clone")) + + require.ErrorContains(t, err, "not allowed") + assert.NoFileExists(t, marker) +} + +func TestCLIProvider_env(t *testing.T) { + t.Setenv("SSL_CERT_DIR", "") + t.Setenv("SSL_CERT_FILE", "") + + key := testSSHKey(t) + tmp := t.TempDir() + p := NewCLIProvider(Config{ + // A spelling go-git reads and ssh does not. + SSHKey: "\n" + strings.ReplaceAll(key, "\n", "\r\n") + " \n", + SSHUser: "edp-ci", + GitProvider: codebaseApi.GitProviderGitlab, + Token: "glpat-secret", + }) + + env, err := p.env(tmp, "https://git.example.com:8443/group/repo.git") + require.NoError(t, err) + + vars := map[string]string{} + + for _, kv := range env { + k, v, _ := strings.Cut(kv, "=") + vars[k] = v + } + + count, err := strconv.Atoi(vars["GIT_CONFIG_COUNT"]) + require.NoError(t, err) + + gitConfig := make([]string, 0, count) + + for i := range count { + key, value := vars[fmt.Sprintf("GIT_CONFIG_KEY_%d", i)], vars[fmt.Sprintf("GIT_CONFIG_VALUE_%d", i)] + gitConfig = append(gitConfig, key+"="+value) + } + + helper := filepath.Join(tmp, "credential-helper") + assert.Equal(t, []string{ + "maintenance.auto=false", + "pack.threads=2", + "pack.windowMemory=100m", + "core.bigFileThreshold=64m", + "http.sslVerify=true", + "credential.helper=", + "credential.helper=" + helper, + }, gitConfig) + assert.Equal(t, "https://git.example.com:8443", vars["KRCI_GIT_REMOTE"]) + assert.Equal(t, "oauth2", vars["KRCI_GIT_USERNAME"]) + assert.Equal(t, "glpat-secret", vars["KRCI_GIT_PASSWORD"]) + assert.NotContains(t, vars, "GIT_SSL_CAINFO", "without SSL_CERT_FILE and SSL_CERT_DIR git keeps its own trust store") + + written, err := os.ReadFile(filepath.Join(tmp, "id")) + require.NoError(t, err) + assert.True(t, strings.HasPrefix(string(written), "-----BEGIN OPENSSH PRIVATE KEY-----\n")) + assert.NotContains(t, string(written), "\r") + + got, err := ssh2.ParsePrivateKey(written) + require.NoError(t, err) + want, err := ssh2.ParsePrivateKey([]byte(key)) + require.NoError(t, err) + assert.Equal(t, want.PublicKey().Marshal(), got.PublicKey().Marshal(), "the same key, re-encoded") + + _, err = NewCLIProvider(Config{SSHKey: "not a key"}).env(t.TempDir(), "") + require.ErrorContains(t, err, "failed to parse SSH private key") +} + +func TestCredentialHelper(t *testing.T) { + helper := filepath.Join(t.TempDir(), "credential-helper") + require.NoError(t, os.WriteFile(helper, []byte(credentialHelper), 0o700)) + + t.Setenv("KRCI_GIT_REMOTE", "https://git.example.com:8443") + t.Setenv("KRCI_GIT_USERNAME", "oauth2") + t.Setenv("KRCI_GIT_PASSWORD", "glpat-secret") + + answer := func(operation, request string) string { + cmd := exec.Command(helper, operation) + cmd.Stdin = strings.NewReader(request) + + out, err := cmd.Output() + require.NoError(t, err) + + return string(out) + } + + assert.Equal(t, "username=oauth2\npassword=glpat-secret\n", + answer("get", "protocol=https\nhost=git.example.com:8443\npath=group/repo.git\n")) + assert.Empty(t, answer("get", "protocol=https\nhost=other.example.com\n"), "another host") + assert.Empty(t, answer("get", "protocol=http\nhost=git.example.com:8443\n"), "plain http") + assert.Empty(t, answer("store", "protocol=https\nhost=git.example.com:8443\n"), "only get is answered") +} + +// Credentials are scoped to the remote: after a redirect to another host git sends none, as go-git. +func TestCLIProvider_HTTPS_RedirectGetsNoCredentials(t *testing.T) { + hermeticGit(t) + + root := t.TempDir() + initOrigin(t, filepath.Join(root, "repo.git")) + backend := gitBackend(t, root, "") + + var targetSawAuth atomic.Bool + + // The target asks for credentials, so git would send whatever it holds for the target's host. + target := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Header.Get("Authorization") == "" { + w.Header().Set("WWW-Authenticate", `Basic realm="git"`) + http.Error(w, "unauthorized", http.StatusUnauthorized) + + return + } + + targetSawAuth.Store(true) + backend.ServeHTTP(w, r) + })) + t.Cleanup(target.Close) + + // The remote moves git to the target on the first request, before any credentials are asked for. + origin := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Redirect(w, r, target.URL+r.URL.RequestURI(), http.StatusMovedPermanently) + })) + t.Cleanup(origin.Close) + + trusted := t.TempDir() + certPEM := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: origin.Certificate().Raw}) + require.NoError(t, os.WriteFile(filepath.Join(trusted, "ca.crt"), certPEM, 0o600)) + t.Setenv("SSL_CERT_DIR", trusted) + + p := NewCLIProvider(Config{GitProvider: codebaseApi.GitProviderGitlab, Token: "glpat-secret"}) + require.ErrorIs(t, p.Clone(t.Context(), origin.URL+"/repo.git", filepath.Join(t.TempDir(), "clone")), + transport.ErrAuthenticationRequired) + assert.False(t, targetSawAuth.Load(), "the redirect target must not get the remote's credentials") +} + +func TestCLIProvider_Push_Atomic(t *testing.T) { + hermeticGit(t) + + p := NewCLIProvider(Config{}) + + setup := func(t *testing.T) (string, string) { + t.Helper() + + origin := t.TempDir() + initOrigin(t, origin) + + workDir := filepath.Join(t.TempDir(), "clone") + require.NoError(t, p.Clone(t.Context(), origin, workDir)) + + target := t.TempDir() + _, err := gogit.PlainInit(target, true) + require.NoError(t, err) + require.NoError(t, p.AddRemoteLink(t.Context(), workDir, target)) + + return workDir, target + } + + t.Run("a rejected ref stops every ref, as with go-git", func(t *testing.T) { + workDir, target := setup(t) + seedCommit(t, target, "unrelated", "+refs/heads/master:refs/heads/master") + + require.Error(t, p.Push(t.Context(), workDir, RefSpecPushAllBranches)) + assert.Equal(t, []string{"refs/heads/master"}, localRefs(t, target), "feature must not land alone") + }) + + t.Run("a server without atomic push gets a plain one", func(t *testing.T) { + workDir, target := setup(t) + runGit(t, target, "config", "receive.advertiseAtomic", "false") + + require.NoError(t, p.Push(t.Context(), workDir, RefSpecPushAllBranches)) + assert.Equal(t, []string{"refs/heads/feature", "refs/heads/master"}, localRefs(t, target)) + }) +} + +// GIT_DIR or GIT_WORK_TREE in the operator's environment, which a git hook that runs these tests sets, +// must not point git at another repository. +func TestCLIProvider_IgnoresInheritedGitDir(t *testing.T) { + hermeticGit(t) + + other := t.TempDir() + runGit(t, other, "init", "--quiet", "-b", "master") + require.NoError(t, os.WriteFile(filepath.Join(other, "other.txt"), []byte("other"), 0o600)) + runGit(t, other, "add", ".") + runGit(t, other, "-c", "user.name=t", "-c", "user.email=t@t", "commit", "--quiet", "-m", "other") + + origin := t.TempDir() + initOrigin(t, origin) + + head := workdirHead(t, other).Hash() + + t.Setenv("GIT_DIR", filepath.Join(other, ".git")) + t.Setenv("GIT_WORK_TREE", other) + + dir := filepath.Join(t.TempDir(), "clone") + require.NoError(t, NewCLIProvider(Config{}).Clone(t.Context(), origin, dir)) + + assert.Equal(t, []string{"refs/heads/feature", "refs/heads/master", "refs/tags/v1"}, localRefs(t, dir)) + assert.FileExists(t, filepath.Join(dir, "f.txt")) + assert.Equal(t, head, workdirHead(t, other).Hash(), "the other repository must stay untouched") +} + +func TestGitError(t *testing.T) { + cause := errors.New("exit status 128") + + assert.Equal(t, "git push: exit status 128", gitError("push", nil, cause).Error()) + + long := gitError("push", []byte(strings.Repeat("ж", maxErrorOutput)+"fatal: the end"), cause) + assert.True(t, utf8.ValidString(long.Error())) + assert.Contains(t, long.Error(), "git push: ...") + assert.Contains(t, long.Error(), "fatal: the end: exit status 128") + assert.Less(t, len(long.Error()), maxErrorOutput+64) + assert.ErrorIs(t, long, cause) + + for _, out := range []string{ + "fatal: could not read Username for 'https://git.example.com': terminal prompts disabled", + "fatal: Authentication failed for 'https://git.example.com/repo.git/'", + } { + err := gitError("clone", []byte(out), cause) + assert.ErrorIs(t, err, transport.ErrAuthenticationRequired) + assert.ErrorIs(t, err, cause) + } +} + +func TestShellJoin(t *testing.T) { + args := []string{"ssh", "-l", "it's", "-o", "UserKnownHostsFile=/a b /c", "$(touch x)"} + + cmd := exec.Command("sh", "-c", `printf '%s\n' `+shellJoin(args)) + cmd.Dir = t.TempDir() + + out, err := cmd.Output() + require.NoError(t, err) + assert.Equal(t, strings.Join(args, "\n")+"\n", string(out), "the shell must get back the exact arguments") + assert.NoFileExists(t, filepath.Join(cmd.Dir, "x")) +} + +func TestCaBundle(t *testing.T) { + t.Setenv("SSL_CERT_DIR", "") + t.Setenv("SSL_CERT_FILE", "") + + bundle, err := caBundle(t.TempDir()) + require.NoError(t, err) + assert.Empty(t, bundle, "without SSL_CERT_FILE and SSL_CERT_DIR git keeps its own trust store") + + cert := testCertPEM(t) + certs := t.TempDir() + files := map[string]string{ + "ca.crt": string(cert), + "tls.pem": "-----BEGIN PRIVATE KEY-----\nMIIB\n-----END PRIVATE KEY-----\n" + string(cert), + "broken.crt": "-----BEGIN CERTIFICATE-----\nMIIB\n-----END CERTIFICATE-----\n", + "README": "not a certificate", + } + + for name, data := range files { + require.NoError(t, os.WriteFile(filepath.Join(certs, name), []byte(data), 0o600)) + } + + require.NoError(t, os.Mkdir(filepath.Join(certs, "..data"), 0o700)) + + tests := []struct { + name string + certFile string + certDir string + }{ + {name: "SSL_CERT_DIR", certDir: certs + string(filepath.ListSeparator) + filepath.Join(t.TempDir(), "missing")}, + {name: "SSL_CERT_FILE", certFile: filepath.Join(certs, "ca.crt")}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Setenv("SSL_CERT_FILE", tt.certFile) + t.Setenv("SSL_CERT_DIR", tt.certDir) + + bundle, err := caBundle(t.TempDir()) + require.NoError(t, err) + + data, err := os.ReadFile(bundle) + require.NoError(t, err) + assert.Equal(t, 1, strings.Count(string(data), string(cert)), "the certificate, once") + assert.NotContains(t, string(data), "PRIVATE KEY") + + // OpenSSL rejects the whole file for one block that does not parse. + for block, rest := pem.Decode(data); block != nil; block, rest = pem.Decode(rest) { + _, err := x509.ParseCertificate(block.Bytes) + require.NoError(t, err) + } + }) + } +} diff --git a/pkg/git/factory.go b/pkg/git/factory.go index 55a5c01d..d3caa1cf 100644 --- a/pkg/git/factory.go +++ b/pkg/git/factory.go @@ -1,25 +1,43 @@ package v2 import ( + "os" + "strconv" + corev1 "k8s.io/api/core/v1" codebaseApi "github.com/epam/edp-codebase-operator/v2/api/v1" "github.com/epam/edp-codebase-operator/v2/pkg/util" ) +// CLIEnabledEnv set to false makes Clone and Push use go-git instead of the git binary. +const CLIEnabledEnv = "GIT_CLI_ENABLED" + // GitProviderFactory creates a Git provider from GitServer and Secret. // This is a factory pattern to enable dependency injection and mocking in tests. type GitProviderFactory func(cfg Config) Git -// DefaultGitProviderFactory is the default factory implementation for creating GitProvider instances. +// DefaultGitProviderFactory is the default factory implementation for creating Git providers. func DefaultGitProviderFactory(gitServer *codebaseApi.GitServer, secret *corev1.Secret) Git { - return NewGitProvider(NewConfigFromGitServerAndSecret(gitServer, secret)) + return NewGitProviderFactory(NewConfigFromGitServerAndSecret(gitServer, secret)) } +// NewGitProviderFactory returns a CLIProvider, or a GitProvider when CLIEnabled is false. func NewGitProviderFactory(cfg Config) Git { + if CLIEnabled() { + return NewCLIProvider(cfg) + } + return NewGitProvider(cfg) } +// CLIEnabled reports whether Clone and Push run the git binary: unless GIT_CLI_ENABLED parses as false. +func CLIEnabled() bool { + enabled, err := strconv.ParseBool(os.Getenv(CLIEnabledEnv)) + + return err != nil || enabled +} + func NewConfigFromGitServerAndSecret(gitServer *codebaseApi.GitServer, secret *corev1.Secret) Config { return Config{ SSHKey: string(secret.Data[util.PrivateSShKeyName]), diff --git a/pkg/git/git.go b/pkg/git/git.go index 436ec365..7a74f246 100644 --- a/pkg/git/git.go +++ b/pkg/git/git.go @@ -2,7 +2,7 @@ package v2 import "context" -// Git interface provides methods for working with git using v2 GitProvider. +// Git interface provides methods for working with git, implemented by GitProvider and CLIProvider. // This interface uses context-aware methods and handles authentication via Config. type Git interface { // Clone clones a repository to the specified destination with full history. diff --git a/pkg/git/provider.go b/pkg/git/provider.go index 8d620553..5194e0a8 100644 --- a/pkg/git/provider.go +++ b/pkg/git/provider.go @@ -35,6 +35,12 @@ const ( // RefSpecPushAllTags is the refspec for pushing all tags. // Equivalent to git push --tags RefSpecPushAllTags = "refs/tags/*:refs/tags/*" + + // Fetch refspecs: every remote branch and tag becomes a local ref. The + lets a force-pushed + // branch or a moved tag overwrite its local copy. Do not widen to refs/*: it also downloads + // every pull and merge request ref into memory. + refSpecFetchAllBranches = "+refs/heads/*:refs/heads/*" + refSpecFetchAllTags = "+refs/tags/*:refs/tags/*" ) // commitOps holds options for commit operations. @@ -148,7 +154,7 @@ func originURL(repo *git.Repository) string { } // getTokenAuth formats token authentication based on the git provider type. -func (p *GitProvider) getTokenAuth() transport.AuthMethod { +func (p *GitProvider) getTokenAuth() *http.BasicAuth { switch p.config.GitProvider { case codebaseApi.GitProviderGithub: // GitHub: username=token, password="" @@ -179,42 +185,66 @@ func (p *GitProvider) getTokenAuth() transport.AuthMethod { // Clone clones a repository to the specified destination. func (p *GitProvider) Clone(ctx context.Context, repoURL, destination string) error { - log := ctrl.LoggerFrom(ctx).WithValues("repository", repoURL, "destination", destination) - log.Info("Cloning repository") - auth, err := p.getAuth() if err != nil { return fmt.Errorf("failed to get authentication: %w", err) } - // Clone the repository (gets default branch) - cloneOptions := &git.CloneOptions{ - URL: repoURL, - Progress: os.Stdout, - Auth: auth, + var repo *git.Repository + + clone := func() error { + repo, err = git.PlainCloneContext(ctx, destination, false, &git.CloneOptions{ + URL: repoURL, + Progress: os.Stdout, + Auth: auth, + NoCheckout: true, + }) + + return remoteErr(err, repoURL) } - repo, err := git.PlainCloneContext(ctx, destination, false, cloneOptions) - if err != nil { - return fmt.Errorf("failed to clone repository: %w", remoteErr(err, repoURL)) + fetchAndCheckout := func() error { + err := repo.FetchContext(ctx, fetchAllOptions(auth)) + if err != nil && !errors.Is(err, git.NoErrAlreadyUpToDate) { + return fmt.Errorf("failed to fetch all branches and tags: %w", remoteErr(err, repoURL)) + } + + worktree, err := repo.Worktree() + if err != nil { + return fmt.Errorf("failed to get worktree: %w", err) + } + + if err := worktree.Reset(&git.ResetOptions{Mode: git.HardReset}); err != nil { + return fmt.Errorf("failed to check out the default branch: %w", err) + } + + return nil } - log.Info("Repository cloned successfully, now fetching all branches and tags") + return cloneThenFetch(ctx, repoURL, destination, clone, fetchAndCheckout) +} - // Fetch all refs (branches, tags, etc.) to get everything similar to --mirror - fetchOptions := &git.FetchOptions{ - RemoteName: "origin", - Auth: auth, - Progress: os.Stdout, - RefSpecs: []config.RefSpec{ - config.RefSpec("+refs/heads/*:refs/heads/*"), - config.RefSpec("+refs/tags/*:refs/tags/*"), - }, +// cloneThenFetch is Clone for both engines: clone runs without a checkout, then +// fetchAndCheckout makes every remote branch and tag a local ref and checks out +// HEAD, which the fetch can move. A failure in either step removes the destination: +// callers skip a destination that is not empty and would take a partial clone, such +// as the .git a killed git leaves behind, for a complete one. +func cloneThenFetch(ctx context.Context, repoURL, destination string, clone, fetchAndCheckout func() error) error { + log := ctrl.LoggerFrom(ctx).WithValues("repository", repoURL, "destination", destination) + log.Info("Cloning repository") + + if err := clone(); err != nil { + _ = os.RemoveAll(destination) + + return fmt.Errorf("failed to clone repository: %w", err) } - err = repo.FetchContext(ctx, fetchOptions) - if err != nil && !errors.Is(err, git.NoErrAlreadyUpToDate) { - return fmt.Errorf("failed to fetch all branches and tags: %w", remoteErr(err, repoURL)) + log.Info("Repository cloned successfully, now fetching all branches and tags") + + if err := fetchAndCheckout(); err != nil { + _ = os.RemoveAll(destination) + + return err } log.Info("All branches and tags fetched successfully") @@ -222,6 +252,16 @@ func (p *GitProvider) Clone(ctx context.Context, repoURL, destination string) er return nil } +// fetchAllOptions returns the options that make every branch and tag of origin a local ref. +func fetchAllOptions(auth transport.AuthMethod) *git.FetchOptions { + return &git.FetchOptions{ + RemoteName: "origin", + RefSpecs: []config.RefSpec{refSpecFetchAllBranches, refSpecFetchAllTags}, + Auth: auth, + Progress: os.Stdout, + } +} + // Commit commits changes in the working directory. func (p *GitProvider) Commit(ctx context.Context, directory, message string, ops ...CommitOps) error { log := ctrl.LoggerFrom(ctx).WithValues("directory", directory) @@ -348,17 +388,7 @@ func (p *GitProvider) Checkout(ctx context.Context, directory, branchName string return fmt.Errorf("failed to get authentication: %w", err) } - fetchOptions := &git.FetchOptions{ - RefSpecs: []config.RefSpec{"refs/*:refs/*"}, - Auth: auth, - Progress: os.Stdout, - // The refspec has no force prefix, so without this a rebased or - // force-pushed upstream branch fails the fetch with ErrForceNeeded - // against a cached workdir. - Force: true, - } - - err = repo.FetchContext(ctx, fetchOptions) + err = repo.FetchContext(ctx, fetchAllOptions(auth)) if err != nil && !errors.Is(err, git.NoErrAlreadyUpToDate) { return fmt.Errorf("failed to fetch: %w", remoteErr(err, originURL(repo))) } @@ -627,17 +657,9 @@ func (p *GitProvider) CheckoutRemoteBranch(ctx context.Context, directory, branc return fmt.Errorf("failed to get authentication: %w", err) } - fetchOptions := &git.FetchOptions{ - RemoteName: "origin", - RefSpecs: []config.RefSpec{"refs/*:refs/*"}, - Auth: auth, - Progress: os.Stdout, - Force: true, // Equivalent to --update-head-ok - } - - err = repo.FetchContext(ctx, fetchOptions) + err = repo.FetchContext(ctx, fetchAllOptions(auth)) if err != nil && !errors.Is(err, git.NoErrAlreadyUpToDate) { - return fmt.Errorf("failed to fetch: %w", err) + return fmt.Errorf("failed to fetch: %w", remoteErr(err, originURL(repo))) } // Checkout the branch (expects it exists remotely) diff --git a/pkg/git/provider_test.go b/pkg/git/provider_test.go index 141990e7..a4f2fecb 100644 --- a/pkg/git/provider_test.go +++ b/pkg/git/provider_test.go @@ -11,7 +11,6 @@ import ( gogit "github.com/go-git/go-git/v5" "github.com/go-git/go-git/v5/plumbing" "github.com/go-git/go-git/v5/plumbing/object" - githttp "github.com/go-git/go-git/v5/plumbing/transport/http" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ctrl "sigs.k8s.io/controller-runtime" @@ -497,12 +496,8 @@ func TestGitProvider_getTokenAuth(t *testing.T) { auth := gp.getTokenAuth() require.NotNil(t, auth) - - // Type assert to *githttp.BasicAuth to access Username and Password - basicAuth, ok := auth.(*githttp.BasicAuth) - require.True(t, ok, "auth should be *githttp.BasicAuth") - assert.Equal(t, tt.wantUsername, basicAuth.Username) - assert.Equal(t, tt.wantPassword, basicAuth.Password) + assert.Equal(t, tt.wantUsername, auth.Username) + assert.Equal(t, tt.wantPassword, auth.Password) }) } } diff --git a/pkg/sshhostkey/hostkey.go b/pkg/sshhostkey/hostkey.go index 963c62e9..a5874475 100644 --- a/pkg/sshhostkey/hostkey.go +++ b/pkg/sshhostkey/hostkey.go @@ -11,6 +11,7 @@ import ( "fmt" "net" "os" + "path/filepath" "strconv" "strings" @@ -20,8 +21,8 @@ import ( ) // knownHostsEnvVar is go-git's own lookup variable, reused so that the go-git -// code paths and the raw golang.org/x/crypto/ssh code paths always agree on -// which file is authoritative. +// code paths, the raw golang.org/x/crypto/ssh code paths and the ssh binary +// (OpenSSHOptions) always agree on which files are authoritative. const knownHostsEnvVar = "SSH_KNOWN_HOSTS" // ClientConfig returns the host key callback and the host key algorithms to set @@ -51,7 +52,7 @@ func HostPort(host string, port int) string { return net.JoinHostPort(host, strconv.Itoa(port)) } -// Source is for error messages only; the file itself is opened by go-git. +// Source is for error messages only; the file itself is opened by go-git or ssh. func Source() string { if path := os.Getenv(knownHostsEnvVar); path != "" { return path @@ -60,6 +61,22 @@ func Source() string { return "~/.ssh/known_hosts, /etc/ssh/ssh_known_hosts" } +// OpenSSHOptions returns the ssh(1) options that verify host keys as go-git +// does: a host missing from known_hosts is refused, no key is ever added, and +// the files in SSH_KNOWN_HOSTS, when it is set, are the only source. +func OpenSSHOptions() []string { + opts := []string{"-o", "StrictHostKeyChecking=yes", "-o", "UpdateHostKeys=no"} + + if files := filepath.SplitList(os.Getenv(knownHostsEnvVar)); len(files) > 0 { + opts = append(opts, + "-o", "UserKnownHostsFile="+strings.Join(files, " "), + "-o", "GlobalKnownHostsFile=/dev/null", + ) + } + + return opts +} + // Enrich turns a host key verification failure into an actionable message. // Errors of any other kind are returned unchanged, so it is safe to apply to // every error returned by an SSH operation. @@ -70,10 +87,7 @@ func Enrich(err error, hostPort string) error { var revokedErr *knownhosts.RevokedError if errors.As(err, &revokedErr) { - return fmt.Errorf( - "SSH host key for %s is marked as revoked in known_hosts (%s), refusing to connect: %w", - hostPort, Source(), err, - ) + return revoked(err, hostPort) } var keyErr *knownhosts.KeyError @@ -85,14 +99,44 @@ func Enrich(err error, hostPort string) error { // different key: either the server was rekeyed, or the connection is being // intercepted. Never suggest "just add the key" for this case. if len(keyErr.Want) > 0 { - return fmt.Errorf( - "SSH host key mismatch for %s: the server presented a key that does not match known_hosts (%s). "+ - "This means either the git server's host key was rotated, or the connection is being intercepted. "+ - "Verify the new key out-of-band before replacing the entry: %w", - hostPort, Source(), err, - ) + return mismatch(err, hostPort) + } + + return missing(err, hostPort) +} + +// EnrichOutput is Enrich for the OpenSSH client, which reports a failed host +// key check only as text; output is what ssh printed. +func EnrichOutput(err error, output, hostPort string) error { + switch { + case err == nil || !strings.Contains(output, "Host key verification failed"): + return err + case strings.Contains(output, "REVOKED HOST KEY"): + return revoked(err, hostPort) + case strings.Contains(output, "REMOTE HOST IDENTIFICATION HAS CHANGED"): + return mismatch(err, hostPort) + default: + return missing(err, hostPort) } +} + +func revoked(err error, hostPort string) error { + return fmt.Errorf( + "SSH host key for %s is marked as revoked in known_hosts (%s), refusing to connect: %w", + hostPort, Source(), err, + ) +} + +func mismatch(err error, hostPort string) error { + return fmt.Errorf( + "SSH host key mismatch for %s: the server presented a key that does not match known_hosts (%s). "+ + "This means either the git server's host key was rotated, or the connection is being intercepted. "+ + "Verify the new key out-of-band before replacing the entry: %w", + hostPort, Source(), err, + ) +} +func missing(err error, hostPort string) error { return fmt.Errorf( "SSH host key for %s is not present in known_hosts (%s). "+ "Add it to the operator's ssh-known-hosts ConfigMap, for example: ssh-keyscan -p %s %s: %w", diff --git a/pkg/sshhostkey/hostkey_test.go b/pkg/sshhostkey/hostkey_test.go index 1003d656..0ba86cef 100644 --- a/pkg/sshhostkey/hostkey_test.go +++ b/pkg/sshhostkey/hostkey_test.go @@ -131,15 +131,13 @@ func TestEnrich(t *testing.T) { tests := []struct { name string err error - wantNil bool wantSame bool wantContain []string wantAbsent []string }{ { - name: "nil passes through", - err: nil, - wantNil: true, + name: "nil passes through", + wantSame: true, }, { name: "unrelated error is untouched", @@ -173,27 +171,108 @@ func TestEnrich(t *testing.T) { got := Enrich(tt.err, HostPort(testHost, testPort)) - if tt.wantNil { - assert.NoError(t, got) + if tt.wantSame { + assert.Equal(t, tt.err, got) return } - require.Error(t, got) + assertEnriched(t, got, tt.err, tt.wantContain, tt.wantAbsent) + }) + } +} + +// assertEnriched checks a message from Enrich or EnrichOutput. +func assertEnriched(t *testing.T, got, orig error, contain, absent []string) { + t.Helper() + + require.Error(t, got) + + for _, want := range contain { + assert.Contains(t, got.Error(), want) + } + + for _, s := range absent { + assert.NotContains(t, got.Error(), s) + } + + assert.ErrorIs(t, got, orig, "the original error must stay in the chain") +} + +func TestOpenSSHOptions(t *testing.T) { + want := []string{ + "-o", "StrictHostKeyChecking=yes", "-o", "UpdateHostKeys=no", + "-o", "UserKnownHostsFile=/etc/a /etc/b", "-o", "GlobalKnownHostsFile=/dev/null", + } + + t.Setenv(knownHostsEnvVar, "") + assert.Equal(t, want[:4], OpenSSHOptions(), "without SSH_KNOWN_HOSTS ssh keeps its default files") + + t.Setenv(knownHostsEnvVar, "/etc/a"+string(filepath.ListSeparator)+"/etc/b") + assert.Equal(t, want, OpenSSHOptions()) +} + +func TestEnrichOutput(t *testing.T) { + t.Parallel() + + gitErr := errors.New("exit status 128") + + tests := []struct { + name string + err error + output string + wantSame bool + wantContain []string + wantAbsent []string + }{ + { + name: "nil passes through", + output: "Host key verification failed.", + wantSame: true, + }, + { + name: "other failure is untouched", + err: gitErr, + output: "git@git.example.com: Permission denied (publickey).", + wantSame: true, + }, + { + name: "unknown host suggests ssh-keyscan", + err: gitErr, + output: "No ED25519 host key is known for [git.example.com]:2222 and you have requested strict checking.\n" + + "Host key verification failed.", + wantContain: []string{"not present in known_hosts", "ssh-keyscan -p 2222 git.example.com"}, + }, + { + name: "changed key warns about interception", + err: gitErr, + output: "@ WARNING: REMOTE HOST IDENTIFICATION HAS CHANGED! @\n" + + "Host key for [git.example.com]:2222 has changed and you have requested strict checking.\n" + + "Host key verification failed.", + wantContain: []string{"mismatch", "intercepted", "out-of-band"}, + wantAbsent: []string{"ssh-keyscan"}, + }, + { + name: "revoked key is refused", + err: gitErr, + output: "@ WARNING: REVOKED HOST KEY DETECTED! @\n" + + "Host key verification failed.", + wantContain: []string{"revoked"}, + wantAbsent: []string{"ssh-keyscan"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + got := EnrichOutput(tt.err, tt.output, HostPort(testHost, testPort)) if tt.wantSame { assert.Equal(t, tt.err, got) return } - for _, want := range tt.wantContain { - assert.Contains(t, got.Error(), want) - } - - for _, absent := range tt.wantAbsent { - assert.NotContains(t, got.Error(), absent) - } - - assert.ErrorIs(t, got, tt.err, "the original error must stay in the chain") + assertEnriched(t, got, tt.err, tt.wantContain, tt.wantAbsent) }) } }