feat: preserve non-hydrated files during hydration (#24129)

Signed-off-by: nitishfy <[email protected]>

Author: nitishfy

assets/swagger.json

@@ -7,6 +7,8 @@ import (
 	"os"
 	"time"
 
+	"github.com/argoproj/argo-cd/v3/controller/hydrator"
+
 	log "github.com/sirupsen/logrus"
 
 	"github.com/argoproj/argo-cd/v3/commitserver/apiclient"
@@ -31,6 +33,43 @@ func NewService(gitCredsStore git.CredsStore, metricsServer *metrics.Server) *Se
 	}
 }
 
+type hydratorMetadataFile struct {
+	RepoURL  string   `json:"repoURL,omitempty"`
+	DrySHA   string   `json:"drySha,omitempty"`
+	Commands []string `json:"commands,omitempty"`
+	Author   string   `json:"author,omitempty"`
+	Date     string   `json:"date,omitempty"`
+	// Subject is the subject line of the DRY commit message, i.e. `git show --format=%s`.
+	Subject string `json:"subject,omitempty"`
+	// Body is the body of the DRY commit message, excluding the subject line, i.e. `git show --format=%b`.
+	// Known Argocd- trailers with valid values are removed, but all other trailers are kept.
+	Body       string                       `json:"body,omitempty"`
+	References []v1alpha1.RevisionReference `json:"references,omitempty"`
+}
+
+// TODO: make this configurable via ConfigMap.
+var manifestHydrationReadmeTemplate = `# Manifest Hydration
+
+To hydrate the manifests in this repository, run the following commands:
+
+` + "```shell" + `
+git clone {{ .RepoURL }}
+# cd into the cloned directory
+git checkout {{ .DrySHA }}
+{{ range $command := .Commands -}}
+{{ $command }}
+{{ end -}}` + "```" + `
+{{ if .References -}}
+
+## References
+
+{{ range $ref := .References -}}
+{{ if $ref.Commit -}}
+* [{{ $ref.Commit.SHA | mustRegexFind "[0-9a-f]+" | trunc 7 }}]({{ $ref.Commit.RepoURL }}): {{ $ref.Commit.Subject }} ({{ $ref.Commit.Author }})
+{{ end -}}
+{{ end -}}
+{{ end -}}`
+
 // CommitHydratedManifests handles a commit request. It clones the repository, checks out the sync branch, checks out
 // the target branch, clears the repository contents, writes the manifests to the repository, commits the changes, and
 // pushes the changes. It returns the hydrated revision SHA and an error if one occurred.
