From d67d1f4bba32535095738cb0a535c1469d92a500 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=82=9F=E7=A9=BA?= Date: Thu, 19 Jun 2025 11:27:46 +0800 Subject: [PATCH 1/8] feat: enhance ApisixUpstream with health checks, TLS, and CRD validations --- api/v2/apisixupstream_types.go | 41 +++-- api/v2/shared_types.go | 16 ++ .../apisix.apache.org_apisixupstreams.yaml | 67 ++++++++ .../provider/adc/translator/apisixupstream.go | 147 +++++++++++++++--- 4 files changed, 238 insertions(+), 33 deletions(-) diff --git a/api/v2/apisixupstream_types.go b/api/v2/apisixupstream_types.go index 27c80c70c..e88051a71 100644 --- a/api/v2/apisixupstream_types.go +++ b/api/v2/apisixupstream_types.go @@ -19,6 +19,7 @@ import ( ) // ApisixUpstreamSpec describes the specification of ApisixUpstream. +// +kubebuilder:validation:XValidation:rule="has(self.externalNodes)!=has(discovery)" type ApisixUpstreamSpec struct { // IngressClassName is the name of an IngressClass cluster resource. // controller implementations use this field to know whether they should be @@ -29,6 +30,7 @@ type ApisixUpstreamSpec struct { // ExternalNodes contains external nodes the Upstream should use // If this field is set, the upstream will use these nodes directly without any further resolves // +kubebuilder:validation:Optional + // +kubebuilder:validation:MinItems=1 ExternalNodes []ApisixUpstreamExternalNode `json:"externalNodes,omitempty" yaml:"externalNodes,omitempty"` ApisixUpstreamConfig `json:",inline" yaml:",inline"` @@ -76,6 +78,7 @@ type ApisixUpstreamConfig struct { // The scheme used to talk with the upstream. // Now value can be http, grpc. // +kubebuilder:validation:Optional + // +kubebuilder:validation:Enum=http;https;grpc;grpcs; Scheme string `json:"scheme,omitempty" yaml:"scheme,omitempty"` // How many times that the proxy (Apache APISIX) should do when @@ -103,6 +106,7 @@ type ApisixUpstreamConfig struct { // Configures the host when the request is forwarded to the upstream. // Can be one of pass, node or rewrite. // +kubebuilder:validation:Optional + // +kubebuilder:validation:Enum=pass;node;rewrite; PassHost string `json:"passHost,omitempty" yaml:"passHost,omitempty"` // Specifies the host of the Upstream request. This is only valid if @@ -140,7 +144,9 @@ type LoadBalancer struct { // HealthCheck describes the upstream health check parameters. type HealthCheck struct { - Active *ActiveHealthCheck `json:"active" yaml:"active"` + // +kubebuilder:validation:Required + Active *ActiveHealthCheck `json:"active" yaml:"active"` + // +kubebuilder:validation:Optional Passive *PassiveHealthCheck `json:"passive,omitempty" yaml:"passive,omitempty"` } @@ -161,10 +167,15 @@ type Discovery struct { // ActiveHealthCheck defines the active kind of upstream health check. type ActiveHealthCheck struct { - Type string `json:"type,omitempty" yaml:"type,omitempty"` - Timeout time.Duration `json:"timeout,omitempty" yaml:"timeout,omitempty"` - Concurrency int `json:"concurrency,omitempty" yaml:"concurrency,omitempty"` - Host string `json:"host,omitempty" yaml:"host,omitempty"` + // +kubebuilder:validation:Optional + // +kubebuilder:validation:Enum=http;https;tcp; + Type string `json:"type,omitempty" yaml:"type,omitempty"` + Timeout time.Duration `json:"timeout,omitempty" yaml:"timeout,omitempty"` + // +kubebuilder:validation:Minimum=0 + Concurrency int `json:"concurrency,omitempty" yaml:"concurrency,omitempty"` + Host string `json:"host,omitempty" yaml:"host,omitempty"` + // +kubebuilder:validation:Minimum=0 + // +kubebuilder:validation:Maximum=65535 Port int32 `json:"port,omitempty" yaml:"port,omitempty"` HTTPPath string `json:"httpPath,omitempty" yaml:"httpPath,omitempty"` StrictTLS *bool `json:"strictTLS,omitempty" yaml:"strictTLS,omitempty"` @@ -200,17 +211,27 @@ type ActiveHealthCheckUnhealthy struct { // PassiveHealthCheckHealthy defines the conditions to judge whether // an upstream node is healthy with the passive manner. type PassiveHealthCheckHealthy struct { + // +kubebuilder:validation:Optional + // +kubebuilder:validation:MinItems=1 HTTPCodes []int `json:"httpCodes,omitempty" yaml:"httpCodes,omitempty"` - Successes int `json:"successes,omitempty" yaml:"successes,omitempty"` + // +kubebuilder:validation:Minimum=0 + // +kubebuilder:validation:Maximum=254 + Successes int `json:"successes,omitempty" yaml:"successes,omitempty"` } // PassiveHealthCheckUnhealthy defines the conditions to judge whether // an upstream node is unhealthy with the passive manager. type PassiveHealthCheckUnhealthy struct { - HTTPCodes []int `json:"httpCodes,omitempty" yaml:"httpCodes,omitempty"` - HTTPFailures int `json:"httpFailures,omitempty" yaml:"http_failures,omitempty"` - TCPFailures int `json:"tcpFailures,omitempty" yaml:"tcpFailures,omitempty"` - Timeouts int `json:"timeout,omitempty" yaml:"timeout,omitempty"` + // +kubebuilder:validation:Optional + // +kubebuilder:validation:MinItems=1 + HTTPCodes []int `json:"httpCodes,omitempty" yaml:"httpCodes,omitempty"` + // +kubebuilder:validation:Minimum=0 + // +kubebuilder:validation:Maximum=254 + HTTPFailures int `json:"httpFailures,omitempty" yaml:"http_failures,omitempty"` + // +kubebuilder:validation:Minimum=0 + // +kubebuilder:validation:Maximum=254 + TCPFailures int `json:"tcpFailures,omitempty" yaml:"tcpFailures,omitempty"` + Timeouts int `json:"timeout,omitempty" yaml:"timeout,omitempty"` } func init() { diff --git a/api/v2/shared_types.go b/api/v2/shared_types.go index cda28a2f5..e654606fb 100644 --- a/api/v2/shared_types.go +++ b/api/v2/shared_types.go @@ -136,6 +136,22 @@ const ( ExternalTypeService ApisixUpstreamExternalType = "Service" ) +const ( + // HealthCheckHTTP represents the HTTP kind health check. + HealthCheckHTTP = "http" + // HealthCheckHTTPS represents the HTTPS kind health check. + HealthCheckHTTPS = "https" + // HealthCheckTCP represents the TCP kind health check. + HealthCheckTCP = "tcp" + + // HealthCheckMaxConsecutiveNumber is the max number for + // the consecutive success/failure in upstream health check. + HealthCheckMaxConsecutiveNumber = 254 + // ActiveHealthCheckMinInterval is the minimum interval for + // the active health check. + ActiveHealthCheckMinInterval = time.Second +) + var schemeToPortMaps = map[string]int{ SchemeHTTP: 80, SchemeHTTPS: 443, diff --git a/config/crd/bases/apisix.apache.org_apisixupstreams.yaml b/config/crd/bases/apisix.apache.org_apisixupstreams.yaml index c4efa840c..8fa9adaf6 100644 --- a/config/crd/bases/apisix.apache.org_apisixupstreams.yaml +++ b/config/crd/bases/apisix.apache.org_apisixupstreams.yaml @@ -74,6 +74,7 @@ spec: weight: type: integer type: object + minItems: 1 type: array healthCheck: description: The health check configurations for the upstream. @@ -83,6 +84,7 @@ spec: health check. properties: concurrency: + minimum: 0 type: integer healthy: description: |- @@ -92,10 +94,13 @@ spec: httpCodes: items: type: integer + minItems: 1 type: array interval: type: string successes: + maximum: 254 + minimum: 0 type: integer type: object host: @@ -104,6 +109,8 @@ spec: type: string port: format: int32 + maximum: 65535 + minimum: 0 type: integer requestHeaders: items: @@ -119,6 +126,10 @@ spec: format: int64 type: integer type: + enum: + - http + - https + - tcp type: string unhealthy: description: |- @@ -128,12 +139,17 @@ spec: httpCodes: items: type: integer + minItems: 1 type: array httpFailures: + maximum: 254 + minimum: 0 type: integer interval: type: string tcpFailures: + maximum: 254 + minimum: 0 type: integer timeout: type: integer @@ -152,8 +168,11 @@ spec: httpCodes: items: type: integer + minItems: 1 type: array successes: + maximum: 254 + minimum: 0 type: integer type: object type: @@ -166,10 +185,15 @@ spec: httpCodes: items: type: integer + minItems: 1 type: array httpFailures: + maximum: 254 + minimum: 0 type: integer tcpFailures: + maximum: 254 + minimum: 0 type: integer timeout: type: integer @@ -207,6 +231,10 @@ spec: description: |- Configures the host when the request is forwarded to the upstream. Can be one of pass, node or rewrite. + enum: + - pass + - node + - rewrite type: string portLevelSettings: items: @@ -239,6 +267,7 @@ spec: upstream health check. properties: concurrency: + minimum: 0 type: integer healthy: description: |- @@ -248,10 +277,13 @@ spec: httpCodes: items: type: integer + minItems: 1 type: array interval: type: string successes: + maximum: 254 + minimum: 0 type: integer type: object host: @@ -260,6 +292,8 @@ spec: type: string port: format: int32 + maximum: 65535 + minimum: 0 type: integer requestHeaders: items: @@ -275,6 +309,10 @@ spec: format: int64 type: integer type: + enum: + - http + - https + - tcp type: string unhealthy: description: |- @@ -284,12 +322,17 @@ spec: httpCodes: items: type: integer + minItems: 1 type: array httpFailures: + maximum: 254 + minimum: 0 type: integer interval: type: string tcpFailures: + maximum: 254 + minimum: 0 type: integer timeout: type: integer @@ -308,8 +351,11 @@ spec: httpCodes: items: type: integer + minItems: 1 type: array successes: + maximum: 254 + minimum: 0 type: integer type: object type: @@ -322,10 +368,15 @@ spec: httpCodes: items: type: integer + minItems: 1 type: array httpFailures: + maximum: 254 + minimum: 0 type: integer tcpFailures: + maximum: 254 + minimum: 0 type: integer timeout: type: integer @@ -356,6 +407,10 @@ spec: description: |- Configures the host when the request is forwarded to the upstream. Can be one of pass, node or rewrite. + enum: + - pass + - node + - rewrite type: string port: description: Port is a Kubernetes Service port, it should be @@ -372,6 +427,11 @@ spec: description: |- The scheme used to talk with the upstream. Now value can be http, grpc. + enum: + - http + - https + - grpc + - grpcs type: string subsets: description: |- @@ -438,6 +498,11 @@ spec: description: |- The scheme used to talk with the upstream. Now value can be http, grpc. + enum: + - http + - https + - grpc + - grpcs type: string subsets: description: |- @@ -490,6 +555,8 @@ spec: the pass_host is set to rewrite type: string type: object + x-kubernetes-validations: + - rule: has(self.externalNodes)!=has(discovery) type: object served: true storage: true diff --git a/internal/provider/adc/translator/apisixupstream.go b/internal/provider/adc/translator/apisixupstream.go index 34b6f24f6..2243b2bc3 100644 --- a/internal/provider/adc/translator/apisixupstream.go +++ b/internal/provider/adc/translator/apisixupstream.go @@ -27,17 +27,13 @@ import ( ) func (t *Translator) translateApisixUpstream(tctx *provider.TranslateContext, au *apiv2.ApisixUpstream) (ups *adc.Upstream, err error) { - if len(au.Spec.ExternalNodes) == 0 && au.Spec.Discovery == nil { - return nil, errors.Errorf("%s has empty externalNodes or discovery configuration", utils.NamespacedName(au)) - } - ups = adc.NewDefaultUpstream() for _, f := range []func(*apiv2.ApisixUpstream, *adc.Upstream) error{ + patchApisixUpstreamBasics, translateApisixUpstreamScheme, translateApisixUpstreamLoadBalancer, translateApisixUpstreamHealthCheck, translateApisixUpstreamRetriesAndTimeout, - translateApisixUpstreamClientTLS, translateApisixUpstreamPassHost, translateApisixUpstreamDiscovery, } { @@ -46,6 +42,7 @@ func (t *Translator) translateApisixUpstream(tctx *provider.TranslateContext, au } } for _, f := range []func(*provider.TranslateContext, *apiv2.ApisixUpstream, *adc.Upstream) error{ + translateApisixUpstreamClientTLS, translateApisixUpstreamExternalNodes, } { if err = f(tctx, au, ups); err != nil { @@ -56,16 +53,19 @@ func (t *Translator) translateApisixUpstream(tctx *provider.TranslateContext, au return } -func translateApisixUpstreamScheme(au *apiv2.ApisixUpstream, ups *adc.Upstream) error { - switch au.Spec.Scheme { - case apiv2.SchemeHTTP, apiv2.SchemeHTTPS, apiv2.SchemeGRPC, apiv2.SchemeGRPCS: - ups.Scheme = au.Spec.Scheme - default: - ups.Scheme = apiv2.SchemeHTTP +func patchApisixUpstreamBasics(au *apiv2.ApisixUpstream, ups *adc.Upstream) error { + ups.Name = composeExternalUpstreamName(au) + for k, v := range au.Labels { + ups.Labels[k] = v } return nil } +func translateApisixUpstreamScheme(au *apiv2.ApisixUpstream, ups *adc.Upstream) error { + ups.Scheme = cmp.Or(au.Spec.Scheme, apiv2.SchemeHTTP) + return nil +} + func translateApisixUpstreamLoadBalancer(au *apiv2.ApisixUpstream, ups *adc.Upstream) error { lb := au.Spec.LoadBalancer if lb == nil || lb.Type == "" { @@ -99,10 +99,82 @@ func translateApisixUpstreamLoadBalancer(au *apiv2.ApisixUpstream, ups *adc.Upst } func translateApisixUpstreamHealthCheck(au *apiv2.ApisixUpstream, ups *adc.Upstream) error { - // todo: handle `.Checks` in next PR + check := au.Spec.HealthCheck + if check == nil { + return nil + } + + ups.Checks = new(adc.UpstreamHealthCheck) + ups.Checks.Active = translateApisixUpstreamHealthCheckActive(check.Active) + + if check.Passive != nil { + ups.Checks.Passive = translateApisixUpstreamHealthCheckPassive(check.Passive) + } + return nil } +func translateApisixUpstreamHealthCheckActive(config *apiv2.ActiveHealthCheck) *adc.UpstreamActiveHealthCheck { + var active adc.UpstreamActiveHealthCheck + + active.Type = cmp.Or(config.Type, apiv2.HealthCheckHTTP) + active.Timeout = int(config.Timeout.Seconds()) + active.Port = config.Port + active.Concurrency = config.Concurrency + active.Host = config.Host + active.HTTPPath = config.HTTPPath + active.HTTPRequestHeaders = config.RequestHeaders + active.HTTPSVerifyCert = config.StrictTLS != nil && *config.StrictTLS + + if config.Healthy != nil { + active.Healthy = adc.UpstreamActiveHealthCheckHealthy{ + UpstreamPassiveHealthCheckHealthy: adc.UpstreamPassiveHealthCheckHealthy{ + HTTPStatuses: config.Healthy.HTTPCodes, + Successes: config.Healthy.Successes, + }, + Interval: int(config.Healthy.Interval.Seconds()), + } + } + + if config.Unhealthy != nil { + active.Unhealthy = adc.UpstreamActiveHealthCheckUnhealthy{ + UpstreamPassiveHealthCheckUnhealthy: adc.UpstreamPassiveHealthCheckUnhealthy{ + HTTPStatuses: config.Unhealthy.HTTPCodes, + HTTPFailures: config.Unhealthy.HTTPFailures, + TCPFailures: config.Unhealthy.TCPFailures, + Timeouts: int(config.Timeout.Seconds()), + }, + Interval: int(config.Unhealthy.Interval.Seconds()), + } + } + + return &active +} + +func translateApisixUpstreamHealthCheckPassive(config *apiv2.PassiveHealthCheck) *adc.UpstreamPassiveHealthCheck { + var passive adc.UpstreamPassiveHealthCheck + + passive.Type = cmp.Or(config.Type, apiv2.HealthCheckHTTP) + + if config.Healthy != nil { + passive.Healthy = adc.UpstreamPassiveHealthCheckHealthy{ + HTTPStatuses: config.Healthy.HTTPCodes, + Successes: config.Healthy.Successes, + } + } + + if config.Unhealthy != nil { + passive.Unhealthy = adc.UpstreamPassiveHealthCheckUnhealthy{ + HTTPStatuses: config.Unhealthy.HTTPCodes, + HTTPFailures: config.Unhealthy.HTTPFailures, + TCPFailures: config.Unhealthy.TCPFailures, + Timeouts: config.Unhealthy.Timeouts, + } + } + + return &passive +} + func translateApisixUpstreamRetriesAndTimeout(au *apiv2.ApisixUpstream, ups *adc.Upstream) error { retries := au.Spec.Retries timeout := au.Spec.Timeout @@ -140,29 +212,58 @@ func translateApisixUpstreamRetriesAndTimeout(au *apiv2.ApisixUpstream, ups *adc return nil } -func translateApisixUpstreamClientTLS(au *apiv2.ApisixUpstream, ups *adc.Upstream) error { - // todo: handle `.TLS` in next PR +func translateApisixUpstreamClientTLS(tctx *provider.TranslateContext, au *apiv2.ApisixUpstream, ups *adc.Upstream) error { + if au.Spec.TLSSecret == nil { + return nil + } + + var ( + secretNN = types.NamespacedName{ + Namespace: au.Spec.TLSSecret.Namespace, + Name: au.Spec.TLSSecret.Name, + } + ) + secret, ok := tctx.Secrets[secretNN] + if !ok { + return nil + } + + cert, key, err := extractKeyPair(secret, true) + if err != nil { + return err + } + + ups.TLS = &adc.ClientTLS{ + Cert: string(cert), + Key: string(key), + } + return nil } func translateApisixUpstreamPassHost(au *apiv2.ApisixUpstream, ups *adc.Upstream) error { - switch passHost := au.Spec.PassHost; passHost { - case apiv2.PassHostPass, apiv2.PassHostNode, apiv2.PassHostRewrite: - ups.PassHost = passHost - default: - ups.PassHost = "" - } - + ups.PassHost = au.Spec.PassHost ups.UpstreamHost = au.Spec.UpstreamHost return nil } -func translateApisixUpstreamDiscovery(upstream *apiv2.ApisixUpstream, upstream2 *adc.Upstream) error { - // todo: handle `.Discovery*` in next PR +func translateApisixUpstreamDiscovery(au *apiv2.ApisixUpstream, ups *adc.Upstream) error { + if au.Spec.Discovery == nil { + return nil + } + + ups.ServiceName = au.Spec.Discovery.ServiceName + ups.DiscoveryType = au.Spec.Discovery.Type + ups.DiscoveryArgs = au.Spec.Discovery.Args + return nil } +func composeExternalUpstreamName(au *apiv2.ApisixUpstream) string { + return au.GetGenerateName() + "_" + au.GetName() +} + func translateApisixUpstreamExternalNodes(tctx *provider.TranslateContext, au *apiv2.ApisixUpstream, ups *adc.Upstream) error { for _, node := range au.Spec.ExternalNodes { switch node.Type { From 1b6ae5cb5b2548496396a834b047465b1fa17fa2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=82=9F=E7=A9=BA?= Date: Thu, 19 Jun 2025 13:32:23 +0800 Subject: [PATCH 2/8] feat: add e2e tests and fix ApisixUpstream validation logic in CRD --- api/v2/apisixupstream_types.go | 2 +- .../apisix.apache.org_apisixupstreams.yaml | 2 +- test/e2e/apisix/upstream.go | 74 +++++++++++++++++++ 3 files changed, 76 insertions(+), 2 deletions(-) create mode 100644 test/e2e/apisix/upstream.go diff --git a/api/v2/apisixupstream_types.go b/api/v2/apisixupstream_types.go index e88051a71..adbd83219 100644 --- a/api/v2/apisixupstream_types.go +++ b/api/v2/apisixupstream_types.go @@ -19,7 +19,7 @@ import ( ) // ApisixUpstreamSpec describes the specification of ApisixUpstream. -// +kubebuilder:validation:XValidation:rule="has(self.externalNodes)!=has(discovery)" +// +kubebuilder:validation:XValidation:rule="has(self.externalNodes)!=has(self.discovery)" type ApisixUpstreamSpec struct { // IngressClassName is the name of an IngressClass cluster resource. // controller implementations use this field to know whether they should be diff --git a/config/crd/bases/apisix.apache.org_apisixupstreams.yaml b/config/crd/bases/apisix.apache.org_apisixupstreams.yaml index 8fa9adaf6..a6d59b72f 100644 --- a/config/crd/bases/apisix.apache.org_apisixupstreams.yaml +++ b/config/crd/bases/apisix.apache.org_apisixupstreams.yaml @@ -556,7 +556,7 @@ spec: type: string type: object x-kubernetes-validations: - - rule: has(self.externalNodes)!=has(discovery) + - rule: has(self.externalNodes)!=has(self.discovery) type: object served: true storage: true diff --git a/test/e2e/apisix/upstream.go b/test/e2e/apisix/upstream.go new file mode 100644 index 000000000..6e01b8300 --- /dev/null +++ b/test/e2e/apisix/upstream.go @@ -0,0 +1,74 @@ +// Licensed 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 ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/apache/apisix-ingress-controller/test/e2e/scaffold" +) + +var _ = Describe("Test ApisixUpstream", func() { + var ( + s = scaffold.NewScaffold(&scaffold.Options{ + ControllerName: "apisix.apache.org/apisix-ingress-controller", + }) + err error + ) + + Context("Test ApisixUpstream validation", func() { + It("validation of externalNodes and discovery", func() { + const apisixUpstreamSpec0 = ` +apiVersion: apisix.apache.org/v2 +kind: ApisixUpstream +metadata: + name: default-upstream +spec: + ingressClassName: apisix + externalNodes: + - type: Service + name: httpbin-service-e2e-test + discovery: + serviceName: xx + type: nacos +` + const apisixUpstreamSpec1 = ` +apiVersion: apisix.apache.org/v2 +kind: ApisixUpstream +metadata: + name: default-upstream +spec: + ingressClassName: apisix +` + err = s.CreateResourceFromString(apisixUpstreamSpec0) + Expect(err).Should(HaveOccurred()) + Expect(err.Error()).Should(ContainSubstring("failed rule: has(self.externalNodes)!=has(self.discovery)")) + + err = s.CreateResourceFromString(apisixUpstreamSpec1) + Expect(err).Should(HaveOccurred()) + Expect(err.Error()).Should(ContainSubstring("failed rule: has(self.externalNodes)!=has(self.discovery)")) + + }) + + It("", func() {}) + + It("", func() {}) + + It("", func() {}) + + It("", func() {}) + + It("", func() {}) + }) +}) From 12f72c0cffeb19e6bdac24860c9aba8fdae2ee1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=82=9F=E7=A9=BA?= Date: Thu, 19 Jun 2025 16:02:50 +0800 Subject: [PATCH 3/8] feat: add subset support for ApisixUpstream and filter endpoints by subset labels --- api/v2/apisixupstream_types.go | 7 +- .../apisix.apache.org_apisixupstreams.yaml | 6 ++ internal/controller/apisixroute_controller.go | 77 +++++++++++++++++++ internal/controller/utils.go | 1 + internal/provider/provider.go | 4 +- internal/utils/k8s.go | 26 +++++++ test/e2e/apisix/pluginconfig.go | 2 +- 7 files changed, 118 insertions(+), 5 deletions(-) diff --git a/api/v2/apisixupstream_types.go b/api/v2/apisixupstream_types.go index 2c9aa057c..342bcfd9a 100644 --- a/api/v2/apisixupstream_types.go +++ b/api/v2/apisixupstream_types.go @@ -165,9 +165,10 @@ type ApisixUpstreamSubset struct { // Discovery defines Service discovery related configuration. type Discovery struct { - ServiceName string `json:"serviceName" yaml:"serviceName"` - Type string `json:"type" yaml:"type"` - Args map[string]string `json:"args,omitempty" yaml:"args,omitempty"` + ServiceName string `json:"serviceName" yaml:"serviceName"` + // +kubebuilder:validation:Enum=kubernetes;nacos; + Type string `json:"type" yaml:"type"` + Args map[string]string `json:"args,omitempty" yaml:"args,omitempty"` } // ActiveHealthCheck defines the active kind of upstream health check. diff --git a/config/crd/bases/apisix.apache.org_apisixupstreams.yaml b/config/crd/bases/apisix.apache.org_apisixupstreams.yaml index 524c8eba4..1c2218279 100644 --- a/config/crd/bases/apisix.apache.org_apisixupstreams.yaml +++ b/config/crd/bases/apisix.apache.org_apisixupstreams.yaml @@ -50,6 +50,9 @@ spec: serviceName: type: string type: + enum: + - kubernetes + - nacos type: string required: - serviceName @@ -254,6 +257,9 @@ spec: serviceName: type: string type: + enum: + - kubernetes + - nacos type: string required: - serviceName diff --git a/internal/controller/apisixroute_controller.go b/internal/controller/apisixroute_controller.go index 4b52e530d..52a4a8962 100644 --- a/internal/controller/apisixroute_controller.go +++ b/internal/controller/apisixroute_controller.go @@ -343,6 +343,15 @@ func (r *ApisixRouteReconciler) validateBackends(ctx context.Context, tc *provid Message: fmt.Sprintf("failed to list endpoint slices: %v", err), } } + + // backend.subset specifies a subset of upstream nodes. + // It specifies that the target pod's label should be a superset of the subset labels of the ApisixUpstream of the serviceName + subsetLabels, err := r.getSubsetLabels(ctx, in, backend) + if err != nil { + return err + } + endpoints.Items = r.filterEndpointSlicesBySubsetLabels(ctx, endpoints.Items, subsetLabels) + tc.EndpointSlices[serviceNN] = endpoints.Items } @@ -684,3 +693,71 @@ func (r *ApisixRouteReconciler) listApisixRoutesForPluginConfig(ctx context.Cont } return pkgutils.DedupComparable(requests) } + +func (r *ApisixRouteReconciler) getSubsetLabels(ctx context.Context, ar *apiv2.ApisixRoute, backend apiv2.ApisixRouteHTTPBackend) (map[string]string, error) { + if backend.Subset == "" { + return make(map[string]string), nil + } + + // Try to Get the ApisixUpstream with the same name as backend.ServiceName + var ( + auNN = types.NamespacedName{ + Namespace: ar.GetNamespace(), + Name: backend.ServiceName, + } + au apiv2.ApisixUpstream + ) + if err := r.Get(ctx, auNN, &au); err != nil { + if client.IgnoreNotFound(err) == nil { + return make(map[string]string), nil + } + return nil, err + } + + // try tro get the subset labels from the ApisixUpstream subsets + for _, subset := range au.Spec.Subsets { + if backend.Subset == subset.Name { + return subset.Labels, nil + } + } + + return make(map[string]string), nil +} + +func (r *ApisixRouteReconciler) filterEndpointSlicesBySubsetLabels(ctx context.Context, in []discoveryv1.EndpointSlice, labels map[string]string) []discoveryv1.EndpointSlice { + if len(labels) == 0 { + return in + } + + for i := range in { + in[i] = r.filterEndpointSliceByTargetPod(ctx, in[i], labels) + } + + return utils.Filter(in, func(v discoveryv1.EndpointSlice) bool { + return len(v.Endpoints) > 0 + }) +} + +// filterEndpointSliceByTargetPod filters item.Endpoints which is not a subset of labels +func (r *ApisixRouteReconciler) filterEndpointSliceByTargetPod(ctx context.Context, item discoveryv1.EndpointSlice, labels map[string]string) discoveryv1.EndpointSlice { + item.Endpoints = utils.Filter(item.Endpoints, func(v discoveryv1.Endpoint) bool { + if v.TargetRef == nil || v.TargetRef.Kind != KindPod { + return true + } + + var ( + pod corev1.Pod + podNN = types.NamespacedName{ + Namespace: v.TargetRef.Namespace, + Name: v.TargetRef.Name, + } + ) + if err := r.Get(ctx, podNN, &pod); err != nil { + return false + } + + return utils.IsSubsetOf(labels, pod.GetLabels()) + }) + + return item +} diff --git a/internal/controller/utils.go b/internal/controller/utils.go index e4a2c8bd0..96721c64a 100644 --- a/internal/controller/utils.go +++ b/internal/controller/utils.go @@ -58,6 +58,7 @@ const ( KindApisixRoute = "ApisixRoute" KindApisixGlobalRule = "ApisixGlobalRule" KindApisixPluginConfig = "ApisixPluginConfig" + KindPod = "Pod" ) const defaultIngressClassAnnotation = "ingressclass.kubernetes.io/is-default-class" diff --git a/internal/provider/provider.go b/internal/provider/provider.go index db52a3826..b31eae8f1 100644 --- a/internal/provider/provider.go +++ b/internal/provider/provider.go @@ -46,6 +46,7 @@ type TranslateContext struct { PluginConfigs map[k8stypes.NamespacedName]*v1alpha1.PluginConfig ApisixPluginConfigs map[k8stypes.NamespacedName]*apiv2.ApisixPluginConfig Services map[k8stypes.NamespacedName]*corev1.Service + Pods map[k8stypes.NamespacedName][]*corev1.Pod BackendTrafficPolicies map[k8stypes.NamespacedName]*v1alpha1.BackendTrafficPolicy Upstreams map[k8stypes.NamespacedName]*apiv2.ApisixUpstream GatewayProxies map[types.NamespacedNameKind]v1alpha1.GatewayProxy @@ -63,8 +64,9 @@ func NewDefaultTranslateContext(ctx context.Context) *TranslateContext { PluginConfigs: make(map[k8stypes.NamespacedName]*v1alpha1.PluginConfig), ApisixPluginConfigs: make(map[k8stypes.NamespacedName]*apiv2.ApisixPluginConfig), Services: make(map[k8stypes.NamespacedName]*corev1.Service), - Upstreams: make(map[k8stypes.NamespacedName]*apiv2.ApisixUpstream), + Pods: make(map[k8stypes.NamespacedName][]*corev1.Pod), BackendTrafficPolicies: make(map[k8stypes.NamespacedName]*v1alpha1.BackendTrafficPolicy), + Upstreams: make(map[k8stypes.NamespacedName]*apiv2.ApisixUpstream), GatewayProxies: make(map[types.NamespacedNameKind]v1alpha1.GatewayProxy), ResourceParentRefs: make(map[types.NamespacedNameKind][]types.NamespacedNameKind), } diff --git a/internal/utils/k8s.go b/internal/utils/k8s.go index 7cc3767d5..d2e5abe48 100644 --- a/internal/utils/k8s.go +++ b/internal/utils/k8s.go @@ -60,3 +60,29 @@ var hostDefRegex = regexp.MustCompile(hostDef) func MatchHostDef(host string) bool { return hostDefRegex.MatchString(host) } + +func AppendFunc[T any](s []T, keep func(v T) bool, values ...T) []T { + for _, v := range values { + if keep(v) { + s = append(s, v) + } + } + return s +} + +func Filter[T any](s []T, keep func(v T) bool) []T { + return AppendFunc(make([]T, 0), keep, s...) +} + +func IsSubsetOf(a, b map[string]string) bool { + if len(a) == 0 { + // Empty labels matches everything. + return true + } + for k, v := range a { + if vv, ok := b[k]; !ok || vv != v { + return false + } + } + return true +} diff --git a/test/e2e/apisix/pluginconfig.go b/test/e2e/apisix/pluginconfig.go index fdcaac9a7..1dda08a68 100644 --- a/test/e2e/apisix/pluginconfig.go +++ b/test/e2e/apisix/pluginconfig.go @@ -67,7 +67,7 @@ var _ = Describe("Test ApisixPluginConfig", func() { applier = framework.NewApplier(s.GinkgoT, s.K8sClient, s.CreateResourceFromString) ) - FContext("Test ApisixPluginConfig", func() { + Context("Test ApisixPluginConfig", func() { BeforeEach(func() { By("create GatewayProxy") gatewayProxy := fmt.Sprintf(gatewayProxyYamlPluginConfig, s.Deployer.GetAdminEndpoint(), s.AdminKey()) From 93583e9d7e89b2fd77b0caba6ceae09f3b49deff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=82=9F=E7=A9=BA?= Date: Thu, 19 Jun 2025 18:22:42 +0800 Subject: [PATCH 4/8] feat: enhance subset support for ApisixUpstream and update validations, RBAC, and e2e tests --- api/v2/apisixupstream_types.go | 2 +- .../apisix.apache.org_apisixupstreams.yaml | 2 +- config/rbac/role.yaml | 1 + internal/controller/apisixroute_controller.go | 5 +- internal/controller/indexer/indexer.go | 5 ++ internal/manager/controllers.go | 1 + test/e2e/apisix/route.go | 65 +++++++++++++++++-- test/e2e/apisix/upstream.go | 14 +--- test/e2e/framework/manifests/ingress.yaml | 1 + 9 files changed, 75 insertions(+), 21 deletions(-) diff --git a/api/v2/apisixupstream_types.go b/api/v2/apisixupstream_types.go index 342bcfd9a..1b7d4bcbf 100644 --- a/api/v2/apisixupstream_types.go +++ b/api/v2/apisixupstream_types.go @@ -19,7 +19,7 @@ import ( ) // ApisixUpstreamSpec describes the specification of ApisixUpstream. -// +kubebuilder:validation:XValidation:rule="has(self.externalNodes)!=has(self.discovery)" +// +kubebuilder:validation:XValidation:rule="has(self.subsets) || (has(self.externalNodes)!=has(self.discovery))" type ApisixUpstreamSpec struct { // IngressClassName is the name of an IngressClass cluster resource. // controller implementations use this field to know whether they should be diff --git a/config/crd/bases/apisix.apache.org_apisixupstreams.yaml b/config/crd/bases/apisix.apache.org_apisixupstreams.yaml index 1c2218279..c2145656f 100644 --- a/config/crd/bases/apisix.apache.org_apisixupstreams.yaml +++ b/config/crd/bases/apisix.apache.org_apisixupstreams.yaml @@ -562,7 +562,7 @@ spec: type: string type: object x-kubernetes-validations: - - rule: has(self.externalNodes)!=has(self.discovery) + - rule: has(self.subsets) || (has(self.externalNodes)!=has(self.discovery)) status: description: ApisixStatus is the status report for Apisix ingress Resources properties: diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index 552fc1c41..516b74bec 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -15,6 +15,7 @@ rules: - "" resources: - namespaces + - pods - secrets - services verbs: diff --git a/internal/controller/apisixroute_controller.go b/internal/controller/apisixroute_controller.go index 52a4a8962..da9df3b02 100644 --- a/internal/controller/apisixroute_controller.go +++ b/internal/controller/apisixroute_controller.go @@ -350,9 +350,8 @@ func (r *ApisixRouteReconciler) validateBackends(ctx context.Context, tc *provid if err != nil { return err } - endpoints.Items = r.filterEndpointSlicesBySubsetLabels(ctx, endpoints.Items, subsetLabels) - tc.EndpointSlices[serviceNN] = endpoints.Items + tc.EndpointSlices[serviceNN] = r.filterEndpointSlicesBySubsetLabels(ctx, endpoints.Items, subsetLabels) } return nil @@ -714,7 +713,7 @@ func (r *ApisixRouteReconciler) getSubsetLabels(ctx context.Context, ar *apiv2.A return nil, err } - // try tro get the subset labels from the ApisixUpstream subsets + // try to get the subset labels from the ApisixUpstream subsets for _, subset := range au.Spec.Subsets { if backend.Subset == subset.Name { return subset.Labels, nil diff --git a/internal/controller/indexer/indexer.go b/internal/controller/indexer/indexer.go index 252931185..26d32f9f5 100644 --- a/internal/controller/indexer/indexer.go +++ b/internal/controller/indexer/indexer.go @@ -527,6 +527,11 @@ func ApisixRouteSecretIndexFunc(cli client.Client) func(client.Object) []string func ApisixRouteApisixUpstreamIndexFunc(obj client.Object) (keys []string) { ar := obj.(*apiv2.ApisixRoute) for _, rule := range ar.Spec.HTTP { + for _, backend := range rule.Backends { + if backend.Subset != "" && backend.ServiceName != "" { + keys = append(keys, GenIndexKey(ar.GetNamespace(), backend.ServiceName)) + } + } for _, upstream := range rule.Upstreams { if upstream.Name != "" { keys = append(keys, GenIndexKey(ar.GetNamespace(), upstream.Name)) diff --git a/internal/manager/controllers.go b/internal/manager/controllers.go index 0a52499c2..98e95e206 100644 --- a/internal/manager/controllers.go +++ b/internal/manager/controllers.go @@ -29,6 +29,7 @@ import ( // +kubebuilder:rbac:groups=coordination.k8s.io,resources=leases,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups="discovery.k8s.io",resources=endpointslices,verbs=get;list;watch // +kubebuilder:rbac:groups="",resources=services,verbs=get;list;watch +// +kubebuilder:rbac:groups="",resources=pods,verbs=get;list;watch // +kubebuilder:rbac:groups="",resources=secrets,verbs=get;list;watch // +kubebuilder:rbac:groups="",resources=namespaces,verbs=get;list;watch diff --git a/test/e2e/apisix/route.go b/test/e2e/apisix/route.go index 67e1e5ffb..9e19fe89d 100644 --- a/test/e2e/apisix/route.go +++ b/test/e2e/apisix/route.go @@ -291,10 +291,67 @@ spec: s.NewAPISIXClient().GET("/get").Expect().Header("X-Upstream-IP").IsEqual(clusterIP) }) - PIt("Test ApisixRoute subset", func() { - // route.Spec.HTTP[].Backends[].Subset depends on ApisixUpstream. - // ApisixUpstream is not implemented yet. - // So the case is pending for now + It("Test ApisixRoute subset", func() { + const apisixRouteSpec = ` +apiVersion: apisix.apache.org/v2 +kind: ApisixRoute +metadata: + name: default +spec: + ingressClassName: apisix + http: + - name: rule0 + match: + hosts: + - httpbin + paths: + - /* + backends: + - serviceName: httpbin-service-e2e-test + servicePort: 80 + subset: test-subset +` + const apisixUpstreamSpec0 = ` +apiVersion: apisix.apache.org/v2 +kind: ApisixUpstream +metadata: + name: httpbin-service-e2e-test +spec: + ingressClassName: apisix + subsets: + - name: test-subset + labels: + unknown-key: unknown-value +` + const apisixUpstreamSpec1 = ` +apiVersion: apisix.apache.org/v2 +kind: ApisixUpstream +metadata: + name: httpbin-service-e2e-test +spec: + ingressClassName: apisix + subsets: + - name: test-subset + labels: + app: httpbin-deployment-e2e-test +` + request := func() int { + return s.NewAPISIXClient().GET("/get").WithHost("httpbin").Expect().Raw().StatusCode + } + By("apply ApisixRoute") + var apisixRoute apiv2.ApisixRoute + applier.MustApplyAPIv2(types.NamespacedName{Namespace: s.Namespace(), Name: "default"}, &apisixRoute, apisixRouteSpec) + Eventually(request).WithTimeout(8 * time.Second).ProbeEvery(time.Second).Should(Equal(http.StatusOK)) + + // no pod matches the subset label "unknown-key: unknown-value" so there will be no node in the upstream, + // to request the route will get http.StatusServiceUnavailable + applier.MustApplyAPIv2(types.NamespacedName{Namespace: s.Namespace(), Name: "httpbin-service-e2e-test"}, new(apiv2.ApisixUpstream), apisixUpstreamSpec0) + Eventually(request).WithTimeout(8 * time.Second).ProbeEvery(time.Second).Should(Equal(http.StatusServiceUnavailable)) + + // the pod matches the subset label "app: httpbin-deployment-e2e-test", + // to request the route will be OK + applier.MustApplyAPIv2(types.NamespacedName{Namespace: s.Namespace(), Name: "httpbin-service-e2e-test"}, new(apiv2.ApisixUpstream), apisixUpstreamSpec1) + Eventually(request).WithTimeout(8 * time.Second).ProbeEvery(time.Second).Should(Equal(http.StatusOK)) }) }) diff --git a/test/e2e/apisix/upstream.go b/test/e2e/apisix/upstream.go index 6e01b8300..0f1f721aa 100644 --- a/test/e2e/apisix/upstream.go +++ b/test/e2e/apisix/upstream.go @@ -53,22 +53,12 @@ spec: ` err = s.CreateResourceFromString(apisixUpstreamSpec0) Expect(err).Should(HaveOccurred()) - Expect(err.Error()).Should(ContainSubstring("failed rule: has(self.externalNodes)!=has(self.discovery)")) + Expect(err.Error()).Should(ContainSubstring("has(self.externalNodes)!=has(self.discovery)")) err = s.CreateResourceFromString(apisixUpstreamSpec1) Expect(err).Should(HaveOccurred()) - Expect(err.Error()).Should(ContainSubstring("failed rule: has(self.externalNodes)!=has(self.discovery)")) + Expect(err.Error()).Should(ContainSubstring("has(self.externalNodes)!=has(self.discovery)")) }) - - It("", func() {}) - - It("", func() {}) - - It("", func() {}) - - It("", func() {}) - - It("", func() {}) }) }) diff --git a/test/e2e/framework/manifests/ingress.yaml b/test/e2e/framework/manifests/ingress.yaml index 1b086ff54..72dc0cf6a 100644 --- a/test/e2e/framework/manifests/ingress.yaml +++ b/test/e2e/framework/manifests/ingress.yaml @@ -82,6 +82,7 @@ rules: - "" resources: - services + - pods verbs: - get - list From 1f4dea4f5daab3326793956242df4b9e3edd4ec0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=82=9F=E7=A9=BA?= Date: Fri, 20 Jun 2025 16:29:15 +0800 Subject: [PATCH 5/8] more tests --- api/adc/types.go | 2 +- api/v2/apisixupstream_types.go | 4 +- .../apisix.apache.org_apisixupstreams.yaml | 6 - .../provider/adc/translator/apisixupstream.go | 5 + test/e2e/apisix/route.go | 121 +++++++++++++++++- 5 files changed, 127 insertions(+), 11 deletions(-) diff --git a/api/adc/types.go b/api/adc/types.go index 28bdeca98..414dc73a1 100644 --- a/api/adc/types.go +++ b/api/adc/types.go @@ -197,7 +197,7 @@ type Upstream struct { HashOn string `json:"hash_on,omitempty" yaml:"hash_on,omitempty"` Key string `json:"key,omitempty" yaml:"key,omitempty"` - Nodes UpstreamNodes `json:"nodes" yaml:"nodes"` + Nodes UpstreamNodes `json:"nodes,omitempty" yaml:"nodes,omitempty"` PassHost string `json:"pass_host,omitempty" yaml:"pass_host,omitempty"` Retries *int64 `json:"retries,omitempty" yaml:"retries,omitempty"` RetryTimeout *float64 `json:"retry_timeout,omitempty" yaml:"retry_timeout,omitempty"` diff --git a/api/v2/apisixupstream_types.go b/api/v2/apisixupstream_types.go index 1b7d4bcbf..ba918f86c 100644 --- a/api/v2/apisixupstream_types.go +++ b/api/v2/apisixupstream_types.go @@ -166,8 +166,8 @@ type ApisixUpstreamSubset struct { // Discovery defines Service discovery related configuration. type Discovery struct { ServiceName string `json:"serviceName" yaml:"serviceName"` - // +kubebuilder:validation:Enum=kubernetes;nacos; - Type string `json:"type" yaml:"type"` + Type string `json:"type" yaml:"type"` + // +kubebuilder:validation:Optional Args map[string]string `json:"args,omitempty" yaml:"args,omitempty"` } diff --git a/config/crd/bases/apisix.apache.org_apisixupstreams.yaml b/config/crd/bases/apisix.apache.org_apisixupstreams.yaml index c2145656f..5b901231a 100644 --- a/config/crd/bases/apisix.apache.org_apisixupstreams.yaml +++ b/config/crd/bases/apisix.apache.org_apisixupstreams.yaml @@ -50,9 +50,6 @@ spec: serviceName: type: string type: - enum: - - kubernetes - - nacos type: string required: - serviceName @@ -257,9 +254,6 @@ spec: serviceName: type: string type: - enum: - - kubernetes - - nacos type: string required: - serviceName diff --git a/internal/provider/adc/translator/apisixupstream.go b/internal/provider/adc/translator/apisixupstream.go index 2243b2bc3..293e68545 100644 --- a/internal/provider/adc/translator/apisixupstream.go +++ b/internal/provider/adc/translator/apisixupstream.go @@ -256,6 +256,7 @@ func translateApisixUpstreamDiscovery(au *apiv2.ApisixUpstream, ups *adc.Upstrea ups.ServiceName = au.Spec.Discovery.ServiceName ups.DiscoveryType = au.Spec.Discovery.Type ups.DiscoveryArgs = au.Spec.Discovery.Args + ups.Nodes = nil return nil } @@ -265,6 +266,10 @@ func composeExternalUpstreamName(au *apiv2.ApisixUpstream) string { } func translateApisixUpstreamExternalNodes(tctx *provider.TranslateContext, au *apiv2.ApisixUpstream, ups *adc.Upstream) error { + if au.Spec.Discovery != nil { + ups.Nodes = nil + return nil + } for _, node := range au.Spec.ExternalNodes { switch node.Type { case apiv2.ExternalTypeDomain: diff --git a/test/e2e/apisix/route.go b/test/e2e/apisix/route.go index 9e19fe89d..40fe08e6e 100644 --- a/test/e2e/apisix/route.go +++ b/test/e2e/apisix/route.go @@ -408,8 +408,7 @@ spec: Expect(err).ShouldNot(HaveOccurred(), "apply service") applier.MustApplyAPIv2(types.NamespacedName{Namespace: s.Namespace(), Name: "default-upstream"}, new(apiv2.ApisixUpstream), apisixUpstreamSpec0) - var apisxiRoute apiv2.ApisixRoute - applier.MustApplyAPIv2(types.NamespacedName{Namespace: s.Namespace(), Name: "default"}, &apisxiRoute, apisixRouteSpec) + applier.MustApplyAPIv2(types.NamespacedName{Namespace: s.Namespace(), Name: "default"}, new(apiv2.ApisixRoute), apisixRouteSpec) By("verify that the ApisixUpstream reference a Service which is not ExternalName should not request OK") request := func(path string) int { @@ -497,5 +496,123 @@ spec: Eventually(upstreamAddrs).Should(HaveKey(endpoint)) Eventually(upstreamAddrs).Should(HaveKey(clusterIP)) }) + + PIt("Test Upstream healthcheck", func() { + const apisixRouteSpec = ` +apiVersion: apisix.apache.org/v2 +kind: ApisixRoute +metadata: + name: default +spec: + ingressClassName: apisix + http: + - name: rule0 + match: + paths: + - /* + upstreams: + - name: default-upstream +` + const apisixUpstreamSpec0 = ` +apiVersion: apisix.apache.org/v2 +kind: ApisixUpstream +metadata: + name: default-upstream +spec: + ingressClassName: apisix + externalNodes: + - type: Domain + name: httpbin-service-e2e-test +` + const apisixUpstreamSpec1 = ` +apiVersion: apisix.apache.org/v2 +kind: ApisixUpstream +metadata: + name: default-upstream +spec: + ingressClassName: apisix + externalNodes: + - type: Domain + name: httpbin-service-e2e-test + healthCheck: + active: + type: http + httpPath: %s + healthy: + httpCodes: [200] + interval: 1s + unhealthy: + httpFailures: 2 + interval: 1s +` + By("apply ApisixRoute") + applier.MustApplyAPIv2(types.NamespacedName{Namespace: s.Namespace(), Name: "default"}, new(apiv2.ApisixRoute), apisixRouteSpec) + + By("apply ApisixUpstream without healthCheck") + applier.MustApplyAPIv2(types.NamespacedName{Namespace: s.Namespace(), Name: "default-upstream"}, new(apiv2.ApisixUpstream), apisixUpstreamSpec0) + + By("verify ApisixRoute and ApisixUpstream works") + request := func() int { + return s.NewAPISIXClient().GET("/get").Expect().Raw().StatusCode + } + Eventually(request).WithTimeout(8 * time.Second).ProbeEvery(time.Second).Should(Equal(http.StatusOK)) + + By("apply ApisixUpstream with a healthCheck") + auSpec := fmt.Sprintf(apisixUpstreamSpec1, "/status/200") + applier.MustApplyAPIv2(types.NamespacedName{Namespace: s.Namespace(), Name: "default-upstream"}, new(apiv2.ApisixUpstream), auSpec) + + By("verify ApisixRoute and ApisixUpstream with a healthCheck works") + Eventually(request).WithTimeout(8 * time.Second).ProbeEvery(time.Second).Should(Equal(http.StatusOK)) + + By("apply ApisixUpstream with the healthCheck which is always failure") + auSpec = fmt.Sprintf(apisixUpstreamSpec1, "/status/502") + applier.MustApplyAPIv2(types.NamespacedName{Namespace: s.Namespace(), Name: "default-upstream"}, new(apiv2.ApisixUpstream), auSpec) + + // FIXME: can not pass yet + Skip("checks did not sync to the dataplane") + By("verify ApisixRoute and ApisixUpstream works") + Eventually(request).WithTimeout(8 * time.Second).ProbeEvery(time.Second).Should(Equal(http.StatusServiceUnavailable)) + }) + + PIt("Test discovery", func() { + const apisixRouteSpec = ` +apiVersion: apisix.apache.org/v2 +kind: ApisixRoute +metadata: + name: default +spec: + ingressClassName: apisix + http: + - name: rule0 + match: + paths: + - /* + upstreams: + - name: default-upstream +` + const apisixUpstreamSpec0 = ` +apiVersion: apisix.apache.org/v2 +kind: ApisixUpstream +metadata: + name: default-upstream +spec: + ingressClassName: apisix + discovery: + type: dns + serviceName: httpbin-service-e2e-test.%s.svc.cluster.local +` + By("apply ApisixRoute") + applier.MustApplyAPIv2(types.NamespacedName{Namespace: s.Namespace(), Name: "default"}, new(apiv2.ApisixRoute), apisixRouteSpec) + + By("apply ApisixUpstream with discovery") + applier.MustApplyAPIv2(types.NamespacedName{Namespace: s.Namespace(), Name: "default-upstream"}, new(apiv2.ApisixUpstream), fmt.Sprintf(apisixUpstreamSpec0, s.Namespace())) + + By("verify ApisixUpstream works") + time.Sleep(time.Hour) + request := func() int { + return s.NewAPISIXClient().GET("/get").Expect().Raw().StatusCode + } + Eventually(request).WithTimeout(8 * time.Second).ProbeEvery(time.Second).Should(Equal(http.StatusOK)) + }) }) }) From b9e1aaeb0ce72f077aa63ae910261392e8387bfd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=82=9F=E7=A9=BA?= Date: Fri, 20 Jun 2025 16:37:27 +0800 Subject: [PATCH 6/8] refactor: remove unused Pods map from provider struct --- internal/provider/provider.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/internal/provider/provider.go b/internal/provider/provider.go index b31eae8f1..288811b45 100644 --- a/internal/provider/provider.go +++ b/internal/provider/provider.go @@ -46,7 +46,6 @@ type TranslateContext struct { PluginConfigs map[k8stypes.NamespacedName]*v1alpha1.PluginConfig ApisixPluginConfigs map[k8stypes.NamespacedName]*apiv2.ApisixPluginConfig Services map[k8stypes.NamespacedName]*corev1.Service - Pods map[k8stypes.NamespacedName][]*corev1.Pod BackendTrafficPolicies map[k8stypes.NamespacedName]*v1alpha1.BackendTrafficPolicy Upstreams map[k8stypes.NamespacedName]*apiv2.ApisixUpstream GatewayProxies map[types.NamespacedNameKind]v1alpha1.GatewayProxy @@ -64,7 +63,6 @@ func NewDefaultTranslateContext(ctx context.Context) *TranslateContext { PluginConfigs: make(map[k8stypes.NamespacedName]*v1alpha1.PluginConfig), ApisixPluginConfigs: make(map[k8stypes.NamespacedName]*apiv2.ApisixPluginConfig), Services: make(map[k8stypes.NamespacedName]*corev1.Service), - Pods: make(map[k8stypes.NamespacedName][]*corev1.Pod), BackendTrafficPolicies: make(map[k8stypes.NamespacedName]*v1alpha1.BackendTrafficPolicy), Upstreams: make(map[k8stypes.NamespacedName]*apiv2.ApisixUpstream), GatewayProxies: make(map[types.NamespacedNameKind]v1alpha1.GatewayProxy), From 9c2c810c20c9e29dc9dd1f6efcdd1bb6eb2a2b40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=82=9F=E7=A9=BA?= Date: Fri, 20 Jun 2025 17:12:53 +0800 Subject: [PATCH 7/8] refactor: remove deprecated discovery and health check features from ApisixUpstream in standalone mode --- api/adc/types.go | 2 +- api/v2/apisixupstream_types.go | 3 +- .../apisix.apache.org_apisixupstreams.yaml | 20 +-- docs/crd/api.md | 12 +- .../provider/adc/translator/apisixupstream.go | 96 -------------- test/e2e/apisix/route.go | 118 ------------------ test/e2e/apisix/upstream.go | 64 ---------- 7 files changed, 21 insertions(+), 294 deletions(-) delete mode 100644 test/e2e/apisix/upstream.go diff --git a/api/adc/types.go b/api/adc/types.go index 414dc73a1..28bdeca98 100644 --- a/api/adc/types.go +++ b/api/adc/types.go @@ -197,7 +197,7 @@ type Upstream struct { HashOn string `json:"hash_on,omitempty" yaml:"hash_on,omitempty"` Key string `json:"key,omitempty" yaml:"key,omitempty"` - Nodes UpstreamNodes `json:"nodes,omitempty" yaml:"nodes,omitempty"` + Nodes UpstreamNodes `json:"nodes" yaml:"nodes"` PassHost string `json:"pass_host,omitempty" yaml:"pass_host,omitempty"` Retries *int64 `json:"retries,omitempty" yaml:"retries,omitempty"` RetryTimeout *float64 `json:"retry_timeout,omitempty" yaml:"retry_timeout,omitempty"` diff --git a/api/v2/apisixupstream_types.go b/api/v2/apisixupstream_types.go index ba918f86c..23e47425d 100644 --- a/api/v2/apisixupstream_types.go +++ b/api/v2/apisixupstream_types.go @@ -19,7 +19,6 @@ import ( ) // ApisixUpstreamSpec describes the specification of ApisixUpstream. -// +kubebuilder:validation:XValidation:rule="has(self.subsets) || (has(self.externalNodes)!=has(self.discovery))" type ApisixUpstreamSpec struct { // IngressClassName is the name of an IngressClass cluster resource. // controller implementations use this field to know whether they should be @@ -95,6 +94,7 @@ type ApisixUpstreamConfig struct { // +kubebuilder:validation:Optional Timeout *UpstreamTimeout `json:"timeout,omitempty" yaml:"timeout,omitempty"` + // Deprecated: this is no longer support on standalone mode. // The health check configurations for the upstream. // +kubebuilder:validation:Optional HealthCheck *HealthCheck `json:"healthCheck,omitempty" yaml:"healthCheck,omitempty"` @@ -119,6 +119,7 @@ type ApisixUpstreamConfig struct { // +kubebuilder:validation:Optional UpstreamHost string `json:"upstreamHost,omitempty" yaml:"upstreamHost,omitempty"` + // Deprecated: this is no longer support on standalone mode. // Discovery is used to configure service discovery for upstream. // +kubebuilder:validation:Optional Discovery *Discovery `json:"discovery,omitempty" yaml:"discovery,omitempty"` diff --git a/config/crd/bases/apisix.apache.org_apisixupstreams.yaml b/config/crd/bases/apisix.apache.org_apisixupstreams.yaml index 5b901231a..9da6294ed 100644 --- a/config/crd/bases/apisix.apache.org_apisixupstreams.yaml +++ b/config/crd/bases/apisix.apache.org_apisixupstreams.yaml @@ -40,8 +40,9 @@ spec: description: ApisixUpstreamSpec describes the specification of ApisixUpstream. properties: discovery: - description: Discovery is used to configure service discovery for - upstream. + description: |- + Deprecated: this is no longer support on standalone mode. + Discovery is used to configure service discovery for upstream. properties: args: additionalProperties: @@ -77,7 +78,9 @@ spec: minItems: 1 type: array healthCheck: - description: The health check configurations for the upstream. + description: |- + Deprecated: this is no longer support on standalone mode. + The health check configurations for the upstream. properties: active: description: ActiveHealthCheck defines the active kind of upstream @@ -244,8 +247,9 @@ spec: them if they are set on the port level. properties: discovery: - description: Discovery is used to configure service discovery - for upstream. + description: |- + Deprecated: this is no longer support on standalone mode. + Discovery is used to configure service discovery for upstream. properties: args: additionalProperties: @@ -260,7 +264,9 @@ spec: - type type: object healthCheck: - description: The health check configurations for the upstream. + description: |- + Deprecated: this is no longer support on standalone mode. + The health check configurations for the upstream. properties: active: description: ActiveHealthCheck defines the active kind of @@ -555,8 +561,6 @@ spec: the pass_host is set to rewrite type: string type: object - x-kubernetes-validations: - - rule: has(self.subsets) || (has(self.externalNodes)!=has(self.discovery)) status: description: ApisixStatus is the status report for Apisix ingress Resources properties: diff --git a/docs/crd/api.md b/docs/crd/api.md index a337c2db5..2c1481dd7 100644 --- a/docs/crd/api.md +++ b/docs/crd/api.md @@ -1334,12 +1334,12 @@ load balancer, health check, etc. | `scheme` _string_ | The scheme used to talk with the upstream. Now value can be http, grpc. | | `retries` _integer_ | How many times that the proxy (Apache APISIX) should do when errors occur (error, timeout or bad http status codes like 500, 502). | | `timeout` _[UpstreamTimeout](#upstreamtimeout)_ | Timeout settings for the read, send and connect to the upstream. | -| `healthCheck` _[HealthCheck](#healthcheck)_ | The health check configurations for the upstream. | +| `healthCheck` _[HealthCheck](#healthcheck)_ | Deprecated: this is no longer support on standalone mode. The health check configurations for the upstream. | | `tlsSecret` _[ApisixSecret](#apisixsecret)_ | Set the client certificate when connecting to TLS upstream. | | `subsets` _[ApisixUpstreamSubset](#apisixupstreamsubset) array_ | Subsets groups the service endpoints by their labels. Usually used to differentiate service versions. | | `passHost` _string_ | Configures the host when the request is forwarded to the upstream. Can be one of pass, node or rewrite. | | `upstreamHost` _string_ | Specifies the host of the Upstream request. This is only valid if the pass_host is set to rewrite | -| `discovery` _[Discovery](#discovery)_ | Discovery is used to configure service discovery for upstream. | +| `discovery` _[Discovery](#discovery)_ | Deprecated: this is no longer support on standalone mode. Discovery is used to configure service discovery for upstream. | _Appears in:_ @@ -1391,12 +1391,12 @@ ApisixUpstreamSpec describes the specification of ApisixUpstream. | `scheme` _string_ | The scheme used to talk with the upstream. Now value can be http, grpc. | | `retries` _integer_ | How many times that the proxy (Apache APISIX) should do when errors occur (error, timeout or bad http status codes like 500, 502). | | `timeout` _[UpstreamTimeout](#upstreamtimeout)_ | Timeout settings for the read, send and connect to the upstream. | -| `healthCheck` _[HealthCheck](#healthcheck)_ | The health check configurations for the upstream. | +| `healthCheck` _[HealthCheck](#healthcheck)_ | Deprecated: this is no longer support on standalone mode. The health check configurations for the upstream. | | `tlsSecret` _[ApisixSecret](#apisixsecret)_ | Set the client certificate when connecting to TLS upstream. | | `subsets` _[ApisixUpstreamSubset](#apisixupstreamsubset) array_ | Subsets groups the service endpoints by their labels. Usually used to differentiate service versions. | | `passHost` _string_ | Configures the host when the request is forwarded to the upstream. Can be one of pass, node or rewrite. | | `upstreamHost` _string_ | Specifies the host of the Upstream request. This is only valid if the pass_host is set to rewrite | -| `discovery` _[Discovery](#discovery)_ | Discovery is used to configure service discovery for upstream. | +| `discovery` _[Discovery](#discovery)_ | Deprecated: this is no longer support on standalone mode. Discovery is used to configure service discovery for upstream. | | `portLevelSettings` _[PortLevelSettings](#portlevelsettings) array_ | | @@ -1560,12 +1560,12 @@ them if they are set on the port level. | `scheme` _string_ | The scheme used to talk with the upstream. Now value can be http, grpc. | | `retries` _integer_ | How many times that the proxy (Apache APISIX) should do when errors occur (error, timeout or bad http status codes like 500, 502). | | `timeout` _[UpstreamTimeout](#upstreamtimeout)_ | Timeout settings for the read, send and connect to the upstream. | -| `healthCheck` _[HealthCheck](#healthcheck)_ | The health check configurations for the upstream. | +| `healthCheck` _[HealthCheck](#healthcheck)_ | Deprecated: this is no longer support on standalone mode. The health check configurations for the upstream. | | `tlsSecret` _[ApisixSecret](#apisixsecret)_ | Set the client certificate when connecting to TLS upstream. | | `subsets` _[ApisixUpstreamSubset](#apisixupstreamsubset) array_ | Subsets groups the service endpoints by their labels. Usually used to differentiate service versions. | | `passHost` _string_ | Configures the host when the request is forwarded to the upstream. Can be one of pass, node or rewrite. | | `upstreamHost` _string_ | Specifies the host of the Upstream request. This is only valid if the pass_host is set to rewrite | -| `discovery` _[Discovery](#discovery)_ | Discovery is used to configure service discovery for upstream. | +| `discovery` _[Discovery](#discovery)_ | Deprecated: this is no longer support on standalone mode. Discovery is used to configure service discovery for upstream. | | `port` _integer_ | Port is a Kubernetes Service port, it should be already defined. | diff --git a/internal/provider/adc/translator/apisixupstream.go b/internal/provider/adc/translator/apisixupstream.go index 293e68545..63b8e366c 100644 --- a/internal/provider/adc/translator/apisixupstream.go +++ b/internal/provider/adc/translator/apisixupstream.go @@ -32,10 +32,8 @@ func (t *Translator) translateApisixUpstream(tctx *provider.TranslateContext, au patchApisixUpstreamBasics, translateApisixUpstreamScheme, translateApisixUpstreamLoadBalancer, - translateApisixUpstreamHealthCheck, translateApisixUpstreamRetriesAndTimeout, translateApisixUpstreamPassHost, - translateApisixUpstreamDiscovery, } { if err = f(au, ups); err != nil { return @@ -98,83 +96,6 @@ func translateApisixUpstreamLoadBalancer(au *apiv2.ApisixUpstream, ups *adc.Upst return nil } -func translateApisixUpstreamHealthCheck(au *apiv2.ApisixUpstream, ups *adc.Upstream) error { - check := au.Spec.HealthCheck - if check == nil { - return nil - } - - ups.Checks = new(adc.UpstreamHealthCheck) - ups.Checks.Active = translateApisixUpstreamHealthCheckActive(check.Active) - - if check.Passive != nil { - ups.Checks.Passive = translateApisixUpstreamHealthCheckPassive(check.Passive) - } - - return nil -} - -func translateApisixUpstreamHealthCheckActive(config *apiv2.ActiveHealthCheck) *adc.UpstreamActiveHealthCheck { - var active adc.UpstreamActiveHealthCheck - - active.Type = cmp.Or(config.Type, apiv2.HealthCheckHTTP) - active.Timeout = int(config.Timeout.Seconds()) - active.Port = config.Port - active.Concurrency = config.Concurrency - active.Host = config.Host - active.HTTPPath = config.HTTPPath - active.HTTPRequestHeaders = config.RequestHeaders - active.HTTPSVerifyCert = config.StrictTLS != nil && *config.StrictTLS - - if config.Healthy != nil { - active.Healthy = adc.UpstreamActiveHealthCheckHealthy{ - UpstreamPassiveHealthCheckHealthy: adc.UpstreamPassiveHealthCheckHealthy{ - HTTPStatuses: config.Healthy.HTTPCodes, - Successes: config.Healthy.Successes, - }, - Interval: int(config.Healthy.Interval.Seconds()), - } - } - - if config.Unhealthy != nil { - active.Unhealthy = adc.UpstreamActiveHealthCheckUnhealthy{ - UpstreamPassiveHealthCheckUnhealthy: adc.UpstreamPassiveHealthCheckUnhealthy{ - HTTPStatuses: config.Unhealthy.HTTPCodes, - HTTPFailures: config.Unhealthy.HTTPFailures, - TCPFailures: config.Unhealthy.TCPFailures, - Timeouts: int(config.Timeout.Seconds()), - }, - Interval: int(config.Unhealthy.Interval.Seconds()), - } - } - - return &active -} - -func translateApisixUpstreamHealthCheckPassive(config *apiv2.PassiveHealthCheck) *adc.UpstreamPassiveHealthCheck { - var passive adc.UpstreamPassiveHealthCheck - - passive.Type = cmp.Or(config.Type, apiv2.HealthCheckHTTP) - - if config.Healthy != nil { - passive.Healthy = adc.UpstreamPassiveHealthCheckHealthy{ - HTTPStatuses: config.Healthy.HTTPCodes, - Successes: config.Healthy.Successes, - } - } - - if config.Unhealthy != nil { - passive.Unhealthy = adc.UpstreamPassiveHealthCheckUnhealthy{ - HTTPStatuses: config.Unhealthy.HTTPCodes, - HTTPFailures: config.Unhealthy.HTTPFailures, - TCPFailures: config.Unhealthy.TCPFailures, - Timeouts: config.Unhealthy.Timeouts, - } - } - - return &passive -} - func translateApisixUpstreamRetriesAndTimeout(au *apiv2.ApisixUpstream, ups *adc.Upstream) error { retries := au.Spec.Retries timeout := au.Spec.Timeout @@ -248,28 +169,11 @@ func translateApisixUpstreamPassHost(au *apiv2.ApisixUpstream, ups *adc.Upstream return nil } -func translateApisixUpstreamDiscovery(au *apiv2.ApisixUpstream, ups *adc.Upstream) error { - if au.Spec.Discovery == nil { - return nil - } - - ups.ServiceName = au.Spec.Discovery.ServiceName - ups.DiscoveryType = au.Spec.Discovery.Type - ups.DiscoveryArgs = au.Spec.Discovery.Args - ups.Nodes = nil - - return nil -} - func composeExternalUpstreamName(au *apiv2.ApisixUpstream) string { return au.GetGenerateName() + "_" + au.GetName() } func translateApisixUpstreamExternalNodes(tctx *provider.TranslateContext, au *apiv2.ApisixUpstream, ups *adc.Upstream) error { - if au.Spec.Discovery != nil { - ups.Nodes = nil - return nil - } for _, node := range au.Spec.ExternalNodes { switch node.Type { case apiv2.ExternalTypeDomain: diff --git a/test/e2e/apisix/route.go b/test/e2e/apisix/route.go index 40fe08e6e..3e24082c7 100644 --- a/test/e2e/apisix/route.go +++ b/test/e2e/apisix/route.go @@ -496,123 +496,5 @@ spec: Eventually(upstreamAddrs).Should(HaveKey(endpoint)) Eventually(upstreamAddrs).Should(HaveKey(clusterIP)) }) - - PIt("Test Upstream healthcheck", func() { - const apisixRouteSpec = ` -apiVersion: apisix.apache.org/v2 -kind: ApisixRoute -metadata: - name: default -spec: - ingressClassName: apisix - http: - - name: rule0 - match: - paths: - - /* - upstreams: - - name: default-upstream -` - const apisixUpstreamSpec0 = ` -apiVersion: apisix.apache.org/v2 -kind: ApisixUpstream -metadata: - name: default-upstream -spec: - ingressClassName: apisix - externalNodes: - - type: Domain - name: httpbin-service-e2e-test -` - const apisixUpstreamSpec1 = ` -apiVersion: apisix.apache.org/v2 -kind: ApisixUpstream -metadata: - name: default-upstream -spec: - ingressClassName: apisix - externalNodes: - - type: Domain - name: httpbin-service-e2e-test - healthCheck: - active: - type: http - httpPath: %s - healthy: - httpCodes: [200] - interval: 1s - unhealthy: - httpFailures: 2 - interval: 1s -` - By("apply ApisixRoute") - applier.MustApplyAPIv2(types.NamespacedName{Namespace: s.Namespace(), Name: "default"}, new(apiv2.ApisixRoute), apisixRouteSpec) - - By("apply ApisixUpstream without healthCheck") - applier.MustApplyAPIv2(types.NamespacedName{Namespace: s.Namespace(), Name: "default-upstream"}, new(apiv2.ApisixUpstream), apisixUpstreamSpec0) - - By("verify ApisixRoute and ApisixUpstream works") - request := func() int { - return s.NewAPISIXClient().GET("/get").Expect().Raw().StatusCode - } - Eventually(request).WithTimeout(8 * time.Second).ProbeEvery(time.Second).Should(Equal(http.StatusOK)) - - By("apply ApisixUpstream with a healthCheck") - auSpec := fmt.Sprintf(apisixUpstreamSpec1, "/status/200") - applier.MustApplyAPIv2(types.NamespacedName{Namespace: s.Namespace(), Name: "default-upstream"}, new(apiv2.ApisixUpstream), auSpec) - - By("verify ApisixRoute and ApisixUpstream with a healthCheck works") - Eventually(request).WithTimeout(8 * time.Second).ProbeEvery(time.Second).Should(Equal(http.StatusOK)) - - By("apply ApisixUpstream with the healthCheck which is always failure") - auSpec = fmt.Sprintf(apisixUpstreamSpec1, "/status/502") - applier.MustApplyAPIv2(types.NamespacedName{Namespace: s.Namespace(), Name: "default-upstream"}, new(apiv2.ApisixUpstream), auSpec) - - // FIXME: can not pass yet - Skip("checks did not sync to the dataplane") - By("verify ApisixRoute and ApisixUpstream works") - Eventually(request).WithTimeout(8 * time.Second).ProbeEvery(time.Second).Should(Equal(http.StatusServiceUnavailable)) - }) - - PIt("Test discovery", func() { - const apisixRouteSpec = ` -apiVersion: apisix.apache.org/v2 -kind: ApisixRoute -metadata: - name: default -spec: - ingressClassName: apisix - http: - - name: rule0 - match: - paths: - - /* - upstreams: - - name: default-upstream -` - const apisixUpstreamSpec0 = ` -apiVersion: apisix.apache.org/v2 -kind: ApisixUpstream -metadata: - name: default-upstream -spec: - ingressClassName: apisix - discovery: - type: dns - serviceName: httpbin-service-e2e-test.%s.svc.cluster.local -` - By("apply ApisixRoute") - applier.MustApplyAPIv2(types.NamespacedName{Namespace: s.Namespace(), Name: "default"}, new(apiv2.ApisixRoute), apisixRouteSpec) - - By("apply ApisixUpstream with discovery") - applier.MustApplyAPIv2(types.NamespacedName{Namespace: s.Namespace(), Name: "default-upstream"}, new(apiv2.ApisixUpstream), fmt.Sprintf(apisixUpstreamSpec0, s.Namespace())) - - By("verify ApisixUpstream works") - time.Sleep(time.Hour) - request := func() int { - return s.NewAPISIXClient().GET("/get").Expect().Raw().StatusCode - } - Eventually(request).WithTimeout(8 * time.Second).ProbeEvery(time.Second).Should(Equal(http.StatusOK)) - }) }) }) diff --git a/test/e2e/apisix/upstream.go b/test/e2e/apisix/upstream.go deleted file mode 100644 index 0f1f721aa..000000000 --- a/test/e2e/apisix/upstream.go +++ /dev/null @@ -1,64 +0,0 @@ -// Licensed 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 ( - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - - "github.com/apache/apisix-ingress-controller/test/e2e/scaffold" -) - -var _ = Describe("Test ApisixUpstream", func() { - var ( - s = scaffold.NewScaffold(&scaffold.Options{ - ControllerName: "apisix.apache.org/apisix-ingress-controller", - }) - err error - ) - - Context("Test ApisixUpstream validation", func() { - It("validation of externalNodes and discovery", func() { - const apisixUpstreamSpec0 = ` -apiVersion: apisix.apache.org/v2 -kind: ApisixUpstream -metadata: - name: default-upstream -spec: - ingressClassName: apisix - externalNodes: - - type: Service - name: httpbin-service-e2e-test - discovery: - serviceName: xx - type: nacos -` - const apisixUpstreamSpec1 = ` -apiVersion: apisix.apache.org/v2 -kind: ApisixUpstream -metadata: - name: default-upstream -spec: - ingressClassName: apisix -` - err = s.CreateResourceFromString(apisixUpstreamSpec0) - Expect(err).Should(HaveOccurred()) - Expect(err.Error()).Should(ContainSubstring("has(self.externalNodes)!=has(self.discovery)")) - - err = s.CreateResourceFromString(apisixUpstreamSpec1) - Expect(err).Should(HaveOccurred()) - Expect(err.Error()).Should(ContainSubstring("has(self.externalNodes)!=has(self.discovery)")) - - }) - }) -}) From 04584e965bbc61c1fdc3e0cddb7e22fd998be568 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=82=9F=E7=A9=BA?= Date: Mon, 23 Jun 2025 09:56:34 +0800 Subject: [PATCH 8/8] resolve comments --- internal/controller/apisixroute_controller.go | 7 ++++--- internal/provider/adc/translator/apisixupstream.go | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/internal/controller/apisixroute_controller.go b/internal/controller/apisixroute_controller.go index da9df3b02..36c6d2e4d 100644 --- a/internal/controller/apisixroute_controller.go +++ b/internal/controller/apisixroute_controller.go @@ -694,8 +694,9 @@ func (r *ApisixRouteReconciler) listApisixRoutesForPluginConfig(ctx context.Cont } func (r *ApisixRouteReconciler) getSubsetLabels(ctx context.Context, ar *apiv2.ApisixRoute, backend apiv2.ApisixRouteHTTPBackend) (map[string]string, error) { + empty := make(map[string]string) if backend.Subset == "" { - return make(map[string]string), nil + return empty, nil } // Try to Get the ApisixUpstream with the same name as backend.ServiceName @@ -708,7 +709,7 @@ func (r *ApisixRouteReconciler) getSubsetLabels(ctx context.Context, ar *apiv2.A ) if err := r.Get(ctx, auNN, &au); err != nil { if client.IgnoreNotFound(err) == nil { - return make(map[string]string), nil + return empty, nil } return nil, err } @@ -720,7 +721,7 @@ func (r *ApisixRouteReconciler) getSubsetLabels(ctx context.Context, ar *apiv2.A } } - return make(map[string]string), nil + return empty, nil } func (r *ApisixRouteReconciler) filterEndpointSlicesBySubsetLabels(ctx context.Context, in []discoveryv1.EndpointSlice, labels map[string]string) []discoveryv1.EndpointSlice { diff --git a/internal/provider/adc/translator/apisixupstream.go b/internal/provider/adc/translator/apisixupstream.go index 63b8e366c..936a05284 100644 --- a/internal/provider/adc/translator/apisixupstream.go +++ b/internal/provider/adc/translator/apisixupstream.go @@ -146,7 +146,7 @@ func translateApisixUpstreamClientTLS(tctx *provider.TranslateContext, au *apiv2 ) secret, ok := tctx.Secrets[secretNN] if !ok { - return nil + return errors.Errorf("sercret %s not found", secretNN) } cert, key, err := extractKeyPair(secret, true)