Repository: clouditor/clouditor
Pull request #671 (2022-05-07 17:33:45)

Persisting metrics in storage
This PR adds metrics to the persistence layer instead of an in-memory map of the orchestrator service. The only struct left in a map is the `MetricConfiguration`, since its tricky to persist that.

codecov[bot]: # [Codecov](https://codecov.io/gh/clouditor/clouditor/pull/671?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clouditor) Report
> Merging [#671](https://codecov.io/gh/clouditor/clouditor/pull/671?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clouditor) (e0485ea) into [add-metric-conf](https://codecov.io/gh/clouditor/clouditor/commit/a7ab6383c70c7f73cdbfa600ef058977970ca72f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clouditor) (a7ab638) will **decrease** coverage by `0.23%`.
> The diff coverage is `68.75%`.

```diff
@@                 Coverage Diff                 @@
##           add-metric-conf     #671      +/-   ##
===================================================
- Coverage            81.52%   81.28%   -0.24%     
===================================================
  Files                   64       64              
  Lines                 5819     5862      +43     
===================================================
+ Hits                  4744     4765      +21     
- Misses                 832      846      +14     
- Partials               243      251       +8     
```

| Flag | Coverage Δ | |
|---|---|---|
| unittests | `81.28% <68.75%> (-0.24%)` | :arrow_down: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clouditor#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Impacted Files](https://codecov.io/gh/clouditor/clouditor/pull/671?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clouditor) | Coverage Δ | |
|---|---|---|
| [service/orchestrator/orchestrator.go](https://codecov.io/gh/clouditor/clouditor/pull/671/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clouditor#diff-c2VydmljZS9vcmNoZXN0cmF0b3Ivb3JjaGVzdHJhdG9yLmdv) | `76.74% <ø> (ø)` | |
| [service/orchestrator/metrics.go](https://codecov.io/gh/clouditor/clouditor/pull/671/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clouditor#diff-c2VydmljZS9vcmNoZXN0cmF0b3IvbWV0cmljcy5nbw==) | `81.16% <60.78%> (-7.62%)` | :arrow_down: |
| [api/assessment/metric.go](https://codecov.io/gh/clouditor/clouditor/pull/671/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clouditor#diff-YXBpL2Fzc2Vzc21lbnQvbWV0cmljLmdv) | `87.20% <77.27%> (-0.89%)` | :arrow_down: |
| [persistence/gorm/gorm.go](https://codecov.io/gh/clouditor/clouditor/pull/671/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clouditor#diff-cGVyc2lzdGVuY2UvZ29ybS9nb3JtLmdv) | `94.66% <100.00%> (+0.07%)` | :arrow_up: |
| [service/discovery/azure/azure.go](https://codecov.io/gh/clouditor/clouditor/pull/671/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clouditor#diff-c2VydmljZS9kaXNjb3ZlcnkvYXp1cmUvYXp1cmUuZ28=) | `84.41% <0.00%> (+5.19%)` | :arrow_up: |
| [internal/testutil/storage.go](https://codecov.io/gh/clouditor/clouditor/pull/671/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clouditor#diff-aW50ZXJuYWwvdGVzdHV0aWwvc3RvcmFnZS5nbw==) | `57.14% <0.00%> (+9.52%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/clouditor/clouditor/pull/671?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clouditor).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clouditor)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/clouditor/clouditor/pull/671?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clouditor). Last update [a7ab638...e0485ea](https://codecov.io/gh/clouditor/clouditor/pull/671?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clouditor). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clouditor).


Patch:
diff --git a/api/assessment/metric.go b/api/assessment/metric.go
index 2a937bf6..c2ea28da 100644
--- a/api/assessment/metric.go
+++ b/api/assessment/metric.go
@@ -1,6 +1,7 @@
 package assessment
 
 import (
+	"database/sql/driver"
 	"encoding/base64"
 	"encoding/json"
 	"errors"
@@ -10,6 +11,7 @@ import (
 var (
 	ErrMetricNameMissing = errors.New("metric name is missing")
 	ErrMetricEmpty       = errors.New("metric is missing or empty")
+	ErrUnsupportedType   = errors.New("unsupported type")
 )
 
 func (r *Range) UnmarshalJSON(b []byte) (err error) {
@@ -38,6 +40,73 @@ func (r *Range) UnmarshalJSON(b []byte) (err error) {
 	return
 }
 
+// MarshalJSON is a custom implementation of JSON marshalling to correctly
+// serialize the Range type because the inner types, such as Range_AllowedValues
+// are missing json struct tags. This is needed if the Range type is marshalled
+// on its own (for example) as a single field in a database. In gRPC messages,
+// the protojson.Marshal function takes care of this.
+func (r *Range) MarshalJSON() (b []byte, err error) {
+	switch v := r.Range.(type) {
+	case *Range_AllowedValues:
+		return json.Marshal(&struct {
+			AllowedValues *AllowedValues `json:"allowedValues"`
+		}{
+			AllowedValues: v.AllowedValues,
+		})
+	case *Range_Order:
+		return json.Marshal(&struct {
+			Order *Order `json:"order"`
+		}{
+			Order: v.Order,
+		})
+	case *Range_MinMax:
+		return json.Marshal(&struct {
+			MinMax *MinMax `json:"minMax"`
+		}{
+			MinMax: v.MinMax,
+		})
+	default:
+		return nil, ErrUnsupportedType
+	}
+}
+
+// Value implements https://pkg.go.dev/database/sql/driver#Valuer to indicate
+// how this struct will be saved into an SQL database field.
+func (r *Range) Value() (val driver.Value, err error) {
+	if r == nil {
+		return
+	} else {
+		val, err = json.Marshal(r)
+		if err != nil {
+			err = fmt.Errorf("could not marshal JSON: %w", err)
+		}
+
+		return
+	}
+}
+
+// Scan implements https://pkg.go.dev/database/sql#Scanner to indicate how
+// this struct can be loaded from an SQL database field.
+func (r *Range) Scan(value interface{}) (err error) {
+	switch v := value.(type) {
+	case []byte:
+		err = json.Unmarshal(v, r)
+		if err != nil {
+			err = fmt.Errorf("could not unmarshal JSON: %w", err)
+		}
+	default:
+		err = ErrUnsupportedType
+	}
+
+	return
+}
+
+// GormDataType implements GormDataTypeInterface to give an indication how
+// this struct will be serialized into a database using GORM.
+func (r *Range) GormDataType() string {
+	return "jsonb"
+}
+
 // MetricValidationOption is a function-style option to fine-tune metric validation.
 type MetricValidationOption func(*Metric) error
 
diff --git a/clouditor.code-workspace b/clouditor.code-workspace
index d657c902..7fe3c8a9 100644
--- a/clouditor.code-workspace
+++ b/clouditor.code-workspace
@@ -21,4 +21,4 @@
 			"https://json.schemastore.org/github-workflow.json": ".github/workflows/*"
 		}
 	}
-}
\ No newline at end of file
+}
diff --git a/persistence/gorm/gorm.go b/persistence/gorm/gorm.go
index a03df1eb..dfa01557 100644
--- a/persistence/gorm/gorm.go
+++ b/persistence/gorm/gorm.go
@@ -107,6 +107,7 @@ func NewStorage(opts ...StorageOption) (s persistence.Storage, err error) {
 		&auth.User{},
 		&orchestrator.CloudService{},
 		&assessment.MetricImplementation{},
+		&assessment.Metric{},
 		&orchestrator.Certificate{},
 		&orchestrator.State{},
 	}
diff --git a/service/orchestrator/metrics.go b/service/orchestrator/metrics.go
index 2d3ecae8..d20cce47 100644
--- a/service/orchestrator/metrics.go
+++ b/service/orchestrator/metrics.go
@@ -62,7 +62,6 @@ func (svc *Service) loadMetrics() (err error) {
 		return fmt.Errorf("could not load metrics: %w", err)
 	}
 
-	svc.metrics = make(map[string]*assessment.Metric)
 	defaultMetricConfigurations = make(map[string]*assessment.MetricConfiguration)
 
 	for _, m := range metrics {
@@ -98,7 +97,12 @@ func (svc *Service) loadMetrics() (err error) {
 
 		config.IsDefault = true
 
-		svc.metrics[m.Id] = m
+		err = svc.storage.Save(m)
+		if err != nil {
+			log.Errorf("Error while saving metric %s: %v. Ignoring metric", m.Id, err)
+			continue
+		}
+
 		defaultMetricConfigurations[m.Id] = &config
 	}
 
@@ -142,6 +146,8 @@ func loadMetricImplementation(metricID, file string) (impl *assessment.MetricImp
 
 // CreateMetric creates a new metric in the database.
 func (svc *Service) CreateMetric(_ context.Context, req *orchestrator.CreateMetricRequest) (metric *assessment.Metric, err error) {
+	var count int64
+
 	// Validate the metric request
 	err = req.Metric.Validate(assessment.WithMetricRequiresId())
 	if err != nil {
@@ -150,12 +156,13 @@ func (svc *Service) CreateMetric(_ context.Context, req *orchestrator.CreateMetr
 		return nil, status.Errorf(codes.InvalidArgument, "%v", newError)
 	}
 
-	// Lock metrics mutex
-	svc.mm.Lock()
-	defer svc.mm.Unlock()
-
 	// Check, if metric id already exists
-	if _, ok := svc.metrics[req.Metric.Id]; ok {
+	count, err = svc.storage.Count(metric, "Id = ?", req.Metric.Id)
+	if err != nil {
+		return nil, status.Errorf(codes.Internal, "database error: %s", err)
+	}
+
+	if count > 0 {
 		return nil, status.Error(codes.AlreadyExists, "metric already exists")
 	}
 
@@ -163,7 +170,10 @@ func (svc *Service) CreateMetric(_ context.Context, req *orchestrator.CreateMetr
 	metric = req.Metric
 
 	// Append metric
-	svc.metrics[req.Metric.Id] = metric
+	err = svc.storage.Create(metric)
+	if err != nil {
+		return nil, status.Errorf(codes.Internal, "database error: %s", err)
+	}
 
 	// Notify event listeners
 	go func() {
@@ -178,8 +188,6 @@ func (svc *Service) CreateMetric(_ context.Context, req *orchestrator.CreateMetr
 
 // UpdateMetric updates an existing metric, specified by the identifier in req.MetricId.
 func (svc *Service) UpdateMetric(_ context.Context, req *orchestrator.UpdateMetricRequest) (metric *assessment.Metric, err error) {
-	var ok bool
-
 	// Validate the metric request
 	err = req.Metric.Validate()
 	if err != nil {
@@ -188,15 +196,12 @@ func (svc *Service) UpdateMetric(_ context.Context, req *orchestrator.UpdateMetr
 		return nil, status.Errorf(codes.InvalidArgument, "%v", newError)
 	}
 
-	// Lock metrics mutex
-	svc.mm.Lock()
-	defer svc.mm.Unlock()
-
 	// Check, if metric exists according to req.MetricId
-	if metric, ok = svc.metrics[req.MetricId]; !ok {
-		newError := fmt.Errorf("metric with identifier %s does not exist", req.MetricId)
-		log.Error(newError)
-		return nil, status.Errorf(codes.NotFound, "%v", newError)
+	err = svc.storage.Get(&metric, "id = ?", req.MetricId)
+	if errors.Is(err, persistence.ErrRecordNotFound) {
+		return nil, status.Error(codes.NotFound, "metric not found")
+	} else if err != nil {
+		return nil, status.Errorf(codes.Internal, "database error: %s", err)
 	}
 
 	// Update metric
@@ -206,6 +211,11 @@ func (svc *Service) UpdateMetric(_ context.Context, req *orchestrator.UpdateMetr
 	metric.Range = req.Metric.Range
 	metric.Scale = req.Metric.Scale
 
+	err = svc.storage.Save(metric, "id = ? ", metric.Id)
+	if err != nil {
+		return nil, status.Errorf(codes.Internal, "database error: %s", err)
+	}
+
 	// Notify event listeners
 	go func() {
 		svc.events <- &orchestrator.MetricChangeEvent{
@@ -220,21 +230,17 @@ func (svc *Service) UpdateMetric(_ context.Context, req *orchestrator.UpdateMetr
 // UpdateMetricImplementation updates an existing metric implementation, specified by the identifier in req.MetricId.
 func (svc *Service) UpdateMetricImplementation(_ context.Context, req *orchestrator.UpdateMetricImplementationRequest) (impl *assessment.MetricImplementation, err error) {
 	var (
-		ok     bool
 		metric *assessment.Metric
 	)
 
 	// TODO(oxisto): Validate the metric implementation request
 
-	// Lock metrics mutex
-	svc.mm.Lock()
-	defer svc.mm.Unlock()
-
 	// Check, if metric exists according to req.MetricId
-	if metric, ok = svc.metrics[req.MetricId]; !ok {
-		err := fmt.Errorf("metric with identifier %s does not exist", req.MetricId)
-		log.Error(err)
-		return nil, status.Errorf(codes.NotFound, "%v", err)
+	err = svc.storage.Get(&metric, "id = ?", req.MetricId)
+	if errors.Is(err, persistence.ErrRecordNotFound) {
+		return nil, status.Error(codes.NotFound, "metric not found")
+	} else if err != nil {
+		return nil, status.Errorf(codes.Internal, "database error: %s", err)
 	}
 
 	// Update implementation
@@ -262,14 +268,8 @@ func (svc *Service) UpdateMetricImplementation(_ context.Context, req *orchestra
 func (svc *Service) ListMetrics(_ context.Context, req *orchestrator.ListMetricsRequest) (res *orchestrator.ListMetricsResponse, err error) {
 	res = new(orchestrator.ListMetricsResponse)
 
-	// Lock metrics mutex
-	svc.mm.Lock()
-	defer svc.mm.Unlock()
-
 	// Paginate the metrics according to the request
-	res.Metrics, res.NextPageToken, err = service.PaginateMapValues(req, svc.metrics, func(a *assessment.Metric, b *assessment.Metric) bool {
-		return a.Id < b.Id
-	}, service.DefaultPaginationOpts)
+	res.Metrics, res.NextPageToken, err = service.PaginateStorage[*assessment.Metric](req, svc.storage, service.DefaultPaginationOpts)
 	if err != nil {
 		return nil, status.Errorf(codes.Internal, "could not paginate metrics: %v", err)
 	}
@@ -279,14 +279,11 @@ func (svc *Service) ListMetrics(_ context.Context, req *orchestrator.ListMetrics
 
 // GetMetric retrieves a metric specified by req.MetridId
 func (svc *Service) GetMetric(_ context.Context, req *orchestrator.GetMetricRequest) (metric *assessment.Metric, err error) {
-	var ok bool
-
-	// Lock metrics mutex
-	svc.mm.Lock()
-	defer svc.mm.Unlock()
-
-	if metric, ok = svc.metrics[req.MetricId]; !ok {
-		return nil, status.Errorf(codes.NotFound, "could not find metric with id %s", req.MetricId)
+	err = svc.storage.Get(&metric, "id = ?", req.MetricId)
+	if errors.Is(err, persistence.ErrRecordNotFound) {
+		return nil, status.Error(codes.NotFound, "metric not found")
+	} else if err != nil {
+		return nil, status.Errorf(codes.Internal, "database error: %s", err)
 	}
 
 	return
@@ -316,8 +313,9 @@ func (svc *Service) GetMetricConfiguration(_ context.Context, req *orchestrator.
 // UpdateMetricConfiguration updates the configuration for a metric, specified by the identifier in req.MetricId.
 func (svc *Service) UpdateMetricConfiguration(_ context.Context, req *orchestrator.UpdateMetricConfigurationRequest) (res *assessment.MetricConfiguration, err error) {
 	var (
-		ok  bool
-		cld orchestrator.CloudService
+		ok     bool
+		metric assessment.Metric
+		cld    orchestrator.CloudService
 	)
 
 	// TODO(oxisto): Validate the metric configuration request
@@ -327,8 +325,11 @@ func (svc *Service) UpdateMetricConfiguration(_ context.Context, req *orchestrat
 	defer svc.mm.Unlock()
 
 	// Check, if metric exists according to req.MetricId
-	if _, ok = svc.metrics[req.MetricId]; !ok {
-		return nil, status.Errorf(codes.NotFound, "could not find metric with id %s", req.MetricId)
+	err = svc.storage.Get(&metric, "id = ?", req.MetricId)
+	if errors.Is(err, persistence.ErrRecordNotFound) {
+		return nil, status.Error(codes.NotFound, "metric not found")
+	} else if err != nil {
+		return nil, status.Errorf(codes.Internal, "database error: %s", err)
 	}
 
 	err = svc.storage.Get(&cld, "Id = ?", req.ServiceId)
@@ -371,16 +372,22 @@ func (svc *Service) ListMetricConfigurations(ctx context.Context, req *orchestra
 		Configurations: make(map[string]*assessment.MetricConfiguration),
 	}
 
+	var metrics []*assessment.Metric
+	err = svc.storage.List(&metrics, 0, -1)
+	if err != nil {
+		return nil, status.Errorf(codes.Internal, "database error: %s", err)
+	}
+
 	// TODO(oxisto): This is not very efficient, we should do this once at startup so that we can just return the map
-	for metricId := range svc.metrics {
-		config, err := svc.GetMetricConfiguration(ctx, &orchestrator.GetMetricConfigurationRequest{ServiceId: req.ServiceId, MetricId: metricId})
+	for _, metric := range metrics {
+		config, err := svc.GetMetricConfiguration(ctx, &orchestrator.GetMetricConfigurationRequest{ServiceId: req.ServiceId, MetricId: metric.Id})
 
 		if err != nil {
 			log.Errorf("Error getting metric configuration: %v", err)
 			return nil, err
 		}
 
-		response.Configurations[metricId] = config
+		response.Configurations[metric.Id] = config
 	}
 
 	return
diff --git a/service/orchestrator/orchestrator.go b/service/orchestrator/orchestrator.go
index 4b14c9a9..82bdb9d2 100644
--- a/service/orchestrator/orchestrator.go
+++ b/service/orchestrator/orchestrator.go
@@ -61,9 +61,6 @@ type Service struct {
 	// metricConfigurations holds a double-map of metric configurations associated first by service ID and then metric ID
 	metricConfigurations map[string]map[string]*assessment.MetricConfiguration
 
-	// metrics contains map of our metric definitions
-	metrics map[string]*assessment.Metric
-
 	// mm is a mutex for metric related maps
 	mm sync.Mutex
 


Test patch:
diff --git a/persistence/gorm/gorm_test.go b/persistence/gorm/gorm_test.go
index c484b4a9..533cbbcd 100644
--- a/persistence/gorm/gorm_test.go
+++ b/persistence/gorm/gorm_test.go
@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"testing"
 
+	"clouditor.io/clouditor/api/assessment"
 	"clouditor.io/clouditor/api/auth"
 	"clouditor.io/clouditor/api/orchestrator"
 	"clouditor.io/clouditor/internal/testutil/orchestratortest"
@@ -84,6 +85,26 @@ func TestStorageOptions(t *testing.T) {
 	}
 }
 
+func Test_storage_Create(t *testing.T) {
+	var (
+		err    error
+		s      persistence.Storage
+		metric *assessment.Metric
+	)
+
+	metric = &assessment.Metric{Id: "Test"}
+
+	// Create storage
+	s, err = NewStorage()
+	assert.NoError(t, err)
+
+	err = s.Create(metric)
+	assert.NoError(t, err)
+
+	err = s.Create(metric)
+	assert.Error(t, err)
+}
+
 func Test_storage_Get(t *testing.T) {
 	var (
 		err  error
@@ -121,6 +142,7 @@ func Test_storage_Get(t *testing.T) {
 	err = s.Get(gotUser2, "username = ?", user.Username)
 	assert.NoError(t, err)
 	assert.Equal(t, user, gotUser2)
+	
 
 	// Get user via mail
 	gotUser3 := &auth.User{}
@@ -128,6 +150,20 @@ func Test_storage_Get(t *testing.T) {
 	assert.NoError(t, err)
 	assert.Equal(t, user, gotUser3)
 
+	var metric *assessment.Metric = &assessment.Metric{
+		Id:    "test",
+		Range: &assessment.Range{Range: &assessment.Range_MinMax{MinMax: &assessment.MinMax{Min: 1, Max: 2}}},
+	}
+
+	// Create metric
+	err = s.Create(metric)
+	assert.NoError(t, err)
+
+	// Get metric via Id
+	gotMetric := &assessment.Metric{}
+	err = s.Get(gotMetric, "id = ?", "test")
+	assert.NoError(t, err)
+	assert.Equal(t, metric, gotMetric)
 }
 
 func Test_storage_List(t *testing.T) {
diff --git a/service/orchestrator/metrics_test.go b/service/orchestrator/metrics_test.go
index d2b3657c..854a6c7e 100644
--- a/service/orchestrator/metrics_test.go
+++ b/service/orchestrator/metrics_test.go
@@ -47,13 +47,122 @@ var ErrSomeError = errors.New("some error")
 
 const MockMetricID = "SomeMetric"
 
+func TestService_loadMetrics(t *testing.T) {
+	type fields struct {
+		metricConfigurations  map[string]map[string]*assessment.MetricConfiguration
+		results               map[string]*assessment.AssessmentResult
+		AssessmentResultHooks []func(result *assessment.AssessmentResult, err error)
+		storage               persistence.Storage
+		metricsFile           string
+		loadMetricsFunc       func() ([]*assessment.Metric, error)
+		requirements          []*orchestrator.Requirement
+		events                chan *orchestrator.MetricChangeEvent
+	}
+	tests := []struct {
+		name    string
+		fields  fields
+		wantErr assert.ErrorAssertionFunc
+	}{
+		{
+			name: "json not found",
+			fields: fields{
+				metricsFile: "notfound.json",
+			},
+			wantErr: func(tt assert.TestingT, err error, i ...interface{}) bool {
+				return assert.ErrorIs(t, err, os.ErrNotExist)
+			},
+		},
+		{
+			name: "storage error",
+			fields: fields{
+				metricsFile: "metrics.json",
+				storage:     &testutil.StorageWithError{SaveErr: ErrSomeError},
+			},
+			wantErr: func(tt assert.TestingT, err error, i ...interface{}) bool {
+				return assert.ErrorIs(t, err, ErrSomeError)
+			},
+		},
+		{
+			name: "custom loading function with error",
+			fields: fields{
+				loadMetricsFunc: func() ([]*assessment.Metric, error) {
+					return nil, ErrSomeError
+				},
+			},
+			wantErr: func(tt assert.TestingT, err error, i ...interface{}) bool {
+				return assert.ErrorIs(t, err, ErrSomeError)
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			svc := &Service{
+				metricConfigurations:  tt.fields.metricConfigurations,
+				results:               tt.fields.results,
+				AssessmentResultHooks: tt.fields.AssessmentResultHooks,
+				storage:               tt.fields.storage,
+				metricsFile:           tt.fields.metricsFile,
+				loadMetricsFunc:       tt.fields.loadMetricsFunc,
+				requirements:          tt.fields.requirements,
+				events:                tt.fields.events,
+			}
+
+			err := svc.loadMetrics()
+			if tt.wantErr != nil {
+				tt.wantErr(t, err)
+			}
+		})
+	}
+}
+
+func Test_loadMetricImplementation(t *testing.T) {
+	type args struct {
+		metricID string
+		file     string
+	}
+	tests := []struct {
+		name     string
+		args     args
+		wantImpl *assessment.MetricImplementation
+		wantErr  assert.ErrorAssertionFunc
+	}{
+		{
+			name: "not existing",
+			args: args{
+				metricID: MockMetricID,
+				file:     "doesnotexist.rego",
+			},
+			wantErr: func(tt assert.TestingT, err error, i ...interface{}) bool {
+				return assert.ErrorIs(t, err, os.ErrNotExist)
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			gotImpl, err := loadMetricImplementation(tt.args.metricID, tt.args.file)
+			if tt.wantErr != nil {
+				tt.wantErr(t, err, tt.args)
+			}
+			if !reflect.DeepEqual(gotImpl, tt.wantImpl) {
+				t.Errorf("loadMetricImplementation() = %v, want %v", gotImpl, tt.wantImpl)
+			}
+		})
+	}
+}
+
 func TestService_CreateMetric(t *testing.T) {
+	type fields struct {
+		storage persistence.Storage
+	}
 	type args struct {
 		in0 context.Context
 		req *orchestrator.CreateMetricRequest
 	}
 	tests := []struct {
 		name           string
+		fields         fields
 		args           args
 		wantMetric     *assessment.Metric
 		wantErr        bool
@@ -61,6 +170,9 @@ func TestService_CreateMetric(t *testing.T) {
 	}{
 		{
 			name: "Create valid metric",
+			fields: fields{
+				storage: testutil.NewInMemoryStorage(t),
+			},
 			args: args{
 				context.TODO(),
 				&orchestrator.CreateMetricRequest{
@@ -78,6 +190,9 @@ func TestService_CreateMetric(t *testing.T) {
 		},
 		{
 			name: "Create invalid metric",
+			fields: fields{
+				storage: testutil.NewInMemoryStorage(t),
+			},
 			args: args{
 				context.TODO(),
 				&orchestrator.CreateMetricRequest{
@@ -90,6 +205,11 @@ func TestService_CreateMetric(t *testing.T) {
 		},
 		{
 			name: "Create metric which already exists",
+			fields: fields{
+				storage: testutil.NewInMemoryStorage(t, func(s persistence.Storage) {
+					_ = s.Create(assessment.Metric{Id: "TLSVersion"})
+				}),
+			},
 			args: args{
 				context.TODO(),
 				&orchestrator.CreateMetricRequest{
@@ -105,12 +225,44 @@ func TestService_CreateMetric(t *testing.T) {
 			wantErr:        true,
 			wantErrMessage: "rpc error: code = AlreadyExists desc = metric already exists",
 		},
+		{
+			name: "Error while counting",
+			fields: fields{
+				storage: &testutil.StorageWithError{CountErr: ErrSomeError},
+			},
+			args: args{
+				context.TODO(),
+				&orchestrator.CreateMetricRequest{
+					Metric: &assessment.Metric{Id: "SomeMetric", Name: "A very good metric"},
+				},
+			},
+			wantMetric:     nil,
+			wantErr:        true,
+			wantErrMessage: "rpc error: code = Internal desc = database error: some error",
+		},
+		{
+			name: "Error while creating",
+			fields: fields{
+				storage: &testutil.StorageWithError{CreateErr: ErrSomeError},
+			},
+			args: args{
+				context.TODO(),
+				&orchestrator.CreateMetricRequest{
+					Metric: &assessment.Metric{Id: "SomeMetric", Name: "A very good metric"},
+				},
+			},
+			wantMetric:     nil,
+			wantErr:        true,
+			wantErrMessage: "rpc error: code = Internal desc = database error: some error",
+		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			service := NewService()
-			gotMetric, err := service.CreateMetric(tt.args.in0, tt.args.req)
+			svc := &Service{
+				storage: tt.fields.storage,
+			}
+			gotMetric, err := svc.CreateMetric(tt.args.in0, tt.args.req)
 
 			if (err != nil) != tt.wantErr {
 				t.Errorf("Service.CreateMetric() error = %v, wantErrMessage %v", err, tt.wantErr)
@@ -124,18 +276,21 @@ func TestService_CreateMetric(t *testing.T) {
 			if !proto.Equal(gotMetric, tt.wantMetric) {
 				t.Errorf("Service.CreateMetric() = %v, want %v", gotMetric, tt.wantMetric)
 			}
-
 		})
 	}
 }
 
 func TestService_UpdateMetric(t *testing.T) {
+	type fields struct {
+		storage persistence.Storage
+	}
 	type args struct {
 		in0 context.Context
 		req *orchestrator.UpdateMetricRequest
 	}
 	tests := []struct {
 		name           string
+		fields         fields
 		args           args
 		wantMetric     *assessment.Metric
 		wantErr        bool
@@ -143,6 +298,11 @@ func TestService_UpdateMetric(t *testing.T) {
 	}{
 		{
 			name: "Update existing metric",
+			fields: fields{
+				storage: testutil.NewInMemoryStorage(t, func(s persistence.Storage) {
+					_ = s.Create(assessment.Metric{Id: "TransportEncryptionEnabled"})
+				}),
+			},
 			args: args{
 				context.TODO(),
 				&orchestrator.UpdateMetricRequest{
@@ -160,6 +320,9 @@ func TestService_UpdateMetric(t *testing.T) {
 		},
 		{
 			name: "Update non-existing metric",
+			fields: fields{
+				storage: testutil.NewInMemoryStorage(t),
+			},
 			args: args{
 				context.TODO(),
 				&orchestrator.UpdateMetricRequest{
@@ -172,7 +335,7 @@ func TestService_UpdateMetric(t *testing.T) {
 			},
 			wantMetric:     nil,
 			wantErr:        true,
-			wantErrMessage: "rpc error: code = NotFound desc = metric with identifier DoesProbablyNotExist does not exist",
+			wantErrMessage: "rpc error: code = NotFound desc = metric not found",
 		},
 		{
 			name: "Updating invalid metric",
@@ -191,8 +354,10 @@ func TestService_UpdateMetric(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			service := NewService()
-			gotMetric, err := service.UpdateMetric(tt.args.in0, tt.args.req)
+			svc := &Service{
+				storage: tt.fields.storage,
+			}
+			gotMetric, err := svc.UpdateMetric(tt.args.in0, tt.args.req)
 
 			if (err != nil) != tt.wantErr {
 				t.Errorf("Service.UpdateMetric() error = %v, wantErrMessage %v", err, tt.wantErr)
@@ -211,18 +376,38 @@ func TestService_UpdateMetric(t *testing.T) {
 }
 
 func TestService_GetMetric(t *testing.T) {
+	type fields struct {
+		storage persistence.Storage
+	}
 	type args struct {
 		in0 context.Context
 		req *orchestrator.GetMetricRequest
 	}
 	tests := []struct {
 		name       string
+		fields     fields
 		args       args
 		wantMetric *assessment.Metric
 		wantErr    bool
 	}{
 		{
 			name: "Get existing metric",
+			fields: fields{
+				storage: testutil.NewInMemoryStorage(t, func(s persistence.Storage) {
+					_ = s.Create(assessment.Metric{
+						Id:          "TransportEncryptionEnabled",
+						Name:        "Transport Encryption: Enabled",
+						Description: "This metric describes, whether transport encryption is turned on or not",
+						Scale:       assessment.Metric_ORDINAL,
+						Range: &assessment.Range{
+							Range: &assessment.Range_AllowedValues{AllowedValues: &assessment.AllowedValues{
+								Values: []*structpb.Value{
+									structpb.NewBoolValue(false),
+									structpb.NewBoolValue(true),
+								}}}},
+					})
+				}),
+			},
 			args: args{
 				context.TODO(),
 				&orchestrator.GetMetricRequest{
@@ -244,7 +429,8 @@ func TestService_GetMetric(t *testing.T) {
 			wantErr: false,
 		},
 		{
-			name: "Get non-existing metric",
+			name:   "Get non-existing metric",
+			fields: fields{storage: testutil.NewInMemoryStorage(t)},
 			args: args{
 				context.TODO(),
 				&orchestrator.GetMetricRequest{
@@ -258,8 +444,10 @@ func TestService_GetMetric(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			service := NewService()
-			gotMetric, err := service.GetMetric(tt.args.in0, tt.args.req)
+			svc := &Service{
+				storage: tt.fields.storage,
+			}
+			gotMetric, err := svc.GetMetric(tt.args.in0, tt.args.req)
 
 			if (err != nil) != tt.wantErr {
 				t.Errorf("Service.GetMetric() error = %v, wantErrMessage %v", err, tt.wantErr)
@@ -381,75 +569,6 @@ func TestService_GetMetricImplementation(t *testing.T) {
 	}
 }
 
-func TestService_loadMetrics(t *testing.T) {
-	type fields struct {
-		metricConfigurations  map[string]map[string]*assessment.MetricConfiguration
-		results               map[string]*assessment.AssessmentResult
-		AssessmentResultHooks []func(result *assessment.AssessmentResult, err error)
-		storage               persistence.Storage
-		metricsFile           string
-		loadMetricsFunc       func() ([]*assessment.Metric, error)
-		requirements          []*orchestrator.Requirement
-		events                chan *orchestrator.MetricChangeEvent
-	}
-	tests := []struct {
-		name    string
-		fields  fields
-		wantErr assert.ErrorAssertionFunc
-	}{
-		{
-			name: "json not found",
-			fields: fields{
-				metricsFile: "notfound.json",
-			},
-			wantErr: func(tt assert.TestingT, err error, i ...interface{}) bool {
-				return assert.ErrorIs(t, err, os.ErrNotExist)
-			},
-		},
-		{
-			name: "storage error",
-			fields: fields{
-				metricsFile: "metrics.json",
-				storage:     &testutil.StorageWithError{SaveErr: ErrSomeError},
-			},
-			wantErr: func(tt assert.TestingT, err error, i ...interface{}) bool {
-				return assert.ErrorIs(t, err, ErrSomeError)
-			},
-		},
-		{
-			name: "custom loading function with error",
-			fields: fields{
-				loadMetricsFunc: func() ([]*assessment.Metric, error) {
-					return nil, ErrSomeError
-				},
-			},
-			wantErr: func(tt assert.TestingT, err error, i ...interface{}) bool {
-				return assert.ErrorIs(t, err, ErrSomeError)
-			},
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			svc := &Service{
-				metricConfigurations:  tt.fields.metricConfigurations,
-				results:               tt.fields.results,
-				AssessmentResultHooks: tt.fields.AssessmentResultHooks,
-				storage:               tt.fields.storage,
-				metricsFile:           tt.fields.metricsFile,
-				loadMetricsFunc:       tt.fields.loadMetricsFunc,
-				requirements:          tt.fields.requirements,
-				events:                tt.fields.events,
-			}
-
-			err := svc.loadMetrics()
-			if tt.wantErr != nil {
-				tt.wantErr(t, err)
-			}
-		})
-	}
-}
-
 func TestService_UpdateMetricImplementation(t *testing.T) {
 	type fields struct {
 		metricConfigurations  map[string]map[string]*assessment.MetricConfiguration
@@ -457,7 +576,6 @@ func TestService_UpdateMetricImplementation(t *testing.T) {
 		AssessmentResultHooks []func(result *assessment.AssessmentResult, err error)
 		storage               persistence.Storage
 		metricsFile           string
-		metrics               map[string]*assessment.Metric
 		requirements          []*orchestrator.Requirement
 		events                chan *orchestrator.MetricChangeEvent
 	}
@@ -504,11 +622,10 @@ func TestService_UpdateMetricImplementation(t *testing.T) {
 		{
 			name: "update",
 			fields: fields{
-				storage:     testutil.NewInMemoryStorage(t),
+				storage: testutil.NewInMemoryStorage(t, func(s persistence.Storage) {
+					_ = s.Create(assessment.Metric{Id: "TransportEncryptionEnabled"})
+				}),
 				metricsFile: "metrics.json",
-				metrics: map[string]*assessment.Metric{
-					"TransportEncryptionEnabled": {},
-				},
 			},
 			args: args{
 				req: &orchestrator.UpdateMetricImplementationRequest{
@@ -535,7 +652,6 @@ func TestService_UpdateMetricImplementation(t *testing.T) {
 				AssessmentResultHooks: tt.fields.AssessmentResultHooks,
 				storage:               tt.fields.storage,
 				metricsFile:           tt.fields.metricsFile,
-				metrics:               tt.fields.metrics,
 				requirements:          tt.fields.requirements,
 				events:                tt.fields.events,
 			}
@@ -551,49 +667,12 @@ func TestService_UpdateMetricImplementation(t *testing.T) {
 	}
 }
 
-func Test_loadMetricImplementation(t *testing.T) {
-	type args struct {
-		metricID string
-		file     string
-	}
-	tests := []struct {
-		name     string
-		args     args
-		wantImpl *assessment.MetricImplementation
-		wantErr  assert.ErrorAssertionFunc
-	}{
-		{
-			name: "not existing",
-			args: args{
-				metricID: MockMetricID,
-				file:     "doesnotexist.rego",
-			},
-			wantErr: func(tt assert.TestingT, err error, i ...interface{}) bool {
-				return assert.ErrorIs(t, err, os.ErrNotExist)
-			},
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			gotImpl, err := loadMetricImplementation(tt.args.metricID, tt.args.file)
-			if tt.wantErr != nil {
-				tt.wantErr(t, err, tt.args)
-			}
-			if !reflect.DeepEqual(gotImpl, tt.wantImpl) {
-				t.Errorf("loadMetricImplementation() = %v, want %v", gotImpl, tt.wantImpl)
-			}
-		})
-	}
-}
-
 func TestService_GetMetricConfiguration(t *testing.T) {
 	type fields struct {
 		metricConfigurations  map[string]map[string]*assessment.MetricConfiguration
 		results               map[string]*assessment.AssessmentResult
 		AssessmentResultHooks []func(result *assessment.AssessmentResult, err error)
 		storage               persistence.Storage
-		metrics               map[string]*assessment.Metric
 		metricsFile           string
 		requirements          []*orchestrator.Requirement
 		events                chan *orchestrator.MetricChangeEvent
@@ -634,7 +713,6 @@ func TestService_GetMetricConfiguration(t *testing.T) {
 				results:               tt.fields.results,
 				AssessmentResultHooks: tt.fields.AssessmentResultHooks,
 				storage:               tt.fields.storage,
-				metrics:               tt.fields.metrics,
 				metricsFile:           tt.fields.metricsFile,
 				requirements:          tt.fields.requirements,
 				events:                tt.fields.events,
@@ -656,7 +734,6 @@ func TestService_ListMetricConfigurations(t *testing.T) {
 		results               map[string]*assessment.AssessmentResult
 		AssessmentResultHooks []func(result *assessment.AssessmentResult, err error)
 		storage               persistence.Storage
-		metrics               map[string]*assessment.Metric
 		metricsFile           string
 		requirements          []*orchestrator.Requirement
 		events                chan *orchestrator.MetricChangeEvent
@@ -675,15 +752,40 @@ func TestService_ListMetricConfigurations(t *testing.T) {
 		{
 			name: "error",
 			fields: fields{
-				metrics: map[string]*assessment.Metric{
-					MockMetricID: {},
-				},
+				storage: &testutil.StorageWithError{ListErr: ErrSomeError},
 			},
 			args: args{
 				req: &orchestrator.ListMetricConfigurationRequest{},
 			},
 			wantErr: true,
 		},
+		{
+			name: "no error",
+			fields: fields{
+				storage: testutil.NewInMemoryStorage(t, func(s persistence.Storage) {
+					_ = s.Create(assessment.Metric{Id: MockMetricID})
+				}),
+				metricConfigurations: map[string]map[string]*assessment.MetricConfiguration{
+					DefaultTargetCloudServiceId: {
+						MockMetricID: {
+							Operator: "==",
+						},
+					},
+				},
+			},
+			args: args{
+				req: &orchestrator.ListMetricConfigurationRequest{
+					ServiceId: DefaultTargetCloudServiceId,
+				},
+			},
+			wantResponse: &orchestrator.ListMetricConfigurationResponse{
+				Configurations: map[string]*assessment.MetricConfiguration{
+					MockMetricID: {
+						Operator: "==",
+					},
+				},
+			},
+		},
 	}
 
 	for _, tt := range tests {
@@ -693,7 +795,6 @@ func TestService_ListMetricConfigurations(t *testing.T) {
 				results:               tt.fields.results,
 				AssessmentResultHooks: tt.fields.AssessmentResultHooks,
 				storage:               tt.fields.storage,
-				metrics:               tt.fields.metrics,
 				metricsFile:           tt.fields.metricsFile,
 				requirements:          tt.fields.requirements,
 				events:                tt.fields.events,
@@ -716,7 +817,6 @@ func TestService_UpdateMetricConfiguration(t *testing.T) {
 		results               map[string]*assessment.AssessmentResult
 		AssessmentResultHooks []func(result *assessment.AssessmentResult, err error)
 		storage               persistence.Storage
-		metrics               map[string]*assessment.Metric
 		metricsFile           string
 		loadMetricsFunc       func() ([]*assessment.Metric, error)
 		requirements          []*orchestrator.Requirement
@@ -734,10 +834,8 @@ func TestService_UpdateMetricConfiguration(t *testing.T) {
 		wantErr bool
 	}{
 		{
-			name: "metric does not exist",
-			fields: fields{
-				metrics: map[string]*assessment.Metric{},
-			},
+			name:   "metric does not exist",
+			fields: fields{storage: testutil.NewInMemoryStorage(t)},
 			args: args{
 				req: &orchestrator.UpdateMetricConfigurationRequest{ServiceId: DefaultTargetCloudServiceId, MetricId: "MyMetric"},
 			},
@@ -746,8 +844,9 @@ func TestService_UpdateMetricConfiguration(t *testing.T) {
 		{
 			name: "service does not exist",
 			fields: fields{
-				metrics: map[string]*assessment.Metric{"MyMetric": {}},
-				storage: testutil.NewInMemoryStorage(t),
+				storage: testutil.NewInMemoryStorage(t, func(s persistence.Storage) {
+					_ = s.Create(assessment.Metric{Id: "MyMetric"})
+				}),
 			},
 			args: args{
 				req: &orchestrator.UpdateMetricConfigurationRequest{ServiceId: "MyService", MetricId: "MyMetric"},
@@ -757,8 +856,8 @@ func TestService_UpdateMetricConfiguration(t *testing.T) {
 		{
 			name: "first metric in service",
 			fields: fields{
-				metrics: map[string]*assessment.Metric{"MyMetric": {}},
 				storage: testutil.NewInMemoryStorage(t, func(s persistence.Storage) {
+					_ = s.Create(assessment.Metric{Id: "MyMetric"})
 					_ = s.Create(&orchestrator.CloudService{Id: DefaultTargetCloudServiceId})
 				}),
 				metricConfigurations: map[string]map[string]*assessment.MetricConfiguration{},
@@ -777,7 +876,6 @@ func TestService_UpdateMetricConfiguration(t *testing.T) {
 				results:               tt.fields.results,
 				AssessmentResultHooks: tt.fields.AssessmentResultHooks,
 				storage:               tt.fields.storage,
-				metrics:               tt.fields.metrics,
 				metricsFile:           tt.fields.metricsFile,
 				loadMetricsFunc:       tt.fields.loadMetricsFunc,
 				requirements:          tt.fields.requirements,
diff --git a/service/orchestrator/orchestrator_test.go b/service/orchestrator/orchestrator_test.go
index b04c6c04..2ab629da 100644
--- a/service/orchestrator/orchestrator_test.go
+++ b/service/orchestrator/orchestrator_test.go
@@ -161,19 +161,6 @@ func TestAssessmentResultHook(t *testing.T) {
 	}
 }
 
-// TODO(lebogg): Convert to table tests
-func TestListMetricConfigurations(t *testing.T) {
-	var (
-		response *orchestrator.ListMetricConfigurationResponse
-		err      error
-	)
-	s := NewService()
-	response, err = s.ListMetricConfigurations(context.TODO(), &orchestrator.ListMetricConfigurationRequest{})
-
-	assert.NoError(t, err)
-	assert.NotEmpty(t, response.Configurations)
-}
-
 func TestStoreAssessmentResult(t *testing.T) {
 	type args struct {
 		in0        context.Context