[PR] fix: reject an ApisixRoute servicePort that cannot resolve [apisix-ingress-controller]

["AlinsRan (via GitHub)" <gi...@apache.org>]

AlinsRan opened a new pull request, #2860:
URL: https://github.com/apache/apisix-ingress-controller/pull/2860

   ### Type of change:
   
   - [x] Bugfix
   
   ### What this PR does / why we need it:
   
   `spec.http[].backends[].servicePort: ""` is accepted by the API server, reported as `Accepted=True` with an empty message, and answers 503. Three separate problems stack up to produce that.
   
   **1. Nothing rejects the value.** `servicePort` is an int-or-string with no constraint beyond `required`. The string form is compared against Service port names, so `""` either matches nothing, or matches a single-port Service that omits its port name, which Kubernetes allows. The same misconfiguration therefore works on one Service and 503s on another, and the working case is worse: the user never learns the field is wrong.
   
   **2. The failure never reaches the status.** `validateHTTPBackend` logs and returns nil when the port does not match:
   
   ```go
   if !slices.ContainsFunc(service.Spec.Ports, func(port corev1.ServicePort) bool { ... }) {
       r.Log.Error(errors.New("service port not found"), "failed to match service port", ...)
       return nil                       // no error, so processApisixRoute succeeds
   }
   tctx.Services[serviceNN] = &service  // skipped
   ```
   
   `processApisixRoute` returns nil, so `updateStatus` writes `Accepted=True`.
   
   **3. The message blames the wrong object.** Because the Service was never added to `tctx.Services`, the translator reports `service not found` for a Service that exists with healthy endpoints. `getPortFromService` has the accurate error, but nothing reaches it. The translation error is then swallowed by `buildUpstream`, which `continue`s, leaving the default upstream with no nodes, which is the 503.
   
   This PR rejects the value in three places, each covering a case the others cannot:
   
   - a CEL rule on the CRD, which is the only point that can name the offending field at apply time
   - `validateHTTPBackend`, before the reference is resolved, so objects that predate the CRD rule are reported rather than published, and the accidental match against an unnamed Service port cannot happen
   - `getPortFromService`, so the translator cannot make that match through another path
   
   A Service that resolves but has no such port now reports `InvalidSpec` and says the port was not found rather than the Service. A missing Service is left alone: that path stays lenient so applying a route alongside its Service still works.
   
   The CEL test harness needed a fix to exercise any int-or-string rule: it decoded JSON numbers with `encoding/json`, producing `float64`, and CEL rejects that where the schema declares `x-kubernetes-int-or-string`. It now decodes with the apimachinery helper that yields `int64`, as the API server does.
   
   ### Pre-submission checklist:
   
   - [x] Did you explain what problem does this PR solve? Or what new features have been added?
   - [x] Have you added corresponding test cases?
   - [ ] Have you modified the corresponding document?
   - [ ] Is this PR backward compatible?
   
   Behavior changes worth calling out:
   
   - An ApisixRoute whose Service exists but has no matching port is now rejected instead of accepted-and-503. Since `processApisixRoute` is shared with the ADC admission webhook, that also means such a route is refused at apply time. A route whose Service does not exist yet is unaffected.
   - An empty `servicePort` that happened to work against an unnamed single-port Service now fails. That match was accidental and would have broken as soon as a second port was added to the Service.
   
   Tests: CEL coverage in `api/v2/apisixroute_types_test.go` for the rejected and accepted forms, and reconciler coverage in `internal/controller/apisixroute_serviceport_test.go` for the empty port against both a named and an unnamed Service port, the unknown-port message, and the healthy path. The two reconciler tests fail on `master`.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] fix: reject an ApisixRoute servicePort that cannot resolve [apisix-ingress-controller]

["AlinsRan (via GitHub)" <gi...@apache.org>]

AlinsRan merged PR #2860:
URL: https://github.com/apache/apisix-ingress-controller/pull/2860


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org