Skip to content

Commit

Permalink
Gateway-related conformance tests (#2774)
Browse files Browse the repository at this point in the history
* test: gateway-related conformance tests

Signed-off-by: Mattia Lavacca <[email protected]>
  • Loading branch information
mlavacca authored Aug 8, 2022
1 parent 4cdc076 commit 03e9900
Show file tree
Hide file tree
Showing 6 changed files with 255 additions and 22 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@
upgrading if you're using the alpha APIs, switch your feature gate flags to
`--feature-gates=GatewayAlpha=true` to keep them enabled.
[#2781](https:/Kong/kubernetes-ingress-controller/pull/2781)
- Added all the Gateway-related conformance tests.
[#2777](https:/Kong/kubernetes-ingress-controller/issues/2777)

#### Fixed

Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ require (
k8s.io/apimachinery v0.24.3
k8s.io/client-go v0.24.3
k8s.io/component-base v0.24.3
k8s.io/utils v0.0.0-20220210201930-3a6ce19ff2f9
knative.dev/networking v0.0.0-20220302134042-e8b2eb995165
knative.dev/pkg v0.0.0-20220301181942-2fdd5f232e77
sigs.k8s.io/controller-runtime v0.12.3
Expand Down Expand Up @@ -170,7 +171,6 @@ require (
k8s.io/klog/v2 v2.60.1 // indirect
k8s.io/kube-openapi v0.0.0-20220401212409-b28bf2818661 // indirect
k8s.io/kubectl v0.24.3 // indirect
k8s.io/utils v0.0.0-20220210201930-3a6ce19ff2f9 // indirect
sigs.k8s.io/json v0.0.0-20211208200746-9f7c6b3444d2 // indirect
sigs.k8s.io/kind v0.14.0 // indirect
sigs.k8s.io/structured-merge-diff/v4 v4.2.1 // indirect
Expand Down
9 changes: 8 additions & 1 deletion internal/controllers/gateway/gateway_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -342,14 +342,21 @@ func (r *GatewayReconciler) reconcileUnmanagedGateway(ctx context.Context, log l
return ctrl.Result{}, nil // dont requeue here because spec update will trigger new reconciliation
}

// the ReferenceGrants need to be retrieved to ensure that all gateway listeners reference
// TLS secrets they are granted for
referenceGrantList := &gatewayv1alpha2.ReferenceGrantList{}
if err := r.Client.List(ctx, referenceGrantList); err != nil {
return ctrl.Result{}, err
}

// TODO https:/Kong/kubernetes-ingress-controller/issues/2559 check cross-Gateway compatibility
// When all Listeners on all Gateways were derived from Kong's configuration, they were guaranteed to be compatible
// because they were all identical, though there may have been some ambiguity re de facto merged Gateways that
// used different allowedRoutes. We only merged allowedRoutes within a single Gateway, but merged all Gateways into
// a single set of shared listens. We lack knowledge of whether this is compatible with user intent, and it may
// be incompatible with the spec, so we should consider evaluating cross-Gateway compatibility and raising error
// conditions in the event of a problem
listenerStatuses := getListenerStatus(gateway, kongListeners)
listenerStatuses := getListenerStatus(gateway, kongListeners, referenceGrantList.Items)

// once specification matches the reference Service, all that's left to do is ensure that the
// Gateway status reflects the spec. As the status is simply a mirror of the Service, this is
Expand Down
134 changes: 134 additions & 0 deletions internal/controllers/gateway/gateway_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"github.com/stretchr/testify/assert"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/utils/pointer"
"sigs.k8s.io/controller-runtime/pkg/reconcile"
gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2"

Expand Down Expand Up @@ -405,3 +406,136 @@ func Test_areAllowedRoutesConsistentByProtocol(t *testing.T) {
assert.Equal(t, input.expected, areAllowedRoutesConsistentByProtocol(input.l), input.message)
}
}

func Test_getReferenceGrantConditionReason(t *testing.T) {
testCases := []struct {
name string
gatewayNamespace string
certRef gatewayv1alpha2.SecretObjectReference
referenceGrants []gatewayv1alpha2.ReferenceGrant
expectedReason string
}{
{
name: "no need for reference",
gatewayNamespace: "test",
certRef: gatewayv1alpha2.SecretObjectReference{
Kind: (*gatewayv1alpha2.Kind)(pointer.StringPtr("secret")),
Name: "testSecret",
},
expectedReason: string(gatewayv1alpha2.ListenerReasonResolvedRefs),
},
{
name: "reference not granted",
gatewayNamespace: "test",
certRef: gatewayv1alpha2.SecretObjectReference{
Kind: (*gatewayv1alpha2.Kind)(pointer.StringPtr("secret")),
Name: "testSecret",
Namespace: (*gatewayv1alpha2.Namespace)(pointer.StringPtr("otherNamespace")),
},
referenceGrants: []gatewayv1alpha2.ReferenceGrant{
{
ObjectMeta: metav1.ObjectMeta{
Namespace: "otherNamespace",
},
Spec: gatewayv1alpha2.ReferenceGrantSpec{
From: []gatewayv1alpha2.ReferenceGrantFrom{
{
Group: gatewayV1alpha2Group,
Kind: "Gateway",
Namespace: "test",
},
},
To: []gatewayv1alpha2.ReferenceGrantTo{
{
Group: "",
Kind: "Secret",
Name: (*gatewayv1alpha2.ObjectName)(pointer.StringPtr("anotherSecret")),
},
},
},
},
},
expectedReason: string(gatewayv1alpha2.ListenerReasonInvalidCertificateRef),
},
{
name: "reference granted, secret name not specified",
gatewayNamespace: "test",
certRef: gatewayv1alpha2.SecretObjectReference{
Kind: (*gatewayv1alpha2.Kind)(pointer.StringPtr("secret")),
Name: "testSecret",
Namespace: (*gatewayv1alpha2.Namespace)(pointer.StringPtr("otherNamespace")),
},
referenceGrants: []gatewayv1alpha2.ReferenceGrant{
{
ObjectMeta: metav1.ObjectMeta{
Namespace: "otherNamespace",
},
Spec: gatewayv1alpha2.ReferenceGrantSpec{
From: []gatewayv1alpha2.ReferenceGrantFrom{
// useless entry, just to furtherly test the function
{
Group: "otherGroup",
Kind: "otherKind",
Namespace: "test",
},
// good entry
{
Group: gatewayV1alpha2Group,
Kind: "Gateway",
Namespace: "test",
},
},
To: []gatewayv1alpha2.ReferenceGrantTo{
{
Group: "",
Kind: "Secret",
},
},
},
},
},
expectedReason: string(gatewayv1alpha2.ListenerReasonResolvedRefs),
},
{
name: "reference granted, secret name specified",
gatewayNamespace: "test",
certRef: gatewayv1alpha2.SecretObjectReference{
Kind: (*gatewayv1alpha2.Kind)(pointer.StringPtr("secret")),
Name: "testSecret",
Namespace: (*gatewayv1alpha2.Namespace)(pointer.StringPtr("otherNamespace")),
},
referenceGrants: []gatewayv1alpha2.ReferenceGrant{
{
ObjectMeta: metav1.ObjectMeta{
Namespace: "otherNamespace",
},
Spec: gatewayv1alpha2.ReferenceGrantSpec{
From: []gatewayv1alpha2.ReferenceGrantFrom{
{
Group: gatewayV1alpha2Group,
Kind: "Gateway",
Namespace: "test",
},
},
To: []gatewayv1alpha2.ReferenceGrantTo{
{
Group: "",
Kind: "Secret",
Name: (*gatewayv1alpha2.ObjectName)(pointer.StringPtr("testSecret")),
},
},
},
},
},
expectedReason: string(gatewayv1alpha2.ListenerReasonResolvedRefs),
},
}

for _, tc := range testCases {
assert.Equal(t, tc.expectedReason, getReferenceGrantConditionReason(
tc.gatewayNamespace,
tc.certRef,
tc.referenceGrants,
))
}
}
106 changes: 100 additions & 6 deletions internal/controllers/gateway/gateway_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,7 @@ func canSharePort(requested gatewayv1alpha2.ProtocolType, existing gatewayv1alph
func getListenerStatus(
gateway *gatewayv1alpha2.Gateway,
kongListens []gatewayv1alpha2.Listener,
referenceGrants []gatewayv1alpha2.ReferenceGrant,
) []gatewayv1alpha2.ListenerStatus {
statuses := make(map[gatewayv1alpha2.SectionName]gatewayv1alpha2.ListenerStatus, len(gateway.Spec.Listeners))
portToProtocol, portToHostname, listenerToAttached := initializeListenerMaps(gateway)
Expand Down Expand Up @@ -342,23 +343,78 @@ func getListenerStatus(
// https:/Kong/kubernetes-ingress-controller/pull/2555#issuecomment-1154579046 for discussion)
// if we encountered conflicts, we must strip the ready status we originally set
for _, listener := range gateway.Spec.Listeners {
var reason string
var conflictReason string
var resolvedRefReason string

var hostname gatewayv1alpha2.Hostname
if listener.Hostname != nil {
hostname = *listener.Hostname
}
// there's no filter for protocols that don't use Hostname, but this won't be populated from earlier for those
if _, ok := conflictedHostnames[listener.Port][hostname]; ok {
reason = string(gatewayv1alpha2.ListenerReasonHostnameConflict)
conflictReason = string(gatewayv1alpha2.ListenerReasonHostnameConflict)
}

if _, ok := conflictedPorts[listener.Port]; ok {
reason = string(gatewayv1alpha2.ListenerReasonProtocolConflict)
conflictReason = string(gatewayv1alpha2.ListenerReasonProtocolConflict)
}

if len(reason) > 0 {
newConditions := []metav1.Condition{}
// If the listener uses TLS, we need to ensure that the gateway is granted to reference
// all the secrets it references
if listener.TLS != nil {
resolvedRefReason = string(gatewayv1alpha2.ListenerReasonResolvedRefs)
for _, certRef := range listener.TLS.CertificateRefs {
// only secrets are supported as certificate references
if (certRef.Group != nil && (*certRef.Group != "core" && *certRef.Group != "")) ||
(certRef.Kind != nil && *certRef.Kind != "Secret") {
resolvedRefReason = string(gatewayv1alpha2.ListenerReasonInvalidCertificateRef)
break
}
// if the certificate is in the same namespace of the gateway, no ReferenceGrant is needed
if certRef.Namespace == nil || *certRef.Namespace == gatewayv1alpha2.Namespace(gateway.Namespace) {
continue
}
// get the result of the certificate reference. If the returned reason is not successful, the loop
// must be broken because a secret reference isn't granted
resolvedRefReason = getReferenceGrantConditionReason(gateway.Namespace, certRef, referenceGrants)
if resolvedRefReason != string(gatewayv1alpha2.ListenerReasonResolvedRefs) {
break
}
}
}

newConditions := []metav1.Condition{}

// if resolvedRefReason has any value, it means that the listener references a secret.
// A ListenerConditionResolvedRefs condition must be set to reflect in the gateway status
// the outcome of that reference (that means if the gateway is granted to access that secret)
if resolvedRefReason != "" {
conditionStatus := metav1.ConditionTrue
message := "the listener is ready and available for routing"
if resolvedRefReason != string(gatewayv1alpha2.ListenerReasonResolvedRefs) {
conditionStatus = metav1.ConditionFalse
message = "the listener is not ready and cannot route requests"
}
newConditions = append(newConditions,
metav1.Condition{
Type: string(gatewayv1alpha2.ListenerConditionResolvedRefs),
Status: conditionStatus,
ObservedGeneration: gateway.Generation,
LastTransitionTime: metav1.Now(),
Reason: resolvedRefReason,
},
metav1.Condition{
Type: string(gatewayv1alpha2.ListenerConditionReady),
Status: conditionStatus,
ObservedGeneration: gateway.Generation,
LastTransitionTime: metav1.Now(),
Reason: string(gatewayv1alpha2.ListenerReasonReady),
Message: message,
},
)
}

if len(conflictReason) > 0 {
for _, cond := range statuses[listener.Name].Conditions {
// shut up linter, there's a default
switch gatewayv1alpha2.ListenerConditionType(cond.Type) { //nolint:exhaustive
Expand All @@ -374,7 +430,7 @@ func getListenerStatus(
Status: metav1.ConditionTrue,
ObservedGeneration: gateway.Generation,
LastTransitionTime: metav1.Now(),
Reason: reason,
Reason: conflictReason,
},
metav1.Condition{
Type: string(gatewayv1alpha2.ListenerConditionReady),
Expand All @@ -385,6 +441,8 @@ func getListenerStatus(
Message: "the listener is not ready and cannot route requests",
},
)
}
if len(newConditions) > 0 {
status := statuses[listener.Name]
status.Conditions = newConditions
statuses[listener.Name] = status
Expand All @@ -398,6 +456,42 @@ func getListenerStatus(
return statusArray
}

// getReferenceGrantConditionReason gets a certRef belonging to a specific listener and a slice of referenceGrants.
func getReferenceGrantConditionReason(gatewayNamespace string, certRef gatewayv1alpha2.SecretObjectReference, referenceGrants []gatewayv1alpha2.ReferenceGrant) string {
// no need to have this reference granted
if certRef.Namespace == nil || *certRef.Namespace == gatewayv1alpha2.Namespace(gatewayNamespace) {
return string(gatewayv1alpha2.ListenerReasonResolvedRefs)
}

certRefNamespace := string(*certRef.Namespace)
for _, grant := range referenceGrants {
// the grant must exist in the same namespace of the referenced resource
if grant.Namespace != certRefNamespace {
continue
}
for _, from := range grant.Spec.From {
// we are interested only in grants for gateways that want to reference secrets
if from.Group != gatewayV1alpha2Group || from.Kind != "Gateway" {
continue
}
if from.Namespace == gatewayv1alpha2.Namespace(gatewayNamespace) {
for _, to := range grant.Spec.To {
if (to.Group != "" && to.Group != "core") || to.Kind != "Secret" {
continue
}
// if all the above conditions are satisfied, and the name of the referenced secret matches
// the granted resource name, then return a reason "ResolvedRefs"
if to.Name == nil || string(*to.Name) == string(certRef.Name) {
return string(gatewayv1alpha2.ListenerReasonResolvedRefs)
}
}
}
}
}
// if no grants have been found for the reference, return an "InvalidCertificateRef" reason
return string(gatewayv1alpha2.ListenerReasonInvalidCertificateRef)
}

// -----------------------------------------------------------------------------
// Gateway Utils - Watch Predicate Helpers
// -----------------------------------------------------------------------------
Expand Down
Loading

0 comments on commit 03e9900

Please sign in to comment.