Skip to content

Comments

policy: mark policy progress vertex on DENY build failures#3656

Merged
tonistiigi merged 2 commits intodocker:masterfrom
tonistiigi:policy-error
Feb 20, 2026
Merged

policy: mark policy progress vertex on DENY build failures#3656
tonistiigi merged 2 commits intodocker:masterfrom
tonistiigi:policy-error

Conversation

@tonistiigi
Copy link
Member

fix #3644

Track denied source identifiers during policy evaluation and flag the policy progress vertex as failed when BuildKit returns a matching DENY error pattern.

This improves the progress output of policy error and shows last policy logs with the build error.

Track denied source identifiers during policy evaluation and flag the policy
progress vertex as failed when BuildKit returns a matching DENY error pattern.

This improves the progress output of policy error and shows last
policy logs with the build error.

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Comment on lines 13 to 56
func TestPolicyIsPolicyErrorMatchesRecordedSource(t *testing.T) {
p := NewPolicy(Opt{})
req := &policysession.CheckPolicyRequest{
Source: &gwpb.ResolveSourceMetaResponse{
Source: &solverpb.SourceOp{
Identifier: "docker-image://busybox:latest",
},
},
}
p.recordDenyIdentifier(req)

err := errors.New("failed to solve: error evaluating the source policy: source \"docker-image://busybox:latest\" not allowed by policy: action DENY")
require.True(t, p.IsPolicyError(err))
}

func TestPolicyIsPolicyErrorDoesNotMatchWithoutBuildkitPattern(t *testing.T) {
p := NewPolicy(Opt{})
req := &policysession.CheckPolicyRequest{
Source: &gwpb.ResolveSourceMetaResponse{
Source: &solverpb.SourceOp{
Identifier: "docker-image://busybox:latest",
},
},
}
p.recordDenyIdentifier(req)

err := errors.New("failed to parse dockerfile for docker-image://busybox:latest")
require.False(t, p.IsPolicyError(err))
}

func TestPolicyIsPolicyErrorDoesNotMatchUnrelatedError(t *testing.T) {
p := NewPolicy(Opt{})
req := &policysession.CheckPolicyRequest{
Source: &gwpb.ResolveSourceMetaResponse{
Source: &solverpb.SourceOp{
Identifier: "docker-image://busybox:latest",
},
},
}
p.recordDenyIdentifier(req)

err := errors.New("failed to solve: error evaluating the source policy: source \"docker-image://alpine:latest\" not allowed by policy: action DENY")
require.False(t, p.IsPolicyError(err))
}
Copy link
Member

@crazy-max crazy-max Feb 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: these tests look like a good fit for a single table-driven test ([]struct{name, err, want}) since setup is identical across cases. And could maybe also moved to validate_test.go and named func TestPolicyIsPolicyError?

@crazy-max
Copy link
Member

crazy-max commented Feb 19, 2026

Could we have an integration test around the release(err)/policyLogger.Close(inErr) flow? Similar to

func testBuildProgress(t *testing.T, sb integration.Sandbox) {
just to test progress.

@crazy-max crazy-max added this to the v0.32.0 milestone Feb 19, 2026
@tonistiigi
Copy link
Member Author

Added progress integration test and refactored unit test.

Refactor policy error unit tests to table-driven subtests with slug names.
Add rawjson integration coverage to verify policy vertex captures DENY build
errors in progress output.

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
@crazy-max
Copy link
Member

Seems there is a deadlock in sourcemeta resolver, opened #3659

@tonistiigi
Copy link
Member Author

Fix for that deadlock is in #3657. If you want, I can pull it out, but #3657 has some other changes to sourcemeta as well that would cause rebase conflicts.

@tonistiigi tonistiigi merged commit 7d3b698 into docker:master Feb 20, 2026
291 of 293 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Policy: associate policy error with the build step progress item

2 participants