Skip to content

Commit 5b788c0

Browse files
grokspawngrokspawn
committed
adding olm.bundle image/relatedImages pullspec format validation
Signed-off-by: grokspawn <jordan@nimblewidget.com> Assisted-By: Claude
1 parent a40ca89 commit 5b788c0

File tree

4 files changed

+90
-0
lines changed

4 files changed

+90
-0
lines changed

‎alpha/declcfg/declcfg_to_model.go‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"fmt"
55

66
"github.com/blang/semver/v4"
7+
"github.com/containers/image/v5/docker/reference"
78
"k8s.io/apimachinery/pkg/util/sets"
89
"k8s.io/apimachinery/pkg/util/validation"
910

@@ -128,6 +129,15 @@ func ConvertToModel(cfg DeclarativeConfig) (model.Model, error) {
128129
return nil, fmt.Errorf("package %q does not match %q property %q", b.Package, property.TypePackage, props.Packages[0].PackageName)
129130
}
130131

132+
if err := validateImagePullSpec(b.Image, "package %q bundle %q image", b.Package, b.Name); err != nil {
133+
return nil, err
134+
}
135+
for i, rel := range b.RelatedImages {
136+
if err := validateImagePullSpec(rel.Image, "package %q bundle %q relatedImages[%d].image", b.Package, b.Name, i); err != nil {
137+
return nil, err
138+
}
139+
}
140+
131141
// Parse version from the package property.
132142
rawVersion := props.Packages[0].Version
133143
ver, err := semver.Parse(rawVersion)
@@ -269,3 +279,15 @@ func relatedImagesToModelRelatedImages(in []RelatedImage) []model.RelatedImage {
269279
}
270280
return out
271281
}
282+
283+
// validateImagePullSpec checks that a non-empty image pull spec is valid
284+
// Empty pull specs are not validated.
285+
func validateImagePullSpec(pullSpec, errFormat string, errArgs ...interface{}) error {
286+
if pullSpec == "" {
287+
return nil
288+
}
289+
if _, err := reference.ParseNormalizedNamed(pullSpec); err != nil {
290+
return fmt.Errorf(errFormat+": invalid image pull spec %q: %w", append(errArgs, pullSpec, err)...)
291+
}
292+
return nil
293+
}

‎alpha/declcfg/declcfg_to_model_test.go‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -506,6 +506,57 @@ func TestConvertToModel(t *testing.T) {
506506
})},
507507
},
508508
},
509+
{
510+
name: "Error/BundleImageInvalidPullSpecUnsupportedDigestSsha256",
511+
assertion: hasErrorContaining("invalid image pull spec"),
512+
cfg: DeclarativeConfig{
513+
Packages: []Package{newTestPackage("foo", "alpha", svgSmallCircle)},
514+
Channels: []Channel{newTestChannel("foo", "alpha", ChannelEntry{Name: testBundleName("foo", "0.1.0")})},
515+
Bundles: []Bundle{newTestBundle("foo", "0.1.0", func(b *Bundle) {
516+
// Misspelled digest algorithm: ssha256 instead of sha256 (unsupported hash type)
517+
b.Image = "quay.io/operator-framework/foo-bundle@ssha256:abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234"
518+
})},
519+
},
520+
},
521+
{
522+
name: "Error/BundleImageInvalidPullSpecUnsupportedDigestMd5",
523+
assertion: hasErrorContaining("invalid image pull spec"),
524+
cfg: DeclarativeConfig{
525+
Packages: []Package{newTestPackage("foo", "alpha", svgSmallCircle)},
526+
Channels: []Channel{newTestChannel("foo", "alpha", ChannelEntry{Name: testBundleName("foo", "0.1.0")})},
527+
Bundles: []Bundle{newTestBundle("foo", "0.1.0", func(b *Bundle) {
528+
b.Image = "quay.io/operator-framework/foo-bundle@md5:abcd1234abcd1234abcd1234abcd1234"
529+
})},
530+
},
531+
},
532+
{
533+
name: "Error/BundleRelatedImageInvalidPullSpecSsha256",
534+
assertion: hasErrorContaining("invalid image pull spec"),
535+
cfg: DeclarativeConfig{
536+
Packages: []Package{newTestPackage("foo", "alpha", svgSmallCircle)},
537+
Channels: []Channel{newTestChannel("foo", "alpha", ChannelEntry{Name: testBundleName("foo", "0.1.0")})},
538+
Bundles: []Bundle{newTestBundle("foo", "0.1.0", func(b *Bundle) {
539+
b.RelatedImages = []RelatedImage{
540+
{Name: "bundle", Image: testBundleImage("foo", "0.1.0")},
541+
{Name: "operator", Image: "quay.io/operator-framework/my-operator@ssha256:abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234"},
542+
}
543+
})},
544+
},
545+
},
546+
{
547+
name: "Success/BundleImageValidSha256Digest",
548+
assertion: require.NoError,
549+
cfg: DeclarativeConfig{
550+
Packages: []Package{newTestPackage("foo", "alpha", svgSmallCircle)},
551+
Channels: []Channel{newTestChannel("foo", "alpha", ChannelEntry{Name: testBundleName("foo", "0.1.0")})},
552+
Bundles: []Bundle{newTestBundle("foo", "0.1.0", func(b *Bundle) {
553+
b.Image = "quay.io/operator-framework/foo-bundle@sha256:abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234"
554+
b.RelatedImages = []RelatedImage{
555+
{Name: "bundle", Image: "quay.io/operator-framework/foo-bundle@sha256:abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234"},
556+
}
557+
})},
558+
},
559+
},
509560
}
510561

