Repository: clouditor/clouditor
Pull request #702 (2022-05-23 10:55:25)

Add check for existing resourceTypes in assessment result
Merge after MEDINA midterm review

codecov[bot]: # [Codecov](https://codecov.io/gh/clouditor/clouditor/pull/702?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clouditor) Report
> Merging [#702](https://codecov.io/gh/clouditor/clouditor/pull/702?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clouditor) (0c0393e) into [main](https://codecov.io/gh/clouditor/clouditor/commit/c641879888128bfcce3cc85da3b2a9910b759d90?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clouditor) (c641879) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.

```diff
@@           Coverage Diff           @@
##             main     #702   +/-   ##
=======================================
  Coverage   81.31%   81.32%           
=======================================
  Files          65       65           
  Lines        6092     6095    +3     
=======================================
+ Hits         4954     4957    +3     
  Misses        879      879           
  Partials      259      259           
```

| Flag | Coverage Δ | |
|---|---|---|
| unittests | `81.32% <100.00%> (+<0.01%)` | :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/702?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clouditor) | Coverage Δ | |
|---|---|---|
| [api/assessment/assessment.go](https://codecov.io/gh/clouditor/clouditor/pull/702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clouditor#diff-YXBpL2Fzc2Vzc21lbnQvYXNzZXNzbWVudC5nbw==) | `89.65% <100.00%> (+1.19%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/clouditor/clouditor/pull/702?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/702?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=clouditor). Last update [c641879...0c0393e](https://codecov.io/gh/clouditor/clouditor/pull/702?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).

oxisto: Blocked until MEDINA review is done.
anatheka: @oxisto can we now merge that to main? 

Patch:
diff --git a/api/assessment/assessment.go b/api/assessment/assessment.go
index adfe3342..8a52d669 100644
--- a/api/assessment/assessment.go
+++ b/api/assessment/assessment.go
@@ -39,6 +39,7 @@ var (
 	ErrTimestampMissing                      = errors.New("timestamp in assessment result is missing")
 	ErrResourceIdMissing                     = errors.New("resource id in assessment result is missing")
 	ErrMetricIdMissing                       = errors.New("metric id is missing")
+	ErrResourceTypesMissing                  = errors.New("resource types in assessment result is missing")
 	ErrMetricConfigurationMissing            = errors.New("metric configuration in assessment result is missing")
 	ErrMetricConfigurationOperatorMissing    = errors.New("operator in metric data is missing")
 	ErrMetricConfigurationTargetValueMissing = errors.New("target value in metric data is missing")
@@ -78,5 +79,8 @@ func (result *AssessmentResult) Validate() (resourceId string, err error) {
 		return "", ErrResourceIdMissing
 	}
 
+	if len(result.ResourceTypes) == 0 {
+		return "", ErrResourceTypesMissing
+	}
 	return
 }


Test patch:
diff --git a/api/assessment/assessment_test.go b/api/assessment/assessment_test.go
index 5e70af57..4be5d2dd 100644
--- a/api/assessment/assessment_test.go
+++ b/api/assessment/assessment_test.go
@@ -63,7 +63,8 @@ func Test_ValidateAssessmentResult(t *testing.T) {
 							},
 						},
 					},
-					EvidenceId: mockEvidenceID,
+					EvidenceId:    mockEvidenceID,
+					ResourceTypes: []string{"Resource"},
 				},
 			},
 			wantResp: "",
@@ -86,7 +87,8 @@ func Test_ValidateAssessmentResult(t *testing.T) {
 							},
 						},
 					},
-					EvidenceId: mockEvidenceID,
+					EvidenceId:    mockEvidenceID,
+					ResourceTypes: []string{"Resource"},
 				},
 			},
 			wantResp: "",
@@ -109,7 +111,8 @@ func Test_ValidateAssessmentResult(t *testing.T) {
 							},
 						},
 					},
-					EvidenceId: mockEvidenceID,
+					EvidenceId:    mockEvidenceID,
+					ResourceTypes: []string{"Resource"},
 				},
 			},
 			wantResp: "",
