From 763cc96cac024623bc4ae2188e191860dd091f52 Mon Sep 17 00:00:00 2001 From: grokspawn Date: Thu, 5 Feb 2026 16:12:00 -0600 Subject: [PATCH] add bundle relatedimage image pullspec validation Signed-off-by: grokspawn --- go.mod | 2 + go.sum | 4 + pkg/validation/internal/bundle.go | 29 +++++ pkg/validation/internal/bundle_test.go | 150 +++++++++++++++++++++++++ 4 files changed, 185 insertions(+) diff --git a/go.mod b/go.mod index 5ca112dfd..cf10da54a 100644 --- a/go.mod +++ b/go.mod @@ -4,6 +4,7 @@ go 1.25.3 require ( github.com/blang/semver/v4 v4.0.0 + github.com/distribution/reference v0.6.0 github.com/go-bindata/go-bindata/v3 v3.1.3 github.com/google/cel-go v0.27.0 github.com/sirupsen/logrus v1.9.4 @@ -52,6 +53,7 @@ require ( github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect github.com/modern-go/reflect2 v1.0.3-0.20250322232337-35a7c28c31ee // indirect github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect + github.com/opencontainers/go-digest v1.0.0 // indirect github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect github.com/prometheus/client_golang v1.23.2 // indirect github.com/prometheus/client_model v0.6.2 // indirect diff --git a/go.sum b/go.sum index 6f48c1297..3a5e9f41a 100644 --- a/go.sum +++ b/go.sum @@ -21,6 +21,8 @@ github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSs github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1VwoXQT9A3Wy9MM3WgvqSxFWenqJduM= github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/distribution/reference v0.6.0 h1:0IXCQ5g4/QMHHkarYzh5l+u8T3t73zM5QvfrDyIgxBk= +github.com/distribution/reference v0.6.0/go.mod h1:BbU0aIcezP1/5jX/8MP0YiH4SdvB5Y4f/wlDRiLyi3E= github.com/emicklei/go-restful/v3 v3.12.2 h1:DhwDP0vY3k8ZzE0RunuJy8GhNpPL6zqLkDf9B/a0/xU= github.com/emicklei/go-restful/v3 v3.12.2/go.mod h1:6n3XBCmQQb25CM2LCACGz8ukIrRry+4bhvbpWn3mrbc= github.com/felixge/httpsnoop v1.0.4 h1:NFTV2Zj1bL4mc9sqWACXbQFVBBg2W3GPvqp8/ESS2Wg= @@ -118,6 +120,8 @@ github.com/onsi/ginkgo/v2 v2.27.2 h1:LzwLj0b89qtIy6SSASkzlNvX6WktqurSHwkk2ipF/Ns github.com/onsi/ginkgo/v2 v2.27.2/go.mod h1:ArE1D/XhNXBXCBkKOLkbsb2c81dQHCRcF5zwn/ykDRo= github.com/onsi/gomega v1.38.2 h1:eZCjf2xjZAqe+LeWvKb5weQ+NcPwX84kqJ0cZNxok2A= github.com/onsi/gomega v1.38.2/go.mod h1:W2MJcYxRGV63b418Ai34Ud0hEdTVXq9NW9+Sx6uXf3k= +github.com/opencontainers/go-digest v1.0.0 h1:apOUWs51W5PlhuyGyz9FCeeBIOUDA/6nW8Oi/yOhh5U= +github.com/opencontainers/go-digest v1.0.0/go.mod h1:0JzlMkj0TRzQZfJkVvzbP0HBR3IKzErnv2BNG4W4MAM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 h1:Jamvg5psRIccs7FGNTlIRMkT8wgtp5eCXdBlqhYGL6U= github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= diff --git a/pkg/validation/internal/bundle.go b/pkg/validation/internal/bundle.go index c5f7ba18d..a3e492be2 100644 --- a/pkg/validation/internal/bundle.go +++ b/pkg/validation/internal/bundle.go @@ -4,6 +4,7 @@ import ( "fmt" "strings" + "github.com/distribution/reference" "github.com/operator-framework/api/pkg/manifests" operatorsv1alpha1 "github.com/operator-framework/api/pkg/operators/v1alpha1" "github.com/operator-framework/api/pkg/validation/errors" @@ -46,6 +47,10 @@ func validateBundle(bundle *manifests.Bundle) (result errors.ManifestResult) { if nameErrors != nil { result.Add(nameErrors...) } + relatedImagesErrors := validateRelatedImages(bundle) + if relatedImagesErrors != nil { + result.Add(relatedImagesErrors...) + } return result } @@ -62,6 +67,30 @@ func validateBundleName(bundle *manifests.Bundle) []errors.Error { return errs } +// validateRelatedImages checks that all relatedImages[].image pullspecs are valid +// using github.com/distribution/reference.ParseNormalizedNamed +func validateRelatedImages(bundle *manifests.Bundle) []errors.Error { + var errs []errors.Error + + for i, relatedImage := range bundle.CSV.Spec.RelatedImages { + if relatedImage.Image == "" { + errs = append(errs, errors.ErrInvalidBundle( + fmt.Sprintf("relatedImages[%d] has an empty image field", i), + fmt.Sprintf("spec.relatedImages[%d].image", i))) + continue + } + + // Parse and validate the image reference + if _, err := reference.ParseNormalizedNamed(relatedImage.Image); err != nil { + errs = append(errs, errors.ErrInvalidBundle( + fmt.Sprintf("relatedImages[%d] has an invalid image pullspec %q: %v", i, relatedImage.Image, err), + fmt.Sprintf("spec.relatedImages[%d].image", i))) + } + } + + return errs +} + func validateServiceAccounts(bundle *manifests.Bundle) []errors.Error { // get service account names defined in the csv saNamesFromCSV := make(map[string]struct{}, 0) diff --git a/pkg/validation/internal/bundle_test.go b/pkg/validation/internal/bundle_test.go index 4bb9ad597..b0b7ef65a 100644 --- a/pkg/validation/internal/bundle_test.go +++ b/pkg/validation/internal/bundle_test.go @@ -2,6 +2,7 @@ package internal import ( "fmt" + "strings" "testing" "github.com/stretchr/testify/require" @@ -302,3 +303,152 @@ func Test_EnsureGetBundleSizeValue(t *testing.T) { }) } } + +func TestValidateRelatedImages(t *testing.T) { + tests := []struct { + name string + relatedImages []v1alpha1.RelatedImage + wantError bool + errCount int + errContains []string + }{ + { + name: "no related images should pass", + relatedImages: []v1alpha1.RelatedImage{}, + wantError: false, + }, + { + name: "valid image with tag should pass", + relatedImages: []v1alpha1.RelatedImage{ + {Name: "operator", Image: "quay.io/operator-framework/my-operator:v1.0.0"}, + }, + wantError: false, + }, + { + name: "valid image with digest should pass", + relatedImages: []v1alpha1.RelatedImage{ + {Name: "operator", Image: "quay.io/operator-framework/my-operator@sha256:abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234"}, + }, + wantError: false, + }, + { + name: "valid image with tag and digest should pass", + relatedImages: []v1alpha1.RelatedImage{ + {Name: "operator", Image: "quay.io/operator-framework/my-operator:v1.0.0@sha256:abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234"}, + }, + wantError: false, + }, + { + name: "valid image without tag (latest implied) should pass", + relatedImages: []v1alpha1.RelatedImage{ + {Name: "operator", Image: "quay.io/operator-framework/my-operator"}, + }, + wantError: false, + }, + { + name: "multiple valid images should pass", + relatedImages: []v1alpha1.RelatedImage{ + {Name: "operator", Image: "quay.io/operator-framework/my-operator:v1.0.0"}, + {Name: "operand", Image: "gcr.io/my-project/my-operand@sha256:abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234"}, + {Name: "init", Image: "docker.io/library/busybox:latest"}, + }, + wantError: false, + }, + { + name: "empty image field should error", + relatedImages: []v1alpha1.RelatedImage{ + {Name: "operator", Image: ""}, + }, + wantError: true, + errCount: 1, + errContains: []string{"empty image field"}, + }, + { + name: "invalid image with spaces should error", + relatedImages: []v1alpha1.RelatedImage{ + {Name: "operator", Image: "invalid image name"}, + }, + wantError: true, + errCount: 1, + errContains: []string{"invalid image pullspec"}, + }, + { + name: "invalid image with uppercase should error", + relatedImages: []v1alpha1.RelatedImage{ + {Name: "operator", Image: "quay.io/Operator-Framework/my-operator:v1.0.0"}, + }, + wantError: true, + errCount: 1, + errContains: []string{"invalid image pullspec", "Operator-Framework"}, + }, + { + name: "invalid image with special characters should error", + relatedImages: []v1alpha1.RelatedImage{ + {Name: "operator", Image: "quay.io/operator-framework/my-operator:v1.0.0!"}, + }, + wantError: true, + errCount: 1, + errContains: []string{"invalid image pullspec"}, + }, + { + name: "invalid digest algorithm should error", + relatedImages: []v1alpha1.RelatedImage{ + {Name: "operator", Image: "quay.io/operator-framework/my-operator@ssha256:abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234"}, + }, + wantError: true, + errCount: 1, + errContains: []string{"invalid image pullspec"}, + }, + { + name: "multiple errors should all be reported", + relatedImages: []v1alpha1.RelatedImage{ + {Name: "operator", Image: ""}, + {Name: "operand", Image: "invalid image"}, + }, + wantError: true, + errCount: 2, + errContains: []string{"relatedImages[0]", "relatedImages[1]"}, + }, + { + name: "mixed valid and invalid images should error", + relatedImages: []v1alpha1.RelatedImage{ + {Name: "operator", Image: "quay.io/operator-framework/my-operator:v1.0.0"}, + {Name: "bad", Image: "invalid image"}, + }, + wantError: true, + errCount: 1, + errContains: []string{"relatedImages[1]", "invalid image pullspec"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + bundle := &manifests.Bundle{ + CSV: &v1alpha1.ClusterServiceVersion{ + Spec: v1alpha1.ClusterServiceVersionSpec{ + RelatedImages: tt.relatedImages, + }, + }, + } + + errs := validateRelatedImages(bundle) + + if tt.wantError { + require.Equal(t, tt.errCount, len(errs), "expected %d errors but got %d", tt.errCount, len(errs)) + // Check that each expected string appears in at least one error + for _, expectedStr := range tt.errContains { + found := false + for _, err := range errs { + if strings.Contains(err.Error(), expectedStr) { + found = true + break + } + } + require.True(t, found, "expected to find %q in error messages", expectedStr) + } + } else { + require.Equal(t, 0, len(errs), "expected no errors but got: %v", errs) + } + }) + } +}