Skip to content

Commit e394cf5

Browse files
wagoodmanwagoodman
authored
Add more strict verifications when user provides a platform (#336)
* more strict rules around platform verification Signed-off-by: Alex Goodman <[email protected]> * raise up platform mismatch error Signed-off-by: Alex Goodman <[email protected]> --------- Signed-off-by: Alex Goodman <[email protected]>
1 parent 86fec8e commit e394cf5

9 files changed

+414
-22
lines changed

‎.gitignore

+1
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ bin/
1313
/snapshot
1414
/.tool
1515
/.task
16+
.mise.toml
1617

1718
# changelog generation
1819
CHANGELOG.md

‎pkg/image/containerd/daemon_provider.go

+18-11
Original file line numberDiff line numberDiff line change
@@ -341,37 +341,44 @@ func (p *daemonImageProvider) pullImageIfMissing(ctx context.Context, client *co
341341
}
342342
}
343343

344-
if err := p.validatePlatform(resolvedPlatform); err != nil {
344+
if err := validatePlatform(p.platform, resolvedPlatform); err != nil {
345345
return "", nil, fmt.Errorf("platform validation failed: %w", err)
346346
}
347347

348348
return resolvedImage, resolvedPlatform, nil
349349
}
350350

351-
func (p *daemonImageProvider) validatePlatform(platform *platforms.Platform) error {
352-
if p.platform == nil {
351+
func validatePlatform(expected *image.Platform, given *platforms.Platform) error {
352+
if expected == nil {
353353
return nil
354354
}
355355

356-
if platform == nil {
357-
return fmt.Errorf("image has no platform information (might be a manifest list)")
356+
if given == nil {
357+
return newErrPlatformMismatch(expected, fmt.Errorf("image has no platform information (might be a manifest list)"))
358358
}
359359

360-
if platform.OS != p.platform.OS {
361-
return fmt.Errorf("image has unexpected OS %q, which differs from the user specified PS %q", platform.OS, p.platform.OS)
360+
if given.OS != expected.OS {
361+
return newErrPlatformMismatch(expected, fmt.Errorf("image has unexpected OS %q, which differs from the user specified PS %q", given.OS, expected.OS))
362362
}
363363

364-
if platform.Architecture != p.platform.Architecture {
365-
return fmt.Errorf("image has unexpected architecture %q, which differs from the user specified architecture %q", platform.Architecture, p.platform.Architecture)
364+
if given.Architecture != expected.Architecture {
365+
return newErrPlatformMismatch(expected, fmt.Errorf("image has unexpected architecture %q, which differs from the user specified architecture %q", given.Architecture, expected.Architecture))
366366
}
367367

368-
if platform.Variant != p.platform.Variant {
369-
return fmt.Errorf("image has unexpected architecture %q, which differs from the user specified architecture %q", platform.Variant, p.platform.Variant)
368+
if given.Variant != expected.Variant {
369+
return newErrPlatformMismatch(expected, fmt.Errorf("image has unexpected architecture %q, which differs from the user specified architecture %q", given.Variant, expected.Variant))
370370
}
371371

372372
return nil
373373
}
374374

375+
func newErrPlatformMismatch(expected *image.Platform, err error) *image.ErrPlatformMismatch {
376+
return &image.ErrPlatformMismatch{
377+
ExpectedPlatform: expected.String(),
378+
Err: err,
379+
}
380+
}
381+
375382
// save the image from the containerd daemon to a tar file
376383
func (p *daemonImageProvider) saveImage(ctx context.Context, client *containerd.Client, resolvedImage string) (string, error) {
377384
imageTempDir, err := p.tmpDirGen.NewDirectory("containerd-daemon-image")

‎pkg/image/containerd/daemon_provider_test.go

+66
Original file line numberDiff line numberDiff line change
@@ -94,3 +94,69 @@ func Test_exportPlatformComparer(t *testing.T) {
9494
})
9595
}
9696
}
97+
98+
func TestValidatePlatform(t *testing.T) {
99+
isFetchError := func(t require.TestingT, err error, args ...interface{}) {
100+
var pErr *image.ErrPlatformMismatch
101+
require.ErrorAs(t, err, &pErr)
102+
}
103+
104+
tests := []struct {
105+
name string
106+
expected *image.Platform
107+
given *platforms.Platform
108+
expectedErrMsg string
109+
expectedErr require.ErrorAssertionFunc
110+
}{
111+
{
112+
name: "nil expected platform",
113+
expected: nil,
114+
given: &platforms.Platform{OS: "linux", Architecture: "amd64"},
115+
expectedErr: require.NoError,
116+
},
117+
{
118+
name: "nil given platform",
119+
expected: &image.Platform{OS: "linux", Architecture: "amd64"},
120+
given: nil,
121+
expectedErr: isFetchError,
122+
expectedErrMsg: "image has no platform information",
123+
},
124+
{
125+
name: "OS mismatch",
126+
expected: &image.Platform{OS: "linux", Architecture: "amd64"},
127+
given: &platforms.Platform{OS: "windows", Architecture: "amd64"},
128+
expectedErr: isFetchError,
129+
expectedErrMsg: `image has unexpected OS "windows", which differs from the user specified PS "linux"`,
130+
},
131+
{
132+
name: "architecture mismatch",
133+
expected: &image.Platform{OS: "linux", Architecture: "amd64"},
134+
given: &platforms.Platform{OS: "linux", Architecture: "arm64"},
135+
expectedErr: isFetchError,
136+
expectedErrMsg: `image has unexpected architecture "arm64", which differs from the user specified architecture "amd64"`,
137+
},
138+
{
139+
name: "variant mismatch",
140+
expected: &image.Platform{OS: "linux", Architecture: "arm64", Variant: "v8"},
141+
given: &platforms.Platform{OS: "linux", Architecture: "arm64", Variant: "v7"},
142+
expectedErr: isFetchError,
143+
expectedErrMsg: `image has unexpected architecture "v7", which differs from the user specified architecture "v8"`,
144+
},
145+
{
146+
name: "matching platform",
147+
expected: &image.Platform{OS: "linux", Architecture: "amd64", Variant: ""},
148+
given: &platforms.Platform{OS: "linux", Architecture: "amd64", Variant: ""},
149+
expectedErr: require.NoError,
150+
},
151+
}
152+
153+
for _, tt := range tests {
154+
t.Run(tt.name, func(t *testing.T) {
155+
err := validatePlatform(tt.expected, tt.given)
156+
tt.expectedErr(t, err)
157+
if err != nil {
158+
assert.ErrorContains(t, err, tt.expectedErrMsg)
159+
}
160+
})
161+
}
162+
}

‎pkg/image/docker/daemon_provider.go

+24-4
Original file line numberDiff line numberDiff line change
@@ -151,15 +151,35 @@ func (p *daemonImageProvider) pull(ctx context.Context, client client.APIClient,
151151

152152
return fmt.Errorf("failed to pull image: %w", err)
153153
}
154+
if err := handlePullEvent(status, thePullEvent); err != nil {
155+
return err
156+
}
157+
}
158+
159+
return nil
160+
}
154161

155-
// check for the last two events indicating the pull is complete
156-
if strings.HasPrefix(thePullEvent.Status, "Digest:") || strings.HasPrefix(thePullEvent.Status, "Status:") {
157-
continue
162+
type emitter interface {
163+
onEvent(event *pullEvent)
164+
}
165+
166+
func handlePullEvent(status emitter, event *pullEvent) error {
167+
if event.Error != "" {
168+
if strings.Contains(event.Error, "does not match the specified platform") {
169+
return &image.ErrPlatformMismatch{
170+
Err: errors.New(event.Error),
171+
}
158172
}
173+
return errors.New(event.Error)
174+
}
159175

160-
status.onEvent(thePullEvent)
176+
// check for the last two events indicating the pull is complete
177+
if strings.HasPrefix(event.Status, "Digest:") || strings.HasPrefix(event.Status, "Status:") {
178+
return nil
161179
}
162180

181+
status.onEvent(event)
182+
163183
return nil
164184
}
165185

‎pkg/image/docker/daemon_provider_test.go

+83
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ import (
88
configTypes "github.com/docker/cli/cli/config/types"
99
"github.com/stretchr/testify/assert"
1010
"github.com/stretchr/testify/require"
11+
12+
"github.com/anchore/stereoscope/pkg/image"
1113
)
1214

1315
func TestEncodeCredentials(t *testing.T) {
@@ -85,3 +87,84 @@ func Test_authURL(t *testing.T) {
8587
})
8688
}
8789
}
90+
91+
type mockEmitter struct {
92+
calledEvents []*pullEvent
93+
}
94+
95+
func (m *mockEmitter) onEvent(event *pullEvent) {
96+
m.calledEvents = append(m.calledEvents, event)
97+
}
98+
99+
func TestHandlePullEventWithMockEmitter(t *testing.T) {
100+
tests := []struct {
101+
name string
102+
event *pullEvent
103+
expectOnEvent bool
104+
assertFunc require.ErrorAssertionFunc
105+
}{
106+
{
107+
name: "error in event",
108+
event: &pullEvent{
109+
Error: "fetch failed",
110+
},
111+
expectOnEvent: false,
112+
assertFunc: func(t require.TestingT, err error, args ...interface{}) {
113+
require.Error(t, err)
114+
var pErr *image.ErrPlatformMismatch
115+
require.NotErrorAs(t, err, &pErr)
116+
},
117+
},
118+
{
119+
name: "platform error in event",
120+
event: &pullEvent{
121+
Error: "image with reference anchore/test_images:golang was found but its platform (linux/amd64) does not match the specified platform (linux/arm64)",
122+
},
123+
expectOnEvent: false,
124+
assertFunc: func(t require.TestingT, err error, args ...interface{}) {
125+
require.Error(t, err)
126+
var pErr *image.ErrPlatformMismatch
127+
require.ErrorAs(t, err, &pErr)
128+
},
129+
},
130+
{
131+
name: "digest event",
132+
event: &pullEvent{
133+
Status: "Digest: abc123",
134+
},
135+
expectOnEvent: false,
136+
assertFunc: require.NoError,
137+
},
138+
{
139+
name: "status event",
140+
event: &pullEvent{
141+
Status: "Status: Downloaded",
142+
},
143+
expectOnEvent: false,
144+
assertFunc: require.NoError,
145+
},
146+
{
147+
name: "non-terminal event",
148+
event: &pullEvent{
149+
Status: "Downloading layer",
150+
},
151+
expectOnEvent: true,
152+
assertFunc: require.NoError,
153+
},
154+
}
155+
156+
for _, tt := range tests {
157+
t.Run(tt.name, func(t *testing.T) {
158+
mock := &mockEmitter{}
159+
err := handlePullEvent(mock, tt.event)
160+
tt.assertFunc(t, err)
161+
162+
if tt.expectOnEvent {
163+
require.Len(t, mock.calledEvents, 1)
164+
require.Equal(t, tt.event, mock.calledEvents[0])
165+
} else {
166+
require.Empty(t, mock.calledEvents)
167+
}
168+
})
169+
}
170+
}

0 commit comments

Comments
 (0)