fix: cannot use ingress when disabling gateway API#388
Conversation
📝 WalkthroughWalkthroughThis PR adds HTTPRoutePolicy indexer configuration and modifies error handling in HTTPRoute and Ingress controllers. Errors during NotFound scenarios or status updates are now logged and processing continues, rather than causing early termination. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@internal/controller/httproutepolicy.go`:
- Around line 47-50: The current handler swallows all errors from r.List (the
call using client.MatchingFields{indexer.PolicyTargetRefs: key}), returning nil
and preventing requeue; change it to import k8serrors
("k8s.io/apimachinery/pkg/api/errors") and, after r.List, return the error
unless k8serrors.IsNotFound(err) is true — i.e., only ignore not-found errors
(CRD missing) and otherwise log and return the error so the reconcile will
requeue and transient/RBAC issues are retried.
| if err := r.List(context.Background(), &list, client.MatchingFields{indexer.PolicyTargetRefs: key}); err != nil { | ||
| return err | ||
| r.Log.Error(err, "failed to list HTTPRoutePolicies for HTTPRoute", "httproute", utils.NamespacedName(httpRoute)) | ||
| return nil | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find and examine the httproutepolicy.go file
find . -name "httproutepolicy.go" -type fRepository: api7/api7-ingress-controller
Length of output: 112
🏁 Script executed:
# Read the file to understand the error handling patterns
head -200 internal/controller/httproutepolicy.go | cat -nRepository: api7/api7-ingress-controller
Length of output: 9114
🏁 Script executed:
# Check the imports in the file
grep -n "^import\|^\"" internal/controller/httproutepolicy.go | head -30Repository: api7/api7-ingress-controller
Length of output: 83
🏁 Script executed:
# Get the complete imports section
sed -n '20,39p' internal/controller/httproutepolicy.goRepository: api7/api7-ingress-controller
Length of output: 796
🏁 Script executed:
# Check if apierrors is used elsewhere in the controller package
grep -r "apierrors" internal/controller/ --include="*.go" | head -20Repository: api7/api7-ingress-controller
Length of output: 54
🏁 Script executed:
# Check for meta.IsNoMatchError usage in the codebase
grep -r "IsNoMatchError" . --include="*.go"Repository: api7/api7-ingress-controller
Length of output: 54
🏁 Script executed:
# Verify all 4 error handling patterns exist as claimed
grep -n "if err := r.List" internal/controller/httproutepolicy.goRepository: api7/api7-ingress-controller
Length of output: 512
🏁 Script executed:
# Check what error handling patterns are used elsewhere in the controller package
grep -r "apierrors\|IsNotFound\|IsNoMatchError" internal/controller/ --include="*.go" -B2 -A2 | head -50Repository: api7/api7-ingress-controller
Length of output: 2475
🏁 Script executed:
# Check the Go modules to understand available Kubernetes API dependencies
cat go.mod | grep -E "k8s.io|sigs.k8s.io"Repository: api7/api7-ingress-controller
Length of output: 877
🏁 Script executed:
# Look for error handling patterns in other reconcilers in this package
grep -n "r.List" internal/controller/*.go | head -20Repository: api7/api7-ingress-controller
Length of output: 2610
🏁 Script executed:
# Check if there are other error handling patterns used for client.List errors
grep -A3 "if err := r.List" internal/controller/*.go | grep -E "apierrors|IsNotFound|IsNoMatch|meta\." | head -20Repository: api7/api7-ingress-controller
Length of output: 54
🏁 Script executed:
# Check what k8serrors import is used in the codebase
grep -r "k8serrors\|apierrors" internal/controller/ --include="*.go" | grep "^[^:]*:.*import" -A20 | head -30Repository: api7/api7-ingress-controller
Length of output: 54
🏁 Script executed:
# Look at the actual import in the files that use k8serrors
grep -r "k8serrors" internal/controller/ --include="*.go" -l | head -1 | xargs grep -B30 "k8serrors" | grep import -A15Repository: api7/api7-ingress-controller
Length of output: 706
🏁 Script executed:
# Check Go documentation for meta.IsNoMatchError
grep -r "meta\\.Is" . --include="*.go"Repository: api7/api7-ingress-controller
Length of output: 54
🏁 Script executed:
# Look for any List error handling with specific error checks in the codebase
grep -A5 "if err := r.List" internal/controller/*.go | grep -E "IsNotFound|apierrors|meta\.|errors\\.Is"Repository: api7/api7-ingress-controller
Length of output: 54
🏁 Script executed:
# Check what's imported from apimachinery in files that do error checking
grep -l "k8serrors" internal/controller/*.go | head -1 | xargs head -50 | grep -A20 "^import"Repository: api7/api7-ingress-controller
Length of output: 785
🏁 Script executed:
# Check if IsNoMatchError is available in the meta package
python3 << 'EOF'
import subprocess
result = subprocess.run(['go', 'doc', 'k8s.io/apimachinery/pkg/api/meta', 'IsNoMatchError'],
capture_output=True, text=True, cwd='/tmp')
print("STDOUT:", result.stdout)
print("STDERR:", result.stderr)
print("Return code:", result.returncode)
EOFRepository: api7/api7-ingress-controller
Length of output: 159
🏁 Script executed:
# Check the actual context around the List calls to understand what errors could occur
sed -n '40,55p' internal/controller/httproutepolicy.go
sed -n '107,120p' internal/controller/httproutepolicy.goRepository: api7/api7-ingress-controller
Length of output: 1373
🏁 Script executed:
# Look for any other CRD-missing or custom resource error handling in the codebase
grep -r "CRD\|no match\|NotFound" internal/controller/ --include="*.go" -i | grep -i "list\|error" | head -10Repository: api7/api7-ingress-controller
Length of output: 809
🏁 Script executed:
# Check what happens when IndexedFields don't exist - look at indexer setup
grep -r "PolicyTargetRefs" . --include="*.go" -B3 -A3Repository: api7/api7-ingress-controller
Length of output: 4898
🏁 Script executed:
# Check the Kubernetes API to understand what errors can be returned from List operations
grep -r "IsNotFound\|IsAlreadyExists\|IsForbidden\|IsUnauthorized" internal/controller/ --include="*.go" -h | sort | uniqRepository: api7/api7-ingress-controller
Length of output: 138
🏁 Script executed:
# Look for any documentation on FieldIndexer errors in the codebase
find . -name "*.go" -type f -exec grep -l "FieldIndexer\|IndexField" {} \; | head -3 | xargs grep -A10 "IndexField"Repository: api7/api7-ingress-controller
Length of output: 24077
🏁 Script executed:
# Check if there's a meta package import anywhere that might have IsNoMatchError
grep -r "apimachinery/pkg/api/meta" . --include="*.go"Repository: api7/api7-ingress-controller
Length of output: 357
🏁 Script executed:
# Check what's being used from the meta package in files that import it
grep -A5 -B5 "apimachinery/pkg/api/meta" internal/controller/consumer_controller.go | head -30Repository: api7/api7-ingress-controller
Length of output: 382
🏁 Script executed:
# Check the actual usage of meta package in those files
grep "meta\." internal/controller/consumer_controller.go internal/controller/policies.go internal/controller/status.go internal/controller/utils.goRepository: api7/api7-ingress-controller
Length of output: 443
🏁 Script executed:
# Check if there's any comment or documentation about handling "no match" errors in field indexing
grep -r "no match\|NoMatch" . --include="*.go" -iRepository: api7/api7-ingress-controller
Length of output: 2216
🏁 Script executed:
# Verify what error types can be returned from List with field indexers
# Check controller-runtime documentation by looking at how List is used in the codebase
grep -r "FieldIndexer\|field.Indexer" . --include="*.go" | head -10Repository: api7/api7-ingress-controller
Length of output: 1003
🏁 Script executed:
# Look for any error handling patterns for List calls in similar controllers
grep -B2 -A8 "if err := r.List" internal/controller/consumer_controller.go | head -30Repository: api7/api7-ingress-controller
Length of output: 1260
🏁 Script executed:
# Check if there's documentation about what errors can occur from field-indexed List operations
grep -r "indexer\|IndexField" internal/controller/*.go | grep -i "error" | head -10Repository: api7/api7-ingress-controller
Length of output: 54
Don't swallow List errors unconditionally.
Returning nil here prevents requeue on transient/RBAC failures and can leave policies stale. Consider ignoring only "not found" errors when the HTTPRoutePolicy CRD is missing, and return other errors to trigger requeue.
✅ Safer pattern
- if err := r.List(context.Background(), &list, client.MatchingFields{indexer.PolicyTargetRefs: key}); err != nil {
- r.Log.Error(err, "failed to list HTTPRoutePolicies for HTTPRoute", "httproute", utils.NamespacedName(httpRoute))
- return nil
- }
+ if err := r.List(context.Background(), &list, client.MatchingFields{indexer.PolicyTargetRefs: key}); err != nil {
+ if k8serrors.IsNotFound(err) {
+ r.Log.Info("HTTPRoutePolicy API not available; skipping", "httproute", utils.NamespacedName(httpRoute))
+ return nil
+ }
+ return err
+ }(Requires adding: k8serrors "k8s.io/apimachinery/pkg/api/errors")
🤖 Prompt for AI Agents
In `@internal/controller/httproutepolicy.go` around lines 47 - 50, The current
handler swallows all errors from r.List (the call using
client.MatchingFields{indexer.PolicyTargetRefs: key}), returning nil and
preventing requeue; change it to import k8serrors
("k8s.io/apimachinery/pkg/api/errors") and, after r.List, return the error
unless k8serrors.IsNotFound(err) is true — i.e., only ignore not-found errors
(CRD missing) and otherwise log and return the error so the reconcile will
requeue and transient/RBAC issues are retried.
| ) | ||
| if err := r.List(context.Background(), &list, client.MatchingFields{indexer.PolicyTargetRefs: key}); err != nil { | ||
| return err | ||
| r.Log.Error(err, "failed to list HTTPRoutePolicies for HTTPRoute", "httproute", utils.NamespacedName(httpRoute)) |
There was a problem hiding this comment.
We should consider adding some code comments, otherwise it doesn't seem to explain why we return nil instead of an error.
There was a problem hiding this comment.
This method of handling is not very good, I have updated a new version.
There was a problem hiding this comment.
The processing of ingress should be consistent with httproute, rather than returning directly.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/controller/ingress_controller.go (1)
213-215:⚠️ Potential issue | 🟠 MajorDon’t apply an update when HTTPRoutePolicy processing fails unexpectedly.
Line 213-215 now logs and continues, which can push an incomplete TranslateContext to the provider (dropping policies on transient errors). Please return/requeue for unexpected failures and only suppress errors that indicate the policy resource/index is unavailable.
Suggested guard
- if err := r.processHTTPRoutePolicies(tctx, ingress); err != nil { - r.Log.Error(err, "failed to process HTTPRoutePolicy", "ingress", ingress.Name) - } + if err := r.processHTTPRoutePolicies(tctx, ingress); err != nil { + if isHTTPRoutePolicyUnavailable(err) { + r.Log.V(1).Info("skip HTTPRoutePolicy processing; policy resource unavailable", "ingress", ingress.Name, "err", err) + } else { + r.Log.Error(err, "failed to process HTTPRoutePolicy", "ingress", ingress.Name) + return ctrl.Result{}, err + } + }
🧹 Nitpick comments (2)
internal/controller/ingress_controller.go (1)
145-147: Gate status-update suppression to known “policy unavailable” errors.Line 145-147 now logs and continues on any error. If the failure is transient (API outage/conflict), the HTTPRoutePolicy status cleanup is never retried. Consider only suppressing errors that indicate the HTTPRoutePolicy CRD/index is unavailable (Gateway API disabled), and requeue for the rest.
internal/controller/httproute_controller.go (1)
157-159: Avoid swallowing unexpected status-update errors.Line 157-159 logs and continues for any error. If this is a transient API/server failure, the HTTPRoutePolicy status won’t be retried. Consider suppressing only “policy unavailable” errors and requeueing for everything else.
conformance test report - apisix-standalone modeapiVersion: gateway.networking.k8s.io/v1
date: "2026-02-04T09:27:07Z"
gatewayAPIChannel: experimental
gatewayAPIVersion: v1.3.0
implementation:
contact: null
organization: APISIX
project: apisix-ingress-controller
url: https://github.com/apache/apisix-ingress-controller.git
version: v2.0.0
kind: ConformanceReport
mode: default
profiles:
- core:
result: success
statistics:
Failed: 0
Passed: 12
Skipped: 0
name: GATEWAY-GRPC
summary: Core tests succeeded.
- core:
result: partial
skippedTests:
- HTTPRouteHTTPSListener
statistics:
Failed: 0
Passed: 32
Skipped: 1
extended:
result: partial
skippedTests:
- HTTPRouteRedirectPortAndScheme
statistics:
Failed: 0
Passed: 11
Skipped: 1
supportedFeatures:
- GatewayAddressEmpty
- GatewayPort8080
- HTTPRouteBackendProtocolWebSocket
- HTTPRouteDestinationPortMatching
- HTTPRouteHostRewrite
- HTTPRouteMethodMatching
- HTTPRoutePathRewrite
- HTTPRoutePortRedirect
- HTTPRouteQueryParamMatching
- HTTPRouteRequestMirror
- HTTPRouteResponseHeaderModification
- HTTPRouteSchemeRedirect
unsupportedFeatures:
- GatewayHTTPListenerIsolation
- GatewayInfrastructurePropagation
- GatewayStaticAddresses
- HTTPRouteBackendProtocolH2C
- HTTPRouteBackendRequestHeaderModification
- HTTPRouteBackendTimeout
- HTTPRouteParentRefPort
- HTTPRoutePathRedirect
- HTTPRouteRequestMultipleMirrors
- HTTPRouteRequestPercentageMirror
- HTTPRouteRequestTimeout
name: GATEWAY-HTTP
summary: Core tests partially succeeded with 1 test skips. Extended tests partially
succeeded with 1 test skips.
- core:
result: partial
skippedTests:
- TLSRouteSimpleSameNamespace
statistics:
Failed: 0
Passed: 10
Skipped: 1
name: GATEWAY-TLS
summary: Core tests partially succeeded with 1 test skips. |
conformance test report - apisix modeapiVersion: gateway.networking.k8s.io/v1
date: "2026-02-04T09:27:34Z"
gatewayAPIChannel: experimental
gatewayAPIVersion: v1.3.0
implementation:
contact: null
organization: APISIX
project: apisix-ingress-controller
url: https://github.com/apache/apisix-ingress-controller.git
version: v2.0.0
kind: ConformanceReport
mode: default
profiles:
- core:
result: success
statistics:
Failed: 0
Passed: 12
Skipped: 0
name: GATEWAY-GRPC
summary: Core tests succeeded.
- core:
failedTests:
- HTTPRouteInvalidBackendRefUnknownKind
result: failure
skippedTests:
- HTTPRouteHTTPSListener
statistics:
Failed: 1
Passed: 31
Skipped: 1
extended:
result: partial
skippedTests:
- HTTPRouteRedirectPortAndScheme
statistics:
Failed: 0
Passed: 11
Skipped: 1
supportedFeatures:
- GatewayAddressEmpty
- GatewayPort8080
- HTTPRouteBackendProtocolWebSocket
- HTTPRouteDestinationPortMatching
- HTTPRouteHostRewrite
- HTTPRouteMethodMatching
- HTTPRoutePathRewrite
- HTTPRoutePortRedirect
- HTTPRouteQueryParamMatching
- HTTPRouteRequestMirror
- HTTPRouteResponseHeaderModification
- HTTPRouteSchemeRedirect
unsupportedFeatures:
- GatewayHTTPListenerIsolation
- GatewayInfrastructurePropagation
- GatewayStaticAddresses
- HTTPRouteBackendProtocolH2C
- HTTPRouteBackendRequestHeaderModification
- HTTPRouteBackendTimeout
- HTTPRouteParentRefPort
- HTTPRoutePathRedirect
- HTTPRouteRequestMultipleMirrors
- HTTPRouteRequestPercentageMirror
- HTTPRouteRequestTimeout
name: GATEWAY-HTTP
summary: Core tests failed with 1 test failures. Extended tests partially succeeded
with 1 test skips.
- core:
result: partial
skippedTests:
- TLSRouteSimpleSameNamespace
statistics:
Failed: 0
Passed: 10
Skipped: 1
name: GATEWAY-TLS
summary: Core tests partially succeeded with 1 test skips. |
conformance test reportapiVersion: gateway.networking.k8s.io/v1
date: "2026-02-04T09:49:22Z"
gatewayAPIChannel: experimental
gatewayAPIVersion: v1.3.0
implementation:
contact: null
organization: APISIX
project: apisix-ingress-controller
url: https://github.com/apache/apisix-ingress-controller.git
version: v2.0.0
kind: ConformanceReport
mode: default
profiles:
- core:
failedTests:
- GatewayModifyListeners
result: failure
skippedTests:
- HTTPRouteHTTPSListener
statistics:
Failed: 1
Passed: 31
Skipped: 1
extended:
failedTests:
- HTTPRouteBackendProtocolWebSocket
result: failure
skippedTests:
- HTTPRouteRedirectPortAndScheme
statistics:
Failed: 1
Passed: 10
Skipped: 1
supportedFeatures:
- GatewayAddressEmpty
- GatewayPort8080
- HTTPRouteBackendProtocolWebSocket
- HTTPRouteDestinationPortMatching
- HTTPRouteHostRewrite
- HTTPRouteMethodMatching
- HTTPRoutePathRewrite
- HTTPRoutePortRedirect
- HTTPRouteQueryParamMatching
- HTTPRouteRequestMirror
- HTTPRouteResponseHeaderModification
- HTTPRouteSchemeRedirect
unsupportedFeatures:
- GatewayHTTPListenerIsolation
- GatewayInfrastructurePropagation
- GatewayStaticAddresses
- HTTPRouteBackendProtocolH2C
- HTTPRouteBackendRequestHeaderModification
- HTTPRouteBackendTimeout
- HTTPRouteParentRefPort
- HTTPRoutePathRedirect
- HTTPRouteRequestMultipleMirrors
- HTTPRouteRequestPercentageMirror
- HTTPRouteRequestTimeout
name: GATEWAY-HTTP
summary: Core tests failed with 1 test failures. Extended tests failed with 1 test
failures.
- core:
failedTests:
- GatewayModifyListeners
- TLSRouteSimpleSameNamespace
result: failure
statistics:
Failed: 2
Passed: 9
Skipped: 0
name: GATEWAY-TLS
summary: Core tests failed with 2 test failures.
- core:
failedTests:
- GRPCExactMethodMatching
- GRPCRouteHeaderMatching
- GRPCRouteListenerHostnameMatching
- GatewayModifyListeners
result: failure
statistics:
Failed: 4
Passed: 8
Skipped: 0
name: GATEWAY-GRPC
summary: Core tests failed with 4 test failures. |
2 participants
Type of change:
What this PR does / why we need it:
When disabling the gateway API, the ingress resource cannot be used because the httproutepolicy does not have an index set.
Pre-submission checklist:
Summary by CodeRabbit
Bug Fixes
Refactor