fix: the concurrency issue with git detached processing in Repo Server (#25101) (cherry-pick #25127 for 3.2) (#25448)

Signed-off-by: Eugene Doudine <eugene.doudine@octopus.com>
This commit is contained in:
dudinea 2025-11-30 13:24:37 +02:00 committed by GitHub
parent c11e67d4bf
commit 29f869c82f
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
19 changed files with 213 additions and 8 deletions

View file

@ -80,6 +80,7 @@ func NewCommand() *cobra.Command {
includeHiddenDirectories bool
cmpUseManifestGeneratePaths bool
ociMediaTypes []string
enableBuiltinGitConfig bool
)
command := cobra.Command{
Use: cliName,
@ -155,6 +156,7 @@ func NewCommand() *cobra.Command {
IncludeHiddenDirectories: includeHiddenDirectories,
CMPUseManifestGeneratePaths: cmpUseManifestGeneratePaths,
OCIMediaTypes: ociMediaTypes,
EnableBuiltinGitConfig: enableBuiltinGitConfig,
}, askPassServer)
errors.CheckError(err)
@ -264,6 +266,7 @@ func NewCommand() *cobra.Command {
command.Flags().BoolVar(&includeHiddenDirectories, "include-hidden-directories", env.ParseBoolFromEnv("ARGOCD_REPO_SERVER_INCLUDE_HIDDEN_DIRECTORIES", false), "Include hidden directories from Git")
command.Flags().BoolVar(&cmpUseManifestGeneratePaths, "plugin-use-manifest-generate-paths", env.ParseBoolFromEnv("ARGOCD_REPO_SERVER_PLUGIN_USE_MANIFEST_GENERATE_PATHS", false), "Pass the resources described in argocd.argoproj.io/manifest-generate-paths value to the cmpserver to generate the application manifests.")
command.Flags().StringSliceVar(&ociMediaTypes, "oci-layer-media-types", env.StringsFromEnv("ARGOCD_REPO_SERVER_OCI_LAYER_MEDIA_TYPES", []string{"application/vnd.oci.image.layer.v1.tar", "application/vnd.oci.image.layer.v1.tar+gzip", "application/vnd.cncf.helm.chart.content.v1.tar+gzip"}, ","), "Comma separated list of allowed media types for OCI media types. This only accounts for media types within layers.")
command.Flags().BoolVar(&enableBuiltinGitConfig, "enable-builtin-git-config", env.ParseBoolFromEnv("ARGOCD_REPO_SERVER_ENABLE_BUILTIN_GIT_CONFIG", true), "Enable builtin git configuration options that are required for correct argocd-repo-server operation.")
tlsConfigCustomizerSrc = tls.AddTLSFlagsToCmd(&command)
cacheSrc = reposervercache.AddCacheFlagsToCmd(&command, cacheutil.Options{
OnClientCreated: func(client *redis.Client) {

View file

@ -219,6 +219,8 @@ data:
reposerver.git.lsremote.parallelism.limit: "0"
# Git requests timeout.
reposerver.git.request.timeout: "15s"
# Enable builtin git configuration options that are required for correct argocd-repo-server operation (default "true")
reposerver.enable.builtin.git.config: "true"
# Include hidden directories from Git
reposerver.include.hidden.directories: "false"

View file

@ -0,0 +1,43 @@
# Git Configuration
## System Configuration
Argo CD uses the Git installation from its base image (Ubuntu), which
includes a standard system configuration file located at
`/etc/gitconfig`. This file is minimal, just defining filters
necessary for Git LFS functionality.
You can customize Git's system configuration by mounting a file from a
ConfigMap or by creating a custom Argo CD image.
## Global Configuration
Argo CD runs Git with the `HOME` environment variable set to
`/dev/null`. As a result, global Git configuration is not supported.
## Built-in Configuration
The `argocd-repo-server` adds specific configuration parameters to the
Git environment to ensure proper Argo CD operation. These built-in
settings override any conflicting values from the system Git
configuration.
Currently, the following built-in configuration options are set:
- `maintenance.autoDetach=false`
- `gc.autoDetach=false`
These settings force Git's repository maintenance tasks to run in the
foreground. This prevents Git from running detached background
processes that could modify the repository and interfere with
subsequent Git invocations from `argocd-repo-server`.
You can disable these built-in settings by setting the
`argocd-cmd-params-cm` value `reposerver.enable.builtin.git.config` to
`"false"`. This allows you to experiment with background processing or
if you are certain that concurrency issues will not occur in your
environment.
> [!NOTE]
> Disabling this is not recommended and is not supported!

View file

@ -21,6 +21,7 @@ argocd-repo-server [flags]
--disable-helm-manifest-max-extracted-size Disable maximum size of helm manifest archives when extracted
--disable-oci-manifest-max-extracted-size Disable maximum size of oci manifest archives when extracted
--disable-tls Disable TLS on the gRPC endpoint
--enable-builtin-git-config Enable builtin git configuration options that are required for correct argocd-repo-server operation. (default true)
--helm-manifest-max-extracted-size string Maximum size of helm manifest archives when extracted (default "1G")
--helm-registry-max-index-size string Maximum size of registry index file (default "1G")
-h, --help help for argocd-repo-server

View file

@ -239,6 +239,12 @@ spec:
key: reposerver.git.request.timeout
name: argocd-cmd-params-cm
optional: true
- name: ARGOCD_REPO_SERVER_ENABLE_BUILTIN_GIT_CONFIG
valueFrom:
configMapKeyRef:
name: argocd-cmd-params-cm
key: reposerver.enable.builtin.git.config
optional: true
- name: ARGOCD_GRPC_MAX_SIZE_MB
valueFrom:
configMapKeyRef:

View file

@ -25386,6 +25386,12 @@ spec:
key: reposerver.git.request.timeout
name: argocd-cmd-params-cm
optional: true
- name: ARGOCD_REPO_SERVER_ENABLE_BUILTIN_GIT_CONFIG
valueFrom:
configMapKeyRef:
key: reposerver.enable.builtin.git.config
name: argocd-cmd-params-cm
optional: true
- name: ARGOCD_GRPC_MAX_SIZE_MB
valueFrom:
configMapKeyRef:

View file

@ -25220,6 +25220,12 @@ spec:
key: reposerver.git.request.timeout
name: argocd-cmd-params-cm
optional: true
- name: ARGOCD_REPO_SERVER_ENABLE_BUILTIN_GIT_CONFIG
valueFrom:
configMapKeyRef:
key: reposerver.enable.builtin.git.config
name: argocd-cmd-params-cm
optional: true
- name: ARGOCD_GRPC_MAX_SIZE_MB
valueFrom:
configMapKeyRef:

View file

@ -27021,6 +27021,12 @@ spec:
key: reposerver.git.request.timeout
name: argocd-cmd-params-cm
optional: true
- name: ARGOCD_REPO_SERVER_ENABLE_BUILTIN_GIT_CONFIG
valueFrom:
configMapKeyRef:
key: reposerver.enable.builtin.git.config
name: argocd-cmd-params-cm
optional: true
- name: ARGOCD_GRPC_MAX_SIZE_MB
valueFrom:
configMapKeyRef:

View file

@ -26857,6 +26857,12 @@ spec:
key: reposerver.git.request.timeout
name: argocd-cmd-params-cm
optional: true
- name: ARGOCD_REPO_SERVER_ENABLE_BUILTIN_GIT_CONFIG
valueFrom:
configMapKeyRef:
key: reposerver.enable.builtin.git.config
name: argocd-cmd-params-cm
optional: true
- name: ARGOCD_GRPC_MAX_SIZE_MB
valueFrom:
configMapKeyRef:

View file

@ -2702,6 +2702,12 @@ spec:
key: reposerver.git.request.timeout
name: argocd-cmd-params-cm
optional: true
- name: ARGOCD_REPO_SERVER_ENABLE_BUILTIN_GIT_CONFIG
valueFrom:
configMapKeyRef:
key: reposerver.enable.builtin.git.config
name: argocd-cmd-params-cm
optional: true
- name: ARGOCD_GRPC_MAX_SIZE_MB
valueFrom:
configMapKeyRef:

View file

@ -2538,6 +2538,12 @@ spec:
key: reposerver.git.request.timeout
name: argocd-cmd-params-cm
optional: true
- name: ARGOCD_REPO_SERVER_ENABLE_BUILTIN_GIT_CONFIG
valueFrom:
configMapKeyRef:
key: reposerver.enable.builtin.git.config
name: argocd-cmd-params-cm
optional: true
- name: ARGOCD_GRPC_MAX_SIZE_MB
valueFrom:
configMapKeyRef:

View file

@ -26051,6 +26051,12 @@ spec:
key: reposerver.git.request.timeout
name: argocd-cmd-params-cm
optional: true
- name: ARGOCD_REPO_SERVER_ENABLE_BUILTIN_GIT_CONFIG
valueFrom:
configMapKeyRef:
key: reposerver.enable.builtin.git.config
name: argocd-cmd-params-cm
optional: true
- name: ARGOCD_GRPC_MAX_SIZE_MB
valueFrom:
configMapKeyRef:

View file

@ -25885,6 +25885,12 @@ spec:
key: reposerver.git.request.timeout
name: argocd-cmd-params-cm
optional: true
- name: ARGOCD_REPO_SERVER_ENABLE_BUILTIN_GIT_CONFIG
valueFrom:
configMapKeyRef:
key: reposerver.enable.builtin.git.config
name: argocd-cmd-params-cm
optional: true
- name: ARGOCD_GRPC_MAX_SIZE_MB
valueFrom:
configMapKeyRef:

View file

@ -1732,6 +1732,12 @@ spec:
key: reposerver.git.request.timeout
name: argocd-cmd-params-cm
optional: true
- name: ARGOCD_REPO_SERVER_ENABLE_BUILTIN_GIT_CONFIG
valueFrom:
configMapKeyRef:
key: reposerver.enable.builtin.git.config
name: argocd-cmd-params-cm
optional: true
- name: ARGOCD_GRPC_MAX_SIZE_MB
valueFrom:
configMapKeyRef:

View file

@ -1566,6 +1566,12 @@ spec:
key: reposerver.git.request.timeout
name: argocd-cmd-params-cm
optional: true
- name: ARGOCD_REPO_SERVER_ENABLE_BUILTIN_GIT_CONFIG
valueFrom:
configMapKeyRef:
key: reposerver.enable.builtin.git.config
name: argocd-cmd-params-cm
optional: true
- name: ARGOCD_GRPC_MAX_SIZE_MB
valueFrom:
configMapKeyRef:

View file

@ -63,6 +63,7 @@ nav:
- operator-manual/web_based_terminal.md
- operator-manual/config-management-plugins.md
- operator-manual/deep_links.md
- operator-manual/git_configuration.md
- Notifications:
- Overview: operator-manual/notifications/index.md
- operator-manual/notifications/triggers.md

View file

@ -117,6 +117,7 @@ type RepoServerInitConstants struct {
DisableHelmManifestMaxExtractedSize bool
IncludeHiddenDirectories bool
CMPUseManifestGeneratePaths bool
EnableBuiltinGitConfig bool
}
var manifestGenerateLock = sync.NewKeyLock()
@ -2566,7 +2567,9 @@ func (s *Service) newClient(repo *v1alpha1.Repository, opts ...git.ClientOpts) (
if err != nil {
return nil, err
}
opts = append(opts, git.WithEventHandlers(metrics.NewGitClientEventHandlers(s.metricsServer)))
opts = append(opts,
git.WithEventHandlers(metrics.NewGitClientEventHandlers(s.metricsServer)),
git.WithBuiltinGitConfig(s.initConstants.EnableBuiltinGitConfig))
return s.newGitClient(repo.Repo, repoPath, repo.GetGitCreds(s.gitCredsStore), repo.IsInsecure(), repo.EnableLFS, repo.Proxy, repo.NoProxy, opts...)
}

View file

@ -45,6 +45,18 @@ import (
var ErrInvalidRepoURL = errors.New("repo URL is invalid")
// builtinGitConfig configuration contains statements that are needed
// for correct ArgoCD operation. These settings will override any
// user-provided configuration of same options.
var builtinGitConfig = map[string]string{
"maintenance.autoDetach": "false",
"gc.autoDetach": "false",
}
// BuiltinGitConfigEnv contains builtin git configuration in the
// format acceptable by Git.
var BuiltinGitConfigEnv []string
// CommitMetadata contains metadata about a commit that is related in some way to another commit.
type CommitMetadata struct {
// Author is the author of the commit.
@ -162,6 +174,8 @@ type nativeGitClient struct {
proxy string
// list of targets that shouldn't use the proxy, applies only if the proxy is set
noProxy string
// git configuration environment variables
gitConfigEnv []string
}
type runOpts struct {
@ -188,6 +202,14 @@ func init() {
maxRetryDuration = env.ParseDurationFromEnv(common.EnvGitRetryMaxDuration, common.DefaultGitRetryMaxDuration, 0, math.MaxInt64)
retryDuration = env.ParseDurationFromEnv(common.EnvGitRetryDuration, common.DefaultGitRetryDuration, 0, math.MaxInt64)
factor = env.ParseInt64FromEnv(common.EnvGitRetryFactor, common.DefaultGitRetryFactor, 0, math.MaxInt64)
BuiltinGitConfigEnv = append(BuiltinGitConfigEnv, fmt.Sprintf("GIT_CONFIG_COUNT=%d", len(builtinGitConfig)))
idx := 0
for k, v := range builtinGitConfig {
BuiltinGitConfigEnv = append(BuiltinGitConfigEnv, fmt.Sprintf("GIT_CONFIG_KEY_%d=%s", idx, k))
BuiltinGitConfigEnv = append(BuiltinGitConfigEnv, fmt.Sprintf("GIT_CONFIG_VALUE_%d=%s", idx, v))
idx++
}
}
type ClientOpts func(c *nativeGitClient)
@ -200,6 +222,16 @@ func WithCache(cache gitRefCache, loadRefFromCache bool) ClientOpts {
}
}
func WithBuiltinGitConfig(enable bool) ClientOpts {
return func(c *nativeGitClient) {
if enable {
c.gitConfigEnv = BuiltinGitConfigEnv
} else {
c.gitConfigEnv = nil
}
}
}
// WithEventHandlers sets the git client event handlers
func WithEventHandlers(handlers EventHandlers) ClientOpts {
return func(c *nativeGitClient) {
@ -222,13 +254,14 @@ func NewClient(rawRepoURL string, creds Creds, insecure bool, enableLfs bool, pr
func NewClientExt(rawRepoURL string, root string, creds Creds, insecure bool, enableLfs bool, proxy string, noProxy string, opts ...ClientOpts) (Client, error) {
client := &nativeGitClient{
repoURL: rawRepoURL,
root: root,
creds: creds,
insecure: insecure,
enableLfs: enableLfs,
proxy: proxy,
noProxy: noProxy,
repoURL: rawRepoURL,
root: root,
creds: creds,
insecure: insecure,
enableLfs: enableLfs,
proxy: proxy,
noProxy: noProxy,
gitConfigEnv: BuiltinGitConfigEnv,
}
for i := range opts {
opts[i](client)
@ -1111,6 +1144,8 @@ func (m *nativeGitClient) runCmdOutput(cmd *exec.Cmd, ropts runOpts) (string, er
cmd.Env = append(cmd.Env, "GIT_LFS_SKIP_SMUDGE=1")
// Disable Git terminal prompts in case we're running with a tty
cmd.Env = append(cmd.Env, "GIT_TERMINAL_PROMPT=false")
// Add Git configuration options that are essential for ArgoCD operation
cmd.Env = append(cmd.Env, m.gitConfigEnv...)
// For HTTPS repositories, we need to consider insecure repositories as well
// as custom CA bundles from the cert database.

View file

@ -10,6 +10,7 @@ import (
"os/exec"
"path"
"path/filepath"
"regexp"
"strings"
"sync"
"testing"
@ -1210,3 +1211,53 @@ Argocd-reference-commit-repourl: https://github.com/another/repo.git`,
})
}
}
func Test_BuiltinConfig(t *testing.T) {
tempDir := t.TempDir()
for _, enabled := range []bool{false, true} {
client, err := NewClientExt("file://"+tempDir, tempDir, NopCreds{}, true, false, "", "", WithBuiltinGitConfig(enabled))
require.NoError(t, err)
native := client.(*nativeGitClient)
configOut, err := native.config("--list", "--show-origin")
require.NoError(t, err)
for k, v := range builtinGitConfig {
r := regexp.MustCompile(fmt.Sprintf("(?m)^command line:\\s+%s=%s$", strings.ToLower(k), regexp.QuoteMeta(v)))
matches := r.FindString(configOut)
if enabled {
assert.NotEmpty(t, matches, "missing builtin configuration option: %s=%s", k, v)
} else {
assert.Empty(t, matches, "unexpected builtin configuration when builtin config is disabled: %s=%s", k, v)
}
}
}
}
func Test_GitNoDetachedMaintenance(t *testing.T) {
tempDir := t.TempDir()
ctx := t.Context()
client, err := NewClientExt("file://"+tempDir, tempDir, NopCreds{}, true, false, "", "")
require.NoError(t, err)
native := client.(*nativeGitClient)
err = client.Init()
require.NoError(t, err)
cmd := exec.CommandContext(ctx, "git", "fetch")
// trace execution of Git subcommands and their arguments to stderr
cmd.Env = append(cmd.Env, "GIT_TRACE=true")
// Ignore system config in case it disables auto maintenance
cmd.Env = append(cmd.Env, "GIT_CONFIG_NOSYSTEM=true")
output, err := native.runCmdOutput(cmd, runOpts{CaptureStderr: true})
require.NoError(t, err)
lines := strings.Split(output, "\n")
for _, line := range lines {
if strings.Contains(line, "git maintenance run") {
assert.NotContains(t, output, "--detach", "Unexpected --detach when running git maintenance")
return
}
}
assert.Fail(t, "Expected to see `git maintenance` run after `git fetch`")
}