From 361379541d88f3a60cfed7bf9761bd49a7f8c3d9 Mon Sep 17 00:00:00 2001 From: AlinsRan Date: Tue, 2 Sep 2025 08:59:31 +0800 Subject: [PATCH 1/4] feat: support retry in case of sync failure (#2534) Signed-off-by: Ashing Zheng --- internal/provider/apisix/provider.go | 41 +++++++----- internal/provider/common/retrier.go | 96 ++++++++++++++++++++++++++++ test/e2e/crds/v2/route.go | 59 +++++++++++++++++ 3 files changed, 179 insertions(+), 17 deletions(-) create mode 100644 internal/provider/common/retrier.go diff --git a/internal/provider/apisix/provider.go b/internal/provider/apisix/provider.go index e277f672f..4742d6ab1 100644 --- a/internal/provider/apisix/provider.go +++ b/internal/provider/apisix/provider.go @@ -38,12 +38,20 @@ import ( "github.com/apache/apisix-ingress-controller/internal/controller/status" "github.com/apache/apisix-ingress-controller/internal/manager/readiness" "github.com/apache/apisix-ingress-controller/internal/provider" + "github.com/apache/apisix-ingress-controller/internal/provider/common" "github.com/apache/apisix-ingress-controller/internal/types" "github.com/apache/apisix-ingress-controller/internal/utils" pkgutils "github.com/apache/apisix-ingress-controller/pkg/utils" ) -const ProviderTypeAPISIX = "apisix" +const ( + ProviderTypeAPISIX = "apisix" + + RetryBaseDelay = 1 * time.Second + RetryMaxDelay = 1000 * time.Second + + MinSyncPeriod = 1 * time.Second +) type apisixProvider struct { provider.Options @@ -229,33 +237,32 @@ func (d *apisixProvider) Start(ctx context.Context) error { initalSyncDelay := d.InitSyncDelay if initalSyncDelay > 0 { - time.AfterFunc(initalSyncDelay, func() { - if err := d.sync(ctx); err != nil { - log.Error(err) - return - } - }) + time.AfterFunc(initalSyncDelay, d.syncNotify) } - if d.SyncPeriod < 1 { - return nil + syncPeriod := d.SyncPeriod + if syncPeriod < MinSyncPeriod { + syncPeriod = MinSyncPeriod } - ticker := time.NewTicker(d.SyncPeriod) + ticker := time.NewTicker(syncPeriod) defer ticker.Stop() + + retrier := common.NewRetrier(common.NewExponentialBackoff(RetryBaseDelay, RetryMaxDelay)) + for { - synced := false select { case <-d.syncCh: - synced = true case <-ticker.C: - synced = true + case <-retrier.C(): case <-ctx.Done(): + retrier.Reset() return nil } - if synced { - if err := d.sync(ctx); err != nil { - log.Error(err) - } + if err := d.sync(ctx); err != nil { + log.Error(err) + retrier.Next() + } else { + retrier.Reset() } } } diff --git a/internal/provider/common/retrier.go b/internal/provider/common/retrier.go new file mode 100644 index 000000000..1277ee93a --- /dev/null +++ b/internal/provider/common/retrier.go @@ -0,0 +1,96 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package common + +import ( + "sync" + "time" +) + +type Backoff interface { + Next() time.Duration + Reset() +} + +type ExponentialBackoff struct { + base, max, current time.Duration +} + +func NewExponentialBackoff(base, max time.Duration) *ExponentialBackoff { + return &ExponentialBackoff{base: base, max: max, current: base} +} + +func (b *ExponentialBackoff) Next() time.Duration { + delay := b.current + b.current *= 2 + if b.current > b.max { + b.current = b.max + } + return delay +} + +func (b *ExponentialBackoff) Reset() { + b.current = b.base +} + +type Retrier struct { + mu sync.Mutex + ch chan struct{} + timer *time.Timer + backoff Backoff +} + +func NewRetrier(b Backoff) *Retrier { + return &Retrier{ + ch: make(chan struct{}, 1), + backoff: b, + } +} + +func (r *Retrier) Reset() { + r.mu.Lock() + defer r.mu.Unlock() + + if r.timer != nil { + r.timer.Stop() + r.timer = nil + } + r.backoff.Reset() +} + +func (r *Retrier) Next() { + r.mu.Lock() + defer r.mu.Unlock() + + if r.timer != nil { + r.timer.Stop() + r.timer = nil + } + + delay := r.backoff.Next() + r.timer = time.AfterFunc(delay, func() { + select { + case r.ch <- struct{}{}: + default: + } + }) +} + +func (r *Retrier) C() <-chan struct{} { + return r.ch +} diff --git a/test/e2e/crds/v2/route.go b/test/e2e/crds/v2/route.go index 98cc5580e..d47baefee 100644 --- a/test/e2e/crds/v2/route.go +++ b/test/e2e/crds/v2/route.go @@ -25,6 +25,7 @@ import ( "net" "net/http" "net/url" + "os" "strings" "time" @@ -1762,4 +1763,62 @@ spec: }) }) }) + + Context("Exception Test", func() { + const apisixRouteSpec = ` +apiVersion: apisix.apache.org/v2 +kind: ApisixRoute +metadata: + name: default +spec: + ingressClassName: %s + http: + - name: rule0 + match: + hosts: + - httpbin + paths: + - /* + backends: + - serviceName: httpbin-service-e2e-test + servicePort: 80 +` + It("try again when sync failed", func() { + if os.Getenv("PROVIDER_TYPE") == framework.ProviderTypeAPI7EE { + Skip("skipping test in API7EE mode") + } + s.Deployer.ScaleDataplane(0) + + err := s.CreateResourceFromString(fmt.Sprintf(apisixRouteSpec, s.Namespace())) + Expect(err).NotTo(HaveOccurred(), "creating ApisixRoute") + + By("check ApisixRoute status") + s.RetryAssertion(func() string { + output, _ := s.GetOutputFromString("ar", "default", "-o", "yaml", "-n", s.Namespace()) + return output + }).WithTimeout(30 * time.Second). + Should( + And( + ContainSubstring(`status: "False"`), + ContainSubstring(`reason: SyncFailed`), + ), + ) + + s.Deployer.ScaleDataplane(1) + + s.RetryAssertion(func() string { + output, _ := s.GetOutputFromString("ar", "default", "-o", "yaml", "-n", s.Namespace()) + return output + }).WithTimeout(60 * time.Second). + Should(ContainSubstring(`status: "True"`)) + + By("check route in APISIX") + s.RequestAssert(&scaffold.RequestAssert{ + Method: "GET", + Path: "/get", + Host: "httpbin", + Check: scaffold.WithExpectedStatus(200), + }) + }) + }) }) From 3df52c495089e490ce8ffcbd625dca44c5354426 Mon Sep 17 00:00:00 2001 From: AlinsRan Date: Wed, 3 Sep 2025 16:25:37 +0800 Subject: [PATCH 2/4] fix: sync exception caused by ingress endpoint 0 (#2538) --- api/adc/types.go | 14 ++++++ internal/adc/translator/httproute.go | 2 +- internal/adc/translator/ingress.go | 2 +- test/e2e/crds/v2/route.go | 55 +++++++++++++++++++++++ test/e2e/gatewayapi/httproute.go | 34 +++++++++++++++ test/e2e/ingress/ingress.go | 65 ++++++++++++++-------------- 6 files changed, 138 insertions(+), 34 deletions(-) diff --git a/api/adc/types.go b/api/adc/types.go index eccae1fed..618fb3ce8 100644 --- a/api/adc/types.go +++ b/api/adc/types.go @@ -505,6 +505,20 @@ func (n *UpstreamNodes) UnmarshalJSON(p []byte) error { return nil } +// MarshalJSON implements the json.Marshaler interface for UpstreamNodes. +// By default, Go serializes a nil slice as JSON null. However, for compatibility +// with APISIX semantics, we want a nil UpstreamNodes to be encoded as an empty +// array ([]) instead of null. Non-nil slices are marshaled as usual. +// +// See APISIX upstream nodes schema definition for details: +// https://github.com/apache/apisix/blob/77dacda31277a31d6014b4970e36bae2a5c30907/apisix/schema_def.lua#L295-L338 +func (n UpstreamNodes) MarshalJSON() ([]byte, error) { + if n == nil { + return []byte("[]"), nil + } + return json.Marshal([]UpstreamNode(n)) +} + // ComposeRouteName uses namespace, name and rule name to compose // the route name. func ComposeRouteName(namespace, name string, rule string) string { diff --git a/internal/adc/translator/httproute.go b/internal/adc/translator/httproute.go index 10bcd951f..4c3f64264 100644 --- a/internal/adc/translator/httproute.go +++ b/internal/adc/translator/httproute.go @@ -285,7 +285,7 @@ func (t *Translator) fillHTTPRoutePolicies(routes []*adctypes.Route, policies [] } func (t *Translator) translateEndpointSlice(portName *string, weight int, endpointSlices []discoveryv1.EndpointSlice, endpointFilter func(*discoveryv1.Endpoint) bool) adctypes.UpstreamNodes { - var nodes adctypes.UpstreamNodes + nodes := adctypes.UpstreamNodes{} if len(endpointSlices) == 0 { return nodes } diff --git a/internal/adc/translator/ingress.go b/internal/adc/translator/ingress.go index 7293558bb..04aa75384 100644 --- a/internal/adc/translator/ingress.go +++ b/internal/adc/translator/ingress.go @@ -222,7 +222,7 @@ func (t *Translator) TranslateIngress(tctx *provider.TranslateContext, obj *netw // translateEndpointSliceForIngress create upstream nodes from EndpointSlice func (t *Translator) translateEndpointSliceForIngress(weight int, endpointSlices []discoveryv1.EndpointSlice, servicePort *corev1.ServicePort) adctypes.UpstreamNodes { - var nodes adctypes.UpstreamNodes + nodes := adctypes.UpstreamNodes{} if len(endpointSlices) == 0 { return nodes } diff --git a/test/e2e/crds/v2/route.go b/test/e2e/crds/v2/route.go index d47baefee..a36d660f2 100644 --- a/test/e2e/crds/v2/route.go +++ b/test/e2e/crds/v2/route.go @@ -471,6 +471,61 @@ spec: }).WithTimeout(30 * time.Second).ProbeEvery(1 * time.Second).Should(Equal(http.StatusOK)) } }) + + It("Service Endpoints Changed", func() { + const apisixRouteSpec = ` +apiVersion: apisix.apache.org/v2 +kind: ApisixRoute +metadata: + name: default +spec: + ingressClassName: %s + http: + - name: rule0 + match: + hosts: + - httpbin + paths: + - /* + backends: + - serviceName: httpbin-service-e2e-test + servicePort: 80 +` + + By("apply ApisixRoute") + var apisixRoute apiv2.ApisixRoute + applier.MustApplyAPIv2(types.NamespacedName{Namespace: s.Namespace(), Name: "default"}, + &apisixRoute, fmt.Sprintf(apisixRouteSpec, s.Namespace())) + + s.RequestAssert(&scaffold.RequestAssert{ + Method: "GET", + Path: "/get", + Host: "httpbin", + Check: scaffold.WithExpectedStatus(http.StatusOK), + }) + + By("scale httpbin deployment to 0") + err := s.ScaleHTTPBIN(0) + Expect(err).NotTo(HaveOccurred(), "scaling httpbin deployment to 0") + + s.RequestAssert(&scaffold.RequestAssert{ + Method: "GET", + Path: "/get", + Host: "httpbin", + Check: scaffold.WithExpectedStatus(http.StatusServiceUnavailable), + }) + + By("scale httpbin deployment to 1") + err = s.ScaleHTTPBIN(1) + Expect(err).NotTo(HaveOccurred(), "scaling httpbin deployment to 1") + + s.RequestAssert(&scaffold.RequestAssert{ + Method: "GET", + Path: "/get", + Host: "httpbin", + Check: scaffold.WithExpectedStatus(http.StatusOK), + }) + }) }) Context("Ingress Scale and Route Management", func() { diff --git a/test/e2e/gatewayapi/httproute.go b/test/e2e/gatewayapi/httproute.go index b2ba3aaa5..44bc33b26 100644 --- a/test/e2e/gatewayapi/httproute.go +++ b/test/e2e/gatewayapi/httproute.go @@ -738,6 +738,40 @@ spec: applyHTTPRouteAndAssert(s, route, asserts) }) + It("Service Endpoints changed", func() { + gatewayName := s.Namespace() + By("create HTTPRoute") + s.ResourceApplied("HTTPRoute", "httpbin", fmt.Sprintf(exactRouteByGet, gatewayName), 1) + + s.RequestAssert(&scaffold.RequestAssert{ + Method: "GET", + Path: "/get", + Host: "httpbin.example", + Check: scaffold.WithExpectedStatus(http.StatusOK), + }) + + By("scale httpbin deployment to 0") + err := s.ScaleHTTPBIN(0) + Expect(err).NotTo(HaveOccurred(), "scaling httpbin deployment to 0") + + s.RequestAssert(&scaffold.RequestAssert{ + Method: "GET", + Path: "/get", + Host: "httpbin.example", + Check: scaffold.WithExpectedStatus(http.StatusServiceUnavailable), + }) + + By("scale httpbin deployment to 1") + err = s.ScaleHTTPBIN(1) + Expect(err).NotTo(HaveOccurred(), "scaling httpbin deployment to 1") + + s.RequestAssert(&scaffold.RequestAssert{ + Method: "GET", + Path: "/get", + Host: "httpbin.example", + Check: scaffold.WithExpectedStatus(http.StatusOK), + }) + }) }) Context("HTTPRoute Rule Match", func() { diff --git a/test/e2e/ingress/ingress.go b/test/e2e/ingress/ingress.go index ed0922cd8..eb93db676 100644 --- a/test/e2e/ingress/ingress.go +++ b/test/e2e/ingress/ingress.go @@ -204,8 +204,7 @@ spec: port: number: 80 ` - - It("Test IngressClass Selection", func() { + BeforeEach(func() { By("create GatewayProxy") gatewayProxy := fmt.Sprintf(gatewayProxyYaml, s.Namespace(), s.Deployer.GetAdminEndpoint(), s.AdminKey()) err := s.CreateResourceFromStringWithNamespace(gatewayProxy, s.Namespace()) @@ -217,34 +216,47 @@ spec: Expect(err).NotTo(HaveOccurred(), "creating Default IngressClass") time.Sleep(5 * time.Second) + }) + + It("Service Endpoints Changed", func() { By("create Ingress without IngressClass") - err = s.CreateResourceFromStringWithNamespace(fmt.Sprintf(defaultIngress, s.Namespace()), s.Namespace()) + err := s.CreateResourceFromStringWithNamespace(fmt.Sprintf(defaultIngress, s.Namespace()), s.Namespace()) Expect(err).NotTo(HaveOccurred(), "creating Ingress without IngressClass") time.Sleep(5 * time.Second) By("verify default ingress") - s.NewAPISIXClient(). - GET("/get"). - WithHost("default.example.com"). - Expect(). - Status(200) - }) + s.RequestAssert(&scaffold.RequestAssert{ + Method: "GET", + Path: "/get", + Host: "default.example.com", + Check: scaffold.WithExpectedStatus(http.StatusOK), + }) - It("Proxy External Service", func() { - By("create GatewayProxy") - gatewayProxy := fmt.Sprintf(gatewayProxyYaml, s.Namespace(), s.Deployer.GetAdminEndpoint(), s.AdminKey()) - err := s.CreateResourceFromStringWithNamespace(gatewayProxy, s.Namespace()) - Expect(err).NotTo(HaveOccurred(), "creating GatewayProxy") - time.Sleep(5 * time.Second) + err = s.ScaleHTTPBIN(0) + Expect(err).NotTo(HaveOccurred(), "scaling httpbin to 0") - By("create Default IngressClass") - err = s.CreateResourceFromStringWithNamespace(fmt.Sprintf(defaultIngressClass, s.GetControllerName(), s.Namespace()), s.Namespace()) - Expect(err).NotTo(HaveOccurred(), "creating Default IngressClass") - time.Sleep(5 * time.Second) + s.RequestAssert(&scaffold.RequestAssert{ + Method: "GET", + Path: "/get", + Host: "default.example.com", + Check: scaffold.WithExpectedStatus(http.StatusServiceUnavailable), + }) + + err = s.ScaleHTTPBIN(1) + Expect(err).NotTo(HaveOccurred(), "scaling httpbin to 1") + s.RequestAssert(&scaffold.RequestAssert{ + Method: "GET", + Path: "/get", + Host: "default.example.com", + Check: scaffold.WithExpectedStatus(http.StatusOK), + }) + }) + + It("Proxy External Service", func() { By("create Ingress") ingressName := s.Namespace() + "-external" - err = s.CreateResourceFromStringWithNamespace(fmt.Sprintf(ingressWithExternalName, ingressName), s.Namespace()) + err := s.CreateResourceFromStringWithNamespace(fmt.Sprintf(ingressWithExternalName, ingressName), s.Namespace()) Expect(err).NotTo(HaveOccurred(), "creating Ingress without IngressClass") By("checking the external service response") @@ -259,20 +271,9 @@ spec: }) It("Delete Ingress during restart", func() { - By("create GatewayProxy") - gatewayProxy := fmt.Sprintf(gatewayProxyYaml, s.Namespace(), s.Deployer.GetAdminEndpoint(), s.AdminKey()) - err := s.CreateResourceFromStringWithNamespace(gatewayProxy, s.Namespace()) - Expect(err).NotTo(HaveOccurred(), "creating GatewayProxy") - time.Sleep(5 * time.Second) - - By("create Default IngressClass") - err = s.CreateResourceFromStringWithNamespace(fmt.Sprintf(defaultIngressClass, s.GetControllerName(), s.Namespace()), s.Namespace()) - Expect(err).NotTo(HaveOccurred(), "creating Default IngressClass") - time.Sleep(5 * time.Second) - By("create Ingress with ExternalName") ingressName := s.Namespace() + "-external" - err = s.CreateResourceFromStringWithNamespace(fmt.Sprintf(ingressWithExternalName, ingressName), s.Namespace()) + err := s.CreateResourceFromStringWithNamespace(fmt.Sprintf(ingressWithExternalName, ingressName), s.Namespace()) Expect(err).NotTo(HaveOccurred(), "creating Ingress without IngressClass") time.Sleep(5 * time.Second) From 0bcf983aef35cf2d5f901d54b6162fa6da505de8 Mon Sep 17 00:00:00 2001 From: Ashing Zheng Date: Thu, 4 Sep 2025 15:20:28 +0800 Subject: [PATCH 3/4] fix: handle httproute multi backend refs (#2540) Signed-off-by: Ashing Zheng Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Ashing Zheng --- internal/adc/translator/httproute.go | 80 +++++++++++++++++++++---- test/e2e/framework/manifests/nginx.yaml | 2 +- test/e2e/framework/nginx.go | 1 + test/e2e/gatewayapi/httproute.go | 2 + 4 files changed, 73 insertions(+), 12 deletions(-) diff --git a/internal/adc/translator/httproute.go b/internal/adc/translator/httproute.go index 4c3f64264..90816258d 100644 --- a/internal/adc/translator/httproute.go +++ b/internal/adc/translator/httproute.go @@ -33,6 +33,7 @@ import ( adctypes "github.com/apache/apisix-ingress-controller/api/adc" "github.com/apache/apisix-ingress-controller/api/v1alpha1" + apiv2 "github.com/apache/apisix-ingress-controller/api/v2" "github.com/apache/apisix-ingress-controller/internal/controller/label" "github.com/apache/apisix-ingress-controller/internal/id" "github.com/apache/apisix-ingress-controller/internal/provider" @@ -466,32 +467,89 @@ func (t *Translator) TranslateHTTPRoute(tctx *provider.TranslateContext, httpRou labels := label.GenLabel(httpRoute) for ruleIndex, rule := range rules { - upstream := adctypes.NewDefaultUpstream() - var backendErr error + service := adctypes.NewDefaultService() + service.Labels = labels + + service.Name = adctypes.ComposeServiceNameWithRule(httpRoute.Namespace, httpRoute.Name, fmt.Sprintf("%d", ruleIndex)) + service.ID = id.GenID(service.Name) + service.Hosts = hosts + + var ( + upstreams = make([]*adctypes.Upstream, 0) + weightedUpstreams = make([]adctypes.TrafficSplitConfigRuleWeightedUpstream, 0) + backendErr error + ) + for _, backend := range rule.BackendRefs { if backend.Namespace == nil { namespace := gatewayv1.Namespace(httpRoute.Namespace) backend.Namespace = &namespace } + upstream := adctypes.NewDefaultUpstream() upNodes, err := t.translateBackendRef(tctx, backend.BackendRef, DefaultEndpointFilter) if err != nil { backendErr = err continue } + if len(upNodes) == 0 { + continue + } + t.AttachBackendTrafficPolicyToUpstream(backend.BackendRef, tctx.BackendTrafficPolicies, upstream) - upstream.Nodes = append(upstream.Nodes, upNodes...) + upstream.Nodes = upNodes + upstreams = append(upstreams, upstream) } - // todo: support multiple backends - service := adctypes.NewDefaultService() - service.Labels = labels + // Handle multiple backends with traffic-split plugin + if len(upstreams) == 0 { + // Create a default upstream if no valid backends + upstream := adctypes.NewDefaultUpstream() + service.Upstream = upstream + } else if len(upstreams) == 1 { + // Single backend - use directly as service upstream + service.Upstream = upstreams[0] + } else { + // Multiple backends - use traffic-split plugin + service.Upstream = upstreams[0] + upstreams = upstreams[1:] + + // Set weight in traffic-split for the default upstream + weight := apiv2.DefaultWeight + if rule.BackendRefs[0].Weight != nil { + weight = int(*rule.BackendRefs[0].Weight) + } + weightedUpstreams = append(weightedUpstreams, adctypes.TrafficSplitConfigRuleWeightedUpstream{ + Weight: weight, + }) - service.Name = adctypes.ComposeServiceNameWithRule(httpRoute.Namespace, httpRoute.Name, fmt.Sprintf("%d", ruleIndex)) - service.ID = id.GenID(service.Name) - service.Hosts = hosts - service.Upstream = upstream + // Set other upstreams in traffic-split + for i, upstream := range upstreams { + weight := apiv2.DefaultWeight + // get weight from the backend refs starting from the second backend + if i+1 < len(rule.BackendRefs) && rule.BackendRefs[i+1].Weight != nil { + weight = int(*rule.BackendRefs[i+1].Weight) + } + weightedUpstreams = append(weightedUpstreams, adctypes.TrafficSplitConfigRuleWeightedUpstream{ + Upstream: upstream, + Weight: weight, + }) + } + + if len(weightedUpstreams) > 0 { + if service.Plugins == nil { + service.Plugins = make(map[string]any) + } + service.Plugins["traffic-split"] = &adctypes.TrafficSplitConfig{ + Rules: []adctypes.TrafficSplitConfigRule{ + { + WeightedUpstreams: weightedUpstreams, + }, + }, + } + } + } - if backendErr != nil && len(upstream.Nodes) == 0 { + if backendErr != nil && (service.Upstream == nil || len(service.Upstream.Nodes) == 0) { if service.Plugins == nil { service.Plugins = make(map[string]any) } diff --git a/test/e2e/framework/manifests/nginx.yaml b/test/e2e/framework/manifests/nginx.yaml index c0d973737..7fb93f08e 100644 --- a/test/e2e/framework/manifests/nginx.yaml +++ b/test/e2e/framework/manifests/nginx.yaml @@ -44,7 +44,7 @@ kind: Deployment metadata: name: nginx spec: - replicas: 1 + replicas: {{ .Replicas | default 1 }} selector: matchLabels: app: nginx diff --git a/test/e2e/framework/nginx.go b/test/e2e/framework/nginx.go index ae1795972..9612c4e33 100644 --- a/test/e2e/framework/nginx.go +++ b/test/e2e/framework/nginx.go @@ -37,6 +37,7 @@ var ( type NginxOptions struct { Namespace string + Replicas *int32 } func init() { diff --git a/test/e2e/gatewayapi/httproute.go b/test/e2e/gatewayapi/httproute.go index 44bc33b26..93fd06800 100644 --- a/test/e2e/gatewayapi/httproute.go +++ b/test/e2e/gatewayapi/httproute.go @@ -30,6 +30,7 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "k8s.io/utils/ptr" "sigs.k8s.io/gateway-api/apis/v1alpha2" "github.com/apache/apisix-ingress-controller/api/v1alpha1" @@ -1873,6 +1874,7 @@ spec: beforeEachHTTP() s.DeployNginx(framework.NginxOptions{ Namespace: s.Namespace(), + Replicas: ptr.To(int32(2)), }) }) It("HTTPRoute Canary", func() { From f607e9c730381f35ec88c61cbb9d713208182f13 Mon Sep 17 00:00:00 2001 From: AlinsRan Date: Fri, 5 Sep 2025 10:44:45 +0800 Subject: [PATCH 4/4] fix: responseHeaderModifier fails to synchronize (#2544) --- api/adc/types.go | 6 +- test/e2e/gatewayapi/httproute.go | 113 ++++++++++++++++++++++++++++++- test/e2e/scaffold/assertion.go | 22 ++++++ 3 files changed, 135 insertions(+), 6 deletions(-) diff --git a/api/adc/types.go b/api/adc/types.go index 618fb3ce8..db4d2d71b 100644 --- a/api/adc/types.go +++ b/api/adc/types.go @@ -635,9 +635,9 @@ type ResponseRewriteConfig struct { } type ResponseHeaders struct { - Set map[string]string `json:"set" yaml:"set"` - Add []string `json:"add" yaml:"add"` - Remove []string `json:"remove" yaml:"remove"` + Set map[string]string `json:"set,omitempty" yaml:"set,omitempty"` + Add []string `json:"add,omitempty" yaml:"add,omitempty"` + Remove []string `json:"remove,omitempty" yaml:"remove,omitempty"` } // RequestMirror is the rule config for proxy-mirror plugin. diff --git a/test/e2e/gatewayapi/httproute.go b/test/e2e/gatewayapi/httproute.go index 93fd06800..ac0d1070b 100644 --- a/test/e2e/gatewayapi/httproute.go +++ b/test/e2e/gatewayapi/httproute.go @@ -1401,6 +1401,82 @@ spec: port: 80 ` + var respHeaderModifyWithAdd = ` +apiVersion: gateway.networking.k8s.io/v1 +kind: HTTPRoute +metadata: + name: add +spec: + parentRefs: + - name: %s + hostnames: + - httpbin.example.resp-header-modify.add + rules: + - matches: + - path: + type: Exact + value: /headers + filters: + - type: ResponseHeaderModifier + responseHeaderModifier: + add: + - name: X-Resp-Add + value: "resp-add" + backendRefs: + - name: httpbin-service-e2e-test + port: 80 +` + + var respHeaderModifyWithSet = ` +apiVersion: gateway.networking.k8s.io/v1 +kind: HTTPRoute +metadata: + name: set +spec: + parentRefs: + - name: %s + hostnames: + - httpbin.example.resp-header-modify.set + rules: + - matches: + - path: + type: Exact + value: /headers + filters: + - type: ResponseHeaderModifier + responseHeaderModifier: + set: + - name: X-Resp-Set + value: "resp-set" + backendRefs: + - name: httpbin-service-e2e-test + port: 80 +` + + var respHeaderModifyWithRemove = ` +apiVersion: gateway.networking.k8s.io/v1 +kind: HTTPRoute +metadata: + name: remove +spec: + parentRefs: + - name: %s + hostnames: + - httpbin.example.resp-header-modify.remove + rules: + - matches: + - path: + type: Exact + value: /headers + filters: + - type: ResponseHeaderModifier + responseHeaderModifier: + remove: + - Server + backendRefs: + - name: httpbin-service-e2e-test + port: 80 +` var respHeaderModifyByHeaders = ` apiVersion: gateway.networking.k8s.io/v1 kind: HTTPRoute @@ -1612,6 +1688,9 @@ spec: It("HTTPRoute ResponseHeaderModifier", func() { By("create HTTPRoute") s.ResourceApplied("HTTPRoute", "httpbin", fmt.Sprintf(respHeaderModifyByHeaders, s.Namespace(), s.Namespace()), 1) + s.ResourceApplied("HTTPRoute", "add", fmt.Sprintf(respHeaderModifyWithAdd, s.Namespace()), 1) + s.ResourceApplied("HTTPRoute", "set", fmt.Sprintf(respHeaderModifyWithSet, s.Namespace()), 1) + s.ResourceApplied("HTTPRoute", "remove", fmt.Sprintf(respHeaderModifyWithRemove, s.Namespace()), 1) By("access daataplane to check the HTTPRoute") s.RequestAssert(&scaffold.RequestAssert{ @@ -1623,12 +1702,40 @@ spec: scaffold.WithExpectedHeaders(map[string]string{ "X-Resp-Add": "add", "X-Resp-Set": "set", - "Server": "", }), + scaffold.WithExpectedNotHeader("Server"), scaffold.WithExpectedBodyNotContains(`"X-Resp-Add": "add"`, `"X-Resp-Set": "set"`, `"Server"`), }, - Timeout: time.Second * 30, - Interval: time.Second * 2, + }) + s.RequestAssert(&scaffold.RequestAssert{ + Method: "GET", + Path: "/headers", + Host: "httpbin.example.resp-header-modify.add", + Checks: []scaffold.ResponseCheckFunc{ + scaffold.WithExpectedStatus(http.StatusOK), + scaffold.WithExpectedHeader("X-Resp-Add", "resp-add"), + scaffold.WithExpectedBodyNotContains(`"X-Resp-Add": "resp-add"`), + }, + }) + s.RequestAssert(&scaffold.RequestAssert{ + Method: "GET", + Path: "/headers", + Host: "httpbin.example.resp-header-modify.set", + Checks: []scaffold.ResponseCheckFunc{ + scaffold.WithExpectedStatus(http.StatusOK), + scaffold.WithExpectedHeader("X-Resp-Set", "resp-set"), + scaffold.WithExpectedBodyNotContains(`"Server"`), + }, + }) + s.RequestAssert(&scaffold.RequestAssert{ + Method: "GET", + Path: "/headers", + Host: "httpbin.example.resp-header-modify.remove", + Checks: []scaffold.ResponseCheckFunc{ + scaffold.WithExpectedStatus(http.StatusOK), + scaffold.WithExpectedNotHeader("Server"), + scaffold.WithExpectedBodyNotContains(`"Server"`), + }, }) }) diff --git a/test/e2e/scaffold/assertion.go b/test/e2e/scaffold/assertion.go index ef2c35ebd..30c8cd8ef 100644 --- a/test/e2e/scaffold/assertion.go +++ b/test/e2e/scaffold/assertion.go @@ -149,6 +149,28 @@ func WithExpectedHeaders(expectedHeaders map[string]string) ResponseCheckFunc { } } +func WithExpectedNotHeader(key string) ResponseCheckFunc { + return func(resp *HTTPResponse) error { + if resp.Header.Get(key) != "" { + return fmt.Errorf("expected header %q to be empty, but got %q", + key, resp.Header.Get(key)) + } + return nil + } +} + +func WithExpectedNotHeaders(unexpectedHeaders []string) ResponseCheckFunc { + return func(resp *HTTPResponse) error { + for _, key := range unexpectedHeaders { + if resp.Header.Get(key) != "" { + return fmt.Errorf("expected header %q to be empty, but got %q", + key, resp.Header.Get(key)) + } + } + return nil + } +} + func (s *Scaffold) RequestAssert(r *RequestAssert) bool { if r.Client == nil { r.Client = s.NewAPISIXClient()