Skip to content

Commit

Permalink
use dedicated DB query to fetch all repo rules (#702)
Browse files Browse the repository at this point in the history
  • Loading branch information
marko-gacesa authored and Harness committed Oct 20, 2023
1 parent 363ad84 commit fcc8c0b
Show file tree
Hide file tree
Showing 6 changed files with 136 additions and 54 deletions.
2 changes: 1 addition & 1 deletion app/api/controller/pullreq/merge.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ func (c *Controller) Merge(
return types.MergeResponse{}, fmt.Errorf("failed to list status checks: %w", err)
}

protectionRules, err := c.protectionManager.Repo(ctx, targetRepo.ID, enum.RuleStateActive, enum.RuleStateMonitor)
protectionRules, err := c.protectionManager.ForRepository(ctx, targetRepo.ID)
if err != nil {
return types.MergeResponse{}, fmt.Errorf("failed to fetch protection rules for the repository: %w", err)
}
Expand Down
49 changes: 4 additions & 45 deletions app/services/protection/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (

"github.com/harness/gitness/app/store"
"github.com/harness/gitness/types"
"github.com/harness/gitness/types/enum"
)

// NewManager creates new protection Manager.
Expand Down Expand Up @@ -88,54 +87,14 @@ func (m *Manager) SanitizeJSON(ruleType types.RuleType, message json.RawMessage)
return toJSON(r)
}

