From d30e2c11726348a5f2464768e5a57a8b0d778789 Mon Sep 17 00:00:00 2001 From: Michelle Noorali Date: Mon, 28 Mar 2016 14:16:54 -0600 Subject: [PATCH 1/4] feat(repo): validate repo url passed in --- cmd/helm/repository.go | 49 +++++++++++++++++++++++++++++++++---- cmd/helm/repository_test.go | 37 ++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+), 5 deletions(-) create mode 100644 cmd/helm/repository_test.go diff --git a/cmd/helm/repository.go b/cmd/helm/repository.go index 9ff75eb07..6b6ca38ea 100644 --- a/cmd/helm/repository.go +++ b/cmd/helm/repository.go @@ -19,7 +19,10 @@ package main import ( "encoding/json" "errors" + "net/url" "path/filepath" + "regexp" + "strings" "github.com/codegangsta/cli" "github.com/kubernetes/helm/pkg/format" @@ -32,6 +35,9 @@ func init() { const chartRepoPath = "repositories" +// URL is the url pattern used to check if a given repo url is valid +const URL string = `^((http|gs|https?):\/\/)?(\S+(:\S*)?@)?((([1-9]\d?|1\d\d|2[01]\d|22[0-3])(\.(1?\d{1,2}|2[0-4]\d|25[0-5])){2}(?:\.([0-9]\d?|1\d\d|2[0-4]\d|25[0-4]))|(([a-zA-Z0-9]+([-\.][a-zA-Z0-9]+)*)|((www\.)?))?(([a-zA-Z\x{00a1}-\x{ffff}0-9]+-?-?)*[a-zA-Z\x{00a1}-\x{ffff}0-9]+)(?:\.([a-zA-Z\x{00a1}-\x{ffff}]{2,}))?))(:(\d{1,5}))?((\/|\?|#)[^\s]*)?$` + const repoDesc = `Helm repositories store Helm charts. The repository commands are used to manage which Helm repositories Helm may @@ -43,6 +49,16 @@ const repoDesc = `Helm repositories store Helm charts. For more details, use 'helm repo CMD -h'. ` +const addRepoDesc = `helm repository|repo add [NAME] [REPOSITORY_URL] + + The add repository command is used to add a name a repository url to your + chart repository list. The repository url must begin with a valid protocoal + These include https, http, and gs. + + A valid command might look like: + $ helm repo add charts gs://kubernetes-charts +` + func repoCommands() cli.Command { return cli.Command{ Name: "repository", @@ -51,10 +67,11 @@ func repoCommands() cli.Command { Description: repoDesc, Subcommands: []cli.Command{ { - Name: "add", - Usage: "Add a chart repository to the remote manager.", - ArgsUsage: "[NAME] [REPOSITORY_URL]", - Action: func(c *cli.Context) { run(c, addRepo) }, + Name: "add", + Usage: "Add a chart repository to the remote manager.", + Description: addRepoDesc, + ArgsUsage: "[NAME] [REPOSITORY_URL]", + Action: func(c *cli.Context) { run(c, addRepo) }, }, { Name: "list", @@ -80,10 +97,13 @@ func addRepo(c *cli.Context) error { } name := args[0] repoURL := args[1] + valid := IsValidURL(repoURL) + if !valid { + return errors.New(repoURL + " is not a valid URL") + } 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.Info(name + " has been added to your chart repositories!") @@ -120,3 +140,22 @@ func removeRepo(c *cli.Context) error { format.Msg(name + " has been removed.\n") return nil } + +// IsValidURL checks if the string is a valid URL. +// This was inspired by the IsURL function in govalidator https://github.com/asaskevich/govalidator +func IsValidURL(str string) bool { + if str == "" || len(str) >= 2083 || len(str) <= 3 || strings.HasPrefix(str, ".") { + return false + } + u, err := url.Parse(str) + if err != nil { + return false + } + if strings.HasPrefix(u.Host, ".") { + return false + } + if u.Host == "" && (u.Path != "" && !strings.Contains(u.Path, ".")) { + return false + } + return regexp.MustCompile(URL).MatchString(str) +} diff --git a/cmd/helm/repository_test.go b/cmd/helm/repository_test.go new file mode 100644 index 000000000..d3327599b --- /dev/null +++ b/cmd/helm/repository_test.go @@ -0,0 +1,37 @@ +/* +Copyright 2016 The Kubernetes Authors All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package main + +import ( + "testing" +) + +func TestHasValidPrefix(t *testing.T) { + tests := map[string]bool{ + "https://host/bucket": true, + "http://host/bucket": true, + "gs://bucket-name": true, + "charts": false, + } + + for url, expectedResult := range tests { + result := IsValidURL(url) + if result != expectedResult { + t.Errorf("Expected: %v, Got: %v for url: %v", expectedResult, result, url) + } + } +} From 21a6764f6b36a64409678b55f5af925ff649e40b Mon Sep 17 00:00:00 2001 From: Michelle Noorali Date: Mon, 28 Mar 2016 14:17:41 -0600 Subject: [PATCH 2/4] fix(repo): fix repo rm args description --- cmd/helm/repository.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/helm/repository.go b/cmd/helm/repository.go index 6b6ca38ea..5342ee9dc 100644 --- a/cmd/helm/repository.go +++ b/cmd/helm/repository.go @@ -83,7 +83,7 @@ func repoCommands() cli.Command { Name: "remove", Aliases: []string{"rm"}, Usage: "Remove a chart repository from the remote manager.", - ArgsUsage: "REPOSITORY_URL", + ArgsUsage: "REPOSITORY_NAME", Action: func(c *cli.Context) { run(c, removeRepo) }, }, }, From a2ac45917d036cd34f4f328792fda69df1ceb61e Mon Sep 17 00:00:00 2001 From: Michelle Noorali Date: Mon, 28 Mar 2016 14:35:26 -0600 Subject: [PATCH 3/4] ref(repo): modify help text --- cmd/helm/repository.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/cmd/helm/repository.go b/cmd/helm/repository.go index 5342ee9dc..b18193add 100644 --- a/cmd/helm/repository.go +++ b/cmd/helm/repository.go @@ -49,9 +49,7 @@ const repoDesc = `Helm repositories store Helm charts. For more details, use 'helm repo CMD -h'. ` -const addRepoDesc = `helm repository|repo add [NAME] [REPOSITORY_URL] - - The add repository command is used to add a name a repository url to your +const addRepoDesc = ` The add repository command is used to add a name a repository url to your chart repository list. The repository url must begin with a valid protocoal These include https, http, and gs. @@ -99,7 +97,7 @@ func addRepo(c *cli.Context) error { repoURL := args[1] valid := IsValidURL(repoURL) if !valid { - return errors.New(repoURL + " is not a valid URL") + return errors.New(repoURL + " is not a valid REPOSITOTY_URL argument \n" + addRepoDesc) } payload, _ := json.Marshal(repo.Repo{URL: repoURL, Name: name}) msg := "" From d4c3df79980936ceced76d5129c3d8e06c728805 Mon Sep 17 00:00:00 2001 From: Michelle Noorali Date: Mon, 28 Mar 2016 17:12:21 -0600 Subject: [PATCH 4/4] ref(repo): move validate repo url to pkg/repo --- cmd/helm/repository.go | 31 +------------------------------ cmd/helm/repository_test.go | 37 ------------------------------------- cmd/manager/chartrepos.go | 3 +++ pkg/repo/repo.go | 29 +++++++++++++++++++++++++++++ pkg/repo/repo_test.go | 23 +++++++++++++++++++++++ 5 files changed, 56 insertions(+), 67 deletions(-) delete mode 100644 cmd/helm/repository_test.go diff --git a/cmd/helm/repository.go b/cmd/helm/repository.go index b18193add..2084e753d 100644 --- a/cmd/helm/repository.go +++ b/cmd/helm/repository.go @@ -19,10 +19,7 @@ package main import ( "encoding/json" "errors" - "net/url" "path/filepath" - "regexp" - "strings" "github.com/codegangsta/cli" "github.com/kubernetes/helm/pkg/format" @@ -35,9 +32,6 @@ func init() { const chartRepoPath = "repositories" -// URL is the url pattern used to check if a given repo url is valid -const URL string = `^((http|gs|https?):\/\/)?(\S+(:\S*)?@)?((([1-9]\d?|1\d\d|2[01]\d|22[0-3])(\.(1?\d{1,2}|2[0-4]\d|25[0-5])){2}(?:\.([0-9]\d?|1\d\d|2[0-4]\d|25[0-4]))|(([a-zA-Z0-9]+([-\.][a-zA-Z0-9]+)*)|((www\.)?))?(([a-zA-Z\x{00a1}-\x{ffff}0-9]+-?-?)*[a-zA-Z\x{00a1}-\x{ffff}0-9]+)(?:\.([a-zA-Z\x{00a1}-\x{ffff}]{2,}))?))(:(\d{1,5}))?((\/|\?|#)[^\s]*)?$` - const repoDesc = `Helm repositories store Helm charts. The repository commands are used to manage which Helm repositories Helm may @@ -49,7 +43,7 @@ const repoDesc = `Helm repositories store Helm charts. For more details, use 'helm repo CMD -h'. ` -const addRepoDesc = ` The add repository command is used to add a name a repository url to your +const addRepoDesc = `The add repository command is used to add a name a repository url to your chart repository list. The repository url must begin with a valid protocoal These include https, http, and gs. @@ -95,10 +89,6 @@ func addRepo(c *cli.Context) error { } name := args[0] repoURL := args[1] - valid := IsValidURL(repoURL) - if !valid { - return errors.New(repoURL + " is not a valid REPOSITOTY_URL argument \n" + addRepoDesc) - } payload, _ := json.Marshal(repo.Repo{URL: repoURL, Name: name}) msg := "" if _, err := NewClient(c).Post(chartRepoPath, payload, &msg); err != nil { @@ -138,22 +128,3 @@ func removeRepo(c *cli.Context) error { format.Msg(name + " has been removed.\n") return nil } - -// IsValidURL checks if the string is a valid URL. -// This was inspired by the IsURL function in govalidator https://github.com/asaskevich/govalidator -func IsValidURL(str string) bool { - if str == "" || len(str) >= 2083 || len(str) <= 3 || strings.HasPrefix(str, ".") { - return false - } - u, err := url.Parse(str) - if err != nil { - return false - } - if strings.HasPrefix(u.Host, ".") { - return false - } - if u.Host == "" && (u.Path != "" && !strings.Contains(u.Path, ".")) { - return false - } - return regexp.MustCompile(URL).MatchString(str) -} diff --git a/cmd/helm/repository_test.go b/cmd/helm/repository_test.go deleted file mode 100644 index d3327599b..000000000 --- a/cmd/helm/repository_test.go +++ /dev/null @@ -1,37 +0,0 @@ -/* -Copyright 2016 The Kubernetes Authors All rights reserved. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package main - -import ( - "testing" -) - -func TestHasValidPrefix(t *testing.T) { - tests := map[string]bool{ - "https://host/bucket": true, - "http://host/bucket": true, - "gs://bucket-name": true, - "charts": false, - } - - for url, expectedResult := range tests { - result := IsValidURL(url) - if result != expectedResult { - t.Errorf("Expected: %v, Got: %v for url: %v", expectedResult, result, url) - } - } -} diff --git a/cmd/manager/chartrepos.go b/cmd/manager/chartrepos.go index 64ae8ddd8..2c056e4dd 100644 --- a/cmd/manager/chartrepos.go +++ b/cmd/manager/chartrepos.go @@ -57,6 +57,9 @@ func addChartRepoHandlerFunc(w http.ResponseWriter, r *http.Request, c *router.C httputil.BadRequest(w, r, err) return nil } + if err := repo.ValidateRepoURL(cr.URL); err != nil { + return err + } if err := c.Manager.AddRepo(cr); err != nil { httputil.BadRequest(w, r, err) diff --git a/pkg/repo/repo.go b/pkg/repo/repo.go index ce2a29cb3..ce3e9c3c9 100644 --- a/pkg/repo/repo.go +++ b/pkg/repo/repo.go @@ -17,10 +17,16 @@ limitations under the License. package repo import ( + "errors" "fmt" "net/url" + "regexp" + "strings" ) +// URL is the url pattern used to check if a given repo url is valid +const URL string = `^((http|gs|https?):\/\/)?(\S+(:\S*)?@)?((([1-9]\d?|1\d\d|2[01]\d|22[0-3])(\.(1?\d{1,2}|2[0-4]\d|25[0-5])){2}(?:\.([0-9]\d?|1\d\d|2[0-4]\d|25[0-4]))|(([a-zA-Z0-9]+([-\.][a-zA-Z0-9]+)*)|((www\.)?))?(([a-zA-Z\x{00a1}-\x{ffff}0-9]+-?-?)*[a-zA-Z\x{00a1}-\x{ffff}0-9]+)(?:\.([a-zA-Z\x{00a1}-\x{ffff}]{2,}))?))(:(\d{1,5}))?((\/|\?|#)[^\s]*)?$` + // NewRepo takes params and returns a IRepo func NewRepo(URL, credentialName, repoName, repoFormat, repoType string) (IRepo, error) { return newRepo(URL, credentialName, repoName, ERepoFormat(repoFormat), ERepoType(repoType)) @@ -113,3 +119,26 @@ func validateRepo(tr IRepo, wantURL, wantCredentialName string, wantFormat ERepo return nil } + +// ValidateRepoURL checks if the string is a valid URL. +// This was inspired by the IsURL function in govalidator https://github.com/asaskevich/govalidator +func ValidateRepoURL(str string) error { + err := errors.New("Invalid Repository URL") + if str == "" || len(str) >= 2083 || len(str) <= 3 || strings.HasPrefix(str, ".") { + return err + } + u, err := url.Parse(str) + if err != nil { + return err + } + if strings.HasPrefix(u.Host, ".") { + return err + } + if u.Host == "" && (u.Path != "" && !strings.Contains(u.Path, ".")) { + return err + } + if !regexp.MustCompile(URL).MatchString(str) { + return err + } + return nil +} diff --git a/pkg/repo/repo_test.go b/pkg/repo/repo_test.go index 1415f7332..9bce229e3 100644 --- a/pkg/repo/repo_test.go +++ b/pkg/repo/repo_test.go @@ -65,3 +65,26 @@ func TestInvalidRepoFormat(t *testing.T) { t.Fatalf("expected error did not occur for invalid format") } } + +func TestValidateRepoURL(t *testing.T) { + validURLs := []string{ + "https://host/bucket", + "http://host/bucket", + "gs://bucket-name", + } + invalidURLs := []string{"charts"} + + for _, url := range validURLs { + err := ValidateRepoURL(url) + if err != nil { + t.Fatalf("Expected repo url: %v to be valid but threw an error") + } + } + + for _, url := range invalidURLs { + err := ValidateRepoURL(url) + if err == nil { + t.Fatalf("Expected repo url: %v to be invalid but did not throw an error") + } + } +}