Merge pull request #1026 from alecmerdler/bugzilla-1748771

Bug 1748771: Default Icon for PackageManifest Subresource

Author: openshift-merge-robot

pkg/package-server/storage/subresources.go

@@ -46,22 +46,29 @@ func (s *LogoStorage) Connect(ctx context.Context, name string, options runtime.
 
 		namespace := genericreq.NamespaceValue(ctx)
 		pkg, err := s.prov.Get(namespace, name)
-		if err != nil || pkg == nil || len(pkg.Status.Channels) == 0 || len(pkg.Status.Channels[0].CurrentCSVDesc.Icon) == 0 {
+		if err != nil || pkg == nil {
 			w.WriteHeader(http.StatusNotFound)
 			return
 		}
+		imgBytes, mimeType, etag := func() ([]byte, string, string) {
+			if len(pkg.Status.Channels) == 0 || len(pkg.Status.Channels[0].CurrentCSVDesc.Icon) == 0 {
+				return []byte(defaultIcon), "image/svg+xml", ""
+			} else {
+				data := pkg.Status.Channels[0].CurrentCSVDesc.Icon[0].Base64Data
+				mimeType := pkg.Status.Channels[0].CurrentCSVDesc.Icon[0].Mediatype
+				etag := `"` + strings.Join([]string{name, pkg.Status.Channels[0].Name, pkg.Status.Channels[0].CurrentCSV}, ".") + `"`
 
-		data := pkg.Status.Channels[0].CurrentCSVDesc.Icon[0].Base64Data
-		mimeType := pkg.Status.Channels[0].CurrentCSVDesc.Icon[0].Mediatype
-		etag := `"` + strings.Join([]string{name, pkg.Status.Channels[0].Name, pkg.Status.Channels[0].CurrentCSV}, ".") + `"`
+				reader := base64.NewDecoder(base64.StdEncoding, strings.NewReader(data))
+				imgBytes, _ := ioutil.ReadAll(reader)
 
-		reader := base64.NewDecoder(base64.StdEncoding, strings.NewReader(data))
-		imgBytes, err := ioutil.ReadAll(reader)
+				return imgBytes, mimeType, etag
+			}
+		}()
 
 		w.Header().Set("Content-Type", mimeType)
 		w.Header().Set("Content-Length", strconv.Itoa(len(imgBytes)))
 		w.Header().Set("Etag", etag)
-		_, err = w.Write(imgBytes)
+		w.Write(imgBytes)
 	}
 
 	return handler, nil
@@ -90,3 +97,7 @@ func (s *LogoStorage) ProducesMIMETypes(verb string) []string {
 func (s *LogoStorage) ProducesObject(verb string) interface{} {
 	return ""
 }
+
+const defaultIcon string = `
+<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 258.51 258.51"><defs><style>.cls-1{fill:#d1d1d1;}.cls-2{fill:#8d8d8f;}</style></defs><title>Asset 4</title><g id="Layer_2" data-name="Layer 2"><g id="Layer_1-2" data-name="Layer 1"><path class="cls-1" d="M129.25,20A109.1,109.1,0,0,1,206.4,206.4,109.1,109.1,0,1,1,52.11,52.11,108.45,108.45,0,0,1,129.25,20m0-20h0C58.16,0,0,58.16,0,129.25H0c0,71.09,58.16,129.26,129.25,129.26h0c71.09,0,129.26-58.17,129.26-129.26h0C258.51,58.16,200.34,0,129.25,0Z"/><path class="cls-2" d="M177.54,103.41H141.66L154.9,65.76c1.25-4.4-2.33-8.76-7.21-8.76H102.93a7.32,7.32,0,0,0-7.4,6l-10,69.61c-.59,4.17,2.89,7.89,7.4,7.89h36.9L115.55,197c-1.12,4.41,2.48,8.55,7.24,8.55a7.58,7.58,0,0,0,6.47-3.48L184,113.85C186.86,109.24,183.29,103.41,177.54,103.41Z"/></g></g></svg>
+`

pkg/package-server/storage/subresources_test.go

@@ -13,26 +13,16 @@ import (
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/package-server/provider"
 )
 
-type fakeProvider struct{}
+type fakeProvider struct {
+	packages []*operators.PackageManifest
 
-var getCalls = 0
+	getCalls int
+}
 
 func (p *fakeProvider) Get(namespace, name string) (*operators.PackageManifest, error) {
-	getCalls = getCalls + 1
+	p.getCalls = p.getCalls + 1
 
-	return &operators.PackageManifest{
-		Status: operators.PackageManifestStatus{
-			Channels: []operators.PackageChannel{
-				{
-					Name:       "stable",
-					CurrentCSV: "csv-a",
-					CurrentCSVDesc: operators.CSVDescription{
-						Icon: []operators.Icon{{Mediatype: "image/png", Base64Data: iconData}},
-					},
-				},
-			},
-		},
-	}, nil
+	return p.packages[0], nil
 }
 
 func (p *fakeProvider) List(namespace string) (*operators.PackageManifestList, error) {
@@ -43,6 +33,7 @@ var _ provider.PackageManifestProvider = &fakeProvider{}
 
 func TestLogoStorageConnect(t *testing.T) {
 	provider := fakeProvider{}
+	provider.packages = append(provider.packages, testPackage())
 	ctx, cancel := context.WithCancel(context.TODO())
 	defer cancel()
 
@@ -70,11 +61,50 @@ func TestLogoStorageConnect(t *testing.T) {
 
 	handler.ServeHTTP(cachedRR, cachedReq)
 	require.Equal(t, http.StatusNotModified, cachedRR.Code)
-	require.Equal(t, 1, getCalls, "PackageManifestProvider.Get() should not be called for cached icon")
+	require.Equal(t, 1, provider.getCalls, "PackageManifestProvider.Get() should not be called for cached icon")
 
 	handler.ServeHTTP(rr, req)
 	require.Equal(t, http.StatusOK, rr.Code)
-	require.Equal(t, 2, getCalls, "PackageManifestProvider.Get() should be called again to fetch icon")
+	require.Equal(t, 2, provider.getCalls, "PackageManifestProvider.Get() should be called again to fetch icon")
+}
+
+func TestLogoStorageConnectDefaultIcon(t *testing.T) {
+	provider := fakeProvider{}
+	provider.packages = append(provider.packages, testPackage())
+	provider.packages[0].Status.Channels = nil
+	ctx, cancel := context.WithCancel(context.TODO())
+	defer cancel()
+
+	storage := NewLogoStorage(v1.Resource("packagemanifests/icon"), &provider)
+
+	rr := httptest.NewRecorder()
+	req, err := http.NewRequest("GET", "", nil)
+	require.NoError(t, err)
+
+	handler, err := storage.Connect(ctx, "pkg-a", nil, nil)
+	require.NoError(t, err)
+
+	handler.ServeHTTP(rr, req)
+	require.Equal(t, http.StatusOK, rr.Code)
+	require.Equal(t, "image/svg+xml", rr.Header().Get("Content-Type"))
+	require.NotNil(t, rr.Body)
+	require.Equal(t, 1, provider.getCalls, "PackageManifestProvider.Get() should be called for missing icon")
+}
+
+func testPackage() *operators.PackageManifest {
+	return &operators.PackageManifest{
+		Status: operators.PackageManifestStatus{
+			Channels: []operators.PackageChannel{
+				{
+					Name:       "stable",
+					CurrentCSV: "csv-a",
+					CurrentCSVDesc: operators.CSVDescription{
+						Icon: []operators.Icon{{Mediatype: "image/png", Base64Data: iconData}},
+					},
+				},
+			},
+		},
+	}
 }
 
 const iconData = `