From 5fd79e1e5b43077953197ff0b715e7fb117f926e Mon Sep 17 00:00:00 2001
From: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Date: Thu, 3 Oct 2024 10:46:51 -0400
Subject: [PATCH 1/6] chore(cmp): clean up and test discovery logic

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
---
 .mockery.yaml                                 |   4 +
 .../ConfigManagementPluginServiceClient.go    | 182 ++++++++++++++++
 ...mentPluginService_MatchRepositoryClient.go | 204 ++++++++++++++++++
 cmpserver/apiclient/plugin.pb.go              |   5 +-
 cmpserver/plugin/plugin.go                    |  20 +-
 cmpserver/plugin/plugin.proto                 |   3 +
 cmpserver/plugin/plugin_test.go               |  34 ++-
 util/app/discovery/discovery.go               |  41 ++--
 util/app/discovery/discovery_test.go          |  76 +++++++
 9 files changed, 517 insertions(+), 52 deletions(-)
 create mode 100644 cmpserver/apiclient/mocks/ConfigManagementPluginServiceClient.go
 create mode 100644 cmpserver/apiclient/mocks/ConfigManagementPluginService_MatchRepositoryClient.go

diff --git a/.mockery.yaml b/.mockery.yaml
index 3a8b437ef347d..03b3072f3c97c 100644
--- a/.mockery.yaml
+++ b/.mockery.yaml
@@ -29,6 +29,10 @@ packages:
   github.com/argoproj/argo-cd/v2/controller/cache:
     interfaces:
       LiveStateCache:
+  github.com/argoproj/argo-cd/v2/cmpserver/apiclient:
+    interfaces:
+      ConfigManagementPluginServiceClient:
+      ConfigManagementPluginService_MatchRepositoryClient:
   github.com/argoproj/argo-cd/v2/reposerver/apiclient:
     interfaces:
       RepoServerServiceClient:
