Skip to content

Commit 3d1a42b

Browse files
ronethingronething
authored
fix: list matching request (#219)
Signed-off-by: Ashing Zheng <[email protected]>
1 parent 379f3a6 commit 3d1a42b

File tree

12 files changed

+193
-215
lines changed

12 files changed

+193
-215
lines changed

‎internal/controller/indexer/indexer.go‎

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import (
3333
"github.com/apache/apisix-ingress-controller/api/v1alpha1"
3434
apiv2 "github.com/apache/apisix-ingress-controller/api/v2"
3535
intypes "github.com/apache/apisix-ingress-controller/internal/types"
36+
k8sutils "github.com/apache/apisix-ingress-controller/internal/utils"
3637
"github.com/apache/apisix-ingress-controller/pkg/utils"
3738
)
3839

@@ -737,11 +738,8 @@ func IngressClassParametersRefIndexFunc(rawObj client.Object) []string {
737738
ingressClass.Spec.Parameters.APIGroup != nil &&
738739
*ingressClass.Spec.Parameters.APIGroup == v1alpha1.GroupVersion.Group &&
739740
ingressClass.Spec.Parameters.Kind == intypes.KindGatewayProxy {
740-
ns := ingressClass.GetNamespace()
741-
if ingressClass.Spec.Parameters.Namespace != nil {
742-
ns = *ingressClass.Spec.Parameters.Namespace
743-
}
744-
return []string{GenIndexKey(ns, ingressClass.Spec.Parameters.Name)}
741+
namespace := k8sutils.GetIngressClassParametersNamespace(*ingressClass)
742+
return []string{GenIndexKey(namespace, ingressClass.Spec.Parameters.Name)}
745743
}
746744
return nil
747745
}
@@ -753,11 +751,8 @@ func IngressClassV1beta1ParametersRefIndexFunc(rawObj client.Object) []string {
753751
ingressClass.Spec.Parameters.APIGroup != nil &&
754752
*ingressClass.Spec.Parameters.APIGroup == v1alpha1.GroupVersion.Group &&
755753
ingressClass.Spec.Parameters.Kind == intypes.KindGatewayProxy {
756-
ns := ingressClass.GetNamespace()
757-
if ingressClass.Spec.Parameters.Namespace != nil {
758-
ns = *ingressClass.Spec.Parameters.Namespace
759-
}
760-
return []string{GenIndexKey(ns, ingressClass.Spec.Parameters.Name)}
754+
namespace := k8sutils.GetIngressClassV1beta1ParametersNamespace(*ingressClass)
755+
return []string{GenIndexKey(namespace, ingressClass.Spec.Parameters.Name)}
761756
}
762757
return nil
763758
}

‎internal/controller/ingressclass_controller.go‎

Lines changed: 1 addition & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ import (
2626
networkingv1 "k8s.io/api/networking/v1"
2727
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2828
"k8s.io/apimachinery/pkg/runtime"
29-
"k8s.io/apimachinery/pkg/types"
3029
ctrl "sigs.k8s.io/controller-runtime"
3130
"sigs.k8s.io/controller-runtime/pkg/builder"
3231
"sigs.k8s.io/controller-runtime/pkg/client"
@@ -37,7 +36,6 @@ import (
3736
"github.com/apache/apisix-ingress-controller/api/v1alpha1"
3837
"github.com/apache/apisix-ingress-controller/internal/controller/indexer"
3938
"github.com/apache/apisix-ingress-controller/internal/provider"
40-
"github.com/apache/apisix-ingress-controller/internal/utils"
4139
)
4240

4341
// IngressClassReconciler reconciles a IngressClass object.
@@ -98,7 +96,7 @@ func (r *IngressClassReconciler) Reconcile(ctx context.Context, req ctrl.Request
9896
// Create a translate context
9997
tctx := provider.NewDefaultTranslateContext(ctx)
10098

101-
if err := r.processInfrastructure(tctx, ingressClass); err != nil {
99+
if err := ProcessIngressClassParameters(tctx, r.Client, r.Log, ingressClass, ingressClass); err != nil {
102100
r.Log.Error(err, "failed to process infrastructure for ingressclass", "ingressclass", ingressClass.GetName())
103101
return ctrl.Result{}, err
104102
}
@@ -175,75 +173,3 @@ func (r *IngressClassReconciler) listIngressClassesForSecret(ctx context.Context
175173

176174
return distinctRequests(requests)
177175
}
178-
179-
func (r *IngressClassReconciler) processInfrastructure(tctx *provider.TranslateContext, ingressClass *networkingv1.IngressClass) error {
180-
if ingressClass.Spec.Parameters == nil {
181-
return nil
182-
}
183-
184-
if ingressClass.Spec.Parameters.APIGroup == nil ||
185-
*ingressClass.Spec.Parameters.APIGroup != v1alpha1.GroupVersion.Group ||
186-
ingressClass.Spec.Parameters.Kind != KindGatewayProxy {
187-
return nil
188-
}
189-
190-
namespace := ingressClass.Namespace
191-
if ingressClass.Spec.Parameters.Namespace != nil {
192-
namespace = *ingressClass.Spec.Parameters.Namespace
193-
}
194-
// Check for annotation override
195-
if annotationNamespace, exists := ingressClass.Annotations[parametersNamespaceAnnotation]; exists && annotationNamespace != "" {
196-
namespace = annotationNamespace
197-
}
198-
199-
gatewayProxy := new(v1alpha1.GatewayProxy)
200-
if err := r.Get(context.Background(), client.ObjectKey{
201-
Namespace: namespace,
202-
Name: ingressClass.Spec.Parameters.Name,
203-
}, gatewayProxy); err != nil {
204-
return fmt.Errorf("failed to get gateway proxy: %w", err)
205-
}
206-
207-
rk := utils.NamespacedNameKind(ingressClass)
208-
209-
tctx.GatewayProxies[rk] = *gatewayProxy
210-
tctx.ResourceParentRefs[rk] = append(tctx.ResourceParentRefs[rk], rk)
211-
212-
// Load secrets if needed
213-
if gatewayProxy.Spec.Provider != nil && gatewayProxy.Spec.Provider.ControlPlane != nil {
214-
auth := gatewayProxy.Spec.Provider.ControlPlane.Auth
215-
if auth.Type == v1alpha1.AuthTypeAdminKey && auth.AdminKey != nil && auth.AdminKey.ValueFrom != nil {
216-
if auth.AdminKey.ValueFrom.SecretKeyRef != nil {
217-
secretRef := auth.AdminKey.ValueFrom.SecretKeyRef
218-
secret := &corev1.Secret{}
219-
if err := r.Get(context.Background(), client.ObjectKey{
220-
Namespace: namespace,
221-
Name: secretRef.Name,
222-
}, secret); err != nil {
223-
r.Log.Error(err, "failed to get secret for gateway proxy", "namespace", namespace, "name", secretRef.Name)
224-
return err
225-
}
226-
tctx.Secrets[client.ObjectKey{
227-
Namespace: namespace,
228-
Name: secretRef.Name,
229-
}] = secret
230-
}
231-
}
232-
}
233-
234-
if service := gatewayProxy.Spec.Provider.ControlPlane.Service; service != nil {
235-
if err := addProviderEndpointsToTranslateContext(tctx, r.Client, types.NamespacedName{
236-
Namespace: gatewayProxy.GetNamespace(),
237-
Name: service.Name,
238-
}); err != nil {
239-
return err
240-
}
241-
}
242-
243-
_, ok := tctx.GatewayProxies[rk]
244-
if !ok {
245-
return fmt.Errorf("no gateway proxy found for ingress class")
246-
}
247-
248-
return nil
249-
}

‎internal/controller/utils.go‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1203,7 +1203,7 @@ func ListMatchingRequests(
12031203
matchFunc func(obj client.Object) bool,
12041204
opts ...client.ListOption,
12051205
) []reconcile.Request {
1206-
if err := c.List(ctx, listObj); err != nil {
1206+
if err := c.List(ctx, listObj, opts...); err != nil {
12071207
logger.Error(err, "failed to list resource")
12081208
return nil
12091209
}
@@ -1323,13 +1323,7 @@ func ProcessIngressClassParameters(tctx *provider.TranslateContext, c client.Cli
13231323
parameters := ingressClass.Spec.Parameters
13241324
// check if the parameters reference GatewayProxy
13251325
if parameters.APIGroup != nil && *parameters.APIGroup == v1alpha1.GroupVersion.Group && parameters.Kind == KindGatewayProxy {
1326-
ns := "default"
1327-
if parameters.Namespace != nil {
1328-
ns = *parameters.Namespace
1329-
}
1330-
if annotationNamespace, exists := ingressClass.Annotations[parametersNamespaceAnnotation]; exists && annotationNamespace != "" {
1331-
ns = annotationNamespace
1332-
}
1326+
ns := utils.GetIngressClassParametersNamespace(*ingressClass)
13331327

13341328
gatewayProxy := &v1alpha1.GatewayProxy{}
13351329
if err := c.Get(tctx, client.ObjectKey{

‎internal/utils/k8s.go‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ import (
2121
"net"
2222
"regexp"
2323

24+
networkingv1 "k8s.io/api/networking/v1"
25+
networkingv1beta1 "k8s.io/api/networking/v1beta1"
2426
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2527
k8stypes "k8s.io/apimachinery/pkg/types"
2628
"sigs.k8s.io/controller-runtime/pkg/client"
@@ -99,3 +101,25 @@ func ConditionStatus(status bool) metav1.ConditionStatus {
99101
}
100102
return metav1.ConditionFalse
101103
}
104+
105+
func GetIngressClassParametersNamespace(ingressClass networkingv1.IngressClass) string {
106+
namespace := "default"
107+
if ingressClass.Spec.Parameters.Namespace != nil {
108+
namespace = *ingressClass.Spec.Parameters.Namespace
109+
}
110+
if annotationNamespace, exists := ingressClass.Annotations["apisix.apache.org/parameters-namespace"]; exists && annotationNamespace != "" {
111+
namespace = annotationNamespace
112+
}
113+
return namespace
114+
}
115+
116+
func GetIngressClassV1beta1ParametersNamespace(ingressClass networkingv1beta1.IngressClass) string {
117+
namespace := "default"
118+
if ingressClass.Spec.Parameters.Namespace != nil {
119+
namespace = *ingressClass.Spec.Parameters.Namespace
120+
}
121+
if annotationNamespace, exists := ingressClass.Annotations["apisix.apache.org/parameters-namespace"]; exists && annotationNamespace != "" {
122+
namespace = annotationNamespace
123+
}
124+
return namespace
125+
}

‎test/e2e/crds/v1alpha1/backendtrafficpolicy.go‎

Lines changed: 1 addition & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -29,23 +29,6 @@ import (
2929
var _ = Describe("Test BackendTrafficPolicy base on HTTPRoute", Label("apisix.apache.org", "v1alpha1", "backendtrafficpolicy"), func() {
3030
s := scaffold.NewDefaultScaffold()
3131

32-
var defaultGatewayProxy = `
33-
apiVersion: apisix.apache.org/v1alpha1
34-
kind: GatewayProxy
35-
metadata:
36-
name: apisix-proxy-config
37-
spec:
38-
provider:
39-
type: ControlPlane
40-
controlPlane:
41-
endpoints:
42-
- %s
43-
auth:
44-
type: AdminKey
45-
adminKey:
46-
value: "%s"
47-
`
48-
4932
var defaultGatewayClass = `
5033
apiVersion: gateway.networking.k8s.io/v1
5134
kind: GatewayClass
@@ -125,7 +108,7 @@ spec:
125108
`
126109

127110
BeforeEach(func() {
128-
s.ApplyDefaultGatewayResource(defaultGatewayProxy, defaultGatewayClass, defaultGateway, defaultHTTPRoute)
111+
s.ApplyDefaultGatewayResource(s.GetGatewayProxySpec(), defaultGatewayClass, defaultGateway, defaultHTTPRoute)
129112
})
130113
It("should rewrite upstream host", func() {
131114
s.ResourceApplied("BackendTrafficPolicy", "httpbin", createUpstreamHost, 1)

‎test/e2e/crds/v1alpha1/consumer.go‎

Lines changed: 13 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -19,33 +19,18 @@ package v1alpha1
1919

2020
import (
2121
"fmt"
22+
"time"
2223

2324
. "github.com/onsi/ginkgo/v2"
2425
. "github.com/onsi/gomega"
2526

27+
"github.com/apache/apisix-ingress-controller/internal/provider/adc"
2628
"github.com/apache/apisix-ingress-controller/test/e2e/scaffold"
2729
)
2830

2931
var _ = Describe("Test Consumer", Label("apisix.apache.org", "v1alpha1", "consumer"), func() {
3032
s := scaffold.NewDefaultScaffold()
3133

32-
var defaultGatewayProxy = `
33-
apiVersion: apisix.apache.org/v1alpha1
34-
kind: GatewayProxy
35-
metadata:
36-
name: apisix-proxy-config
37-
spec:
38-
provider:
39-
type: ControlPlane
40-
controlPlane:
41-
endpoints:
42-
- %s
43-
auth:
44-
type: AdminKey
45-
adminKey:
46-
value: "%s"
47-
`
48-
4934
var defaultGatewayClass = `
5035
apiVersion: gateway.networking.k8s.io/v1
5136
kind: GatewayClass
@@ -152,7 +137,7 @@ spec:
152137
`
153138

154139
BeforeEach(func() {
155-
s.ApplyDefaultGatewayResource(defaultGatewayProxy, defaultGatewayClass, defaultGateway, defaultHTTPRoute)
140+
s.ApplyDefaultGatewayResource(s.GetGatewayProxySpec(), defaultGatewayClass, defaultGateway, defaultHTTPRoute)
156141
})
157142

158143
It("limit-count plugin", func() {
@@ -248,7 +233,7 @@ spec:
248233
`
249234

250235
BeforeEach(func() {
251-
s.ApplyDefaultGatewayResource(defaultGatewayProxy, defaultGatewayClass, defaultGateway, defaultHTTPRoute)
236+
s.ApplyDefaultGatewayResource(s.GetGatewayProxySpec(), defaultGatewayClass, defaultGateway, defaultHTTPRoute)
252237
})
253238

254239
It("Create/Update/Delete", func() {
@@ -397,7 +382,7 @@ spec:
397382
`
398383

399384
BeforeEach(func() {
400-
s.ApplyDefaultGatewayResource(defaultGatewayProxy, defaultGatewayClass, defaultGateway, defaultHTTPRoute)
385+
s.ApplyDefaultGatewayResource(s.GetGatewayProxySpec(), defaultGatewayClass, defaultGateway, defaultHTTPRoute)
401386
})
402387
It("Create/Update/Delete", func() {
403388
err := s.CreateResourceFromString(keyAuthSecret)
@@ -518,7 +503,7 @@ spec:
518503
`
519504

520505
BeforeEach(func() {
521-
s.ApplyDefaultGatewayResource(defaultGatewayProxy, defaultGatewayClass, defaultGateway, defaultHTTPRoute)
506+
s.ApplyDefaultGatewayResource(s.GetGatewayProxySpec(), defaultGatewayClass, defaultGateway, defaultHTTPRoute)
522507
})
523508

524509
It("Should sync consumer when GatewayProxy is updated", func() {
@@ -611,10 +596,13 @@ spec:
611596
`
612597

613598
BeforeEach(func() {
614-
s.ApplyDefaultGatewayResource(defaultGatewayProxy, defaultGatewayClass, defaultGateway, defaultHTTPRoute)
599+
s.ApplyDefaultGatewayResource(s.GetGatewayProxySpec(), defaultGatewayClass, defaultGateway, defaultHTTPRoute)
615600
})
616601

617602
It("Should sync Consumer during startup", func() {
603+
if s.Deployer.Name() == adc.BackendModeAPI7EE {
604+
Skip("skipping test in API7EE mode")
605+
}
618606
Expect(s.CreateResourceFromString(consumer2)).NotTo(HaveOccurred(), "creating unused consumer")
619607
s.ResourceApplied("Consumer", "consumer-sample", consumer1, 1)
620608

@@ -644,14 +632,16 @@ spec:
644632
s.Deployer.ScaleDataplane(1)
645633
s.Deployer.ScaleIngress(1)
646634

635+
By("check consumer sync")
647636
s.RequestAssert(&scaffold.RequestAssert{
648637
Method: "GET",
649638
Path: "/get",
650639
Host: "httpbin.org",
651640
Headers: map[string]string{
652641
"apikey": "sample-key",
653642
},
654-
Check: scaffold.WithExpectedStatus(200),
643+
Timeout: 1 * time.Minute,
644+
Check: scaffold.WithExpectedStatus(200),
655645
})
656646

657647
s.RequestAssert(&scaffold.RequestAssert{

‎test/e2e/crds/v2/route.go‎

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -44,16 +44,29 @@ var _ = Describe("Test ApisixRoute", Label("apisix.apache.org", "v2", "apisixrou
4444
applier = framework.NewApplier(s.GinkgoT, s.K8sClient, s.CreateResourceFromString)
4545
)
4646

47+
const ingressClassYaml = `
48+
apiVersion: networking.k8s.io/%s
49+
kind: IngressClass
50+
metadata:
51+
name: apisix
52+
annotations:
53+
apisix.apache.org/parameters-namespace: %s
54+
spec:
55+
controller: "apisix.apache.org/apisix-ingress-controller"
56+
parameters:
57+
apiGroup: "apisix.apache.org"
58+
kind: "GatewayProxy"
59+
name: "apisix-proxy-config"
60+
`
4761
BeforeEach(func() {
4862
By("create GatewayProxy")
49-
gatewayProxy := fmt.Sprintf(gatewayProxyYaml, s.Deployer.GetAdminEndpoint(), s.AdminKey())
50-
err := s.CreateResourceFromStringWithNamespace(gatewayProxy, "default")
63+
err := s.CreateResourceFromString(s.GetGatewayProxySpec())
5164
Expect(err).NotTo(HaveOccurred(), "creating GatewayProxy")
5265
time.Sleep(5 * time.Second)
5366

5467
By("create IngressClass")
55-
ingressClass := fmt.Sprintf(ingressClassYaml, framework.IngressVersion)
56-
err = s.CreateResourceFromStringWithNamespace(ingressClass, "")
68+
ingressClass := fmt.Sprintf(ingressClassYaml, framework.IngressVersion, s.Namespace())
69+
err = s.CreateResourceFromString(ingressClass)
5770
Expect(err).NotTo(HaveOccurred(), "creating IngressClass")
5871
time.Sleep(5 * time.Second)
5972
})
@@ -714,6 +727,9 @@ spec:
714727
servicePort: 80
715728
`
716729
It("Should sync ApisixRoute during startup", func() {
730+
if s.Deployer.Name() == adc.BackendModeAPI7EE {
731+
Skip("skipping test in API7EE mode")
732+
}
717733
By("apply ApisixRoute")
718734
Expect(s.CreateResourceFromString(route2)).ShouldNot(HaveOccurred(), "apply ApisixRoute with nonexistent ingressClassName")
719735
Expect(s.CreateResourceFromString(route3)).ShouldNot(HaveOccurred(), "apply ApisixRoute without ingressClassName")
@@ -745,10 +761,11 @@ spec:
745761
s.Deployer.ScaleIngress(1)
746762

747763
s.RequestAssert(&scaffold.RequestAssert{
748-
Method: "GET",
749-
Path: "/get",
750-
Host: "httpbin",
751-
Check: scaffold.WithExpectedStatus(http.StatusOK),
764+
Method: "GET",
765+
Path: "/get",
766+
Host: "httpbin",
767+
Timeout: 1 * time.Minute,
768+
Check: scaffold.WithExpectedStatus(http.StatusOK),
752769
})
753770
s.RequestAssert(&scaffold.RequestAssert{
754771
Method: "GET",

0 commit comments

Comments
 (0)