Skip to content

Commit bcf0ace

Browse files
committed
Annotate CSVs with the properties used during dependency resolution.
This ensures that property information is not lost after an operator is installed, and that the properties of an operator are the same before and after installation. Preserving properties also allows installed operators to satisfy dependencies on properties that cannot be inferred from a ClusterServiceVersion spec, and is a step toward unifying installed and available operators from the perspective of resolution.
1 parent c106946 commit bcf0ace

File tree

14 files changed

+347
-13
lines changed

14 files changed

+347
-13
lines changed

deploy/chart/crds/0000_50_olm_00-installplans.crd.yaml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,9 @@ spec:
214214
description: Path refers to the location of a bundle to pull.
215215
It's typically an image reference.
216216
type: string
217+
properties:
218+
description: The effective properties of the unpacked bundle.
219+
type: string
217220
replaces:
218221
description: Replaces is the name of the bundle to replace with
219222
the one found at Path.

pkg/controller/bundle/bundle_unpacker.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"github.com/operator-framework/api/pkg/operators/reference"
2222
operatorsv1alpha1 "github.com/operator-framework/api/pkg/operators/v1alpha1"
2323
listersoperatorsv1alpha1 "github.com/operator-framework/operator-lifecycle-manager/pkg/api/client/listers/operators/v1alpha1"
24+
"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/resolver/projection"
2425
)
2526

2627
type BundleUnpackResult struct {
@@ -355,6 +356,14 @@ func (c *ConfigMapUnpacker) UnpackBundle(lookup *operatorsv1alpha1.BundleLookup)
355356
return
356357
}
357358

359+
if result.BundleLookup.Properties != "" {
360+
props, err := projection.PropertyListFromPropertiesAnnotation(lookup.Properties)
361+
if err != nil {
362+
return nil, fmt.Errorf("failed to load bundle properties for %q: %w", lookup.Identifier, err)
363+
}
364+
result.bundle.Properties = props
365+
}
366+
358367
// A successful load should remove the pending condition
359368
result.RemoveCondition(operatorsv1alpha1.BundleLookupPending)
360369

pkg/controller/operators/catalog/manifests.go

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
v1 "k8s.io/client-go/listers/core/v1"
1212

1313
"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/resolver"
14+
"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/resolver/projection"
1415
)
1516

1617
// ManifestResolver can dereference a manifest for a step. Steps may embed manifests directly or reference content
@@ -49,7 +50,7 @@ func (r *manifestResolver) ManifestForStep(step *v1alpha1.Step) (string, error)
4950
log.WithField("ref", ref).Debug("step is a reference to configmap")
5051

