Skip to content
This repository has been archived by the owner on Mar 19, 2024. It is now read-only.

Commit

Permalink
gateway-controller: watch ReferencePolicy lifecycle (#207)
Browse files Browse the repository at this point in the history 
* Stub gateway controller watch on ReferencePolicy

* Reconcile all Gateways in referenced namespace on ReferencePolicy watch

* Regenerate mock gatewayclient

* Remove unnecessary middle functions for fetching impacted xRoutes

* Add e2e test for ReferencePolicy lifecycle applied to Gateways

* Remove redundant assertion

* Add changelog entry

* 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

* Remove inapplicable comment

* Add comment explaining Gateway config

* Remove debug log
  • Loading branch information
@nathancoleman
nathancoleman authored Jun 2, 2022
1 parent c8dd3ef commit 358445d
Show file tree
Hide file tree
Showing 9 changed files with 214 additions and 56 deletions.
3 changes: 3 additions & 0 deletions .changelog/207.txt
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:enhancement
k8s/controllers: watch for ReferencePolicy changes to reconcile and revalidate affected Gateways
```
143 changes: 123 additions & 20 deletions internal/commands/server/k8s_e2e_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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)

Expand All @@ -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
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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
Expand All @@ -220,11 +221,9 @@ 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")

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
Expand Down Expand Up @@ -271,7 +270,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{})
Expand Down Expand Up @@ -334,7 +333,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{}
Expand Down Expand Up @@ -397,7 +396,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)
Expand Down Expand Up @@ -523,7 +522,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
Expand Down Expand Up @@ -980,7 +979,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),
Expand Down Expand Up @@ -1252,6 +1251,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)
gatewayNamespace := namespace
gatewayName := envconf.RandomName("gw", 16)
certNamespace := envconf.RandomName("ns", 16)
certName := "consul-server-cert"
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")

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{
{
Name: "https",
Port: gateway.PortNumber(e2e.HTTPReferencePolicyPort(ctx)),
Protocol: gateway.HTTPSProtocolType,
TLS: &gateway.GatewayTLSConfig{
CertificateRefs: []*gateway.SecretObjectReference{{
Name: gateway.ObjectName(certName),
Namespace: &certNamespaceTyped,
}},
},
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
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
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
})

Expand Down Expand Up @@ -1279,6 +1382,7 @@ func TestRouteParentRefChange(t *testing.T) {
t,
resources,
firstGatewayName,
namespace,
gc,
[]gateway.Listener{createHTTPSListener(ctx, t, gateway.PortNumber(firstGatewayCheckPort))},
)
Expand Down Expand Up @@ -1344,6 +1448,7 @@ func TestRouteParentRefChange(t *testing.T) {
t,
resources,
secondGatewayName,
namespace,
gc,
[]gateway.Listener{createHTTPSListener(ctx, t, gateway.PortNumber(secondGatewayCheckPort))},
)
Expand Down Expand Up @@ -1565,15 +1670,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),
Expand Down
1 change: 1 addition & 0 deletions internal/k8s/controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
48 changes: 46 additions & 2 deletions internal/k8s/controllers/gateway_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (

// GatewayReconciler reconciles a Gateway object
type GatewayReconciler struct {
Context context.Context
Client gatewayclient.Client
Log hclog.Logger
ControllerName string
Expand Down Expand Up @@ -93,19 +94,62 @@ 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))
}

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(),
}},
}
}
return nil
}

func (r *GatewayReconciler) referencePolicyToGatewayRequests(object client.Object) []reconcile.Request {
refPolicy := object.(*gateway.ReferencePolicy)

gateways := r.getGatewaysAffectedByReferencePolicy(refPolicy)

requests := make([]reconcile.Request, 0, len(gateways))

for _, gw := range gateways {
requests = append(requests, reconcile.Request{
NamespacedName: types.NamespacedName{
Name: gw.Name,
Namespace: gw.Namespace,
},
})
}

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: 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)
return matches
}

matches = append(matches, gateways...)
}

return matches
}
23 changes: 7 additions & 16 deletions internal/k8s/controllers/http_route_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -94,24 +95,14 @@ 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{}

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 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)
Expand Down
Loading

0 comments on commit 358445d

Please sign in to comment.