Merge pull request #1292 from technosophos/fix/1227-broken-helm-dep-up

fix(helm): resolve URLs and SemVers correctly

cmd/helm/downloader/manager.go

@@ -22,10 +22,12 @@ import (
 	"io/ioutil"
 	"net/url"
 	"os"
+	"path"
 	"path/filepath"
 	"strings"
 	"sync"
 
+	"github.com/Masterminds/semver"
 	"github.com/ghodss/yaml"
 
 	"k8s.io/helm/cmd/helm/helmpath"
@@ -175,7 +177,7 @@ func (m *Manager) downloadAll(deps []*chartutil.Dependency) error {
 	for _, dep := range deps {
 		fmt.Fprintf(m.Out, "Downloading %s from repo %s\n", dep.Name, dep.Repository)
 
-		churl, err := findChartURL(dep.Name, dep.Repository, repos)
+		churl, err := findChartURL(dep.Name, dep.Version, dep.Repository, repos)
 		if err != nil {
 			fmt.Fprintf(m.Out, "WARNING: %s (skipped)", err)
 			continue
@@ -270,21 +272,28 @@ func urlsAreEqual(a, b string) bool {
 
 // findChartURL searches the cache of repo data for a chart that has the name and the repourl specified.
 //
-// In this current version, name is of the form 'foo-1.2.3'. This will change when
-// the repository index stucture changes.
-func findChartURL(name, repourl string, repos map[string]*repo.ChartRepository) (string, error) {
+// 'name' is the name of the chart. Version is an exact semver, or an empty string. If empty, the
+// newest version will be returned.
+//
+// repourl is the repository to search
+//
+// If it finds a URL that is "relative", it will prepend the repourl.
+func findChartURL(name, version, repourl string, repos map[string]*repo.ChartRepository) (string, error) {
 	for _, cr := range repos {
 		if urlsAreEqual(repourl, cr.URL) {
 			for ename, entry := range cr.IndexFile.Entries {
 				if ename == name {
 					for _, verEntry := range entry {
 						if len(verEntry.URLs) == 0 {
-							// Not totally sure what to do here. Returning an
-							// error is the strictest option. Skipping it might
-							// be preferable.
-							return "", fmt.Errorf("chart %q has no download URL", name)
+							// Not a legit entry.
+							continue
 						}
-						return verEntry.URLs[0], nil
+
+						if version == "" || versionEquals(version, verEntry.Version) {
+							return normalizeURL(repourl, verEntry.URLs[0])
+						}
+
+						return normalizeURL(repourl, verEntry.URLs[0])
 					}
 				}
 			}
@@ -293,6 +302,36 @@ func findChartURL(name, repourl string, repos map[string]*repo.ChartRepository)
 	return "", fmt.Errorf("chart %s not found in %s", name, repourl)
 }
 
+func versionEquals(v1, v2 string) bool {
+	sv1, err := semver.NewVersion(v1)
+	if err != nil {
+		// Fallback to string comparison.
+		return v1 == v2
+	}
+	sv2, err := semver.NewVersion(v2)
+	if err != nil {
+		return false
+	}
+	return sv1.Equal(sv2)
+}
+
+func normalizeURL(baseURL, urlOrPath string) (string, error) {
+	u, err := url.Parse(urlOrPath)
+	if err != nil {
+		return urlOrPath, err
+	}
+	if u.IsAbs() {
+		return u.String(), nil
+	}
+	u2, err := url.Parse(baseURL)
+	if err != nil {
+		return urlOrPath, fmt.Errorf("Base URL failed to parse: %s", err)
+	}
+
+	u2.Path = path.Join(u2.Path, urlOrPath)
+	return u2.String(), nil
+}
+
 // loadChartRepositories reads the repositories.yaml, and then builds a map of
 // ChartRepositories.
 //

cmd/helm/downloader/manager_test.go

+/*
+Copyright 2016 The Kubernetes Authors All rights reserved.
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package downloader
+
+import (
+	"bytes"
+	"testing"
+
+	"k8s.io/helm/cmd/helm/helmpath"
+	//"k8s.io/helm/pkg/repo"
+)
+
+func TestVersionEquals(t *testing.T) {
+	tests := []struct {
+		name, v1, v2 string
+		expect       bool
+	}{
+		{name: "semver match", v1: "1.2.3-beta.11", v2: "1.2.3-beta.11", expect: true},
+		{name: "semver match, build info", v1: "1.2.3-beta.11+a", v2: "1.2.3-beta.11+b", expect: true},
+		{name: "string match", v1: "abcdef123", v2: "abcdef123", expect: true},
+		{name: "semver mismatch", v1: "1.2.3-beta.11", v2: "1.2.3-beta.22", expect: false},
+		{name: "semver mismatch, invalid semver", v1: "1.2.3-beta.11", v2: "stinkycheese", expect: false},
+	}
+
+	for _, tt := range tests {
+		if versionEquals(tt.v1, tt.v2) != tt.expect {
+			t.Errorf("%s: failed comparison of %q and %q (expect equal: %t)", tt.name, tt.v1, tt.v2, tt.expect)
+		}
+	}
+}
+
+func TestNormalizeURL(t *testing.T) {
+	tests := []struct {
+		name, base, path, expect string
+	}{
+		{name: "basic URL", base: "https://example.com", path: "http://helm.sh/foo", expect: "http://helm.sh/foo"},
+		{name: "relative path", base: "https://helm.sh/charts", path: "foo", expect: "https://helm.sh/charts/foo"},
+	}
+
+	for _, tt := range tests {
+		got, err := normalizeURL(tt.base, tt.path)
+		if err != nil {
+			t.Errorf("%s: error %s", tt.name, err)
+			continue
+		} else if got != tt.expect {
+			t.Errorf("%s: expected %q, got %q", tt.name, tt.expect, got)
+		}
+	}
+}
+
+func TestFindChartURL(t *testing.T) {
+	b := bytes.NewBuffer(nil)
+	m := &Manager{
+		Out:      b,
+		HelmHome: helmpath.Home("testdata/helmhome"),
+	}
+	repos, err := m.loadChartRepositories()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	name := "alpine"
+	version := "0.1.0"
+	repoURL := "http://example.com/charts"
+
+	churl, err := findChartURL(name, version, repoURL, repos)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if churl != "http://storage.googleapis.com/kubernetes-charts/alpine-0.1.0.tgz" {
+		t.Errorf("Unexpected URL %q", churl)
+	}
+
+}

cmd/helm/downloader/testdata/helmhome/repository/cache/kubernetes-charts-index.yaml

@@ -2,7 +2,8 @@ apiVersion: v1
 entries:
   alpine:
     - name: alpine
-      url: http://storage.googleapis.com/kubernetes-charts/alpine-0.1.0.tgz
+      urls:
+        - http://storage.googleapis.com/kubernetes-charts/alpine-0.1.0.tgz
       checksum: 0e6661f193211d7a5206918d42f5c2a9470b737d
       home: https://k8s.io/helm
       sources:
@@ -14,7 +15,8 @@ entries:
       engine: ""
       icon: ""
     - name: alpine
-      url: http://storage.googleapis.com/kubernetes-charts/alpine-0.2.0.tgz
+      urls:
+        - http://storage.googleapis.com/kubernetes-charts/alpine-0.2.0.tgz
       checksum: 0e6661f193211d7a5206918d42f5c2a9470b737d
       home: https://k8s.io/helm
       sources:
@@ -27,7 +29,8 @@ entries:
       icon: ""
   mariadb:
     - name: mariadb
-      url: http://storage.googleapis.com/kubernetes-charts/mariadb-0.3.0.tgz
+      urls:
+        - http://storage.googleapis.com/kubernetes-charts/mariadb-0.3.0.tgz
       checksum: 65229f6de44a2be9f215d11dbff311673fc8ba56
       home: https://mariadb.org
       sources:

cmd/helm/downloader/testdata/helmhome/repository/cache/testing-index.yaml

+apiVersion: v1

cmd/helm/downloader/testdata/helmhome/repository/repositories.yaml

@@ -2,3 +2,5 @@ apiVersion: v1
 repositories:
   - name: testing
     url: "http://example.com"
+  - name: kubernetes-charts
+    url: "http://example.com/charts"