Add support for SVG icons (#631)

cmd/chart-repo/utils.go

@@ -310,20 +310,35 @@ func fetchAndImportIcon(dbSession datastore.Session, c chart) error {
 		return fmt.Errorf("%d %s", res.StatusCode, c.Icon)
 	}
 
-	orig, err := imaging.Decode(res.Body)
-	if err != nil {
-		return err
-	}
+	b := []byte{}
+	contentType := ""
+	if strings.Contains(res.Header.Get("Content-Type"), "image/svg") {
+		// if the icon is a SVG file simply read it
+		b, err = ioutil.ReadAll(res.Body)
+		if err != nil {
+			return err
+		}
+		contentType = res.Header.Get("Content-Type")
+	} else {
+		// if the icon is in any other format try to convert it to PNG
+		orig, err := imaging.Decode(res.Body)
+		if err != nil {
+			log.WithFields(log.Fields{"name": c.Name}).WithError(err).Error("failed to decode icon")
+			return err
+		}
 
-	// TODO: make this configurable?
-	icon := imaging.Fit(orig, 160, 160, imaging.Lanczos)
+		// TODO: make this configurable?
+		icon := imaging.Fit(orig, 160, 160, imaging.Lanczos)
 
-	var b bytes.Buffer
-	imaging.Encode(&b, icon, imaging.PNG)
+		var buf bytes.Buffer
+		imaging.Encode(&buf, icon, imaging.PNG)
+		b = buf.Bytes()
+		contentType = "image/png"
+	}
 
 	db, closer := dbSession.DB()
 	defer closer()
-	return db.C(chartCollection).UpdateId(c.ID, bson.M{"$set": bson.M{"raw_icon": b.Bytes()}})
+	return db.C(chartCollection).UpdateId(c.ID, bson.M{"$set": bson.M{"raw_icon": b, "icon_content_type": contentType}})
 }
 
 func fetchAndImportFiles(dbSession datastore.Session, name string, r repo, cv chartVersion) error {

cmd/chart-repo/utils_test.go

@@ -109,6 +109,16 @@ func (h *goodIconClient) Do(req *http.Request) (*http.Response, error) {
 	return w.Result(), nil
 }
 
+type svgIconClient struct{}
+
+func (h *svgIconClient) Do(req *http.Request) (*http.Response, error) {
+	w := httptest.NewRecorder()
+	w.Write([]byte("foo"))
+	res := w.Result()
+	res.Header.Set("Content-Type", "image/svg")
+	return res, nil
+}
+
 type goodTarballClient struct {
 	c          chart
 	skipReadme bool
@@ -360,7 +370,21 @@ func Test_fetchAndImportIcon(t *testing.T) {
 		c := charts[0]
 		m := mock.Mock{}
 		dbSession := mockstore.NewMockSession(&m)
-		m.On("UpdateId", c.ID, bson.M{"$set": bson.M{"raw_icon": iconBytes()}}).Return(nil)
+		m.On("UpdateId", c.ID, bson.M{"$set": bson.M{"raw_icon": iconBytes(), "icon_content_type": "image/png"}}).Return(nil)
+		assert.NoErr(t, fetchAndImportIcon(dbSession, c))
+		m.AssertExpectations(t)
+	})
+
+	t.Run("valid SVG icon", func(t *testing.T) {
+		netClient = &svgIconClient{}
+		c := chart{
+			ID:   "foo",
+			Icon: "https://foo/bar/logo.svg",
+			Repo: repo{},
+		}
+		m := mock.Mock{}
+		dbSession := mockstore.NewMockSession(&m)
+		m.On("UpdateId", c.ID, bson.M{"$set": bson.M{"raw_icon": []byte("foo"), "icon_content_type": "image/svg"}}).Return(nil)
 		assert.NoErr(t, fetchAndImportIcon(dbSession, c))
 		m.AssertExpectations(t)
 	})

cmd/chartsvc/handler.go

@@ -266,6 +266,12 @@ func getChartIcon(w http.ResponseWriter, req *http.Request, params Params) {
 		return
 	}
 
+	if chart.IconContentType != "" {
+		// Force the Content-Type header because the autogenerated type does not work for
+		// image/svg+xml. It is detected as plain text
+		w.Header().Set("Content-Type", chart.IconContentType)
+	}
+
 	w.Write(chart.RawIcon)
 }
 
@@ -407,7 +413,7 @@ func chartVersionAttributes(cid string, cv models.ChartVersion) models.ChartVers
 
 func chartAttributes(c models.Chart) models.Chart {
 	if c.RawIcon != nil {
-		c.Icon = pathPrefix + "/assets/" + c.ID + "/logo-160x160-fit.png"
+		c.Icon = pathPrefix + "/assets/" + c.ID + "/logo"
 	} else {
 		// If the icon wasn't processed, it is either not set or invalid
 		c.Icon = ""

cmd/chartsvc/handler_test.go

@@ -64,7 +64,7 @@ func Test_chartAttributes(t *testing.T) {
 			ID: "stable/wordpress",
 		}},
 		{"chart has a icon", models.Chart{
-			ID: "repo/mychart", RawIcon: iconBytes(),
+			ID: "repo/mychart", RawIcon: iconBytes(), IconContentType: "image/svg",
 		}},
 	}
 