511562
for _, s := range specs {
@@ -577,3 +628,14 @@ func hasError(expectedError string) require.ErrorAssertionFunc {
577628
t.FailNow()
578629
}
579630
}
631+
632+
// hasErrorContaining returns an ErrorAssertionFunc that passes when the error message contains the given substring.
633+
func hasErrorContaining(substring string) require.ErrorAssertionFunc {
634+
return func(t require.TestingT, actualError error, args ...interface{}) {
635+
if stdt, ok := t.(*testing.T); ok {
636+
stdt.Helper()
637+
}
638+
require.Error(t, actualError)
639+
require.Contains(t, actualError.Error(), substring, "expected error to contain %q", substring)
640+
}
641+
}

‎go.mod‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ require (
77
github.com/blang/semver/v4 v4.0.0
88
github.com/containerd/containerd v1.7.30
99
github.com/containerd/errdefs v1.0.0
10+
github.com/containers/image/v5 v5.36.2
1011
github.com/distribution/distribution/v3 v3.0.0
1112
github.com/distribution/reference v0.6.0
1213
github.com/docker/cli v29.2.1+incompatible
@@ -83,6 +84,7 @@ require (
8384
github.com/containerd/typeurl/v2 v2.2.3 // indirect
8485
github.com/containers/libtrust v0.0.0-20230121012942-c1716e8a8d01 // indirect
8586
github.com/containers/ocicrypt v1.2.1 // indirect
87+
github.com/containers/storage v1.59.1 // indirect
8688
github.com/cyberphone/json-canonicalization v0.0.0-20241213102144-19d51d7fe467 // indirect
8789
github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect
8890
github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f // indirect

‎go.sum‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,10 +71,14 @@ github.com/containerd/ttrpc v1.2.7 h1:qIrroQvuOL9HQ1X6KHe2ohc7p+HP/0VE6XPU7elJRq
7171
github.com/containerd/ttrpc v1.2.7/go.mod h1:YCXHsb32f+Sq5/72xHubdiJRQY9inL4a4ZQrAbN1q9o=
7272
github.com/containerd/typeurl/v2 v2.2.3 h1:yNA/94zxWdvYACdYO8zofhrTVuQY73fFU1y++dYSw40=
7373
github.com/containerd/typeurl/v2 v2.2.3/go.mod h1:95ljDnPfD3bAbDJRugOiShd/DlAAsxGtUBhJxIn7SCk=
74+
github.com/containers/image/v5 v5.36.2 h1:GcxYQyAHRF/pLqR4p4RpvKllnNL8mOBn0eZnqJbfTwk=
75+
github.com/containers/image/v5 v5.36.2/go.mod h1:b4GMKH2z/5t6/09utbse2ZiLK/c72GuGLFdp7K69eA4=
7476
github.com/containers/libtrust v0.0.0-20230121012942-c1716e8a8d01 h1:Qzk5C6cYglewc+UyGf6lc8Mj2UaPTHy/iF2De0/77CA=
7577
github.com/containers/libtrust v0.0.0-20230121012942-c1716e8a8d01/go.mod h1:9rfv8iPl1ZP7aqh9YA68wnZv2NUDbXdcdPHVz0pFbPY=
7678
github.com/containers/ocicrypt v1.2.1 h1:0qIOTT9DoYwcKmxSt8QJt+VzMY18onl9jUXsxpVhSmM=
7779
github.com/containers/ocicrypt v1.2.1/go.mod h1:aD0AAqfMp0MtwqWgHM1bUwe1anx0VazI108CRrSKINQ=
80+
github.com/containers/storage v1.59.1 h1:11Zu68MXsEQGBBd+GadPrHPpWeqjKS8hJDGiAHgIqDs=
81+
github.com/containers/storage v1.59.1/go.mod h1:KoAYHnAjP3/cTsRS+mmWZGkufSY2GACiKQ4V3ZLQnR0=
7882
github.com/coreos/go-semver v0.3.1 h1:yi21YpKnrx1gt5R+la8n5WgS0kCrsPp33dmEyHReZr4=
7983
github.com/coreos/go-semver v0.3.1/go.mod h1:irMmmIw/7yzSRPWryHsK7EYSg09caPQL03VsM8rvUec=
8084
github.com/coreos/go-systemd/v22 v22.6.0 h1:aGVa/v8B7hpb0TKl0MWoAavPDmHvobFe5R5zn0bCJWo=

0 commit comments

Comments
 (0)