@@ -120,13 +159,17 @@ func (s *Service) handleCommitRequest(logCtx *log.Entry, r *apiclient.CommitHydr
 
 	logCtx.Debug("Clearing and preparing paths")
 	var pathsToClear []string
+	// range over the paths configured and skip those application
+	// paths that are referencing to root path
 	for _, p := range r.Paths {
-		if p.Path == "" || p.Path == "." {
-			logCtx.Debug("Using root directory for manifests, no directory removal needed")
-		} else {
-			pathsToClear = append(pathsToClear, p.Path)
+		if hydrator.IsRootPath(p.Path) {
+			// skip adding paths that are referencing root directory
+			logCtx.Debugf("Path %s is referencing root directory, ignoring the path", p.Path)
+			continue
 		}
+		pathsToClear = append(pathsToClear, p.Path)
 	}
+
 	if len(pathsToClear) > 0 {
 		logCtx.Debugf("Clearing paths: %v", pathsToClear)
 		out, err := gitClient.RemoveContents(pathsToClear)
@@ -221,40 +264,3 @@ func (s *Service) initGitClient(logCtx *log.Entry, r *apiclient.CommitHydratedMa
 
 	return gitClient, dirPath, cleanupOrLog, nil
 }
-
-type hydratorMetadataFile struct {
-	RepoURL  string   `json:"repoURL,omitempty"`
-	DrySHA   string   `json:"drySha,omitempty"`
-	Commands []string `json:"commands,omitempty"`
-	Author   string   `json:"author,omitempty"`
-	Date     string   `json:"date,omitempty"`
-	// Subject is the subject line of the DRY commit message, i.e. `git show --format=%s`.
-	Subject string `json:"subject,omitempty"`
-	// Body is the body of the DRY commit message, excluding the subject line, i.e. `git show --format=%b`.
-	// Known Argocd- trailers with valid values are removed, but all other trailers are kept.
-	Body       string                       `json:"body,omitempty"`
-	References []v1alpha1.RevisionReference `json:"references,omitempty"`
-}
-
-// TODO: make this configurable via ConfigMap.
-var manifestHydrationReadmeTemplate = `# Manifest Hydration
-
-To hydrate the manifests in this repository, run the following commands:
-
-` + "```shell" + `
-git clone {{ .RepoURL }}
-# cd into the cloned directory
-git checkout {{ .DrySHA }}
-{{ range $command := .Commands -}}
-{{ $command }}
-{{ end -}}` + "```" + `
-{{ if .References -}}
-
-## References
-
-{{ range $ref := .References -}}
-{{ if $ref.Commit -}}
-* [{{ $ref.Commit.SHA | mustRegexFind "[0-9a-f]+" | trunc 7 }}]({{ $ref.Commit.RepoURL }}): {{ $ref.Commit.Subject }} ({{ $ref.Commit.Author }})
-{{ end -}}
-{{ end -}}
-{{ end -}}`

commitserver/commit/commit.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"path/filepath"
 	"sync"
 	"time"
 
@@ -155,6 +156,12 @@ func (h *Hydrator) ProcessHydrationQueueItem(hydrationKey types.HydrationQueueKe
 	})
 
 	relevantApps, drySHA, hydratedSHA, err := h.hydrateAppsLatestCommit(logCtx, hydrationKey)
+	if len(relevantApps) == 0 {
+		// return early if there are no relevant apps found to hydrate
+		// otherwise you'll be stuck in hydrating
+		logCtx.Info("Skipping hydration since there are no relevant apps found to hydrate")
+		return
+	}
 	if drySHA != "" {
 		logCtx = logCtx.WithField("drySHA", drySHA)
 	}
@@ -245,6 +252,12 @@ func (h *Hydrator) getRelevantAppsAndProjectsForHydration(logCtx *log.Entry, hyd
 			continue
 		}
 
+		path := app.Spec.SourceHydrator.SyncSource.Path
+		// ensure that the path is always set to a path that doesn't resolve to the root of the repo
+		if IsRootPath(path) {
+			return nil, nil, fmt.Errorf("app %q has path %q which resolves to repository root", app.QualifiedName(), path)
+		}
+
 		var proj *appv1.AppProject
 		// We can't short-circuit this even if we have seen this project before, because we need to verify that this
 		// particular app is allowed to use this project. That logic is in GetProcessableAppProj.
@@ -262,10 +275,10 @@ func (h *Hydrator) getRelevantAppsAndProjectsForHydration(logCtx *log.Entry, hyd
 
 		// TODO: test the dupe detection
 		// TODO: normalize the path to avoid "path/.." from being treated as different from "."
-		if _, ok := uniquePaths[app.Spec.SourceHydrator.SyncSource.Path]; ok {
+		if _, ok := uniquePaths[path]; ok {
 			return nil, nil, fmt.Errorf("multiple app hydrators use the same destination: %v", app.Spec.SourceHydrator.SyncSource.Path)
 		}
-		uniquePaths[app.Spec.SourceHydrator.SyncSource.Path] = true
+		uniquePaths[path] = true
 
 		relevantApps = append(relevantApps, &app)
 	}
@@ -282,6 +295,21 @@ func (h *Hydrator) hydrate(logCtx *log.Entry, apps []*appv1.Application, project
 	syncBranch := apps[0].Spec.SourceHydrator.SyncSource.TargetBranch
 	targetBranch := apps[0].Spec.GetHydrateToSource().TargetRevision
 
+	// Disallow hydrating to the repository root.
+	// Hydrating to root would overwrite or delete files at the top level of the repo,
+	// which can break other applications or shared configuration.
+	// Every hydrated app must write into a subdirectory instead.
+
+	for _, app := range apps {
+		destPath := app.Spec.SourceHydrator.SyncSource.Path
+		if IsRootPath(destPath) {
+			return "", "", fmt.Errorf(
+				"app %q is configured to hydrate to the repository root (branch %q, path %q) which is not allowed",
+				app.QualifiedName(), targetBranch, destPath,
+			)
+		}
+	}
+
 	// Get a static SHA revision from the first app so that all apps are hydrated from the same revision.
 	targetRevision, pathDetails, err := h.getManifests(context.Background(), apps[0], "", projects[apps[0].Spec.Project])
 	if err != nil {
@@ -468,3 +496,9 @@ func getTemplatedCommitMessage(repoURL, revision, commitMessageTemplate string,
 	}
 	return templatedCommitMsg, nil
 }
+
+// IsRootPath returns whether the path references a root path
+func IsRootPath(path string) bool {
+	clean := filepath.Clean(path)
+	return clean == "" || clean == "." || clean == string(filepath.Separator)
+}

controller/hydrator/hydrator.go

@@ -1,6 +1,7 @@
 package hydrator
 
 import (
+	"path/filepath"
 	"testing"
 	"time"
 
@@ -251,3 +252,72 @@ Co-authored-by: test [email protected]
 		})
 	}
 }
+
+func Test_getRelevantAppsForHydration_RootPathSkipped(t *testing.T) {
+	t.Parallel()
+
+	d := mocks.NewDependencies(t)
+	// create an app that has a SyncSource.Path set to root
+	d.On("GetProcessableApps").Return(&v1alpha1.ApplicationList{
+		Items: []v1alpha1.Application{
+			{
+				Spec: v1alpha1.ApplicationSpec{
+					Project: "project",
+					SourceHydrator: &v1alpha1.SourceHydrator{
+						DrySource: v1alpha1.DrySource{
+							RepoURL:        "https://example.com/repo",
+							TargetRevision: "main",
+							Path:           ".", // root path
+						},
+						SyncSource: v1alpha1.SyncSource{
+							TargetBranch: "main",
+							Path:         ".", // root path
+						},
+					},
+				},
+			},
+		},
+	}, nil)
+
+	d.On("GetProcessableAppProj", mock.Anything).Return(&v1alpha1.AppProject{
+		Spec: v1alpha1.AppProjectSpec{
+			SourceRepos: []string{"https://example.com/*"},
+		},
+	}, nil).Maybe()
+
+	hydrator := &Hydrator{dependencies: d}
+
+	hydrationKey := types.HydrationQueueKey{
+		SourceRepoURL:        "https://example.com/repo",
+		SourceTargetRevision: "main",
+		DestinationBranch:    "main",
+	}
+
+	logCtx := log.WithField("test", "RootPathSkipped")
+	relevantApps, proj, err := hydrator.getRelevantAppsAndProjectsForHydration(logCtx, hydrationKey)
+	require.Error(t, err)
+	assert.Empty(t, relevantApps, "Expected no apps to be returned because SyncSource.Path resolves to root")
+	assert.Nil(t, proj)
+}
+
+func TestIsRootPath(t *testing.T) {
+	tests := []struct {
+		name     string
+		path     string
+		expected bool
+	}{
+		{"empty string", "", true},
+		{"dot path", ".", true},
+		{"slash", string(filepath.Separator), true},
+		{"nested path", "app", false},
+		{"nested path with slash", "app/", false},
+		{"deep path", "app/config", false},
+		{"current dir with trailing slash", "./", true},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := IsRootPath(tt.path)
+			require.Equal(t, tt.expected, result)
+		})
+	}
+}

