fix: CMPv2 does not allow symlinks to adjacent files in same git repo. …

…Fixes #13342 (#13360)

fix: CMPv2 does not allow symlinks to adjacent files in same git repo. Fixes #13342 (#13360)

Signed-off-by: Jiacheng Xu <xjcmaxwellcjx@gmail.com>

reposerver/repository/repository.go

@@ -221,7 +221,7 @@ func (s *Service) ListApps(ctx context.Context, q *apiclient.ListAppsRequest) (*
 	}
 
 	defer io.Close(closer)
-	apps, err := discovery.Discover(ctx, gitClient.Root(), q.EnabledSourceTypes, s.initConstants.CMPTarExcludedGlobs)
+	apps, err := discovery.Discover(ctx, gitClient.Root(), gitClient.Root(), q.EnabledSourceTypes, s.initConstants.CMPTarExcludedGlobs)
 	if err != nil {
 		return nil, err
 	}
@@ -1294,7 +1294,7 @@ func GenerateManifests(ctx context.Context, appPath, repoRoot, revision string,
 
 	resourceTracking := argo.NewResourceTracking()
 
-	appSourceType, err := GetAppSourceType(ctx, q.ApplicationSource, appPath, q.AppName, q.EnabledSourceTypes, opt.cmpTarExcludedGlobs)
+	appSourceType, err := GetAppSourceType(ctx, q.ApplicationSource, appPath, repoRoot, q.AppName, q.EnabledSourceTypes, opt.cmpTarExcludedGlobs)
 	if err != nil {
 		return nil, err
 	}
@@ -1471,8 +1471,8 @@ func mergeSourceParameters(source *v1alpha1.ApplicationSource, path, appName str
 }
 
 // GetAppSourceType returns explicit application source type or examines a directory and determines its application source type
-func GetAppSourceType(ctx context.Context, source *v1alpha1.ApplicationSource, path, appName string, enableGenerateManifests map[string]bool, tarExcludedGlobs []string) (v1alpha1.ApplicationSourceType, error) {
-	err := mergeSourceParameters(source, path, appName)
+func GetAppSourceType(ctx context.Context, source *v1alpha1.ApplicationSource, appPath, repoPath, appName string, enableGenerateManifests map[string]bool, tarExcludedGlobs []string) (v1alpha1.ApplicationSourceType, error) {
+	err := mergeSourceParameters(source, appPath, appName)
 	if err != nil {
 		return "", fmt.Errorf("error while parsing source parameters: %v", err)
 	}
@@ -1488,7 +1488,7 @@ func GetAppSourceType(ctx context.Context, source *v1alpha1.ApplicationSource, p
 		}
 		return *appSourceType, nil
 	}
-	appType, err := discovery.AppType(ctx, path, enableGenerateManifests, tarExcludedGlobs)
+	appType, err := discovery.AppType(ctx, appPath, repoPath, enableGenerateManifests, tarExcludedGlobs)
 	if err != nil {
 		return "", err
 	}
@@ -1908,7 +1908,7 @@ func runConfigManagementPluginSidecars(ctx context.Context, appPath, repoPath, p
 	}
 
 	// detect config management plugin server (sidecar)
-	conn, cmpClient, err := discovery.DetectConfigManagementPlugin(ctx, appPath, pluginName, env, tarExcludedGlobs)
+	conn, cmpClient, err := discovery.DetectConfigManagementPlugin(ctx, appPath, repoPath, pluginName, env, tarExcludedGlobs)
 	if err != nil {
 		return nil, err
 	}
@@ -1965,7 +1965,7 @@ func (s *Service) GetAppDetails(ctx context.Context, q *apiclient.RepoServerAppD
 			return err
 		}
 
-		appSourceType, err := GetAppSourceType(ctx, q.Source, opContext.appPath, q.AppName, q.EnabledSourceTypes, s.initConstants.CMPTarExcludedGlobs)
+		appSourceType, err := GetAppSourceType(ctx, q.Source, opContext.appPath, repoRoot, q.AppName, q.EnabledSourceTypes, s.initConstants.CMPTarExcludedGlobs)
 		if err != nil {
 			return err
 		}
@@ -2159,7 +2159,7 @@ 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, pluginName, env, tarExcludedGlobs)
+	conn, cmpClient, err := discovery.DetectConfigManagementPlugin(ctx, appPath, repoPath, pluginName, env, tarExcludedGlobs)
 	if err != nil {
 		return fmt.Errorf("failed to detect CMP for app: %w", err)
 	}

reposerver/repository/repository_test.go

@@ -1188,15 +1188,15 @@ func TestGenerateNullList(t *testing.T) {
 }
 
 func TestIdentifyAppSourceTypeByAppDirWithKustomizations(t *testing.T) {
-	sourceType, err := GetAppSourceType(context.Background(), &argoappv1.ApplicationSource{}, "./testdata/kustomization_yaml", "testapp", map[string]bool{}, []string{})
+	sourceType, err := GetAppSourceType(context.Background(), &argoappv1.ApplicationSource{}, "./testdata/kustomization_yaml", "./testdata", "testapp", map[string]bool{}, []string{})
 	assert.Nil(t, err)
 	assert.Equal(t, argoappv1.ApplicationSourceTypeKustomize, sourceType)
 
-	sourceType, err = GetAppSourceType(context.Background(), &argoappv1.ApplicationSource{}, "./testdata/kustomization_yml", "testapp", map[string]bool{}, []string{})
+	sourceType, err = GetAppSourceType(context.Background(), &argoappv1.ApplicationSource{}, "./testdata/kustomization_yml", "./testdata", "testapp", map[string]bool{}, []string{})
 	assert.Nil(t, err)
 	assert.Equal(t, argoappv1.ApplicationSourceTypeKustomize, sourceType)
 
-	sourceType, err = GetAppSourceType(context.Background(), &argoappv1.ApplicationSource{}, "./testdata/Kustomization", "testapp", map[string]bool{}, []string{})
+	sourceType, err = GetAppSourceType(context.Background(), &argoappv1.ApplicationSource{}, "./testdata/Kustomization", "./testdata", "testapp", map[string]bool{}, []string{})
 	assert.Nil(t, err)
 	assert.Equal(t, argoappv1.ApplicationSourceTypeKustomize, sourceType)
 }

test/e2e/custom_tool_test.go

@@ -213,7 +213,7 @@ func TestCMPDiscoverWithFileName(t *testing.T) {
 			time.Sleep(1 * time.Second)
 			os.Setenv("ARGOCD_BINARY_NAME", "argocd")
 		}).
