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: implement source refs for helm set-file (Beta) (#13220) #17941

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 0 additions & 3 deletions docs/user-guide/helm.md
Original file line number Diff line number Diff line change
Expand Up @@ -236,9 +236,6 @@ source:
value: path/to/file.ext
```

!!! warning "Reference in multiple sources not supported"
Please note that using a multiple sources application will not let you load the file by reference. See [argoproj/argo-cd#13220](https://github.com/argoproj/argo-cd/issues/13220)

## Helm Release Name

By default, the Helm release name is equal to the Application name to which it belongs. Sometimes, especially on a centralised Argo CD,
Expand Down
32 changes: 27 additions & 5 deletions reposerver/repository/repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -472,7 +472,13 @@ func resolveReferencedSources(hasMultipleSources bool, source *v1alpha1.Applicat
return repoRefs, nil
}

for _, valueFile := range source.ValueFiles {
refFileParams := make([]string, 0)
for _, fileParam := range source.FileParameters {
refFileParams = append(refFileParams, fileParam.Path)
}
refCandidates := append(source.ValueFiles, refFileParams...)

for _, valueFile := range refCandidates {
if strings.HasPrefix(valueFile, "$") {
refVar := strings.Split(valueFile, "/")[0]

Expand Down Expand Up @@ -710,8 +716,14 @@ func (s *Service) runManifestGenAsync(ctx context.Context, repoRoot, commitSHA,
// check whether they should be replicated in resolveReferencedSources.
if q.HasMultipleSources {
if q.ApplicationSource.Helm != nil {
refFileParams := make([]string, 0)
for _, fileParam := range q.ApplicationSource.Helm.FileParameters {
refFileParams = append(refFileParams, fileParam.Path)
}
refCandidates := append(q.ApplicationSource.Helm.ValueFiles, refFileParams...)

// Checkout every one of the referenced sources to the target revision before generating Manifests
for _, valueFile := range q.ApplicationSource.Helm.ValueFiles {
for _, valueFile := range refCandidates {
if strings.HasPrefix(valueFile, "$") {
refVar := strings.Split(valueFile, "/")[0]

Expand Down Expand Up @@ -1158,9 +1170,19 @@ func helmTemplate(appPath string, repoRoot string, env *v1alpha1.Env, q *apiclie
}
}
for _, p := range appHelm.FileParameters {
resolvedPath, _, err := pathutil.ResolveValueFilePathOrUrl(appPath, repoRoot, env.Envsubst(p.Path), q.GetValuesFileSchemes())
if err != nil {
return nil, fmt.Errorf("error resolving helm value file path: %w", err)
var resolvedPath pathutil.ResolvedFilePath
referencedSource := getReferencedSource(p.Path, q.RefSources)
if referencedSource != nil {
// If the $-prefixed path appears to reference another source, do env substitution _after_ resolving the source
resolvedPath, err = getResolvedRefValueFile(p.Path, env, q.GetValuesFileSchemes(), referencedSource.Repo.Repo, gitRepoPaths, referencedSource.Repo.Project)
if err != nil {
return nil, fmt.Errorf("error resolving set-file path: %w", err)
}
} else {
resolvedPath, _, err = pathutil.ResolveValueFilePathOrUrl(appPath, repoRoot, env.Envsubst(p.Path), q.GetValuesFileSchemes())
if err != nil {
return nil, fmt.Errorf("error resolving helm value file path: %w", err)
}
}
templateOpts.SetFile[p.Name] = resolvedPath
}
Expand Down
67 changes: 67 additions & 0 deletions reposerver/repository/repository_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"path"
"path/filepath"
"regexp"
"slices"
"sort"
"strings"
"sync"
Expand Down Expand Up @@ -2235,6 +2236,72 @@ func TestGenerateManifestWithAnnotatedTagsAndMultiSourceApp(t *testing.T) {
}
}

func TestGenerateMultiSourceHelmWithFileParameter(t *testing.T) {
expectedFileContent, err := os.ReadFile("../../util/helm/testdata/external/external-secret.txt")
assert.NoError(t, err)

service := newService(t, "../../util/helm/testdata")

testCases := []struct {
name string
refSources map[string]*argoappv1.RefTarget
expectedContent string
expectedErr bool
}{{
name: "Successfully resolve multi-source ref for helm set-file",
refSources: map[string]*argoappv1.RefTarget{
"$global": {
TargetRevision: "HEAD",
},
},
expectedContent: string(expectedFileContent),
expectedErr: false,
}, {
name: "Failed to resolve multi-source ref for helm set-file",
refSources: map[string]*argoappv1.RefTarget{},
expectedContent: "DOES-NOT-EXIST",
expectedErr: true,
}}

for i := range testCases {
tc := testCases[i]
t.Run(tc.name, func(t *testing.T) {
manifestRequest := &apiclient.ManifestRequest{
Repo: &argoappv1.Repository{},
ApplicationSource: &argoappv1.ApplicationSource{
Ref: "$global",
Path: "./redis",
TargetRevision: "HEAD",
Helm: &argoappv1.ApplicationSourceHelm{
ValueFiles: []string{"$global/redis/values-production.yaml"},
FileParameters: []argoappv1.HelmFileParameter{{
Name: "passwordContent",
Path: "$global/external/external-secret.txt",
}},
},
},
HasMultipleSources: true,
NoCache: true,
RefSources: tc.refSources,
}

res, err := service.GenerateManifest(context.Background(), manifestRequest)

if !tc.expectedErr {
assert.NoError(t, err)

// Check that any of the manifests contains the secret
idx := slices.IndexFunc(res.Manifests, func(content string) bool {
return strings.Contains(content, tc.expectedContent)
})
assert.GreaterOrEqual(t, idx, 0, "No manifest contains the value set with the helm fileParameters")
} else {
assert.Error(t, err)
}
})
}
}

func TestFindResources(t *testing.T) {
testCases := []struct {
name string
Expand Down