Skip to content

Commit

Permalink
fix: allow the specific image platform os/arch/variant to meet the re…
Browse files Browse the repository at this point in the history
…quirement for a broader image platform os/arch (#858)

Signed-off-by: Cheng Fang <[email protected]>
  • Loading branch information
chengfang authored Sep 6, 2024
1 parent 40f260a commit e1cc998
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 16 deletions.
18 changes: 14 additions & 4 deletions pkg/image/options_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"fmt"
"regexp"
"runtime"
"strings"
"testing"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -380,8 +381,17 @@ func Test_GetPlatformOptions(t *testing.T) {
opts := img.GetPlatformOptions(annotations, false)
os := runtime.GOOS
arch := runtime.GOARCH
assert.True(t, opts.WantsPlatform(os, arch, ""))
assert.False(t, opts.WantsPlatform(os, arch, "invalid"))
platform := opts.Platforms()[0]
slashCount := strings.Count(platform, "/")
if slashCount == 1 {
assert.True(t, opts.WantsPlatform(os, arch, ""))
assert.True(t, opts.WantsPlatform(os, arch, "invalid"))
} else if slashCount == 2 {
assert.False(t, opts.WantsPlatform(os, arch, ""))
assert.False(t, opts.WantsPlatform(os, arch, "invalid"))
} else {
t.Fatal("invalid platform options ", platform)
}
})
t.Run("Empty platform options without restriction", func(t *testing.T) {
annotations := map[string]string{}
Expand All @@ -396,7 +406,7 @@ func Test_GetPlatformOptions(t *testing.T) {
t.Run("Single platform without variant requested", func(t *testing.T) {
os := "linux"
arch := "arm64"
variant := ""
variant := "v8"
annotations := map[string]string{
fmt.Sprintf(common.PlatformsAnnotation, "dummy"): options.PlatformKey(os, arch, variant),
}
Expand Down Expand Up @@ -431,7 +441,7 @@ func Test_GetPlatformOptions(t *testing.T) {
assert.True(t, opts.WantsPlatform(os, arch, variant))
assert.True(t, opts.WantsPlatform(runtime.GOOS, runtime.GOARCH, ""))
assert.False(t, opts.WantsPlatform(os, arch, ""))
assert.False(t, opts.WantsPlatform(runtime.GOOS, runtime.GOARCH, variant))
assert.True(t, opts.WantsPlatform(runtime.GOOS, runtime.GOARCH, variant))
})
t.Run("Invalid platform requested", func(t *testing.T) {
os := "linux"
Expand Down
14 changes: 12 additions & 2 deletions pkg/options/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,25 @@ func PlatformKey(os string, arch string, variant string) string {
return key
}

// MatchesPlatform returns true if given OS name matches the OS set in options
// WantsPlatform returns true if given platform matches the platform set in options
func (o *ManifestOptions) WantsPlatform(os string, arch string, variant string) bool {
o.mutex.RLock()
defer o.mutex.RUnlock()
if len(o.platforms) == 0 {
return true
}
_, ok := o.platforms[PlatformKey(os, arch, variant)]
return ok
if ok {
return true
}

// if no exact match, and the passed platform has variant, it may be a more
// specific variant of the platform specified in options. So compare os/arch only
if variant != "" {
_, ok = o.platforms[PlatformKey(os, arch, "")]
return ok
}
return false
}

// WithPlatform sets a platform filter for options o
Expand Down
20 changes: 19 additions & 1 deletion pkg/options/options_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,28 @@ func Test_WantsPlatform(t *testing.T) {
opts = opts.WithPlatform("linux", "arm", "v7")
assert.True(t, opts.WantsPlatform("linux", "arm", "v7"))
})
t.Run("Platform appended", func(t *testing.T) {
t.Run("Platform appended and non-match", func(t *testing.T) {
opts = opts.WithPlatform("linux", "arm", "v8")
assert.True(t, opts.WantsPlatform("linux", "arm", "v7"))
assert.True(t, opts.WantsPlatform("linux", "arm", "v8"))

assert.False(t, opts.WantsPlatform("linux", "arm", "v6"))
assert.False(t, opts.WantsPlatform("linux", "arm", ""))
assert.False(t, opts.WantsPlatform("linux", "", ""))

assert.False(t, opts.WantsPlatform("linux", "amd64", "v7"))
assert.False(t, opts.WantsPlatform("linux", "amd64", ""))

assert.False(t, opts.WantsPlatform("darwin", "arm", "v7"))
assert.False(t, opts.WantsPlatform("darwin", "arm", ""))
})
t.Run("Platform lenient match", func(t *testing.T) {
opts := &ManifestOptions{}
opts = opts.WithPlatform("linux", "arm", "")
opts = opts.WithPlatform("linux", "arm", "v7")
assert.True(t, opts.WantsPlatform("linux", "arm", "v8"))
assert.True(t, opts.WantsPlatform("linux", "arm", "v7"))
assert.True(t, opts.WantsPlatform("linux", "arm", ""))
})
t.Run("Uninitialized options", func(t *testing.T) {
opts := &ManifestOptions{}
Expand Down
19 changes: 10 additions & 9 deletions pkg/registry/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ type registryClient struct {
}

// credentials is an implementation of distribution/V3/session struct
// to mangage registry credentials and token
// to manage registry credentials and token
type credentials struct {
username string
password string
Expand Down Expand Up @@ -306,8 +306,8 @@ func (client *registryClient) TagMetadata(manifest distribution.Manifest, opts *
}

if !opts.WantsPlatform(info.OS, info.Arch, info.Variant) {
logCtx.Debugf("ignoring v2 manifest %v. Manifest platform: %s/%s, requested: %s",
ti.EncodedDigest(), info.OS, info.Arch, strings.Join(opts.Platforms(), ","))
logCtx.Debugf("ignoring v2 manifest %v. Manifest platform: %s, requested: %s",
ti.EncodedDigest(), options.PlatformKey(info.OS, info.Arch, info.Variant), strings.Join(opts.Platforms(), ","))
return nil, nil
}

Expand Down Expand Up @@ -338,8 +338,8 @@ func (client *registryClient) TagMetadata(manifest distribution.Manifest, opts *
}

if !opts.WantsPlatform(info.OS, info.Arch, info.Variant) {
logCtx.Debugf("ignoring OCI manifest %v. Manifest platform: %s/%s, requested: %s",
ti.EncodedDigest(), info.OS, info.Arch, strings.Join(opts.Platforms(), ","))
logCtx.Debugf("ignoring OCI manifest %v. Manifest platform: %s, requested: %s",
ti.EncodedDigest(), options.PlatformKey(info.OS, info.Arch, info.Variant), strings.Join(opts.Platforms(), ","))
return nil, nil
}

Expand Down Expand Up @@ -367,19 +367,20 @@ func TagInfoFromReferences(client *registryClient, opts *options.ManifestOptions
refArch = ref.Platform.Architecture
refVariant = ref.Platform.Variant
}
platforms = append(platforms, refOS+"/"+refArch)
logCtx.Tracef("Found %s", options.PlatformKey(refOS, refArch, refVariant))
platform1 := options.PlatformKey(refOS, refArch, refVariant)
platforms = append(platforms, platform1)
logCtx.Tracef("Found %s", platform1)
if !opts.WantsPlatform(refOS, refArch, refVariant) {
logCtx.Tracef("Ignoring referenced manifest %v because platform %s does not match any of: %s",
ref.Digest,
options.PlatformKey(refOS, refArch, refVariant),
platform1,
strings.Join(opts.Platforms(), ","))
continue
}
ml = append(ml, ref)
}

// We need at least one reference that matches requested plaforms
// We need at least one reference that matches requested platforms
if len(ml) == 0 {
logCtx.Debugf("Manifest list did not contain any usable reference. Platforms requested: (%s), platforms included: (%s)",
strings.Join(opts.Platforms(), ","), strings.Join(platforms, ","))
Expand Down

0 comments on commit e1cc998

Please sign in to comment.