Repository: clouditor/clouditor
Pull request #607 (2022-03-30 15:18:25)

Fix initXXXStream error
initXXXStream was called in NewService(). The problem is, that in engine.go NewService() is called before the server are running, thus the initXXXStream methods always fail. 

For now I moved the initXXXStream() calls from NewService() to AssessEvidence(). It is checked if  a stream exists and if not, the streams are initialized. 

It is not very nice, but it was the fastest solution. 

codecov[bot]: # [Codecov](https://codecov.io/gh/clouditor/clouditor/pull/607?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clouditor) Report
> Merging [#607](https://codecov.io/gh/clouditor/clouditor/pull/607?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clouditor) (044278f) into [main](https://codecov.io/gh/clouditor/clouditor/commit/696b2093c9abb6cf2591d3944347160ccb252c58?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clouditor) (696b209) will **decrease** coverage by `0.20%`.
> The diff coverage is `0.00%`.

```diff
@@            Coverage Diff             @@
##             main     #607      +/-   ##
==========================================
- Coverage   81.63%   81.42%   -0.21%     
==========================================
  Files          58       58              
  Lines        5172     5175       +3     
==========================================
- Hits         4222     4214       -8     
- Misses        729      738       +9     
- Partials      221      223       +2     
```

| Flag | Coverage Δ | |
|---|---|---|
| unittests | `81.42% <0.00%> (-0.21%)` | :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/607?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clouditor) | Coverage Δ | |
|---|---|---|
| [service/assessment/assessment.go](https://codecov.io/gh/clouditor/clouditor/pull/607/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clouditor#diff-c2VydmljZS9hc3Nlc3NtZW50L2Fzc2Vzc21lbnQuZ28=) | `76.47% <0.00%> (-3.06%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/clouditor/clouditor/pull/607?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/607?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clouditor). Last update [696b209...044278f](https://codecov.io/gh/clouditor/clouditor/pull/607?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/service/assessment/assessment.go b/service/assessment/assessment.go
index b8089627..d8bdbdde 100644
--- a/service/assessment/assessment.go
+++ b/service/assessment/assessment.go
@@ -155,18 +155,6 @@ func NewService(opts ...ServiceOption) *Service {
 		o(s)
 	}
 
-	// Initialise Evidence Store stream
-	err := s.initEvidenceStoreStream()
-	if err != nil {
-		log.Errorf("Error while initializing evidence store stream: %v", err)
-	}
-
-	// Initialise Orchestrator stream
-	err = s.initOrchestratorStream()
-	if err != nil {
-		log.Errorf("Error while initializing orchestrator stream: %v", err)
-	}
-
 	return s
 }
 
@@ -198,6 +186,22 @@ func (s *Service) AssessEvidence(_ context.Context, req *assessment.AssessEviden
 		return res, status.Errorf(codes.InvalidArgument, "%v", newError)
 	}
 
+	// Check if evidence store stream exists
+	if s.evidenceStoreStream == nil {
+		err = s.initEvidenceStoreStream()
+		if err != nil {
+			log.Errorf("error initialising evidence store stream: %v", err)
+		}
+	}
+
+	// Check if orchestrator stream exists
+	if s.orchestratorStream == nil {
+		err = s.initOrchestratorStream()
+		if err != nil {
+			log.Errorf("error initialising orchestrator stream: %v", err)
+		}
+	}
+
 	// Assess evidence
 	err = s.handleEvidence(req.Evidence, resourceId)
 	if err != nil {


Test patch:
diff --git a/service/assessment/assessment_test.go b/service/assessment/assessment_test.go
index bb852860..425f0c21 100644
--- a/service/assessment/assessment_test.go
+++ b/service/assessment/assessment_test.go
@@ -112,13 +112,10 @@ func TestNewService(t *testing.T) {
 			// Check channels have been created
 			assert.NotNil(t, s.evidenceStoreChannel)
 			assert.NotNil(t, s.orchestratorChannel)
-			assert.NotNil(t, s.orchestratorClient)
 
 			// Ignore pointers to channel in subsequent DeepEqual check
 			s.evidenceStoreChannel = nil
 			s.orchestratorChannel = nil
-			// Ignore pointer to orchestrator client in subsequent DeepEqual check
-			s.orchestratorClient = nil
 
 			if got := s; !reflect.DeepEqual(got, tt.want) {
 				t.Errorf("NewService() = %v, want %v", got, tt.want)
@@ -208,6 +205,24 @@ func TestAssessEvidence(t *testing.T) {
 			},
 			wantErr: false,
 		},
+		{
+			name: "No RPC connections",
+			args: args{
+				in0: context.TODO(),
+				evidence: &evidence.Evidence{
+					Id:        "11111111-1111-1111-1111-111111111111",
+					ToolId:    "mock",
+					Timestamp: timestamppb.Now(),
+					Resource:  toStruct(voc.VirtualMachine{Compute: &voc.Compute{Resource: &voc.Resource{ID: "my-resource-id", Type: []string{"VirtualMachine"}}}}, t),
+				},
+			},
+			hasRPCConnection: false,
+			wantResp: &assessment.AssessEvidenceResponse{
+				Status:        assessment.AssessEvidenceResponse_FAILED,
+				StatusMessage: "could not evaluate evidence: could not fetch metric configuration: could not retrieve metric configuration for",
+			},
+			wantErr: true,
+		},
 	}
 
 	for _, tt := range tests {
@@ -223,14 +238,15 @@ func TestAssessEvidence(t *testing.T) {
 			}
 
 			gotResp, err := s.AssessEvidence(tt.args.in0, &assessment.AssessEvidenceRequest{Evidence: tt.args.evidence})
+
 			if (err != nil) != tt.wantErr {
 				t.Errorf("AssessEvidence() error = %v, wantErr %v", err, tt.wantErr)
 				return
 			}
 
-			if !reflect.DeepEqual(gotResp, tt.wantResp) {
-				t.Errorf("AssessEvidence() gotResp = %v, want %v", gotResp, tt.wantResp)
-			}
+			// Check response
+			assert.Equal(t, tt.wantResp.Status, gotResp.Status)
+			assert.Contains(t, gotResp.StatusMessage, tt.wantResp.StatusMessage)
 		})
 	}
 }