Skip to content
Open
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
27 changes: 22 additions & 5 deletions internal/operator-controller/applier/boxcutter.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import (
ocv1 "github.com/operator-framework/operator-controller/api/v1"
"github.com/operator-framework/operator-controller/internal/operator-controller/authorization"
"github.com/operator-framework/operator-controller/internal/operator-controller/labels"
"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/bundle/source"
"github.com/operator-framework/operator-controller/internal/shared/util/cache"
)

Expand Down Expand Up @@ -105,6 +106,27 @@ func (r *SimpleRevisionGenerator) GenerateRevision(
return nil, err
}

if revisionAnnotations == nil {
revisionAnnotations = map[string]string{}
}

// add bundle properties of interest to revision annotations
bundleAnnotations, err := getBundleAnnotations(bundleFS)
if err != nil {
return nil, fmt.Errorf("error getting bundle annotations: %w", err)
}

// we don't care about all of the bundle and csv annotations as they can be quite confusing
// e.g. 'createdAt', 'capabilities', etc.
for _, key := range []string{
// used by other operators that care about the bundle properties (e.g. maxOpenShiftVersion)
source.PropertyOLMProperties,
} {
if value, ok := bundleAnnotations[key]; ok {
revisionAnnotations[key] = value
}
}

// objectLabels
objs := make([]ocv1.ClusterExtensionRevisionObject, 0, len(plain))
for _, obj := range plain {
Expand Down Expand Up @@ -136,11 +158,6 @@ func (r *SimpleRevisionGenerator) GenerateRevision(
Object: unstr,
})
}

if revisionAnnotations == nil {
revisionAnnotations = map[string]string{}
}

return r.buildClusterExtensionRevision(objs, ext, revisionAnnotations), nil
}

Expand Down
95 changes: 89 additions & 6 deletions internal/operator-controller/applier/boxcutter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,15 @@ import (
"github.com/operator-framework/operator-controller/internal/operator-controller/applier"
"github.com/operator-framework/operator-controller/internal/operator-controller/authorization"
"github.com/operator-framework/operator-controller/internal/operator-controller/labels"
"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util/testing/bundlefs"
"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util/testing/clusterserviceversion"
)

var (
dummyBundle = bundlefs.Builder().
WithPackageName("test-package").
WithCSV(clusterserviceversion.Builder().WithName("test-csv").Build()).
Build()
)

func Test_SimpleRevisionGenerator_GenerateRevisionFromHelmRelease(t *testing.T) {
Expand Down Expand Up @@ -194,7 +203,7 @@ func Test_SimpleRevisionGenerator_GenerateRevision(t *testing.T) {
},
}

rev, err := b.GenerateRevision(t.Context(), fstest.MapFS{}, ext, map[string]string{}, map[string]string{})
rev, err := b.GenerateRevision(t.Context(), dummyBundle, ext, map[string]string{}, map[string]string{})
require.NoError(t, err)

t.Log("by checking the olm.operatorframework.io/owner-name label is set to the name of the ClusterExtension")
Expand Down Expand Up @@ -253,8 +262,82 @@ func Test_SimpleRevisionGenerator_GenerateRevision(t *testing.T) {
}, rev.Spec.Phases)
}

func Test_SimpleRevisionGenerator_GenerateRevision_BundleAnnotations(t *testing.T) {
r := &FakeManifestProvider{
GetFn: func(_ fs.FS, _ *ocv1.ClusterExtension) ([]client.Object, error) {
return []client.Object{}, nil
},
}

b := applier.SimpleRevisionGenerator{
Scheme: k8scheme.Scheme,
ManifestProvider: r,
}

ext := &ocv1.ClusterExtension{
ObjectMeta: metav1.ObjectMeta{
Name: "test-extension",
},
Spec: ocv1.ClusterExtensionSpec{
Namespace: "test-namespace",
ServiceAccount: ocv1.ServiceAccountReference{
Name: "test-sa",
},
},
}

t.Run("bundle properties are copied to the olm.properties annotation", func(t *testing.T) {
bundleFS := bundlefs.Builder().
WithPackageName("test-package").
WithBundleProperty("olm.bundle.property", "some-value").
WithBundleProperty("olm.another.bundle.property", "some-other-value").
WithCSV(clusterserviceversion.Builder().WithName("test-csv").Build()).
Build()

rev, err := b.GenerateRevision(t.Context(), bundleFS, ext, map[string]string{}, map[string]string{})
require.NoError(t, err)

t.Log("by checking bundle properties are added to the revision annotations")
require.NotNil(t, rev.Annotations)
require.JSONEq(t, `[{"type":"olm.bundle.property","value":"some-value"},{"type":"olm.another.bundle.property","value":"some-other-value"}]`, rev.Annotations["olm.properties"])
})

t.Run("olm.properties should not be present if there are no bundle properties", func(t *testing.T) {
bundleFS := bundlefs.Builder().
WithPackageName("test-package").
WithCSV(clusterserviceversion.Builder().WithName("test-csv").Build()).
Build()

rev, err := b.GenerateRevision(t.Context(), bundleFS, ext, map[string]string{}, map[string]string{})
require.NoError(t, err)

t.Log("by checking olm.properties is not present in the revision annotations")
_, ok := rev.Annotations["olm.properties"]
require.False(t, ok, "olm.properties should not be present in the revision annotations")
})

t.Run("csv annotations are not added to the revision annotations", func(t *testing.T) {
bundleFS := bundlefs.Builder().
WithPackageName("test-package").
WithBundleProperty("olm.bundle.property", "some-value").
WithCSV(clusterserviceversion.Builder().
WithName("test-csv").
WithAnnotations(map[string]string{
"some.csv.annotation": "some-other-value",
}).
Build()).
Build()

rev, err := b.GenerateRevision(t.Context(), bundleFS, ext, map[string]string{}, map[string]string{})
require.NoError(t, err)

t.Log("by checking csv annotations are not added to the revision annotations")
_, ok := rev.Annotations["olm.csv.annotation"]
require.False(t, ok, "csv annotation should not be present in the revision annotations")
})
}

