Merge pull request #1503 from p0lyn0mial/fix-ocpbugs-13946

OCPBUGS-13946: degraded_webhook.go x509: certificate signed by unknown authority
openshift · Jun 6, 2023 · a1de1b1 · a1de1b1
2 parents 0afbcf1 + c7dff8a
commit a1de1b1
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 16 deletions.
diff --git 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
 		}

diff --git 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
@@ -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