Skip to content

Commit 0d225ad

Browse files
authored
fix: remove redundant route.Hosts to prevent false diffs in ADC sync (#2743)
1 parent f34a259 commit 0d225ad

5 files changed

Lines changed: 96 additions & 9 deletions

File tree

.github/workflows/apisix-e2e-test.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ jobs:
9595
if: ${{ env.ADC_VERSION == 'dev' }}
9696
run: |
9797
docker create --name adc-temp ghcr.io/api7/adc:dev
98-
docker cp adc-temp:main.js adc.js
98+
docker cp adc-temp:main.cjs adc.js
9999
docker rm adc-temp
100100
node $(pwd)/adc.js -v
101101
echo "ADC_BIN=node $(pwd)/adc.js" >> $GITHUB_ENV
@@ -165,7 +165,7 @@ jobs:
165165
if: ${{ env.ADC_VERSION == 'dev' }}
166166
run: |
167167
docker create --name adc-temp ghcr.io/api7/adc:dev
168-
docker cp adc-temp:main.js adc.js
168+
docker cp adc-temp:main.cjs adc.js
169169
docker rm adc-temp
170170
node $(pwd)/adc.js -v
171171
echo "ADC_BIN=node $(pwd)/adc.js" >> $GITHUB_ENV

.github/workflows/benchmark-test.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ jobs:
9898
if: ${{ env.ADC_VERSION == 'dev' }}
9999
run: |
100100
docker create --name adc-temp ghcr.io/api7/adc:dev
101-
docker cp adc-temp:main.js adc.js
101+
docker cp adc-temp:main.cjs adc.js
102102
docker rm adc-temp
103103
node $(pwd)/adc.js -v
104104
echo "ADC_BIN=node $(pwd)/adc.js" >> $GITHUB_ENV

internal/adc/translator/apisixroute.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,6 @@ func (t *Translator) buildRoute(ar *apiv2.ApisixRoute, service *adc.Service, rul
190190
route.EnableWebsocket = *enableWebsocket
191191
}
192192
route.FilterFunc = rule.Match.FilterFunc
193-
route.Hosts = rule.Match.Hosts
194193
route.Methods = rule.Match.Methods
195194
route.Plugins = plugins
196195
route.Priority = ptr.To(int64(rule.Priority))
Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
// Licensed to the Apache Software Foundation (ASF) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The ASF licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// with the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
18+
package translator
19+
20+
import (
21+
"testing"
22+
23+
"github.com/go-logr/logr"
24+
"github.com/stretchr/testify/assert"
25+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
26+
27+
adc "github.com/apache/apisix-ingress-controller/api/adc"
28+
apiv2 "github.com/apache/apisix-ingress-controller/api/v2"
29+
)
30+
31+
func TestBuildRoute_HostsNotSet(t *testing.T) {
32+
translator := NewTranslator(logr.Discard())
33+
34+
ar := &apiv2.ApisixRoute{
35+
ObjectMeta: metav1.ObjectMeta{
36+
Name: "test-route",
37+
Namespace: "default",
38+
},
39+
}
40+
41+
service := &adc.Service{}
42+
rule := apiv2.ApisixRouteHTTP{
43+
Name: "rule1",
44+
Match: apiv2.ApisixRouteHTTPMatch{
45+
Hosts: []string{"example.com", "foo.com"},
46+
Paths: []string{"/api/*"},
47+
},
48+
}
49+
50+
var enableWebsocket *bool
51+
translator.buildRoute(ar, service, rule, nil, nil, nil, &enableWebsocket)
52+
53+
assert.Len(t, service.Routes, 1)
54+
route := service.Routes[0]
55+
// route.Hosts should NOT be set — hosts belong on Service, not Route.
56+
// Setting hosts on Route causes false diffs in backends that don't
57+
// support route-level hosts, triggering unnecessary PUT requests.
58+
assert.Nil(t, route.Hosts, "route.Hosts should not be set; hosts should only be on Service")
59+
assert.Equal(t, []string{"/api/*"}, route.Uris)
60+
}
61+
62+
func TestBuildService_HostsSet(t *testing.T) {
63+
translator := NewTranslator(logr.Discard())
64+
65+
ar := &apiv2.ApisixRoute{
66+
ObjectMeta: metav1.ObjectMeta{
67+
Name: "test-route",
68+
Namespace: "default",
69+
},
70+
}
71+
72+
rule := apiv2.ApisixRouteHTTP{
73+
Name: "rule1",
74+
Match: apiv2.ApisixRouteHTTPMatch{
75+
Hosts: []string{"example.com", "foo.com"},
76+
Paths: []string{"/api/*"},
77+
},
78+
}
79+
80+
service := translator.buildService(ar, rule, 0)
81+
82+
// service.Hosts SHOULD be set — this is the canonical location for hosts.
83+
assert.Equal(t, []string{"example.com", "foo.com"}, service.Hosts)
84+
}

test/e2e/crds/v2/route.go

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ spec:
139139
applier.MustApplyAPIv2(types.NamespacedName{Namespace: s.Namespace(), Name: "default"},
140140
&apisixRoute, fmt.Sprintf(apisixRouteSpec, s.Namespace(), s.Namespace(), "/headers"))
141141
Eventually(request).WithArguments("/get").WithTimeout(20 * time.Second).ProbeEvery(time.Second).Should(Equal(http.StatusNotFound))
142-
s.NewAPISIXClient().GET("/headers").WithHost("httpbin").Expect().Status(http.StatusOK)
142+
Eventually(request).WithArguments("/headers").WithTimeout(20 * time.Second).ProbeEvery(time.Second).Should(Equal(http.StatusOK))
143143

144144
By("delete ApisixRoute")
145145
err := s.DeleteResource("ApisixRoute", "default")
@@ -1347,7 +1347,6 @@ spec:
13471347
&apisixRouteWithoutWS,
13481348
fmt.Sprintf(apisixRouteSpec2, s.Namespace(), s.Namespace()),
13491349
)
1350-
time.Sleep(12 * time.Second)
13511350

13521351
By("verify WebSocket connection fails without WebSocket enabled")
13531352
u := url.URL{
@@ -1356,9 +1355,14 @@ spec:
13561355
Path: "/echo",
13571356
}
13581357
headers := http.Header{"Host": []string{"httpbin.org"}}
1359-
_, resp, _ := websocket.DefaultDialer.Dial(u.String(), headers)
1360-
// should receive 200 instead of 101
1361-
Expect(resp.StatusCode).Should(Equal(http.StatusOK))
1358+
// In standalone mode, config application is async — retry until the route is active
1359+
Eventually(func() int {
1360+
_, resp, _ := websocket.DefaultDialer.Dial(u.String(), headers)
1361+
if resp == nil {
1362+
return 0
1363+
}
1364+
return resp.StatusCode
1365+
}).WithTimeout(20 * time.Second).ProbeEvery(time.Second).Should(Equal(http.StatusOK))
13621366
By("apply ApisixRoute for WebSocket")
13631367
var apisixRoute apiv2.ApisixRoute
13641368
applier.MustApplyAPIv2(

0 commit comments

Comments
 (0)