Elgato_dark/argo-cd
1
0
Fork 0
mirror of https://github.com/argoproj/argo-cd synced 2026-04-21 17:07:16 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 22

fix: fix generator bug with templating of parameters in values (#25342) (#26696)

Browse source 
Signed-off-by: Eugene Doudine <eugene.doudine@octopus.com>
This commit is contained in:
dudinea 2026-03-15 16:11:46 +02:00 committed by GitHub
parent 06719071d6
commit f0c694d9f0
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 365 additions and 13 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

179
applicationset/generators/generator_spec_processor_test.go
View file

@ -753,7 +753,7 @@ func TestInterpolateGeneratorError(t *testing.T) {
{name: "Error templating", args: args{
requestedGenerator: &argov1alpha1.ApplicationSetGenerator{Git: &argov1alpha1.GitGenerator{
RepoURL: "foo",
Files: []argov1alpha1.GitFileGeneratorItem{{Path: "bar/"}},
Files: []argov1alpha1.GitFileGeneratorItem{{Path: "path/{{ index .rmap (default .override .test) }}"}},
Revision: "main",
Values: map[string]string{
"git_test": "{{ toPrettyJson . }}",
@ -767,7 +767,7 @@ func TestInterpolateGeneratorError(t *testing.T) {
},
useGoTemplate: true,
goTemplateOptions: []string{},
}, want: argov1alpha1.ApplicationSetGenerator{}, expectedErrStr: "failed to replace parameters in generator: failed to execute go template {{ index .rmap (default .override .test) }}: template: base:1:3: executing \"base\" at <index .rmap (default .override .test)>: error calling index: index of untyped nil"},
}, want: argov1alpha1.ApplicationSetGenerator{}, expectedErrStr: "failed to replace parameters in generator: failed to execute go template path/{{ index .rmap (default .override .test) }}: template: base:1:8: executing \"base\" at <index .rmap (default .override .test)>: error calling index: index of untyped nil"},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
@ -781,3 +781,178 @@ func TestInterpolateGeneratorError(t *testing.T) {
})
}
}
func TestInterpolateGeneratorValuesHandling(t *testing.T) {
applicationSetTemplate := argov1alpha1.ApplicationSetTemplate{
ApplicationSetTemplateMeta: argov1alpha1.ApplicationSetTemplateMeta{
Labels: map[string]string{},
Annotations: map[string]string{},
Finalizers: []string{},
},
Spec: argov1alpha1.ApplicationSpec{
IgnoreDifferences: argov1alpha1.IgnoreDifferences{},
Info: []argov1alpha1.Info{},
Sources: argov1alpha1.ApplicationSources{},
},
}
type args struct {
requestedGenerator *argov1alpha1.ApplicationSetGenerator
params map[string]any
useGoTemplate bool
goTemplateOptions []string
}
tests := []struct {
name string
args args
want argov1alpha1.ApplicationSetGenerator
expectedErrStr string
}{
{
name: "Generator with values set",
args: args{
requestedGenerator: &argov1alpha1.ApplicationSetGenerator{
Git: &argov1alpha1.GitGenerator{
RepoURL: "https://somewhere.com/per-cluster/{{.name}}.git",
Files: []argov1alpha1.GitFileGeneratorItem{{Path: "somedir/*"}},
Revision: "main",
Values: map[string]string{
"appname": "{{ .path.basenameNormalized }}",
},
Template: applicationSetTemplate,
},
},
params: map[string]any{
"name": "in-cluster",
"server": "https://kubernetes.default.svc",
},
useGoTemplate: true,
goTemplateOptions: []string{},
},
want: argov1alpha1.ApplicationSetGenerator{
Git: &argov1alpha1.GitGenerator{
RepoURL: "https://somewhere.com/per-cluster/in-cluster.git",
Files: []argov1alpha1.GitFileGeneratorItem{{Path: "somedir/*"}},
Revision: "main",
Values: map[string]string{
// must stay not interpolated
"appname": "{{ .path.basenameNormalized }}",
},
Directories: []argov1alpha1.GitDirectoryGeneratorItem{},
Template: applicationSetTemplate,
},
},
expectedErrStr: "",
},
{
name: "Generator with no values set",
args: args{
requestedGenerator: &argov1alpha1.ApplicationSetGenerator{
Git: &argov1alpha1.GitGenerator{
RepoURL: "https://somewhere.com/per-cluster/{{.name}}.git",
Files: []argov1alpha1.GitFileGeneratorItem{{Path: "somedir/*"}},
Revision: "main",
Values: map[string]string{},
Template: applicationSetTemplate,
},
},
params: map[string]any{
"name": "in-cluster",
"server": "https://kubernetes.default.svc",
},
useGoTemplate: true,
goTemplateOptions: []string{},
},
want: argov1alpha1.ApplicationSetGenerator{
Git: &argov1alpha1.GitGenerator{
RepoURL: "https://somewhere.com/per-cluster/in-cluster.git",
Files: []argov1alpha1.GitFileGeneratorItem{{Path: "somedir/*"}},
Revision: "main",
Values: map[string]string{},
Directories: []argov1alpha1.GitDirectoryGeneratorItem{},
Template: applicationSetTemplate,
},
},
expectedErrStr: "",
},
{
name: "Generator with values set without go templates",
args: args{
requestedGenerator: &argov1alpha1.ApplicationSetGenerator{
Git: &argov1alpha1.GitGenerator{
RepoURL: "https://somewhere.com/per-cluster/{{name}}.git",
Files: []argov1alpha1.GitFileGeneratorItem{{Path: "somedir/*"}},
Revision: "main",
Values: map[string]string{
"appname": "{{ .path.basenameNormalized }}",
},
Template: applicationSetTemplate,
},
},
params: map[string]any{
"name": "in-cluster",
"server": "https://kubernetes.default.svc",
},
useGoTemplate: false,
},
want: argov1alpha1.ApplicationSetGenerator{
Git: &argov1alpha1.GitGenerator{
RepoURL: "https://somewhere.com/per-cluster/in-cluster.git",
Files: []argov1alpha1.GitFileGeneratorItem{{Path: "somedir/*"}},
Revision: "main",
Values: map[string]string{
"appname": "{{ .path.basenameNormalized }}",
},
Directories: []argov1alpha1.GitDirectoryGeneratorItem{},
Template: applicationSetTemplate,
},
},
expectedErrStr: "",
},
{
name: "Generator without values set and no go templates",
args: args{
requestedGenerator: &argov1alpha1.ApplicationSetGenerator{
Git: &argov1alpha1.GitGenerator{
RepoURL: "https://somewhere.com/per-cluster/{{name}}.git",
Files: []argov1alpha1.GitFileGeneratorItem{{Path: "somedir/*"}},
Revision: "main",
Values: map[string]string{},
Template: applicationSetTemplate,
},
},
params: map[string]any{
"name": "in-cluster",
"server": "https://kubernetes.default.svc",
},
useGoTemplate: false,
},
want: argov1alpha1.ApplicationSetGenerator{
Git: &argov1alpha1.GitGenerator{
RepoURL: "https://somewhere.com/per-cluster/in-cluster.git",
Files: []argov1alpha1.GitFileGeneratorItem{{Path: "somedir/*"}},
Revision: "main",
Values: map[string]string{},
Directories: []argov1alpha1.GitDirectoryGeneratorItem{},
Template: applicationSetTemplate,
},
},
expectedErrStr: "",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, err := InterpolateGenerator(tt.args.requestedGenerator, tt.args.params, tt.args.useGoTemplate, tt.args.goTemplateOptions)
if tt.expectedErrStr != "" {
require.EqualError(t, err, tt.expectedErrStr)
} else {
require.NoError(t, err)
}
assert.Equal(t, tt.want.Git.Values, got.Git.Values)
assert.Equal(t, tt.want.Git.Revision, got.Git.Revision)
assert.Equal(t, tt.want.Git.RepoURL, got.Git.RepoURL)
assert.Equal(t, tt.want.Git.Files, got.Git.Files)
assert.Equal(t, tt.want.Git.Directories, got.Git.Directories)
assert.Equal(t, tt.want.Git.Template, got.Git.Template)
})
}
}

