Skip to content

Commit 15b2f39

Browse files
authored
fix: remove redundant route.Hosts to prevent false diffs in ADC sync (#392)
1 parent 21f983b commit 15b2f39

6 files changed

Lines changed: 96 additions & 8 deletions

File tree

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

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

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

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

.github/workflows/e2e-test.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ jobs:
102102
if: ${{ env.ADC_VERSION == 'dev' }}
103103
run: |
104104
docker create --name adc-temp ghcr.io/api7/adc:dev
105-
docker cp adc-temp:main.js adc.js
105+
docker cp adc-temp:main.cjs adc.js
106106
docker rm adc-temp
107107
node $(pwd)/adc.js -v
108108
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
@@ -192,7 +192,6 @@ func (t *Translator) buildRoute(ar *apiv2.ApisixRoute, service *adc.Service, rul
192192
route.EnableWebsocket = *enableWebsocket
193193
}
194194
route.FilterFunc = rule.Match.FilterFunc
195-
route.Hosts = rule.Match.Hosts
196195
route.Methods = rule.Match.Methods
197196
route.Plugins = plugins
198197
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 (e.g., API7 EE).
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 & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ spec:
141141
applier.MustApplyAPIv2(types.NamespacedName{Namespace: s.Namespace(), Name: "default"},
142142
&apisixRoute, fmt.Sprintf(apisixRouteSpec, s.Namespace(), s.Namespace(), "/headers"))
143143
Eventually(request).WithArguments("/get").WithTimeout(20 * time.Second).ProbeEvery(time.Second).Should(Equal(http.StatusNotFound))
144-
s.NewAPISIXClient().GET("/headers").WithHost("httpbin").Expect().Status(http.StatusOK)
144+
Eventually(request).WithArguments("/headers").WithTimeout(20 * time.Second).ProbeEvery(time.Second).Should(Equal(http.StatusOK))
145145

146146
By("delete ApisixRoute")
147147
err := s.DeleteResource("ApisixRoute", "default")
@@ -1448,9 +1448,14 @@ spec:
14481448
Path: "/echo",
14491449
}
14501450
headers := http.Header{"Host": []string{"httpbin.org"}}
1451-
_, resp, _ := websocket.DefaultDialer.Dial(u.String(), headers)
1452-
// should receive 200 instead of 101
1453-
Expect(resp.StatusCode).Should(Equal(http.StatusOK))
1451+
// In standalone mode, config application is async — retry until the route is active
1452+
Eventually(func() int {
1453+
_, resp, _ := websocket.DefaultDialer.Dial(u.String(), headers)
1454+
if resp == nil {
1455+
return 0
1456+
}
1457+
return resp.StatusCode
1458+
}).WithTimeout(20 * time.Second).ProbeEvery(time.Second).Should(Equal(http.StatusOK))
14541459
By("apply ApisixRoute for WebSocket")
14551460
var apisixRoute apiv2.ApisixRoute
14561461
applier.MustApplyAPIv2(

0 commit comments

Comments
 (0)