diff --git a/docs/en/latest/FAQ.md b/docs/en/latest/FAQ.md index 4ad421cdb..5c20004a5 100644 --- a/docs/en/latest/FAQ.md +++ b/docs/en/latest/FAQ.md @@ -54,3 +54,28 @@ 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, +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. + +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..965b19b8e 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,21 @@ 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 != 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 { + 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 +453,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 +463,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 +503,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 +514,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..60b15103c --- /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", "custom", "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/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..1cdd0143d --- /dev/null +++ b/internal/provider/common/retrier_test.go @@ -0,0 +1,82 @@ +// 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 - 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(): + 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/apisix/managed_resources.go b/test/e2e/apisix/managed_resources.go new file mode 100644 index 000000000..7dbc59c1d --- /dev/null +++ b/test/e2e/apisix/managed_resources.go @@ -0,0 +1,141 @@ +// 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/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") + } + 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() + }) +}) diff --git a/test/e2e/crds/v2/pluginconfig.go b/test/e2e/crds/v2/pluginconfig.go index 8112a1efc..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() { @@ -261,14 +265,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"}, @@ -342,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") @@ -414,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") @@ -479,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") @@ -557,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") 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()) }) })