Skip to content

Commit 1d5c8b0

Browse files
FourFifthsCodepbhatnagar-ossagaudreault
committed
fix(hydrator): use refresh paths from drySource when source hydration is enabled (argoproj#25516)
Signed-off-by: pbhatnagar-oss <pbhatifiwork@gmail.com> Co-authored-by: pbhatnagar-oss <pbhatifiwork@gmail.com> Co-authored-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com> Signed-off-by: Codey Jenkins <FourFifthsCode@users.noreply.github.com>
1 parent 9a49562 commit 1d5c8b0

File tree

5 files changed

+691
-254
lines changed

5 files changed

+691
-254
lines changed

‎controller/state.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,7 @@ func (m *appStateManager) GetRepoObjs(app *v1alpha1.Application, sources []v1alp
247247
Revision: revision,
248248
SyncedRevision: syncedRevision,
249249
NoRevisionCache: noRevisionCache,
250-
Paths: path.GetAppRefreshPaths(app),
250+
Paths: path.GetSourceRefreshPaths(app, source),
251251
AppLabelKey: appLabelKey,
252252
AppName: app.InstanceName(m.namespace),
253253
Namespace: appNamespace,

‎util/app/path/path.go‎

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -97,23 +97,41 @@ func CheckOutOfBoundsSymlinks(basePath string) error {
9797
})
9898
}
9999

