From 54f89d5163262d9b4844928f8d64714ef77e5d8c Mon Sep 17 00:00:00 2001 From: Alexander Greene Date: Fri, 29 Apr 2022 07:12:50 -0700 Subject: [PATCH] Fix GRPC CheckRegistryServer function (#2756) Problem: The CheckRegistryServer function used by grpc catalogSources does not confirm that the serviceAccount associated with the catalogSource exists. Solution: Update the GRPC CheckRegistryServer function to check if the serviceAccount exists. Signed-off-by: Alexander Greene Upstream-repository: operator-lifecycle-manager Upstream-commit: 7e8e9f77d08d1bf451ce61b7bb2610473e793ded --- .../controller/operators/catalog/operator_test.go | 1 + .../pkg/controller/registry/reconciler/grpc.go | 12 +++++++++++- .../pkg/controller/registry/reconciler/grpc_test.go | 12 ++++++++++++ .../pkg/controller/registry/reconciler/grpc.go | 12 +++++++++++- 4 files changed, 35 insertions(+), 2 deletions(-) diff --git a/staging/operator-lifecycle-manager/pkg/controller/operators/catalog/operator_test.go b/staging/operator-lifecycle-manager/pkg/controller/operators/catalog/operator_test.go index 5518bf1d3d..67c9c11957 100644 --- a/staging/operator-lifecycle-manager/pkg/controller/operators/catalog/operator_test.go +++ b/staging/operator-lifecycle-manager/pkg/controller/operators/catalog/operator_test.go @@ -1037,6 +1037,7 @@ func TestSyncCatalogSources(t *testing.T) { k8sObjs: []runtime.Object{ pod(*grpcCatalog), service(grpcCatalog.GetName(), grpcCatalog.GetNamespace()), + serviceAccount(grpcCatalog.GetName(), grpcCatalog.GetNamespace(), "", objectReference("init secret")), }, existingSources: []sourceAddress{ { diff --git a/staging/operator-lifecycle-manager/pkg/controller/registry/reconciler/grpc.go b/staging/operator-lifecycle-manager/pkg/controller/registry/reconciler/grpc.go index 6da3b58aa1..94494ac752 100644 --- a/staging/operator-lifecycle-manager/pkg/controller/registry/reconciler/grpc.go +++ b/staging/operator-lifecycle-manager/pkg/controller/registry/reconciler/grpc.go @@ -146,6 +146,16 @@ func (c *GrpcRegistryReconciler) currentService(source grpcCatalogSourceDecorato return service } +func (c *GrpcRegistryReconciler) currentServiceAccount(source grpcCatalogSourceDecorator) *corev1.ServiceAccount { + serviceAccountName := source.ServiceAccount().GetName() + serviceAccount, err := c.Lister.CoreV1().ServiceAccountLister().ServiceAccounts(source.GetNamespace()).Get(serviceAccountName) + if err != nil { + logrus.WithField("service", serviceAccount).Debug("couldn't find service in cache") + return nil + } + return serviceAccount +} + func (c *GrpcRegistryReconciler) currentPods(source grpcCatalogSourceDecorator) []*corev1.Pod { pods, err := c.Lister.CoreV1().PodLister().Pods(source.GetNamespace()).List(source.Selector()) if err != nil { @@ -441,7 +451,7 @@ func (c *GrpcRegistryReconciler) CheckRegistryServer(catalogSource *v1alpha1.Cat // Check on registry resources // TODO: add gRPC health check if len(c.currentPodsWithCorrectImageAndSpec(source, source.ServiceAccount().GetName())) < 1 || - c.currentService(source) == nil { + c.currentService(source) == nil || c.currentServiceAccount(source) == nil { healthy = false return } diff --git a/staging/operator-lifecycle-manager/pkg/controller/registry/reconciler/grpc_test.go b/staging/operator-lifecycle-manager/pkg/controller/registry/reconciler/grpc_test.go index 23135185a9..42a2bde3ad 100644 --- a/staging/operator-lifecycle-manager/pkg/controller/registry/reconciler/grpc_test.go +++ b/staging/operator-lifecycle-manager/pkg/controller/registry/reconciler/grpc_test.go @@ -484,6 +484,18 @@ func TestGrpcRegistryChecker(t *testing.T) { healthy: false, }, }, + { + testName: "Grpc/ExistingRegistry/Image/BadServiceAccount", + in: in{ + cluster: cluster{ + k8sObjs: modifyObjName(objectsForCatalogSource(validGrpcCatalogSource("test-img", "")), &corev1.ServiceAccount{}, "badName"), + }, + catsrc: validGrpcCatalogSource("test-img", ""), + }, + out: out{ + healthy: false, + }, + }, { testName: "Grpc/ExistingRegistry/Image/BadPod", in: in{ diff --git a/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/reconciler/grpc.go b/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/reconciler/grpc.go index 6da3b58aa1..94494ac752 100644 --- a/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/reconciler/grpc.go +++ b/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/reconciler/grpc.go @@ -146,6 +146,16 @@ func (c *GrpcRegistryReconciler) currentService(source grpcCatalogSourceDecorato return service } +func (c *GrpcRegistryReconciler) currentServiceAccount(source grpcCatalogSourceDecorator) *corev1.ServiceAccount { + serviceAccountName := source.ServiceAccount().GetName() + serviceAccount, err := c.Lister.CoreV1().ServiceAccountLister().ServiceAccounts(source.GetNamespace()).Get(serviceAccountName) + if err != nil { + logrus.WithField("service", serviceAccount).Debug("couldn't find service in cache") + return nil + } + return serviceAccount +} + func (c *GrpcRegistryReconciler) currentPods(source grpcCatalogSourceDecorator) []*corev1.Pod { pods, err := c.Lister.CoreV1().PodLister().Pods(source.GetNamespace()).List(source.Selector()) if err != nil { @@ -441,7 +451,7 @@ func (c *GrpcRegistryReconciler) CheckRegistryServer(catalogSource *v1alpha1.Cat // Check on registry resources // TODO: add gRPC health check if len(c.currentPodsWithCorrectImageAndSpec(source, source.ServiceAccount().GetName())) < 1 || - c.currentService(source) == nil { + c.currentService(source) == nil || c.currentServiceAccount(source) == nil { healthy = false return }