mirror of
https://github.com/argoproj/argo-cd.git
synced 2026-02-20 01:28:45 +01:00
feat(hydrator): don't push commits if manifests don't change (#25056)
Signed-off-by: pbhatnagar-oss <pbhatifiwork@gmail.com> Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> Co-authored-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
This commit is contained in:
@@ -7,8 +7,6 @@ import (
|
||||
"os"
|
||||
"time"
|
||||
|
||||
"github.com/argoproj/argo-cd/v3/controller/hydrator"
|
||||
|
||||
log "github.com/sirupsen/logrus"
|
||||
|
||||
"github.com/argoproj/argo-cd/v3/commitserver/apiclient"
|
||||
@@ -19,6 +17,11 @@ import (
|
||||
"github.com/argoproj/argo-cd/v3/util/io/files"
|
||||
)
|
||||
|
||||
const (
|
||||
NoteNamespace = "hydrator.metadata" // NoteNamespace is the custom git notes namespace used by the hydrator to store and retrieve commit-related metadata.
|
||||
ManifestYaml = "manifest.yaml" // ManifestYaml constant for the manifest yaml
|
||||
)
|
||||
|
||||
// Service is the service that handles commit requests.
|
||||
type Service struct {
|
||||
metricsServer *metrics.Server
|
||||
@@ -47,6 +50,13 @@ type hydratorMetadataFile struct {
|
||||
References []v1alpha1.RevisionReference `json:"references,omitempty"`
|
||||
}
|
||||
|
||||
// CommitNote represents the structure of the git note associated with a hydrated commit.
|
||||
// This struct is used to serialize/deserialize commit metadata (such as the dry run SHA)
|
||||
// stored in the custom note namespace by the hydrator.
|
||||
type CommitNote struct {
|
||||
DrySHA string `json:"drySha"` // SHA of original commit that triggerd the hydrator
|
||||
}
|
||||
|
||||
// TODO: make this configurable via ConfigMap.
|
||||
var manifestHydrationReadmeTemplate = `# Manifest Hydration
|
||||
|
||||
@@ -157,33 +167,44 @@ func (s *Service) handleCommitRequest(logCtx *log.Entry, r *apiclient.CommitHydr
|
||||
return out, "", fmt.Errorf("failed to checkout target branch: %w", err)
|
||||
}
|
||||
|
||||
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 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)
|
||||
hydratedSha, err := gitClient.CommitSHA()
|
||||
if err != nil {
|
||||
return "", "", fmt.Errorf("failed to get commit SHA: %w", err)
|
||||
}
|
||||
|
||||
if len(pathsToClear) > 0 {
|
||||
logCtx.Debugf("Clearing paths: %v", pathsToClear)
|
||||
out, err := gitClient.RemoveContents(pathsToClear)
|
||||
if err != nil {
|
||||
return out, "", fmt.Errorf("failed to clear paths %v: %w", pathsToClear, err)
|
||||
}
|
||||
/* git note changes
|
||||
1. Get the git note
|
||||
2. If found, short-circuit, log a warn and return
|
||||
3. If not, get the last manifest from git for every path, compare it with the hydrated manifest
|
||||
3a. If manifest has no changes, continue.. no need to commit it
|
||||
3b. Else, hydrate the manifest.
|
||||
3c. Push the updated note
|
||||
*/
|
||||
isHydrated, err := IsHydrated(gitClient, r.DrySha, hydratedSha)
|
||||
if err != nil {
|
||||
return "", "", fmt.Errorf("failed to get notes from git %w", err)
|
||||
}
|
||||
// short-circuit if already hydrated
|
||||
if isHydrated {
|
||||
logCtx.Debugf("this dry sha %s is already hydrated", r.DrySha)
|
||||
return "", "", nil
|
||||
}
|
||||
|
||||
logCtx.Debug("Writing manifests")
|
||||
err = WriteForPaths(root, r.Repo.Repo, r.DrySha, r.DryCommitMetadata, r.Paths)
|
||||
shouldCommit, err := WriteForPaths(root, r.Repo.Repo, r.DrySha, r.DryCommitMetadata, r.Paths, gitClient)
|
||||
// When there are no new manifests to commit, err will be nil and success will be false as nothing to commit. Else or every other error err will not be nil
|
||||
if err != nil {
|
||||
return "", "", fmt.Errorf("failed to write manifests: %w", err)
|
||||
}
|
||||
|
||||
if !shouldCommit {
|
||||
// add the note and return
|
||||
logCtx.Debug("Adding commit note")
|
||||
err = AddNote(gitClient, r.DrySha, hydratedSha)
|
||||
if err != nil {
|
||||
return "", "", fmt.Errorf("failed to add commit note: %w", err)
|
||||
}
|
||||
return "", "", nil
|
||||
}
|
||||
logCtx.Debug("Committing and pushing changes")
|
||||
out, err = gitClient.CommitAndPush(r.TargetBranch, r.CommitMessage)
|
||||
if err != nil {
|
||||
@@ -195,7 +216,12 @@ func (s *Service) handleCommitRequest(logCtx *log.Entry, r *apiclient.CommitHydr
|
||||
if err != nil {
|
||||
return "", "", fmt.Errorf("failed to get commit SHA: %w", err)
|
||||
}
|
||||
|
||||
// add the commit note
|
||||
logCtx.Debug("Adding commit note")
|
||||
err = AddNote(gitClient, r.DrySha, sha)
|
||||
if err != nil {
|
||||
return "", "", fmt.Errorf("failed to add commit note: %w", err)
|
||||
}
|
||||
return "", sha, nil
|
||||
}
|
||||
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
package commit
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
@@ -99,14 +100,15 @@ func Test_CommitHydratedManifests(t *testing.T) {
|
||||
mockGitClient.EXPECT().SetAuthor("Argo CD", "argo-cd@example.com").Return("", nil).Once()
|
||||
mockGitClient.EXPECT().CheckoutOrOrphan("env/test", false).Return("", nil).Once()
|
||||
mockGitClient.EXPECT().CheckoutOrNew("main", "env/test", false).Return("", nil).Once()
|
||||
mockGitClient.EXPECT().CommitAndPush("main", "test commit message").Return("", nil).Once()
|
||||
mockGitClient.EXPECT().GetCommitNote(mock.Anything, mock.Anything).Return("", fmt.Errorf("test %w", git.ErrNoNoteFound)).Once()
|
||||
mockGitClient.EXPECT().AddAndPushNote(mock.Anything, mock.Anything, mock.Anything).Return(nil).Once()
|
||||
mockGitClient.EXPECT().CommitSHA().Return("it-worked!", nil).Once()
|
||||
mockRepoClientFactory.EXPECT().NewClient(mock.Anything, mock.Anything).Return(mockGitClient, nil).Once()
|
||||
|
||||
resp, err := service.CommitHydratedManifests(t.Context(), validRequest)
|
||||
require.NoError(t, err)
|
||||
require.NotNil(t, resp)
|
||||
assert.Equal(t, "it-worked!", resp.HydratedSha)
|
||||
assert.Empty(t, resp.HydratedSha) // changes introduced by commit note. hydration won't happen if there are no new manifest|s to commit
|
||||
})
|
||||
|
||||
t.Run("root path with dot and blank - no directory removal", func(t *testing.T) {
|
||||
@@ -119,8 +121,11 @@ func Test_CommitHydratedManifests(t *testing.T) {
|
||||
mockGitClient.EXPECT().SetAuthor("Argo CD", "argo-cd@example.com").Return("", nil).Once()
|
||||
mockGitClient.EXPECT().CheckoutOrOrphan("env/test", false).Return("", nil).Once()
|
||||
mockGitClient.EXPECT().CheckoutOrNew("main", "env/test", false).Return("", nil).Once()
|
||||
mockGitClient.EXPECT().GetCommitNote(mock.Anything, mock.Anything).Return("", fmt.Errorf("test %w", git.ErrNoNoteFound)).Once()
|
||||
mockGitClient.EXPECT().HasFileChanged(mock.Anything).Return(true, nil).Twice()
|
||||
mockGitClient.EXPECT().CommitAndPush("main", "test commit message").Return("", nil).Once()
|
||||
mockGitClient.EXPECT().CommitSHA().Return("root-and-blank-sha", nil).Once()
|
||||
mockGitClient.EXPECT().AddAndPushNote(mock.Anything, mock.Anything, mock.Anything).Return(nil).Once()
|
||||
mockGitClient.EXPECT().CommitSHA().Return("root-and-blank-sha", nil).Twice()
|
||||
mockRepoClientFactory.EXPECT().NewClient(mock.Anything, mock.Anything).Return(mockGitClient, nil).Once()
|
||||
|
||||
requestWithRootAndBlank := &apiclient.CommitHydratedManifestsRequest{
|
||||
@@ -158,7 +163,6 @@ func Test_CommitHydratedManifests(t *testing.T) {
|
||||
|
||||
t.Run("subdirectory path - triggers directory removal", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
service, mockRepoClientFactory := newServiceWithMocks(t)
|
||||
mockGitClient := gitmocks.NewClient(t)
|
||||
mockGitClient.EXPECT().Init().Return(nil).Once()
|
||||
@@ -166,17 +170,20 @@ func Test_CommitHydratedManifests(t *testing.T) {
|
||||
mockGitClient.EXPECT().SetAuthor("Argo CD", "argo-cd@example.com").Return("", nil).Once()
|
||||
mockGitClient.EXPECT().CheckoutOrOrphan("env/test", false).Return("", nil).Once()
|
||||
mockGitClient.EXPECT().CheckoutOrNew("main", "env/test", false).Return("", nil).Once()
|
||||
mockGitClient.EXPECT().RemoveContents([]string{"apps/staging"}).Return("", nil).Once()
|
||||
mockGitClient.EXPECT().GetCommitNote(mock.Anything, mock.Anything).Return("", fmt.Errorf("test %w", git.ErrNoNoteFound)).Once()
|
||||
mockGitClient.EXPECT().HasFileChanged(mock.Anything).Return(true, nil).Once()
|
||||
mockGitClient.EXPECT().CommitAndPush("main", "test commit message").Return("", nil).Once()
|
||||
mockGitClient.EXPECT().CommitSHA().Return("subdir-path-sha", nil).Once()
|
||||
mockGitClient.EXPECT().AddAndPushNote(mock.Anything, mock.Anything, mock.Anything).Return(nil).Once()
|
||||
mockGitClient.EXPECT().CommitSHA().Return("subdir-path-sha", nil).Twice()
|
||||
mockRepoClientFactory.EXPECT().NewClient(mock.Anything, mock.Anything).Return(mockGitClient, nil).Once()
|
||||
|
||||
requestWithSubdirPath := &apiclient.CommitHydratedManifestsRequest{
|
||||
Repo: &v1alpha1.Repository{
|
||||
Repo: "https://github.com/argoproj/argocd-example-apps.git",
|
||||
},
|
||||
TargetBranch: "main",
|
||||
SyncBranch: "env/test",
|
||||
TargetBranch: "main",
|
||||
SyncBranch: "env/test",
|
||||
|
||||
CommitMessage: "test commit message",
|
||||
Paths: []*apiclient.PathDetails{
|
||||
{
|
||||
@@ -206,9 +213,11 @@ func Test_CommitHydratedManifests(t *testing.T) {
|
||||
mockGitClient.EXPECT().SetAuthor("Argo CD", "argo-cd@example.com").Return("", nil).Once()
|
||||
mockGitClient.EXPECT().CheckoutOrOrphan("env/test", false).Return("", nil).Once()
|
||||
mockGitClient.EXPECT().CheckoutOrNew("main", "env/test", false).Return("", nil).Once()
|
||||
mockGitClient.EXPECT().RemoveContents([]string{"apps/production", "apps/staging"}).Return("", nil).Once()
|
||||
mockGitClient.EXPECT().GetCommitNote(mock.Anything, mock.Anything).Return("", fmt.Errorf("test %w", git.ErrNoNoteFound)).Once()
|
||||
mockGitClient.EXPECT().HasFileChanged(mock.Anything).Return(true, nil).Times(3)
|
||||
mockGitClient.EXPECT().CommitAndPush("main", "test commit message").Return("", nil).Once()
|
||||
mockGitClient.EXPECT().CommitSHA().Return("mixed-paths-sha", nil).Once()
|
||||
mockGitClient.EXPECT().AddAndPushNote(mock.Anything, mock.Anything, mock.Anything).Return(nil).Once()
|
||||
mockGitClient.EXPECT().CommitSHA().Return("mixed-paths-sha", nil).Twice()
|
||||
mockRepoClientFactory.EXPECT().NewClient(mock.Anything, mock.Anything).Return(mockGitClient, nil).Once()
|
||||
|
||||
requestWithMixedPaths := &apiclient.CommitHydratedManifestsRequest{
|
||||
@@ -262,8 +271,9 @@ func Test_CommitHydratedManifests(t *testing.T) {
|
||||
mockGitClient.EXPECT().SetAuthor("Argo CD", "argo-cd@example.com").Return("", nil).Once()
|
||||
mockGitClient.EXPECT().CheckoutOrOrphan("env/test", false).Return("", nil).Once()
|
||||
mockGitClient.EXPECT().CheckoutOrNew("main", "env/test", false).Return("", nil).Once()
|
||||
mockGitClient.EXPECT().CommitAndPush("main", "test commit message").Return("", nil).Once()
|
||||
mockGitClient.EXPECT().CommitSHA().Return("it-worked!", nil).Once()
|
||||
mockGitClient.EXPECT().GetCommitNote(mock.Anything, mock.Anything).Return("", fmt.Errorf("test %w", git.ErrNoNoteFound)).Once()
|
||||
mockGitClient.EXPECT().AddAndPushNote(mock.Anything, mock.Anything, mock.Anything).Return(nil).Once()
|
||||
mockGitClient.EXPECT().CommitSHA().Return("empty-paths-sha", nil).Once()
|
||||
mockRepoClientFactory.EXPECT().NewClient(mock.Anything, mock.Anything).Return(mockGitClient, nil).Once()
|
||||
|
||||
requestWithEmptyPaths := &apiclient.CommitHydratedManifestsRequest{
|
||||
@@ -278,7 +288,89 @@ func Test_CommitHydratedManifests(t *testing.T) {
|
||||
resp, err := service.CommitHydratedManifests(t.Context(), requestWithEmptyPaths)
|
||||
require.NoError(t, err)
|
||||
require.NotNil(t, resp)
|
||||
assert.Equal(t, "it-worked!", resp.HydratedSha)
|
||||
assert.Empty(t, resp.HydratedSha) // changes introduced by commit note. hydration won't happen if there are no new manifest|s to commit
|
||||
})
|
||||
|
||||
t.Run("duplicate request already hydrated", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
strnote := "{\"drySha\":\"abc123\"}"
|
||||
service, mockRepoClientFactory := newServiceWithMocks(t)
|
||||
mockGitClient := gitmocks.NewClient(t)
|
||||
mockGitClient.EXPECT().Init().Return(nil).Once()
|
||||
mockGitClient.EXPECT().Fetch(mock.Anything, mock.Anything).Return(nil).Once()
|
||||
mockGitClient.EXPECT().SetAuthor("Argo CD", "argo-cd@example.com").Return("", nil).Once()
|
||||
mockGitClient.EXPECT().CheckoutOrOrphan("env/test", false).Return("", nil).Once()
|
||||
mockGitClient.EXPECT().CheckoutOrNew("main", "env/test", false).Return("", nil).Once()
|
||||
mockGitClient.EXPECT().GetCommitNote(mock.Anything, mock.Anything).Return(strnote, nil).Once()
|
||||
mockGitClient.EXPECT().CommitSHA().Return("dupe-test-sha", nil).Once()
|
||||
mockRepoClientFactory.EXPECT().NewClient(mock.Anything, mock.Anything).Return(mockGitClient, nil).Once()
|
||||
|
||||
request := &apiclient.CommitHydratedManifestsRequest{
|
||||
Repo: &v1alpha1.Repository{
|
||||
Repo: "https://github.com/argoproj/argocd-example-apps.git",
|
||||
},
|
||||
TargetBranch: "main",
|
||||
SyncBranch: "env/test",
|
||||
DrySha: "abc123",
|
||||
CommitMessage: "test commit message",
|
||||
Paths: []*apiclient.PathDetails{
|
||||
{
|
||||
Path: ".",
|
||||
Manifests: []*apiclient.HydratedManifestDetails{
|
||||
{
|
||||
ManifestJSON: `{"apiVersion":"v1","kind":"Deployment","metadata":{"name":"test-app"}}`,
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
resp, err := service.CommitHydratedManifests(t.Context(), request)
|
||||
require.NoError(t, err)
|
||||
require.NotNil(t, resp)
|
||||
assert.Empty(t, resp.HydratedSha) // changes introduced by commit note. hydration won't happen if there are no new manifest|s to commit
|
||||
})
|
||||
|
||||
t.Run("root path with dot - no changes to manifest - should commit note only", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
service, mockRepoClientFactory := newServiceWithMocks(t)
|
||||
mockGitClient := gitmocks.NewClient(t)
|
||||
mockGitClient.EXPECT().Init().Return(nil).Once()
|
||||
mockGitClient.EXPECT().Fetch(mock.Anything, mock.Anything).Return(nil).Once()
|
||||
mockGitClient.EXPECT().SetAuthor("Argo CD", "argo-cd@example.com").Return("", nil).Once()
|
||||
mockGitClient.EXPECT().CheckoutOrOrphan("env/test", false).Return("", nil).Once()
|
||||
mockGitClient.EXPECT().CheckoutOrNew("main", "env/test", false).Return("", nil).Once()
|
||||
mockGitClient.EXPECT().GetCommitNote(mock.Anything, mock.Anything).Return("", fmt.Errorf("test %w", git.ErrNoNoteFound)).Once()
|
||||
mockGitClient.EXPECT().HasFileChanged(mock.Anything).Return(false, nil).Once()
|
||||
mockGitClient.EXPECT().AddAndPushNote(mock.Anything, mock.Anything, mock.Anything).Return(nil).Once()
|
||||
mockGitClient.EXPECT().CommitSHA().Return("root-and-blank-sha", nil).Once()
|
||||
mockRepoClientFactory.EXPECT().NewClient(mock.Anything, mock.Anything).Return(mockGitClient, nil).Once()
|
||||
|
||||
requestWithRootAndBlank := &apiclient.CommitHydratedManifestsRequest{
|
||||
Repo: &v1alpha1.Repository{
|
||||
Repo: "https://github.com/argoproj/argocd-example-apps.git",
|
||||
},
|
||||
TargetBranch: "main",
|
||||
SyncBranch: "env/test",
|
||||
CommitMessage: "test commit message",
|
||||
Paths: []*apiclient.PathDetails{
|
||||
{
|
||||
Path: ".",
|
||||
Manifests: []*apiclient.HydratedManifestDetails{
|
||||
{
|
||||
ManifestJSON: `{"apiVersion":"v1","kind":"ConfigMap","metadata":{"name":"test-dot"}}`,
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
resp, err := service.CommitHydratedManifests(t.Context(), requestWithRootAndBlank)
|
||||
require.NoError(t, err)
|
||||
require.NotNil(t, resp)
|
||||
assert.Empty(t, resp.HydratedSha)
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
@@ -2,6 +2,7 @@ package commit
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
"os"
|
||||
"path/filepath"
|
||||
@@ -15,6 +16,7 @@ import (
|
||||
"github.com/argoproj/argo-cd/v3/commitserver/apiclient"
|
||||
"github.com/argoproj/argo-cd/v3/common"
|
||||
appv1 "github.com/argoproj/argo-cd/v3/pkg/apis/application/v1alpha1"
|
||||
"github.com/argoproj/argo-cd/v3/util/git"
|
||||
"github.com/argoproj/argo-cd/v3/util/hydrator"
|
||||
"github.com/argoproj/argo-cd/v3/util/io"
|
||||
)
|
||||
@@ -33,24 +35,24 @@ func init() {
|
||||
|
||||
// WriteForPaths writes the manifests, hydrator.metadata, and README.md files for each path in the provided paths. It
|
||||
// also writes a root-level hydrator.metadata file containing the repo URL and dry SHA.
|
||||
func WriteForPaths(root *os.Root, repoUrl, drySha string, dryCommitMetadata *appv1.RevisionMetadata, paths []*apiclient.PathDetails) error { //nolint:revive //FIXME(var-naming)
|
||||
func WriteForPaths(root *os.Root, repoUrl, drySha string, dryCommitMetadata *appv1.RevisionMetadata, paths []*apiclient.PathDetails, gitClient git.Client) (bool, error) { //nolint:revive //FIXME(var-naming)
|
||||
hydratorMetadata, err := hydrator.GetCommitMetadata(repoUrl, drySha, dryCommitMetadata)
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to retrieve hydrator metadata: %w", err)
|
||||
return false, fmt.Errorf("failed to retrieve hydrator metadata: %w", err)
|
||||
}
|
||||
|
||||
// Write the top-level readme.
|
||||
err = writeMetadata(root, "", hydratorMetadata)
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to write top-level hydrator metadata: %w", err)
|
||||
return false, fmt.Errorf("failed to write top-level hydrator metadata: %w", err)
|
||||
}
|
||||
|
||||
// Write .gitattributes
|
||||
err = writeGitAttributes(root)
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to write git attributes: %w", err)
|
||||
return false, fmt.Errorf("failed to write git attributes: %w", err)
|
||||
}
|
||||
|
||||
var atleastOneManifestChanged bool
|
||||
for _, p := range paths {
|
||||
hydratePath := p.Path
|
||||
if hydratePath == "." {
|
||||
@@ -61,15 +63,26 @@ func WriteForPaths(root *os.Root, repoUrl, drySha string, dryCommitMetadata *app
|
||||
if hydratePath != "" {
|
||||
err = root.MkdirAll(hydratePath, 0o755)
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to create path: %w", err)
|
||||
return false, fmt.Errorf("failed to create path: %w", err)
|
||||
}
|
||||
}
|
||||
|
||||
// Write the manifests
|
||||
err = writeManifests(root, hydratePath, p.Manifests)
|
||||
err := writeManifests(root, hydratePath, p.Manifests)
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to write manifests: %w", err)
|
||||
return false, fmt.Errorf("failed to write manifests: %w", err)
|
||||
}
|
||||
// Check if the manifest file has been modified compared to the git index
|
||||
changed, err := gitClient.HasFileChanged(filepath.Join(hydratePath, ManifestYaml))
|
||||
if err != nil {
|
||||
return false, fmt.Errorf("failed to check if anything changed on the manifest: %w", err)
|
||||
}
|
||||
|
||||
if !changed {
|
||||
continue
|
||||
}
|
||||
// If any manifest has changed, signal that a commit should occur. If none have changed, skip committing.
|
||||
atleastOneManifestChanged = changed
|
||||
|
||||
// Write hydrator.metadata containing information about the hydration process.
|
||||
hydratorMetadata := hydrator.HydratorCommitMetadata{
|
||||
@@ -79,16 +92,20 @@ func WriteForPaths(root *os.Root, repoUrl, drySha string, dryCommitMetadata *app
|
||||
}
|
||||
err = writeMetadata(root, hydratePath, hydratorMetadata)
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to write hydrator metadata: %w", err)
|
||||
return false, fmt.Errorf("failed to write hydrator metadata: %w", err)
|
||||
}
|
||||
|
||||
// Write README
|
||||
err = writeReadme(root, hydratePath, hydratorMetadata)
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to write readme: %w", err)
|
||||
return false, fmt.Errorf("failed to write readme: %w", err)
|
||||
}
|
||||
}
|
||||
return nil
|
||||
// if no manifest changes then skip commit
|
||||
if !atleastOneManifestChanged {
|
||||
return false, nil
|
||||
}
|
||||
return atleastOneManifestChanged, nil
|
||||
}
|
||||
|
||||
// writeMetadata writes the metadata to the hydrator.metadata file.
|
||||
@@ -163,7 +180,7 @@ func writeGitAttributes(root *os.Root) error {
|
||||
func writeManifests(root *os.Root, dirPath string, manifests []*apiclient.HydratedManifestDetails) error {
|
||||
// If the file exists, truncate it.
|
||||
// No need to use SecureJoin here, as the path is already sanitized.
|
||||
manifestPath := filepath.Join(dirPath, "manifest.yaml")
|
||||
manifestPath := filepath.Join(dirPath, ManifestYaml)
|
||||
|
||||
file, err := root.OpenFile(manifestPath, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, os.ModePerm)
|
||||
if err != nil {
|
||||
@@ -196,6 +213,43 @@ func writeManifests(root *os.Root, dirPath string, manifests []*apiclient.Hydrat
|
||||
return fmt.Errorf("failed to encode manifest: %w", err)
|
||||
}
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
// IsHydrated checks whether the given commit (commitSha) has already been hydrated with the specified Dry SHA (drySha).
|
||||
// It does this by retrieving the commit note in the NoteNamespace and examining the DrySHA value.
|
||||
// Returns true if the stored DrySHA matches the provided drySha, false if not or if no note exists.
|
||||
// Gracefully handles missing notes as a normal outcome (not an error), but returns an error on retrieval or parse failures.
|
||||
func IsHydrated(gitClient git.Client, drySha, commitSha string) (bool, error) {
|
||||
note, err := gitClient.GetCommitNote(commitSha, NoteNamespace)
|
||||
if err != nil {
|
||||
// note not found is a valid and acceptable outcome in this context so returning false and nil to let the hydration continue
|
||||
unwrappedError := errors.Unwrap(err)
|
||||
if unwrappedError != nil && errors.Is(unwrappedError, git.ErrNoNoteFound) {
|
||||
return false, nil
|
||||
}
|
||||
return false, err
|
||||
}
|
||||
var commitNote CommitNote
|
||||
err = json.Unmarshal([]byte(note), &commitNote)
|
||||
if err != nil {
|
||||
return false, fmt.Errorf("json unmarshal failed %w", err)
|
||||
}
|
||||
return commitNote.DrySHA == drySha, nil
|
||||
}
|
||||
|
||||
// AddNote attaches a commit note containing the specified dry SHA (`drySha`) to the given commit (`commitSha`)
|
||||
// in the configured note namespace. The note is marshaled as JSON and pushed to the remote repository using
|
||||
// the provided gitClient. Returns an error if marshalling or note addition fails.
|
||||
func AddNote(gitClient git.Client, drySha, commitSha string) error {
|
||||
note := CommitNote{DrySHA: drySha}
|
||||
jsonBytes, err := json.Marshal(note)
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to marshal commit note: %w", err)
|
||||
}
|
||||
err = gitClient.AddAndPushNote(commitSha, NoteNamespace, string(jsonBytes))
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to add commit note: %w", err)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -5,6 +5,7 @@ import (
|
||||
"crypto/sha256"
|
||||
"encoding/hex"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
"os"
|
||||
"path"
|
||||
@@ -13,11 +14,14 @@ import (
|
||||
"time"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/mock"
|
||||
"github.com/stretchr/testify/require"
|
||||
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
|
||||
|
||||
"github.com/argoproj/argo-cd/v3/commitserver/apiclient"
|
||||
appsv1 "github.com/argoproj/argo-cd/v3/pkg/apis/application/v1alpha1"
|
||||
"github.com/argoproj/argo-cd/v3/util/git"
|
||||
gitmocks "github.com/argoproj/argo-cd/v3/util/git/mocks"
|
||||
"github.com/argoproj/argo-cd/v3/util/hydrator"
|
||||
)
|
||||
|
||||
@@ -92,9 +96,12 @@ Argocd-reference-commit-sha: abc123
|
||||
},
|
||||
},
|
||||
}
|
||||
mockGitClient := gitmocks.NewClient(t)
|
||||
mockGitClient.On("HasFileChanged", mock.Anything).Return(true, nil).Times(len(paths))
|
||||
|
||||
err := WriteForPaths(root, repoURL, drySha, metadata, paths)
|
||||
shouldCommit, err := WriteForPaths(root, repoURL, drySha, metadata, paths, mockGitClient)
|
||||
require.NoError(t, err)
|
||||
require.True(t, shouldCommit)
|
||||
|
||||
// Check if the top-level hydrator.metadata exists and contains the repo URL and dry SHA
|
||||
topMetadataPath := filepath.Join(root.Name(), "hydrator.metadata")
|
||||
@@ -142,6 +149,117 @@ Argocd-reference-commit-sha: abc123
|
||||
}
|
||||
}
|
||||
|
||||
func TestWriteForPaths_WithOneManifestMatchesExisting(t *testing.T) {
|
||||
root := tempRoot(t)
|
||||
|
||||
repoURL := "https://github.com/example/repo"
|
||||
drySha := "abc123"
|
||||
paths := []*apiclient.PathDetails{
|
||||
{
|
||||
Path: "path1",
|
||||
Manifests: []*apiclient.HydratedManifestDetails{
|
||||
{ManifestJSON: `{"kind":"Pod","apiVersion":"v1"}`},
|
||||
},
|
||||
Commands: []string{"command1", "command2"},
|
||||
},
|
||||
{
|
||||
Path: "path2",
|
||||
Manifests: []*apiclient.HydratedManifestDetails{
|
||||
{ManifestJSON: `{"kind":"Service","apiVersion":"v1"}`},
|
||||
},
|
||||
Commands: []string{"command3"},
|
||||
},
|
||||
{
|
||||
Path: "path3/nested",
|
||||
Manifests: []*apiclient.HydratedManifestDetails{
|
||||
{ManifestJSON: `{"kind":"Deployment","apiVersion":"apps/v1"}`},
|
||||
},
|
||||
Commands: []string{"command4"},
|
||||
},
|
||||
}
|
||||
|
||||
now := metav1.NewTime(time.Now())
|
||||
metadata := &appsv1.RevisionMetadata{
|
||||
Author: "test-author",
|
||||
Date: &now,
|
||||
Message: `test-message
|
||||
|
||||
Signed-off-by: Test User <test@example.com>
|
||||
Argocd-reference-commit-sha: abc123
|
||||
`,
|
||||
References: []appsv1.RevisionReference{
|
||||
{
|
||||
Commit: &appsv1.CommitMetadata{
|
||||
Author: "test-code-author <test-email-author@example.com>",
|
||||
Date: now.Format(time.RFC3339),
|
||||
Subject: "test-code-subject",
|
||||
SHA: "test-code-sha",
|
||||
RepoURL: "https://example.com/test/repo.git",
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
mockGitClient := gitmocks.NewClient(t)
|
||||
mockGitClient.On("HasFileChanged", "path1/manifest.yaml").Return(true, nil).Once()
|
||||
mockGitClient.On("HasFileChanged", "path2/manifest.yaml").Return(true, nil).Once()
|
||||
mockGitClient.On("HasFileChanged", "path3/nested/manifest.yaml").Return(false, nil).Once()
|
||||
|
||||
shouldCommit, err := WriteForPaths(root, repoURL, drySha, metadata, paths, mockGitClient)
|
||||
require.NoError(t, err)
|
||||
require.True(t, shouldCommit)
|
||||
|
||||
// Check if the top-level hydrator.metadata exists and contains the repo URL and dry SHA
|
||||
topMetadataPath := filepath.Join(root.Name(), "hydrator.metadata")
|
||||
topMetadataBytes, err := os.ReadFile(topMetadataPath)
|
||||
require.NoError(t, err)
|
||||
|
||||
var topMetadata hydratorMetadataFile
|
||||
err = json.Unmarshal(topMetadataBytes, &topMetadata)
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, repoURL, topMetadata.RepoURL)
|
||||
assert.Equal(t, drySha, topMetadata.DrySHA)
|
||||
assert.Equal(t, metadata.Author, topMetadata.Author)
|
||||
assert.Equal(t, "test-message", topMetadata.Subject)
|
||||
// The body should exclude the Argocd- trailers.
|
||||
assert.Equal(t, "Signed-off-by: Test User <test@example.com>\n", topMetadata.Body)
|
||||
assert.Equal(t, metadata.Date.Format(time.RFC3339), topMetadata.Date)
|
||||
assert.Equal(t, metadata.References, topMetadata.References)
|
||||
|
||||
for _, p := range paths {
|
||||
fullHydratePath := filepath.Join(root.Name(), p.Path)
|
||||
if p.Path == "path3/nested" {
|
||||
assert.DirExists(t, fullHydratePath)
|
||||
manifestPath := path.Join(fullHydratePath, "manifest.yaml")
|
||||
_, err := os.ReadFile(manifestPath)
|
||||
require.NoError(t, err)
|
||||
continue
|
||||
}
|
||||
// Check if each path directory exists
|
||||
assert.DirExists(t, fullHydratePath)
|
||||
|
||||
// Check if each path contains a hydrator.metadata file and contains the repo URL
|
||||
metadataPath := path.Join(fullHydratePath, "hydrator.metadata")
|
||||
metadataBytes, err := os.ReadFile(metadataPath)
|
||||
require.NoError(t, err)
|
||||
|
||||
var readMetadata hydratorMetadataFile
|
||||
err = json.Unmarshal(metadataBytes, &readMetadata)
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, repoURL, readMetadata.RepoURL)
|
||||
// Check if each path contains a README.md file and contains the repo URL
|
||||
readmePath := path.Join(fullHydratePath, "README.md")
|
||||
readmeBytes, err := os.ReadFile(readmePath)
|
||||
require.NoError(t, err)
|
||||
assert.Contains(t, string(readmeBytes), repoURL)
|
||||
|
||||
// Check if each path contains a manifest.yaml file and contains the word kind
|
||||
manifestPath := path.Join(fullHydratePath, "manifest.yaml")
|
||||
manifestBytes, err := os.ReadFile(manifestPath)
|
||||
require.NoError(t, err)
|
||||
assert.Contains(t, string(manifestBytes), "kind")
|
||||
}
|
||||
}
|
||||
|
||||
func TestWriteMetadata(t *testing.T) {
|
||||
root := tempRoot(t)
|
||||
|
||||
@@ -237,3 +355,49 @@ func TestWriteGitAttributes(t *testing.T) {
|
||||
assert.Contains(t, string(gitAttributesBytes), "*/README.md linguist-generated=true")
|
||||
assert.Contains(t, string(gitAttributesBytes), "*/hydrator.metadata linguist-generated=true")
|
||||
}
|
||||
|
||||
func TestIsHydrated(t *testing.T) {
|
||||
mockGitClient := gitmocks.NewClient(t)
|
||||
drySha := "abc123"
|
||||
commitSha := "fff456"
|
||||
commitShaNoNoteFoundErr := "abc456"
|
||||
commitShaErr := "abc999"
|
||||
strnote := "{\"drySha\":\"abc123\"}"
|
||||
mockGitClient.On("GetCommitNote", commitSha, mock.Anything).Return(strnote, nil).Once()
|
||||
mockGitClient.On("GetCommitNote", commitShaNoNoteFoundErr, mock.Anything).Return("", fmt.Errorf("wrapped error %w", git.ErrNoNoteFound)).Once()
|
||||
// an existing note
|
||||
isHydrated, err := IsHydrated(mockGitClient, drySha, commitSha)
|
||||
require.NoError(t, err)
|
||||
assert.True(t, isHydrated)
|
||||
|
||||
// no note found treated as success.. no error returned
|
||||
isHydrated, err = IsHydrated(mockGitClient, drySha, commitShaNoNoteFoundErr)
|
||||
require.NoError(t, err)
|
||||
assert.False(t, isHydrated)
|
||||
|
||||
// Test that non-ErrNoNoteFound errors are propagated: when GetCommitNote fails with
|
||||
// an error other than "no note found", IsHydrated should return that error to the caller
|
||||
err = errors.New("some other error")
|
||||
mockGitClient.On("GetCommitNote", commitShaErr, mock.Anything).Return("", fmt.Errorf("wrapped error %w", err)).Once()
|
||||
isHydrated, err = IsHydrated(mockGitClient, drySha, commitShaErr)
|
||||
require.Error(t, err)
|
||||
assert.False(t, isHydrated)
|
||||
}
|
||||
|
||||
func TestAddNote(t *testing.T) {
|
||||
mockGitClient := gitmocks.NewClient(t)
|
||||
drySha := "abc123"
|
||||
commitSha := "fff456"
|
||||
commitShaErr := "abc456"
|
||||
err := errors.New("test error")
|
||||
mockGitClient.On("AddAndPushNote", commitSha, mock.Anything, mock.Anything).Return(nil).Once()
|
||||
mockGitClient.On("AddAndPushNote", commitShaErr, mock.Anything, mock.Anything).Return(err).Once()
|
||||
|
||||
// success
|
||||
err = AddNote(mockGitClient, drySha, commitSha)
|
||||
require.NoError(t, err)
|
||||
|
||||
// failure
|
||||
err = AddNote(mockGitClient, drySha, commitShaErr)
|
||||
require.Error(t, err)
|
||||
}
|
||||
|
||||
@@ -1,5 +1,64 @@
|
||||
# v3.2 to 3.3
|
||||
|
||||
## Breaking Changes
|
||||
|
||||
### Source Hydrator Now Tracks Hydration State Using Git Notes
|
||||
|
||||
Previously, Argo CD's Source Hydrator pushed a new hydrated commit for every DRY (source) commit, regardless of whether any manifest files (`manifest.yaml`) actually changed. This was necessary for the hydrator to track which DRY commit had last been hydrated: it embedded this information in the `hydrator.metadata` file's `drySha` field in each hydrated commit.
|
||||
|
||||
**Starting with v3.3, the Source Hydrator uses [git notes](https://git-scm.com/docs/git-notes) to record the state of the most recently hydrated DRY commit instead of creating a hydrated commit on every DRY commit.** The git note is stored in a dedicated namespace reserved for the Source Hydrator, making the state both reliable and isolated from other repository operations.
|
||||
|
||||
This design change improves repository cleanliness, reduces unnecessary commit noise, and lowers risk from excessive branch divergence due to frequent automated commits.
|
||||
|
||||
#### How It Works
|
||||
|
||||
- **On each hydration run:**
|
||||
1. The hydrator first pulls the existing git note in its namespace to check the last hydrated DRY commit SHA.
|
||||
2. If the note SHA matches the latest DRY SHA, the hydrator logs a debug message and skips hydration.
|
||||
3. If files such as `manifest.yaml` have *not* changed (even if DRY SHA is new), the hydrator skips committing manifests and only updates the git note to reflect the latest hydrated DRY SHA.
|
||||
4. If manifest files have changed, the hydrator commits the updated manifests and also updates the git note.
|
||||
|
||||
#### Migration Impact
|
||||
|
||||
- **Behavioral:**
|
||||
- Users will no longer see a hydrated commit for every DRY commit. Only actual manifest changes will create a new commit; otherwise, the hydration state is tracked in the git note namespace.
|
||||
- **Operational:**
|
||||
- Applications and tools that depended on a hydrated commit for every DRY commit should now use the git note in the Source Hydrator namespace to determine the hydration state.
|
||||
- **No action needed** for most users, but if you have automations dependent on hydrated commits as signals, please update them to consult the new git note.
|
||||
|
||||
#### Rationale and Benefits
|
||||
|
||||
- **Reduces repository clutter**: fewer unnecessary commits.
|
||||
- **Improves performance** for teams with high-frequency DRY changes and rare manifest changes.
|
||||
- **Decreases risk** of merge conflicts and branch bloat in automation scenarios.
|
||||
|
||||
### Removal of Application Path Cleaning During Hydration
|
||||
|
||||
- **Behavioral Change:**
|
||||
In Argo CD versions prior to v3.3, the Source Hydrator would automatically clean (delete all files in) the application's configured path before writing out new manifest files during each hydration run. Starting with v3.3, this cleaning logic has been removed. The hydrator will now only overwrite or create files that correspond to the current manifest output; any additional files or obsolete data in the application path will remain untouched unless explicitly overwritten.
|
||||
- **Operational Impact:**
|
||||
If your repository or automation relied on automatic cleanup of stale or obsolete files in application paths, you must now handle this cleanup manually. This change avoids the risk of accidental deletion of unrelated files (for instance, if application paths overlap with directories containing other assets or when applications are restructured).
|
||||
- **Recommended Action:**
|
||||
Review your automation workflows and repository maintenance scripts to ensure that old or unwanted files in application paths are cleaned up if necessary. Consider implementing a periodic manual or automated cleanup procedure if your use case requires it.
|
||||
- For more details on current behavior, see the [Source Hydrator user guide](../../user-guide/source-hydrator.md).
|
||||
|
||||
### Anonymous call to Settings API returns fewer fields
|
||||
|
||||
The Settings API now returns less information when accessed anonymously.
|
||||
It no longer returns the `resourceOverrides` field which is considered sensitive information.
|
||||
|
||||
### New ENV Variable to control K8s Server Side Timeout of API Requests
|
||||
|
||||
The new environment variable `ARGOCD_K8S_SERVER_SIDE_TIMEOUT` can be used to control the K8s server side timeout of API requests.
|
||||
In 3.2 and before this change, the K8s server side timeout was controlled by `ARGOCD_K8S_TCP_TIMEOUT`
|
||||
which is also used to control the TCP timeout when communicating with the K8s API server.
|
||||
From now onwards, the Kubernetes server-side timeout is controlled by a separate environment variable.
|
||||
|
||||
### Deprecated --self-heal-backoff-cooldown-seconds flag
|
||||
|
||||
The `--self-heal-backoff-cooldown-seconds` flag of the `argocd-application-controller` has been deprecated and will be
|
||||
removed in a future release.
|
||||
|
||||
## Helm Upgraded to 3.19.2
|
||||
|
||||
Argo CD v3.3 upgrades the bundled Helm version to 3.19.2. There are no breaking changes in Helm 3.19.2 according to the
|
||||
@@ -19,21 +78,3 @@ Kustomize 5.8.0 also resolves an issue where namespaces were not properly propag
|
||||
If you rely on Helm charts within kustomization files, please review the details in the associated fix:
|
||||
[kubernetes-sigs/kustomize#5940](https://github.com/kubernetes-sigs/kustomize/pull/5940).
|
||||
|
||||
## Breaking Changes
|
||||
|
||||
### Anonymous call to Settings API returns fewer fields
|
||||
|
||||
The Settings API now returns less information when accessed anonymously.
|
||||
It no longer returns the `resourceOverrides` field which is considered sensitive information.
|
||||
|
||||
### New ENV Variable to control K8s Server Side Timeout of API Requests
|
||||
|
||||
The new environment variable `ARGOCD_K8S_SERVER_SIDE_TIMEOUT` can be used to control the K8s server side timeout of API requests.
|
||||
In 3.2 and before this change, the K8s server side timeout was controlled by `ARGOCD_K8S_TCP_TIMEOUT`
|
||||
which is also used to control the TCP timeout when communicating with the K8s API server.
|
||||
From now onwards, the Kubernetes server-side timeout is controlled by a separate environment variable.
|
||||
|
||||
### Deprecated --self-heal-backoff-cooldown-seconds flag
|
||||
|
||||
The `--self-heal-backoff-cooldown-seconds` flag of the `argocd-application-controller` has been deprecated and will be
|
||||
removed in a future release.
|
||||
@@ -437,6 +437,17 @@ stringData:
|
||||
username: my-username
|
||||
```
|
||||
|
||||
## Git Note-based Hydration State Tracking
|
||||
|
||||
The Source Hydrator does not create a new hydrated commit for a DRY commit if the commit doesn't affect the hydrated manifests. Instead, the hydration state (the DRY SHA last hydrated) is tracked using a [git note](https://git-scm.com/docs/git-notes) in a dedicated `source-hydrator` namespace.
|
||||
|
||||
On each run, the hydrator:
|
||||
- Checks the git note for the last hydrated DRY SHA.
|
||||
- If manifests have not changed since that SHA, only updates the note.
|
||||
- If manifests have changed, commits the new manifests and updates the note as well.
|
||||
|
||||
This improves efficiency and reduces commit noise in your repository.
|
||||
|
||||
## Limitations
|
||||
|
||||
### Signature Verification
|
||||
@@ -498,3 +509,7 @@ 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.
|
||||
|
||||
### Application Path Cleaning Behavior
|
||||
|
||||
The Source Hydrator does not clean (remove) files from the application's configured output path before writing new manifests. This means that any files previously generated by hydration (or otherwise present) that are not overwritten by the new hydration run will remain in the output directory.
|
||||
@@ -85,7 +85,7 @@ func TestAddingApp(t *testing.T) {
|
||||
DrySourcePath("guestbook").
|
||||
DrySourceRevision("HEAD").
|
||||
SyncSourcePath("guestbook-2").
|
||||
SyncSourceBranch("env/test").
|
||||
SyncSourceBranch("env/test2").
|
||||
When().
|
||||
CreateApp().
|
||||
// a new git commit, that has a new revisionHistoryLimit.
|
||||
|
||||
@@ -44,7 +44,10 @@ import (
|
||||
"github.com/argoproj/argo-cd/v3/util/versions"
|
||||
)
|
||||
|
||||
var ErrInvalidRepoURL = errors.New("repo URL is invalid")
|
||||
var (
|
||||
ErrInvalidRepoURL = errors.New("repo URL is invalid")
|
||||
ErrNoNoteFound = errors.New("no note found")
|
||||
)
|
||||
|
||||
// builtinGitConfig configuration contains statements that are needed
|
||||
// for correct ArgoCD operation. These settings will override any
|
||||
@@ -145,6 +148,12 @@ type Client interface {
|
||||
RemoveContents(paths []string) (string, error)
|
||||
// CommitAndPush commits and pushes changes to the target branch.
|
||||
CommitAndPush(branch, message string) (string, error)
|
||||
// GetCommitNote gets the note associated with the DRY sha stored in the specific namespace
|
||||
GetCommitNote(sha string, namespace string) (string, error)
|
||||
// AddAndPushNote adds a note to a DRY sha and then pushes it.
|
||||
AddAndPushNote(sha string, namespace string, note string) error
|
||||
// HasFileChanged returns the outout of git diff considering whether it is tracked or un-tracked
|
||||
HasFileChanged(filePath string) (bool, error)
|
||||
}
|
||||
|
||||
type EventHandlers struct {
|
||||
@@ -1110,6 +1119,73 @@ func (m *nativeGitClient) CommitAndPush(branch, message string) (string, error)
|
||||
return "", nil
|
||||
}
|
||||
|
||||
// GetCommitNote gets the note associated with the DRY sha stored in the specific namespace
|
||||
func (m *nativeGitClient) GetCommitNote(sha string, namespace string) (string, error) {
|
||||
if strings.TrimSpace(namespace) == "" {
|
||||
namespace = "commit"
|
||||
}
|
||||
ctx := context.Background()
|
||||
// fetch first
|
||||
// cli command: git fetch origin refs/notes/source-hydrator:refs/notes/source-hydrator
|
||||
notesRef := "refs/notes/" + namespace
|
||||
_, _ = m.runCmd(ctx, "fetch", "origin", fmt.Sprintf("%s:%s", notesRef, notesRef)) // Ignore fetch error for best effort
|
||||
|
||||
ref := "--ref=" + namespace
|
||||
out, err := m.runCmd(ctx, "notes", ref, "show", sha)
|
||||
if err != nil {
|
||||
if strings.Contains(err.Error(), "no note found") {
|
||||
return out, fmt.Errorf("failed to get commit note: %w", ErrNoNoteFound)
|
||||
}
|
||||
return out, fmt.Errorf("failed to get commit note: %w", err)
|
||||
}
|
||||
return strings.TrimSpace(out), nil
|
||||
}
|
||||
|
||||
// AddAndPushNote adds a note to a DRY sha and then pushes it.
|
||||
func (m *nativeGitClient) AddAndPushNote(sha string, namespace string, note string) error {
|
||||
if namespace == "" {
|
||||
namespace = "commit"
|
||||
}
|
||||
ctx := context.Background()
|
||||
ref := "--ref=" + namespace
|
||||
_, err := m.runCmd(ctx, "notes", ref, "add", "-f", "-m", note, sha)
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to push: %w", err)
|
||||
}
|
||||
if m.OnPush != nil {
|
||||
done := m.OnPush(m.repoURL)
|
||||
defer done()
|
||||
}
|
||||
|
||||
err = m.runCredentialedCmd(ctx, "push", "-f", "origin", "refs/notes/"+namespace)
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to push: %w", err)
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
// HasFileChanged returns the outout of git diff considering whether it is tracked or un-tracked
|
||||
func (m *nativeGitClient) HasFileChanged(filePath string) (bool, error) {
|
||||
// Step 1: Is it UNTRACKED? (file is new to git)
|
||||
_, err := m.runCmd(context.Background(), "ls-files", "--error-unmatch", filePath)
|
||||
if err != nil {
|
||||
// File is NOT tracked by git → means it's new/unadded
|
||||
return true, nil
|
||||
}
|
||||
// use git diff --quiet and check exit code .. --cached is to consider files staged for deletion
|
||||
_, err = m.runCmd(context.Background(), "diff", "--quiet", "--", filePath)
|
||||
if err == nil {
|
||||
return false, nil // No changes
|
||||
}
|
||||
// Exit code 1 indicates: changes found
|
||||
if strings.Contains(err.Error(), "exit status 1") {
|
||||
return true, nil
|
||||
}
|
||||
// always return the actual wrapped error
|
||||
return false, fmt.Errorf("git diff failed: %w", err)
|
||||
}
|
||||
|
||||
// runWrapper runs a custom command with all the semantics of running the Git client
|
||||
func (m *nativeGitClient) runGnuPGWrapper(ctx context.Context, wrapper string, args ...string) (string, error) {
|
||||
cmd := exec.CommandContext(ctx, wrapper, args...)
|
||||
|
||||
@@ -1279,3 +1279,182 @@ func Test_GitNoDetachedMaintenance(t *testing.T) {
|
||||
}
|
||||
assert.Fail(t, "Expected to see `git maintenance` run after `git fetch`")
|
||||
}
|
||||
|
||||
func Test_nativeGitClient_GetCommitNote(t *testing.T) {
|
||||
ctx := t.Context()
|
||||
tempDir, err := _createEmptyGitRepo(ctx)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Allow pushing to the same local repo (non-bare)
|
||||
err = runCmd(ctx, tempDir, "git", "config", "--local", "receive.denyCurrentBranch", "updateInstead")
|
||||
require.NoError(t, err)
|
||||
|
||||
// Get the current branch name
|
||||
gitCurrentBranch, err := outputCmd(ctx, tempDir, "git", "rev-parse", "--abbrev-ref", "HEAD")
|
||||
require.NoError(t, err)
|
||||
branch := strings.TrimSpace(string(gitCurrentBranch))
|
||||
|
||||
// Initialize client that uses this same repo as origin
|
||||
client, err := NewClient("file://"+tempDir, NopCreds{}, true, false, "", "")
|
||||
require.NoError(t, err)
|
||||
|
||||
err = client.Init()
|
||||
require.NoError(t, err)
|
||||
|
||||
out, err := client.SetAuthor("test", "test@example.com")
|
||||
require.NoError(t, err, "error output: ", out)
|
||||
|
||||
err = client.Fetch(branch, 0)
|
||||
require.NoError(t, err)
|
||||
|
||||
out, err = client.Checkout(branch, false)
|
||||
require.NoError(t, err, "error output: ", out)
|
||||
|
||||
// Create and commit a test file
|
||||
err = os.WriteFile(filepath.Join(client.Root(), "README.md"), []byte("content"), 0o644)
|
||||
require.NoError(t, err)
|
||||
out, err = client.CommitAndPush(branch, "initial commit")
|
||||
require.NoError(t, err, "error output: %s", out)
|
||||
|
||||
// Get the latest commit SHA
|
||||
sha, err := client.CommitSHA()
|
||||
require.NoError(t, err)
|
||||
require.NotEmpty(t, sha)
|
||||
|
||||
// No note found, should return ErrNoNoteFound
|
||||
got, err := client.GetCommitNote(sha, "")
|
||||
require.Empty(t, got)
|
||||
unwrappedError := errors.Unwrap(err)
|
||||
require.ErrorIs(t, unwrappedError, ErrNoNoteFound)
|
||||
|
||||
// Add a git note for this commit manually
|
||||
noteMsg := "this is a test note"
|
||||
err = runCmd(ctx, client.Root(), "git", "notes", "--ref=commit", "add", "-m", noteMsg, sha)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Call the method under test
|
||||
got, err = client.GetCommitNote(sha, "")
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, noteMsg, got)
|
||||
}
|
||||
|
||||
func Test_nativeGitClient_AddAndPushNote(t *testing.T) {
|
||||
ctx := t.Context()
|
||||
tempDir, err := _createEmptyGitRepo(ctx)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Allow pushing to the same local repo (non-bare)
|
||||
err = runCmd(ctx, tempDir, "git", "config", "--local", "receive.denyCurrentBranch", "updateInstead")
|
||||
require.NoError(t, err)
|
||||
|
||||
// Get the current branch name
|
||||
gitCurrentBranch, err := outputCmd(ctx, tempDir, "git", "rev-parse", "--abbrev-ref", "HEAD")
|
||||
require.NoError(t, err)
|
||||
branch := strings.TrimSpace(string(gitCurrentBranch))
|
||||
|
||||
// Initialize client that uses this same repo as origin
|
||||
client, err := NewClient("file://"+tempDir, NopCreds{}, true, false, "", "")
|
||||
require.NoError(t, err)
|
||||
|
||||
err = client.Init()
|
||||
require.NoError(t, err)
|
||||
|
||||
out, err := client.SetAuthor("test", "test@example.com")
|
||||
require.NoError(t, err, "error output: ", out)
|
||||
|
||||
err = client.Fetch(branch, 0)
|
||||
require.NoError(t, err)
|
||||
|
||||
out, err = client.Checkout(branch, false)
|
||||
require.NoError(t, err, "error output: ", out)
|
||||
|
||||
// Create and commit a test file
|
||||
err = os.WriteFile(filepath.Join(client.Root(), "README.md"), []byte("content"), 0o644)
|
||||
require.NoError(t, err)
|
||||
out, err = client.CommitAndPush(branch, "initial commit")
|
||||
require.NoError(t, err, "error output: %s", out)
|
||||
|
||||
// Get current commit SHA
|
||||
sha, err := client.CommitSHA()
|
||||
require.NoError(t, err)
|
||||
require.NotEmpty(t, sha)
|
||||
|
||||
// Add and push a note (to the same repo acting as its own origin)
|
||||
note := "this is a test note"
|
||||
err = client.AddAndPushNote(sha, "", note)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Verify the note exists
|
||||
outBytes, err := outputCmd(ctx, client.Root(), "git", "notes", "--ref=commit", "show", sha)
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, note, strings.TrimSpace(string(outBytes)))
|
||||
|
||||
// test with a custom namespace too
|
||||
t.Run("custom namespace", func(t *testing.T) {
|
||||
customNS := "source-hydrator"
|
||||
customNote := "custom namespace note"
|
||||
err = client.AddAndPushNote(sha, customNS, customNote)
|
||||
require.NoError(t, err)
|
||||
|
||||
outBytes, err := outputCmd(ctx, client.Root(), "git", "notes", "--ref="+customNS, "show", sha)
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, customNote, strings.TrimSpace(string(outBytes)))
|
||||
})
|
||||
}
|
||||
|
||||
func Test_nativeGitClient_HasFileChanged(t *testing.T) {
|
||||
ctx := t.Context()
|
||||
tempDir, err := _createEmptyGitRepo(ctx)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Allow pushing to the same local repo (non-bare)
|
||||
err = runCmd(ctx, tempDir, "git", "config", "--local", "receive.denyCurrentBranch", "updateInstead")
|
||||
require.NoError(t, err)
|
||||
|
||||
// Get the current branch name
|
||||
gitCurrentBranch, err := outputCmd(ctx, tempDir, "git", "rev-parse", "--abbrev-ref", "HEAD")
|
||||
require.NoError(t, err)
|
||||
branch := strings.TrimSpace(string(gitCurrentBranch))
|
||||
|
||||
// Initialize client that uses this same repo as origin
|
||||
client, err := NewClient("file://"+tempDir, NopCreds{}, true, false, "", "")
|
||||
require.NoError(t, err)
|
||||
|
||||
err = client.Init()
|
||||
require.NoError(t, err)
|
||||
|
||||
out, err := client.SetAuthor("test", "test@example.com")
|
||||
require.NoError(t, err, "error output: ", out)
|
||||
|
||||
err = client.Fetch(branch, 0)
|
||||
require.NoError(t, err)
|
||||
|
||||
out, err = client.Checkout(branch, false)
|
||||
require.NoError(t, err, "error output: ", out)
|
||||
|
||||
// Create the file inside repo root
|
||||
fileName := "sample.txt"
|
||||
filePath := filepath.Join(client.Root(), fileName)
|
||||
|
||||
err = os.WriteFile(filePath, []byte("first version"), 0o644)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Untracked file, should be reported as changed
|
||||
changed, err := client.HasFileChanged(filePath)
|
||||
require.NoError(t, err)
|
||||
require.True(t, changed, "expected untracked file to be reported as changed")
|
||||
|
||||
// After commit, should NOT be changed
|
||||
out, err = client.CommitAndPush(branch, "add sample.txt")
|
||||
require.NoError(t, err, "error output: %s", out)
|
||||
changed, err = client.HasFileChanged(filePath)
|
||||
require.NoError(t, err)
|
||||
require.False(t, changed, "expected committed file to not be changed")
|
||||
|
||||
// Modify the file should be reported as changed
|
||||
err = os.WriteFile(filePath, []byte("modified content"), 0o644)
|
||||
require.NoError(t, err)
|
||||
changed, err = client.HasFileChanged(filePath)
|
||||
require.NoError(t, err)
|
||||
require.True(t, changed, "expected modified file to be reported as changed")
|
||||
}
|
||||
|
||||
189
util/git/mocks/Client.go
generated
189
util/git/mocks/Client.go
generated
@@ -36,6 +36,69 @@ func (_m *Client) EXPECT() *Client_Expecter {
|
||||
return &Client_Expecter{mock: &_m.Mock}
|
||||
}
|
||||
|
||||
// AddAndPushNote provides a mock function for the type Client
|
||||
func (_mock *Client) AddAndPushNote(sha string, namespace string, note string) error {
|
||||
ret := _mock.Called(sha, namespace, note)
|
||||
|
||||
if len(ret) == 0 {
|
||||
panic("no return value specified for AddAndPushNote")
|
||||
}
|
||||
|
||||
var r0 error
|
||||
if returnFunc, ok := ret.Get(0).(func(string, string, string) error); ok {
|
||||
r0 = returnFunc(sha, namespace, note)
|
||||
} else {
|
||||
r0 = ret.Error(0)
|
||||
}
|
||||
return r0
|
||||
}
|
||||
|
||||
// Client_AddAndPushNote_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'AddAndPushNote'
|
||||
type Client_AddAndPushNote_Call struct {
|
||||
*mock.Call
|
||||
}
|
||||
|
||||
// AddAndPushNote is a helper method to define mock.On call
|
||||
// - sha string
|
||||
// - namespace string
|
||||
// - note string
|
||||
func (_e *Client_Expecter) AddAndPushNote(sha interface{}, namespace interface{}, note interface{}) *Client_AddAndPushNote_Call {
|
||||
return &Client_AddAndPushNote_Call{Call: _e.mock.On("AddAndPushNote", sha, namespace, note)}
|
||||
}
|
||||
|
||||
func (_c *Client_AddAndPushNote_Call) Run(run func(sha string, namespace string, note string)) *Client_AddAndPushNote_Call {
|
||||
_c.Call.Run(func(args mock.Arguments) {
|
||||
var arg0 string
|
||||
if args[0] != nil {
|
||||
arg0 = args[0].(string)
|
||||
}
|
||||
var arg1 string
|
||||
if args[1] != nil {
|
||||
arg1 = args[1].(string)
|
||||
}
|
||||
var arg2 string
|
||||
if args[2] != nil {
|
||||
arg2 = args[2].(string)
|
||||
}
|
||||
run(
|
||||
arg0,
|
||||
arg1,
|
||||
arg2,
|
||||
)
|
||||
})
|
||||
return _c
|
||||
}
|
||||
|
||||
func (_c *Client_AddAndPushNote_Call) Return(err error) *Client_AddAndPushNote_Call {
|
||||
_c.Call.Return(err)
|
||||
return _c
|
||||
}
|
||||
|
||||
func (_c *Client_AddAndPushNote_Call) RunAndReturn(run func(sha string, namespace string, note string) error) *Client_AddAndPushNote_Call {
|
||||
_c.Call.Return(run)
|
||||
return _c
|
||||
}
|
||||
|
||||
// ChangedFiles provides a mock function for the type Client
|
||||
func (_mock *Client) ChangedFiles(revision string, targetRevision string) ([]string, error) {
|
||||
ret := _mock.Called(revision, targetRevision)
|
||||
@@ -484,6 +547,132 @@ func (_c *Client_Fetch_Call) RunAndReturn(run func(revision string, depth int64)
|
||||
return _c
|
||||
}
|
||||
|
||||
// GetCommitNote provides a mock function for the type Client
|
||||
func (_mock *Client) GetCommitNote(sha string, namespace string) (string, error) {
|
||||
ret := _mock.Called(sha, namespace)
|
||||
|
||||
if len(ret) == 0 {
|
||||
panic("no return value specified for GetCommitNote")
|
||||
}
|
||||
|
||||
var r0 string
|
||||
var r1 error
|
||||
if returnFunc, ok := ret.Get(0).(func(string, string) (string, error)); ok {
|
||||
return returnFunc(sha, namespace)
|
||||
}
|
||||
if returnFunc, ok := ret.Get(0).(func(string, string) string); ok {
|
||||
r0 = returnFunc(sha, namespace)
|
||||
} else {
|
||||
r0 = ret.Get(0).(string)
|
||||
}
|
||||
if returnFunc, ok := ret.Get(1).(func(string, string) error); ok {
|
||||
r1 = returnFunc(sha, namespace)
|
||||
} else {
|
||||
r1 = ret.Error(1)
|
||||
}
|
||||
return r0, r1
|
||||
}
|
||||
|
||||
// Client_GetCommitNote_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'GetCommitNote'
|
||||
type Client_GetCommitNote_Call struct {
|
||||
*mock.Call
|
||||
}
|
||||
|
||||
// GetCommitNote is a helper method to define mock.On call
|
||||
// - sha string
|
||||
// - namespace string
|
||||
func (_e *Client_Expecter) GetCommitNote(sha interface{}, namespace interface{}) *Client_GetCommitNote_Call {
|
||||
return &Client_GetCommitNote_Call{Call: _e.mock.On("GetCommitNote", sha, namespace)}
|
||||
}
|
||||
|
||||
func (_c *Client_GetCommitNote_Call) Run(run func(sha string, namespace string)) *Client_GetCommitNote_Call {
|
||||
_c.Call.Run(func(args mock.Arguments) {
|
||||
var arg0 string
|
||||
if args[0] != nil {
|
||||
arg0 = args[0].(string)
|
||||
}
|
||||
var arg1 string
|
||||
if args[1] != nil {
|
||||
arg1 = args[1].(string)
|
||||
}
|
||||
run(
|
||||
arg0,
|
||||
arg1,
|
||||
)
|
||||
})
|
||||
return _c
|
||||
}
|
||||
|
||||
func (_c *Client_GetCommitNote_Call) Return(s string, err error) *Client_GetCommitNote_Call {
|
||||
_c.Call.Return(s, err)
|
||||
return _c
|
||||
}
|
||||
|
||||
func (_c *Client_GetCommitNote_Call) RunAndReturn(run func(sha string, namespace string) (string, error)) *Client_GetCommitNote_Call {
|
||||
_c.Call.Return(run)
|
||||
return _c
|
||||
}
|
||||
|
||||
// HasFileChanged provides a mock function for the type Client
|
||||
func (_mock *Client) HasFileChanged(filePath string) (bool, error) {
|
||||
ret := _mock.Called(filePath)
|
||||
|
||||
if len(ret) == 0 {
|
||||
panic("no return value specified for HasFileChanged")
|
||||
}
|
||||
|
||||
var r0 bool
|
||||
var r1 error
|
||||
if returnFunc, ok := ret.Get(0).(func(string) (bool, error)); ok {
|
||||
return returnFunc(filePath)
|
||||
}
|
||||
if returnFunc, ok := ret.Get(0).(func(string) bool); ok {
|
||||
r0 = returnFunc(filePath)
|
||||
} else {
|
||||
r0 = ret.Get(0).(bool)
|
||||
}
|
||||
if returnFunc, ok := ret.Get(1).(func(string) error); ok {
|
||||
r1 = returnFunc(filePath)
|
||||
} else {
|
||||
r1 = ret.Error(1)
|
||||
}
|
||||
return r0, r1
|
||||
}
|
||||
|
||||
// Client_HasFileChanged_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'HasFileChanged'
|
||||
type Client_HasFileChanged_Call struct {
|
||||
*mock.Call
|
||||
}
|
||||
|
||||
// HasFileChanged is a helper method to define mock.On call
|
||||
// - filePath string
|
||||
func (_e *Client_Expecter) HasFileChanged(filePath interface{}) *Client_HasFileChanged_Call {
|
||||
return &Client_HasFileChanged_Call{Call: _e.mock.On("HasFileChanged", filePath)}
|
||||
}
|
||||
|
||||
func (_c *Client_HasFileChanged_Call) Run(run func(filePath string)) *Client_HasFileChanged_Call {
|
||||
_c.Call.Run(func(args mock.Arguments) {
|
||||
var arg0 string
|
||||
if args[0] != nil {
|
||||
arg0 = args[0].(string)
|
||||
}
|
||||
run(
|
||||
arg0,
|
||||
)
|
||||
})
|
||||
return _c
|
||||
}
|
||||
|
||||
func (_c *Client_HasFileChanged_Call) Return(b bool, err error) *Client_HasFileChanged_Call {
|
||||
_c.Call.Return(b, err)
|
||||
return _c
|
||||
}
|
||||
|
||||
func (_c *Client_HasFileChanged_Call) RunAndReturn(run func(filePath string) (bool, error)) *Client_HasFileChanged_Call {
|
||||
_c.Call.Return(run)
|
||||
return _c
|
||||
}
|
||||
|
||||
// Init provides a mock function for the type Client
|
||||
func (_mock *Client) Init() error {
|
||||
ret := _mock.Called()
|
||||
|
||||
Reference in New Issue
Block a user