Skip to content

Commit dd94e8f

Browse files
tae2089tae2089
committed
feat: add per-repo branch filtering and graceful shutdown context propagation
- RepoFilter: per-repo branch rules via --allow-repo "org/api:main,develop" ParseRepoRule, NewRepoFilterFromRules, IsAllowedRef with glob patterns Default branches [main, master] when unspecified Handler simplified from 2-step to single IsAllowedRef call - Graceful shutdown: NewSyncQueueWithContext propagates parent context to clone/build handlers, enabling cancel on SIGINT/SIGTERM Shutdown order: HTTP stop → syncCancel → worker drain
1 parent 37fa4de commit dd94e8f

8 files changed

Lines changed: 384 additions & 104 deletions

File tree

‎cmd/ccg/main.go‎

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -244,13 +244,20 @@ func serveStreamableHTTP(deps *cli.Deps, srv *server.MCPServer, cfg cli.ServeCon
244244
mux.HandleFunc("/health", handleHealth)
245245

246246
var syncQueue *webhook.SyncQueue
247+
syncCtx, syncCancel := context.WithCancel(context.Background())
248+
defer syncCancel()
249+
247250
if len(cfg.AllowRepo) > 0 {
248-
allowlist := webhook.NewRepoAllowlist(cfg.AllowRepo)
251+
rules := make([]webhook.RepoRule, 0, len(cfg.AllowRepo))
252+
for _, s := range cfg.AllowRepo {
253+
rules = append(rules, webhook.ParseRepoRule(s))
254+
}
255+
filter := webhook.NewRepoFilterFromRules(rules)
249256
secret := []byte(cfg.WebhookSecret)
250-
syncHandler := func(repoFullName, cloneURL string) {
257+
syncHandler := func(ctx context.Context, repoFullName, cloneURL string) {
251258
ns := webhook.ExtractNamespace(repoFullName)
252259
deps.Logger.Info("webhook sync started", "repo", repoFullName, "namespace", ns)
253-
cloneCtx, cloneCancel := context.WithTimeout(context.Background(), 10*time.Minute)
260+
cloneCtx, cloneCancel := context.WithTimeout(ctx, 10*time.Minute)
254261
defer cloneCancel()
255262
if err := webhook.CloneOrPull(cloneCtx, cloneURL, cfg.RepoRoot, ns, nil); err != nil {
256263
deps.Logger.Error("webhook clone/pull failed", "repo", repoFullName, "error", err)
@@ -265,7 +272,7 @@ func serveStreamableHTTP(deps *cli.Deps, srv *server.MCPServer, cfg cli.ServeCon
265272
Walkers: deps.Walkers,
266273
Logger: deps.Logger,
267274
}
268-
buildCtx, buildCancel := context.WithTimeout(context.Background(), 10*time.Minute)
275+
buildCtx, buildCancel := context.WithTimeout(ctx, 10*time.Minute)
269276
defer buildCancel()
270277
stats, err := graphSvc.Build(buildCtx, service.BuildOptions{Dir: repoDir})
271278
if err != nil {
@@ -275,8 +282,8 @@ func serveStreamableHTTP(deps *cli.Deps, srv *server.MCPServer, cfg cli.ServeCon
275282
deps.Logger.Info("webhook sync completed", "repo", repoFullName, "namespace", ns,
276283
"files", stats.TotalFiles, "nodes", stats.TotalNodes, "edges", stats.TotalEdges)
277284
}
278-
syncQueue = webhook.NewSyncQueue(4, syncHandler)
279-
mux.Handle("/webhook", webhook.NewWebhookHandler(secret, allowlist, syncQueue.Add))
285+
syncQueue = webhook.NewSyncQueueWithContext(syncCtx, 4, syncHandler)
286+
mux.Handle("/webhook", webhook.NewWebhookHandler(secret, filter, syncQueue.Add))
280287
deps.Logger.Info("webhook endpoint registered", "path", "/webhook", "allowedRepos", cfg.AllowRepo)
281288
}
282289

@@ -310,7 +317,8 @@ func serveStreamableHTTP(deps *cli.Deps, srv *server.MCPServer, cfg cli.ServeCon
310317
return trace.Wrap(err, "HTTP server shutdown")
311318
}
312319
if syncQueue != nil {
313-
deps.Logger.Info("draining sync queue workers")
320+
deps.Logger.Info("cancelling sync context and draining workers")
321+
syncCancel()
314322
syncQueue.Shutdown()
315323
}
316324
return nil

‎internal/webhook/allowlist.go‎

Lines changed: 82 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
package webhook
22

3-
import "strings"
3+
import (
4+
"path"
5+
"strings"
6+
)
47

58
type allowRule struct {
69
pattern string
@@ -9,33 +12,53 @@ type allowRule struct {
912
prefix string
1013
}
1114

12-
type RepoAllowlist struct {
13-
rules []allowRule
15+
type RepoRule struct {
16+
Pattern string
17+
Branches []string
1418
}
1519

16-
func NewRepoAllowlist(patterns []string) *RepoAllowlist {
17-
rules := make([]allowRule, 0, len(patterns))
20+
type repoFilterRule struct {
21+
allowRule
22+
branches []string
23+
}
24+
25+
type RepoFilter struct {
26+
rulesFull []repoFilterRule
27+
}
28+
29+
func NewRepoFilter(patterns []string) *RepoFilter {
30+
rules := make([]RepoRule, 0, len(patterns))
1831
for _, p := range patterns {
19-
r := allowRule{}
20-
if strings.HasPrefix(p, "!") {
21-
r.negate = true
22-
p = p[1:]
32+
rules = append(rules, RepoRule{Pattern: p})
33+
}
34+
return NewRepoFilterFromRules(rules)
35+
}
36+
37+
func NewRepoFilterFromRules(rules []RepoRule) *RepoFilter {
38+
full := make([]repoFilterRule, 0, len(rules))
39+
for _, r := range rules {
40+
pat := r.Pattern
41+
ar := allowRule{}
42+
if strings.HasPrefix(pat, "!") {
43+
ar.negate = true
44+
pat = pat[1:]
2345
}
24-
if strings.HasSuffix(p, "/*") {
25-
r.wildcard = true
26-
r.prefix = p[:len(p)-2]
46+
if strings.HasSuffix(pat, "/*") {
47+
ar.wildcard = true
48+
ar.prefix = pat[:len(pat)-2]
2749
}
28-
r.pattern = p
29-
rules = append(rules, r)
50+
ar.pattern = pat
51+
full = append(full, repoFilterRule{allowRule: ar, branches: r.Branches})
3052
}
31-
return &RepoAllowlist{rules: rules}
53+
return &RepoFilter{rulesFull: full}
3254
}
3355

34-
func (a *RepoAllowlist) IsAllowed(repoFullName string) bool {
56+
var defaultBranches = []string{"main", "master"}
57+
58+
func (f *RepoFilter) IsAllowed(repoFullName string) bool {
3559
allowed := false
36-
for _, r := range a.rules {
37-
matched := r.match(repoFullName)
38-
if !matched {
60+
for _, r := range f.rulesFull {
61+
if !r.match(repoFullName) {
3962
continue
4063
}
4164
if r.negate {
@@ -47,13 +70,52 @@ func (a *RepoAllowlist) IsAllowed(repoFullName string) bool {
4770
return allowed
4871
}
4972

73+
func (f *RepoFilter) IsAllowedRef(repoFullName, ref string) bool {
74+
if len(f.rulesFull) == 0 {
75+
return false
76+
}
77+
78+
for _, r := range f.rulesFull {
79+
if !r.match(repoFullName) {
80+
continue
81+
}
82+
if r.negate {
83+
return false
84+
}
85+
branches := r.branches
86+
if len(branches) == 0 {
87+
branches = defaultBranches
88+
}
89+
return matchBranchPatterns(ref, branches)
90+
}
91+
return false
92+
}
93+
94+
func matchBranchPatterns(ref string, patterns []string) bool {
95+
branch := strings.TrimPrefix(ref, "refs/heads/")
96+
for _, p := range patterns {
97+
if matched, _ := path.Match(p, branch); matched {
98+
return true
99+
}
100+
}
101+
return false
102+
}
103+
104+
func ParseRepoRule(s string) RepoRule {
105+
pattern, branchStr, found := strings.Cut(s, ":")
106+
if !found {
107+
return RepoRule{Pattern: s}
108+
}
109+
branches := strings.Split(branchStr, ",")
110+
return RepoRule{Pattern: pattern, Branches: branches}
111+
}
112+
50113
func (r *allowRule) match(repoFullName string) bool {
51114
if r.wildcard {
52115
parts := strings.SplitN(repoFullName, "/", 2)
53116
if len(parts) != 2 {
54117
return false
55118
}
56-
// "*" prefix matches any org (e.g. "*/*" allows all repos)
57119
return r.prefix == "*" || parts[0] == r.prefix
58120
}
59121
return repoFullName == r.pattern

‎internal/webhook/allowlist_test.go‎

Lines changed: 141 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ package webhook
33
import "testing"
44

55
func TestRepoAllowlist_ExactMatch(t *testing.T) {
6-
al := NewRepoAllowlist([]string{"org/svc"})
6+
al := NewRepoFilter([]string{"org/svc"})
77

88
if !al.IsAllowed("org/svc") {
99
t.Error("expected org/svc to be allowed")
@@ -14,7 +14,7 @@ func TestRepoAllowlist_ExactMatch(t *testing.T) {
1414
}
1515

1616
func TestRepoAllowlist_WildcardOrg(t *testing.T) {
17-
al := NewRepoAllowlist([]string{"org/*"})
17+
al := NewRepoFilter([]string{"org/*"})
1818

1919
if !al.IsAllowed("org/svc") {
2020
t.Error("expected org/svc to be allowed")
@@ -28,7 +28,7 @@ func TestRepoAllowlist_WildcardOrg(t *testing.T) {
2828
}
2929

3030
func TestRepoAllowlist_Negation(t *testing.T) {
31-
al := NewRepoAllowlist([]string{"org/*", "!org/private"})
31+
al := NewRepoFilter([]string{"org/*", "!org/private"})
3232

3333
if !al.IsAllowed("org/svc") {
3434
t.Error("expected org/svc to be allowed")
@@ -39,7 +39,7 @@ func TestRepoAllowlist_Negation(t *testing.T) {
3939
}
4040

4141
func TestRepoAllowlist_EmptyAllowsNothing(t *testing.T) {
42-
al := NewRepoAllowlist([]string{})
42+
al := NewRepoFilter([]string{})
4343

4444
if al.IsAllowed("org/svc") {
4545
t.Error("expected org/svc to be denied when allowlist is empty")
@@ -50,7 +50,7 @@ func TestRepoAllowlist_EmptyAllowsNothing(t *testing.T) {
5050
}
5151

5252
func TestRepoAllowlist_MultipleRules(t *testing.T) {
53-
al := NewRepoAllowlist([]string{
53+
al := NewRepoFilter([]string{
5454
"acme/*",
5555
"!acme/internal",
5656
"external/shared",
@@ -80,7 +80,7 @@ func TestRepoAllowlist_MultipleRules(t *testing.T) {
8080
}
8181

8282
func TestRepoAllowlist_GlobalWildcard(t *testing.T) {
83-
al := NewRepoAllowlist([]string{"*/*"})
83+
al := NewRepoFilter([]string{"*/*"})
8484

8585
if !al.IsAllowed("any-org/any-repo") {
8686
t.Error("expected any-org/any-repo to be allowed with */*")
@@ -94,7 +94,7 @@ func TestRepoAllowlist_GlobalWildcard(t *testing.T) {
9494
}
9595

9696
func TestRepoAllowlist_GlobalWildcardWithNegation(t *testing.T) {
97-
al := NewRepoAllowlist([]string{"*/*", "!secret/private"})
97+
al := NewRepoFilter([]string{"*/*", "!secret/private"})
9898

9999
if !al.IsAllowed("org/repo") {
100100
t.Error("expected org/repo to be allowed")
@@ -103,3 +103,137 @@ func TestRepoAllowlist_GlobalWildcardWithNegation(t *testing.T) {
103103
t.Error("expected secret/private to be denied by negation")
104104
}
105105
}
106+
107+
func TestRepoFilter_PerRepoBranch_ExactRepo(t *testing.T) {
108+
f := NewRepoFilterFromRules([]RepoRule{
109+
{Pattern: "org/api", Branches: []string{"main", "develop"}},
110+
})
111+
112+
if !f.IsAllowedRef("org/api", "refs/heads/main") {
113+
t.Error("expected org/api + refs/heads/main to be allowed")
114+
}
115+
if !f.IsAllowedRef("org/api", "refs/heads/develop") {
116+
t.Error("expected org/api + refs/heads/develop to be allowed")
117+
}
118+
if f.IsAllowedRef("org/api", "refs/heads/feature/x") {
119+
t.Error("expected org/api + refs/heads/feature/x to be denied")
120+
}
121+
if f.IsAllowedRef("org/other", "refs/heads/main") {
122+
t.Error("expected org/other to be denied (not in rules)")
123+
}
124+
}
125+
126+
func TestRepoFilter_PerRepoBranch_WildcardRepo(t *testing.T) {
127+
f := NewRepoFilterFromRules([]RepoRule{
128+
{Pattern: "org/*", Branches: []string{"main"}},
129+
})
130+
131+
if !f.IsAllowedRef("org/svc", "refs/heads/main") {
132+
t.Error("expected org/svc + main to be allowed")
133+
}
134+
if f.IsAllowedRef("org/svc", "refs/heads/develop") {
135+
t.Error("expected org/svc + develop to be denied")
136+
}
137+
if f.IsAllowedRef("other/svc", "refs/heads/main") {
138+
t.Error("expected other/svc to be denied")
139+
}
140+
}
141+
142+
func TestRepoFilter_PerRepoBranch_DefaultBranches(t *testing.T) {
143+
f := NewRepoFilterFromRules([]RepoRule{
144+
{Pattern: "org/*"},
145+
})
146+
147+
if !f.IsAllowedRef("org/svc", "refs/heads/main") {
148+
t.Error("expected default branch main to be allowed")
149+
}
150+
if !f.IsAllowedRef("org/svc", "refs/heads/master") {
151+
t.Error("expected default branch master to be allowed")
152+
}
153+
if f.IsAllowedRef("org/svc", "refs/heads/develop") {
154+
t.Error("expected develop to be denied when no branches configured")
155+
}
156+
}
157+
158+
func TestRepoFilter_PerRepoBranch_GlobPattern(t *testing.T) {
159+
f := NewRepoFilterFromRules([]RepoRule{
160+
{Pattern: "org/web", Branches: []string{"release/*"}},
161+
})
162+
163+
if !f.IsAllowedRef("org/web", "refs/heads/release/v1.0") {
164+
t.Error("expected release/v1.0 to match release/*")
165+
}
166+
if !f.IsAllowedRef("org/web", "refs/heads/release/hotfix") {
167+
t.Error("expected release/hotfix to match release/*")
168+
}
169+
if f.IsAllowedRef("org/web", "refs/heads/main") {
170+
t.Error("expected main to be denied when only release/* configured")
171+
}
172+
}
173+
174+
func TestRepoFilter_PerRepoBranch_FirstMatchWins(t *testing.T) {
175+
f := NewRepoFilterFromRules([]RepoRule{
176+
{Pattern: "org/api", Branches: []string{"develop"}},
177+
{Pattern: "org/*", Branches: []string{"main"}},
178+
})
179+
180+
if !f.IsAllowedRef("org/api", "refs/heads/develop") {
181+
t.Error("expected org/api to use first match: develop")
182+
}
183+
if f.IsAllowedRef("org/api", "refs/heads/main") {
184+
t.Error("expected org/api NOT to fall through to org/* rule")
185+
}
186+
if !f.IsAllowedRef("org/web", "refs/heads/main") {
187+
t.Error("expected org/web to match org/* rule with main")
188+
}
189+
}
190+
191+
func TestRepoFilter_PerRepoBranch_Negation(t *testing.T) {
192+
f := NewRepoFilterFromRules([]RepoRule{
193+
{Pattern: "!org/private"},
194+
{Pattern: "org/*", Branches: []string{"main"}},
195+
})
196+
197+
if f.IsAllowedRef("org/private", "refs/heads/main") {
198+
t.Error("expected org/private to be denied by negation")
199+
}
200+
if !f.IsAllowedRef("org/svc", "refs/heads/main") {
201+
t.Error("expected org/svc + main to be allowed")
202+
}
203+
}
204+
205+
func TestRepoFilter_PerRepoBranch_EmptyRules(t *testing.T) {
206+
f := NewRepoFilterFromRules([]RepoRule{})
207+
208+
if f.IsAllowedRef("org/svc", "refs/heads/main") {
209+
t.Error("expected empty rules to deny everything")
210+
}
211+
}
212+
213+
func TestParseRepoRules(t *testing.T) {
214+
tests := []struct {
215+
input string
216+
want RepoRule
217+
}{
218+
{"org/api:main,develop", RepoRule{Pattern: "org/api", Branches: []string{"main", "develop"}}},
219+
{"org/*", RepoRule{Pattern: "org/*", Branches: nil}},
220+
{"!org/private", RepoRule{Pattern: "!org/private", Branches: nil}},
221+
{"org/web:release/*", RepoRule{Pattern: "org/web", Branches: []string{"release/*"}}},
222+
}
223+
224+
for _, tt := range tests {
225+
got := ParseRepoRule(tt.input)
226+
if got.Pattern != tt.want.Pattern {
227+
t.Errorf("ParseRepoRule(%q).Pattern = %q, want %q", tt.input, got.Pattern, tt.want.Pattern)
228+
}
229+
if len(got.Branches) != len(tt.want.Branches) {
230+
t.Errorf("ParseRepoRule(%q).Branches = %v, want %v", tt.input, got.Branches, tt.want.Branches)
231+
continue
232+
}
233+
for i, b := range got.Branches {
234+
if b != tt.want.Branches[i] {
235+
t.Errorf("ParseRepoRule(%q).Branches[%d] = %q, want %q", tt.input, i, b, tt.want.Branches[i])
236+
}
237+
}
238+
}
239+
}

0 commit comments

Comments
 (0)