From 8f1d46d1828bd6215758991b69635840a1ae4d59 Mon Sep 17 00:00:00 2001 From: Lukasz Szaszkiewicz Date: Tue, 30 May 2023 11:14:37 +0200 Subject: [PATCH 1/2] webhooksupportabilitycontroller: react only to changes to a conversion webhook configuration of a CRD --- .../degraded_webhook_conversion.go | 30 ++++++++++++------- .../webhook_supportability_controller.go | 11 +++++-- 2 files changed, 28 insertions(+), 13 deletions(-) diff --git a/pkg/operator/webhooksupportabilitycontroller/degraded_webhook_conversion.go b/pkg/operator/webhooksupportabilitycontroller/degraded_webhook_conversion.go index 79adaf8d7f..c6f25ad52e 100644 --- a/pkg/operator/webhooksupportabilitycontroller/degraded_webhook_conversion.go +++ b/pkg/operator/webhooksupportabilitycontroller/degraded_webhook_conversion.go @@ -2,9 +2,10 @@ package webhooksupportabilitycontroller import ( "context" - operatorv1 "github.com/openshift/api/operator/v1" "github.com/openshift/library-go/pkg/operator/v1helpers" + + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" "k8s.io/apimachinery/pkg/labels" ) @@ -21,24 +22,31 @@ func (c *webhookSupportabilityController) updateCRDConversionWebhookConfiguratio } var webhookInfos []webhookInfo for _, crd := range crds { - conversion := crd.Spec.Conversion - if conversion == nil || conversion.Strategy != v1.WebhookConverter { - continue - } - clientConfig := conversion.Webhook.ClientConfig - if clientConfig == nil || clientConfig.Service == nil { + if !hasCRDConversionWebhookConfiguration(crd) { continue } info := webhookInfo{ Name: crd.Name, - CABundle: clientConfig.CABundle, + CABundle: crd.Spec.Conversion.Webhook.ClientConfig.CABundle, Service: &serviceReference{ - Namespace: clientConfig.Service.Namespace, - Name: clientConfig.Service.Name, - Port: clientConfig.Service.Port, + Namespace: crd.Spec.Conversion.Webhook.ClientConfig.Service.Namespace, + Name: crd.Spec.Conversion.Webhook.ClientConfig.Service.Name, + Port: crd.Spec.Conversion.Webhook.ClientConfig.Service.Port, }, } webhookInfos = append(webhookInfos, info) } return c.updateWebhookConfigurationDegraded(ctx, condition, webhookInfos) } + +func hasCRDConversionWebhookConfiguration(crd *apiextensionsv1.CustomResourceDefinition) bool { + conversion := crd.Spec.Conversion + if conversion == nil || conversion.Strategy != v1.WebhookConverter { + return false + } + clientConfig := conversion.Webhook.ClientConfig + if clientConfig == nil || clientConfig.Service == nil { + return false + } + return true +} diff --git a/pkg/operator/webhooksupportabilitycontroller/webhook_supportability_controller.go b/pkg/operator/webhooksupportabilitycontroller/webhook_supportability_controller.go index f1932dd7a4..c5dbd77574 100644 --- a/pkg/operator/webhooksupportabilitycontroller/webhook_supportability_controller.go +++ b/pkg/operator/webhooksupportabilitycontroller/webhook_supportability_controller.go @@ -7,6 +7,8 @@ import ( "github.com/openshift/library-go/pkg/operator/events" "github.com/openshift/library-go/pkg/operator/management" "github.com/openshift/library-go/pkg/operator/v1helpers" + + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" apiextensionsinformers "k8s.io/apiextensions-apiserver/pkg/client/informers/externalversions" apiextensionslistersv1 "k8s.io/apiextensions-apiserver/pkg/client/listers/apiextensions/v1" admissionregistrationlistersv1 "k8s.io/client-go/listers/admissionregistration/v1" @@ -43,14 +45,19 @@ func NewWebhookSupportabilityController( kubeInformersForAllNamespaces.Admissionregistration().V1().MutatingWebhookConfigurations().Informer(), kubeInformersForAllNamespaces.Admissionregistration().V1().ValidatingWebhookConfigurations().Informer(), kubeInformersForAllNamespaces.Core().V1().Services().Informer(), - apiExtensionsInformers.Apiextensions().V1().CustomResourceDefinitions().Informer(), ). + WithFilteredEventsInformers(func(obj interface{}) bool { + if crd, ok := obj.(*apiextensionsv1.CustomResourceDefinition); ok { + return hasCRDConversionWebhookConfiguration(crd) + } + return true // re-queue just in case, the checks are fairly cheap + }, apiExtensionsInformers.Apiextensions().V1().CustomResourceDefinitions().Informer()). WithSync(c.sync). ToController("webhookSupportabilityController", recorder) return c } -func (c *webhookSupportabilityController) sync(ctx context.Context, controllerContext factory.SyncContext) error { +func (c *webhookSupportabilityController) sync(ctx context.Context, _ factory.SyncContext) error { operatorSpec, _, _, err := c.operatorClient.GetOperatorState() if err != nil { return err From c7dff8a92e3b6719b120506b1a2c475f62ffe528 Mon Sep 17 00:00:00 2001 From: Lukasz Szaszkiewicz Date: Tue, 30 May 2023 11:16:15 +0200 Subject: [PATCH 2/2] webhooksupportabilitycontroller: log the name of the webhook on dial errors --- .../webhooksupportabilitycontroller/degraded_webhook.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/operator/webhooksupportabilitycontroller/degraded_webhook.go b/pkg/operator/webhooksupportabilitycontroller/degraded_webhook.go index d81847ce27..944dbe54cc 100644 --- a/pkg/operator/webhooksupportabilitycontroller/degraded_webhook.go +++ b/pkg/operator/webhooksupportabilitycontroller/degraded_webhook.go @@ -49,7 +49,7 @@ func (c *webhookSupportabilityController) updateWebhookConfigurationDegraded(ctx serviceMsgs = append(serviceMsgs, msg) continue } - err = c.assertConnect(ctx, webhook.Service, webhook.CABundle) + err = c.assertConnect(ctx, webhook.Name, webhook.Service, webhook.CABundle) if err != nil { msg := fmt.Sprintf("%s: %s", webhook.Name, err) if webhook.FailurePolicyIsIgnore { @@ -94,7 +94,7 @@ func (c *webhookSupportabilityController) assertService(reference *serviceRefere } // assertConnect performs a dns lookup of service, opens a tcp connection, and performs a tls handshake. -func (c *webhookSupportabilityController) assertConnect(ctx context.Context, reference *serviceReference, caBundle []byte) error { +func (c *webhookSupportabilityController) assertConnect(ctx context.Context, webhookName string, reference *serviceReference, caBundle []byte) error { host := reference.Name + "." + reference.Namespace + ".svc" port := "443" if reference.Port != nil { @@ -125,7 +125,7 @@ func (c *webhookSupportabilityController) assertConnect(ctx context.Context, ref if err != nil { if i != 2 { // log err since only last one is reported - runtime.HandleError(err) + runtime.HandleError(fmt.Errorf("%s: %v", webhookName, err)) } continue }