controller/hydrator/hydrator_test.go

@@ -1,5 +1,26 @@
 # v3.1 to 3.2
 
+## Breaking Changes
+
+### Hydration paths must now be non-root
+
+Source hydration now requires that every application specify a non-root path.
+Using the repository root (for example, "" or ".") is no longer supported. This change ensures
+that hydration outputs are isolated to a dedicated subdirectory and prevents accidental overwrites
+or deletions of important files stored at the root, such as CI pipelines, documentation, or configuration files.
+
+Previously, it was possible for hydration to write manifests directly into the repository root. While convenient, this had two major drawbacks:
+
+1. Hydration would wipe and replace files at the root on every run, which risked deleting important files such as CI/CD workflows, project-level READMEs, or other configuration.
+2. It made it harder to clearly separate hydrated application outputs from unrelated repository content.
+
+To identify affected applications, review your Application manifests and look for `.spec.sourceHydrator.syncSource.path` values that are empty, missing,
+`"."`, or otherwise point to the repository root. These applications must be updated to use a subdirectory path, such as `apps/guestbook`.
+
+After migration, check your repository root for any stale hydration output from earlier versions.
+Common leftovers include files such as `manifest.yaml` or `README.md`. These will not be cleaned up
+automatically and should be deleted manually if no longer needed.
+
 ## Argo CD Now Respects Kustomize Version in `.argocd-source.yaml`
 
 Argo CD provides a way to [override Application `spec.source` values](../../user-guide/parameters.md#store-overrides-in-git)

docs/operator-manual/upgrading/3.1-3.2.md

@@ -108,6 +108,14 @@ spec:
 In this example, the hydrated manifests will be pushed to the `environments/dev` branch of the `argocd-example-apps`
 repository.
 
+When using source hydration, the `syncSource.path` field is required and must always point to a non-root
+directory in the repository. Setting the path to the repository root (for eg. `"."` or `""`) is not
+supported. This ensures that hydration is always scoped to a dedicated subdirectory, which avoids unintentionally overwriting or removing files that may exist in the repository root.
+
+During each hydration run, Argo CD cleans the application's configured path before writing out newly generated manifests. This guarantees that old or stale files from previous hydration do not linger in the output directory. However, the repository root is never cleaned, so files such as CI/CD configuration, README files, or other root-level assets remain untouched.
+
+It is important to note that hydration only cleans the currently configured application path. If an application’s path changes, the old directory is not removed automatically. Likewise, if an application is deleted, its output path remains in the repository and must be cleaned up manually by the repository owner if desired. This design is intentional: it prevents accidental deletion of files when applications are restructured or removed, and it protects critical files like CI pipelines that may coexist in the repository.
+
 !!! important "Project-Scoped Repositories"
 
     Repository Secrets may contain a `project` field, making the secret only usable by Applications in that project.
@@ -372,4 +380,3 @@ to configure branch protection rules on the destination repository.
     Argo CD-specific metadata (such as `argocd.argoproj.io/tracking-id`) is
     not written to Git during hydration. These annotations are added dynamically
     during application sync and comparison.
-!!!