fix(docker): fix change detect error (#1137)

* fix(docker): fix change detect error

#1132

* chore: golangci-lint fix

* fix(docker): fix path check logical error

* chore(docker): add test not deps git repo & fix error check path affected

* chore: dockerignore ignore docs test git and bin

* chore: golangci-lint fix

* test(docker): add more test case for checkPathAffected

* fix(docker): test setAgeKey

* fix(docker): t.Parallel not use with setenv

* fix(compose): resolve variable shadowing in volume change detection loop

---------

Co-authored-by: Kim Oliver Drechsel <kim@drechsel.xyz>
Co-authored-by: Kim Oliver Drechsel <kimoliver.drechsel@adorsys.com>

Author: qianlongzt

.dockerignore

@@ -1,8 +1,13 @@
 .bin/
+bin
 .idea/
 .vscode/
 .git/
 docs/
 test/
 test-cases/
 **/testdata
+Dockerfile
+*.compose.yaml
+docker-compose.yaml
+.dockerignore

internal/docker/compose.go

@@ -538,7 +538,7 @@ func DeployStack(
 			}
 		}
 	} else {
-		detectedChanges, err := ProjectFilesHaveChanges(changedFiles, project)
+		detectedChanges, err := ProjectFilesHaveChanges(externalRepoPath, changedFiles, project)
 		if err != nil {
 			errMsg := "failed to check for changed project files"
 			return fmt.Errorf("%s: %w", errMsg, err)
@@ -642,39 +642,54 @@ func getPaths(changedFiles []gitInternal.ChangedFile, basePath string) []string
 				p = filepath.Join(basePath, p)
 			}
 
-			if !slices.Contains(absPaths, p) {
-				absPaths = append(absPaths, p)
-			}
+			absPaths = append(absPaths, p)
 		}
 	}
 
-	return absPaths
+	return slice.Unique(absPaths)
+}
+
+// checkPathAffected checks if a changed file is affected by a used file.
+func checkPathAffected(changed string, used string) bool {
+	used = filepath.Clean(used)
+	changed = filepath.Clean(changed)
+
+	rel, err := filepath.Rel(used, changed)
+	if err != nil {
+		// It' share same reporoot, so it should not happen
+		slog.Debug("checkPathAffected ",
+			slog.String("used", used),
+			slog.String("changed", changed),
+			slog.Any("error", err),
+		)
+
+		return false
+	}
+
+	return !strings.HasPrefix(rel, "..")
 }
 
 // HasChangedConfigs checks if any files used in docker compose `configs:` definitions have changed using the Git status.
