Conditionally inject pod readiness gates (#629)

Fix to pre-qualify pods before injecting the Lattice readiness gate

Makefile

@@ -146,9 +146,12 @@ docs:
 	mkdir -p site
 	mkdocs build
 
+e2e-webhook-namespace := "webhook-e2e-test"
+
 # NB webhook tests can only run if the controller is deployed to the cluster
 .PHONY: webhook-e2e-test
 webhook-e2e-test:
+	@kubectl create namespace $(e2e-webhook-namespace) > /dev/null 2>&1 || true # ignore already exists error
 	LOG_LEVEL=debug
 	cd test && go test \
 		-p 1 \

cmd/aws-application-networking-k8s/main.go

@@ -163,11 +163,12 @@ func main() {
 	}
 
 	if enableWebhook {
+		logger := log.Named("pod-readiness-gate-injector")
 		readinessGateInjector := webhook.NewPodReadinessGateInjector(
 			mgr.GetClient(),
-			log.Named("pod-readiness-gate-injector"),
+			logger,
 		)
-		webhook.NewPodMutator(scheme, readinessGateInjector).SetupWithManager(mgr)
+		webhook.NewPodMutator(logger, scheme, readinessGateInjector).SetupWithManager(logger, mgr)
 	}
 
 	finalizerManager := k8s.NewDefaultFinalizerManager(mgr.GetClient())

pkg/controllers/eventhandlers/mapper.go

@@ -193,7 +193,15 @@ func (r *resourceMapper) backendRefToRoutes(ctx context.Context, obj client.Obje
 func (r *resourceMapper) isBackendRefUsedByRoute(route core.Route, obj client.Object, group, kind string) bool {
 	for _, rule := range route.Spec().Rules() {
 		for _, backendRef := range rule.BackendRefs() {
-			isGroupEqual := backendRef.Group() != nil && string(*backendRef.Group()) == group
+			var isGroupEqual bool
+			if group == corev1.GroupName || (group == anv1alpha1.GroupName && kind == serviceImportKind) {
+				// from spec: "When [Group] unspecified or empty string, core API group is inferred."
+				// we deviate from spec slightly that for ServiceImport we have not historically required a Group
+				isGroupEqual = backendRef.Group() == nil || string(*backendRef.Group()) == group
+			} else {
+				// otherwise, make sure the group matches
+				isGroupEqual = backendRef.Group() != nil && string(*backendRef.Group()) == group
+			}
 			isKindEqual := backendRef.Kind() != nil && string(*backendRef.Kind()) == kind
 			isNameEqual := string(backendRef.Name()) == obj.GetName()
 

pkg/controllers/eventhandlers/mapper_test.go

@@ -56,7 +56,8 @@ func TestServiceToRoutes(t *testing.T) {
 			Namespace: nil,
 			Name:      "test-service",
 		}),
-		createHTTPRoute("invalid-nil-group", "ns1", gwv1beta1.BackendObjectReference{
+		createHTTPRoute("valid-nil-group", "ns1", gwv1beta1.BackendObjectReference{
+			Group:     nil,
 			Kind:      (*gwv1beta1.Kind)(ptr.To("Service")),
 			Namespace: nil,
 			Name:      "test-service",
@@ -91,6 +92,7 @@ func TestServiceToRoutes(t *testing.T) {
 		}),
 	}
 	validRoutes := []string{
+		"valid-nil-group",
 		"valid-inferred-namespace",
 		"valid-explicit-namespace",
 	}

pkg/webhook/core/mutating_handler.go

@@ -3,15 +3,14 @@ package core
 import (
 	"context"
 	"encoding/json"
+	"github.com/aws/aws-application-networking-k8s/pkg/utils/gwlog"
 	admissionv1 "k8s.io/api/admission/v1"
 	"net/http"
-	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 )
 
-var mutatingHandlerLog = ctrl.Log.WithName("mutating_handler")
-
 type mutatingHandler struct {
+	log     gwlog.Logger
 	mutator Mutator
 	decoder *admission.Decoder
 }
@@ -22,7 +21,7 @@ func (h *mutatingHandler) SetDecoder(d *admission.Decoder) {
 
 // Handle handles admission requests.
 func (h *mutatingHandler) Handle(ctx context.Context, req admission.Request) admission.Response {
-	mutatingHandlerLog.V(1).Info("mutating webhook request", "request", req)
+	h.log.Debugw("mutating webhook request", "operation", req.Operation, "name", req.Name, "namespace", req.Namespace)
 	var resp admission.Response
 	switch req.Operation {
 	case admissionv1.Create:
@@ -32,7 +31,7 @@ func (h *mutatingHandler) Handle(ctx context.Context, req admission.Request) adm
 	default:
 		resp = admission.Allowed("")
 	}
-	mutatingHandlerLog.V(1).Info("mutating webhook response", "response", resp)
+	h.log.Debugw("mutating webhook response", "patches", resp.Patches)
 	return resp
 }
 

pkg/webhook/core/mutating_handler_test.go

@@ -3,6 +3,7 @@ package core
 import (
 	"context"
 	"encoding/json"
+	"github.com/aws/aws-application-networking-k8s/pkg/utils/gwlog"
 	"github.com/golang/mock/gomock"
 	"github.com/pkg/errors"
 	"github.com/stretchr/testify/assert"
@@ -389,6 +390,7 @@ func Test_mutatingHandler_Handle(t *testing.T) {
 			}
 
 			h := &mutatingHandler{
+				log:     gwlog.FallbackLogger,
 				mutator: mutator,
 				decoder: tt.fields.decoder,
 			}

pkg/webhook/core/mutator.go

@@ -2,6 +2,7 @@ package core
 
 import (
 	"context"
+	"github.com/aws/aws-application-networking-k8s/pkg/utils/gwlog"
 	"k8s.io/apimachinery/pkg/runtime"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 )
@@ -18,9 +19,10 @@ type Mutator interface {
 }
 
 // MutatingWebhookForMutator creates a new mutating Webhook.
-func MutatingWebhookForMutator(scheme *runtime.Scheme, mutator Mutator) *admission.Webhook {
+func MutatingWebhookForMutator(log gwlog.Logger, scheme *runtime.Scheme, mutator Mutator) *admission.Webhook {
 	return &admission.Webhook{
 		Handler: &mutatingHandler{
+			log:     log,
 			mutator: mutator,
 			decoder: admission.NewDecoder(scheme),
 		},

pkg/webhook/pod_mutator.go

@@ -2,6 +2,7 @@ package webhook
 
 import (
 	"context"
+	"github.com/aws/aws-application-networking-k8s/pkg/utils/gwlog"
 	"github.com/aws/aws-application-networking-k8s/pkg/webhook/core"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -13,8 +14,9 @@ const (
 	apiPathMutatePod = "/mutate-pod"
 )
 
-func NewPodMutator(scheme *runtime.Scheme, podReadinessGateInjector *PodReadinessGateInjector) *podMutator {
+func NewPodMutator(log gwlog.Logger, scheme *runtime.Scheme, podReadinessGateInjector *PodReadinessGateInjector) *podMutator {
 	return &podMutator{
+		log:                      log,
 		podReadinessGateInjector: podReadinessGateInjector,
 		scheme:                   scheme,
 	}
@@ -23,6 +25,7 @@ func NewPodMutator(scheme *runtime.Scheme, podReadinessGateInjector *PodReadines
 var _ core.Mutator = &podMutator{}
 
 type podMutator struct {
+	log                      gwlog.Logger
 	podReadinessGateInjector *PodReadinessGateInjector
 	scheme                   *runtime.Scheme
 }
@@ -33,7 +36,7 @@ func (m *podMutator) Prototype(_ admission.Request) (runtime.Object, error) {
 
 func (m *podMutator) MutateCreate(ctx context.Context, obj runtime.Object) (runtime.Object, error) {
 	pod := obj.(*corev1.Pod)
-	if err := m.podReadinessGateInjector.Mutate(ctx, pod); err != nil {
+	if err := m.podReadinessGateInjector.MutateCreate(ctx, pod); err != nil {
 		return pod, err
 	}
 	return pod, nil
@@ -43,6 +46,6 @@ func (m *podMutator) MutateUpdate(ctx context.Context, obj runtime.Object, oldOb
 	return obj, nil
 }
 
-func (m *podMutator) SetupWithManager(mgr ctrl.Manager) {
-	mgr.GetWebhookServer().Register(apiPathMutatePod, core.MutatingWebhookForMutator(m.scheme, m))
+func (m *podMutator) SetupWithManager(log gwlog.Logger, mgr ctrl.Manager) {
+	mgr.GetWebhookServer().Register(apiPathMutatePod, core.MutatingWebhookForMutator(log, m.scheme, m))
 }