Skip to content

Commit f517a10

Browse files
committed
feat(policies): parallelize policy evaluations with bounded concurrency
Use errgroup to evaluate policy attachments concurrently within VerifyStatement and VerifyMaterial, bounded by max(NumCPU*2, 10). This reduces evaluation time from O(n*t) to O(t) where n is the number of policies and t is the slowest single evaluation. Concurrency safety fixes: - Move remotePolicyCache/remoteGroupCache mutexes from instance-level to package-level to prevent data races under concurrent access - Use singleflight to coalesce concurrent gRPC fetches for the same policy/group ref, ensuring exactly one fetch per key - Move extism.SetLogLevel() to sync.Once to guarantee it is called exactly once across all concurrent WASM engine instances Adds race detector tests covering: - Concurrent VerifyStatement from 10 goroutines - Concurrent VerifyMaterial from 10 goroutines - WithMaxConcurrency option validation - Sequential (maxConcurrency=1) vs parallel result equivalence - errgroup cancellation on policy load failure Fixes #2899 Signed-off-by: Vibhav Bobade <vibhav.bobde@gmail.com>
1 parent df7e3ff commit f517a10

5 files changed

Lines changed: 336 additions & 61 deletions

File tree

pkg/policies/concurrency_test.go

Lines changed: 236 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,236 @@
1+
//
2+
// Copyright 2024-2026 The Chainloop Authors.
3+
//
4+
// Licensed under the Apache License, Version 2.0 (the "License");
5+
// you may not use this file except in compliance with the License.
6+
// You may obtain a copy of the License at
7+
//
8+
// http://www.apache.org/licenses/LICENSE-2.0
9+
//
10+
// Unless required by applicable law or agreed to in writing, software
11+
// distributed under the License is distributed on an "AS IS" BASIS,
12+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
// See the License for the specific language governing permissions and
14+
// limitations under the License.
15+
16+
package policies
17+
18+
import (
19+
"context"
20+
"os"
21+
"sync"
22+
"testing"
23+
24+
v12 "github.com/chainloop-dev/chainloop/app/controlplane/api/workflowcontract/v1"
25+
v1 "github.com/chainloop-dev/chainloop/pkg/attestation/crafter/api/attestation/v1"
26+
intoto "github.com/in-toto/attestation/go/v1"
27+
"github.com/rs/zerolog"
28+
"github.com/stretchr/testify/assert"
29+
"github.com/stretchr/testify/require"
30+
"google.golang.org/protobuf/encoding/protojson"
31+
)
32+
33+
// TestConcurrentVerifyStatement runs VerifyStatement from multiple goroutines
34+
// to exercise errgroup parallelism and catch race conditions.
35+
// Run with: go test -race -count=1 -run TestConcurrentVerifyStatement ./pkg/policies/...
36+
func TestConcurrentVerifyStatement(t *testing.T) {
37+
logger := zerolog.Nop()
38+
39+
schema := &v12.CraftingSchema{
40+
Policies: &v12.Policies{
41+
Attestation: []*v12.PolicyAttachment{
42+
{Policy: &v12.PolicyAttachment_Ref{Ref: "file://testdata/workflow.yaml"}},
43+
{Policy: &v12.PolicyAttachment_Ref{Ref: "file://testdata/materials.yaml"}},
44+
},
45+
},
46+
}
47+
48+
statement := loadStatementForTest(t, "testdata/statement.json")
49+
50+
pv := NewPolicyVerifier(schema.GetPolicies(), nil, &logger)
51+
52+
// Run multiple VerifyStatement calls concurrently
53+
const goroutines = 10
54+
var wg sync.WaitGroup
55+
errs := make([]error, goroutines)
56+
results := make([][]*v1.PolicyEvaluation, goroutines)
57+
58+
for i := range goroutines {
59+
wg.Add(1)
60+
go func() {
61+
defer wg.Done()
62+
res, err := pv.VerifyStatement(context.Background(), statement)
63+
errs[i] = err
64+
results[i] = res
65+
}()
66+
}
67+
68+
wg.Wait()
69+
70+
// All calls should succeed
71+
for i := range goroutines {
72+
assert.NoError(t, errs[i], "goroutine %d failed", i)
73+
}
74+
75+
// All calls should return the same number of evaluations
76+
for i := 1; i < goroutines; i++ {
77+
assert.Equal(t, len(results[0]), len(results[i]),
78+
"goroutine %d returned different number of evaluations", i)
79+
}
80+
}
81+
82+
// TestConcurrentVerifyMaterial runs VerifyMaterial from multiple goroutines.
83+
func TestConcurrentVerifyMaterial(t *testing.T) {
84+
logger := zerolog.Nop()
85+
86+
schema := &v12.CraftingSchema{
87+
Policies: &v12.Policies{
88+
Materials: []*v12.PolicyAttachment{
89+
{
90+
Policy: &v12.PolicyAttachment_Ref{Ref: "file://testdata/materials.yaml"},
91+
Selector: &v12.PolicyAttachment_MaterialSelector{Name: "sbom"},
92+
},
93+
},
94+
},
95+
}
96+
97+
material := &v1.Attestation_Material{
98+
MaterialType: v12.CraftingSchema_Material_SBOM_CYCLONEDX_JSON,
99+
M: &v1.Attestation_Material_String_{
100+
String_: &v1.Attestation_Material_KeyVal{
101+
Id: "sbom",
102+
Value: "testdata/sbom.cyclonedx.json",
103+
},
104+
},
105+
}
106+
107+
pv := NewPolicyVerifier(schema.GetPolicies(), nil, &logger)
108+
109+
const goroutines = 10
110+
var wg sync.WaitGroup
111+
errs := make([]error, goroutines)
112+
results := make([][]*v1.PolicyEvaluation, goroutines)
113+
114+
for i := range goroutines {
115+
wg.Add(1)
116+
go func() {
117+
defer wg.Done()
118+
res, err := pv.VerifyMaterial(context.Background(), material, "")
119+
errs[i] = err
120+
results[i] = res
121+
}()
122+
}
123+
124+
wg.Wait()
125+
126+
for i := range goroutines {
127+
assert.NoError(t, errs[i], "goroutine %d failed", i)
128+
}
129+
130+
for i := 1; i < goroutines; i++ {
131+
assert.Equal(t, len(results[0]), len(results[i]),
132+
"goroutine %d returned different number of evaluations", i)
133+
}
134+
}
135+
136+
// TestWithMaxConcurrency verifies the WithMaxConcurrency option is applied.
137+
func TestWithMaxConcurrency(t *testing.T) {
138+
logger := zerolog.Nop()
139+
140+
// Test default
141+
pv := NewPolicyVerifier(nil, nil, &logger)
142+
assert.Greater(t, pv.maxConcurrency, 0, "default maxConcurrency should be positive")
143+
144+
// Test custom value
145+
pv = NewPolicyVerifier(nil, nil, &logger, WithMaxConcurrency(5))
146+
assert.Equal(t, 5, pv.maxConcurrency)
147+
148+
// Test zero defaults to computed default
149+
pv = NewPolicyVerifier(nil, nil, &logger, WithMaxConcurrency(0))
150+
assert.Equal(t, defaultMaxConcurrency, pv.maxConcurrency)
151+
152+
// Test negative defaults to computed default
153+
pv = NewPolicyVerifier(nil, nil, &logger, WithMaxConcurrency(-1))
154+
assert.Equal(t, defaultMaxConcurrency, pv.maxConcurrency)
155+
}
156+
157+
// TestVerifyStatementWithConcurrencyOne ensures sequential mode (maxConcurrency=1)
158+
// produces identical results to default parallel mode.
159+
func TestVerifyStatementWithConcurrencyOne(t *testing.T) {
160+
logger := zerolog.Nop()
161+
162+
schema := &v12.CraftingSchema{
163+
Policies: &v12.Policies{
164+
Attestation: []*v12.PolicyAttachment{
165+
{Policy: &v12.PolicyAttachment_Ref{Ref: "file://testdata/workflow.yaml"}},
166+
{Policy: &v12.PolicyAttachment_Ref{Ref: "file://testdata/materials.yaml"}},
167+
},
168+
},
169+
}
170+
171+
statement := loadStatementForTest(t, "testdata/statement.json")
172+
173+
// Sequential
174+
pvSeq := NewPolicyVerifier(schema.GetPolicies(), nil, &logger, WithMaxConcurrency(1))
175+
seqResults, seqErr := pvSeq.VerifyStatement(context.Background(), statement)
176+
177+
// Parallel (default)
178+
pvPar := NewPolicyVerifier(schema.GetPolicies(), nil, &logger)
179+
parResults, parErr := pvPar.VerifyStatement(context.Background(), statement)
180+
181+
require.NoError(t, seqErr)
182+
require.NoError(t, parErr)
183+
assert.Equal(t, len(seqResults), len(parResults),
184+
"sequential and parallel should return same number of evaluations")
185+
186+
// Build name→result maps for comparison (order may differ)
187+
seqByName := make(map[string]*v1.PolicyEvaluation)
188+
for _, ev := range seqResults {
189+
seqByName[ev.Name] = ev
190+
}
191+
192+
for _, ev := range parResults {
193+
seqEv, ok := seqByName[ev.Name]
194+
assert.True(t, ok, "parallel result has policy %q not found in sequential", ev.Name)
195+
if ok {
196+
assert.Equal(t, len(seqEv.Violations), len(ev.Violations),
197+
"policy %q: violation count mismatch", ev.Name)
198+
assert.Equal(t, seqEv.Skipped, ev.Skipped,
199+
"policy %q: skipped mismatch", ev.Name)
200+
}
201+
}
202+
}
203+
204+
// TestErrGroupCancellation verifies that when one policy evaluation fails,
205+
// the errgroup context is cancelled and propagated.
206+
func TestErrGroupCancellation(t *testing.T) {
207+
logger := zerolog.Nop()
208+
209+
schema := &v12.CraftingSchema{
210+
Policies: &v12.Policies{
211+
Attestation: []*v12.PolicyAttachment{
212+
{Policy: &v12.PolicyAttachment_Ref{Ref: "file://testdata/workflow.yaml"}},
213+
// This policy ref does not exist — will cause an error
214+
{Policy: &v12.PolicyAttachment_Ref{Ref: "file://testdata/nonexistent_policy.yaml"}},
215+
},
216+
},
217+
}
218+
219+
statement := loadStatementForTest(t, "testdata/statement.json")
220+
221+
pv := NewPolicyVerifier(schema.GetPolicies(), nil, &logger)
222+
_, err := pv.VerifyStatement(context.Background(), statement)
223+
224+
assert.Error(t, err, "should fail when a policy file does not exist")
225+
assert.IsType(t, &PolicyError{}, err, "error should be a PolicyError")
226+
}
227+
228+
func loadStatementForTest(t *testing.T, file string) *intoto.Statement {
229+
t.Helper()
230+
stContent, err := os.ReadFile(file)
231+
require.NoError(t, err)
232+
var statement intoto.Statement
233+
err = protojson.Unmarshal(stContent, &statement)
234+
require.NoError(t, err)
235+
return &statement
236+
}

