Skip to content
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.

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

Fix duplicate IDB entries #990

Merged
merged 1 commit into from
Dec 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 1 addition & 3 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ module chainguard.dev/apko
go 1.21

require (
github.com/chainguard-dev/go-apk v0.0.0-20231120201550-7b08e8f3b0fc
github.com/chainguard-dev/go-apk v0.0.0-20231218235333-2acefacd5846
github.com/dominodatalab/os-release v0.0.0-20190522011736-bcdb4a3e3c2f
github.com/go-git/go-git/v5 v5.10.0
github.com/google/go-cmp v0.6.0
Expand Down Expand Up @@ -111,5 +111,3 @@ require (
gopkg.in/warnings.v0 v0.1.2 // indirect
gotest.tools/v3 v3.5.1 // indirect
)

replace github.com/chainguard-dev/go-apk => github.com/chainguard-dev/go-apk v0.0.0-20231212142345-85b9cc9d5ea2
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ github.com/buger/jsonparser v1.1.1/go.mod h1:6RYKKt7H4d4+iWqouImQ9R2FZql3VbhNgx2
github.com/bwesterb/go-ristretto v1.2.3/go.mod h1:fUIoIZaG73pV5biE2Blr2xEzDoMj7NFEuV9ekS419A0=
github.com/cespare/xxhash/v2 v2.2.0 h1:DC2CZ1Ep5Y4k3ZQ899DldepgrayRUGE6BBZ/cd9Cj44=
github.com/cespare/xxhash/v2 v2.2.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs=
github.com/chainguard-dev/go-apk v0.0.0-20231212142345-85b9cc9d5ea2 h1:dAYjSiV4nzMm6jpmA6j2fB684l7m5Fx+ux1Y4GkozjE=
github.com/chainguard-dev/go-apk v0.0.0-20231212142345-85b9cc9d5ea2/go.mod h1:y0BbOQALsoi1T2Lt5KmFNn92G+fRFSUuogQI2171HS8=
github.com/chainguard-dev/go-apk v0.0.0-20231218235333-2acefacd5846 h1:5IIBDHIW3qkVVYtwOK6Lsx/loxl+2lHXisPCKGrwBk8=
github.com/chainguard-dev/go-apk v0.0.0-20231218235333-2acefacd5846/go.mod h1:y0BbOQALsoi1T2Lt5KmFNn92G+fRFSUuogQI2171HS8=
github.com/cloudflare/circl v1.3.3/go.mod h1:5XYMA4rFBvNIrhs50XuiBJ15vF2pZn4nnUKZrLbUZFA=
github.com/cloudflare/circl v1.3.5 h1:g+wWynZqVALYAlpSQFAa7TscDnUK8mKYtrxMpw6AUKo=
github.com/cloudflare/circl v1.3.5/go.mod h1:5XYMA4rFBvNIrhs50XuiBJ15vF2pZn4nnUKZrLbUZFA=
Expand Down
54 changes: 29 additions & 25 deletions pkg/tarfs/fs.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ const (
xattrTarPAXRecordsPrefix = "SCHILY.xattr."
)

var _ apk.WriteHeaderer = (*memFS)(nil)

type tarEntry struct {
tfs fs.FS
header tar.Header
Expand Down Expand Up @@ -100,20 +102,20 @@ func checksumFromHeader(header *tar.Header) ([]byte, error) {
return checksum, nil
}

func (m *memFS) WriteHeader(hdr tar.Header, tfs fs.FS, pkg *apk.Package) error {
func (m *memFS) WriteHeader(hdr tar.Header, tfs fs.FS, pkg *apk.Package) (bool, error) {
switch hdr.Typeflag {
case tar.TypeDir:
// special case, if the target already exists, and it is a symlink to a directory, we can accept it as is
// otherwise, we need to create the directory.
if fi, err := m.Stat(hdr.Name); err == nil && fi.Mode()&os.ModeSymlink != 0 {
if target, err := m.Readlink(hdr.Name); err == nil {
if fi, err = m.Stat(target); err == nil && fi.IsDir() {
return nil
return false, nil
}
}
}
if err := m.MkdirAll(hdr.Name, hdr.FileInfo().Mode().Perm()); err != nil {
return fmt.Errorf("error creating directory %s: %w", hdr.Name, err)
return false, fmt.Errorf("error creating directory %s: %w", hdr.Name, err)
}

for k, v := range hdr.PAXRecords {
Expand All @@ -122,19 +124,19 @@ func (m *memFS) WriteHeader(hdr tar.Header, tfs fs.FS, pkg *apk.Package) error {
}
attrName := strings.TrimPrefix(k, xattrTarPAXRecordsPrefix)
if err := m.SetXattr(hdr.Name, attrName, []byte(v)); err != nil {
return fmt.Errorf("error setting xattr %s on %s: %w", attrName, hdr.Name, err)
return false, fmt.Errorf("error setting xattr %s on %s: %w", attrName, hdr.Name, err)
}
}

case tar.TypeReg:
// We trust this because we verify it earlier in ExpandAPK.
checksum, err := checksumFromHeader(&hdr)
if err != nil {
return err
return false, err
}

if checksum == nil {
return fmt.Errorf("checksum is nil for %s", hdr.Name)
return false, fmt.Errorf("checksum is nil for %s", hdr.Name)
}

te := tarEntry{
Expand All @@ -144,8 +146,9 @@ func (m *memFS) WriteHeader(hdr tar.Header, tfs fs.FS, pkg *apk.Package) error {
pkg: pkg,
}

if err := m.writeHeader(hdr.Name, te); err != nil {
return fmt.Errorf("writing header for %q: %w", hdr.Name, err)
installed, err := m.writeHeader(hdr.Name, te)
if err != nil {
return false, fmt.Errorf("writing header for %q: %w", hdr.Name, err)
}

for k, v := range hdr.PAXRecords {
Expand All @@ -154,29 +157,30 @@ func (m *memFS) WriteHeader(hdr tar.Header, tfs fs.FS, pkg *apk.Package) error {
}
attrName := strings.TrimPrefix(k, xattrTarPAXRecordsPrefix)
if err := m.SetXattr(hdr.Name, attrName, []byte(v)); err != nil {
return fmt.Errorf("error setting xattr %s on %s: %w", attrName, hdr.Name, err)
return false, fmt.Errorf("error setting xattr %s on %s: %w", attrName, hdr.Name, err)
}
}

return installed, nil
case tar.TypeSymlink:
// some underlying filesystems and some memfs that we use in tests do not support symlinks.
// attempt it, and if it fails, just copy it.
// if it already exists, pointing to the same target, we can ignore it
if target, err := m.Readlink(hdr.Name); err == nil && target == hdr.Linkname {
return nil
return false, nil
}
if err := m.Symlink(hdr.Linkname, hdr.Name); err != nil {
return fmt.Errorf("unable to install symlink from %s -> %s: %w", hdr.Name, hdr.Linkname, err)
return false, fmt.Errorf("unable to install symlink from %s -> %s: %w", hdr.Name, hdr.Linkname, err)
}
case tar.TypeLink:
if err := m.Link(hdr.Linkname, hdr.Name); err != nil {
return err
return false, err
}
default:
return fmt.Errorf("unsupported file type %s %v", hdr.Name, hdr.Typeflag)
return false, fmt.Errorf("unsupported file type %s %v", hdr.Name, hdr.Typeflag)
}

return nil
return true, nil
}

// getNode returns the node for the given path. If the path is not found, it
Expand Down Expand Up @@ -431,17 +435,17 @@ func (m *memFS) openFile(name string, flag int, perm fs.FileMode, linkCount int)
return newMemFile(anode, name, m, flag), nil
}

func (m *memFS) writeHeader(name string, te tarEntry) error {
func (m *memFS) writeHeader(name string, te tarEntry) (bool, error) {
parent := filepath.Dir(name)
base := filepath.Base(name)

parentAnode, err := m.getNode(parent)
if err != nil {
return err
return false, err
}

if !parentAnode.dir {
return fmt.Errorf("parent is not a directory")
return false, fmt.Errorf("parent is not a directory")
}
if parentAnode.children == nil {
parentAnode.children = map[string]*node{}
Expand All @@ -461,14 +465,14 @@ func (m *memFS) writeHeader(name string, te tarEntry) error {
te: &te,
}
parentAnode.children[base] = anode
return nil
return true, nil
}

want, got := te, existing.te

if got == nil {
if existing.data == nil {
return fmt.Errorf("conflicting file for %q has no tar entry", name)
return false, fmt.Errorf("conflicting file for %q has no tar entry", name)
}

// This can happen when go-apk's InitKeyring conflicts with alpine-keys.
Expand All @@ -479,21 +483,21 @@ func (m *memFS) writeHeader(name string, te tarEntry) error {
checksum := h.Sum(nil)

if bytes.Equal(want.checksum, checksum) {
return nil
return false, nil
}

return fmt.Errorf("conflicting file for %q with checksum %x, existing has checksum %x", name, want.checksum, checksum)
return false, fmt.Errorf("conflicting file for %q with checksum %x, existing has checksum %x", name, want.checksum, checksum)
}

// Files have the same checksum, that's fine.
if bytes.Equal(got.checksum, want.checksum) {
return nil
return false, nil
}

// If the existing file's package replaces the package we want to install, we don't need to write this file.
for _, replace := range got.pkg.Replaces {
if want.pkg.Name == replace {
return nil
return false, nil
}
}

Expand All @@ -511,7 +515,7 @@ func (m *memFS) writeHeader(name string, te tarEntry) error {

// At this point we know the files conflict, but it's okay if this file replaces that one.
if !sameOrigin && !replaces {
return fmt.Errorf("conflicting file %q in %q has different origin %q != %q in %q", name, got.pkg.Name, got.pkg.Origin, want.pkg.Origin, want.pkg.Name)
return false, fmt.Errorf("conflicting file %q in %q has different origin %q != %q in %q", name, got.pkg.Name, got.pkg.Origin, want.pkg.Origin, want.pkg.Name)
}

anode := &node{
Expand All @@ -526,7 +530,7 @@ func (m *memFS) writeHeader(name string, te tarEntry) error {
parentAnode.children[base] = anode

// If we got here, they're different, but want replaces got, so it's all cool.
return nil
return true, nil
}

func (m *memFS) ReadFile(name string) ([]byte, error) {
Expand Down
8 changes: 4 additions & 4 deletions pkg/tarfs/fs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,27 +80,27 @@ func TestTarFS(t *testing.T) {
}
file.Typeflag = tar.TypeReg

if err := tfs.WriteHeader(*file, tfs, &pkg.Package); err == nil {
if _, err := tfs.WriteHeader(*file, tfs, &pkg.Package); err == nil {
t.Errorf("wanted missing checksum err, got nil")
}

file.PAXRecords = map[string]string{
"APK-TOOLS.checksum.SHA1": "Q1v+13wxZjoZUgI11oT2c7+ZUPjgw=",
}
if err := tfs.WriteHeader(*file, tfs, &pkg.Package); err != nil {
if _, err := tfs.WriteHeader(*file, tfs, &pkg.Package); err != nil {
t.Errorf("matching checksum should be skipped, got %v", err)
}

file.PAXRecords = map[string]string{
"APK-TOOLS.checksum.SHA1": "Q1v+12wxZjoZUgI11oT2c7+ZUPjgw=",
}
pkg.Origin += "-different"
if err := tfs.WriteHeader(*file, tfs, &pkg.Package); err == nil {
if _, err := tfs.WriteHeader(*file, tfs, &pkg.Package); err == nil {
t.Errorf("wanted conflicting checksum err, got nil")
}

pkg.Replaces = []string{pkg.Name}
if err := tfs.WriteHeader(*file, tfs, &pkg.Package); err != nil {
if _, err := tfs.WriteHeader(*file, tfs, &pkg.Package); err != nil {
t.Errorf("pkg replaces file, got %v", err)
}
}
Loading