Add reference to central entities table in new rule eval history tabl…

…es (#4204) This starts populating the relevant data so we can migrate to using the central table in a further increment. Signed-off-by: Juan Antonio Osorio <[email protected]>
mindersec · Aug 20, 2024 · 4c18b4d · 4c18b4d
1 parent d0952b8
commit 4c18b4d
Show file tree

Hide file tree

Showing 13 changed files with 180 additions and 28 deletions.
diff --git a/cmd/server/app/history_purge_test.go b/cmd/server/app/history_purge_test.go
@@ -47,7 +47,7 @@ func TestRecordSize(t *testing.T) {
 		},
 	)
 
-	require.Equal(t, 80, int(size))
+	require.Equal(t, 96, int(size))
 }
 
 func TestPurgeLoop(t *testing.T) {

diff --git a/cmd/server/app/migrate_up.go b/cmd/server/app/migrate_up.go
@@ -127,6 +127,10 @@ var upCmd = &cobra.Command{
 			cmd.Printf("Error while populating entities table with pull requests: %v\n", err)
 		}
 
+		if err := store.TemporaryPopulateEvaluationHistory(ctx); err != nil {
+			cmd.Printf("Error while populating entities table with evaluation history: %v\n", err)
+		}
+
 		return nil
 	},
 }

diff --git a/database/migrations/000095_eval_central_entities.down.sql b/database/migrations/000095_eval_central_entities.down.sql
@@ -0,0 +1,21 @@
+-- Copyright 2024 Stacklok, Inc
+--
+-- Licensed under the Apache License, Version 2.0 (the "License");
+-- you may not use this file except in compliance with the License.
+-- You may obtain a copy of the License at
+--
+--      http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+
+BEGIN;
+
+-- Drop optional FK towards entity_instances(id) in evaluation_rule_entities
+
+ALTER TABLE evaluation_rule_entities DROP COLUMN entity_instance_id;
+
+COMMIT;
diff --git a/database/migrations/000095_eval_central_entities.up.sql b/database/migrations/000095_eval_central_entities.up.sql
@@ -0,0 +1,22 @@
+-- Copyright 2024 Stacklok, Inc
+--
+-- Licensed under the Apache License, Version 2.0 (the "License");
+-- you may not use this file except in compliance with the License.
+-- You may obtain a copy of the License at
+--
+--      http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+
+BEGIN;
+
+-- Add optional FK towards entity_instances(id) in evaluation_rule_entities
+
+ALTER TABLE evaluation_rule_entities ADD COLUMN entity_instance_id UUID;
+ALTER TABLE evaluation_rule_entities ADD CONSTRAINT fk_entity_instance_id FOREIGN KEY (entity_instance_id) REFERENCES entity_instances(id) ON DELETE CASCADE;
+
+COMMIT;
diff --git a/database/mock/store.go b/database/mock/store.go
diff --git a/database/query/eval_history.sql b/database/query/eval_history.sql
@@ -30,13 +30,15 @@ INSERT INTO evaluation_rule_entities(
     repository_id,
     pull_request_id,
     artifact_id,
-    entity_type
+    entity_type,
+    entity_instance_id
 ) VALUES (
     $1,
     $2,
     $3,
     $4,
-    $5
+    $5,
+    $6
 )
 RETURNING id;
 
@@ -105,6 +107,7 @@ SELECT s.id::uuid AS evaluation_id,
                WHEN ere.artifact_id IS NOT NULL THEN a.id
            END AS uuid
        ) AS entity_id,
+    ere.entity_instance_id as new_entity_id,
     -- raw fields for entity names
     r.repo_owner,
     r.repo_name,
@@ -150,6 +153,7 @@ SELECT s.id::uuid AS evaluation_id,
          WHEN ere.artifact_id IS NOT NULL THEN a.id
          END AS uuid
        ) AS entity_id,
+        ere.entity_instance_id as new_entity_id,
        -- raw fields for entity names
        r.repo_owner,
        r.repo_name,
@@ -230,7 +234,8 @@ SELECT s.evaluation_time,
          WHEN ere.pull_request_id IS NOT NULL THEN ere.pull_request_id
          WHEN ere.artifact_id IS NOT NULL THEN ere.artifact_id
          END AS uuid
-       ) AS entity_id
+       ) AS entity_id,
+       ere.entity_instance_id as new_entity_id
   FROM evaluation_statuses s
        JOIN evaluation_rule_entities ere ON s.rule_entity_id = ere.id
        LEFT JOIN latest_evaluation_statuses l
@@ -246,3 +251,20 @@ SELECT s.evaluation_time,
 -- name: DeleteEvaluationHistoryByIDs :execrows
 DELETE FROM evaluation_statuses s
  WHERE s.id = ANY(sqlc.slice(evaluationIds));
+
+-- TemporaryPopulateEvaluationHistory sets the entity_instance_id column for 
+-- all existing evaluation_rule_entities records to the id of the entity
+-- instance that the rule entity is associated with. We derive this from the entity_type
+-- and the corresponding entity id (repository_id, pull_request_id, or artifact_id).
+-- Note that there are cases where repository_id and pull_request_id will both be set,
+-- so we need to rely on the entity_type to determine which one to use. The same
+-- applies to repository_id and artifact_id.
+
+-- name: TemporaryPopulateEvaluationHistory :exec
+UPDATE evaluation_rule_entities ere
+SET entity_instance_id = CASE
+    WHEN ere.entity_type = 'repository' THEN ere.repository_id
+    WHEN ere.entity_type = 'pull_request' THEN ere.pull_request_id
+    WHEN ere.entity_type = 'artifact' THEN ere.artifact_id
+END
+WHERE entity_instance_id IS NULL;
diff --git a/internal/db/eval_history.sql.go b/internal/db/eval_history.sql.go
diff --git a/internal/db/eval_history_test.go b/internal/db/eval_history_test.go
@@ -719,6 +719,10 @@ func createRandomEvaluationRuleEntity(
 				Valid: true,
 			},
 			EntityType: EntitiesRepository,
+			EntityInstanceID: uuid.NullUUID{
+				UUID:  entityID,
+				Valid: true,
+			},
 		},
 	)
 	require.NoError(t, err)

diff --git a/internal/db/models.go b/internal/db/models.go
diff --git a/internal/db/profiles_test.go b/internal/db/profiles_test.go
@@ -165,6 +165,10 @@ func createRuleEntity(
 			PullRequestID: uuid.NullUUID{},
 			ArtifactID:    uuid.NullUUID{},
 			EntityType:    EntitiesRepository,
+			EntityInstanceID: uuid.NullUUID{
+				UUID:  repoID,
+				Valid: true,
+			},
 		},
 	)
 	require.NoError(t, err)

diff --git a/internal/db/querier.go b/internal/db/querier.go
diff --git a/internal/db/repositories_test.go b/internal/db/repositories_test.go
@@ -67,6 +67,16 @@ func createRandomRepository(t *testing.T, project uuid.UUID, prov Provider, opts
 	require.NoError(t, err)
 	require.NotEmpty(t, repo)
 
+	ei, err := testQueries.CreateEntityWithID(context.Background(), CreateEntityWithIDParams{
+		ID:         repo.ID,
+		Name:       arg.RepoName,
+		ProjectID:  project,
+		ProviderID: prov.ID,
+		EntityType: EntitiesRepository,
+	})
+	require.NoError(t, err)
+	require.NotEmpty(t, ei)
+
 	require.Equal(t, arg.Provider, repo.Provider)
 	require.Equal(t, arg.ProjectID, repo.ProjectID)
 	require.Equal(t, arg.RepoOwner, repo.RepoOwner)