@@ -131,7 +134,8 @@ func Test_ValidateAssessmentResult(t *testing.T) {
 							},
 						},
 					},
-					EvidenceId: mockEvidenceID,
+					EvidenceId:    mockEvidenceID,
+					ResourceTypes: []string{"Resource"},
 				},
 			},
 			wantResp:      "",
@@ -152,7 +156,8 @@ func Test_ValidateAssessmentResult(t *testing.T) {
 							},
 						},
 					},
-					EvidenceId: mockEvidenceID,
+					ResourceTypes: []string{"Resource"},
+					EvidenceId:    mockEvidenceID,
 				},
 			},
 			wantResp:      "",
@@ -160,16 +165,40 @@ func Test_ValidateAssessmentResult(t *testing.T) {
 			wantErr:       true,
 		},
 		{
-			name: "Missing assessment result metric configuration",
+			name: "Missing assessment result resource types",
 			args: args{
 				&AssessmentResult{
-					Id:         assessmentResultID,
-					Timestamp:  timestamppb.Now(),
-					MetricId:   "MockMetricID",
+					Id:        assessmentResultID,
+					Timestamp: timestamppb.Now(),
+					MetricId:  "MockMetricID",
+					MetricConfiguration: &MetricConfiguration{
+						Operator: "MockOperator",
+						TargetValue: &structpb.Value{
+							Kind: &structpb.Value_StringValue{
+								StringValue: "MockTargetValue",
+							},
+						},
+					},
+					ResourceId: "myResource",
 					EvidenceId: mockEvidenceID,
 				},
 			},
 			wantResp:      "",
+			wantRespError: ErrResourceTypesMissing,
+			wantErr:       true,
+		},
+		{
+			name: "Missing assessment result metric configuration",
+			args: args{
+				&AssessmentResult{
+					Id:            assessmentResultID,
+					Timestamp:     timestamppb.Now(),
+					MetricId:      "MockMetricID",
+					EvidenceId:    mockEvidenceID,
+					ResourceTypes: []string{"Resource"},
+				},
+			},
+			wantResp:      "",
 			wantRespError: ErrMetricConfigurationMissing,
 			wantErr:       true,
 		},
@@ -187,7 +216,8 @@ func Test_ValidateAssessmentResult(t *testing.T) {
 							},
 						},
 					},
-					EvidenceId: mockEvidenceID,
+					EvidenceId:    mockEvidenceID,
+					ResourceTypes: []string{"Resource"},
 				},
 			},
 			wantResp:      "",
@@ -204,7 +234,8 @@ func Test_ValidateAssessmentResult(t *testing.T) {
 					MetricConfiguration: &MetricConfiguration{
 						Operator: "MockOperator",
 					},
-					EvidenceId: mockEvidenceID,
+					EvidenceId:    mockEvidenceID,
+					ResourceTypes: []string{"Resource"},
 				},
 			},
 			wantResp:      "",
@@ -226,6 +257,7 @@ func Test_ValidateAssessmentResult(t *testing.T) {
 							},
 						},
 					},
+					ResourceTypes: []string{"Resource"},
 				},
 			},
 			wantResp:      "",
@@ -247,8 +279,9 @@ func Test_ValidateAssessmentResult(t *testing.T) {
 							},
 						},
 					},
-					EvidenceId: mockEvidenceID,
-					ResourceId: "myResource",
+					EvidenceId:    mockEvidenceID,
+					ResourceId:    "myResource",
+					ResourceTypes: []string{"Resource"},
 				},
 			},
 			wantResp:      "",
