Kaniko: Question: Unneeded Code?

[cvgw]

https://github.com/GoogleContainerTools/kaniko/pull/693#issue-285063736

What is the meaning of this code? Is it a waste code?

for p := range l {
    if strings.HasPrefix(filepath.Base(p), ".wh.") {
        delete(paths, p)
    }
    paths[p] = struct{}{}
}

cc @orisano

[cvgw]

from https://github.com/GoogleContainerTools/kaniko/pull/693

[priyawadhwa]

so this code is storing all existing files in paths. Any file that has a .wh. prefix is a whiteout file, and indicates that that file should be deleted from the filesystem (it may have existed in an earlier layer & been extracted, so this is how we keep track of deleted files)

There's more info on whiteout files here, for reference!

[cvgw]

@orisano is there anything else we can help clarify about this code?

[orisano]

I read the image/spec already.
I believe that the below code is correct.

for p := range l {
    if strings.HasPrefix(filepath.Base(p), ".wh.") {
        base := filepath.Base(p)
        x := p[:len(p)-len(base)] + strings.TrimPrefix(base, ".wh.")
        delete(paths, x)
    }
    paths[p] = struct{}{}
}

please tell me why the original code delete paths entry, then immediately add paths entry.

[cvgw]

@priyawadhwa I'm not understanding what the code does either.
https://github.com/GoogleContainerTools/kaniko/blob/3422d5572af36c70c7875e4861703cb1c8588548/pkg/snapshot/layered_map.go#L65

Take the following example
```
l.layers == []map[string]string{
map[string]string{
"/tmp/foo/bar": "....",
},
map[string]string{
"/tmp/foo/.wh.bar": "....",
},
}

......

for _, l := range l.layers {
// p == /tmp/foo/bar
// p == /tmp/foo/.wh.bar
for p := range l {
// filepath.Base(p) == bar
// filepath.Base(p) == .wh.bar
if strings.HasPrefix(filepath.Base(p), ".wh.") {
// p == /tmp/foo/.wh.bar
delete(paths, p)
}
// p == /tmp/foo/bar
// p == /tmp/foo/.wh.bar
paths[p] = struct{}{}
}
}
````

Why is it required to delete /tmp/foo/.wh.bar from paths and then add /tmp/foo/.wh.bar to paths?

Apologies if I am misunderstanding the code.

[priyawadhwa]

This looks like a bug! I think the correct code would be:

    paths := map[string]struct{}{}
    for _, l := range l.layers {
        for p := range l {
            if strings.HasPrefix(filepath.Base(p), ".wh.") {
                delete(paths, strings.TrimPrefix(p, ".wh."))
                                continue
            }
            paths[p] = struct{}{}
        }
    }
    return paths

so if we find a whiteout file we remove the file it refers to from paths, otherwise we add it.

[cvgw]

Thanks @priyawadhwa! That makes sense to me; though I'd suggest a slight change

    paths := map[string]struct{}{}
    for _, l := range l.layers {
        for p := range l {
                        base := filepath.Base(p)
            if strings.HasPrefix(base, ".wh.") {
                                base = strings.TrimPrefix(base, ".wh.")
                                delete(paths, filepath.Join(filepath.Dir(p), base))
                                continue
            }
            paths[p] = struct{}{}
        }
    }
    return paths