Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions alpha/action/list_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ foo Foo Operator beta
{
name: "Error/UnknownIndex",
list: ListPackages{IndexReference: "unknown-index"},
expectedErr: `render reference "unknown-index": repository name must be canonical`,
expectedErr: `render reference "unknown-index": failed to pull image "unknown-index": repository name must be canonical`,
},
}
for _, s := range specs {
Expand Down Expand Up @@ -79,7 +79,7 @@ foo stable foo.v0.2.0
{
name: "Error/UnknownIndex",
list: ListChannels{IndexReference: "unknown-index"},
expectedErr: `render reference "unknown-index": repository name must be canonical`,
expectedErr: `render reference "unknown-index": failed to pull image "unknown-index": repository name must be canonical`,
},
{
name: "Error/UnknownPackage",
Expand Down Expand Up @@ -138,7 +138,7 @@ foo stable foo.v0.2.0 foo.v0.1.0 foo.v0.1.1,foo.v0.1.2 <0.2.0 tes
{
name: "Error/UnknownIndex",
list: ListBundles{IndexReference: "unknown-index"},
expectedErr: `render reference "unknown-index": repository name must be canonical`,
expectedErr: `render reference "unknown-index": failed to pull image "unknown-index": repository name must be canonical`,
},
{
name: "Error/UnknownPackage",
Expand Down
8 changes: 4 additions & 4 deletions alpha/action/render.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,19 +162,19 @@ func (r Render) renderReference(ctx context.Context, ref string) (*declcfg.Decla
func (r Render) imageToDeclcfg(ctx context.Context, imageRef string) (*declcfg.DeclarativeConfig, error) {
ref := image.SimpleReference(imageRef)
if err := r.Registry.Pull(ctx, ref); err != nil {
return nil, err
return nil, fmt.Errorf("failed to pull image %q: %v", ref, err)
}
labels, err := r.Registry.Labels(ctx, ref)
if err != nil {
return nil, err
return nil, fmt.Errorf("failed to get labels for image %q: %v", ref, err)
}
tmpDir, err := os.MkdirTemp("", "render-unpack-")
if err != nil {
return nil, err
return nil, fmt.Errorf("create tempdir: %v", err)
}
defer os.RemoveAll(tmpDir)
if err := r.Registry.Unpack(ctx, ref, tmpDir); err != nil {
return nil, err
return nil, fmt.Errorf("failed to unpack image %q: %v", ref, err)
}

var cfg *declcfg.DeclarativeConfig
Expand Down
16 changes: 2 additions & 14 deletions alpha/template/basic/basic.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,13 @@ import (

"k8s.io/apimachinery/pkg/util/yaml"

"github.com/operator-framework/operator-registry/alpha/action"
"github.com/operator-framework/operator-registry/alpha/action/migrations"
"github.com/operator-framework/operator-registry/alpha/declcfg"
"github.com/operator-framework/operator-registry/pkg/image"
)

const schema string = "olm.template.basic"

type Template struct {
Registry image.Registry
Migrations *migrations.Migrations
RenderBundle func(context.Context, string) (*declcfg.DeclarativeConfig, error)
}

type BasicTemplate struct {
Expand Down Expand Up @@ -57,19 +53,11 @@ func (t Template) Render(ctx context.Context, reader io.Reader) (*declcfg.Declar
}

outb := cfg.Bundles[:0]
// populate registry, incl any flags from CLI, and enforce only rendering bundle images
r := action.Render{
Registry: t.Registry,
AllowedRefMask: action.RefBundleImage,
Migrations: t.Migrations,
}

for _, b := range cfg.Bundles {
if !isBundleTemplate(&b) {
return nil, fmt.Errorf("unexpected fields present in basic template bundle")
}
r.Refs = []string{b.Image}
contributor, err := r.Run(ctx)
contributor, err := t.RenderBundle(ctx, b.Image)
if err != nil {
return nil, err
}
Expand Down
45 changes: 21 additions & 24 deletions alpha/template/semver/semver.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (
"k8s.io/apimachinery/pkg/util/errors"
"sigs.k8s.io/yaml"

"github.com/operator-framework/operator-registry/alpha/action"
"github.com/operator-framework/operator-registry/alpha/declcfg"
"github.com/operator-framework/operator-registry/alpha/property"
)
Expand All @@ -25,22 +24,16 @@ func (t Template) Render(ctx context.Context) (*declcfg.DeclarativeConfig, error

var cfgs []declcfg.DeclarativeConfig

bundleDict := make(map[string]struct{})
buildBundleList(&sv.Candidate.Bundles, &bundleDict)
buildBundleList(&sv.Fast.Bundles, &bundleDict)
buildBundleList(&sv.Stable.Bundles, &bundleDict)

bundleDict := buildBundleList(*sv)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: would it be worth renaming sv to make the code a bit more readable?

for b := range bundleDict {
r := action.Render{
AllowedRefMask: action.RefBundleImage,
Refs: []string{b},
Registry: t.Registry,
Migrations: t.Migrations,
}
c, err := r.Run(ctx)
c, err := t.RenderBundle(ctx, b)
if err != nil {
return nil, err
}
if len(c.Bundles) != 1 {
return nil, fmt.Errorf("bundle reference %q resulted in %d bundles, expected 1", b, len(c.Bundles))
}
bundleDict[b] = c.Bundles[0].Image
cfgs = append(cfgs, *c)
}
out = *combineConfigs(cfgs)
Expand All @@ -49,7 +42,7 @@ func (t Template) Render(ctx context.Context) (*declcfg.DeclarativeConfig, error
return nil, fmt.Errorf("render: no bundles specified or no bundles could be rendered")
}

channelBundleVersions, err := sv.getVersionsFromStandardChannels(&out)
channelBundleVersions, err := sv.getVersionsFromStandardChannels(&out, bundleDict)
if err != nil {
return nil, fmt.Errorf("render: unable to post-process bundle info: %v", err)
}
Expand All @@ -61,12 +54,16 @@ func (t Template) Render(ctx context.Context) (*declcfg.DeclarativeConfig, error
return &out, nil
}

func buildBundleList(bundles *[]semverTemplateBundleEntry, dict *map[string]struct{}) {
for _, b := range *bundles {
if _, ok := (*dict)[b.Image]; !ok {
(*dict)[b.Image] = struct{}{}
func buildBundleList(t semverTemplate) map[string]string {
dict := make(map[string]string)
for _, bl := range []semverTemplateChannelBundles{t.Candidate, t.Fast, t.Stable} {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: wdyt about making the making the semverTemplateChannelBundles list a parameter? Before we could see which channel templates were being used to create the bundle list at the top level. Might also make the method more flexible? (though given that it isn't exported - maybe not...XD).

for _, b := range bl.Bundles {
if _, ok := dict[b.Image]; !ok {
dict[b.Image] = b.Image
}
}
}
return dict
}

func readFile(reader io.Reader) (*semverTemplate, error) {
Expand Down Expand Up @@ -114,10 +111,10 @@ func readFile(reader io.Reader) (*semverTemplate, error) {
return &sv, nil
}

func (sv *semverTemplate) getVersionsFromStandardChannels(cfg *declcfg.DeclarativeConfig) (*bundleVersions, error) {
func (sv *semverTemplate) getVersionsFromStandardChannels(cfg *declcfg.DeclarativeConfig, bundleDict map[string]string) (*bundleVersions, error) {
versions := bundleVersions{}

bdm, err := sv.getVersionsFromChannel(sv.Candidate.Bundles, cfg)
bdm, err := sv.getVersionsFromChannel(sv.Candidate.Bundles, bundleDict, cfg)
if err != nil {
return nil, err
}
Expand All @@ -126,7 +123,7 @@ func (sv *semverTemplate) getVersionsFromStandardChannels(cfg *declcfg.Declarati
}
versions[candidateChannelArchetype] = bdm

bdm, err = sv.getVersionsFromChannel(sv.Fast.Bundles, cfg)
bdm, err = sv.getVersionsFromChannel(sv.Fast.Bundles, bundleDict, cfg)
if err != nil {
return nil, err
}
Expand All @@ -135,7 +132,7 @@ func (sv *semverTemplate) getVersionsFromStandardChannels(cfg *declcfg.Declarati
}
versions[fastChannelArchetype] = bdm

bdm, err = sv.getVersionsFromChannel(sv.Stable.Bundles, cfg)
bdm, err = sv.getVersionsFromChannel(sv.Stable.Bundles, bundleDict, cfg)
if err != nil {
return nil, err
}
Expand All @@ -147,7 +144,7 @@ func (sv *semverTemplate) getVersionsFromStandardChannels(cfg *declcfg.Declarati
return &versions, nil
}

func (sv *semverTemplate) getVersionsFromChannel(semverBundles []semverTemplateBundleEntry, cfg *declcfg.DeclarativeConfig) (map[string]semver.Version, error) {
func (sv *semverTemplate) getVersionsFromChannel(semverBundles []semverTemplateBundleEntry, bundleDict map[string]string, cfg *declcfg.DeclarativeConfig) (map[string]semver.Version, error) {
entries := make(map[string]semver.Version)

// we iterate over the channel bundles from the template, to:
Expand All @@ -158,7 +155,7 @@ func (sv *semverTemplate) getVersionsFromChannel(semverBundles []semverTemplateB
// test if the bundle specified in the template is present in the successfully-rendered bundles
index := 0
for index < len(cfg.Bundles) {
if cfg.Bundles[index].Image == semverBundle.Image {
if cfg.Bundles[index].Image == bundleDict[semverBundle.Image] {
break
}
index++
Expand Down
4 changes: 2 additions & 2 deletions alpha/template/semver/semver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -538,7 +538,7 @@ func TestGetVersionsFromStandardChannel(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
iosv := tt.sv
versions, err := iosv.getVersionsFromStandardChannels(&tt.dc)
versions, err := iosv.getVersionsFromStandardChannels(&tt.dc, buildBundleList(tt.sv))
require.NoError(t, err)
require.EqualValues(t, tt.outVersions, *versions)
require.EqualValues(t, "a", iosv.pkg) // verify that we learned the package name and stashed it in the receiver
Expand Down Expand Up @@ -591,7 +591,7 @@ func TestBailOnVersionBuildMetadata(t *testing.T) {
}

t.Run("Abort on unorderable build metadata version data", func(t *testing.T) {
_, err := sv.getVersionsFromStandardChannels(&dc)
_, err := sv.getVersionsFromStandardChannels(&dc, buildBundleList(sv))
require.Error(t, err)
})
}
Expand Down
9 changes: 4 additions & 5 deletions alpha/template/semver/types.go
Original file line number Diff line number Diff line change
@@ -1,19 +1,18 @@
package semver

import (
"context"
"io"

"github.com/blang/semver/v4"

"github.com/operator-framework/operator-registry/alpha/action/migrations"
"github.com/operator-framework/operator-registry/pkg/image"
"github.com/operator-framework/operator-registry/alpha/declcfg"
)

// data passed into this module externally
type Template struct {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: should we just make Template (as defined in basic with just the RenderBundle method) a shared type that basic and semver sub-type and implement? The basic and semver packages could even export a factory functions to get new instances of templates?

Data io.Reader
Registry image.Registry
Migrations *migrations.Migrations
Data io.Reader
RenderBundle func(context.Context, string) (*declcfg.DeclarativeConfig, error)
}

// IO structs -- BEGIN
Expand Down
19 changes: 15 additions & 4 deletions cmd/opm/alpha/template/basic.go
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
package template

import (
"context"
"io"
"log"
"os"

"github.com/sirupsen/logrus"
"github.com/spf13/cobra"

"github.com/operator-framework/operator-registry/alpha/action"
"github.com/operator-framework/operator-registry/alpha/action/migrations"
"github.com/operator-framework/operator-registry/alpha/declcfg"
"github.com/operator-framework/operator-registry/alpha/template/basic"
Expand Down Expand Up @@ -62,14 +64,23 @@ When FILE is '-' or not provided, the template is read from standard input`,
}
defer reg.Destroy()

template.Registry = reg

var m *migrations.Migrations
if migrateLevel != "" {
m, err := migrations.NewMigrations(migrateLevel)
m, err = migrations.NewMigrations(migrateLevel)
if err != nil {
log.Fatal(err)
}
template.Migrations = m
}

template.RenderBundle = func(ctx context.Context, image string) (*declcfg.DeclarativeConfig, error) {
// populate registry, incl any flags from CLI, and enforce only rendering bundle images
r := action.Render{
Refs: []string{image},
Registry: reg,
AllowedRefMask: action.RefBundleImage,
Migrations: m,
}
return r.Run(ctx)
}

// only taking first file argument
Expand Down
24 changes: 18 additions & 6 deletions cmd/opm/alpha/template/semver.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package template

import (
"context"
"fmt"
"io"
"log"
Expand All @@ -9,6 +10,7 @@ import (
"github.com/sirupsen/logrus"
"github.com/spf13/cobra"

"github.com/operator-framework/operator-registry/alpha/action"
"github.com/operator-framework/operator-registry/alpha/action/migrations"
"github.com/operator-framework/operator-registry/alpha/declcfg"
"github.com/operator-framework/operator-registry/alpha/template/semver"
Expand Down Expand Up @@ -68,17 +70,27 @@ When FILE is '-' or not provided, the template is read from standard input`,
}
defer reg.Destroy()

template := semver.Template{
Data: data,
Registry: reg,
}
var m *migrations.Migrations
if migrateLevel != "" {
m, err := migrations.NewMigrations(migrateLevel)
m, err = migrations.NewMigrations(migrateLevel)
if err != nil {
log.Fatal(err)
}
template.Migrations = m
}

template := semver.Template{
Data: data,
RenderBundle: func(ctx context.Context, ref string) (*declcfg.DeclarativeConfig, error) {
renderer := action.Render{
Refs: []string{ref},
Registry: reg,
AllowedRefMask: action.RefBundleImage,
Migrations: m,
}
return renderer.Run(ctx)
},
}

out, err := template.Render(cmd.Context())
if err != nil {
log.Fatalf("semver %q: %v", source, err)
Expand Down
Loading