sourcepolicy: Do not pass state between policies

Per our discussion on github, each policy should be evaluated on it's
own.
ie. an "allow" in one policy should be able to change to a "deny" in
another policy.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
This commit is contained in:
Brian Goff
2022-12-08 18:51:51 +00:00
committed by Tonis Tiigi
parent c7a54c7453
commit 3e93b541fd
3 changed files with 45 additions and 24 deletions

View File

@@ -8612,21 +8612,5 @@ func testSourcePolicy(t *testing.T, sb integration.Sandbox) {
_, err = c.Build(sb.Context(), SolveOpt{}, "", frontend, nil)
require.ErrorContains(t, err, sourcepolicy.ErrSourceDenied.Error())
// Override frontend policy
_, err = c.Build(sb.Context(), SolveOpt{
SourcePolicy: &sourcepolicypb.Policy{
Rules: []*sourcepolicypb.Rule{
{
Action: sourcepolicypb.PolicyAction_ALLOW,
Source: &sourcepolicypb.Source{
Type: "http",
Identifier: denied,
},
},
},
},
}, "", frontend, nil)
require.NoError(t, err)
})
}

View File

@@ -65,10 +65,7 @@ func (e *Engine) Evaluate(ctx context.Context, op *pb.Op) (bool, error) {
return false, nil
}
var (
st evalState
mutated bool
)
var mutated bool
const maxIterr = 20
for i := 0; ; i++ {
@@ -84,7 +81,7 @@ func (e *Engine) Evaluate(ctx context.Context, op *pb.Op) (bool, error) {
ctx = bklog.WithLogger(ctx, bklog.G(ctx).WithField("orig", *srcOp).WithField("updated", op.GetSource()))
}
mut, err := e.evaluatePolicies(ctx, srcOp, &st)
mut, err := e.evaluatePolicies(ctx, srcOp)
if mut {
mutated = true
}
@@ -99,7 +96,7 @@ func (e *Engine) Evaluate(ctx context.Context, op *pb.Op) (bool, error) {
return mutated, nil
}
func (e *Engine) evaluatePolicies(ctx context.Context, srcOp *pb.SourceOp, st *evalState) (bool, error) {
func (e *Engine) evaluatePolicies(ctx context.Context, srcOp *pb.SourceOp) (bool, error) {
ident := srcOp.GetIdentifier()
scheme, ref, found := strings.Cut(ident, "://")
if !found || ref == "" {
@@ -112,7 +109,7 @@ func (e *Engine) evaluatePolicies(ctx context.Context, srcOp *pb.SourceOp, st *e
}))
for _, pol := range e.pol {
mut, err := e.evaluatePolicy(ctx, pol, srcOp, st, scheme, ref)
mut, err := e.evaluatePolicy(ctx, pol, srcOp, scheme, ref)
if mut || err != nil {
return mut, err
}
@@ -120,7 +117,8 @@ func (e *Engine) evaluatePolicies(ctx context.Context, srcOp *pb.SourceOp, st *e
return false, nil
}
func (e *Engine) evaluatePolicy(ctx context.Context, pol *spb.Policy, srcOp *pb.SourceOp, st *evalState, scheme, ref string) (bool, error) {
func (e *Engine) evaluatePolicy(ctx context.Context, pol *spb.Policy, srcOp *pb.SourceOp, scheme, ref string) (bool, error) {
st := &evalState{}
for _, rule := range pol.Rules {
mut, err := e.evaluateRule(ctx, rule, scheme, ref, srcOp, st)
if mut || err != nil {

View File

@@ -23,6 +23,45 @@ func TestEngineEvaluate(t *testing.T) {
t.Run("Test convert regex", testConvertRegex)
t.Run("Test convert wildcard", testConvertWildcard)
t.Run("Test convert multiple", testConvertMultiple)
t.Run("test multiple policies", testMultiplePolicies)
}
func testMultiplePolicies(t *testing.T) {
pol := []*spb.Policy{
{
Rules: []*spb.Rule{
{
Action: spb.PolicyAction_ALLOW,
Source: &spb.Source{
Type: "docker-image",
Identifier: "docker.io/library/busybox:latest",
},
},
},
},
{
Rules: []*spb.Rule{
{
Action: spb.PolicyAction_DENY,
Source: &spb.Source{
Type: "docker-image",
Identifier: "docker.io/library/busybox:latest",
},
},
},
},
}
e := NewEngine(pol)
mut, err := e.Evaluate(context.Background(), &pb.Op{
Op: &pb.Op_Source{
Source: &pb.SourceOp{
Identifier: "docker-image://docker.io/library/busybox:latest",
},
},
})
require.ErrorIs(t, err, ErrSourceDenied)
require.False(t, mut)
}
func testConvertMultiple(t *testing.T) {