@@ -76,7 +76,8 @@ func Test_chartAttributes(t *testing.T) {
 			if len(tt.chart.RawIcon) == 0 {
 				assert.Equal(t, len(c.Icon), 0, "icon url should be undefined")
 			} else {
-				assert.Equal(t, c.Icon, pathPrefix+"/assets/"+tt.chart.ID+"/logo-160x160-fit.png", "the icon url should be the same")
+				assert.Equal(t, c.Icon, pathPrefix+"/assets/"+tt.chart.ID+"/logo", "the icon url should be the same")
+				assert.Equal(t, c.IconContentType, tt.chart.IconContentType, "the icon content type should be the same")
 			}
 		})
 	}
@@ -551,7 +552,7 @@ func Test_getChartIcon(t *testing.T) {
 		{
 			"chart has icon",
 			nil,
-			models.Chart{ID: "my-repo/my-chart", RawIcon: iconBytes()},
+			models.Chart{ID: "my-repo/my-chart", RawIcon: iconBytes(), IconContentType: "image/png"},
 			http.StatusOK,
 		},
 		{
@@ -560,6 +561,12 @@ func Test_getChartIcon(t *testing.T) {
 			models.Chart{ID: "my-repo/my-chart"},
 			http.StatusNotFound,
 		},
+		{
+			"chart has icon with custom type",
+			nil,
+			models.Chart{ID: "my-repo/my-chart", RawIcon: iconBytes(), IconContentType: "image/svg"},
+			http.StatusOK,
+		},
 	}
 
 	for _, tt := range tests {
@@ -576,7 +583,7 @@ func Test_getChartIcon(t *testing.T) {
 			}
 
 			w := httptest.NewRecorder()
-			req := httptest.NewRequest("GET", "/assets/"+tt.chart.ID+"/logo-160x160-fit.png", nil)
+			req := httptest.NewRequest("GET", "/assets/"+tt.chart.ID+"/logo", nil)
 			parts := strings.Split(tt.chart.ID, "/")
 			params := Params{
 				"repo":      parts[0],
@@ -589,6 +596,7 @@ func Test_getChartIcon(t *testing.T) {
 			assert.Equal(t, tt.wantCode, w.Code, "http status code should match")
 			if tt.wantCode == http.StatusOK {
 				assert.Equal(t, w.Body.Bytes(), tt.chart.RawIcon, "raw icon data should match")
+				assert.Equal(t, w.Header().Get("Content-Type"), tt.chart.IconContentType, "icon content type should match")
 			}
 		})
 	}

cmd/chartsvc/main.go

@@ -54,6 +54,8 @@ func setupRoutes() http.Handler {
 	apiv1.Methods("GET").Path("/charts/{repo}/{chartName}").Handler(WithParams(getChart))
 	apiv1.Methods("GET").Path("/charts/{repo}/{chartName}/versions").Handler(WithParams(listChartVersions))
 	apiv1.Methods("GET").Path("/charts/{repo}/{chartName}/versions/{version}").Handler(WithParams(getChartVersion))
+	apiv1.Methods("GET").Path("/assets/{repo}/{chartName}/logo").Handler(WithParams(getChartIcon))
+	// Maintain the logo-160x160-fit.png endpoint for backward compatibility /assets/{repo}/{chartName}/logo should be used instead
 	apiv1.Methods("GET").Path("/assets/{repo}/{chartName}/logo-160x160-fit.png").Handler(WithParams(getChartIcon))
 	apiv1.Methods("GET").Path("/assets/{repo}/{chartName}/versions/{version}/README.md").Handler(WithParams(getChartVersionReadme))
 	apiv1.Methods("GET").Path("/assets/{repo}/{chartName}/versions/{version}/values.yaml").Handler(WithParams(getChartVersionValues))

cmd/chartsvc/main_test.go

@@ -342,7 +342,7 @@ func Test_GetChartIcon(t *testing.T) {
 				})
 			}
 
-			res, err := http.Get(ts.URL + pathPrefix + "/assets/" + tt.chart.ID + "/logo-160x160-fit.png")
+			res, err := http.Get(ts.URL + pathPrefix + "/assets/" + tt.chart.ID + "/logo")
 			assert.NoError(t, err)
 			defer res.Body.Close()
 

cmd/chartsvc/models/chart.go

@@ -30,17 +30,18 @@ type Repo struct {
 
 // Chart is a higher-level representation of a chart package
 type Chart struct {
-	ID            string             `json:"-" bson:"_id"`
-	Name          string             `json:"name"`
-	Repo          Repo               `json:"repo"`
-	Description   string             `json:"description"`
-	Home          string             `json:"home"`
-	Keywords      []string           `json:"keywords"`
-	Maintainers   []chart.Maintainer `json:"maintainers"`
-	Sources       []string           `json:"sources"`
-	Icon          string             `json:"icon"`
-	RawIcon       []byte             `json:"-" bson:"raw_icon"`
-	ChartVersions []ChartVersion     `json:"-"`
+	ID              string             `json:"-" bson:"_id"`
+	Name            string             `json:"name"`
+	Repo            Repo               `json:"repo"`
+	Description     string             `json:"description"`
+	Home            string             `json:"home"`
+	Keywords        []string           `json:"keywords"`
+	Maintainers     []chart.Maintainer `json:"maintainers"`
+	Sources         []string           `json:"sources"`
+	Icon            string             `json:"icon"`
+	RawIcon         []byte             `json:"-" bson:"raw_icon"`
+	IconContentType string             `json:"-" bson:"icon_content_type,omitempty"`
+	ChartVersions   []ChartVersion     `json:"-"`
 }
 
 // ChartVersion is a representation of a specific version of a chart