46
applicationset/generators/git_test.go
View file

@ -110,13 +110,18 @@ foo:
{
name: "file parameters are added to params with go template",
args: args{
filePath: "path/dir/file_name.yaml",
fileContent: defaultContent,
values: map[string]string{},
filePath: "path/dir/file_name.yaml",
fileContent: defaultContent,
values: map[string]string{
"somekey": "{{.path.basename}}",
},
useGoTemplate: true,
},
want: []map[string]any{
{
"values": map[string]string{
"somekey": "dir",
},
"foo": map[string]any{
"bar": "baz",
},
@ -164,6 +169,41 @@ foo:
},
},
},
{
name: "path parameter are prefixed with go template and values",
args: args{
filePath: "path/dir/file_name.yaml",
fileContent: defaultContent,
values: map[string]string{
"somekey": "{{.myRepo.path.basename}}",
},
useGoTemplate: true,
pathParamPrefix: "myRepo",
},
want: []map[string]any{
{
"foo": map[string]any{
"bar": "baz",
},
"myRepo": map[string]any{
"path": map[string]any{
"path": "path/dir",
"basename": "dir",
"filename": "file_name.yaml",
"basenameNormalized": "dir",
"filenameNormalized": "file-name.yaml",
"segments": []string{
"path",
"dir",
},
},
},
"values": map[string]string{
"somekey": "dir",
},
},
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {

82
applicationset/generators/matrix_test.go
View file

@ -1100,3 +1100,85 @@ func TestGitGenerator_GenerateParams_list_x_git_matrix_generator(t *testing.T) {
"test": "content",
}}, params)
}
func TestGitGenerator_GenerateParams_list_x_git_matrix_generator_go_templates_values(t *testing.T) {
// Given a matrix generator over a list generator and a git generator with values,
// that contain a template that refers to got generator output parameters.
// This tests for a specific bug where the second generator in the matrix
// failed to evaluate value templates that referred to generator output parameters.
listGeneratorMock := &generatorsMock.Generator{}
listGeneratorMock.EXPECT().GenerateParams(mock.AnythingOfType("*v1alpha1.ApplicationSetGenerator"), mock.AnythingOfType("*v1alpha1.ApplicationSet"), mock.Anything).Return([]map[string]any{
{"some": "value"},
}, nil)
listGeneratorMock.EXPECT().GetTemplate(mock.AnythingOfType("*v1alpha1.ApplicationSetGenerator")).Return(&v1alpha1.ApplicationSetTemplate{})
gitGeneratorSpec := &v1alpha1.GitGenerator{
RepoURL: "https://git.example.com",
Files: []v1alpha1.GitFileGeneratorItem{
{Path: "some/path.json"},
},
Values: map[string]string{
"foo": "{{.path.basename}}",
},
}
repoServiceMock := &servicesMocks.Repos{}
repoServiceMock.EXPECT().GetFiles(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(map[string][]byte{
"some/path.json": []byte("test: content"),
}, nil).Maybe()
gitGenerator := NewGitGenerator(repoServiceMock, "")
matrixGenerator := NewMatrixGenerator(map[string]Generator{
"List": listGeneratorMock,
"Git": gitGenerator,
})
matrixGeneratorSpec := &v1alpha1.MatrixGenerator{
Generators: []v1alpha1.ApplicationSetNestedGenerator{
{
List: &v1alpha1.ListGenerator{
Elements: []apiextensionsv1.JSON{
{
Raw: []byte(`{"some": "value"}`),
},
},
},
},
{
Git: gitGeneratorSpec,
},
},
}
scheme := runtime.NewScheme()
err := v1alpha1.AddToScheme(scheme)
require.NoError(t, err)
appProject := v1alpha1.AppProject{}
client := fake.NewClientBuilder().WithScheme(scheme).WithObjects(&appProject).Build()
params, err := matrixGenerator.GenerateParams(&v1alpha1.ApplicationSetGenerator{
Matrix: matrixGeneratorSpec,
}, &v1alpha1.ApplicationSet{
Spec: v1alpha1.ApplicationSetSpec{
GoTemplate: true,
},
}, client)
require.NoError(t, err)
assert.Equal(t, []map[string]any{{
"path": map[string]any{
"basename": "some",
"basenameNormalized": "some",
"filename": "path.json",
"filenameNormalized": "path.json",
"path": "some",
"segments": []string{"some"},
},
"some": "value",
"test": "content",
"values": map[string]string{
"foo": "some",
},
}}, params)
}

61
applicationset/utils/utils.go
View file

@ -90,7 +90,7 @@ func ConvertYAMLToJSON(str string) (string, error) {
// This function is in charge of searching all String fields of the object recursively and apply templating
// thanks to https://gist.github.com/randallmlough/1fd78ec8a1034916ca52281e3b886dc7
func (r *Render) deeplyReplace(destination, original reflect.Value, replaceMap map[string]any, useGoTemplate bool, goTemplateOptions []string) error {
func (r *Render) deeplyReplaceWithFilter(destination, original reflect.Value, replaceMap map[string]any, useGoTemplate bool, goTemplateOptions []string, filter func(destination, original, parent reflect.Value, field reflect.StructField) (bool, error)) error {
switch original.Kind() {
// The first cases handle nested structures and translate them recursively
// If it is a pointer we need to unwrap and call once again
@ -110,7 +110,7 @@ func (r *Render) deeplyReplace(destination, original reflect.Value, replaceMap m
copyUnexported(destination, original)
}
// Unwrap the newly created pointer
if err := r.deeplyReplace(destination.Elem(), originalValue, replaceMap, useGoTemplate, goTemplateOptions); err != nil {
if err := r.deeplyReplaceWithFilter(destination.Elem(), originalValue, replaceMap, useGoTemplate, goTemplateOptions, filter); err != nil {
// Not wrapping the error, since this is a recursive function. Avoids excessively long error messages.
return err
}
@ -131,7 +131,7 @@ func (r *Render) deeplyReplace(destination, original reflect.Value, replaceMap m
reflectValue := reflect.New(reflectType)
copyValue := reflectValue.Elem()
if err := r.deeplyReplace(copyValue, originalValue, replaceMap, useGoTemplate, goTemplateOptions); err != nil {
if err := r.deeplyReplaceWithFilter(copyValue, originalValue, replaceMap, useGoTemplate, goTemplateOptions, filter); err != nil {
// Not wrapping the error, since this is a recursive function. Avoids excessively long error messages.
return err
}
@ -142,6 +142,16 @@ func (r *Render) deeplyReplace(destination, original reflect.Value, replaceMap m
case reflect.Struct:
for i := 0; i < original.NumField(); i++ {
currentType := fmt.Sprintf("%s.%s", original.Type().Field(i).Name, original.Type().PkgPath())
if filter != nil {
matched, filterErr := filter(destination.Field(i), original.Field(i), original,
original.Type().Field(i))
if matched {
if filterErr != nil {
return filterErr
}
continue
}
}
// specific case time
if currentType == "time.Time" {
destination.Field(i).Set(original.Field(i))
@ -158,7 +168,7 @@ func (r *Render) deeplyReplace(destination, original reflect.Value, replaceMap m
}
jsonOriginal := reflect.ValueOf(&unmarshaled)
jsonCopy := reflect.New(jsonOriginal.Type()).Elem()
err = r.deeplyReplace(jsonCopy, jsonOriginal, replaceMap, useGoTemplate, goTemplateOptions)
err = r.deeplyReplaceWithFilter(jsonCopy, jsonOriginal, replaceMap, useGoTemplate, goTemplateOptions, filter)
if err != nil {
return fmt.Errorf("failed to deeply replace JSON field contents: %w", err)
}
@ -168,7 +178,7 @@ func (r *Render) deeplyReplace(destination, original reflect.Value, replaceMap m
return fmt.Errorf("failed to marshal templated JSON field: %w", err)
}
destination.Field(i).Set(reflect.ValueOf(data))
} else if err := r.deeplyReplace(destination.Field(i), original.Field(i), replaceMap, useGoTemplate, goTemplateOptions); err != nil {
} else if err := r.deeplyReplaceWithFilter(destination.Field(i), original.Field(i), replaceMap, useGoTemplate, goTemplateOptions, filter); err != nil {
// Not wrapping the error, since this is a recursive function. Avoids excessively long error messages.
return err
}
@ -183,7 +193,7 @@ func (r *Render) deeplyReplace(destination, original reflect.Value, replaceMap m
}
for i := 0; i < original.Len(); i++ {
if err := r.deeplyReplace(destination.Index(i), original.Index(i), replaceMap, useGoTemplate, goTemplateOptions); err != nil {
if err := r.deeplyReplaceWithFilter(destination.Index(i), original.Index(i), replaceMap, useGoTemplate, goTemplateOptions, filter); err != nil {
// Not wrapping the error, since this is a recursive function. Avoids excessively long error messages.
return err
}
@ -204,7 +214,7 @@ func (r *Render) deeplyReplace(destination, original reflect.Value, replaceMap m
// New gives us a pointer, but again we want the value
copyValue := reflect.New(originalValue.Type()).Elem()
if err := r.deeplyReplace(copyValue, originalValue, replaceMap, useGoTemplate, goTemplateOptions); err != nil {
if err := r.deeplyReplaceWithFilter(copyValue, originalValue, replaceMap, useGoTemplate, goTemplateOptions, filter); err != nil {
// Not wrapping the error, since this is a recursive function. Avoids excessively long error messages.
return err
}
@ -249,6 +259,10 @@ func (r *Render) deeplyReplace(destination, original reflect.Value, replaceMap m
return nil
}
func (r *Render) deeplyReplace(destination, original reflect.Value, replaceMap map[string]any, useGoTemplate bool, goTemplateOptions []string) error {
return r.deeplyReplaceWithFilter(destination, original, replaceMap, useGoTemplate, goTemplateOptions, nil)
}
// isNillable returns true if the value is something which may be set to nil. This function is meant to guard against a
// panic from calling IsNil on a non-pointer type.
func isNillable(v reflect.Value) bool {
@ -290,6 +304,27 @@ func (r *Render) RenderTemplateParams(tmpl *argoappsv1.Application, syncPolicy *
return replacedTmpl, nil
}
// Generator types that have Value field
var filteredGeneratorTypes = getFilteredGeneratorTypes()
// find generator types that have Values field
func getFilteredGeneratorTypes() map[string]bool {
result := map[string]bool{}
t := reflect.TypeFor[argoappsv1.ApplicationSetGenerator]()
for field := range t.Fields() {
genPtrType := field.Type
if genPtrType.Kind() == reflect.Ptr && strings.HasSuffix(genPtrType.String(), "Generator") {
genType := genPtrType.Elem()
for field := range genType.Fields() {
if field.Name == "Values" && field.Type.String() == "map[string]string" {
result[genType.Name()] = true
}
}
}
}
return result
}
func (r *Render) RenderGeneratorParams(gen *argoappsv1.ApplicationSetGenerator, params map[string]any, useGoTemplate bool, goTemplateOptions []string) (*argoappsv1.ApplicationSetGenerator, error) {
if gen == nil {
return nil, errors.New("generator is empty")
@ -302,7 +337,17 @@ func (r *Render) RenderGeneratorParams(gen *argoappsv1.ApplicationSetGenerator,
original := reflect.ValueOf(gen)
destination := reflect.New(original.Type()).Elem()
if err := r.deeplyReplace(destination, original, params, useGoTemplate, goTemplateOptions); err != nil {
filter := func(destination, original, parent reflect.Value, field reflect.StructField) (bool, error) {
if field.Name == "Values" && field.Type.String() == "map[string]string" && filteredGeneratorTypes[parent.Type().Name()] {
if !destination.CanSet() {
return false, fmt.Errorf("cannot copy %s.Values, this cannot happen", parent.Type().Name())
}
destination.Set(original)
return true, nil
}
return false, nil
}
if err := r.deeplyReplaceWithFilter(destination, original, params, useGoTemplate, goTemplateOptions, filter); err != nil {
return nil, fmt.Errorf("failed to replace parameters in generator: %w", err)
}

10
applicationset/utils/utils_test.go
View file

@ -5,6 +5,7 @@ import (
"encoding/json"
"os"
"path"
"strings"
"testing"
"time"
@ -1390,3 +1391,12 @@ WkBKOclmOV2xlTVuPw==
})
}
}
func Test_getFilteredGeneratorTypes(t *testing.T) {
generators := getFilteredGeneratorTypes()
assert.Less(t, 1, len(generators))
for name, val := range generators {
assert.True(t, val)
assert.True(t, strings.HasSuffix(name, "Generator"))
}
}