Skip to content

Commit bd8af5e

Browse files
committed
fix(update): Cannot use digest update strategy with forceUpdate: true without version constraint (#1345)
Assisted-by: Claude Code Signed-off-by: Cheng Fang <[email protected]>
1 parent 9e0f52f commit bd8af5e

File tree

3 files changed

+195
-2
lines changed

3 files changed

+195
-2
lines changed

pkg/argocd/argocd.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -807,8 +807,14 @@ func GetImagesFromApplication(applicationImages *ApplicationImages) image.Contai
807807

808808
for _, img := range applicationImages.Images {
809809
if img.ForceUpdate {
810-
img.ImageTag = nil // the tag from the image list will be a version constraint, which isn't a valid tag
811-
images = append(images, img.ContainerImage)
810+
// Create a copy of the container image with nil tag to add to the live images list.
811+
// The tag from the image list will be a version constraint, which isn't a valid tag.
812+
// We preserve the original img.ContainerImage so the constraint is available later.
813+
imgCopy := img.ContainerImage.WithTag(nil)
814+
// Avoid duplicates if an entry already exists for this image (ignore tag for match).
815+
if images.ContainsImage(img.ContainerImage, false) == nil {
816+
images = append(images, imgCopy)
817+
}
812818
}
813819
}
814820

pkg/argocd/argocd_test.go

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,85 @@ func Test_GetImagesFromApplication(t *testing.T) {
9393
assert.Equal(t, "nginx", imageList[0].ImageName)
9494
assert.Nil(t, imageList[0].ImageTag)
9595
})
96+
97+
t.Run("Force-update with digest strategy preserves constraint (issue #1344)", func(t *testing.T) {
98+
application := &v1alpha1.Application{
99+
ObjectMeta: v1.ObjectMeta{
100+
Name: "test-app",
101+
Namespace: "argocd",
102+
},
103+
Spec: v1alpha1.ApplicationSpec{},
104+
Status: v1alpha1.ApplicationStatus{
105+
Summary: v1alpha1.ApplicationSummary{},
106+
},
107+
}
108+
// Create image with tag (which will be used as constraint for digest strategy)
109+
// This mimics the scenario: forceUpdate=true + updateStrategy=digest + tag as version constraint
110+
imgToUpdate := image.NewFromIdentifier("nginx:1.14")
111+
img := NewImage(imgToUpdate)
112+
img.ForceUpdate = true
113+
img.UpdateStrategy = image.StrategyDigest
114+
115+
applicationImages := &ApplicationImages{
116+
Application: *application,
117+
Images: ImageList{img},
118+
}
119+
120+
// Get the live images list
121+
imageList := GetImagesFromApplication(applicationImages)
122+
require.Len(t, imageList, 1)
123+
assert.Equal(t, "nginx", imageList[0].ImageName)
124+
// The returned image should have nil tag (it's for matching with live images)
125+
assert.Nil(t, imageList[0].ImageTag)
126+
// Ensure a copy was created (no in-place mutation of the original).
127+
require.NotSame(t, img.ContainerImage, imageList[0])
128+
129+
// CRITICAL: The original image's tag should be preserved for use as version constraint
130+
// This is the fix for issue #1344 - without the fix, img.ImageTag would be nil here
131+
require.NotNil(t, img.ImageTag, "Original image tag should be preserved for constraint")
132+
assert.Equal(t, "1.14", img.ImageTag.TagName, "Constraint tag should be available")
133+
134+
// Verify the update strategy is still set correctly
135+
assert.Equal(t, image.StrategyDigest, img.UpdateStrategy, "Update strategy should remain digest")
136+
137+
// Verify that the constraint would work with digest strategy validation
138+
// Digest strategy requires a non-empty constraint (vc.Constraint != "")
139+
assert.NotEmpty(t, img.ImageTag.TagName, "Constraint must be non-empty for digest strategy")
140+
})
141+
142+
t.Run("Force-update avoids duplicates when image already in status", func(t *testing.T) {
143+
application := &v1alpha1.Application{
144+
ObjectMeta: v1.ObjectMeta{
145+
Name: "test-app",
146+
Namespace: "argocd",
147+
},
148+
Spec: v1alpha1.ApplicationSpec{},
149+
Status: v1alpha1.ApplicationStatus{
150+
Summary: v1alpha1.ApplicationSummary{
151+
// Image already exists in the application status
152+
Images: []string{
153+
"nginx:1.14",
154+
},
155+
},
156+
},
157+
}
158+
// Create image with forceUpdate that has the same name/registry as the status image
159+
imgToUpdate := image.NewFromIdentifier("nginx:1.14")
160+
img := NewImage(imgToUpdate)
161+
img.ForceUpdate = true
162+
163+
applicationImages := &ApplicationImages{
164+
Application: *application,
165+
Images: ImageList{img},
166+
}
167+
168+
// Get the live images list
169+
imageList := GetImagesFromApplication(applicationImages)
170+
171+
// Should only have 1 image (from status), not 2 (duplicate prevented)
172+
require.Len(t, imageList, 1, "Should not create duplicate entry for image already in status")
173+
assert.Equal(t, "nginx", imageList[0].ImageName)
174+
})
96175
}
97176

98177
func Test_GetImagesAndAliasesFromApplication(t *testing.T) {

pkg/argocd/update_test.go

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1236,6 +1236,114 @@ func Test_UpdateApplication(t *testing.T) {
12361236
assert.Equal(t, 0, res.NumImagesUpdated)
12371237
})
12381238

