diff --git a/pkg/image/containerd/daemon_provider.go b/pkg/image/containerd/daemon_provider.go index 26b4476f..d0c1d20b 100644 --- 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 index 07e0b402..df06718a 100644 --- 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 index 4baab37b..d508241f 100644 --- 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 index ef30a56d..caecf267 100644 --- 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) + } + }) + } +} diff --git a/pkg/image/oci/registry_provider.go b/pkg/image/oci/registry_provider.go index 29f2f75f..e64bb895 100644 --- a/pkg/image/oci/registry_provider.go +++ b/pkg/image/oci/registry_provider.go @@ -69,6 +69,15 @@ func (p *registryImageProvider) Provide(ctx context.Context) (*image.Image, erro return nil, fmt.Errorf("failed to get image from registry: %+v", err) } + c, err := img.ConfigFile() + if err != nil { + return nil, fmt.Errorf("failed to get image config from registry: %+v", err) + } + + if err := validatePlatform(platform, c.OS, c.Architecture); err != nil { + return nil, err + } + // craft a repo digest from the registry reference and the known digest // note: the descriptor is fetched from the registry, and the descriptor digest is the same as the repo digest repoDigest := fmt.Sprintf("%s/%s@%s", ref.Context().RegistryStr(), ref.Context().RepositoryStr(), descriptor.Digest.String()) @@ -97,6 +106,33 @@ func (p *registryImageProvider) Provide(ctx context.Context) (*image.Image, erro return out, err } +func validatePlatform(platform *image.Platform, givenOs, givenArch string) error { + if platform == nil { + return nil + } + if givenArch == "" || givenOs == "" { + return newErrFetchingImage(fmt.Errorf("missing architecture or OS from image config when user specified platform=%q", platform.String())) + } + platformStr := fmt.Sprintf("%s/%s", givenOs, givenArch) + actualPlatform, err := containerregistryV1.ParsePlatform(platformStr) + if err != nil { + return newErrFetchingImage(fmt.Errorf("failed to parse platform from image config: %w", err)) + } + if actualPlatform == nil { + return newErrFetchingImage(fmt.Errorf("not platform from image config (from %q)", platformStr)) + } + if !matchesPlatform(*actualPlatform, *toContainerRegistryPlatform(platform)) { + return newErrFetchingImage(fmt.Errorf("image platform=%q does not match user specified platform=%q", actualPlatform.String(), platform.String())) + } + return nil +} + +func newErrFetchingImage(err error) *image.ErrFetchingImage { + return &image.ErrFetchingImage{ + Reason: err.Error(), + } +} + func prepareReferenceOptions(registryOptions image.RegistryOptions) []name.Option { var options []name.Option if registryOptions.InsecureUseHTTP { @@ -105,15 +141,22 @@ func prepareReferenceOptions(registryOptions image.RegistryOptions) []name.Optio return options } +func toContainerRegistryPlatform(p *image.Platform) *containerregistryV1.Platform { + if p == nil { + return nil + } + return &containerregistryV1.Platform{ + Architecture: p.Architecture, + OS: p.OS, + Variant: p.Variant, + } +} + func prepareRemoteOptions(ctx context.Context, ref name.Reference, registryOptions image.RegistryOptions, p *image.Platform) (options []remote.Option) { options = append(options, remote.WithContext(ctx)) if p != nil { - options = append(options, remote.WithPlatform(containerregistryV1.Platform{ - Architecture: p.Architecture, - OS: p.OS, - Variant: p.Variant, - })) + options = append(options, remote.WithPlatform(*toContainerRegistryPlatform(p))) } registryName := ref.Context().RegistryStr() @@ -166,3 +209,51 @@ func defaultPlatformIfNil(platform *image.Platform) *image.Platform { } return platform } + +// matchesPlatform checks if the given platform matches the required platforms. +// The given platform matches the required platform if +// - architecture and OS are identical. +// - OS version and variant are identical if provided. +// - features and OS features of the required platform are subsets of those of the given platform. +// note: this function was copied from the GGCR repo, as it is not exported. +func matchesPlatform(given, required containerregistryV1.Platform) bool { + // Required fields that must be identical. + if given.Architecture != required.Architecture || given.OS != required.OS { + return false + } + + // Optional fields that may be empty, but must be identical if provided. + if required.OSVersion != "" && given.OSVersion != required.OSVersion { + return false + } + if required.Variant != "" && given.Variant != required.Variant { + return false + } + + // Verify required platform's features are a subset of given platform's features. + if !isSubset(given.OSFeatures, required.OSFeatures) { + return false + } + if !isSubset(given.Features, required.Features) { + return false + } + + return true +} + +// isSubset checks if the required array of strings is a subset of the given lst. +// note: this function was copied from the GGCR repo, as it is not exported. +func isSubset(lst, required []string) bool { + set := make(map[string]bool) + for _, value := range lst { + set[value] = true + } + + for _, value := range required { + if _, ok := set[value]; !ok { + return false + } + } + + return true +} diff --git a/pkg/image/oci/registry_provider_test.go b/pkg/image/oci/registry_provider_test.go index 5df09cd6..811e7d38 100644 --- a/pkg/image/oci/registry_provider_test.go +++ b/pkg/image/oci/registry_provider_test.go @@ -6,6 +6,7 @@ import ( "net/http" "net/http/httptest" "reflect" + "runtime" "strings" "testing" @@ -13,6 +14,7 @@ import ( "github.com/google/go-cmp/cmp/cmpopts" "github.com/google/go-containerregistry/pkg/name" "github.com/google/go-containerregistry/pkg/registry" + "github.com/google/go-containerregistry/pkg/v1/mutate" "github.com/google/go-containerregistry/pkg/v1/random" "github.com/google/go-containerregistry/pkg/v1/remote" "github.com/stretchr/testify/assert" @@ -22,6 +24,78 @@ import ( "github.com/anchore/stereoscope/pkg/image" ) +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 + platform *image.Platform + givenOs string + givenArch string + expectedErrMsg string + expectedErr require.ErrorAssertionFunc + }{ + { + name: "nil platform", + platform: nil, + givenOs: "linux", + givenArch: "amd64", + expectedErr: require.NoError, + }, + { + name: "missing OS", + platform: &image.Platform{OS: "linux", Architecture: "amd64"}, + givenOs: "", + givenArch: "amd64", + expectedErr: isFetchError, + expectedErrMsg: "missing architecture or OS", + }, + { + name: "missing architecture", + platform: &image.Platform{OS: "linux", Architecture: "amd64"}, + givenOs: "linux", + givenArch: "", + expectedErr: isFetchError, + expectedErrMsg: "missing architecture or OS", + }, + { + name: "invalid platform string", + platform: &image.Platform{OS: "linux", Architecture: "amd64"}, + givenOs: "invalid/thing/place", + givenArch: "platform", + expectedErr: isFetchError, + expectedErrMsg: "failed to parse platform from image config", + }, + { + name: "mismatched platform", + platform: &image.Platform{OS: "linux", Architecture: "amd64"}, + givenOs: "windows", + givenArch: "arm64", + expectedErr: isFetchError, + expectedErrMsg: `image platform="windows/arm64" does not match user specified platform="linux/amd64"`, + }, + { + name: "matching platform", + platform: &image.Platform{OS: "linux", Architecture: "amd64"}, + givenOs: "linux", + givenArch: "amd64", + expectedErr: require.NoError, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validatePlatform(tt.platform, tt.givenOs, tt.givenArch) + tt.expectedErr(t, err) + if err != nil { + assert.ErrorContains(t, err, tt.expectedErrMsg) + } + }) + } +} + func Test_RegistryProvider(t *testing.T) { imageName := "my-image" imageTag := "the-tag" @@ -181,7 +255,18 @@ func pushRandomRegistryImage(t *testing.T, registryHost, repo, tag string) { repoTag := repo + ":" + tag - img, err := random.Image(1024, 2) + baseImg, err := random.Image(1024, 2) + require.NoError(t, err) + + cfg, err := baseImg.ConfigFile() + require.NoError(t, err) + + // match the default values that stereoscope uses + cfg.OS = "linux" + cfg.Architecture = runtime.GOARCH + + // update the image with the modified config with os/arch info + img, err := mutate.ConfigFile(baseImg, cfg) require.NoError(t, err) opts := []name.Option{name.Insecure, name.WithDefaultRegistry(registryHost)} diff --git a/pkg/image/provider.go b/pkg/image/provider.go index a3c70e2c..a94bb909 100644 --- a/pkg/image/provider.go +++ b/pkg/image/provider.go @@ -2,8 +2,22 @@ package image import ( "context" + "fmt" ) +// ErrFetchingImage is meant to be used when a provider has positively resolved the image but, while fetching the +// image, an error occurred. The goal is to differentiate between a provider that cannot resolve an image (thus +// if the caller has a set of providers, it can try another provider) and a provider that can resolve an image but +// there is an unresolvable problem (e.g. network error, mismatched architecture, etc... thus the caller should +// not try any further providers). +type ErrFetchingImage struct { + Reason string +} + +func (e *ErrFetchingImage) Error() string { + return fmt.Sprintf("error fetching image: %s", e.Reason) +} + // Provider is an abstraction for any object that provides image objects (e.g. the docker daemon API, a tar file of // an OCI image, podman varlink API, etc.). type Provider interface {