From deaa9929a3c9b9a4ce8ef8cf01ce292a45ed3828 Mon Sep 17 00:00:00 2001
From: Mateusz Szostok <mateusz@capact.io>
Date: Thu, 17 Mar 2022 13:05:34 +0100
Subject: [PATCH 1/5] Implement Helm release storage backend

---
 cmd/helm-storage-backend/main.go         |   3 +-
 internal/helm-storage-backend/common.go  |   3 +
 internal/helm-storage-backend/helm.go    |  32 +++++
 internal/helm-storage-backend/release.go | 160 ++++++++++++++++++++++-
 4 files changed, 190 insertions(+), 8 deletions(-)
 create mode 100644 internal/helm-storage-backend/common.go
 create mode 100644 internal/helm-storage-backend/helm.go

diff --git a/cmd/helm-storage-backend/main.go b/cmd/helm-storage-backend/main.go
index 3b61714e4..ff3b8d761 100644
--- a/cmd/helm-storage-backend/main.go
+++ b/cmd/helm-storage-backend/main.go
@@ -74,7 +74,8 @@ func main() {
 	var handler storage_backend.StorageBackendServer
 	switch cfg.Mode {
 	case HelmReleaseMode:
-		handler = helm_storage_backend.NewReleaseHandler(logger, helmCfgFlags)
+		handler, err = helm_storage_backend.NewReleaseHandler(logger, helmCfgFlags)
+		exitOnError(err, "while creating Helm Release backend storage")
 	case HelmTemplateMode:
 		handler = helm_storage_backend.NewTemplateHandler(logger, helmCfgFlags)
 	default:
diff --git a/internal/helm-storage-backend/common.go b/internal/helm-storage-backend/common.go
new file mode 100644
index 000000000..588d916b9
--- /dev/null
+++ b/internal/helm-storage-backend/common.go
@@ -0,0 +1,3 @@
+package helmstoragebackend
+
+const defaultHelmDriver = "secrets"
diff --git a/internal/helm-storage-backend/helm.go b/internal/helm-storage-backend/helm.go
new file mode 100644
index 000000000..0474ef526
--- /dev/null
+++ b/internal/helm-storage-backend/helm.go
@@ -0,0 +1,32 @@
+package helmstoragebackend
+
+import (
+	"github.com/pkg/errors"
+	"helm.sh/helm/v3/pkg/action"
+	"k8s.io/cli-runtime/pkg/genericclioptions"
+
+	"capact.io/capact/internal/ptr"
+)
+
+// NewHelmGet create a new Helm release get client.
+func NewHelmGet(flags *genericclioptions.ConfigFlags, driver, ns string) (*action.Get, error) {
+	actionConfig := new(action.Configuration)
+	helmCfg := &genericclioptions.ConfigFlags{
+		APIServer:   flags.APIServer,
+		Insecure:    flags.Insecure,
+		CAFile:      flags.CAFile,
+		BearerToken: flags.BearerToken,
+		Namespace:   ptr.String(ns),
+	}
+
+	debugLog := func(format string, v ...interface{}) {
+		// noop
+	}
+
+	err := actionConfig.Init(helmCfg, ns, driver, debugLog)
+	if err != nil {
+		return nil, errors.Wrap(err, "while initializing Helm configuration")
+	}
+
+	return action.NewGet(actionConfig), nil
+}
diff --git a/internal/helm-storage-backend/release.go b/internal/helm-storage-backend/release.go
index 3649be15e..3625ac84b 100644
--- a/internal/helm-storage-backend/release.go
+++ b/internal/helm-storage-backend/release.go
@@ -2,32 +2,178 @@ package helmstoragebackend
 
 import (
 	"context"
+	"encoding/json"
+	"fmt"
 
-	pb "capact.io/capact/pkg/hub/api/grpc/storage_backend"
+	"github.com/pkg/errors"
 	"go.uber.org/zap"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
+	"helm.sh/helm/v3/pkg/storage/driver"
 	"k8s.io/cli-runtime/pkg/genericclioptions"
+
+	"capact.io/capact/internal/ptr"
+	pb "capact.io/capact/pkg/hub/api/grpc/storage_backend"
 )
 
 var _ pb.StorageBackendServer = &ReleaseHandler{}
 
+type (
+	ReleaseDetails struct {
+		// Name specifies installed Helm release name.
+		Name string `json:"name"`
+		// Namespace specifies in which Kubernetes Namespace Helm release was installed.
+		Namespace string `json:"namespace"`
+		// Chart holds Helm Chart details.
+		Chart ChartDetails `json:"chart"`
+	}
+	ChartDetails struct {
+		// Name specifies Helm Chart name.
+		Name string `json:"name"`
+		// Version specifies the exact chart version.
+		Version string `json:"version"`
+		// Repo specifies URL where to locate the requested chart.
+		Repo string `json:"repo"`
+	}
+
+	ReleaseContext struct {
+		// Name specifies Helm release name for a given request.
+		Name string `json:"name"`
+		// Namespace specifies in which Kubernetes Namespace Helm release is located.
+		Namespace string `json:"namespace"`
+		// ChartLocation specifies Helm Chart location.
+		ChartLocation string `json:"chartLocation"`
+		// Driver specifies drivers used for storing the Helm release.
+		Driver *string `json:"driver,omitempty"`
+	}
+)
+
 // ReleaseHandler handles incoming requests to the Helm release storage backend gRPC server.
 type ReleaseHandler struct {
 	pb.UnimplementedStorageBackendServer
 
-	log *zap.Logger
+	log          *zap.Logger
+	helmCfgFlags *genericclioptions.ConfigFlags
 }
 
 // NewReleaseHandler returns new ReleaseHandler.
-func NewReleaseHandler(log *zap.Logger, helmCfgFlags *genericclioptions.ConfigFlags) *ReleaseHandler {
+func NewReleaseHandler(log *zap.Logger, helmCfgFlags *genericclioptions.ConfigFlags) (*ReleaseHandler, error) {
 	return &ReleaseHandler{
-		log: log,
-	}
+		log:          log,
+		helmCfgFlags: helmCfgFlags,
+	}, nil
+}
+
+func (h *ReleaseHandler) OnCreate(_ context.Context, req *pb.OnCreateRequest) (*pb.OnCreateResponse, error) {
+	return &pb.OnCreateResponse{}, h.checkIfHelmReleaseExist(req.TypeInstanceId, req.Context)
 }
 
 // GetValue returns a value for a given TypeInstance.
-func (h *ReleaseHandler) GetValue(_ context.Context, _ *pb.GetValueRequest) (*pb.GetValueResponse, error) {
+func (h *ReleaseHandler) GetValue(_ context.Context, req *pb.GetValueRequest) (*pb.GetValueResponse, error) {
 	h.log.Info("Getting value")
+
+	releaseContext, err := h.getReleaseContext(req.Context)
+	if err != nil {
+		return nil, err
+	}
+
+	helmGet, err := NewHelmGet(h.helmCfgFlags, *releaseContext.Driver, releaseContext.Namespace)
+	if err != nil {
+		return nil, h.internalError(errors.Wrap(err, "while creating Helm get release client"))
+	}
+
+	release, err := helmGet.Run(releaseContext.Name)
+	switch {
+	case err == nil:
+	case errors.Is(err, driver.ErrReleaseNotFound):
+		return nil, status.Error(codes.NotFound, fmt.Sprintf("Helm release '%s/%s' for TypeInstance '%s' was not found", releaseContext.Namespace, releaseContext.Name, req.TypeInstanceId))
+	default:
+		return nil, h.internalError(errors.Wrap(err, "while getting Helm release"))
+	}
+
+	releaseData := ReleaseDetails{
+		Name:      release.Name,
+		Namespace: release.Namespace,
+		Chart: ChartDetails{
+			Name:    release.Chart.Metadata.Name,
+			Version: release.Chart.Metadata.Version,
+			Repo:    releaseContext.ChartLocation,
+		},
+	}
+
+	value, err := json.Marshal(releaseData)
+	if err != nil {
+		return nil, errors.Wrap(err, "while marshaling response value")
+	}
+
 	return &pb.GetValueResponse{
-		Value: []byte(`{"handler": "release"}`),
+		Value: value,
 	}, nil
 }
+
+func (h *ReleaseHandler) OnUpdate(_ context.Context, req *pb.OnUpdateRequest) (*pb.OnUpdateResponse, error) {
+	return &pb.OnUpdateResponse{}, h.checkIfHelmReleaseExist(req.TypeInstanceId, req.Context)
+}
+
+func (h *ReleaseHandler) OnDelete(ctx context.Context, request *pb.OnDeleteRequest) (*pb.OnDeleteResponse, error) {
+	// currently, we are not sure whether the release should be deleted or this should be more an information
+	// that someone wants to deregister this TypeInstance. For now, delete is NOP.
+	return &pb.OnDeleteResponse{}, nil
+}
+
+func (h *ReleaseHandler) GetLockedBy(_ context.Context, _ *pb.GetLockedByRequest) (*pb.GetLockedByResponse, error) {
+	return &pb.GetLockedByResponse{}, nil
+}
+
+func (h *ReleaseHandler) OnLock(ctx context.Context, request *pb.OnLockRequest) (*pb.OnLockResponse, error) {
+	return &pb.OnLockResponse{}, nil
+}
+
+func (h *ReleaseHandler) OnUnlock(ctx context.Context, request *pb.OnUnlockRequest) (*pb.OnUnlockResponse, error) {
+	return &pb.OnUnlockResponse{}, nil
+}
+
+func (h *ReleaseHandler) getReleaseContext(contextBytes []byte) (*ReleaseContext, error) {
+	if len(contextBytes) == 0 {
+		return nil, nil
+	}
+
+	var ctx ReleaseContext
+	err := json.Unmarshal(contextBytes, &ctx)
+	if err != nil {
+		return nil, h.internalError(errors.Wrap(err, "while unmarshaling context"))
+	}
+
+	if ctx.Driver == nil {
+		ctx.Driver = ptr.String(defaultHelmDriver)
+	}
+
+	return &ctx, nil
+}
+
+func (h *ReleaseHandler) checkIfHelmReleaseExist(ti string, ctx []byte) error {
+	releaseContext, err := h.getReleaseContext(ctx)
+	if err != nil {
+		return err
+	}
+
+	helmGet, err := NewHelmGet(h.helmCfgFlags, *releaseContext.Driver, releaseContext.Namespace)
+	if err != nil {
+		return h.internalError(errors.Wrap(err, "while creating Helm get release client"))
+	}
+
+	_, err = helmGet.Run(releaseContext.Name)
+	switch {
+	case err == nil:
+	case errors.Is(err, driver.ErrReleaseNotFound):
+		return status.Error(codes.NotFound, fmt.Sprintf("Helm release '%s/%s' for TypeInstance '%s' was not found", releaseContext.Namespace, releaseContext.Name, ti))
+	default:
+		return h.internalError(errors.Wrap(err, "while checking if Helm release exists"))
+	}
+
+	return nil
+}
+
+func (h *ReleaseHandler) internalError(err error) error {
+	return status.Error(codes.Internal, err.Error())
+}

From d404ff87f950f80104e1f7d9a8673b382524c9fb Mon Sep 17 00:00:00 2001
From: Mateusz Szostok <mateusz@capact.io>
Date: Thu, 17 Mar 2022 16:40:18 +0100
Subject: [PATCH 2/5] Add test coverage for getValue

---
 internal/helm-storage-backend/helm.go         |   7 +-
 internal/helm-storage-backend/release.go      |  37 ++-
 internal/helm-storage-backend/release_test.go | 235 ++++++++++++++++++
 3 files changed, 267 insertions(+), 12 deletions(-)
 create mode 100644 internal/helm-storage-backend/release_test.go

diff --git a/internal/helm-storage-backend/helm.go b/internal/helm-storage-backend/helm.go
index 0474ef526..de42e9136 100644
--- a/internal/helm-storage-backend/helm.go
+++ b/internal/helm-storage-backend/helm.go
@@ -8,8 +8,9 @@ import (
 	"capact.io/capact/internal/ptr"
 )
 
-// NewHelmGet create a new Helm release get client.
-func NewHelmGet(flags *genericclioptions.ConfigFlags, driver, ns string) (*action.Get, error) {
+type actionConfigurationProducerFn func(flags *genericclioptions.ConfigFlags, driver string, ns string) (*action.Configuration, error)
+
+func ActionConfigurationProducer(flags *genericclioptions.ConfigFlags, driver, ns string) (*action.Configuration, error) {
 	actionConfig := new(action.Configuration)
 	helmCfg := &genericclioptions.ConfigFlags{
 		APIServer:   flags.APIServer,
@@ -28,5 +29,5 @@ func NewHelmGet(flags *genericclioptions.ConfigFlags, driver, ns string) (*actio
 		return nil, errors.Wrap(err, "while initializing Helm configuration")
 	}
 
-	return action.NewGet(actionConfig), nil
+	return actionConfig, nil
 }
diff --git a/internal/helm-storage-backend/release.go b/internal/helm-storage-backend/release.go
index 3625ac84b..eecbdeae5 100644
--- a/internal/helm-storage-backend/release.go
+++ b/internal/helm-storage-backend/release.go
@@ -9,6 +9,7 @@ import (
 	"go.uber.org/zap"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
+	"helm.sh/helm/v3/pkg/action"
 	"helm.sh/helm/v3/pkg/storage/driver"
 	"k8s.io/cli-runtime/pkg/genericclioptions"
 
@@ -18,6 +19,8 @@ import (
 
 var _ pb.StorageBackendServer = &ReleaseHandler{}
 
+const latestRevisionIndicator = 0
+
 type (
 	ReleaseDetails struct {
 		// Name specifies installed Helm release name.
@@ -52,15 +55,17 @@ type (
 type ReleaseHandler struct {
 	pb.UnimplementedStorageBackendServer
 
-	log          *zap.Logger
-	helmCfgFlags *genericclioptions.ConfigFlags
+	log                         *zap.Logger
+	helmCfgFlags                *genericclioptions.ConfigFlags
+	actionConfigurationProducer actionConfigurationProducerFn
 }
 
 // NewReleaseHandler returns new ReleaseHandler.
 func NewReleaseHandler(log *zap.Logger, helmCfgFlags *genericclioptions.ConfigFlags) (*ReleaseHandler, error) {
 	return &ReleaseHandler{
-		log:          log,
-		helmCfgFlags: helmCfgFlags,
+		log:                         log,
+		helmCfgFlags:                helmCfgFlags,
+		actionConfigurationProducer: ActionConfigurationProducer,
 	}, nil
 }
 
@@ -77,11 +82,15 @@ func (h *ReleaseHandler) GetValue(_ context.Context, req *pb.GetValueRequest) (*
 		return nil, err
 	}
 
-	helmGet, err := NewHelmGet(h.helmCfgFlags, *releaseContext.Driver, releaseContext.Namespace)
+	helmGet, err := h.newHelmGet(h.helmCfgFlags, *releaseContext.Driver, releaseContext.Namespace)
 	if err != nil {
 		return nil, h.internalError(errors.Wrap(err, "while creating Helm get release client"))
 	}
 
+	// NOTE: req.resourceVersion is ignored on purpose.
+	// Based on our contract we always return the latest Helm release revision.
+	helmGet.Version = latestRevisionIndicator
+
 	release, err := helmGet.Run(releaseContext.Name)
 	switch {
 	case err == nil:
@@ -115,7 +124,7 @@ func (h *ReleaseHandler) OnUpdate(_ context.Context, req *pb.OnUpdateRequest) (*
 	return &pb.OnUpdateResponse{}, h.checkIfHelmReleaseExist(req.TypeInstanceId, req.Context)
 }
 
-func (h *ReleaseHandler) OnDelete(ctx context.Context, request *pb.OnDeleteRequest) (*pb.OnDeleteResponse, error) {
+func (h *ReleaseHandler) OnDelete(_ context.Context, _ *pb.OnDeleteRequest) (*pb.OnDeleteResponse, error) {
 	// currently, we are not sure whether the release should be deleted or this should be more an information
 	// that someone wants to deregister this TypeInstance. For now, delete is NOP.
 	return &pb.OnDeleteResponse{}, nil
@@ -125,11 +134,11 @@ func (h *ReleaseHandler) GetLockedBy(_ context.Context, _ *pb.GetLockedByRequest
 	return &pb.GetLockedByResponse{}, nil
 }
 
-func (h *ReleaseHandler) OnLock(ctx context.Context, request *pb.OnLockRequest) (*pb.OnLockResponse, error) {
+func (h *ReleaseHandler) OnLock(_ context.Context, _ *pb.OnLockRequest) (*pb.OnLockResponse, error) {
 	return &pb.OnLockResponse{}, nil
 }
 
-func (h *ReleaseHandler) OnUnlock(ctx context.Context, request *pb.OnUnlockRequest) (*pb.OnUnlockResponse, error) {
+func (h *ReleaseHandler) OnUnlock(_ context.Context, _ *pb.OnUnlockRequest) (*pb.OnUnlockResponse, error) {
 	return &pb.OnUnlockResponse{}, nil
 }
 
@@ -157,7 +166,7 @@ func (h *ReleaseHandler) checkIfHelmReleaseExist(ti string, ctx []byte) error {
 		return err
 	}
 
-	helmGet, err := NewHelmGet(h.helmCfgFlags, *releaseContext.Driver, releaseContext.Namespace)
+	helmGet, err := h.newHelmGet(h.helmCfgFlags, *releaseContext.Driver, releaseContext.Namespace)
 	if err != nil {
 		return h.internalError(errors.Wrap(err, "while creating Helm get release client"))
 	}
@@ -174,6 +183,16 @@ func (h *ReleaseHandler) checkIfHelmReleaseExist(ti string, ctx []byte) error {
 	return nil
 }
 
+// newHelmGet create a new Helm release get client.
+func (h *ReleaseHandler) newHelmGet(flags *genericclioptions.ConfigFlags, driver, ns string) (*action.Get, error) {
+	actionConfig, err := h.actionConfigurationProducer(flags, driver, ns)
+	if err != nil {
+		return nil, err
+	}
+
+	return action.NewGet(actionConfig), nil
+}
+
 func (h *ReleaseHandler) internalError(err error) error {
 	return status.Error(codes.Internal, err.Error())
 }
diff --git a/internal/helm-storage-backend/release_test.go b/internal/helm-storage-backend/release_test.go
new file mode 100644
index 000000000..37f2b9d73
--- /dev/null
+++ b/internal/helm-storage-backend/release_test.go
@@ -0,0 +1,235 @@
+package helmstoragebackend
+
+import (
+	"context"
+	"encoding/json"
+	"errors"
+	"fmt"
+	"io/ioutil"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"helm.sh/helm/v3/pkg/action"
+	"helm.sh/helm/v3/pkg/chart"
+	kubefake "helm.sh/helm/v3/pkg/kube/fake"
+	"helm.sh/helm/v3/pkg/release"
+	"helm.sh/helm/v3/pkg/storage"
+	"helm.sh/helm/v3/pkg/storage/driver"
+	"helm.sh/helm/v3/pkg/time"
+	"k8s.io/cli-runtime/pkg/genericclioptions"
+
+	"capact.io/capact/internal/logger"
+	"capact.io/capact/internal/ptr"
+	pb "capact.io/capact/pkg/hub/api/grpc/storage_backend"
+)
+
+func TestRelease_GetValue_Success(t *testing.T) {
+	tests := []struct {
+		name string
+
+		givenDriver          *string
+		givenTypeInstanceID  *string
+		givenResourceVersion int
+		expectedDriver       string
+	}{
+		{
+			name:                "should use default driver and return the latest release",
+			givenTypeInstanceID: ptr.String("123"),
+			givenDriver:         nil,
+			expectedDriver:      "secrets",
+		},
+		{
+			name:                "should use configmap driver and return the latest release",
+			givenTypeInstanceID: ptr.String("123"),
+			givenDriver:         ptr.String("configmaps"),
+			expectedDriver:      "configmaps",
+		},
+		{
+			name:                 "should ignore resourceVersion and return the latest release",
+			givenTypeInstanceID:  ptr.String("123"),
+			givenResourceVersion: 42, // should be ignored
+			expectedDriver:       "secrets",
+		},
+		{
+			name:           "should return the latest release even if TypeInstance's id and revision are not specified",
+			expectedDriver: "secrets",
+		},
+	}
+	for _, test := range tests {
+		test := test
+		t.Run(test.name, func(t *testing.T) {
+			t.Parallel()
+			// given
+			const (
+				releaseName      = "test-release"
+				releaseNamespace = "test-namespace"
+				chartLocation    = "http://example.com/charts"
+			)
+			expHelmRelease := fixHelmRelease(releaseName, releaseNamespace)
+			expFlags := &genericclioptions.ConfigFlags{ClusterName: ptr.String("testing")}
+			mockConfigurationProducer := mockConfigurationProducer(t, expHelmRelease, expFlags, test.expectedDriver)
+
+			givenReq := &pb.GetValueRequest{
+				TypeInstanceId: ptr.StringPtrToString(test.givenTypeInstanceID),
+				Context: mustMarshal(t, ReleaseContext{
+					Name:          releaseName,
+					Namespace:     releaseNamespace,
+					ChartLocation: chartLocation,
+					Driver:        test.givenDriver,
+				}),
+			}
+
+			expResponse := &pb.GetValueResponse{
+				Value: mustMarshal(t, ReleaseDetails{
+					Name:      expHelmRelease.Name,
+					Namespace: expHelmRelease.Namespace,
+					Chart: ChartDetails{
+						Name:    expHelmRelease.Chart.Metadata.Name,
+						Version: expHelmRelease.Chart.Metadata.Version,
+						Repo:    chartLocation,
+					},
+				}),
+			}
+
+			svc, err := NewReleaseHandler(logger.Noop(), expFlags)
+			svc.actionConfigurationProducer = mockConfigurationProducer
+			require.NoError(t, err)
+
+			// when
+			outVal, gotErr := svc.GetValue(context.Background(), givenReq)
+
+			// then
+			assert.NoError(t, gotErr)
+			assert.Equal(t, outVal, expResponse)
+		})
+	}
+}
+
+func TestRelease_GetValue_Failures(t *testing.T) {
+	// globally given
+	const (
+		releaseName      = "test-release"
+		releaseNamespace = "test-namespace"
+	)
+	tests := []struct {
+		name string
+
+		request       *pb.GetValueRequest
+		internalError error
+
+		expErrMsg string
+	}{
+		{
+			name: "should return not found error if release name is wrong",
+			request: &pb.GetValueRequest{
+				TypeInstanceId: "123",
+				Context: mustMarshal(t, ReleaseContext{
+					Name:          "other-release",
+					Namespace:     releaseNamespace,
+					ChartLocation: "http://example.com/charts",
+				}),
+			},
+			expErrMsg: "rpc error: code = NotFound desc = Helm release 'test-namespace/other-release' for TypeInstance '123' was not found",
+		},
+		{
+			name: "should return not found error if release namespace is wrong",
+			request: &pb.GetValueRequest{
+				TypeInstanceId: "123",
+				Context: mustMarshal(t, ReleaseContext{
+					Name:          releaseName,
+					Namespace:     "other-ns",
+					ChartLocation: "http://example.com/charts",
+				}),
+			},
+			expErrMsg: "rpc error: code = NotFound desc = Helm release 'other-ns/test-release' for TypeInstance '123' was not found",
+		},
+		{
+			name: "should return internal error",
+			request: &pb.GetValueRequest{
+				TypeInstanceId: "123",
+				Context: mustMarshal(t, ReleaseContext{
+					Name:          releaseName,
+					Namespace:     "other-ns",
+					ChartLocation: "http://example.com/charts",
+				}),
+			},
+			internalError: errors.New("internal error"),
+			expErrMsg:     "rpc error: code = Internal desc = while creating Helm get release client: internal error",
+		},
+	}
+	for _, test := range tests {
+		test := test
+		t.Run(test.name, func(t *testing.T) {
+			t.Parallel()
+			// given
+			expHelmRelease := fixHelmRelease(releaseName, releaseNamespace)
+			expFlags := &genericclioptions.ConfigFlags{ClusterName: ptr.String("testing")}
+
+			mockConfigurationProducer := func(inputFlags *genericclioptions.ConfigFlags, inputDriver, inputNs string) (*action.Configuration, error) {
+				if test.internalError != nil {
+					return nil, test.internalError
+				}
+				producer := mockConfigurationProducer(t, expHelmRelease, expFlags, "secrets")
+				return producer(inputFlags, inputDriver, inputNs)
+			}
+
+			svc, err := NewReleaseHandler(logger.Noop(), expFlags)
+			svc.actionConfigurationProducer = mockConfigurationProducer
+			require.NoError(t, err)
+
+			// when
+			outVal, gotErr := svc.GetValue(context.Background(), test.request)
+
+			// then
+			assert.EqualError(t, gotErr, test.expErrMsg)
+			assert.Nil(t, outVal)
+		})
+	}
+}
+
+func mockConfigurationProducer(t *testing.T, expHelmRelease *release.Release, expFlags *genericclioptions.ConfigFlags, expDriver string) actionConfigurationProducerFn {
+	t.Helper()
+	inMemoryDriver := driver.NewMemory()
+	err := inMemoryDriver.Create("1", expHelmRelease)
+	require.NoError(t, err)
+
+	return func(inputFlags *genericclioptions.ConfigFlags, inputDriver, inputNs string) (*action.Configuration, error) {
+		assert.Equal(t, expFlags, inputFlags)
+		assert.Equal(t, expDriver, inputDriver)
+
+		inMemoryDriver.SetNamespace(inputNs)
+		return &action.Configuration{
+			Releases:   storage.Init(inMemoryDriver),
+			KubeClient: &kubefake.FailingKubeClient{PrintingKubeClient: kubefake.PrintingKubeClient{Out: ioutil.Discard}},
+		}, nil
+	}
+}
+
+func mustMarshal(t *testing.T, v interface{}) []byte {
+	t.Helper()
+	out, err := json.Marshal(v)
+	if err != nil {
+		t.Fatal(err)
+	}
+	return out
+}
+
+func fixHelmRelease(name, ns string) *release.Release {
+	now := time.Now()
+	return &release.Release{
+		Name:      name,
+		Namespace: ns,
+		Info: &release.Info{
+			FirstDeployed: now,
+			LastDeployed:  now,
+			Description:   "Named Release Stub",
+		},
+		Chart: &chart.Chart{
+			Metadata: &chart.Metadata{
+				Name:    fmt.Sprintf("%s-chart", name),
+				Version: "0.1.0",
+			},
+		},
+	}
+}

From e53abe115c24e46c7dcbb49a9aa7c01f1a6ee6fa Mon Sep 17 00:00:00 2001
From: Mateusz Szostok <mateusz@capact.io>
Date: Sun, 20 Mar 2022 18:51:55 +0100
Subject: [PATCH 3/5] Add more test coverage

---
 internal/helm-storage-backend/helm.go         |   1 +
 internal/helm-storage-backend/release.go      |  31 +-
 internal/helm-storage-backend/release_test.go | 264 +++++++++++++++++-
 pkg/runner/cloudsql/create.go                 |   3 +-
 4 files changed, 286 insertions(+), 13 deletions(-)

diff --git a/internal/helm-storage-backend/helm.go b/internal/helm-storage-backend/helm.go
index de42e9136..a64f25dd9 100644
--- a/internal/helm-storage-backend/helm.go
+++ b/internal/helm-storage-backend/helm.go
@@ -10,6 +10,7 @@ import (
 
 type actionConfigurationProducerFn func(flags *genericclioptions.ConfigFlags, driver string, ns string) (*action.Configuration, error)
 
+// ActionConfigurationProducer returns Configuration with a given input settings.
 func ActionConfigurationProducer(flags *genericclioptions.ConfigFlags, driver, ns string) (*action.Configuration, error) {
 	actionConfig := new(action.Configuration)
 	helmCfg := &genericclioptions.ConfigFlags{
diff --git a/internal/helm-storage-backend/release.go b/internal/helm-storage-backend/release.go
index eecbdeae5..4fa20f532 100644
--- a/internal/helm-storage-backend/release.go
+++ b/internal/helm-storage-backend/release.go
@@ -22,6 +22,7 @@ var _ pb.StorageBackendServer = &ReleaseHandler{}
 const latestRevisionIndicator = 0
 
 type (
+	// ReleaseDetails holds Helm release details.
 	ReleaseDetails struct {
 		// Name specifies installed Helm release name.
 		Name string `json:"name"`
@@ -30,6 +31,8 @@ type (
 		// Chart holds Helm Chart details.
 		Chart ChartDetails `json:"chart"`
 	}
+
+	// ChartDetails holds Helm chart details.
 	ChartDetails struct {
 		// Name specifies Helm Chart name.
 		Name string `json:"name"`
@@ -39,6 +42,7 @@ type (
 		Repo string `json:"repo"`
 	}
 
+	// ReleaseContext holds context used by Helm release storage backend.
 	ReleaseContext struct {
 		// Name specifies Helm release name for a given request.
 		Name string `json:"name"`
@@ -69,14 +73,17 @@ func NewReleaseHandler(log *zap.Logger, helmCfgFlags *genericclioptions.ConfigFl
 	}, nil
 }
 
+// OnCreate checks whether a given Helm release is accessible this storage backend.
 func (h *ReleaseHandler) OnCreate(_ context.Context, req *pb.OnCreateRequest) (*pb.OnCreateResponse, error) {
-	return &pb.OnCreateResponse{}, h.checkIfHelmReleaseExist(req.TypeInstanceId, req.Context)
+	if err := h.checkIfHelmReleaseExist(req.TypeInstanceId, req.Context); err != nil {
+		return nil, err
+	}
+
+	return &pb.OnCreateResponse{}, nil
 }
 
 // GetValue returns a value for a given TypeInstance.
 func (h *ReleaseHandler) GetValue(_ context.Context, req *pb.GetValueRequest) (*pb.GetValueResponse, error) {
-	h.log.Info("Getting value")
-
 	releaseContext, err := h.getReleaseContext(req.Context)
 	if err != nil {
 		return nil, err
@@ -120,33 +127,36 @@ func (h *ReleaseHandler) GetValue(_ context.Context, req *pb.GetValueRequest) (*
 	}, nil
 }
 
+// OnUpdate checks whether a given Helm release is accessible this storage backend.
 func (h *ReleaseHandler) OnUpdate(_ context.Context, req *pb.OnUpdateRequest) (*pb.OnUpdateResponse, error) {
-	return &pb.OnUpdateResponse{}, h.checkIfHelmReleaseExist(req.TypeInstanceId, req.Context)
+	if err := h.checkIfHelmReleaseExist(req.TypeInstanceId, req.Context); err != nil {
+		return nil, err
+	}
+	return &pb.OnUpdateResponse{}, nil
 }
 
+// OnDelete is NOP. Currently, we are not sure whether the release should be deleted or this should be more an information
+// that someone wants to deregister this TypeInstance.
 func (h *ReleaseHandler) OnDelete(_ context.Context, _ *pb.OnDeleteRequest) (*pb.OnDeleteResponse, error) {
-	// currently, we are not sure whether the release should be deleted or this should be more an information
-	// that someone wants to deregister this TypeInstance. For now, delete is NOP.
 	return &pb.OnDeleteResponse{}, nil
 }
 
+// GetLockedBy is NOP.
 func (h *ReleaseHandler) GetLockedBy(_ context.Context, _ *pb.GetLockedByRequest) (*pb.GetLockedByResponse, error) {
 	return &pb.GetLockedByResponse{}, nil
 }
 
+// OnLock is NOP.
 func (h *ReleaseHandler) OnLock(_ context.Context, _ *pb.OnLockRequest) (*pb.OnLockResponse, error) {
 	return &pb.OnLockResponse{}, nil
 }
 
+// OnUnlock is NOP.
 func (h *ReleaseHandler) OnUnlock(_ context.Context, _ *pb.OnUnlockRequest) (*pb.OnUnlockResponse, error) {
 	return &pb.OnUnlockResponse{}, nil
 }
 
 func (h *ReleaseHandler) getReleaseContext(contextBytes []byte) (*ReleaseContext, error) {
-	if len(contextBytes) == 0 {
-		return nil, nil
-	}
-
 	var ctx ReleaseContext
 	err := json.Unmarshal(contextBytes, &ctx)
 	if err != nil {
@@ -183,7 +193,6 @@ func (h *ReleaseHandler) checkIfHelmReleaseExist(ti string, ctx []byte) error {
 	return nil
 }
 
-// newHelmGet create a new Helm release get client.
 func (h *ReleaseHandler) newHelmGet(flags *genericclioptions.ConfigFlags, driver, ns string) (*action.Get, error) {
 	actionConfig, err := h.actionConfigurationProducer(flags, driver, ns)
 	if err != nil {
diff --git a/internal/helm-storage-backend/release_test.go b/internal/helm-storage-backend/release_test.go
index 37f2b9d73..330c2de59 100644
--- a/internal/helm-storage-backend/release_test.go
+++ b/internal/helm-storage-backend/release_test.go
@@ -62,7 +62,7 @@ func TestRelease_GetValue_Success(t *testing.T) {
 			t.Parallel()
 			// given
 			const (
-				releaseName      = "test-release"
+				releaseName      = "test-get-release"
 				releaseNamespace = "test-namespace"
 				chartLocation    = "http://example.com/charts"
 			)
@@ -188,6 +188,268 @@ func TestRelease_GetValue_Failures(t *testing.T) {
 	}
 }
 
+func TestRelease_OnCreate_Success(t *testing.T) {
+	// given
+	const (
+		releaseName      = "test-create-release"
+		releaseNamespace = "test-namespace"
+		releaseDriver    = "configmap"
+		chartLocation    = "http://example.com/charts"
+	)
+	expHelmRelease := fixHelmRelease(releaseName, releaseNamespace)
+	expFlags := &genericclioptions.ConfigFlags{ClusterName: ptr.String("testing")}
+	mockConfigurationProducer := mockConfigurationProducer(t, expHelmRelease, expFlags, releaseDriver)
+
+	givenReq := &pb.OnCreateRequest{
+		TypeInstanceId: "42",
+		Context: mustMarshal(t, ReleaseContext{
+			Name:          releaseName,
+			Namespace:     releaseNamespace,
+			ChartLocation: chartLocation,
+			Driver:        ptr.String(releaseDriver),
+		}),
+	}
+
+	svc, err := NewReleaseHandler(logger.Noop(), expFlags)
+	svc.actionConfigurationProducer = mockConfigurationProducer
+	require.NoError(t, err)
+
+	// when
+	gotOut, gotErr := svc.OnCreate(context.Background(), givenReq)
+
+	// then
+	assert.NoError(t, gotErr)
+	assert.Empty(t, gotOut)
+}
+
+func TestRelease_OnCreate_Failures(t *testing.T) {
+	// globally given
+	const (
+		releaseName      = "test-release"
+		releaseNamespace = "test-namespace"
+	)
+	tests := []struct {
+		name string
+
+		request       *pb.OnCreateRequest
+		internalError error
+
+		expErrMsg string
+	}{
+		{
+			name: "should return not found error if release name is wrong",
+			request: &pb.OnCreateRequest{
+				TypeInstanceId: "123",
+				Context: mustMarshal(t, ReleaseContext{
+					Name:          "other-release",
+					Namespace:     releaseNamespace,
+					ChartLocation: "http://example.com/charts",
+				}),
+			},
+			expErrMsg: "rpc error: code = NotFound desc = Helm release 'test-namespace/other-release' for TypeInstance '123' was not found",
+		},
+		{
+			name: "should return not found error if release namespace is wrong",
+			request: &pb.OnCreateRequest{
+				TypeInstanceId: "123",
+				Context: mustMarshal(t, ReleaseContext{
+					Name:          releaseName,
+					Namespace:     "other-ns",
+					ChartLocation: "http://example.com/charts",
+				}),
+			},
+			expErrMsg: "rpc error: code = NotFound desc = Helm release 'other-ns/test-release' for TypeInstance '123' was not found",
+		},
+	}
+	for _, test := range tests {
+		test := test
+		t.Run(test.name, func(t *testing.T) {
+			t.Parallel()
+			// given
+			expHelmRelease := fixHelmRelease(releaseName, releaseNamespace)
+			expFlags := &genericclioptions.ConfigFlags{ClusterName: ptr.String("testing")}
+
+			mockConfigurationProducer := func(inputFlags *genericclioptions.ConfigFlags, inputDriver, inputNs string) (*action.Configuration, error) {
+				if test.internalError != nil {
+					return nil, test.internalError
+				}
+				producer := mockConfigurationProducer(t, expHelmRelease, expFlags, "secrets")
+				return producer(inputFlags, inputDriver, inputNs)
+			}
+
+			svc, err := NewReleaseHandler(logger.Noop(), expFlags)
+			svc.actionConfigurationProducer = mockConfigurationProducer
+			require.NoError(t, err)
+
+			// when
+			outVal, gotErr := svc.OnCreate(context.Background(), test.request)
+
+			// then
+			assert.EqualError(t, gotErr, test.expErrMsg)
+			assert.Nil(t, outVal)
+		})
+	}
+}
+
+func TestRelease_OnUpdate_Success(t *testing.T) {
+	// given
+	const (
+		releaseName      = "test-update-release"
+		releaseNamespace = "test-namespace"
+		releaseDriver    = "configmap"
+		chartLocation    = "http://example.com/charts"
+	)
+	expHelmRelease := fixHelmRelease(releaseName, releaseNamespace)
+	expFlags := &genericclioptions.ConfigFlags{ClusterName: ptr.String("testing")}
+	mockConfigurationProducer := mockConfigurationProducer(t, expHelmRelease, expFlags, releaseDriver)
+
+	givenReq := &pb.OnUpdateRequest{
+		TypeInstanceId: "42",
+		Context: mustMarshal(t, ReleaseContext{
+			Name:          releaseName,
+			Namespace:     releaseNamespace,
+			ChartLocation: chartLocation,
+			Driver:        ptr.String(releaseDriver),
+		}),
+	}
+
+	svc, err := NewReleaseHandler(logger.Noop(), expFlags)
+	svc.actionConfigurationProducer = mockConfigurationProducer
+	require.NoError(t, err)
+
+	// when
+	gotOut, gotErr := svc.OnUpdate(context.Background(), givenReq)
+
+	// then
+	assert.NoError(t, gotErr)
+	assert.Empty(t, gotOut)
+}
+
+func TestRelease_OnUpdate_Failures(t *testing.T) {
+	// globally given
+	const (
+		releaseName      = "test-release"
+		releaseNamespace = "test-namespace"
+	)
+	tests := []struct {
+		name string
+
+		request       *pb.OnUpdateRequest
+		internalError error
+
+		expErrMsg string
+	}{
+		{
+			name: "should return not found error if release name is wrong",
+			request: &pb.OnUpdateRequest{
+				TypeInstanceId: "123",
+				Context: mustMarshal(t, ReleaseContext{
+					Name:          "other-release",
+					Namespace:     releaseNamespace,
+					ChartLocation: "http://example.com/charts",
+				}),
+			},
+			expErrMsg: "rpc error: code = NotFound desc = Helm release 'test-namespace/other-release' for TypeInstance '123' was not found",
+		},
+		{
+			name: "should return not found error if release namespace is wrong",
+			request: &pb.OnUpdateRequest{
+				TypeInstanceId: "123",
+				Context: mustMarshal(t, ReleaseContext{
+					Name:          releaseName,
+					Namespace:     "other-ns",
+					ChartLocation: "http://example.com/charts",
+				}),
+			},
+			expErrMsg: "rpc error: code = NotFound desc = Helm release 'other-ns/test-release' for TypeInstance '123' was not found",
+		},
+	}
+	for _, test := range tests {
+		test := test
+		t.Run(test.name, func(t *testing.T) {
+			t.Parallel()
+			// given
+			expHelmRelease := fixHelmRelease(releaseName, releaseNamespace)
+			expFlags := &genericclioptions.ConfigFlags{ClusterName: ptr.String("testing")}
+
+			mockConfigurationProducer := func(inputFlags *genericclioptions.ConfigFlags, inputDriver, inputNs string) (*action.Configuration, error) {
+				if test.internalError != nil {
+					return nil, test.internalError
+				}
+				producer := mockConfigurationProducer(t, expHelmRelease, expFlags, "secrets")
+				return producer(inputFlags, inputDriver, inputNs)
+			}
+
+			svc, err := NewReleaseHandler(logger.Noop(), expFlags)
+			svc.actionConfigurationProducer = mockConfigurationProducer
+			require.NoError(t, err)
+
+			// when
+			outVal, gotErr := svc.OnUpdate(context.Background(), test.request)
+
+			// then
+			assert.EqualError(t, gotErr, test.expErrMsg)
+			assert.Nil(t, outVal)
+		})
+	}
+}
+
+func TestRelease_NOP_Methods(t *testing.T) {
+	// globally given
+	tests := []struct {
+		name    string
+		handler func(ctx context.Context, svc *ReleaseHandler) (interface{}, error)
+	}{
+		{
+			name: "no operation for OnDelete",
+			handler: func(ctx context.Context, svc *ReleaseHandler) (interface{}, error) {
+				return svc.OnDelete(ctx, nil)
+			},
+		},
+		{
+			name: "no operation for GetLockedBy",
+			handler: func(ctx context.Context, svc *ReleaseHandler) (interface{}, error) {
+				return svc.GetLockedBy(ctx, nil)
+			},
+		},
+		{
+			name: "no operation for OnLock",
+			handler: func(ctx context.Context, svc *ReleaseHandler) (interface{}, error) {
+				return svc.OnLock(ctx, nil)
+			},
+		},
+		{
+			name: "no operation for OnUnlock",
+			handler: func(ctx context.Context, svc *ReleaseHandler) (interface{}, error) {
+				return svc.OnUnlock(ctx, nil)
+			},
+		},
+	}
+	for _, test := range tests {
+		test := test
+		t.Run(test.name, func(t *testing.T) {
+			t.Parallel()
+			// given
+			producerCalled := false
+			mockConfigurationProducer := func(_ *genericclioptions.ConfigFlags, _, _ string) (*action.Configuration, error) {
+				producerCalled = true
+				return nil, nil
+			}
+			svc, err := NewReleaseHandler(logger.Noop(), nil)
+			svc.actionConfigurationProducer = mockConfigurationProducer
+			require.NoError(t, err)
+
+			// when
+			outVal, gotErr := test.handler(context.Background(), svc)
+
+			// then
+			assert.NoError(t, gotErr)
+			assert.False(t, producerCalled)
+			assert.Empty(t, outVal)
+		})
+	}
+}
+
 func mockConfigurationProducer(t *testing.T, expHelmRelease *release.Release, expFlags *genericclioptions.ConfigFlags, expDriver string) actionConfigurationProducerFn {
 	t.Helper()
 	inMemoryDriver := driver.NewMemory()
diff --git a/pkg/runner/cloudsql/create.go b/pkg/runner/cloudsql/create.go
index 9812d68b5..cd86ea1c1 100644
--- a/pkg/runner/cloudsql/create.go
+++ b/pkg/runner/cloudsql/create.go
@@ -6,6 +6,7 @@ import (
 	"html/template"
 	"io/ioutil"
 	"os"
+	"path/filepath"
 	"time"
 
 	"capact.io/capact/pkg/runner"
@@ -174,7 +175,7 @@ func (a *createAction) createAdditionalOutputFile(path string, args *OutputArgs,
 		return errors.Wrap(err, "failed to load template")
 	}
 
-	fd, err := os.Create(path)
+	fd, err := os.Create(filepath.Clean(path))
 	if err != nil {
 		return errors.Wrap(err, "cannot open output file to write")
 	}

From e638a5bbc135c050e6c4c3bf1b816a1c7390ba53 Mon Sep 17 00:00:00 2001
From: Mateusz Szostok <mateusz@capact.io>
Date: Sun, 20 Mar 2022 18:52:05 +0100
Subject: [PATCH 4/5] Add showcase test - to revert

---
 internal/helm-storage-backend/release_test.go |  6 +-
 .../helm-storage-backend/showcase_test.go     | 96 +++++++++++++++++++
 2 files changed, 99 insertions(+), 3 deletions(-)
 create mode 100644 internal/helm-storage-backend/showcase_test.go

diff --git a/internal/helm-storage-backend/release_test.go b/internal/helm-storage-backend/release_test.go
index 330c2de59..71f168b7a 100644
--- a/internal/helm-storage-backend/release_test.go
+++ b/internal/helm-storage-backend/release_test.go
@@ -63,7 +63,7 @@ func TestRelease_GetValue_Success(t *testing.T) {
 			// given
 			const (
 				releaseName      = "test-get-release"
-				releaseNamespace = "test-namespace"
+				releaseNamespace = "test-get-namespace"
 				chartLocation    = "http://example.com/charts"
 			)
 			expHelmRelease := fixHelmRelease(releaseName, releaseNamespace)
@@ -192,7 +192,7 @@ func TestRelease_OnCreate_Success(t *testing.T) {
 	// given
 	const (
 		releaseName      = "test-create-release"
-		releaseNamespace = "test-namespace"
+		releaseNamespace = "test-create-namespace"
 		releaseDriver    = "configmap"
 		chartLocation    = "http://example.com/charts"
 	)
@@ -295,7 +295,7 @@ func TestRelease_OnUpdate_Success(t *testing.T) {
 	// given
 	const (
 		releaseName      = "test-update-release"
-		releaseNamespace = "test-namespace"
+		releaseNamespace = "test-update-namespace"
 		releaseDriver    = "configmap"
 		chartLocation    = "http://example.com/charts"
 	)
diff --git a/internal/helm-storage-backend/showcase_test.go b/internal/helm-storage-backend/showcase_test.go
new file mode 100644
index 000000000..5dc070ba2
--- /dev/null
+++ b/internal/helm-storage-backend/showcase_test.go
@@ -0,0 +1,96 @@
+package helmstoragebackend
+
+import (
+	"context"
+	"encoding/json"
+	"fmt"
+	"os"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	"google.golang.org/grpc"
+
+	pb "capact.io/capact/pkg/hub/api/grpc/storage_backend"
+)
+
+// To run this test, execute:
+// GRPC_SECRET_STORAGE_BACKEND_ADDR=":50051" go test ./internal/helm-storage-backend/... -run "^TestShowcase$" -v -count 1
+func TestShowcase(t *testing.T) {
+	srvAddr := os.Getenv("GRPC_SECRET_STORAGE_BACKEND_ADDR")
+	if srvAddr == "" {
+		t.Skip()
+	}
+
+	conn, err := grpc.Dial(srvAddr, grpc.WithInsecure())
+	require.NoError(t, err)
+
+	ctx := context.Background()
+	client := pb.NewStorageBackendClient(conn)
+
+	// ===== GET =====
+	out, err := client.GetValue(ctx, &pb.GetValueRequest{
+		TypeInstanceId: "42",
+		Context: mustMarshal(t, ReleaseContext{
+			Name:          "example-release",
+			Namespace:     "default",
+			ChartLocation: "https://charts.bitnami.com/bitnami",
+		})})
+
+	require.NoError(t, err)
+
+	details := &ReleaseDetails{}
+	require.NoError(t, json.Unmarshal(out.Value, details))
+
+	fmt.Printf("GetValue for valid release")
+	fmt.Printf("\t\t Name: %s\n", details.Name)
+	fmt.Printf("\t\t Namespace: %s\n", details.Namespace)
+	fmt.Printf("\t\t Chart.Name: %s\n", details.Chart.Name)
+	fmt.Printf("\t\t Chart.Version: %s\n", details.Chart.Version)
+	fmt.Printf("\t\t Chart.Repo: %s\n", details.Chart.Repo)
+
+	_, err = client.GetValue(ctx, &pb.GetValueRequest{
+		TypeInstanceId: "42",
+		Context: mustMarshal(t, ReleaseContext{
+			Name:          "fake-release",
+			Namespace:     "default",
+			ChartLocation: "https://charts.bitnami.com/bitnami",
+		})})
+
+	fmt.Printf("GetValue err if release doesn't exist: %v\n", err)
+
+	// ===== UPDATE =====
+	_, err = client.OnUpdate(ctx, &pb.OnUpdateRequest{
+		Context: mustMarshal(t, ReleaseContext{
+			Name:          "example-release",
+			Namespace:     "default",
+			ChartLocation: "https://charts.bitnami.com/bitnami",
+		})})
+	fmt.Printf("OnUpdate err if release exists: %v\n", err)
+
+	_, err = client.OnUpdate(ctx, &pb.OnUpdateRequest{
+		TypeInstanceId: "42",
+		Context: mustMarshal(t, ReleaseContext{
+			Name:          "fake-release",
+			Namespace:     "default",
+			ChartLocation: "https://charts.bitnami.com/bitnami",
+		})})
+	fmt.Printf("OnUpdate err if release doesn't exist: %v\n", err)
+
+	// ===== CREATE =====
+	_, err = client.OnCreate(ctx, &pb.OnCreateRequest{
+		Context: mustMarshal(t, ReleaseContext{
+			Name:          "example-release",
+			Namespace:     "default",
+			ChartLocation: "https://charts.bitnami.com/bitnami",
+		})})
+	fmt.Printf("OnCreate err if release exists: %v\n", err)
+
+	_, err = client.OnCreate(ctx, &pb.OnCreateRequest{
+		TypeInstanceId: "42",
+		Context: mustMarshal(t, ReleaseContext{
+			Name:          "fake-release",
+			Namespace:     "default",
+			ChartLocation: "https://charts.bitnami.com/bitnami",
+		})})
+	fmt.Printf("OnCreate err if release doesn't exist: %v\n", err)
+}

From 34a18123153a89611d044f7b9434a067a9114bd8 Mon Sep 17 00:00:00 2001
From: Mateusz Szostok <mateusz@capact.io>
Date: Mon, 21 Mar 2022 10:46:59 +0100
Subject: [PATCH 5/5] Apply suggestions after review

---
 internal/helm-storage-backend/common.go       |  3 -
 internal/helm-storage-backend/helm.go         |  2 +
 internal/helm-storage-backend/release.go      | 57 +++++++------------
 internal/helm-storage-backend/release_test.go | 43 ++++++++++----
 4 files changed, 57 insertions(+), 48 deletions(-)
 delete mode 100644 internal/helm-storage-backend/common.go

diff --git a/internal/helm-storage-backend/common.go b/internal/helm-storage-backend/common.go
deleted file mode 100644
index 588d916b9..000000000
--- a/internal/helm-storage-backend/common.go
+++ /dev/null
@@ -1,3 +0,0 @@
-package helmstoragebackend
-
-const defaultHelmDriver = "secrets"
diff --git a/internal/helm-storage-backend/helm.go b/internal/helm-storage-backend/helm.go
index a64f25dd9..a6d7fec9c 100644
--- a/internal/helm-storage-backend/helm.go
+++ b/internal/helm-storage-backend/helm.go
@@ -8,6 +8,8 @@ import (
 	"capact.io/capact/internal/ptr"
 )
 
+const defaultHelmDriver = "secrets"
+
 type actionConfigurationProducerFn func(flags *genericclioptions.ConfigFlags, driver string, ns string) (*action.Configuration, error)
 
 // ActionConfigurationProducer returns Configuration with a given input settings.
diff --git a/internal/helm-storage-backend/release.go b/internal/helm-storage-backend/release.go
index 4fa20f532..1b8e6be35 100644
--- a/internal/helm-storage-backend/release.go
+++ b/internal/helm-storage-backend/release.go
@@ -10,6 +10,7 @@ import (
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
 	"helm.sh/helm/v3/pkg/action"
+	"helm.sh/helm/v3/pkg/release"
 	"helm.sh/helm/v3/pkg/storage/driver"
 	"k8s.io/cli-runtime/pkg/genericclioptions"
 
@@ -75,7 +76,7 @@ func NewReleaseHandler(log *zap.Logger, helmCfgFlags *genericclioptions.ConfigFl
 
 // OnCreate checks whether a given Helm release is accessible this storage backend.
 func (h *ReleaseHandler) OnCreate(_ context.Context, req *pb.OnCreateRequest) (*pb.OnCreateResponse, error) {
-	if err := h.checkIfHelmReleaseExist(req.TypeInstanceId, req.Context); err != nil {
+	if _, _, err := h.fetchHelmRelease(req.TypeInstanceId, req.Context); err != nil { // check if accessible
 		return nil, err
 	}
 
@@ -84,36 +85,18 @@ func (h *ReleaseHandler) OnCreate(_ context.Context, req *pb.OnCreateRequest) (*
 
 // GetValue returns a value for a given TypeInstance.
 func (h *ReleaseHandler) GetValue(_ context.Context, req *pb.GetValueRequest) (*pb.GetValueResponse, error) {
-	releaseContext, err := h.getReleaseContext(req.Context)
+	rel, relCtx, err := h.fetchHelmRelease(req.TypeInstanceId, req.Context)
 	if err != nil {
 		return nil, err
 	}
 
-	helmGet, err := h.newHelmGet(h.helmCfgFlags, *releaseContext.Driver, releaseContext.Namespace)
-	if err != nil {
-		return nil, h.internalError(errors.Wrap(err, "while creating Helm get release client"))
-	}
-
-	// NOTE: req.resourceVersion is ignored on purpose.
-	// Based on our contract we always return the latest Helm release revision.
-	helmGet.Version = latestRevisionIndicator
-
-	release, err := helmGet.Run(releaseContext.Name)
-	switch {
-	case err == nil:
-	case errors.Is(err, driver.ErrReleaseNotFound):
-		return nil, status.Error(codes.NotFound, fmt.Sprintf("Helm release '%s/%s' for TypeInstance '%s' was not found", releaseContext.Namespace, releaseContext.Name, req.TypeInstanceId))
-	default:
-		return nil, h.internalError(errors.Wrap(err, "while getting Helm release"))
-	}
-
 	releaseData := ReleaseDetails{
-		Name:      release.Name,
-		Namespace: release.Namespace,
+		Name:      rel.Name,
+		Namespace: rel.Namespace,
 		Chart: ChartDetails{
-			Name:    release.Chart.Metadata.Name,
-			Version: release.Chart.Metadata.Version,
-			Repo:    releaseContext.ChartLocation,
+			Name:    rel.Chart.Metadata.Name,
+			Version: rel.Chart.Metadata.Version,
+			Repo:    relCtx.ChartLocation,
 		},
 	}
 
@@ -129,7 +112,7 @@ func (h *ReleaseHandler) GetValue(_ context.Context, req *pb.GetValueRequest) (*
 
 // OnUpdate checks whether a given Helm release is accessible this storage backend.
 func (h *ReleaseHandler) OnUpdate(_ context.Context, req *pb.OnUpdateRequest) (*pb.OnUpdateResponse, error) {
-	if err := h.checkIfHelmReleaseExist(req.TypeInstanceId, req.Context); err != nil {
+	if _, _, err := h.fetchHelmRelease(req.TypeInstanceId, req.Context); err != nil { // check if accessible
 		return nil, err
 	}
 	return &pb.OnUpdateResponse{}, nil
@@ -170,27 +153,31 @@ func (h *ReleaseHandler) getReleaseContext(contextBytes []byte) (*ReleaseContext
 	return &ctx, nil
 }
 
-func (h *ReleaseHandler) checkIfHelmReleaseExist(ti string, ctx []byte) error {
-	releaseContext, err := h.getReleaseContext(ctx)
+func (h *ReleaseHandler) fetchHelmRelease(ti string, ctx []byte) (*release.Release, *ReleaseContext, error) {
+	relCtx, err := h.getReleaseContext(ctx)
 	if err != nil {
-		return err
+		return nil, nil, err
 	}
 
-	helmGet, err := h.newHelmGet(h.helmCfgFlags, *releaseContext.Driver, releaseContext.Namespace)
+	helmGet, err := h.newHelmGet(h.helmCfgFlags, *relCtx.Driver, relCtx.Namespace)
 	if err != nil {
-		return h.internalError(errors.Wrap(err, "while creating Helm get release client"))
+		return nil, nil, h.internalError(errors.Wrap(err, "while creating Helm get release client"))
 	}
 
-	_, err = helmGet.Run(releaseContext.Name)
+	// NOTE: req.resourceVersion is ignored on purpose.
+	// Based on our contract we always return the latest Helm release revision.
+	helmGet.Version = latestRevisionIndicator
+
+	rel, err := helmGet.Run(relCtx.Name)
 	switch {
 	case err == nil:
 	case errors.Is(err, driver.ErrReleaseNotFound):
-		return status.Error(codes.NotFound, fmt.Sprintf("Helm release '%s/%s' for TypeInstance '%s' was not found", releaseContext.Namespace, releaseContext.Name, ti))
+		return nil, nil, status.Error(codes.NotFound, fmt.Sprintf("Helm release '%s/%s' for TypeInstance '%s' was not found", relCtx.Namespace, relCtx.Name, ti))
 	default:
-		return h.internalError(errors.Wrap(err, "while checking if Helm release exists"))
+		return nil, nil, h.internalError(errors.Wrap(err, "while fetching Helm release"))
 	}
 
-	return nil
+	return rel, relCtx, nil
 }
 
 func (h *ReleaseHandler) newHelmGet(flags *genericclioptions.ConfigFlags, driver, ns string) (*action.Get, error) {
diff --git a/internal/helm-storage-backend/release_test.go b/internal/helm-storage-backend/release_test.go
index 71f168b7a..4fc0e7847 100644
--- a/internal/helm-storage-backend/release_test.go
+++ b/internal/helm-storage-backend/release_test.go
@@ -29,32 +29,28 @@ func TestRelease_GetValue_Success(t *testing.T) {
 		name string
 
 		givenDriver          *string
-		givenTypeInstanceID  *string
-		givenResourceVersion int
+		givenTypeInstanceID  string
+		givenResourceVersion uint32
 		expectedDriver       string
 	}{
 		{
 			name:                "should use default driver and return the latest release",
-			givenTypeInstanceID: ptr.String("123"),
+			givenTypeInstanceID: "123",
 			givenDriver:         nil,
 			expectedDriver:      "secrets",
 		},
 		{
 			name:                "should use configmap driver and return the latest release",
-			givenTypeInstanceID: ptr.String("123"),
+			givenTypeInstanceID: "123",
 			givenDriver:         ptr.String("configmaps"),
 			expectedDriver:      "configmaps",
 		},
 		{
 			name:                 "should ignore resourceVersion and return the latest release",
-			givenTypeInstanceID:  ptr.String("123"),
+			givenTypeInstanceID:  "123",
 			givenResourceVersion: 42, // should be ignored
 			expectedDriver:       "secrets",
 		},
-		{
-			name:           "should return the latest release even if TypeInstance's id and revision are not specified",
-			expectedDriver: "secrets",
-		},
 	}
 	for _, test := range tests {
 		test := test
@@ -71,7 +67,8 @@ func TestRelease_GetValue_Success(t *testing.T) {
 			mockConfigurationProducer := mockConfigurationProducer(t, expHelmRelease, expFlags, test.expectedDriver)
 
 			givenReq := &pb.GetValueRequest{
-				TypeInstanceId: ptr.StringPtrToString(test.givenTypeInstanceID),
+				TypeInstanceId:  test.givenTypeInstanceID,
+				ResourceVersion: test.givenResourceVersion,
 				Context: mustMarshal(t, ReleaseContext{
 					Name:          releaseName,
 					Namespace:     releaseNamespace,
@@ -260,6 +257,19 @@ func TestRelease_OnCreate_Failures(t *testing.T) {
 			},
 			expErrMsg: "rpc error: code = NotFound desc = Helm release 'other-ns/test-release' for TypeInstance '123' was not found",
 		},
+		{
+			name: "should return internal error",
+			request: &pb.OnCreateRequest{
+				TypeInstanceId: "123",
+				Context: mustMarshal(t, ReleaseContext{
+					Name:          releaseName,
+					Namespace:     releaseNamespace,
+					ChartLocation: "http://example.com/charts",
+				}),
+			},
+			internalError: errors.New("internal error"),
+			expErrMsg:     "rpc error: code = Internal desc = while creating Helm get release client: internal error",
+		},
 	}
 	for _, test := range tests {
 		test := test
@@ -363,6 +373,19 @@ func TestRelease_OnUpdate_Failures(t *testing.T) {
 			},
 			expErrMsg: "rpc error: code = NotFound desc = Helm release 'other-ns/test-release' for TypeInstance '123' was not found",
 		},
+		{
+			name: "should return internal error",
+			request: &pb.OnUpdateRequest{
+				TypeInstanceId: "123",
+				Context: mustMarshal(t, ReleaseContext{
+					Name:          releaseName,
+					Namespace:     releaseNamespace,
+					ChartLocation: "http://example.com/charts",
+				}),
+			},
+			internalError: errors.New("internal error"),
+			expErrMsg:     "rpc error: code = Internal desc = while creating Helm get release client: internal error",
+		},
 	}
 	for _, test := range tests {
 		test := test