render: improve olm.bundle.object rendering for bundles

alpha/action/migrate.go

@@ -1,13 +1,10 @@
 package action
 
 import (
-	"bytes"
 	"context"
 	"fmt"
-	"io"
 	"io/ioutil"
 	"os"
-	"path/filepath"
 
 	"github.com/operator-framework/operator-registry/alpha/declcfg"
 	"github.com/operator-framework/operator-registry/pkg/image"
@@ -17,13 +14,11 @@ type Migrate struct {
 	CatalogRef string
 	OutputDir  string
 
-	WriteFunc WriteFunc
+	WriteFunc declcfg.WriteFunc
 	FileExt   string
 	Registry  image.Registry
 }
 
-type WriteFunc func(config declcfg.DeclarativeConfig, w io.Writer) error
-
 func (m Migrate) Run(ctx context.Context) error {
 	entries, err := ioutil.ReadDir(m.OutputDir)
 	if err != nil && !os.IsNotExist(err) {
@@ -52,48 +47,5 @@ func (m Migrate) Run(ctx context.Context) error {
 		return fmt.Errorf("render catalog image: %w", err)
 	}
 
-	return writeToFS(*cfg, m.OutputDir, m.WriteFunc, m.FileExt)
-}
-
-func writeToFS(cfg declcfg.DeclarativeConfig, rootDir string, writeFunc WriteFunc, fileExt string) error {
-	channelsByPackage := map[string][]declcfg.Channel{}
-	for _, c := range cfg.Channels {
-		channelsByPackage[c.Package] = append(channelsByPackage[c.Package], c)
-	}
-	bundlesByPackage := map[string][]declcfg.Bundle{}
-	for _, b := range cfg.Bundles {
-		bundlesByPackage[b.Package] = append(bundlesByPackage[b.Package], b)
-	}
-
-	if err := os.MkdirAll(rootDir, 0777); err != nil {
-		return err
-	}
-
-	for _, p := range cfg.Packages {
-		fcfg := declcfg.DeclarativeConfig{
-			Packages: []declcfg.Package{p},
-			Channels: channelsByPackage[p.Name],
-			Bundles:  bundlesByPackage[p.Name],
-		}
-		pkgDir := filepath.Join(rootDir, p.Name)
-		if err := os.MkdirAll(pkgDir, 0777); err != nil {
-			return err
-		}
-		filename := filepath.Join(pkgDir, fmt.Sprintf("catalog%s", fileExt))
-		if err := writeFile(fcfg, filename, writeFunc); err != nil {
-			return err
-		}
-	}
-	return nil
-}
-
-func writeFile(cfg declcfg.DeclarativeConfig, filename string, writeFunc WriteFunc) error {
-	buf := &bytes.Buffer{}
-	if err := writeFunc(cfg, buf); err != nil {
-		return fmt.Errorf("write to buffer for %q: %v", filename, err)
-	}
-	if err := ioutil.WriteFile(filename, buf.Bytes(), 0666); err != nil {
-		return fmt.Errorf("write file %q: %v", filename, err)
-	}
-	return nil
+	return declcfg.WriteFS(*cfg, m.OutputDir, m.WriteFunc, m.FileExt)
 }

alpha/action/migrate_test.go

@@ -2,7 +2,6 @@ package action_test
 
 import (
 	"context"
-	"fmt"
 	"io/fs"
 	"os"
 	"path/filepath"
@@ -111,7 +110,6 @@ func TestMigrate(t *testing.T) {
 				path := filepath.Join(s.migrate.OutputDir, file)
 				actualData, err := os.ReadFile(path)
 				require.NoError(t, err)
-				fmt.Println(string(actualData))
 				require.Equal(t, expectedData, string(actualData))
 			}
 		})
@@ -212,12 +210,18 @@ properties:
   value:
     packageName: bar
     versionRange: <0.1.0
-- type: olm.bundle.object
+- type: olm.csv.metadata
   value:
-    data: eyJhcGlWZXJzaW9uIjoib3BlcmF0b3JzLmNvcmVvcy5jb20vdjFhbHBoYTEiLCJraW5kIjoiQ2x1c3RlclNlcnZpY2VWZXJzaW9uIiwibWV0YWRhdGEiOnsiYW5ub3RhdGlvbnMiOnsib2xtLnNraXBSYW5nZSI6Ilx1MDAzYzAuMS4wIn0sIm5hbWUiOiJmb28udjAuMS4wIn0sInNwZWMiOnsiY3VzdG9tcmVzb3VyY2VkZWZpbml0aW9ucyI6eyJvd25lZCI6W3siZ3JvdXAiOiJ0ZXN0LmZvbyIsImtpbmQiOiJGb28iLCJuYW1lIjoiZm9vcy50ZXN0LmZvbyIsInZlcnNpb24iOiJ2MSJ9XX0sImRpc3BsYXlOYW1lIjoiRm9vIE9wZXJhdG9yIiwicmVsYXRlZEltYWdlcyI6W3siaW1hZ2UiOiJ0ZXN0LnJlZ2lzdHJ5L2Zvby1vcGVyYXRvci9mb286djAuMS4wIiwibmFtZSI6Im9wZXJhdG9yIn1dLCJ2ZXJzaW9uIjoiMC4xLjAifX0=
-- type: olm.bundle.object
-  value:
-    data: eyJhcGlWZXJzaW9uIjoiYXBpZXh0ZW5zaW9ucy5rOHMuaW8vdjEiLCJraW5kIjoiQ3VzdG9tUmVzb3VyY2VEZWZpbml0aW9uIiwibWV0YWRhdGEiOnsibmFtZSI6ImZvb3MudGVzdC5mb28ifSwic3BlYyI6eyJncm91cCI6InRlc3QuZm9vIiwibmFtZXMiOnsia2luZCI6IkZvbyIsInBsdXJhbCI6ImZvb3MifSwidmVyc2lvbnMiOlt7Im5hbWUiOiJ2MSJ9XX19
+    annotations:
+      olm.skipRange: <0.1.0
+    apiServiceDefinitions: {}
+    crdDescriptions:
+      owned:
+      - kind: Foo
+        name: foos.test.foo
+        version: v1
+    displayName: Foo Operator
+    provider: {}
 relatedImages:
 - image: test.registry/foo-operator/foo-bundle:v0.1.0
   name: ""
@@ -247,12 +251,18 @@ properties:
   value:
     packageName: bar
     versionRange: <0.1.0
-- type: olm.bundle.object
-  value:
-    data: eyJhcGlWZXJzaW9uIjoib3BlcmF0b3JzLmNvcmVvcy5jb20vdjFhbHBoYTEiLCJraW5kIjoiQ2x1c3RlclNlcnZpY2VWZXJzaW9uIiwibWV0YWRhdGEiOnsiYW5ub3RhdGlvbnMiOnsib2xtLnNraXBSYW5nZSI6Ilx1MDAzYzAuMi4wIn0sIm5hbWUiOiJmb28udjAuMi4wIn0sInNwZWMiOnsiY3VzdG9tcmVzb3VyY2VkZWZpbml0aW9ucyI6eyJvd25lZCI6W3siZ3JvdXAiOiJ0ZXN0LmZvbyIsImtpbmQiOiJGb28iLCJuYW1lIjoiZm9vcy50ZXN0LmZvbyIsInZlcnNpb24iOiJ2MSJ9XX0sImRpc3BsYXlOYW1lIjoiRm9vIE9wZXJhdG9yIiwiaW5zdGFsbCI6eyJzcGVjIjp7ImRlcGxveW1lbnRzIjpbeyJuYW1lIjoiZm9vLW9wZXJhdG9yIiwic3BlYyI6eyJ0ZW1wbGF0ZSI6eyJzcGVjIjp7ImNvbnRhaW5lcnMiOlt7ImltYWdlIjoidGVzdC5yZWdpc3RyeS9mb28tb3BlcmF0b3IvZm9vOnYwLjIuMCJ9XSwiaW5pdENvbnRhaW5lcnMiOlt7ImltYWdlIjoidGVzdC5yZWdpc3RyeS9mb28tb3BlcmF0b3IvZm9vLWluaXQ6djAuMi4wIn1dfX19fSx7Im5hbWUiOiJmb28tb3BlcmF0b3ItMiIsInNwZWMiOnsidGVtcGxhdGUiOnsic3BlYyI6eyJjb250YWluZXJzIjpbeyJpbWFnZSI6InRlc3QucmVnaXN0cnkvZm9vLW9wZXJhdG9yL2Zvby0yOnYwLjIuMCJ9XSwiaW5pdENvbnRhaW5lcnMiOlt7ImltYWdlIjoidGVzdC5yZWdpc3RyeS9mb28tb3BlcmF0b3IvZm9vLWluaXQtMjp2MC4yLjAifV19fX19XX0sInN0cmF0ZWd5IjoiZGVwbG95bWVudCJ9LCJyZWxhdGVkSW1hZ2VzIjpbeyJpbWFnZSI6InRlc3QucmVnaXN0cnkvZm9vLW9wZXJhdG9yL2Zvbzp2MC4yLjAiLCJuYW1lIjoib3BlcmF0b3IifSx7ImltYWdlIjoidGVzdC5yZWdpc3RyeS9mb28tb3BlcmF0b3IvZm9vLW90aGVyOnYwLjIuMCIsIm5hbWUiOiJvdGhlciJ9XSwicmVwbGFjZXMiOiJmb28udjAuMS4wIiwic2tpcHMiOlsiZm9vLnYwLjEuMSIsImZvby52MC4xLjIiXSwidmVyc2lvbiI6IjAuMi4wIn19
-- type: olm.bundle.object
+- type: olm.csv.metadata
   value:
-    data: eyJhcGlWZXJzaW9uIjoiYXBpZXh0ZW5zaW9ucy5rOHMuaW8vdjEiLCJraW5kIjoiQ3VzdG9tUmVzb3VyY2VEZWZpbml0aW9uIiwibWV0YWRhdGEiOnsibmFtZSI6ImZvb3MudGVzdC5mb28ifSwic3BlYyI6eyJncm91cCI6InRlc3QuZm9vIiwibmFtZXMiOnsia2luZCI6IkZvbyIsInBsdXJhbCI6ImZvb3MifSwidmVyc2lvbnMiOlt7Im5hbWUiOiJ2MSJ9XX19
+    annotations:
+      olm.skipRange: <0.2.0
+    apiServiceDefinitions: {}
+    crdDescriptions:
+      owned:
+      - kind: Foo
+        name: foos.test.foo
+        version: v1
+    displayName: Foo Operator
+    provider: {}
 relatedImages:
 - image: test.registry/foo-operator/foo-2:v0.2.0
   name: ""
@@ -299,12 +309,15 @@ properties:
   value:
     packageName: bar
     version: 0.1.0
-- type: olm.bundle.object
+- type: olm.csv.metadata
   value:
-    data: eyJhcGlWZXJzaW9uIjoib3BlcmF0b3JzLmNvcmVvcy5jb20vdjFhbHBoYTEiLCJraW5kIjoiQ2x1c3RlclNlcnZpY2VWZXJzaW9uIiwibWV0YWRhdGEiOnsibmFtZSI6ImJhci52MC4xLjAifSwic3BlYyI6eyJjdXN0b21yZXNvdXJjZWRlZmluaXRpb25zIjp7Im93bmVkIjpbeyJncm91cCI6InRlc3QuYmFyIiwia2luZCI6IkJhciIsIm5hbWUiOiJiYXJzLnRlc3QuYmFyIiwidmVyc2lvbiI6InYxYWxwaGExIn1dfSwicmVsYXRlZEltYWdlcyI6W3siaW1hZ2UiOiJ0ZXN0LnJlZ2lzdHJ5L2Jhci1vcGVyYXRvci9iYXI6djAuMS4wIiwibmFtZSI6Im9wZXJhdG9yIn1dLCJ2ZXJzaW9uIjoiMC4xLjAifX0=
-- type: olm.bundle.object
-  value:
-    data: eyJhcGlWZXJzaW9uIjoiYXBpZXh0ZW5zaW9ucy5rOHMuaW8vdjEiLCJraW5kIjoiQ3VzdG9tUmVzb3VyY2VEZWZpbml0aW9uIiwibWV0YWRhdGEiOnsibmFtZSI6ImJhcnMudGVzdC5iYXIifSwic3BlYyI6eyJncm91cCI6InRlc3QuYmFyIiwibmFtZXMiOnsia2luZCI6IkJhciIsInBsdXJhbCI6ImJhcnMifSwidmVyc2lvbnMiOlt7Im5hbWUiOiJ2MWFscGhhMSJ9XX19
+    apiServiceDefinitions: {}
+    crdDescriptions:
+      owned:
+      - kind: Bar
+        name: bars.test.bar
+        version: v1alpha1
+    provider: {}
 relatedImages:
 - image: test.registry/bar-operator/bar-bundle:v0.1.0
   name: ""
@@ -325,12 +338,17 @@ properties:
   value:
     packageName: bar
     version: 0.2.0
-- type: olm.bundle.object
-  value:
-    data: eyJhcGlWZXJzaW9uIjoib3BlcmF0b3JzLmNvcmVvcy5jb20vdjFhbHBoYTEiLCJraW5kIjoiQ2x1c3RlclNlcnZpY2VWZXJzaW9uIiwibWV0YWRhdGEiOnsiYW5ub3RhdGlvbnMiOnsib2xtLnNraXBSYW5nZSI6Ilx1MDAzYzAuMi4wIn0sIm5hbWUiOiJiYXIudjAuMi4wIn0sInNwZWMiOnsiY3VzdG9tcmVzb3VyY2VkZWZpbml0aW9ucyI6eyJvd25lZCI6W3siZ3JvdXAiOiJ0ZXN0LmJhciIsImtpbmQiOiJCYXIiLCJuYW1lIjoiYmFycy50ZXN0LmJhciIsInZlcnNpb24iOiJ2MWFscGhhMSJ9XX0sInJlbGF0ZWRJbWFnZXMiOlt7ImltYWdlIjoidGVzdC5yZWdpc3RyeS9iYXItb3BlcmF0b3IvYmFyOnYwLjIuMCIsIm5hbWUiOiJvcGVyYXRvciJ9XSwic2tpcHMiOlsiYmFyLnYwLjEuMCJdLCJ2ZXJzaW9uIjoiMC4yLjAifX0=
-- type: olm.bundle.object
+- type: olm.csv.metadata
   value:
-    data: eyJhcGlWZXJzaW9uIjoiYXBpZXh0ZW5zaW9ucy5rOHMuaW8vdjEiLCJraW5kIjoiQ3VzdG9tUmVzb3VyY2VEZWZpbml0aW9uIiwibWV0YWRhdGEiOnsibmFtZSI6ImJhcnMudGVzdC5iYXIifSwic3BlYyI6eyJncm91cCI6InRlc3QuYmFyIiwibmFtZXMiOnsia2luZCI6IkJhciIsInBsdXJhbCI6ImJhcnMifSwidmVyc2lvbnMiOlt7Im5hbWUiOiJ2MWFscGhhMSJ9XX19
+    annotations:
+      olm.skipRange: <0.2.0
+    apiServiceDefinitions: {}
+    crdDescriptions:
+      owned:
+      - kind: Bar
+        name: bars.test.bar
+        version: v1alpha1
+    provider: {}
 relatedImages:
 - image: test.registry/bar-operator/bar-bundle:v0.2.0
   name: ""

alpha/action/render.go

@@ -82,7 +82,7 @@ func (r Render) Run(ctx context.Context) (*declcfg.DeclarativeConfig, error) {
 		if err != nil {
 			return nil, fmt.Errorf("render reference %q: %w", ref, err)
 		}
-		renderBundleObjects(cfg)
+		moveBundleObjectsToEndOfPropertySlices(cfg)
 
 		for _, b := range cfg.Bundles {
 			sort.Slice(b.RelatedImages, func(i, j int) bool {
@@ -304,6 +304,7 @@ func bundleToDeclcfg(bundle *registry.Bundle) (*declcfg.DeclarativeConfig, error
 	if err != nil {
 		return nil, fmt.Errorf("get related images for bundle %q: %v", bundle.Name, err)
 	}
+
 	var csvJson []byte
 	for _, obj := range bundle.Objects {
 		if obj.GetKind() == "ClusterServiceVersion" {
@@ -376,19 +377,21 @@ func getRelatedImages(b *registry.Bundle) ([]declcfg.RelatedImage, error) {
 	return relatedImages, nil
 }
 
-func renderBundleObjects(cfg *declcfg.DeclarativeConfig) {
+func moveBundleObjectsToEndOfPropertySlices(cfg *declcfg.DeclarativeConfig) {
 	for bi, b := range cfg.Bundles {
-		props := b.Properties[:0]
+		var (
+			others []property.Property
+			objs   []property.Property
+		)
 		for _, p := range b.Properties {
-			if p.Type != property.TypeBundleObject {
-				props = append(props, p)
+			switch p.Type {
+			case property.TypeBundleObject, property.TypeCSVMetadata:
+				objs = append(objs, p)
+			default:
+				others = append(others, p)
 			}
 		}
-
-		for _, obj := range b.Objects {
-			props = append(props, property.MustBuildBundleObjectData([]byte(obj)))
-		}
-		cfg.Bundles[bi].Properties = props
+		cfg.Bundles[bi].Properties = append(others, objs...)
 	}
 }
 

alpha/action/render_test.go

@@ -1,16 +1,19 @@
 package action_test
 
 import (
+	"bytes"
 	"context"
 	"embed"
 	"encoding/json"
 	"errors"
+	"io"
 	"io/fs"
 	"os"
 	"path/filepath"
 	"testing"
 	"testing/fstest"
 
+	"github.com/operator-framework/api/pkg/operators/v1alpha1"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"k8s.io/apimachinery/pkg/util/yaml"
@@ -105,8 +108,7 @@ func TestRender(t *testing.T) {
 							property.MustBuildGVKRequired("test.bar", "v1alpha1", "Bar"),
 							property.MustBuildPackage("foo", "0.1.0"),
 							property.MustBuildPackageRequired("bar", "<0.1.0"),
-							property.MustBuildBundleObjectData(foov1csv),
-							property.MustBuildBundleObjectData(foov1crd),
+							mustBuildCSVMetadata(bytes.NewReader(foov1csv)),
 						},
 						RelatedImages: []declcfg.RelatedImage{
 							{
@@ -130,8 +132,7 @@ func TestRender(t *testing.T) {
 							property.MustBuildGVKRequired("test.bar", "v1alpha1", "Bar"),
 							property.MustBuildPackage("foo", "0.2.0"),
 							property.MustBuildPackageRequired("bar", "<0.1.0"),
-							property.MustBuildBundleObjectData(foov2csv),
-							property.MustBuildBundleObjectData(foov2crd),
+							mustBuildCSVMetadata(bytes.NewReader(foov2csv)),
 						},
 						RelatedImages: []declcfg.RelatedImage{
 							{
@@ -197,8 +198,7 @@ func TestRender(t *testing.T) {
 							property.MustBuildGVKRequired("test.bar", "v1alpha1", "Bar"),
 							property.MustBuildPackage("foo", "0.1.0"),
 							property.MustBuildPackageRequired("bar", "<0.1.0"),
-							property.MustBuildBundleObjectData(foov1csv),
-							property.MustBuildBundleObjectData(foov1crd),
+							mustBuildCSVMetadata(bytes.NewReader(foov1csv)),
 						},
 						RelatedImages: []declcfg.RelatedImage{
 							{
@@ -222,8 +222,7 @@ func TestRender(t *testing.T) {
 							property.MustBuildGVKRequired("test.bar", "v1alpha1", "Bar"),
 							property.MustBuildPackage("foo", "0.2.0"),
 							property.MustBuildPackageRequired("bar", "<0.1.0"),
-							property.MustBuildBundleObjectData(foov2csv),
-							property.MustBuildBundleObjectData(foov2crd),
+							mustBuildCSVMetadata(bytes.NewReader(foov2csv)),
 						},
 						RelatedImages: []declcfg.RelatedImage{
 							{
@@ -468,8 +467,7 @@ func TestRender(t *testing.T) {
 							property.MustBuildGVKRequired("test.bar", "v1alpha1", "Bar"),
 							property.MustBuildPackage("foo", "0.2.0"),
 							property.MustBuildPackageRequired("bar", "<0.1.0"),
-							property.MustBuildBundleObjectData(foov2csv),
-							property.MustBuildBundleObjectData(foov2crd),
+							mustBuildCSVMetadata(bytes.NewReader(foov2csv)),
 						},
 						Objects: []string{string(foov2csv), string(foov2crd)},
 						CsvJSON: string(foov2csv),
@@ -518,8 +516,7 @@ func TestRender(t *testing.T) {
 							property.MustBuildGVKRequired("test.bar", "v1alpha1", "Bar"),
 							property.MustBuildPackage("foo", "0.2.0"),
 							property.MustBuildPackageRequired("bar", "<0.1.0"),
-							property.MustBuildBundleObjectData(foov2csvNoRelatedImages),
-							property.MustBuildBundleObjectData(foov2crdNoRelatedImages),
+							mustBuildCSVMetadata(bytes.NewReader(foov2csvNoRelatedImages)),
 						},
 						Objects: []string{string(foov2csvNoRelatedImages), string(foov2crdNoRelatedImages)},
 						CsvJSON: string(foov2csvNoRelatedImages),
@@ -551,7 +548,17 @@ func TestRender(t *testing.T) {
 		t.Run(s.name, func(t *testing.T) {
 			actualCfg, actualErr := s.render.Run(context.Background())
 			s.assertion(t, actualErr)
-			require.Equal(t, s.expectCfg, actualCfg)
+			require.Equal(t, len(s.expectCfg.Packages), len(actualCfg.Packages))
+			require.Equal(t, s.expectCfg.Packages, actualCfg.Packages)
+			require.Equal(t, len(s.expectCfg.Channels), len(actualCfg.Channels))
+			require.Equal(t, s.expectCfg.Channels, actualCfg.Channels)
+			require.Equal(t, len(s.expectCfg.Bundles), len(actualCfg.Bundles))
+			for i := range s.expectCfg.Bundles {
+				actual, expected := actualCfg.Bundles[i], s.expectCfg.Bundles[i]
+				require.Equal(t, expected, actual, "bundle %d", i)
+			}
+			require.Equal(t, len(s.expectCfg.Others), len(actualCfg.Others))
+			require.Equal(t, s.expectCfg.Others, actualCfg.Others)
 		})
 	}
 }
@@ -880,3 +887,11 @@ func generateSqliteFile(path string, imageMap map[image.Reference]string) error
 	}
 	return nil
 }
+
+func mustBuildCSVMetadata(r io.Reader) property.Property {
+	var csv v1alpha1.ClusterServiceVersion
+	if err := json.NewDecoder(r).Decode(&csv); err != nil {
+		panic(err)
+	}
+	return property.MustBuildCSVMetadata(csv)
+}