From 087a5daa6aab92946e07139a9fa5aa0b4bd545d3 Mon Sep 17 00:00:00 2001 From: Nathan Coleman Date: Tue, 31 May 2022 18:21:53 -0400 Subject: [PATCH 01/11] Stub gateway controller watch on ReferencePolicy --- internal/k8s/controller.go | 1 + .../k8s/controllers/gateway_controller.go | 40 +++++++++++++++++++ .../k8s/controllers/http_route_controller.go | 4 +- 3 files changed, 44 insertions(+), 1 deletion(-) diff --git a/internal/k8s/controller.go b/internal/k8s/controller.go index 4f97ea46b..e44cb2bab 100644 --- a/internal/k8s/controller.go +++ b/internal/k8s/controller.go @@ -179,6 +179,7 @@ func (k *Kubernetes) Start(ctx context.Context) error { } err = (&controllers.GatewayReconciler{ + Context: ctx, Client: gwClient, Log: k.logger.Named("Gateway"), Manager: reconcileManager, diff --git a/internal/k8s/controllers/gateway_controller.go b/internal/k8s/controllers/gateway_controller.go index 9777c7fc1..18b47482f 100644 --- a/internal/k8s/controllers/gateway_controller.go +++ b/internal/k8s/controllers/gateway_controller.go @@ -27,6 +27,7 @@ import ( // GatewayReconciler reconciles a Gateway object type GatewayReconciler struct { + Context context.Context Client gatewayclient.Client Log hclog.Logger ControllerName string @@ -93,6 +94,10 @@ func (r *GatewayReconciler) SetupWithManager(mgr ctrl.Manager) error { handler.EnqueueRequestsFromMapFunc(podToGatewayRequest), builder.WithPredicates(predicate), ). + Watches( + &source.Kind{Type: &gateway.ReferencePolicy{}}, + handler.EnqueueRequestsFromMapFunc(r.referencePolicyToGatewayRequests), + ). Complete(gatewayclient.NewRequeueingMiddleware(r.Log, r)) } @@ -109,3 +114,38 @@ func podToGatewayRequest(object client.Object) []reconcile.Request { } return nil } + +func (r *GatewayReconciler) referencePolicyToGatewayRequests(object client.Object) []reconcile.Request { + refPolicy := object.(*gateway.ReferencePolicy) + + gateways := r.getGatewaysAffectedByReferencePolicy(refPolicy) + requests := []reconcile.Request{} + + for _, gw := range gateways { + requests = append(requests, reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: gw.Name, + Namespace: gw.Namespace, + }, + }) + } + + return requests +} + +func (r *GatewayReconciler) getGatewaysAffectedByReferencePolicy(refPolicy *gateway.ReferencePolicy) []gateway.Gateway { + var matches []gateway.Gateway + + for _, from := range refPolicy.Spec.From { + // TODO + gateways, err := r.Client.GetGatewaysInNamespace(r.Context, string(from.Namespace)) + if err != nil { + r.Log.Error("error fetching gateways", err) + return matches + } + + matches = append(matches, gateways...) + } + + return matches +} diff --git a/internal/k8s/controllers/http_route_controller.go b/internal/k8s/controllers/http_route_controller.go index f4c90dd0b..60bbb64e7 100644 --- a/internal/k8s/controllers/http_route_controller.go +++ b/internal/k8s/controllers/http_route_controller.go @@ -11,9 +11,10 @@ import ( "sigs.k8s.io/controller-runtime/pkg/source" gateway "sigs.k8s.io/gateway-api/apis/v1alpha2" + "github.com/hashicorp/go-hclog" + "github.com/hashicorp/consul-api-gateway/internal/k8s/gatewayclient" "github.com/hashicorp/consul-api-gateway/internal/k8s/reconciler" - "github.com/hashicorp/go-hclog" ) // HTTPRouteReconciler reconciles a HTTPRoute object @@ -97,6 +98,7 @@ func (r *HTTPRouteReconciler) referencePolicyToRouteRequests(object client.Objec func (r *HTTPRouteReconciler) getRoutesAffectedByReferencePolicy(refPolicy *gateway.ReferencePolicy) []gateway.HTTPRoute { matches := []gateway.HTTPRoute{} + // TODO Why doesn't this function just return the value here? routes := r.getReferencePolicyObjectsFrom(refPolicy) // TODO: match only routes with BackendRefs selectable by a From d11b3769ba3e2880855ba610a3e6defb4406c771 Mon Sep 17 00:00:00 2001 From: Nathan Coleman Date: Tue, 31 May 2022 18:36:19 -0400 Subject: [PATCH 02/11] Reconcile all Gateways in referenced namespace on ReferencePolicy watch --- internal/k8s/controllers/gateway_controller.go | 12 ++++++++---- internal/k8s/controllers/http_route_controller.go | 2 +- internal/k8s/gatewayclient/gatewayclient.go | 9 +++++++++ 3 files changed, 18 insertions(+), 5 deletions(-) diff --git a/internal/k8s/controllers/gateway_controller.go b/internal/k8s/controllers/gateway_controller.go index 18b47482f..0573c6626 100644 --- a/internal/k8s/controllers/gateway_controller.go +++ b/internal/k8s/controllers/gateway_controller.go @@ -102,12 +102,12 @@ func (r *GatewayReconciler) SetupWithManager(mgr ctrl.Manager) error { } func podToGatewayRequest(object client.Object) []reconcile.Request { - gateway, managed := utils.IsManagedGateway(object.GetLabels()) + gw, managed := utils.IsManagedGateway(object.GetLabels()) if managed { return []reconcile.Request{ {NamespacedName: types.NamespacedName{ - Name: gateway, + Name: gw, Namespace: object.GetNamespace(), }}, } @@ -119,7 +119,8 @@ func (r *GatewayReconciler) referencePolicyToGatewayRequests(object client.Objec refPolicy := object.(*gateway.ReferencePolicy) gateways := r.getGatewaysAffectedByReferencePolicy(refPolicy) - requests := []reconcile.Request{} + + requests := make([]reconcile.Request, 0, len(gateways)) for _, gw := range gateways { requests = append(requests, reconcile.Request{ @@ -133,11 +134,14 @@ func (r *GatewayReconciler) referencePolicyToGatewayRequests(object client.Objec return requests } +// getGatewaysAffectedByReferencePolicy retrieves all Gateways potentially impacted by the ReferencePolicy +// modification. Currently, this is unfiltered and so returns all Gateways in the namespace referenced by +// the ReferencePolicy. func (r *GatewayReconciler) getGatewaysAffectedByReferencePolicy(refPolicy *gateway.ReferencePolicy) []gateway.Gateway { var matches []gateway.Gateway for _, from := range refPolicy.Spec.From { - // TODO + // TODO: search by from.Group and from.Kind instead of assuming this ReferencePolicy references a Gateway gateways, err := r.Client.GetGatewaysInNamespace(r.Context, string(from.Namespace)) if err != nil { r.Log.Error("error fetching gateways", err) diff --git a/internal/k8s/controllers/http_route_controller.go b/internal/k8s/controllers/http_route_controller.go index 60bbb64e7..f56335000 100644 --- a/internal/k8s/controllers/http_route_controller.go +++ b/internal/k8s/controllers/http_route_controller.go @@ -113,7 +113,7 @@ func (r *HTTPRouteReconciler) getReferencePolicyObjectsFrom(refPolicy *gateway.R matches := []gateway.HTTPRoute{} for _, from := range refPolicy.Spec.From { - // TODO: search by from.Group and from.Kind instead of assuming HTTPRoute + // TODO: search by from.Group and from.Kind instead of assuming this ReferencePolicy references a HTTPRoute routes, err := r.Client.GetHTTPRoutesInNamespace(r.Context, string(from.Namespace)) if err != nil { r.Log.Error("error fetching routes", err) diff --git a/internal/k8s/gatewayclient/gatewayclient.go b/internal/k8s/gatewayclient/gatewayclient.go index f558d89fc..169670506 100644 --- a/internal/k8s/gatewayclient/gatewayclient.go +++ b/internal/k8s/gatewayclient/gatewayclient.go @@ -30,6 +30,7 @@ type Client interface { GetGatewayClassConfig(ctx context.Context, key types.NamespacedName) (*apigwv1alpha1.GatewayClassConfig, error) GetGatewayClass(ctx context.Context, key types.NamespacedName) (*gateway.GatewayClass, error) GetGateway(ctx context.Context, key types.NamespacedName) (*gateway.Gateway, error) + GetGatewaysInNamespace(ctx context.Context, ns string) ([]gateway.Gateway, error) GetSecret(ctx context.Context, key types.NamespacedName) (*core.Secret, error) GetService(ctx context.Context, key types.NamespacedName) (*core.Service, error) GetHTTPRoute(ctx context.Context, key types.NamespacedName) (*gateway.HTTPRoute, error) @@ -216,6 +217,14 @@ func (g *gatewayClient) GetGateway(ctx context.Context, key types.NamespacedName return gw, nil } +func (g *gatewayClient) GetGatewaysInNamespace(ctx context.Context, ns string) ([]gateway.Gateway, error) { + gwList := &gateway.GatewayList{} + if err := g.Client.List(ctx, gwList, client.InNamespace(ns)); err != nil { + return []gateway.Gateway{}, NewK8sError(err) + } + return gwList.Items, nil +} + func (g *gatewayClient) GetService(ctx context.Context, key types.NamespacedName) (*core.Service, error) { svc := &core.Service{} if err := g.Client.Get(ctx, key, svc); err != nil { From 039fcad0764026318e88cd445f5c088574f3f954 Mon Sep 17 00:00:00 2001 From: Nathan Coleman Date: Wed, 1 Jun 2022 11:51:56 -0400 Subject: [PATCH 03/11] Regenerate mock gatewayclient --- internal/k8s/gatewayclient/mocks/gatewayclient.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/internal/k8s/gatewayclient/mocks/gatewayclient.go b/internal/k8s/gatewayclient/mocks/gatewayclient.go index ece191f89..10b22cf3d 100644 --- a/internal/k8s/gatewayclient/mocks/gatewayclient.go +++ b/internal/k8s/gatewayclient/mocks/gatewayclient.go @@ -259,6 +259,21 @@ func (mr *MockClientMockRecorder) GetGatewayClassConfig(ctx, key interface{}) *g return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetGatewayClassConfig", reflect.TypeOf((*MockClient)(nil).GetGatewayClassConfig), ctx, key) } +// GetGatewaysInNamespace mocks base method. +func (m *MockClient) GetGatewaysInNamespace(ctx context.Context, ns string) ([]v1alpha2.Gateway, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetGatewaysInNamespace", ctx, ns) + ret0, _ := ret[0].([]v1alpha2.Gateway) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetGatewaysInNamespace indicates an expected call of GetGatewaysInNamespace. +func (mr *MockClientMockRecorder) GetGatewaysInNamespace(ctx, ns interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetGatewaysInNamespace", reflect.TypeOf((*MockClient)(nil).GetGatewaysInNamespace), ctx, ns) +} + // GetHTTPRoute mocks base method. func (m *MockClient) GetHTTPRoute(ctx context.Context, key types.NamespacedName) (*v1alpha2.HTTPRoute, error) { m.ctrl.T.Helper() From 383448168c6a2e436e4328d42c680dc0140efdec Mon Sep 17 00:00:00 2001 From: Nathan Coleman Date: Wed, 1 Jun 2022 15:11:49 -0400 Subject: [PATCH 04/11] Remove unnecessary middle functions for fetching impacted xRoutes --- .../k8s/controllers/http_route_controller.go | 19 ++++----------- .../k8s/controllers/tcp_route_controller.go | 23 ++++++------------- 2 files changed, 11 insertions(+), 31 deletions(-) diff --git a/internal/k8s/controllers/http_route_controller.go b/internal/k8s/controllers/http_route_controller.go index f56335000..cf2c3e4ad 100644 --- a/internal/k8s/controllers/http_route_controller.go +++ b/internal/k8s/controllers/http_route_controller.go @@ -95,22 +95,11 @@ func (r *HTTPRouteReconciler) referencePolicyToRouteRequests(object client.Objec return requests } +// getRoutesAffectedByReferencePolicy retrieves all HTTPRoutes potentially impacted +// by the ReferencePolicy being modified. Currently, this is unfiltered and so returns +// all HTTPRoutes in the namespace referenced by the ReferencePolicy. func (r *HTTPRouteReconciler) getRoutesAffectedByReferencePolicy(refPolicy *gateway.ReferencePolicy) []gateway.HTTPRoute { - matches := []gateway.HTTPRoute{} - - // TODO Why doesn't this function just return the value here? - routes := r.getReferencePolicyObjectsFrom(refPolicy) - - // TODO: match only routes with BackendRefs selectable by a - // ReferencePolicyTo instead of appending all routes. This seems expensive, - // so not sure if it would actually improve performance or not. - matches = append(matches, routes...) - - return matches -} - -func (r *HTTPRouteReconciler) getReferencePolicyObjectsFrom(refPolicy *gateway.ReferencePolicy) []gateway.HTTPRoute { - matches := []gateway.HTTPRoute{} + var matches []gateway.HTTPRoute for _, from := range refPolicy.Spec.From { // TODO: search by from.Group and from.Kind instead of assuming this ReferencePolicy references a HTTPRoute diff --git a/internal/k8s/controllers/tcp_route_controller.go b/internal/k8s/controllers/tcp_route_controller.go index bd87f6a8e..d3373a70e 100644 --- a/internal/k8s/controllers/tcp_route_controller.go +++ b/internal/k8s/controllers/tcp_route_controller.go @@ -11,9 +11,10 @@ import ( "sigs.k8s.io/controller-runtime/pkg/source" gateway "sigs.k8s.io/gateway-api/apis/v1alpha2" + "github.com/hashicorp/go-hclog" + "github.com/hashicorp/consul-api-gateway/internal/k8s/gatewayclient" "github.com/hashicorp/consul-api-gateway/internal/k8s/reconciler" - "github.com/hashicorp/go-hclog" ) // TCPRouteReconciler reconciles a TCPRoute object @@ -94,24 +95,14 @@ func (r *TCPRouteReconciler) referencePolicyToRouteRequests(object client.Object return requests } +// getRoutesAffectedByReferencePolicy retrieves all TCPRoutes potentially impacted +// by the ReferencePolicy being modified. Currently, this is unfiltered and so returns +// all TCPRoutes in the namespace referenced by the ReferencePolicy. func (r *TCPRouteReconciler) getRoutesAffectedByReferencePolicy(refPolicy *gateway.ReferencePolicy) []gateway.TCPRoute { - matches := []gateway.TCPRoute{} - - routes := r.getReferencePolicyObjectsFrom(refPolicy) - - // TODO: match only routes with BackendRefs selectable by a - // ReferencePolicyTo instead of appending all routes. This seems expensive, - // so not sure if it would actually improve performance or not. - matches = append(matches, routes...) - - return matches -} - -func (r *TCPRouteReconciler) getReferencePolicyObjectsFrom(refPolicy *gateway.ReferencePolicy) []gateway.TCPRoute { - matches := []gateway.TCPRoute{} + var matches []gateway.TCPRoute for _, from := range refPolicy.Spec.From { - // TODO: search by from.Group and from.Kind instead of assuming TCPRoute + // TODO: search by from.Group and from.Kind instead of assuming this ReferencePolicy references a TCPRoute routes, err := r.Client.GetTCPRoutesInNamespace(r.Context, string(from.Namespace)) if err != nil { r.Log.Error("error fetching routes", err) From afaf5c93b97cc5a85cbff66002a33146b6a00852 Mon Sep 17 00:00:00 2001 From: Nathan Coleman Date: Wed, 1 Jun 2022 19:15:56 -0400 Subject: [PATCH 05/11] Add e2e test for ReferencePolicy lifecycle applied to Gateways --- internal/commands/server/k8s_e2e_test.go | 141 ++++++++++++++++++++--- 1 file changed, 123 insertions(+), 18 deletions(-) diff --git a/internal/commands/server/k8s_e2e_test.go b/internal/commands/server/k8s_e2e_test.go index 00bf80b98..ca92a57bc 100644 --- a/internal/commands/server/k8s_e2e_test.go +++ b/internal/commands/server/k8s_e2e_test.go @@ -28,11 +28,12 @@ import ( "sigs.k8s.io/e2e-framework/pkg/features" gateway "sigs.k8s.io/gateway-api/apis/v1alpha2" + "github.com/hashicorp/consul/api" + appsv1 "k8s.io/api/apps/v1" + "github.com/hashicorp/consul-api-gateway/internal/k8s" "github.com/hashicorp/consul-api-gateway/internal/testing/e2e" apigwv1alpha1 "github.com/hashicorp/consul-api-gateway/pkg/apis/v1alpha1" - "github.com/hashicorp/consul/api" - appsv1 "k8s.io/api/apps/v1" ) var ( @@ -83,7 +84,7 @@ func TestGatewayWithClassConfigChange(t *testing.T) { // Create a Gateway and wait for it to be ready firstGatewayName := envconf.RandomName("gw", 16) - firstGateway := createGateway(ctx, t, resources, firstGatewayName, gc, []gateway.Listener{httpsListener}) + firstGateway := createGateway(ctx, t, resources, firstGatewayName, namespace, gc, []gateway.Listener{httpsListener}) require.Eventually(t, gatewayStatusCheck(ctx, resources, firstGatewayName, namespace, conditionReady), checkTimeout, checkInterval, "no gateway found in the allotted time") checkGatewayConfigAnnotation(ctx, t, resources, firstGatewayName, namespace, firstConfig) @@ -97,7 +98,7 @@ func TestGatewayWithClassConfigChange(t *testing.T) { // Create a second Gateway and wait for it to be ready secondGatewayName := envconf.RandomName("gw", 16) - secondGateway := createGateway(ctx, t, resources, secondGatewayName, gc, []gateway.Listener{httpsListener}) + secondGateway := createGateway(ctx, t, resources, secondGatewayName, namespace, gc, []gateway.Listener{httpsListener}) require.Eventually(t, gatewayStatusCheck(ctx, resources, secondGatewayName, namespace, conditionReady), checkTimeout, checkInterval, "no gateway found in the allotted time") // Verify that 1st Gateway retains initial GatewayClassConfig and 2nd Gateway retains updated GatewayClassConfig @@ -130,7 +131,7 @@ func TestGatewayWithReplicas(t *testing.T) { // Create a Gateway and wait for it to be ready gatewayName := envconf.RandomName("gw", 16) - gw := createGateway(ctx, t, resources, gatewayName, gc, []gateway.Listener{createHTTPSListener(ctx, t, 443)}) + gw := createGateway(ctx, t, resources, gatewayName, namespace, gc, []gateway.Listener{createHTTPSListener(ctx, t, 443)}) require.Eventually(t, gatewayStatusCheck(ctx, resources, gatewayName, namespace, conditionReady), checkTimeout, checkInterval, "no gateway found in the allotted time") checkGatewayConfigAnnotation(ctx, t, resources, gatewayName, namespace, gcc) @@ -166,7 +167,7 @@ func TestGatewayWithReplicasCanScale(t *testing.T) { // Create a Gateway and wait for it to be ready gatewayName := envconf.RandomName("gw", 16) - gateway := createGateway(ctx, t, resources, gatewayName, gc, []gateway.Listener{createHTTPSListener(ctx, t, 443)}) + gateway := createGateway(ctx, t, resources, gatewayName, namespace, gc, []gateway.Listener{createHTTPSListener(ctx, t, 443)}) require.Eventually(t, gatewayStatusCheck(ctx, resources, gatewayName, namespace, conditionReady), checkTimeout, checkInterval, "no gateway found in the allotted time") checkGatewayConfigAnnotation(ctx, t, resources, gatewayName, namespace, gcc) @@ -201,8 +202,8 @@ func TestGatewayWithReplicasRespectMinMax(t *testing.T) { var initialReplicas int32 = 3 var minReplicas int32 = 2 var maxReplicas int32 = 8 - var exceedsMin int32 = minReplicas - 1 - var exceedsMax int32 = maxReplicas + 1 + var exceedsMin = minReplicas - 1 + var exceedsMax = maxReplicas + 1 useHostPorts := false // Create a GatewayClassConfig @@ -220,7 +221,7 @@ func TestGatewayWithReplicasRespectMinMax(t *testing.T) { // Create a Gateway and wait for it to be ready gatewayName := envconf.RandomName("gw", 16) - gateway := createGateway(ctx, t, resources, gatewayName, gatewayClass, []gateway.Listener{httpsListener}) + gateway := createGateway(ctx, t, resources, gatewayName, namespace, gatewayClass, []gateway.Listener{httpsListener}) require.Eventually(t, gatewayStatusCheck(ctx, resources, gatewayName, namespace, conditionReady), checkTimeout, checkInterval, "no gateway found in the allotted time") @@ -271,7 +272,7 @@ func TestGatewayBasic(t *testing.T) { require.Eventually(t, gatewayClassStatusCheck(ctx, resources, gc.Name, namespace, conditionAccepted), checkTimeout, checkInterval, "gatewayclass not accepted in the allotted time") httpsListener := createHTTPSListener(ctx, t, 443) - gw := createGateway(ctx, t, resources, gatewayName, gc, []gateway.Listener{httpsListener}) + gw := createGateway(ctx, t, resources, gatewayName, namespace, gc, []gateway.Listener{httpsListener}) require.Eventually(t, func() bool { err := resources.Get(ctx, gatewayName, namespace, &apps.Deployment{}) @@ -334,7 +335,7 @@ func TestServiceListeners(t *testing.T) { require.Eventually(t, gatewayClassStatusCheck(ctx, resources, gc.Name, namespace, conditionAccepted), checkTimeout, checkInterval, "gatewayclass not accepted in the allotted time") httpsListener := createHTTPSListener(ctx, t, 443) - gw := createGateway(ctx, t, resources, gatewayName, gc, []gateway.Listener{httpsListener}) + gw := createGateway(ctx, t, resources, gatewayName, namespace, gc, []gateway.Listener{httpsListener}) require.Eventually(t, func() bool { service := &core.Service{} @@ -397,7 +398,7 @@ func TestHTTPRouteFlattening(t *testing.T) { checkPort := e2e.HTTPFlattenedPort(ctx) httpsListener := createHTTPSListener(ctx, t, gateway.PortNumber(checkPort)) - gw := createGateway(ctx, t, resources, gatewayName, gc, []gateway.Listener{httpsListener}) + gw := createGateway(ctx, t, resources, gatewayName, namespace, gc, []gateway.Listener{httpsListener}) require.Eventually(t, gatewayStatusCheck(ctx, resources, gatewayName, namespace, conditionReady), checkTimeout, checkInterval, "no gateway found in the allotted time") port := gateway.PortNumber(serviceOne.Spec.Ports[0].Port) @@ -523,7 +524,7 @@ func TestHTTPMeshService(t *testing.T) { require.Eventually(t, gatewayClassStatusCheck(ctx, resources, gc.Name, namespace, conditionAccepted), checkTimeout, checkInterval, "gatewayclass not accepted in the allotted time") httpsListener := createHTTPSListener(ctx, t, gateway.PortNumber(e2e.HTTPPort(ctx))) - gw := createGateway(ctx, t, resources, gatewayName, gc, []gateway.Listener{httpsListener}) + gw := createGateway(ctx, t, resources, gatewayName, namespace, gc, []gateway.Listener{httpsListener}) require.Eventually(t, gatewayStatusCheck(ctx, resources, gatewayName, namespace, conditionReady), checkTimeout, checkInterval, "no gateway found in the allotted time") // route 1 @@ -980,7 +981,7 @@ func TestReferencePolicyLifecycle(t *testing.T) { fromSelector := gateway.NamespacesFromSelector gwNamespace := gateway.Namespace(namespace) - gw := createGateway(ctx, t, resources, gatewayName, gc, []gateway.Listener{ + gw := createGateway(ctx, t, resources, gatewayName, namespace, gc, []gateway.Listener{ { Name: "https", Port: gateway.PortNumber(httpCheckPort), @@ -1252,6 +1253,110 @@ func TestReferencePolicyLifecycle(t *testing.T) { err = resources.Delete(ctx, gw) require.NoError(t, err) + return ctx + }). + Assess("gateway controller watches reference policy changes", func(ctx context.Context, t *testing.T, cfg *envconf.Config) context.Context { + namespace := e2e.Namespace(ctx) + gatewayName := envconf.RandomName("gw", 16) + gatewayNamespace := envconf.RandomName("ns", 16) + certName := "consul-server-cert" + certNamespace := gateway.Namespace(namespace) + gatewayRefPolicyName := envconf.RandomName("refpolicy", 16) + + resources := cfg.Client().Resources(namespace) + + _, gc := createGatewayClass(ctx, t, resources) + require.Eventually(t, gatewayClassStatusCheck(ctx, resources, gc.Name, namespace, conditionAccepted), checkTimeout, checkInterval, "gatewayclass not accepted in the allotted time") + + // Allow routes to bind from a different namespace for testing + // cross-namespace ReferencePolicy enforcement + fromSelector := gateway.NamespacesFromAll + + // Create a different namespace for the Gateway + require.NoError(t, resources.Create(ctx, &core.Namespace{ + ObjectMeta: meta.ObjectMeta{ + Name: gatewayNamespace, + }, + })) + + gw := createGateway(ctx, t, resources, gatewayName, gatewayNamespace, gc, []gateway.Listener{ + { + Name: "https", + Port: gateway.PortNumber(e2e.HTTPReferencePolicyPort(ctx)), + Protocol: gateway.HTTPSProtocolType, + TLS: &gateway.GatewayTLSConfig{ + CertificateRefs: []*gateway.SecretObjectReference{{ + Name: gateway.ObjectName(certName), + Namespace: &certNamespace, + }}, + }, + AllowedRoutes: &gateway.AllowedRoutes{ + Namespaces: &gateway.RouteNamespaces{ + From: &fromSelector, + }, + }, + }, + }) + + // Expect that Gateway has expected error condition + // due to missing ReferencePolicy for CertificateRef in other namespace + gatewayConditionCheck := createConditionsCheck([]meta.Condition{{Type: "Ready", Status: "False", Reason: "ListenersNotValid"}}) + gatewayCheck := gatewayStatusCheck(ctx, resources, gatewayName, gatewayNamespace, gatewayConditionCheck) + require.Eventually(t, gatewayCheck, checkTimeout, checkInterval, "Gateway status not set in allotted time") + + // Expect that Gateway listener has expected error condition + // due to missing ReferencePolicy for CertificateRef in other namespace + listenerConditionCheck := createListenerStatusConditionsCheck([]meta.Condition{{Type: "ResolvedRefs", Status: "False", Reason: "InvalidCertificateRef"}}) + listenerCheck := listenerStatusCheck(ctx, resources, gatewayName, gatewayNamespace, listenerConditionCheck) + require.Eventually(t, listenerCheck, checkTimeout, checkInterval, "Gateway listener status not set in allotted time") + + // Create ReferencePolicy allowing Gateway CertificateRef + certReferencePolicy := &gateway.ReferencePolicy{ + ObjectMeta: meta.ObjectMeta{ + Name: gatewayRefPolicyName, + Namespace: string(certNamespace), + }, + Spec: gateway.ReferencePolicySpec{ + From: []gateway.ReferencePolicyFrom{{ + Group: "gateway.networking.k8s.io", + Kind: "Gateway", + Namespace: gateway.Namespace(gatewayNamespace), + }}, + To: []gateway.ReferencePolicyTo{{ + Group: "", + Kind: "Secret", + Name: nil, + }}, + }, + } + require.NoError(t, resources.Create(ctx, certReferencePolicy)) + + // Expect that Gateway has expected success condition + // TODO Newly created ReferencePolicy doesn't trigger reconcile of broken Gateway + gatewayConditionCheck = createConditionsCheck([]meta.Condition{{Type: "Ready", Status: "True", Reason: "Ready"}}) + gatewayCheck = gatewayStatusCheck(ctx, resources, gatewayName, gatewayNamespace, gatewayConditionCheck) + require.Eventually(t, gatewayCheck, checkTimeout, checkInterval, "Gateway status not set in allotted time") + + // Expect that Gateway listener has expected success condition + listenerConditionCheck = createListenerStatusConditionsCheck([]meta.Condition{{Type: "ResolvedRefs", Status: "True", Reason: "ResolvedRefs"}}) + listenerCheck = listenerStatusCheck(ctx, resources, gatewayName, gatewayNamespace, listenerConditionCheck) + require.Eventually(t, listenerCheck, checkTimeout, checkInterval, "Gateway listener status not set in allotted time") + + // Delete Gateway ReferencePolicy + require.NoError(t, resources.Delete(ctx, certReferencePolicy)) + + // Check for error status conditions again + gatewayConditionCheck = createConditionsCheck([]meta.Condition{{Type: "Ready", Status: "False", Reason: "ListenersNotValid"}}) + gatewayCheck = gatewayStatusCheck(ctx, resources, gatewayName, gatewayNamespace, gatewayConditionCheck) + require.Eventually(t, gatewayCheck, checkTimeout, checkInterval, "Gateway status not set in allotted time") + + listenerConditionCheck = createListenerStatusConditionsCheck([]meta.Condition{{Type: "ResolvedRefs", Status: "False", Reason: "InvalidCertificateRef"}}) + listenerCheck = listenerStatusCheck(ctx, resources, gatewayName, gatewayNamespace, listenerConditionCheck) + require.Eventually(t, listenerCheck, checkTimeout, checkInterval, "Gateway listener status not set in allotted time") + + // Clean up + require.NoError(t, resources.Delete(ctx, gw)) + return ctx }) @@ -1279,6 +1384,7 @@ func TestRouteParentRefChange(t *testing.T) { t, resources, firstGatewayName, + namespace, gc, []gateway.Listener{createHTTPSListener(ctx, t, gateway.PortNumber(firstGatewayCheckPort))}, ) @@ -1344,6 +1450,7 @@ func TestRouteParentRefChange(t *testing.T) { t, resources, secondGatewayName, + namespace, gc, []gateway.Listener{createHTTPSListener(ctx, t, gateway.PortNumber(secondGatewayCheckPort))}, ) @@ -1565,15 +1672,13 @@ func createHTTPSListener(ctx context.Context, t *testing.T, port gateway.PortNum } } -func createGateway(ctx context.Context, t *testing.T, resources *resources.Resources, gatewayName string, gc *gateway.GatewayClass, listeners []gateway.Listener) *gateway.Gateway { +func createGateway(ctx context.Context, t *testing.T, resources *resources.Resources, gatewayName, gatewayNamespace string, gc *gateway.GatewayClass, listeners []gateway.Listener) *gateway.Gateway { t.Helper() - namespace := e2e.Namespace(ctx) - gw := &gateway.Gateway{ ObjectMeta: meta.ObjectMeta{ Name: gatewayName, - Namespace: namespace, + Namespace: gatewayNamespace, }, Spec: gateway.GatewaySpec{ GatewayClassName: gateway.ObjectName(gc.Name), From 6c5c6db9121088fd5ba7b26cd9c635caaa6eab64 Mon Sep 17 00:00:00 2001 From: Nathan Coleman Date: Thu, 2 Jun 2022 13:40:58 -0400 Subject: [PATCH 06/11] Remove redundant assertion --- internal/commands/server/k8s_e2e_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/internal/commands/server/k8s_e2e_test.go b/internal/commands/server/k8s_e2e_test.go index ca92a57bc..d198d84cd 100644 --- a/internal/commands/server/k8s_e2e_test.go +++ b/internal/commands/server/k8s_e2e_test.go @@ -224,8 +224,6 @@ func TestGatewayWithReplicasRespectMinMax(t *testing.T) { gateway := createGateway(ctx, t, resources, gatewayName, namespace, gatewayClass, []gateway.Listener{httpsListener}) require.Eventually(t, gatewayStatusCheck(ctx, resources, gatewayName, namespace, conditionReady), checkTimeout, checkInterval, "no gateway found in the allotted time") - - require.Eventually(t, gatewayStatusCheck(ctx, resources, gatewayName, namespace, conditionReady), 30*time.Second, checkInterval, "no gateway found in the allotted time") checkGatewayConfigAnnotation(ctx, t, resources, gatewayName, namespace, gatewayClassConfig) // Fetch the deployment created by the gateway and check the number of replicas From ec4c36efae56e70f82cd83ec5a98e273f07dffc3 Mon Sep 17 00:00:00 2001 From: Nathan Coleman Date: Thu, 2 Jun 2022 13:46:36 -0400 Subject: [PATCH 07/11] Add changelog entry --- .changelog/207.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/207.txt diff --git a/.changelog/207.txt b/.changelog/207.txt new file mode 100644 index 000000000..0c52e5f11 --- /dev/null +++ b/.changelog/207.txt @@ -0,0 +1,3 @@ +```release-note:enhancement +k8s/controllers: watch for ReferencePolicy changes to reconcile and revalidate affected Gateways +``` From aee35c719f67e1a2dbbfc3bf0da8f98a5daabb81 Mon Sep 17 00:00:00 2001 From: Nathan Coleman Date: Thu, 2 Jun 2022 16:49:22 -0400 Subject: [PATCH 08/11] Use non-default namespace for Secret instead of Gateway Creating a Gateway in a namespace other than the one set up for the test encounters issues such as a ServiceAccount that doesn't exist in the newly-created namespace --- internal/commands/server/k8s_e2e_test.go | 27 ++++++++++++------------ internal/k8s/reconciler/gateway.go | 6 ++++-- 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/internal/commands/server/k8s_e2e_test.go b/internal/commands/server/k8s_e2e_test.go index d198d84cd..24d40658a 100644 --- a/internal/commands/server/k8s_e2e_test.go +++ b/internal/commands/server/k8s_e2e_test.go @@ -1255,14 +1255,23 @@ func TestReferencePolicyLifecycle(t *testing.T) { }). Assess("gateway controller watches reference policy changes", func(ctx context.Context, t *testing.T, cfg *envconf.Config) context.Context { namespace := e2e.Namespace(ctx) + gatewayNamespace := namespace gatewayName := envconf.RandomName("gw", 16) - gatewayNamespace := envconf.RandomName("ns", 16) + certNamespace := envconf.RandomName("ns", 16) certName := "consul-server-cert" - certNamespace := gateway.Namespace(namespace) gatewayRefPolicyName := envconf.RandomName("refpolicy", 16) resources := cfg.Client().Resources(namespace) + // Make a copy of the certificate Secret in a different namespace for the Gateway to reference. + // This is easier than creating the Gateway in a different namespace due to pre-installed ServiceAccount dependency. + certCopy := &core.Secret{} + require.NoError(t, resources.Get(ctx, certName, namespace, certCopy)) + certCopy.SetNamespace(certNamespace) + certCopy.SetResourceVersion("") + require.NoError(t, resources.Create(ctx, &core.Namespace{ObjectMeta: meta.ObjectMeta{Name: certNamespace}})) + require.NoError(t, resources.Create(ctx, certCopy)) + _, gc := createGatewayClass(ctx, t, resources) require.Eventually(t, gatewayClassStatusCheck(ctx, resources, gc.Name, namespace, conditionAccepted), checkTimeout, checkInterval, "gatewayclass not accepted in the allotted time") @@ -1270,13 +1279,7 @@ func TestReferencePolicyLifecycle(t *testing.T) { // cross-namespace ReferencePolicy enforcement fromSelector := gateway.NamespacesFromAll - // Create a different namespace for the Gateway - require.NoError(t, resources.Create(ctx, &core.Namespace{ - ObjectMeta: meta.ObjectMeta{ - Name: gatewayNamespace, - }, - })) - + certNamespaceTyped := gateway.Namespace(certNamespace) gw := createGateway(ctx, t, resources, gatewayName, gatewayNamespace, gc, []gateway.Listener{ { Name: "https", @@ -1285,7 +1288,7 @@ func TestReferencePolicyLifecycle(t *testing.T) { TLS: &gateway.GatewayTLSConfig{ CertificateRefs: []*gateway.SecretObjectReference{{ Name: gateway.ObjectName(certName), - Namespace: &certNamespace, + Namespace: &certNamespaceTyped, }}, }, AllowedRoutes: &gateway.AllowedRoutes{ @@ -1330,9 +1333,7 @@ func TestReferencePolicyLifecycle(t *testing.T) { require.NoError(t, resources.Create(ctx, certReferencePolicy)) // Expect that Gateway has expected success condition - // TODO Newly created ReferencePolicy doesn't trigger reconcile of broken Gateway - gatewayConditionCheck = createConditionsCheck([]meta.Condition{{Type: "Ready", Status: "True", Reason: "Ready"}}) - gatewayCheck = gatewayStatusCheck(ctx, resources, gatewayName, gatewayNamespace, gatewayConditionCheck) + gatewayCheck = gatewayStatusCheck(ctx, resources, gatewayName, gatewayNamespace, conditionReady) require.Eventually(t, gatewayCheck, checkTimeout, checkInterval, "Gateway status not set in allotted time") // Expect that Gateway listener has expected success condition diff --git a/internal/k8s/reconciler/gateway.go b/internal/k8s/reconciler/gateway.go index 9202a3e5a..5792df707 100644 --- a/internal/k8s/reconciler/gateway.go +++ b/internal/k8s/reconciler/gateway.go @@ -11,14 +11,15 @@ import ( "k8s.io/apimachinery/pkg/types" gw "sigs.k8s.io/gateway-api/apis/v1alpha2" + "github.com/hashicorp/go-hclog" + "golang.org/x/exp/slices" + "github.com/hashicorp/consul-api-gateway/internal/core" "github.com/hashicorp/consul-api-gateway/internal/k8s/builder" "github.com/hashicorp/consul-api-gateway/internal/k8s/gatewayclient" "github.com/hashicorp/consul-api-gateway/internal/k8s/utils" "github.com/hashicorp/consul-api-gateway/internal/store" apigwv1alpha1 "github.com/hashicorp/consul-api-gateway/pkg/apis/v1alpha1" - "github.com/hashicorp/go-hclog" - "golang.org/x/exp/slices" ) type K8sGateway struct { @@ -411,6 +412,7 @@ func (g *K8sGateway) Status() gw.GatewayStatus { if listenersInvalid { g.status.Ready.ListenersNotValid = errors.New("gateway listeners not valid") } else if !g.podReady || !g.serviceReady || !listenersReady { + g.logger.Warn("gateway listeners not ready", "podReady", g.podReady, "serviceReady", g.serviceReady, "listenersReady", listenersReady) g.status.Ready.ListenersNotReady = errors.New("gateway listeners not ready") } else if len(g.gateway.Spec.Addresses) != 0 { g.status.Ready.AddressNotAssigned = errors.New("gateway does not support requesting addresses") From 8c1296dcf48d4c9cff7262ad5465f35613f641a6 Mon Sep 17 00:00:00 2001 From: Nathan Coleman Date: Thu, 2 Jun 2022 17:16:39 -0400 Subject: [PATCH 09/11] Remove inapplicable comment --- internal/commands/server/k8s_e2e_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/internal/commands/server/k8s_e2e_test.go b/internal/commands/server/k8s_e2e_test.go index 24d40658a..eb5e3b5a5 100644 --- a/internal/commands/server/k8s_e2e_test.go +++ b/internal/commands/server/k8s_e2e_test.go @@ -1275,8 +1275,6 @@ func TestReferencePolicyLifecycle(t *testing.T) { _, gc := createGatewayClass(ctx, t, resources) require.Eventually(t, gatewayClassStatusCheck(ctx, resources, gc.Name, namespace, conditionAccepted), checkTimeout, checkInterval, "gatewayclass not accepted in the allotted time") - // Allow routes to bind from a different namespace for testing - // cross-namespace ReferencePolicy enforcement fromSelector := gateway.NamespacesFromAll certNamespaceTyped := gateway.Namespace(certNamespace) From a00ae896f9f25fa832bfc9ee42f0588fbd194aa5 Mon Sep 17 00:00:00 2001 From: Nathan Coleman Date: Thu, 2 Jun 2022 17:18:15 -0400 Subject: [PATCH 10/11] Add comment explaining Gateway config --- internal/commands/server/k8s_e2e_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/commands/server/k8s_e2e_test.go b/internal/commands/server/k8s_e2e_test.go index eb5e3b5a5..a3895eb84 100644 --- a/internal/commands/server/k8s_e2e_test.go +++ b/internal/commands/server/k8s_e2e_test.go @@ -1277,6 +1277,7 @@ func TestReferencePolicyLifecycle(t *testing.T) { fromSelector := gateway.NamespacesFromAll + // Create a Gateway with a listener that has a CertificateRef to a different namespace certNamespaceTyped := gateway.Namespace(certNamespace) gw := createGateway(ctx, t, resources, gatewayName, gatewayNamespace, gc, []gateway.Listener{ { From 3f5882cf0ff91d61d9fc0ea2c64b9a8036795095 Mon Sep 17 00:00:00 2001 From: Nathan Coleman Date: Thu, 2 Jun 2022 17:21:21 -0400 Subject: [PATCH 11/11] Remove debug log --- internal/k8s/reconciler/gateway.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/k8s/reconciler/gateway.go b/internal/k8s/reconciler/gateway.go index 5792df707..1b23ba9fa 100644 --- a/internal/k8s/reconciler/gateway.go +++ b/internal/k8s/reconciler/gateway.go @@ -412,7 +412,6 @@ func (g *K8sGateway) Status() gw.GatewayStatus { if listenersInvalid { g.status.Ready.ListenersNotValid = errors.New("gateway listeners not valid") } else if !g.podReady || !g.serviceReady || !listenersReady { - g.logger.Warn("gateway listeners not ready", "podReady", g.podReady, "serviceReady", g.serviceReady, "listenersReady", listenersReady) g.status.Ready.ListenersNotReady = errors.New("gateway listeners not ready") } else if len(g.gateway.Spec.Addresses) != 0 { g.status.Ready.AddressNotAssigned = errors.New("gateway does not support requesting addresses")