fix(appset): prevent idle connection buildup by cloning http.DefaultTransport in Bitbucket SCM/PR generator (#24264)

Signed-off-by: portly-halicore-76 <170707699+portly-halicore-76@users.noreply.github.com>
Signed-off-by: anandf <anjoseph@redhat.com>
Co-authored-by: portly-halicore-76 <170707699+portly-halicore-76@users.noreply.github.com>

Author: anandf

applicationset/services/pull_request/bitbucket_server.go

@@ -3,12 +3,11 @@ package pull_request
 import (
 	"context"
 	"fmt"
-	"net/http"
 
 	bitbucketv1 "github.com/gfleury/go-bitbucket-v1"
 	log "github.com/sirupsen/logrus"
 
-	"github.com/argoproj/argo-cd/v3/applicationset/utils"
+	"github.com/argoproj/argo-cd/v3/applicationset/services"
 )
 
 type BitbucketService struct {
@@ -49,15 +48,10 @@ func NewBitbucketServiceNoAuth(ctx context.Context, url, projectKey, repositoryS
 }
 
 func newBitbucketService(ctx context.Context, bitbucketConfig *bitbucketv1.Configuration, projectKey, repositorySlug string, scmRootCAPath string, insecure bool, caCerts []byte) (PullRequestService, error) {
-	bitbucketConfig.BasePath = utils.NormalizeBitbucketBasePath(bitbucketConfig.BasePath)
-	tlsConfig := utils.GetTlsConfig(scmRootCAPath, insecure, caCerts)
-	bitbucketConfig.HTTPClient = &http.Client{Transport: &http.Transport{
-		TLSClientConfig: tlsConfig,
-	}}
-	bitbucketClient := bitbucketv1.NewAPIClient(ctx, bitbucketConfig)
+	bbClient := services.SetupBitbucketClient(ctx, bitbucketConfig, scmRootCAPath, insecure, caCerts)
 
 	return &BitbucketService{
-		client:         bitbucketClient,
+		client:         bbClient,
 		projectKey:     projectKey,
 		repositorySlug: repositorySlug,
 	}, nil

applicationset/services/scm_provider/bitbucket_server.go

@@ -10,7 +10,7 @@ import (
 	bitbucketv1 "github.com/gfleury/go-bitbucket-v1"
 	log "github.com/sirupsen/logrus"
 
-	"github.com/argoproj/argo-cd/v3/applicationset/utils"
+	"github.com/argoproj/argo-cd/v3/applicationset/services"
 )
 
 type BitbucketServerProvider struct {
@@ -49,15 +49,10 @@ func NewBitbucketServerProviderNoAuth(ctx context.Context, url, projectKey strin
 }
 
 func newBitbucketServerProvider(ctx context.Context, bitbucketConfig *bitbucketv1.Configuration, projectKey string, allBranches bool, scmRootCAPath string, insecure bool, caCerts []byte) (*BitbucketServerProvider, error) {
-	bitbucketConfig.BasePath = utils.NormalizeBitbucketBasePath(bitbucketConfig.BasePath)
-	tlsConfig := utils.GetTlsConfig(scmRootCAPath, insecure, caCerts)
-	bitbucketConfig.HTTPClient = &http.Client{Transport: &http.Transport{
-		TLSClientConfig: tlsConfig,
-	}}
-	bitbucketClient := bitbucketv1.NewAPIClient(ctx, bitbucketConfig)
+	bbClient := services.SetupBitbucketClient(ctx, bitbucketConfig, scmRootCAPath, insecure, caCerts)
 
 	return &BitbucketServerProvider{
-		client:      bitbucketClient,
+		client:      bbClient,
 		projectKey:  projectKey,
 		allBranches: allBranches,
 	}, nil

applicationset/services/util.go

@@ -0,0 +1,22 @@
+package services
+
+import (
+	"context"
+	"net/http"
+
+	bitbucketv1 "github.com/gfleury/go-bitbucket-v1"
+
+	"github.com/argoproj/argo-cd/v3/applicationset/utils"
+)
+
+// SetupBitbucketClient configures and creates a Bitbucket API client with TLS settings
+func SetupBitbucketClient(ctx context.Context, config *bitbucketv1.Configuration, scmRootCAPath string, insecure bool, caCerts []byte) *bitbucketv1.APIClient {
+	config.BasePath = utils.NormalizeBitbucketBasePath(config.BasePath)
+	tlsConfig := utils.GetTlsConfig(scmRootCAPath, insecure, caCerts)
+
+	transport := http.DefaultTransport.(*http.Transport).Clone()
+	transport.TLSClientConfig = tlsConfig
+	config.HTTPClient = &http.Client{Transport: transport}
+
+	return bitbucketv1.NewAPIClient(ctx, config)
+}

applicationset/services/util_test.go

@@ -0,0 +1,36 @@
+package services
+
+import (
+	"crypto/tls"
+	"net/http"
+	"testing"
+	"time"
+
+	bitbucketv1 "github.com/gfleury/go-bitbucket-v1"
+	"github.com/stretchr/testify/require"
+)
+
+func TestSetupBitbucketClient(t *testing.T) {
+	ctx := t.Context()
+	cfg := &bitbucketv1.Configuration{}
+
+	// Act
+	client := SetupBitbucketClient(ctx, cfg, "", false, nil)
+
+	// Assert
+	require.NotNil(t, client, "expected client to be created")
+	require.NotNil(t, cfg.HTTPClient, "expected HTTPClient to be set")
+
+	// The transport should be a clone of DefaultTransport
+	tr, ok := cfg.HTTPClient.Transport.(*http.Transport)
+	require.True(t, ok, "expected HTTPClient.Transport to be *http.Transport")
+	require.NotSame(t, http.DefaultTransport, tr, "transport should be a clone, not the global DefaultTransport")
+
+	// Ensure TLSClientConfig is set
+	require.IsType(t, &tls.Config{}, tr.TLSClientConfig)
+
+	// Defaults from http.DefaultTransport.Clone() should be preserved
+	require.Greater(t, tr.IdleConnTimeout, time.Duration(0), "IdleConnTimeout should be non-zero")
+	require.Positive(t, tr.MaxIdleConns, "MaxIdleConns should be non-zero")
+	require.Greater(t, tr.TLSHandshakeTimeout, time.Duration(0), "TLSHandshakeTimeout should be non-zero")
+}