Skip to content

Commit 90bb6cc

Browse files
salonichf5salonichf5
committed
update based on reviews
1 parent 2a8b719 commit 90bb6cc

File tree

3 files changed

+33
-176
lines changed

3 files changed

+33
-176
lines changed

‎internal/mode/static/nginx/config/servers_test.go‎

Lines changed: 19 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -589,20 +589,6 @@ func TestExecuteForDefaultServers(t *testing.T) {
589589
}
590590
}
591591

592-
func getHeaderMatchType(matchType string) dataplane.MatchType {
593-
if len(matchType) == 0 {
594-
return dataplane.MatchTypeExact
595-
}
596-
return dataplane.MatchType(matchType)
597-
}
598-
599-
func getQueryParamMatchType(matchType string) dataplane.MatchType {
600-
if len(matchType) == 0 {
601-
return dataplane.MatchTypeExact
602-
}
603-
return dataplane.MatchType(matchType)
604-
}
605-
606592
func TestCreateServers(t *testing.T) {
607593
t.Parallel()
608594
const (
@@ -724,30 +710,30 @@ func TestCreateServers(t *testing.T) {
724710
{
725711
Name: "Version",
726712
Value: "V1",
727-
Type: getHeaderMatchType(""),
713+
Type: dataplane.MatchTypeExact,
728714
},
729715
{
730716
Name: "test",
731717
Value: "foo",
732-
Type: getHeaderMatchType(""),
718+
Type: dataplane.MatchTypeExact,
733719
},
734720
{
735721
Name: "my-header",
736722
Value: "my-value",
737-
Type: getHeaderMatchType(""),
723+
Type: dataplane.MatchTypeExact,
738724
},
739725
},
740726
QueryParams: []dataplane.HTTPQueryParamMatch{
741727
{
742728
// query names and values should not be normalized to lowercase
743729
Name: "GrEat",
744730
Value: "EXAMPLE",
745-
Type: getQueryParamMatchType(""),
731+
Type: dataplane.MatchTypeExact,
746732
},
747733
{
748734
Name: "test",
749735
Value: "foo=bar",
750-
Type: getQueryParamMatchType(""),
736+
Type: dataplane.MatchTypeExact,
751737
},
752738
},
753739
},
@@ -816,7 +802,7 @@ func TestCreateServers(t *testing.T) {
816802
{
817803
Name: "redirect",
818804
Value: "this",
819-
Type: getHeaderMatchType(""),
805+
Type: dataplane.MatchTypeExact,
820806
},
821807
},
822808
},
@@ -859,7 +845,7 @@ func TestCreateServers(t *testing.T) {
859845
{
860846
Name: "rewrite",
861847
Value: "this",
862-
Type: getHeaderMatchType(""),
848+
Type: dataplane.MatchTypeExact,
863849
},
864850
},
865851
},
@@ -899,7 +885,7 @@ func TestCreateServers(t *testing.T) {
899885
{
900886
Name: "filter",
901887
Value: "this",
902-
Type: getHeaderMatchType(""),
888+
Type: dataplane.MatchTypeExact,
903889
},
904890
},
905891
},
@@ -2724,17 +2710,17 @@ func TestCreateRouteMatch(t *testing.T) {
27242710
{
27252711
Name: "header-1",
27262712
Value: "val-1",
2727-
Type: getHeaderMatchType(""),
2713+
Type: dataplane.MatchTypeExact,
27282714
},
27292715
{
27302716
Name: "header-2",
27312717
Value: "val-2",
2732-
Type: getHeaderMatchType(""),
2718+
Type: dataplane.MatchTypeExact,
27332719
},
27342720
{
27352721
Name: "header-3",
27362722
Value: "val-3",
2737-
Type: getHeaderMatchType(""),
2723+
Type: dataplane.MatchTypeExact,
27382724
},
27392725
}
27402726

