Merge branch 'openshift:master' into master

Author: grokspawn

staging/operator-lifecycle-manager/pkg/controller/operators/decorators/operator.go

@@ -2,6 +2,7 @@ package decorators
 
 import (
 	"fmt"
+	"sort"
 	"strings"
 
 	"github.com/itchyny/gojq"
@@ -331,6 +332,23 @@ func (o *Operator) AddComponents(components ...runtime.Object) error {
 
 	o.Status.Components.Refs = append(o.Status.Components.Refs, refs...)
 
+	// Sort the component refs to so subsequent reconciles of the object do not change
+	// the status and result in an update to the object.
+	sort.SliceStable(o.Status.Components.Refs, func(i, j int) bool {
+		if o.Status.Components.Refs[i].Kind != o.Status.Components.Refs[j].Kind {
+			return o.Status.Components.Refs[i].Kind < o.Status.Components.Refs[j].Kind
+		}
+
+		if o.Status.Components.Refs[i].APIVersion != o.Status.Components.Refs[j].APIVersion {
+			return o.Status.Components.Refs[i].APIVersion < o.Status.Components.Refs[j].APIVersion
+		}
+
+		if o.Status.Components.Refs[i].Namespace != o.Status.Components.Refs[j].Namespace {
+			return o.Status.Components.Refs[i].Namespace < o.Status.Components.Refs[j].Namespace
+		}
+		return o.Status.Components.Refs[i].Name < o.Status.Components.Refs[j].Name
+	})
+
 	return nil
 }
 

staging/operator-lifecycle-manager/pkg/controller/operators/decorators/operator_test.go

@@ -174,6 +174,22 @@ func TestAddComponents(t *testing.T) {
 						},
 					}
 					operator.Status.Components.Refs = []operatorsv1.RichReference{
+						{
+							ObjectReference: &corev1.ObjectReference{
+								APIVersion: operatorsv1alpha1.SchemeGroupVersion.String(),
+								Kind:       operatorsv1alpha1.ClusterServiceVersionKind,
+								Namespace:  "atlantic",
+								Name:       "puffin",
+							},
+							Conditions: []operatorsv1.Condition{
+								{
+									Type:    operatorsv1.ConditionType(operatorsv1alpha1.CSVPhaseSucceeded),
+									Status:  corev1.ConditionTrue,
+									Reason:  string(operatorsv1alpha1.CSVReasonInstallSuccessful),
+									Message: "this puffin is happy",
+								},
+							},
+						},
 						{
 							ObjectReference: &corev1.ObjectReference{
 								APIVersion: "v1",
@@ -195,22 +211,6 @@ func TestAddComponents(t *testing.T) {
 								},
 							},
 						},
-						{
-							ObjectReference: &corev1.ObjectReference{
-								APIVersion: operatorsv1alpha1.SchemeGroupVersion.String(),
-								Kind:       operatorsv1alpha1.ClusterServiceVersionKind,
-								Namespace:  "atlantic",
-								Name:       "puffin",
-							},
-							Conditions: []operatorsv1.Condition{
-								{
-									Type:    operatorsv1.ConditionType(operatorsv1alpha1.CSVPhaseSucceeded),
-									Status:  corev1.ConditionTrue,
-									Reason:  string(operatorsv1alpha1.CSVReasonInstallSuccessful),
-									Message: "this puffin is happy",
-								},
-							},
-						},
 					}
 
 					return operator

staging/operator-lifecycle-manager/pkg/controller/operators/operator_controller.go

@@ -9,6 +9,7 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
+	"k8s.io/apimachinery/pkg/api/equality"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
 	"k8s.io/apimachinery/pkg/labels"
@@ -152,9 +153,11 @@ func (r *OperatorReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
 			return ctrl.Result{Requeue: true}, nil
 		}
 	} else {
-		if err := r.Status().Update(ctx, operator.Operator); err != nil {
-			log.Error(err, "Could not update Operator status")
-			return ctrl.Result{Requeue: true}, nil
+		if !equality.Semantic.DeepEqual(in.Status, operator.Operator.Status) {
+			if err := r.Status().Update(ctx, operator.Operator); err != nil {
+				log.Error(err, "Could not update Operator status")
+				return ctrl.Result{Requeue: true}, nil
+			}
 		}
 	}
 

staging/operator-lifecycle-manager/pkg/controller/operators/operator_controller_test.go

@@ -2,6 +2,7 @@ package operators
 
 import (
 	"context"
+	"fmt"
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
@@ -204,6 +205,44 @@ var _ = Describe("Operator Controller", func() {
 				})
 			})
 
