Repository: argoproj-labs/applicationset (go)

Issue:
'Error generating params' error when using JSON array in Git files generator file
An 'error generating params' error is output when using the following ApplicationSet on the specified repo.

The following ApplicationSet will trigger the error:
```yaml
apiVersion: argoproj.io/v1alpha1
kind: ApplicationSet
metadata:
  name: guestbook
spec:
  generators:
  - git:
      revision: HEAD
      repoURL: https://github.com/jgwest/argocd-example-apps.git
      files:
        - path: appset-examples/git-files-example/config/clusters.json   # <--- this line is the important one
  template:
    metadata:
      name: '{{cluster.name}}-guestbook'
    spec:
      project: default
      source:
        repoURL: https://github.com/jgwest/argocd-example-apps.git
        targetRevision: HEAD
        path: appset-examples/git-files-example/apps/guestbook/{{appVersions.guestbook}}
      destination:
        server: https://kubernetes.default.svc
        namespace: guestbook
```

This is because the `cluster.json` (referenced above) looks like this [(source)](https://github.com/jgwest/argocd-example-apps/blob/master/appset-examples/git-files-example/config/clusters.json):
```yaml
[
  {
    "account": "123456",
    "asset_id": "11223344",
    // ...
  },
  {
    "account": "456789",
    "asset_id": "55667788",
    // ...
  }
]
```
Notice that this is a JSON array (of objects), rather than a single JSON object... it appears the current Git file generator parsing code assumes that files will only be JSON objects, not JSON arrays.


Error log:

```
INFO[0448] Trace                                         args="[git clean -fdx]" dir="/tmp/https:__github.com_jgwest_argocd-example-apps" operation_name="exec git" time_ms=4.000146000000001
ERRO[0448] error generating params                       error="json: cannot unmarshal array into Go value of type map[string]interface {}" generator="&{0xc0003a4f00}"
2021-01-26T12:01:54.853-0500	ERROR	controller-runtime.manager.controller.applicationset	Reconciler error	{"reconciler group": "argoproj.io", "reconciler kind": "ApplicationSet", "name": "guestbook", "namespace": "argocd", "error": "json: cannot unmarshal array into Go value of type map[string]interface {}"}
```


Pull request:
'Error generating params' error when using JSON array in Git files generator file
Fixes #107 

See comments on parent issue for details.

Patch:
diff --git a/pkg/generators/git.go b/pkg/generators/git.go
index 1e16c2d..4f00617 100644
--- a/pkg/generators/git.go
+++ b/pkg/generators/git.go
@@ -3,6 +3,7 @@ package generators
 import (
 	"context"
 	"encoding/json"
+	"fmt"
 	"path"
 	"sort"
 	"time"
@@ -118,38 +119,59 @@ func (g *GitGenerator) generateParamsForGitFiles(appSetGenerator *argoprojiov1al
 	// Generate params from each path, and return
 	res := []map[string]string{}
 	for _, path := range allPaths {
-		params, err := g.generateParamsFromGitFile(appSetGenerator, path)
+
+		// A JSON file path can contain multiple sets of parameters (ie it is an array)
+		paramsArray, err := g.generateParamsFromGitFile(appSetGenerator, path)
 		if err != nil {
-			return nil, err
+			return nil, fmt.Errorf("unable to process file '%s': %v", path, err)
 		}
 
-		res = append(res, params)
+		for index := range paramsArray {
+			res = append(res, paramsArray[index])
+		}
 	}
 	return res, nil
 }
 
-func (g *GitGenerator) generateParamsFromGitFile(appSetGenerator *argoprojiov1alpha1.ApplicationSetGenerator, path string) (map[string]string, error) {
-	content, err := g.repos.GetFileContent(context.TODO(), appSetGenerator.Git.RepoURL, appSetGenerator.Git.Revision, path)
-	if err != nil {
-		return nil, err
-	}
+func (g *GitGenerator) generateParamsFromGitFile(appSetGenerator *argoprojiov1alpha1.ApplicationSetGenerator, path string) ([]map[string]string, error) {
 
-	config := make(map[string]interface{})
-	err = json.Unmarshal(content, &config)
+	fileContent, err := g.repos.GetFileContent(context.TODO(), appSetGenerator.Git.RepoURL, appSetGenerator.Git.Revision, path)
 	if err != nil {
 		return nil, err
 	}
 
-	flat, err := flatten.Flatten(config, "", flatten.DotStyle)
+	objectsFound := []map[string]interface{}{}
+
+	// First, we attempt to parse as an array
+	err = json.Unmarshal(fileContent, &objectsFound)
 	if err != nil {
-		return nil, err
+		// If unable to parse as an array, attempt to parse as a single JSON object
+		singleJSONObj := make(map[string]interface{})
+		err = json.Unmarshal(fileContent, &singleJSONObj)
+		if err != nil {
+			return nil, fmt.Errorf("unable to parse JSON file: %v", err)
+		}
+		objectsFound = append(objectsFound, singleJSONObj)
 	}
-	params := make(map[string]string)
-	for k, v := range flat {
-		params[k] = v.(string)
+
+	res := []map[string]string{}
+
+	// Flatten all JSON objects found, and return them
+	for _, objectFound := range objectsFound {
+
+		flat, err := flatten.Flatten(objectFound, "", flatten.DotStyle)
+		if err != nil {
+			return nil, err
+		}
+		params := map[string]string{}
+		for k, v := range flat {
+			params[k] = v.(string)
+		}
+		res = append(res, params)
 	}
 
-	return params, nil
+	return res, nil
+
 }
 
 func (g *GitGenerator) filterApps(Directories []argoprojiov1alpha1.GitDirectoryGeneratorItem, allApps []string) []string {


Test patch:
diff --git a/pkg/generators/git_test.go b/pkg/generators/git_test.go
index 8d383b2..f34bd33 100644
--- a/pkg/generators/git_test.go
+++ b/pkg/generators/git_test.go
@@ -146,11 +146,16 @@ func TestGitGenerateParamsFromDirectories(t *testing.T) {
 func TestGitGenerateParamsFromFiles(t *testing.T) {
 
 	cases := []struct {
-		name                   string
-		files                  []argoprojiov1alpha1.GitFileGeneratorItem
-		repoPaths              []string
-		repoFileContents       map[string][]byte
-		repoPathsError         error
+		name string
+		// files is the list of paths/globs to match
+		files []argoprojiov1alpha1.GitFileGeneratorItem
+		// repoPaths is the list of matching paths in the simulated git repository
+		repoPaths []string
+		// repoFileContents maps repo path to the literal contents of that path
+		repoFileContents map[string][]byte
+		// if repoPathsError is non-nil, the call to GetPaths(...) will return this error value
+		repoPathsError error
+		// if repoFileContentsErrors contains a path key, the error value will be returned on the call to GetFileContents(...)
 		repoFileContentsErrors map[string]error
 		expected               []map[string]string
 		expectedError          error
@@ -237,7 +242,66 @@ func TestGitGenerateParamsFromFiles(t *testing.T) {
 				"cluster-config/staging/config.json":    fmt.Errorf("staging config file get content error"),
 			},
 			expected:      []map[string]string{},
-			expectedError: fmt.Errorf("staging config file get content error"),
+			expectedError: fmt.Errorf("unable to process file 'cluster-config/staging/config.json': staging config file get content error"),
+		},
+		{
+			name:  "test invalid JSON file returns error",
+			files: []argoprojiov1alpha1.GitFileGeneratorItem{{Path: "**/config.json"}},
+			repoPaths: []string{
+				"cluster-config/production/config.json",
+			},
+			repoFileContents: map[string][]byte{
+				"cluster-config/production/config.json": []byte(`invalid json file`),
+			},
+			repoPathsError:         nil,
+			repoFileContentsErrors: map[string]error{},
+			expected:               []map[string]string{},
+			expectedError:          fmt.Errorf("unable to process file 'cluster-config/production/config.json': unable to parse JSON file: invalid character 'i' looking for beginning of value"),
+		},
+		{
+			name:  "test JSON array",
+			files: []argoprojiov1alpha1.GitFileGeneratorItem{{Path: "**/config.json"}},
+			repoPaths: []string{
+				"cluster-config/production/config.json",
+			},
+			repoFileContents: map[string][]byte{
+				"cluster-config/production/config.json": []byte(`
+				[
+					{
+						"cluster": {
+							"owner": "john.doe@example.com",
+							"name": "production",
+							"address": "https://kubernetes.default.svc",
+							"inner": {
+								"one" : "two"
+							}
+						}
+					},
+					{
+						"cluster": {
+							"owner": "john.doe@example.com",
+							"name": "staging",
+							"address": "https://kubernetes.default.svc"
+						}
+					}
+				]`),
+			},
+			repoPathsError:         nil,
+			repoFileContentsErrors: map[string]error{},
+			expected: []map[string]string{
+				{
+					"cluster.owner":     "john.doe@example.com",
+					"cluster.name":      "production",
+					"cluster.address":   "https://kubernetes.default.svc",
+					"cluster.inner.one": "two",
+				},
+				{
+					"cluster.owner":   "john.doe@example.com",
+					"cluster.name":    "staging",
+					"cluster.address": "https://kubernetes.default.svc",
+				},
+			},
+			expectedError: nil,
 		},
 	}
 
@@ -245,12 +309,12 @@ func TestGitGenerateParamsFromFiles(t *testing.T) {
 		cc := c
 		t.Run(cc.name, func(t *testing.T) {
 			argoCDServiceMock := argoCDServiceMock{mock: &mock.Mock{}}
-			argoCDServiceMock.mock.On("GetFilePaths", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(c.repoPaths, c.repoPathsError)
-
+			argoCDServiceMock.mock.On("GetFilePaths", mock.Anything, mock.Anything, mock.Anything, mock.Anything).
+				Return(c.repoPaths, c.repoPathsError)
 			if c.repoPaths != nil {
 				for _, repoPath := range c.repoPaths {
-					fmt.Println("repoPath: ", repoPath)
-					argoCDServiceMock.mock.On("GetFileContent", mock.Anything, mock.Anything, mock.Anything, repoPath).Return(c.repoFileContents[repoPath], c.repoFileContentsErrors[repoPath]).Once()
+					argoCDServiceMock.mock.On("GetFileContent", mock.Anything, mock.Anything, mock.Anything, repoPath).
+						Return(c.repoFileContents[repoPath], c.repoFileContentsErrors[repoPath]).Once()
 				}
 			}