Skip to content

Commit 3bb33ee

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

2 files changed

Lines changed: 84 additions & 0 deletions

File tree

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+
"go.podman.io/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+
}

0 commit comments

Comments
 (0)