Repository: clouditor/clouditor
Pull request #591 (2022-03-22 11:45:15)

Add evidenceID check to  assessmentResult.Validate()

codecov[bot]: # [Codecov](https://codecov.io/gh/clouditor/clouditor/pull/591?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clouditor) Report
> Merging [#591](https://codecov.io/gh/clouditor/clouditor/pull/591?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clouditor) (4eb2878) into [main](https://codecov.io/gh/clouditor/clouditor/commit/88aad868caea56eb334baa57f8765ad7c10686af?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clouditor) (88aad86) will **increase** coverage by `0.08%`.
> The diff coverage is `100.00%`.

```diff
@@            Coverage Diff             @@
##             main     #591      +/-   ##
==========================================
+ Coverage   82.16%   82.25%   +0.08%     
==========================================
  Files          58       58              
  Lines        5131     5134       +3     
==========================================
+ Hits         4216     4223       +7     
+ Misses        702      699       -3     
+ Partials      213      212       -1     
```

| Flag | Coverage Δ | |
|---|---|---|
| unittests | `82.25% <100.00%> (+0.08%)` | :arrow_up: |

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/591?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clouditor) | Coverage Δ | |
|---|---|---|
| [api/evidence/evidence.go](https://codecov.io/gh/clouditor/clouditor/pull/591/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clouditor#diff-YXBpL2V2aWRlbmNlL2V2aWRlbmNlLmdv) | `71.87% <100.00%> (+2.90%)` | :arrow_up: |
| [service/assessment/assessment.go](https://codecov.io/gh/clouditor/clouditor/pull/591/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clouditor#diff-c2VydmljZS9hc3Nlc3NtZW50L2Fzc2Vzc21lbnQuZ28=) | `85.11% <100.00%> (ø)` | |
| [service/discovery/azure/azure.go](https://codecov.io/gh/clouditor/clouditor/pull/591/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: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/clouditor/clouditor/pull/591?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/591?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clouditor). Last update [88aad86...4eb2878](https://codecov.io/gh/clouditor/clouditor/pull/591?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/evidence/evidence.go b/api/evidence/evidence.go
index 8d1f2508..0c53c56b 100644
--- a/api/evidence/evidence.go
+++ b/api/evidence/evidence.go
@@ -25,19 +25,23 @@
 
 package evidence
 
-import "errors"
+import (
+	"errors"
+	"github.com/google/uuid"
+)
 
 type EvidenceHookFunc func(result *Evidence, err error)
 
 var (
-	ErrNotValidResource       = errors.New("resource in evidence is missing")
-	ErrResourceNotStruct      = errors.New("resource in evidence is not struct value")
-	ErrResourceNotMap         = errors.New("resource in evidence is not a map")
-	ErrResourceIdMissing      = errors.New("resource in evidence is missing the id field")
-	ErrResourceIdNotString    = errors.New("resource id in evidence is not a string")
-	ErrToolIdMissing          = errors.New("tool id in evidence is missing")
-	ErrTimestampMissing       = errors.New("timestamp in evidence is missing")
-	ErrResourceIdFieldMissing = errors.New("field id is missing")
+	ErrEvidenceIdInvalidFormat = errors.New("evidence id not in expected format (UUID) or missing")
+	ErrNotValidResource        = errors.New("resource in evidence is missing")
+	ErrResourceNotStruct       = errors.New("resource in evidence is not struct value")
+	ErrResourceNotMap          = errors.New("resource in evidence is not a map")
+	ErrResourceIdMissing       = errors.New("resource in evidence is missing the id field")
+	ErrResourceIdNotString     = errors.New("resource id in evidence is not a string")
+	ErrToolIdMissing           = errors.New("tool id in evidence is missing")
+	ErrTimestampMissing        = errors.New("timestamp in evidence is missing")
+	ErrResourceIdFieldMissing  = errors.New("field id is missing")
 )
 
 // Validate validates the evidence according to several required fields
@@ -76,5 +80,9 @@ func (evidence *Evidence) Validate() (resourceId string, err error) {
 		return "", ErrTimestampMissing
 	}
 
+	if _, err = uuid.Parse(evidence.Id); err != nil {
+		return "", ErrEvidenceIdInvalidFormat
+	}
+
 	return
 }
diff --git a/service/assessment/assessment.go b/service/assessment/assessment.go
index d8fef308..e2a562bc 100644
--- a/service/assessment/assessment.go
+++ b/service/assessment/assessment.go
@@ -313,7 +313,7 @@ func (s *Service) handleEvidence(evidence *evidence.Evidence, resourceId string)
 func convertTargetValue(v interface{}) (s *structpb.Value, err error) {
 	var b []byte
 
-	// json.Marshal and json.Unmarshaling is used instead of structpb.NewValue() which cannot handle json numbers
+	// json.Marshal and json.Unmarshal is used instead of structpb.NewValue() which cannot handle json numbers
 	if b, err = json.Marshal(v); err != nil {
 		return nil, fmt.Errorf("JSON marshal failed: %w", err)
 	}


Test patch:
diff --git a/service/assessment/assessment_test.go b/service/assessment/assessment_test.go
index 41e70012..ba7e9004 100644
--- a/service/assessment/assessment_test.go
+++ b/service/assessment/assessment_test.go
@@ -276,6 +276,24 @@ func TestAssessEvidences(t *testing.T) {
 				StatusMessage: "invalid evidence: " + evidence.ErrToolIdMissing.Error(),
 			},
 		},
+		{
+			name: "Missing evidenceID",
+			fields: fields{
+				hasRPCConnection: true,
+				results:          make(map[string]*assessment.AssessmentResult)},
+			args: args{
+				streamToServer: createMockAssessmentServerStream(&assessment.AssessEvidenceRequest{
+					Evidence: &evidence.Evidence{
+						Timestamp: timestamppb.Now(),
+						ToolId:    "2134",
+						Resource:  toStruct(voc.VirtualMachine{Compute: &voc.Compute{Resource: &voc.Resource{ID: "my-resource-id", Type: []string{"VirtualMachine"}}}}, t)}}),
+			},
+			wantErr: false,
+			wantRespMessage: &assessment.AssessEvidenceResponse{
+				Status:        assessment.AssessEvidenceResponse_FAILED,
+				StatusMessage: "invalid evidence: " + evidence.ErrEvidenceIdInvalidFormat.Error(),
+			},
+		},
 		{
 			name: "Assess evidences",
 			fields: fields{
@@ -301,6 +319,7 @@ func TestAssessEvidences(t *testing.T) {
 			args: args{
 				streamToServer: createMockAssessmentServerStream(&assessment.AssessEvidenceRequest{
 					Evidence: &evidence.Evidence{
+						Id:        "11111111-1111-1111-1111-111111111111",
 						Timestamp: timestamppb.Now(),
 						ToolId:    "2134",
 						Resource:  toStruct(voc.VirtualMachine{Compute: &voc.Compute{Resource: &voc.Resource{ID: "my-resource-id", Type: []string{"VirtualMachine"}}}}, t)}}),
diff --git a/service/evidence/evidence_store_test.go b/service/evidence/evidence_store_test.go
index 76a1ae50..0127905c 100644
--- a/service/evidence/evidence_store_test.go
+++ b/service/evidence/evidence_store_test.go
@@ -340,7 +340,7 @@ func TestEvidenceHook(t *testing.T) {
 			args: args{
 				in0: context.TODO(),
 				evidence: &evidence.StoreEvidenceRequest{Evidence: &evidence.Evidence{
-					Id:        "MockEvidenceId-1",
+					Id:        "11111111-1111-1111-1111-111111111111",
 					ServiceId: "MockServiceId-1",
 					Timestamp: timestamppb.Now(),
 					Raw:       "",