Allow exploring templates in private Helm repos (#1201)

Closes #1190

Signed-off-by: Sergio Castaño Arteaga <tegioz@icloud.com>
Signed-off-by: Cintia Sanchez Garcia <cynthiasg@icloud.com>
Co-authored-by: Sergio Castaño Arteaga <tegioz@icloud.com>
Co-authored-by: Cintia Sanchez Garcia <cynthiasg@icloud.com>

cmd/hub/handlers/handlers.go

@@ -89,7 +89,7 @@ func Setup(ctx context.Context, cfg *viper.Viper, svc *Services) (*Handlers, err
 		Organizations: org.NewHandlers(svc.OrganizationManager, svc.Authorizer, cfg),
 		Users:         userHandlers,
 		Repositories:  repo.NewHandlers(svc.RepositoryManager),
-		Packages:      pkg.NewHandlers(svc.PackageManager, cfg, &http.Client{}),
+		Packages:      pkg.NewHandlers(svc.PackageManager, svc.RepositoryManager, cfg, &http.Client{}),
 		Subscriptions: subscription.NewHandlers(svc.SubscriptionManager),
 		Webhooks:      webhook.NewHandlers(svc.WebhookManager),
 		APIKeys:       apikey.NewHandlers(svc.APIKeyManager),

cmd/hub/handlers/pkg/handlers.go

@@ -25,19 +25,26 @@ import (
 // Handlers represents a group of http handlers in charge of handling packages
 // operations.
 type Handlers struct {
-	pkgManager hub.PackageManager
+	pkgManager  hub.PackageManager
-	cfg        *viper.Viper
+	repoManager hub.RepositoryManager
-	logger     zerolog.Logger
+	cfg         *viper.Viper
-	hc         hub.HTTPClient
+	logger      zerolog.Logger
+	hc          hub.HTTPClient
 }
 
 // NewHandlers creates a new Handlers instance.
-func NewHandlers(pkgManager hub.PackageManager, cfg *viper.Viper, hc hub.HTTPClient) *Handlers {
+func NewHandlers(
+	pkgManager hub.PackageManager,
+	repoManager hub.RepositoryManager,
+	cfg *viper.Viper,
+	hc hub.HTTPClient,
+) *Handlers {
 	return &Handlers{
-		pkgManager: pkgManager,
+		pkgManager:  pkgManager,
-		cfg:        cfg,
+		repoManager: repoManager,
-		logger:     log.With().Str("handlers", "pkg").Logger(),
+		cfg:         cfg,
-		hc:         hc,
+		logger:      log.With().Str("handlers", "pkg").Logger(),
+		hc:          hc,
 	}
 }
 
@@ -97,6 +104,16 @@ func (h *Handlers) GetChartTemplates(w http.ResponseWriter, r *http.Request) {
 	// Download chart package from remote source
 	req, _ := http.NewRequest("GET", p.ContentURL, nil)
 	req = req.WithContext(r.Context())
+	if p.Repository.Private {
+		// Get credentials and set them in request if the repository is private
+		repo, err := h.repoManager.GetByID(r.Context(), p.Repository.RepositoryID, true)
+		if err != nil {
+			h.logger.Error().Err(err).Str("method", "GetChartTemplates").Send()
+			helpers.RenderErrorJSON(w, err)
+			return
+		}
+		req.SetBasicAuth(repo.AuthUser, repo.AuthPass)
+	}
 	resp, err := h.hc.Do(req)
 	if err != nil {
 		h.logger.Error().Err(err).Str("method", "GetChartTemplates").Send()

cmd/hub/handlers/pkg/handlers_test.go

@@ -15,6 +15,7 @@ import (
 	"github.com/artifacthub/hub/cmd/hub/handlers/helpers"
 	"github.com/artifacthub/hub/internal/hub"
 	"github.com/artifacthub/hub/internal/pkg"
+	"github.com/artifacthub/hub/internal/repo"
 	"github.com/artifacthub/hub/internal/tests"
 	"github.com/go-chi/chi"
 	"github.com/rs/zerolog"
@@ -167,6 +168,15 @@ func TestGetChartTemplates(t *testing.T) {
 			URL:  "https://repo.url",
 		},
 	}
+	p2 := &hub.Package{
+		ContentURL: contentURL,
+		Repository: &hub.Repository{
+			RepositoryID: "repo2",
+			Kind:         hub.Helm,
+			URL:          "https://repo2.url",
+			Private:      true,
+		},
+	}
 
 	t.Run("error getting package", func(t *testing.T) {
 		t.Parallel()
@@ -227,6 +237,23 @@ func TestGetChartTemplates(t *testing.T) {
 		}
 	})
 
+	t.Run("error downloading repository", func(t *testing.T) {
+		t.Parallel()
+		w := httptest.NewRecorder()
+		r, _ := http.NewRequest("GET", "/", nil)
+		r = r.WithContext(context.WithValue(r.Context(), chi.RouteCtxKey, rctx))
+
+		hw := newHandlersWrapper()
+		hw.pm.On("Get", r.Context(), getPkgInput).Return(p2, nil)
+		hw.rm.On("GetByID", r.Context(), p2.Repository.RepositoryID, true).Return(nil, tests.ErrFake)
+		hw.h.GetChartTemplates(w, r)
+		resp := w.Result()
+		defer resp.Body.Close()
+
+		assert.Equal(t, http.StatusInternalServerError, resp.StatusCode)
+		hw.assertExpectations(t)
+	})
+
 	t.Run("error downloading chart package", func(t *testing.T) {
 		testCases := []struct {
 			description string
@@ -317,6 +344,37 @@ func TestGetChartTemplates(t *testing.T) {
 		assert.Equal(t, expectedData, data)
 		hw.assertExpectations(t)
 	})
+
+	t.Run("package templates returned successfully (private repository)", func(t *testing.T) {
+		t.Parallel()
+		w := httptest.NewRecorder()
+		r, _ := http.NewRequest("GET", "/", nil)
+		r = r.WithContext(context.WithValue(r.Context(), chi.RouteCtxKey, rctx))
+
+		hw := newHandlersWrapper()
+		hw.pm.On("Get", r.Context(), getPkgInput).Return(p2, nil)
+		hw.rm.On("GetByID", r.Context(), p2.Repository.RepositoryID, true).Return(&hub.Repository{
+			AuthUser: "user",
+			AuthPass: "pass",
+		}, nil)
+		tgzReq, _ := http.NewRequest("GET", contentURL, nil)
+		tgzReq = tgzReq.WithContext(r.Context())
+		tgzReq.SetBasicAuth("user", "pass")
+		f, _ := os.Open("testdata/pkg1-1.0.0.tgz")
+		hw.hc.On("Do", tgzReq).Return(&http.Response{
+			Body:       f,
+			StatusCode: http.StatusOK,
+		}, nil)
+		hw.h.GetChartTemplates(w, r)
+		resp := w.Result()
+		defer resp.Body.Close()
+		data, _ := ioutil.ReadAll(resp.Body)
+
+		assert.Equal(t, http.StatusOK, resp.StatusCode)
+		expectedData := []byte(`{"templates":[{"name":"templates/template.yaml","data":"a2V5OiB7eyAuVmFsdWVzLmtleSB9fQo="}],"values":{"key":"value"}}`)
+		assert.Equal(t, expectedData, data)
+		hw.assertExpectations(t)
+	})
 }
 
 func TestGetHarborReplicationDump(t *testing.T) {
@@ -1126,6 +1184,7 @@ func TestBuildPackageURL(t *testing.T) {
 
 type handlersWrapper struct {
 	pm *pkg.ManagerMock
+	rm *repo.ManagerMock
 	hc *tests.HTTPClientMock
 	h  *Handlers
 }
@@ -1134,16 +1193,19 @@ func newHandlersWrapper() *handlersWrapper {
 	cfg := viper.New()
 	cfg.Set("server.baseURL", "baseURL")
 	pm := &pkg.ManagerMock{}
+	rm := &repo.ManagerMock{}
 	hc := &tests.HTTPClientMock{}
 
 	return &handlersWrapper{
 		pm: pm,
+		rm: rm,
 		hc: hc,
-		h:  NewHandlers(pm, cfg, hc),
+		h:  NewHandlers(pm, rm, cfg, hc),
 	}
 }
 
 func (hw *handlersWrapper) assertExpectations(t *testing.T) {
 	hw.pm.AssertExpectations(t)
+	hw.rm.AssertExpectations(t)
 	hw.hc.AssertExpectations(t)
 }

internal/notification/worker_test.go

@@ -111,7 +111,7 @@ func TestWorker(t *testing.T) {
 		sw := newServicesWrapper()
 		sw.db.On("Begin", sw.ctx).Return(sw.tx, nil)
 		sw.nm.On("GetPending", sw.ctx, sw.tx).Return(n3, nil)
-		sw.rm.On("GetByID", sw.ctx, "repositoryID").Return(nil, tests.ErrFake)
+		sw.rm.On("GetByID", sw.ctx, "repositoryID", false).Return(nil, tests.ErrFake)
 		sw.tx.On("Rollback", sw.ctx).Return(nil)
 
 		w := NewWorker(sw.svc, sw.cache, "", sw.hc)
@@ -139,7 +139,7 @@ func TestWorker(t *testing.T) {
 		sw := newServicesWrapper()
 		sw.db.On("Begin", sw.ctx).Return(sw.tx, nil)
 		sw.nm.On("GetPending", sw.ctx, sw.tx).Return(n3, nil)
-		sw.rm.On("GetByID", sw.ctx, "repositoryID").Return(r, nil)
+		sw.rm.On("GetByID", sw.ctx, "repositoryID", false).Return(r, nil)
 		sw.es.On("SendEmail", mock.Anything).Return(tests.ErrFake)
 		sw.nm.On("UpdateStatus", sw.ctx, sw.tx, n3.NotificationID, true, tests.ErrFake).Return(nil)
 		sw.tx.On("Commit", sw.ctx).Return(nil)
@@ -169,7 +169,7 @@ func TestWorker(t *testing.T) {
 		sw := newServicesWrapper()
 		sw.db.On("Begin", sw.ctx).Return(sw.tx, nil)
 		sw.nm.On("GetPending", sw.ctx, sw.tx).Return(n3, nil)
-		sw.rm.On("GetByID", sw.ctx, "repositoryID").Return(r, nil)
+		sw.rm.On("GetByID", sw.ctx, "repositoryID", false).Return(r, nil)
 		sw.es.On("SendEmail", mock.Anything).Return(nil)
 		sw.nm.On("UpdateStatus", sw.ctx, sw.tx, n3.NotificationID, true, nil).Return(nil)
 		sw.tx.On("Commit", sw.ctx).Return(nil)

internal/repo/mock.go

@@ -101,7 +101,7 @@ func (m *ManagerMock) GetByID(
 	repositoryID string,
 	includeCredentials bool,
 ) (*hub.Repository, error) {
-	args := m.Called(ctx, repositoryID)
+	args := m.Called(ctx, repositoryID, includeCredentials)
 	data, _ := args.Get(0).(*hub.Repository)
 	return data, args.Error(1)
 }

web/src/layout/package/chartTemplates/index.test.tsx

@@ -243,4 +243,24 @@ describe('ChartTemplatesModal', () => {
       });
     });
   });
+
+  describe('does not render component', () => {
+    it('when repo is not Helm kind', () => {
+      const { container } = render(
+        <Router>
+          <ChartTemplatesModal {...defaultProps} repoKind={RepositoryKind.Krew} visibleChartTemplates />
+        </Router>
+      );
+
+      expect(container).toBeEmptyDOMElement();
+      expect(mockHistoryReplace).toHaveBeenCalledTimes(1);
+      expect(mockHistoryReplace).toHaveBeenCalledWith({
+        search: '',
+        state: {
+          fromStarredPage: undefined,
+          searchUrlReferer: undefined,
+        },
+      });
+    });
+  });
 });

web/src/layout/package/chartTemplates/index.tsx

@@ -1,4 +1,4 @@
-import { compact, isNull, isUndefined, uniq } from 'lodash';
+import { compact, isNull, uniq } from 'lodash';
 import React, { useEffect, useRef, useState } from 'react';
 import { ImInsertTemplate } from 'react-icons/im';
 import { MdClose } from 'react-icons/md';
@@ -19,7 +19,6 @@ interface Props {
   packageId: string;
   version: string;
   repoKind: RepositoryKind;
-  private?: boolean;
   visibleChartTemplates: boolean;
   visibleTemplate?: string;
   searchUrlReferer?: SearchFiltersURL;
@@ -129,8 +128,10 @@ const ChartTemplatesModal = (props: Props) => {
   };
 
   useEffect(() => {
-    if (props.visibleChartTemplates && !openStatus && (isUndefined(props.private) || !props.private)) {
+    if (props.visibleChartTemplates && !openStatus && props.repoKind === RepositoryKind.Helm) {
       onOpenModal();
+    } else {
+      cleanUrl();
     }
   }, []); /* eslint-disable-line react-hooks/exhaustive-deps */
 
@@ -212,7 +213,6 @@ const ChartTemplatesModal = (props: Props) => {
           data-testid="tmplModalBtn"
           className={`btn btn-secondary btn-sm text-nowrap ${props.btnClassName}`}
           onClick={onOpenModal}
-          disabled={!isUndefined(props.private) && props.private}
         >
           <div className="d-flex flex-row align-items-center justify-content-center">
             {isLoading ? (

web/src/layout/package/index.tsx

@@ -549,7 +549,6 @@ const PackageView = (props: Props) => {
                               packageId={detail.packageId}
                               version={detail.version!}
                               repoKind={detail.repository.kind}
-                              private={detail.repository.private}
                               visibleChartTemplates={
                                 !isUndefined(props.visibleModal) && props.visibleModal === 'template'
                               }