Merge pull request #448 from michelleN/helm-repo-ref

feat(repo): remove repo by name + refactors

cmd/helm/repository.go

@@ -17,51 +17,44 @@ limitations under the License.
 package main
 
 import (
+	"encoding/json"
 	"errors"
+	"path/filepath"
+
 	"github.com/codegangsta/cli"
 	"github.com/kubernetes/helm/pkg/format"
+	"github.com/kubernetes/helm/pkg/repo"
 )
 
 func init() {
 	addCommands(repoCommands())
 }
 
-const chartRepoPath = "chart_repositories"
+const chartRepoPath = "repositories"
 
 func repoCommands() cli.Command {
 	return cli.Command{
 		Name:    "repository",
 		Aliases: []string{"repo"},
-		Usage:   "Perform repository operations.",
+		Usage:   "Perform chart repository operations.",
 		Subcommands: []cli.Command{
 			{
 				Name:      "add",
-				Usage:     "Add a repository to the remote manager.",
-				ArgsUsage: "REPOSITORY",
-				Flags: []cli.Flag{
-					cli.StringFlag{
-						Name:  "cred",
-						Usage: "The name of the credential.",
-					},
-				},
+				Usage:     "Add a chart repository to the remote manager.",
+				ArgsUsage: "[NAME] [REPOSITORY_URL]",
 				Action:    func(c *cli.Context) { run(c, addRepo) },
 			},
-			{
-				Name:      "show",
-				Usage:     "Show the repository details for a given repository.",
-				ArgsUsage: "REPOSITORY",
-			},
 			{
 				Name:      "list",
-				Usage:     "List the repositories on the remote manager.",
+				Usage:     "List the chart repositories on the remote manager.",
 				ArgsUsage: "",
 				Action:    func(c *cli.Context) { run(c, listRepos) },
 			},
 			{
 				Name:      "remove",
 				Aliases:   []string{"rm"},
-				Usage:     "Remove a repository from the remote manager.",
-				ArgsUsage: "REPOSITORY",
+				Usage:     "Remove a chart repository from the remote manager.",
+				ArgsUsage: "REPOSITORY_URL",
 				Action:    func(c *cli.Context) { run(c, removeRepo) },
 			},
 		},
@@ -70,35 +63,48 @@ func repoCommands() cli.Command {
 
 func addRepo(c *cli.Context) error {
 	args := c.Args()
-	if len(args) < 1 {
-		return errors.New("'helm repo add' requires a repository as an argument")
+	if len(args) < 2 {
+		return errors.New("'helm repo add' requires a name and repository url as arguments")
 	}
-	dest := ""
-	if _, err := NewClient(c).Post(chartRepoPath, nil, &dest); err != nil {
+	name := args[0]
+	repoURL := args[1]
+	payload, _ := json.Marshal(repo.Repo{URL: repoURL, Name: name})
+	msg := ""
+	if _, err := NewClient(c).Post(chartRepoPath, payload, &msg); err != nil {
+		//TODO: Return more specific errors to the user
 		return err
 	}
-	format.Msg(dest)
+	format.Info(name + " has been added to your chart repositories!")
 	return nil
 }
 
 func listRepos(c *cli.Context) error {
-	dest := ""
+	dest := map[string]string{}
 	if _, err := NewClient(c).Get(chartRepoPath, &dest); err != nil {
 		return err
 	}
-	format.Msg(dest)
+	if len(dest) < 1 {
+		format.Info("Looks like you don't have any chart repositories.")
+		format.Info("Add a chart repository using the `helm repo add [REPOSITORY_URL]` command.")
+	} else {
+		format.Msg("Chart Repositories:\n")
+		for k, v := range dest {
+			//TODO: make formatting pretty
+			format.Msg(k + "\t" + v + "\n")
+		}
+	}
 	return nil
 }
 
 func removeRepo(c *cli.Context) error {
 	args := c.Args()
 	if len(args) < 1 {
-		return errors.New("'helm repo remove' requires a repository as an argument")
+		return errors.New("'helm repo remove' requires a repository name as an argument")
 	}
-	dest := ""
-	if _, err := NewClient(c).Delete(chartRepoPath, &dest); err != nil {
+	name := args[0]
+	if _, err := NewClient(c).Delete(filepath.Join(chartRepoPath, name), nil); err != nil {
 		return err
 	}
-	format.Msg(dest)
+	format.Msg(name + " has been removed.\n")
 	return nil
 }

cmd/manager/chartrepos.go

@@ -22,6 +22,7 @@ import (
 	"github.com/kubernetes/helm/pkg/repo"
 	"github.com/kubernetes/helm/pkg/util"
 
+	"encoding/json"
 	"net/http"
 	"net/url"
 	"regexp"
@@ -33,7 +34,7 @@ func registerChartRepoRoutes(c *router.Context, h *router.Handler) {
 	h.Add("GET /repositories/*/charts", listRepoChartsHandlerFunc)
 	h.Add("GET /repositories/*/charts/*", getRepoChartHandlerFunc)
 	h.Add("POST /repositories", addChartRepoHandlerFunc)
-	h.Add("DELETE /repositories", removeChartRepoHandlerFunc)
+	h.Add("DELETE /repositories/*", removeChartRepoHandlerFunc)
 }
 
 func listChartReposHandlerFunc(w http.ResponseWriter, r *http.Request, c *router.Context) error {
@@ -62,24 +63,27 @@ func addChartRepoHandlerFunc(w http.ResponseWriter, r *http.Request, c *router.C
 		return nil
 	}
 
-	util.LogHandlerExitWithText(handler, w, "added", http.StatusOK)
+	msg, _ := json.Marshal(cr.Name + " has been added to the list of chart repositories.")
+	util.LogHandlerExitWithJSON(handler, w, msg, http.StatusCreated)
 	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)
+	defer r.Body.Close()
+	name, err := pos(w, r, 2)
 	if err != nil {
 		return err
 	}
 
-	err = c.Manager.RemoveChartRepo(URL)
+	err = c.Manager.RemoveChartRepo(name)
 	if err != nil {
 		return err
 	}
 
-	util.LogHandlerExitWithText(handler, w, "removed", http.StatusOK)
+	msg, _ := json.Marshal(name + " has been removed from the list of chart repositories.")
+	util.LogHandlerExitWithJSON(handler, w, msg, http.StatusOK)
 	return nil
 }
 

cmd/manager/manager/manager.go

@@ -58,7 +58,7 @@ type Manager interface {
 	GetCredential(name string) (*repo.Credential, error)
 
 	// Chart Repositories
-	ListChartRepos() ([]string, error)
+	ListChartRepos() (map[string]string, error)
 	AddChartRepo(addition repo.IRepo) error
 	RemoveChartRepo(name string) error
 	GetChartRepo(URL string) (repo.IRepo, error)
@@ -367,7 +367,7 @@ func (m *manager) GetChart(reference string) (*chart.Chart, error) {
 }
 
 // ListChartRepos returns the list of available chart repository URLs
-func (m *manager) ListChartRepos() ([]string, error) {
+func (m *manager) ListChartRepos() (map[string]string, error) {
 	return m.service.ListRepos()
 }
 
@@ -377,8 +377,12 @@ func (m *manager) AddChartRepo(addition repo.IRepo) error {
 }
 
 // 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) RemoveChartRepo(name string) error {
+	url, err := m.service.GetRepoURLByName(name)
+	if err != nil {
+		return err
+	}
+	return m.service.DeleteRepo(url)
 }
 
 // GetChartRepo returns the chart repository with the given URL

cmd/manager/testutil.go

@@ -111,8 +111,8 @@ func (m *mockManager) AddChartRepo(addition repo.IRepo) error {
 	return nil
 }
 
-func (m *mockManager) ListChartRepos() ([]string, error) {
-	return []string{}, nil
+func (m *mockManager) ListChartRepos() (map[string]string, error) {
+	return map[string]string{}, nil
 }
 
 func (m *mockManager) RemoveChartRepo(name string) error {

pkg/repo/gcs_repo.go

@@ -60,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(GCSPublicRepoURL, "", nil)
+	return NewGCSRepo(GCSPublicRepoURL, "", GCSPublicRepoBucket, nil)
 }
 
 // NewGCSRepo creates a new IStorageRepo for a given GCS repository.
-func NewGCSRepo(URL, credentialName string, httpClient *http.Client) (IStorageRepo, error) {
-	r, err := newRepo(URL, credentialName, GCSRepoFormat, GCSRepoType)
+func NewGCSRepo(URL, credentialName, repoName string, httpClient *http.Client) (IStorageRepo, error) {
+	r, err := newRepo(URL, credentialName, repoName, GCSRepoFormat, GCSRepoType)
 	if err != nil {
 		return nil, err
 	}

pkg/repo/gcs_repo_test.go

@@ -33,6 +33,7 @@ var (
 	TestChartFile          = "testdata/frobnitz/Chart.yaml"
 	TestShouldFindRegex    = regexp.MustCompile(TestArchiveName)
 	TestShouldNotFindRegex = regexp.MustCompile("foobar")
+	TestName               = "bucket-name"
 )
 
 func TestValidGSURL(t *testing.T) {
@@ -51,7 +52,7 @@ func TestValidGSURL(t *testing.T) {
 
 func TestInvalidGSURL(t *testing.T) {
 	var invalidGSURL = "https://valid.url/wrong/scheme"
-	_, err := NewGCSRepo(invalidGSURL, TestRepoCredentialName, nil)
+	_, err := NewGCSRepo(invalidGSURL, TestRepoCredentialName, TestName, nil)
 	if err == nil {
 		t.Fatalf("expected error did not occur for invalid GS URL")
 	}
@@ -126,7 +127,7 @@ func TestGetChartWithInvalidName(t *testing.T) {
 }
 
 func getTestRepo(t *testing.T) IStorageRepo {
-	tr, err := NewGCSRepo(TestRepoURL, TestRepoCredentialName, nil)
+	tr, err := NewGCSRepo(TestRepoURL, TestRepoCredentialName, TestName, nil)
 	if err != nil {
 		t.Fatal(err)
 	}

pkg/repo/inmem_repo_service.go

@@ -42,13 +42,13 @@ func NewInmemRepoService() IRepoService {
 }
 
 // ListRepos returns the list of all known chart repositories
-func (rs *inmemRepoService) ListRepos() ([]string, error) {
+func (rs *inmemRepoService) ListRepos() (map[string]string, error) {
 	rs.RLock()
 	defer rs.RUnlock()
 
-	ret := []string{}
+	ret := make(map[string]string)
 	for _, r := range rs.repositories {
-		ret = append(ret, r.GetURL())
+		ret[r.GetName()] = r.GetURL()
 	}
 
 	return ret, nil
@@ -60,9 +60,14 @@ func (rs *inmemRepoService) CreateRepo(repository IRepo) error {
 	defer rs.Unlock()
 
 	URL := repository.GetURL()
-	_, ok := rs.repositories[URL]
-	if ok {
+	name := repository.GetName()
+
+	for u, r := range rs.repositories {
+		if u == URL {
 			return fmt.Errorf("Repository with URL %s already exists", URL)
+		} else if r.GetName() == name {
+			return fmt.Errorf("Repository with Name %s already exists", name)
+		}
 	}
 
 	rs.repositories[URL] = repository
@@ -117,3 +122,13 @@ func (rs *inmemRepoService) DeleteRepo(URL string) error {
 	delete(rs.repositories, URL)
 	return nil
 }
+
+func (rs *inmemRepoService) GetRepoURLByName(name string) (string, error) {
+	for url, r := range rs.repositories {
+		if r.GetName() == name {
+			return url, nil
+		}
+	}
+	err := fmt.Errorf("No repository url found with name %s", name)
+	return "", err
+}

pkg/repo/inmem_repo_service_test.go

@@ -32,7 +32,11 @@ func TestService(t *testing.T) {
 		t.Fatalf("unexpected repo count; want: %d, have %d.", 1, len(repos))
 	}
 
-	tr, err := rs.GetRepoByURL(repos[0])
+	u := ""
+	for _, url := range repos {
+		u = url
+	}
+	tr, err := rs.GetRepoByURL(u)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -71,7 +75,7 @@ func TestService(t *testing.T) {
 
 func TestCreateRepoWithDuplicateURL(t *testing.T) {
 	rs := NewInmemRepoService()
-	r, err := newRepo(GCSPublicRepoURL, "", GCSRepoFormat, GCSRepoType)
+	r, err := newRepo(GCSPublicRepoURL, "", TestName, GCSRepoFormat, GCSRepoType)
 	if err != nil {
 		t.Fatalf("cannot create test repo: %s", err)
 	}
@@ -90,6 +94,25 @@ func TestGetRepoWithInvalidURL(t *testing.T) {
 	}
 }
 
+func TestGetRepoURLByName(t *testing.T) {
+	rs := NewInmemRepoService()
+	testURL := "gcs://helm-test-charts"
+	r, err := newRepo(testURL, "", TestName, GCSRepoFormat, GCSRepoType)
+	err = rs.CreateRepo(r)
+	if err != nil {
+		t.Fatalf("Error creating repo: %s", err)
+	}
+	expectedURL := testURL
+	actualURL, err := rs.GetRepoURLByName(TestName)
+	if err != nil {
+		t.Fatalf("%v", err)
+	}
+	if expectedURL != actualURL {
+		t.Fatalf("Incorrect resulting URL. Expected %s. Got %s", expectedURL, actualURL)
+	}
+
+}
+
 func TestGetRepoWithInvalidChartURL(t *testing.T) {
 	invalidURL := "https://not.a.valid/url"
 	rs := NewInmemRepoService()

pkg/repo/repo.go

@@ -22,11 +22,11 @@ import (
 )
 
 // NewRepo takes params and returns a IRepo
-func NewRepo(URL, credentialName, repoFormat, repoType string) (IRepo, error) {
-	return newRepo(URL, credentialName, ERepoFormat(repoFormat), ERepoType(repoType))
+func NewRepo(URL, credentialName, repoName, repoFormat, repoType string) (IRepo, error) {
+	return newRepo(URL, credentialName, repoName, ERepoFormat(repoFormat), ERepoType(repoType))
 }
 
-func newRepo(URL, credentialName string, repoFormat ERepoFormat, repoType ERepoType) (*Repo, error) {
+func newRepo(URL, credentialName, repoName string, repoFormat ERepoFormat, repoType ERepoType) (*Repo, error) {
 	_, err := url.Parse(URL)
 	if err != nil {
 		return nil, fmt.Errorf("invalid URL (%s): %s", URL, err)
@@ -41,6 +41,7 @@ func newRepo(URL, credentialName string, repoFormat ERepoFormat, repoType ERepoT
 	}
 
 	r := &Repo{
+		Name:           repoName,
 		URL:            URL,
 		CredentialName: credentialName,
 		Type:           repoType,
@@ -65,6 +66,11 @@ func (r *Repo) GetType() ERepoType {
 	return r.Type
 }
 
+// GetName returns the name of this repository.
+func (r *Repo) GetName() string {
+	return r.Name
+}
+
 // GetURL returns the URL to the root of this repository.
 func (r *Repo) GetURL() string {
 	return r.URL

pkg/repo/repo_test.go

@@ -29,7 +29,7 @@ var (
 )
 
 func TestValidRepoURL(t *testing.T) {
-	tr, err := NewRepo(TestRepoURL, TestRepoCredentialName, string(TestRepoFormat), string(TestRepoType))
+	tr, err := NewRepo(TestRepoURL, TestRepoCredentialName, TestRepoBucket, string(TestRepoFormat), string(TestRepoType))
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -40,14 +40,14 @@ func TestValidRepoURL(t *testing.T) {
 }
 
 func TestInvalidRepoURL(t *testing.T) {
-	_, err := newRepo("%:invalid&url:%", TestRepoCredentialName, TestRepoFormat, TestRepoType)
+	_, err := newRepo("%:invalid&url:%", TestRepoCredentialName, TestRepoBucket, TestRepoFormat, TestRepoType)
 	if err == nil {
 		t.Fatalf("expected error did not occur for invalid URL")
 	}
 }
 
 func TestDefaultCredentialName(t *testing.T) {
-	tr, err := newRepo(TestRepoURL, "", TestRepoFormat, TestRepoType)
+	tr, err := newRepo(TestRepoURL, "", TestRepoBucket, TestRepoFormat, TestRepoType)
 	if err != nil {
 		t.Fatalf("cannot create repo using default credential name")
 	}
@@ -60,7 +60,7 @@ func TestDefaultCredentialName(t *testing.T) {
 }
 
 func TestInvalidRepoFormat(t *testing.T) {
-	_, err := newRepo(TestRepoURL, TestRepoCredentialName, "", TestRepoType)
+	_, err := newRepo(TestRepoURL, TestRepoCredentialName, TestRepoBucket, "", TestRepoType)
 	if err == nil {
 		t.Fatalf("expected error did not occur for invalid format")
 	}

pkg/repo/repoprovider.go

@@ -225,7 +225,7 @@ func (gcsrp gcsRepoProvider) GetGCSRepo(r IRepo) (IStorageRepo, error) {
 		return nil, err
 	}
 
-	return NewGCSRepo(r.GetURL(), r.GetCredentialName(), client)
+	return NewGCSRepo(r.GetURL(), r.GetCredentialName(), r.GetName(), client)
 }
 
 func (gcsrp gcsRepoProvider) createGCSClient(credentialName string) (*http.Client, error) {

pkg/repo/repoprovider_test.go

@@ -116,7 +116,7 @@ func TestGetChartByReferenceWithValidReferences(t *testing.T) {
 func getTestRepoProvider(t *testing.T) IRepoProvider {
 	rp := newRepoProvider(nil, nil, nil)
 	rs := rp.GetRepoService()
-	tr, err := newRepo(TestRepoURL, TestRepoCredentialName, TestRepoFormat, TestRepoType)
+	tr, err := newRepo(TestRepoURL, TestRepoCredentialName, TestName, TestRepoFormat, TestRepoType)
 	if err != nil {
 		t.Fatalf("cannot create test repository: %s", err)
 	}

pkg/repo/types.go

@@ -70,6 +70,7 @@ const (
 
 // Repo describes a repository
 type Repo struct {
+	Name           string      `json:"name"`                     // Name of repository
 	URL            string      `json:"url"`                      // URL to the root of this repository
 	CredentialName string      `json:"credentialname,omitempty"` // Credential name used to access this repository
 	Format         ERepoFormat `json:"format,omitempty"`         // Format of this repository
@@ -78,6 +79,8 @@ type Repo struct {
 
 // IRepo abstracts a repository.
 type IRepo interface {
+	// GetName returns the name of the 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.
@@ -115,11 +118,13 @@ type IStorageRepo interface {
 // repository based operations, such as search and chart reference resolution.
 type IRepoService interface {
 	// ListRepos returns the list of all known chart repositories
-	ListRepos() ([]string, error)
+	ListRepos() (map[string]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)
+	// GetRepoURLByName return url for a repository with the given name
+	GetRepoURLByName(name string) (string, error)
 	// GetRepoByChartURL returns the repository that backs the given URL
 	GetRepoByChartURL(URL string) (IRepo, error)
 	// DeleteRepo removes a known repository from the list