Skip to content

Commit

Permalink
fix: loosen source not permitted helm errors
Browse files Browse the repository at this point in the history
With argoproj#12255, we check if a source is first permitted before running
`helm template`. This works a bit too well, since this may break
previously working manifests. If an `AppProject` has a set of
`sourceRepos` which are more restrictive than `*`, and it also has Helm
public dependencies (repos with credentials would not work with 2.7x
due to the fact they get filtered out before ending up on the repo
server). Whereas before this would work, this currently fails on
`HEAD` but not in `2.7x`.

What we instead do here is that we only run this check if the chart
failed to download - if it does then we run a check to see if the repo
is in the allowed repos list. If the repo is not in the allowed repos
list, we return the same error as in argoproj#12555, otherwise we bubble up the
error.

Should fix argoproj#13833.

Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>
  • Loading branch information
blakepettersson committed Aug 9, 2023
1 parent 20a1649 commit 1e570b0
Showing 1 changed file with 22 additions and 3 deletions.
25 changes: 22 additions & 3 deletions reposerver/repository/repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -1048,9 +1048,9 @@ func runHelmBuild(appPath string, h helm.Helm) error {
manifestGenerateLock.Lock(appPath)
defer manifestGenerateLock.Unlock(appPath)

// the `helm dependency build` is potentially time consuming 1~2 seconds
// marker file is used to check if command already run to avoid running it again unnecessary
// file is removed when repository re-initialized (e.g. when another commit is processed)
// the `helm dependency build` is potentially a time-consuming 1~2 seconds,
// a marker file is used to check if command already run to avoid running it again unnecessarily
// the file is removed when repository is re-initialized (e.g. when another commit is processed)
markerFile := path.Join(appPath, helmDepUpMarkerFile)
_, err := os.Stat(markerFile)
if err == nil {
Expand All @@ -1066,6 +1066,11 @@ func runHelmBuild(appPath string, h helm.Helm) error {
return os.WriteFile(markerFile, []byte("marker"), 0644)
}

func isSourcePermitted(url string, repos []string) bool {
p := v1alpha1.AppProject{Spec: v1alpha1.AppProjectSpec{SourceRepos: repos}}
return p.IsSourcePermitted(v1alpha1.ApplicationSource{RepoURL: url})
}

func helmTemplate(appPath string, repoRoot string, env *v1alpha1.Env, q *apiclient.ManifestRequest, isLocal bool, gitRepoPaths io.TempPaths) ([]*unstructured.Unstructured, error) {
concurrencyAllowed := isConcurrencyAllowed(appPath)
if !concurrencyAllowed {
Expand Down Expand Up @@ -1186,6 +1191,20 @@ func helmTemplate(appPath string, repoRoot string, env *v1alpha1.Env, q *apiclie
}

if err != nil {
var reposNotPermitted []string
// We do a sanity check here to give a nicer error message in case any of the Helm repositories are not permitted by
// the AppProject which the application is a part of
for _, repo := range helmRepos {
match := regexp.MustCompile(fmt.Sprintf("could not download (oci|https?)://%s", repo.Repo))
if match.MatchString(err.Error()) && !isSourcePermitted(repo.Repo, q.ProjectSourceRepos) {
reposNotPermitted = append(reposNotPermitted, repo.Repo)
}
}

if len(reposNotPermitted) > 0 {
return nil, status.Errorf(codes.PermissionDenied, "helm repos %s are not permitted in project '%s'", strings.Join(reposNotPermitted, ", "), q.ProjectName)
}

return nil, err
}

Expand Down

0 comments on commit 1e570b0

Please sign in to comment.