5152
usteps, err := r.unpackedStepsForBundle(step.Resolving, ref)
52-
if err != nil {
53+
if err != nil {
5354
return "", err
5455
}
5556

@@ -85,6 +86,15 @@ func (r *manifestResolver) unpackedStepsForBundle(bundleName string, ref *Unpack
8586
if err != nil {
8687
return nil, errorwrap.Wrapf(err, "error loading unpacked bundle configmap for ref %v", *ref)
8788
}
89+
90+
if ref.Properties != "" {
91+
props, err := projection.PropertyListFromPropertiesAnnotation(ref.Properties)
92+
if err != nil {
93+
return nil, fmt.Errorf("failed to load bundle properties for %q: %w", bundle.CsvName, err)
94+
}
95+
bundle.Properties = props
96+
}
97+
8898
steps, err := resolver.NewStepResourceFromBundle(bundle, r.namespace, ref.Replaces, ref.CatalogSourceName, ref.CatalogSourceNamespace)
8999
if err != nil {
90100
return nil, errorwrap.Wrapf(err, "error calculating steps for ref %v", *ref)

pkg/controller/operators/catalog/operator.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1169,6 +1169,7 @@ type UnpackedBundleReference struct {
11691169
CatalogSourceName string `json:"catalogSourceName"`
11701170
CatalogSourceNamespace string `json:"catalogSourceNamespace"`
11711171
Replaces string `json:"replaces"`
1172+
Properties string `json:"properties"`
11721173
}
11731174

11741175
// unpackBundles makes one walk through the bundlelookups and attempts to progress them
@@ -1214,6 +1215,7 @@ func (o *Operator) unpackBundles(plan *v1alpha1.InstallPlan) (bool, *v1alpha1.In
12141215
CatalogSourceName: res.CatalogSourceRef.Name,
12151216
CatalogSourceNamespace: res.CatalogSourceRef.Namespace,
12161217
Replaces: res.Replaces,
1218+
Properties: res.Properties,
12171219
}
12181220
r, err := json.Marshal(&ref)
12191221
if err != nil {

pkg/controller/registry/resolver/cache.go

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -436,14 +436,27 @@ func WithChannel(channel string) OperatorPredicate {
436436

437437
func WithPackage(pkg string) OperatorPredicate {
438438
return func(o *Operator) bool {
439+
for _, p := range o.Properties() {
440+
if p.Type != opregistry.PackageType {
441+
continue
442+
}
443+
var prop opregistry.PackageProperty
444+
err := json.Unmarshal([]byte(p.Value), &prop)
445+
if err != nil {
446+
continue
447+
}
448+
if prop.PackageName == pkg {
449+
return true
450+
}
451+
}
439452
return o.Package() == pkg
440453
}
441454
}
442455

443456
func WithoutDeprecatedProperty() OperatorPredicate {
444457
return func(o *Operator) bool {
445458
for _, p := range o.bundle.GetProperties() {
446-
if p.GetType() == string(opregistry.DeprecatedType) {
459+
if p.GetType() == opregistry.DeprecatedType {
447460
return false
448461
}
449462
}
@@ -453,6 +466,23 @@ func WithoutDeprecatedProperty() OperatorPredicate {
453466

454467
func WithVersionInRange(r semver.Range) OperatorPredicate {
455468
return func(o *Operator) bool {
469+
for _, p := range o.Properties() {
470+
if p.Type != opregistry.PackageType {
471+
continue
472+
}
473+
var prop opregistry.PackageProperty
474+
err := json.Unmarshal([]byte(p.Value), &prop)
475+
if err != nil {
476+
continue
477+
}
478+
ver, err := semver.Parse(prop.Version)
479+
if err != nil {
480+
continue
481+
}
482+
if r(ver) {
483+
return true
484+
}
485+
}
456486
return o.version != nil && r(*o.version)
457487
}
458488
}

pkg/controller/registry/resolver/operators.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -260,14 +260,14 @@ func NewOperatorFromBundle(bundle *api.Bundle, startingCSV string, sourceKey reg
260260

261261
// legacy support - if the api doesn't contain properties/dependencies, build them from required/provided apis
262262
properties := bundle.Properties
263-
if properties == nil || len(properties) == 0 {
263+
if len(properties) == 0 {
264264
properties, err = apisToProperties(provided)
265265
if err != nil {
266266
return nil, err
267267
}
268268
}
269269
dependencies := bundle.Dependencies
270-
if dependencies == nil || len(dependencies) == 0 {
270+
if len(dependencies) == 0 {
271271
dependencies, err = apisToDependencies(required)
272272
if err != nil {
273273
return nil, err
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
package projection
2+
3+
import (
4+
"encoding/json"
5+
"fmt"
6+
7+
"github.com/operator-framework/operator-registry/pkg/api"
8+
)
9+
10+
const (
11+
PropertiesAnnotationKey = "operatorframework.io/properties"
12+
)
13+
14+
type property struct {
15+
Type string `json:"type"`
16+
Value json.RawMessage `json:"value"`
17+
}
18+
19+
type propertiesAnnotation struct {
20+
Properties []property `json:"properties,omitempty"`
21+
}
22+
23+
func PropertiesAnnotationFromPropertyList(props []*api.Property) (string, error) {
24+
var anno propertiesAnnotation
25+
for _, prop := range props {
26+
anno.Properties = append(anno.Properties, property{
27+
Type: prop.Type,
28+
Value: json.RawMessage(prop.Value),
29+
})
30+
}
31+
v, err := json.Marshal(&anno)
32+
if err != nil {
33+
return "", fmt.Errorf("failed to marshal properties annotation: %w", err)
34+
}
35+
return string(v), nil
36+
}
37+
38+
func PropertyListFromPropertiesAnnotation(raw string) ([]*api.Property, error) {
39+
var anno propertiesAnnotation
40+
if err := json.Unmarshal([]byte(raw), &anno); err != nil {
41+
return nil, fmt.Errorf("failed to unmarshal properties annotation: %w", err)
42+
}
43+
var result []*api.Property
44+
for _, each := range anno.Properties {
45+
result = append(result, &api.Property{
46+
Type: each.Type,
47+
Value: string(each.Value),
48+
})
49+
}
50+
return result, nil
51+
}
Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
package projection_test
2+
3+
import (
4+
"testing"
5+
6+
"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/resolver/projection"
7+
"github.com/operator-framework/operator-registry/pkg/api"
8+
"github.com/stretchr/testify/assert"
9+
)
10+
11+
func TestPropertiesAnnotationFromPropertyList(t *testing.T) {
12+
for _, tc := range []struct {
13+
name string
14+
properties []*api.Property
15+
expected string
16+
error bool
17+
}{
18+
{
19+
name: "nil property slice",
20+
properties: nil,
21+
expected: "{}",
22+
},
23+
{
24+
name: "empty property slice",
25+
properties: []*api.Property{},
26+
expected: "{}",
27+
},
28+
{
29+
name: "invalid property value",
30+
properties: []*api.Property{{
31+
Type: "bad",
32+
Value: `]`,
33+
}},
34+
error: true,
35+
},
36+
{
37+
name: "nonempty property slice",
38+
properties: []*api.Property{
39+
{
40+
Type: "string",
41+
Value: `"hello"`,
42+
},
43+
{
44+
Type: "number",
45+
Value: `5`,
46+
},
47+
{
48+
Type: "array",
49+
Value: `[1,"two",3,"four"]`,
50+
}, {
51+
Type: "object",
52+
Value: `{"hello":{"worl":"d"}}`,
53+
},
54+
},
55+
expected: `{"properties":[{"type":"string","value":"hello"},{"type":"number","value":5},{"type":"array","value":[1,"two",3,"four"]},{"type":"object","value":{"hello":{"worl":"d"}}}]}`,
56+
},
57+
} {
58+
t.Run(tc.name, func(t *testing.T) {
59+
actual, err := projection.PropertiesAnnotationFromPropertyList(tc.properties)
60+
assert := assert.New(t)
61+
assert.Equal(tc.expected, actual)
62+
if tc.error {
63+
assert.Error(err)
64+
} else {
65+
assert.NoError(err)
66+
}
67+
})
68+
}
69+
}
70+
71+
func TestPropertyListFromPropertiesAnnotation(t *testing.T) {
72+
for _, tc := range []struct {
73+
name string
74+
annotation string
75+
expected []*api.Property
76+
error bool
77+
}{
78+
{
79+
name: "empty",
80+
annotation: "",
81+
error: true,
82+
},
83+
{
84+
name: "invalid json",
85+
annotation: "]",
86+
error: true,
87+
},
88+
{
89+
name: "no properties key",
90+
annotation: "{}",
91+
expected: nil,
92+
},
93+
{
94+
name: "properties value not an array or null",
95+
annotation: `{"properties":5}`,
96+
error: true,
97+
},
98+
{
99+
name: "property element not an object",
100+
annotation: `{"properties":[42]}`,
101+
error: true,
102+
},
103+
{
104+
name: "no properties",
105+
annotation: `{"properties":[]}`,
106+
expected: nil,
107+
},
108+
{
109+
name: "several properties",
110+
annotation: `{"properties":[{"type":"string","value":"hello"},{"type":"number","value":5},{"type":"array","value":[1,"two",3,"four"]},{"type":"object","value":{"hello":{"worl":"d"}}}]}`,
111+
expected: []*api.Property{
112+
{
113+
Type: "string",
114+
Value: `"hello"`,
115+
},
116+
{
117+
Type: "number",
118+
Value: `5`,
119+
},
120+
{
121+
Type: "array",
122+
Value: `[1,"two",3,"four"]`,
123+
}, {
124+
Type: "object",
125+
Value: `{"hello":{"worl":"d"}}`,
126+
},
127+
},
128+
},
129+
} {
130+
t.Run(tc.name, func(t *testing.T) {
131+
actual, err := projection.PropertyListFromPropertiesAnnotation(tc.annotation)
132+
assert := assert.New(t)
133+
assert.Equal(tc.expected, actual)
134+
if tc.error {
135+
assert.Error(err)
136+
} else {
137+
assert.NoError(err)
138+
}
139+
})
140+
}
141+
}

0 commit comments

Comments
 (0)