From 4c206b49084c7a5abb9cf91ddc8023b3e0286381 Mon Sep 17 00:00:00 2001 From: YiscahLevySilas1 Date: Tue, 18 Apr 2023 14:24:15 +0300 Subject: [PATCH 1/8] fix compliance score- case of passed-irrelevant Signed-off-by: YiscahLevySilas1 --- score/score.go | 8 +++++++- score/score_test.go | 40 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/score/score.go b/score/score.go index 26897f5e..ce463683 100644 --- a/score/score.go +++ b/score/score.go @@ -425,7 +425,13 @@ func (su *ScoreUtil) GetControlComplianceScore(ctrl reportsummary.IControlSummar if numOfAllResources > 0 { ctrlScore = (numOfPassedResources / numOfAllResources) * 100 } else { - logger.L().Debug("no resources were given for this control, score is 0", helpers.String("controlID", ctrl.GetID())) + // in case the control didn't run on any resources: + // If the control passed (irrelevant): score = 100 , else (skipped): score = 0 + if ctrl.GetStatus().IsPassed() { + ctrlScore = 100 + } else { + logger.L().Debug("no resources were given for this control, score is 0", helpers.String("controlID", ctrl.GetID())) + } } return ctrlScore diff --git a/score/score_test.go b/score/score_test.go index bf24f120..152e19bd 100644 --- a/score/score_test.go +++ b/score/score_test.go @@ -814,6 +814,46 @@ func TestGetControlComplianceScore(t *testing.T) { ) }) + t.Run("skipped control", func(t *testing.T) { + t.Parallel() + + resources := mockResources(t) + s := ScoreUtil{isDebugMode: true, resources: resources} + controlReport := reportsummary.ControlSummary{ + Name: "skipped-control", + ControlID: "skipped1", + StatusInfo: apis.StatusInfo{ + InnerInfo: "enable-host-scan flag not used. For more information: https://hub.armosec.io/docs/host-sensor", + InnerStatus: "skipped", + }, + ResourceIDs: helpers.AllLists{}, + } + + require.Equal(t, float32(0), s.GetControlComplianceScore(&controlReport, ""), + "skipped control report should return a score equals to 0", + ) + }) + + t.Run("passed (irrelevant) control", func(t *testing.T) { + t.Parallel() + + resources := mockResources(t) + s := ScoreUtil{isDebugMode: true, resources: resources} + controlReport := reportsummary.ControlSummary{ + Name: "irrelevant-control", + ControlID: "irrelevant1", + StatusInfo: apis.StatusInfo{ + SubStatus: "irrelevant", + InnerStatus: "passed", + }, + ResourceIDs: helpers.AllLists{}, + } + + require.Equal(t, float32(100), s.GetControlComplianceScore(&controlReport, ""), + "passed (irrelevant) control report should return a score equals to 100", + ) + }) + t.Run("with control report", func(t *testing.T) { t.Parallel() From 046ef020b698f258c0153090747a276bee0fab7c Mon Sep 17 00:00:00 2001 From: YiscahLevySilas1 Date: Tue, 18 Apr 2023 16:47:32 +0300 Subject: [PATCH 2/8] minor refactor following review Signed-off-by: YiscahLevySilas1 --- score/score.go | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/score/score.go b/score/score.go index ce463683..0bc57cf7 100644 --- a/score/score.go +++ b/score/score.go @@ -392,7 +392,8 @@ func (su *ScoreUtil) ControlsSummariesComplianceScore(ctrls *reportsummary.Contr // GetFrameworkComplianceScore returns the compliance score for a given framework (as a percentage) // The framework compliance score is the average of all controls scores in that framework -func (su *ScoreUtil) GetFrameworkComplianceScore(framework *reportsummary.FrameworkSummary) (frameworkScore float32) { +func (su *ScoreUtil) GetFrameworkComplianceScore(framework *reportsummary.FrameworkSummary) float32 { + frameworkScore := float32(0) sumScore := su.ControlsSummariesComplianceScore(&framework.Controls, framework.GetName()) if len(framework.Controls) > 0 { frameworkScore = sumScore / float32(len(framework.Controls)) @@ -401,7 +402,7 @@ func (su *ScoreUtil) GetFrameworkComplianceScore(framework *reportsummary.Framew } // GetControlComplianceScore returns the compliance score for a given control (as a percentage). -func (su *ScoreUtil) GetControlComplianceScore(ctrl reportsummary.IControlSummary, _ /*frameworkName*/ string) (ctrlScore float32) { +func (su *ScoreUtil) GetControlComplianceScore(ctrl reportsummary.IControlSummary, _ /*frameworkName*/ string) float32 { resourcesIDs := ctrl.ListResourcesIDs() passedResourceIDS := resourcesIDs.Passed() allResourcesIDSIter := resourcesIDs.All() @@ -423,16 +424,14 @@ func (su *ScoreUtil) GetControlComplianceScore(ctrl reportsummary.IControlSummar } if numOfAllResources > 0 { - ctrlScore = (numOfPassedResources / numOfAllResources) * 100 - } else { - // in case the control didn't run on any resources: - // If the control passed (irrelevant): score = 100 , else (skipped): score = 0 - if ctrl.GetStatus().IsPassed() { - ctrlScore = 100 - } else { - logger.L().Debug("no resources were given for this control, score is 0", helpers.String("controlID", ctrl.GetID())) - } + return (numOfPassedResources / numOfAllResources) * 100 } - return ctrlScore + // in case the control didn't run on any resources: + // If the control passed (irrelevant): score = 100 , else (skipped): score = 0 + if ctrl.GetStatus().IsPassed() { + return 100 + } + logger.L().Debug("no resources were given for this control, score is 0", helpers.String("controlID", ctrl.GetID())) + return 0 } From 9b848d3b535b7837f4b3a603acede8cc82f2fb99 Mon Sep 17 00:00:00 2001 From: YiscahLevySilas1 Date: Tue, 18 Apr 2023 16:56:32 +0300 Subject: [PATCH 3/8] delete log Signed-off-by: YiscahLevySilas1 --- score/score.go | 1 - 1 file changed, 1 deletion(-) diff --git a/score/score.go b/score/score.go index 0bc57cf7..0d5e5601 100644 --- a/score/score.go +++ b/score/score.go @@ -432,6 +432,5 @@ func (su *ScoreUtil) GetControlComplianceScore(ctrl reportsummary.IControlSummar if ctrl.GetStatus().IsPassed() { return 100 } - logger.L().Debug("no resources were given for this control, score is 0", helpers.String("controlID", ctrl.GetID())) return 0 } From 96a14d1d10bbd52bf36e809ae425d75cf2d2b524 Mon Sep 17 00:00:00 2001 From: YiscahLevySilas1 Date: Tue, 18 Apr 2023 17:59:20 +0300 Subject: [PATCH 4/8] move passed condition Signed-off-by: YiscahLevySilas1 --- score/score.go | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/score/score.go b/score/score.go index 0d5e5601..638578a4 100644 --- a/score/score.go +++ b/score/score.go @@ -403,6 +403,11 @@ func (su *ScoreUtil) GetFrameworkComplianceScore(framework *reportsummary.Framew // GetControlComplianceScore returns the compliance score for a given control (as a percentage). func (su *ScoreUtil) GetControlComplianceScore(ctrl reportsummary.IControlSummary, _ /*frameworkName*/ string) float32 { + // If a control has status passed it should always be considered as having 100% compliance score + if ctrl.GetStatus().IsPassed() { + return 100 + } + resourcesIDs := ctrl.ListResourcesIDs() passedResourceIDS := resourcesIDs.Passed() allResourcesIDSIter := resourcesIDs.All() @@ -426,11 +431,5 @@ func (su *ScoreUtil) GetControlComplianceScore(ctrl reportsummary.IControlSummar if numOfAllResources > 0 { return (numOfPassedResources / numOfAllResources) * 100 } - - // in case the control didn't run on any resources: - // If the control passed (irrelevant): score = 100 , else (skipped): score = 0 - if ctrl.GetStatus().IsPassed() { - return 100 - } return 0 } From a12f9cccf2218bb95ebb6f8814d82887e87ad5da Mon Sep 17 00:00:00 2001 From: YiscahLevySilas1 Date: Wed, 19 Apr 2023 11:05:01 +0300 Subject: [PATCH 5/8] shorten code, delete logs Signed-off-by: YiscahLevySilas1 --- score/score.go | 26 +++----------------------- 1 file changed, 3 insertions(+), 23 deletions(-) diff --git a/score/score.go b/score/score.go index 638578a4..39f7c0ba 100644 --- a/score/score.go +++ b/score/score.go @@ -8,8 +8,6 @@ import ( "strings" "sync" - "github.com/kubescape/go-logger" - "github.com/kubescape/go-logger/helpers" k8sinterface "github.com/kubescape/k8s-interface/k8sinterface" "github.com/kubescape/k8s-interface/workloadinterface" armoupautils "github.com/kubescape/opa-utils/objectsenvelopes" @@ -363,7 +361,6 @@ func (su *ScoreUtil) SetPostureReportComplianceScores(report *v2.PostureReport) for i := range report.SummaryDetails.Frameworks { // set compliance score for framework and all controls in framework report.SummaryDetails.Frameworks[i].ComplianceScore = su.GetFrameworkComplianceScore(&report.SummaryDetails.Frameworks[i]) - logger.L().Debug("set framework score", helpers.String("framework name", report.SummaryDetails.Frameworks[i].GetName()), helpers.Int("ComplianceScore", int(report.SummaryDetails.Frameworks[i].GetComplianceScore()))) } // set compliance score per control sumScore := su.ControlsSummariesComplianceScore(&report.SummaryDetails.Controls, "") @@ -384,7 +381,6 @@ func (su *ScoreUtil) ControlsSummariesComplianceScore(ctrls *reportsummary.Contr ctrl.Score = 0 ctrl.Score = su.GetControlComplianceScore(&ctrl, frameworkName) (*ctrls)[ctrlID] = ctrl - logger.L().Debug("set control score", helpers.String("controlID", ctrl.GetID()), helpers.Int("score", int(ctrl.GetScore()))) sumScore += ctrl.GetScore() } return sumScore @@ -409,27 +405,11 @@ func (su *ScoreUtil) GetControlComplianceScore(ctrl reportsummary.IControlSummar } resourcesIDs := ctrl.ListResourcesIDs() - passedResourceIDS := resourcesIDs.Passed() - allResourcesIDSIter := resourcesIDs.All() - - numOfPassedResources := float32(0) - numOfAllResources := float32(0) - - for i := range passedResourceIDS { - if _, ok := su.resources[passedResourceIDS[i]]; ok { - numOfPassedResources += 1 - } - } - - for allResourcesIDSIter.HasNext() { - resourceID := allResourcesIDSIter.Next() - if _, ok := su.resources[resourceID]; ok { - numOfAllResources += 1 - } - } + numOfPassedResources := len(resourcesIDs.Passed()) + numOfAllResources := resourcesIDs.All().Len() if numOfAllResources > 0 { - return (numOfPassedResources / numOfAllResources) * 100 + return (float32(numOfPassedResources) / float32(numOfAllResources)) * 100 } return 0 } From 75931e22234dae4883280711286b9d7126997f36 Mon Sep 17 00:00:00 2001 From: YiscahLevySilas1 Date: Wed, 19 Apr 2023 12:51:29 +0300 Subject: [PATCH 6/8] add tests for coverage Signed-off-by: YiscahLevySilas1 --- objectsenvelopes/objectshandler_test.go | 107 ++++++++++++++++++++++++ 1 file changed, 107 insertions(+) create mode 100644 objectsenvelopes/objectshandler_test.go diff --git a/objectsenvelopes/objectshandler_test.go b/objectsenvelopes/objectshandler_test.go new file mode 100644 index 00000000..30d36a2b --- /dev/null +++ b/objectsenvelopes/objectshandler_test.go @@ -0,0 +1,107 @@ +package objectsenvelopes + +import ( + "testing" + + cloudsupportv1 "github.com/kubescape/k8s-interface/cloudsupport/v1" + "github.com/kubescape/k8s-interface/workloadinterface" + "github.com/kubescape/opa-utils/objectsenvelopes/hostsensor" + "github.com/kubescape/opa-utils/objectsenvelopes/localworkload" + "github.com/stretchr/testify/assert" +) + +func TestNewObject(t *testing.T) { + // Test nil input + assert.Nil(t, NewObject(nil)) + + // Test valid input + object := map[string]interface{}{ + "kind": "Pod", + "apiVersion": "v1", + "metadata": map[string]interface{}{ + "name": "test-pod", + }, + } + workloadObj := NewObject(object) + assert.NotNil(t, workloadObj) + assert.Equal(t, "Pod", workloadObj.GetKind()) + + // Test unsupported input + unsupportedObject := map[string]interface{}{ + "unknown": "unknown", + } + assert.Nil(t, NewObject(unsupportedObject)) +} + +func TestGetObjectType(t *testing.T) { + // Test unsupported input + unsupportedObject := map[string]interface{}{ + "unknown": "unknown", + } + assert.Equal(t, workloadinterface.TypeUnknown, GetObjectType(unsupportedObject)) + + // Test RegoResponseVectorObject + relatedObjects := []map[string]interface{}{} + relatedObject := getMock(role) + relatedObject2 := getMock(rolebinding) + relatedObjects = append(relatedObjects, relatedObject) + relatedObjects = append(relatedObjects, relatedObject2) + subject := map[string]interface{}{"name": "user@example.com", "kind": "User", "namespace": "default", "group": "rbac.authorization.k8s.io", RelatedObjectsKey: relatedObjects} + assert.Equal(t, TypeRegoResponseVectorObject, GetObjectType(subject)) + + // Test CloudProviderDescribe + cloudProviderDescribe := map[string]interface{}{ + "apiVersion": "container.googleapis.com/v1", + "kind": "ClusterDescribe", + } + assert.Equal(t, cloudsupportv1.TypeCloudProviderDescribe, GetObjectType(cloudProviderDescribe)) + + // Test HostSensor + hostSensor := map[string]interface{}{ + "apiVersion": "hostdata.kubescape.cloud/v1", + "metadata": map[string]interface{}{ + "name": "test-pod", + }, + } + assert.Equal(t, hostsensor.TypeHostSensor, GetObjectType(hostSensor)) + + // Test LocalWorkload + localWorkload := map[string]interface{}{ + "kind": "b", + "sourcePath": "/path/file", + } + assert.Equal(t, localworkload.TypeLocalWorkload, GetObjectType(localWorkload)) + + // Test WorkloadObject + workloadObject := map[string]interface{}{ + "kind": "Pod", + "apiVersion": "v1", + "metadata": map[string]interface{}{ + "name": "test-pod", + }, + } + assert.Equal(t, workloadinterface.TypeWorkloadObject, GetObjectType(workloadObject)) + + // Test ListWorkloads + listWorkloads := map[string]interface{}{ + "kind": "List", + "items": []interface{}{ + map[string]interface{}{ + "kind": "Pod", + "metadata": map[string]interface{}{ + "name": "test-pod", + }, + }, + }, + } + assert.Equal(t, workloadinterface.TypeListWorkloads, GetObjectType(listWorkloads)) +} + +// // Test BaseObject +// baseObject := map[string]interface{}{ +// "kind": "Pod", +// "metadata": map[string]interface{}{ +// "name": "test-pod", +// }, +// } +// assert.Equal(t From 8490498b42249542eca1e49fc0f91d8a800efabf Mon Sep 17 00:00:00 2001 From: YiscahLevySilas1 Date: Wed, 19 Apr 2023 15:56:13 +0300 Subject: [PATCH 7/8] clean code Signed-off-by: YiscahLevySilas1 --- objectsenvelopes/objectshandler_test.go | 9 --------- 1 file changed, 9 deletions(-) diff --git a/objectsenvelopes/objectshandler_test.go b/objectsenvelopes/objectshandler_test.go index 30d36a2b..1198e3e5 100644 --- a/objectsenvelopes/objectshandler_test.go +++ b/objectsenvelopes/objectshandler_test.go @@ -96,12 +96,3 @@ func TestGetObjectType(t *testing.T) { } assert.Equal(t, workloadinterface.TypeListWorkloads, GetObjectType(listWorkloads)) } - -// // Test BaseObject -// baseObject := map[string]interface{}{ -// "kind": "Pod", -// "metadata": map[string]interface{}{ -// "name": "test-pod", -// }, -// } -// assert.Equal(t From 9ee06755920ca58730c3e8b4a52caeef9928956e Mon Sep 17 00:00:00 2001 From: YiscahLevySilas1 Date: Thu, 27 Apr 2023 15:14:44 +0300 Subject: [PATCH 8/8] add complianceScore field to ControlSummary Signed-off-by: YiscahLevySilas1 --- .../v1/reportsummary/controlsummarymethods.go | 8 +++++++ .../v1/reportsummary/datastructures.go | 1 + .../results/v1/reportsummary/interface.go | 3 +++ score/score.go | 9 ++++---- score/score_test.go | 21 +++++++++++++------ shared/pointer.go | 5 +++++ 6 files changed, 36 insertions(+), 11 deletions(-) create mode 100644 shared/pointer.go diff --git a/reporthandling/results/v1/reportsummary/controlsummarymethods.go b/reporthandling/results/v1/reportsummary/controlsummarymethods.go index 8415c717..0fdb62be 100644 --- a/reporthandling/results/v1/reportsummary/controlsummarymethods.go +++ b/reporthandling/results/v1/reportsummary/controlsummarymethods.go @@ -116,6 +116,14 @@ func (controlSummary *ControlSummary) GetScore() float32 { return controlSummary.Score } +func (controlSummary *ControlSummary) GetComplianceScore() float32 { + // if ComplianceScore field is set return it, else return -1 + if controlSummary.ComplianceScore != nil { + return *controlSummary.ComplianceScore + } + return -1 +} + // GetScoreFactor return control score func (controlSummary *ControlSummary) GetScoreFactor() float32 { return controlSummary.ScoreFactor diff --git a/reporthandling/results/v1/reportsummary/datastructures.go b/reporthandling/results/v1/reportsummary/datastructures.go index 7053b181..292d3e3a 100644 --- a/reporthandling/results/v1/reportsummary/datastructures.go +++ b/reporthandling/results/v1/reportsummary/datastructures.go @@ -47,6 +47,7 @@ type ControlSummary struct { StatusCounters StatusCounters `json:"ResourceCounters"` // Backward compatibility SubStatusCounters SubStatusCounters `json:"subStatusCounters"` Score float32 `json:"score"` + ComplianceScore *float32 `json:"complianceScore,omitempty"` ScoreFactor float32 `json:"scoreFactor"` } diff --git a/reporthandling/results/v1/reportsummary/interface.go b/reporthandling/results/v1/reportsummary/interface.go index d2bdd156..8619a014 100644 --- a/reporthandling/results/v1/reportsummary/interface.go +++ b/reporthandling/results/v1/reportsummary/interface.go @@ -69,6 +69,9 @@ type IPolicies interface { // Score GetScore() float32 + // ComplianceScore + GetComplianceScore() float32 + // Name GetName() string } diff --git a/score/score.go b/score/score.go index 39f7c0ba..9a8cb5f5 100644 --- a/score/score.go +++ b/score/score.go @@ -14,6 +14,7 @@ import ( "github.com/kubescape/opa-utils/reporthandling" "github.com/kubescape/opa-utils/reporthandling/results/v1/reportsummary" v2 "github.com/kubescape/opa-utils/reporthandling/v2" + "github.com/kubescape/opa-utils/shared" "go.uber.org/zap" appsv1 "k8s.io/api/apps/v1" ) @@ -351,8 +352,7 @@ func max32(a, b float32) float32 { // SetPostureReportComplianceScores calculates and populates scores for all controls, frameworks and whole scan. func (su *ScoreUtil) SetPostureReportComplianceScores(report *v2.PostureReport) error { // call CalculatePostureReportV2 to set frameworks.score and summaryDetails.score for backward compatibility - // afterwards we will override the controls.score - // and set frameworks.complianceScore and summaryDetails.complianceScore + // afterwards we set complianceScore for frameworks, controls and summaryDetails // TODO: remove CalculatePostureReportV2 call after we deprecate frameworks.score and summaryDetails.score if err := su.CalculatePostureReportV2(report); err != nil { return err @@ -378,10 +378,9 @@ func (su *ScoreUtil) SetPostureReportComplianceScores(report *v2.PostureReport) func (su *ScoreUtil) ControlsSummariesComplianceScore(ctrls *reportsummary.ControlSummaries, frameworkName string) (sumScore float32) { for ctrlID := range *ctrls { ctrl := (*ctrls)[ctrlID] - ctrl.Score = 0 - ctrl.Score = su.GetControlComplianceScore(&ctrl, frameworkName) + ctrl.ComplianceScore = shared.Ptr(su.GetControlComplianceScore(&ctrl, frameworkName)) (*ctrls)[ctrlID] = ctrl - sumScore += ctrl.GetScore() + sumScore += ctrl.GetComplianceScore() } return sumScore } diff --git a/score/score_test.go b/score/score_test.go index 152e19bd..15673b64 100644 --- a/score/score_test.go +++ b/score/score_test.go @@ -947,20 +947,29 @@ func TestSetPostureReportComplianceScores(t *testing.T) { t.Run("assert control scores", func(t *testing.T) { require.Len(t, report.SummaryDetails.Controls, 4) for _, control := range report.SummaryDetails.Controls { - var expectedForControl float64 + var expectedComplianceScore float64 + var expectedScore float64 switch control.ControlID { case "control-1": - expectedForControl = 33.333336 + expectedComplianceScore = 33.333336 + expectedScore = 81.13208 case "control-2": - expectedForControl = 100 // passed + expectedComplianceScore = 100 // passed + expectedScore = 0 case "control-3": - expectedForControl = 50 + expectedComplianceScore = 50 + expectedScore = 66.666664 case "control-4": - expectedForControl = 100 // passed + expectedComplianceScore = 100 // passed + expectedScore = 0 } - assert.InDeltaf(t, expectedForControl, control.Score, 1e-6, + assert.InDeltaf(t, expectedComplianceScore, *control.ComplianceScore, 1e-6, + "unexpected summarized score for control %q", control.ControlID, + ) + // check that control score wasn't overridden by compliance score + assert.InDeltaf(t, expectedScore, control.Score, 1e-6, "unexpected summarized score for control %q", control.ControlID, ) } diff --git a/shared/pointer.go b/shared/pointer.go new file mode 100644 index 00000000..b6399a0f --- /dev/null +++ b/shared/pointer.go @@ -0,0 +1,5 @@ +package shared + +func Ptr[T any](v T) *T { + return &v +}