Repository: clouditor/clouditor (go)

Issue:
Target addresses of gRPC services must be configurable
Mainly in the assessment and collector service.

Pull request:
Made gRPC target adresses configurable
Made gRPC target adresses configurable. Fixes #481

Patch:
diff --git a/cmd/engine/engine.go b/cmd/engine/engine.go
index 121d90d3..e67dc05c 100644
--- a/cmd/engine/engine.go
+++ b/cmd/engine/engine.go
@@ -95,10 +95,10 @@ const (
 
 var server *grpc.Server
 var authService *service_auth.Service
-var discoveryService *service_discovery.Service
+var discoveryService discovery.DiscoveryServer
 var orchestratorService *service_orchestrator.Service
-var assessmentService *service_assessment.Service
-var evidenceStoreService *service_evidenceStore.Service
+var assessmentService assessment.AssessmentServer
+var evidenceStoreService evidence.EvidenceStoreServer
 
 var log *logrus.Entry
 
diff --git a/service/assessment/assessment.go b/service/assessment/assessment.go
index 44eb0d27..15048869 100644
--- a/service/assessment/assessment.go
+++ b/service/assessment/assessment.go
@@ -26,13 +26,16 @@
 package assessment
 
 import (
+	"context"
+	"encoding/json"
+	"fmt"
+	"io"
+	"sync"
+
 	"clouditor.io/clouditor/api/assessment"
 	"clouditor.io/clouditor/api/evidence"
 	"clouditor.io/clouditor/api/orchestrator"
 	"clouditor.io/clouditor/policies"
-	"context"
-	"encoding/json"
-	"fmt"
 	"github.com/google/uuid"
 	"github.com/sirupsen/logrus"
 	"google.golang.org/grpc"
@@ -42,8 +45,6 @@ import (
 	"google.golang.org/protobuf/types/known/emptypb"
 	"google.golang.org/protobuf/types/known/structpb"
 	"google.golang.org/protobuf/types/known/timestamppb"
-	"io"
-	"sync"
 )
 
 var log *logrus.Entry
@@ -52,21 +53,19 @@ func init() {
 	log = logrus.WithField("component", "assessment")
 }
 
-/*
-Service is an implementation of the Clouditor Assessment service. It should not be used directly, but rather the
-NewService constructor should be used. It implements the AssessmentServer interface
-*/
+// Service is an implementation of the Clouditor Assessment service. It should not be used directly,
+// but rather the NewService constructor should be used. It implements the AssessmentServer interface.
 type Service struct {
 	// Embedded for FWD compatibility
 	assessment.UnimplementedAssessmentServer
 
 	// evidenceStoreStream sends evidences to the Evidence Store
 	evidenceStoreStream  evidence.EvidenceStore_StoreEvidencesClient
-	EvidenceStoreAddress string
+	evidenceStoreAddress string
 
 	// orchestratorStream sends ARs to the Orchestrator
 	orchestratorStream  orchestrator.Orchestrator_StoreAssessmentResultsClient
-	OrchestratorAddress string
+	orchestratorAddress string
 
 	// resultHooks is a list of hook functions that can be used if one wants to be
 	// informed about each assessment result
@@ -78,13 +77,45 @@ type Service struct {
 	results map[string]*assessment.AssessmentResult
 }
 
-// NewService creates a new assessment service with default values
-func NewService() *Service {
-	return &Service{
+const (
+	// DefaultEvidenceStoreAddress specifies the default gRPC address of the evidence store.
+	DefaultEvidenceStoreAddress = "localhost:9090"
+
+	// DefaultEvidenceStoreAddress specifies the default gRPC address of the orchestrator.
+	DefaultOrchestratorAddress = "localhost:9090"
+)
+
+// ServiceOption is a functional option type to configure the assessment service.
+type ServiceOption func(*Service)
+
+// WithEvidenceStoreAddress is an option to configure the evidence store gRPC address.
+func WithEvidenceStoreAddress(address string) ServiceOption {
+	return func(s *Service) {
+		s.evidenceStoreAddress = address
+	}
+}
+
+// WithEvidenceStoreAddress is an option to configure the orchestrator gRPC address.
+func WithOrchestratorAddress(address string) ServiceOption {
+	return func(s *Service) {
+		s.orchestratorAddress = address
+	}
+}
+
+// NewService creates a new assessment service with default values.
+func NewService(opts ...ServiceOption) *Service {
+	s := &Service{
 		results:              make(map[string]*assessment.AssessmentResult),
-		EvidenceStoreAddress: "localhost:9090",
-		OrchestratorAddress:  "localhost:9090",
+		evidenceStoreAddress: DefaultEvidenceStoreAddress,
+		orchestratorAddress:  DefaultOrchestratorAddress,
+	}
+
+	// Apply any options
+	for _, o := range opts {
+		o(s)
 	}
+
+	return s
 }
 
 // AssessEvidence is a method implementation of the assessment interface: It assesses a single evidence
@@ -153,7 +184,6 @@ func (s *Service) AssessEvidences(stream assessment.Assessment_AssessEvidencesSe
 
 // handleEvidence is the helper method for the actual assessment used by AssessEvidence and AssessEvidences
 func (s *Service) handleEvidence(evidence *evidence.Evidence, resourceId string) (err error) {
-
 	log.Infof("Running evidence %s (%s) collected by %s at %v", evidence.Id, resourceId, evidence.ToolId, evidence.Timestamp)
 	log.Debugf("Evidence: %+v", evidence)
 
@@ -264,7 +294,7 @@ func (s *Service) RegisterAssessmentResultHook(assessmentResultsHook func(result
 // initEvidenceStoreStream initializes the stream to the Evidence Store
 func (s *Service) initEvidenceStoreStream() error {
 	// Establish connection to evidenceStore component
-	target := s.EvidenceStoreAddress
+	target := s.evidenceStoreAddress
 	log.Infof("Establishing connection to Evidence Store (%v)", target)
 	conn, err := grpc.Dial(target, grpc.WithTransportCredentials(insecure.NewCredentials()))
 	if err != nil {
@@ -298,7 +328,7 @@ func (s *Service) sendToEvidenceStore(e *evidence.Evidence) error {
 // initOrchestratorStream initializes the stream to the Orchestrator
 func (s *Service) initOrchestratorStream() error {
 	// Establish connection to orchestrator component
-	target := s.OrchestratorAddress
+	target := s.orchestratorAddress
 	log.Infof("Establishing connection to Orchestrator (%v)", target)
 	conn, err := grpc.Dial(target, grpc.WithTransportCredentials(insecure.NewCredentials()))
 	if err != nil {
diff --git a/service/discovery/discovery.go b/service/discovery/discovery.go
index 6c1c15e3..b2b982bc 100644
--- a/service/discovery/discovery.go
+++ b/service/discovery/discovery.go
@@ -59,10 +59,10 @@ var log *logrus.Entry
 type Service struct {
 	discovery.UnimplementedDiscoveryServer
 
-	Configurations map[discovery.Discoverer]*Configuration
+	configurations map[discovery.Discoverer]*Configuration
 
 	assessmentStream  assessment.Assessment_AssessEvidencesClient
-	AssessmentAddress string
+	assessmentAddress string
 
 	resources map[string]voc.IsCloudResource
 	scheduler *gocron.Scheduler
@@ -72,32 +72,49 @@ type Configuration struct {
 	Interval time.Duration
 }
 
-type ResultOntology struct {
-	Result *structpb.ListValue `json:"result"`
-}
-
 func init() {
 	log = logrus.WithField("component", "discovery")
 }
 
-func NewService() *Service {
-	return &Service{
-		AssessmentAddress: "localhost:9090",
+const (
+	// DefaultEvidenceStoreAddress specifies the default gRPC address of the assessment service.
+	DefaultAssessmentAddress = "localhost:9090"
+)
+
+// ServiceOption is a functional option type to configure the discovery service.
+type ServiceOption func(*Service)
+
+// WithEvidenceStoreAddress is an option to configure the assessment service gRPC address.
+func WithAssessmentAddress(address string) ServiceOption {
+	return func(s *Service) {
+		s.assessmentAddress = address
+	}
+}
+
+func NewService(opts ...ServiceOption) *Service {
+	s := &Service{
+		assessmentAddress: DefaultAssessmentAddress,
 		resources:         make(map[string]voc.IsCloudResource),
 		scheduler:         gocron.NewScheduler(time.UTC),
-		Configurations:    make(map[discovery.Discoverer]*Configuration),
+		configurations:    make(map[discovery.Discoverer]*Configuration),
 	}
+
+	// Apply any options
+	for _, o := range opts {
+		o(s)
+	}
+
+	return s
 }
 
 func (s *Service) initAssessmentStream() error {
 	// Establish connection to assessment component
-	// TODO(oxisto): support assessment on Another tcp/port
-	target := s.AssessmentAddress
-	log.Infof("Establishing connection to Assessment (%v)", target)
+	target := s.assessmentAddress
+	log.Infof("Establishing connection to assessment (%v)", target)
 
-	conn, err := grpc.Dial(s.AssessmentAddress, grpc.WithTransportCredentials(insecure.NewCredentials()))
+	conn, err := grpc.Dial(s.assessmentAddress, grpc.WithTransportCredentials(insecure.NewCredentials()))
 	if err != nil {
-		return fmt.Errorf("could not connect to Assessment service: %v", err)
+		return fmt.Errorf("could not connect to assessment service: %v", err)
 	}
 
 	client := assessment.NewAssessmentClient(conn)
@@ -158,7 +175,7 @@ func (s *Service) Start(_ context.Context, _ *discovery.StartDiscoveryRequest) (
 	)
 
 	for _, v := range discoverer {
-		s.Configurations[v] = &Configuration{
+		s.configurations[v] = &Configuration{
 			Interval: 5 * time.Minute,
 		}
 


Test patch:
diff --git a/cli/commands/resource/resource_test.go b/cli/commands/resource/resource_test.go
index 9af92e8d..b7c86314 100644
--- a/cli/commands/resource/resource_test.go
+++ b/cli/commands/resource/resource_test.go
@@ -27,6 +27,13 @@ package resource
 
 import (
 	"bytes"
+	"fmt"
+	"io/ioutil"
+	"net"
+	"os"
+	"testing"
+	"time"
+
 	"clouditor.io/clouditor/api/discovery"
 	"clouditor.io/clouditor/cli"
 	"clouditor.io/clouditor/cli/commands/login"
@@ -34,16 +41,10 @@ import (
 	service_auth "clouditor.io/clouditor/service/auth"
 	service_discovery "clouditor.io/clouditor/service/discovery"
 	"clouditor.io/clouditor/voc"
-	"fmt"
 	"github.com/spf13/viper"
 	"github.com/stretchr/testify/assert"
 	"google.golang.org/grpc"
 	"google.golang.org/protobuf/encoding/protojson"
-	"io/ioutil"
-	"net"
-	"os"
-	"testing"
-	"time"
 )
 
 var sock net.Listener
diff --git a/service/assessment/assessment_test.go b/service/assessment/assessment_test.go
index 01f87e8c..d5a4686d 100644
--- a/service/assessment/assessment_test.go
+++ b/service/assessment/assessment_test.go
@@ -81,23 +81,40 @@ func TestMain(m *testing.M) {
 
 // TestNewService is a simply test for NewService
 func TestNewService(t *testing.T) {
+	type args struct {
+		opts []ServiceOption
+	}
 	tests := []struct {
 		name string
-		want assessment.AssessmentServer
+		args args
+		want *Service
 	}{
 		{
 			name: "AssessmentServer created with empty results map",
 			want: &Service{
-				results:                       make(map[string]*assessment.AssessmentResult),
-				UnimplementedAssessmentServer: assessment.UnimplementedAssessmentServer{},
-				EvidenceStoreAddress:          "localhost:9090",
-				OrchestratorAddress:           "localhost:9090",
+				results:              make(map[string]*assessment.AssessmentResult),
+				evidenceStoreAddress: "localhost:9090",
+				orchestratorAddress:  "localhost:9090",
+			},
+		},
+		{
+			name: "AssessmentServer created with options",
+			args: args{
+				opts: []ServiceOption{
+					WithEvidenceStoreAddress("localhost:9091"),
+					WithOrchestratorAddress("localhost:9092"),
+				},
+			},
+			want: &Service{
+				results:              make(map[string]*assessment.AssessmentResult),
+				evidenceStoreAddress: "localhost:9091",
+				orchestratorAddress:  "localhost:9092",
 			},
 		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			if got := NewService(); !reflect.DeepEqual(got, tt.want) {
+			if got := NewService(tt.args.opts...); !reflect.DeepEqual(got, tt.want) {
 				t.Errorf("NewService() = %v, want %v", got, tt.want)
 			}
 		})
diff --git a/service/discovery/discovery_test.go b/service/discovery/discovery_test.go
index 1236619a..a319ab39 100644
--- a/service/discovery/discovery_test.go
+++ b/service/discovery/discovery_test.go
@@ -26,26 +26,27 @@
 package discovery
 
 import (
-	service_assessment "clouditor.io/clouditor/service/assessment"
 	"context"
 	"errors"
 	"fmt"
-	"google.golang.org/grpc"
-	"google.golang.org/grpc/codes"
-	"google.golang.org/grpc/credentials/insecure"
-	"google.golang.org/grpc/status"
-	"google.golang.org/grpc/test/bufconn"
 	"net"
 	"os"
+	"reflect"
 	"testing"
 	"time"
 
 	"clouditor.io/clouditor/api/assessment"
 	"clouditor.io/clouditor/api/discovery"
 	"clouditor.io/clouditor/api/evidence"
+	service_assessment "clouditor.io/clouditor/service/assessment"
 	"clouditor.io/clouditor/voc"
 	"github.com/stretchr/testify/assert"
+	"google.golang.org/grpc"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/credentials/insecure"
 	"google.golang.org/grpc/metadata"
+	"google.golang.org/grpc/status"
+	"google.golang.org/grpc/test/bufconn"
 	"google.golang.org/protobuf/types/known/emptypb"
 )
 
@@ -72,6 +73,45 @@ func TestMain(m *testing.M) {
 	os.Exit(m.Run())
 }
 
+func TestNewService(t *testing.T) {
+	type args struct {
+		opts []ServiceOption
+	}
+	tests := []struct {
+		name string
+		args args
+		want *Service
+	}{
+		{
+			name: "Create service with options",
+			args: args{
+				opts: []ServiceOption{
+					WithAssessmentAddress("localhost:9091"),
+				},
+			},
+			want: &Service{
+				assessmentAddress: "localhost:9091",
+				resources:         make(map[string]voc.IsCloudResource),
+				configurations:    make(map[discovery.Discoverer]*Configuration),
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := NewService(tt.args.opts...)
+
+			// we cannot compare the scheduler, so we need to nil it
+			got.scheduler = nil
+			tt.want.scheduler = nil
+
+			if !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("NewService() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
+
 func TestStartDiscovery(t *testing.T) {
 	discoveryService := NewService()