Skip to content

Commit

Permalink
render: improve olm.bundle.object rendering for bundles (#1094)
Browse files Browse the repository at this point in the history 
* render: improve olm.bundle.object rendering for bundles

When rendering individual bundles, only generate olm.bundle.object
properties for the CSV if there is an image reference for the bundle.

Signed-off-by: Joe Lanford <joe.lanford@gmail.com>

* render: improve olm.bundle.object rendering for sqlite dbs

When rendering sqlite-based catalogs, only generate olm.bundle.object
properties for the CSV if there is an image reference for the bundle.

Signed-off-by: Joe Lanford <joe.lanford@gmail.com>

* introduce benchmark for declcfg.LoadFS

Signed-off-by: Joe Lanford <joe.lanford@gmail.com>

* concurrent LoadFS

Signed-off-by: Joe Lanford <joe.lanford@gmail.com>

* add olm.csv.metadata property

1. When rendering sqlite DBs and bundle images, generate an
   "olm.csv.metadata" property instead of a full CSV (so long as
   there is a bundle image reference associated with the corresponding
   bundle)
2. When serving the GRPC interface and a full CSV is not present in an
   "olm.bundle.object" property, generate a CSV from (a) the
   "olm.csv.metadata" property. Also include the bundle's related
   images, and the package's icon, if defined. If there is no
   description in the CSV metadata, also include the package's
   description in the generated CSV.

Signed-off-by: Joe Lanford <joe.lanford@gmail.com>

* Meta: more correct handling of HTML-unescaping

It turns out that straight byte-based replacements of unicode escape
characters back to their ascii representations is invalid if the unicode
escape character itself is escaped (e.g. "\u003c" => "\\u003c" => "\<").

To solve this, we will instead unmarshal Meta objects to
map[string]interface{}, extract the expected Meta fields from the map,
and then use a JSON encoder with SetEscapeHTML(false) to re-encode the
map back to JSON to be stored in Meta.Blob.

Signed-off-by: Joe Lanford <joe.lanford@gmail.com>

* updated LoadFS benchmark to use csv metadata properties instead of olm.bundle.object properties

Signed-off-by: Joe Lanford <joe.lanford@gmail.com>

---------

Signed-off-by: Joe Lanford <joe.lanford@gmail.com>
  • Loading branch information
@joelanford
joelanford authored Jun 6, 2023
1 parent 7629c6f commit 2ee231b
Show file tree
Hide file tree
Showing 19 changed files with 1,038 additions and 222 deletions.
52 changes: 2 additions & 50 deletions alpha/action/migrate.go
View file
Original file line number Diff line number Diff line change
@@ -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"
Expand All @@ -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) {
Expand Down Expand Up @@ -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)
}
62 changes: 40 additions & 22 deletions alpha/action/migrate_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package action_test

import (
"context"
"fmt"
"io/fs"
"os"
"path/filepath"
Expand Down Expand Up @@ -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))
}
})
Expand Down Expand Up @@ -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: ""
Expand Down Expand Up @@ -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: ""
Expand Down Expand Up @@ -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: ""
Expand All @@ -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: ""
Expand Down
23 changes: 13 additions & 10 deletions alpha/action/render.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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" {
Expand Down Expand Up @@ -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...)
}
}

Expand Down
41 changes: 28 additions & 13 deletions alpha/action/render_test.go
View file
Original file line number Diff line number Diff line change
@@ -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"
Expand Down Expand Up @@ -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{
{
Expand All @@ -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{
{
Expand Down Expand Up @@ -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{
{
Expand All @@ -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{
{
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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)
})
}
}
Expand Down Expand Up @@ -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)
}
Loading

0 comments on commit 2ee231b

Please sign in to comment.