func Test_SimpleRevisionGenerator_Renderer_Integration(t *testing.T) {
bundleFS := fstest.MapFS{}
ext := &ocv1.ClusterExtension{
ObjectMeta: metav1.ObjectMeta{
Name: "test-extension",
Expand All @@ -263,7 +346,7 @@ func Test_SimpleRevisionGenerator_Renderer_Integration(t *testing.T) {
r := &FakeManifestProvider{
GetFn: func(b fs.FS, e *ocv1.ClusterExtension) ([]client.Object, error) {
t.Log("by checking renderer was called with the correct parameters")
require.Equal(t, bundleFS, b)
require.Equal(t, dummyBundle, b)
require.Equal(t, ext, e)
return nil, nil
},
Expand All @@ -273,7 +356,7 @@ func Test_SimpleRevisionGenerator_Renderer_Integration(t *testing.T) {
ManifestProvider: r,
}

_, err := b.GenerateRevision(t.Context(), bundleFS, ext, map[string]string{}, map[string]string{})
_, err := b.GenerateRevision(t.Context(), dummyBundle, ext, map[string]string{}, map[string]string{})
require.NoError(t, err)
}

Expand Down Expand Up @@ -311,7 +394,7 @@ func Test_SimpleRevisionGenerator_AppliesObjectLabelsAndRevisionAnnotations(t *t
"other": "value",
}

rev, err := b.GenerateRevision(t.Context(), fstest.MapFS{}, &ocv1.ClusterExtension{
rev, err := b.GenerateRevision(t.Context(), dummyBundle, &ocv1.ClusterExtension{
Spec: ocv1.ClusterExtensionSpec{
Namespace: "test-namespace",
ServiceAccount: ocv1.ServiceAccountReference{Name: "test-sa"},
Expand Down Expand Up @@ -385,7 +468,7 @@ func Test_SimpleRevisionGenerator_PropagatesProgressDeadlineMinutes(t *testing.T
ext.Spec.ProgressDeadlineMinutes = *pd
}

rev, err := b.GenerateRevision(t.Context(), fstest.MapFS{}, ext, empty, empty)
rev, err := b.GenerateRevision(t.Context(), dummyBundle, ext, empty, empty)
require.NoError(t, err)
require.Equal(t, tc.want.progressDeadlineMinutes, rev.Spec.ProgressDeadlineMinutes)
})
Expand Down
14 changes: 12 additions & 2 deletions internal/operator-controller/applier/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,6 @@ func (r *RegistryV1ManifestProvider) Get(bundleFS fs.FS, ext *ocv1.ClusterExtens
opts = append(opts, render.WithTargetNamespaces(*watchNS))
}
}

return r.BundleRenderer.Render(rv1, ext.Spec.Namespace, opts...)
}

Expand All @@ -101,7 +100,7 @@ func (r *RegistryV1HelmChartProvider) Get(bundleFS fs.FS, ext *ocv1.ClusterExten

chrt := &chart.Chart{Metadata: &chart.Metadata{}}
// The need to get the underlying bundle in order to extract its annotations
// will go away once with have a bundle interface that can surface the annotations independently of the
// will go away once we have a bundle interface that can surface the annotations independently of the
// underlying bundle format...
rv1, err := source.FromFS(bundleFS).GetBundle()
if err != nil {
Expand Down Expand Up @@ -148,3 +147,14 @@ func extensionConfigBytes(ext *ocv1.ClusterExtension) []byte {
}
return nil
}

func getBundleAnnotations(bundleFS fs.FS) (map[string]string, error) {
// The need to get the underlying bundle in order to extract its annotations
// will go away once with have a bundle interface that can surface the annotations independently of the
// underlying bundle format...
rv1, err := source.FromFS(bundleFS).GetBundle()
if err != nil {
return nil, err
}
return rv1.CSV.GetAnnotations(), nil
}
9 changes: 8 additions & 1 deletion internal/operator-controller/rukpak/bundle/source/source.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ import (
registry "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/operator-registry"
)

const (
PropertyOLMProperties = "olm.properties"
)

type BundleSource interface {
GetBundle() (bundle.RegistryV1, error)
}
Expand Down Expand Up @@ -173,6 +177,9 @@ func copyMetadataPropertiesToCSV(csv *v1alpha1.ClusterServiceVersion, fsys fs.FS
if err != nil {
return fmt.Errorf("failed to marshal registry+v1 properties to json: %w", err)
}
csv.Annotations["olm.properties"] = string(allPropertiesJSON)
if csv.Annotations == nil {
csv.Annotations = map[string]string{}
}
csv.Annotations[PropertyOLMProperties] = string(allPropertiesJSON)
return nil
}
27 changes: 27 additions & 0 deletions test/e2e/features/install.feature
Original file line number Diff line number Diff line change
Expand Up @@ -329,3 +329,30 @@ Feature: Install ClusterExtension
Revision has not rolled out for 1 minutes.
"""
And ClusterExtension reports Progressing transition between 1 and 2 minutes since its creation

@BoxcutterRuntime
Scenario: ClusterExtensionRevision is annotated with bundle properties and csv annotations
When ClusterExtension is applied
"""
apiVersion: olm.operatorframework.io/v1
kind: ClusterExtension
metadata:
name: ${NAME}
spec:
namespace: ${TEST_NAMESPACE}
serviceAccount:
name: olm-sa
source:
sourceType: Catalog
catalog:
packageName: test
version: 1.2.0
selector:
matchLabels:
"olm.operatorframework.io/metadata.name": test-catalog
"""
# The annotation key and value come from the bundle's metadata/properties.yaml file
Then ClusterExtensionRevision "${NAME}-1" contains annotation "olm.properties" with value
"""
[{"type":"olm.test-property","value":"some-value"}]
"""
25 changes: 25 additions & 0 deletions test/e2e/steps/steps.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ func RegisterSteps(sc *godog.ScenarioContext) {
sc.Step(`^(?i)ClusterExtensionRevision "([^"]+)" reports ([[:alnum:]]+) as ([[:alnum:]]+) with Reason ([[:alnum:]]+)$`, ClusterExtensionRevisionReportsConditionWithoutMsg)
sc.Step(`^(?i)ClusterExtension reports ([[:alnum:]]+) transition between (\d+) and (\d+) minutes since its creation$`, ClusterExtensionReportsConditionTransitionTime)
sc.Step(`^(?i)ClusterExtensionRevision "([^"]+)" is archived$`, ClusterExtensionRevisionIsArchived)
sc.Step(`^(?i)ClusterExtensionRevision "([^"]+)" contains annotation "([^"]+)" with value$`, ClusterExtensionRevisionHasAnnotationWithValue)

sc.Step(`^(?i)resource "([^"]+)" is installed$`, ResourceAvailable)
sc.Step(`^(?i)resource "([^"]+)" is available$`, ResourceAvailable)
Expand Down Expand Up @@ -485,6 +486,30 @@ func ClusterExtensionRevisionIsArchived(ctx context.Context, revisionName string
return waitForCondition(ctx, "clusterextensionrevision", substituteScenarioVars(revisionName, scenarioCtx(ctx)), "Progressing", "False", ptr.To("Archived"), nil)
}

func ClusterExtensionRevisionHasAnnotationWithValue(ctx context.Context, revisionName, annotationKey string, annotationValue *godog.DocString) error {
sc := scenarioCtx(ctx)
revisionName = substituteScenarioVars(strings.TrimSpace(revisionName), sc)
expectedValue := ""
if annotationValue != nil {
expectedValue = annotationValue.Content
}
waitFor(ctx, func() bool {
obj, err := getResource("clusterextensionrevision", revisionName, "")
if err != nil {
logger.V(1).Error(err, "failed to get clusterextensionrevision", "name", revisionName)
return false
}
if obj.GetAnnotations() == nil {
return false
}
if obj.GetAnnotations()[annotationKey] != expectedValue {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the rest of the function could collapsed into

return obj.GetAnnotations()[annotationKey] == expectedValue

return false
}
return true
})
return nil
}

func ResourceAvailable(ctx context.Context, resource string) error {
sc := scenarioCtx(ctx)
resource = substituteScenarioVars(resource, sc)
Expand Down
Loading