func (m *Manager) Space(ctx context.Context, spaceID int64, ruleStates ...enum.RuleState) (Protection, error) {
// TODO: Include the rules of the parent spaces if any.
// TODO: Use some other function to fetch the rules, that returns just basic info about the rule

rules, err := m.ruleStore.List(ctx, &spaceID, nil, &types.RuleFilter{
ListQueryFilter: types.ListQueryFilter{
Pagination: types.Pagination{
Page: 1,
Size: 1000,
},
Query: "",
},
States: ruleStates,
Sort: enum.RuleSortCreated,
Order: enum.OrderAsc,
})
func (m *Manager) ForRepository(ctx context.Context, repoID int64) (Protection, error) {
ruleInfos, err := m.ruleStore.ListAllRepoRules(ctx, repoID)
if err != nil {
return ruleSet{}, fmt.Errorf("failed to list rules for space: %w", err)
return nil, fmt.Errorf("failed to list rules for repository: %w", err)
}

return ruleSet{
rules: rules,
manager: m,
}, nil
}

func (m *Manager) Repo(ctx context.Context, repoID int64, ruleStates ...enum.RuleState) (Protection, error) {
// TODO: Include rules of the space and its parent spaces.
// TODO: Use some other function to fetch the rules, that returns just basic info about the rule

rules, err := m.ruleStore.List(ctx, nil, &repoID, &types.RuleFilter{
ListQueryFilter: types.ListQueryFilter{
Pagination: types.Pagination{
Page: 1,
Size: 1000,
},
Query: "",
},
States: ruleStates,
Sort: enum.RuleSortCreated,
Order: enum.OrderAsc,
})
if err != nil {
return ruleSet{}, fmt.Errorf("failed to list rules for repository: %w", err)
}

return ruleSet{
rules: rules,
rules: ruleInfos,
manager: m,
}, nil
}
10 changes: 4 additions & 6 deletions app/services/protection/set.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import (
)

type ruleSet struct {
rules []types.Rule
rules []types.RuleInfoInternal
manager *Manager
}

Expand All @@ -33,9 +33,7 @@ func (s ruleSet) CanMerge(ctx context.Context, in CanMergeInput) (CanMergeOutput
var out CanMergeOutput
var violations []types.RuleViolations

for i := range s.rules {
r := s.rules[i]

for _, r := range s.rules {
bp := Pattern{}

if err := json.Unmarshal(r.Pattern, &bp); err != nil {
Expand All @@ -57,14 +55,14 @@ func (s ruleSet) CanMerge(ctx context.Context, in CanMergeInput) (CanMergeOutput
return out, nil, err
}

violations = append(violations, backFillRule(rVs, &r)...)
violations = append(violations, backFillRule(rVs, r.RuleInfo)...)
out.DeleteSourceBranch = out.DeleteSourceBranch || rOut.DeleteSourceBranch
}

return out, violations, nil
}

func backFillRule(vs []types.RuleViolations, rule *types.Rule) []types.RuleViolations {
func backFillRule(vs []types.RuleViolations, rule types.RuleInfo) []types.RuleViolations {
violations := make([]types.RuleViolations, 0, len(vs))

for i := range vs {
Expand Down
3 changes: 3 additions & 0 deletions app/store/database.go
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,9 @@ type (

// List returns a list of protection rules of a repository or a space that matches the provided criteria.
List(ctx context.Context, spaceID, repoID *int64, filter *types.RuleFilter) ([]types.Rule, error)

// ListAllRepoRules returns a list of all protection rules that can be applied on a repository.
ListAllRepoRules(ctx context.Context, repoID int64) ([]types.RuleInfoInternal, error)
}

// WebhookStore defines the webhook data storage.
Expand Down
105 changes: 105 additions & 0 deletions app/store/database/rule.go
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,86 @@ func (s *RuleStore) List(ctx context.Context, spaceID, repoID *int64, filter *ty
return s.mapToRules(ctx, dst), nil
}

type ruleInfo struct {
SpacePath string `db:"space_path"`
RepoPath string `db:"repo_path"`
ID int64 `db:"rule_id"`
UID string `db:"rule_uid"`
Type types.RuleType `db:"rule_type"`
State enum.RuleState `db:"rule_state"`
Pattern string `db:"rule_pattern"`
Definition string `db:"rule_definition"`
}

// ListAllRepoRules returns a list of all protection rules that can be applied on a repository.
// This includes the rules defined directly on the repository and all those defined on the parent spaces.
func (s *RuleStore) ListAllRepoRules(ctx context.Context, repoID int64) ([]types.RuleInfoInternal, error) {
const query = `
WITH RECURSIVE
repo_info(repo_id, repo_uid, repo_space_id) AS (
SELECT repo_id, repo_uid, repo_parent_id
FROM repositories
WHERE repo_id = $1
),
space_parents(space_id, space_uid, space_parent_id) AS (
SELECT space_id, space_uid, space_parent_id
FROM spaces
INNER JOIN repo_info ON repo_info.repo_space_id = spaces.space_id
UNION ALL
SELECT spaces.space_id, spaces.space_uid, spaces.space_parent_id
FROM spaces
INNER JOIN space_parents ON space_parents.space_parent_id = spaces.space_id
),
spaces_with_path(space_id, space_parent_id, space_uid, space_full_path) AS (
SELECT space_id, space_parent_id, space_uid, space_uid
FROM space_parents
WHERE space_parent_id IS NULL
UNION ALL
SELECT
space_parents.space_id,
space_parents.space_parent_id,
space_parents.space_uid,
spaces_with_path.space_full_path || '/' || space_parents.space_uid
FROM space_parents
INNER JOIN spaces_with_path ON spaces_with_path.space_id = space_parents.space_parent_id
)
SELECT
space_full_path AS "space_path"
,'' as "repo_path"
,rule_id
,rule_uid
,rule_type
,rule_state
,rule_pattern
,rule_definition
FROM spaces_with_path
INNER JOIN rules ON rules.rule_space_id = spaces_with_path.space_id
WHERE rule_state IN ('active', 'monitor')
UNION ALL
SELECT
'' as "space_path"
,space_full_path || '/' || repo_info.repo_uid AS "repo_path"
,rule_id
,rule_uid
,rule_type
,rule_state
,rule_pattern
,rule_definition
FROM rules
INNER JOIN repo_info ON repo_info.repo_id = rules.rule_repo_id
INNER JOIN spaces_with_path ON spaces_with_path.space_id = repo_info.repo_space_id
WHERE rule_state IN ('active', 'monitor')`

db := dbtx.GetAccessor(ctx, s.db)

result := make([]ruleInfo, 0)
if err := db.SelectContext(ctx, &result, query, repoID); err != nil {
return nil, database.ProcessSQLErrorf(err, "Failed executing custom list query")
}

return s.mapToRuleInfos(result), nil
}

func (*RuleStore) applyParentID(
stmt squirrel.SelectBuilder,
spaceID, repoID *int64,
Expand Down Expand Up @@ -433,3 +513,28 @@ func mapToInternalRule(in *types.Rule) rule {
Definition: string(in.Definition),
}
}

func (*RuleStore) mapToRuleInfo(in *ruleInfo) types.RuleInfoInternal {
return types.RuleInfoInternal{
RuleInfo: types.RuleInfo{
SpacePath: in.SpacePath,
RepoPath: in.RepoPath,
ID: in.ID,
UID: in.UID,
Type: in.Type,
State: in.State,
},
Pattern: json.RawMessage(in.Pattern),
Definition: json.RawMessage(in.Definition),
}
}

func (s *RuleStore) mapToRuleInfos(
ruleInfos []ruleInfo,
) []types.RuleInfoInternal {
res := make([]types.RuleInfoInternal, len(ruleInfos))
for i := 0; i < len(ruleInfos); i++ {
res[i] = s.mapToRuleInfo(&ruleInfos[i])
}
return res
}
21 changes: 19 additions & 2 deletions types/rule.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ type Violation struct {

// RuleViolations holds several violations of a rule.
type RuleViolations struct {
Rule *Rule `json:"rule"`
Rule RuleInfo `json:"rule"`
Violations []Violation `json:"violations"`
}

Expand All @@ -80,5 +80,22 @@ func (violations *RuleViolations) Addf(code, format string, params ...any) {
}

func (violations *RuleViolations) IsCritical() bool {
return violations.Rule == nil || violations.Rule.State == enum.RuleStateActive
return violations.Rule.State == enum.RuleStateActive
}

// RuleInfo holds basic info about a rule that is used to describe the rule in RuleViolations.
type RuleInfo struct {
SpacePath string `json:"space_path,omitempty"`
RepoPath string `json:"repo_path,omitempty"`

ID int64 `json:"-"`
UID string `json:"uid"`
Type RuleType `json:"type"`
State enum.RuleState `json:"state"`
}

type RuleInfoInternal struct {
RuleInfo
Pattern json.RawMessage
Definition json.RawMessage
}

0 comments on commit fcc8c0b

Please sign in to comment.