Skip to content

Commit ac4b1f9

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #4446 from shuqz/conformance-reference-grant
[feat gw-api]update reference grant check and route status
2 parents a3b5439 + fefbf4d commit ac4b1f9

16 files changed

+335
-127
lines changed

‎controllers/gateway/route_reconciler.go‎

Lines changed: 38 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -226,69 +226,48 @@ func (d *routeReconcilerImpl) resolveRefGateway(parentRef gwv1.ParentReference,
226226
// setCondition based on RouteStatusInfo
227227
func (d *routeReconcilerImpl) setConditionsWithRouteStatusInfo(route client.Object, parentStatus *gwv1.RouteParentStatus, info routeutils.RouteStatusInfo) {
228228
timeNow := metav1.NewTime(time.Now())
229+
var conditions []metav1.Condition
229230
if !info.ResolvedRefs {
230-
// resolvedRef rejected
231-
parentStatus.Conditions = []metav1.Condition{
232-
{
233-
Type: string(gwv1.RouteConditionAccepted),
234-
Status: metav1.ConditionFalse,
235-
Reason: info.Reason,
236-
Message: info.Message,
237-
LastTransitionTime: timeNow,
238-
ObservedGeneration: route.GetGeneration(),
239-
},
240-
{
241-
Type: string(gwv1.RouteConditionResolvedRefs),
242-
Status: metav1.ConditionFalse,
243-
Reason: info.Reason,
244-
Message: info.Message,
245-
LastTransitionTime: timeNow,
246-
ObservedGeneration: route.GetGeneration(),
247-
},
248-
}
249-
return
231+
conditions = append(conditions, metav1.Condition{
232+
Type: string(gwv1.RouteConditionResolvedRefs),
233+
Status: metav1.ConditionFalse,
234+
Reason: info.Reason,
235+
Message: info.Message,
236+
LastTransitionTime: timeNow,
237+
ObservedGeneration: route.GetGeneration(),
238+
})
239+
} else {
240+
conditions = append(conditions, metav1.Condition{
241+
Type: string(gwv1.RouteConditionResolvedRefs),
242+
Status: metav1.ConditionTrue,
243+
Reason: string(gwv1.RouteReasonResolvedRefs),
244+
Message: "",
245+
LastTransitionTime: timeNow,
246+
ObservedGeneration: route.GetGeneration(),
247+
})
250248
}
251-
// resolveRef accepted and route accepted
252-
if info.Accepted {
253-
parentStatus.Conditions = []metav1.Condition{
254-
{
255-
Type: string(gwv1.RouteConditionAccepted),
256-
Status: metav1.ConditionTrue,
257-
Reason: info.Reason,
258-
Message: info.Message,
259-
LastTransitionTime: timeNow,
260-
ObservedGeneration: route.GetGeneration(),
261-
},
262-
{
263-
Type: string(gwv1.RouteConditionResolvedRefs),
264-
Status: metav1.ConditionTrue,
265-
Reason: string(gwv1.RouteReasonResolvedRefs),
266-
LastTransitionTime: timeNow,
267-
ObservedGeneration: route.GetGeneration(),
268-
},
269-
}
270-
return
249+
250+
if !info.Accepted {
251+
conditions = append(conditions, metav1.Condition{
252+
Type: string(gwv1.RouteConditionAccepted),
253+
Status: metav1.ConditionFalse,
254+
Reason: info.Reason,
255+
Message: info.Message,
256+
LastTransitionTime: timeNow,
257+
ObservedGeneration: route.GetGeneration(),
258+
})
271259
} else {
272-
// resolveRef accepted but route rejected
273-
parentStatus.Conditions = []metav1.Condition{
274-
{
275-
Type: string(gwv1.RouteConditionAccepted),
276-
Status: metav1.ConditionFalse,
277-
Reason: info.Reason,
278-
Message: info.Message,
279-
LastTransitionTime: timeNow,
280-
ObservedGeneration: route.GetGeneration(),
281-
},
282-
{
283-
Type: string(gwv1.RouteConditionResolvedRefs),
284-
Status: metav1.ConditionTrue,
285-
Reason: string(gwv1.RouteReasonAccepted),
286-
LastTransitionTime: timeNow,
287-
ObservedGeneration: route.GetGeneration(),
288-
},
289-
}
290-
return
260+
conditions = append(conditions, metav1.Condition{
261+
Type: string(gwv1.RouteConditionAccepted),
262+
Status: metav1.ConditionTrue,
263+
Reason: string(gwv1.RouteReasonAccepted),
264+
Message: "",
265+
LastTransitionTime: timeNow,
266+
ObservedGeneration: route.GetGeneration(),
267+
})
291268
}
269+
270+
parentStatus.Conditions = conditions
292271
}
293272

294273
func (d *routeReconcilerImpl) setConditionsBasedOnResolveRefGateway(route client.Object, parentStatus *gwv1.RouteParentStatus, resolveErr error) {

‎controllers/gateway/route_reconciler_test.go‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -503,10 +503,12 @@ func Test_setConditionsWithRouteStatusInfo(t *testing.T) {
503503
acceptedCondition := findCondition(conditions, string(gwv1.RouteConditionAccepted))
504504
assert.NotNil(t, acceptedCondition)
505505
assert.Equal(t, metav1.ConditionTrue, acceptedCondition.Status)
506+
assert.Equal(t, string(gwv1.RouteReasonAccepted), acceptedCondition.Reason)
506507

507508
resolvedRefCondition := findCondition(conditions, string(gwv1.RouteConditionResolvedRefs))
508509
assert.NotNil(t, resolvedRefCondition)
509510
assert.Equal(t, metav1.ConditionTrue, resolvedRefCondition.Status)
511+
assert.Equal(t, string(gwv1.RouteReasonResolvedRefs), resolvedRefCondition.Reason)
510512
},
511513
},
512514
{
@@ -529,18 +531,18 @@ func Test_setConditionsWithRouteStatusInfo(t *testing.T) {
529531
},
530532
},
531533
{
532-
name: "accepted false and resolvedRef false",
534+
name: "accepted true and resolvedRef false",
533535
info: routeutils.RouteStatusInfo{
534-
Accepted: false,
536+
Accepted: true,
535537
ResolvedRefs: false,
536-
Reason: string(gwv1.RouteReasonBackendNotFound),
537-
Message: "backend not found",
538+
Reason: string(gwv1.RouteReasonRefNotPermitted),
539+
Message: "ref not permitted",
538540
},
539541
validateResult: func(t *testing.T, conditions []metav1.Condition) {
540542
assert.Len(t, conditions, 2)
541543
acceptedCondition := findCondition(conditions, string(gwv1.RouteConditionAccepted))
542544
assert.NotNil(t, acceptedCondition)
543-
assert.Equal(t, metav1.ConditionFalse, acceptedCondition.Status)
545+
assert.Equal(t, metav1.ConditionTrue, acceptedCondition.Status)
544546

545547
resolvedRefCondition := findCondition(conditions, string(gwv1.RouteConditionResolvedRefs))
546548
assert.NotNil(t, resolvedRefCondition)

‎docs/guide/gateway/gateway.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ spec:
127127

128128
When `my-http-service` or the configured service port can't be found,
129129
the target group will not be materialized on any ALBs that the route attaches to.
130-
An [503 Fixed Response](https://docs.aws.amazon.com/elasticloadbalancing/latest/APIReference/API_FixedResponseActionConfig.html)
130+
An [500 Fixed Response](https://docs.aws.amazon.com/elasticloadbalancing/latest/APIReference/API_FixedResponseActionConfig.html)
131131
will be added to any Listener Rules that would have referenced the invalid backend.
132132

133133
## Specify out-of-band Target Groups

‎pkg/gateway/model/model_build_listener.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -381,16 +381,16 @@ func buildL7ListenerDefaultActions() []elbv2model.Action {
381381
return []elbv2model.Action{action404}
382382
}
383383

384-
// returns 503 when no backends are configured
384+
// returns 500 when no backends are configured
385385
func buildL7ListenerNoBackendActions() elbv2model.Action {
386-
action503 := elbv2model.Action{
386+
action500 := elbv2model.Action{
387387
Type: elbv2model.ActionTypeFixedResponse,
388388
FixedResponseConfig: &elbv2model.FixedResponseActionConfig{
389389
ContentType: awssdk.String("text/plain"),
390-
StatusCode: "503",
390+
StatusCode: "500",
391391
},
392392
}
393-
return action503
393+
return action500
394394
}
395395

396396
func buildL4ListenerDefaultActions(arn core.StringToken) []elbv2model.Action {

‎pkg/gateway/model/model_build_listener_test.go‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,19 +2,20 @@ package model
22

33
import (
44
"context"
5+
"reflect"
6+
"strings"
7+
"testing"
8+
59
awssdk "github.com/aws/aws-sdk-go-v2/aws"
610
elbv2sdk "github.com/aws/aws-sdk-go-v2/service/elasticloadbalancingv2"
711
elbv2types "github.com/aws/aws-sdk-go-v2/service/elasticloadbalancingv2/types"
812
"github.com/google/go-cmp/cmp"
913
"github.com/google/go-cmp/cmp/cmpopts"
1014
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1115
"k8s.io/apimachinery/pkg/util/sets"
12-
"reflect"
1316
"sigs.k8s.io/aws-load-balancer-controller/pkg/aws/services"
1417
"sigs.k8s.io/aws-load-balancer-controller/pkg/gateway/routeutils"
1518
coremodel "sigs.k8s.io/aws-load-balancer-controller/pkg/model/core"
16-
"strings"
17-
"testing"
1819

1920
"github.com/golang/mock/gomock"
2021
"github.com/pkg/errors"
@@ -1242,7 +1243,7 @@ func Test_BuildListenerRules(t *testing.T) {
12421243
tagErr error
12431244
}{
12441245
{
1245-
name: "no backends should result in 503 fixed response",
1246+
name: "no backends should result in 500 fixed response",
12461247
port: 80,
12471248
listenerProtocol: elbv2model.ProtocolHTTP,
12481249
ipAddressType: elbv2model.IPAddressTypeIPV4,
@@ -1277,7 +1278,7 @@ func Test_BuildListenerRules(t *testing.T) {
12771278
Type: "fixed-response",
12781279
FixedResponseConfig: &elbv2model.FixedResponseActionConfig{
12791280
ContentType: awssdk.String("text/plain"),
1280-
StatusCode: "503",
1281+
StatusCode: "500",
12811282
},
12821283
},
12831284
},
@@ -1590,7 +1591,7 @@ func Test_BuildListenerRules(t *testing.T) {
15901591
},
15911592
},
15921593
{
1593-
name: "listener rule config with authenticate-cognito and no backends should result in auth + 503 fixed response",
1594+
name: "listener rule config with authenticate-cognito and no backends should result in auth + 500 fixed response",
15941595
port: 80,
15951596
listenerProtocol: elbv2model.ProtocolHTTPS,
15961597
ipAddressType: elbv2model.IPAddressTypeIPV4,
@@ -1663,7 +1664,7 @@ func Test_BuildListenerRules(t *testing.T) {
16631664
Type: "fixed-response",
16641665
FixedResponseConfig: &elbv2model.FixedResponseActionConfig{
16651666
ContentType: awssdk.String("text/plain"),
1666-
StatusCode: "503",
1667+
StatusCode: "500",
16671668
},
16681669
},
16691670
},

‎pkg/gateway/routeutils/backend.go‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package routeutils
33
import (
44
"context"
55
"fmt"
6+
67
"github.com/pkg/errors"
78
corev1 "k8s.io/api/core/v1"
89
"k8s.io/apimachinery/pkg/types"
@@ -21,6 +22,8 @@ const (
2122
gatewayKind = "Gateway"
2223
referenceGrantNotExists = "No explicit ReferenceGrant exists to allow the reference."
2324
maxWeight = 999
25+
gatewayAPIGroup = "gateway.networking.k8s.io"
26+
coreAPIGroup = ""
2427
)
2528

2629
var (
@@ -216,7 +219,7 @@ func LookUpTargetGroupConfiguration(ctx context.Context, k8sClient client.Client
216219

217220
// Implements the reference grant API
218221
// https://gateway-api.sigs.k8s.io/api-types/referencegrant/
219-
func referenceGrantCheck(ctx context.Context, k8sClient client.Client, objKind string, objIdentifier types.NamespacedName, routeIdentifier types.NamespacedName, routeKind RouteKind) (bool, error) {
222+
func referenceGrantCheck(ctx context.Context, k8sClient client.Client, objKind string, objGroup string, objIdentifier types.NamespacedName, routeIdentifier types.NamespacedName, routeKind RouteKind, routeGroup string) (bool, error) {
220223
referenceGrantList := &gwbeta1.ReferenceGrantList{}
221224
if err := k8sClient.List(ctx, referenceGrantList, client.InNamespace(objIdentifier.Namespace)); err != nil {
222225
return false, err
@@ -226,17 +229,15 @@ func referenceGrantCheck(ctx context.Context, k8sClient client.Client, objKind s
226229
var routeAllowed bool
227230

228231
for _, from := range grant.Spec.From {
229-
// Kind check maybe?
230-
if string(from.Kind) == string(routeKind) && string(from.Namespace) == routeIdentifier.Namespace {
232+
if string(from.Group) == routeGroup && string(from.Kind) == string(routeKind) && string(from.Namespace) == routeIdentifier.Namespace {
231233
routeAllowed = true
232234
break
233235
}
234236
}
235237

236238
if routeAllowed {
237239
for _, to := range grant.Spec.To {
238-
// Make sure the kind is correct for our query.
239-
if string(to.Kind) != objKind {
240+
if string(to.Group) != objGroup || string(to.Kind) != objKind {
240241
continue
241242
}
242243

‎pkg/gateway/routeutils/backend_gateway.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ package routeutils
33
import (
44
"context"
55
"fmt"
6+
"strings"
7+
68
"github.com/pkg/errors"
79
corev1 "k8s.io/api/core/v1"
810
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -14,7 +16,6 @@ import (
1416
"sigs.k8s.io/aws-load-balancer-controller/pkg/shared_constants"
1517
"sigs.k8s.io/controller-runtime/pkg/client"
1618
gwv1 "sigs.k8s.io/gateway-api/apis/v1"
17-
"strings"
1819
)
1920

2021
var _ TargetGroupConfigurator = &GatewayBackendConfig{}
@@ -101,7 +102,7 @@ func gatewayLoader(ctx context.Context, k8sClient client.Client, routeIdentifier
101102

102103
// Check for reference grant when performing cross namespace gateway -> route attachment
103104
if gwIdentifier.Namespace != routeIdentifier.Namespace {
104-
allowed, err := referenceGrantCheck(ctx, k8sClient, gatewayKind, gwIdentifier, routeIdentifier, routeKind)
105+
allowed, err := referenceGrantCheck(ctx, k8sClient, gatewayKind, gatewayAPIGroup, gwIdentifier, routeIdentifier, routeKind, gatewayAPIGroup)
105106
if err != nil {
106107
// Currently, this API only fails for a k8s related error message, hence no status update + make the error fatal.
107108
return nil, nil, errors.Wrapf(err, "Unable to perform reference grant check")

‎pkg/gateway/routeutils/backend_service.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ package routeutils
33
import (
44
"context"
55
"fmt"
6+
"strings"
7+
68
"github.com/pkg/errors"
79
corev1 "k8s.io/api/core/v1"
810
"k8s.io/apimachinery/pkg/types"
@@ -13,7 +15,6 @@ import (
1315
"sigs.k8s.io/aws-load-balancer-controller/pkg/shared_constants"
1416
"sigs.k8s.io/controller-runtime/pkg/client"
1517
gwv1 "sigs.k8s.io/gateway-api/apis/v1"
16-
"strings"
1718
)
1819

1920
type ServiceBackendConfig struct {
@@ -141,7 +142,7 @@ func serviceLoader(ctx context.Context, k8sClient client.Client, routeIdentifier
141142

142143
// Check for reference grant when performing cross namespace gateway -> route attachment
143144
if svcNamespace != routeIdentifier.Namespace {
144-
allowed, err := referenceGrantCheck(ctx, k8sClient, serviceKind, svcIdentifier, routeIdentifier, routeKind)
145+
allowed, err := referenceGrantCheck(ctx, k8sClient, serviceKind, coreAPIGroup, svcIdentifier, routeIdentifier, routeKind, gatewayAPIGroup)
145146
if err != nil {
146147
// Currently, this API only fails for a k8s related error message, hence no status update + make the error fatal.
147148
return nil, nil, errors.Wrapf(err, "Unable to perform reference grant check")

0 commit comments

Comments
 (0)