@@ -2750,17 +2736,17 @@ func TestCreateRouteMatch(t *testing.T) {
27502736
{
27512737
Name: "arg1",
27522738
Value: "val1",
2753-
Type: getQueryParamMatchType(""),
2739+
Type: dataplane.MatchTypeExact,
27542740
},
27552741
{
27562742
Name: "arg2",
27572743
Value: "val2=another-val",
2758-
Type: getQueryParamMatchType(""),
2744+
Type: dataplane.MatchTypeExact,
27592745
},
27602746
{
27612747
Name: "arg3",
27622748
Value: "==val3",
2763-
Type: getQueryParamMatchType(""),
2749+
Type: dataplane.MatchTypeExact,
27642750
},
27652751
}
27662752

@@ -2895,7 +2881,7 @@ func TestCreateQueryParamKeyValString(t *testing.T) {
28952881
input: dataplane.HTTPQueryParamMatch{
28962882
Name: "key",
28972883
Value: "value",
2898-
Type: getQueryParamMatchType(string(dataplane.MatchTypeExact)),
2884+
Type: dataplane.MatchTypeExact,
28992885
},
29002886
expected: "key=Exact=value",
29012887
},
@@ -2904,7 +2890,7 @@ func TestCreateQueryParamKeyValString(t *testing.T) {
29042890
input: dataplane.HTTPQueryParamMatch{
29052891
Name: "KeY",
29062892
Value: "vaLUe-[a-z]==",
2907-
Type: getQueryParamMatchType(string(dataplane.MatchTypeRegularExpression)),
2893+
Type: dataplane.MatchTypeRegularExpression,
29082894
},
29092895
expected: "KeY=RegularExpression=vaLUe-[a-z]==",
29102896
},
@@ -2913,7 +2899,7 @@ func TestCreateQueryParamKeyValString(t *testing.T) {
29132899
input: dataplane.HTTPQueryParamMatch{
29142900
Name: "keY",
29152901
Value: "vaLUe==",
2916-
Type: getQueryParamMatchType(""),
2902+
Type: dataplane.MatchTypeExact,
29172903
},
29182904
expected: "keY=Exact=vaLUe==",
29192905
},
@@ -2939,7 +2925,7 @@ func TestCreateHeaderKeyValString(t *testing.T) {
29392925
dataplane.HTTPHeaderMatch{
29402926
Name: "kEy",
29412927
Value: "vALUe",
2942-
Type: getHeaderMatchType(string(dataplane.MatchTypeExact)),
2928+
Type: dataplane.MatchTypeExact,
29432929
},
29442930
)
29452931

@@ -2951,7 +2937,7 @@ func TestCreateHeaderKeyValString(t *testing.T) {
29512937
dataplane.HTTPHeaderMatch{
29522938
Name: "kEy",
29532939
Value: "vALUe-[0-9]",
2954-
Type: getHeaderMatchType(string(dataplane.MatchTypeRegularExpression)),
2940+
Type: dataplane.MatchTypeRegularExpression,
29552941
},
29562942
)
29572943

‎tests/framework/resourcemanager.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -338,11 +338,11 @@ func (rm *ResourceManager) WaitForAppsToBeReadyWithCtx(ctx context.Context, name
338338
return err
339339
}
340340

