diff --git a/database/migrations/000061_rule_instance_not_null_def.down.sql b/database/migrations/000061_rule_instance_not_null_def.down.sql new file mode 100644 index 0000000000..e1cb2a6518 --- /dev/null +++ b/database/migrations/000061_rule_instance_not_null_def.down.sql @@ -0,0 +1,20 @@ +-- 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; + +ALTER TABLE rule_instances ALTER COLUMN def DROP NOT NULL; +ALTER TABLE rule_instances ALTER COLUMN params DROP NOT NULL; + +COMMIT; diff --git a/database/migrations/000061_rule_instance_not_null_def.up.sql b/database/migrations/000061_rule_instance_not_null_def.up.sql new file mode 100644 index 0000000000..221892237a --- /dev/null +++ b/database/migrations/000061_rule_instance_not_null_def.up.sql @@ -0,0 +1,20 @@ +-- 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; + +ALTER TABLE rule_instances ALTER COLUMN def SET NOT NULL; +ALTER TABLE rule_instances ALTER COLUMN params SET NOT NULL; + +COMMIT; diff --git a/database/mock/store.go b/database/mock/store.go index 13e1eadb68..cd9465d282 100644 --- a/database/mock/store.go +++ b/database/mock/store.go @@ -354,6 +354,20 @@ func (mr *MockStoreMockRecorder) DeleteInstallationIDByAppID(arg0, arg1 any) *go return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteInstallationIDByAppID", reflect.TypeOf((*MockStore)(nil).DeleteInstallationIDByAppID), arg0, arg1) } +// DeleteNonUpdatedRules mocks base method. +func (m *MockStore) DeleteNonUpdatedRules(arg0 context.Context, arg1 db.DeleteNonUpdatedRulesParams) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "DeleteNonUpdatedRules", arg0, arg1) + ret0, _ := ret[0].(error) + return ret0 +} + +// DeleteNonUpdatedRules indicates an expected call of DeleteNonUpdatedRules. +func (mr *MockStoreMockRecorder) DeleteNonUpdatedRules(arg0, arg1 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteNonUpdatedRules", reflect.TypeOf((*MockStore)(nil).DeleteNonUpdatedRules), arg0, arg1) +} + // DeleteProfile mocks base method. func (m *MockStore) DeleteProfile(arg0 context.Context, arg1 db.DeleteProfileParams) error { m.ctrl.T.Helper() @@ -1108,6 +1122,36 @@ func (mr *MockStoreMockRecorder) GetRuleEvaluationByProfileIdAndRuleType(arg0, a return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetRuleEvaluationByProfileIdAndRuleType", reflect.TypeOf((*MockStore)(nil).GetRuleEvaluationByProfileIdAndRuleType), arg0, arg1, arg2, arg3, arg4, arg5) } +// GetRuleInstancesForProfile mocks base method. +func (m *MockStore) GetRuleInstancesForProfile(arg0 context.Context, arg1 uuid.UUID) ([]db.RuleInstance, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetRuleInstancesForProfile", arg0, arg1) + ret0, _ := ret[0].([]db.RuleInstance) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetRuleInstancesForProfile indicates an expected call of GetRuleInstancesForProfile. +func (mr *MockStoreMockRecorder) GetRuleInstancesForProfile(arg0, arg1 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetRuleInstancesForProfile", reflect.TypeOf((*MockStore)(nil).GetRuleInstancesForProfile), arg0, arg1) +} + +// GetRuleInstancesForProfileEntity mocks base method. +func (m *MockStore) GetRuleInstancesForProfileEntity(arg0 context.Context, arg1 db.GetRuleInstancesForProfileEntityParams) ([]db.RuleInstance, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetRuleInstancesForProfileEntity", arg0, arg1) + ret0, _ := ret[0].([]db.RuleInstance) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetRuleInstancesForProfileEntity indicates an expected call of GetRuleInstancesForProfileEntity. +func (mr *MockStoreMockRecorder) GetRuleInstancesForProfileEntity(arg0, arg1 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetRuleInstancesForProfileEntity", reflect.TypeOf((*MockStore)(nil).GetRuleInstancesForProfileEntity), arg0, arg1) +} + // GetRuleTypeByID mocks base method. func (m *MockStore) GetRuleTypeByID(arg0 context.Context, arg1 uuid.UUID) (db.RuleType, error) { m.ctrl.T.Helper() @@ -1761,6 +1805,21 @@ func (mr *MockStoreMockRecorder) UpsertRuleEvaluations(arg0, arg1 any) *gomock.C return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpsertRuleEvaluations", reflect.TypeOf((*MockStore)(nil).UpsertRuleEvaluations), arg0, arg1) } +// UpsertRuleInstance mocks base method. +func (m *MockStore) UpsertRuleInstance(arg0 context.Context, arg1 db.UpsertRuleInstanceParams) (uuid.UUID, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "UpsertRuleInstance", arg0, arg1) + ret0, _ := ret[0].(uuid.UUID) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// UpsertRuleInstance indicates an expected call of UpsertRuleInstance. +func (mr *MockStoreMockRecorder) UpsertRuleInstance(arg0, arg1 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpsertRuleInstance", reflect.TypeOf((*MockStore)(nil).UpsertRuleInstance), arg0, arg1) +} + // UpsertRuleInstantiation mocks base method. func (m *MockStore) UpsertRuleInstantiation(arg0 context.Context, arg1 db.UpsertRuleInstantiationParams) (db.EntityProfileRule, error) { m.ctrl.T.Helper() diff --git a/database/query/profiles.sql b/database/query/profiles.sql index 9e9755faf3..df12c01b5b 100644 --- a/database/query/profiles.sql +++ b/database/query/profiles.sql @@ -28,7 +28,7 @@ INSERT INTO entity_profiles ( $1, $2, sqlc.arg(contextual_rules)::jsonb, - FALSE + TRUE ) RETURNING *; -- name: UpsertProfileForEntity :one @@ -40,7 +40,7 @@ INSERT INTO entity_profiles ( ) VALUES ($1, $2, sqlc.arg(contextual_rules)::jsonb, false) ON CONFLICT (entity, profile_id) DO UPDATE SET contextual_rules = sqlc.arg(contextual_rules)::jsonb, - migrated = FALSE + migrated = TRUE RETURNING *; -- name: DeleteProfileForEntity :exec @@ -110,7 +110,7 @@ GROUP BY profiles.id; -- name: CountProfilesByEntityType :many SELECT COUNT(p.id) AS num_profiles, ep.entity AS profile_entity FROM profiles AS p - JOIN entity_profiles AS ep ON p.id = ep.profile_id +JOIN entity_profiles AS ep ON p.id = ep.profile_id GROUP BY ep.entity; -- name: CountProfilesByName :one diff --git a/database/query/rule_instances.sql b/database/query/rule_instances.sql new file mode 100644 index 0000000000..f2e7ea1135 --- /dev/null +++ b/database/query/rule_instances.sql @@ -0,0 +1,52 @@ +-- 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. + +-- name: UpsertRuleInstance :one +INSERT INTO rule_instances ( + profile_id, + rule_type_id, + name, + entity_type, + def, + params, + created_at, + updated_at +) VALUES( + $1, + $2, + $3, + $4, + $5, + $6, + NOW(), + NOW() +) +ON CONFLICT (profile_id, entity_type, name) DO UPDATE SET + rule_type_id = $2, + def = $5, + params = $6, + updated_at = NOW() +RETURNING id; + +-- name: GetRuleInstancesForProfile :many +SELECT * FROM rule_instances WHERE profile_id = $1; + +-- name: GetRuleInstancesForProfileEntity :many +SELECT * FROM rule_instances WHERE profile_id = $1 AND entity_type = $2; + +-- name: DeleteNonUpdatedRules :exec +DELETE FROM rule_instances +WHERE profile_id = $1 +AND entity_type = $2 +AND NOT id = ANY(sqlc.arg(updated_ids)::UUID[]); \ No newline at end of file diff --git a/internal/db/models.go b/internal/db/models.go index 3d425e72cd..57e08af417 100644 --- a/internal/db/models.go +++ b/internal/db/models.go @@ -639,15 +639,15 @@ type RuleEvaluation struct { } type RuleInstance struct { - ID uuid.UUID `json:"id"` - ProfileID uuid.UUID `json:"profile_id"` - RuleTypeID uuid.UUID `json:"rule_type_id"` - Name string `json:"name"` - EntityType Entities `json:"entity_type"` - Def pqtype.NullRawMessage `json:"def"` - Params pqtype.NullRawMessage `json:"params"` - CreatedAt time.Time `json:"created_at"` - UpdatedAt time.Time `json:"updated_at"` + ID uuid.UUID `json:"id"` + ProfileID uuid.UUID `json:"profile_id"` + RuleTypeID uuid.UUID `json:"rule_type_id"` + Name string `json:"name"` + EntityType Entities `json:"entity_type"` + Def json.RawMessage `json:"def"` + Params json.RawMessage `json:"params"` + CreatedAt time.Time `json:"created_at"` + UpdatedAt time.Time `json:"updated_at"` } type RuleType struct { diff --git a/internal/db/profiles.sql.go b/internal/db/profiles.sql.go index bdee5a8724..8a984351e4 100644 --- a/internal/db/profiles.sql.go +++ b/internal/db/profiles.sql.go @@ -17,7 +17,7 @@ import ( const countProfilesByEntityType = `-- name: CountProfilesByEntityType :many SELECT COUNT(p.id) AS num_profiles, ep.entity AS profile_entity FROM profiles AS p - JOIN entity_profiles AS ep ON p.id = ep.profile_id +JOIN entity_profiles AS ep ON p.id = ep.profile_id GROUP BY ep.entity ` @@ -120,7 +120,7 @@ INSERT INTO entity_profiles ( $1, $2, $3::jsonb, - FALSE + TRUE ) RETURNING id, entity, profile_id, contextual_rules, created_at, updated_at, migrated ` @@ -578,7 +578,7 @@ INSERT INTO entity_profiles ( ) VALUES ($1, $2, $3::jsonb, false) ON CONFLICT (entity, profile_id) DO UPDATE SET contextual_rules = $3::jsonb, - migrated = FALSE + migrated = TRUE RETURNING id, entity, profile_id, contextual_rules, created_at, updated_at, migrated ` diff --git a/internal/db/querier.go b/internal/db/querier.go index e95228a69e..5d26672009 100644 --- a/internal/db/querier.go +++ b/internal/db/querier.go @@ -32,6 +32,7 @@ type Querier interface { DeleteArtifact(ctx context.Context, id uuid.UUID) error DeleteExpiredSessionStates(ctx context.Context) (int64, error) DeleteInstallationIDByAppID(ctx context.Context, appInstallationID int64) error + DeleteNonUpdatedRules(ctx context.Context, arg DeleteNonUpdatedRulesParams) error DeleteProfile(ctx context.Context, arg DeleteProfileParams) error DeleteProfileForEntity(ctx context.Context, arg DeleteProfileForEntityParams) error DeleteProject(ctx context.Context, id uuid.UUID) ([]DeleteProjectRow, error) @@ -95,6 +96,8 @@ type Querier interface { GetRepositoryByIDAndProject(ctx context.Context, arg GetRepositoryByIDAndProjectParams) (Repository, error) GetRepositoryByRepoID(ctx context.Context, repoID int64) (Repository, error) GetRepositoryByRepoName(ctx context.Context, arg GetRepositoryByRepoNameParams) (Repository, error) + GetRuleInstancesForProfile(ctx context.Context, profileID uuid.UUID) ([]RuleInstance, error) + GetRuleInstancesForProfileEntity(ctx context.Context, arg GetRuleInstancesForProfileEntityParams) ([]RuleInstance, error) GetRuleTypeByID(ctx context.Context, id uuid.UUID) (RuleType, error) GetRuleTypeByName(ctx context.Context, arg GetRuleTypeByNameParams) (RuleType, error) GetSubscriptionByProjectBundle(ctx context.Context, arg GetSubscriptionByProjectBundleParams) (Subscription, error) @@ -182,6 +185,20 @@ type Querier interface { UpsertRuleDetailsEval(ctx context.Context, arg UpsertRuleDetailsEvalParams) (uuid.UUID, error) UpsertRuleDetailsRemediate(ctx context.Context, arg UpsertRuleDetailsRemediateParams) (uuid.UUID, error) UpsertRuleEvaluations(ctx context.Context, arg UpsertRuleEvaluationsParams) (uuid.UUID, error) + // 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. + UpsertRuleInstance(ctx context.Context, arg UpsertRuleInstanceParams) (uuid.UUID, error) UpsertRuleInstantiation(ctx context.Context, arg UpsertRuleInstantiationParams) (EntityProfileRule, error) } diff --git a/internal/db/rule_instances.sql.go b/internal/db/rule_instances.sql.go new file mode 100644 index 0000000000..2fc742221e --- /dev/null +++ b/internal/db/rule_instances.sql.go @@ -0,0 +1,176 @@ +// Code generated by sqlc. DO NOT EDIT. +// versions: +// sqlc v1.26.0 +// source: rule_instances.sql + +package db + +import ( + "context" + "encoding/json" + + "github.com/google/uuid" + "github.com/lib/pq" +) + +const deleteNonUpdatedRules = `-- name: DeleteNonUpdatedRules :exec +DELETE FROM rule_instances +WHERE profile_id = $1 +AND entity_type = $2 +AND NOT id = ANY($3::UUID[]) +` + +type DeleteNonUpdatedRulesParams struct { + ProfileID uuid.UUID `json:"profile_id"` + EntityType Entities `json:"entity_type"` + UpdatedIds []uuid.UUID `json:"updated_ids"` +} + +func (q *Queries) DeleteNonUpdatedRules(ctx context.Context, arg DeleteNonUpdatedRulesParams) error { + _, err := q.db.ExecContext(ctx, deleteNonUpdatedRules, arg.ProfileID, arg.EntityType, pq.Array(arg.UpdatedIds)) + return err +} + +const getRuleInstancesForProfile = `-- name: GetRuleInstancesForProfile :many +SELECT id, profile_id, rule_type_id, name, entity_type, def, params, created_at, updated_at FROM rule_instances WHERE profile_id = $1 +` + +func (q *Queries) GetRuleInstancesForProfile(ctx context.Context, profileID uuid.UUID) ([]RuleInstance, error) { + rows, err := q.db.QueryContext(ctx, getRuleInstancesForProfile, profileID) + if err != nil { + return nil, err + } + defer rows.Close() + items := []RuleInstance{} + for rows.Next() { + var i RuleInstance + if err := rows.Scan( + &i.ID, + &i.ProfileID, + &i.RuleTypeID, + &i.Name, + &i.EntityType, + &i.Def, + &i.Params, + &i.CreatedAt, + &i.UpdatedAt, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Close(); err != nil { + return nil, err + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + +const getRuleInstancesForProfileEntity = `-- name: GetRuleInstancesForProfileEntity :many +SELECT id, profile_id, rule_type_id, name, entity_type, def, params, created_at, updated_at FROM rule_instances WHERE profile_id = $1 AND entity_type = $2 +` + +type GetRuleInstancesForProfileEntityParams struct { + ProfileID uuid.UUID `json:"profile_id"` + EntityType Entities `json:"entity_type"` +} + +func (q *Queries) GetRuleInstancesForProfileEntity(ctx context.Context, arg GetRuleInstancesForProfileEntityParams) ([]RuleInstance, error) { + rows, err := q.db.QueryContext(ctx, getRuleInstancesForProfileEntity, arg.ProfileID, arg.EntityType) + if err != nil { + return nil, err + } + defer rows.Close() + items := []RuleInstance{} + for rows.Next() { + var i RuleInstance + if err := rows.Scan( + &i.ID, + &i.ProfileID, + &i.RuleTypeID, + &i.Name, + &i.EntityType, + &i.Def, + &i.Params, + &i.CreatedAt, + &i.UpdatedAt, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Close(); err != nil { + return nil, err + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + +const upsertRuleInstance = `-- name: UpsertRuleInstance :one + +INSERT INTO rule_instances ( + profile_id, + rule_type_id, + name, + entity_type, + def, + params, + created_at, + updated_at +) VALUES( + $1, + $2, + $3, + $4, + $5, + $6, + NOW(), + NOW() +) +ON CONFLICT (profile_id, entity_type, name) DO UPDATE SET + rule_type_id = $2, + def = $5, + params = $6, + updated_at = NOW() +RETURNING id +` + +type UpsertRuleInstanceParams struct { + ProfileID uuid.UUID `json:"profile_id"` + RuleTypeID uuid.UUID `json:"rule_type_id"` + Name string `json:"name"` + EntityType Entities `json:"entity_type"` + Def json.RawMessage `json:"def"` + Params json.RawMessage `json:"params"` +} + +// 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. +func (q *Queries) UpsertRuleInstance(ctx context.Context, arg UpsertRuleInstanceParams) (uuid.UUID, error) { + row := q.db.QueryRowContext(ctx, upsertRuleInstance, + arg.ProfileID, + arg.RuleTypeID, + arg.Name, + arg.EntityType, + arg.Def, + arg.Params, + ) + var id uuid.UUID + err := row.Scan(&id) + return id, err +} diff --git a/internal/profiles/service.go b/internal/profiles/service.go index 5d77bcac4c..7cb3fb9a40 100644 --- a/internal/profiles/service.go +++ b/internal/profiles/service.go @@ -260,9 +260,32 @@ func (p *profileService) UpdateProfile( minderv1.Entity_ENTITY_BUILD_ENVIRONMENTS: profile.GetBuildEnvironment(), minderv1.Entity_ENTITY_PULL_REQUESTS: profile.GetPullRequest(), } { - if err := updateProfileRulesForEntity(ctx, ent, &updatedProfile, qtx, entRules, rules); err != nil { + if err = updateProfileRulesForEntity(ctx, ent, &updatedProfile, qtx, entRules, rules); err != nil { return nil, err } + + updatedIDs, err := upsertRuleInstances( + ctx, + qtx, + updatedProfile.ID, + entRules, + entities.EntityTypeToDB(ent), + rules, + ) + if err != nil { + return nil, err + } + + // Any rule which was not updated was deleted from the profile. + // Remove from the database as well. + err = qtx.DeleteNonUpdatedRules(ctx, db.DeleteNonUpdatedRulesParams{ + ProfileID: updatedProfile.ID, + EntityType: entities.EntityTypeToDB(ent), + UpdatedIds: updatedIDs, + }) + if err != nil { + return nil, fmt.Errorf("error while cleaning up rule instances: %w", err) + } } unusedRuleStatuses := getUnusedOldRuleStatuses(rules, oldRules) @@ -360,6 +383,18 @@ func createProfileRulesForEntity( return nil } + _, err := upsertRuleInstances( + ctx, + qtx, + profile.ID, + rules, + entities.EntityTypeToDB(entity), + rulesInProf, + ) + if err != nil { + return fmt.Errorf("error while creating rule instances: %w", err) + } + marshalled, err := json.Marshal(rules) if err != nil { log.Printf("error marshalling %s rules: %v", entity, err) @@ -707,3 +742,50 @@ func deleteRuleStatusesForProfile( return nil } + +func upsertRuleInstances( + ctx context.Context, + qtx db.Querier, + profileID uuid.UUID, + newRules []*minderv1.Profile_Rule, + entityType db.Entities, + rulesInProf RuleMapping, +) ([]uuid.UUID, error) { + updatedIDs := make([]uuid.UUID, len(newRules)) + for i, rule := range newRules { + // TODO: Clean up this logic once we no longer have to support the old tables. + entityRuleTuple, ok := rulesInProf[RuleTypeAndNamePair{ + RuleType: rule.Type, + RuleName: rule.Name, + }] + if !ok { + return nil, fmt.Errorf("unable to find rule type ID for %s/%s", rule.Name, rule.Type) + } + + def, err := json.Marshal(rule.Def) + if err != nil { + return nil, fmt.Errorf("unable to serialize rule def: %w", err) + } + + params, err := json.Marshal(rule.Params) + if err != nil { + return nil, fmt.Errorf("unable to serialize rule params: %w", err) + } + + id, err := qtx.UpsertRuleInstance(ctx, db.UpsertRuleInstanceParams{ + ProfileID: profileID, + RuleTypeID: entityRuleTuple.RuleID, + Name: rule.Name, + EntityType: entityType, + Def: def, + Params: params, + }) + if err != nil { + return nil, fmt.Errorf("unable to insert new rule instance: %w", err) + } + + updatedIDs[i] = id + } + + return updatedIDs, nil +}