more strict rules around platform verification

Signed-off-by: Alex Goodman <[email protected]>
anchore · Jan 4, 2025 · 9c7e279 · 9c7e279
1 parent 6633b73
commit 9c7e279
Show file tree

Hide file tree

Showing 8 changed files with 389 additions and 23 deletions.
diff --git a/pkg/image/containerd/daemon_provider.go b/pkg/image/containerd/daemon_provider.go
@@ -341,37 +341,43 @@ func (p *daemonImageProvider) pullImageIfMissing(ctx context.Context, client *co
 		}
 	}
 
-	if err := p.validatePlatform(resolvedPlatform); err != nil {
+	if err := validatePlatform(p.platform, resolvedPlatform); err != nil {
 		return "", nil, fmt.Errorf("platform validation failed: %w", err)
 	}
 
 	return resolvedImage, resolvedPlatform, nil
 }
 
-func (p *daemonImageProvider) validatePlatform(platform *platforms.Platform) error {
-	if p.platform == nil {
+func validatePlatform(expected *image.Platform, given *platforms.Platform) error {
+	if expected == nil {
 		return nil
 	}
 
-	if platform == nil {
-		return fmt.Errorf("image has no platform information (might be a manifest list)")
+	if given == nil {
+		return newErrFetchingImage(fmt.Errorf("image has no platform information (might be a manifest list)"))
 	}
 
-	if platform.OS != p.platform.OS {
-		return fmt.Errorf("image has unexpected OS %q, which differs from the user specified PS %q", platform.OS, p.platform.OS)
+	if given.OS != expected.OS {
+		return newErrFetchingImage(fmt.Errorf("image has unexpected OS %q, which differs from the user specified PS %q", given.OS, expected.OS))
 	}
 
-	if platform.Architecture != p.platform.Architecture {
-		return fmt.Errorf("image has unexpected architecture %q, which differs from the user specified architecture %q", platform.Architecture, p.platform.Architecture)
+	if given.Architecture != expected.Architecture {
+		return newErrFetchingImage(fmt.Errorf("image has unexpected architecture %q, which differs from the user specified architecture %q", given.Architecture, expected.Architecture))
 	}
 
-	if platform.Variant != p.platform.Variant {
-		return fmt.Errorf("image has unexpected architecture %q, which differs from the user specified architecture %q", platform.Variant, p.platform.Variant)
+	if given.Variant != expected.Variant {
+		return newErrFetchingImage(fmt.Errorf("image has unexpected architecture %q, which differs from the user specified architecture %q", given.Variant, expected.Variant))
 	}
 
 	return nil
 }
 
+func newErrFetchingImage(err error) *image.ErrFetchingImage {
+	return &image.ErrFetchingImage{
+		Reason: err.Error(),
+	}
+}
+
 // save the image from the containerd daemon to a tar file
 func (p *daemonImageProvider) saveImage(ctx context.Context, client *containerd.Client, resolvedImage string) (string, error) {
 	imageTempDir, err := p.tmpDirGen.NewDirectory("containerd-daemon-image")

diff --git a/pkg/image/containerd/daemon_provider_test.go b/pkg/image/containerd/daemon_provider_test.go
@@ -94,3 +94,69 @@ func Test_exportPlatformComparer(t *testing.T) {
 		})
 	}
 }
+
+func TestValidatePlatform(t *testing.T) {
+	isFetchError := func(t require.TestingT, err error, args ...interface{}) {
+		var pErr *image.ErrFetchingImage
+		require.ErrorAs(t, err, &pErr)
+	}
+
+	tests := []struct {
+		name           string
+		expected       *image.Platform
+		given          *platforms.Platform
+		expectedErrMsg string
+		expectedErr    require.ErrorAssertionFunc
+	}{
+		{
+			name:        "nil expected platform",
+			expected:    nil,
+			given:       &platforms.Platform{OS: "linux", Architecture: "amd64"},
+			expectedErr: require.NoError,
+		},
+		{
+			name:           "nil given platform",
+			expected:       &image.Platform{OS: "linux", Architecture: "amd64"},
+			given:          nil,
+			expectedErr:    isFetchError,
+			expectedErrMsg: "image has no platform information",
+		},
+		{
+			name:           "OS mismatch",
+			expected:       &image.Platform{OS: "linux", Architecture: "amd64"},
+			given:          &platforms.Platform{OS: "windows", Architecture: "amd64"},
+			expectedErr:    isFetchError,
+			expectedErrMsg: `image has unexpected OS "windows", which differs from the user specified PS "linux"`,
+		},
+		{
+			name:           "architecture mismatch",
+			expected:       &image.Platform{OS: "linux", Architecture: "amd64"},
+			given:          &platforms.Platform{OS: "linux", Architecture: "arm64"},
+			expectedErr:    isFetchError,
+			expectedErrMsg: `image has unexpected architecture "arm64", which differs from the user specified architecture "amd64"`,
+		},
+		{
+			name:           "variant mismatch",
+			expected:       &image.Platform{OS: "linux", Architecture: "arm64", Variant: "v8"},
+			given:          &platforms.Platform{OS: "linux", Architecture: "arm64", Variant: "v7"},
+			expectedErr:    isFetchError,
+			expectedErrMsg: `image has unexpected architecture "v7", which differs from the user specified architecture "v8"`,
+		},
+		{
+			name:        "matching platform",
+			expected:    &image.Platform{OS: "linux", Architecture: "amd64", Variant: ""},
+			given:       &platforms.Platform{OS: "linux", Architecture: "amd64", Variant: ""},
+			expectedErr: require.NoError,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := validatePlatform(tt.expected, tt.given)
+			tt.expectedErr(t, err)
+			if err != nil {
+				assert.ErrorContains(t, err, tt.expectedErrMsg)
+			}
+		})
+	}
+}
diff --git a/pkg/image/docker/daemon_provider.go b/pkg/image/docker/daemon_provider.go
@@ -151,15 +151,30 @@ func (p *daemonImageProvider) pull(ctx context.Context, client client.APIClient,
 
 			return fmt.Errorf("failed to pull image: %w", err)
 		}
-
-		// check for the last two events indicating the pull is complete
-		if strings.HasPrefix(thePullEvent.Status, "Digest:") || strings.HasPrefix(thePullEvent.Status, "Status:") {
-			continue
+		if err := handlePullEvent(status, thePullEvent); err != nil {
+			return err
 		}
+	}
+
+	return nil
+}
 
-		status.onEvent(thePullEvent)
+type emitter interface {
+	onEvent(event *pullEvent)
+}
+
+func handlePullEvent(status emitter, event *pullEvent) error {
+	if event.Error != "" {
+		return &image.ErrFetchingImage{Reason: event.Error}
 	}
 
+	// check for the last two events indicating the pull is complete
+	if strings.HasPrefix(event.Status, "Digest:") || strings.HasPrefix(event.Status, "Status:") {
+		return nil
+	}
+
+	status.onEvent(event)
+
 	return nil
 }
 

diff --git a/pkg/image/docker/daemon_provider_test.go b/pkg/image/docker/daemon_provider_test.go
@@ -8,6 +8,8 @@ import (
 	configTypes "github.com/docker/cli/cli/config/types"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+
+	"github.com/anchore/stereoscope/pkg/image"
 )
 
 func TestEncodeCredentials(t *testing.T) {
@@ -85,3 +87,71 @@ func Test_authURL(t *testing.T) {
 		})
 	}
 }