diff --git a/cli/commands/assessmentresult/assessment_result_test.go b/cli/commands/assessmentresult/assessment_result_test.go
index 9983c789..a2def110 100644
--- a/cli/commands/assessmentresult/assessment_result_test.go
+++ b/cli/commands/assessmentresult/assessment_result_test.go
@@ -58,11 +58,12 @@ func TestMain(m *testing.M) {
 	// Store an assessment result so that output of CMD 'list' is not empty
 	_, err = svc.StoreAssessmentResult(context.TODO(), &orchestrator.StoreAssessmentResultRequest{
 		Result: &assessment.AssessmentResult{
-			Id:         "11111111-1111-1111-1111-111111111111",
-			MetricId:   "assessmentResultMetricID",
-			EvidenceId: "11111111-1111-1111-1111-111111111111",
-			Timestamp:  timestamppb.Now(),
-			ResourceId: "myResource",
+			Id:            "11111111-1111-1111-1111-111111111111",
+			MetricId:      "assessmentResultMetricID",
+			EvidenceId:    "11111111-1111-1111-1111-111111111111",
+			Timestamp:     timestamppb.Now(),
+			ResourceId:    "myResource",
+			ResourceTypes: []string{"ResourceType"},
 			MetricConfiguration: &assessment.MetricConfiguration{
 				TargetValue: toStruct(1.0),
 				Operator:    "operator",
diff --git a/cli/commands/service/orchestrator/orchestrator_test.go b/cli/commands/service/orchestrator/orchestrator_test.go
index c391ea7a..d0b4a0b1 100644
--- a/cli/commands/service/orchestrator/orchestrator_test.go
+++ b/cli/commands/service/orchestrator/orchestrator_test.go
@@ -60,11 +60,12 @@ func TestMain(m *testing.M) {
 	// Store an assessment result so that output of CMD 'list' is not empty
 	_, err = svc.StoreAssessmentResult(context.TODO(), &orchestrator.StoreAssessmentResultRequest{
 		Result: &assessment.AssessmentResult{
-			Id:         "11111111-1111-1111-1111-111111111111",
-			MetricId:   "assessmentResultMetricID",
-			EvidenceId: "11111111-1111-1111-1111-111111111111",
-			Timestamp:  timestamppb.Now(),
-			ResourceId: "myResource",
+			Id:            "11111111-1111-1111-1111-111111111111",
+			MetricId:      "assessmentResultMetricID",
+			EvidenceId:    "11111111-1111-1111-1111-111111111111",
+			Timestamp:     timestamppb.Now(),
+			ResourceId:    "myResource",
+			ResourceTypes: []string{"ResourceType"},
 			MetricConfiguration: &assessment.MetricConfiguration{
 				TargetValue: toStruct(1.0),
 				Operator:    "operator",
diff --git a/service/orchestrator/orchestrator_test.go b/service/orchestrator/orchestrator_test.go
index 2925fc68..1ea0facc 100644
--- a/service/orchestrator/orchestrator_test.go
+++ b/service/orchestrator/orchestrator_test.go
@@ -129,6 +129,7 @@ func TestAssessmentResultHook(t *testing.T) {
 						NonComplianceComments: "non_compliance_comment",
 						Compliant:             true,
 						ResourceId:            "resourceID",
+						ResourceTypes:         []string{"ResourceType"},
 					},
 				},
 			},
@@ -190,6 +191,7 @@ func TestStoreAssessmentResult(t *testing.T) {
 						NonComplianceComments: "non_compliance_comment",
 						Compliant:             true,
 						ResourceId:            "resourceID",
+						ResourceTypes:         []string{"ResourceType"},
 					},
 				},
 			},
@@ -215,6 +217,7 @@ func TestStoreAssessmentResult(t *testing.T) {
 						NonComplianceComments: "non_compliance_comment",
 						Compliant:             true,
 						ResourceId:            "resourceID",
+						ResourceTypes:         []string{"ResourceType"},
 					},
 				},
 			},
@@ -394,6 +397,7 @@ func createStoreAssessmentResultRequestsMock(count int) []*orchestrator.StoreAss
 				NonComplianceComments: "non_compliance_comment",
 				Compliant:             true,
 				ResourceId:            "resourceID",
+				ResourceTypes:         []string{"ResourceType"},
 			},
 		}