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

@@ -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

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/[email protected]?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/[email protected]?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")
+	}
+}

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)
 	}