diff --git a/cmpserver/apiclient/mocks/ConfigManagementPluginServiceClient.go b/cmpserver/apiclient/mocks/ConfigManagementPluginServiceClient.go
new file mode 100644
index 0000000000000..18a508e0bf94c
--- /dev/null
+++ b/cmpserver/apiclient/mocks/ConfigManagementPluginServiceClient.go
@@ -0,0 +1,182 @@
+// Code generated by mockery v2.43.2. DO NOT EDIT.
+
+package mocks
+
+import (
+	context "context"
+
+	apiclient "github.com/argoproj/argo-cd/v2/cmpserver/apiclient"
+
+	emptypb "google.golang.org/protobuf/types/known/emptypb"
+
+	grpc "google.golang.org/grpc"
+
+	mock "github.com/stretchr/testify/mock"
+)
+
+// ConfigManagementPluginServiceClient is an autogenerated mock type for the ConfigManagementPluginServiceClient type
+type ConfigManagementPluginServiceClient struct {
+	mock.Mock
+}
+
+// CheckPluginConfiguration provides a mock function with given fields: ctx, in, opts
+func (_m *ConfigManagementPluginServiceClient) CheckPluginConfiguration(ctx context.Context, in *emptypb.Empty, opts ...grpc.CallOption) (*apiclient.CheckPluginConfigurationResponse, error) {
+	_va := make([]interface{}, len(opts))
+	for _i := range opts {
+		_va[_i] = opts[_i]
+	}
+	var _ca []interface{}
+	_ca = append(_ca, ctx, in)
+	_ca = append(_ca, _va...)
+	ret := _m.Called(_ca...)
+
+	if len(ret) == 0 {
+		panic("no return value specified for CheckPluginConfiguration")
+	}
+
+	var r0 *apiclient.CheckPluginConfigurationResponse
+	var r1 error
+	if rf, ok := ret.Get(0).(func(context.Context, *emptypb.Empty, ...grpc.CallOption) (*apiclient.CheckPluginConfigurationResponse, error)); ok {
+		return rf(ctx, in, opts...)
+	}
+	if rf, ok := ret.Get(0).(func(context.Context, *emptypb.Empty, ...grpc.CallOption) *apiclient.CheckPluginConfigurationResponse); ok {
+		r0 = rf(ctx, in, opts...)
+	} else {
+		if ret.Get(0) != nil {
+			r0 = ret.Get(0).(*apiclient.CheckPluginConfigurationResponse)
+		}
+	}
+
+	if rf, ok := ret.Get(1).(func(context.Context, *emptypb.Empty, ...grpc.CallOption) error); ok {
+		r1 = rf(ctx, in, opts...)
+	} else {
+		r1 = ret.Error(1)
+	}
+
+	return r0, r1
+}
+
+// GenerateManifest provides a mock function with given fields: ctx, opts
+func (_m *ConfigManagementPluginServiceClient) GenerateManifest(ctx context.Context, opts ...grpc.CallOption) (apiclient.ConfigManagementPluginService_GenerateManifestClient, error) {
+	_va := make([]interface{}, len(opts))
+	for _i := range opts {
+		_va[_i] = opts[_i]
+	}
+	var _ca []interface{}
+	_ca = append(_ca, ctx)
+	_ca = append(_ca, _va...)
+	ret := _m.Called(_ca...)
+
+	if len(ret) == 0 {
+		panic("no return value specified for GenerateManifest")
+	}
+
+	var r0 apiclient.ConfigManagementPluginService_GenerateManifestClient
+	var r1 error
+	if rf, ok := ret.Get(0).(func(context.Context, ...grpc.CallOption) (apiclient.ConfigManagementPluginService_GenerateManifestClient, error)); ok {
+		return rf(ctx, opts...)
+	}
+	if rf, ok := ret.Get(0).(func(context.Context, ...grpc.CallOption) apiclient.ConfigManagementPluginService_GenerateManifestClient); ok {
+		r0 = rf(ctx, opts...)
+	} else {
+		if ret.Get(0) != nil {
+			r0 = ret.Get(0).(apiclient.ConfigManagementPluginService_GenerateManifestClient)
+		}
+	}
+
+	if rf, ok := ret.Get(1).(func(context.Context, ...grpc.CallOption) error); ok {
+		r1 = rf(ctx, opts...)
+	} else {
+		r1 = ret.Error(1)
+	}
+
+	return r0, r1
+}
+
+// GetParametersAnnouncement provides a mock function with given fields: ctx, opts
+func (_m *ConfigManagementPluginServiceClient) GetParametersAnnouncement(ctx context.Context, opts ...grpc.CallOption) (apiclient.ConfigManagementPluginService_GetParametersAnnouncementClient, error) {
+	_va := make([]interface{}, len(opts))
+	for _i := range opts {
+		_va[_i] = opts[_i]
+	}
+	var _ca []interface{}
+	_ca = append(_ca, ctx)
+	_ca = append(_ca, _va...)
+	ret := _m.Called(_ca...)
+
+	if len(ret) == 0 {
+		panic("no return value specified for GetParametersAnnouncement")
+	}
+
+	var r0 apiclient.ConfigManagementPluginService_GetParametersAnnouncementClient
+	var r1 error
+	if rf, ok := ret.Get(0).(func(context.Context, ...grpc.CallOption) (apiclient.ConfigManagementPluginService_GetParametersAnnouncementClient, error)); ok {
+		return rf(ctx, opts...)
+	}
+	if rf, ok := ret.Get(0).(func(context.Context, ...grpc.CallOption) apiclient.ConfigManagementPluginService_GetParametersAnnouncementClient); ok {
+		r0 = rf(ctx, opts...)
+	} else {
+		if ret.Get(0) != nil {
+			r0 = ret.Get(0).(apiclient.ConfigManagementPluginService_GetParametersAnnouncementClient)
+		}
+	}
+
+	if rf, ok := ret.Get(1).(func(context.Context, ...grpc.CallOption) error); ok {
+		r1 = rf(ctx, opts...)
+	} else {
+		r1 = ret.Error(1)
+	}
+
+	return r0, r1
+}
+
+// MatchRepository provides a mock function with given fields: ctx, opts
+func (_m *ConfigManagementPluginServiceClient) MatchRepository(ctx context.Context, opts ...grpc.CallOption) (apiclient.ConfigManagementPluginService_MatchRepositoryClient, error) {
+	_va := make([]interface{}, len(opts))
+	for _i := range opts {
+		_va[_i] = opts[_i]
+	}
+	var _ca []interface{}
+	_ca = append(_ca, ctx)
+	_ca = append(_ca, _va...)
+	ret := _m.Called(_ca...)
+
+	if len(ret) == 0 {
+		panic("no return value specified for MatchRepository")
+	}
+
+	var r0 apiclient.ConfigManagementPluginService_MatchRepositoryClient
+	var r1 error
+	if rf, ok := ret.Get(0).(func(context.Context, ...grpc.CallOption) (apiclient.ConfigManagementPluginService_MatchRepositoryClient, error)); ok {
+		return rf(ctx, opts...)
+	}
+	if rf, ok := ret.Get(0).(func(context.Context, ...grpc.CallOption) apiclient.ConfigManagementPluginService_MatchRepositoryClient); ok {
+		r0 = rf(ctx, opts...)
+	} else {
+		if ret.Get(0) != nil {
+			r0 = ret.Get(0).(apiclient.ConfigManagementPluginService_MatchRepositoryClient)
+		}
+	}
+
+	if rf, ok := ret.Get(1).(func(context.Context, ...grpc.CallOption) error); ok {
+		r1 = rf(ctx, opts...)
+	} else {
+		r1 = ret.Error(1)
+	}
+
+	return r0, r1
+}
+
+// NewConfigManagementPluginServiceClient creates a new instance of ConfigManagementPluginServiceClient. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations.
+// The first argument is typically a *testing.T value.
+func NewConfigManagementPluginServiceClient(t interface {
+	mock.TestingT
+	Cleanup(func())
+}) *ConfigManagementPluginServiceClient {
+	mock := &ConfigManagementPluginServiceClient{}
+	mock.Mock.Test(t)
+
+	t.Cleanup(func() { mock.AssertExpectations(t) })
+
+	return mock
+}
diff --git a/cmpserver/apiclient/mocks/ConfigManagementPluginService_MatchRepositoryClient.go b/cmpserver/apiclient/mocks/ConfigManagementPluginService_MatchRepositoryClient.go
new file mode 100644
index 0000000000000..948164bfc577a
--- /dev/null
+++ b/cmpserver/apiclient/mocks/ConfigManagementPluginService_MatchRepositoryClient.go
@@ -0,0 +1,204 @@
+// Code generated by mockery v2.43.2. DO NOT EDIT.
+
+package mocks
+
+import (
+	context "context"
+
+	apiclient "github.com/argoproj/argo-cd/v2/cmpserver/apiclient"
+
+	metadata "google.golang.org/grpc/metadata"
+
+	mock "github.com/stretchr/testify/mock"
+)
+
+// ConfigManagementPluginService_MatchRepositoryClient is an autogenerated mock type for the ConfigManagementPluginService_MatchRepositoryClient type
+type ConfigManagementPluginService_MatchRepositoryClient struct {
+	mock.Mock
+}
+
+// CloseAndRecv provides a mock function with given fields:
+func (_m *ConfigManagementPluginService_MatchRepositoryClient) CloseAndRecv() (*apiclient.RepositoryResponse, error) {
+	ret := _m.Called()
+
+	if len(ret) == 0 {
+		panic("no return value specified for CloseAndRecv")
+	}
+
+	var r0 *apiclient.RepositoryResponse
+	var r1 error
+	if rf, ok := ret.Get(0).(func() (*apiclient.RepositoryResponse, error)); ok {
+		return rf()
+	}
+	if rf, ok := ret.Get(0).(func() *apiclient.RepositoryResponse); ok {
+		r0 = rf()
+	} else {
+		if ret.Get(0) != nil {
+			r0 = ret.Get(0).(*apiclient.RepositoryResponse)
+		}
+	}
+
+	if rf, ok := ret.Get(1).(func() error); ok {
+		r1 = rf()
+	} else {
+		r1 = ret.Error(1)
+	}
+
+	return r0, r1
+}
+
+// CloseSend provides a mock function with given fields:
+func (_m *ConfigManagementPluginService_MatchRepositoryClient) CloseSend() error {
+	ret := _m.Called()
+
+	if len(ret) == 0 {
+		panic("no return value specified for CloseSend")
+	}
+
+	var r0 error
+	if rf, ok := ret.Get(0).(func() error); ok {
+		r0 = rf()
+	} else {
+		r0 = ret.Error(0)
+	}
+
+	return r0
+}
+
+// Context provides a mock function with given fields:
+func (_m *ConfigManagementPluginService_MatchRepositoryClient) Context() context.Context {
+	ret := _m.Called()
+
+	if len(ret) == 0 {
+		panic("no return value specified for Context")
+	}
+
+	var r0 context.Context
+	if rf, ok := ret.Get(0).(func() context.Context); ok {
+		r0 = rf()
+	} else {
+		if ret.Get(0) != nil {
+			r0 = ret.Get(0).(context.Context)
+		}
+	}
+
+	return r0
+}
+
+// Header provides a mock function with given fields:
+func (_m *ConfigManagementPluginService_MatchRepositoryClient) Header() (metadata.MD, error) {
+	ret := _m.Called()
+
+	if len(ret) == 0 {
+		panic("no return value specified for Header")
+	}
+
+	var r0 metadata.MD
+	var r1 error
+	if rf, ok := ret.Get(0).(func() (metadata.MD, error)); ok {
+		return rf()
+	}
+	if rf, ok := ret.Get(0).(func() metadata.MD); ok {
+		r0 = rf()
+	} else {
+		if ret.Get(0) != nil {
+			r0 = ret.Get(0).(metadata.MD)
+		}
+	}
+
+	if rf, ok := ret.Get(1).(func() error); ok {
+		r1 = rf()
+	} else {
+		r1 = ret.Error(1)
+	}
+
+	return r0, r1
+}
+
+// RecvMsg provides a mock function with given fields: m
+func (_m *ConfigManagementPluginService_MatchRepositoryClient) RecvMsg(m interface{}) error {
+	ret := _m.Called(m)
+
+	if len(ret) == 0 {
+		panic("no return value specified for RecvMsg")
+	}
+
+	var r0 error
+	if rf, ok := ret.Get(0).(func(interface{}) error); ok {
+		r0 = rf(m)
+	} else {
+		r0 = ret.Error(0)
+	}
+
+	return r0
+}
+
+// Send provides a mock function with given fields: _a0
+func (_m *ConfigManagementPluginService_MatchRepositoryClient) Send(_a0 *apiclient.AppStreamRequest) error {
+	ret := _m.Called(_a0)
+
+	if len(ret) == 0 {
+		panic("no return value specified for Send")
+	}
+
+	var r0 error
+	if rf, ok := ret.Get(0).(func(*apiclient.AppStreamRequest) error); ok {
+		r0 = rf(_a0)
+	} else {
+		r0 = ret.Error(0)
+	}
+
+	return r0
+}
+
+// SendMsg provides a mock function with given fields: m
+func (_m *ConfigManagementPluginService_MatchRepositoryClient) SendMsg(m interface{}) error {
+	ret := _m.Called(m)
+
+	if len(ret) == 0 {
+		panic("no return value specified for SendMsg")
+	}
+
+	var r0 error
+	if rf, ok := ret.Get(0).(func(interface{}) error); ok {
+		r0 = rf(m)
+	} else {
+		r0 = ret.Error(0)
+	}
+
+	return r0
+}
+
+// Trailer provides a mock function with given fields:
+func (_m *ConfigManagementPluginService_MatchRepositoryClient) Trailer() metadata.MD {
+	ret := _m.Called()
+
+	if len(ret) == 0 {
+		panic("no return value specified for Trailer")
+	}
+
+	var r0 metadata.MD
+	if rf, ok := ret.Get(0).(func() metadata.MD); ok {
+		r0 = rf()
+	} else {
+		if ret.Get(0) != nil {
+			r0 = ret.Get(0).(metadata.MD)
+		}
+	}
+
+	return r0
+}
+
+// NewConfigManagementPluginService_MatchRepositoryClient creates a new instance of ConfigManagementPluginService_MatchRepositoryClient. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations.
+// The first argument is typically a *testing.T value.
+func NewConfigManagementPluginService_MatchRepositoryClient(t interface {
+	mock.TestingT
+	Cleanup(func())
+}) *ConfigManagementPluginService_MatchRepositoryClient {
+	mock := &ConfigManagementPluginService_MatchRepositoryClient{}
+	mock.Mock.Test(t)
+
+	t.Cleanup(func() { mock.AssertExpectations(t) })
+
+	return mock
+}
diff --git a/cmpserver/apiclient/plugin.pb.go b/cmpserver/apiclient/plugin.pb.go
index b6fb8fca109b9..e64adfb5b6ebf 100644
--- a/cmpserver/apiclient/plugin.pb.go
+++ b/cmpserver/apiclient/plugin.pb.go
@@ -318,7 +318,10 @@ func (m *ManifestResponse) GetSourceType() string {
 }
 
 type RepositoryResponse struct {
-	IsSupported          bool     `protobuf:"varint,1,opt,name=isSupported,proto3" json:"isSupported,omitempty"`
+	IsSupported bool `protobuf:"varint,1,opt,name=isSupported,proto3" json:"isSupported,omitempty"`
+	// IsDiscoveryEnabled is a flag that indicates if the CMP supports discovery.
+	//
+	// Deprecated: Use the IsDiscoveryConfigured field from the CheckPluginConfigurationResponse instead.
 	IsDiscoveryEnabled   bool     `protobuf:"varint,2,opt,name=isDiscoveryEnabled,proto3" json:"isDiscoveryEnabled,omitempty"`
 	XXX_NoUnkeyedLiteral struct{} `json:"-"`
 	XXX_unrecognized     []byte   `json:"-"`
diff --git a/cmpserver/plugin/plugin.go b/cmpserver/plugin/plugin.go
index ee00f66ad9ecf..2e9330da3f61a 100644
--- a/cmpserver/plugin/plugin.go
+++ b/cmpserver/plugin/plugin.go
@@ -297,11 +297,11 @@ func (s *Service) matchRepositoryGeneric(stream MatchRepositoryStream) error {
 		return fmt.Errorf("match repository error receiving stream: %w", err)
 	}
 
-	isSupported, isDiscoveryEnabled, err := s.matchRepository(bufferedCtx, workDir, metadata.GetEnv(), metadata.GetAppRelPath())
+	isSupported, err := s.matchRepository(bufferedCtx, workDir, metadata.GetEnv(), metadata.GetAppRelPath())
 	if err != nil {
 		return fmt.Errorf("match repository error: %w", err)
 	}
-	repoResponse := &apiclient.RepositoryResponse{IsSupported: isSupported, IsDiscoveryEnabled: isDiscoveryEnabled}
+	repoResponse := &apiclient.RepositoryResponse{IsSupported: isSupported}
 
 	err = stream.SendAndClose(repoResponse)
 	if err != nil {
@@ -310,7 +310,7 @@ func (s *Service) matchRepositoryGeneric(stream MatchRepositoryStream) error {
 	return nil
 }
 
-func (s *Service) matchRepository(ctx context.Context, workdir string, envEntries []*apiclient.EnvEntry, appRelPath string) (isSupported bool, isDiscoveryEnabled bool, err error) {
+func (s *Service) matchRepository(ctx context.Context, workdir string, envEntries []*apiclient.EnvEntry, appRelPath string) (isSupported bool, err error) {
 	config := s.initConstants.PluginConfig
 
 	appPath, err := securejoin.SecureJoin(workdir, appRelPath)
@@ -328,9 +328,9 @@ func (s *Service) matchRepository(ctx context.Context, workdir string, envEntrie
 		if err != nil {
 			e := fmt.Errorf("error finding filename match for pattern %q: %w", pattern, err)
 			log.Debug(e)
-			return false, true, e
+			return false, e
 		}
-		return len(matches) > 0, true, nil
+		return len(matches) > 0, nil
 	}
 
 	if config.Spec.Discover.Find.Glob != "" {
@@ -342,10 +342,10 @@ func (s *Service) matchRepository(ctx context.Context, workdir string, envEntrie
 		if err != nil {
 			e := fmt.Errorf("error finding glob match for pattern %q: %w", pattern, err)
 			log.Debug(e)
-			return false, true, e
+			return false, e
 		}
 
-		return len(matches) > 0, true, nil
+		return len(matches) > 0, nil
 	}
 
 	if len(config.Spec.Discover.Find.Command.Command) > 0 {
@@ -353,12 +353,12 @@ func (s *Service) matchRepository(ctx context.Context, workdir string, envEntrie
 		env := append(os.Environ(), environ(envEntries)...)
 		find, err := runCommand(ctx, config.Spec.Discover.Find.Command, appPath, env)
 		if err != nil {
-			return false, true, fmt.Errorf("error running find command: %w", err)
+			return false, fmt.Errorf("error running find command: %w", err)
 		}
-		return find != "", true, nil
+		return find != "", nil
 	}
 
-	return false, false, nil
+	return false, nil
 }
 
 // ParametersAnnouncementStream defines an interface able to send/receive a stream of parameter announcements.
diff --git a/cmpserver/plugin/plugin.proto b/cmpserver/plugin/plugin.proto
index 6f5b0d0cbf7b6..47667b2a9c320 100644
--- a/cmpserver/plugin/plugin.proto
+++ b/cmpserver/plugin/plugin.proto
@@ -45,6 +45,9 @@ message ManifestResponse {
 
 message RepositoryResponse {
     bool isSupported = 1;
+    // IsDiscoveryEnabled is a flag that indicates if the CMP supports discovery.
+    //
+    // Deprecated: Use the IsDiscoveryConfigured field from the CheckPluginConfigurationResponse instead.
     bool isDiscoveryEnabled = 2;
 }
 
diff --git a/cmpserver/plugin/plugin_test.go b/cmpserver/plugin/plugin_test.go
index 05001c31b3837..488376bf2a4a4 100644
--- a/cmpserver/plugin/plugin_test.go
+++ b/cmpserver/plugin/plugin_test.go
@@ -100,12 +100,11 @@ func TestMatchRepository(t *testing.T) {
 		f := setup(t, withDiscover(d))
 
 		// when
-		match, discovery, err := f.service.matchRepository(context.Background(), f.path, f.env, ".")
+		match, err := f.service.matchRepository(context.Background(), f.path, f.env, ".")
 
 		// then
 		require.NoError(t, err)
 		assert.True(t, match)
-		assert.True(t, discovery)
 	})
 	t.Run("will not match plugin by filename if file not found", func(t *testing.T) {
 		// given
@@ -115,12 +114,11 @@ func TestMatchRepository(t *testing.T) {
 		f := setup(t, withDiscover(d))
 
 		// when
-		match, discovery, err := f.service.matchRepository(context.Background(), f.path, f.env, ".")
+		match, err := f.service.matchRepository(context.Background(), f.path, f.env, ".")
 
 		// then
 		require.NoError(t, err)
 		assert.False(t, match)
-		assert.True(t, discovery)
 	})
 	t.Run("will not match a pattern with a syntax error", func(t *testing.T) {
 		// given
@@ -130,7 +128,7 @@ func TestMatchRepository(t *testing.T) {
 		f := setup(t, withDiscover(d))
 
 		// when
-		_, _, err := f.service.matchRepository(context.Background(), f.path, f.env, ".")
+		_, err := f.service.matchRepository(context.Background(), f.path, f.env, ".")
 
 		// then
 		require.ErrorContains(t, err, "syntax error")
@@ -145,12 +143,11 @@ func TestMatchRepository(t *testing.T) {
 		f := setup(t, withDiscover(d))
 
 		// when
-		match, discovery, err := f.service.matchRepository(context.Background(), f.path, f.env, ".")
+		match, err := f.service.matchRepository(context.Background(), f.path, f.env, ".")
 
 		// then
 		require.NoError(t, err)
 		assert.True(t, match)
-		assert.True(t, discovery)
 	})
 	t.Run("will not match plugin by glob if not found", func(t *testing.T) {
 		// given
@@ -162,12 +159,11 @@ func TestMatchRepository(t *testing.T) {
 		f := setup(t, withDiscover(d))
 
 		// when
-		match, discovery, err := f.service.matchRepository(context.Background(), f.path, f.env, ".")
+		match, err := f.service.matchRepository(context.Background(), f.path, f.env, ".")
 
 		// then
 		require.NoError(t, err)
 		assert.False(t, match)
-		assert.True(t, discovery)
 	})
 	t.Run("will throw an error for a bad pattern", func(t *testing.T) {
 		// given
@@ -179,7 +175,7 @@ func TestMatchRepository(t *testing.T) {
 		f := setup(t, withDiscover(d))
 
 		// when
-		_, _, err := f.service.matchRepository(context.Background(), f.path, f.env, ".")
+		_, err := f.service.matchRepository(context.Background(), f.path, f.env, ".")
 
 		// then
 		require.ErrorContains(t, err, "error finding glob match for pattern")
@@ -196,12 +192,11 @@ func TestMatchRepository(t *testing.T) {
 		f := setup(t, withDiscover(d))
 
 		// when
-		match, discovery, err := f.service.matchRepository(context.Background(), f.path, f.env, ".")
+		match, err := f.service.matchRepository(context.Background(), f.path, f.env, ".")
 
 		// then
 		require.NoError(t, err)
 		assert.True(t, match)
-		assert.True(t, discovery)
 	})
 	t.Run("will not match plugin by command when returns no output", func(t *testing.T) {
 		// given
@@ -215,11 +210,10 @@ func TestMatchRepository(t *testing.T) {
 		f := setup(t, withDiscover(d))
 
 		// when
-		match, discovery, err := f.service.matchRepository(context.Background(), f.path, f.env, ".")
+		match, err := f.service.matchRepository(context.Background(), f.path, f.env, ".")
 		// then
 		require.NoError(t, err)
 		assert.False(t, match)
-		assert.True(t, discovery)
 	})
 	t.Run("will match plugin because env var defined", func(t *testing.T) {
 		// given
@@ -233,12 +227,11 @@ func TestMatchRepository(t *testing.T) {
 		f := setup(t, withDiscover(d))
 
 		// when
-		match, discovery, err := f.service.matchRepository(context.Background(), f.path, f.env, ".")
+		match, err := f.service.matchRepository(context.Background(), f.path, f.env, ".")
 
 		// then
 		require.NoError(t, err)
 		assert.True(t, match)
-		assert.True(t, discovery)
 	})
 	t.Run("will not match plugin because no env var defined", func(t *testing.T) {
 		// given
@@ -253,12 +246,11 @@ func TestMatchRepository(t *testing.T) {
 		f := setup(t, withDiscover(d))
 
 		// when
-		match, discovery, err := f.service.matchRepository(context.Background(), f.path, f.env, ".")
+		match, err := f.service.matchRepository(context.Background(), f.path, f.env, ".")
 
 		// then
 		require.NoError(t, err)
 		assert.False(t, match)
-		assert.True(t, discovery)
 	})
 	t.Run("will not match plugin by command when command fails", func(t *testing.T) {
 		// given
@@ -272,12 +264,11 @@ func TestMatchRepository(t *testing.T) {
 		f := setup(t, withDiscover(d))
 
 		// when
-		match, discovery, err := f.service.matchRepository(context.Background(), f.path, f.env, ".")
+		match, err := f.service.matchRepository(context.Background(), f.path, f.env, ".")
 
 		// then
 		require.Error(t, err)
 		assert.False(t, match)
-		assert.True(t, discovery)
 	})
 	t.Run("will not match plugin as discovery is not set", func(t *testing.T) {
 		// given
@@ -285,12 +276,11 @@ func TestMatchRepository(t *testing.T) {
 		f := setup(t, withDiscover(d))
 
 		// when
-		match, discovery, err := f.service.matchRepository(context.Background(), f.path, f.env, ".")
+		match, err := f.service.matchRepository(context.Background(), f.path, f.env, ".")
 
 		// then
 		require.NoError(t, err)
 		assert.False(t, match)
-		assert.False(t, discovery)
 	})
 }
 
diff --git a/util/app/discovery/discovery.go b/util/app/discovery/discovery.go
index df80e3bf987c2..d6c923a74268c 100644
--- a/util/app/discovery/discovery.go
+++ b/util/app/discovery/discovery.go
@@ -127,21 +127,21 @@ func DetectConfigManagementPlugin(ctx context.Context, appPath, repoPath, plugin
 // matchRepositoryCMP will send the repoPath to the cmp-server. The cmp-server will
 // inspect the files and return true if the repo is supported for manifest generation.
 // Will return false otherwise.
-func matchRepositoryCMP(ctx context.Context, appPath, repoPath string, client pluginclient.ConfigManagementPluginServiceClient, env []string, tarExcludedGlobs []string) (bool, bool, error) {
+func matchRepositoryCMP(ctx context.Context, appPath, repoPath string, client pluginclient.ConfigManagementPluginServiceClient, env, tarExcludedGlobs []string) (bool, error) {
 	matchRepoStream, err := client.MatchRepository(ctx, grpc_retry.Disable())
 	if err != nil {
-		return false, false, fmt.Errorf("error getting stream client: %w", err)
+		return false, fmt.Errorf("error getting stream client: %w", err)
 	}
 
 	err = cmp.SendRepoStream(ctx, appPath, repoPath, matchRepoStream, env, tarExcludedGlobs)
 	if err != nil {
-		return false, false, fmt.Errorf("error sending stream: %w", err)
+		return false, fmt.Errorf("error sending stream: %w", err)
 	}
 	resp, err := matchRepoStream.CloseAndRecv()
 	if err != nil {
-		return false, false, fmt.Errorf("error receiving stream response: %w", err)
+		return false, fmt.Errorf("error receiving stream response: %w", err)
 	}
-	return resp.GetIsSupported(), resp.GetIsDiscoveryEnabled(), nil
+	return resp.GetIsSupported(), nil
 }
 
 func cmpSupports(ctx context.Context, pluginSockFilePath, appPath, repoPath, fileName string, env []string, tarExcludedGlobs []string, namedPlugin bool) (io.Closer, pluginclient.ConfigManagementPluginServiceClient, bool) {
@@ -172,33 +172,36 @@ func cmpSupports(ctx context.Context, pluginSockFilePath, appPath, repoPath, fil
 		log.Errorf("error checking plugin configuration %s, %v", fileName, err)
 		return nil, nil, false
 	}
+	usePlugin := cmpSupportsForClient(ctx, cmpClient, appPath, repoPath, env, tarExcludedGlobs, cfg.IsDiscoveryConfigured, namedPlugin)
+	if !usePlugin {
+		io.Close(conn)
+		return nil, nil, false
+	}
+	return conn, cmpClient, true
+}
 
-	// if discovery is not configured, return the client without further checks
-	if !cfg.IsDiscoveryConfigured {
-		return conn, cmpClient, true
+// cmpSupportsForClient will check if the given plugin should be used for the given repository and plugin client.
+func cmpSupportsForClient(ctx context.Context, cmpClient pluginclient.ConfigManagementPluginServiceClient, appPath string, repoPath string, env []string, tarExcludedGlobs []string, isDiscoveryConfigured bool, namedPlugin bool) bool {
+	if !isDiscoveryConfigured {
+		// if discovery is not configured, return the client without further checks
+		return true
 	}
 
-	isSupported, isDiscoveryEnabled, err := matchRepositoryCMP(ctx, appPath, repoPath, cmpClient, env, tarExcludedGlobs)
+	isSupported, err := matchRepositoryCMP(ctx, appPath, repoPath, cmpClient, env, tarExcludedGlobs)
 	if err != nil {
 		log.WithFields(log.Fields{
 			common.SecurityField:    common.SecurityMedium,
 			common.SecurityCWEField: common.SecurityCWEMissingReleaseOfFileDescriptor,
 		}).Errorf("repository %s is not the match because %v", repoPath, err)
-		io.Close(conn)
-		return nil, nil, false
+		return false
 	}
 
 	if !isSupported {
-		// if discovery is not set and the plugin name is specified, let app use the plugin
-		if !isDiscoveryEnabled && namedPlugin {
-			return conn, cmpClient, true
-		}
 		log.WithFields(log.Fields{
 			common.SecurityField:    common.SecurityLow,
 			common.SecurityCWEField: common.SecurityCWEMissingReleaseOfFileDescriptor,
-		}).Debugf("Response from socket file %s does not support %v", fileName, repoPath)
-		io.Close(conn)
-		return nil, nil, false
+		}).Debugf("Plugin does not support %v", repoPath)
+		return false
 	}
-	return conn, cmpClient, true
+	return true
 }
diff --git a/util/app/discovery/discovery_test.go b/util/app/discovery/discovery_test.go
index c8340847e7702..fa639e6ac140e 100644
--- a/util/app/discovery/discovery_test.go
+++ b/util/app/discovery/discovery_test.go
@@ -2,6 +2,9 @@ package discovery
 
 import (
 	"context"
+	"github.com/argoproj/argo-cd/v2/cmpserver/apiclient"
+	"github.com/argoproj/argo-cd/v2/cmpserver/apiclient/mocks"
+	"github.com/stretchr/testify/mock"
 	"testing"
 
 	"github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
@@ -50,3 +53,76 @@ func TestAppType_Disabled(t *testing.T) {
 	require.NoError(t, err)
 	assert.Equal(t, "Directory", appType)
 }
+
+func Test_cmpSupportsForClient(t *testing.T) {
+	t.Parallel()
+
+	testCases := []struct {
+		name                  string
+		namedPlugin           bool
+		isDiscoveryConfigured bool
+		isSupported           bool
+		expected              bool
+	}{
+		{
+			name:                  "named plugin, no discovery",
+			namedPlugin:           true,
+			isDiscoveryConfigured: false,
+			expected:              true,
+		},
+		{
+			name:                  "discovery configured, repo not supported, not named plugin",
+			namedPlugin:           false,
+			isDiscoveryConfigured: true,
+			isSupported:           false,
+			expected:              false,
+		},
+		{
+			// If it's a named plugin and discovery is configured, we want the discovery rules to apply.
+			name:                  "discovery configured, repo not supported, named plugin",
+			namedPlugin:           true,
+			isDiscoveryConfigured: true,
+			isSupported:           false,
+			expected:              false,
+		},
+		{
+			name:                  "plugin not named and discovery not configured",
+			namedPlugin:           false,
+			isDiscoveryConfigured: false,
+			expected:              true,
+		},
+		{
+			name:                  "discovery configured, not named plugin",
+			namedPlugin:           false,
+			isDiscoveryConfigured: true,
+			isSupported:           true,
+			expected:              true,
+		},
+		{
+			name:                  "discovery configured and named plugin",
+			namedPlugin:           true,
+			isDiscoveryConfigured: true,
+			isSupported:           true,
+			expected:              true,
+		},
+	}
+
+	for _, tc := range testCases {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			rc := mocks.NewConfigManagementPluginService_MatchRepositoryClient(t)
+			rc.On("Send", mock.Anything).Maybe().Return(nil)
+			rc.On("CloseAndRecv").Maybe().Return(&apiclient.RepositoryResponse{
+				IsSupported: tc.isSupported,
+			}, nil)
+
+			c := mocks.NewConfigManagementPluginServiceClient(t)
+			c.On("MatchRepository", mock.Anything, mock.Anything).Maybe().Return(rc, nil)
+
+			actual := cmpSupportsForClient(context.Background(), c, "./testdata", "./testdata", nil, nil, tc.isDiscoveryConfigured, tc.namedPlugin)
+			assert.Equal(t, tc.expected, actual)
+		})
+	}
+}

From 2f197e24b76a10733762431c3f44e498feeca780 Mon Sep 17 00:00:00 2001
From: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Date: Thu, 3 Oct 2024 12:24:26 -0400
Subject: [PATCH 2/6] refactor

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
---
 .mockery.yaml                                 |   4 +
 util/app/discovery/discovery.go               | 166 ++++++++++++------
 util/app/discovery/discovery_test.go          |  34 +++-
 .../discovery/mocks/CMPClientConstructor.go   |  69 ++++++++
 4 files changed, 209 insertions(+), 64 deletions(-)
 create mode 100644 util/app/discovery/mocks/CMPClientConstructor.go

diff --git a/.mockery.yaml b/.mockery.yaml
index 03b3072f3c97c..181bd271f60d3 100644
--- a/.mockery.yaml
+++ b/.mockery.yaml
@@ -48,6 +48,10 @@ packages:
       RbacEnforcer:
       SettingsGetter:
       UserGetter:
+  github.com/argoproj/argo-cd/v2/util/app/discovery:
+    interfaces:
+      CMPClientConstructor:
+
   github.com/argoproj/argo-cd/v2/util/db:
     interfaces:
       ArgoDB:
diff --git a/util/app/discovery/discovery.go b/util/app/discovery/discovery.go
index d6c923a74268c..9205acd668c6c 100644
--- a/util/app/discovery/discovery.go
+++ b/util/app/discovery/discovery.go
@@ -3,14 +3,13 @@ package discovery
 import (
 	"context"
 	"fmt"
+	securejoin "github.com/cyphar/filepath-securejoin"
 	"os"
 	"path/filepath"
 	"strings"
 
 	"github.com/golang/protobuf/ptypes/empty"
 
-	"github.com/argoproj/argo-cd/v2/util/io/files"
-
 	grpc_retry "github.com/grpc-ecosystem/go-grpc-middleware/retry"
 	log "github.com/sirupsen/logrus"
 
@@ -81,16 +80,9 @@ func AppType(ctx context.Context, appPath, repoPath string, enableGenerateManife
 	return "Directory", nil
 }
 
-// if pluginName is provided setup connection to that cmp-server
-// else
-// list all plugins in /plugins folder and foreach plugin
-// check cmpSupports()
-// if supported return conn for the cmp-server
-
 func DetectConfigManagementPlugin(ctx context.Context, appPath, repoPath, pluginName string, env []string, tarExcludedGlobs []string) (io.Closer, pluginclient.ConfigManagementPluginServiceClient, error) {
 	var conn io.Closer
 	var cmpClient pluginclient.ConfigManagementPluginServiceClient
-	var connFound bool
 
 	pluginSockFilePath := common.GetPluginSockFilePath()
 	log.WithFields(log.Fields{
@@ -99,94 +91,133 @@ func DetectConfigManagementPlugin(ctx context.Context, appPath, repoPath, plugin
 	}).Debugf("pluginSockFilePath is: %s", pluginSockFilePath)
 
 	if pluginName != "" {
-		// check if the given plugin supports the repo
-		conn, cmpClient, connFound = cmpSupports(ctx, pluginSockFilePath, appPath, repoPath, fmt.Sprintf("%v.sock", pluginName), env, tarExcludedGlobs, true)
-		if !connFound {
+		c := newSocketCMPClientConstructorForPluginName(pluginSockFilePath, pluginName)
+		cmpClient, conn = namedCMPSupports(ctx, c, appPath, repoPath, env, tarExcludedGlobs)
+		if cmpClient == nil {
 			return nil, nil, fmt.Errorf("couldn't find cmp-server plugin with name %q supporting the given repository", pluginName)
 		}
 	} else {
 		fileList, err := os.ReadDir(pluginSockFilePath)
 		if err != nil {
-			return nil, nil, fmt.Errorf("Failed to list all plugins in dir, error=%w", err)
+			return nil, nil, fmt.Errorf("failed to list all plugins in dir: %w", err)
 		}
 		for _, file := range fileList {
 			if file.Type() == os.ModeSocket {
-				conn, cmpClient, connFound = cmpSupports(ctx, pluginSockFilePath, appPath, repoPath, file.Name(), env, tarExcludedGlobs, false)
-				if connFound {
+				c := newSocketCMPClientConstructorForPath(pluginSockFilePath, file.Name())
+				cmpClient, conn = unnamedCMPSupports(ctx, c, appPath, repoPath, env, tarExcludedGlobs)
+				if cmpClient != nil {
 					break
 				}
 			}
 		}
-		if !connFound {
+		if cmpClient == nil {
 			return nil, nil, fmt.Errorf("could not find plugin supporting the given repository")
 		}
 	}
 	return conn, cmpClient, nil
 }
 
-// matchRepositoryCMP will send the repoPath to the cmp-server. The cmp-server will
-// inspect the files and return true if the repo is supported for manifest generation.
-// Will return false otherwise.
-func matchRepositoryCMP(ctx context.Context, appPath, repoPath string, client pluginclient.ConfigManagementPluginServiceClient, env, tarExcludedGlobs []string) (bool, error) {
-	matchRepoStream, err := client.MatchRepository(ctx, grpc_retry.Disable())
+// namedCMPSupports will return a client for the named plugin if it supports the given repository. It will also return
+// a closer for the connection.
+func namedCMPSupports(ctx context.Context, c CMPClientConstructor, appPath, repoPath string, env, tarExcludedGlobs []string) (pluginclient.ConfigManagementPluginServiceClient, io.Closer) {
+	cmpClient, closer, isDiscoveryConfigured, err := getClientForFilepath(ctx, c)
 	if err != nil {
-		return false, fmt.Errorf("error getting stream client: %w", err)
+		log.Errorf("error getting client for plugin: %v", err)
+		return nil, nil
 	}
+	if !isDiscoveryConfigured {
+		// If discovery isn't configured, assume the CMP supports the plugin since it was explicitly named.
+		return cmpClient, closer
+	}
+	usePlugin := cmpSupportsForClient(ctx, cmpClient, appPath, repoPath, env, tarExcludedGlobs)
+	if !usePlugin {
+		io.Close(closer)
+		return nil, nil
+	}
+	return cmpClient, closer
+}
 
-	err = cmp.SendRepoStream(ctx, appPath, repoPath, matchRepoStream, env, tarExcludedGlobs)
+// unnamedCMPSupports will return a client if the plugin supports the given repository. It will also return a closer for
+// the connection.
+func unnamedCMPSupports(ctx context.Context, c CMPClientConstructor, appPath, repoPath string, env, tarExcludedGlobs []string) (pluginclient.ConfigManagementPluginServiceClient, io.Closer) {
+	cmpClient, closer, isDiscoveryConfigured, err := getClientForFilepath(ctx, c)
 	if err != nil {
-		return false, fmt.Errorf("error sending stream: %w", err)
+		log.Errorf("error getting client for plugin: %v", err)
+		return nil, nil
 	}
-	resp, err := matchRepoStream.CloseAndRecv()
-	if err != nil {
-		return false, fmt.Errorf("error receiving stream response: %w", err)
+	if !isDiscoveryConfigured {
+		// If discovery isn't configured, we can't assume the CMP supports the repo.
+		return nil, nil
 	}
-	return resp.GetIsSupported(), nil
+	usePlugin := cmpSupportsForClient(ctx, cmpClient, appPath, repoPath, env, tarExcludedGlobs)
+	if !usePlugin {
+		io.Close(closer)
+		return nil, nil
+	}
+	return cmpClient, closer
+}
+
+// CMPClientConstructor is an interface for creating a client for a CMP.
+type CMPClientConstructor interface {
+	// NewConfigManagementPluginClient returns a client for the CMP. It also returns a closer for the connection.
+	NewConfigManagementPluginClient() (pluginclient.ConfigManagementPluginServiceClient, io.Closer, error)
+}
+
+// socketCMPClientConstructor is a CMPClientConstructor for a CMP that uses a socket file.
+type socketCMPClientConstructor struct {
+	pluginSockFilePath string
+	filename           string
 }
 
-func cmpSupports(ctx context.Context, pluginSockFilePath, appPath, repoPath, fileName string, env []string, tarExcludedGlobs []string, namedPlugin bool) (io.Closer, pluginclient.ConfigManagementPluginServiceClient, bool) {
-	absPluginSockFilePath, err := filepath.Abs(pluginSockFilePath)
+// newSocketCMPClientConstructorForPath returns a new socketCMPClientConstructor.
+func newSocketCMPClientConstructorForPath(pluginSockFilePath, filename string) CMPClientConstructor {
+	return socketCMPClientConstructor{
+		pluginSockFilePath: pluginSockFilePath,
+		filename:           filename,
+	}
+}
+
+// newSocketCMPClientConstructorForPluginName returns a new socketCMPClientConstructor for the given plugin name.
+func newSocketCMPClientConstructorForPluginName(pluginSockFilePath, pluginName string) CMPClientConstructor {
+	return newSocketCMPClientConstructorForPath(pluginSockFilePath, pluginName+".sock")
+}
+
+// NewConfigManagementPluginClient returns a client for the CMP. It also returns a closer for the connection.
+func (c socketCMPClientConstructor) NewConfigManagementPluginClient() (pluginclient.ConfigManagementPluginServiceClient, io.Closer, error) {
+	absPluginSockFilePath, err := filepath.Abs(c.pluginSockFilePath)
 	if err != nil {
-		log.Errorf("error getting absolute path for plugin socket dir %v, %v", pluginSockFilePath, err)
-		return nil, nil, false
+		return nil, nil, fmt.Errorf("error getting absolute path for plugin socket dir %q: %w", c.pluginSockFilePath, err)
 	}
-	address := filepath.Join(absPluginSockFilePath, fileName)
-	if !files.Inbound(address, absPluginSockFilePath) {
-		log.Errorf("invalid socket file path, %v is outside plugin socket dir %v", fileName, pluginSockFilePath)
-		return nil, nil, false
+	address, err := securejoin.SecureJoin(absPluginSockFilePath, c.filename)
+	if err != nil {
+		return nil, nil, fmt.Errorf("invalid socket file path, %v is outside plugin socket dir %v: %w", c.filename, c.pluginSockFilePath, err)
 	}
-
 	cmpclientset := pluginclient.NewConfigManagementPluginClientSet(address)
-
 	conn, cmpClient, err := cmpclientset.NewConfigManagementPluginClient()
 	if err != nil {
-		log.WithFields(log.Fields{
-			common.SecurityField:    common.SecurityMedium,
-			common.SecurityCWEField: common.SecurityCWEMissingReleaseOfFileDescriptor,
-		}).Errorf("error dialing to cmp-server for plugin %s, %v", fileName, err)
-		return nil, nil, false
+		return nil, nil, fmt.Errorf("error dialing to cmp-server for plugin: %w", err)
 	}
+	return cmpClient, conn, nil
+}
 
-	cfg, err := cmpClient.CheckPluginConfiguration(ctx, &empty.Empty{})
+// getClientForFilepath returns a client for the given filepath. It also returns a closer for the connection and
+// a boolean indicating if the plugin has discovery configured.
+func getClientForFilepath(ctx context.Context, c CMPClientConstructor) (pluginclient.ConfigManagementPluginServiceClient, io.Closer, bool, error) {
+	cmpClient, closer, err := c.NewConfigManagementPluginClient()
 	if err != nil {
-		log.Errorf("error checking plugin configuration %s, %v", fileName, err)
-		return nil, nil, false
+		return nil, nil, false, fmt.Errorf("error getting client for plugin: %w", err)
 	}
-	usePlugin := cmpSupportsForClient(ctx, cmpClient, appPath, repoPath, env, tarExcludedGlobs, cfg.IsDiscoveryConfigured, namedPlugin)
-	if !usePlugin {
-		io.Close(conn)
-		return nil, nil, false
+	cfg, err := cmpClient.CheckPluginConfiguration(ctx, &empty.Empty{})
+	if err != nil {
+		return nil, nil, false, fmt.Errorf("error checking plugin configuration: %w", err)
 	}
-	return conn, cmpClient, true
+	return cmpClient, closer, cfg.IsDiscoveryConfigured, nil
 }
 
-// cmpSupportsForClient will check if the given plugin should be used for the given repository and plugin client.
-func cmpSupportsForClient(ctx context.Context, cmpClient pluginclient.ConfigManagementPluginServiceClient, appPath string, repoPath string, env []string, tarExcludedGlobs []string, isDiscoveryConfigured bool, namedPlugin bool) bool {
-	if !isDiscoveryConfigured {
-		// if discovery is not configured, return the client without further checks
-		return true
-	}
-
+// matchRepositoryCMP will send the repoPath to the cmp-server. The cmp-server will
+// inspect the files and return true if the repo is supported for manifest generation.
+// Will return false otherwise.
+func cmpSupportsForClient(ctx context.Context, cmpClient pluginclient.ConfigManagementPluginServiceClient, appPath string, repoPath string, env []string, tarExcludedGlobs []string) bool {
 	isSupported, err := matchRepositoryCMP(ctx, appPath, repoPath, cmpClient, env, tarExcludedGlobs)
 	if err != nil {
 		log.WithFields(log.Fields{
@@ -205,3 +236,22 @@ func cmpSupportsForClient(ctx context.Context, cmpClient pluginclient.ConfigMana
 	}
 	return true
 }
+
+// matchRepositoryCMP will send the repoPath to the cmp-server. The cmp-server will inspect the files and return true if
+// the repo is supported for manifest generation. Will return false otherwise.
+func matchRepositoryCMP(ctx context.Context, appPath, repoPath string, client pluginclient.ConfigManagementPluginServiceClient, env, tarExcludedGlobs []string) (bool, error) {
+	matchRepoStream, err := client.MatchRepository(ctx, grpc_retry.Disable())
+	if err != nil {
+		return false, fmt.Errorf("error getting stream client: %w", err)
+	}
+
+	err = cmp.SendRepoStream(ctx, appPath, repoPath, matchRepoStream, env, tarExcludedGlobs)
+	if err != nil {
+		return false, fmt.Errorf("error sending stream: %w", err)
+	}
+	resp, err := matchRepoStream.CloseAndRecv()
+	if err != nil {
+		return false, fmt.Errorf("error receiving stream response: %w", err)
+	}
+	return resp.GetIsSupported(), nil
+}
diff --git a/util/app/discovery/discovery_test.go b/util/app/discovery/discovery_test.go
index fa639e6ac140e..c2eaf6bfdda37 100644
--- a/util/app/discovery/discovery_test.go
+++ b/util/app/discovery/discovery_test.go
@@ -3,7 +3,8 @@ package discovery
 import (
 	"context"
 	"github.com/argoproj/argo-cd/v2/cmpserver/apiclient"
-	"github.com/argoproj/argo-cd/v2/cmpserver/apiclient/mocks"
+	cmpmocks "github.com/argoproj/argo-cd/v2/cmpserver/apiclient/mocks"
+	"github.com/argoproj/argo-cd/v2/util/app/discovery/mocks"
 	"github.com/stretchr/testify/mock"
 	"testing"
 
@@ -54,6 +55,12 @@ func TestAppType_Disabled(t *testing.T) {
 	assert.Equal(t, "Directory", appType)
 }
 
+type nopCloser struct{}
+
+func (c nopCloser) Close() error {
+	return nil
+}
+
 func Test_cmpSupportsForClient(t *testing.T) {
 	t.Parallel()
 
@@ -89,7 +96,7 @@ func Test_cmpSupportsForClient(t *testing.T) {
 			name:                  "plugin not named and discovery not configured",
 			namedPlugin:           false,
 			isDiscoveryConfigured: false,
-			expected:              true,
+			expected:              false,
 		},
 		{
 			name:                  "discovery configured, not named plugin",
@@ -112,17 +119,32 @@ func Test_cmpSupportsForClient(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			t.Parallel()
 
-			rc := mocks.NewConfigManagementPluginService_MatchRepositoryClient(t)
+			rc := cmpmocks.NewConfigManagementPluginService_MatchRepositoryClient(t)
 			rc.On("Send", mock.Anything).Maybe().Return(nil)
 			rc.On("CloseAndRecv").Maybe().Return(&apiclient.RepositoryResponse{
 				IsSupported: tc.isSupported,
 			}, nil)
 
-			c := mocks.NewConfigManagementPluginServiceClient(t)
+			c := cmpmocks.NewConfigManagementPluginServiceClient(t)
+			c.On("CheckPluginConfiguration", mock.Anything, mock.Anything, mock.Anything).Return(&apiclient.CheckPluginConfigurationResponse{
+				IsDiscoveryConfigured: tc.isDiscoveryConfigured,
+			}, nil, nil)
 			c.On("MatchRepository", mock.Anything, mock.Anything).Maybe().Return(rc, nil)
 
-			actual := cmpSupportsForClient(context.Background(), c, "./testdata", "./testdata", nil, nil, tc.isDiscoveryConfigured, tc.namedPlugin)
-			assert.Equal(t, tc.expected, actual)
+			cc := mocks.NewCMPClientConstructor(t)
+			cc.On("NewConfigManagementPluginClient").Return(c, nopCloser{}, nil)
+
+			var client apiclient.ConfigManagementPluginServiceClient
+			if tc.namedPlugin {
+				client, _ = namedCMPSupports(context.Background(), cc, "./testdata", "./testdata", nil, nil)
+			} else {
+				client, _ = unnamedCMPSupports(context.Background(), cc, "./testdata", "./testdata", nil, nil)
+			}
+			if tc.expected {
+				assert.NotNil(t, client)
+			} else {
+				assert.Nil(t, client)
+			}
 		})
 	}
 }
diff --git a/util/app/discovery/mocks/CMPClientConstructor.go b/util/app/discovery/mocks/CMPClientConstructor.go
new file mode 100644
index 0000000000000..8edfcb3ff005b
--- /dev/null
+++ b/util/app/discovery/mocks/CMPClientConstructor.go
@@ -0,0 +1,69 @@
+// Code generated by mockery v2.43.2. DO NOT EDIT.
+
+package mocks
+
+import (
+	apiclient "github.com/argoproj/argo-cd/v2/cmpserver/apiclient"
+
+	io "github.com/argoproj/argo-cd/v2/util/io"
+
+	mock "github.com/stretchr/testify/mock"
+)
+
+// CMPClientConstructor is an autogenerated mock type for the CMPClientConstructor type
+type CMPClientConstructor struct {
+	mock.Mock
+}
+
+// NewConfigManagementPluginClient provides a mock function with given fields:
+func (_m *CMPClientConstructor) NewConfigManagementPluginClient() (apiclient.ConfigManagementPluginServiceClient, io.Closer, error) {
+	ret := _m.Called()
+
+	if len(ret) == 0 {
+		panic("no return value specified for NewConfigManagementPluginClient")
+	}
+
+	var r0 apiclient.ConfigManagementPluginServiceClient
+	var r1 io.Closer
+	var r2 error
+	if rf, ok := ret.Get(0).(func() (apiclient.ConfigManagementPluginServiceClient, io.Closer, error)); ok {
+		return rf()
+	}
+	if rf, ok := ret.Get(0).(func() apiclient.ConfigManagementPluginServiceClient); ok {
+		r0 = rf()
+	} else {
+		if ret.Get(0) != nil {
+			r0 = ret.Get(0).(apiclient.ConfigManagementPluginServiceClient)
+		}
+	}
+
+	if rf, ok := ret.Get(1).(func() io.Closer); ok {
+		r1 = rf()
+	} else {
+		if ret.Get(1) != nil {
+			r1 = ret.Get(1).(io.Closer)
+		}
+	}
+
+	if rf, ok := ret.Get(2).(func() error); ok {
+		r2 = rf()
+	} else {
+		r2 = ret.Error(2)
+	}
+
+	return r0, r1, r2
+}
+
+// NewCMPClientConstructor creates a new instance of CMPClientConstructor. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations.
+// The first argument is typically a *testing.T value.
+func NewCMPClientConstructor(t interface {
+	mock.TestingT
+	Cleanup(func())
+}) *CMPClientConstructor {
+	mock := &CMPClientConstructor{}
+	mock.Mock.Test(t)
+
+	t.Cleanup(func() { mock.AssertExpectations(t) })
+
+	return mock
+}

From ecfe24f9141bf73edce1d19d2d9a5340d056372a Mon Sep 17 00:00:00 2001
From: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Date: Thu, 3 Oct 2024 13:15:00 -0400
Subject: [PATCH 3/6] better comment

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
---
 cmpserver/apiclient/plugin.pb.go | 5 ++++-
 cmpserver/plugin/plugin.proto    | 5 ++++-
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/cmpserver/apiclient/plugin.pb.go b/cmpserver/apiclient/plugin.pb.go
index e64adfb5b6ebf..07083d0a4906c 100644
--- a/cmpserver/apiclient/plugin.pb.go
+++ b/cmpserver/apiclient/plugin.pb.go
@@ -321,7 +321,10 @@ type RepositoryResponse struct {
 	IsSupported bool `protobuf:"varint,1,opt,name=isSupported,proto3" json:"isSupported,omitempty"`
 	// IsDiscoveryEnabled is a flag that indicates if the CMP supports discovery.
 	//
-	// Deprecated: Use the IsDiscoveryConfigured field from the CheckPluginConfigurationResponse instead.
+	// Deprecated: Use the IsDiscoveryConfigured field from the CheckPluginConfigurationResponse instead. Argo CD no
+	//             longer uses an expensive MatchRepository service to determine if discovery is enabled. Instead, we
+	//             make a relatively cheap pre-flight request to the CheckPluginConfiguration service. We'll leave this
+	//             field here to avoid errors on mismatched repo-server and cmp-server versions.
 	IsDiscoveryEnabled   bool     `protobuf:"varint,2,opt,name=isDiscoveryEnabled,proto3" json:"isDiscoveryEnabled,omitempty"`
 	XXX_NoUnkeyedLiteral struct{} `json:"-"`
 	XXX_unrecognized     []byte   `json:"-"`
diff --git a/cmpserver/plugin/plugin.proto b/cmpserver/plugin/plugin.proto
index 47667b2a9c320..c85e616aed57d 100644
--- a/cmpserver/plugin/plugin.proto
+++ b/cmpserver/plugin/plugin.proto
@@ -47,7 +47,10 @@ message RepositoryResponse {
     bool isSupported = 1;
     // IsDiscoveryEnabled is a flag that indicates if the CMP supports discovery.
     //
-    // Deprecated: Use the IsDiscoveryConfigured field from the CheckPluginConfigurationResponse instead.
+    // Deprecated: Use the IsDiscoveryConfigured field from the CheckPluginConfigurationResponse instead. Argo CD no
+    //             longer uses an expensive MatchRepository service to determine if discovery is enabled. Instead, we
+    //             make a relatively cheap pre-flight request to the CheckPluginConfiguration service. We'll leave this
+    //             field here to avoid errors on mismatched repo-server and cmp-server versions.
     bool isDiscoveryEnabled = 2;
 }
 

From 932beb707f21c861ca6c461981e08b478a36f6f8 Mon Sep 17 00:00:00 2001
From: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Date: Thu, 3 Oct 2024 13:23:21 -0400
Subject: [PATCH 4/6] sort imports

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
---
 util/app/discovery/discovery.go      | 3 +--
 util/app/discovery/discovery_test.go | 8 ++++----
 2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/util/app/discovery/discovery.go b/util/app/discovery/discovery.go
index 9205acd668c6c..c7d3eabf1788e 100644
--- a/util/app/discovery/discovery.go
+++ b/util/app/discovery/discovery.go
@@ -3,13 +3,12 @@ package discovery
 import (
 	"context"
 	"fmt"
-	securejoin "github.com/cyphar/filepath-securejoin"
 	"os"
 	"path/filepath"
 	"strings"
 
+	securejoin "github.com/cyphar/filepath-securejoin"
 	"github.com/golang/protobuf/ptypes/empty"
-
 	grpc_retry "github.com/grpc-ecosystem/go-grpc-middleware/retry"
 	log "github.com/sirupsen/logrus"
 
diff --git a/util/app/discovery/discovery_test.go b/util/app/discovery/discovery_test.go
index c2eaf6bfdda37..bad68517f2187 100644
--- a/util/app/discovery/discovery_test.go
+++ b/util/app/discovery/discovery_test.go
@@ -2,15 +2,15 @@ package discovery
 
 import (
 	"context"
-	"github.com/argoproj/argo-cd/v2/cmpserver/apiclient"
-	cmpmocks "github.com/argoproj/argo-cd/v2/cmpserver/apiclient/mocks"
-	"github.com/argoproj/argo-cd/v2/util/app/discovery/mocks"
-	"github.com/stretchr/testify/mock"
 	"testing"
 
+	"github.com/argoproj/argo-cd/v2/cmpserver/apiclient"
+	cmpmocks "github.com/argoproj/argo-cd/v2/cmpserver/apiclient/mocks"
 	"github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
+	"github.com/argoproj/argo-cd/v2/util/app/discovery/mocks"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"
 )
 

From 7911b2f1c02747b6c5e12f0c9612bde122001525 Mon Sep 17 00:00:00 2001
From: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Date: Thu, 3 Oct 2024 13:24:29 -0400
Subject: [PATCH 5/6] remove needless whitespace

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
---
 .mockery.yaml | 1 -
 1 file changed, 1 deletion(-)

diff --git a/.mockery.yaml b/.mockery.yaml
index 181bd271f60d3..4d0207ef0a2ae 100644
--- a/.mockery.yaml
+++ b/.mockery.yaml
@@ -51,7 +51,6 @@ packages:
   github.com/argoproj/argo-cd/v2/util/app/discovery:
     interfaces:
       CMPClientConstructor:
-
   github.com/argoproj/argo-cd/v2/util/db:
     interfaces:
       ArgoDB:

From cf553e6feabb91a4fba8597334d764fd0ec61ab4 Mon Sep 17 00:00:00 2001
From: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Date: Fri, 4 Oct 2024 10:30:24 -0400
Subject: [PATCH 6/6] fixes from comments

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
---
 reposerver/repository/repository.go |  8 ++++----
 util/app/discovery/discovery.go     | 31 ++++++++++++++++-------------
 2 files changed, 21 insertions(+), 18 deletions(-)

diff --git a/reposerver/repository/repository.go b/reposerver/repository/repository.go
index 7115c1bedd9aa..914059f9c8ad1 100644
--- a/reposerver/repository/repository.go
+++ b/reposerver/repository/repository.go
@@ -1982,11 +1982,11 @@ func runConfigManagementPluginSidecars(ctx context.Context, appPath, repoPath, p
 	}
 
 	// detect config management plugin server
-	conn, cmpClient, err := discovery.DetectConfigManagementPlugin(ctx, appPath, repoPath, pluginName, env, tarExcludedGlobs)
+	cmpClient, closer, err := discovery.DetectConfigManagementPlugin(ctx, appPath, repoPath, pluginName, env, tarExcludedGlobs)
 	if err != nil {
 		return nil, err
 	}
-	defer io.Close(conn)
+	defer io.Close(closer)
 
 	rootPath := repoPath
 	if useManifestGeneratePaths {
@@ -2239,11 +2239,11 @@ func populatePluginAppDetails(ctx context.Context, res *apiclient.RepoAppDetails
 		pluginName = q.Source.Plugin.Name
 	}
 	// detect config management plugin server (sidecar)
-	conn, cmpClient, err := discovery.DetectConfigManagementPlugin(ctx, appPath, repoPath, pluginName, env, tarExcludedGlobs)
+	cmpClient, closer, err := discovery.DetectConfigManagementPlugin(ctx, appPath, repoPath, pluginName, env, tarExcludedGlobs)
 	if err != nil {
 		return fmt.Errorf("failed to detect CMP for app: %w", err)
 	}
-	defer io.Close(conn)
+	defer io.Close(closer)
 
 	parametersAnnouncementStream, err := cmpClient.GetParametersAnnouncement(ctx, grpc_retry.Disable())
 	if err != nil {
diff --git a/util/app/discovery/discovery.go b/util/app/discovery/discovery.go
index c7d3eabf1788e..8802be5e34e9e 100644
--- a/util/app/discovery/discovery.go
+++ b/util/app/discovery/discovery.go
@@ -35,10 +35,10 @@ func Discover(ctx context.Context, appPath, repoPath string, enableGenerateManif
 	apps := make(map[string]string)
 
 	// Check if it is CMP
-	conn, _, err := DetectConfigManagementPlugin(ctx, appPath, repoPath, "", env, tarExcludedGlobs)
+	_, closer, err := DetectConfigManagementPlugin(ctx, appPath, repoPath, "", env, tarExcludedGlobs)
 	if err == nil {
 		// Found CMP
-		io.Close(conn)
+		io.Close(closer)
 
 		apps["."] = string(v1alpha1.ApplicationSourceTypePlugin)
 		return apps, nil
@@ -79,9 +79,12 @@ func AppType(ctx context.Context, appPath, repoPath string, enableGenerateManife
 	return "Directory", nil
 }
 
-func DetectConfigManagementPlugin(ctx context.Context, appPath, repoPath, pluginName string, env []string, tarExcludedGlobs []string) (io.Closer, pluginclient.ConfigManagementPluginServiceClient, error) {
-	var conn io.Closer
+// DetectConfigManagementPlugin will return a client for the CMP plugin if a plugin is found supporting the given
+// repository. It will also return a closer for the connection. If a compatible plugin is not found, it wil return an
+// error.
+func DetectConfigManagementPlugin(ctx context.Context, appPath, repoPath, pluginName string, env, tarExcludedGlobs []string) (pluginclient.ConfigManagementPluginServiceClient, io.Closer, error) {
 	var cmpClient pluginclient.ConfigManagementPluginServiceClient
+	var closer io.Closer
 
 	pluginSockFilePath := common.GetPluginSockFilePath()
 	log.WithFields(log.Fields{
@@ -91,7 +94,7 @@ func DetectConfigManagementPlugin(ctx context.Context, appPath, repoPath, plugin
 
 	if pluginName != "" {
 		c := newSocketCMPClientConstructorForPluginName(pluginSockFilePath, pluginName)
-		cmpClient, conn = namedCMPSupports(ctx, c, appPath, repoPath, env, tarExcludedGlobs)
+		cmpClient, closer = namedCMPSupports(ctx, c, appPath, repoPath, env, tarExcludedGlobs)
 		if cmpClient == nil {
 			return nil, nil, fmt.Errorf("couldn't find cmp-server plugin with name %q supporting the given repository", pluginName)
 		}
@@ -103,7 +106,7 @@ func DetectConfigManagementPlugin(ctx context.Context, appPath, repoPath, plugin
 		for _, file := range fileList {
 			if file.Type() == os.ModeSocket {
 				c := newSocketCMPClientConstructorForPath(pluginSockFilePath, file.Name())
-				cmpClient, conn = unnamedCMPSupports(ctx, c, appPath, repoPath, env, tarExcludedGlobs)
+				cmpClient, closer = unnamedCMPSupports(ctx, c, appPath, repoPath, env, tarExcludedGlobs)
 				if cmpClient != nil {
 					break
 				}
@@ -113,13 +116,13 @@ func DetectConfigManagementPlugin(ctx context.Context, appPath, repoPath, plugin
 			return nil, nil, fmt.Errorf("could not find plugin supporting the given repository")
 		}
 	}
-	return conn, cmpClient, nil
+	return cmpClient, closer, nil
 }
 
 // namedCMPSupports will return a client for the named plugin if it supports the given repository. It will also return
 // a closer for the connection.
 func namedCMPSupports(ctx context.Context, c CMPClientConstructor, appPath, repoPath string, env, tarExcludedGlobs []string) (pluginclient.ConfigManagementPluginServiceClient, io.Closer) {
-	cmpClient, closer, isDiscoveryConfigured, err := getClientForFilepath(ctx, c)
+	cmpClient, closer, isDiscoveryConfigured, err := getClientAndConfig(ctx, c)
 	if err != nil {
 		log.Errorf("error getting client for plugin: %v", err)
 		return nil, nil
@@ -139,7 +142,7 @@ func namedCMPSupports(ctx context.Context, c CMPClientConstructor, appPath, repo
 // unnamedCMPSupports will return a client if the plugin supports the given repository. It will also return a closer for
 // the connection.
 func unnamedCMPSupports(ctx context.Context, c CMPClientConstructor, appPath, repoPath string, env, tarExcludedGlobs []string) (pluginclient.ConfigManagementPluginServiceClient, io.Closer) {
-	cmpClient, closer, isDiscoveryConfigured, err := getClientForFilepath(ctx, c)
+	cmpClient, closer, isDiscoveryConfigured, err := getClientAndConfig(ctx, c)
 	if err != nil {
 		log.Errorf("error getting client for plugin: %v", err)
 		return nil, nil
@@ -169,7 +172,7 @@ type socketCMPClientConstructor struct {
 }
 
 // newSocketCMPClientConstructorForPath returns a new socketCMPClientConstructor.
-func newSocketCMPClientConstructorForPath(pluginSockFilePath, filename string) CMPClientConstructor {
+func newSocketCMPClientConstructorForPath(pluginSockFilePath, filename string) socketCMPClientConstructor {
 	return socketCMPClientConstructor{
 		pluginSockFilePath: pluginSockFilePath,
 		filename:           filename,
@@ -177,7 +180,7 @@ func newSocketCMPClientConstructorForPath(pluginSockFilePath, filename string) C
 }
 
 // newSocketCMPClientConstructorForPluginName returns a new socketCMPClientConstructor for the given plugin name.
-func newSocketCMPClientConstructorForPluginName(pluginSockFilePath, pluginName string) CMPClientConstructor {
+func newSocketCMPClientConstructorForPluginName(pluginSockFilePath, pluginName string) socketCMPClientConstructor {
 	return newSocketCMPClientConstructorForPath(pluginSockFilePath, pluginName+".sock")
 }
 
@@ -199,9 +202,9 @@ func (c socketCMPClientConstructor) NewConfigManagementPluginClient() (plugincli
 	return cmpClient, conn, nil
 }
 
-// getClientForFilepath returns a client for the given filepath. It also returns a closer for the connection and
+// getClientAndConfig returns a client for the given filepath. It also returns a closer for the connection and
 // a boolean indicating if the plugin has discovery configured.
-func getClientForFilepath(ctx context.Context, c CMPClientConstructor) (pluginclient.ConfigManagementPluginServiceClient, io.Closer, bool, error) {
+func getClientAndConfig(ctx context.Context, c CMPClientConstructor) (pluginclient.ConfigManagementPluginServiceClient, io.Closer, bool, error) {
 	cmpClient, closer, err := c.NewConfigManagementPluginClient()
 	if err != nil {
 		return nil, nil, false, fmt.Errorf("error getting client for plugin: %w", err)
@@ -213,7 +216,7 @@ func getClientForFilepath(ctx context.Context, c CMPClientConstructor) (plugincl
 	return cmpClient, closer, cfg.IsDiscoveryConfigured, nil
 }
 
-// matchRepositoryCMP will send the repoPath to the cmp-server. The cmp-server will
+// cmpSupportsForClient will send the repoPath to the cmp-server. The cmp-server will
 // inspect the files and return true if the repo is supported for manifest generation.
 // Will return false otherwise.
 func cmpSupportsForClient(ctx context.Context, cmpClient pluginclient.ConfigManagementPluginServiceClient, appPath string, repoPath string, env []string, tarExcludedGlobs []string) bool {