-		Path(pluginName).
+		Path(pluginName + "/subdir").
 		When().
 		CreateApp().
 		Sync().
@@ -350,3 +350,54 @@ func TestPreserveFileModeForCMP(t *testing.T) {
 			assert.Equal(t, "ConfigMap", app.Status.Resources[0].Kind)
 		})
 }
+
+func TestCMPWithSymlinkPartialFiles(t *testing.T) {
+	Given(t, WithTestData("testdata2")).
+		And(func() {
+			go startCMPServer("./testdata2/cmp-symlink")
+			time.Sleep(1 * time.Second)
+			os.Setenv("ARGOCD_BINARY_NAME", "argocd")
+		}).
+		Path("guestbook-partial-symlink-files").
+		When().
+		CreateApp().
+		Sync().
+		Then().
+		Expect(OperationPhaseIs(OperationSucceeded)).
+		Expect(SyncStatusIs(SyncStatusCodeSynced)).
+		Expect(HealthIs(health.HealthStatusHealthy))
+}
+
+func TestCMPWithSymlinkFiles(t *testing.T) {
+	Given(t, WithTestData("testdata2")).
+		And(func() {
+			go startCMPServer("./testdata2/cmp-symlink")
+			time.Sleep(1 * time.Second)
+			os.Setenv("ARGOCD_BINARY_NAME", "argocd")
+		}).
+		Path("guestbook-symlink-files").
+		When().
+		CreateApp().
+		Sync().
+		Then().
+		Expect(OperationPhaseIs(OperationSucceeded)).
+		Expect(SyncStatusIs(SyncStatusCodeSynced)).
+		Expect(HealthIs(health.HealthStatusHealthy))
+}
+
+func TestCMPWithSymlinkFolder(t *testing.T) {
+	Given(t, WithTestData("testdata2")).
+		And(func() {
+			go startCMPServer("./testdata2/cmp-symlink")
+			time.Sleep(1 * time.Second)
+			os.Setenv("ARGOCD_BINARY_NAME", "argocd")
+		}).
+		Path("guestbook-symlink-folder").
+		When().
+		CreateApp().
+		Sync().
+		Then().
+		Expect(OperationPhaseIs(OperationSucceeded)).
+		Expect(SyncStatusIs(SyncStatusCodeSynced)).
+		Expect(HealthIs(health.HealthStatusHealthy))
+}

