diff --git a/cmd/helm/repository.go b/cmd/helm/repository.go index b128cb361..3ec8b1b15 100644 --- a/cmd/helm/repository.go +++ b/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.", - }, - }, - Action: func(c *cli.Context) { run(c, addRepo) }, - }, - { - Name: "show", - Usage: "Show the repository details for a given repository.", - ArgsUsage: "REPOSITORY", + Usage: "Add a chart repository to the remote manager.", + ArgsUsage: "[NAME] [REPOSITORY_URL]", + Action: func(c *cli.Context) { run(c, addRepo) }, }, { 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 } diff --git a/cmd/manager/chartrepos.go b/cmd/manager/chartrepos.go index 7ab873d18..c94107dbc 100644 --- a/cmd/manager/chartrepos.go +++ b/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 } diff --git a/cmd/manager/manager/manager.go b/cmd/manager/manager/manager.go index 9edf38ce3..6649738db 100644 --- a/cmd/manager/manager/manager.go +++ b/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 diff --git a/cmd/manager/testutil.go b/cmd/manager/testutil.go index a12db82ff..8d8923b4b 100644 --- a/cmd/manager/testutil.go +++ b/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 { diff --git a/pkg/repo/gcs_repo.go b/pkg/repo/gcs_repo.go index 95d7b2705..8209fd7f5 100644 --- a/pkg/repo/gcs_repo.go +++ b/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 } diff --git a/pkg/repo/gcs_repo_test.go b/pkg/repo/gcs_repo_test.go index b91826632..b2aa952f6 100644 --- a/pkg/repo/gcs_repo_test.go +++ b/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) } diff --git a/pkg/repo/inmem_repo_service.go b/pkg/repo/inmem_repo_service.go index 6fa91247d..86d05e9db 100644 --- a/pkg/repo/inmem_repo_service.go +++ b/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 { - return fmt.Errorf("Repository with URL %s already exists", URL) + 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 +} diff --git a/pkg/repo/inmem_repo_service_test.go b/pkg/repo/inmem_repo_service_test.go index a6898cad5..4d837c3db 100644 --- a/pkg/repo/inmem_repo_service_test.go +++ b/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() diff --git a/pkg/repo/repo.go b/pkg/repo/repo.go index ec5751ea4..ce2a29cb3 100644 --- a/pkg/repo/repo.go +++ b/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 diff --git a/pkg/repo/repo_test.go b/pkg/repo/repo_test.go index 9736d42f3..1415f7332 100644 --- a/pkg/repo/repo_test.go +++ b/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") } diff --git a/pkg/repo/repoprovider.go b/pkg/repo/repoprovider.go index 4661717e3..f5139d8d6 100644 --- a/pkg/repo/repoprovider.go +++ b/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) { diff --git a/pkg/repo/repoprovider_test.go b/pkg/repo/repoprovider_test.go index d81993244..c55b82ae8 100644 --- a/pkg/repo/repoprovider_test.go +++ b/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) } diff --git a/pkg/repo/types.go b/pkg/repo/types.go index d65e7b2ab..497f653cd 100644 --- a/pkg/repo/types.go +++ b/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