Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Matrix generator where a generator can reference items of another one #9080

Merged
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
d4da216
Upgraded Go to 1.18, & have changes related to matrix generator's chi…
jkulkarn Apr 12, 2022
38a2b56
Added import for applicationSet in matrix.go
jkulkarn Apr 12, 2022
5c7ae5a
Added import for applicationSet in matrix.go
jkulkarn Apr 12, 2022
f09d41e
Added logging + small fixes for ArgoCD PR
jkulkarn Apr 12, 2022
491d301
Added testing for GetRelevantGenerators
jkulkarn Apr 13, 2022
535a391
Added a test for interpolating Generators
jkulkarn Apr 13, 2022
4dfa276
Added a 2nd test for interpolating Generators
jkulkarn Apr 13, 2022
424f0c9
Merge branch 'master' into feat/add-intra-generator-param-support
KojoRising Apr 14, 2022
4edb6fe
Updated Generators-Matrix.md documentation to include an example + re…
jkulkarn Apr 14, 2022
ad633e2
Small wording fix.
jkulkarn Apr 14, 2022
62c0426
Merge branch 'master' into feat/add-intra-generator-param-support
KojoRising Apr 15, 2022
a719638
Small change to generator_spec_processor.go
jkulkarn Apr 16, 2022
78705ce
Merge remote-tracking branch 'origin/feat/add-intra-generator-param-s…
jkulkarn Apr 19, 2022
a6a915b
Fixing Test case
jkulkarn Apr 20, 2022
6001908
Small changes for matrix + generator_spec_processor.go
jkulkarn Apr 20, 2022
295a6ed
Fixed (I believe) the issue that @Lobstrosity mentioned.
jkulkarn Apr 22, 2022
4bed874
Refactored code to accept map[string]string instead of []map[string]s…
jkulkarn Apr 22, 2022
54deaed
Fixing test cases
jkulkarn Apr 22, 2022
cc8d8bc
Merge branch 'master' into feat/add-intra-generator-param-support
jkulkarn Apr 25, 2022
5785aab
Fixing lint error.
jkulkarn Apr 25, 2022
e59253c
Merge branch 'master' into feat/add-intra-generator-param-support
KojoRising May 3, 2022
6bf05d1
Merge branch 'master' into feat/add-intra-generator-param-support
KojoRising May 23, 2022
db5e8af
Using test-case suggestion from @rumstead.
jkulkarn May 24, 2022
0dae059
Changing up naming from testing.
jkulkarn May 25, 2022
ec0854a
Merge branch 'master' into feat/add-intra-generator-param-support
jkulkarn May 25, 2022
2ec3e94
Updated go.sum
jkulkarn May 25, 2022
cd3602d
Cleaning up for linter.
jkulkarn May 25, 2022
848f957
Merge branch 'master' into feat/add-intra-generator-param-support
KojoRising May 27, 2022
b297f20
Merge branch 'master' into feat/add-intra-generator-param-support
KojoRising Jun 2, 2022
b2d336e
Update Generators-Matrix.md
KojoRising Jun 2, 2022
c4c4654
Added changes as asked by @crenshaw-dev. These include
jkulkarn Jun 3, 2022
598eff0
Merge branch 'master' into feat/add-intra-generator-param-support
KojoRising Jun 6, 2022
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
2 changes: 1 addition & 1 deletion applicationset/controllers/applicationset_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,7 @@ func (r *ApplicationSetReconciler) generateApplications(applicationSetInfo argop
var applicationSetReason argoprojiov1alpha1.ApplicationSetReasonType

for _, requestedGenerator := range applicationSetInfo.Spec.Generators {
t, err := generators.Transform(requestedGenerator, r.Generators, applicationSetInfo.Spec.Template, &applicationSetInfo)
t, err := generators.Transform(requestedGenerator, r.Generators, applicationSetInfo.Spec.Template, &applicationSetInfo, []map[string]string{})
if err != nil {
log.WithError(err).WithField("generator", requestedGenerator).
Error("error generating application from params")
Expand Down
49 changes: 43 additions & 6 deletions applicationset/generators/generator_spec_processor.go
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
package generators

import (
"encoding/json"
"github.com/argoproj/argo-cd/v2/applicationset/utils"
"github.com/valyala/fasttemplate"
"reflect"

argoprojiov1alpha1 "github.com/argoproj/argo-cd/v2/pkg/apis/applicationset/v1alpha1"
"github.com/imdario/mergo"
log "github.com/sirupsen/logrus"

argoprojiov1alpha1 "github.com/argoproj/argo-cd/v2/pkg/apis/applicationset/v1alpha1"
)

type TransformResult struct {
Expand All @@ -15,7 +17,7 @@ type TransformResult struct {
}

//Transform a spec generator to list of paramSets and a template
func Transform(requestedGenerator argoprojiov1alpha1.ApplicationSetGenerator, allGenerators map[string]Generator, baseTemplate argoprojiov1alpha1.ApplicationSetTemplate, appSet *argoprojiov1alpha1.ApplicationSet) ([]TransformResult, error) {
func Transform(requestedGenerator argoprojiov1alpha1.ApplicationSetGenerator, allGenerators map[string]Generator, baseTemplate argoprojiov1alpha1.ApplicationSetTemplate, appSet *argoprojiov1alpha1.ApplicationSet, genParams []map[string]string) ([]TransformResult, error) {
res := []TransformResult{}
var firstError error

Expand All @@ -31,7 +33,17 @@ func Transform(requestedGenerator argoprojiov1alpha1.ApplicationSetGenerator, al
}
continue
}

if len(genParams) != 0 {
crenshaw-dev marked this conversation as resolved.
Show resolved Hide resolved
err := interpolateGenerator(&requestedGenerator, genParams)
if err != nil {
log.WithError(err).WithField("genParams", genParams).
Error("error interpolating params for generator")
if firstError == nil {
firstError = err
}
continue
}
}
params, err := g.GenerateParams(&requestedGenerator, appSet)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in addition to above change replace requestedGenerator with newrequestedgenerator

if err != nil {
log.WithError(err).WithField("generator", g).
Expand All @@ -46,11 +58,9 @@ func Transform(requestedGenerator argoprojiov1alpha1.ApplicationSetGenerator, al
Params: params,
Template: mergedTemplate,
})

}

return res, firstError

}

func GetRelevantGenerators(requestedGenerator *argoprojiov1alpha1.ApplicationSetGenerator, generators map[string]Generator) []Generator {
Expand Down Expand Up @@ -81,3 +91,30 @@ func mergeGeneratorTemplate(g Generator, requestedGenerator *argoprojiov1alpha1.

return *dest, err
}

// Currently for Matrix Generator. Allows interpolating the matrix's 2nd child generator with values from the 1st child generator
// "params" parameter is an array, where each index corresponds to a generator. Each index contains a map w/ that generator's parameters.
// Since the Matrix generator currently only allows 2 child generators, effectively this params array will always be size 1 (containing the 1st child generator's params)
crenshaw-dev marked this conversation as resolved.
Show resolved Hide resolved
// Uses similar process for interpreting values as the actual Application Template
func interpolateGenerator(requestedGenerator *argoprojiov1alpha1.ApplicationSetGenerator, params []map[string]string) error {
Copy link
Contributor

@rishabh625 rishabh625 Apr 21, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@KojoRising like @Lobstrosity said returning generator from interpolation would solve ur problem of resusbstituting as original generator will not be replaced like this

func interpolateGenerator(requestedGenerator *argoprojiov1alpha1.ApplicationSetGenerator, params []map[string]string) (argoprojiov1alpha1.ApplicationSetGenerator, error) {
	newrequestedGenerator := *requestedGenerator
	tmplBytes, err := json.Marshal(newrequestedGenerator)
	if err != nil {
		log.WithError(err).WithField("requestedGenerator", requestedGenerator).Error("error marshalling requested generator for interpolation")
		return newrequestedGenerator, err
	}

	render := utils.Render{}
	fstTmpl := fasttemplate.New(string(tmplBytes), "{{", "}}")
	replacedTmplStr, err := render.Replace(fstTmpl, params[0], true)
	if err != nil {
		log.WithError(err).WithField("interpolatedGeneratorString", replacedTmplStr).Error("error interpolating generator with other generator's parameter")
		return newrequestedGenerator, err
	}

	err = json.Unmarshal([]byte(replacedTmplStr), requestedGenerator)
	if err != nil {
		log.WithError(err).WithField("requestedGenerator", requestedGenerator).Error("error unmarshalling requested generator for interpolation")
		return newrequestedGenerator, err
	}
	return newrequestedGenerator, nil
}

and yes we also need to re-evaluate the 2nd generator for each result from the 1st generator

tmplBytes, err := json.Marshal(requestedGenerator)
if err != nil {
log.WithError(err).WithField("requestedGenerator", requestedGenerator).Error("error marshalling requested generator for interpolation")
return err
}

render := utils.Render{}
fstTmpl := fasttemplate.New(string(tmplBytes), "{{", "}}")
replacedTmplStr, err := render.Replace(fstTmpl, params[0], true)
if err != nil {
log.WithError(err).WithField("interpolatedGeneratorString", replacedTmplStr).Error("error interpolating generator with other generator's parameter")
return err
}

err = json.Unmarshal([]byte(replacedTmplStr), requestedGenerator)
if err != nil {
log.WithError(err).WithField("requestedGenerator", requestedGenerator).Error("error unmarshalling requested generator for interpolation")
return err
}
return nil
}
300 changes: 300 additions & 0 deletions applicationset/generators/generator_spec_processor_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,300 @@
package generators

import (
"context"
"fmt"
log "github.com/sirupsen/logrus"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
corev1 "k8s.io/api/core/v1"
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
kubefake "k8s.io/client-go/kubernetes/fake"
crtclient "sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/fake"
"testing"

argoprojiov1alpha1 "github.com/argoproj/argo-cd/v2/pkg/apis/applicationset/v1alpha1"
)

type genProcessorMock struct {
mock.Mock
}

func (g *genProcessorMock) Transform(requestedGenerator argoprojiov1alpha1.ApplicationSetGenerator, allGenerators map[string]Generator, baseTemplate argoprojiov1alpha1.ApplicationSetTemplate, appSet *argoprojiov1alpha1.ApplicationSet, genParams []map[string]string) ([]TransformResult, error) {
args := g.Called(requestedGenerator)

return args.Get(0).([]TransformResult), args.Error(1)
}

func (g *genProcessorMock) GetRelevantGenerators(requestedGenerator argoprojiov1alpha1.ApplicationSetGenerator, generators map[string]Generator) []Generator {
args := g.Called(requestedGenerator)

return args.Get(0).([]Generator)
}

func (g *genProcessorMock) interpolateGenerator(requestedGenerator *argoprojiov1alpha1.ApplicationSetGenerator, params []map[string]string) error {
args := g.Called(requestedGenerator)

return args.Error(1)
}

func getMockClusterGenerator() Generator {
clusters := []crtclient.Object{
&corev1.Secret{
TypeMeta: metav1.TypeMeta{
Kind: "Secret",
APIVersion: "v1",
},
ObjectMeta: metav1.ObjectMeta{
Name: "staging-01",
Namespace: "namespace",
Labels: map[string]string{
"argocd.argoproj.io/secret-type": "cluster",
"environment": "staging",
"org": "foo",
},
Annotations: map[string]string{
"foo.argoproj.io": "staging",
},
},
Data: map[string][]byte{
"config": []byte("{}"),
"name": []byte("staging-01"),
"server": []byte("https://staging-01.example.com"),
},
Type: corev1.SecretType("Opaque"),
},
&corev1.Secret{
TypeMeta: metav1.TypeMeta{
Kind: "Secret",
APIVersion: "v1",
},
ObjectMeta: metav1.ObjectMeta{
Name: "production-01",
Namespace: "namespace",
Labels: map[string]string{
"argocd.argoproj.io/secret-type": "cluster",
"environment": "production",
"org": "bar",
},
Annotations: map[string]string{
"foo.argoproj.io": "production",
},
},
Data: map[string][]byte{
"config": []byte("{}"),
"name": []byte("production_01/west"),
"server": []byte("https://production-01.example.com"),
},
Type: corev1.SecretType("Opaque"),
},
}
runtimeClusters := []runtime.Object{}
appClientset := kubefake.NewSimpleClientset(runtimeClusters...)

fakeClient := fake.NewClientBuilder().WithObjects(clusters...).Build()
return NewClusterGenerator(fakeClient, context.Background(), appClientset, "namespace")
}

func getMockGitGenerator() Generator {
cases := []struct {
name string
directories []argoprojiov1alpha1.GitDirectoryGeneratorItem
repoApps []string
repoError error
expected []map[string]string
expectedError error
}{
{
name: "happy flow - created apps",
directories: []argoprojiov1alpha1.GitDirectoryGeneratorItem{{Path: "*"}},
repoApps: []string{
"app1",
"app2",
"app_3",
"p1/app4",
},
repoError: nil,
expected: []map[string]string{
{"path": "app1", "path.basename": "app1", "path.basenameNormalized": "app1"},
{"path": "app2", "path.basename": "app2", "path.basenameNormalized": "app2"},
{"path": "app_3", "path.basename": "app_3", "path.basenameNormalized": "app-3"},
},
expectedError: nil,
},
{
name: "It filters application according to the paths",
directories: []argoprojiov1alpha1.GitDirectoryGeneratorItem{{Path: "p1/*"}, {Path: "p1/*/*"}},
repoApps: []string{
"app1",
"p1/app2",
"p1/p2/app3",
"p1/p2/p3/app4",
},
repoError: nil,
expected: []map[string]string{
{"path": "p1/app2", "path.basename": "app2", "path[0]": "p1", "path.basenameNormalized": "app2"},
{"path": "p1/p2/app3", "path.basename": "app3", "path[0]": "p1", "path[1]": "p2", "path.basenameNormalized": "app3"},
},
expectedError: nil,
},
{
name: "It filters application according to the paths with Exclude",
directories: []argoprojiov1alpha1.GitDirectoryGeneratorItem{{Path: "p1/*", Exclude: true}, {Path: "*"}, {Path: "*/*"}},
repoApps: []string{
"app1",
"app2",
"p1/app2",
"p1/app3",
"p2/app3",
},
repoError: nil,
expected: []map[string]string{
{"path": "app1", "path.basename": "app1", "path.basenameNormalized": "app1"},
{"path": "app2", "path.basename": "app2", "path.basenameNormalized": "app2"},
{"path": "p2/app3", "path.basename": "app3", "path[0]": "p2", "path.basenameNormalized": "app3"},
},
expectedError: nil,
},
{
name: "Expecting same exclude behavior with different order",
directories: []argoprojiov1alpha1.GitDirectoryGeneratorItem{{Path: "*"}, {Path: "*/*"}, {Path: "p1/*", Exclude: true}},
repoApps: []string{
"app1",
"app2",
"p1/app2",
"p1/app3",
"p2/app3",
},
repoError: nil,
expected: []map[string]string{
{"path": "app1", "path.basename": "app1", "path.basenameNormalized": "app1"},
{"path": "app2", "path.basename": "app2", "path.basenameNormalized": "app2"},
{"path": "p2/app3", "path.basename": "app3", "path[0]": "p2", "path.basenameNormalized": "app3"},
},
expectedError: nil,
},
{
name: "handles empty response from repo server",
directories: []argoprojiov1alpha1.GitDirectoryGeneratorItem{{Path: "*"}},
repoApps: []string{},
repoError: nil,
expected: []map[string]string{},
expectedError: nil,
},
{
name: "handles error from repo server",
directories: []argoprojiov1alpha1.GitDirectoryGeneratorItem{{Path: "*"}},
repoApps: []string{},
repoError: fmt.Errorf("error"),
expected: []map[string]string{},
expectedError: fmt.Errorf("error"),
},
}
argoCDServiceMock := argoCDServiceMock{mock: &mock.Mock{}}

argoCDServiceMock.mock.On("GetDirectories", mock.Anything, mock.Anything, mock.Anything).Return(cases[0].repoApps, cases[0].repoError)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You define several cases but only appear to use the first one.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh yeah, this was more for testing the GetRelevantGenerators function (not mine). I'm fairly new to Go (I'm more on the ArgoCD Yaml side :), so I was using some tests that were used for the Git Generators (where they were specifically testing the Git Generator).

My test cases were below in the TestGetRelevantGenerators function:

image

Anyways, I cleaned up the above function you mentioned. Thanks for pointing it out!


var gitGenerator = NewGitGenerator(argoCDServiceMock)
return gitGenerator
}

func TestGetRelevantGenerators(t *testing.T) {

testGenerators := map[string]Generator{
"Clusters": getMockClusterGenerator(),
"Git": getMockGitGenerator(),
}

testGenerators["Matrix"] = NewMatrixGenerator(testGenerators)
testGenerators["Merge"] = NewMergeGenerator(testGenerators)
testGenerators["List"] = NewListGenerator()

requestedGenerator := &argoprojiov1alpha1.ApplicationSetGenerator{
List: &argoprojiov1alpha1.ListGenerator{
Elements: []apiextensionsv1.JSON{{Raw: []byte(`{"cluster": "cluster","url": "url","values":{"foo":"bar"}}`)}},
}}

relevantGenerators := GetRelevantGenerators(requestedGenerator, testGenerators)
assert.Len(t, relevantGenerators, 1)
assert.IsType(t, &ListGenerator{}, relevantGenerators[0])

requestedGenerator = &argoprojiov1alpha1.ApplicationSetGenerator{
Clusters: &argoprojiov1alpha1.ClusterGenerator{
Selector: metav1.LabelSelector{},
Template: argoprojiov1alpha1.ApplicationSetTemplate{},
Values: nil,
},
}

relevantGenerators = GetRelevantGenerators(requestedGenerator, testGenerators)
assert.Len(t, relevantGenerators, 1)
assert.IsType(t, &ClusterGenerator{}, relevantGenerators[0])

requestedGenerator = &argoprojiov1alpha1.ApplicationSetGenerator{
Git: &argoprojiov1alpha1.GitGenerator{
RepoURL: "",
Directories: nil,
Files: nil,
Revision: "",
RequeueAfterSeconds: nil,
Template: argoprojiov1alpha1.ApplicationSetTemplate{},
},
}

relevantGenerators = GetRelevantGenerators(requestedGenerator, testGenerators)
assert.Len(t, relevantGenerators, 1)
assert.IsType(t, &GitGenerator{}, relevantGenerators[0])
}

func TestInterpolateGenerator(t *testing.T) {
requestedGenerator := &argoprojiov1alpha1.ApplicationSetGenerator{
Clusters: &argoprojiov1alpha1.ClusterGenerator{
Selector: metav1.LabelSelector{
MatchLabels: map[string]string{
"argocd.argoproj.io/secret-type": "cluster",
"path-basename": "{{path.basename}}",
"path-zero": "{{path[0]}}",
"path-full": "{{path}}",
}},
},
}
//mockGitGenerator := getMockGitGenerator()
gitGeneratorParams := []map[string]string{
{"path": "p1/p2/app3", "path.basename": "app3", "path[0]": "p1", "path[1]": "p2", "path.basenameNormalized": "app3"},
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@KojoRising Regarding this comment and the screenshot with the arrows:

This gitGeneratorParams mock represents a git generator that yields a single result. My concern is, what happens when the git generator yields multiple results? All of those results would be in g0 over in matrix.go but interpolateGenerator is only interpolating values from the first result.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Lobstrosity

Ahh, got it got it. Yes, you’re right, for multiple paths specified in a git generator, it wouldnt pick those up.

I’ll update the code/test-cases to support this. Thank you for pointing it out!

err := interpolateGenerator(requestedGenerator, gitGeneratorParams)
if err != nil {
log.WithError(err).WithField("requestedGenerator", requestedGenerator).Error("error interpolating Generator")
return
}
assert.Equal(t, "app3", requestedGenerator.Clusters.Selector.MatchLabels["path-basename"])
assert.Equal(t, "p1", requestedGenerator.Clusters.Selector.MatchLabels["path-zero"])
assert.Equal(t, "p1/p2/app3", requestedGenerator.Clusters.Selector.MatchLabels["path-full"])

fileNamePath := argoprojiov1alpha1.GitFileGeneratorItem{
Path: "{{name}}",
}
fileServerPath := argoprojiov1alpha1.GitFileGeneratorItem{
Path: "{{server}}",
}

requestedGenerator = &argoprojiov1alpha1.ApplicationSetGenerator{
Git: &argoprojiov1alpha1.GitGenerator{
Files: append([]argoprojiov1alpha1.GitFileGeneratorItem{}, fileNamePath, fileServerPath),
Template: argoprojiov1alpha1.ApplicationSetTemplate{},
},
}
clusterGeneratorParams := []map[string]string{
{"name": "production_01/west", "server": "https://production-01.example.com"},
}
err = interpolateGenerator(requestedGenerator, clusterGeneratorParams)
if err != nil {
log.WithError(err).WithField("requestedGenerator", requestedGenerator).Error("error interpolating Generator")
return
}
assert.Equal(t, "production_01/west", requestedGenerator.Git.Files[0].Path)
assert.Equal(t, "https://production-01.example.com", requestedGenerator.Git.Files[1].Path)
}
Loading