100-
// GetAppRefreshPaths returns the list of paths that should trigger a refresh for an application
101-
func GetAppRefreshPaths(app *v1alpha1.Application) []string {
100+
// GetSourceRefreshPaths returns the list of paths that should trigger a refresh for an application.
101+
// The source parameter influences the returned refresh paths:
102+
// - if source hydrator configured AND source is syncSource: use sync source path (ignores annotation)
103+
// - if source hydrator configured AND source is drySource WITH annotation: use annotation paths with drySource base
104+
// - if source hydrator not configured: use annotation paths with source base, or empty if no annotation
105+
func GetSourceRefreshPaths(app *v1alpha1.Application, source v1alpha1.ApplicationSource) []string {
106+
annotationPaths, hasAnnotation := app.Annotations[v1alpha1.AnnotationKeyManifestGeneratePaths]
107+
108+
if app.Spec.SourceHydrator != nil {
109+
syncSource := app.Spec.SourceHydrator.GetSyncSource()
110+
111+
// if source is syncSource use the source path
112+
if (source).Equals(&syncSource) {
113+
return []string{source.Path}
114+
}
115+
}
116+
102117
var paths []string
103-
if val, ok := app.Annotations[v1alpha1.AnnotationKeyManifestGeneratePaths]; ok && val != "" {
104-
for _, item := range strings.Split(val, ";") {
118+
if hasAnnotation && annotationPaths != "" {
119+
for _, item := range strings.Split(annotationPaths, ";") {
120+
// skip empty paths
105121
if item == "" {
106122
continue
107123
}
124+
// if absolute path, add as is
108125
if filepath.IsAbs(item) {
109126
paths = append(paths, item[1:])
110-
} else {
111-
for _, source := range app.Spec.GetSources() {
112-
paths = append(paths, filepath.Clean(filepath.Join(source.Path, item)))
113-
}
127+
continue
114128
}
129+
130+
// add the path relative to the source path
131+
paths = append(paths, filepath.Clean(filepath.Join(source.Path, item)))
115132
}
116133
}
134+
117135
return paths
118136
}
119137

‎util/app/path/path_test.go‎

Lines changed: 75 additions & 89 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"github.com/stretchr/testify/require"
1010

1111
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
12+
"k8s.io/utils/ptr"
1213

1314
"github.com/argoproj/argo-cd/v3/pkg/apis/application/v1alpha1"
1415
fileutil "github.com/argoproj/argo-cd/v3/test/fixture/path"
@@ -100,117 +101,102 @@ func TestAbsSymlink(t *testing.T) {
100101
assert.Equal(t, "abslink", oobError.File)
101102
}
102103

103-
func getApp(annotation string, sourcePath string) *v1alpha1.Application {
104-
return &v1alpha1.Application{
104+
func getApp(annotation *string, sourcePath *string) *v1alpha1.Application {
105+
app := &v1alpha1.Application{
105106
ObjectMeta: metav1.ObjectMeta{
106-
Annotations: map[string]string{
107-
v1alpha1.AnnotationKeyManifestGeneratePaths: annotation,
108-
},
109-
},
110-
Spec: v1alpha1.ApplicationSpec{
111-
Source: &v1alpha1.ApplicationSource{
112-
Path: sourcePath,
113-
},
107+
Name: "test-app",
114108
},
115109
}
116-
}
110+
if annotation != nil {
111+
app.Annotations = make(map[string]string)
112+
app.Annotations[v1alpha1.AnnotationKeyManifestGeneratePaths] = *annotation
113+
}
117114

118-
func getMultiSourceApp(annotation string, paths ...string) *v1alpha1.Application {
119-
var sources v1alpha1.ApplicationSources
120-
for _, path := range paths {
121-
sources = append(sources, v1alpha1.ApplicationSource{Path: path})
115+
if sourcePath != nil {
116+
app.Spec.Source = &v1alpha1.ApplicationSource{
117+
Path: *sourcePath,
118+
}
122119
}
123-
return &v1alpha1.Application{
124-
ObjectMeta: metav1.ObjectMeta{
125-
Annotations: map[string]string{
126-
v1alpha1.AnnotationKeyManifestGeneratePaths: annotation,
127-
},
120+
121+
return app
122+
}
123+
124+
func getSourceHydratorApp(annotation *string, drySourcePath string, syncSourcePath string) *v1alpha1.Application {
125+
app := getApp(annotation, nil)
126+
app.Spec.SourceHydrator = &v1alpha1.SourceHydrator{
127+
DrySource: v1alpha1.DrySource{
128+
Path: drySourcePath,
128129
},
129-
Spec: v1alpha1.ApplicationSpec{
130-
Sources: sources,
130+
SyncSource: v1alpha1.SyncSource{
131+
Path: syncSourcePath,
131132
},
132133
}
133-
}
134134

135-
func Test_AppFilesHaveChanged(t *testing.T) {
136-
tests := []struct {
137-
name string
138-
app *v1alpha1.Application
139-
files []string
140-
changeExpected bool
141-
}{
142-
{"default no path", &v1alpha1.Application{}, []string{"README.md"}, true},
143-
{"no files changed", getApp(".", "source/path"), []string{}, true},
144-
{"relative path - matching", getApp(".", "source/path"), []string{"source/path/my-deployment.yaml"}, true},
145-
{"relative path, multi source - matching #1", getMultiSourceApp(".", "source/path", "other/path"), []string{"source/path/my-deployment.yaml"}, true},
146-
{"relative path, multi source - matching #2", getMultiSourceApp(".", "other/path", "source/path"), []string{"source/path/my-deployment.yaml"}, true},
147-
{"relative path - not matching", getApp(".", "source/path"), []string{"README.md"}, false},
148-
{"relative path, multi source - not matching", getMultiSourceApp(".", "other/path", "unrelated/path"), []string{"README.md"}, false},
149-
{"absolute path - matching", getApp("/source/path", "source/path"), []string{"source/path/my-deployment.yaml"}, true},
150-
{"absolute path, multi source - matching #1", getMultiSourceApp("/source/path", "source/path", "other/path"), []string{"source/path/my-deployment.yaml"}, true},
151-
{"absolute path, multi source - matching #2", getMultiSourceApp("/source/path", "other/path", "source/path"), []string{"source/path/my-deployment.yaml"}, true},
152-
{"absolute path - not matching", getApp("/source/path1", "source/path"), []string{"source/path/my-deployment.yaml"}, false},
153-
{"absolute path, multi source - not matching", getMultiSourceApp("/source/path1", "other/path", "source/path"), []string{"source/path/my-deployment.yaml"}, false},
154-
{"glob path * - matching", getApp("/source/**/my-deployment.yaml", "source/path"), []string{"source/path/my-deployment.yaml"}, true},
155-
{"glob path * - not matching", getApp("/source/**/my-service.yaml", "source/path"), []string{"source/path/my-deployment.yaml"}, false},
156-
{"glob path ? - matching", getApp("/source/path/my-deployment-?.yaml", "source/path"), []string{"source/path/my-deployment-0.yaml"}, true},
157-
{"glob path ? - not matching", getApp("/source/path/my-deployment-?.yaml", "source/path"), []string{"source/path/my-deployment.yaml"}, false},
158-
{"glob path char range - matching", getApp("/source/path[0-9]/my-deployment.yaml", "source/path"), []string{"source/path1/my-deployment.yaml"}, true},
159-
{"glob path char range - not matching", getApp("/source/path[0-9]/my-deployment.yaml", "source/path"), []string{"source/path/my-deployment.yaml"}, false},
160-
{"mixed glob path - matching", getApp("/source/path[0-9]/my-*.yaml", "source/path"), []string{"source/path1/my-deployment.yaml"}, true},
161-
{"mixed glob path - not matching", getApp("/source/path[0-9]/my-*.yaml", "source/path"), []string{"README.md"}, false},
162-
{"two relative paths - matching", getApp(".;../shared", "my-app"), []string{"shared/my-deployment.yaml"}, true},
163-
{"two relative paths, multi source - matching #1", getMultiSourceApp(".;../shared", "my-app", "other/path"), []string{"shared/my-deployment.yaml"}, true},
164-
{"two relative paths, multi source - matching #2", getMultiSourceApp(".;../shared", "my-app", "other/path"), []string{"shared/my-deployment.yaml"}, true},
165-
{"two relative paths - not matching", getApp(".;../shared", "my-app"), []string{"README.md"}, false},
166-
{"two relative paths, multi source - not matching", getMultiSourceApp(".;../shared", "my-app", "other/path"), []string{"README.md"}, false},
167-
{"file relative path - matching", getApp("./my-deployment.yaml", "source/path"), []string{"source/path/my-deployment.yaml"}, true},
168-
{"file relative path, multi source - matching #1", getMultiSourceApp("./my-deployment.yaml", "source/path", "other/path"), []string{"source/path/my-deployment.yaml"}, true},
169-
{"file relative path, multi source - matching #2", getMultiSourceApp("./my-deployment.yaml", "other/path", "source/path"), []string{"source/path/my-deployment.yaml"}, true},
170-
{"file relative path - not matching", getApp("./my-deployment.yaml", "source/path"), []string{"README.md"}, false},
171-
{"file relative path, multi source - not matching", getMultiSourceApp("./my-deployment.yaml", "source/path", "other/path"), []string{"README.md"}, false},
172-
{"file absolute path - matching", getApp("/source/path/my-deployment.yaml", "source/path"), []string{"source/path/my-deployment.yaml"}, true},
173-
{"file absolute path, multi source - matching #1", getMultiSourceApp("/source/path/my-deployment.yaml", "source/path", "other/path"), []string{"source/path/my-deployment.yaml"}, true},
174-
{"file absolute path, multi source - matching #2", getMultiSourceApp("/source/path/my-deployment.yaml", "other/path", "source/path"), []string{"source/path/my-deployment.yaml"}, true},
175-
{"file absolute path - not matching", getApp("/source/path1/README.md", "source/path"), []string{"source/path/my-deployment.yaml"}, false},
176-
{"file absolute path, multi source - not matching", getMultiSourceApp("/source/path1/README.md", "source/path", "other/path"), []string{"source/path/my-deployment.yaml"}, false},
177-
{"file two relative paths - matching", getApp("./README.md;../shared/my-deployment.yaml", "my-app"), []string{"shared/my-deployment.yaml"}, true},
178-
{"file two relative paths, multi source - matching", getMultiSourceApp("./README.md;../shared/my-deployment.yaml", "my-app", "other-path"), []string{"shared/my-deployment.yaml"}, true},
179-
{"file two relative paths - not matching", getApp(".README.md;../shared/my-deployment.yaml", "my-app"), []string{"kustomization.yaml"}, false},
180-
{"file two relative paths, multi source - not matching", getMultiSourceApp(".README.md;../shared/my-deployment.yaml", "my-app", "other-path"), []string{"kustomization.yaml"}, false},
181-
{"changed file absolute path - matching", getApp(".", "source/path"), []string{"/source/path/my-deployment.yaml"}, true},
182-
}
183-
for _, tt := range tests {
184-
ttc := tt
185-
t.Run(ttc.name, func(t *testing.T) {
186-
t.Parallel()
187-
refreshPaths := GetAppRefreshPaths(ttc.app)
188-
assert.Equal(t, ttc.changeExpected, AppFilesHaveChanged(refreshPaths, ttc.files), "AppFilesHaveChanged()")
189-
})
190-
}
135+
return app
191136
}
192137

193138
func Test_GetAppRefreshPaths(t *testing.T) {
194139
tests := []struct {
195140
name string
196141
app *v1alpha1.Application
142+
source v1alpha1.ApplicationSource
197143
expectedPaths []string
198144
}{
199-
{"default no path", &v1alpha1.Application{}, []string{}},
200-
{"relative path", getApp(".", "source/path"), []string{"source/path"}},
201-
{"absolute path - multi source", getMultiSourceApp("/source/path", "source/path", "other/path"), []string{"source/path"}},
202-
{"two relative paths ", getApp(".;../shared", "my-app"), []string{"my-app", "shared"}},
203-
{"file relative path", getApp("./my-deployment.yaml", "source/path"), []string{"source/path/my-deployment.yaml"}},
204-
{"file absolute path", getApp("/source/path/my-deployment.yaml", "source/path"), []string{"source/path/my-deployment.yaml"}},
205-
{"file two relative paths", getApp("./README.md;../shared/my-deployment.yaml", "my-app"), []string{"my-app/README.md", "shared/my-deployment.yaml"}},
206-
{"glob path", getApp("/source/*/my-deployment.yaml", "source/path"), []string{"source/*/my-deployment.yaml"}},
207-
{"empty path", getApp(".;", "source/path"), []string{"source/path"}},
145+
{
146+
name: "single source without annotation",
147+
app: getApp(nil, ptr.To("source/path")),
148+
source: v1alpha1.ApplicationSource{Path: "source/path"},
149+
expectedPaths: []string{},
150+
},
151+
{
152+
name: "single source with annotation",
153+
app: getApp(ptr.To(".;dev/deploy;other/path"), ptr.To("source/path")),
154+
source: v1alpha1.ApplicationSource{Path: "source/path"},
155+
expectedPaths: []string{"source/path", "source/path/dev/deploy", "source/path/other/path"},
156+
},
157+
{
158+
name: "single source with empty annotation",
159+
app: getApp(ptr.To(".;;"), ptr.To("source/path")),
160+
source: v1alpha1.ApplicationSource{Path: "source/path"},
161+
expectedPaths: []string{"source/path"},
162+
},
163+
{
164+
name: "single source with absolute path annotation",
165+
app: getApp(ptr.To("/fullpath/deploy;other/path"), ptr.To("source/path")),
166+
source: v1alpha1.ApplicationSource{Path: "source/path"},
167+
expectedPaths: []string{"fullpath/deploy", "source/path/other/path"},
168+
},
169+
{
170+
name: "source hydrator sync source without annotation",
171+
app: getSourceHydratorApp(nil, "dry/path", "sync/path"),
172+
source: v1alpha1.ApplicationSource{Path: "sync/path"},
173+
expectedPaths: []string{"sync/path"},
174+
},
175+
{
176+
name: "source hydrator dry source without annotation",
177+
app: getSourceHydratorApp(nil, "dry/path", "sync/path"),
178+
source: v1alpha1.ApplicationSource{Path: "dry/path"},
179+
expectedPaths: []string{},
180+
},
181+
{
182+
name: "source hydrator sync source with annotation",
183+
app: getSourceHydratorApp(ptr.To("deploy"), "dry/path", "sync/path"),
184+
source: v1alpha1.ApplicationSource{Path: "sync/path"},
185+
expectedPaths: []string{"sync/path"},
186+
},
187+
{
188+
name: "source hydrator dry source with annotation",
189+
app: getSourceHydratorApp(ptr.To("deploy"), "dry/path", "sync/path"),
190+
source: v1alpha1.ApplicationSource{Path: "dry/path"},
191+
expectedPaths: []string{"dry/path/deploy"},
192+
},
208193
}
194+
209195
for _, tt := range tests {
210196
ttc := tt
211197
t.Run(ttc.name, func(t *testing.T) {
212198
t.Parallel()
213-
assert.ElementsMatch(t, ttc.expectedPaths, GetAppRefreshPaths(ttc.app), "GetAppRefreshPath()")
199+
assert.ElementsMatch(t, ttc.expectedPaths, GetSourceRefreshPaths(ttc.app, ttc.source), "GetAppRefreshPath()")
214200
})
215201
}
216202
}

0 commit comments

Comments
 (0)