Skip to content

Commit

Permalink
Add profile ID to latest_evaluation_statuses (#3993)
Browse files Browse the repository at this point in the history
Adding this information to the `latest_evaluation_statuses` table
simplifies the process of changing the profile status queries to use the
evaluation history tables instead of the old tables I am trying to
replace.
  • Loading branch information
dmjb authored Jul 25, 2024
1 parent 211f5f3 commit 1cff724
Show file tree
Hide file tree
Showing 10 changed files with 68 additions and 16 deletions.
18 changes: 18 additions & 0 deletions database/migrations/000078_latest_status_profile_id.down.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
-- 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.

-- introduce some denormalization to simplify a common access pattern
-- namely: retrieving the latest rule statuses for a specific profile
-- A future PR will backfill this column and make it NOT NULL
ALTER TABLE latest_evaluation_statuses DROP COLUMN profile_id;
19 changes: 19 additions & 0 deletions database/migrations/000078_latest_status_profile_id.up.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
-- 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.

-- introduce some denormalization to simplify a common access pattern
-- namely: retrieving the latest rule statuses for a specific profile
-- A future PR will backfill this column and make it NOT NULL
ALTER TABLE latest_evaluation_statuses ADD COLUMN profile_id UUID REFERENCES profiles(id);
CREATE INDEX idx_profile_id ON latest_evaluation_statuses(profile_id);
6 changes: 4 additions & 2 deletions database/query/eval_history.sql
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,12 @@ RETURNING id;
-- name: UpsertLatestEvaluationStatus :exec
INSERT INTO latest_evaluation_statuses(
rule_entity_id,
evaluation_history_id
evaluation_history_id,
profile_id
) VALUES (
$1,
$2
$2,
$3
)
ON CONFLICT (rule_entity_id) DO UPDATE
SET evaluation_history_id = $2;
Expand Down
13 changes: 8 additions & 5 deletions internal/db/eval_history.sql.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 3 additions & 2 deletions internal/db/models.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions internal/engine/eval_status.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,7 @@ func (e *executor) createOrUpdateEvalStatus(
ctx,
qtx,
params.Rule.ID,
params.Profile.ID,
params.EntityType,
entityID,
params.GetEvalErr(),
Expand Down
2 changes: 1 addition & 1 deletion internal/engine/executor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,7 @@ default allow = true`,

historyService := mockhistory.NewMockEvaluationHistoryService(ctrl)
historyService.EXPECT().
StoreEvaluationStatus(gomock.Any(), gomock.Any(), ruleInstanceID, db.EntitiesRepository, repositoryID, gomock.Any()).
StoreEvaluationStatus(gomock.Any(), gomock.Any(), ruleInstanceID, profileID, db.EntitiesRepository, repositoryID, gomock.Any()).
Return(uuid.New(), nil)

mockStore.EXPECT().
Expand Down
8 changes: 4 additions & 4 deletions internal/history/mock/service.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 8 additions & 1 deletion internal/history/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ type EvaluationHistoryService interface {
ctx context.Context,
qtx db.Querier,
ruleID uuid.UUID,
profileID uuid.UUID,
entityType db.Entities,
entityID uuid.UUID,
evalError error,
Expand All @@ -64,6 +65,7 @@ func (e *evaluationHistoryService) StoreEvaluationStatus(
ctx context.Context,
qtx db.Querier,
ruleID uuid.UUID,
profileID uuid.UUID,
entityType db.Entities,
entityID uuid.UUID,
evalError error,
Expand Down Expand Up @@ -111,7 +113,7 @@ func (e *evaluationHistoryService) StoreEvaluationStatus(
ruleEntityID = latestRecord.RuleEntityID
}

evaluationID, err := e.createNewStatus(ctx, qtx, ruleEntityID, status, details)
evaluationID, err := e.createNewStatus(ctx, qtx, ruleEntityID, profileID, status, details)
if err != nil {
return uuid.Nil, fmt.Errorf("error while creating new evaluation status for rule/entity %s: %w", ruleEntityID, err)
}
Expand All @@ -123,6 +125,7 @@ func (_ *evaluationHistoryService) createNewStatus(
ctx context.Context,
qtx db.Querier,
ruleEntityID uuid.UUID,
profileID uuid.UUID,
status db.EvalStatusTypes,
details string,
) (uuid.UUID, error) {
Expand All @@ -142,6 +145,10 @@ func (_ *evaluationHistoryService) createNewStatus(
db.UpsertLatestEvaluationStatusParams{
RuleEntityID: ruleEntityID,
EvaluationHistoryID: newEvaluationID,
ProfileID: uuid.NullUUID{
UUID: profileID,
Valid: true,
},
},
)
if err != nil {
Expand Down
3 changes: 2 additions & 1 deletion internal/history/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ func TestStoreEvaluationStatus(t *testing.T) {
}

service := NewEvaluationHistoryService()
id, err := service.StoreEvaluationStatus(ctx, store, ruleID, scenario.EntityType, entityID, errTest)
id, err := service.StoreEvaluationStatus(ctx, store, ruleID, profileID, scenario.EntityType, entityID, errTest)
if scenario.ExpectedError == "" {
require.Equal(t, evaluationID, id)
require.NoError(t, err)
Expand Down Expand Up @@ -795,6 +795,7 @@ func makeHistoryRow(

var (
ruleID = uuid.New()
profileID = uuid.New()
entityID = uuid.New()
ruleEntityID = uuid.New()
evaluationID = uuid.New()
Expand Down

0 comments on commit 1cff724

Please sign in to comment.