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

Keep original FileInfo persisted on file.Metadata structs #1794

Merged
merged 4 commits into from
May 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
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ require (
github.com/CycloneDX/cyclonedx-go v0.7.1
github.com/Masterminds/sprig/v3 v3.2.3
github.com/anchore/go-logger v0.0.0-20220728155337-03b66a5207d8
github.com/anchore/stereoscope v0.0.0-20230412183729-8602f1afc574
github.com/anchore/stereoscope v0.0.0-20230508133058-5543439b749f
github.com/deitch/magic v0.0.0-20230404182410-1ff89d7342da
github.com/docker/docker v23.0.5+incompatible
github.com/go-git/go-billy/v5 v5.4.1
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,8 @@ github.com/anchore/go-version v1.2.2-0.20200701162849-18adb9c92b9b h1:e1bmaoJfZV
github.com/anchore/go-version v1.2.2-0.20200701162849-18adb9c92b9b/go.mod h1:Bkc+JYWjMCF8OyZ340IMSIi2Ebf3uwByOk6ho4wne1E=
github.com/anchore/packageurl-go v0.1.1-0.20230104203445-02e0a6721501 h1:AV7qjwMcM4r8wFhJq3jLRztew3ywIyPTRapl2T1s9o8=
github.com/anchore/packageurl-go v0.1.1-0.20230104203445-02e0a6721501/go.mod h1:Blo6OgJNiYF41ufcgHKkbCKF2MDOMlrqhXv/ij6ocR4=
github.com/anchore/stereoscope v0.0.0-20230412183729-8602f1afc574 h1:VFX+FD9EH6am+tfqwr1KeCAmabAknSJQX95aIY3QJJI=
github.com/anchore/stereoscope v0.0.0-20230412183729-8602f1afc574/go.mod h1:2GGFHkHry/xDlEQgBrVGcarq+z7Z6hLnHdyhcKB2lfQ=
github.com/anchore/stereoscope v0.0.0-20230508133058-5543439b749f h1:wiWDirrn2a4gT2TfFeGb5zqFjKoEy3Hx+K8u8lReHzY=
github.com/anchore/stereoscope v0.0.0-20230508133058-5543439b749f/go.mod h1:2GGFHkHry/xDlEQgBrVGcarq+z7Z6hLnHdyhcKB2lfQ=
github.com/andreyvit/diff v0.0.0-20170406064948-c7f18ee00883/go.mod h1:rCTlJbsFo29Kk6CurOXKm700vrz8f0KW0JNfpkRJY/8=
github.com/andybalholm/brotli v1.0.1/go.mod h1:loMXtMfwqflxFJPmdbJO0a3KNoPuLBgiu3qAvBg8x/Y=
github.com/andybalholm/brotli v1.0.4 h1:V7DdXeJtZscaqfNuAdSRuRFzuiKlHSC/Zh3zl9qY3JY=
Expand Down
2 changes: 1 addition & 1 deletion syft/file/contents_cataloger.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func (i *ContentsCataloger) Catalog(resolver source.FileResolver) (map[source.Co
return nil, err
}

if i.skipFilesAboveSizeInBytes > 0 && metadata.Size > i.skipFilesAboveSizeInBytes {
if i.skipFilesAboveSizeInBytes > 0 && metadata.Size() > i.skipFilesAboveSizeInBytes {
continue
}

Expand Down
60 changes: 40 additions & 20 deletions syft/file/metadata_cataloger_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"flag"
"os"
"testing"
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -52,21 +51,27 @@ func TestFileMetadataCataloger(t *testing.T) {
path: "/file-1.txt",
exists: true,
expected: source.FileMetadata{
FileInfo: file.ManualInfo{
NameValue: "file-1.txt",
ModeValue: 0644,
SizeValue: 7,
},
Path: "/file-1.txt",
Mode: 0644,
Type: file.TypeRegular,
UserID: 1,
GroupID: 2,
Size: 7,
MIMEType: "text/plain",
},
},
{
path: "/hardlink-1",
exists: true,
expected: source.FileMetadata{
FileInfo: file.ManualInfo{
NameValue: "hardlink-1",
ModeValue: 0644,
},
Path: "/hardlink-1",
Mode: 0644,
Type: file.TypeHardLink,
LinkDestination: "file-1.txt",
UserID: 1,
Expand All @@ -78,8 +83,11 @@ func TestFileMetadataCataloger(t *testing.T) {
path: "/symlink-1",
exists: true,
expected: source.FileMetadata{
Path: "/symlink-1",
Mode: 0777 | os.ModeSymlink,
Path: "/symlink-1",
FileInfo: file.ManualInfo{
NameValue: "symlink-1",
ModeValue: 0777 | os.ModeSymlink,
},
Type: file.TypeSymLink,
LinkDestination: "file-1.txt",
UserID: 0,
Expand All @@ -91,8 +99,11 @@ func TestFileMetadataCataloger(t *testing.T) {
path: "/char-device-1",
exists: true,
expected: source.FileMetadata{
Path: "/char-device-1",
Mode: 0644 | os.ModeDevice | os.ModeCharDevice,
Path: "/char-device-1",
FileInfo: file.ManualInfo{
NameValue: "char-device-1",
ModeValue: 0644 | os.ModeDevice | os.ModeCharDevice,
},
Type: file.TypeCharacterDevice,
UserID: 0,
GroupID: 0,
Expand All @@ -103,8 +114,11 @@ func TestFileMetadataCataloger(t *testing.T) {
path: "/block-device-1",
exists: true,
expected: source.FileMetadata{
Path: "/block-device-1",
Mode: 0644 | os.ModeDevice,
Path: "/block-device-1",
FileInfo: file.ManualInfo{
NameValue: "block-device-1",
ModeValue: 0644 | os.ModeDevice,
},
Type: file.TypeBlockDevice,
UserID: 0,
GroupID: 0,
Expand All @@ -115,8 +129,11 @@ func TestFileMetadataCataloger(t *testing.T) {
path: "/fifo-1",
exists: true,
expected: source.FileMetadata{
Path: "/fifo-1",
Mode: 0644 | os.ModeNamedPipe,
Path: "/fifo-1",
FileInfo: file.ManualInfo{
NameValue: "fifo-1",
ModeValue: 0644 | os.ModeNamedPipe,
},
Type: file.TypeFIFO,
UserID: 0,
GroupID: 0,
Expand All @@ -127,13 +144,15 @@ func TestFileMetadataCataloger(t *testing.T) {
path: "/bin",
exists: true,
expected: source.FileMetadata{
Path: "/bin",
Mode: 0755 | os.ModeDir,
Path: "/bin",
FileInfo: file.ManualInfo{
NameValue: "bin",
ModeValue: 0755 | os.ModeDir,
},
Type: file.TypeDirectory,
UserID: 0,
GroupID: 0,
MIMEType: "",
IsDir: true,
},
},
}
Expand All @@ -146,13 +165,14 @@ func TestFileMetadataCataloger(t *testing.T) {
l := source.NewLocationFromImage(test.path, *ref.Reference, img)

if _, ok := actual[l.Coordinates]; ok {
redact := actual[l.Coordinates]
redact.ModTime = time.Time{}
actual[l.Coordinates] = redact
// we're not interested in keeping the test fixtures up to date with the latest file modification times
// thus ModTime is not under test
fi := test.expected.FileInfo.(file.ManualInfo)
fi.ModTimeValue = actual[l.Coordinates].ModTime()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: maybe update the actual value here with the hardcoded one?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think I follow -- the comment above explicitly mentions this field isn't intended to be put under test. Are you saying the test should cover this?

test.expected.FileInfo = fi
}

assert.Equal(t, test.expected, actual[l.Coordinates], "mismatched metadata")

assert.True(t, test.expected.Equal(actual[l.Coordinates]))
})
}

Expand Down
4 changes: 2 additions & 2 deletions syft/file/secrets_cataloger.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,11 +71,11 @@ func (i *SecretsCataloger) catalogLocation(resolver source.FileResolver, locatio
return nil, err
}

if metadata.Size == 0 {
if metadata.Size() == 0 {
return nil, nil
}

if i.skipFilesAboveSize > 0 && metadata.Size > i.skipFilesAboveSize {
if i.skipFilesAboveSize > 0 && metadata.Size() > i.skipFilesAboveSize {
return nil, nil
}

Expand Down
20 changes: 16 additions & 4 deletions syft/formats/syftjson/encoder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,26 +103,38 @@ func TestEncodeFullJSONDocument(t *testing.T) {
Packages: catalog,
FileMetadata: map[source.Coordinates]source.FileMetadata{
source.NewLocation("/a/place").Coordinates: {
Mode: 0775,
FileInfo: stereoFile.ManualInfo{
NameValue: "/a/place",
ModeValue: 0775,
},
Type: stereoFile.TypeDirectory,
UserID: 0,
GroupID: 0,
},
source.NewLocation("/a/place/a").Coordinates: {
Mode: 0775,
FileInfo: stereoFile.ManualInfo{
NameValue: "/a/place/a",
ModeValue: 0775,
},
Type: stereoFile.TypeRegular,
UserID: 0,
GroupID: 0,
},
source.NewLocation("/b").Coordinates: {
Mode: 0775,
FileInfo: stereoFile.ManualInfo{
NameValue: "/b",
ModeValue: 0775,
},
Type: stereoFile.TypeSymLink,
LinkDestination: "/c",
UserID: 0,
GroupID: 0,
},
source.NewLocation("/b/place/b").Coordinates: {
Mode: 0644,
FileInfo: stereoFile.ManualInfo{
NameValue: "/b/place/b",
ModeValue: 0644,
},
Type: stereoFile.TypeRegular,
UserID: 1,
GroupID: 2,
Expand Down
18 changes: 13 additions & 5 deletions syft/formats/syftjson/to_format_model.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,10 +131,18 @@ func toFileMetadataEntry(coordinates source.Coordinates, metadata *source.FileMe
return nil
}

mode, err := strconv.Atoi(fmt.Sprintf("%o", metadata.Mode))
if err != nil {
log.Warnf("invalid mode found in file catalog @ location=%+v mode=%q: %+v", coordinates, metadata.Mode, err)
mode = 0
var mode int
var size int64
if metadata != nil && metadata.FileInfo != nil {
var err error

mode, err = strconv.Atoi(fmt.Sprintf("%o", metadata.Mode()))
if err != nil {
log.Warnf("invalid mode found in file catalog @ location=%+v mode=%q: %+v", coordinates, metadata.Mode, err)
mode = 0
}

size = metadata.Size()
}

return &model.FileMetadataEntry{
Expand All @@ -144,7 +152,7 @@ func toFileMetadataEntry(coordinates source.Coordinates, metadata *source.FileMe
UserID: metadata.UserID,
GroupID: metadata.GroupID,
MIMEType: metadata.MIMEType,
Size: metadata.Size,
Size: size,
}
}

Expand Down
42 changes: 42 additions & 0 deletions syft/formats/syftjson/to_format_model_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,3 +152,45 @@ func Test_toFileType(t *testing.T) {

assert.ElementsMatch(t, allTypesTested, file.AllTypes(), "not all file.Types are under test")
}

func Test_toFileMetadataEntry(t *testing.T) {
coords := source.Coordinates{
RealPath: "/path",
FileSystemID: "x",
}
tests := []struct {
name string
metadata *source.FileMetadata
want *model.FileMetadataEntry
}{
{
name: "no metadata",
},
{
name: "no file info",
metadata: &source.FileMetadata{
FileInfo: nil,
},
want: &model.FileMetadataEntry{
Type: file.TypeRegular.String(),
},
},
{
name: "with file info",
metadata: &source.FileMetadata{
FileInfo: &file.ManualInfo{
ModeValue: 1,
},
},
want: &model.FileMetadataEntry{
Mode: 1,
Type: file.TypeRegular.String(),
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
assert.Equal(t, tt.want, toFileMetadataEntry(coords, tt.metadata))
})
}
}
9 changes: 6 additions & 3 deletions syft/formats/syftjson/to_syft_model.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package syftjson

import (
"os"
"path"
"strconv"
"strings"

Expand Down Expand Up @@ -57,14 +58,16 @@ func toSyftFiles(files []model.File) sbom.Artifacts {
fm := os.FileMode(mode)

ret.FileMetadata[coord] = source.FileMetadata{
FileInfo: stereoscopeFile.ManualInfo{
NameValue: path.Base(coord.RealPath),
SizeValue: f.Metadata.Size,
ModeValue: fm,
},
Path: coord.RealPath,
LinkDestination: f.Metadata.LinkDestination,
Size: f.Metadata.Size,
UserID: f.Metadata.UserID,
GroupID: f.Metadata.GroupID,
Type: toSyftFileType(f.Metadata.Type),
IsDir: fm.IsDir(),
Mode: fm,
MIMEType: f.Metadata.MIMEType,
}
}
Expand Down
8 changes: 5 additions & 3 deletions syft/formats/syftjson/to_syft_model_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -200,14 +200,16 @@ func Test_toSyftFiles(t *testing.T) {
want: sbom.Artifacts{
FileMetadata: map[source.Coordinates]source.FileMetadata{
coord: {
FileInfo: stereoFile.ManualInfo{
NameValue: "place",
SizeValue: 92,
ModeValue: 511, // 777 octal = 511 decimal
},
Path: coord.RealPath,
LinkDestination: "",
Size: 92,
UserID: 42,
GroupID: 32,
Type: stereoFile.TypeRegular,
IsDir: false,
Mode: 511, // 777 octal = 511 decimal
MIMEType: "text/plain",
},
},
Expand Down
2 changes: 1 addition & 1 deletion syft/source/directory_indexer.go
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ func (r *directoryIndexer) disallowRevisitingVisitor(path string, _ os.FileInfo,
// - link destinations twice, once for the real file and another through the virtual path
// - infinite link cycles
if indexed, metadata := r.hasBeenIndexed(path); indexed {
if metadata.IsDir {
if metadata.IsDir() {
// signal to walk() that we should skip this directory entirely
return fs.SkipDir
}
Expand Down
4 changes: 2 additions & 2 deletions syft/source/directory_resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ func (r directoryResolver) FilesByPath(userPaths ...string) ([]Location, error)
}

// don't consider directories
if entry.Metadata.IsDir {
if entry.Metadata.IsDir() {
continue
}

Expand Down Expand Up @@ -238,7 +238,7 @@ func (r directoryResolver) FilesByGlob(patterns ...string) ([]Location, error) {
}

// don't consider directories
if entry.Metadata.IsDir {
if entry.Metadata.IsDir() {
continue
}

Expand Down
2 changes: 1 addition & 1 deletion syft/source/directory_resolver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -949,7 +949,7 @@ func TestDirectoryResolver_FilesContents_errorOnDirRequest(t *testing.T) {
for loc := range resolver.AllLocations() {
entry, err := resolver.index.Get(loc.ref)
require.NoError(t, err)
if entry.Metadata.IsDir {
if entry.Metadata.IsDir() {
dirLoc = &loc
break
}
Expand Down
4 changes: 2 additions & 2 deletions syft/source/image_all_layers_resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ func (r *imageAllLayersResolver) FilesByPath(paths ...string) ([]Location, error
if err != nil {
return nil, fmt.Errorf("unable to get file metadata for path=%q: %w", ref.RealPath, err)
}
if metadata.Metadata.IsDir {
if metadata.Metadata.IsDir() {
continue
}
}
Expand Down Expand Up @@ -143,7 +143,7 @@ func (r *imageAllLayersResolver) FilesByGlob(patterns ...string) ([]Location, er
return nil, fmt.Errorf("unable to get file metadata for path=%q: %w", result.RequestPath, err)
}
// don't consider directories
if metadata.Metadata.IsDir {
if metadata.Metadata.IsDir() {
continue
}
}
Expand Down
Loading