Skip to content

Commit 8feb92d

Browse files
committed
Issue #2521: fix(scripts/buildinputs): fix build input detection to deal with mounts (#2522)
``` + /Users/jdanek/IdeaProjects/notebooks/scripts/sandbox.py --dockerfile codeserver/ubi9-python-3.12/Dockerfile.cpu --platform linux/amd64 -- podman build --no-cache --platform=linux/amd64 --label release=2025a --tag quay.io/opendatahub/workbench-images:codeserver-ubi9-python-3.12-2025a_20250919 --file codeserver/ubi9-python-3.12/Dockerfile.cpu --build-arg BASE_IMAGE=registry.access.redhat.com/ubi9/python-312:latest '{};' __file__='/Users/jdanek/IdeaProjects/notebooks/scripts/sandbox.py' started with args=Namespace(dockerfile=PosixPath('codeserver/ubi9-python-3.12/Dockerfile.cpu'), platform='linux/amd64', remaining=['--', 'podman', 'build', '--no-cache', '--platform=linux/amd64', '--label', 'release=2025a', '--tag', 'quay.io/opendatahub/workbench-images:codeserver-ubi9-python-3.12-2025a_20250919', '--file', 'codeserver/ubi9-python-3.12/Dockerfile.cpu', '--build-arg', 'BASE_IMAGE=registry.access.redhat.com/ubi9/python-312:latest', '{};']) prereqs=[PosixPath('codeserver/ubi9-python-3.12/test'), PosixPath('foo'), PosixPath('tmp/test_log.txt')] Traceback (most recent call last): File "/Users/jdanek/IdeaProjects/notebooks/scripts/sandbox.py", line 91, in <module> sys.exit(main()) ^^^^^^ File "/Users/jdanek/IdeaProjects/notebooks/scripts/sandbox.py", line 49, in main setup_sandbox(prereqs, pathlib.Path(tmpdir)) File "/Users/jdanek/IdeaProjects/notebooks/scripts/sandbox.py", line 87, in setup_sandbox shutil.copy(dep, tmpdir / dep.parent) File "/opt/homebrew/Cellar/[email protected]/3.12.11/Frameworks/Python.framework/Versions/3.12/lib/python3.12/shutil.py", line 435, in copy copyfile(src, dst, follow_symlinks=follow_symlinks) File "/opt/homebrew/Cellar/[email protected]/3.12.11/Frameworks/Python.framework/Versions/3.12/lib/python3.12/shutil.py", line 260, in copyfile with open(src, 'rb') as fsrc: ^^^^^^^^^^^^^^^ FileNotFoundError: [Errno 2] No such file or directory: 'foo' gmake: *** [Makefile:177: codeserver-ubi9-python-3.12] Error 1 ```
1 parent c7d14cd commit 8feb92d

File tree

6 files changed

+130
-33
lines changed

6 files changed

+130
-33
lines changed

scripts/buildinputs/buildinputs.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package main
22

