Merge pull request #446 from jackgr/manager

Third round of refactoring for Manager

cmd/expandybird/expander/expander.go

@@ -24,7 +24,7 @@ import (
 	"log"
 	"os/exec"
 
-	"github.com/kubernetes/helm/pkg/common"
+	"github.com/kubernetes/helm/pkg/expansion"
 )
 
 type expander struct {
@@ -32,7 +32,7 @@ type expander struct {
 }
 
 // NewExpander returns an ExpandyBird expander.
-func NewExpander(binary string) common.Expander {
+func NewExpander(binary string) expansion.Expander {
 	return &expander{binary}
 }
 
@@ -47,7 +47,7 @@ type expandyBirdOutput struct {
 
 // ExpandChart passes the given configuration to the expander and returns the
 // expanded configuration as a string on success.
-func (e *expander) ExpandChart(request *common.ExpansionRequest) (*common.ExpansionResponse, error) {
+func (e *expander) ExpandChart(request *expansion.ServiceRequest) (*expansion.ServiceResponse, error) {
 	if request.ChartInvocation == nil {
 		return nil, fmt.Errorf("Request does not have invocation field")
 	}
@@ -155,5 +155,5 @@ func (e *expander) ExpandChart(request *common.ExpansionRequest) (*common.Expans
 		return nil, fmt.Errorf("cannot unmarshal expansion result (%s):\n%s", err, output)
 	}
 
-	return &common.ExpansionResponse{Resources: output.Config.Resources}, nil
+	return &expansion.ServiceResponse{Resources: output.Config.Resources}, nil
 }

cmd/expandybird/expander/expander_test.go

@@ -25,14 +25,15 @@ import (
 
 	"github.com/kubernetes/helm/pkg/chart"
 	"github.com/kubernetes/helm/pkg/common"
+	"github.com/kubernetes/helm/pkg/expansion"
 )
 
 var expanderName = "../../../expansion/expansion.py"
 
 type testCase struct {
 	Description      string
-	Request          *common.ExpansionRequest
-	ExpectedResponse *common.ExpansionResponse
+	Request          *expansion.ServiceRequest
+	ExpectedResponse *expansion.ServiceResponse
 	ExpectedError    string
 }
 
@@ -47,8 +48,8 @@ func funcName() string {
 	return runtime.FuncForPC(pc).Name()
 }
 
-func testExpansion(t *testing.T, req *common.ExpansionRequest,
-	expResponse *common.ExpansionResponse, expError string) {
+func testExpansion(t *testing.T, req *expansion.ServiceRequest,
+	expResponse *expansion.ServiceResponse, expError string) {
 	backend := NewExpander(expanderName)
 	response, err := backend.ExpandChart(req)
 	if err != nil {
@@ -81,7 +82,7 @@ var jinjaExpander = &chart.Expander{
 func TestEmptyJinja(t *testing.T) {
 	testExpansion(
 		t,
-		&common.ExpansionRequest{
+		&expansion.ServiceRequest{
 			ChartInvocation: &common.Resource{
 				Name: "test_invocation",
 				Type: funcName(),
@@ -99,7 +100,7 @@ func TestEmptyJinja(t *testing.T) {
 				},
 			},
 		},
-		&common.ExpansionResponse{
+		&expansion.ServiceResponse{
 			Resources: []interface{}{},
 		},
 		"", // Error
@@ -109,7 +110,7 @@ func TestEmptyJinja(t *testing.T) {
 func TestEmptyPython(t *testing.T) {
 	testExpansion(
 		t,
-		&common.ExpansionRequest{
+		&expansion.ServiceRequest{
 			ChartInvocation: &common.Resource{
 				Name: "test_invocation",
 				Type: funcName(),
@@ -130,7 +131,7 @@ func TestEmptyPython(t *testing.T) {
 				},
 			},
 		},
-		&common.ExpansionResponse{
+		&expansion.ServiceResponse{
 			Resources: []interface{}{},
 		},
 		"", // Error
@@ -140,7 +141,7 @@ func TestEmptyPython(t *testing.T) {
 func TestSimpleJinja(t *testing.T) {
 	testExpansion(
 		t,
-		&common.ExpansionRequest{
+		&expansion.ServiceRequest{
 			ChartInvocation: &common.Resource{
 				Name: "test_invocation",
 				Type: funcName(),
@@ -162,7 +163,7 @@ func TestSimpleJinja(t *testing.T) {
 				},
 			},
 		},
-		&common.ExpansionResponse{
+		&expansion.ServiceResponse{
 			Resources: []interface{}{
 				map[string]interface{}{
 					"name": "foo",
@@ -177,7 +178,7 @@ func TestSimpleJinja(t *testing.T) {
 func TestSimplePython(t *testing.T) {
 	testExpansion(
 		t,
-		&common.ExpansionRequest{
+		&expansion.ServiceRequest{
 			ChartInvocation: &common.Resource{
 				Name: "test_invocation",
 				Type: funcName(),
@@ -201,7 +202,7 @@ func TestSimplePython(t *testing.T) {
 				},
 			},
 		},
-		&common.ExpansionResponse{
+		&expansion.ServiceResponse{
 			Resources: []interface{}{
 				map[string]interface{}{
 					"name": "foo",
@@ -216,7 +217,7 @@ func TestSimplePython(t *testing.T) {
 func TestPropertiesJinja(t *testing.T) {
 	testExpansion(
 		t,
-		&common.ExpansionRequest{
+		&expansion.ServiceRequest{
 			ChartInvocation: &common.Resource{
 				Name: "test_invocation",
 				Type: funcName(),
@@ -244,7 +245,7 @@ func TestPropertiesJinja(t *testing.T) {
 				},
 			},
 		},
-		&common.ExpansionResponse{
+		&expansion.ServiceResponse{
 			Resources: []interface{}{
 				map[string]interface{}{
 					"name": "foo",
@@ -262,7 +263,7 @@ func TestPropertiesJinja(t *testing.T) {
 func TestPropertiesPython(t *testing.T) {
 	testExpansion(
 		t,
-		&common.ExpansionRequest{
+		&expansion.ServiceRequest{
 			ChartInvocation: &common.Resource{
 				Name: "test_invocation",
 				Type: funcName(),
@@ -292,7 +293,7 @@ func TestPropertiesPython(t *testing.T) {
 				},
 			},
 		},
-		&common.ExpansionResponse{
+		&expansion.ServiceResponse{
 			Resources: []interface{}{
 				map[string]interface{}{
 					"name": "foo",
@@ -310,7 +311,7 @@ func TestPropertiesPython(t *testing.T) {
 func TestMultiFileJinja(t *testing.T) {
 	testExpansion(
 		t,
-		&common.ExpansionRequest{
+		&expansion.ServiceRequest{
 			ChartInvocation: &common.Resource{
 				Name: "test_invocation",
 				Type: funcName(),
@@ -336,7 +337,7 @@ func TestMultiFileJinja(t *testing.T) {
 				},
 			},
 		},
-		&common.ExpansionResponse{
+		&expansion.ServiceResponse{
 			Resources: []interface{}{
 				map[string]interface{}{
 					"name": "foo",
@@ -368,7 +369,7 @@ var schemaContent = content([]string{
 func TestSchema(t *testing.T) {
 	testExpansion(
 		t,
-		&common.ExpansionRequest{
+		&expansion.ServiceRequest{
 			ChartInvocation: &common.Resource{
 				Name: "test_invocation",
 				Type: funcName(),
@@ -401,7 +402,7 @@ func TestSchema(t *testing.T) {
 				},
 			},
 		},
-		&common.ExpansionResponse{
+		&expansion.ServiceResponse{
 			Resources: []interface{}{
 				map[string]interface{}{
 					"name": "foo",
@@ -419,7 +420,7 @@ func TestSchema(t *testing.T) {
 func TestSchemaFail(t *testing.T) {
 	testExpansion(
 		t,
-		&common.ExpansionRequest{
+		&expansion.ServiceRequest{
 			ChartInvocation: &common.Resource{
 				Name: "test_invocation",
 				Type: funcName(),
@@ -460,7 +461,7 @@ func TestSchemaFail(t *testing.T) {
 func TestMultiFileJinjaMissing(t *testing.T) {
 	testExpansion(
 		t,
-		&common.ExpansionRequest{
+		&expansion.ServiceRequest{
 			ChartInvocation: &common.Resource{
 				Name: "test_invocation",
 				Type: funcName(),
@@ -486,7 +487,7 @@ func TestMultiFileJinjaMissing(t *testing.T) {
 func TestMultiFilePython(t *testing.T) {
 	testExpansion(
 		t,
-		&common.ExpansionRequest{
+		&expansion.ServiceRequest{
 			ChartInvocation: &common.Resource{
 				Name: "test_invocation",
 				Type: funcName(),
@@ -531,7 +532,7 @@ func TestMultiFilePython(t *testing.T) {
 				},
 			},
 		},
-		&common.ExpansionResponse{
+		&expansion.ServiceResponse{
 			Resources: []interface{}{
 				map[string]interface{}{
 					"name": "foo",
@@ -546,7 +547,7 @@ func TestMultiFilePython(t *testing.T) {
 func TestMultiFilePythonMissing(t *testing.T) {
 	testExpansion(
 		t,
-		&common.ExpansionRequest{
+		&expansion.ServiceRequest{
 			ChartInvocation: &common.Resource{
 				Name: "test_invocation",
 				Type: funcName(),
@@ -574,7 +575,7 @@ func TestMultiFilePythonMissing(t *testing.T) {
 func TestWrongChartName(t *testing.T) {
 	testExpansion(
 		t,
-		&common.ExpansionRequest{
+		&expansion.ServiceRequest{
 			ChartInvocation: &common.Resource{
 				Name: "test_invocation",
 				Type: funcName(),
@@ -600,7 +601,7 @@ func TestWrongChartName(t *testing.T) {
 func TestEntrypointNotFound(t *testing.T) {
 	testExpansion(
 		t,
-		&common.ExpansionRequest{
+		&expansion.ServiceRequest{
 			ChartInvocation: &common.Resource{
 				Name: "test_invocation",
 				Type: funcName(),
@@ -621,7 +622,7 @@ func TestEntrypointNotFound(t *testing.T) {
 func TestMalformedResource(t *testing.T) {
 	testExpansion(
 		t,
-		&common.ExpansionRequest{
+		&expansion.ServiceRequest{
 			ChartInvocation: &common.Resource{
 				Name: "test_invocation",
 				Type: funcName(),
@@ -650,7 +651,7 @@ func TestMalformedResource(t *testing.T) {
 func TestResourceNoName(t *testing.T) {
 	testExpansion(
 		t,
-		&common.ExpansionRequest{
+		&expansion.ServiceRequest{
 			ChartInvocation: &common.Resource{
 				Name: "test_invocation",
 				Type: funcName(),
@@ -679,7 +680,7 @@ func TestResourceNoName(t *testing.T) {
 func TestResourceNoType(t *testing.T) {
 	testExpansion(
 		t,
-		&common.ExpansionRequest{
+		&expansion.ServiceRequest{
 			ChartInvocation: &common.Resource{
 				Name: "test_invocation",
 				Type: funcName(),

cmd/expandybird/service/service.go

@@ -17,7 +17,7 @@ limitations under the License.
 package service
 
 import (
-	"github.com/kubernetes/helm/pkg/common"
+	"github.com/kubernetes/helm/pkg/expansion"
 	"github.com/kubernetes/helm/pkg/util"
 
 	"errors"
@@ -43,8 +43,8 @@ func NewService(handler restful.RouteFunction) *Service {
 	webService.Produces(restful.MIME_JSON, restful.MIME_XML)
 	webService.Route(webService.POST("/expand").To(handler).
 		Doc("Expand a template.").
-		Reads(&common.ExpansionRequest{}).
-		Writes(&common.ExpansionResponse{}))
+		Reads(&expansion.ServiceRequest{}).
+		Writes(&expansion.ServiceResponse{}))
 	return &Service{webService}
 }
 
@@ -61,10 +61,10 @@ func (s *Service) Register(container *restful.Container) {
 
 // NewExpansionHandler returns a route function that handles an incoming
 // template expansion request, bound to the supplied expander.
-func NewExpansionHandler(backend common.Expander) restful.RouteFunction {
+func NewExpansionHandler(backend expansion.Expander) restful.RouteFunction {
 	return func(req *restful.Request, resp *restful.Response) {
 		util.LogHandlerEntry("expandybird: expand", req.Request)
-		request := &common.ExpansionRequest{}
+		request := &expansion.ServiceRequest{}
 		if err := req.ReadEntity(&request); err != nil {
 			logAndReturnErrorFromHandler(http.StatusBadRequest, err.Error(), resp)
 			return

cmd/manager/chartrepos.go

@@ -2,50 +2,140 @@ package main
 
 import (
 	"github.com/kubernetes/helm/cmd/manager/router"
+	"github.com/kubernetes/helm/pkg/httputil"
+	"github.com/kubernetes/helm/pkg/repo"
 	"github.com/kubernetes/helm/pkg/util"
+
 	"net/http"
+	"net/url"
+	"regexp"
 )
 
 func registerChartRepoRoutes(c *router.Context, h *router.Handler) {
-	h.Add("GET /chart_repositories", listChartRepositoriesHandlerFunc)
-	h.Add("POST /chart_repositories", addChartRepoHandlerFunc)
-	h.Add("DELETE /chart_repositories", removeChartRepoHandlerFunc)
+	h.Add("GET /repositories", listChartReposHandlerFunc)
+	h.Add("GET /repositories/*", getChartRepoHandlerFunc)
+	h.Add("GET /repositories/*/charts", listRepoChartsHandlerFunc)
+	h.Add("GET /repositories/*/charts/*", getRepoChartHandlerFunc)
+	h.Add("POST /repositories", addChartRepoHandlerFunc)
+	h.Add("DELETE /repositories", removeChartRepoHandlerFunc)
 }
 
-func listChartRepositoriesHandlerFunc(w http.ResponseWriter, r *http.Request, c *router.Context) error {
+func listChartReposHandlerFunc(w http.ResponseWriter, r *http.Request, c *router.Context) error {
 	handler := "manager: list chart repositories"
 	repos, err := c.Manager.ListChartRepos()
 	if err != nil {
 		return err
 	}
+
 	util.LogHandlerExitWithJSON(handler, w, repos, http.StatusOK)
 	return nil
 }
 
 func addChartRepoHandlerFunc(w http.ResponseWriter, r *http.Request, c *router.Context) error {
 	handler := "manager: add chart repository"
-	name, err := pos(w, r, 2)
+	util.LogHandlerEntry(handler, r)
+	defer r.Body.Close()
+	cr := &repo.Repo{}
+	if err := httputil.Decode(w, r, cr); err != nil {
+		httputil.BadRequest(w, r, err)
+		return nil
+	}
+
+	if err := c.Manager.AddChartRepo(cr); err != nil {
+		httputil.BadRequest(w, r, err)
+		return nil
+	}
+
+	util.LogHandlerExitWithText(handler, w, "added", http.StatusOK)
+	return nil
+}
+
+func removeChartRepoHandlerFunc(w http.ResponseWriter, r *http.Request, c *router.Context) error {
+	handler := "manager: remove chart repository"
+	util.LogHandlerEntry(handler, r)
+	URL, err := pos(w, r, 2)
 	if err != nil {
 		return err
 	}
-	err = c.Manager.AddChartRepo(name)
+
+	err = c.Manager.RemoveChartRepo(URL)
 	if err != nil {
 		return err
 	}
-	util.LogHandlerExitWithJSON(handler, w, "added", http.StatusOK)
+
+	util.LogHandlerExitWithText(handler, w, "removed", http.StatusOK)
 	return nil
 }
 
-func removeChartRepoHandlerFunc(w http.ResponseWriter, r *http.Request, c *router.Context) error {
-	handler := "manager: remove chart repository"
-	name, err := pos(w, r, 2)
+func getChartRepoHandlerFunc(w http.ResponseWriter, r *http.Request, c *router.Context) error {
+	handler := "manager: get repository"
+	util.LogHandlerEntry(handler, r)
+	repoURL, err := pos(w, r, 2)
+	if err != nil {
+		return err
+	}
+
+	cr, err := c.Manager.GetChartRepo(repoURL)
+	if err != nil {
+		httputil.BadRequest(w, r, err)
+		return nil
+	}
+
+	util.LogHandlerExitWithJSON(handler, w, cr, http.StatusOK)
+	return nil
+}
+
+func listRepoChartsHandlerFunc(w http.ResponseWriter, r *http.Request, c *router.Context) error {
+	handler := "manager: list repository charts"
+	util.LogHandlerEntry(handler, r)
+	repoURL, err := pos(w, r, 2)
 	if err != nil {
 		return err
 	}
-	err = c.Manager.RemoveChartRepo(name)
+
+	values, err := url.ParseQuery(r.URL.RawQuery)
+	if err != nil {
+		httputil.BadRequest(w, r, err)
+		return nil
+	}
+
+	var regex *regexp.Regexp
+	regexString := values.Get("regex")
+	if regexString != "" {
+		regex, err = regexp.Compile(regexString)
+		if err != nil {
+			httputil.BadRequest(w, r, err)
+			return nil
+		}
+	}
+
+	repoCharts, err := c.Manager.ListRepoCharts(repoURL, regex)
 	if err != nil {
 		return err
 	}
-	util.LogHandlerExitWithJSON(handler, w, "removed", http.StatusOK)
+
+	util.LogHandlerExitWithJSON(handler, w, repoCharts, http.StatusOK)
+	return nil
+}
+
+func getRepoChartHandlerFunc(w http.ResponseWriter, r *http.Request, c *router.Context) error {
+	handler := "manager: get repository charts"
+	util.LogHandlerEntry(handler, r)
+	repoURL, err := pos(w, r, 2)
+	if err != nil {
+		return err
+	}
+
+	chartName, err := pos(w, r, 4)
+	if err != nil {
+		return err
+	}
+
+	repoChart, err := c.Manager.GetChartForRepo(repoURL, chartName)
+	if err != nil {
+		return err
+	}
+
+	util.LogHandlerExitWithJSON(handler, w, repoChart, http.StatusOK)
 	return nil
 }

cmd/manager/chartrepos_test.go

@@ -7,10 +7,10 @@ import (
 
 func TestListChartRepositories(t *testing.T) {
 	c := stubContext()
-	s := httpHarness(c, "GET /chart_repositories", listChartRepositoriesHandlerFunc)
+	s := httpHarness(c, "GET /repositories", listChartReposHandlerFunc)
 	defer s.Close()
 
-	res, err := http.Get(s.URL + "/chart_repositories")
+	res, err := http.Get(s.URL + "/repositories")
 	if err != nil {
 		t.Errorf("Failed GET: %s", err)
 	} else if res.StatusCode != http.StatusOK {

cmd/manager/deployments.go

@@ -26,7 +26,6 @@ import (
 	"net/http"
 	"net/url"
 	"os"
-	"regexp"
 	"strings"
 
 	"github.com/ghodss/yaml"
@@ -71,13 +70,6 @@ func registerDeploymentRoutes(c *router.Context, h *router.Handler) {
 	h.Add("GET /charts/*/repository", getRepoForChartHandlerFunc)
 	h.Add("GET /charts/*/metadata", getMetadataForChartHandlerFunc)
 	h.Add("GET /charts/*", getChartHandlerFunc)
-
-	// TODO: Refactor these commented out routes
-	//	h.Add("GET /repositories/*", getRepoHandlerFunc)
-	//	h.Add("POST /repositories/*", createRepoHandlerFunc)
-
-	h.Add("GET /repositories/*/charts", listRepositoryChartsHandlerFunc)
-	h.Add("GET /repositories/*/charts/*", getRepositoryChartHandlerFunc)
 	h.Add("POST /credentials/*", createCredentialHandlerFunc)
 	h.Add("GET /credentials/*", getCredentialHandlerFunc)
 }
@@ -456,122 +448,6 @@ func getChartHandlerFunc(w http.ResponseWriter, r *http.Request, c *router.Conte
 	return nil
 }
 
-// TODO: Refactor this commented out code.
-
-/*
-
-func getRepoHandlerFunc(w http.ResponseWriter, r *http.Request, c *router.Context) error {
-	handler := "manager: get repository"
-	util.LogHandlerEntry(handler, r)
-	repoName, err := pos(w, r, 2)
-	if err != nil {
-		return err
-	}
-
-	cr, err := c.Manager.GetRepo(repoName)
-	if err != nil {
-		httputil.BadRequest(w, r, err)
-		return nil
-	}
-
-	util.LogHandlerExitWithJSON(handler, w, cr, http.StatusOK)
-	return nil
-}
-
-func getRepo(w http.ResponseWriter, r *http.Request, handler string) *repo.Repo {
-	util.LogHandlerEntry(handler, r)
-
-	t := &repo.Repo{}
-	if err := httputil.Decode(w, r, t); err != nil {
-		httputil.BadRequest(w, r, err)
-		return nil
-	}
-	return t
-}
-
-func createRepoHandlerFunc(w http.ResponseWriter, r *http.Request, c *router.Context) error {
-	handler := "manager: create repository"
-	util.LogHandlerEntry(handler, r)
-	defer r.Body.Close()
-	repoName, err := pos(w, r, 2)
-	if err != nil {
-		return err
-	}
-
-	reg := getRepo(w, r, handler)
-	if reg.Name != repoName {
-		e := fmt.Errorf("Repo name does not match %s != %s", reg.Name, repoName)
-		httputil.BadRequest(w, r, e)
-		return nil
-	}
-	if reg != nil {
-		err = c.Manager.CreateRepo(reg)
-		if err != nil {
-			httputil.BadRequest(w, r, err)
-			return nil
-		}
-	}
-
-	util.LogHandlerExitWithJSON(handler, w, reg, http.StatusOK)
-	return nil
-}
-*/
-
-func listRepositoryChartsHandlerFunc(w http.ResponseWriter, r *http.Request, c *router.Context) error {
-	handler := "manager: list repository charts"
-	util.LogHandlerEntry(handler, r)
-	repoName, err := pos(w, r, 2)
-	if err != nil {
-		return err
-	}
-
-	values, err := url.ParseQuery(r.URL.RawQuery)
-	if err != nil {
-		httputil.BadRequest(w, r, err)
-		return nil
-	}
-
-	var regex *regexp.Regexp
-	regexString := values.Get("regex")
-	if regexString != "" {
-		regex, err = regexp.Compile(regexString)
-		if err != nil {
-			httputil.BadRequest(w, r, err)
-			return nil
-		}
-	}
-
-	repoCharts, err := c.Manager.ListRepoCharts(repoName, regex)
-	if err != nil {
-		return err
-	}
-
-	util.LogHandlerExitWithJSON(handler, w, repoCharts, http.StatusOK)
-	return nil
-}
-
-func getRepositoryChartHandlerFunc(w http.ResponseWriter, r *http.Request, c *router.Context) error {
-	handler := "manager: get repository charts"
-	util.LogHandlerEntry(handler, r)
-	repoName, err := pos(w, r, 2)
-	if err != nil {
-		return err
-	}
-
-	chartName, err := pos(w, r, 4)
-	if err != nil {
-		return err
-	}
-
-	repoChart, err := c.Manager.GetChartForRepo(repoName, chartName)
-	if err != nil {
-		return err
-	}
-
-	util.LogHandlerExitWithJSON(handler, w, repoChart, http.StatusOK)
-	return nil
-}
-
 func getCredential(w http.ResponseWriter, r *http.Request, handler string) *repo.Credential {
 	util.LogHandlerEntry(handler, r)
 	t := &repo.Credential{}

cmd/manager/manager/manager.go

@@ -50,8 +50,8 @@ type Manager interface {
 	GetChart(chartName string) (*chart.Chart, error)
 
 	// Repo Charts
-	ListRepoCharts(repoName string, regex *regexp.Regexp) ([]string, error)
-	GetChartForRepo(repoName, chartName string) (*chart.Chart, error)
+	ListRepoCharts(repoURL string, regex *regexp.Regexp) ([]string, error)
+	GetChartForRepo(repoURL, chartName string) (*chart.Chart, error)
 
 	// Credentials
 	CreateCredential(name string, c *repo.Credential) error
@@ -59,8 +59,9 @@ type Manager interface {
 
 	// Chart Repositories
 	ListChartRepos() ([]string, error)
-	AddChartRepo(name string) error
+	AddChartRepo(addition repo.IRepo) error
 	RemoveChartRepo(name string) error
+	GetChartRepo(URL string) (repo.IRepo, error)
 }
 
 type manager struct {
@@ -335,19 +336,19 @@ func (m *manager) ListChartInstances(chartName string) ([]*common.ChartInstance,
 	return m.repository.GetChartInstances(chartName)
 }
 
-// GetRepoForChart returns the repository where a chart resides.
-func (m *manager) GetRepoForChart(chartName string) (string, error) {
-	_, r, err := m.repoProvider.GetChartByReference(chartName)
+// GetRepoForChart returns the repository where the referenced chart resides.
+func (m *manager) GetRepoForChart(reference string) (string, error) {
+	_, r, err := m.repoProvider.GetChartByReference(reference)
 	if err != nil {
 		return "", err
 	}
 
-	return r.GetName(), nil
+	return r.GetURL(), nil
 }
 
-// GetMetadataForChart returns the metadata for a chart.
-func (m *manager) GetMetadataForChart(chartName string) (*chart.Chartfile, error) {
-	c, _, err := m.repoProvider.GetChartByReference(chartName)
+// GetMetadataForChart returns the metadata for the referenced chart.
+func (m *manager) GetMetadataForChart(reference string) (*chart.Chartfile, error) {
+	c, _, err := m.repoProvider.GetChartByReference(reference)
 	if err != nil {
 		return nil, err
 	}
@@ -355,9 +356,9 @@ func (m *manager) GetMetadataForChart(chartName string) (*chart.Chartfile, error
 	return c.Chartfile(), nil
 }
 
-// GetChart returns a chart.
-func (m *manager) GetChart(chartName string) (*chart.Chart, error) {
-	c, _, err := m.repoProvider.GetChartByReference(chartName)
+// GetChart returns the referenced chart.
+func (m *manager) GetChart(reference string) (*chart.Chart, error) {
+	c, _, err := m.repoProvider.GetChartByReference(reference)
 	if err != nil {
 		return nil, err
 	}
@@ -365,28 +366,24 @@ func (m *manager) GetChart(chartName string) (*chart.Chart, error) {
 	return c, nil
 }
 
-// ListChartRepos returns the list of chart repositories
+// ListChartRepos returns the list of available chart repository URLs
 func (m *manager) ListChartRepos() ([]string, error) {
-	return m.service.List()
+	return m.service.ListRepos()
 }
 
-// AddChartRepo adds a chart repository to list of available chart repositories
-func (m *manager) AddChartRepo(name string) error {
-	//TODO: implement
-	return nil
+// AddChartRepo adds a chart repository to the list
+func (m *manager) AddChartRepo(addition repo.IRepo) error {
+	return m.service.CreateRepo(addition)
 }
 
-// RemoveChartRepo removes a chart repository to list of available chart repositories
-func (m *manager) RemoveChartRepo(name string) error {
-	return m.service.Delete(name)
+// RemoveChartRepo removes a chart repository from the list by URL
+func (m *manager) RemoveChartRepo(URL string) error {
+	return m.service.DeleteRepo(URL)
 }
 
-func (m *manager) CreateRepo(pr repo.IRepo) error {
-	return m.service.Create(pr)
-}
-
-func (m *manager) GetRepo(name string) (repo.IRepo, error) {
-	return m.service.Get(name)
+// GetChartRepo returns the chart repository with the given URL
+func (m *manager) GetChartRepo(URL string) (repo.IRepo, error) {
+	return m.service.GetRepoByURL(URL)
 }
 
 func generateManifestName() string {
@@ -411,11 +408,11 @@ func getResourceErrors(c *common.Configuration) []string {
 	return errs
 }
 
-// ListRepoCharts lists charts in a given repository whose names
+// ListRepoCharts lists charts in a given repository whose URLs
 // conform to the supplied regular expression, or all charts, if the regular
 // expression is nil.
-func (m *manager) ListRepoCharts(repoName string, regex *regexp.Regexp) ([]string, error) {
-	r, err := m.repoProvider.GetRepoByName(repoName)
+func (m *manager) ListRepoCharts(repoURL string, regex *regexp.Regexp) ([]string, error) {
+	r, err := m.repoProvider.GetRepoByURL(repoURL)
 	if err != nil {
 		return nil, err
 	}
@@ -423,9 +420,9 @@ func (m *manager) ListRepoCharts(repoName string, regex *regexp.Regexp) ([]strin
 	return r.ListCharts(regex)
 }
 
-// GetChartForRepo returns a chart from a given repository.
-func (m *manager) GetChartForRepo(repoName, chartName string) (*chart.Chart, error) {
-	r, err := m.repoProvider.GetRepoByName(repoName)
+// GetChartForRepo returns a chart by name from a given repository.
+func (m *manager) GetChartForRepo(repoURL, chartName string) (*chart.Chart, error) {
+	r, err := m.repoProvider.GetRepoByURL(repoURL)
 	if err != nil {
 		return nil, err
 	}

cmd/manager/testutil.go

@@ -91,7 +91,7 @@ func (m *mockManager) GetChart(chartName string) (*chart.Chart, error) {
 	return nil, nil
 }
 
-func (m *mockManager) AddChartRepo(name string) error {
+func (m *mockManager) AddChartRepo(addition repo.IRepo) error {
 	return nil
 }
 
@@ -103,6 +103,10 @@ func (m *mockManager) RemoveChartRepo(name string) error {
 	return nil
 }
 
+func (m *mockManager) GetChartRepo(URL string) (repo.IRepo, error) {
+	return nil, nil
+}
+
 func (m *mockManager) ListRepos() ([]*repo.Repo, error) {
 	return []*repo.Repo{}, nil
 }

pkg/common/types.go

@@ -17,7 +17,6 @@ limitations under the License.
 package common
 
 import (
-	"github.com/kubernetes/helm/pkg/chart"
 	"time"
 )
 
@@ -90,35 +89,6 @@ type ChartInstance struct {
 	Path       string `json:"path"`       // JSON path within manifest
 }
 
-// TODO: Remove the following section when the refactoring of templates is complete.
-
-// Template describes a set of resources to be deployed.
-// Manager expands a Template into a Configuration, which
-// describes the set in a form that can be instantiated.
-type Template struct {
-	Name    string        `json:"name"`
-	Content string        `json:"content"`
-	Imports []*ImportFile `json:"imports"`
-}
-
-// ImportFile describes a base64 encoded file imported by a Template.
-type ImportFile struct {
-	Name    string `json:"name,omitempty"`
-	Path    string `json:"path,omitempty"` // Actual URL for the file
-	Content string `json:"content"`
-}
-
-// SchemaImport represents an import as declared in a schema file.
-type SchemaImport struct {
-	Path string `json:"path"`
-	Name string `json:"name"`
-}
-
-// Schema is a partial DM schema. We only need access to the imports object at this level.
-type Schema struct {
-	Imports []SchemaImport `json:"imports"`
-}
-
 // LayoutResource defines the structure of resources in the manifest layout.
 type LayoutResource struct {
 	Resource
@@ -130,22 +100,6 @@ type Layout struct {
 	Resources []*LayoutResource `json:"resources,omitempty"`
 }
 
-// ExpansionRequest defines the API to expander.
-type ExpansionRequest struct {
-	ChartInvocation *Resource      `json:"chart_invocation"`
-	Chart           *chart.Content `json:"chart"`
-}
-
-// ExpansionResponse defines the API to expander.
-type ExpansionResponse struct {
-	Resources []interface{} `json:"resources"`
-}
-
-// Expander abstracts interactions with the expander and deployer services.
-type Expander interface {
-	ExpandChart(request *ExpansionRequest) (*ExpansionResponse, error)
-}
-
 // Configuration describes a set of resources in a form
 // that can be instantiated.
 type Configuration struct {
@@ -180,3 +134,21 @@ type Resource struct {
 	Properties map[string]interface{} `json:"properties,omitempty"`
 	State      *ResourceState         `json:"state,omitempty"`
 }
+
+// TODO: Remove the following section when the refactoring of templates is complete.
+
+// Template describes a set of resources to be deployed.
+// Manager expands a Template into a Configuration, which
+// describes the set in a form that can be instantiated.
+type Template struct {
+	Name    string        `json:"name"`
+	Content string        `json:"content"`
+	Imports []*ImportFile `json:"imports"`
+}
+
+// ImportFile describes a base64 encoded file imported by a Template.
+type ImportFile struct {
+	Name    string `json:"name,omitempty"`
+	Path    string `json:"path,omitempty"` // Actual URL for the file
+	Content string `json:"content"`
+}

pkg/expander/types.go

-/*
-Copyright 2015 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 expander
-
-import (
-	"github.com/kubernetes/helm/pkg/chart"
-)
-
-// SchemaImport represents an import as declared in a schema file.
-type SchemaImport struct {
-	Path string `json:"path"`
-	Name string `json:"name"`
-}
-
-// Schema is a partial DM schema. We only need access to the imports object at this level.
-type Schema struct {
-	Imports []SchemaImport `json:"imports"`
-}
-
-// LayoutResource defines the structure of resources in the manifest layout.
-type LayoutResource struct {
-	Resource
-	Layout
-}
-
-// Layout defines the structure of a layout as returned from expansion.
-type Layout struct {
-	Resources []*LayoutResource `json:"resources,omitempty"`
-}
-
-// ExpansionRequest defines the API to expander.
-type ExpansionRequest struct {
-	ChartInvocation *Resource      `json:"chart_invocation"`
-	Chart           *chart.Content `json:"chart"`
-}
-
-// ExpansionResponse defines the API to expander.
-type ExpansionResponse struct {
-	Resources []interface{} `json:"resources"`
-}
-
-// Expander abstracts interactions with the expander and deployer services.
-type Expander interface {
-	ExpandChart(request *ExpansionRequest) (*ExpansionResponse, error)
-}
-
-// Configuration describes a set of resources in a form
-// that can be instantiated.
-type Configuration struct {
-	Resources []*Resource `json:"resources"`
-}
-
-// ResourceStatus is an enumeration type for the status of a resource.
-type ResourceStatus string
-
-// These constants implement the resourceStatus enumeration type.
-const (
-	Created ResourceStatus = "Created"
-	Failed  ResourceStatus = "Failed"
-	Aborted ResourceStatus = "Aborted"
-)
-
-// ResourceState describes the state of a resource.
-// Status is set during resource creation and is a terminal state.
-type ResourceState struct {
-	Status   ResourceStatus `json:"status,omitempty"`
-	SelfLink string         `json:"selflink,omitempty"`
-	Errors   []string       `json:"errors,omitempty"`
-}
-
-// Resource describes a resource in a configuration. A resource has
-// a name, a type and a set of properties. The name and type are used
-// to identify the resource in Kubernetes. The properties are passed
-// to Kubernetes as the resource configuration.
-type Resource struct {
-	Name       string                 `json:"name"`
-	Type       string                 `json:"type"`
-	Properties map[string]interface{} `json:"properties,omitempty"`
-	State      *ResourceState         `json:"state,omitempty"`
-}

pkg/expansion/types.go

+/*
+Copyright 2015 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 expansion
+
+import (
+	"github.com/kubernetes/helm/pkg/chart"
+	"github.com/kubernetes/helm/pkg/common"
+)
+
+// ServiceRequest defines the API to expander.
+type ServiceRequest struct {
+	ChartInvocation *common.Resource `json:"chart_invocation"`
+	Chart           *chart.Content   `json:"chart"`
+}
+
+// ServiceResponse defines the API to expander.
+type ServiceResponse struct {
+	Resources []interface{} `json:"resources"`
+}
+
+// Expander abstracts interactions with the expander and deployer services.
+type Expander interface {
+	ExpandChart(request *ServiceRequest) (*ServiceResponse, error)
+}

pkg/repo/gcs_repo.go

@@ -43,14 +43,11 @@ const (
 	// In a GCS repository all charts appear at the top level.
 	GCSRepoFormat = FlatRepoFormat
 
-	// GCSPublicRepoName is the name of the public GCS repository.
-	GCSPublicRepoName = "kubernetes-charts"
+	// GCSPublicRepoBucket is the name of the public GCS repository bucket.
+	GCSPublicRepoBucket = "kubernetes-charts"
 
 	// GCSPublicRepoURL is the URL for the public GCS repository.
-	GCSPublicRepoURL = "gs://" + GCSPublicRepoName
-
-	// GCSPublicRepoBucket is the name of the public GCS repository bucket.
-	GCSPublicRepoBucket = GCSPublicRepoName
+	GCSPublicRepoURL = "gs://" + GCSPublicRepoBucket
 )
 
 // GCSRepo implements the IStorageRepo interface for Google Cloud Storage.
@@ -63,12 +60,12 @@ type GCSRepo struct {
 
 // NewPublicGCSRepo creates a new an IStorageRepo for the public GCS repository.
 func NewPublicGCSRepo(httpClient *http.Client) (IStorageRepo, error) {
-	return NewGCSRepo(GCSPublicRepoName, GCSPublicRepoURL, "", nil)
+	return NewGCSRepo(GCSPublicRepoURL, "", nil)
 }
 
 // NewGCSRepo creates a new IStorageRepo for a given GCS repository.
-func NewGCSRepo(name, URL, credentialName string, httpClient *http.Client) (IStorageRepo, error) {
-	r, err := newRepo(name, URL, credentialName, GCSRepoFormat, GCSRepoType)
+func NewGCSRepo(URL, credentialName string, httpClient *http.Client) (IStorageRepo, error) {
+	r, err := newRepo(URL, credentialName, GCSRepoFormat, GCSRepoType)
 	if err != nil {
 		return nil, err
 	}
@@ -120,7 +117,7 @@ func validateRepoType(repoType ERepoType) error {
 func (g *GCSRepo) ListCharts(regex *regexp.Regexp) ([]string, error) {
 	charts := []string{}
 
-	// List all objects in a bucket using pagination
+	// ListRepos all objects in a bucket using pagination
 	pageToken := ""
 	for {
 		call := g.service.Objects.List(g.bucket)
@@ -135,7 +132,7 @@ func (g *GCSRepo) ListCharts(regex *regexp.Regexp) ([]string, error) {
 		}
 
 		for _, object := range res.Items {
-			// Charts should be named bucket/chart-X.Y.Z.tgz, so tease apart the name
+			// Charts should be named chart-X.Y.Z.tgz, so tease apart the name
 			m := ChartNameMatcher.FindStringSubmatch(object.Name)
 			if len(m) != 3 {
 				continue
@@ -156,7 +153,7 @@ func (g *GCSRepo) ListCharts(regex *regexp.Regexp) ([]string, error) {
 
 // GetChart retrieves, unpacks and returns a chart by name.
 func (g *GCSRepo) GetChart(name string) (*chart.Chart, error) {
-	// Charts should be named bucket/chart-X.Y.Z.tgz, so check that the name matches
+	// Charts should be named chart-X.Y.Z.tgz, so check that the name matches
 	if !ChartNameMatcher.MatchString(name) {
 		return nil, fmt.Errorf("name must be of the form <name>-<version>.tgz, was %s", name)
 	}

pkg/repo/gcs_repo_test.go

@@ -37,7 +37,7 @@ var (
 
 func TestValidGSURL(t *testing.T) {
 	tr := getTestRepo(t)
-	err := validateRepo(tr, TestRepoName, TestRepoURL, TestRepoCredentialName, TestRepoFormat, TestRepoType)
+	err := validateRepo(tr, TestRepoURL, TestRepoCredentialName, TestRepoFormat, TestRepoType)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -51,7 +51,7 @@ func TestValidGSURL(t *testing.T) {
 
 func TestInvalidGSURL(t *testing.T) {
 	var invalidGSURL = "https://valid.url/wrong/scheme"
-	_, err := NewGCSRepo(TestRepoName, invalidGSURL, TestRepoCredentialName, nil)
+	_, err := NewGCSRepo(invalidGSURL, TestRepoCredentialName, nil)
 	if err == nil {
 		t.Fatalf("expected error did not occur for invalid GS URL")
 	}
@@ -126,7 +126,7 @@ func TestGetChartWithInvalidName(t *testing.T) {
 }
 
 func getTestRepo(t *testing.T) IStorageRepo {
-	tr, err := NewGCSRepo(TestRepoName, TestRepoURL, TestRepoCredentialName, nil)
+	tr, err := NewGCSRepo(TestRepoURL, TestRepoCredentialName, nil)
 	if err != nil {
 		t.Fatal(err)
 	}

pkg/repo/inmem_repo_service.go

@@ -35,55 +35,55 @@ func NewInmemRepoService() IRepoService {
 
 	r, err := NewPublicGCSRepo(nil)
 	if err == nil {
-		rs.Create(r)
+		rs.CreateRepo(r)
 	}
 
 	return rs
 }
 
-// List returns the list of all known chart repositories
-func (rs *inmemRepoService) List() ([]string, error) {
+// ListRepos returns the list of all known chart repositories
+func (rs *inmemRepoService) ListRepos() ([]string, error) {
 	rs.RLock()
 	defer rs.RUnlock()
 
 	ret := []string{}
 	for _, r := range rs.repositories {
-		ret = append(ret, r.GetName())
+		ret = append(ret, r.GetURL())
 	}
 
 	return ret, nil
 }
 
-// Create adds a known repository to the list
-func (rs *inmemRepoService) Create(repository IRepo) error {
+// CreateRepo adds a known repository to the list
+func (rs *inmemRepoService) CreateRepo(repository IRepo) error {
 	rs.Lock()
 	defer rs.Unlock()
 
-	name := repository.GetName()
-	_, ok := rs.repositories[name]
+	URL := repository.GetURL()
+	_, ok := rs.repositories[URL]
 	if ok {
-		return fmt.Errorf("Repository named %s already exists", name)
+		return fmt.Errorf("Repository with URL %s already exists", URL)
 	}
 
-	rs.repositories[name] = repository
+	rs.repositories[URL] = repository
 	return nil
 }
 
-// Get returns the repository with the given name
-func (rs *inmemRepoService) Get(name string) (IRepo, error) {
+// GetRepoByURL returns the repository with the given URL
+func (rs *inmemRepoService) GetRepoByURL(URL string) (IRepo, error) {
 	rs.RLock()
 	defer rs.RUnlock()
 
-	r, ok := rs.repositories[name]
+	r, ok := rs.repositories[URL]
 	if !ok {
-		return nil, fmt.Errorf("Failed to find repository named %s", name)
+		return nil, fmt.Errorf("No repository with URL %s", URL)
 	}
 
 	return r, nil
 }
 
-// GetByURL returns the repository that backs the given URL
-func (rs *inmemRepoService) GetByURL(URL string) (IRepo, error) {
+// GetRepoByChartURL returns the repository that backs the given chart URL
+func (rs *inmemRepoService) GetRepoByChartURL(URL string) (IRepo, error) {
 	rs.RLock()
 	defer rs.RUnlock()
 
@@ -98,22 +98,22 @@ func (rs *inmemRepoService) GetByURL(URL string) (IRepo, error) {
 	}
 
 	if found == nil {
-		return nil, fmt.Errorf("Failed to find repository for url: %s", URL)
+		return nil, fmt.Errorf("No repository for url %s", URL)
 	}
 
 	return found, nil
 }
 
-// Delete removes a known repository from the list
-func (rs *inmemRepoService) Delete(name string) error {
+// DeleteRepo removes a known repository from the list
+func (rs *inmemRepoService) DeleteRepo(URL string) error {
 	rs.Lock()
 	defer rs.Unlock()
 
-	_, ok := rs.repositories[name]
+	_, ok := rs.repositories[URL]
 	if !ok {
-		return fmt.Errorf("Failed to find repository named %s", name)
+		return fmt.Errorf("No repository with URL %s", URL)
 	}
 
-	delete(rs.repositories, name)
+	delete(rs.repositories, URL)
 	return nil
 }

pkg/repo/inmem_repo_service_test.go

@@ -23,7 +23,7 @@ import (
 
 func TestService(t *testing.T) {
 	rs := NewInmemRepoService()
-	repos, err := rs.List()
+	repos, err := rs.ListRepos()
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -32,16 +32,16 @@ func TestService(t *testing.T) {
 		t.Fatalf("unexpected repo count; want: %d, have %d.", 1, len(repos))
 	}
 
-	tr, err := rs.Get(repos[0])
+	tr, err := rs.GetRepoByURL(repos[0])
 	if err != nil {
-		t.Fatalf("cannot get repo named %s: %s", repos[0], err)
+		t.Fatal(err)
 	}
 
-	if err := validateRepo(tr, GCSPublicRepoName, GCSPublicRepoURL, "", GCSRepoFormat, GCSRepoType); err != nil {
+	if err := validateRepo(tr, GCSPublicRepoURL, "", GCSRepoFormat, GCSRepoType); err != nil {
 		t.Fatal(err)
 	}
 
-	r1, err := rs.Get(GCSPublicRepoName)
+	r1, err := rs.GetRepoByURL(GCSPublicRepoURL)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -50,7 +50,8 @@ func TestService(t *testing.T) {
 		t.Fatalf("invalid repo returned; want: %#v, have %#v.", tr, r1)
 	}
 
-	r2, err := rs.GetByURL(GCSPublicRepoURL)
+	URL := GCSPublicRepoURL + TestArchiveName
+	r2, err := rs.GetRepoByChartURL(URL)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -59,50 +60,50 @@ func TestService(t *testing.T) {
 		t.Fatalf("invalid repo returned; want: %#v, have %#v.", tr, r2)
 	}
 
-	if err := rs.Delete(GCSPublicRepoName); err != nil {
+	if err := rs.DeleteRepo(GCSPublicRepoURL); err != nil {
 		t.Fatal(err)
 	}
 
-	if _, err := rs.Get(GCSPublicRepoName); err == nil {
-		t.Fatalf("deleted repo named %s returned", GCSPublicRepoName)
+	if _, err := rs.GetRepoByURL(GCSPublicRepoURL); err == nil {
+		t.Fatalf("deleted repo with URL %s returned", GCSPublicRepoURL)
 	}
 }
 
-func TestCreateRepoWithDuplicateName(t *testing.T) {
+func TestCreateRepoWithDuplicateURL(t *testing.T) {
 	rs := NewInmemRepoService()
-	r, err := newRepo(GCSPublicRepoName, GCSPublicRepoURL, "", GCSRepoFormat, GCSRepoType)
+	r, err := newRepo(GCSPublicRepoURL, "", GCSRepoFormat, GCSRepoType)
 	if err != nil {
 		t.Fatalf("cannot create test repo: %s", err)
 	}
 
-	if err := rs.Create(r); err == nil {
-		t.Fatalf("created repo with duplicate name: %s", GCSPublicRepoName)
+	if err := rs.CreateRepo(r); err == nil {
+		t.Fatalf("created repo with duplicate URL: %s", GCSPublicRepoURL)
 	}
 }
 
-func TestGetRepoWithInvalidName(t *testing.T) {
-	invalidName := "InvalidRepoName"
+func TestGetRepoWithInvalidURL(t *testing.T) {
+	invalidURL := "https://not.a.valid/url"
 	rs := NewInmemRepoService()
-	_, err := rs.Get(invalidName)
+	_, err := rs.GetRepoByURL(invalidURL)
 	if err == nil {
-		t.Fatalf("found repo with invalid name: %s", invalidName)
+		t.Fatalf("found repo with invalid URL: %s", invalidURL)
 	}
 }
 
-func TestGetRepoWithInvalidURL(t *testing.T) {
+func TestGetRepoWithInvalidChartURL(t *testing.T) {
 	invalidURL := "https://not.a.valid/url"
 	rs := NewInmemRepoService()
-	_, err := rs.GetByURL(invalidURL)
+	_, err := rs.GetRepoByChartURL(invalidURL)
 	if err == nil {
-		t.Fatalf("found repo with invalid URL: %s", invalidURL)
+		t.Fatalf("found repo with invalid chart URL: %s", invalidURL)
 	}
 }
 
-func TestDeleteRepoWithInvalidName(t *testing.T) {
-	invalidName := "InvalidRepoName"
+func TestDeleteRepoWithInvalidURL(t *testing.T) {
+	invalidURL := "https://not.a.valid/url"
 	rs := NewInmemRepoService()
-	err := rs.Delete(invalidName)
+	err := rs.DeleteRepo(invalidURL)
 	if err == nil {
-		t.Fatalf("deleted repo with invalid name: %s", invalidName)
+		t.Fatalf("deleted repo with invalid name: %s", invalidURL)
 	}
 }

pkg/repo/repo.go

@@ -22,15 +22,11 @@ import (
 )
 
 // NewRepo takes params and returns a IRepo
-func NewRepo(name, URL, credentialName, repoFormat, repoType string) (IRepo, error) {
-	return newRepo(name, URL, credentialName, ERepoFormat(repoFormat), ERepoType(repoType))
+func NewRepo(URL, credentialName, repoFormat, repoType string) (IRepo, error) {
+	return newRepo(URL, credentialName, ERepoFormat(repoFormat), ERepoType(repoType))
 }
 
-func newRepo(name, URL, credentialName string, repoFormat ERepoFormat, repoType ERepoType) (*Repo, error) {
-	if name == "" {
-		return nil, fmt.Errorf("name must not be empty")
-	}
-
+func newRepo(URL, credentialName string, repoFormat ERepoFormat, repoType ERepoType) (*Repo, error) {
 	_, err := url.Parse(URL)
 	if err != nil {
 		return nil, fmt.Errorf("invalid URL (%s): %s", URL, err)
@@ -45,11 +41,10 @@ func newRepo(name, URL, credentialName string, repoFormat ERepoFormat, repoType
 	}
 
 	r := &Repo{
-		Name:           name,
-		Type:           repoType,
 		URL:            URL,
-		Format:         repoFormat,
 		CredentialName: credentialName,
+		Type:           repoType,
+		Format:         repoFormat,
 	}
 
 	return r, nil
@@ -65,11 +60,6 @@ func validateRepoFormat(repoFormat ERepoFormat) error {
 	return fmt.Errorf("unknown repository format: %s", repoFormat)
 }
 
-// GetName returns the friendly name of this repository.
-func (r *Repo) GetName() string {
-	return r.Name
-}
-
 // GetType returns the technology implementing this repository.
 func (r *Repo) GetType() ERepoType {
 	return r.Type
@@ -90,12 +80,7 @@ func (r *Repo) GetCredentialName() string {
 	return r.CredentialName
 }
 
-func validateRepo(tr IRepo, wantName, wantURL, wantCredentialName string, wantFormat ERepoFormat, wantType ERepoType) error {
-	haveName := tr.GetName()
-	if haveName != wantName {
-		return fmt.Errorf("unexpected repository name; want: %s, have %s", wantName, haveName)
-	}
-
+func validateRepo(tr IRepo, wantURL, wantCredentialName string, wantFormat ERepoFormat, wantType ERepoType) error {
 	haveURL := tr.GetURL()
 	if haveURL != wantURL {
 		return fmt.Errorf("unexpected repository url; want: %s, have %s", wantURL, haveURL)

pkg/repo/repo_test.go

@@ -21,8 +21,7 @@ import (
 )
 
 var (
-	TestRepoName           = "kubernetes-charts-testing"
-	TestRepoBucket         = TestRepoName
+	TestRepoBucket         = "kubernetes-charts-testing"
 	TestRepoURL            = "gs://" + TestRepoBucket
 	TestRepoType           = GCSRepoType
 	TestRepoFormat         = GCSRepoFormat
@@ -30,32 +29,25 @@ var (
 )
 
 func TestValidRepoURL(t *testing.T) {
-	tr, err := NewRepo(TestRepoName, TestRepoURL, TestRepoCredentialName, string(TestRepoFormat), string(TestRepoType))
+	tr, err := NewRepo(TestRepoURL, TestRepoCredentialName, string(TestRepoFormat), string(TestRepoType))
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	if err := validateRepo(tr, TestRepoName, TestRepoURL, TestRepoCredentialName, TestRepoFormat, TestRepoType); err != nil {
+	if err := validateRepo(tr, TestRepoURL, TestRepoCredentialName, TestRepoFormat, TestRepoType); err != nil {
 		t.Fatal(err)
 	}
 }
 
-func TestInvalidRepoName(t *testing.T) {
-	_, err := newRepo("", TestRepoURL, TestRepoCredentialName, TestRepoFormat, TestRepoType)
-	if err == nil {
-		t.Fatalf("expected error did not occur for invalid name")
-	}
-}
-
 func TestInvalidRepoURL(t *testing.T) {
-	_, err := newRepo(TestRepoName, "%:invalid&url:%", TestRepoCredentialName, TestRepoFormat, TestRepoType)
+	_, err := newRepo("%:invalid&url:%", TestRepoCredentialName, TestRepoFormat, TestRepoType)
 	if err == nil {
 		t.Fatalf("expected error did not occur for invalid URL")
 	}
 }
 
 func TestDefaultCredentialName(t *testing.T) {
-	tr, err := newRepo(TestRepoName, TestRepoURL, "", TestRepoFormat, TestRepoType)
+	tr, err := newRepo(TestRepoURL, "", TestRepoFormat, TestRepoType)
 	if err != nil {
 		t.Fatalf("cannot create repo using default credential name")
 	}
@@ -68,7 +60,7 @@ func TestDefaultCredentialName(t *testing.T) {
 }
 
 func TestInvalidRepoFormat(t *testing.T) {
-	_, err := newRepo(TestRepoName, TestRepoURL, TestRepoCredentialName, "", TestRepoType)
+	_, err := newRepo(TestRepoURL, TestRepoCredentialName, "", TestRepoType)
 	if err == nil {
 		t.Fatalf("expected error did not occur for invalid format")
 	}

pkg/repo/repoprovider.go

@@ -29,28 +29,21 @@ import (
 	"sync"
 )
 
-// IRepoProvider is a factory for IChartRepo instances.
-type IRepoProvider interface {
-	GetRepoByURL(URL string) (IChartRepo, error)
-	GetRepoByName(repoName string) (IChartRepo, error)
-	GetChartByReference(reference string) (*chart.Chart, IChartRepo, error)
-}
-
 type repoProvider struct {
 	sync.RWMutex
 	rs    IRepoService
 	cp    ICredentialProvider
-	gcsrp GCSRepoProvider
+	gcsrp IGCSRepoProvider
 	repos map[string]IChartRepo
 }
 
 // NewRepoProvider creates a new repository provider.
-func NewRepoProvider(rs IRepoService, gcsrp GCSRepoProvider, cp ICredentialProvider) IRepoProvider {
+func NewRepoProvider(rs IRepoService, gcsrp IGCSRepoProvider, cp ICredentialProvider) IRepoProvider {
 	return newRepoProvider(rs, gcsrp, cp)
 }
 
 // newRepoProvider creates a new repository provider.
-func newRepoProvider(rs IRepoService, gcsrp GCSRepoProvider, cp ICredentialProvider) *repoProvider {
+func newRepoProvider(rs IRepoService, gcsrp IGCSRepoProvider, cp ICredentialProvider) *repoProvider {
 	if rs == nil {
 		rs = NewInmemRepoService()
 	}
@@ -79,20 +72,20 @@ func (rp *repoProvider) GetCredentialProvider() ICredentialProvider {
 }
 
 // GetGCSRepoProvider returns the GCS repository provider used by this repository provider.
-func (rp *repoProvider) GetGCSRepoProvider() GCSRepoProvider {
+func (rp *repoProvider) GetGCSRepoProvider() IGCSRepoProvider {
 	return rp.gcsrp
 }
 
-// GetRepoByName returns the repository with the given name.
-func (rp *repoProvider) GetRepoByName(repoName string) (IChartRepo, error) {
+// GetRepoByURL returns the repository with the given name.
+func (rp *repoProvider) GetRepoByURL(URL string) (IChartRepo, error) {
 	rp.Lock()
 	defer rp.Unlock()
 
-	if r, ok := rp.repos[repoName]; ok {
+	if r, ok := rp.repos[URL]; ok {
 		return r, nil
 	}
 
-	cr, err := rp.rs.Get(repoName)
+	cr, err := rp.rs.GetRepoByURL(URL)
 	if err != nil {
 		return nil, err
 	}
@@ -115,25 +108,25 @@ func (rp *repoProvider) createRepoByType(r IRepo) (IChartRepo, error) {
 }
 
 func (rp *repoProvider) createRepo(cr IChartRepo) (IChartRepo, error) {
-	name := cr.GetName()
-	if _, ok := rp.repos[name]; ok {
-		return nil, fmt.Errorf("respository named %s already exists", name)
+	URL := cr.GetURL()
+	if _, ok := rp.repos[URL]; ok {
+		return nil, fmt.Errorf("respository with URL %s already exists", URL)
 	}
 
-	rp.repos[name] = cr
+	rp.repos[URL] = cr
 	return cr, nil
 }
 
-// GetRepoByURL returns the repository whose URL is a prefix of the given URL.
-func (rp *repoProvider) GetRepoByURL(URL string) (IChartRepo, error) {
+// GetRepoByChartURL returns the repository whose URL is a prefix of the given URL.
+func (rp *repoProvider) GetRepoByChartURL(URL string) (IChartRepo, error) {
 	rp.Lock()
 	defer rp.Unlock()
 
-	if r := rp.findRepoByURL(URL); r != nil {
+	if r := rp.findRepoByChartURL(URL); r != nil {
 		return r, nil
 	}
 
-	cr, err := rp.rs.GetByURL(URL)
+	cr, err := rp.rs.GetRepoByChartURL(URL)
 	if err != nil {
 		return nil, err
 	}
@@ -141,7 +134,7 @@ func (rp *repoProvider) GetRepoByURL(URL string) (IChartRepo, error) {
 	return rp.createRepoByType(cr)
 }
 
-func (rp *repoProvider) findRepoByURL(URL string) IChartRepo {
+func (rp *repoProvider) findRepoByChartURL(URL string) IChartRepo {
 	var found IChartRepo
 	for _, r := range rp.repos {
 		rURL := r.GetURL()
@@ -157,19 +150,14 @@ func (rp *repoProvider) findRepoByURL(URL string) IChartRepo {
 
 // GetChartByReference maps the supplied chart reference into a fully qualified
 // URL, uses the URL to find the repository it references, queries the repository
-// for the chart by URL, and returns the chart and the repository that backs it.
+// for the chart, and then returns the chart and the repository that backs it.
 func (rp *repoProvider) GetChartByReference(reference string) (*chart.Chart, IChartRepo, error) {
-	l, err := ParseGCSChartReference(reference)
+	l, URL, err := parseChartReference(reference)
 	if err != nil {
 		return nil, nil, err
 	}
 
-	URL, err := l.Long(true)
-	if err != nil {
-		return nil, nil, fmt.Errorf("invalid reference %s: %s", reference, err)
-	}
-
-	r, err := rp.GetRepoByURL(URL)
+	r, err := rp.GetRepoByChartURL(URL)
 	if err != nil {
 		return nil, nil, err
 	}
@@ -183,17 +171,45 @@ func (rp *repoProvider) GetChartByReference(reference string) (*chart.Chart, ICh
 	return c, r, nil
 }
 
-// GCSRepoProvider is a factory for GCS IRepo instances.
-type GCSRepoProvider interface {
-	GetGCSRepo(r IRepo) (IStorageRepo, error)
+// IsChartReference returns true if the supplied string is a reference to a chart in a repository
+func IsChartReference(reference string) bool {
+	if _, err := ParseChartReference(reference); err != nil {
+		return false
+	}
+
+	return true
+}
+
+// ParseChartReference parses a reference to a chart in a repository and returns the URL for the chart
+func ParseChartReference(reference string) (*chart.Locator, error) {
+	l, _, err := parseChartReference(reference)
+	if err != nil {
+		return nil, err
+	}
+
+	return l, nil
+}
+
+func parseChartReference(reference string) (*chart.Locator, string, error) {
+	l, err := chart.Parse(reference)
+	if err != nil {
+		return nil, "", fmt.Errorf("cannot parse chart reference %s: %s", reference, err)
+	}
+
+	URL, err := l.Long(true)
+	if err != nil {
+		return nil, "", fmt.Errorf("chart reference %s does not resolve to a URL: %s", reference, err)
+	}
+
+	return l, URL, nil
 }
 
 type gcsRepoProvider struct {
 	cp ICredentialProvider
 }
 
-// NewGCSRepoProvider creates a GCSRepoProvider.
-func NewGCSRepoProvider(cp ICredentialProvider) GCSRepoProvider {
+// NewGCSRepoProvider creates a IGCSRepoProvider.
+func NewGCSRepoProvider(cp ICredentialProvider) IGCSRepoProvider {
 	if cp == nil {
 		cp = NewInmemCredentialProvider()
 	}
@@ -209,7 +225,7 @@ func (gcsrp gcsRepoProvider) GetGCSRepo(r IRepo) (IStorageRepo, error) {
 		return nil, err
 	}
 
-	return NewGCSRepo(r.GetName(), r.GetURL(), r.GetCredentialName(), client)
+	return NewGCSRepo(r.GetURL(), r.GetCredentialName(), client)
 }
 
 func (gcsrp gcsRepoProvider) createGCSClient(credentialName string) (*http.Client, error) {
@@ -233,8 +249,8 @@ func (gcsrp gcsRepoProvider) createGCSClient(credentialName string) (*http.Clien
 }
 
 // IsGCSChartReference returns true if the supplied string is a reference to a chart in a GCS repository
-func IsGCSChartReference(r string) bool {
-	if _, err := ParseGCSChartReference(r); err != nil {
+func IsGCSChartReference(reference string) bool {
+	if _, err := ParseGCSChartReference(reference); err != nil {
 		return false
 	}
 
@@ -242,20 +258,15 @@ func IsGCSChartReference(r string) bool {
 }
 
 // ParseGCSChartReference parses a reference to a chart in a GCS repository and returns the URL for the chart
-func ParseGCSChartReference(r string) (*chart.Locator, error) {
-	l, err := chart.Parse(r)
-	if err != nil {
-		return nil, fmt.Errorf("cannot parse chart reference %s: %s", r, err)
-	}
-
-	URL, err := l.Long(true)
+func ParseGCSChartReference(reference string) (*chart.Locator, error) {
+	l, URL, err := parseChartReference(reference)
 	if err != nil {
-		return nil, fmt.Errorf("chart reference %s does not resolve to a URL: %s", r, err)
+		return nil, err
 	}
 
 	m := GCSChartURLMatcher.FindStringSubmatch(URL)
 	if len(m) != 4 {
-		return nil, fmt.Errorf("chart reference %s resolve to invalid URL: %s", r, URL)
+		return nil, fmt.Errorf("chart reference %s resolve to invalid URL: %s", reference, URL)
 	}
 
 	return l, nil

pkg/repo/repoprovider_test.go

@@ -41,12 +41,12 @@ var InvalidChartReferences = []string{
 
 func TestRepoProvider(t *testing.T) {
 	rp := NewRepoProvider(nil, nil, nil)
-	haveRepo, err := rp.GetRepoByName(GCSPublicRepoName)
+	haveRepo, err := rp.GetRepoByURL(GCSPublicRepoURL)
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	if err := validateRepo(haveRepo, GCSPublicRepoName, GCSPublicRepoURL, "", GCSRepoFormat, GCSRepoType); err != nil {
+	if err := validateRepo(haveRepo, GCSPublicRepoURL, "", GCSRepoFormat, GCSRepoType); err != nil {
 		t.Fatal(err)
 	}
 
@@ -62,7 +62,8 @@ func TestRepoProvider(t *testing.T) {
 	}
 
 	wantRepo := haveRepo
-	haveRepo, err = rp.GetRepoByURL(GCSPublicRepoURL)
+	URL := GCSPublicRepoURL + TestArchiveName
+	haveRepo, err = rp.GetRepoByChartURL(URL)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -72,21 +73,21 @@ func TestRepoProvider(t *testing.T) {
 	}
 }
 
-func TestGetRepoByNameWithInvalidName(t *testing.T) {
-	var invalidName = "InvalidRepoName"
+func TestGetRepoByURLWithInvalidURL(t *testing.T) {
+	var invalidURL = "https://valid.url/wrong/scheme"
 	rp := NewRepoProvider(nil, nil, nil)
-	_, err := rp.GetRepoByName(invalidName)
+	_, err := rp.GetRepoByURL(invalidURL)
 	if err == nil {
-		t.Fatalf("found repo using invalid name: %s", invalidName)
+		t.Fatalf("found repo using invalid URL: %s", invalidURL)
 	}
 }
 
-func TestGetRepoByURLWithInvalidURL(t *testing.T) {
+func TestGetRepoByChartURLWithInvalidChartURL(t *testing.T) {
 	var invalidURL = "https://valid.url/wrong/scheme"
 	rp := NewRepoProvider(nil, nil, nil)
-	_, err := rp.GetRepoByURL(invalidURL)
+	_, err := rp.GetRepoByChartURL(invalidURL)
 	if err == nil {
-		t.Fatalf("found repo using invalid URL: %s", invalidURL)
+		t.Fatalf("found repo using invalid chart URL: %s", invalidURL)
 	}
 }
 
@@ -115,12 +116,12 @@ func TestGetChartByReferenceWithValidReferences(t *testing.T) {
 func getTestRepoProvider(t *testing.T) IRepoProvider {
 	rp := newRepoProvider(nil, nil, nil)
 	rs := rp.GetRepoService()
-	tr, err := newRepo(TestRepoName, TestRepoURL, TestRepoCredentialName, TestRepoFormat, TestRepoType)
+	tr, err := newRepo(TestRepoURL, TestRepoCredentialName, TestRepoFormat, TestRepoType)
 	if err != nil {
 		t.Fatalf("cannot create test repository: %s", err)
 	}
 
-	if err := rs.Create(tr); err != nil {
+	if err := rs.CreateRepo(tr); err != nil {
 		t.Fatalf("cannot initialize repository service: %s", err)
 	}
 

pkg/repo/types.go

@@ -70,17 +70,14 @@ const (
 
 // Repo describes a repository
 type Repo struct {
-	Name           string      `json:"name"`           // Friendly name for this repository
 	URL            string      `json:"url"`                      // URL to the root of this repository
-	CredentialName string      `json:"credentialname"` // Credential name used to access this repository
-	Format         ERepoFormat `json:"format"`         // Format of this repository
-	Type           ERepoType   `json:"type"`           // Technology implementing this repository
+	CredentialName string      `json:"credentialname,omitempty"` // Credential name used to access this repository
+	Format         ERepoFormat `json:"format,omitempty"`         // Format of this repository
+	Type           ERepoType   `json:"type,omitempty"`           // Technology implementing this repository
 }
 
 // IRepo abstracts a repository.
 type IRepo interface {
-	// GetName returns the friendly name of this repository.
-	GetName() string
 	// GetURL returns the URL to the root of this repository.
 	GetURL() string
 	// GetCredentialName returns the credential name used to access this repository.
@@ -96,7 +93,7 @@ type IChartRepo interface {
 	// A IChartRepo is a IRepo
 	IRepo
 
-	// ListCharts lists charts in this repository whose string values
+	// ListCharts lists the URLs for charts in this repository that
 	// conform to the supplied regular expression, or all charts if regex is nil
 	ListCharts(regex *regexp.Regexp) ([]string, error)
 
@@ -117,14 +114,26 @@ type IStorageRepo interface {
 // IRepoService maintains a list of chart repositories that defines the scope of all
 // repository based operations, such as search and chart reference resolution.
 type IRepoService interface {
-	// List returns the list of all known chart repositories
-	List() ([]string, error)
-	// Create adds a known repository to the list
-	Create(repository IRepo) error
-	// Get returns the repository with the given name
-	Get(name string) (IRepo, error)
-	// GetByURL returns the repository that backs the given URL
-	GetByURL(URL string) (IRepo, error)
-	// Delete removes a known repository from the list
-	Delete(name string) error
+	// ListRepos returns the list of all known chart repositories
+	ListRepos() ([]string, error)
+	// CreateRepo adds a known repository to the list
+	CreateRepo(repository IRepo) error
+	// GetRepoByURL returns the repository with the given name
+	GetRepoByURL(name string) (IRepo, error)
+	// GetRepoByChartURL returns the repository that backs the given URL
+	GetRepoByChartURL(URL string) (IRepo, error)
+	// DeleteRepo removes a known repository from the list
+	DeleteRepo(name string) error
+}
+
+// IRepoProvider is a factory for IChartRepo instances.
+type IRepoProvider interface {
+	GetChartByReference(reference string) (*chart.Chart, IChartRepo, error)
+	GetRepoByChartURL(URL string) (IChartRepo, error)
+	GetRepoByURL(URL string) (IChartRepo, error)
+}
+
+// IGCSRepoProvider is a factory for GCS IRepo instances.
+type IGCSRepoProvider interface {
+	GetGCSRepo(r IRepo) (IStorageRepo, error)
 }