Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 40 additions & 1 deletion controller/appcontroller.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ import (
argodiff "github.com/argoproj/argo-cd/v3/util/argo/diff"
"github.com/argoproj/argo-cd/v3/util/argo/normalizers"
"github.com/argoproj/argo-cd/v3/util/env"
"github.com/argoproj/argo-cd/v3/util/git"
"github.com/argoproj/argo-cd/v3/util/stats"

"github.com/argoproj/argo-cd/v3/pkg/ratelimiter"
Expand Down Expand Up @@ -1482,7 +1483,7 @@ func (ctrl *ApplicationController) processRequestedAppOperation(app *appv1.Appli

// Remove the desired revisions if the sync failed and we are retrying. The latest revision from the source will be used.
extraMsg := ""
if state.Operation.Retry.Refresh {
if state.Operation.Retry.Refresh || hasSymbolicTargetRevision(state.Operation.Sync, app) {
extraMsg += " with latest revisions"
state.Operation.Sync.Revision = ""
state.Operation.Sync.Revisions = nil
Expand Down Expand Up @@ -2325,6 +2326,44 @@ func (ctrl *ApplicationController) autoSync(app *appv1.Application, syncStatus *
return nil, setOpTime
}

// hasSymbolicTargetRevision returns true if any source in the sync operation has
// a target revision that is not a literal commit SHA (e.g., HEAD, a branch name,
// or a tag). When a sync retry occurs, symbolic target revisions should be
// re-resolved to the latest commit to avoid applying stale manifests.
func hasSymbolicTargetRevision(syncOp *appv1.SyncOperation, app *appv1.Application) bool {
if syncOp == nil {
return false
}

// For multi-source apps, check each source's target revision.
if len(syncOp.Sources) > 0 {
for _, source := range syncOp.Sources {
targetRev := source.TargetRevision
if targetRev == "" {
// Empty target revision defaults to HEAD, which is symbolic.
return true
}
if !git.IsCommitSHA(targetRev) {
return true
}
}
return false
}

// For single-source apps, check the operation's source or fall back to the app spec.
var targetRev string
if syncOp.Source != nil {
targetRev = syncOp.Source.TargetRevision
} else {
targetRev = app.Spec.GetSource().TargetRevision
}
if targetRev == "" {
// Empty target revision defaults to HEAD, which is symbolic.
return true
}
return !git.IsCommitSHA(targetRev)
}

// alreadyAttemptedSync returns whether the most recently synced revision(s) exactly match the given desiredRevisions
// and for the same application source. If the revision(s) have changed or the Application source configuration has been updated,
// it will return false, indicating that a new sync should be attempted.
Expand Down
247 changes: 247 additions & 0 deletions controller/appcontroller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2640,6 +2640,253 @@ func TestProcessRequestedAppOperation_RunningPreviouslyFailedBackoff(t *testing.
ctrl.processRequestedAppOperation(app)
}

func TestHasSymbolicTargetRevision(t *testing.T) {
tests := []struct {
name string
syncOp *v1alpha1.SyncOperation
app *v1alpha1.Application
expected bool
}{
{
name: "nil sync operation",
syncOp: nil,
app: newFakeApp(),
expected: false,
},
{
name: "single source with HEAD target revision",
syncOp: &v1alpha1.SyncOperation{
Source: &v1alpha1.ApplicationSource{
RepoURL: "https://github.com/example/repo.git",
TargetRevision: "HEAD",
},
},
app: newFakeApp(),
expected: true,
},
{
name: "single source with branch name target revision",
syncOp: &v1alpha1.SyncOperation{
Source: &v1alpha1.ApplicationSource{
RepoURL: "https://github.com/example/repo.git",
TargetRevision: "main",
},
},
app: newFakeApp(),
expected: true,
},
{
name: "single source with tag target revision",
syncOp: &v1alpha1.SyncOperation{
Source: &v1alpha1.ApplicationSource{
RepoURL: "https://github.com/example/repo.git",
TargetRevision: "v1.0.0",
},
},
app: newFakeApp(),
expected: true,
},
{
name: "single source with empty target revision (defaults to HEAD)",
syncOp: &v1alpha1.SyncOperation{
Source: &v1alpha1.ApplicationSource{
RepoURL: "https://github.com/example/repo.git",
TargetRevision: "",
},
},
app: newFakeApp(),
expected: true,
},
{
name: "single source with literal SHA target revision",
syncOp: &v1alpha1.SyncOperation{
Source: &v1alpha1.ApplicationSource{
RepoURL: "https://github.com/example/repo.git",
TargetRevision: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
},
},
app: newFakeApp(),
expected: false,
},
{
name: "no source in sync op falls back to app spec with empty target revision",
syncOp: &v1alpha1.SyncOperation{},
app: newFakeApp(),
// The fake app's spec source has no targetRevision, which defaults to HEAD (symbolic).
expected: true,
},
{
name: "multi-source with all SHAs",
syncOp: &v1alpha1.SyncOperation{
Sources: v1alpha1.ApplicationSources{
{RepoURL: "https://github.com/example/repo1.git", TargetRevision: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"},
{RepoURL: "https://github.com/example/repo2.git", TargetRevision: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"},
},
},
app: newFakeApp(),
expected: false,
},
{
name: "multi-source with one symbolic ref",
syncOp: &v1alpha1.SyncOperation{
Sources: v1alpha1.ApplicationSources{
{RepoURL: "https://github.com/example/repo1.git", TargetRevision: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"},
{RepoURL: "https://github.com/example/repo2.git", TargetRevision: "main"},
},
},
app: newFakeApp(),
expected: true,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
result := hasSymbolicTargetRevision(tt.syncOp, tt.app)
assert.Equal(t, tt.expected, result)
})
}
}

func TestProcessRequestedAppOperation_RetryClearsRevisionForSymbolicRef(t *testing.T) {
// Simulate a retry scenario where the sync operation has a resolved (stale) SHA
// but the source's targetRevision is symbolic (HEAD). The retry should clear
// the resolved revision so it gets re-resolved to the latest commit.
failedAttemptFinishedAt := time.Now().Add(-time.Minute * 5)
app := newFakeApp()
staleSHA := "cccccccccccccccccccccccccccccccccccccccc"
app.Operation = &v1alpha1.Operation{
Sync: &v1alpha1.SyncOperation{
Revision: staleSHA,
Source: &v1alpha1.ApplicationSource{
RepoURL: "https://github.com/argoproj/argocd-example-apps.git",
Path: "some/path",
TargetRevision: "HEAD",
},
},
Retry: v1alpha1.RetryStrategy{Limit: 3},
}
app.Status.OperationState.Operation = *app.Operation
app.Status.OperationState.Phase = synccommon.OperationRunning
app.Status.OperationState.RetryCount = 1
app.Status.OperationState.FinishedAt = &metav1.Time{Time: failedAttemptFinishedAt}
app.Status.OperationState.SyncResult = &v1alpha1.SyncOperationResult{
Revision: staleSHA,
Source: v1alpha1.ApplicationSource{
RepoURL: "https://github.com/argoproj/argocd-example-apps.git",
Path: "some/path",
},
Resources: []*v1alpha1.ResourceResult{{
Name: "guestbook",
Kind: "Deployment",
Group: "apps",
Status: synccommon.ResultCodeSyncFailed,
}},
}

data := &fakeData{
apps: []runtime.Object{app, &defaultProj},
manifestResponse: &apiclient.ManifestResponse{
Manifests: []string{},
Namespace: test.FakeDestNamespace,
Server: test.FakeClusterURL,
Revision: "abc123",
},
}
ctrl := newFakeController(t.Context(), data, nil)
fakeAppCs := ctrl.applicationClientset.(*appclientset.Clientset)
var allPatches []map[string]any
fakeAppCs.PrependReactor("patch", "*", func(action kubetesting.Action) (handled bool, ret runtime.Object, err error) {
if patchAction, ok := action.(kubetesting.PatchAction); ok {
patch := map[string]any{}
require.NoError(t, json.Unmarshal(patchAction.GetPatch(), &patch))
allPatches = append(allPatches, patch)
}
return true, &v1alpha1.Application{}, nil
})

ctrl.processRequestedAppOperation(app)

// The first patch is the retry state update, subsequent patches are from the sync.
// Check the first patch to verify the retry cleared the revision.
require.NotEmpty(t, allPatches, "expected at least one patch")
retryPatch := allPatches[0]

// Verify the retry message indicates latest revisions are being used
message, _, _ := unstructured.NestedString(retryPatch, "status", "operationState", "message")
assert.Contains(t, message, "with latest revisions", "retry with symbolic targetRevision should re-resolve revisions")

// Verify the operation's sync revision was cleared (not the stale SHA)
syncRevision, _, _ := unstructured.NestedString(retryPatch, "status", "operationState", "operation", "sync", "revision")
assert.Empty(t, syncRevision, "resolved revision should be cleared on retry for symbolic target revision")
}

func TestProcessRequestedAppOperation_RetryPreservesRevisionForLiteralSHA(t *testing.T) {
// When the source's targetRevision is a literal commit SHA, the retry should
// preserve the resolved revision since a SHA always points to the same commit.
failedAttemptFinishedAt := time.Now().Add(-time.Minute * 5)
app := newFakeApp()
literalSHA := "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
app.Operation = &v1alpha1.Operation{
Sync: &v1alpha1.SyncOperation{
Revision: literalSHA,
Source: &v1alpha1.ApplicationSource{
RepoURL: "https://github.com/argoproj/argocd-example-apps.git",
Path: "some/path",
TargetRevision: literalSHA,
},
},
Retry: v1alpha1.RetryStrategy{Limit: 3},
}
app.Status.OperationState.Operation = *app.Operation
app.Status.OperationState.Phase = synccommon.OperationRunning
app.Status.OperationState.RetryCount = 1
app.Status.OperationState.FinishedAt = &metav1.Time{Time: failedAttemptFinishedAt}
app.Status.OperationState.SyncResult = &v1alpha1.SyncOperationResult{
Revision: literalSHA,
Source: v1alpha1.ApplicationSource{
RepoURL: "https://github.com/argoproj/argocd-example-apps.git",
Path: "some/path",
},
Resources: []*v1alpha1.ResourceResult{{
Name: "guestbook",
Kind: "Deployment",
Group: "apps",
Status: synccommon.ResultCodeSyncFailed,
}},
}

data := &fakeData{
apps: []runtime.Object{app, &defaultProj},
manifestResponse: &apiclient.ManifestResponse{
Manifests: []string{},
Namespace: test.FakeDestNamespace,
Server: test.FakeClusterURL,
Revision: "abc123",
},
}
ctrl := newFakeController(t.Context(), data, nil)
fakeAppCs := ctrl.applicationClientset.(*appclientset.Clientset)
var allPatches []map[string]any
fakeAppCs.PrependReactor("patch", "*", func(action kubetesting.Action) (handled bool, ret runtime.Object, err error) {
if patchAction, ok := action.(kubetesting.PatchAction); ok {
patch := map[string]any{}
require.NoError(t, json.Unmarshal(patchAction.GetPatch(), &patch))
allPatches = append(allPatches, patch)
}
return true, &v1alpha1.Application{}, nil
})

ctrl.processRequestedAppOperation(app)

// The first patch is the retry state update.
require.NotEmpty(t, allPatches, "expected at least one patch")
retryPatch := allPatches[0]

// Verify the retry message does NOT mention latest revisions
message, _, _ := unstructured.NestedString(retryPatch, "status", "operationState", "message")
assert.NotContains(t, message, "with latest revisions", "retry with literal SHA targetRevision should not re-resolve revisions")
}

func TestProcessRequestedAppOperation_HasRetriesTerminated(t *testing.T) {
app := newFakeApp()
app.Operation = &v1alpha1.Operation{
Expand Down
Loading