Merge pull request #203 from marccampbell/safe-split-url

Handle invalid github urls on the init command

pkg/specs/chart.go

@@ -49,11 +49,11 @@ func (g *GithubClient) GetChartAndReadmeContents(ctx context.Context, chartURLSt
 	if err != nil {
 		return err
 	}
-	chartPath := chartURL.Path
-	splitPath := strings.Split(chartPath, "/")
-	owner := splitPath[1]
-	repo := splitPath[2]
-	path := strings.Join(splitPath[3:], "/")
+
+	owner, repo, path, err := decodeGitHubUrl(chartURL.Path)
+	if err != nil {
+		return err
+	}
 
 	debug.Log("event", "checkExists", "path", constants.KustomizeHelmPath)
 	saveDirExists, err := g.fs.Exists(constants.KustomizeHelmPath)
@@ -168,3 +168,20 @@ func (r *Resolver) ResolveChartMetadata(ctx context.Context, path string) (api.H
 	md.Readme = string(readme)
 	return md, nil
 }
+
+func decodeGitHubUrl(chartPath string) (string, string, string, error) {
+	splitPath := strings.Split(chartPath, "/")
+
+	if len(splitPath) < 3 {
+		return "", "", "", errors.Wrapf(errors.New("unable to decode github url"), chartPath)
+	}
+
+	owner := splitPath[1]
+	repo := splitPath[2]
+	path := ""
+	if len(splitPath) > 3 {
+		path = strings.Join(splitPath[3:], "/")
+	}
+
+	return owner, repo, path, nil
+}

pkg/specs/chart_test.go

@@ -86,7 +86,7 @@ var _ = Describe("GithubClient", func() {
 
 		Context("With a url not prefixed with http", func() {
 			It("should fetch and persist README.md and Chart.yaml", func() {
-				validGitURLWithoutPrefix := "github.com/o/r/"
+				validGitURLWithoutPrefix := "github.com/o/r"
 				mockFs := afero.Afero{Fs: afero.NewMemMapFs()}
 				gitClient := GithubClient{
 					client: client,
@@ -111,6 +111,46 @@ var _ = Describe("GithubClient", func() {
 			})
 		})
 	})
+
+	Describe("decodeGitHubUrl", func() {
+		Context("With a valid github url", func() {
+			It("should decode a valid url without a path", func() {
+				chartPath := "github.com/o/r"
+				o, r, p, err := decodeGitHubUrl(chartPath)
+				Expect(err).NotTo(HaveOccurred())
+
+				Expect(o).To(Equal("o"))
+				Expect(r).To(Equal("r"))
+				Expect(p).To(Equal(""))
+			})
+
+			It("should decode a valid url with a path", func() {
+				chartPath := "github.com/o/r/stable/chart"
+				o, r, p, err := decodeGitHubUrl(chartPath)
+				Expect(err).NotTo(HaveOccurred())
+
+				Expect(o).To(Equal("o"))
+				Expect(r).To(Equal("r"))
+				Expect(p).To(Equal("stable/chart"))
+			})
+		})
+
+		Context("With an invalid github url", func() {
+			It("should failed to decode a url without a path", func() {
+				chartPath := "github.com"
+				_, _, _, err := decodeGitHubUrl(chartPath)
+				Expect(err).NotTo(BeNil())
+				Expect(err.Error()).To(Equal("github.com: unable to decode github url"))
+			})
+
+			It("should failed to decode a url with a path", func() {
+				chartPath := "github.com/o"
+				_, _, _, err := decodeGitHubUrl(chartPath)
+				Expect(err).NotTo(BeNil())
+				Expect(err.Error()).To(Equal("github.com/o: unable to decode github url"))
+			})
+		})
+	})
 })
 
 var _ = AfterSuite(func() {