+
+type mockEmitter struct {
+	calledEvents []*pullEvent
+}
+
+func (m *mockEmitter) onEvent(event *pullEvent) {
+	m.calledEvents = append(m.calledEvents, event)
+}
+
+func TestHandlePullEventWithMockEmitter(t *testing.T) {
+	tests := []struct {
+		name          string
+		event         *pullEvent
+		expectOnEvent bool
+		assertFunc    require.ErrorAssertionFunc
+	}{
+		{
+			name: "error in event",
+			event: &pullEvent{
+				Error: "fetch failed",
+			},
+			expectOnEvent: false,
+			assertFunc: func(t require.TestingT, err error, args ...interface{}) {
+				var pErr *image.ErrFetchingImage
+				require.ErrorAs(t, err, &pErr)
+			},
+		},
+		{
+			name: "digest event",
+			event: &pullEvent{
+				Status: "Digest: abc123",
+			},
+			expectOnEvent: false,
+			assertFunc:    require.NoError,
+		},
+		{
+			name: "status event",
+			event: &pullEvent{
+				Status: "Status: Downloaded",
+			},
+			expectOnEvent: false,
+			assertFunc:    require.NoError,
+		},
+		{
+			name: "non-terminal event",
+			event: &pullEvent{
+				Status: "Downloading layer",
+			},
+			expectOnEvent: true,
+			assertFunc:    require.NoError,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			mock := &mockEmitter{}
+			err := handlePullEvent(mock, tt.event)
+			tt.assertFunc(t, err)
+
+			if tt.expectOnEvent {
+				require.Len(t, mock.calledEvents, 1)
+				require.Equal(t, tt.event, mock.calledEvents[0])
+			} else {
+				require.Empty(t, mock.calledEvents)
+			}
+		})
+	}
+}