Merge pull request #12905 from furkatgofurov7/dedup-CallAllExtensions-code

🌱  Deduplicate extension filtering and response validation logic

Author: k8s-ci-robot

internal/runtime/client/client.go

@@ -32,6 +32,7 @@ import (
 	"strings"
 	"time"
 
+	"github.com/go-logr/logr"
 	"github.com/pkg/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
@@ -123,15 +124,8 @@ func (c *client) Discover(ctx context.Context, extensionConfig *runtimev1.Extens
 	}
 
 	// Check to see if the response is not a success and handle the failure accordingly.
-	if response.GetStatus() != runtimehooksv1.ResponseStatusSuccess {
-		if response.GetStatus() == runtimehooksv1.ResponseStatusFailure {
-			log.Info(fmt.Sprintf("Failed to discover extension %q: got failure response with message %v", extensionConfig.Name, response.GetMessage()))
-			// Don't add the message to the error as it is may be unique causing too many reconciliations. Ref: https://github.com/kubernetes-sigs/cluster-api/issues/6921
-			return nil, errors.Errorf("failed to discover extension %q: got failure response, please check controller logs for errors", extensionConfig.Name)
-		}
-		// Handle unknown status.
-		log.Info(fmt.Sprintf("Failed to discover extension %q: got unknown response status %q with message %v", extensionConfig.Name, response.GetStatus(), response.GetMessage()))
-		return nil, errors.Errorf("failed to discover extension %q: got unknown response status %q, please check controller logs for errors", extensionConfig.Name, response.GetStatus())
+	if err := validateResponseStatus(log, response, "discover extension", extensionConfig.Name); err != nil {
+		return nil, err
 	}
 
 	// Check to see if the response is valid.
@@ -229,10 +223,6 @@ func (c *client) CallAllExtensions(ctx context.Context, hook runtimecatalog.Hook
 	if err != nil {
 		return errors.Wrapf(err, "failed to call extension handlers for hook %q: failed to compute GroupVersionHook", hookName)
 	}
-	forObjectGVK, err := apiutil.GVKForObject(forObject, c.client.Scheme())
-	if err != nil {
-		return errors.Wrapf(err, "failed to call extension handlers for hook %q: failed to get GroupVersionKind for the object the hook is executed for", hookName)
-	}
 	// Make sure the request is compatible with the hook.
 	if err := c.catalog.ValidateRequest(gvh, request); err != nil {
 		return errors.Wrapf(err, "failed to call extension handlers for hook %q: request object is invalid for hook", gvh.GroupHook())
@@ -242,33 +232,22 @@ func (c *client) CallAllExtensions(ctx context.Context, hook runtimecatalog.Hook
 		return errors.Wrapf(err, "failed to call extension handlers for hook %q: response object is invalid for hook", gvh.GroupHook())
 	}
 
-	registrations, err := c.registry.List(gvh.GroupHook())
+	// Get all matching extension handlers for this hook and object.
+	matchingHandlers, err := c.GetAllExtensions(ctx, hook, forObject)
 	if err != nil {
 		return errors.Wrapf(err, "failed to call extension handlers for hook %q", gvh.GroupHook())
 	}
 
-	log.V(4).Info(fmt.Sprintf("Calling all extensions of hook %q for %s %s", hookName, forObjectGVK.Kind, klog.KObj(forObject)))
 	responses := []runtimehooksv1.ResponseObject{}
-	for _, registration := range registrations {
+	for _, handlerName := range matchingHandlers {
 		// Creates a new instance of the response parameter.
 		responseObject, err := c.catalog.NewResponse(gvh)
 		if err != nil {
-			return errors.Wrapf(err, "failed to call extension handlers for hook %q: failed to call extension handler %q", gvh.GroupHook(), registration.Name)
+			return errors.Wrapf(err, "failed to call extension handlers for hook %q: failed to call extension handler %q", gvh.GroupHook(), handlerName)
 		}
 		tmpResponse := responseObject.(runtimehooksv1.ResponseObject)
 
-		// Compute whether the object the call is being made for matches the namespaceSelector
-		namespaceMatches, err := c.matchNamespace(ctx, registration.NamespaceSelector, forObject.GetNamespace())
-		if err != nil {
-			return errors.Wrapf(err, "failed to call extension handlers for hook %q: failed to call extension handler %q", gvh.GroupHook(), registration.Name)
-		}
-		// If the object namespace isn't matched by the registration NamespaceSelector skip the call.
-		if !namespaceMatches {
-			log.V(5).Info(fmt.Sprintf("skipping extension handler %q as object '%s/%s' does not match selector %q of ExtensionConfig", registration.Name, forObject.GetNamespace(), forObject.GetName(), registration.NamespaceSelector))
-			continue
-		}
-
-		err = c.CallExtension(ctx, hook, forObject, registration.Name, request, tmpResponse)
+		err = c.CallExtension(ctx, hook, forObject, handlerName, request, tmpResponse)
 		// If one of the extension handlers fails lets short-circuit here and return early.
 		if err != nil {
 			log.Error(err, "failed to call extension handlers")
@@ -411,15 +390,8 @@ func (c *client) CallExtension(ctx context.Context, hook runtimecatalog.Hook, fo
 	}
 
 	// If the received response is not a success then return an error.
-	if response.GetStatus() != runtimehooksv1.ResponseStatusSuccess {
-		if response.GetStatus() == runtimehooksv1.ResponseStatusFailure {
-			log.Info(fmt.Sprintf("Failed to call extension handler %q: got failure response with message %v", name, response.GetMessage()))
-			// Don't add the message to the error as it is may be unique causing too many reconciliations. Ref: https://github.com/kubernetes-sigs/cluster-api/issues/6921
-			return errors.Errorf("failed to call extension handler %q: got failure response, please check controller logs for errors", name)
-		}
-		// Handle unknown status.
-		log.Info(fmt.Sprintf("Failed to call extension handler %q: got unknown response status %q with message %v", name, response.GetStatus(), response.GetMessage()))
-		return errors.Errorf("failed to call extension handler %q: got unknown response status %q, please check controller logs for errors", name, response.GetStatus())
+	if err := validateResponseStatus(log, response, "call extension handler", name); err != nil {
+		return err
 	}
 
 	if retryResponse, ok := response.(runtimehooksv1.RetryResponseObject); ok && retryResponse.GetRetryAfterSeconds() != 0 {
@@ -748,3 +720,19 @@ func ExtensionNameFromHandlerName(registeredHandlerName string) (string, error)
 	}
 	return parts[1], nil
 }
+
+// validateResponseStatus checks if the response status is successful and returns an error otherwise.
+// It logs appropriate messages for failure and unknown statuses.
+func validateResponseStatus(log logr.Logger, response runtimehooksv1.ResponseObject, operationName, targetName string) error {
+	if response.GetStatus() != runtimehooksv1.ResponseStatusSuccess {
+		if response.GetStatus() == runtimehooksv1.ResponseStatusFailure {
+			log.Info(fmt.Sprintf("Failed to %s %q: got failure response with message %v", operationName, targetName, response.GetMessage()))
+			// Don't add the message to the error as it is may be unique causing too many reconciliations. Ref: https://github.com/kubernetes-sigs/cluster-api/issues/6921
+			return errors.Errorf("failed to %s %q: got failure response, please check controller logs for errors", operationName, targetName)
+		}
+		// Handle unknown status.
+		log.Info(fmt.Sprintf("Failed to %s %q: got unknown response status %q with message %v", operationName, targetName, response.GetStatus(), response.GetMessage()))
+		return errors.Errorf("failed to %s %q: got unknown response status %q, please check controller logs for errors", operationName, targetName, response.GetStatus())
+	}
+	return nil
+}