diff --git a/artifact/image/layerscanning/image/image.go b/artifact/image/layerscanning/image/image.go index 90fbb584..6b0dfc33 100644 --- a/artifact/image/layerscanning/image/image.go +++ b/artifact/image/layerscanning/image/image.go @@ -282,32 +282,40 @@ func FromV1Image(v1Image v1.Image, config *Config) (*Image, error) { // initializeChainLayers initializes the chain layers based on the config file history, the // v1.Layers found in the image from the tarball, and the max symlink depth. func initializeChainLayers(v1Layers []v1.Layer, configFile *v1.ConfigFile, maxSymlinkDepth int) ([]*chainLayer, error) { - layerIndex := 0 - if configFile == nil { return nil, fmt.Errorf("config file is nil") } - chainLayers := make([]*chainLayer, 0, len(configFile.History)) + var chainLayers []*chainLayer + // v1LayerIndex tracks the next v1.Layer that should populated in a chain layer. This does not + // include empty layers. + v1LayerIndex := 0 + // historyIndex tracks the chain layer index including empty layers. + historyIndex := 0 + + // First loop through the history entries found in the config file. If the entry is an empty + // layer, then create an empty chain layer. Otherwise, convert the v1.Layer to a scalibr Layer + // and create a chain layer with it. for _, entry := range configFile.History { if entry.EmptyLayer { chainLayers = append(chainLayers, &chainLayer{ fileNodeTree: pathtree.NewNode[fileNode](), - index: layerIndex, + index: historyIndex, latestLayer: &Layer{ buildCommand: entry.CreatedBy, isEmpty: true, }, maxSymlinkDepth: maxSymlinkDepth, }) + historyIndex++ continue } - if layerIndex >= len(v1Layers) { + if v1LayerIndex >= len(v1Layers) { return nil, fmt.Errorf("config history contains more non-empty layers than expected (%d)", len(v1Layers)) } - nextNonEmptyLayer := v1Layers[layerIndex] + nextNonEmptyLayer := v1Layers[v1LayerIndex] layer, err := convertV1Layer(nextNonEmptyLayer, entry.CreatedBy, false) if err != nil { return nil, err @@ -315,27 +323,31 @@ func initializeChainLayers(v1Layers []v1.Layer, configFile *v1.ConfigFile, maxSy chainLayer := &chainLayer{ fileNodeTree: pathtree.NewNode[fileNode](), - index: layerIndex, + index: historyIndex, latestLayer: layer, maxSymlinkDepth: maxSymlinkDepth, } chainLayers = append(chainLayers, chainLayer) - layerIndex++ + historyIndex++ + v1LayerIndex++ } - for layerIndex < len(v1Layers) { - layer, err := convertV1Layer(v1Layers[layerIndex], "", false) + // If there are any remaining v1.Layers, then the history in the config file is missing entries. + // This can happen depending on the build process used to create an image. + for v1LayerIndex < len(v1Layers) { + layer, err := convertV1Layer(v1Layers[v1LayerIndex], "", false) if err != nil { return nil, err } chainLayers = append(chainLayers, &chainLayer{ fileNodeTree: pathtree.NewNode[fileNode](), - index: layerIndex, + index: historyIndex, latestLayer: layer, maxSymlinkDepth: maxSymlinkDepth, }) - layerIndex++ + v1LayerIndex++ + historyIndex++ } return chainLayers, nil diff --git a/artifact/image/layerscanning/image/image_test.go b/artifact/image/layerscanning/image/image_test.go index 0117db85..9cc6d0ee 100644 --- a/artifact/image/layerscanning/image/image_test.go +++ b/artifact/image/layerscanning/image/image_test.go @@ -24,9 +24,12 @@ import ( "testing" "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" v1 "github.com/google/go-containerregistry/pkg/v1" "github.com/google/go-containerregistry/pkg/v1/types" "github.com/google/osv-scalibr/artifact/image" + "github.com/google/osv-scalibr/artifact/image/layerscanning/testing/fakev1layer" + "github.com/google/osv-scalibr/artifact/image/pathtree" "github.com/google/osv-scalibr/artifact/image/require" ) @@ -549,11 +552,14 @@ func TestFromTarball(t *testing.T) { // Only defer call to CleanUp if the image was created successfully. defer gotImage.CleanUp() + // Make sure the expected files are in the chain layers. chainLayers, err := gotImage.ChainLayers() if err != nil { t.Fatalf("ChainLayers() returned error: %v", err) } + // If the number of chain layers does not match the number of expected chain layer entries, + // then there is no point in continuing the test. if len(chainLayers) != len(tc.wantChainLayerEntries) { t.Fatalf("ChainLayers() returned incorrect number of chain layers: got %d chain layers, want %d chain layers", len(chainLayers), len(tc.wantChainLayerEntries)) } @@ -657,3 +663,282 @@ func compareChainLayerEntries(t *testing.T, gotChainLayer image.ChainLayer, want }() } } + +func TestInitializeChainLayers(t *testing.T) { + fakeV1Layer1 := fakev1layer.New("123", "COPY ./foo.txt /foo.txt # buildkit", false, nil) + fakeV1Layer2 := fakev1layer.New("456", "COPY ./bar.txt /bar.txt # buildkit", false, nil) + fakeV1Layer3 := fakev1layer.New("789", "COPY ./baz.txt /baz.txt # buildkit", false, nil) + + tests := []struct { + name string + v1Layers []v1.Layer + configFile *v1.ConfigFile + maxSymlinkDepth int + want []*chainLayer + wantErr bool + }{ + { + name: "nil config file", + v1Layers: []v1.Layer{ + fakeV1Layer1, + }, + wantErr: true, + }, + { + name: "no config file history entries", + v1Layers: []v1.Layer{ + fakeV1Layer1, + }, + configFile: &v1.ConfigFile{ + History: []v1.History{}, + }, + want: []*chainLayer{ + { + fileNodeTree: pathtree.NewNode[fileNode](), + index: 0, + latestLayer: &Layer{ + diffID: "sha256:123", + v1Layer: fakeV1Layer1, + isEmpty: false, + }, + }, + }, + }, + { + name: "single non-empty layer with history entry", + v1Layers: []v1.Layer{ + fakeV1Layer1, + }, + configFile: &v1.ConfigFile{ + History: []v1.History{ + { + CreatedBy: "COPY ./foo.txt /foo.txt # buildkit", + }, + }, + }, + want: []*chainLayer{ + { + fileNodeTree: pathtree.NewNode[fileNode](), + index: 0, + latestLayer: &Layer{ + buildCommand: "COPY ./foo.txt /foo.txt # buildkit", + diffID: "sha256:123", + v1Layer: fakeV1Layer1, + isEmpty: false, + }, + }, + }, + }, + { + name: "multiple non-empty layer with history entries", + v1Layers: []v1.Layer{ + fakeV1Layer1, + fakeV1Layer2, + fakeV1Layer3, + }, + configFile: &v1.ConfigFile{ + History: []v1.History{ + { + CreatedBy: "COPY ./foo.txt /foo.txt # buildkit", + }, + { + CreatedBy: "COPY ./bar.txt /bar.txt # buildkit", + }, + { + CreatedBy: "COPY ./baz.txt /baz.txt # buildkit", + }, + }, + }, + want: []*chainLayer{ + { + fileNodeTree: pathtree.NewNode[fileNode](), + index: 0, + latestLayer: &Layer{ + buildCommand: "COPY ./foo.txt /foo.txt # buildkit", + diffID: "sha256:123", + v1Layer: fakeV1Layer1, + isEmpty: false, + }, + }, + { + fileNodeTree: pathtree.NewNode[fileNode](), + index: 1, + latestLayer: &Layer{ + buildCommand: "COPY ./bar.txt /bar.txt # buildkit", + diffID: "sha256:456", + v1Layer: fakeV1Layer2, + isEmpty: false, + }, + }, + { + fileNodeTree: pathtree.NewNode[fileNode](), + index: 2, + latestLayer: &Layer{ + buildCommand: "COPY ./baz.txt /baz.txt # buildkit", + diffID: "sha256:789", + v1Layer: fakeV1Layer3, + isEmpty: false, + }, + }, + }, + }, + { + name: "mix of filled and empty layers with history entries", + v1Layers: []v1.Layer{ + fakeV1Layer1, + fakeV1Layer2, + fakeV1Layer3, + }, + configFile: &v1.ConfigFile{ + History: []v1.History{ + { + CreatedBy: "COPY ./foo.txt /foo.txt # buildkit", + EmptyLayer: false, + }, + { + CreatedBy: "ENTRYPOINT [\"/bin/sh\"]", + EmptyLayer: true, + }, + { + CreatedBy: "COPY ./bar.txt /bar.txt # buildkit", + EmptyLayer: false, + }, + { + CreatedBy: "RANDOM DOCKER COMMAND", + EmptyLayer: true, + }, + { + CreatedBy: "COPY ./baz.txt /baz.txt # buildkit", + EmptyLayer: false, + }, + { + CreatedBy: "RUN [\"/bin/sh\"]", + EmptyLayer: true, + }, + }, + }, + want: []*chainLayer{ + { + fileNodeTree: pathtree.NewNode[fileNode](), + index: 0, + latestLayer: &Layer{ + buildCommand: "COPY ./foo.txt /foo.txt # buildkit", + diffID: "sha256:123", + v1Layer: fakeV1Layer1, + isEmpty: false, + }, + }, + { + fileNodeTree: pathtree.NewNode[fileNode](), + index: 1, + latestLayer: &Layer{ + buildCommand: "ENTRYPOINT [\"/bin/sh\"]", + isEmpty: true, + }, + }, + { + fileNodeTree: pathtree.NewNode[fileNode](), + index: 2, + latestLayer: &Layer{ + buildCommand: "COPY ./bar.txt /bar.txt # buildkit", + diffID: "sha256:456", + v1Layer: fakeV1Layer2, + isEmpty: false, + }, + }, + { + fileNodeTree: pathtree.NewNode[fileNode](), + index: 3, + latestLayer: &Layer{ + buildCommand: "RANDOM DOCKER COMMAND", + isEmpty: true, + }, + }, + { + fileNodeTree: pathtree.NewNode[fileNode](), + index: 4, + latestLayer: &Layer{ + buildCommand: "COPY ./baz.txt /baz.txt # buildkit", + diffID: "sha256:789", + v1Layer: fakeV1Layer3, + isEmpty: false, + }, + }, + { + fileNodeTree: pathtree.NewNode[fileNode](), + index: 5, + latestLayer: &Layer{ + buildCommand: "RUN [\"/bin/sh\"]", + isEmpty: true, + }, + }, + }, + }, + { + name: "more layers than history entries", + v1Layers: []v1.Layer{ + fakeV1Layer1, + fakeV1Layer2, + fakeV1Layer3, + }, + configFile: &v1.ConfigFile{ + History: []v1.History{ + { + CreatedBy: "COPY ./foo.txt /foo.txt # buildkit", + EmptyLayer: false, + }, + }, + }, + want: []*chainLayer{ + { + fileNodeTree: pathtree.NewNode[fileNode](), + index: 0, + latestLayer: &Layer{ + buildCommand: "COPY ./foo.txt /foo.txt # buildkit", + diffID: "sha256:123", + v1Layer: fakeV1Layer1, + isEmpty: false, + }, + }, + { + fileNodeTree: pathtree.NewNode[fileNode](), + index: 1, + latestLayer: &Layer{ + diffID: "sha256:456", + v1Layer: fakeV1Layer2, + isEmpty: false, + }, + }, + { + fileNodeTree: pathtree.NewNode[fileNode](), + index: 2, + latestLayer: &Layer{ + diffID: "sha256:789", + v1Layer: fakeV1Layer3, + isEmpty: false, + }, + }, + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + gotChainLayers, err := initializeChainLayers(tc.v1Layers, tc.configFile, tc.maxSymlinkDepth) + if tc.wantErr { + if err != nil { + return + } + t.Fatalf("initializeChainLayers(%v, %v, %v) returned nil error, want error", tc.v1Layers, tc.configFile, tc.maxSymlinkDepth) + } + + if err != nil { + t.Fatalf("initializeChainLayers(%v, %v, %v) returned an unexpected error: %v", tc.v1Layers, tc.configFile, tc.maxSymlinkDepth, err) + } + + if diff := cmp.Diff(tc.want, gotChainLayers, cmp.AllowUnexported(chainLayer{}, Layer{}, fakev1layer.FakeV1Layer{}), cmpopts.IgnoreFields(chainLayer{}, "fileNodeTree")); diff != "" { + t.Fatalf("initializeChainLayers(%v, %v, %v) returned an unexpected diff (-want +got): %v", tc.v1Layers, tc.configFile, tc.maxSymlinkDepth, diff) + } + }) + } +}