fix: Support source references in remote values

docs/user-guide/multiple_sources.md

@@ -55,17 +55,18 @@ spec:
     targetRevision: 15.7.1
     helm:
       valueFiles:
-      - $values/charts/prometheus/values.yaml
+      - '$(values)/charts/prometheus/values.yaml'
+      - 'secrets://$(values)/charts/prometheus/values.yaml'
   - repoURL: 'https://git.example.gom/org/value-files.git'
     targetRevision: dev
     ref: values
 ```
 
 In the above example, the `prometheus` chart will use the value file from `git.example.gom/org/value-files.git`. 
-`$values` resolves to the root of the `value-files` repository. The `$values` variable may only be specified at the 
-beginning of the value file path.
+`$(values)` resolves to the root of the `value-files` repository. The `$(values)` reference can be specified anywhere at the value file path.
+To prevent the evaluation inside of `$(values)` inside URL, you have to url-encode the string, e.g. `%24%28values%29`
 
-If the `path` field is set in the `$values` source, Argo CD will attempt to generate resources from the git repository
+If the `path` field is set in the `$(values)` source, Argo CD will attempt to generate resources from the git repository
 at that URL. If the `path` field is not set, Argo CD will use the repository solely as a source of value files.
 
 !!! note

reposerver/repository/repository.go

@@ -1118,10 +1118,16 @@ func getResolvedValueFiles(
 		var resolvedPath pathutil.ResolvedFilePath
 		var err error
 
-		referencedSource := getReferencedSource(rawValueFile, refSources)
+		regexSourceReference, _ := regexp.Compile(`\$\(([a-z]+)\)(?:/|$)`)
+
+		referencedSource, err := getReferencedSource(rawValueFile, refSources, regexSourceReference)
+		if err != nil {
+			return nil, err
+		}
+
 		if referencedSource != nil {
 			// If the $-prefixed path appears to reference another source, do env substitution _after_ resolving that source.
-			resolvedPath, err = getResolvedRefValueFile(rawValueFile, env, allowedValueFilesSchemas, referencedSource.Repo.Repo, gitRepoPaths)
+			resolvedPath, err = getResolvedRefValueFile(rawValueFile, env, allowedValueFilesSchemas, referencedSource.Repo.Repo, gitRepoPaths, regexSourceReference)
 			if err != nil {
 				return nil, err
 			}
@@ -1154,14 +1160,15 @@ func getResolvedRefValueFile(
 	allowedValueFilesSchemas []string,
 	refSourceRepo string,
 	gitRepoPaths io.TempPaths,
+	regexSourceReference *regexp.Regexp,
 ) (pathutil.ResolvedFilePath, error) {
-	pathStrings := strings.Split(rawValueFile, "/")
+	matched := regexSourceReference.FindStringSubmatch(rawValueFile)
+
 	repoPath := gitRepoPaths.GetPathIfExists(git.NormalizeGitURL(refSourceRepo))
 	if repoPath == "" {
 		return "", fmt.Errorf("failed to find repo %q", refSourceRepo)
 	}
-	pathStrings[0] = "" // Remove first segment. It will be inserted by pathutil.ResolveValueFilePathOrUrl.
-	substitutedPath := strings.Join(pathStrings, "/")
+	substitutedPath := strings.ReplaceAll(rawValueFile, matched[0], repoPath+"/")
 
 	// Resolve the path relative to the referenced repo and block any attempt at traversal.
 	resolvedPath, _, err := pathutil.ResolveValueFilePathOrUrl(repoPath, repoPath, env.Envsubst(substitutedPath), allowedValueFilesSchemas)
@@ -1171,13 +1178,19 @@ func getResolvedRefValueFile(
 	return resolvedPath, nil
 }
 
-func getReferencedSource(rawValueFile string, refSources map[string]*v1alpha1.RefTarget) *v1alpha1.RefTarget {
-	if !strings.HasPrefix(rawValueFile, "$") {
-		return nil
+func getReferencedSource(rawValueFile string, refSources map[string]*v1alpha1.RefTarget, regexSourceReference *regexp.Regexp) (*v1alpha1.RefTarget, error) {
+	matched := regexSourceReference.FindStringSubmatch(rawValueFile)
+
+	if matched == nil {
+		return nil, nil
+	}
+
+	refVar := matched[1]
+	referencedSource, ok := refSources[refVar]
+	if !ok {
+		return nil, fmt.Errorf("source ref '%q' does not exists", refVar)
 	}
-	refVar := strings.Split(rawValueFile, "/")[0]
-	referencedSource := refSources[refVar]
-	return referencedSource
+	return referencedSource, nil
 }
 
 func getRepoCredential(repoCredentials []*v1alpha1.RepoCreds, repoURL string) *v1alpha1.RepoCreds {

reposerver/repository/repository_test.go

@@ -301,7 +301,7 @@ func TestHelmChartReferencingExternalValues(t *testing.T) {
 	spec := argoappv1.ApplicationSpec{
 		Sources: []argoappv1.ApplicationSource{
 			{RepoURL: "https://helm.example.com", Chart: "my-chart", TargetRevision: ">= 1.0.0", Helm: &argoappv1.ApplicationSourceHelm{
-				ValueFiles: []string{"$ref/testdata/my-chart/my-chart-values.yaml"},
+				ValueFiles: []string{"$(ref)/testdata/my-chart/my-chart-values.yaml"},
 			}},
 			{Ref: "ref", RepoURL: "https://git.example.com/test/repo"},
 		},
@@ -343,7 +343,7 @@ func TestHelmChartReferencingExternalValues_OutOfBounds_Symlink(t *testing.T) {
 			{RepoURL: "https://helm.example.com", Chart: "my-chart", TargetRevision: ">= 1.0.0", Helm: &argoappv1.ApplicationSourceHelm{
 				// Reference `ref` but do not use the oob symlink. The mere existence of the link should be enough to
 				// cause an error.
-				ValueFiles: []string{"$ref/testdata/oob-symlink/values.yaml"},
+				ValueFiles: []string{"$(ref)/testdata/oob-symlink/values.yaml"},
 			}},
 			{Ref: "ref", RepoURL: "https://git.example.com/test/repo"},
 		},
@@ -2639,23 +2639,49 @@ func Test_getResolvedValueFiles(t *testing.T) {
 		},
 		{
 			name:    "simple ref",
-			rawPath: "$ref/values.yaml",
+			rawPath: "$(ref)/values.yaml",
 			env:     &argoappv1.Env{},
 			refSources: map[string]*argoappv1.RefTarget{
-				"$ref": {
+				"ref": {
 					Repo: argoappv1.Repository{
 						Repo: "https://github.com/org/repo1",
 					},
 				},
 			},
 			expectedPath: path.Join(tempDir, "repo1", "values.yaml"),
 		},
+		{
+			name:    "url ref",
+			rawPath: "https://$(ref)/values.yaml",
+			env:     &argoappv1.Env{},
+			refSources: map[string]*argoappv1.RefTarget{
+				"ref": {
+					Repo: argoappv1.Repository{
+						Repo: "https://github.com/org/repo1",
+					},
+				},
+			},
+			expectedPath: "https://" + path.Join(tempDir, "repo1", "values.yaml"),
+		},
+		{
+			name:    "multiple ref",
+			rawPath: "https://$(ref)/values.yaml?$(ref)/other-values.yaml",
+			env:     &argoappv1.Env{},
+			refSources: map[string]*argoappv1.RefTarget{
+				"ref": {
+					Repo: argoappv1.Repository{
+						Repo: "https://github.com/org/repo1",
+					},
+				},
+			},
+			expectedPath: "https://" + path.Join(tempDir, "repo1", "values.yaml") + "?" + path.Join(tempDir, "repo1", "other-values.yaml"),
+		},
 		{
 			name:    "only ref",
-			rawPath: "$ref",
+			rawPath: "$(ref)",
 			env:     &argoappv1.Env{},
 			refSources: map[string]*argoappv1.RefTarget{
-				"$ref": {
+				"ref": {
 					Repo: argoappv1.Repository{
 						Repo: "https://github.com/org/repo1",
 					},
@@ -2665,10 +2691,10 @@ func Test_getResolvedValueFiles(t *testing.T) {
 		},
 		{
 			name:    "attempted traversal",
-			rawPath: "$ref/../values.yaml",
+			rawPath: "$(ref)/../values.yaml",
 			env:     &argoappv1.Env{},
 			refSources: map[string]*argoappv1.RefTarget{
-				"$ref": {
+				"ref": {
 					Repo: argoappv1.Repository{
 						Repo: "https://github.com/org/repo1",
 					},
@@ -2677,21 +2703,18 @@ func Test_getResolvedValueFiles(t *testing.T) {
 			expectedErr: true,
 		},
 		{
-			// Since $ref doesn't resolve to a ref target, we assume it's an env var. Since the env var isn't specified,
-			// it's replaced with an empty string. This is necessary for backwards compatibility with behavior before
-			// ref targets were introduced.
-			name:         "ref doesn't exist",
-			rawPath:      "$ref/values.yaml",
-			env:          &argoappv1.Env{},
-			refSources:   map[string]*argoappv1.RefTarget{},
-			expectedPath: path.Join(tempDir, "main-repo", "values.yaml"),
+			name:        "ref doesn't exist",
+			rawPath:     "$(ref)/values.yaml",
+			env:         &argoappv1.Env{},
+			refSources:  map[string]*argoappv1.RefTarget{},
+			expectedErr: true,
 		},
 		{
 			name:    "repo doesn't exist",
-			rawPath: "$ref/values.yaml",
+			rawPath: "$(ref)/values.yaml",
 			env:     &argoappv1.Env{},
 			refSources: map[string]*argoappv1.RefTarget{
-				"$ref": {
+				"ref": {
 					Repo: argoappv1.Repository{
 						Repo: "https://github.com/org/repo2",
 					},
@@ -2701,15 +2724,15 @@ func Test_getResolvedValueFiles(t *testing.T) {
 		},
 		{
 			name:    "env var is resolved",
-			rawPath: "$ref/$APP_PATH/values.yaml",
+			rawPath: "$(ref)/$APP_PATH/values.yaml",
 			env: &argoappv1.Env{
 				&argoappv1.EnvEntry{
 					Name:  "APP_PATH",
 					Value: "app-path",
 				},
 			},
 			refSources: map[string]*argoappv1.RefTarget{
-				"$ref": {
+				"ref": {
 					Repo: argoappv1.Repository{
 						Repo: "https://github.com/org/repo1",
 					},
@@ -2719,15 +2742,15 @@ func Test_getResolvedValueFiles(t *testing.T) {
 		},
 		{
 			name:    "traversal in env var is blocked",
-			rawPath: "$ref/$APP_PATH/values.yaml",
+			rawPath: "$(ref)/$APP_PATH/values.yaml",
 			env: &argoappv1.Env{
 				&argoappv1.EnvEntry{
 					Name:  "APP_PATH",
 					Value: "..",
 				},
 			},
 			refSources: map[string]*argoappv1.RefTarget{
-				"$ref": {
+				"ref": {
 					Repo: argoappv1.Repository{
 						Repo: "https://github.com/org/repo1",
 					},
@@ -2760,7 +2783,7 @@ func Test_getResolvedValueFiles(t *testing.T) {
 		tcc := tc
 		t.Run(tcc.name, func(t *testing.T) {
 			t.Parallel()
-			resolvedPaths, err := getResolvedValueFiles(path.Join(tempDir, "main-repo"), path.Join(tempDir, "main-repo"), tcc.env, []string{}, []string{tcc.rawPath}, tcc.refSources, paths, false)
+			resolvedPaths, err := getResolvedValueFiles(path.Join(tempDir, "main-repo"), path.Join(tempDir, "main-repo"), tcc.env, []string{"https"}, []string{tcc.rawPath}, tcc.refSources, paths, false)
 			if !tcc.expectedErr {
 				assert.NoError(t, err)
 				require.Len(t, resolvedPaths, 1)

util/io/path/resolved.go

@@ -148,7 +148,7 @@ func resolveFileOrDirectory(appPath string, repoRoot string, fileOrDirectory str
 			return "", resolveFailure(repoRoot, err)
 		}
 		path = filepath.Join(absWorkDir, path)
-	} else {
+	} else if !strings.HasPrefix(path, absRepoPath) {
 		path = filepath.Join(absRepoPath, path)
 	}