From 409ec70c6c889945acb0447397305aa5c2150ea2 Mon Sep 17 00:00:00 2001 From: Steve Kuznetsov Date: Thu, 3 Aug 2023 10:12:23 -0400 Subject: [PATCH] operators/v1alpha1: expose CSV copied logic (#289) We want to use a partial object metadata query for CSVs to reduce resource utilization. We still want to ask questions about whether these CSVs are copied, so we need to refactor this method to take only object metadata. While it's not a perfect port of the previous logic, it defies explanation how an object would have all the other markings of being copied but the wrong status. Signed-off-by: Steve Kuznetsov --- .../v1alpha1/clusterserviceversion.go | 15 +++-- .../v1alpha1/clusterserviceversion_test.go | 65 +++++++++++++++++++ 2 files changed, 76 insertions(+), 4 deletions(-) diff --git a/pkg/operators/v1alpha1/clusterserviceversion.go b/pkg/operators/v1alpha1/clusterserviceversion.go index ffc357b12..a4c8d1746 100644 --- a/pkg/operators/v1alpha1/clusterserviceversion.go +++ b/pkg/operators/v1alpha1/clusterserviceversion.go @@ -120,12 +120,19 @@ func (c *ClusterServiceVersion) IsObsolete() bool { // IsCopied returns true if the CSV has been copied and false otherwise. func (c *ClusterServiceVersion) IsCopied() bool { - operatorNamespace, ok := c.GetAnnotations()[OperatorGroupNamespaceAnnotationKey] - if c.Status.Reason == CSVReasonCopied || ok && c.GetNamespace() != operatorNamespace { - return true + return c.Status.Reason == CSVReasonCopied || IsCopied(c) +} + +func IsCopied(o metav1.Object) bool { + annotations := o.GetAnnotations() + if annotations != nil { + operatorNamespace, ok := annotations[OperatorGroupNamespaceAnnotationKey] + if ok && o.GetNamespace() != operatorNamespace { + return true + } } - if labels := c.GetLabels(); labels != nil { + if labels := o.GetLabels(); labels != nil { if _, ok := labels[CopiedLabelKey]; ok { return true } diff --git a/pkg/operators/v1alpha1/clusterserviceversion_test.go b/pkg/operators/v1alpha1/clusterserviceversion_test.go index 26136c99d..0f73c906b 100644 --- a/pkg/operators/v1alpha1/clusterserviceversion_test.go +++ b/pkg/operators/v1alpha1/clusterserviceversion_test.go @@ -430,3 +430,68 @@ func helperNewConditions(count int) []ClusterServiceVersionCondition { return conditions } + +func TestIsCopied(t *testing.T) { + var testCases = []struct { + name string + input metav1.Object + expected bool + }{ + { + name: "no labels or annotations", + input: &metav1.ObjectMeta{}, + expected: false, + }, + { + name: "no labels, has annotations but missing operatorgroup namespace annotation", + input: &metav1.ObjectMeta{ + Annotations: map[string]string{}, + }, + expected: false, + }, + { + name: "no labels, has operatorgroup namespace annotation matching self", + input: &metav1.ObjectMeta{ + Namespace: "whatever", + Annotations: map[string]string{ + "olm.operatorNamespace": "whatever", + }, + }, + expected: false, + }, + { + name: "no labels, has operatorgroup namespace annotation not matching self", + input: &metav1.ObjectMeta{ + Namespace: "whatever", + Annotations: map[string]string{ + "olm.operatorNamespace": "other", + }, + }, + expected: true, + }, + { + name: "no annotations, labels missing copied key", + input: &metav1.ObjectMeta{ + Labels: map[string]string{}, + }, + expected: false, + }, + { + name: "no annotations, labels has copied key", + input: &metav1.ObjectMeta{ + Labels: map[string]string{ + "olm.copiedFrom": "whatever", + }, + }, + expected: true, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + if got, expected := IsCopied(testCase.input), testCase.expected; got != expected { + t.Errorf("got %v, expected %v", got, expected) + } + }) + } +}