test/e2e/fixture/app/context.go

@@ -50,8 +50,8 @@ type ContextArgs struct {
 	AppNamespace string
 }
 
-func Given(t *testing.T) *Context {
-	fixture.EnsureCleanState(t)
+func Given(t *testing.T, opts ...fixture.TestOption) *Context {
+	fixture.EnsureCleanState(t, opts...)
 	return GivenWithSameState(t)
 }
 

test/e2e/fixture/fixture.go

@@ -514,7 +514,30 @@ func SetParamInNotificationsConfigMap(key, value string) {
 	})
 }
 
-func EnsureCleanState(t *testing.T) {
+type TestOption func(option *testOption)
+
+type testOption struct {
+	testdata string
+}
+
+func newTestOption(opts ...TestOption) *testOption {
+	to := &testOption{
+		testdata: "testdata",
+	}
+	for _, opt := range opts {
+		opt(to)
+	}
+	return to
+}
+
+func WithTestData(testdata string) TestOption {
+	return func(option *testOption) {
+		option.testdata = testdata
+	}
+}
+
+func EnsureCleanState(t *testing.T, opts ...TestOption) {
+	opt := newTestOption(opts...)
 	// In large scenarios, we can skip tests that already run
 	SkipIfAlreadyRun(t)
 	// Register this test after it has been run & was successfull
@@ -632,7 +655,7 @@ func EnsureCleanState(t *testing.T) {
 	}
 
 	// set-up tmp repo, must have unique name
-	FailOnErr(Run("", "cp", "-Rf", "testdata", repoDirectory()))
+	FailOnErr(Run("", "cp", "-Rf", opt.testdata, repoDirectory()))
 	FailOnErr(Run(repoDirectory(), "chmod", "777", "."))
 	FailOnErr(Run(repoDirectory(), "git", "init"))
 	FailOnErr(Run(repoDirectory(), "git", "add", "."))

test/e2e/testdata/cmp-fileName/plugin.yaml

@@ -7,4 +7,4 @@ spec:
   generate:
     command: [sh, -c, 'echo "{\"kind\": \"ConfigMap\", \"apiVersion\": \"v1\", \"metadata\": { \"name\": \"$ARGOCD_APP_NAME\", \"namespace\": \"$ARGOCD_APP_NAMESPACE\", \"annotations\": {\"Foo\": \"$FOO\", \"KubeVersion\": \"$KUBE_VERSION\", \"KubeApiVersion\": \"$KUBE_API_VERSIONS\",\"Bar\": \"baz\"}}}"']
   discover:
-    fileName: "subdir/s*.yaml"
+    fileName: "cmp-fileName/subdir/s*.yaml"

test/e2e/testdata2/cmp-symlink/plugin.yaml

@@ -0,0 +1,13 @@
+apiVersion: argoproj.io/v1alpha1
+kind: ConfigManagementPlugin
+metadata:
+  name: cmp-symlink
+spec:
+  version: v1.0
+  init:
+    command: [kustomize, version]
+  generate:
+    command: [sh, -c, 'kustomize edit set image test=quay.io/argoprojlabs/argocd-e2e-container:0.2 && kustomize build --load-restrictor LoadRestrictionsNone']
+  discover:
+    find:
+      glob: "**/kustomization.yaml"

test/e2e/testdata2/guestbook-partial-symlink-files/guestbook-ui-deployment.yaml

@@ -0,0 +1 @@
+../guestbook/guestbook-ui-deployment.yaml

test/e2e/testdata2/guestbook-partial-symlink-files/guestbook-ui-svc.yaml

@@ -0,0 +1,10 @@
+apiVersion: v1
+kind: Service
+metadata:
+  name: guestbook-ui
+spec:
+  ports:
+  - port: 80
+    targetPort: 80
+  selector:
+    app: guestbook-ui

test/e2e/testdata2/guestbook-partial-symlink-files/kustomization.yaml

@@ -0,0 +1,6 @@
+apiVersion: kustomize.config.k8s.io/v1beta1
+kind: Kustomization
+
+resources:
+  - ./guestbook-ui-deployment.yaml
+  - ./guestbook-ui-svc.yaml

test/e2e/testdata2/guestbook-symlink-files/guestbook-ui-deployment.yaml

@@ -0,0 +1 @@
+../guestbook/guestbook-ui-deployment.yaml

test/e2e/testdata2/guestbook-symlink-files/guestbook-ui-svc.yaml

@@ -0,0 +1 @@
+../guestbook/guestbook-ui-svc.yaml

test/e2e/testdata2/guestbook-symlink-files/kustomization.yaml

@@ -0,0 +1 @@
+../guestbook/kustomization.yaml

test/e2e/testdata2/guestbook-symlink-folder

@@ -0,0 +1 @@
+guestbook

test/e2e/testdata2/guestbook/guestbook-ui-deployment.yaml

@@ -0,0 +1,23 @@
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: guestbook-ui
+  labels:
+    test: "true"
+spec:
+  replicas: 0
+  revisionHistoryLimit: 3
+  selector:
+    matchLabels:
+      app: guestbook-ui
+  template:
+    metadata:
+      labels:
+        app: guestbook-ui
+    spec:
+      containers:
+      - image: test
+        imagePullPolicy: IfNotPresent
+        name: guestbook-ui
+        ports:
+        - containerPort: 80

test/e2e/testdata2/guestbook/guestbook-ui-svc.yaml

@@ -0,0 +1,10 @@
+apiVersion: v1
+kind: Service
+metadata:
+  name: guestbook-ui
+spec:
+  ports:
+  - port: 80
+    targetPort: 80
+  selector:
+    app: guestbook-ui

test/e2e/testdata2/guestbook/kustomization.yaml

@@ -0,0 +1,6 @@
+apiVersion: kustomize.config.k8s.io/v1beta1
+kind: Kustomization
+
+resources:
+  - ./guestbook-ui-deployment.yaml
+  - ./guestbook-ui-svc.yaml

util/app/discovery/discovery.go

@@ -31,11 +31,11 @@ func IsManifestGenerationEnabled(sourceType v1alpha1.ApplicationSourceType, enab
 	return enabled
 }
 
-func Discover(ctx context.Context, repoPath string, enableGenerateManifests map[string]bool, tarExcludedGlobs []string) (map[string]string, error) {
+func Discover(ctx context.Context, appPath, repoPath string, enableGenerateManifests map[string]bool, tarExcludedGlobs []string) (map[string]string, error) {
 	apps := make(map[string]string)
 
 	// Check if it is CMP
-	conn, _, err := DetectConfigManagementPlugin(ctx, repoPath, "", []string{}, tarExcludedGlobs)
+	conn, _, err := DetectConfigManagementPlugin(ctx, appPath, repoPath, "", []string{}, tarExcludedGlobs)
 	if err == nil {
 		// Found CMP
 		io.Close(conn)
@@ -44,14 +44,14 @@ func Discover(ctx context.Context, repoPath string, enableGenerateManifests map[
 		return apps, nil
 	}
 
-	err = filepath.Walk(repoPath, func(path string, info os.FileInfo, err error) error {
+	err = filepath.Walk(appPath, func(path string, info os.FileInfo, err error) error {
 		if err != nil {
 			return err
 		}
 		if info.IsDir() {
 			return nil
 		}
-		dir, err := filepath.Rel(repoPath, filepath.Dir(path))
+		dir, err := filepath.Rel(appPath, filepath.Dir(path))
 		if err != nil {
 			return err
 		}
@@ -67,8 +67,8 @@ func Discover(ctx context.Context, repoPath string, enableGenerateManifests map[
 	return apps, err
 }
 
-func AppType(ctx context.Context, path string, enableGenerateManifests map[string]bool, tarExcludedGlobs []string) (string, error) {
-	apps, err := Discover(ctx, path, enableGenerateManifests, tarExcludedGlobs)
+func AppType(ctx context.Context, appPath, repoPath string, enableGenerateManifests map[string]bool, tarExcludedGlobs []string) (string, error) {
+	apps, err := Discover(ctx, appPath, repoPath, enableGenerateManifests, tarExcludedGlobs)
 	if err != nil {
 		return "", err
 	}
@@ -85,7 +85,7 @@ func AppType(ctx context.Context, path string, enableGenerateManifests map[strin
 // check cmpSupports()
 // if supported return conn for the cmp-server
 
-func DetectConfigManagementPlugin(ctx context.Context, repoPath, pluginName string, env []string, tarExcludedGlobs []string) (io.Closer, pluginclient.ConfigManagementPluginServiceClient, error) {
+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
@@ -98,7 +98,7 @@ func DetectConfigManagementPlugin(ctx context.Context, repoPath, pluginName stri
 
 	if pluginName != "" {
 		// check if the given plugin supports the repo
-		conn, cmpClient, connFound = cmpSupports(ctx, pluginSockFilePath, repoPath, fmt.Sprintf("%v.sock", pluginName), env, tarExcludedGlobs, true)
+		conn, cmpClient, connFound = cmpSupports(ctx, pluginSockFilePath, appPath, repoPath, fmt.Sprintf("%v.sock", pluginName), env, tarExcludedGlobs, true)
 		if !connFound {
 			return nil, nil, fmt.Errorf("couldn't find cmp-server plugin with name %v supporting the given repository", pluginName)
 		}
@@ -109,7 +109,7 @@ func DetectConfigManagementPlugin(ctx context.Context, repoPath, pluginName stri
 		}
 		for _, file := range fileList {
 			if file.Type() == os.ModeSocket {
-				conn, cmpClient, connFound = cmpSupports(ctx, pluginSockFilePath, repoPath, file.Name(), env, tarExcludedGlobs, false)
+				conn, cmpClient, connFound = cmpSupports(ctx, pluginSockFilePath, appPath, repoPath, file.Name(), env, tarExcludedGlobs, false)
 				if connFound {
 					break
 				}
@@ -125,13 +125,13 @@ func DetectConfigManagementPlugin(ctx context.Context, repoPath, pluginName stri
 // 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, repoPath string, client pluginclient.ConfigManagementPluginServiceClient, env []string, tarExcludedGlobs []string) (bool, bool, error) {
+func matchRepositoryCMP(ctx context.Context, appPath, repoPath string, client pluginclient.ConfigManagementPluginServiceClient, env []string, tarExcludedGlobs []string) (bool, bool, error) {
 	matchRepoStream, err := client.MatchRepository(ctx, grpc_retry.Disable())
 	if err != nil {
 		return false, false, fmt.Errorf("error getting stream client: %s", err)
 	}
 
-	err = cmp.SendRepoStream(ctx, repoPath, repoPath, matchRepoStream, env, tarExcludedGlobs)
+	err = cmp.SendRepoStream(ctx, appPath, repoPath, matchRepoStream, env, tarExcludedGlobs)
 	if err != nil {
 		return false, false, fmt.Errorf("error sending stream: %s", err)
 	}
@@ -142,7 +142,7 @@ func matchRepositoryCMP(ctx context.Context, repoPath string, client pluginclien
 	return resp.GetIsSupported(), resp.GetIsDiscoveryEnabled(), nil
 }
 
-func cmpSupports(ctx context.Context, pluginSockFilePath, repoPath, fileName string, env []string, tarExcludedGlobs []string, namedPlugin bool) (io.Closer, pluginclient.ConfigManagementPluginServiceClient, bool) {
+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)
 	if err != nil {
 		log.Errorf("error getting absolute path for plugin socket dir %v, %v", pluginSockFilePath, err)
@@ -165,7 +165,7 @@ func cmpSupports(ctx context.Context, pluginSockFilePath, repoPath, fileName str
 		return nil, nil, false
 	}
 
-	isSupported, isDiscoveryEnabled, err := matchRepositoryCMP(ctx, repoPath, cmpClient, env, tarExcludedGlobs)
+	isSupported, isDiscoveryEnabled, err := matchRepositoryCMP(ctx, appPath, repoPath, cmpClient, env, tarExcludedGlobs)
 	if err != nil {
 		log.WithFields(log.Fields{
 			common.SecurityField:    common.SecurityMedium,