Merge pull request #1781 from benluddy/property-projection

Bug 1881222: Annotate CSVs with the properties used during dependency resolution.

Author: openshift-merge-robot

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

@@ -214,6 +214,9 @@ spec:
                       description: Path refers to the location of a bundle to pull.
                         It's typically an image reference.
                       type: string
+                    properties:
+                      description: The effective properties of the unpacked bundle.
+                      type: string
                     replaces:
                       description: Replaces is the name of the bundle to replace with
                         the one found at Path.

go.mod

@@ -23,7 +23,7 @@ require (
 	github.com/onsi/gomega v1.9.0
 	github.com/openshift/api v0.0.0-20200331152225-585af27e34fd
 	github.com/openshift/client-go v0.0.0-20200326155132-2a6cd50aedd0
-	github.com/operator-framework/api v0.3.15
+	github.com/operator-framework/api v0.3.16
 	github.com/operator-framework/operator-registry v1.13.6
 	github.com/otiai10/copy v1.2.0
 	github.com/pkg/errors v0.9.1

go.sum

@@ -599,8 +599,8 @@ github.com/openshift/client-go v0.0.0-20200326155132-2a6cd50aedd0 h1:kMiuiZXH1Gd
 github.com/openshift/client-go v0.0.0-20200326155132-2a6cd50aedd0/go.mod h1:uUQ4LClRO+fg5MF/P6QxjMCb1C9f7Oh4RKepftDnEJE=
 github.com/openzipkin/zipkin-go v0.1.6/go.mod h1:QgAqvLzwWbR/WpD4A3cGpPtJrZXNIiJc5AZX7/PBEpw=
 github.com/operator-framework/api v0.3.7-0.20200602203552-431198de9fc2/go.mod h1:Xbje9x0SHmh0nihE21kpesB38vk3cyxnE6JdDS8Jo1Q=
-github.com/operator-framework/api v0.3.15 h1:gVVx9y8o9RpY8+bzSCo+WLi+LZylYI8zp4Gtlei8Mac=
-github.com/operator-framework/api v0.3.15/go.mod h1:Xbje9x0SHmh0nihE21kpesB38vk3cyxnE6JdDS8Jo1Q=
+github.com/operator-framework/api v0.3.16 h1:rMePhgntSMdU5mIifF78WaQa99oGR/RKQ4H6+Hrpv+o=
+github.com/operator-framework/api v0.3.16/go.mod h1:Xbje9x0SHmh0nihE21kpesB38vk3cyxnE6JdDS8Jo1Q=
 github.com/operator-framework/operator-registry v1.13.6 h1:h/dIjQQS7uneQNRifrSz7h0xg4Xyjg6C9f6XZofbMPg=
 github.com/operator-framework/operator-registry v1.13.6/go.mod h1:YhnIzOVjRU2ZwZtzt+fjcjW8ujJaSFynBEu7QVKaSdU=
 github.com/otiai10/copy v1.2.0 h1:HvG945u96iNadPoG2/Ja2+AUJeW5YuFQMixq9yirC+k=

pkg/controller/bundle/bundle_unpacker.go

@@ -21,6 +21,7 @@ import (
 	"github.com/operator-framework/api/pkg/operators/reference"
 	operatorsv1alpha1 "github.com/operator-framework/api/pkg/operators/v1alpha1"
 	listersoperatorsv1alpha1 "github.com/operator-framework/operator-lifecycle-manager/pkg/api/client/listers/operators/v1alpha1"
+	"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/resolver/projection"
 )
 
 type BundleUnpackResult struct {
@@ -355,6 +356,14 @@ func (c *ConfigMapUnpacker) UnpackBundle(lookup *operatorsv1alpha1.BundleLookup)
 		return
 	}
 
+	if result.BundleLookup.Properties != "" {
+		props, err := projection.PropertyListFromPropertiesAnnotation(lookup.Properties)
+		if err != nil {
+			return nil, fmt.Errorf("failed to load bundle properties for %q: %w", lookup.Identifier, err)
+		}
+		result.bundle.Properties = props
+	}
+
 	// A successful load should remove the pending condition
 	result.RemoveCondition(operatorsv1alpha1.BundleLookupPending)
 

pkg/controller/operators/catalog/manifests.go

@@ -11,6 +11,7 @@ import (
 	v1 "k8s.io/client-go/listers/core/v1"
 
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/resolver"
+	"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/resolver/projection"
 )
 
 // 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)
 	log.WithField("ref", ref).Debug("step is a reference to configmap")
 
 	usteps, err := r.unpackedStepsForBundle(step.Resolving, ref)
-	if err != nil  {
+	if err != nil {
 		return "", err
 	}
 
@@ -85,6 +86,15 @@ func (r *manifestResolver) unpackedStepsForBundle(bundleName string, ref *Unpack
 	if err != nil {
 		return nil, errorwrap.Wrapf(err, "error loading unpacked bundle configmap for ref %v", *ref)
 	}
+
+	if ref.Properties != "" {
+		props, err := projection.PropertyListFromPropertiesAnnotation(ref.Properties)
+		if err != nil {
+			return nil, fmt.Errorf("failed to load bundle properties for %q: %w", bundle.CsvName, err)
+		}
+		bundle.Properties = props
+	}
+
 	steps, err := resolver.NewStepResourceFromBundle(bundle, r.namespace, ref.Replaces, ref.CatalogSourceName, ref.CatalogSourceNamespace)
 	if err != nil {
 		return nil, errorwrap.Wrapf(err, "error calculating steps for ref %v", *ref)

pkg/controller/operators/catalog/operator.go

@@ -1169,6 +1169,7 @@ type UnpackedBundleReference struct {
 	CatalogSourceName      string `json:"catalogSourceName"`
 	CatalogSourceNamespace string `json:"catalogSourceNamespace"`
 	Replaces               string `json:"replaces"`
+	Properties             string `json:"properties"`
 }
 
 // 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
 				CatalogSourceName:      res.CatalogSourceRef.Name,
 				CatalogSourceNamespace: res.CatalogSourceRef.Namespace,
 				Replaces:               res.Replaces,
+				Properties:             res.Properties,
 			}
 			r, err := json.Marshal(&ref)
 			if err != nil {

pkg/controller/registry/resolver/cache.go

@@ -436,14 +436,27 @@ func WithChannel(channel string) OperatorPredicate {
 
 func WithPackage(pkg string) OperatorPredicate {
 	return func(o *Operator) bool {
+		for _, p := range o.Properties() {
+			if p.Type != opregistry.PackageType {
+				continue
+			}
+			var prop opregistry.PackageProperty
+			err := json.Unmarshal([]byte(p.Value), &prop)
+			if err != nil {
+				continue
+			}
+			if prop.PackageName == pkg {
+				return true
+			}
+		}
 		return o.Package() == pkg
 	}
 }
 
 func WithoutDeprecatedProperty() OperatorPredicate {
 	return func(o *Operator) bool {
 		for _, p := range o.bundle.GetProperties() {
-			if p.GetType() == string(opregistry.DeprecatedType) {
+			if p.GetType() == opregistry.DeprecatedType {
 				return false
 			}
 		}
@@ -453,6 +466,23 @@ func WithoutDeprecatedProperty() OperatorPredicate {
 
 func WithVersionInRange(r semver.Range) OperatorPredicate {
 	return func(o *Operator) bool {
+		for _, p := range o.Properties() {
+			if p.Type != opregistry.PackageType {
+				continue
+			}
+			var prop opregistry.PackageProperty
+			err := json.Unmarshal([]byte(p.Value), &prop)
+			if err != nil {
+				continue
+			}
+			ver, err := semver.Parse(prop.Version)
+			if err != nil {
+				continue
+			}
+			if r(ver) {
+				return true
+			}
+		}
 		return o.version != nil && r(*o.version)
 	}
 }

pkg/controller/registry/resolver/operators.go

@@ -260,14 +260,14 @@ func NewOperatorFromBundle(bundle *api.Bundle, startingCSV string, sourceKey reg
 
 	// legacy support - if the api doesn't contain properties/dependencies, build them from required/provided apis
 	properties := bundle.Properties
-	if properties == nil || len(properties) == 0 {
+	if len(properties) == 0 {
 		properties, err = apisToProperties(provided)
 		if err != nil {
 			return nil, err
 		}
 	}
 	dependencies := bundle.Dependencies
-	if dependencies == nil || len(dependencies) == 0 {
+	if len(dependencies) == 0 {
 		dependencies, err = apisToDependencies(required)
 		if err != nil {
 			return nil, err

pkg/controller/registry/resolver/projection/properties.go

@@ -0,0 +1,51 @@
+package projection
+
+import (
+	"encoding/json"
+	"fmt"
+
+	"github.com/operator-framework/operator-registry/pkg/api"
+)
+
+const (
+	PropertiesAnnotationKey = "operatorframework.io/properties"
+)
+
+type property struct {
+	Type  string          `json:"type"`
+	Value json.RawMessage `json:"value"`
+}
+
+type propertiesAnnotation struct {
+	Properties []property `json:"properties,omitempty"`
+}
+
+func PropertiesAnnotationFromPropertyList(props []*api.Property) (string, error) {
+	var anno propertiesAnnotation
+	for _, prop := range props {
+		anno.Properties = append(anno.Properties, property{
+			Type:  prop.Type,
+			Value: json.RawMessage(prop.Value),
+		})
+	}
+	v, err := json.Marshal(&anno)
+	if err != nil {
+		return "", fmt.Errorf("failed to marshal properties annotation: %w", err)
+	}
+	return string(v), nil
+}
+
+func PropertyListFromPropertiesAnnotation(raw string) ([]*api.Property, error) {
+	var anno propertiesAnnotation
+	if err := json.Unmarshal([]byte(raw), &anno); err != nil {
+		return nil, fmt.Errorf("failed to unmarshal properties annotation: %w", err)
+	}
+	var result []*api.Property
+	for _, each := range anno.Properties {
+		result = append(result, &api.Property{
+			Type:  each.Type,
+			Value: string(each.Value),
+		})
+	}
+	return result, nil
+}

pkg/controller/registry/resolver/projection/properties_test.go

@@ -0,0 +1,141 @@
+package projection_test
+
+import (
+	"testing"
+
+	"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/resolver/projection"
+	"github.com/operator-framework/operator-registry/pkg/api"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestPropertiesAnnotationFromPropertyList(t *testing.T) {
+	for _, tc := range []struct {
+		name       string
+		properties []*api.Property
+		expected   string
+		error      bool
+	}{
+		{
+			name:       "nil property slice",
+			properties: nil,
+			expected:   "{}",
+		},
+		{
+			name:       "empty property slice",
+			properties: []*api.Property{},
+			expected:   "{}",
+		},
+		{
+			name: "invalid property value",
+			properties: []*api.Property{{
+				Type:  "bad",
+				Value: `]`,
+			}},
+			error: true,
+		},
+		{
+			name: "nonempty property slice",
+			properties: []*api.Property{
+				{
+					Type:  "string",
+					Value: `"hello"`,
+				},
+				{
+					Type:  "number",
+					Value: `5`,
+				},
+				{
+					Type:  "array",
+					Value: `[1,"two",3,"four"]`,
+				}, {
+					Type:  "object",
+					Value: `{"hello":{"worl":"d"}}`,
+				},
+			},
+			expected: `{"properties":[{"type":"string","value":"hello"},{"type":"number","value":5},{"type":"array","value":[1,"two",3,"four"]},{"type":"object","value":{"hello":{"worl":"d"}}}]}`,
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			actual, err := projection.PropertiesAnnotationFromPropertyList(tc.properties)
+			assert := assert.New(t)
+			assert.Equal(tc.expected, actual)
+			if tc.error {
+				assert.Error(err)
+			} else {
+				assert.NoError(err)
+			}
+		})
+	}
+}
+
+func TestPropertyListFromPropertiesAnnotation(t *testing.T) {
+	for _, tc := range []struct {
+		name       string
+		annotation string
+		expected   []*api.Property
+		error      bool
+	}{
+		{
+			name:       "empty",
+			annotation: "",
+			error:      true,
+		},
+		{
+			name:       "invalid json",
+			annotation: "]",
+			error:      true,
+		},
+		{
+			name:       "no properties key",
+			annotation: "{}",
+			expected:   nil,
+		},
+		{
+			name:       "properties value not an array or null",
+			annotation: `{"properties":5}`,
+			error:      true,
+		},
+		{
+			name:       "property element not an object",
+			annotation: `{"properties":[42]}`,
+			error:      true,
+		},
+		{
+			name:       "no properties",
+			annotation: `{"properties":[]}`,
+			expected:   nil,
+		},
+		{
+			name:       "several properties",
+			annotation: `{"properties":[{"type":"string","value":"hello"},{"type":"number","value":5},{"type":"array","value":[1,"two",3,"four"]},{"type":"object","value":{"hello":{"worl":"d"}}}]}`,
+			expected: []*api.Property{
+				{
+					Type:  "string",
+					Value: `"hello"`,
+				},
+				{
+					Type:  "number",
+					Value: `5`,
+				},
+				{
+					Type:  "array",
+					Value: `[1,"two",3,"four"]`,
+				}, {
+					Type:  "object",
+					Value: `{"hello":{"worl":"d"}}`,
+				},
+			},
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			actual, err := projection.PropertyListFromPropertiesAnnotation(tc.annotation)
+			assert := assert.New(t)
+			assert.Equal(tc.expected, actual)
+			if tc.error {
+				assert.Error(err)
+			} else {
+				assert.NoError(err)
+			}
+		})
+	}
+}