Skip to content

Commit

Permalink
Fix SIGSEGV when resolving charts dependencies
Browse files Browse the repository at this point in the history
If implemented, this make sure than we clear only referenced
downloaders.

It is also checked if the repository url is supported.

Signed-off-by: Soule BA <[email protected]>
  • Loading branch information
souleb committed Jul 13, 2022
1 parent 22c9e2e commit 6eb7c1a
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 1 deletion.
9 changes: 8 additions & 1 deletion internal/helm/chart/dependency_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,9 @@ func NewDependencyManager(opts ...DependencyManagerOption) *DependencyManager {
func (dm *DependencyManager) Clear() error {
var errs []error
for _, v := range dm.downloaders {
errs = append(errs, v.Clear())
if v != nil {
errs = append(errs, v.Clear())
}
}
return errors.NewAggregate(errs)
}
Expand Down Expand Up @@ -257,6 +259,11 @@ func (dm *DependencyManager) resolveRepository(url string) (repo repository.Down
defer dm.mu.Unlock()

nUrl := repository.NormalizeURL(url)
supported := repository.IsDepSupported(nUrl)
if !supported {
err = fmt.Errorf("unsupported chart repository URL: %s", nUrl)
return
}
if _, ok := dm.downloaders[nUrl]; !ok {
if dm.getChartDownloaderCallback == nil {
err = fmt.Errorf("no chart repository for URL '%s'", nUrl)
Expand Down
9 changes: 9 additions & 0 deletions internal/helm/chart/dependency_manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ func TestDependencyManager_Clear(t *testing.T) {
},
"with credentials": ociRepoWithCreds,
"without credentials": &repository.OCIChartRepository{},
"nil downloader": nil,
}

dm := NewDependencyManager(WithRepositories(downloaders))
Expand Down Expand Up @@ -428,6 +429,14 @@ func TestDependencyManager_addRemoteDependency(t *testing.T) {
},
wantErr: "no chart repository for URL",
},
{
name: "resolve aliased repository error",
downloaders: map[string]repository.Downloader{},
dep: &helmchart.Dependency{
Repository: "@fantastic-charts",
},
wantErr: "unsupported chart repository URL",
},
{
name: "strategic load error",
downloaders: map[string]repository.Downloader{
Expand Down
14 changes: 14 additions & 0 deletions internal/helm/repository/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,17 @@ func NormalizeURL(repositoryURL string) string {
return strings.TrimRight(repositoryURL, "/") + "/"

}

// IsDepSupported returns true if the given depended repository URL declaration is supported
// The reason for this is that the dependency manager will not be able to resolve the alias declaration
// e.g. repository: "@fantastic-charts"
func IsDepSupported(repositoryURL string) bool {
switch {
case strings.Contains(repositoryURL, helmreg.OCIScheme):
return true
case strings.Contains(repositoryURL, "https://") || strings.Contains(repositoryURL, "http://"):
return true
default:
return false
}
}

0 comments on commit 6eb7c1a

Please sign in to comment.