Skip to content

Commit 0952bae

Browse files
author
Per G. da Silva
committed
Refactor RegistryV1 out of render package and move BundleSource into source package
Signed-off-by: Per G. da Silva <[email protected]>
1 parent 0b14839 commit 0952bae

File tree

16 files changed

+529
-466
lines changed

16 files changed

+529
-466
lines changed

internal/operator-controller/applier/helm.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ import (
2626

2727
ocv1 "github.com/operator-framework/operator-controller/api/v1"
2828
"github.com/operator-framework/operator-controller/internal/operator-controller/authorization"
29-
"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/convert"
29+
"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/bundle/source"
3030
"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/preflights/crdupgradesafety"
3131
"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util"
3232
)
@@ -55,7 +55,7 @@ type Preflight interface {
5555
}
5656

5757
type BundleToHelmChartConverter interface {
58-
ToHelmChart(bundle convert.BundleSource, installNamespace string, watchNamespace string) (*chart.Chart, error)
58+
ToHelmChart(bundle source.BundleSource, installNamespace string, watchNamespace string) (*chart.Chart, error)
5959
}
6060

6161
type Helm struct {
@@ -209,7 +209,7 @@ func (h *Helm) buildHelmChart(bundleFS fs.FS, ext *ocv1.ClusterExtension) (*char
209209
if err != nil {
210210
return nil, err
211211
}
212-
return h.BundleToHelmChartConverter.ToHelmChart(convert.FSBundleSource{FS: bundleFS}, ext.Spec.Namespace, watchNamespace)
212+
return h.BundleToHelmChartConverter.ToHelmChart(source.FromFS(bundleFS), ext.Spec.Namespace, watchNamespace)
213213
}
214214

215215
func (h *Helm) renderClientOnlyRelease(ctx context.Context, ext *ocv1.ClusterExtension, chrt *chart.Chart, values chartutil.Values, post postrender.PostRenderer) (*release.Release, error) {

internal/operator-controller/applier/helm_test.go

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
"github.com/operator-framework/operator-controller/internal/operator-controller/applier"
2727
"github.com/operator-framework/operator-controller/internal/operator-controller/authorization"
2828
"github.com/operator-framework/operator-controller/internal/operator-controller/features"
29+
"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/bundle/source"
2930
"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/convert"
3031
)
3132

@@ -556,7 +557,7 @@ func TestApply_InstallationWithSingleOwnNamespaceInstallSupportEnabled(t *testin
556557
},
557558
},
558559
BundleToHelmChartConverter: &fakeBundleToHelmChartConverter{
559-
fn: func(bundle convert.BundleSource, installNamespace string, watchNamespace string) (*chart.Chart, error) {
560+
fn: func(bundle source.BundleSource, installNamespace string, watchNamespace string) (*chart.Chart, error) {
560561
require.Equal(t, expectedWatchNamespace, watchNamespace)
561562
return nil, nil
562563
},
@@ -589,7 +590,7 @@ func TestApply_RegistryV1ToChartConverterIntegration(t *testing.T) {
589590
},
590591
},
591592
BundleToHelmChartConverter: &fakeBundleToHelmChartConverter{
592-
fn: func(bundle convert.BundleSource, installNamespace string, watchNamespace string) (*chart.Chart, error) {
593+
fn: func(bundle source.BundleSource, installNamespace string, watchNamespace string) (*chart.Chart, error) {
593594
require.Equal(t, expectedWatchNamespace, watchNamespace)
594595
return nil, nil
595596
},
@@ -609,7 +610,7 @@ func TestApply_RegistryV1ToChartConverterIntegration(t *testing.T) {
609610
},
610611
},
611612
BundleToHelmChartConverter: &fakeBundleToHelmChartConverter{
612-
fn: func(bundle convert.BundleSource, installNamespace string, watchNamespace string) (*chart.Chart, error) {
613+
fn: func(bundle source.BundleSource, installNamespace string, watchNamespace string) (*chart.Chart, error) {
613614
return nil, errors.New("some error")
614615
},
615616
},
@@ -621,9 +622,9 @@ func TestApply_RegistryV1ToChartConverterIntegration(t *testing.T) {
621622
}
622623

623624
type fakeBundleToHelmChartConverter struct {
624-
fn func(convert.BundleSource, string, string) (*chart.Chart, error)
625+
fn func(source.BundleSource, string, string) (*chart.Chart, error)
625626
}
626627

627-
func (f fakeBundleToHelmChartConverter) ToHelmChart(bundle convert.BundleSource, installNamespace string, watchNamespace string) (*chart.Chart, error) {
628+
func (f fakeBundleToHelmChartConverter) ToHelmChart(bundle source.BundleSource, installNamespace string, watchNamespace string) (*chart.Chart, error) {
628629
return f.fn(bundle, installNamespace, watchNamespace)
629630
}
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
package bundle
2+
3+
import (
4+
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
5+
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
6+
7+
"github.com/operator-framework/api/pkg/operators/v1alpha1"
8+
)
9+
10+
type RegistryV1 struct {
11+
PackageName string
12+
CSV v1alpha1.ClusterServiceVersion
13+
CRDs []apiextensionsv1.CustomResourceDefinition
14+
Others []unstructured.Unstructured
15+
}
Lines changed: 178 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,178 @@
1+
package source
2+
3+
import (
4+
"encoding/json"
5+
"errors"
6+
"fmt"
7+
"io/fs"
8+
"path/filepath"
9+
10+
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
11+
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
12+
"k8s.io/apimachinery/pkg/runtime"
13+
"k8s.io/cli-runtime/pkg/resource"
14+
"sigs.k8s.io/yaml"
15+
16+
"github.com/operator-framework/api/pkg/operators/v1alpha1"
17+
"github.com/operator-framework/operator-registry/alpha/property"
18+
19+
"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/bundle"
20+
registry "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/operator-registry"
21+
)
22+
23+
type BundleSource interface {
24+
GetBundle() (bundle.RegistryV1, error)
25+
}
26+
27+
// identitySource is a bundle source that returns itself
28+
type identitySource bundle.RegistryV1
29+
30+
func (r identitySource) GetBundle() (bundle.RegistryV1, error) {
31+
return bundle.RegistryV1(r), nil
32+
}
33+
34+
func FromBundle(rv1 bundle.RegistryV1) BundleSource {
35+
return identitySource(rv1)
36+
}
37+
38+
// FromFS returns a BundleSource that loads a registry+v1 bundle from a filesystem.
39+
// The filesystem is expected to conform to the registry+v1 format:
40+
// metadata/annotations.yaml
41+
// metadata/properties.yaml
42+
// manifests/
43+
// - csv.yaml
44+
// - ...
45+
//
46+
// manifests directory should not contain subdirectories
47+
func FromFS(fs fs.FS) BundleSource {
48+
return fsBundleSource{
49+
FS: fs,
50+
}
51+
}
52+
53+
type fsBundleSource struct {
54+
FS fs.FS
55+
}
56+
57+
func (f fsBundleSource) GetBundle() (bundle.RegistryV1, error) {
58+
reg := bundle.RegistryV1{}
59+
annotationsFileData, err := fs.ReadFile(f.FS, filepath.Join("metadata", "annotations.yaml"))
60+
if err != nil {
61+
return reg, err
62+
}
63+
annotationsFile := registry.AnnotationsFile{}
64+
if err := yaml.Unmarshal(annotationsFileData, &annotationsFile); err != nil {
65+
return reg, err
66+
}
67+
reg.PackageName = annotationsFile.Annotations.PackageName
68+
69+
const manifestsDir = "manifests"
70+
foundCSV := false
71+
if err := fs.WalkDir(f.FS, manifestsDir, func(path string, e fs.DirEntry, err error) error {
72+
if err != nil {
73+
return err
74+
}
75+
if e.IsDir() {
76+
if path == manifestsDir {
77+
return nil
78+
}
79+
return fmt.Errorf("subdirectories are not allowed within the %q directory of the bundle image filesystem: found %q", manifestsDir, path)
80+
}
81+
manifestFile, err := f.FS.Open(path)
82+
if err != nil {
83+
return err
84+
}
85+
defer manifestFile.Close()
86+
87+
result := resource.NewLocalBuilder().Unstructured().Flatten().Stream(manifestFile, path).Do()
88+
if err := result.Err(); err != nil {
89+
return err
90+
}
91+
if err := result.Visit(func(info *resource.Info, err error) error {
92+
if err != nil {
93+
return err
94+
}
95+
switch info.Object.GetObjectKind().GroupVersionKind().Kind {
96+
case "ClusterServiceVersion":
97+
csv := v1alpha1.ClusterServiceVersion{}
98+
if err := runtime.DefaultUnstructuredConverter.FromUnstructured(info.Object.(*unstructured.Unstructured).Object, &csv); err != nil {
99+
return err
100+
}
101+
reg.CSV = csv
102+
foundCSV = true
103+
case "CustomResourceDefinition":
104+
crd := apiextensionsv1.CustomResourceDefinition{}
105+
if err := runtime.DefaultUnstructuredConverter.FromUnstructured(info.Object.(*unstructured.Unstructured).Object, &crd); err != nil {
106+
return err
107+
}
108+
reg.CRDs = append(reg.CRDs, crd)
109+
default:
110+
reg.Others = append(reg.Others, *info.Object.(*unstructured.Unstructured))
111+
}
112+
return nil
113+
}); err != nil {
114+
return fmt.Errorf("error parsing objects in %q: %v", path, err)
115+
}
116+
return nil
117+
}); err != nil {
118+
return reg, err
119+
}
120+
121+
if !foundCSV {
122+
return reg, fmt.Errorf("no ClusterServiceVersion found in %q", manifestsDir)
123+
}
124+
125+
if err := copyMetadataPropertiesToCSV(&reg.CSV, f.FS); err != nil {
126+
return reg, err
127+
}
128+
129+
return reg, nil
130+
}
131+
132+
// copyMetadataPropertiesToCSV copies properties from `metadata/propeties.yaml` (in the filesystem fsys) into
133+
// the CSV's `.metadata.annotations['olm.properties']` value, preserving any properties that are already
134+
// present in the annotations.
135+
func copyMetadataPropertiesToCSV(csv *v1alpha1.ClusterServiceVersion, fsys fs.FS) error {
136+
var allProperties []property.Property
137+
138+
// First load existing properties from the CSV. We want to preserve these.
139+
if csvPropertiesJSON, ok := csv.Annotations["olm.properties"]; ok {
140+
var csvProperties []property.Property
141+
if err := json.Unmarshal([]byte(csvPropertiesJSON), &csvProperties); err != nil {
142+
return fmt.Errorf("failed to unmarshal csv.metadata.annotations['olm.properties']: %w", err)
143+
}
144+
allProperties = append(allProperties, csvProperties...)
145+
}
146+
147+
// Next, load properties from the metadata/properties.yaml file, if it exists.
148+
metadataPropertiesJSON, err := fs.ReadFile(fsys, filepath.Join("metadata", "properties.yaml"))
149+
if err != nil && !errors.Is(err, fs.ErrNotExist) {
150+
return fmt.Errorf("failed to read properties.yaml file: %w", err)
151+
}
152+
153+
// If there are no properties, we can stick with whatever
154+
// was already present in the CSV annotations.
155+
if len(metadataPropertiesJSON) == 0 {
156+
return nil
157+
}
158+
159+
// Otherwise, we need to parse the properties.yaml file and
160+
// append its properties into the CSV annotation.
161+
type registryV1Properties struct {
162+
Properties []property.Property `json:"properties"`
163+
}
164+
165+
var metadataProperties registryV1Properties
166+
if err := yaml.Unmarshal(metadataPropertiesJSON, &metadataProperties); err != nil {
167+
return fmt.Errorf("failed to unmarshal metadata/properties.yaml: %w", err)
168+
}
169+
allProperties = append(allProperties, metadataProperties.Properties...)
170+
171+
// Lastly re-marshal all the properties back into a JSON array and update the CSV annotation
172+
allPropertiesJSON, err := json.Marshal(allProperties)
173+
if err != nil {
174+
return fmt.Errorf("failed to marshal registry+v1 properties to json: %w", err)
175+
}
176+
csv.Annotations["olm.properties"] = string(allPropertiesJSON)
177+
return nil
178+
}
Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
package source_test
2+
3+
import (
4+
"io/fs"
5+
"strings"
6+
"testing"
7+
"testing/fstest"
8+
9+
"github.com/stretchr/testify/require"
10+
11+
"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/bundle"
12+
"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/bundle/source"
13+
)
14+
15+
const (
16+
olmProperties = "olm.properties"
17+
18+
bundlePathAnnotations = "metadata/annotations.yaml"
19+
bundlePathProperties = "metadata/properties.yaml"
20+
bundlePathCSV = "manifests/csv.yaml"
21+
)
22+
23+
func Test_FromBundle_Success(t *testing.T) {
24+
expectedBundle := bundle.RegistryV1{
25+
PackageName: "my-package",
26+
}
27+
b, err := source.FromBundle(expectedBundle).GetBundle()
28+
require.NoError(t, err)
29+
require.Equal(t, expectedBundle, b)
30+
}
31+
32+
func Test_FromFS_Success(t *testing.T) {
33+
rv1, err := source.FromFS(newBundleFS()).GetBundle()
34+
require.NoError(t, err)
35+
36+
t.Log("Check package name is correctly taken from metadata/annotations.yaml")
37+
require.Equal(t, "test", rv1.PackageName)
38+
39+
t.Log("Check metadata/properties.yaml is merged into csv.annotations[olm.properties]")
40+
require.JSONEq(t, `[{"type":"from-csv-annotations-key","value":"from-csv-annotations-value"},{"type":"from-file-key","value":"from-file-value"}]`, rv1.CSV.Annotations[olmProperties])
41+
}
42+
43+
func Test_FromFS_Fails(t *testing.T) {
44+
for _, tt := range []struct {
45+
name string
46+
FS fs.FS
47+
}{
48+
{
49+
name: "bundle missing ClusterServiceVersion manifest",
50+
FS: removePaths(newBundleFS(), bundlePathCSV),
51+
}, {
52+
name: "bundle missing metadata/annotations.yaml",
53+
FS: removePaths(newBundleFS(), bundlePathAnnotations),
54+
}, {
55+
name: "bundle missing metadata/ directory",
56+
FS: removePaths(newBundleFS(), "metadata/"),
57+
}, {
58+
name: "bundle missing manifests/ directory",
59+
FS: removePaths(newBundleFS(), "manifests/"),
60+
},
61+
} {
62+
t.Run(tt.name, func(t *testing.T) {
63+
_, err := source.FromFS(tt.FS).GetBundle()
64+
require.Error(t, err)
65+
})
66+
}
67+
}
68+
69+
func newBundleFS() fstest.MapFS {
70+
annotationsYml := `
71+
annotations:
72+
operators.operatorframework.io.bundle.mediatype.v1: registry+v1
73+
operators.operatorframework.io.bundle.package.v1: test
74+
`
75+
76+
propertiesYml := `
77+
properties:
78+
- type: "from-file-key"
79+
value: "from-file-value"
80+
`
81+
82+
csvYml := `
83+
apiVersion: operators.operatorframework.io/v1alpha1
84+
kind: ClusterServiceVersion
85+
metadata:
86+
name: test.v1.0.0
87+
annotations:
88+
olm.properties: '[{"type":"from-csv-annotations-key", "value":"from-csv-annotations-value"}]'
89+
spec:
90+
installModes:
91+
- type: AllNamespaces
92+
supported: true
93+
`
94+
95+
return fstest.MapFS{
96+
bundlePathAnnotations: &fstest.MapFile{Data: []byte(strings.Trim(annotationsYml, "\n"))},
97+
bundlePathProperties: &fstest.MapFile{Data: []byte(strings.Trim(propertiesYml, "\n"))},
98+
bundlePathCSV: &fstest.MapFile{Data: []byte(strings.Trim(csvYml, "\n"))},
99+
}
100+
}
101+
102+
func removePaths(mapFs fstest.MapFS, paths ...string) fstest.MapFS {
103+
for k := range mapFs {
104+
for _, path := range paths {
105+
if strings.HasPrefix(k, path) {
106+
delete(mapFs, k)
107+
}
108+
}
109+
}
110+
return mapFs
111+
}

0 commit comments

Comments
 (0)