Skip to content

Commit 0e2923f

Browse files
dspodspo
committed
refactor: Extract HTTP rule processing logic into a reusable function
- Moved HTTP rule handling to `processApisixRouteHTTPRule` for improved modularity. - Updated ApisixRoute controller to integrate the new function. - Enhanced indexing for ApisixRoute to include ApisixUpstream secrets and services. - Refactored e2e tests to validate updated ApisixRoute and ApisixUpstream logic.
1 parent 18955cd commit 0e2923f

File tree

6 files changed

+260
-138
lines changed

6 files changed

+260
-138
lines changed

‎internal/controller/apisixroute_controller.go‎

Lines changed: 136 additions & 96 deletions
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ func (r *ApisixRouteReconciler) processApisixRoute(ctx context.Context, tc *prov
141141
var (
142142
rules = make(map[string]struct{})
143143
)
144-
for httpIndex, http := range in.Spec.HTTP {
144+
for _, http := range in.Spec.HTTP {
145145
// check rule names
146146
if _, ok := rules[http.Name]; ok {
147147
return ReasonError{
@@ -151,126 +151,166 @@ func (r *ApisixRouteReconciler) processApisixRoute(ctx context.Context, tc *prov
151151
}
152152
rules[http.Name] = struct{}{}
153153

154-
// check secret
155-
for _, plugin := range http.Plugins {
156-
if !plugin.Enable || plugin.Config == nil || plugin.SecretRef == "" {
157-
continue
158-
}
159-
var (
160-
secret = corev1.Secret{
161-
ObjectMeta: metav1.ObjectMeta{
162-
Name: plugin.SecretRef,
163-
Namespace: in.Namespace,
164-
},
165-
}
166-
secretNN = utils.NamespacedName(&secret)
167-
)
168-
if err := r.Get(ctx, secretNN, &secret); err != nil {
169-
return ReasonError{
170-
Reason: string(apiv2.ConditionReasonInvalidSpec),
171-
Message: fmt.Sprintf("failed to get Secret: %s", secretNN),
172-
}
173-
}
174-
175-
tc.Secrets[utils.NamespacedName(&secret)] = &secret
154+
if err := r.processApisixRouteHTTPRule(ctx, tc, in, http); err != nil {
155+
r.Log.Error(err, "failed to process ApisixRoute http rule")
156+
return err
176157
}
158+
}
159+
160+
return nil
161+
}
177162

178-
// check vars
179-
// todo: cache the result to tctx
180-
if _, err := http.Match.NginxVars.ToVars(); err != nil {
163+
func (r *ApisixRouteReconciler) processApisixRouteHTTPRule(ctx context.Context, tc *provider.TranslateContext, in *apiv2.ApisixRoute, rule apiv2.ApisixRouteHTTP) error {
164+
// check secret
165+
for _, plugin := range rule.Plugins {
166+
if !plugin.Enable || plugin.Config == nil || plugin.SecretRef == "" {
167+
continue
168+
}
169+
var (
170+
secret = corev1.Secret{
171+
ObjectMeta: metav1.ObjectMeta{
172+
Name: plugin.SecretRef,
173+
Namespace: in.Namespace,
174+
},
175+
}
176+
secretNN = utils.NamespacedName(&secret)
177+
)
178+
if err := r.Get(ctx, secretNN, &secret); err != nil {
181179
return ReasonError{
182180
Reason: string(apiv2.ConditionReasonInvalidSpec),
183-
Message: fmt.Sprintf(".spec.http[%d].match.exprs: %s", httpIndex, err.Error()),
181+
Message: fmt.Sprintf("failed to get Secret: %s", secretNN),
184182
}
185183
}
186184

187-
// validate remote address
188-
if err := utils.ValidateRemoteAddrs(http.Match.RemoteAddrs); err != nil {
189-
return ReasonError{
190-
Reason: string(apiv2.ConditionReasonInvalidSpec),
191-
Message: fmt.Sprintf(".spec.http[%d].match.remoteAddrs: %s", httpIndex, err.Error()),
192-
}
185+
tc.Secrets[utils.NamespacedName(&secret)] = &secret
186+
}
187+
188+
// check vars
189+
// todo: cache the result to tctx
190+
if _, err := rule.Match.NginxVars.ToVars(); err != nil {
191+
return ReasonError{
192+
Reason: string(apiv2.ConditionReasonInvalidSpec),
193+
Message: fmt.Sprintf(".spec.http[].match.exprs: %v", err),
193194
}
195+
}
194196

195-
// process backend
196-
var backends = make(map[types.NamespacedName]struct{})
197-
for _, backend := range http.Backends {
198-
var (
199-
service = corev1.Service{
200-
ObjectMeta: metav1.ObjectMeta{
201-
Name: backend.ServiceName,
202-
Namespace: in.Namespace,
203-
},
204-
}
205-
serviceNN = utils.NamespacedName(&service)
206-
)
207-
if _, ok := backends[serviceNN]; ok {
208-
return ReasonError{
209-
Reason: string(apiv2.ConditionReasonInvalidSpec),
210-
Message: fmt.Sprintf("duplicate backend service: %s", serviceNN),
211-
}
212-
}
213-
backends[serviceNN] = struct{}{}
197+
// validate remote address
198+
if err := utils.ValidateRemoteAddrs(rule.Match.RemoteAddrs); err != nil {
199+
return ReasonError{
200+
Reason: string(apiv2.ConditionReasonInvalidSpec),
201+
Message: fmt.Sprintf(".spec.http[].match.remoteAddrs: %v", err),
202+
}
203+
}
214204

215-
if err := r.Get(ctx, serviceNN, &service); err != nil {
216-
if err := client.IgnoreNotFound(err); err == nil {
217-
r.Log.Error(errors.New("service not found"), "Service", serviceNN)
218-
continue
219-
}
220-
return err
221-
}
222-
if service.Spec.Type == corev1.ServiceTypeExternalName {
223-
tc.Services[serviceNN] = &service
224-
continue
205+
// process backend
206+
var backends = make(map[types.NamespacedName]struct{})
207+
for _, backend := range rule.Backends {
208+
var (
209+
service = corev1.Service{
210+
ObjectMeta: metav1.ObjectMeta{
211+
Name: backend.ServiceName,
212+
Namespace: in.Namespace,
213+
},
225214
}
226-
227-
if backend.ResolveGranularity == "service" && service.Spec.ClusterIP == "" {
228-
r.Log.Error(errors.New("service has no ClusterIP"), "Service", serviceNN, "ResolveGranularity", backend.ResolveGranularity)
229-
continue
215+
serviceNN = utils.NamespacedName(&service)
216+
)
217+
if _, ok := backends[serviceNN]; ok {
218+
return ReasonError{
219+
Reason: string(apiv2.ConditionReasonInvalidSpec),
220+
Message: fmt.Sprintf("duplicate backend service: %s", serviceNN),
230221
}
222+
}
223+
backends[serviceNN] = struct{}{}
231224

232-
if !slices.ContainsFunc(service.Spec.Ports, func(port corev1.ServicePort) bool {
233-
return port.Port == int32(backend.ServicePort.IntValue())
234-
}) {
235-
r.Log.Error(errors.New("port not found in service"), "Service", serviceNN, "port", backend.ServicePort.String())
225+
if err := r.Get(ctx, serviceNN, &service); err != nil {
226+
if err := client.IgnoreNotFound(err); err == nil {
227+
r.Log.Error(errors.New("service not found"), "Service", serviceNN)
236228
continue
237229
}
230+
return err
231+
}
232+
if service.Spec.Type == corev1.ServiceTypeExternalName {
238233
tc.Services[serviceNN] = &service
234+
continue
235+
}
239236

240-
var endpoints discoveryv1.EndpointSliceList
241-
if err := r.List(ctx, &endpoints,
242-
client.InNamespace(service.Namespace),
243-
client.MatchingLabels{
244-
discoveryv1.LabelServiceName: service.Name,
245-
},
246-
); err != nil {
247-
return ReasonError{
248-
Reason: string(apiv2.ConditionReasonInvalidSpec),
249-
Message: fmt.Sprintf("failed to list endpoint slices: %v", err),
250-
}
237+
if backend.ResolveGranularity == "service" && service.Spec.ClusterIP == "" {
238+
r.Log.Error(errors.New("service has no ClusterIP"), "Service", serviceNN, "ResolveGranularity", backend.ResolveGranularity)
239+
continue
240+
}
241+
242+
if !slices.ContainsFunc(service.Spec.Ports, func(port corev1.ServicePort) bool {
243+
return port.Port == int32(backend.ServicePort.IntValue())
244+
}) {
245+
r.Log.Error(errors.New("port not found in service"), "Service", serviceNN, "port", backend.ServicePort.String())
246+
continue
247+
}
248+
tc.Services[serviceNN] = &service
249+
250+
var endpoints discoveryv1.EndpointSliceList
251+
if err := r.List(ctx, &endpoints,
252+
client.InNamespace(service.Namespace),
253+
client.MatchingLabels{
254+
discoveryv1.LabelServiceName: service.Name,
255+
},
256+
); err != nil {
257+
return ReasonError{
258+
Reason: string(apiv2.ConditionReasonInvalidSpec),
259+
Message: fmt.Sprintf("failed to list endpoint slices: %v", err),
251260
}
252-
tc.EndpointSlices[serviceNN] = endpoints.Items
253261
}
262+
tc.EndpointSlices[serviceNN] = endpoints.Items
263+
}
254264

255-
for _, upstream := range http.Upstreams {
256-
if upstream.Name == "" {
265+
for _, upstream := range rule.Upstreams {
266+
if upstream.Name == "" {
267+
continue
268+
}
269+
var (
270+
ups apiv2.ApisixUpstream
271+
upsNN = types.NamespacedName{
272+
Namespace: in.GetNamespace(),
273+
Name: upstream.Name,
274+
}
275+
)
276+
if err := r.Get(ctx, upsNN, &ups); err != nil {
277+
r.Log.Error(err, "failed to get ApisixUpstream", "ApisixUpstream", upsNN)
278+
if client.IgnoreNotFound(err) == nil {
257279
continue
258280
}
259-
var ups = apiv2.ApisixUpstream{
260-
ObjectMeta: metav1.ObjectMeta{
261-
Name: upstream.Name,
262-
SelfLink: in.GetNamespace(),
263-
},
281+
return err
282+
}
283+
tc.Upstreams[upsNN] = &ups
284+
285+
for _, node := range ups.Spec.ExternalNodes {
286+
if node.Type == apiv2.ExternalTypeService {
287+
var (
288+
service corev1.Service
289+
serviceNN = types.NamespacedName{Namespace: ups.GetNamespace(), Name: node.Name}
290+
)
291+
if err := r.Get(ctx, serviceNN, &service); err != nil {
292+
r.Log.Error(err, "failed to get service in ApisixUpstream", "ApisixUpstream", upsNN, "Service", serviceNN)
293+
if client.IgnoreNotFound(err) == nil {
294+
continue
295+
}
296+
return err
297+
}
298+
tc.Services[utils.NamespacedName(&service)] = &service
264299
}
265-
upsNN := utils.NamespacedName(&ups)
266-
if err := r.Get(ctx, upsNN, &ups); err != nil {
267-
if client.IgnoreNotFound(err) == nil {
268-
r.Log.Error(err, "ApisixUpstream not found")
269-
continue
300+
}
301+
302+
if ups.Spec.TLSSecret != nil && ups.Spec.TLSSecret.Name != "" {
303+
var (
304+
secret corev1.Secret
305+
secretNN = types.NamespacedName{Namespace: cmp.Or(ups.Spec.TLSSecret.Namespace, in.GetNamespace()), Name: ups.Spec.TLSSecret.Name}
306+
)
307+
if err := r.Get(ctx, secretNN, &secret); err != nil {
308+
r.Log.Error(err, "failed to get secret in ApisixUpstream", "ApisixUpstream", upsNN, "Secret", secretNN)
309+
if client.IgnoreNotFound(err) != nil {
310+
return err
270311
}
271-
return err
272312
}
273-
tc.Upstreams[utils.NamespacedNameKind(&ups)] = &ups
313+
tc.Secrets[secretNN] = &secret
274314
}
275315
}
276316

‎internal/controller/indexer/indexer.go‎

Lines changed: 70 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
package indexer
1414

1515
import (
16+
"cmp"
1617
"context"
1718

1819
networkingv1 "k8s.io/api/networking/v1"
@@ -95,8 +96,8 @@ func setupConsumerIndexer(mgr ctrl.Manager) error {
9596

9697
func setupApisixRouteIndexer(mgr ctrl.Manager) error {
9798
var indexers = map[string]func(client.Object) []string{
98-
ServiceIndexRef: ApisixRouteServiceIndexFunc,
99-
SecretIndexRef: ApisixRouteSecretIndexFunc,
99+
ServiceIndexRef: ApisixRouteServiceIndexFunc(mgr.GetClient()),
100+
SecretIndexRef: ApisixRouteSecretIndexFunc(mgr.GetClient()),
100101
ApisixUpstreamRef: ApisixRouteApisixUpstreamIndexFunc,
101102
}
102103
for key, f := range indexers {
@@ -430,36 +431,82 @@ func HTTPRouteServiceIndexFunc(rawObj client.Object) []string {
430431
return keys
431432
}
432433

433-
func ApisixRouteServiceIndexFunc(obj client.Object) (keys []string) {
434-
ar := obj.(*apiv2.ApisixRoute)
435-
for _, http := range ar.Spec.HTTP {
436-
for _, backend := range http.Backends {
437-
keys = append(keys, GenIndexKey(ar.GetNamespace(), backend.ServiceName))
434+
func ApisixRouteServiceIndexFunc(cli client.Client) func(client.Object) []string {
435+
return func(obj client.Object) (keys []string) {
436+
ar := obj.(*apiv2.ApisixRoute)
437+
for _, http := range ar.Spec.HTTP {
438+
// service reference in .backends
439+
for _, backend := range http.Backends {
440+
keys = append(keys, GenIndexKey(ar.GetNamespace(), backend.ServiceName))
441+
}
442+
// service reference in .upstreams
443+
for _, upstream := range http.Upstreams {
444+
if upstream.Name == "" {
445+
continue
446+
}
447+
var (
448+
au apiv2.ApisixUpstream
449+
auNN = types.NamespacedName{
450+
Namespace: ar.GetNamespace(),
451+
Name: upstream.Name,
452+
}
453+
)
454+
if err := cli.Get(context.Background(), auNN, &au); err != nil {
455+
continue
456+
}
457+
for _, node := range au.Spec.ExternalNodes {
458+
if node.Type == apiv2.ExternalTypeService && node.Name != "" {
459+
keys = append(keys, GenIndexKey(au.GetNamespace(), node.Name))
460+
}
461+
}
462+
}
438463
}
464+
for _, stream := range ar.Spec.Stream {
465+
keys = append(keys, GenIndexKey(ar.GetNamespace(), stream.Backend.ServiceName))
466+
}
467+
return keys
439468
}
440-
for _, stream := range ar.Spec.Stream {
441-
keys = append(keys, GenIndexKey(ar.GetNamespace(), stream.Backend.ServiceName))
442-
}
443-
return
444469
}
445470

446-
func ApisixRouteSecretIndexFunc(obj client.Object) (keys []string) {
447-
ar := obj.(*apiv2.ApisixRoute)
448-
for _, http := range ar.Spec.HTTP {
449-
for _, plugin := range http.Plugins {
450-
if plugin.Enable && plugin.SecretRef != "" {
451-
keys = append(keys, GenIndexKey(ar.GetNamespace(), plugin.SecretRef))
471+
func ApisixRouteSecretIndexFunc(cli client.Client) func(client.Object) []string {
472+
return func(obj client.Object) (keys []string) {
473+
ar := obj.(*apiv2.ApisixRoute)
474+
for _, http := range ar.Spec.HTTP {
475+
// secret reference in .plugins
476+
for _, plugin := range http.Plugins {
477+
if plugin.Enable && plugin.SecretRef != "" {
478+
keys = append(keys, GenIndexKey(ar.GetNamespace(), plugin.SecretRef))
479+
}
480+
}
481+
// secret reference in .upstreams
482+
for _, upstream := range http.Upstreams {
483+
if upstream.Name == "" {
484+
continue
485+
}
486+
var (
487+
au apiv2.ApisixUpstream
488+
auNN = types.NamespacedName{
489+
Namespace: ar.GetNamespace(),
490+
Name: upstream.Name,
491+
}
492+
)
493+
if err := cli.Get(context.Background(), auNN, &au); err != nil {
494+
continue
495+
}
496+
if secret := au.Spec.TLSSecret; secret != nil && secret.Name != "" {
497+
keys = append(keys, GenIndexKey(cmp.Or(secret.Namespace, au.GetNamespace()), secret.Name))
498+
}
452499
}
453500
}
454-
}
455-
for _, stream := range ar.Spec.Stream {
456-
for _, plugin := range stream.Plugins {
457-
if plugin.Enable && plugin.SecretRef != "" {
458-
keys = append(keys, GenIndexKey(ar.GetNamespace(), plugin.SecretRef))
501+
for _, stream := range ar.Spec.Stream {
502+
for _, plugin := range stream.Plugins {
503+
if plugin.Enable && plugin.SecretRef != "" {
504+
keys = append(keys, GenIndexKey(ar.GetNamespace(), plugin.SecretRef))
505+
}
459506
}
460507
}
508+
return keys
461509
}
462-
return
463510
}
464511

465512
func ApisixRouteApisixUpstreamIndexFunc(obj client.Object) (keys []string) {

0 commit comments

Comments
 (0)