Skip to content

feat: Refactor to support generalised directory symlink traversal #852

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 15 commits into from
Jun 26, 2025
Merged

feat: Refactor to support generalised directory symlink traversal #852

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
cc73291
refactor: Separate node and rootnode concept
another-rex Jun 24, 2025
d0132c3
Simplify symlink resolving code
another-rex Jun 24, 2025
d18f7e2
Make relative symlinks work
another-rex Jun 24, 2025
af63e25
Fix bug and add symlink tests
another-rex Jun 24, 2025
ca5ebb1
Remove old commented out symlink code
another-rex Jun 24, 2025
e12ba53
Remove logs
another-rex Jun 24, 2025
18c6866
Remove redundant function
another-rex Jun 25, 2025
25c2430
Fix the rest of the tests
another-rex Jun 25, 2025
59110f6
Run linter
another-rex Jun 25, 2025
5a10408
Replace filepath with path
another-rex Jun 25, 2025
b5e7ab5
Expose final symlink resolution to the callers
another-rex Jun 25, 2025
0999b73
Merge remote-tracking branch 'upstream/main' into rootnode-migration
another-rex Jun 25, 2025
712bdca
Add missing argument
another-rex Jun 25, 2025
5eeee91
Address PR comments
another-rex Jun 25, 2025
52dac84
Fix tests
another-rex Jun 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 22 additions & 22 deletions artifact/image/layerscanning/image/image.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ type Image struct {
func (img *Image) FS() scalibrfs.FS {
if len(img.chainLayers) == 0 {
emptyChainLayer := &chainLayer{
fileNodeTree: NewNode(),
fileNodeTree: NewNode(img.config.MaxSymlinkDepth),
}
return emptyChainLayer.FS()
}
Expand Down Expand Up @@ -367,11 +367,10 @@ func initializeChainLayers(v1Layers []v1.Layer, history []v1.History, maxSymlink

for i, v1Layer := range v1Layers {
chainLayers = append(chainLayers, &chainLayer{
fileNodeTree: NewNode(),
index: i,
chainID: chainIDs[i],
latestLayer: convertV1Layer(v1Layer, "", false),
maxSymlinkDepth: maxSymlinkDepth,
fileNodeTree: NewNode(maxSymlinkDepth),
index: i,
chainID: chainIDs[i],
latestLayer: convertV1Layer(v1Layer, "", false, maxSymlinkDepth),
})
}
return chainLayers, nil
Expand All @@ -389,14 +388,13 @@ func initializeChainLayers(v1Layers []v1.Layer, history []v1.History, maxSymlink
for _, entry := range history {
if entry.EmptyLayer {
chainLayers = append(chainLayers, &chainLayer{
fileNodeTree: NewNode(),
fileNodeTree: NewNode(maxSymlinkDepth),
index: historyIndex,
latestLayer: &Layer{
buildCommand: entry.CreatedBy,
isEmpty: true,
fileNodeTree: NewNode(),
fileNodeTree: NewNode(maxSymlinkDepth),
},
maxSymlinkDepth: maxSymlinkDepth,
})
historyIndex++
continue
Expand All @@ -408,11 +406,10 @@ func initializeChainLayers(v1Layers []v1.Layer, history []v1.History, maxSymlink

nextNonEmptyLayer := v1Layers[v1LayerIndex]
chainLayer := &chainLayer{
fileNodeTree: NewNode(),
index: historyIndex,
chainID: chainIDs[v1LayerIndex],
latestLayer: convertV1Layer(nextNonEmptyLayer, entry.CreatedBy, false),
maxSymlinkDepth: maxSymlinkDepth,
fileNodeTree: NewNode(maxSymlinkDepth),
index: historyIndex,
chainID: chainIDs[v1LayerIndex],
latestLayer: convertV1Layer(nextNonEmptyLayer, entry.CreatedBy, false, maxSymlinkDepth),
}
chainLayers = append(chainLayers, chainLayer)

Expand All @@ -424,11 +421,10 @@ func initializeChainLayers(v1Layers []v1.Layer, history []v1.History, maxSymlink
// This can happen depending on the build process used to create an image.
for v1LayerIndex < len(v1Layers) {
chainLayers = append(chainLayers, &chainLayer{
fileNodeTree: NewNode(),
index: historyIndex,
chainID: chainIDs[v1LayerIndex],
latestLayer: convertV1Layer(v1Layers[v1LayerIndex], "", false),
maxSymlinkDepth: maxSymlinkDepth,
fileNodeTree: NewNode(maxSymlinkDepth),
index: historyIndex,
chainID: chainIDs[v1LayerIndex],
latestLayer: convertV1Layer(v1Layers[v1LayerIndex], "", false, maxSymlinkDepth),
})
v1LayerIndex++
historyIndex++
Expand Down Expand Up @@ -563,7 +559,7 @@ func populateEmptyDirectoryNodes(virtualPath string, chainLayersToFill []*chainL
runningDir = path.Join(runningDir, dir)

// If the directory already exists in the current chain layer, then skip it.
if currentChainLayer.fileNodeTree.Get(runningDir) != nil {
if vf, _ := currentChainLayer.fileNodeTree.Get(runningDir, false); vf != nil {
continue
}

Expand Down Expand Up @@ -650,7 +646,8 @@ func (img *Image) handleFile(virtualPath string, tarReader *tar.Reader, header *
func fillChainLayersWithVirtualFile(chainLayersToFill []*chainLayer, newNode *virtualFile) {
virtualPath := newNode.virtualPath
for _, chainLayer := range chainLayersToFill {
if node := chainLayer.fileNodeTree.Get(virtualPath); node != nil {
// We want the raw final symlink when checking for existence.
if node, _ := chainLayer.fileNodeTree.Get(virtualPath, false); node != nil {
// A newer version of the file already exists on a later chainLayer.
// Since we do not want to overwrite a later layer with information
// written in an earlier layer, skip this file.
Expand Down Expand Up @@ -682,7 +679,10 @@ func inWhiteoutDir(layer *chainLayer, filePath string) bool {
break
}

node := layer.fileNodeTree.Get(dirname)
node, err := layer.fileNodeTree.Get(dirname, false)
if err != nil {
return false
}
if node == nil {
return false
}
Expand Down
53 changes: 30 additions & 23 deletions artifact/image/layerscanning/image/image_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -456,7 +456,10 @@ func TestFromTarball(t *testing.T) {
},
},
},
wantErrWhileReadingFiles: ErrSymlinkCycle,
//wantErrWhileReadingFiles: ErrSymlinkCycle,
// New method cannot determine links, just depth exceeded.
// If symlink depths is a reasonable number it should not matter.
wantErrWhileReadingFiles: ErrSymlinkDepthExceeded,
},
{
name: "image with symlink depth exceeded",
Expand Down Expand Up @@ -1026,7 +1029,7 @@ func TestInitializeChainLayers(t *testing.T) {
history: []v1.History{},
want: []*chainLayer{
{
fileNodeTree: NewNode(),
fileNodeTree: NewNode(DefaultMaxSymlinkDepth),
index: 0,
latestLayer: &Layer{
diffID: diffID1,
Expand All @@ -1048,7 +1051,7 @@ func TestInitializeChainLayers(t *testing.T) {
},
want: []*chainLayer{
{
fileNodeTree: NewNode(),
fileNodeTree: NewNode(DefaultMaxSymlinkDepth),
index: 0,
latestLayer: &Layer{
buildCommand: "COPY ./foo.txt /foo.txt # buildkit",
Expand Down Expand Up @@ -1079,7 +1082,7 @@ func TestInitializeChainLayers(t *testing.T) {
},
want: []*chainLayer{
{
fileNodeTree: NewNode(),
fileNodeTree: NewNode(DefaultMaxSymlinkDepth),
index: 0,
chainID: chainID1,
latestLayer: &Layer{
Expand All @@ -1089,7 +1092,7 @@ func TestInitializeChainLayers(t *testing.T) {
},
},
{
fileNodeTree: NewNode(),
fileNodeTree: NewNode(DefaultMaxSymlinkDepth),
index: 1,
chainID: chainID2,
latestLayer: &Layer{
Expand All @@ -1099,7 +1102,7 @@ func TestInitializeChainLayers(t *testing.T) {
},
},
{
fileNodeTree: NewNode(),
fileNodeTree: NewNode(DefaultMaxSymlinkDepth),
index: 2,
chainID: chainID3,
latestLayer: &Layer{
Expand Down Expand Up @@ -1145,7 +1148,7 @@ func TestInitializeChainLayers(t *testing.T) {
},
want: []*chainLayer{
{
fileNodeTree: NewNode(),
fileNodeTree: NewNode(DefaultMaxSymlinkDepth),
index: 0,
chainID: chainID1,
latestLayer: &Layer{
Expand All @@ -1155,15 +1158,15 @@ func TestInitializeChainLayers(t *testing.T) {
},
},
{
fileNodeTree: NewNode(),
fileNodeTree: NewNode(DefaultMaxSymlinkDepth),
index: 1,
latestLayer: &Layer{
buildCommand: "ENTRYPOINT [\"/bin/sh\"]",
isEmpty: true,
},
},
{
fileNodeTree: NewNode(),
fileNodeTree: NewNode(DefaultMaxSymlinkDepth),
index: 2,
chainID: chainID2,
latestLayer: &Layer{
Expand All @@ -1173,15 +1176,15 @@ func TestInitializeChainLayers(t *testing.T) {
},
},
{
fileNodeTree: NewNode(),
fileNodeTree: NewNode(DefaultMaxSymlinkDepth),
index: 3,
latestLayer: &Layer{
buildCommand: "RANDOM DOCKER COMMAND",
isEmpty: true,
},
},
{
fileNodeTree: NewNode(),
fileNodeTree: NewNode(DefaultMaxSymlinkDepth),
index: 4,
chainID: chainID3,
latestLayer: &Layer{
Expand All @@ -1191,7 +1194,7 @@ func TestInitializeChainLayers(t *testing.T) {
},
},
{
fileNodeTree: NewNode(),
fileNodeTree: NewNode(DefaultMaxSymlinkDepth),
index: 5,
latestLayer: &Layer{
buildCommand: "RUN [\"/bin/sh\"]",
Expand All @@ -1218,7 +1221,7 @@ func TestInitializeChainLayers(t *testing.T) {
},
want: []*chainLayer{
{
fileNodeTree: NewNode(),
fileNodeTree: NewNode(DefaultMaxSymlinkDepth),
index: 0,
chainID: chainID1,
latestLayer: &Layer{
Expand All @@ -1228,7 +1231,7 @@ func TestInitializeChainLayers(t *testing.T) {
},
},
{
fileNodeTree: NewNode(),
fileNodeTree: NewNode(DefaultMaxSymlinkDepth),
index: 1,
chainID: chainID2,
latestLayer: &Layer{
Expand All @@ -1237,7 +1240,7 @@ func TestInitializeChainLayers(t *testing.T) {
},
},
{
fileNodeTree: NewNode(),
fileNodeTree: NewNode(DefaultMaxSymlinkDepth),
index: 2,
chainID: chainID3,
latestLayer: &Layer{
Expand Down Expand Up @@ -1278,17 +1281,19 @@ func TestFS(t *testing.T) {
wantErr bool
}{
{
name: "no chain layers",
image: &Image{},
name: "no chain layers",
image: &Image{
config: DefaultConfig(),
},
wantErr: true,
},
{
name: "single chain layer",
image: &Image{
chainLayers: []*chainLayer{
{
fileNodeTree: func() *Node {
root := NewNode()
fileNodeTree: func() *RootNode {
root := NewNode(DefaultMaxSymlinkDepth)
_ = root.Insert("/", &virtualFile{
virtualPath: "/",
isWhiteout: false,
Expand All @@ -1307,6 +1312,7 @@ func TestFS(t *testing.T) {
},
},
},
config: DefaultConfig(),
},
wantFilesFromFS: []string{"/foo.txt"},
},
Expand All @@ -1315,8 +1321,8 @@ func TestFS(t *testing.T) {
image: &Image{
chainLayers: []*chainLayer{
{
fileNodeTree: func() *Node {
root := NewNode()
fileNodeTree: func() *RootNode {
root := NewNode(DefaultMaxSymlinkDepth)
_ = root.Insert("/", &virtualFile{
virtualPath: "/",
isWhiteout: false,
Expand All @@ -1335,8 +1341,8 @@ func TestFS(t *testing.T) {
},
},
{
fileNodeTree: func() *Node {
root := NewNode()
fileNodeTree: func() *RootNode {
root := NewNode(DefaultMaxSymlinkDepth)
_ = root.Insert("/", &virtualFile{
virtualPath: "/",
isWhiteout: false,
Expand All @@ -1359,6 +1365,7 @@ func TestFS(t *testing.T) {
},
},
},
config: DefaultConfig(),
},
wantFilesFromFS: []string{"/foo.txt", "/bar.txt"},
},
Expand Down
Loading