+			Context("when multiple types of a gvk are labeled", func() {
+				BeforeEach(func() {
+					newObjs := make([]runtime.Object, 9)
+
+					// Create objects in reverse order to ensure they are eventually ordered alphabetically in the status of the operator.
+					for i := 8; i >= 0; i-- {
+						newObjs[8-i] = testobj.WithLabels(map[string]string{expectedKey: ""}, testobj.WithNamespacedName(
+							&types.NamespacedName{Namespace: namespace, Name: fmt.Sprintf("sa-%d", i)}, &corev1.ServiceAccount{},
+						))
+					}
+
+					for _, obj := range newObjs {
+						Expect(k8sClient.Create(ctx, obj.(client.Object))).To(Succeed())
+					}
+
+					objs = append(objs, newObjs...)
+					expectedRefs = append(expectedRefs, toRefs(scheme, newObjs...)...)
+				})
+
+				It("should list each of the component references in alphabetical order by namespace and name", func() {
+					Eventually(func() ([]operatorsv1.RichReference, error) {
+						err := k8sClient.Get(ctx, name, operator)
+						return operator.Status.Components.Refs, err
+					}, timeout, interval).Should(ConsistOf(expectedRefs))
+
+					serviceAccountCount := 0
+					for _, ref := range operator.Status.Components.Refs {
+						if ref.Kind != rbacv1.ServiceAccountKind {
+							continue
+						}
+
+						Expect(ref.Name).Should(Equal(fmt.Sprintf("sa-%d", serviceAccountCount)))
+						serviceAccountCount++
+					}
+					Expect(serviceAccountCount).To(Equal(9))
+				})
+			})
+
 			Context("when component labels are removed", func() {
 
 				BeforeEach(func() {

staging/operator-lifecycle-manager/pkg/lib/scoped/token_retriever.go

@@ -82,6 +82,12 @@ func getAPISecret(logger logrus.FieldLogger, kubeclient operatorclient.ClientInt
 func filterSecretsBySAName(saName string, secrets *corev1.SecretList) map[string]*corev1.Secret {
 	secretMap := make(map[string]*corev1.Secret)
 	for _, ref := range secrets.Items {
+		// Avoid referencing the "ref" created by the range-for loop as
+		// the secret stored in the map will change if there are more
+		// entries in the list of secrets that the range-for loop is
+		// iterating over.
+		ref := ref
+
 		annotations := ref.GetAnnotations()
 		value := annotations[corev1.ServiceAccountNameKey]
 		if value == saName {

staging/operator-lifecycle-manager/pkg/lib/scoped/token_retriever_test.go

@@ -0,0 +1,117 @@
+package scoped
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+const serviceAccountName = "foo"
+
+func TestFilterSecretsBySAName(t *testing.T) {
+	tests := []struct {
+		name              string
+		secrets           *corev1.SecretList
+		wantedSecretNames []string
+	}{
+		{
+			name: "NoSecretFound",
+			secrets: &corev1.SecretList{
+				Items: []corev1.Secret{
+					*newSecret("aSecret"),
+					*newSecret("someSecret"),
+					*newSecret("zSecret"),
+				},
+			},
+			wantedSecretNames: []string{},
+		},
+		{
+			name: "FirstSecretFound",
+			secrets: &corev1.SecretList{
+				Items: []corev1.Secret{
+					*newSecret("aSecret", withAnnotations(map[string]string{corev1.ServiceAccountNameKey: serviceAccountName})),
+					*newSecret("someSecret"),
+					*newSecret("zSecret"),
+				},
+			},
+			wantedSecretNames: []string{"aSecret"},
+		},
+		{
+			name: "SecondSecretFound",
+			secrets: &corev1.SecretList{
+				Items: []corev1.Secret{
+					*newSecret("aSecret"),
+					*newSecret("someSecret", withAnnotations(map[string]string{corev1.ServiceAccountNameKey: serviceAccountName})),
+					*newSecret("zSecret"),
+				},
+			},
+			wantedSecretNames: []string{"someSecret"},
+		},
+		{
+			name: "ThirdSecretFound",
+			secrets: &corev1.SecretList{
+				Items: []corev1.Secret{
+					*newSecret("aSecret"),
+					*newSecret("someSecret"),
+					*newSecret("zSecret", withAnnotations(map[string]string{corev1.ServiceAccountNameKey: serviceAccountName})),
+				},
+			},
+			wantedSecretNames: []string{"zSecret"},
+		},
+		{
+			name: "TwoSecretsFound",
+			secrets: &corev1.SecretList{
+				Items: []corev1.Secret{
+					*newSecret("aSecret"),
+					*newSecret("someSecret", withAnnotations(map[string]string{corev1.ServiceAccountNameKey: serviceAccountName})),
+					*newSecret("zSecret", withAnnotations(map[string]string{corev1.ServiceAccountNameKey: serviceAccountName})),
+				},
+			},
+			wantedSecretNames: []string{"someSecret", "zSecret"},
+		},
+		{
+			name: "AllSecretsFound",
+			secrets: &corev1.SecretList{
+				Items: []corev1.Secret{
+					*newSecret("aSecret", withAnnotations(map[string]string{corev1.ServiceAccountNameKey: serviceAccountName})),
+					*newSecret("someSecret", withAnnotations(map[string]string{corev1.ServiceAccountNameKey: serviceAccountName})),
+					*newSecret("zSecret", withAnnotations(map[string]string{corev1.ServiceAccountNameKey: serviceAccountName})),
+				},
+			},
+			wantedSecretNames: []string{"aSecret", "someSecret", "zSecret"},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := filterSecretsBySAName(serviceAccountName, tt.secrets)
+			require.Equal(t, len(tt.wantedSecretNames), len(got))
+			for _, wantedSecretName := range tt.wantedSecretNames {
+				require.NotNil(t, got[wantedSecretName])
+				require.Equal(t, wantedSecretName, got[wantedSecretName].GetName())
+			}
+		})
+	}
+}
+
+type secretOption func(*corev1.Secret)
+
+func withAnnotations(annotations map[string]string) secretOption {
+	return func(s *corev1.Secret) {
+		s.SetAnnotations(annotations)
+	}
+}
+
+func newSecret(name string, opts ...secretOption) *corev1.Secret {
+	s := &corev1.Secret{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: name,
+		},
+	}
+	for _, opt := range opts {
+		opt(s)
+	}
+	return s
+}