diff --git a/internal/operator-controller/applier/boxcutter.go b/internal/operator-controller/applier/boxcutter.go index f71650329f..15017af5ea 100644 --- a/internal/operator-controller/applier/boxcutter.go +++ b/internal/operator-controller/applier/boxcutter.go @@ -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" ) @@ -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 { @@ -136,11 +158,6 @@ func (r *SimpleRevisionGenerator) GenerateRevision( Object: unstr, }) } - - if revisionAnnotations == nil { - revisionAnnotations = map[string]string{} - } - return r.buildClusterExtensionRevision(objs, ext, revisionAnnotations), nil } diff --git a/internal/operator-controller/applier/boxcutter_test.go b/internal/operator-controller/applier/boxcutter_test.go index 02194550f2..d433965324 100644 --- a/internal/operator-controller/applier/boxcutter_test.go +++ b/internal/operator-controller/applier/boxcutter_test.go @@ -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) { @@ -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") @@ -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", @@ -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 }, @@ -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) } @@ -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"}, @@ -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) }) diff --git a/internal/operator-controller/applier/provider.go b/internal/operator-controller/applier/provider.go index cf75b28c87..da22067c94 100644 --- a/internal/operator-controller/applier/provider.go +++ b/internal/operator-controller/applier/provider.go @@ -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...) } @@ -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 { @@ -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 +} diff --git a/internal/operator-controller/rukpak/bundle/source/source.go b/internal/operator-controller/rukpak/bundle/source/source.go index 0f5d3f185c..49ed0c32bf 100644 --- a/internal/operator-controller/rukpak/bundle/source/source.go +++ b/internal/operator-controller/rukpak/bundle/source/source.go @@ -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) } @@ -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 } diff --git a/test/e2e/features/install.feature b/test/e2e/features/install.feature index ab87b5f31c..d6f3a01562 100644 --- a/test/e2e/features/install.feature +++ b/test/e2e/features/install.feature @@ -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"}] + """ diff --git a/test/e2e/steps/steps.go b/test/e2e/steps/steps.go index 9bcffa159d..1605eef58f 100644 --- a/test/e2e/steps/steps.go +++ b/test/e2e/steps/steps.go @@ -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) @@ -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 { + return false + } + return true + }) + return nil +} + func ResourceAvailable(ctx context.Context, resource string) error { sc := scenarioCtx(ctx) resource = substituteScenarioVars(resource, sc)