341-
if err := rm.waitForHTTPRoutesToBeReady(ctx, namespace); err != nil {
341+
if err := rm.WaitForHTTPRoutesToBeReady(ctx, namespace); err != nil {
342342
return err
343343
}
344344

345-
if err := rm.waitForGRPCRoutesToBeReady(ctx, namespace); err != nil {
345+
if err := rm.WaitForGRPCRoutesToBeReady(ctx, namespace); err != nil {
346346
return err
347347
}
348348

@@ -400,7 +400,7 @@ func (rm *ResourceManager) waitForGatewaysToBeReady(ctx context.Context, namespa
400400
)
401401
}
402402

403-
func (rm *ResourceManager) waitForHTTPRoutesToBeReady(ctx context.Context, namespace string) error {
403+
func (rm *ResourceManager) WaitForHTTPRoutesToBeReady(ctx context.Context, namespace string) error {
404404
return wait.PollUntilContextCancel(
405405
ctx,
406406
500*time.Millisecond,
@@ -422,7 +422,7 @@ func (rm *ResourceManager) waitForHTTPRoutesToBeReady(ctx context.Context, names
422422
)
423423
}
424424

425-
func (rm *ResourceManager) waitForGRPCRoutesToBeReady(ctx context.Context, namespace string) error {
425+
func (rm *ResourceManager) WaitForGRPCRoutesToBeReady(ctx context.Context, namespace string) error {
426426
// First, check if grpcroute even exists for v1. If not, ignore.
427427
var routeList v1.GRPCRouteList
428428
err := rm.K8sClient.List(ctx, &routeList, client.InNamespace(namespace))
@@ -749,11 +749,11 @@ func (rm *ResourceManager) WaitForAppsToBeReadyWithCtxWithPodCount(
749749
return err
750750
}
751751

752-
if err := rm.waitForHTTPRoutesToBeReady(ctx, namespace); err != nil {
752+
if err := rm.WaitForHTTPRoutesToBeReady(ctx, namespace); err != nil {
753753
return err
754754
}
755755

756-
if err := rm.waitForGRPCRoutesToBeReady(ctx, namespace); err != nil {
756+
if err := rm.WaitForGRPCRoutesToBeReady(ctx, namespace); err != nil {
757757
return err
758758
}
759759

‎tests/suite/advanced_routing_test.go‎

Lines changed: 8 additions & 137 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,7 @@ import (
1313
. "github.com/onsi/gomega"
1414
core "k8s.io/api/core/v1"
1515
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
16-
"k8s.io/apimachinery/pkg/types"
17-
"k8s.io/apimachinery/pkg/util/wait"
1816
"sigs.k8s.io/controller-runtime/pkg/client"
19-
gatewayv1 "sigs.k8s.io/gateway-api/apis/v1"
2017

2118
"github.com/nginx/nginx-gateway-fabric/tests/framework"
2219
)
@@ -65,13 +62,14 @@ var _ = Describe("AdvancedRouting", Ordered, Label("functional", "routing"), fun
6562
httpRoute := "http-header-and-query-matching"
6663
grpcRoute := "grpc-header-matching"
6764

68-
nsname := types.NamespacedName{Name: httpRoute, Namespace: namespace}
69-
err := waitForHTTPRouteToBeAccepted(nsname)
70-
Expect(err).ToNot(HaveOccurred(), fmt.Sprintf("%s was not accepted", httpRoute))
65+
ctx, cancel := context.WithTimeout(context.Background(), timeoutConfig.GetStatusTimeout*2)
66+
defer cancel()
7167

72-
nsname = types.NamespacedName{Name: grpcRoute, Namespace: namespace}
73-
err = waitForGRPCRouteToBeAccepted(nsname)
68+
err := resourceManager.WaitForHTTPRoutesToBeReady(ctx, namespace)
7469
Expect(err).ToNot(HaveOccurred(), fmt.Sprintf("%s was not accepted", httpRoute))
70+
71+
err = resourceManager.WaitForGRPCRoutesToBeReady(ctx, namespace)
72+
Expect(err).ToNot(HaveOccurred(), fmt.Sprintf("%s was not accepted", grpcRoute))
7573
})
7674

7775
DescribeTable("verify working traffic for HTTPRoute",
@@ -81,7 +79,7 @@ var _ = Describe("AdvancedRouting", Ordered, Label("functional", "routing"), fun
8179
func() error {
8280
return expectRequestToRespondFromExpectedServer(url, address, serverName, headers, queryParams)
8381
}).
84-
WithTimeout(timeoutConfig.ContainerRestartTimeout).
82+
WithTimeout(timeoutConfig.GetTimeout).
8583
WithPolling(500 * time.Millisecond).
8684
Should(Succeed())
8785
},
@@ -142,7 +140,7 @@ func expectRequestToRespondFromExpectedServer(
142140
return errors.New("expected response body to contain correct server name")
143141
}
144142

145-
return err
143+
return nil
146144
}
147145

148146
func extractServerName(responseBody string) (string, error) {
@@ -153,130 +151,3 @@ func extractServerName(responseBody string) (string, error) {
153151
}
154152
return matches[1], nil
155153
}
156-
157-
func waitForHTTPRouteToBeAccepted(nsname types.NamespacedName) error {
158-
ctx, cancel := context.WithTimeout(context.Background(), timeoutConfig.GetStatusTimeout*2)
159-
defer cancel()
160-
161-
GinkgoWriter.Printf(
162-
"Waiting for HTTPRoute %q to have the condition Accepted/True/Accepted\n",
163-
nsname,
164-
)
165-
166-
return waitForHTTPRouteToHaveExpectedCondition(
167-
ctx,
168-
nsname,
169-
metav1.ConditionTrue,
170-
string(gatewayv1.RouteConditionAccepted),
171-
)
172-
}
173-
174-
func waitForGRPCRouteToBeAccepted(nsname types.NamespacedName) error {
175-
ctx, cancel := context.WithTimeout(context.Background(), timeoutConfig.GetStatusTimeout*2)
176-
defer cancel()
177-
178-
GinkgoWriter.Printf(
179-
"Waiting for GRPCRoute %q to have the condition Accepted/True/Accepted\n",
180-
nsname,
181-
)
182-
183-
return waitForGRPCRouteToHaveExpectedCondition(
184-
ctx,
185-
nsname,
186-
metav1.ConditionTrue,
187-
string(gatewayv1.RouteConditionAccepted),
188-
)
189-
}
190-
191-
func waitForHTTPRouteToHaveExpectedCondition(
192-
ctx context.Context,
193-
nsname types.NamespacedName,
194-
conditionStatus metav1.ConditionStatus,
195-
reason string,
196-
) error {
197-
return wait.PollUntilContextCancel(
198-
ctx,
199-
500*time.Millisecond,
200-
true, /* poll immediately */
201-
func(ctx context.Context) (bool, error) {
202-
var route gatewayv1.HTTPRoute
203-
204-
if err := k8sClient.Get(ctx, nsname, &route); err != nil {
205-
return false, err
206-
}
207-
208-
return verifyConditions(route, nsname, conditionStatus, reason)
209-
},
210-
)
211-
}
212-
213-
func waitForGRPCRouteToHaveExpectedCondition(
214-
ctx context.Context,
215-
nsname types.NamespacedName,
216-
conditionStatus metav1.ConditionStatus,
217-
reason string,
218-
) error {
219-
return wait.PollUntilContextCancel(
220-
ctx,
221-
500*time.Millisecond,
222-
true, /* poll immediately */
223-
func(ctx context.Context) (bool, error) {
224-
var route gatewayv1.GRPCRoute
225-
226-
if err := k8sClient.Get(ctx, nsname, &route); err != nil {
227-
return false, err
228-
}
229-
230-
return verifyConditions(route, nsname, conditionStatus, reason)
231-
},
232-
)
233-
}
234-
235-
func verifyConditions(
236-
route interface{},
237-
nsname types.NamespacedName,
238-
conditionStatus metav1.ConditionStatus,
239-
reason string,
240-
) (bool, error) {
241-
var err error
242-
var parents []gatewayv1.RouteParentStatus
243-
244-
switch r := route.(type) {
245-
case gatewayv1.HTTPRoute:
246-
parents = r.Status.Parents
247-
case gatewayv1.GRPCRoute:
248-
parents = r.Status.Parents
249-
default:
250-
return false, fmt.Errorf("unsupported route type")
251-
}
252-
253-
if len(parents) == 0 {
254-
GinkgoWriter.Printf("Route %q does not have a parent status yet\n", nsname)
255-
256-
return false, nil
257-
}
258-
259-
if len(parents) != 1 {
260-
return false, fmt.Errorf("route has %d parents, expected 1", len(parents))
261-
}
262-
263-
parent := parents[0]
264-
if parent.Conditions == nil {
265-
return false, fmt.Errorf("expected parent conditions to not be nil")
266-
}
267-
268-
cond := parent.Conditions[0]
269-
if cond.Type != string(gatewayv1.RouteConditionAccepted) {
270-
return false, fmt.Errorf("expected condition type to be Accepted, got %s", cond.Type)
271-
}
272-
273-
if cond.Status != conditionStatus {
274-
return false, fmt.Errorf("expected condition status to be True, got %s", cond.Status)
275-
}
276-
277-
if cond.Reason != reason {
278-
return false, fmt.Errorf("expected condition reason to be Accepted, got %s", cond.Reason)
279-
}
280-
281-
return err == nil, err
282-
}

0 commit comments

Comments
 (0)