From d72ff653257ecc8214f781231cadc3b173b7dfc2 Mon Sep 17 00:00:00 2001 From: Qin Wang Date: Thu, 9 Feb 2017 19:43:18 +0000 Subject: [PATCH 1/4] feat(helm): add local path support for deps in requirements.yaml The following commands: helm dep update helm dep build are now able to take a requirements.yaml with dependency charts' repo defined as: file://../local/path or file:///root/path closes: #1884 --- cmd/helm/dependency.go | 2 +- pkg/downloader/manager.go | 69 +++++++++++++++++++++++++++++++++- pkg/downloader/manager_test.go | 7 ++++ pkg/resolver/resolver.go | 19 ++++++++++ pkg/resolver/resolver_test.go | 9 +++++ 5 files changed, 104 insertions(+), 2 deletions(-) diff --git a/cmd/helm/dependency.go b/cmd/helm/dependency.go index b22c3926e..fe1cde114 100644 --- a/cmd/helm/dependency.go +++ b/cmd/helm/dependency.go @@ -127,7 +127,7 @@ func (l *dependencyListCmd) run() error { r, err := chartutil.LoadRequirements(c) if err != nil { if err == chartutil.ErrRequirementsNotFound { - fmt.Fprintf(l.out, "WARNING: no requirements at %s/charts", l.chartpath) + fmt.Fprintf(l.out, "WARNING: no requirements at %s/charts\n", l.chartpath) return nil } return err diff --git a/pkg/downloader/manager.go b/pkg/downloader/manager.go index a6106796d..60b8518ff 100644 --- a/pkg/downloader/manager.go +++ b/pkg/downloader/manager.go @@ -116,7 +116,6 @@ func (m *Manager) Update() error { } return err } - // Check that all of the repos we're dependent on actually exist and // the repo index names. repoNames, err := m.getRepoNames(req.Dependencies) @@ -201,6 +200,17 @@ func (m *Manager) downloadAll(deps []*chartutil.Dependency) error { if err := m.safeDeleteDep(dep.Name, destPath); err != nil { return err } + + if strings.HasPrefix(dep.Repository, "file://") { + fmt.Fprintf(m.Out, "Archiving %s from repo %s\n", dep.Name, dep.Repository) + ver, err := tarFromLocalDir(m.ChartPath, dep.Name, dep.Repository, dep.Version) + if err != nil { + return err + } + dep.Version = ver + continue + } + fmt.Fprintf(m.Out, "Downloading %s from repo %s\n", dep.Name, dep.Repository) // Any failure to resolve/download a chart should fail: @@ -257,10 +267,16 @@ func (m *Manager) hasAllRepos(deps []*chartutil.Dependency) error { return err } repos := rf.Repositories + // Verify that all repositories referenced in the deps are actually known // by Helm. missing := []string{} for _, dd := range deps { + // If repo is from local path, continue + if strings.HasPrefix(dd.Repository, "file://") { + continue + } + found := false if dd.Repository == "" { found = true @@ -295,6 +311,22 @@ func (m *Manager) getRepoNames(deps []*chartutil.Dependency) (map[string]string, // by Helm. missing := []string{} for _, dd := range deps { + // if dep chart is from local path, verify the path is valid + if strings.HasPrefix(dd.Repository, "file://") { + depPath, err := filepath.Abs(dd.Repository[7:]) + if err != nil { + return nil, err + } + + if _, err = os.Stat(depPath); os.IsNotExist(err) { + return nil, fmt.Errorf("directory %s not found: %s", depPath, err) + } + + fmt.Fprintf(m.Out, "Repository from local path: %s\n", dd.Repository) + reposMap[dd.Name] = dd.Repository + continue + } + found := false for _, repo := range repos { @@ -480,3 +512,38 @@ func writeLock(chartpath string, lock *chartutil.RequirementsLock) error { dest := filepath.Join(chartpath, "requirements.lock") return ioutil.WriteFile(dest, data, 0644) } + +// archive a dep chart from local directory and save it into charts/ +func tarFromLocalDir(chartpath string, name string, repo string, version string) (string, error) { + destPath := filepath.Join(chartpath, "charts") + origPath, err := filepath.Abs(repo[7:]) + if err != nil { + return "", err + } + + if _, err = os.Stat(origPath); os.IsNotExist(err) { + return "", fmt.Errorf("directory %s not found: %s", origPath, err) + } + + ch, err := chartutil.LoadDir(origPath) + if err != nil { + return "", err + } + + constraint, err := semver.NewConstraint(version) + if err != nil { + return "", fmt.Errorf("dependency %s has an invalid version/constraint format: %s", name, err) + } + + v, err := semver.NewVersion(ch.Metadata.Version) + if err != nil { + return "", err + } + + if constraint.Check(v) { + _, err = chartutil.Save(ch, destPath) + return ch.Metadata.Version, err + } + + return "", fmt.Errorf("Can't get a valid version for dependency %s.", name) +} diff --git a/pkg/downloader/manager_test.go b/pkg/downloader/manager_test.go index 7224ccc41..4b15e6e09 100644 --- a/pkg/downloader/manager_test.go +++ b/pkg/downloader/manager_test.go @@ -113,6 +113,13 @@ func TestGetRepoNames(t *testing.T) { }, expect: map[string]string{"oedipus-rex": "testing"}, }, + { + name: "repo from local path", + req: []*chartutil.Dependency{ + {Name: "local-dep", Repository: "file://./testdata/signtest"}, + }, + expect: map[string]string{"local-dep": "file://./testdata/signtest"}, + }, } for _, tt := range tests { diff --git a/pkg/resolver/resolver.go b/pkg/resolver/resolver.go index 480cea674..294ff33e7 100644 --- a/pkg/resolver/resolver.go +++ b/pkg/resolver/resolver.go @@ -19,6 +19,8 @@ import ( "bytes" "encoding/json" "fmt" + "os" + "path/filepath" "strings" "time" @@ -55,6 +57,23 @@ 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 strings.HasPrefix(d.Repository, "file://") { + depPath, err := filepath.Abs(d.Repository[7:]) + if err != nil { + return nil, err + } + + if _, err = os.Stat(depPath); os.IsNotExist(err) { + return nil, fmt.Errorf("directory %s not found", depPath) + } + + locked[i] = &chartutil.Dependency{ + Name: d.Name, + Repository: d.Repository, + Version: d.Version, + } + continue + } constraint, err := semver.NewConstraint(d.Version) if err != nil { return nil, fmt.Errorf("dependency %q has an invalid version/constraint format: %s", d.Name, err) diff --git a/pkg/resolver/resolver_test.go b/pkg/resolver/resolver_test.go index 1f5409985..c284597a1 100644 --- a/pkg/resolver/resolver_test.go +++ b/pkg/resolver/resolver_test.go @@ -77,6 +77,15 @@ func TestResolve(t *testing.T) { }, }, }, + { + name: "repo from local invalid path", + req: &chartutil.Requirements{ + Dependencies: []*chartutil.Dependency{ + {Name: "notexist", Repository: "file://../testdata/notexist", Version: "0.1.0"}, + }, + }, + err: true, + }, } repoNames := map[string]string{"alpine": "kubernetes-charts", "redis": "kubernetes-charts"} From 39a2d5ec29caaa10418e34bf8ef1699530cdcd01 Mon Sep 17 00:00:00 2001 From: Qin Wang Date: Fri, 10 Feb 2017 00:40:27 +0000 Subject: [PATCH 2/4] feat(helm): add local path support for deps in requirements.yaml fix change requests --- docs/helm/helm_dependency.md | 10 ++++++++++ pkg/downloader/manager.go | 4 ++-- pkg/resolver/resolver.go | 2 +- pkg/resolver/resolver_test.go | 15 ++++++++++++++- 4 files changed, 27 insertions(+), 4 deletions(-) diff --git a/docs/helm/helm_dependency.md b/docs/helm/helm_dependency.md index 4b05101f8..a3fc9d60d 100644 --- a/docs/helm/helm_dependency.md +++ b/docs/helm/helm_dependency.md @@ -39,6 +39,16 @@ appending '/index.yaml' to the URL, it should be able to retrieve the chart repository's index. Note: 'repository' cannot be a repository alias. It must be a URL. +Starting from 2.2.0, repository can be defined as the path to the directory of +the dependency charts stored locally. The path should start with a prefix of "file://". +For example, + # requirements.yaml + dependencies: + - name: nginx + version: "1.2.3" + repository: "file://../depedency_chart/nginx" +If the dependency chart is retrieved locally, it is not required to have the repository +added to helm by "helm add repo". Version matching is also supported for this case. ### Options inherited from parent commands diff --git a/pkg/downloader/manager.go b/pkg/downloader/manager.go index 60b8518ff..dc6508d42 100644 --- a/pkg/downloader/manager.go +++ b/pkg/downloader/manager.go @@ -313,13 +313,13 @@ func (m *Manager) getRepoNames(deps []*chartutil.Dependency) (map[string]string, for _, dd := range deps { // if dep chart is from local path, verify the path is valid if strings.HasPrefix(dd.Repository, "file://") { - depPath, err := filepath.Abs(dd.Repository[7:]) + depPath, err := filepath.Abs(strings.TrimPrefix(dd.Repository, "file://")) if err != nil { return nil, err } if _, err = os.Stat(depPath); os.IsNotExist(err) { - return nil, fmt.Errorf("directory %s not found: %s", depPath, err) + return nil, fmt.Errorf("directory %s not found", depPath) } fmt.Fprintf(m.Out, "Repository from local path: %s\n", dd.Repository) diff --git a/pkg/resolver/resolver.go b/pkg/resolver/resolver.go index 294ff33e7..b0b2000fb 100644 --- a/pkg/resolver/resolver.go +++ b/pkg/resolver/resolver.go @@ -58,7 +58,7 @@ func (r *Resolver) Resolve(reqs *chartutil.Requirements, repoNames map[string]st missing := []string{} for i, d := range reqs.Dependencies { if strings.HasPrefix(d.Repository, "file://") { - depPath, err := filepath.Abs(d.Repository[7:]) + depPath, err := filepath.Abs(strings.TrimPrefix(d.Repository, "file://")) if err != nil { return nil, err } diff --git a/pkg/resolver/resolver_test.go b/pkg/resolver/resolver_test.go index c284597a1..b4b77794f 100644 --- a/pkg/resolver/resolver_test.go +++ b/pkg/resolver/resolver_test.go @@ -78,7 +78,20 @@ func TestResolve(t *testing.T) { }, }, { - name: "repo from local invalid path", + name: "repo from valid local path", + req: &chartutil.Requirements{ + Dependencies: []*chartutil.Dependency{ + {Name: "signtest", Repository: "file://../testdata/testcharts/signtest", Version: "0.1.0"}, + }, + }, + expect: &chartutil.RequirementsLock{ + Dependencies: []*chartutil.Dependency{ + {Name: "signtest", Repository: "file://../testdata/testcharts/signtest", Version: "0.1.0"}, + }, + }, + }, + { + name: "repo from invalid local path", req: &chartutil.Requirements{ Dependencies: []*chartutil.Dependency{ {Name: "notexist", Repository: "file://../testdata/notexist", Version: "0.1.0"}, From 0153d273ce8e14c7567c3562138b26450f29ea85 Mon Sep 17 00:00:00 2001 From: Qin Wang Date: Fri, 10 Feb 2017 19:16:28 +0000 Subject: [PATCH 3/4] fix error checking from os.stat --- pkg/downloader/manager.go | 11 ++++++++++- pkg/resolver/resolver.go | 2 ++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/pkg/downloader/manager.go b/pkg/downloader/manager.go index dc6508d42..9a830c4d5 100644 --- a/pkg/downloader/manager.go +++ b/pkg/downloader/manager.go @@ -320,6 +320,8 @@ func (m *Manager) getRepoNames(deps []*chartutil.Dependency) (map[string]string, if _, err = os.Stat(depPath); os.IsNotExist(err) { return nil, fmt.Errorf("directory %s not found", depPath) + } else if err != nil { + return nil, err } fmt.Fprintf(m.Out, "Repository from local path: %s\n", dd.Repository) @@ -516,13 +518,20 @@ func writeLock(chartpath string, lock *chartutil.RequirementsLock) error { // archive a dep chart from local directory and save it into charts/ func tarFromLocalDir(chartpath string, name string, repo string, version string) (string, error) { destPath := filepath.Join(chartpath, "charts") - origPath, err := filepath.Abs(repo[7:]) + + if !strings.HasPrefix(repo, "file://") { + return "", fmt.Errorf("wrong format: chart %s repository %s", name, repo) + } + + origPath, err := filepath.Abs(strings.TrimPrefix(repo, "file://")) if err != nil { return "", err } if _, err = os.Stat(origPath); os.IsNotExist(err) { return "", fmt.Errorf("directory %s not found: %s", origPath, err) + } else if err != nil { + return "", err } ch, err := chartutil.LoadDir(origPath) diff --git a/pkg/resolver/resolver.go b/pkg/resolver/resolver.go index b0b2000fb..b6654a8ac 100644 --- a/pkg/resolver/resolver.go +++ b/pkg/resolver/resolver.go @@ -65,6 +65,8 @@ func (r *Resolver) Resolve(reqs *chartutil.Requirements, repoNames map[string]st if _, err = os.Stat(depPath); os.IsNotExist(err) { return nil, fmt.Errorf("directory %s not found", depPath) + } else if err != nil { + return nil, err } locked[i] = &chartutil.Dependency{ From 4b6b847b694db4277282b692746df75f68bad665 Mon Sep 17 00:00:00 2001 From: Qin Wang Date: Mon, 13 Feb 2017 21:58:36 +0000 Subject: [PATCH 4/4] fix test chart path change from rebase nit: fix spelling of dependency --- docs/helm/helm_dependency.md | 2 +- pkg/resolver/resolver_test.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/helm/helm_dependency.md b/docs/helm/helm_dependency.md index a3fc9d60d..bccc98594 100644 --- a/docs/helm/helm_dependency.md +++ b/docs/helm/helm_dependency.md @@ -46,7 +46,7 @@ For example, dependencies: - name: nginx version: "1.2.3" - repository: "file://../depedency_chart/nginx" + repository: "file://../dependency_chart/nginx" If the dependency chart is retrieved locally, it is not required to have the repository added to helm by "helm add repo". Version matching is also supported for this case. diff --git a/pkg/resolver/resolver_test.go b/pkg/resolver/resolver_test.go index b4b77794f..edc160a63 100644 --- a/pkg/resolver/resolver_test.go +++ b/pkg/resolver/resolver_test.go @@ -81,12 +81,12 @@ func TestResolve(t *testing.T) { name: "repo from valid local path", req: &chartutil.Requirements{ Dependencies: []*chartutil.Dependency{ - {Name: "signtest", Repository: "file://../testdata/testcharts/signtest", Version: "0.1.0"}, + {Name: "signtest", Repository: "file://../../cmd/helm/testdata/testcharts/signtest", Version: "0.1.0"}, }, }, expect: &chartutil.RequirementsLock{ Dependencies: []*chartutil.Dependency{ - {Name: "signtest", Repository: "file://../testdata/testcharts/signtest", Version: "0.1.0"}, + {Name: "signtest", Repository: "file://../../cmd/helm/testdata/testcharts/signtest", Version: "0.1.0"}, }, }, },