From 5a0f4e150fa502f141523da8ff147d2f458eda0f Mon Sep 17 00:00:00 2001 From: Paul Norton Date: Wed, 31 Jan 2024 10:15:42 -0500 Subject: [PATCH] fix: do not retract from non-existent entities cherry-pick of atomist-skills/goal-evaluation-skill#221 > If we do not yet have results for a specific policy, attempting to retract fields within its result entity fails. --- policy/goals/entities.go | 9 ++++++--- policy/goals/entities_test.go | 22 +++++++++++++++++++--- policy/policy_handler/handler.go | 11 ++++++----- 3 files changed, 31 insertions(+), 11 deletions(-) diff --git a/policy/goals/entities.go b/policy/goals/entities.go index d690d69..31e30eb 100644 --- a/policy/goals/entities.go +++ b/policy/goals/entities.go @@ -18,13 +18,13 @@ package goals import "time" -func CreateEntitiesFromResults(results []GoalEvaluationQueryResult, goalDefinition string, goalConfiguration string, image string, storageId string, configHash string, evaluationTs time.Time, tx int64) GoalEvaluationResultEntity { +func CreateEntitiesFromResults(results []GoalEvaluationQueryResult, goalDefinition string, goalConfiguration string, image string, storageId string, configHash string, evaluationTs time.Time, tx int64, retract bool) GoalEvaluationResultEntity { entity := GoalEvaluationResultEntity{ Definition: goalDefinition, Configuration: goalConfiguration, Subject: DockerImageEntity{Digest: image}, - DeviationCount: RetractionEntity{Retract: true}, - StorageId: RetractionEntity{Retract: true}, + DeviationCount: nil, + StorageId: nil, ConfigHash: configHash, CreatedAt: evaluationTs, TransactionCondition: TransactionConditionEntity{ @@ -39,6 +39,9 @@ func CreateEntitiesFromResults(results []GoalEvaluationQueryResult, goalDefiniti entity.DeviationCount = deviationCount entity.StorageId = storageId + } else if retract { + entity.DeviationCount = RetractionEntity{Retract: true} + entity.StorageId = RetractionEntity{Retract: true} } return entity diff --git a/policy/goals/entities_test.go b/policy/goals/entities_test.go index 17e7d95..9a291ee 100644 --- a/policy/goals/entities_test.go +++ b/policy/goals/entities_test.go @@ -32,7 +32,7 @@ func TestCreateEntitiesFromResult(t *testing.T) { evaluationTs := time.Date(2023, 7, 10, 20, 1, 41, 0, time.UTC) - entity := CreateEntitiesFromResults(resultModel, "test-definition", "test-configuration", "test-image", "storage-id", "config-hash", evaluationTs, 123) + entity := CreateEntitiesFromResults(resultModel, "test-definition", "test-configuration", "test-image", "storage-id", "config-hash", evaluationTs, 123, false) if entity.Definition != "test-definition" || entity.Configuration != "test-configuration" || entity.StorageId != "storage-id" || entity.CreatedAt.Format("2006-01-02T15:04:05.000Z") != "2023-07-10T20:01:41.000Z" { t.Errorf("metadata not set correctly") @@ -43,7 +43,7 @@ func TestCreateEntitiesFromResult(t *testing.T) { } } -func TestNoDataSetsRetraction(t *testing.T) { +func TestNoDataSetsRetractionWhenPreviousResultsExit(t *testing.T) { result := `[{:name "CVE-2023-2650", :details {:purl "pkg:alpine/openssl@3.1.0-r4?os_name=alpine&os_version=3.18", :cve "CVE-2023-2650", :severity "HIGH", :fixed-by "3.1.1-r0"} }]` resultModel := []GoalEvaluationQueryResult{} @@ -52,9 +52,25 @@ func TestNoDataSetsRetraction(t *testing.T) { evaluationTs := time.Date(2023, 7, 10, 20, 1, 41, 0, time.UTC) - entity := CreateEntitiesFromResults(resultModel, "test-definition", "test-configuration", "test-image", "no-data", "config-hash", evaluationTs, 123) + entity := CreateEntitiesFromResults(resultModel, "test-definition", "test-configuration", "test-image", "no-data", "config-hash", evaluationTs, 123, true) if !entity.StorageId.(RetractionEntity).Retract || !entity.DeviationCount.(RetractionEntity).Retract { t.Errorf("metadata not set correctly") } } + +func TestNoDataNilsOutDeviationCountWhenNoPreviousResultsExist(t *testing.T) { + result := `[{:name "CVE-2023-2650", :details {:purl "pkg:alpine/openssl@3.1.0-r4?os_name=alpine&os_version=3.18", :cve "CVE-2023-2650", :severity "HIGH", :fixed-by "3.1.1-r0"} }]` + + resultModel := []GoalEvaluationQueryResult{} + + edn.Unmarshal([]byte(result), &resultModel) + + evaluationTs := time.Date(2023, 7, 10, 20, 1, 41, 0, time.UTC) + + entity := CreateEntitiesFromResults(resultModel, "test-definition", "test-configuration", "test-image", "no-data", "config-hash", evaluationTs, 123, false) + + if entity.StorageId != nil || entity.DeviationCount != nil { + t.Errorf("metadata not set correctly") + } +} diff --git a/policy/policy_handler/handler.go b/policy/policy_handler/handler.go index 0bb36e3..4f4bae9 100644 --- a/policy/policy_handler/handler.go +++ b/policy/policy_handler/handler.go @@ -197,8 +197,8 @@ func transact( tx int64, ) skill.Status { storageTuple := util.Decode[[]string](subscriptionResult[0][1]) - storageId := storageTuple[0] - configHash := storageTuple[1] + previousStorageId := storageTuple[0] + previousConfigHash := storageTuple[1] if goalResults == nil { req.Log.Infof("goal %s returned no data for digest %s", goal.Definition, digest) @@ -209,14 +209,14 @@ func transact( return skill.NewFailedStatus(fmt.Sprintf("Failed to create evaluation storage: %s", err.Error())) } - configDiffer, configHash, err := goals.GoalConfigsDiffer(req.Log, configuration, digest, goal, configHash) + configDiffer, configHash, err := goals.GoalConfigsDiffer(req.Log, configuration, digest, goal, previousConfigHash) if err != nil { req.Log.Errorf("Failed to check if config hash changed for digest: %s", digest, err) req.Log.Warnf("Will continue with the evaluation nonetheless") configDiffer = true } - differ, storageId, err := goals.GoalResultsDiffer(req.Log, goalResults, digest, goal, storageId) + differ, storageId, err := goals.GoalResultsDiffer(req.Log, goalResults, digest, goal, previousStorageId) if err != nil { req.Log.Errorf("Failed to check if goal results changed for digest: %s", digest, err) req.Log.Warnf("Will continue with the evaluation nonetheless") @@ -231,7 +231,8 @@ func transact( var entities []interface{} if differ || configDiffer { - entity := goals.CreateEntitiesFromResults(goalResults, goal.Definition, goal.Configuration, digest, storageId, configHash, evaluationTs, tx) + shouldRetract := previousStorageId != "no-data" && previousStorageId != "n/a" && storageId == "no-data" + entity := goals.CreateEntitiesFromResults(goalResults, goal.Definition, goal.Configuration, digest, storageId, configHash, evaluationTs, tx, shouldRetract) entities = append(entities, entity) }