Skip to content

Commit

Permalink
dockerfile: remove duplicate layer chains from provenance attestation
Browse files Browse the repository at this point in the history
When a step in the dockerfile is a dependency of multiple other steps in
the dockerfile, the provenance attestation would record the layer chain
for that step multiple times even with the same layer chain.

This is because the provenance attestation reuses the exporter mechanic
and the exporter mechanic would need to visit this same step multiple
times to produce the appropriate cache entries.

Since these duplicate layer chains aren't intentional, this modifies the
provenance attestation capture to detect these duplicates and remove
them.

Signed-off-by: Jonathan A. Sternberg <[email protected]>
  • Loading branch information
jsternberg committed Nov 22, 2023
1 parent dd196ce commit 94af8de
Show file tree
Hide file tree
Showing 3 changed files with 105 additions and 1 deletion.
81 changes: 81 additions & 0 deletions frontend/dockerfile/dockerfile_provenance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1268,3 +1268,84 @@ COPY bar bar2
require.Equal(t, dockerfile, sources[0].Data)
require.NotEqual(t, 0, len(sources[0].Definition))
}

func testDuplicateLayersProvenance(t *testing.T, sb integration.Sandbox) {
workers.CheckFeatureCompat(t, sb, workers.FeatureDirectPush, workers.FeatureProvenance)
ctx := sb.Context()

c, err := client.New(ctx, sb.Address())
require.NoError(t, err)
defer c.Close()

registry, err := sb.NewRegistry()
if errors.Is(err, integration.ErrRequirements) {
t.Skip(err.Error())
}
require.NoError(t, err)

f := getFrontend(t, sb)

// Create a triangle shape with the layers.
// This will trigger the provenance attestation to attempt to add the base
// layer multiple times.
dockerfile := []byte(`
FROM busybox:latest AS base
FROM base AS a
RUN date +%s > /a.txt
FROM base AS b
COPY --from=a /a.txt /
RUN date +%s > /b.txt
`)
dir := integration.Tmpdir(
t,
fstest.CreateFile("Dockerfile", dockerfile, 0600),
)

target := registry + "/buildkit/testwithprovenance:dup"

_, err = f.Solve(sb.Context(), c, client.SolveOpt{
LocalDirs: map[string]string{
dockerui.DefaultLocalNameDockerfile: dir,
dockerui.DefaultLocalNameContext: dir,
},
FrontendAttrs: map[string]string{
"attest:provenance": "mode=max",
"filename": "Dockerfile",
},
Exports: []client.ExportEntry{
{
Type: client.ExporterImage,
Attrs: map[string]string{
"name": target,
"push": "true",
},
},
},
}, nil)
require.NoError(t, err)

desc, provider, err := contentutil.ProviderFromRef(target)
require.NoError(t, err)
imgs, err := testutil.ReadImages(sb.Context(), provider, desc)
require.NoError(t, err)
require.Equal(t, 2, len(imgs.Images))

att := imgs.Find("unknown/unknown")
require.NotNil(t, att)

var stmt struct {
Predicate provenance.ProvenancePredicate `json:"predicate"`
}
require.NoError(t, json.Unmarshal(att.LayersRaw[0], &stmt))
pred := stmt.Predicate

// Search for the layer list for step0.
metadata := pred.Metadata
require.NotNil(t, metadata)

layers := metadata.BuildKitMetadata.Layers["step0:0"]
require.NotNil(t, layers)
require.Len(t, layers, 1)
}
1 change: 1 addition & 0 deletions frontend/dockerfile/dockerfile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,7 @@ var allTests = integration.TestFuncs(
testNilContextInSolveGateway,
testCopyUnicodePath,
testFrontendDeduplicateSources,
testDuplicateLayersProvenance,
)

// Tests that depend on the `security.*` entitlements
Expand Down
24 changes: 23 additions & 1 deletion solver/llbsolver/provenance.go
Original file line number Diff line number Diff line change
Expand Up @@ -517,7 +517,7 @@ func (c *cacheRecord) AddResult(dgst digest.Digest, idx int, createdAt time.Time
d.Annotations = containerimage.RemoveInternalLayerAnnotations(d.Annotations, true)
descs[i] = d
}
c.ce.layers[e] = append(c.ce.layers[e], descs)
c.ce.layers[e] = appendLayerChain(c.ce.layers[e], descs)
}

func (c *cacheRecord) LinkFrom(rec solver.CacheExporterRecord, index int, selector string) {
Expand Down Expand Up @@ -696,3 +696,25 @@ func walkDigests(dgsts []digest.Digest, ops map[digest.Digest]*pb.Op, dgst diges
dgsts = append(dgsts, dgst)
return dgsts, nil
}

// appendLayerChain appends a layer chain to the set of layers while checking for duplicate layer chains.
func appendLayerChain(layers [][]ocispecs.Descriptor, descs []ocispecs.Descriptor) [][]ocispecs.Descriptor {
for _, layerDescs := range layers {
if len(layerDescs) != len(descs) {
continue
}

matched := true
for i, d := range layerDescs {
if d.Digest != descs[i].Digest {
matched = false
break
}
}

if matched {
return layers
}
}
return append(layers, descs)
}

0 comments on commit 94af8de

Please sign in to comment.