From 6c6530c1b6a8111a1a1acce2628f24b3d4ff087b Mon Sep 17 00:00:00 2001 From: "Keerthan Reddy Mala (kmala)" Date: Tue, 15 Nov 2016 16:50:19 -0700 Subject: [PATCH] fix(requirements): accept semver constraints for the versions in the requirements.yaml --- cmd/helm/downloader/manager.go | 45 ++++++++++++++-- cmd/helm/downloader/manager_test.go | 51 +++++++++++++++++++ cmd/helm/resolver/resolver.go | 49 ++++++++++++++---- cmd/helm/resolver/resolver_test.go | 36 +++++++++++-- .../cache/kubernetes-charts-index.yaml | 49 ++++++++++++++++++ 5 files changed, 211 insertions(+), 19 deletions(-) create mode 100644 cmd/helm/resolver/testdata/helmhome/repository/cache/kubernetes-charts-index.yaml diff --git a/cmd/helm/downloader/manager.go b/cmd/helm/downloader/manager.go index 82ab83ad5..db3d3c4f0 100644 --- a/cmd/helm/downloader/manager.go +++ b/cmd/helm/downloader/manager.go @@ -116,8 +116,10 @@ func (m *Manager) Update() error { return err } - // Check that all of the repos we're dependent on actually exist. - if err := m.hasAllRepos(req.Dependencies); err != nil { + // Check that all of the repos we're dependent on actually exist and + // the repo index names. + repoNames, err := m.getRepoNames(req.Dependencies) + if err != nil { return err } @@ -128,7 +130,7 @@ func (m *Manager) Update() error { // Now we need to find out which version of a chart best satisfies the // requirements the requirements.yaml - lock, err := m.resolve(req) + lock, err := m.resolve(req, repoNames) if err != nil { return err } @@ -160,9 +162,9 @@ func (m *Manager) loadChartDir() (*chart.Chart, error) { // resolve takes a list of requirements and translates them into an exact version to download. // // This returns a lock file, which has all of the requirements normalized to a specific version. -func (m *Manager) resolve(req *chartutil.Requirements) (*chartutil.RequirementsLock, error) { +func (m *Manager) resolve(req *chartutil.Requirements, repoNames map[string]string) (*chartutil.RequirementsLock, error) { res := resolver.New(m.ChartPath, m.HelmHome) - return res.Resolve(req) + return res.Resolve(req, repoNames) } // downloadAll takes a list of dependencies and downloads them into charts/ @@ -239,6 +241,39 @@ func (m *Manager) hasAllRepos(deps []*chartutil.Dependency) error { return nil } +// getRepoNames returns the repo names of the referenced deps which can be used to fetch the cahced index file. +func (m *Manager) getRepoNames(deps []*chartutil.Dependency) (map[string]string, error) { + rf, err := repo.LoadRepositoriesFile(m.HelmHome.RepositoryFile()) + if err != nil { + return nil, err + } + repos := rf.Repositories + + reposMap := make(map[string]string) + + // Verify that all repositories referenced in the deps are actually known + // by Helm. + missing := []string{} + for _, dd := range deps { + found := false + + for _, repo := range repos { + if urlsAreEqual(repo.URL, dd.Repository) { + found = true + reposMap[dd.Name] = repo.Name + break + } + } + if !found { + missing = append(missing, dd.Repository) + } + } + if len(missing) > 0 { + return nil, fmt.Errorf("no repository definition for %s. Try 'helm repo add'", strings.Join(missing, ", ")) + } + return reposMap, nil +} + // UpdateRepositories updates all of the local repos to the latest. func (m *Manager) UpdateRepositories() error { rf, err := repo.LoadRepositoriesFile(m.HelmHome.RepositoryFile()) diff --git a/cmd/helm/downloader/manager_test.go b/cmd/helm/downloader/manager_test.go index f21eef44b..7224ccc41 100644 --- a/cmd/helm/downloader/manager_test.go +++ b/cmd/helm/downloader/manager_test.go @@ -17,9 +17,11 @@ package downloader import ( "bytes" + "reflect" "testing" "k8s.io/helm/cmd/helm/helmpath" + "k8s.io/helm/pkg/chartutil" ) func TestVersionEquals(t *testing.T) { @@ -84,3 +86,52 @@ func TestFindChartURL(t *testing.T) { } } + +func TestGetRepoNames(t *testing.T) { + b := bytes.NewBuffer(nil) + m := &Manager{ + Out: b, + HelmHome: helmpath.Home("testdata/helmhome"), + } + tests := []struct { + name string + req []*chartutil.Dependency + expect map[string]string + err bool + }{ + { + name: "no repo definition failure", + req: []*chartutil.Dependency{ + {Name: "oedipus-rex", Repository: "http://example.com/test"}, + }, + err: true, + }, + { + name: "no repo definition failure", + req: []*chartutil.Dependency{ + {Name: "oedipus-rex", Repository: "http://example.com"}, + }, + expect: map[string]string{"oedipus-rex": "testing"}, + }, + } + + for _, tt := range tests { + l, err := m.getRepoNames(tt.req) + if err != nil { + if tt.err { + continue + } + t.Fatal(err) + } + + if tt.err { + t.Fatalf("Expected error in test %q", tt.name) + } + + // m1 and m2 are the maps we want to compare + eq := reflect.DeepEqual(l, tt.expect) + if !eq { + t.Errorf("%s: expected map %v, got %v", tt.name, l, tt.name) + } + } +} diff --git a/cmd/helm/resolver/resolver.go b/cmd/helm/resolver/resolver.go index eb878cbb4..480cea674 100644 --- a/cmd/helm/resolver/resolver.go +++ b/cmd/helm/resolver/resolver.go @@ -19,6 +19,7 @@ import ( "bytes" "encoding/json" "fmt" + "strings" "time" "github.com/Masterminds/semver" @@ -26,6 +27,7 @@ import ( "k8s.io/helm/cmd/helm/helmpath" "k8s.io/helm/pkg/chartutil" "k8s.io/helm/pkg/provenance" + "k8s.io/helm/pkg/repo" ) // Resolver resolves dependencies from semantic version ranges to a particular version. @@ -43,7 +45,7 @@ func New(chartpath string, helmhome helmpath.Home) *Resolver { } // Resolve resolves dependencies and returns a lock file with the resolution. -func (r *Resolver) Resolve(reqs *chartutil.Requirements) (*chartutil.RequirementsLock, error) { +func (r *Resolver) Resolve(reqs *chartutil.Requirements, repoNames map[string]string) (*chartutil.RequirementsLock, error) { d, err := HashReq(reqs) if err != nil { return nil, err @@ -51,22 +53,49 @@ func (r *Resolver) Resolve(reqs *chartutil.Requirements) (*chartutil.Requirement // Now we clone the dependencies, locking as we go. locked := make([]*chartutil.Dependency, len(reqs.Dependencies)) + missing := []string{} for i, d := range reqs.Dependencies { - // Right now, we're just copying one entry to another. What we need to - // do here is parse the requirement as a SemVer range, and then look up - // whether a version in index.yaml satisfies this constraint. If so, - // we need to clone the dep, setting Version appropriately. - // If not, we need to error out. - if _, err := semver.NewVersion(d.Version); err != nil { - return nil, fmt.Errorf("dependency %q has an invalid version: %s", d.Name, err) + 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) } + + repoIndex, err := repo.LoadIndexFile(r.helmhome.CacheIndex(repoNames[d.Name])) + if err != nil { + return nil, fmt.Errorf("no cached repo found. (try 'helm repo update'). %s", err) + } + + vs, ok := repoIndex.Entries[d.Name] + if !ok { + return nil, fmt.Errorf("%s chart not found in repo %s", d.Name, d.Repository) + } + locked[i] = &chartutil.Dependency{ Name: d.Name, Repository: d.Repository, - Version: d.Version, } - } + found := false + // The version are already sorted and hence the first one to satisfy the constraint is used + for _, ver := range vs { + v, err := semver.NewVersion(ver.Version) + if err != nil || len(ver.URLs) == 0 { + // Not a legit entry. + continue + } + if constraint.Check(v) { + found = true + locked[i].Version = v.Original() + break + } + } + if !found { + missing = append(missing, d.Name) + } + } + if len(missing) > 0 { + return nil, fmt.Errorf("Can't get a valid version for repositories %s. Try changing the version constraint in requirements.yaml", strings.Join(missing, ", ")) + } return &chartutil.RequirementsLock{ Generated: time.Now(), Digest: d, diff --git a/cmd/helm/resolver/resolver_test.go b/cmd/helm/resolver/resolver_test.go index 440a58af1..1f5409985 100644 --- a/cmd/helm/resolver/resolver_test.go +++ b/cmd/helm/resolver/resolver_test.go @@ -32,7 +32,34 @@ func TestResolve(t *testing.T) { name: "version failure", req: &chartutil.Requirements{ Dependencies: []*chartutil.Dependency{ - {Name: "oedipus-rex", Repository: "http://example.com", Version: ">1"}, + {Name: "oedipus-rex", Repository: "http://example.com", Version: ">a1"}, + }, + }, + err: true, + }, + { + name: "cache index failure", + req: &chartutil.Requirements{ + Dependencies: []*chartutil.Dependency{ + {Name: "oedipus-rex", Repository: "http://example.com", Version: "1.0.0"}, + }, + }, + err: true, + }, + { + name: "chart not found failure", + req: &chartutil.Requirements{ + Dependencies: []*chartutil.Dependency{ + {Name: "redis", Repository: "http://example.com", Version: "1.0.0"}, + }, + }, + err: true, + }, + { + name: "constraint not satisfied failure", + req: &chartutil.Requirements{ + Dependencies: []*chartutil.Dependency{ + {Name: "alpine", Repository: "http://example.com", Version: ">=1.0.0"}, }, }, err: true, @@ -41,20 +68,21 @@ func TestResolve(t *testing.T) { name: "valid lock", req: &chartutil.Requirements{ Dependencies: []*chartutil.Dependency{ - {Name: "antigone", Repository: "http://example.com", Version: "1.0.0"}, + {Name: "alpine", Repository: "http://example.com", Version: ">=0.1.0"}, }, }, expect: &chartutil.RequirementsLock{ Dependencies: []*chartutil.Dependency{ - {Name: "antigone", Repository: "http://example.com", Version: "1.0.0"}, + {Name: "alpine", Repository: "http://example.com", Version: "0.2.0"}, }, }, }, } + repoNames := map[string]string{"alpine": "kubernetes-charts", "redis": "kubernetes-charts"} r := New("testdata/chartpath", "testdata/helmhome") for _, tt := range tests { - l, err := r.Resolve(tt.req) + l, err := r.Resolve(tt.req, repoNames) if err != nil { if tt.err { continue diff --git a/cmd/helm/resolver/testdata/helmhome/repository/cache/kubernetes-charts-index.yaml b/cmd/helm/resolver/testdata/helmhome/repository/cache/kubernetes-charts-index.yaml new file mode 100644 index 000000000..e2d438701 --- /dev/null +++ b/cmd/helm/resolver/testdata/helmhome/repository/cache/kubernetes-charts-index.yaml @@ -0,0 +1,49 @@ +apiVersion: v1 +entries: + alpine: + - name: alpine + urls: + - https://kubernetes-charts.storage.googleapis.com/alpine-0.1.0.tgz + checksum: 0e6661f193211d7a5206918d42f5c2a9470b737d + home: https://k8s.io/helm + sources: + - https://github.com/kubernetes/helm + version: 0.2.0 + description: Deploy a basic Alpine Linux pod + keywords: [] + maintainers: [] + engine: "" + icon: "" + - name: alpine + urls: + - https://kubernetes-charts.storage.googleapis.com/alpine-0.2.0.tgz + checksum: 0e6661f193211d7a5206918d42f5c2a9470b737d + home: https://k8s.io/helm + sources: + - https://github.com/kubernetes/helm + version: 0.1.0 + description: Deploy a basic Alpine Linux pod + keywords: [] + maintainers: [] + engine: "" + icon: "" + mariadb: + - name: mariadb + urls: + - https://kubernetes-charts.storage.googleapis.com/mariadb-0.3.0.tgz + checksum: 65229f6de44a2be9f215d11dbff311673fc8ba56 + home: https://mariadb.org + sources: + - https://github.com/bitnami/bitnami-docker-mariadb + version: 0.3.0 + description: Chart for MariaDB + keywords: + - mariadb + - mysql + - database + - sql + maintainers: + - name: Bitnami + email: containers@bitnami.com + engine: gotpl + icon: ""