webhookcontroller: report when a webhook resource is missing a caBund…

…le provided by the service-ca-operator

pkg/operator/webhooksupportabilitycontroller/degraded_webhook.go

@@ -16,6 +16,12 @@ import (
 	"k8s.io/klog/v2"
 )
 
+const (
+	// injectCABundleAnnotationName is the annotation used by the
+	// service-ca-operator to indicate which resources it should inject the CA into.
+	injectCABundleAnnotationName = "service.beta.openshift.io/inject-cabundle"
+)
+
 // webhookInfo generically represents a webhook
 type webhookInfo struct {
 	Name                  string
@@ -25,6 +31,9 @@ type webhookInfo struct {
 	// TimeoutSeconds specifies the timeout for a webhook.
 	// After the timeout passes, the webhook call will be ignored or the API call will fail
 	TimeoutSeconds *int32
+	// HasServiceCaAnnotation indicates whether a webhook
+	// resource has been annotated for CABundle injection by the service-ca-operator
+	HasServiceCaAnnotation bool
 }
 
 // serviceReference generically represents a service reference
@@ -52,7 +61,7 @@ func (c *webhookSupportabilityController) updateWebhookConfigurationDegraded(ctx
 				serviceMsgs = append(serviceMsgs, msg)
 				continue
 			}
-			err = c.assertConnect(ctx, webhook.Name, webhook.Service, webhook.CABundle, webhook.TimeoutSeconds)
+			err = c.assertConnect(ctx, webhook.Name, webhook.Service, webhook.CABundle, webhook.HasServiceCaAnnotation, webhook.TimeoutSeconds)
 			if err != nil {
 				msg := fmt.Sprintf("%s: %s", webhook.Name, err)
 				if webhook.FailurePolicyIsIgnore {
@@ -97,7 +106,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, webhookName string, reference *serviceReference, caBundle []byte, webhookTimeoutSeconds *int32) error {
+func (c *webhookSupportabilityController) assertConnect(ctx context.Context, webhookName string, reference *serviceReference, caBundle []byte, caBundleProvidedByServiceCA bool, webhookTimeoutSeconds *int32) error {
 	host := reference.Name + "." + reference.Namespace + ".svc"
 	port := "443"
 	if reference.Port != nil {
@@ -106,6 +115,9 @@ func (c *webhookSupportabilityController) assertConnect(ctx context.Context, web
 	rootCAs := x509.NewCertPool()
 	if len(caBundle) > 0 {
 		rootCAs.AppendCertsFromPEM(caBundle)
+	} else if caBundleProvidedByServiceCA {
+		err := fmt.Errorf("skipping checking the webhook via %q service because the caBundle (provided by the service-ca-operator) is empty. Please check the service-ca's logs if the issue persists", net.JoinHostPort(host, port))
+		return err
 	}
 	timeout := 10 * time.Second
 	if webhookTimeoutSeconds != nil {
@@ -142,3 +154,10 @@ func (c *webhookSupportabilityController) assertConnect(ctx context.Context, web
 	}
 	return err
 }
+
+func hasServiceCaAnnotation(annotations map[string]string) bool {
+	if annotations == nil {
+		return false
+	}
+	return strings.EqualFold(annotations[injectCABundleAnnotationName], "true")
+}

pkg/operator/webhooksupportabilitycontroller/degraded_webhook_admission.go

@@ -24,10 +24,11 @@ func (c *webhookSupportabilityController) updateMutatingAdmissionWebhookConfigur
 	for _, webhookConfiguration := range webhookConfigurations {
 		for _, webhook := range webhookConfiguration.Webhooks {
 			info := webhookInfo{
-				Name:                  webhook.Name,
-				CABundle:              webhook.ClientConfig.CABundle,
-				FailurePolicyIsIgnore: webhook.FailurePolicy != nil && *webhook.FailurePolicy == admissionregistrationv1.Ignore,
-				TimeoutSeconds:        webhook.TimeoutSeconds,
+				Name:                   webhook.Name,
+				CABundle:               webhook.ClientConfig.CABundle,
+				HasServiceCaAnnotation: hasServiceCaAnnotation(webhookConfiguration.Annotations),
+				FailurePolicyIsIgnore:  webhook.FailurePolicy != nil && *webhook.FailurePolicy == admissionregistrationv1.Ignore,
+				TimeoutSeconds:         webhook.TimeoutSeconds,
 			}
 			if webhook.ClientConfig.Service != nil {
 				info.Service = &serviceReference{
@@ -56,10 +57,11 @@ func (c *webhookSupportabilityController) updateValidatingAdmissionWebhookConfig
 	for _, webhookConfiguration := range webhookConfigurations {
 		for _, webhook := range webhookConfiguration.Webhooks {
 			info := webhookInfo{
-				Name:                  webhook.Name,
-				CABundle:              webhook.ClientConfig.CABundle,
-				FailurePolicyIsIgnore: webhook.FailurePolicy != nil && (*webhook.FailurePolicy == v1.Ignore),
-				TimeoutSeconds:        webhook.TimeoutSeconds,
+				Name:                   webhook.Name,
+				CABundle:               webhook.ClientConfig.CABundle,
+				HasServiceCaAnnotation: hasServiceCaAnnotation(webhookConfiguration.Annotations),
+				FailurePolicyIsIgnore:  webhook.FailurePolicy != nil && (*webhook.FailurePolicy == v1.Ignore),
+				TimeoutSeconds:         webhook.TimeoutSeconds,
 			}
 
 			if webhook.ClientConfig.Service != nil {

pkg/operator/webhooksupportabilitycontroller/degraded_webhook_admission_test.go

@@ -179,6 +179,27 @@ func TestUpdateMutatingAdmissionWebhookConfigurationDegraded(t *testing.T) {
 				Message: `mw10: (?:.*)?x509: certificate signed by unknown authority`,
 			},
 		},
+		{
+			name: "CABundleNotYetProvided",
+			webhookConfigs: []*admissionregistrationv1.MutatingWebhookConfiguration{
+				mutatingWebhookConfiguration("mwc10",
+					withMutatingWebhook("mw10", withMutatingServiceReference("ns10", "svc10")),
+					withMutatingWebhookAnnotatedWithServiceCABundleInjection,
+				),
+			},
+			services: []*corev1.Service{
+				service("ns10", "svc10"),
+			},
+			webhookServers: []*mockWebhookServer{
+				webhookServer("mwc10", "ns10", "svc10", withEmptyCABundle),
+			},
+			expected: operatorv1.OperatorCondition{
+				Type:    MutatingAdmissionWebhookConfigurationErrorType,
+				Status:  operatorv1.ConditionTrue,
+				Reason:  WebhookServiceConnectionErrorReason,
+				Message: `mw10: skipping checking the webhook via \"([^"]+)\" service because the caBundle \(provided by the service-ca-operator\) is empty. Please check the service-ca's logs if the issue persists`,
+			},
+		},
 	}
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
@@ -385,6 +406,27 @@ func TestUpdateValidatingAdmissionWebhookConfigurationDegradedStatus(t *testing.
 				Message: `mw10: (?:.*)?x509: certificate signed by unknown authority`,
 			},
 		},
+		{
+			name: "CABundleNotYetProvided",
+			webhookConfigs: []*admissionregistrationv1.ValidatingWebhookConfiguration{
+				validatingWebhookConfiguration("mwc10",
+					withValidatingWebhook("mw10", withValidatingServiceReference("ns10", "svc10")),
+					withValidatingWebhookAnnotatedWithServiceCABundleInjection,
+				),
+			},
+			services: []*corev1.Service{
+				service("ns10", "svc10"),
+			},
+			webhookServers: []*mockWebhookServer{
+				webhookServer("mwc10", "ns10", "svc10", withEmptyCABundle),
+			},
+			expected: operatorv1.OperatorCondition{
+				Type:    ValidatingAdmissionWebhookConfigurationErrorType,
+				Status:  operatorv1.ConditionTrue,
+				Reason:  WebhookServiceConnectionErrorReason,
+				Message: `mw10: skipping checking the webhook via \"([^"]+)\" service because the caBundle \(provided by the service-ca-operator\) is empty. Please check the service-ca's logs if the issue persists`,
+			},
+		},
 	}
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
@@ -466,6 +508,13 @@ func mutatingWebhookConfiguration(n string, options ...func(*admissionregistrati
 	return c
 }
 
+func withMutatingWebhookAnnotatedWithServiceCABundleInjection(c *admissionregistrationv1.MutatingWebhookConfiguration) {
+	if c.Annotations == nil {
+		c.Annotations = map[string]string{}
+	}
+	c.Annotations[injectCABundleAnnotationName] = "true"
+}
+
 func withMutatingWebhook(n string, options ...func(*admissionregistrationv1.MutatingWebhook)) func(*admissionregistrationv1.MutatingWebhookConfiguration) {
 	return func(c *admissionregistrationv1.MutatingWebhookConfiguration) {
 		w := &admissionregistrationv1.MutatingWebhook{
@@ -554,6 +603,13 @@ func withValidatingServiceReference(ns, n string) func(*admissionregistrationv1.
 	}
 }
 
+func withValidatingWebhookAnnotatedWithServiceCABundleInjection(c *admissionregistrationv1.ValidatingWebhookConfiguration) {
+	if c.Annotations == nil {
+		c.Annotations = map[string]string{}
+	}
+	c.Annotations[injectCABundleAnnotationName] = "true"
+}
+
 func service(ns, n string) *corev1.Service {
 	s := &corev1.Service{
 		ObjectMeta: metav1.ObjectMeta{Namespace: ns, Name: n},

pkg/operator/webhooksupportabilitycontroller/degraded_webhook_conversion.go

@@ -26,8 +26,9 @@ func (c *webhookSupportabilityController) updateCRDConversionWebhookConfiguratio
 			continue
 		}
 		info := webhookInfo{
-			Name:     crd.Name,
-			CABundle: crd.Spec.Conversion.Webhook.ClientConfig.CABundle,
+			Name:                   crd.Name,
+			CABundle:               crd.Spec.Conversion.Webhook.ClientConfig.CABundle,
+			HasServiceCaAnnotation: hasServiceCaAnnotation(crd.Annotations),
 			Service: &serviceReference{
 				Namespace: crd.Spec.Conversion.Webhook.ClientConfig.Service.Namespace,
 				Name:      crd.Spec.Conversion.Webhook.ClientConfig.Service.Name,

pkg/operator/webhooksupportabilitycontroller/degraded_webhook_conversion_test.go

@@ -147,6 +147,27 @@ func TestUpdateCRDConversionWebhookConfigurationDegraded(t *testing.T) {
 				Message: `crd10: (?:.*)?x509: certificate signed by unknown authority`,
 			},
 		},
+		{
+			name: "CABundleNotYetProvided",
+			crds: []*apiextensionsv1.CustomResourceDefinition{
+				customResourceDefinition("crd10",
+					withConversionServiceReference("ns10", "svc10"),
+					withCRDAnnotatedWithServiceCABundleInjection,
+				),
+			},
+			services: []*corev1.Service{
+				service("ns10", "svc10"),
+			},
+			webhookServers: []*mockWebhookServer{
+				webhookServer("crd10", "ns10", "svc10", withEmptyCABundle),
+			},
+			expected: operatorv1.OperatorCondition{
+				Type:    CRDConversionWebhookConfigurationErrorType,
+				Status:  operatorv1.ConditionTrue,
+				Reason:  WebhookServiceConnectionErrorReason,
+				Message: `crd10: skipping checking the webhook via \"([^"]+)\" service because the caBundle \(provided by the service-ca-operator\) is empty. Please check the service-ca's logs if the issue persists`,
+			},
+		},
 	}
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
@@ -242,3 +263,10 @@ func withConversionServiceReference(ns, n string) func(*apiextensionsv1.CustomRe
 		crd.Spec.Conversion = c
 	}
 }
+
+func withCRDAnnotatedWithServiceCABundleInjection(crd *apiextensionsv1.CustomResourceDefinition) {
+	if crd.Annotations == nil {
+		crd.Annotations = map[string]string{}
+	}
+	crd.Annotations[injectCABundleAnnotationName] = "true"
+}

pkg/operator/webhooksupportabilitycontroller/degraded_webhook_test.go

@@ -109,13 +109,18 @@ func withWrongCABundle(t *testing.T) func(*mockWebhookServer) {
 	}
 }
 
+func withEmptyCABundle(s *mockWebhookServer) {
+	s.skipCABundleInjection = true
+}
+
 type mockWebhookServer struct {
-	Config     string
-	Service    serviceReference
-	Hostname   string
-	Port       *int32
-	CABundle   []byte
-	doNotStart bool
+	Config                string
+	Service               serviceReference
+	Hostname              string
+	Port                  *int32
+	CABundle              []byte
+	skipCABundleInjection bool
+	doNotStart            bool
 }
 
 // Run starts the mock server. Port and CABundle are available after this method returns.
@@ -128,6 +133,12 @@ func (s *mockWebhookServer) Run(t *testing.T, ctx context.Context) {
 	rootCA := &crypto.CA{SerialGenerator: &crypto.RandomSerialGenerator{}, Config: rootCACertCfg}
 	if len(s.CABundle) == 0 {
 		s.CABundle, _, err = rootCA.Config.GetPEMBytes()
+		if err != nil {
+			t.Fatal(err)
+		}
+	}
+	if s.skipCABundleInjection {
+		s.CABundle = []byte{}
 	}
 	// server certs
 	serverCertCfg, err := rootCA.MakeServerCert(sets.NewString(s.Hostname, "127.0.0.1"), 10)