-func HasChangedConfigs(changedFiles []gitInternal.ChangedFile, project *types.Project) ([]string, error) {
-	// We only need the relative part in this case
-	paths := getPaths(changedFiles, ".")
+func HasChangedConfigs(paths []string, project *types.Project) ([]string, error) {
+	configToServicesMap := make(map[string][]string)
+
+	for name, s := range project.Services {
+		for _, cfg := range s.Configs {
+			configToServicesMap[cfg.Source] = append(configToServicesMap[cfg.Source], name)
+		}
+	}
 
 	var changedServices []string
 
-	for _, c := range project.Configs {
+	for name, c := range project.Configs {
 		// Changes in config.Content are handled in project hash comparison
 		if c.File == "" {
 			continue
 		}
 
 		for _, p := range paths {
-			if strings.HasSuffix(c.File, p) {
-				// Find services that use if the changed config
-				for _, s := range project.Services {
-					for _, cfg := range s.Configs {
-						if strings.HasSuffix(c.File, cfg.Source) {
-							changedServices = append(changedServices, s.Name)
-							break
-						}
-					}
-				}
+			if checkPathAffected(p, c.File) {
+				changedServices = append(changedServices, configToServicesMap[name]...)
 			}
 		}
 	}
@@ -683,28 +698,25 @@ func HasChangedConfigs(changedFiles []gitInternal.ChangedFile, project *types.Pr
 }
 
 // HasChangedSecrets checks if any files used in docker compose `secrets:` definitions have changed using the Git status.
-func HasChangedSecrets(changedFiles []gitInternal.ChangedFile, project *types.Project) ([]string, error) {
-	// We only need the relative part in this case
-	paths := getPaths(changedFiles, ".")
+func HasChangedSecrets(paths []string, project *types.Project) ([]string, error) {
+	secretsToServicesMap := make(map[string][]string)
+
+	for name, s := range project.Services {
+		for _, secret := range s.Secrets {
+			secretsToServicesMap[secret.Source] = append(secretsToServicesMap[secret.Source], name)
+		}
+	}
 
 	var changedServices []string
 
-	for _, s := range project.Secrets {
+	for name, s := range project.Secrets {
 		if s.File == "" {
 			continue
 		}
 
 		for _, p := range paths {
-			if strings.HasSuffix(s.File, p) {
-				// Find services that use if the changed config
-				for _, svc := range project.Services {
-					for _, secret := range svc.Secrets {
-						if strings.HasSuffix(s.File, secret.Source) {
-							changedServices = append(changedServices, svc.Name)
-							break
-						}
-					}
-				}
+			if checkPathAffected(p, s.File) {
+				changedServices = append(changedServices, secretsToServicesMap[name]...)
 			}
 		}
 	}
@@ -713,24 +725,17 @@ func HasChangedSecrets(changedFiles []gitInternal.ChangedFile, project *types.Pr
 }
 
 // HasChangedBindMounts checks if any files used in docker compose `volumes:` definitions with type `bind` have changed using the Git status.
-func HasChangedBindMounts(changedFiles []gitInternal.ChangedFile, project *types.Project) ([]string, error) {
-	paths := getPaths(changedFiles, ".")
-
+func HasChangedBindMounts(paths []string, project *types.Project) ([]string, error) {
 	var changedServices []string
 
 	for _, s := range project.Services {
+	out:
 		for _, v := range s.Volumes {
 			if v.Type == "bind" && v.Source != "" {
-				bindSourceAbs := v.Source
-
-				if filesInPath(paths, bindSourceAbs) {
-					for _, svc := range project.Services {
-						for _, vol := range svc.Volumes {
-							if vol.Type == "bind" && vol.Source == v.Source {
-								changedServices = append(changedServices, svc.Name)
-								break
-							}
-						}
+				for _, p := range paths {
+					if checkPathAffected(p, v.Source) {
+						changedServices = append(changedServices, s.Name)
+						break out
 					}
 				}
 			}
@@ -741,25 +746,16 @@ func HasChangedBindMounts(changedFiles []gitInternal.ChangedFile, project *types
 }
 
 // HasChangedEnvFiles checks if any files used in docker compose `env_file:` definitions have changed using the Git status.
-func HasChangedEnvFiles(changedFiles []gitInternal.ChangedFile, project *types.Project) ([]string, error) {
-	// We only need the relative part in this case
-	paths := getPaths(changedFiles, ".")
-
+func HasChangedEnvFiles(paths []string, project *types.Project) ([]string, error) {
 	var changedServices []string
 
 	for _, s := range project.Services {
+	out:
 		for _, envFile := range s.EnvFiles {
 			for _, p := range paths {
-				if strings.HasSuffix(envFile.Path, p) {
-					// Find services that use if the changed env file
-					for _, svc := range project.Services {
-						for _, ef := range svc.EnvFiles {
-							if strings.HasSuffix(envFile.Path, ef.Path) {
-								changedServices = append(changedServices, svc.Name)
-								break
-							}
-						}
-					}
+				if checkPathAffected(p, envFile.Path) {
+					changedServices = append(changedServices, s.Name)
+					break out
 				}
 			}
 		}
@@ -770,9 +766,7 @@ func HasChangedEnvFiles(changedFiles []gitInternal.ChangedFile, project *types.P
 
 // HasChangedBuildFiles checks if any files used as build context in docker compose `build:` definitions have changed using the Git status.
 // This includes any file within the build context directory for each service. If a changed file is within a build context, it returns true.
-func HasChangedBuildFiles(changedFiles []gitInternal.ChangedFile, project *types.Project) ([]string, error) {
-	paths := getPaths(changedFiles, ".")
-
+func HasChangedBuildFiles(paths []string, project *types.Project) ([]string, error) {
 	var changedServices []string
 
 	for _, s := range project.Services {
@@ -811,15 +805,17 @@ func HasChangedBuildFiles(changedFiles []gitInternal.ChangedFile, project *types
 			contexts = append(contexts, dockerFile)
 		}
 
+	out:
+
 		for _, ctxFile := range contexts {
 			if !path.IsAbs(ctxFile) {
 				ctxFile = filepath.Join(project.WorkingDir, ctxFile)
 			}
 
 			for _, p := range paths {
-				if strings.HasSuffix(ctxFile, p) {
+				if checkPathAffected(p, ctxFile) {
 					changedServices = append(changedServices, s.Name)
-					break
+					break out
 				}
 			}
 		}
@@ -845,10 +841,10 @@ func sortChanges(changes []Change) {
 }
 
 // ProjectFilesHaveChanges checks if any files related to the compose project have changed.
-func ProjectFilesHaveChanges(changedFiles []gitInternal.ChangedFile, project *types.Project) ([]Change, error) {
+func ProjectFilesHaveChanges(repoRootExternal string, changedFiles []gitInternal.ChangedFile, project *types.Project) ([]Change, error) {
 	checks := []struct {
 		name string
-		fn   func([]gitInternal.ChangedFile, *types.Project) ([]string, error)
+		fn   func([]string, *types.Project) ([]string, error)
 	}{
 		{"configs", HasChangedConfigs},
 		{"secrets", HasChangedSecrets},
@@ -857,10 +853,12 @@ func ProjectFilesHaveChanges(changedFiles []gitInternal.ChangedFile, project *ty
 		{"buildFiles", HasChangedBuildFiles},
 	}
 
+	paths := getPaths(changedFiles, repoRootExternal)
+
 	var changes []Change
 
 	for _, check := range checks {
-		changedServices, err := check.fn(changedFiles, project)
+		changedServices, err := check.fn(paths, project)
 		if err != nil {
 			return nil, fmt.Errorf("failed to check '%s' for changes: %w", check.name, err)
 		}
@@ -1074,58 +1072,6 @@ func CheckDefaultComposeFiles(composeFiles []string, workingDir string) ([]strin
 	return composeFiles, nil
 }
 
-// filesInPath checks if at least one of the given filePaths is inside the searchPath.
-func filesInPath(filePaths []string, searchPath string) bool {
-	for _, p := range filePaths {
-		if strings.HasSuffix(p, filepath.Base(searchPath)) {
-			p = joinPathsWithoutDuplicates(searchPath, p)
-		}
-
-		rel, err := filepath.Rel(searchPath, p)
-		if err != nil {
-			continue
-		}
-
-		if !strings.HasPrefix(rel, "..") {
-			return true
-		}
-	}
-
-	return false
-}
-
-// joinPathsWithoutDuplicates joins multiple paths, removing duplicate overlapping segments between each pair.
-func joinPathsWithoutDuplicates(paths ...string) string {
-	if len(paths) == 0 {
-		return ""
-	}
-
-	isAbs := filepath.IsAbs(paths[0])
-
-	resultElems := strings.Split(filepath.Clean(paths[0]), string(filepath.Separator))
-	for _, next := range paths[1:] {
-		nextElems := strings.Split(filepath.Clean(next), string(filepath.Separator))
-		maxOverlap := 0
-
-		m := min(len(resultElems), len(nextElems))
-		for k := m; k > 0; k-- {
-			if reflect.DeepEqual(resultElems[len(resultElems)-k:], nextElems[:k]) {
-				maxOverlap = k
-				break
-			}
-		}
-
-		resultElems = append(resultElems, nextElems[maxOverlap:]...)
-	}
-
-	joined := filepath.Join(resultElems...)
-	if isAbs && !strings.HasPrefix(joined, string(filepath.Separator)) {
-		joined = string(filepath.Separator) + joined
-	}
-
-	return joined
-}
-
 // DecryptProjectFiles decrypts all files used in the compose project that are encrypted using doco-cd's encryption mechanism.
 // This includes configs, secrets, bind mounts, env files and build contexts.
 // Since absolute file paths in types.Project are paths on the docker host, repoPath also needs to be the external path to the repository.