1239+
t.Run("Test Kubernetes Job with forceUpdate and digest strategy (issue #1344)", func(t *testing.T) {
1240+
// This test reproduces the scenario from issue #1344:
1241+
// - Application uses a Kubernetes Job (not in app.Status.Summary.Images)
1242+
// - forceUpdate: true is set
1243+
// - updateStrategy: "digest" is used
1244+
// - A version constraint (tag) is provided
1245+
1246+
mockClientFn := func(endpoint *registry.RegistryEndpoint, username, password string) (registry.RegistryClient, error) {
1247+
regMock := regmock.RegistryClient{}
1248+
regMock.On("NewRepository", mock.MatchedBy(func(s string) bool {
1249+
return s == "org/job-image"
1250+
})).Return(nil)
1251+
// Return the tag that matches our version constraint
1252+
regMock.On("Tags", mock.Anything).Return([]string{"latest"}, nil)
1253+
1254+
// For digest strategy, we need to mock ManifestForTag and TagMetadata
1255+
meta1 := &schema1.SignedManifest{} //nolint:staticcheck
1256+
meta1.Name = "org/job-image"
1257+
meta1.Tag = "latest"
1258+
regMock.On("ManifestForTag", mock.Anything, "latest").Return(meta1, nil)
1259+
// Create a digest as [32]byte array
1260+
var digest [32]byte
1261+
copy(digest[:], []byte("abcdef1234567890"))
1262+
regMock.On("TagMetadata", mock.Anything, mock.Anything, mock.Anything).Return(&tag.TagInfo{
1263+
CreatedAt: time.Unix(1234567890, 0),
1264+
Digest: digest,
1265+
}, nil)
1266+
1267+
return &regMock, nil
1268+
}
1269+
1270+
argoClient := argomock.ArgoCD{}
1271+
argoClient.On("UpdateSpec", mock.Anything, mock.Anything).Return(nil, nil)
1272+
1273+
kubeClient := kube.ImageUpdaterKubernetesClient{
1274+
KubeClient: &registryKube.KubernetesClient{
1275+
Clientset: fake.NewFakeKubeClient(),
1276+
},
1277+
}
1278+
1279+
// Image configuration with forceUpdate and digest strategy
1280+
// The tag "latest" serves as the version constraint for digest strategy
1281+
containerImg := image.NewFromIdentifier("job-image=gcr.io/org/job-image:latest")
1282+
iuImg := NewImage(containerImg)
1283+
iuImg.KustomizeImageName = "org/job-image"
1284+
iuImg.ForceUpdate = true
1285+
iuImg.UpdateStrategy = image.StrategyDigest
1286+
1287+
imageList := ImageList{iuImg}
1288+
1289+
appImages := &ApplicationImages{
1290+
Application: v1alpha1.Application{
1291+
ObjectMeta: v1.ObjectMeta{
1292+
Name: "job-app",
1293+
Namespace: "default",
1294+
},
1295+
Spec: v1alpha1.ApplicationSpec{
1296+
Source: &v1alpha1.ApplicationSource{
1297+
Kustomize: &v1alpha1.ApplicationSourceKustomize{
1298+
Images: v1alpha1.KustomizeImages{},
1299+
},
1300+
},
1301+
},
1302+
Status: v1alpha1.ApplicationStatus{
1303+
SourceType: v1alpha1.ApplicationSourceTypeKustomize,
1304+
Summary: v1alpha1.ApplicationSummary{
1305+
// Empty images list - simulating a Kubernetes Job that doesn't
1306+
// appear in the application status
1307+
Images: []string{},
1308+
},
1309+
},
1310+
},
1311+
Images: imageList,
1312+
WriteBackConfig: &WriteBackConfig{
1313+
Method: WriteBackApplication,
1314+
},
1315+
}
1316+
1317+
// Before the fix for issue #1344, this would fail with:
1318+
// "cannot use update strategy 'digest' for image... without a version constraint"
1319+
// because the constraint was lost when setting ImageTag to nil
1320+
res := UpdateApplication(context.Background(), &UpdateConfiguration{
1321+
NewRegFN: mockClientFn,
1322+
ArgoClient: &argoClient,
1323+
KubeClient: &kubeClient,
1324+
UpdateApp: appImages,
1325+
DryRun: false,
1326+
}, NewSyncIterationState())
1327+
1328+
// Verify the update succeeded
1329+
assert.Equal(t, 0, res.NumErrors, "Should not have errors with forceUpdate + digest strategy")
1330+
assert.Equal(t, 0, res.NumSkipped)
1331+
assert.Equal(t, 1, res.NumApplicationsProcessed)
1332+
assert.Equal(t, 1, res.NumImagesConsidered)
1333+
assert.Equal(t, 1, res.NumImagesUpdated, "Image should be updated with digest")
1334+
1335+
// Verify the kustomize image was updated with the digest
1336+
require.Len(t, appImages.Application.Spec.Source.Kustomize.Images, 1)
1337+
updatedImage := string(appImages.Application.Spec.Source.Kustomize.Images[0])
1338+
assert.Contains(t, updatedImage, "gcr.io/org/job-image")
1339+
assert.Contains(t, updatedImage, "latest")
1340+
assert.Contains(t, updatedImage, "sha256:", "Image should include digest")
1341+
1342+
// The constraint on the original image must still be present.
1343+
require.NotNil(t, iuImg.ContainerImage.ImageTag)
1344+
assert.Equal(t, "latest", iuImg.ContainerImage.ImageTag.TagName)
1345+
})
1346+
12391347
}
12401348

12411349
func Test_MarshalParamsOverride(t *testing.T) {

0 commit comments

Comments
 (0)