From 21edd808b646d283f5a18573a9362d5ac023ca05 Mon Sep 17 00:00:00 2001 From: Sebastien Plisson Date: Tue, 30 Jan 2018 09:29:24 -0800 Subject: [PATCH 1/6] #3321 Accept dependency in requirements.yaml from charts directory. Signed-off-by: Sebastien Plisson --- pkg/downloader/manager.go | 91 +++++++++++++++++++++++++++++++++++++-- pkg/resolver/resolver.go | 13 ++++++ 2 files changed, 101 insertions(+), 3 deletions(-) diff --git a/pkg/downloader/manager.go b/pkg/downloader/manager.go index 23a5b587e..cc517424d 100644 --- a/pkg/downloader/manager.go +++ b/pkg/downloader/manager.go @@ -216,6 +216,30 @@ func (m *Manager) downloadAll(deps []*chartutil.Dependency) error { fmt.Fprintf(m.Out, "Saving %d charts\n", len(deps)) var saveError error for _, dep := range deps { + // No repository means the chart is in charts directory + if dep.Repository == "" { + chartPath := filepath.Join(tmpPath, dep.Name) + ch, err := chartutil.LoadDir(chartPath) + if err != nil { + return fmt.Errorf("Unable to load local chart: %v", err) + } + + constraint, err := semver.NewConstraint(dep.Version) + if err != nil { + return fmt.Errorf("Dependency %s has an invalid version/constraint format: %s", dep.Name, err) + } + + v, err := semver.NewVersion(ch.Metadata.Version) + if err != nil { + return fmt.Errorf("Chart %s has an invalid version format: %s", dep.Name, err) + } + + if !constraint.Check(v) { + saveError = fmt.Errorf("Can't get a valid version for dependency %s", dep.Name) + break + } + continue + } if strings.HasPrefix(dep.Repository, "file://") { if m.Debug { fmt.Fprintf(m.Out, "Archiving %s from repo %s\n", dep.Name, dep.Repository) @@ -258,8 +282,11 @@ func (m *Manager) downloadAll(deps []*chartutil.Dependency) error { if saveError == nil { fmt.Fprintln(m.Out, "Deleting outdated charts") for _, dep := range deps { - if err := m.safeDeleteDep(dep.Name, tmpPath); err != nil { - return err + // Chart from local charts directory stays in place + if dep.Repository != "" { + if err := m.safeDeleteDep(dep.Name, tmpPath); err != nil { + return err + } } } if err := move(tmpPath, destPath); err != nil { @@ -371,8 +398,9 @@ func (m *Manager) getRepoNames(deps []*chartutil.Dependency) (map[string]string, // by Helm. missing := []string{} for _, dd := range deps { + // Don't map the repository, we don't need to download chart from charts directory if dd.Repository == "" { - return nil, fmt.Errorf("no 'repository' field specified for dependency: %q", dd.Name) + continue } // if dep chart is from local path, verify the path is valid if strings.HasPrefix(dd.Repository, "file://") { @@ -663,3 +691,60 @@ func move(tmpPath, destPath string) error { } return nil } + +func copyFile(source string, destination string) (err error) { + sourceFile, err := os.Open(source) + if err != nil { + return err + } + defer sourceFile.Close() + destinationFile, err := os.Create(destination) + if err != nil { + return err + } + defer destinationFile.Close() + _, err = io.Copy(destinationFile, sourceFile) + if err == nil { + stats, err := os.Stat(source) + if err == nil { + return os.Chmod(destination, stats.Mode()) + } + } + return err +} + +func copyDir(source string, destination string) (err error) { + fi, err := os.Stat(source) + if err != nil { + return err + } + if !fi.IsDir() { + return fmt.Errorf("Source is not a directory") + } + _, err = os.Open(destination) + if !os.IsNotExist(err) { + return fmt.Errorf("Destination already exists") + } + err = os.MkdirAll(destination, fi.Mode()) + if err != nil { + return err + } + + entries, err := ioutil.ReadDir(source) + for _, entry := range entries { + sourceFile := source + "/" + entry.Name() + destinationFile := destination + "/" + entry.Name() + if entry.IsDir() { + err = copyDir(sourceFile, destinationFile) + if err != nil { + return err + } + } else { + err = copyFile(sourceFile, destinationFile) + if err != nil { + return err + } + } + } + return +} diff --git a/pkg/resolver/resolver.go b/pkg/resolver/resolver.go index 516e9260f..653606df0 100644 --- a/pkg/resolver/resolver.go +++ b/pkg/resolver/resolver.go @@ -53,6 +53,19 @@ func (r *Resolver) Resolve(reqs *chartutil.Requirements, repoNames map[string]st locked := make([]*chartutil.Dependency, len(reqs.Dependencies)) missing := []string{} for i, d := range reqs.Dependencies { + if d.Repository == "" { + // Local chart subfolder + if _, err := GetLocalPath(filepath.Join("charts", d.Name), r.chartpath); err != nil { + return nil, err + } + + locked[i] = &chartutil.Dependency{ + Name: d.Name, + Repository: "", + Version: d.Version, + } + continue + } if strings.HasPrefix(d.Repository, "file://") { if _, err := GetLocalPath(d.Repository, r.chartpath); err != nil { From 8537bb5c42177caeecfa622a35e798b864441fc4 Mon Sep 17 00:00:00 2001 From: Sebastien Plisson Date: Tue, 27 Feb 2018 14:08:51 -0800 Subject: [PATCH 2/6] Added test cases for chart present under charts folder Signed-off-by: Sebastien Plisson --- pkg/downloader/manager_test.go | 7 +++++ .../testdata/local-subchart/Chart.yaml | 3 +++ pkg/resolver/resolver_test.go | 27 +++++++++++++++++++ 3 files changed, 37 insertions(+) create mode 100644 pkg/downloader/testdata/local-subchart/Chart.yaml diff --git a/pkg/downloader/manager_test.go b/pkg/downloader/manager_test.go index ef8b95071..232c0a882 100644 --- a/pkg/downloader/manager_test.go +++ b/pkg/downloader/manager_test.go @@ -156,6 +156,13 @@ func TestGetRepoNames(t *testing.T) { }, expect: map[string]string{"oedipus-rex": "testing"}, }, + { + name: "repo from local chart under charts path", + req: []*chartutil.Dependency{ + {Name: "local-subchart", Repository: ""}, + }, + expect: map[string]string{}, + }, } for _, tt := range tests { diff --git a/pkg/downloader/testdata/local-subchart/Chart.yaml b/pkg/downloader/testdata/local-subchart/Chart.yaml new file mode 100644 index 000000000..1e17203e5 --- /dev/null +++ b/pkg/downloader/testdata/local-subchart/Chart.yaml @@ -0,0 +1,3 @@ +description: A Helm chart for Kubernetes +name: local-subchart +version: 0.1.0 diff --git a/pkg/resolver/resolver_test.go b/pkg/resolver/resolver_test.go index f35e051fa..567eb46d4 100644 --- a/pkg/resolver/resolver_test.go +++ b/pkg/resolver/resolver_test.go @@ -90,6 +90,33 @@ func TestResolve(t *testing.T) { }, err: true, }, + { + name: "repo from valid path under charts path", + req: &chartutil.Requirements{ + Dependencies: []*chartutil.Dependency{ + {Name: "localdependency", Repository: "", Version: "0.1.0"}, + }, + }, + expect: &chartutil.RequirementsLock{ + Dependencies: []*chartutil.Dependency{ + {Name: "localdependency", Repository: "", Version: "0.1.0"}, + }, + }, + }, + { + name: "repo from valid path under charts path", + req: &chartutil.Requirements{ + Dependencies: []*chartutil.Dependency{ + {Name: "inexistentdependency", Repository: "", Version: "0.1.0"}, + }, + }, + expect: &chartutil.RequirementsLock{ + Dependencies: []*chartutil.Dependency{ + {Name: "inexistentlocaldependency", Repository: "", Version: "0.1.0"}, + }, + }, + err: true, + }, } repoNames := map[string]string{"alpine": "kubernetes-charts", "redis": "kubernetes-charts"} From e8ab69ccc66da4efaad5708af0b206a5359e35d3 Mon Sep 17 00:00:00 2001 From: Sebastien Plisson Date: Tue, 27 Feb 2018 17:29:46 -0800 Subject: [PATCH 3/6] missing test file Signed-off-by: Sebastien Plisson --- .../testdata/chartpath/charts/localdependency/Chart.yaml | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 pkg/resolver/testdata/chartpath/charts/localdependency/Chart.yaml diff --git a/pkg/resolver/testdata/chartpath/charts/localdependency/Chart.yaml b/pkg/resolver/testdata/chartpath/charts/localdependency/Chart.yaml new file mode 100644 index 000000000..083c51ee5 --- /dev/null +++ b/pkg/resolver/testdata/chartpath/charts/localdependency/Chart.yaml @@ -0,0 +1,3 @@ +description: A Helm chart for Kubernetes +name: localdependency +version: 0.1.0 From 99b8a0d0d797803ea9627caf1af87837178891a3 Mon Sep 17 00:00:00 2001 From: Sebastien Plisson Date: Mon, 30 Sep 2019 15:11:51 -0700 Subject: [PATCH 4/6] Removed test requiring repository field to allow local chart dependency Signed-off-by: Sebastien Plisson --- pkg/downloader/manager_test.go | 8 -------- 1 file changed, 8 deletions(-) diff --git a/pkg/downloader/manager_test.go b/pkg/downloader/manager_test.go index 232c0a882..026823342 100644 --- a/pkg/downloader/manager_test.go +++ b/pkg/downloader/manager_test.go @@ -113,14 +113,6 @@ func TestGetRepoNames(t *testing.T) { }, err: true, }, - { - name: "dependency entry missing 'repository' field -- e.g. spelled 'repo'", - req: []*chartutil.Dependency{ - {Name: "dependency-missing-repository-field"}, - }, - err: true, - expectedErr: "no 'repository' field specified for dependency: \"dependency-missing-repository-field\"", - }, { name: "dependency repository is url but not exist in repos", req: []*chartutil.Dependency{ From a31d4ad43e785412fa7e69613422d1e9fc0623ab Mon Sep 17 00:00:00 2001 From: Matthew Fisher Date: Mon, 7 Oct 2019 14:19:30 -0700 Subject: [PATCH 5/6] fix: use nonexistent rather than inexistent Signed-off-by: Matthew Fisher --- pkg/resolver/resolver_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/resolver/resolver_test.go b/pkg/resolver/resolver_test.go index 567eb46d4..dca7a8899 100644 --- a/pkg/resolver/resolver_test.go +++ b/pkg/resolver/resolver_test.go @@ -107,12 +107,12 @@ func TestResolve(t *testing.T) { name: "repo from valid path under charts path", req: &chartutil.Requirements{ Dependencies: []*chartutil.Dependency{ - {Name: "inexistentdependency", Repository: "", Version: "0.1.0"}, + {Name: "nonexistentdependency", Repository: "", Version: "0.1.0"}, }, }, expect: &chartutil.RequirementsLock{ Dependencies: []*chartutil.Dependency{ - {Name: "inexistentlocaldependency", Repository: "", Version: "0.1.0"}, + {Name: "nonexistentlocaldependency", Repository: "", Version: "0.1.0"}, }, }, err: true, From 74653d3794617fab35976fea2e5e02269b7a6101 Mon Sep 17 00:00:00 2001 From: Matthew Fisher Date: Mon, 7 Oct 2019 14:41:44 -0700 Subject: [PATCH 6/6] fix: return more information to the user Signed-off-by: Matthew Fisher --- pkg/downloader/manager.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/pkg/downloader/manager.go b/pkg/downloader/manager.go index cc517424d..dc38cce05 100644 --- a/pkg/downloader/manager.go +++ b/pkg/downloader/manager.go @@ -218,10 +218,11 @@ func (m *Manager) downloadAll(deps []*chartutil.Dependency) error { for _, dep := range deps { // No repository means the chart is in charts directory if dep.Repository == "" { + fmt.Fprintf(m.Out, "Dependency %s did not declare a repository. Assuming it exists in the charts directory\n", dep.Name) chartPath := filepath.Join(tmpPath, dep.Name) ch, err := chartutil.LoadDir(chartPath) if err != nil { - return fmt.Errorf("Unable to load local chart: %v", err) + return fmt.Errorf("Unable to load chart: %v", err) } constraint, err := semver.NewConstraint(dep.Version) @@ -231,11 +232,11 @@ func (m *Manager) downloadAll(deps []*chartutil.Dependency) error { v, err := semver.NewVersion(ch.Metadata.Version) if err != nil { - return fmt.Errorf("Chart %s has an invalid version format: %s", dep.Name, err) + return fmt.Errorf("Invalid version %s for dependency %s: %s", dep.Version, dep.Name, err) } if !constraint.Check(v) { - saveError = fmt.Errorf("Can't get a valid version for dependency %s", dep.Name) + saveError = fmt.Errorf("Dependency %s at version %s does not satisfy the constraint %s", dep.Name, ch.Metadata.Version, dep.Version) break } continue