add olm.icon blob and migration

joelanford · joelanford · commit ac0182410afd · 2025-05-30T09:14:45.000-04:00
Signed-off-by: Joe Lanford &lt;joe.lanford@gmail.com&gt;
diff --git a/alpha/action/init.go b/alpha/action/init.go
@@ -43,7 +43,7 @@ func (i Init) Run() (*declcfg.Package, error) {
 		if iconType.MIME.Type != "image" {
 			return nil, fmt.Errorf("detected invalid type %q: not an image", iconType.MIME.Value)
 		}
-		pkg.Icon = &declcfg.Icon{
+		pkg.Icon = &declcfg.PackageIcon{ //nolint:staticcheck
 			Data:      iconData,
 			MediaType: iconType.MIME.Value,
 		}
diff --git a/alpha/action/init_test.go b/alpha/action/init_test.go
@@ -73,7 +73,7 @@ func TestInit(t *testing.T) {
 			},
 			expectPkg: &declcfg.Package{
 				Schema: "olm.package",
-				Icon: &declcfg.Icon{
+				Icon: &declcfg.PackageIcon{
 					Data:      bytes.NewBufferString(svgIcon).Bytes(),
 					MediaType: "image/svg+xml",
 				},
@@ -93,7 +93,7 @@ func TestInit(t *testing.T) {
 				Name:           "a",
 				DefaultChannel: "b",
 				Description:    "c",
-				Icon: &declcfg.Icon{
+				Icon: &declcfg.PackageIcon{
 					Data:      bytes.NewBufferString(svgIcon).Bytes(),
 					MediaType: "image/svg+xml",
 				},
diff --git a/alpha/action/migrations/001_split_icon.go b/alpha/action/migrations/001_split_icon.go
@@ -0,0 +1,32 @@
+package migrations
+
+import (
+	"github.com/operator-framework/operator-registry/alpha/declcfg"
+)
+
+func splitIcon(cfg *declcfg.DeclarativeConfig) error {
+	splitIconFromPackage := func(p *declcfg.Package) error {
+		if p.Icon == nil { //nolint:staticcheck
+			return nil
+		}
+
+		// Make separate olm.icon object
+		cfg.Icons = append(cfg.Icons, declcfg.Icon{
+			Schema:    declcfg.SchemaIcon,
+			Package:   p.Name,
+			MediaType: p.Icon.MediaType, //nolint:staticcheck
+			Data:      p.Icon.Data,      //nolint:staticcheck
+		})
+
+		// Delete original icon from olm.package object
+		p.Icon = nil //nolint:staticcheck
+		return nil
+	}
+
+	for i := range cfg.Packages {
+		if err := splitIconFromPackage(&cfg.Packages[i]); err != nil {
+			return err
+		}
+	}
+	return nil
+}
diff --git a/alpha/action/migrations/migrations.go b/alpha/action/migrations/migrations.go
@@ -54,6 +54,7 @@ type Migrations struct {
 var allMigrations = []Migration{
 	newMigration(NoMigrations, "do nothing", func(_ *declcfg.DeclarativeConfig) error { return nil }),
 	newMigration("bundle-object-to-csv-metadata", `migrates bundles' "olm.bundle.object" to "olm.csv.metadata"`, bundleObjectToCSVMetadata),
+	newMigration("split-icon", `split package icon out into separate "olm.icon" blob`, splitIcon),
 }
 
 func NewMigrations(name string) (*Migrations, error) {
diff --git a/alpha/action/migrations/migrations_test.go b/alpha/action/migrations/migrations_test.go
@@ -37,6 +37,7 @@ func TestMigrations(t *testing.T) {
 			}
 			return nil
 		},
+		MigrationToken("split-icon"): func(d *declcfg.DeclarativeConfig) error { return nil },
 	}
 
 	tests := []struct {
diff --git a/alpha/declcfg/declcfg.go b/alpha/declcfg/declcfg.go
@@ -16,29 +16,43 @@ import (
 
 const (
 	SchemaPackage     = "olm.package"
+	SchemaIcon        = "olm.icon"
 	SchemaChannel     = "olm.channel"
 	SchemaBundle      = "olm.bundle"
 	SchemaDeprecation = "olm.deprecations"
 )
 
 type DeclarativeConfig struct {
 	Packages     []Package
+	Icons        []Icon
 	Channels     []Channel
 	Bundles      []Bundle
 	Deprecations []Deprecation
 	Others       []Meta
 }
 
 type Package struct {
-	Schema         string              `json:"schema"`
-	Name           string              `json:"name"`
-	DefaultChannel string              `json:"defaultChannel"`
-	Icon           *Icon               `json:"icon,omitempty"`
-	Description    string              `json:"description,omitempty"`
-	Properties     []property.Property `json:"properties,omitempty" hash:"set"`
+	Schema         string `json:"schema"`
+	Name           string `json:"name"`
+	DefaultChannel string `json:"defaultChannel"`
+
+	// Deprecated: It is no longer recommended to embed an icon in the package.
+	//   Instead, use separate a Icon item alongside the Package.
+	Icon *PackageIcon `json:"icon,omitempty"`
+
+	Description string              `json:"description,omitempty"`
+	Properties  []property.Property `json:"properties,omitempty" hash:"set"`
 }
 
 type Icon struct {
+	Schema  string `json:"schema"`
+	Package string `json:"package"`
+
+	MediaType string `json:"mediaType"`
+	Data      []byte `json:"data"`
+}
+
+type PackageIcon struct {
 	Data      []byte `json:"base64data"`
 	MediaType string `json:"mediatype"`
 }
@@ -201,6 +215,7 @@ func extractUniqueMetaKeys(blobMap map[string]any, m *Meta) error {
 
 func (destination *DeclarativeConfig) Merge(src *DeclarativeConfig) {
 	destination.Packages = append(destination.Packages, src.Packages...)
+	destination.Icons = append(destination.Icons, src.Icons...)
 	destination.Channels = append(destination.Channels, src.Channels...)
 	destination.Bundles = append(destination.Bundles, src.Bundles...)
 	destination.Others = append(destination.Others, src.Others...)
diff --git a/alpha/declcfg/declcfg_to_model.go b/alpha/declcfg/declcfg_to_model.go
@@ -41,6 +41,19 @@ func ConvertToModel(cfg DeclarativeConfig) (model.Model, error) {
 		defaultChannels[p.Name] = p.DefaultChannel
 		mpkgs[p.Name] = mpkg
 	}
+	for _, i := range cfg.Icons {
+		mpkg, ok := mpkgs[i.Package]
+		if !ok {
+			return nil, fmt.Errorf("unknown package %q found in olm.icon", i.Package)
+		}
+		if mpkg.Icon != nil {
+			return nil, fmt.Errorf("duplicate icon found for package %q; expecting no more than one icon per package", i.Package)
+		}
+		mpkg.Icon = &model.Icon{
+			MediaType: i.MediaType,
+			Data:      i.Data,
+		}
+	}
 
 	channelDefinedEntries := map[string]sets.Set[string]{}
 	for _, c := range cfg.Channels {
diff --git a/alpha/declcfg/helpers_test.go b/alpha/declcfg/helpers_test.go
@@ -191,7 +191,7 @@ func newTestPackage(packageName, defaultChannel, svgData string) Package {
 		Schema:         SchemaPackage,
 		Name:           packageName,
 		DefaultChannel: defaultChannel,
-		Icon:           &Icon{Data: []byte(svgData), MediaType: "image/svg+xml"},
+		Icon:           &PackageIcon{Data: []byte(svgData), MediaType: "image/svg+xml"},
 		Description:    testPackageDescription(packageName),
 	}
 	return p
diff --git a/alpha/declcfg/load.go b/alpha/declcfg/load.go
@@ -279,6 +279,7 @@ type fbcBuilder struct {
 	cfg DeclarativeConfig
 
 	packagesMu     sync.Mutex
+	iconsMu        sync.Mutex
 	channelsMu     sync.Mutex
 	bundlesMu      sync.Mutex
 	deprecationsMu sync.Mutex
@@ -295,6 +296,14 @@ func (c *fbcBuilder) addMeta(in *Meta) error {
 		c.packagesMu.Lock()
 		c.cfg.Packages = append(c.cfg.Packages, p)
 		c.packagesMu.Unlock()
+	case SchemaIcon:
+		var i Icon
+		if err := json.Unmarshal(in.Blob, &i); err != nil {
+			return fmt.Errorf("parse icon: %v", err)
+		}
+		c.iconsMu.Lock()
+		c.cfg.Icons = append(c.cfg.Icons, i)
+		c.iconsMu.Unlock()
 	case SchemaChannel:
 		var ch Channel
 		if err := json.Unmarshal(in.Blob, &ch); err != nil {
diff --git a/alpha/declcfg/load_benchmark_test.go b/alpha/declcfg/load_benchmark_test.go
@@ -59,7 +59,7 @@ func generateFBC(b *testing.B, numPackages, numChannels, numBundles int) *declcf
 			Schema:      declcfg.SchemaPackage,
 			Name:        pkgName,
 			Description: fmt.Sprintf("%s description", pkgName),
-			Icon: &declcfg.Icon{
+			Icon: &declcfg.PackageIcon{
 				Data:      pngData,
 				MediaType: "image/png",
 			},
diff --git a/alpha/declcfg/load_test.go b/alpha/declcfg/load_test.go
diff --git a/alpha/declcfg/model_to_declcfg.go b/alpha/declcfg/model_to_declcfg.go
@@ -12,9 +12,9 @@ func ConvertFromModel(mpkgs model.Model) DeclarativeConfig {
 	for _, mpkg := range mpkgs {
 		channels, bundles := traverseModelChannels(*mpkg)
 
-		var i *Icon
+		var i *PackageIcon
 		if mpkg.Icon != nil {
-			i = &Icon{
+			i = &PackageIcon{
 				Data:      mpkg.Icon.Data,
 				MediaType: mpkg.Icon.MediaType,
 			}
diff --git a/alpha/declcfg/write.go b/alpha/declcfg/write.go
@@ -405,6 +405,12 @@ func writeToEncoder(cfg DeclarativeConfig, enc encoder) error {
 		pkgNames.Insert(pkgName)
 		packagesByName[pkgName] = append(packagesByName[pkgName], p)
 	}
+	iconsByPackage := map[string][]Icon{}
+	for _, i := range cfg.Icons {
+		pkgName := i.Package
+		pkgNames.Insert(pkgName)
+		iconsByPackage[pkgName] = append(iconsByPackage[pkgName], i)
+	}
 	channelsByPackage := map[string][]Channel{}
 	for _, c := range cfg.Channels {
 		pkgName := c.Package
@@ -441,6 +447,13 @@ func writeToEncoder(cfg DeclarativeConfig, enc encoder) error {
 			}
 		}
 
+		icons := iconsByPackage[pName]
+		for _, i := range icons {
+			if err := enc.Encode(i); err != nil {
+				return err
+			}
+		}
+
 		channels := channelsByPackage[pName]
 		sort.Slice(channels, func(i, j int) bool {
 			return channels[i].Name < channels[j].Name
@@ -501,6 +514,10 @@ func writeToEncoder(cfg DeclarativeConfig, enc encoder) error {
 type WriteFunc func(config DeclarativeConfig, w io.Writer) error
 
 func WriteFS(cfg DeclarativeConfig, rootDir string, writeFunc WriteFunc, fileExt string) error {
+	iconsByPackage := map[string][]Icon{}
+	for _, i := range cfg.Icons {
+		iconsByPackage[i.Package] = append(iconsByPackage[i.Package], i)
+	}
 	channelsByPackage := map[string][]Channel{}
 	for _, c := range cfg.Channels {
 		channelsByPackage[c.Package] = append(channelsByPackage[c.Package], c)
@@ -509,16 +526,27 @@ func WriteFS(cfg DeclarativeConfig, rootDir string, writeFunc WriteFunc, fileExt
 	for _, b := range cfg.Bundles {
 		bundlesByPackage[b.Package] = append(bundlesByPackage[b.Package], b)
 	}
+	deprecationsByPackage := map[string][]Deprecation{}
+	for _, d := range cfg.Deprecations {
+		deprecationsByPackage[d.Package] = append(deprecationsByPackage[d.Package], d)
+	}
+	othersByPackage := map[string][]Meta{}
+	for _, o := range cfg.Others {
+		othersByPackage[o.Package] = append(othersByPackage[o.Package], o)
+	}
 
 	if err := os.MkdirAll(rootDir, 0777); err != nil {
 		return err
 	}
 
 	for _, p := range cfg.Packages {
 		fcfg := DeclarativeConfig{
-			Packages: []Package{p},
-			Channels: channelsByPackage[p.Name],
-			Bundles:  bundlesByPackage[p.Name],
+			Packages:     []Package{p},
+			Icons:        iconsByPackage[p.Name],
+			Channels:     channelsByPackage[p.Name],
+			Bundles:      bundlesByPackage[p.Name],
+			Deprecations: deprecationsByPackage[p.Name],
+			Others:       othersByPackage[p.Name],
 		}
 		pkgDir := filepath.Join(rootDir, p.Name)
 		if err := os.MkdirAll(pkgDir, 0777); err != nil {

Original file line number	Diff line number	Diff line change
`@@ -43,7 +43,7 @@ func (i Init) Run() (*declcfg.Package, error) {`
`43`	`43`	`if iconType.MIME.Type != "image" {`
`44`	`44`	`return nil, fmt.Errorf("detected invalid type %q: not an image", iconType.MIME.Value)`
`45`	`45`	`}`
`46`		`- pkg.Icon = &declcfg.Icon{`
	`46`	`+ pkg.Icon = &declcfg.PackageIcon{ //nolint:staticcheck`
`47`	`47`	`Data: iconData,`
`48`	`48`	`MediaType: iconType.MIME.Value,`
`49`	`49`	`}`
Original file line number	Diff line number	Diff line change
`@@ -54,6 +54,7 @@ type Migrations struct {`
`54`	`54`	`var allMigrations = []Migration{`
`55`	`55`	`newMigration(NoMigrations, "do nothing", func(_ *declcfg.DeclarativeConfig) error { return nil }),`
`56`	`56`	newMigration("bundle-object-to-csv-metadata", `migrates bundles' "olm.bundle.object" to "olm.csv.metadata"`, bundleObjectToCSVMetadata),
	`57`	+ newMigration("split-icon", `split package icon out into separate "olm.icon" blob`, splitIcon),
`57`	`58`	`}`
`58`	`59`
`59`	`60`	`func NewMigrations(name string) (*Migrations, error) {`
Original file line number	Diff line number	Diff line change
`@@ -37,6 +37,7 @@ func TestMigrations(t *testing.T) {`
`37`	`37`	`}`
`38`	`38`	`return nil`
`39`	`39`	`},`
	`40`	`+ MigrationToken("split-icon"): func(d *declcfg.DeclarativeConfig) error { return nil },`
`40`	`41`	`}`
`41`	`42`
`42`	`43`	`tests := []struct {`
Original file line number	Diff line number	Diff line change
`@@ -191,7 +191,7 @@ func newTestPackage(packageName, defaultChannel, svgData string) Package {`
`191`	`191`	`Schema: SchemaPackage,`
`192`	`192`	`Name: packageName,`
`193`	`193`	`DefaultChannel: defaultChannel,`
`194`		`- Icon: &Icon{Data: []byte(svgData), MediaType: "image/svg+xml"},`
	`194`	`+ Icon: &PackageIcon{Data: []byte(svgData), MediaType: "image/svg+xml"},`
`195`	`195`	`Description: testPackageDescription(packageName),`
`196`	`196`	`}`
`197`	`197`	`return p`