pkg/policies/engine/wasm/engine.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ import (
3131

3232
// setLogLevelOnce ensures extism.SetLogLevel is called exactly once,
3333
// since it modifies global state and is not safe for concurrent calls.
34+
// Note: the log level is determined by the first NewEngine() call in the
35+
// process. Subsequent engines with different log levels will not override it.
3436
var setLogLevelOnce sync.Once
3537

3638
// Ensure Engine implements PolicyEngine interface

pkg/policies/group_loader.go

Lines changed: 47 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import (
2828
pb "github.com/chainloop-dev/chainloop/app/controlplane/api/controlplane/v1"
2929
v1 "github.com/chainloop-dev/chainloop/app/controlplane/api/workflowcontract/v1"
3030
crv1 "github.com/google/go-containerregistry/pkg/v1"
31+
"golang.org/x/sync/singleflight"
3132
)
3233

3334
// GroupLoader defines the interface for policy loaders from contract attachments
@@ -109,6 +110,7 @@ type groupWithReference struct {
109110
var (
110111
remoteGroupCache = make(map[string]*groupWithReference)
111112
remoteGroupCacheMutex sync.Mutex
113+
remoteGroupFlight singleflight.Group
112114
)
113115

114116
func NewChainloopGroupLoader(client pb.AttestationServiceClient) *ChainloopGroupLoader {
@@ -118,49 +120,64 @@ func NewChainloopGroupLoader(client pb.AttestationServiceClient) *ChainloopGroup
118120
func (c *ChainloopGroupLoader) Load(ctx context.Context, attachment *v1.PolicyGroupAttachment) (*v1.PolicyGroup, *PolicyDescriptor, error) {
119121
ref := attachment.GetRef()
120122

121-
// Check cache (read under lock, release before any I/O)
123+
// Fast path: check cache under lock
122124
remoteGroupCacheMutex.Lock()
123125
if v, ok := remoteGroupCache[ref]; ok {
124126
remoteGroupCacheMutex.Unlock()
125127
return v.group, v.reference, nil
126128
}
127129
remoteGroupCacheMutex.Unlock()
128130

129-
if !IsProviderScheme(ref) {
130-
return nil, nil, fmt.Errorf("invalid group reference %q", ref)
131-
}
131+
// Use singleflight to coalesce concurrent fetches for the same ref.
132+
result, err, _ := remoteGroupFlight.Do(ref, func() (interface{}, error) {
133+
// Re-check cache (another goroutine may have populated it)
134+
remoteGroupCacheMutex.Lock()
135+
if v, ok := remoteGroupCache[ref]; ok {
136+
remoteGroupCacheMutex.Unlock()
137+
return v, nil
138+
}
139+
remoteGroupCacheMutex.Unlock()
132140

133-
providerRef := ProviderParts(ref)
141+
if !IsProviderScheme(ref) {
142+
return nil, fmt.Errorf("invalid group reference %q", ref)
143+
}
134144

135-
// gRPC call happens outside the lock
136-
resp, err := c.Client.GetPolicyGroup(ctx, &pb.AttestationServiceGetPolicyGroupRequest{
137-
Provider: providerRef.Provider,
138-
GroupName: providerRef.Name,
139-
OrgName: providerRef.OrgName,
140-
})
141-
if err != nil {
142-
return nil, nil, fmt.Errorf("requesting remote group (provider: %s, name: %s): %w", providerRef.Provider, providerRef.Name, err)
143-
}
145+
providerRef := ProviderParts(ref)
144146

145-
h, err := crv1.NewHash(resp.Reference.GetDigest())
146-
if err != nil {
147-
return nil, nil, fmt.Errorf("parsing digest: %w", err)
148-
}
147+
resp, err := c.Client.GetPolicyGroup(ctx, &pb.AttestationServiceGetPolicyGroupRequest{
148+
Provider: providerRef.Provider,
149+
GroupName: providerRef.Name,
150+
OrgName: providerRef.OrgName,
151+
})
152+
if err != nil {
153+
return nil, fmt.Errorf("requesting remote group (provider: %s, name: %s): %w", providerRef.Provider, providerRef.Name, err)
154+
}
149155

150-
orgName := providerRef.OrgName
151-
// Extract organization name from URL if present
152-
if u, err := url.Parse(resp.Reference.GetUrl()); err == nil {
153-
if orgParam := u.Query().Get("org"); orgParam != "" {
154-
orgName = orgParam
156+
h, err := crv1.NewHash(resp.Reference.GetDigest())
157+
if err != nil {
158+
return nil, fmt.Errorf("parsing digest: %w", err)
155159
}
156-
}
157160

158-
reference := policyReferenceResourceDescriptor(providerRef.Name, resp.Reference.GetUrl(), orgName, h)
161+
orgName := providerRef.OrgName
162+
if u, err := url.Parse(resp.Reference.GetUrl()); err == nil {
163+
if orgParam := u.Query().Get("org"); orgParam != "" {
164+
orgName = orgParam
165+
}
166+
}
159167

160-
// Write to cache under lock
161-
remoteGroupCacheMutex.Lock()
162-
remoteGroupCache[ref] = &groupWithReference{group: resp.GetGroup(), reference: reference}
163-
remoteGroupCacheMutex.Unlock()
168+
reference := policyReferenceResourceDescriptor(providerRef.Name, resp.Reference.GetUrl(), orgName, h)
169+
cached := &groupWithReference{group: resp.GetGroup(), reference: reference}
170+
171+
remoteGroupCacheMutex.Lock()
172+
remoteGroupCache[ref] = cached
173+
remoteGroupCacheMutex.Unlock()
174+
175+
return cached, nil
176+
})
177+
if err != nil {
178+
return nil, nil, err
179+
}
164180

165-
return resp.GetGroup(), reference, nil
181+
cached := result.(*groupWithReference)
182+
return cached.group, cached.reference, nil
166183
}

0 commit comments

Comments
 (0)