From 2004d9a0f313426f75a45d253b1c7dadede03232 Mon Sep 17 00:00:00 2001 From: AlinsRan Date: Mon, 19 Oct 2026 10:37:47 +0800 Subject: [PATCH 1/6] fix: scope ADC synchronization to controller-managed resources --- docs/en/latest/FAQ.md | 22 ++++ internal/adc/client/client.go | 21 +++- internal/adc/client/ownership_test.go | 109 ++++++++++++++++++++ internal/controller/label/label.go | 3 +- test/e2e/apisix/managed_resources.go | 139 ++++++++++++++++++++++++++ 5 files changed, 289 insertions(+), 5 deletions(-) create mode 100644 internal/adc/client/ownership_test.go create mode 100644 test/e2e/apisix/managed_resources.go diff --git a/docs/en/latest/FAQ.md b/docs/en/latest/FAQ.md index 4ad421cdb..4b02f0283 100644 --- a/docs/en/latest/FAQ.md +++ b/docs/en/latest/FAQ.md @@ -54,3 +54,25 @@ When both filters and a PluginConfig CRD are applied: * If filters are applied first, PluginConfig overrides any overlapping plugin settings. * If PluginConfig is applied first, filters merge with PluginConfig settings, and overlapping fields from filters take precedence. + +### Can Admin API resources coexist with controller-managed resources? + +With `provider.type: apisix` or `api7ee`, synchronization selects resources labeled +`manager-by: apisix-ingress-controller`. Resources created outside the controller +without this label are excluded from deletion, including when a Gateway or +IngressClass is deleted. Do not reuse controller-generated resource IDs for +externally managed resources: a matching ID can still be overwritten. + +ADC filters top-level resources. Routes nested under a selected Service are +included with that Service, so externally managed routes should not share a +controller-managed Service. Global rules and plugin metadata do not support this +label-based isolation and remain subject to full reconciliation. The label also +does not distinguish separate controller installations sharing one APISIX instance or API7 gateway group. +This behavior does not apply to `apisix-standalone`, which manages a complete +configuration. + +For the APISIX backend, unlabeled historical resources with IDs still present in +the controller's desired configuration are updated and labeled during synchronization. +Unlabeled orphaned resources are preserved. Review their ownership before deleting +them or adding the controller label; adding the label allows the next synchronization +to delete resources absent from the controller's desired configuration. diff --git a/internal/adc/client/client.go b/internal/adc/client/client.go index 83d617a1f..f67896c0d 100644 --- a/internal/adc/client/client.go +++ b/internal/adc/client/client.go @@ -21,6 +21,7 @@ import ( "context" "encoding/json" "fmt" + "maps" "os" "slices" "strings" @@ -389,6 +390,20 @@ func (c *Client) push(ctx context.Context, config adctypes.Config, args []string return alsoReport, err } +func buildSyncArgs(filePath string, task Task, backend string) []string { + labels := task.Labels + if backend == "apisix" || backend == "api7ee" { + // Preserve per-resource constraints while scoping full sync and gateway cleanup. + // ADC does not apply label selectors to global rules or plugin metadata. + labels = maps.Clone(labels) + if labels == nil { + labels = make(map[string]string) + } + labels[label.LabelManagedBy] = label.LabelManagedByValue + } + return BuildADCExecuteArgs(filePath, labels, task.ResourceTypes) +} + func (c *Client) sync(ctx context.Context, task Task) error { c.log.V(1).Info("syncing resources", "task", task) @@ -437,8 +452,6 @@ func (c *Client) sync(ctx context.Context, task Task) error { pkgmetrics.RecordFileIODuration("prepare_sync_file", "success", time.Since(fileIOStart).Seconds()) defer cleanup() - args := BuildADCExecuteArgs(syncFilePath, task.Labels, task.ResourceTypes) - // Record sync duration for each config startTime := time.Now() resourceType := strings.Join(task.ResourceTypes, ",") @@ -449,6 +462,7 @@ func (c *Client) sync(ctx context.Context, task Task) error { config.BackendType = c.defaultMode } + args := buildSyncArgs(syncFilePath, task, config.BackendType) err = c.executor.Execute(ctx, config, args) duration := time.Since(startTime).Seconds() @@ -488,8 +502,6 @@ func (c *Client) sync(ctx context.Context, task Task) error { defer cleanup() c.log.V(1).Info("prepared sync file", "path", syncFilePath) - args := BuildADCExecuteArgs(syncFilePath, task.Labels, task.ResourceTypes) - for _, config := range task.Configs { // Record sync duration for each config startTime := time.Now() @@ -501,6 +513,7 @@ func (c *Client) sync(ctx context.Context, task Task) error { config.BackendType = c.defaultMode } + args := buildSyncArgs(syncFilePath, task, config.BackendType) alsoReport, err := c.push(ctx, config, args) errs.Errors = append(errs.Errors, alsoReport...) diff --git a/internal/adc/client/ownership_test.go b/internal/adc/client/ownership_test.go new file mode 100644 index 000000000..922a0d4c5 --- /dev/null +++ b/internal/adc/client/ownership_test.go @@ -0,0 +1,109 @@ +// 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 client + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/go-logr/logr" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + adctypes "github.com/apache/apisix-ingress-controller/api/adc" + "github.com/apache/apisix-ingress-controller/internal/controller/label" + "github.com/apache/apisix-ingress-controller/internal/types" +) + +func TestSyncOwnershipSelector(t *testing.T) { + for _, backend := range []string{"apisix", "api7ee", "apisix-standalone"} { + for _, explicit := range []bool{false, true} { + for _, operation := range []string{"full", "resource", "cleanup", "global-rule"} { + name := backend + "/default/" + operation + if explicit { + name = backend + "/explicit/" + operation + } + t.Run(name, func(t *testing.T) { + received := make(chan ADCServerRequest, 1) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var req ADCServerRequest + if err := json.NewDecoder(r.Body).Decode(&req); err != nil { + t.Error(err) + } + received <- req + _ = json.NewEncoder(w).Encode(adctypes.SyncResult{Status: adctypes.StatusSuccess}) + })) + defer server.Close() + t.Setenv("ADC_SERVER_URL", server.URL) + c, err := New(logr.Discard(), backend, time.Second) + require.NoError(t, err) + config := adctypes.Config{Name: "proxy", ServerAddrs: []string{"http://apisix:9180"}} + if explicit { + config.BackendType = backend + } + owner := types.NamespacedNameKind{Kind: "Ingress", Namespace: "default", Name: "route"} + proxy := types.NamespacedNameKind{Kind: "GatewayProxy", Namespace: "default", Name: "proxy"} + task := Task{ + Key: owner, + Configs: map[types.NamespacedNameKind]adctypes.Config{proxy: config}, + Resources: &adctypes.Resources{}, + } + expected := map[string]string{} + if backend != "apisix-standalone" { + expected[label.LabelManagedBy] = "apisix-ingress-controller" + } + switch operation { + case "full": + require.NoError(t, c.UpdateConfig(context.Background(), task)) + _, err = c.Sync(context.Background()) + case "cleanup": + require.NoError(t, c.UpdateConfig(context.Background(), task)) + err = c.Delete(context.Background(), Task{Key: owner}) + default: + task.Labels = map[string]string{label.LabelResourceKey: "Ingress/default/route", label.LabelNamespace: "default"} + if operation == "global-rule" { + task.ResourceTypes = []string{"global_rule"} + } else { + task.ResourceTypes = []string{"service"} + } + expected[label.LabelResourceKey] = "Ingress/default/route" + require.NoError(t, c.UpdateConfig(context.Background(), task)) + err = c.sync(context.Background(), task) + assert.Equal(t, map[string]string{label.LabelResourceKey: "Ingress/default/route", label.LabelNamespace: "default"}, task.Labels) + } + require.NoError(t, err) + select { + case req := <-received: + assert.Equal(t, backend, req.Task.Opts.Backend) + if len(expected) == 0 { + assert.Empty(t, req.Task.Opts.LabelSelector) + } else { + assert.Equal(t, expected, req.Task.Opts.LabelSelector) + } + default: + t.Fatal("sync did not reach ADC") + } + }) + } + } + } +} diff --git a/internal/controller/label/label.go b/internal/controller/label/label.go index cb5a539da..6afbdfb88 100644 --- a/internal/controller/label/label.go +++ b/internal/controller/label/label.go @@ -33,6 +33,7 @@ const ( LabelNamespace = "k8s/namespace" LabelControllerName = "k8s/controller-name" LabelManagedBy = "manager-by" + LabelManagedByValue = "apisix-ingress-controller" LabelResourceKey = "k8s/resource-key" ) @@ -42,7 +43,7 @@ func GenLabel(obj client.Object, args ...string) Label { label[LabelNamespace] = obj.GetNamespace() label[LabelName] = obj.GetName() label[LabelControllerName] = config.ControllerConfig.ControllerName - label[LabelManagedBy] = "apisix-ingress-controller" + label[LabelManagedBy] = LabelManagedByValue label[LabelResourceKey] = fmt.Sprintf("%s/%s/%s", label[LabelKind], label[LabelNamespace], label[LabelName]) for i := 0; i+1 < len(args); i += 2 { label[args[i]] = args[i+1] diff --git a/test/e2e/apisix/managed_resources.go b/test/e2e/apisix/managed_resources.go new file mode 100644 index 000000000..912fb8dd8 --- /dev/null +++ b/test/e2e/apisix/managed_resources.go @@ -0,0 +1,139 @@ +// 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 apisix + +import ( + "fmt" + "net/http" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/apache/apisix-ingress-controller/test/e2e/framework" + "github.com/apache/apisix-ingress-controller/test/e2e/scaffold" +) + +var _ = Describe("Test managed resource isolation", Label("networking.k8s.io", "ingress"), func() { + if framework.ProviderType != framework.ProviderTypeAPISIX { + return + } + s := scaffold.NewDefaultScaffold() + const gatewayProxyYaml = ` +apiVersion: apisix.apache.org/v1alpha1 +kind: GatewayProxy +metadata: + name: apisix-proxy-config +spec: + provider: + type: ControlPlane + controlPlane: + endpoints: + - %s + auth: + type: AdminKey + adminKey: + value: "%s" +` + + const ingressClassYaml = ` +apiVersion: networking.k8s.io/v1 +kind: IngressClass +metadata: + name: %s +spec: + controller: "%s" + parameters: + apiGroup: "apisix.apache.org" + kind: "GatewayProxy" + name: "apisix-proxy-config" + namespace: "%s" + scope: "Namespace" +` + + const ingressYaml = ` +apiVersion: networking.k8s.io/v1 +kind: Ingress +metadata: + name: httpbin +spec: + ingressClassName: %s + rules: + - host: managed-resources.example + http: + paths: + - path: %s + pathType: Exact + backend: + service: + name: httpbin-service-e2e-test + port: + number: 80 +` + + It("preserves external routes while reconciling and deleting an IngressClass", func() { + deployer, ok := s.Deployer.(*scaffold.APISIXDeployer) + Expect(ok).To(BeTrue()) + admin := deployer.AdminAPIClient() + const externalID = "external-route" + admin.PUT("/apisix/admin/services/external-service"). + WithHeader("X-API-KEY", s.AdminKey()). + WithJSON(map[string]any{ + "upstream": map[string]any{"type": "roundrobin", "nodes": map[string]int{"127.0.0.1:1980": 1}}, + }).Expect().Status(http.StatusCreated) + admin.PUT("/apisix/admin/routes/"+externalID). + WithHeader("X-API-KEY", s.AdminKey()). + WithJSON(map[string]any{ + "uri": "/external", + "service_id": "external-service", + "plugins": map[string]any{"redirect": map[string]any{"uri": "/preserved"}}, + }).Expect().Status(http.StatusCreated) + + proxy := fmt.Sprintf(gatewayProxyYaml, s.Deployer.GetAdminEndpoint(), s.AdminKey()) + class := fmt.Sprintf(ingressClassYaml, s.Namespace(), s.GetControllerName(), s.Namespace()) + ingress := fmt.Sprintf(ingressYaml, s.Namespace(), "/get") + Expect(s.CreateResourceFromString(proxy)).To(Succeed()) + Expect(s.CreateResourceFromStringWithNamespace(class, "")).To(Succeed()) + Expect(s.CreateResourceFromString(ingress)).To(Succeed()) + s.RequestAssert(&scaffold.RequestAssert{ + Method: "GET", Path: "/get", Host: "managed-resources.example", + Check: scaffold.WithExpectedStatus(http.StatusOK), + }) + assertExternal := func() { + admin.GET("/apisix/admin/routes/"+externalID). + WithHeader("X-API-KEY", s.AdminKey()).Expect().Status(http.StatusOK). + JSON().Object().Value("value").Object().Value("uri").IsEqual("/external") + } + assertExternal() + + By("update the managed route without deleting the external route") + Expect(s.CreateResourceFromString(fmt.Sprintf(ingressYaml, s.Namespace(), "/anything"))).To(Succeed()) + s.RequestAssert(&scaffold.RequestAssert{ + Method: "GET", Path: "/anything", Host: "managed-resources.example", + Check: scaffold.WithExpectedStatus(http.StatusOK), + }) + assertExternal() + + By("delete the IngressClass and wait for managed routes to disappear") + Expect(s.DeleteResourceFromStringWithNamespace(class, "")).To(Succeed()) + s.RequestAssert(&scaffold.RequestAssert{ + Method: "GET", Path: "/anything", Host: "managed-resources.example", + Check: scaffold.WithExpectedStatus(http.StatusNotFound), + }) + assertExternal() + }) +}) From aecacac8cb399ac638e74e98cdd94fab01117102 Mon Sep 17 00:00:00 2001 From: AlinsRan Date: Mon, 19 Oct 2026 13:24:08 +0800 Subject: [PATCH 2/6] fix: preserve pending retries and await plugin config convergence --- internal/provider/common/retrier.go | 21 +++++-- internal/provider/common/retrier_test.go | 75 ++++++++++++++++++++++++ test/e2e/crds/v2/pluginconfig.go | 17 +++--- 3 files changed, 101 insertions(+), 12 deletions(-) create mode 100644 internal/provider/common/retrier_test.go diff --git a/internal/provider/common/retrier.go b/internal/provider/common/retrier.go index 1277ee93a..bda03cc29 100644 --- a/internal/provider/common/retrier.go +++ b/internal/provider/common/retrier.go @@ -70,6 +70,10 @@ func (r *Retrier) Reset() { r.timer.Stop() r.timer = nil } + select { + case <-r.ch: + default: + } r.backoff.Reset() } @@ -77,18 +81,27 @@ func (r *Retrier) Next() { r.mu.Lock() defer r.mu.Unlock() - if r.timer != nil { - r.timer.Stop() - r.timer = nil + // Event-triggered failures must not postpone an already scheduled retry. + if r.timer != nil || len(r.ch) > 0 { + return } delay := r.backoff.Next() - r.timer = time.AfterFunc(delay, func() { + var timer *time.Timer + timer = time.AfterFunc(delay, func() { + r.mu.Lock() + defer r.mu.Unlock() + // Reset may have canceled this callback while it waited for the lock. + if r.timer != timer { + return + } + r.timer = nil select { case r.ch <- struct{}{}: default: } }) + r.timer = timer } func (r *Retrier) C() <-chan struct{} { diff --git a/internal/provider/common/retrier_test.go b/internal/provider/common/retrier_test.go new file mode 100644 index 000000000..11fba8db3 --- /dev/null +++ b/internal/provider/common/retrier_test.go @@ -0,0 +1,75 @@ +// 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 ( + "testing" + "testing/synctest" + "time" +) + +func TestRetrierPreservesPendingDeadline(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + r := NewRetrier(NewExponentialBackoff(10*time.Second, time.Minute)) + defer r.Reset() + r.Next() + for range 9 { + time.Sleep(time.Second) + r.Next() + } + time.Sleep(time.Second) + synctest.Wait() + select { + case <-r.C(): + default: + t.Fatal("event-triggered failures postponed the pending retry") + } + r.Next() + time.Sleep(20 * time.Second) + synctest.Wait() + select { + case <-r.C(): + default: + t.Fatal("the next retry must advance backoff only once") + } + }) +} + +func TestRetrierResetClearsPendingRetry(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + r := NewRetrier(NewExponentialBackoff(time.Second, time.Minute)) + defer r.Reset() + r.Next() + time.Sleep(time.Second) + synctest.Wait() + r.Reset() + select { + case <-r.C(): + t.Fatal("reset left a stale retry notification") + default: + } + r.Next() + time.Sleep(time.Second) + synctest.Wait() + select { + case <-r.C(): + default: + t.Fatal("reset did not restore the base delay") + } + }) +} diff --git a/test/e2e/crds/v2/pluginconfig.go b/test/e2e/crds/v2/pluginconfig.go index 8112a1efc..6d917b647 100644 --- a/test/e2e/crds/v2/pluginconfig.go +++ b/test/e2e/crds/v2/pluginconfig.go @@ -261,14 +261,15 @@ spec: &apisixRoute, fmt.Sprintf(apisixRouteSpec, s.Namespace())) By("verify initial plugin config works") - request := func() int { - return s.NewAPISIXClient().GET("/get").Expect().Raw().StatusCode - } - Eventually(request).WithTimeout(30 * time.Second).ProbeEvery(1 * time.Second).Should(Equal(http.StatusOK)) - - resp := s.NewAPISIXClient().GET("/get").Expect().Status(http.StatusOK) - resp.Header("X-Version").IsEqual("v1") - resp.Header("X-Updated").IsEmpty() + s.RequestAssert(&scaffold.RequestAssert{ + Method: "GET", + Path: "/get", + Checks: []scaffold.ResponseCheckFunc{ + scaffold.WithExpectedStatus(http.StatusOK), + scaffold.WithExpectedHeader("X-Version", "v1"), + scaffold.WithExpectedHeader("X-Updated", ""), + }, + }) By("update ApisixPluginConfig") applier.MustApplyAPIv2(types.NamespacedName{Namespace: s.Namespace(), Name: "test-plugin-config-update"}, From cb9ff3c6a1ada4edd00bae82c6d883f6cfc71ce6 Mon Sep 17 00:00:00 2001 From: AlinsRan Date: Mon, 19 Oct 2026 15:46:15 +0800 Subject: [PATCH 3/6] fix: scope every non-standalone backend to controller-managed resources Only the apisix and api7ee backends were scoped, so any other backend type would still sync without the ownership selector. Scope everything except apisix-standalone, and document that GatewayProxies sharing one Admin API endpoint or gateway group are not isolated from each other. --- docs/en/latest/FAQ.md | 5 ++++- internal/adc/client/client.go | 5 +++-- internal/adc/client/ownership_test.go | 2 +- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/docs/en/latest/FAQ.md b/docs/en/latest/FAQ.md index 4b02f0283..5c20004a5 100644 --- a/docs/en/latest/FAQ.md +++ b/docs/en/latest/FAQ.md @@ -67,7 +67,10 @@ ADC filters top-level resources. Routes nested under a selected Service are included with that Service, so externally managed routes should not share a controller-managed Service. Global rules and plugin metadata do not support this label-based isolation and remain subject to full reconciliation. The label also -does not distinguish separate controller installations sharing one APISIX instance or API7 gateway group. +does not distinguish separate controller installations sharing one APISIX instance or API7 gateway group, +or separate GatewayProxy resources that point to the same Admin API endpoint or gateway group: each +GatewayProxy is synchronized on its own and can delete resources the other one created. +Give each GatewayProxy its own APISIX instance or gateway group. This behavior does not apply to `apisix-standalone`, which manages a complete configuration. diff --git a/internal/adc/client/client.go b/internal/adc/client/client.go index f67896c0d..965b19b8e 100644 --- a/internal/adc/client/client.go +++ b/internal/adc/client/client.go @@ -392,8 +392,9 @@ func (c *Client) push(ctx context.Context, config adctypes.Config, args []string func buildSyncArgs(filePath string, task Task, backend string) []string { labels := task.Labels - if backend == "apisix" || backend == "api7ee" { - // Preserve per-resource constraints while scoping full sync and gateway cleanup. + if backend != backendAPISIXStandalone { + // Preserve per-resource constraints while scoping full sync and gateway cleanup; + // only standalone, which replaces the whole configuration, stays unscoped. // ADC does not apply label selectors to global rules or plugin metadata. labels = maps.Clone(labels) if labels == nil { diff --git a/internal/adc/client/ownership_test.go b/internal/adc/client/ownership_test.go index 922a0d4c5..60b15103c 100644 --- a/internal/adc/client/ownership_test.go +++ b/internal/adc/client/ownership_test.go @@ -35,7 +35,7 @@ import ( ) func TestSyncOwnershipSelector(t *testing.T) { - for _, backend := range []string{"apisix", "api7ee", "apisix-standalone"} { + for _, backend := range []string{"apisix", "api7ee", "custom", "apisix-standalone"} { for _, explicit := range []bool{false, true} { for _, operation := range []string{"full", "resource", "cleanup", "global-rule"} { name := backend + "/default/" + operation From 1b3db420606246b96c4e3a538db071c7cb9f9ca5 Mon Sep 17 00:00:00 2001 From: AlinsRan Date: Mon, 19 Oct 2026 17:34:57 +0800 Subject: [PATCH 4/6] test: check retry timing and external Service preservation Fail the backoff test if the second retry fires before its delay, and check that the externally managed Service survives synchronization along with its Route. --- internal/provider/common/retrier_test.go | 9 ++++++++- test/e2e/apisix/managed_resources.go | 2 ++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/internal/provider/common/retrier_test.go b/internal/provider/common/retrier_test.go index 11fba8db3..1cdd0143d 100644 --- a/internal/provider/common/retrier_test.go +++ b/internal/provider/common/retrier_test.go @@ -40,7 +40,14 @@ func TestRetrierPreservesPendingDeadline(t *testing.T) { t.Fatal("event-triggered failures postponed the pending retry") } r.Next() - time.Sleep(20 * time.Second) + time.Sleep(20*time.Second - time.Millisecond) + synctest.Wait() + select { + case <-r.C(): + t.Fatal("the next retry fired before its backoff delay") + default: + } + time.Sleep(time.Millisecond) synctest.Wait() select { case <-r.C(): diff --git a/test/e2e/apisix/managed_resources.go b/test/e2e/apisix/managed_resources.go index 912fb8dd8..7dbc59c1d 100644 --- a/test/e2e/apisix/managed_resources.go +++ b/test/e2e/apisix/managed_resources.go @@ -114,6 +114,8 @@ spec: Check: scaffold.WithExpectedStatus(http.StatusOK), }) assertExternal := func() { + admin.GET("/apisix/admin/services/external-service"). + WithHeader("X-API-KEY", s.AdminKey()).Expect().Status(http.StatusOK) admin.GET("/apisix/admin/routes/"+externalID). WithHeader("X-API-KEY", s.AdminKey()).Expect().Status(http.StatusOK). JSON().Object().Value("value").Object().Value("uri").IsEqual("/external") From 19e56cbe5c5f8d90d176b8a10cbf5563ae1e1f97 Mon Sep 17 00:00:00 2001 From: AlinsRan Date: Mon, 19 Oct 2026 21:01:33 +0800 Subject: [PATCH 5/6] test: retry every PluginConfig response check Each PluginConfig case waited for one successful request and then asserted headers on a separate request that was not retried. On APISIX standalone that second request can reach a worker still serving the previous configuration and get a 404. Check the status and headers together in one retried request, as the update case already does. --- test/e2e/crds/v2/pluginconfig.go | 97 +++++++++++++++++--------------- 1 file changed, 51 insertions(+), 46 deletions(-) diff --git a/test/e2e/crds/v2/pluginconfig.go b/test/e2e/crds/v2/pluginconfig.go index 6d917b647..592a659fa 100644 --- a/test/e2e/crds/v2/pluginconfig.go +++ b/test/e2e/crds/v2/pluginconfig.go @@ -94,15 +94,15 @@ spec: &apisixRoute, fmt.Sprintf(apisixRouteSpec, s.Namespace())) By("verify ApisixRoute works with plugin config") - request := func() int { - return s.NewAPISIXClient().GET("/get").Expect().Raw().StatusCode - } - Eventually(request).WithTimeout(30 * time.Second).ProbeEvery(1 * time.Second).Should(Equal(http.StatusOK)) - - By("verify plugin from ApisixPluginConfig works") - resp := s.NewAPISIXClient().GET("/get").Expect().Status(http.StatusOK) - resp.Header("X-Plugin-Config").IsEqual("test-response-rewrite") - resp.Header("X-Plugin-Test").IsEqual("enabled") + s.RequestAssert(&scaffold.RequestAssert{ + Method: "GET", + Path: "/get", + Checks: []scaffold.ResponseCheckFunc{ + scaffold.WithExpectedStatus(http.StatusOK), + scaffold.WithExpectedHeader("X-Plugin-Config", "test-response-rewrite"), + scaffold.WithExpectedHeader("X-Plugin-Test", "enabled"), + }, + }) By("delete ApisixRoute") err := s.DeleteResource("ApisixRoute", "test-route") @@ -112,7 +112,11 @@ spec: err = s.DeleteResource("ApisixPluginConfig", "test-plugin-config") Expect(err).ShouldNot(HaveOccurred(), "deleting ApisixPluginConfig") - Eventually(request).WithTimeout(30 * time.Second).ProbeEvery(1 * time.Second).Should(Equal(http.StatusNotFound)) + s.RequestAssert(&scaffold.RequestAssert{ + Method: "GET", + Path: "/get", + Check: scaffold.WithExpectedStatus(http.StatusNotFound), + }) }) It("Test ApisixRoute stops serving when its ApisixPluginConfig is deleted", func() { @@ -343,17 +347,16 @@ spec: &apisixRoute, fmt.Sprintf(apisixRouteSpec, s.Namespace())) By("verify ApisixRoute works") - request := func() int { - return s.NewAPISIXClient().GET("/get").Expect().Raw().StatusCode - } - Eventually(request).WithTimeout(60 * time.Second).ProbeEvery(2 * time.Second).Should(Equal(http.StatusOK)) - - By("verify disabled plugin is not applied") - resp := s.NewAPISIXClient().GET("/get").Expect().Status(http.StatusOK) - resp.Header("X-Should-Not-Exist").IsEmpty() - - By("verify enabled plugin is applied") - resp.Header("Access-Control-Allow-Origin").IsEqual("*") + s.RequestAssert(&scaffold.RequestAssert{ + Method: "GET", + Path: "/get", + Timeout: 60 * time.Second, + Checks: []scaffold.ResponseCheckFunc{ + scaffold.WithExpectedStatus(http.StatusOK), + scaffold.WithExpectedNotHeader("X-Should-Not-Exist"), + scaffold.WithExpectedHeader("Access-Control-Allow-Origin", "*"), + }, + }) By("delete resources") err := s.DeleteResource("ApisixRoute", "test-route-disabled") @@ -415,16 +418,16 @@ spec: &apisixRoute, fmt.Sprintf(apisixRouteSpec, s.Namespace())) By("verify ApisixRoute works") - request := func() int { - return s.NewAPISIXClient().GET("/get").Expect().Raw().StatusCode - } - Eventually(request).WithTimeout(30 * time.Second).ProbeEvery(1 * time.Second).Should(Equal(http.StatusOK)) - - By("verify route plugins override plugin config") - resp := s.NewAPISIXClient().GET("/get").Expect().Status(http.StatusOK) - resp.Header("X-From-Config").IsEmpty() - resp.Header("X-From-Route").IsEqual("route") - resp.Header("X-Shared").IsEqual("from-route") + s.RequestAssert(&scaffold.RequestAssert{ + Method: "GET", + Path: "/get", + Checks: []scaffold.ResponseCheckFunc{ + scaffold.WithExpectedStatus(http.StatusOK), + scaffold.WithExpectedNotHeader("X-From-Config"), + scaffold.WithExpectedHeader("X-From-Route", "route"), + scaffold.WithExpectedHeader("X-Shared", "from-route"), + }, + }) By("delete resources") err := s.DeleteResource("ApisixRoute", "test-route-override") @@ -480,14 +483,15 @@ spec: &apisixRoute, fmt.Sprintf(apisixRouteSpec, s.Namespace())) By("verify cross-namespace reference works") - request := func() int { - return s.NewAPISIXClient().GET("/get").Expect().Raw().StatusCode - } - Eventually(request).WithTimeout(30 * time.Second).ProbeEvery(1 * time.Second).Should(Equal(http.StatusOK)) - - resp := s.NewAPISIXClient().GET("/get").Expect().Status(http.StatusOK) - resp.Header("X-Cross-Namespace").IsEqual("true") - resp.Header("X-Namespace").IsEqual("default") + s.RequestAssert(&scaffold.RequestAssert{ + Method: "GET", + Path: "/get", + Checks: []scaffold.ResponseCheckFunc{ + scaffold.WithExpectedStatus(http.StatusOK), + scaffold.WithExpectedHeader("X-Cross-Namespace", "true"), + scaffold.WithExpectedHeader("X-Namespace", "default"), + }, + }) By("delete resources") err = s.DeleteResource("ApisixRoute", "test-route-cross-ns") @@ -558,13 +562,14 @@ spec: &apisixRoute, fmt.Sprintf(apisixRouteSpec, s.Namespace())) By("verify ApisixRoute works with SecretRef") - request := func() int { - return s.NewAPISIXClient().GET("/get").Expect().Raw().StatusCode - } - Eventually(request).WithTimeout(30 * time.Second).ProbeEvery(1 * time.Second).Should(Equal(http.StatusOK)) - - resp := s.NewAPISIXClient().GET("/get").Expect().Status(http.StatusOK) - resp.Header("X-Secret-Ref").IsEqual("true") + s.RequestAssert(&scaffold.RequestAssert{ + Method: "GET", + Path: "/get", + Checks: []scaffold.ResponseCheckFunc{ + scaffold.WithExpectedStatus(http.StatusOK), + scaffold.WithExpectedHeader("X-Secret-Ref", "true"), + }, + }) By("delete resources") err = s.DeleteResource("ApisixRoute", "test-route-secret") From d1a9c686f994102bf4f841b27b9466095c8c0318 Mon Sep 17 00:00:00 2001 From: AlinsRan Date: Mon, 19 Oct 2026 22:31:25 +0800 Subject: [PATCH 6/6] test: retry the remaining response checks that follow a wait Two ApisixRoute secretRef cases checked the body on a separate request after waiting for one success, and the ApisixUpstream health check sent 100 unretried requests right after the upstream appeared. On APISIX standalone a worker still serving the previous configuration answers 404. Check status, headers and body in one retried request, and retry the health check round until all 100 requests succeed. --- test/e2e/crds/v2/route.go | 64 +++++++++++++++--------------------- test/e2e/crds/v2/upstream.go | 10 ++++-- 2 files changed, 34 insertions(+), 40 deletions(-) diff --git a/test/e2e/crds/v2/route.go b/test/e2e/crds/v2/route.go index 2d98d36c4..9bfe9b857 100644 --- a/test/e2e/crds/v2/route.go +++ b/test/e2e/crds/v2/route.go @@ -725,26 +725,22 @@ spec: &apisixRoute, fmt.Sprintf(apisixRouteSpec, s.Namespace(), s.Namespace())) By("verify ApisixRoute with secretRef works") - Eventually(func() *http.Response { - return s.NewAPISIXClient().GET("/ip"). - WithHeader("Host", "httpbin.org"). - Expect(). - Raw() - }).WithTimeout(20 * time.Second).ProbeEvery(time.Second).Should(And( - HaveHTTPStatus(http.StatusOK), - HaveHTTPHeaderWithValue("X-Foo", "v1"), - HaveHTTPHeaderWithValue("X-Foo2", "v2"), - )) - - // Verify the response body contains the secret values - resp := s.NewAPISIXClient().GET("/ip"). - WithHeader("Host", "httpbin.org"). - Expect(). - Status(http.StatusOK) - - resp.Body().Contains("This is the replaced preface") // From secret - resp.Body().Contains("This is the epilogue") // From config - resp.Body().Contains("my custom body") // From secret + s.RequestAssert(&scaffold.RequestAssert{ + Method: "GET", + Path: "/ip", + Host: "httpbin.org", + Timeout: 20 * time.Second, + Checks: []scaffold.ResponseCheckFunc{ + scaffold.WithExpectedStatus(http.StatusOK), + scaffold.WithExpectedHeader("X-Foo", "v1"), + scaffold.WithExpectedHeader("X-Foo2", "v2"), + scaffold.WithExpectedBodyContains( + "This is the replaced preface", // From secret + "This is the epilogue", // From config + "my custom body", // From secret + ), + }, + }) }) It("Test ApisixRoute with secretRef - nested keys", func() { @@ -798,23 +794,17 @@ spec: &apisixRoute, fmt.Sprintf(apisixRouteSpec, s.Namespace(), s.Namespace())) By("verify ApisixRoute with secretRef (nested keys) works") - Eventually(func() *http.Response { - return s.NewAPISIXClient().GET("/ip"). - WithHeader("Host", "httpbin.org"). - Expect(). - Raw() - }).WithTimeout(20 * time.Second).ProbeEvery(time.Second).Should(And( - HaveHTTPStatus(http.StatusOK), - HaveHTTPHeaderWithValue("X-Foo", "v2"), // From secret, overriding config value "v1" - )) - - // Verify the response body contains the secret values - resp := s.NewAPISIXClient().GET("/ip"). - WithHeader("Host", "httpbin.org"). - Expect(). - Status(http.StatusOK) - - resp.Body().Contains("my custom body") // From secret + s.RequestAssert(&scaffold.RequestAssert{ + Method: "GET", + Path: "/ip", + Host: "httpbin.org", + Timeout: 20 * time.Second, + Checks: []scaffold.ResponseCheckFunc{ + scaffold.WithExpectedStatus(http.StatusOK), + scaffold.WithExpectedHeader("X-Foo", "v2"), // From secret, overriding config value "v1" + scaffold.WithExpectedBodyContains("my custom body"), // From secret + }, + }) }) }) diff --git a/test/e2e/crds/v2/upstream.go b/test/e2e/crds/v2/upstream.go index 7d9d400ea..0409fa0a7 100644 --- a/test/e2e/crds/v2/upstream.go +++ b/test/e2e/crds/v2/upstream.go @@ -131,9 +131,13 @@ spec: g.Expect(ups[0].Checks.Passive.Unhealthy.HTTPStatuses).To(Equal([]int{502}), "the passive unhealthy http status") }).WithTimeout(scaffold.DefaultTimeout).ProbeEvery(scaffold.DefaultInterval).Should(Succeed()) - for range 100 { - s.NewAPISIXClient().GET("/ip").WithHost("httpbin.org").Expect().Status(200) - } + // Workers pick up the route at different times; retry until a full round succeeds. + Eventually(func(g Gomega) { + for range 100 { + g.Expect(s.NewAPISIXClient().GET("/ip").WithHost("httpbin.org").Expect().Raw().StatusCode). + To(Equal(200)) + } + }).WithTimeout(scaffold.DefaultTimeout).ProbeEvery(scaffold.DefaultInterval).Should(Succeed()) }) })