From cbd446de1b2bd2d22161387f3e852735ad27e4ae Mon Sep 17 00:00:00 2001 From: Erik Fuller <16261515+erikfuller@users.noreply.github.com> Date: Fri, 3 May 2024 09:50:12 -0700 Subject: [PATCH] Conditionally inject pod readiness gates (#629) Fix to pre-qualify pods before injecting the Lattice readiness gate --- Makefile | 3 + cmd/aws-application-networking-k8s/main.go | 5 +- pkg/controllers/eventhandlers/mapper.go | 10 +- pkg/controllers/eventhandlers/mapper_test.go | 4 +- pkg/webhook/core/mutating_handler.go | 9 +- pkg/webhook/core/mutating_handler_test.go | 2 + pkg/webhook/core/mutator.go | 4 +- pkg/webhook/pod_mutator.go | 11 +- pkg/webhook/pod_mutator_test.go | 1176 ++++++++++++++++- pkg/webhook/pod_readiness_gate_injector.go | 186 ++- test/pkg/test/framework.go | 6 +- test/pkg/test/httproute.go | 42 + test/pkg/test/nginxapp.go | 33 - .../webhook/readiness_gate_inject_test.go | 47 +- test/suites/webhook/suite_test.go | 23 +- 15 files changed, 1444 insertions(+), 117 deletions(-) create mode 100644 test/pkg/test/httproute.go diff --git a/Makefile b/Makefile index ac465aeb..d907f05b 100644 --- a/Makefile +++ b/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 \ diff --git a/cmd/aws-application-networking-k8s/main.go b/cmd/aws-application-networking-k8s/main.go index cc4abdf3..a8bdf164 100644 --- a/cmd/aws-application-networking-k8s/main.go +++ b/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()) diff --git a/pkg/controllers/eventhandlers/mapper.go b/pkg/controllers/eventhandlers/mapper.go index de7e5aa8..716c3268 100644 --- a/pkg/controllers/eventhandlers/mapper.go +++ b/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() diff --git a/pkg/controllers/eventhandlers/mapper_test.go b/pkg/controllers/eventhandlers/mapper_test.go index 156cb452..b6992d2f 100644 --- a/pkg/controllers/eventhandlers/mapper_test.go +++ b/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", } diff --git a/pkg/webhook/core/mutating_handler.go b/pkg/webhook/core/mutating_handler.go index dca280d2..c03cdc2a 100644 --- a/pkg/webhook/core/mutating_handler.go +++ b/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 } diff --git a/pkg/webhook/core/mutating_handler_test.go b/pkg/webhook/core/mutating_handler_test.go index 5f09255d..fecb95cf 100644 --- a/pkg/webhook/core/mutating_handler_test.go +++ b/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, } diff --git a/pkg/webhook/core/mutator.go b/pkg/webhook/core/mutator.go index 2b39750c..b4440653 100644 --- a/pkg/webhook/core/mutator.go +++ b/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), }, diff --git a/pkg/webhook/pod_mutator.go b/pkg/webhook/pod_mutator.go index e3f13346..5e896264 100644 --- a/pkg/webhook/pod_mutator.go +++ b/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)) } diff --git a/pkg/webhook/pod_mutator_test.go b/pkg/webhook/pod_mutator_test.go index 1c899f98..37a46842 100644 --- a/pkg/webhook/pod_mutator_test.go +++ b/pkg/webhook/pod_mutator_test.go @@ -2,80 +2,1144 @@ package webhook import ( "context" + anv1alpha1 "github.com/aws/aws-application-networking-k8s/pkg/apis/applicationnetworking/v1alpha1" "github.com/aws/aws-application-networking-k8s/pkg/utils/gwlog" "github.com/stretchr/testify/assert" corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" clientgoscheme "k8s.io/client-go/kubernetes/scheme" testclient "sigs.k8s.io/controller-runtime/pkg/client/fake" + gwv1 "sigs.k8s.io/gateway-api/apis/v1" + gwv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" + gwv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1" "testing" ) -func TestReadinessGateInjectionNew(t *testing.T) { - k8sScheme := runtime.NewScheme() - clientgoscheme.AddToScheme(k8sScheme) +func Test_ReadinessGateInjection(t *testing.T) { + var serviceKind gwv1beta1.Kind = "Service" + var gwNamespace = gwv1beta1.Namespace("gw-namespace") + var svcNamespace = gwv1beta1.Namespace("test") - k8sClient := testclient. - NewClientBuilder(). - WithScheme(k8sScheme). - Build() + tests := []struct { + name string + omitGatewayClass bool + performUpdate bool + pod corev1.Pod + services []corev1.Service + httpRoutes []gwv1beta1.HTTPRoute + v1HttpRoutes []gwv1.HTTPRoute + grpcRoutes []gwv1alpha2.GRPCRoute + gateways []gwv1beta1.Gateway + svcExport *anv1alpha1.ServiceExport + expectedConditionTypes []corev1.PodConditionType + }{ + { + name: "HTTP route", + pod: corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pod-1", + Namespace: "test", + Labels: map[string]string{ + "env": "test", + }, + }, + }, + services: []corev1.Service{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "svc-1", + Namespace: "test", + }, + Spec: corev1.ServiceSpec{ + Selector: map[string]string{ + "env": "test", + }, + }, + }, + }, + httpRoutes: []gwv1beta1.HTTPRoute{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "http-route-1", + Namespace: "test", + }, + Spec: gwv1beta1.HTTPRouteSpec{ + CommonRouteSpec: gwv1beta1.CommonRouteSpec{ParentRefs: []gwv1beta1.ParentReference{ + {Name: "gw-1"}, + }}, + Rules: []gwv1beta1.HTTPRouteRule{ + { + BackendRefs: []gwv1beta1.HTTPBackendRef{{BackendRef: gwv1beta1.BackendRef{ + BackendObjectReference: gwv1beta1.BackendObjectReference{ + Name: "svc-1", + Kind: &serviceKind, + }, + }}}, + }, + }, + }, + }, + }, + gateways: []gwv1beta1.Gateway{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "gw-1", + Namespace: "test", + }, + Spec: gwv1beta1.GatewaySpec{ + GatewayClassName: "amazon-vpc-lattice", + }, + }, + }, + expectedConditionTypes: []corev1.PodConditionType{ + corev1.PodConditionType(PodReadinessGateConditionType), + }, + }, + { + name: "GRPC Route", + pod: corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pod-1", + Namespace: "test", + Labels: map[string]string{ + "env": "test", + }, + }, + }, + services: []corev1.Service{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "svc-1", + Namespace: "test", + }, + Spec: corev1.ServiceSpec{ + Selector: map[string]string{ + "env": "test", + }, + }, + }, + }, + grpcRoutes: []gwv1alpha2.GRPCRoute{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "http-route-1", + Namespace: "test", + }, + Spec: gwv1alpha2.GRPCRouteSpec{ + CommonRouteSpec: gwv1beta1.CommonRouteSpec{ParentRefs: []gwv1beta1.ParentReference{ + {Name: "gw-1"}, + }}, + Rules: []gwv1alpha2.GRPCRouteRule{ + { + BackendRefs: []gwv1alpha2.GRPCBackendRef{{BackendRef: gwv1beta1.BackendRef{ + BackendObjectReference: gwv1beta1.BackendObjectReference{ + Name: "svc-1", + Kind: &serviceKind, + }, + }}}, + }, + }, + }, + }, + }, + gateways: []gwv1beta1.Gateway{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "gw-1", + Namespace: "test", + }, + Spec: gwv1beta1.GatewaySpec{ + GatewayClassName: "amazon-vpc-lattice", + }, + }, + }, + expectedConditionTypes: []corev1.PodConditionType{ + corev1.PodConditionType(PodReadinessGateConditionType), + }, + }, + { + name: "service export", + pod: corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pod-1", + Namespace: "test", + Labels: map[string]string{ + "env": "test", + }, + }, + }, + services: []corev1.Service{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "svc-1", + Namespace: "test", + }, + Spec: corev1.ServiceSpec{ + Selector: map[string]string{ + "env": "test", + }, + }, + }, + }, + svcExport: &anv1alpha1.ServiceExport{ + ObjectMeta: metav1.ObjectMeta{ + Name: "svc-1", + Namespace: "test", + Annotations: map[string]string{ + "application-networking.k8s.aws/federation": "amazon-vpc-lattice", + }, + }, + }, + expectedConditionTypes: []corev1.PodConditionType{ + corev1.PodConditionType(PodReadinessGateConditionType), + }, + }, + { + name: "service, route, gateway different namespaces, but referencing works", + pod: corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pod-1", + Namespace: "test", + Labels: map[string]string{ + "env": "test", + }, + }, + }, + services: []corev1.Service{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "svc-1", + Namespace: string(svcNamespace), + }, + Spec: corev1.ServiceSpec{ + Selector: map[string]string{ + "env": "test", + }, + }, + }, + }, + httpRoutes: []gwv1beta1.HTTPRoute{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "http-route-1", + Namespace: "route-namespace", + }, + Spec: gwv1beta1.HTTPRouteSpec{ + CommonRouteSpec: gwv1beta1.CommonRouteSpec{ParentRefs: []gwv1beta1.ParentReference{ + { + Name: "gw-1", + Namespace: &gwNamespace, + }, + }}, + Rules: []gwv1beta1.HTTPRouteRule{ + { + BackendRefs: []gwv1beta1.HTTPBackendRef{{BackendRef: gwv1beta1.BackendRef{ + BackendObjectReference: gwv1beta1.BackendObjectReference{ + Name: "svc-1", + Namespace: &svcNamespace, + Kind: &serviceKind, + }, + }}}, + }, + }, + }, + }, + }, + gateways: []gwv1beta1.Gateway{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "gw-1", + Namespace: string(gwNamespace), + }, + Spec: gwv1beta1.GatewaySpec{ + GatewayClassName: "amazon-vpc-lattice", + }, + }, + }, + expectedConditionTypes: []corev1.PodConditionType{ + corev1.PodConditionType(PodReadinessGateConditionType), + }, + }, + { + name: "service, route, gateway different namespaces, do not reference each other", + pod: corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pod-1", + Namespace: "test", + Labels: map[string]string{ + "env": "test", + }, + }, + }, + services: []corev1.Service{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "svc-1", + Namespace: string(svcNamespace), + }, + Spec: corev1.ServiceSpec{ + Selector: map[string]string{ + "env": "test", + }, + }, + }, + }, + httpRoutes: []gwv1beta1.HTTPRoute{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "http-route-1", + Namespace: "route-namespace", + }, + Spec: gwv1beta1.HTTPRouteSpec{ + CommonRouteSpec: gwv1beta1.CommonRouteSpec{ParentRefs: []gwv1beta1.ParentReference{ + { + Name: "gw-1", + }, + }}, + Rules: []gwv1beta1.HTTPRouteRule{ + { + BackendRefs: []gwv1beta1.HTTPBackendRef{{BackendRef: gwv1beta1.BackendRef{ + BackendObjectReference: gwv1beta1.BackendObjectReference{ + Name: "svc-1", + Kind: &serviceKind, + }, + }}}, + }, + }, + }, + }, + }, + gateways: []gwv1beta1.Gateway{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "gw-1", + Namespace: string(gwNamespace), + }, + Spec: gwv1beta1.GatewaySpec{ + GatewayClassName: "amazon-vpc-lattice", + }, + }, + }, + expectedConditionTypes: []corev1.PodConditionType{}, + }, + { + name: "no service", + pod: corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pod-1", + Namespace: "test", + Labels: map[string]string{ + "env": "test", + }, + }, + }, + expectedConditionTypes: []corev1.PodConditionType{}, + }, + { + name: "service labels do not match", + pod: corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pod-1", + Namespace: "test", + Labels: map[string]string{ + "env": "test", + }, + }, + }, + services: []corev1.Service{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "svc-1", + Namespace: "test", + }, + Spec: corev1.ServiceSpec{ + Selector: map[string]string{ + "env": "PROD", + }, + }, + }, + }, + expectedConditionTypes: []corev1.PodConditionType{}, + }, + { + name: "no route", + pod: corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pod-1", + Namespace: "test", + Labels: map[string]string{ + "env": "test", + }, + }, + }, + services: []corev1.Service{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "svc-1", + Namespace: "test", + }, + Spec: corev1.ServiceSpec{ + Selector: map[string]string{ + "env": "test", + }, + }, + }, + }, + gateways: []gwv1beta1.Gateway{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "gw-1", + Namespace: "test", + }, + Spec: gwv1beta1.GatewaySpec{ + GatewayClassName: "amazon-vpc-lattice", + }, + }, + }, + expectedConditionTypes: []corev1.PodConditionType{}, + }, + { + name: "no gateway", + pod: corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pod-1", + Namespace: "test", + Labels: map[string]string{ + "env": "test", + }, + }, + }, + services: []corev1.Service{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "svc-1", + Namespace: "test", + }, + Spec: corev1.ServiceSpec{ + Selector: map[string]string{ + "env": "test", + }, + }, + }, + }, + httpRoutes: []gwv1beta1.HTTPRoute{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "http-route-1", + Namespace: "test", + }, + Spec: gwv1beta1.HTTPRouteSpec{ + CommonRouteSpec: gwv1beta1.CommonRouteSpec{ParentRefs: []gwv1beta1.ParentReference{ + {Name: "gw-1"}, + }}, + Rules: []gwv1beta1.HTTPRouteRule{ + { + BackendRefs: []gwv1beta1.HTTPBackendRef{{BackendRef: gwv1beta1.BackendRef{ + BackendObjectReference: gwv1beta1.BackendObjectReference{ + Name: "svc-1", + Kind: &serviceKind, + }, + }}}, + }, + }, + }, + }, + }, + expectedConditionTypes: []corev1.PodConditionType{}, + }, + { + name: "HTTP route other gateway type", + pod: corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pod-1", + Namespace: "test", + Labels: map[string]string{ + "env": "test", + }, + }, + }, + services: []corev1.Service{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "svc-1", + Namespace: "test", + }, + Spec: corev1.ServiceSpec{ + Selector: map[string]string{ + "env": "test", + }, + }, + }, + }, + httpRoutes: []gwv1beta1.HTTPRoute{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "http-route-1", + Namespace: "test", + }, + Spec: gwv1beta1.HTTPRouteSpec{ + CommonRouteSpec: gwv1beta1.CommonRouteSpec{ParentRefs: []gwv1beta1.ParentReference{ + {Name: "gw-1"}, + }}, + Rules: []gwv1beta1.HTTPRouteRule{ + { + BackendRefs: []gwv1beta1.HTTPBackendRef{{BackendRef: gwv1beta1.BackendRef{ + BackendObjectReference: gwv1beta1.BackendObjectReference{ + Name: "svc-1", + Kind: &serviceKind, + }, + }}}, + }, + }, + }, + }, + }, + gateways: []gwv1beta1.Gateway{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "gw-1", + Namespace: "test", + }, + Spec: gwv1beta1.GatewaySpec{ + GatewayClassName: "some-other-gateway-type", + }, + }, + }, + expectedConditionTypes: []corev1.PodConditionType{}, + }, + { + name: "GRPC route other gateway type", + pod: corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pod-1", + Namespace: "test", + Labels: map[string]string{ + "env": "test", + }, + }, + }, + services: []corev1.Service{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "svc-1", + Namespace: "test", + }, + Spec: corev1.ServiceSpec{ + Selector: map[string]string{ + "env": "test", + }, + }, + }, + }, + grpcRoutes: []gwv1alpha2.GRPCRoute{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "http-route-1", + Namespace: "test", + }, + Spec: gwv1alpha2.GRPCRouteSpec{ + CommonRouteSpec: gwv1beta1.CommonRouteSpec{ParentRefs: []gwv1beta1.ParentReference{ + {Name: "gw-1"}, + }}, + Rules: []gwv1alpha2.GRPCRouteRule{ + { + BackendRefs: []gwv1alpha2.GRPCBackendRef{{BackendRef: gwv1beta1.BackendRef{ + BackendObjectReference: gwv1beta1.BackendObjectReference{ + Name: "svc-1", + Kind: &serviceKind, + }, + }}}, + }, + }, + }, + }, + }, + gateways: []gwv1beta1.Gateway{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "gw-1", + Namespace: "test", + }, + Spec: gwv1beta1.GatewaySpec{ + GatewayClassName: "other-gateway-type", + }, + }, + }, + expectedConditionTypes: []corev1.PodConditionType{}, + }, + { + name: "not modified - empty pod condition remains unchanged", + pod: corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pod-1", + Namespace: "test", + Labels: map[string]string{ + "env": "test", + }, + }, + }, + expectedConditionTypes: []corev1.PodConditionType{}, + }, + { + name: "not modified - existing pod condition remains unchanged", + pod: corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pod-1", + Namespace: "test", + Labels: map[string]string{ + "env": "test", + }, + }, + Spec: corev1.PodSpec{ReadinessGates: []corev1.PodReadinessGate{ + { + ConditionType: corev1.PodConditionType("some-condition"), + }, + }}, + }, + expectedConditionTypes: []corev1.PodConditionType{ + corev1.PodConditionType("some-condition"), + }, + }, + { + name: "appends to existing pod condition", + pod: corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pod-1", + Namespace: "test", + Labels: map[string]string{ + "env": "test", + }, + }, + Spec: corev1.PodSpec{ReadinessGates: []corev1.PodReadinessGate{ + { + ConditionType: corev1.PodConditionType("some-condition"), + }, + }}, + }, + services: []corev1.Service{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "svc-1", + Namespace: "test", + }, + Spec: corev1.ServiceSpec{ + Selector: map[string]string{ + "env": "test", + }, + }, + }, + }, + httpRoutes: []gwv1beta1.HTTPRoute{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "http-route-1", + Namespace: "test", + }, + Spec: gwv1beta1.HTTPRouteSpec{ + CommonRouteSpec: gwv1beta1.CommonRouteSpec{ParentRefs: []gwv1beta1.ParentReference{ + {Name: "gw-1"}, + }}, + Rules: []gwv1beta1.HTTPRouteRule{ + { + BackendRefs: []gwv1beta1.HTTPBackendRef{{BackendRef: gwv1beta1.BackendRef{ + BackendObjectReference: gwv1beta1.BackendObjectReference{ + Name: "svc-1", + Kind: &serviceKind, + }, + }}}, + }, + }, + }, + }, + }, + gateways: []gwv1beta1.Gateway{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "gw-1", + Namespace: "test", + }, + Spec: gwv1beta1.GatewaySpec{ + GatewayClassName: "amazon-vpc-lattice", + }, + }, + }, + expectedConditionTypes: []corev1.PodConditionType{ + corev1.PodConditionType("some-condition"), + corev1.PodConditionType(PodReadinessGateConditionType), + }, + }, + { + name: "service in both GRPC route and HTTP route", + pod: corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pod-1", + Namespace: "test", + Labels: map[string]string{ + "env": "test", + }, + }, + }, + services: []corev1.Service{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "svc-1", + Namespace: "test", + }, + Spec: corev1.ServiceSpec{ + Selector: map[string]string{ + "env": "test", + }, + }, + }, + }, + httpRoutes: []gwv1beta1.HTTPRoute{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "http-route-1", + Namespace: "test", + }, + Spec: gwv1beta1.HTTPRouteSpec{ + CommonRouteSpec: gwv1beta1.CommonRouteSpec{ParentRefs: []gwv1beta1.ParentReference{ + {Name: "gw-1"}, + }}, + Rules: []gwv1beta1.HTTPRouteRule{ + { + BackendRefs: []gwv1beta1.HTTPBackendRef{{BackendRef: gwv1beta1.BackendRef{ + BackendObjectReference: gwv1beta1.BackendObjectReference{ + Name: "svc-1", + Kind: &serviceKind, + }, + }}}, + }, + }, + }, + }, + }, + grpcRoutes: []gwv1alpha2.GRPCRoute{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "http-route-1", + Namespace: "test", + }, + Spec: gwv1alpha2.GRPCRouteSpec{ + CommonRouteSpec: gwv1beta1.CommonRouteSpec{ParentRefs: []gwv1beta1.ParentReference{ + {Name: "gw-1"}, + }}, + Rules: []gwv1alpha2.GRPCRouteRule{ + { + BackendRefs: []gwv1alpha2.GRPCBackendRef{{BackendRef: gwv1beta1.BackendRef{ + BackendObjectReference: gwv1beta1.BackendObjectReference{ + Name: "svc-1", + Kind: &serviceKind, + }, + }}}, + }, + }, + }, + }, + }, + gateways: []gwv1beta1.Gateway{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "gw-1", + Namespace: "test", + }, + Spec: gwv1beta1.GatewaySpec{ + GatewayClassName: "amazon-vpc-lattice", + }, + }, + }, + expectedConditionTypes: []corev1.PodConditionType{ + corev1.PodConditionType(PodReadinessGateConditionType), + }, + }, + { + name: "multiple services multiple routes", + pod: corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pod-1", + Namespace: "test", + Labels: map[string]string{ + "env": "test", + }, + }, + }, + services: []corev1.Service{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "svc-1", + Namespace: "test", + }, + Spec: corev1.ServiceSpec{ + Selector: map[string]string{ + "env": "test", + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "svc-2", + Namespace: "test", + }, + Spec: corev1.ServiceSpec{ + Selector: map[string]string{ + "env": "test", + }, + }, + }, + }, + httpRoutes: []gwv1beta1.HTTPRoute{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "http-route-1", + Namespace: "test", + }, + Spec: gwv1beta1.HTTPRouteSpec{ + CommonRouteSpec: gwv1beta1.CommonRouteSpec{ParentRefs: []gwv1beta1.ParentReference{ + {Name: "gw-1"}, + }}, + Rules: []gwv1beta1.HTTPRouteRule{ + { + BackendRefs: []gwv1beta1.HTTPBackendRef{{BackendRef: gwv1beta1.BackendRef{ + BackendObjectReference: gwv1beta1.BackendObjectReference{ + Name: "svc-1", + Kind: &serviceKind, + }, + }}}, + }, + }, + }, + }, + }, + grpcRoutes: []gwv1alpha2.GRPCRoute{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "http-route-1", + Namespace: "test", + }, + Spec: gwv1alpha2.GRPCRouteSpec{ + CommonRouteSpec: gwv1beta1.CommonRouteSpec{ParentRefs: []gwv1beta1.ParentReference{ + {Name: "gw-1"}, + }}, + Rules: []gwv1alpha2.GRPCRouteRule{ + { + BackendRefs: []gwv1alpha2.GRPCBackendRef{{BackendRef: gwv1beta1.BackendRef{ + BackendObjectReference: gwv1beta1.BackendObjectReference{ + Name: "svc-2", + Kind: &serviceKind, + }, + }}}, + }, + }, + }, + }, + }, + gateways: []gwv1beta1.Gateway{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "gw-1", + Namespace: "test", + }, + Spec: gwv1beta1.GatewaySpec{ + GatewayClassName: "amazon-vpc-lattice", + }, + }, + }, + expectedConditionTypes: []corev1.PodConditionType{ + corev1.PodConditionType(PodReadinessGateConditionType), + }, + }, + { + name: "lots of objects but nothing matches but service", + pod: corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pod-1", + Namespace: "test", + Labels: map[string]string{ + "env": "test", + }, + }, + }, + services: []corev1.Service{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "svc-1", + Namespace: "test", + }, + Spec: corev1.ServiceSpec{ + Selector: map[string]string{ + "env": "test", + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "svc-2", + Namespace: "test", + }, + Spec: corev1.ServiceSpec{ + Selector: map[string]string{ + "env": "test", + }, + }, + }, + }, + httpRoutes: []gwv1beta1.HTTPRoute{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "http-route-1", + Namespace: "test", + }, + Spec: gwv1beta1.HTTPRouteSpec{ + CommonRouteSpec: gwv1beta1.CommonRouteSpec{ParentRefs: []gwv1beta1.ParentReference{ + {Name: "gw-1"}, + }}, + Rules: []gwv1beta1.HTTPRouteRule{ + { + BackendRefs: []gwv1beta1.HTTPBackendRef{{BackendRef: gwv1beta1.BackendRef{ + BackendObjectReference: gwv1beta1.BackendObjectReference{ + Name: "not-a-real-service", + Kind: &serviceKind, + }, + }}}, + }, + }, + }, + }, + }, + grpcRoutes: []gwv1alpha2.GRPCRoute{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "http-route-1", + Namespace: "test", + }, + Spec: gwv1alpha2.GRPCRouteSpec{ + CommonRouteSpec: gwv1beta1.CommonRouteSpec{ParentRefs: []gwv1beta1.ParentReference{ + {Name: "gw-1"}, + }}, + Rules: []gwv1alpha2.GRPCRouteRule{ + { + BackendRefs: []gwv1alpha2.GRPCBackendRef{{BackendRef: gwv1beta1.BackendRef{ + BackendObjectReference: gwv1beta1.BackendObjectReference{ + Name: "not-a-real-service-2", + Kind: &serviceKind, + }, + }}}, + }, + }, + }, + }, + }, + gateways: []gwv1beta1.Gateway{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "gw-1", + Namespace: "test", + }, + Spec: gwv1beta1.GatewaySpec{ + GatewayClassName: "amazon-vpc-lattice", + }, + }, + }, + expectedConditionTypes: []corev1.PodConditionType{}, + }, + { + name: "gateway class missing", + omitGatewayClass: true, + pod: corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pod-1", + Namespace: "test", + Labels: map[string]string{ + "env": "test", + }, + }, + }, + services: []corev1.Service{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "svc-1", + Namespace: "test", + }, + Spec: corev1.ServiceSpec{ + Selector: map[string]string{ + "env": "test", + }, + }, + }, + }, + httpRoutes: []gwv1beta1.HTTPRoute{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "http-route-1", + Namespace: "test", + }, + Spec: gwv1beta1.HTTPRouteSpec{ + CommonRouteSpec: gwv1beta1.CommonRouteSpec{ParentRefs: []gwv1beta1.ParentReference{ + {Name: "gw-1"}, + }}, + Rules: []gwv1beta1.HTTPRouteRule{ + { + BackendRefs: []gwv1beta1.HTTPBackendRef{{BackendRef: gwv1beta1.BackendRef{ + BackendObjectReference: gwv1beta1.BackendObjectReference{ + Name: "svc-1", + Kind: &serviceKind, + }, + }}}, + }, + }, + }, + }, + }, + gateways: []gwv1beta1.Gateway{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "gw-1", + Namespace: "test", + }, + Spec: gwv1beta1.GatewaySpec{ + GatewayClassName: "amazon-vpc-lattice", + }, + }, + }, + expectedConditionTypes: []corev1.PodConditionType{}, + }, + { + name: "Update does nothing", + performUpdate: true, + pod: corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pod-1", + Namespace: "test", + Labels: map[string]string{ + "env": "test", + }, + }, + }, + services: []corev1.Service{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "svc-1", + Namespace: "test", + }, + Spec: corev1.ServiceSpec{ + Selector: map[string]string{ + "env": "test", + }, + }, + }, + }, + httpRoutes: []gwv1beta1.HTTPRoute{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "http-route-1", + Namespace: "test", + }, + Spec: gwv1beta1.HTTPRouteSpec{ + CommonRouteSpec: gwv1beta1.CommonRouteSpec{ParentRefs: []gwv1beta1.ParentReference{ + {Name: "gw-1"}, + }}, + Rules: []gwv1beta1.HTTPRouteRule{ + { + BackendRefs: []gwv1beta1.HTTPBackendRef{{BackendRef: gwv1beta1.BackendRef{ + BackendObjectReference: gwv1beta1.BackendObjectReference{ + Name: "svc-1", + Kind: &serviceKind, + }, + }}}, + }, + }, + }, + }, + }, + gateways: []gwv1beta1.Gateway{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "gw-1", + Namespace: "test", + }, + Spec: gwv1beta1.GatewaySpec{ + GatewayClassName: "amazon-vpc-lattice", + }, + }, + }, + expectedConditionTypes: []corev1.PodConditionType{}, + }, + } - injector := NewPodReadinessGateInjector(k8sClient, gwlog.FallbackLogger) - m := NewPodMutator(k8sScheme, injector) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := context.TODO() - pod := &corev1.Pod{} + k8sScheme := runtime.NewScheme() + clientgoscheme.AddToScheme(k8sScheme) + gwv1.AddToScheme(k8sScheme) + gwv1beta1.AddToScheme(k8sScheme) + gwv1alpha2.AddToScheme(k8sScheme) + anv1alpha1.AddToScheme(k8sScheme) - ret, err := m.MutateCreate(context.TODO(), pod) - newPod := ret.(*corev1.Pod) - assert.Nil(t, err) - assert.Equal(t, 1, len(newPod.Spec.ReadinessGates)) - ct := newPod.Spec.ReadinessGates[0].ConditionType - assert.Equal(t, PodReadinessGateConditionType, string(ct)) -} - -func TestReadinessGateAlreadyExists(t *testing.T) { - k8sScheme := runtime.NewScheme() - clientgoscheme.AddToScheme(k8sScheme) - - k8sClient := testclient. - NewClientBuilder(). - WithScheme(k8sScheme). - Build() + k8sClient := testclient.NewClientBuilder().WithScheme(k8sScheme).Build() - injector := NewPodReadinessGateInjector(k8sClient, gwlog.FallbackLogger) - m := NewPodMutator(k8sScheme, injector) + gwClass := &gwv1beta1.GatewayClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: "amazon-vpc-lattice", + Namespace: "default", + }, + Spec: gwv1beta1.GatewayClassSpec{ + ControllerName: "application-networking.k8s.aws/gateway-api-controller", + }, + } + if !tt.omitGatewayClass { + assert.NoError(t, k8sClient.Create(ctx, gwClass.DeepCopy())) + } - pod := &corev1.Pod{} - prg := corev1.PodReadinessGate{ConditionType: PodReadinessGateConditionType} - pod.Spec.ReadinessGates = append(pod.Spec.ReadinessGates, prg) - - ret, err := m.MutateCreate(context.TODO(), pod) - newPod := ret.(*corev1.Pod) - assert.Nil(t, err) - assert.Equal(t, 1, len(newPod.Spec.ReadinessGates)) - ct := newPod.Spec.ReadinessGates[0].ConditionType - assert.Equal(t, PodReadinessGateConditionType, string(ct)) -} + for _, svc := range tt.services { + assert.NoError(t, k8sClient.Create(ctx, svc.DeepCopy())) + } + for _, httpRoute := range tt.httpRoutes { + assert.NoError(t, k8sClient.Create(ctx, httpRoute.DeepCopy())) + } + for _, v1HttpRoute := range tt.v1HttpRoutes { + assert.NoError(t, k8sClient.Create(ctx, v1HttpRoute.DeepCopy())) + } + for _, grpcRoute := range tt.grpcRoutes { + assert.NoError(t, k8sClient.Create(ctx, grpcRoute.DeepCopy())) + } + for _, gw := range tt.gateways { + assert.NoError(t, k8sClient.Create(ctx, gw.DeepCopy())) + } + if tt.svcExport != nil { + assert.NoError(t, k8sClient.Create(ctx, tt.svcExport.DeepCopy())) + } -func TestUpdateDoesNothing(t *testing.T) { - k8sScheme := runtime.NewScheme() - clientgoscheme.AddToScheme(k8sScheme) + injector := NewPodReadinessGateInjector(k8sClient, gwlog.FallbackLogger) + m := NewPodMutator(gwlog.FallbackLogger, k8sScheme, injector) - k8sClient := testclient. - NewClientBuilder(). - WithScheme(k8sScheme). - Build() + var retPod runtime.Object + var err error - injector := NewPodReadinessGateInjector(k8sClient, gwlog.FallbackLogger) - m := NewPodMutator(k8sScheme, injector) + if tt.performUpdate { + retPod, err = m.MutateUpdate(context.TODO(), &tt.pod, &tt.pod) + } else { + retPod, err = m.MutateCreate(context.TODO(), &tt.pod) + } + assert.Nil(t, err) - p1 := &corev1.Pod{} - p1.Spec.Hostname = "foo" - p2 := &corev1.Pod{} - p2.Spec.Hostname = "bar" + expectedConditionsMap := make(map[string]corev1.PodConditionType) + for _, conditionType := range tt.expectedConditionTypes { + expectedConditionsMap[string(conditionType)] = conditionType + } + actualConditionsMap := make(map[string]corev1.PodConditionType) + for _, gate := range retPod.(*corev1.Pod).Spec.ReadinessGates { + actualConditionsMap[string(gate.ConditionType)] = gate.ConditionType + } - ret, err := m.MutateUpdate(context.TODO(), p1, p2) - newPod := ret.(*corev1.Pod) - assert.Nil(t, err) - assert.Equal(t, 0, len(newPod.Spec.ReadinessGates)) + assert.Equal(t, len(expectedConditionsMap), len(actualConditionsMap)) + for k := range expectedConditionsMap { + _, ok := actualConditionsMap[k] + assert.Truef(t, ok, "expected pod condition type %s not found", k) + } + }) + } } diff --git a/pkg/webhook/pod_readiness_gate_injector.go b/pkg/webhook/pod_readiness_gate_injector.go index 3615a121..424109b7 100644 --- a/pkg/webhook/pod_readiness_gate_injector.go +++ b/pkg/webhook/pod_readiness_gate_injector.go @@ -2,9 +2,18 @@ package webhook import ( "context" + anv1alpha1 "github.com/aws/aws-application-networking-k8s/pkg/apis/applicationnetworking/v1alpha1" + "github.com/aws/aws-application-networking-k8s/pkg/config" + k8sutils "github.com/aws/aws-application-networking-k8s/pkg/k8s" + "github.com/aws/aws-application-networking-k8s/pkg/model/core" "github.com/aws/aws-application-networking-k8s/pkg/utils/gwlog" + "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" + gwv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" + gwv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1" ) const ( @@ -23,9 +32,9 @@ type PodReadinessGateInjector struct { log gwlog.Logger } -func (m *PodReadinessGateInjector) Mutate(ctx context.Context, pod *corev1.Pod) error { +func (m *PodReadinessGateInjector) MutateCreate(ctx context.Context, pod *corev1.Pod) error { pct := corev1.PodConditionType(PodReadinessGateConditionType) - m.log.Debugf("Webhook invoked for pod %s/%s", pod.Name, pod.Namespace) + m.log.Debugf("Webhook invoked for pod %s/%s", pod.Namespace, getPodName(pod)) found := false for _, rg := range pod.Spec.ReadinessGates { @@ -35,9 +44,176 @@ func (m *PodReadinessGateInjector) Mutate(ctx context.Context, pod *corev1.Pod) } } if !found { - pod.Spec.ReadinessGates = append(pod.Spec.ReadinessGates, corev1.PodReadinessGate{ - ConditionType: pct, - }) + requiresGate, err := m.requiresReadinessGate(ctx, pod) + if err != nil { + return err + } + if requiresGate { + pod.Spec.ReadinessGates = append(pod.Spec.ReadinessGates, corev1.PodReadinessGate{ + ConditionType: pct, + }) + } } return nil } + +// checks if the pod requires a readiness gate +// mostly debug logs to reduce noise, intended to be tolerant of most failures +func (m *PodReadinessGateInjector) requiresReadinessGate(ctx context.Context, pod *corev1.Pod) (bool, error) { + // fetch all services in the namespace, see if their selector matches the pod + svcList := &corev1.ServiceList{} + if err := m.k8sClient.List(ctx, svcList, client.InNamespace(pod.Namespace)); err != nil { + return false, errors.Wrap(err, "unable to determine readiness gate requirement") + } + + svcMatches := m.servicesForPod(pod, svcList) + if len(svcMatches) == 0 { + m.log.Debugf("No services found for pod %s/%s", pod.Namespace, getPodName(pod)) + return false, nil + } + + // for each route, check if it has a backendRef to one of the services + routes := m.listAllRoutes(ctx) + for _, route := range routes { + if svc := m.isPodUsedByRoute(route, svcMatches); svc != nil { + if m.routeHasLatticeGateway(ctx, route) { + m.log.Debugf("Pod %s/%s is used by service %s/%s and route %s/%s", pod.Namespace, getPodName(pod), + svc.Namespace, svc.Name, route.Namespace(), route.Name()) + return true, nil + } + } + } + + // lastly, check if there's a service export for any of the services + for _, svc := range svcMatches { + svcExport := &anv1alpha1.ServiceExport{} + if err := m.k8sClient.Get(ctx, k8sutils.NamespacedName(svc), svcExport); err != nil { + continue + } + + m.log.Debugf("Pod %s/%s is used by service %s/%s and service export %s/%s", pod.Namespace, getPodName(pod), + svc.Namespace, svc.Name, svcExport.Namespace, svcExport.Name) + return true, nil + } + + m.log.Debugf("Pod %s/%s does not require a readiness gate", pod.Namespace, getPodName(pod)) + return false, nil +} + +func (m *PodReadinessGateInjector) listAllRoutes(ctx context.Context) []core.Route { + // fetch all routes in all namespaces - backendRefs can reference other namespaces + var routes []core.Route + httpRouteList := &gwv1beta1.HTTPRouteList{} + err := m.k8sClient.List(ctx, httpRouteList) + if err != nil { + m.log.Errorf("Error fetching beta1 HTTPRoutes: %s", err) + } + for _, k8sRoute := range httpRouteList.Items { + routes = append(routes, core.NewHTTPRoute(k8sRoute)) + } + + grpcRouteList := &gwv1alpha2.GRPCRouteList{} + err = m.k8sClient.List(ctx, grpcRouteList) + if err != nil { + m.log.Errorf("Error fetching GRPCRoutes: %s", err) + } + for _, k8sRoute := range grpcRouteList.Items { + routes = append(routes, core.NewGRPCRoute(k8sRoute)) + } + return routes +} + +func getPodName(pod *corev1.Pod) string { + if pod == nil { + return "" + } else if pod.Name == "" { + return pod.GenerateName + } else { + return pod.Name + } +} + +// returns a map of services that match the pod labels +func (m *PodReadinessGateInjector) servicesForPod(pod *corev1.Pod, svcList *corev1.ServiceList) map[string]*corev1.Service { + svcMatches := make(map[string]*corev1.Service) + podLabels := labels.Set(pod.Labels) + for _, svc := range svcList.Items { + svcSelector := labels.SelectorFromSet(svc.Spec.Selector) + if svcSelector.Matches(podLabels) { + m.log.Debugf("Found service %s/%s that matches pod %s/%s", + svc.Namespace, svc.Name, pod.Namespace, getPodName(pod)) + + svcMatches[svc.Name] = &svc + } + } + return svcMatches +} + +func (m *PodReadinessGateInjector) isPodUsedByRoute(route core.Route, svcMap map[string]*corev1.Service) *corev1.Service { + for _, rule := range route.Spec().Rules() { + for _, backendRef := range rule.BackendRefs() { + // from spec: "When [Group] unspecified or empty string, core API group is inferred." + isGroupEqual := backendRef.Group() == nil || string(*backendRef.Group()) == corev1.GroupName + isKindEqual := backendRef.Kind() != nil && string(*backendRef.Kind()) == "Service" + svc, isNameEqual := svcMap[string(backendRef.Name())] + + namespace := route.Namespace() + if backendRef.Namespace() != nil { + namespace = string(*backendRef.Namespace()) + } + isNamespaceEqual := svc != nil && namespace == svc.GetNamespace() + + if isGroupEqual && isKindEqual && isNameEqual && isNamespaceEqual { + m.log.Debugf("Found route %s/%s that matches service %s/%s", + route.Namespace(), route.Name(), svc.Namespace, svc.Name) + + return svc + } + } + } + return nil +} + +func (m *PodReadinessGateInjector) routeHasLatticeGateway(ctx context.Context, route core.Route) bool { + if len(route.Spec().ParentRefs()) == 0 { + m.log.Debugf("Route %s/%s has no parentRefs", route.Namespace(), route.Name()) + return false + } + + gw := &gwv1beta1.Gateway{} + gwNamespace := route.Namespace() + if route.Spec().ParentRefs()[0].Namespace != nil { + gwNamespace = string(*route.Spec().ParentRefs()[0].Namespace) + } + gwName := types.NamespacedName{ + Namespace: gwNamespace, + Name: string(route.Spec().ParentRefs()[0].Name), + } + + if err := m.k8sClient.Get(ctx, gwName, gw); err != nil { + m.log.Debugf("Unable to retrieve gateway %s/%s for route %s/%s, %s", + gwName.Namespace, gwName.Name, route.Namespace(), route.Name(), err) + return false + } + + // make sure gateway is an aws-vpc-lattice + gwClass := &gwv1beta1.GatewayClass{} + gwClassName := types.NamespacedName{ + Namespace: "default", + Name: string(gw.Spec.GatewayClassName), + } + + if err := m.k8sClient.Get(ctx, gwClassName, gwClass); err != nil { + m.log.Debugf("Unable to retrieve gateway class %s/%s for gateway %s/%s, %s", + gwClassName.Namespace, gwClass.Name, gwName.Namespace, gwName.Name, err) + return false + } + + if gwClass.Spec.ControllerName == config.LatticeGatewayControllerName { + m.log.Debugf("Gateway %s/%s is a lattice gateway", gwName.Namespace, gwName.Name) + return true + } + + m.log.Debugf("Gateway %s/%s is not a lattice gateway", gwName.Namespace, gwName.Name) + return false +} diff --git a/test/pkg/test/framework.go b/test/pkg/test/framework.go index a2d4c659..c554ba51 100644 --- a/test/pkg/test/framework.go +++ b/test/pkg/test/framework.go @@ -185,7 +185,11 @@ func objectsInfo(objs []client.Object) string { func (env *Framework) ExpectCreated(ctx context.Context, objects ...client.Object) { env.Log.Infof("Creating objects: %s", objectsInfo(objects)) parallel.ForEach(objects, func(obj client.Object, _ int) { - Expect(env.Create(ctx, obj)).WithOffset(1).To(Succeed()) + err := env.Create(ctx, obj) + if err != nil { + env.Log.Errorf("Error creating object %s, %s", obj, err) + } + Expect(err).WithOffset(1).To(BeNil()) }) } diff --git a/test/pkg/test/httproute.go b/test/pkg/test/httproute.go new file mode 100644 index 00000000..05efdecd --- /dev/null +++ b/test/pkg/test/httproute.go @@ -0,0 +1,42 @@ +package test + +import ( + "github.com/samber/lo" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + gwv1 "sigs.k8s.io/gateway-api/apis/v1" +) + +func (env *Framework) NewHttpRoute(parentRefsGateway *gwv1.Gateway, service *corev1.Service, kind string) *gwv1.HTTPRoute { + var rules []gwv1.HTTPRouteRule + rule := gwv1.HTTPRouteRule{ + BackendRefs: []gwv1.HTTPBackendRef{{ + BackendRef: gwv1.BackendRef{ + BackendObjectReference: gwv1.BackendObjectReference{ + Name: gwv1.ObjectName(service.Name), + Namespace: (*gwv1.Namespace)(&service.Namespace), + Kind: lo.ToPtr(gwv1.Kind(kind)), + Port: (*gwv1.PortNumber)(&service.Spec.Ports[0].Port), + }, + }, + }}, + } + rules = append(rules, rule) + parentNS := gwv1.Namespace(parentRefsGateway.Namespace) + httpRoute := New(&gwv1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: service.Namespace, + Name: service.Name, + }, + Spec: gwv1.HTTPRouteSpec{ + CommonRouteSpec: gwv1.CommonRouteSpec{ + ParentRefs: []gwv1.ParentReference{{ + Name: gwv1.ObjectName(parentRefsGateway.Name), + Namespace: &parentNS, + }}, + }, + Rules: rules, + }, + }) + return httpRoute +} diff --git a/test/pkg/test/nginxapp.go b/test/pkg/test/nginxapp.go index 330aceac..d786d1f7 100644 --- a/test/pkg/test/nginxapp.go +++ b/test/pkg/test/nginxapp.go @@ -7,7 +7,6 @@ import ( v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" - gwv1 "sigs.k8s.io/gateway-api/apis/v1" ) type ElasticSearchOptions struct { @@ -128,35 +127,3 @@ func (env *Framework) NewNginxApp(options ElasticSearchOptions) (*appsv1.Deploym return deployment, service } - -func (env *Framework) NewHttpRoute(parentRefsGateway *gwv1.Gateway, service *v1.Service, kind string) *gwv1.HTTPRoute { - var rules []gwv1.HTTPRouteRule - rule := gwv1.HTTPRouteRule{ - BackendRefs: []gwv1.HTTPBackendRef{{ - BackendRef: gwv1.BackendRef{ - BackendObjectReference: gwv1.BackendObjectReference{ - Name: gwv1.ObjectName(service.Name), - Namespace: (*gwv1.Namespace)(&service.Namespace), - Kind: lo.ToPtr(gwv1.Kind(kind)), - Port: (*gwv1.PortNumber)(&service.Spec.Ports[0].Port), - }, - }, - }}, - } - rules = append(rules, rule) - httpRoute := New(&gwv1.HTTPRoute{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: service.Namespace, - Name: service.Name, - }, - Spec: gwv1.HTTPRouteSpec{ - CommonRouteSpec: gwv1.CommonRouteSpec{ - ParentRefs: []gwv1.ParentReference{{ - Name: gwv1.ObjectName(parentRefsGateway.Name), - }}, - }, - Rules: rules, - }, - }) - return httpRoute -} diff --git a/test/suites/webhook/readiness_gate_inject_test.go b/test/suites/webhook/readiness_gate_inject_test.go index 861cefab..66923bbe 100644 --- a/test/suites/webhook/readiness_gate_inject_test.go +++ b/test/suites/webhook/readiness_gate_inject_test.go @@ -2,6 +2,7 @@ package webhook import ( "fmt" + "github.com/aws/aws-application-networking-k8s/pkg/utils" "github.com/aws/aws-application-networking-k8s/pkg/webhook" "github.com/aws/aws-application-networking-k8s/test/pkg/test" . "github.com/onsi/ginkgo/v2" @@ -40,8 +41,9 @@ var _ = Describe("Readiness Gate Inject", Ordered, func() { }) It("create deployment in untagged namespace, no readiness gate", func() { - deployment, _ := testFramework.NewHttpApp(test.HTTPAppOptions{Name: "untagged-test-pod", Namespace: untaggedNS.Name}) + deployment, service := testFramework.NewHttpApp(test.HTTPAppOptions{Name: "untagged-test-pod", Namespace: untaggedNS.Name}) Eventually(func(g Gomega) { + testFramework.Create(ctx, service) testFramework.Create(ctx, deployment) testFramework.Get(ctx, types.NamespacedName{Name: deployment.Name, Namespace: deployment.Namespace}, deployment) @@ -59,9 +61,10 @@ var _ = Describe("Readiness Gate Inject", Ordered, func() { }).WithTimeout(30 * time.Second).WithOffset(1).Should(Succeed()) }) - It("create deployment in tagged namespace, has readiness gate", func() { - deployment, _ := testFramework.NewHttpApp(test.HTTPAppOptions{Name: "tagged-test-pod", Namespace: taggedNS.Name}) + It("create deployment in tagged namespace, but no gateway/route reference, no readiness gate", func() { + deployment, service := testFramework.NewHttpApp(test.HTTPAppOptions{Name: "tagged-nope-pod", Namespace: taggedNS.Name}) Eventually(func(g Gomega) { + testFramework.Create(ctx, service) testFramework.Create(ctx, deployment) testFramework.Get(ctx, types.NamespacedName{Name: deployment.Name, Namespace: deployment.Namespace}, deployment) @@ -71,6 +74,36 @@ var _ = Describe("Readiness Gate Inject", Ordered, func() { pod := pods[0] pct := corev1.PodConditionType(webhook.PodReadinessGateConditionType) + for _, rg := range pod.Spec.ReadinessGates { + if rg.ConditionType == pct { + g.Expect(true).To(BeFalse(), "Pod readiness gate was injected without gateway/route reference") + } + } + }).WithTimeout(30 * time.Second).WithOffset(1).Should(Succeed()) + }) + + It("create deployment in tagged namespace, gate injected and transitions to healthy", func() { + deployment, service := testFramework.NewHttpApp(test.HTTPAppOptions{Name: "tagged-yes-pod", Namespace: taggedNS.Name}) + httpRoute := testFramework.NewHttpRoute(testGateway, service, "Service") + + // first create the http route so we can be sure the readiness gate gets flagged + err := testFramework.Create(ctx, httpRoute) + Expect(err).ToNot(HaveOccurred()) + err = testFramework.Create(ctx, service) + Expect(err).ToNot(HaveOccurred()) + // creating the deployment will trigger the readiness gate injection + err = testFramework.Create(ctx, deployment) + Expect(err).ToNot(HaveOccurred()) + + Eventually(func(g Gomega) { + testFramework.Get(ctx, types.NamespacedName{Name: deployment.Name, Namespace: deployment.Namespace}, deployment) + + pods := testFramework.GetPodsByDeploymentName(deployment.Name, deployment.Namespace) + g.Expect(len(pods)).To(BeEquivalentTo(1)) + + pod := pods[0] + pct := corev1.PodConditionType(webhook.PodReadinessGateConditionType) + foundCount := 0 for _, rg := range pod.Spec.ReadinessGates { if rg.ConditionType == pct { @@ -79,8 +112,12 @@ var _ = Describe("Readiness Gate Inject", Ordered, func() { } g.Expect(foundCount).To(Equal(1), - fmt.Sprintf("Pod readiness gate was expected on labeled namespace. Found %d times", foundCount)) - }).WithTimeout(30 * time.Second).WithOffset(1).Should(Succeed()) + fmt.Sprintf("One Pod readiness gate is expected. Found %d times", foundCount)) + + status := utils.FindPodStatusCondition(pod.Status.Conditions, pct) + g.Expect(status).ToNot(BeNil(), "Pod status should not be nil") + g.Expect(status.Status).To(Equal(corev1.ConditionTrue), "Pod status should be true") + }).WithTimeout(180 * time.Second).WithOffset(1).Should(Succeed()) }) AfterAll(func() { diff --git a/test/suites/webhook/suite_test.go b/test/suites/webhook/suite_test.go index 1e9cdadd..3f9007db 100644 --- a/test/suites/webhook/suite_test.go +++ b/test/suites/webhook/suite_test.go @@ -2,14 +2,14 @@ package webhook import ( "context" - "os" - "github.com/aws/aws-application-networking-k8s/pkg/utils/gwlog" "github.com/aws/aws-application-networking-k8s/test/pkg/test" + "github.com/aws/aws-sdk-go/service/vpclattice" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "go.uber.org/zap" - + "os" + gwv1 "sigs.k8s.io/gateway-api/apis/v1" "testing" ) @@ -19,13 +19,29 @@ const ( var testFramework *test.Framework var ctx context.Context +var testGateway *gwv1.Gateway +var testServiceNetwork *vpclattice.ServiceNetworkSummary var _ = SynchronizedBeforeSuite(func() { vpcId := os.Getenv("CLUSTER_VPC_ID") if vpcId == "" { Fail("CLUSTER_VPC_ID environment variable must be set to run integration tests") } + + // provision gateway, wait for service network association + testGateway = testFramework.NewGateway("test-gateway", k8snamespace) + testFramework.ExpectCreated(ctx, testGateway) + + testServiceNetwork = testFramework.GetServiceNetwork(ctx, testGateway) + + testFramework.Log.Infof("Expecting VPC %s and service network %s association", vpcId, *testServiceNetwork.Id) + Eventually(func(g Gomega) { + associated, _, _ := testFramework.IsVpcAssociatedWithServiceNetwork(ctx, vpcId, testServiceNetwork) + g.Expect(associated).To(BeTrue()) + }).Should(Succeed()) }, func() { + testGateway = testFramework.NewGateway("test-gateway", k8snamespace) + testServiceNetwork = testFramework.GetServiceNetwork(ctx, testGateway) }) func TestIntegration(t *testing.T) { @@ -37,4 +53,5 @@ func TestIntegration(t *testing.T) { } var _ = SynchronizedAfterSuite(func() {}, func() { + testFramework.ExpectDeletedThenNotFound(ctx, testGateway) })