33
import (
4+
"encoding/json"
45
"flag"
56
"fmt"
67
"os"
@@ -26,6 +27,11 @@ func main() {
2627
flag.Parse()
2728
for _, dockerfile := range flag.Args() {
2829
deps := getDockerfileDeps(dockerfile, targetArch)
29-
fmt.Println(deps)
30+
// nil slice encodes to null, which is not what we want
31+
if deps == nil {
32+
deps = []string{}
33+
}
34+
encoder := json.NewEncoder(os.Stdout)
35+
noErr(encoder.Encode(deps))
3036
}
3137
}

scripts/buildinputs/buildinputs_test.go

Lines changed: 62 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
package main
22

33
import (
4-
"encoding/json"
54
"os"
65
"path/filepath"
76
"reflect"
87
"runtime"
8+
"slices"
99
"strings"
1010
"testing"
1111
)
@@ -40,14 +40,15 @@ func TestParseAllDockerfiles(t *testing.T) {
4040
for _, dockerfile := range dockerfiles {
4141
t.Run(dockerfile, func(t *testing.T) {
4242
result := getDockerfileDeps(dockerfile, "amd64")
43-
if result == "" {
43+
if len(result) == 0 {
4444
// no deps in the dockerfile
4545
return
4646
}
47-
data := make([]string, 0)
48-
noErr(json.Unmarshal([]byte(result), &data))
49-
for _, path := range data {
50-
stat := noErr2(os.Stat(filepath.Join(projectRoot, path)))
47+
for _, path := range result {
48+
stat, err := os.Stat(filepath.Join(projectRoot, path))
49+
if err != nil {
50+
t.Fatal(err)
51+
}
5152
if stat.IsDir() {
5253
// log this very interesting observation
5354
t.Logf("dockerfile copies in a whole directory: %s", path)
@@ -56,3 +57,58 @@ func TestParseAllDockerfiles(t *testing.T) {
5657
})
5758
}
5859
}
60+
61+
// TestParseDockerfileWithBindMount checks for a bug where a Dockerfile with RUN --mount=type=bind,src=foo,dst=bar would report it has no inputs
62+
func TestParseDockerfileWithBindMount(t *testing.T) {
63+
dockerfile := filepath.Join(t.TempDir(), "Dockerfile")
64+
// language=Dockerfile
65+
noErr(os.WriteFile(dockerfile, []byte(Doc(`
66+
FROM codeserver AS tests
67+
ARG CODESERVER_SOURCE_CODE=codeserver/ubi9-python-3.12
68+
COPY ${CODESERVER_SOURCE_CODE}/test /tmp/test
69+
RUN --mount=type=tmpfs,target=/opt/app-root/src --mount=type=bind,src=foo,dst=bar <<'EOF'
70+
set -Eeuxo pipefail
71+
python3 /tmp/test/test_startup.py |& tee /tmp/test_log.txt
72+
EOF
73+
`)), 0644))
74+
75+
//dockerfile = "/Users/jdanek/IdeaProjects/notebooks/jupyter/rocm/pytorch/ubi9-python-3.12/Dockerfile.rocm"
76+
77+
result := getDockerfileDeps(dockerfile, "amd64")
78+
expected := []string{"codeserver/ubi9-python-3.12/test", "foo"}
79+
if !reflect.DeepEqual(
80+
slices.Sorted(slices.Values(result)),
81+
slices.Sorted(slices.Values(expected)),
82+
) {
83+
t.Errorf("expected %v but got %v", expected, result)
84+
}
85+
}
86+
87+
func TestParseFileWithStageCopy(t *testing.T) {
88+
dockerfile := filepath.Join(t.TempDir(), "Dockerfile")
89+
// language=Dockerfile
90+
noErr(os.WriteFile(dockerfile, []byte(Doc(`
91+
FROM codeserver
92+
COPY --from=registry.access.redhat.com/ubi9/ubi /etc/yum.repos.d/ubi.repo /etc/yum.repos.d/ubi.repo
93+
`)), 0644))
94+
95+
result := getDockerfileDeps(dockerfile, "amd64")
96+
if len(result) != 0 {
97+
t.Fatalf("unexpected deps reported for the dockerfile: %s", result)
98+
}
99+
}
100+
101+
func TestParseFileWithStageMount(t *testing.T) {
102+
dockerfile := filepath.Join(t.TempDir(), "Dockerfile")
103+
// language=Dockerfile
104+
noErr(os.WriteFile(dockerfile, []byte(Doc(`
105+
FROM javabuilder
106+
RUN --mount=type=bind,from=build,source=/.m2_repository,target=/.m2_repository \
107+
mvn package
108+
`)), 0644))
109+
110+
result := getDockerfileDeps(dockerfile, "amd64")
111+
if len(result) != 0 {
112+
t.Fatalf("unexpected deps reported for the dockerfile: %s", result)
113+
}
114+
}

scripts/buildinputs/dockerfile.go

Lines changed: 48 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,9 @@ package main
33
import (
44
"context"
55
"encoding/json"
6-
"fmt"
76
"log"
87
"os"
8+
"path/filepath"
99
"strings"
1010

1111
"github.com/containerd/platforms"
@@ -20,7 +20,7 @@ import (
2020
"github.com/pkg/errors"
2121
)
2222

23-
func getDockerfileDeps(dockerfile string, targetArch string) string {
23+
func getDockerfileDeps(dockerfile string, targetArch string) []string {
2424
ctx := context.Background()
2525
data := noErr2(os.ReadFile(dockerfile))
2626

@@ -47,42 +47,69 @@ func getDockerfileDeps(dockerfile string, targetArch string) string {
4747
return getOpSourceFollowPaths(definition)
4848
}
4949

50-
func getOpSourceFollowPaths(definition *llb.Definition) string {
50+
func getOpSourceFollowPaths(definition *llb.Definition) []string {
5151
// https://earthly.dev/blog/compiling-containers-dockerfiles-llvm-and-buildkit/
5252
// https://stackoverflow.com/questions/73067660/what-exactly-is-the-frontend-and-backend-of-docker-buildkit
5353

54-
ops := make([]llbOp, 0)
54+
opsByDigest := make(map[digest.Digest]llbOp, len(definition.Def))
5555
for _, dt := range definition.Def {
5656
var op pb.Op
5757
if err := op.UnmarshalVT(dt); err != nil {
5858
panic("failed to parse op")
5959
}
6060
dgst := digest.FromBytes(dt)
61-
ent := llbOp{Op: &op, Digest: dgst, OpMetadata: definition.Metadata[dgst].ToPB()}
62-
ops = append(ops, ent)
61+
ent := llbOp{
62+
Op: &op,
63+
Digest: dgst,
64+
OpMetadata: definition.Metadata[dgst].ToPB(),
65+
}
66+
opsByDigest[dgst] = ent
6367
}
6468

65-
var result string = ""
66-
for _, op := range ops {
67-
switch op := op.Op.Op.(type) {
68-
case *pb.Op_Source:
69-
if strings.HasPrefix(op.Source.Identifier, "docker-image://") {
70-
// no-op
71-
} else if strings.HasPrefix(op.Source.Identifier, "local://") {
72-
paths := op.Source.Attrs[pb.AttrFollowPaths]
73-
// TODO treat result as a set of strings to get unique set across all layers
74-
// this code "works" as is because it seems the terminal layer is the last one processed - which
75-
// contains all the referenced files - but treating this as a set seems safer (?)
76-
result = paths
77-
log.Printf("found paths: %s", paths)
78-
} else {
79-
panic(fmt.Errorf("unexpected prefix %v", op.Source.Identifier))
69+
var result []string
70+
for _, opDef := range opsByDigest {
71+
switch top := opDef.Op.Op.(type) {
72+
// https://github.com/moby/buildkit/blob/v0.24/solver/pb/ops.proto#L308-L325
73+
case *pb.Op_File:
74+
for _, a := range top.File.Actions {
75+
// NOTE CAREFULLY: FileActionCopy copies files from secondaryInput on top of input
76+
if cpy := a.GetCopy(); cpy != nil {
77+
if inputIsFromLocalContext(a.SecondaryInput, opDef.Op.Inputs, opsByDigest) {
78+
result = append(result, cleanPath(cpy.Src))
79+
}
80+
}
81+
}
82+
case *pb.Op_Exec:
83+
for _, m := range top.Exec.Mounts {
84+
if inputIsFromLocalContext(m.Input, opDef.Op.Inputs, opsByDigest) {
85+
result = append(result, cleanPath(m.Selector))
86+
}
8087
}
8188
}
8289
}
90+
8391
return result
8492
}
8593

94+
func cleanPath(path string) string {
95+
return noErr2(filepath.Rel("/", filepath.Clean(path)))
96+
}
97+
98+
func inputIsFromLocalContext(input int64, inputs []*pb.Input, opsByDigest map[digest.Digest]llbOp) bool {
99+
// input is -1 if the input is a FROM scratch or equivalent
100+
if input == -1 {
101+
return false
102+
}
103+
104+
srcDigest := digest.Digest(inputs[input].Digest)
105+
sourceOp := opsByDigest[srcDigest]
106+
if src, ok := sourceOp.Op.Op.(*pb.Op_Source); ok {
107+
// local://context is the primary context, but there may be multiple named contexts
108+
return strings.HasPrefix(src.Source.Identifier, "local://")
109+
}
110+
return false
111+
}
112+
86113
// llbOp holds data for a single loaded LLB op
87114
type llbOp struct {
88115
Op *pb.Op

scripts/buildinputs/go.mod

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,21 @@
11
module dockerfile
22

3-
go 1.24
3+
go 1.24.0
4+
5+
toolchain go1.24.5
46

57
require (
68
github.com/containerd/platforms v1.0.0-rc.1
79
github.com/google/go-cmp v0.7.0
8-
github.com/moby/buildkit v0.23.2
10+
github.com/moby/buildkit v0.24.0
911
github.com/opencontainers/go-digest v1.0.0
1012
github.com/opencontainers/image-spec v1.1.1
1113
github.com/pkg/errors v0.9.1
1214
)
1315

1416
require (
1517
github.com/agext/levenshtein v1.2.3 // indirect
16-
github.com/containerd/containerd/v2 v2.1.3 // indirect
18+
github.com/containerd/containerd/v2 v2.1.4 // indirect
1719
github.com/containerd/errdefs v1.0.0 // indirect
1820
github.com/containerd/log v0.1.0 // indirect
1921
github.com/containerd/ttrpc v1.2.7 // indirect
@@ -54,7 +56,7 @@ require (
5456
golang.org/x/crypto v0.40.0 // indirect
5557
golang.org/x/net v0.42.0 // indirect
5658
golang.org/x/sync v0.16.0 // indirect
57-
golang.org/x/sys v0.34.0 // indirect
59+
golang.org/x/sys v0.36.0 // indirect
5860
golang.org/x/text v0.27.0 // indirect
5961
google.golang.org/genproto/googleapis/rpc v0.0.0-20250715232539-7130f93afb79 // indirect
6062
google.golang.org/grpc v1.74.0 // indirect

scripts/buildinputs/go.sum

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ github.com/codahale/rfc6979 v0.0.0-20141003034818-6a90f24967eb h1:EDmT6Q9Zs+SbUo
66
github.com/codahale/rfc6979 v0.0.0-20141003034818-6a90f24967eb/go.mod h1:ZjrT6AXHbDs86ZSdt/osfBi5qfexBrKUdONk989Wnk4=
77
github.com/containerd/containerd/v2 v2.1.3 h1:eMD2SLcIQPdMlnlNF6fatlrlRLAeDaiGPGwmRKLZKNs=
88
github.com/containerd/containerd/v2 v2.1.3/go.mod h1:8C5QV9djwsYDNhxfTCFjWtTBZrqjditQ4/ghHSYjnHM=
9+
github.com/containerd/containerd/v2 v2.1.4 h1:/hXWjiSFd6ftrBOBGfAZ6T30LJcx1dBjdKEeI8xucKQ=
10+
github.com/containerd/containerd/v2 v2.1.4/go.mod h1:8C5QV9djwsYDNhxfTCFjWtTBZrqjditQ4/ghHSYjnHM=
911
github.com/containerd/errdefs v1.0.0 h1:tg5yIfIlQIrxYtu9ajqY42W3lpS19XqdxRQeEwYG8PI=
1012
github.com/containerd/errdefs v1.0.0/go.mod h1:+YBYIdtsnF4Iw6nWZhJcqGSg/dwvV7tyJ/kCkyJ2k+M=
1113
github.com/containerd/log v0.1.0 h1:TCJt7ioM2cr/tfR8GPbGf9/VRAX8D2B4PjzCpfX540I=
@@ -55,6 +57,8 @@ github.com/klauspost/compress v1.18.0 h1:c/Cqfb0r+Yi+JtIEq73FWXVkRonBlf0CRNYc8Zt
5557
github.com/klauspost/compress v1.18.0/go.mod h1:2Pp+KzxcywXVXMr50+X0Q/Lsb43OQHYWRCY2AiWywWQ=
5658
github.com/moby/buildkit v0.23.2 h1:gt/dkfcpgTXKx+B9I310kV767hhVqTvEyxGgI3mqsGQ=
5759
github.com/moby/buildkit v0.23.2/go.mod h1:iEjAfPQKIuO+8y6OcInInvzqTMiKMbb2RdJz1K/95a0=
60+
github.com/moby/buildkit v0.24.0 h1:qYfTl7W1SIJzWDIDCcPT8FboHIZCYfi++wvySi3eyFE=
61+
github.com/moby/buildkit v0.24.0/go.mod h1:4qovICAdR2H4C7+EGMRva5zgHW1gyhT4/flHI7F5F9k=
5862
github.com/moby/docker-image-spec v1.3.1 h1:jMKff3w6PgbfSa69GfNg+zN/XLhfXJGnEx3Nl2EsFP0=
5963
github.com/moby/docker-image-spec v1.3.1/go.mod h1:eKmb5VW8vQEh/BAr2yvVNvuiJuY6UIocYsFu/DxxRpo=
6064
github.com/moby/locker v1.0.1 h1:fOXqR41zeveg4fFODix+1Ch4mj/gT0NE1XJbp/epuBg=
@@ -137,6 +141,8 @@ golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7w
137141
golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
138142
golang.org/x/sys v0.34.0 h1:H5Y5sJ2L2JRdyv7ROF1he/lPdvFsd0mJHFw2ThKHxLA=
139143
golang.org/x/sys v0.34.0/go.mod h1:BJP2sWEmIv4KK5OTEluFJCKSidICx8ciO85XgH3Ak8k=
144+
golang.org/x/sys v0.36.0 h1:KVRy2GtZBrk1cBYA7MKu5bEZFxQk4NIDV6RLVcC8o0k=
145+
golang.org/x/sys v0.36.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks=
140146
golang.org/x/term v0.33.0 h1:NuFncQrRcaRvVmgRkvM3j/F00gWIAlcmlB8ACEKmGIg=
141147
golang.org/x/term v0.33.0/go.mod h1:s18+ql9tYWp1IfpV9DmCtQDDSRBUjKaw9M1eAv5UeF0=
142148
golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=

scripts/sandbox.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ def buildinputs(
6666
stdout = subprocess.check_output([ROOT_DIR / "bin/buildinputs", str(dockerfile)],
6767
text=True, cwd=ROOT_DIR,
6868
env={"TARGETPLATFORM": platform, **os.environ})
69-
prereqs = [pathlib.Path(file) for file in json.loads(stdout)] if stdout != "\n" else []
69+
prereqs = [pathlib.Path(file) for file in json.loads(stdout)]
7070
print(f"{prereqs=}")
7171
return prereqs
7272

0 commit comments

Comments
 (0)